From: j.anaszewski@samsung•com (Jacek Anaszewski)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH v2 19/21] leds: leds-pasic3: Add support for PASIC3 LED controller
Date: Tue, 18 Aug 2015 12:27:01 +0200 [thread overview]
Message-ID: <55D30875.3090103@samsung.com> (raw)
In-Reply-To: <55D25ADC.10703@tul.cz>
Hi Petr,
On 08/18/2015 12:06 AM, Petr Cvek wrote:
> Add support for LEDs inside the PASIC3 chip.
>
> Signed-off-by: Petr Cvek <petr.cvek@tul•cz>
> ---
> drivers/leds/Kconfig | 9 ++
> drivers/leds/Makefile | 1 +
> drivers/leds/leds-pasic3.c | 232 +++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 242 insertions(+)
> create mode 100644 drivers/leds/leds-pasic3.c
>
> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> index 9ad35f7..516ba65 100644
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -586,6 +586,15 @@ config LEDS_PM8941_WLED
> This option enables support for the 'White' LED block
> on Qualcomm PM8941 PMICs.
>
> +config LEDS_PASIC3
> + tristate "LED support for the HTC Magician/Alpine PASIC3"
> + depends on LEDS_CLASS
> + depends on HTC_PASIC3
> + select REGMAP
> + help
> + This option enables support for the PASIC3 chip (different chip
> + than Compaq ASIC3).
> +
> comment "LED Triggers"
> source "drivers/leds/trigger/Kconfig"
>
> diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
> index 8d6a24a..b1c659c 100644
> --- a/drivers/leds/Makefile
> +++ b/drivers/leds/Makefile
> @@ -65,6 +65,7 @@ obj-$(CONFIG_LEDS_VERSATILE) += leds-versatile.o
> obj-$(CONFIG_LEDS_MENF21BMC) += leds-menf21bmc.o
> obj-$(CONFIG_LEDS_PM8941_WLED) += leds-pm8941-wled.o
> obj-$(CONFIG_LEDS_KTD2692) += leds-ktd2692.o
> +obj-$(CONFIG_LEDS_PASIC3) += leds-pasic3.o
>
> # LED SPI Drivers
> obj-$(CONFIG_LEDS_DAC124S085) += leds-dac124s085.o
> diff --git a/drivers/leds/leds-pasic3.c b/drivers/leds/leds-pasic3.c
> new file mode 100644
> index 0000000..0f95230
> --- /dev/null
> +++ b/drivers/leds/leds-pasic3.c
> @@ -0,0 +1,232 @@
> +/*
> + * AIC3 (or PASIC3) LED driver for HTC Magician/Alpine/.. (not Compaq ASIC3)
s/AIC3/ASIC3/
I am a bit confused. What's the relation between ASIC3 and PASIC3?
> + *
> + * Copyright (c) 2015 Petr Cvek <petr.cvek@tul•cz>
> + *
> + * CLK to MS calculation based on leds-asic3.c
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the
> + * Free Software Foundation; either version 2 of the License, or (at your
> + * option) any later version.
> + *
> + */
> +
> +#include <linux/gpio.h>
> +#include <linux/kernel.h>
> +#include <linux/leds.h>
> +#include <linux/mfd/htc-pasic3.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +
> +/*
> + * 1 tick is around 62-63 ms, blinking settings (on+off):
> + * Min: 1+1 ticks ~125ms
> + * Max: 127+127 ticks ~15?875ms
> + * HW sometimes takes time to change (counter overflow after write?)
> + */
> +
> +#define MS_TO_CLK(ms) DIV_ROUND_CLOSEST(((ms)*1024), 64000)
> +#define CLK_TO_MS(clk) (((clk)*64000)/1024)
> +#define MAX_CLK 254 /* 127 + 127 (2x 7 bit reg) */
> +#define MAX_MS CLK_TO_MS(MAX_CLK)
> +
> +spinlock_t lock;
Please move it to the struct pasic3_leds_data.
> +
> +static void brightness_set(struct led_classdev *cdev,
> + enum led_brightness value)
> +{
> + struct platform_device *pdev = to_platform_device(cdev->dev->parent);
> + struct pasic3_led *led = dev_get_platdata(cdev->dev);
> + struct pasic3_leds_pdata *pdata = dev_get_platdata(&pdev->dev);
> + unsigned long flags;
> + u8 val;
> +
> + spin_lock_irqsave(&lock, flags);
> +
> + if (value == LED_OFF) {
> + val = pasic3_read_register(pdata->mfd_dev, PASIC3_CH_CONTROL);
> + pasic3_write_register(pdata->mfd_dev, PASIC3_CH_CONTROL,
> + val & ~(led->bit_blink_en | led->bit_force_on));
> +
> + val = pasic3_read_register(pdata->mfd_dev, PASIC3_MASK_A);
> + pasic3_write_register(pdata->mfd_dev, PASIC3_MASK_A,
> + val & ~led->bit_mask);
> +
> + if (pdata->power_gpio) {
> + pdata->power_gpio_users--;
Now it is possible that power_gpio_users will have negative value.
You should check the value before decrementing it.
> +
> + if (pdata->power_gpio_users == 0)
> + gpio_set_value(pdata->power_gpio, 0);
> + }
> + } else {
> + if (pdata->power_gpio) {
> + pdata->power_gpio_users++;
> +
> + if (pdata->power_gpio_users != 0)
> + gpio_set_value(pdata->power_gpio, 1);
> + }
Now you are setting power_gpio_users to 1 each time brightness is set,
whereas it should be set to 1 only if it was 0.
This could be changed to:
if (pdata->power_gpio_users == 0)
gpio_set_value(pdata->power_gpio, 1);
pdata->power_gpio_users++;
> +
> + val = pasic3_read_register(pdata->mfd_dev, PASIC3_CH_CONTROL);
> + pasic3_write_register(pdata->mfd_dev, PASIC3_CH_CONTROL,
> + val | led->bit_force_on);
> + }
> +
> + spin_unlock_irqrestore(&lock, flags);
> +}
> +
> +static int blink_set(struct led_classdev *cdev,
> + unsigned long *delay_on, unsigned long *delay_off)
> +{
> + struct platform_device *pdev = to_platform_device(cdev->dev->parent);
> + struct pasic3_led *led = dev_get_platdata(cdev->dev);
> + struct pasic3_leds_pdata *pdata = dev_get_platdata(&pdev->dev);
> + unsigned long flags;
> + u32 on, off;
> + u8 val;
> + if (*delay_on > MAX_MS || *delay_off > MAX_MS)
> + return -EINVAL;
> +
> + if (*delay_on == 0 && *delay_off == 0) {
> + /* Choose sensible default delays */
> + on = MS_TO_CLK(500);
> + off = MS_TO_CLK(500);
> + } else {
> + on = MS_TO_CLK(*delay_on);
> + off = MS_TO_CLK(*delay_off);
> + if ((on + off) > MAX_CLK)
> + return -EINVAL;
> + /* Minimal on/off value must be 1 */
> + on = on ? on : 1;
> + off = off ? off : 1;
> + }
> +
> + spin_lock_irqsave(&lock, flags);
Shouldn't you check if also power_gpio state doesn't need to be altered
here?
> +
> + pasic3_write_register(pdata->mfd_dev, led->reg_delay_on, on);
> + pasic3_write_register(pdata->mfd_dev, led->reg_delay_off, off);
> +
> + val = pasic3_read_register(pdata->mfd_dev, PASIC3_CH_CONTROL);
> + pasic3_write_register(pdata->mfd_dev, PASIC3_CH_CONTROL,
> + val | led->bit_blink_en);
> +
> + spin_unlock_irqrestore(&lock, flags);
> +
> + *delay_on = CLK_TO_MS(on);
> + *delay_off = CLK_TO_MS(off);
> +
> + return 0;
> +}
> +
> +static int pasic3_led_probe(struct platform_device *pdev)
> +{
> + struct pasic3_leds_pdata *pdata = dev_get_platdata(&pdev->dev);
> + struct led_classdev *cdev;
> + int ret;
> + int i;
> +
> + if (!pdata) {
> + dev_err(&pdev->dev, "no platform data\n");
> + return -EINVAL;
> + }
> +
> + if (!pdata->leds) {
> + dev_err(&pdev->dev, "no leds data\n");
> + return -EINVAL;
> + }
> +
> + if (pdata->power_gpio) {
> + pdata->power_gpio_users = 0;
> +
> + ret = gpio_request(pdata->power_gpio,
> + "Red-Blue LED Power");
Please use devm_gpio_request. Then you will not need pasic3_led_remove.
> + if (ret < 0) {
> + dev_warn(&pdev->dev, "failed to request power GPIO\n");
> + goto err;
> + }
> + }
> +
> + spin_lock_init(&lock);
> +
> + cdev = devm_kzalloc(&pdev->dev,
> + sizeof(struct led_classdev) * pdata->num_leds, GFP_KERNEL);
> + if (!cdev) {
> + ret = -ENOMEM;
> + goto err;
> + }
> +
> + for (i = 0; i < pdata->num_leds; i++) {
> + cdev[i].name = pdata->leds[i].name;
> + cdev[i].default_trigger = pdata->leds[i].default_trigger;
> + cdev[i].flags = LED_CORE_SUSPENDRESUME;
> + cdev[i].brightness_set = brightness_set;
> + cdev[i].blink_set = blink_set;
> +
> + ret = devm_led_classdev_register(&pdev->dev, &cdev[i]);
> + if (ret < 0)
> + goto err;
> +
> + cdev[i].dev->platform_data = &pdata->leds[i];
> + }
> +
> + return 0;
> +
> +err:
> + if (pdata->power_gpio)
> + gpio_free(pdata->power_gpio);
> +
> + return ret;
> +}
> +
> +static int pasic3_led_remove(struct platform_device *pdev)
> +{
> + struct pasic3_leds_pdata *pdata = dev_get_platdata(&pdev->dev);
> +
> + if (pdata->power_gpio)
> + gpio_free(pdata->power_gpio);
> +
> + return 0;
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int pasic3_led_suspend(struct device *dev)
> +{
> + struct pasic3_leds_pdata *pdata = dev_get_platdata(dev);
> +
> + if (pdata->power_gpio)
> + gpio_set_value(pdata->power_gpio, 0);
> +
> + return 0;
> +}
> +
> +static int pasic3_led_resume(struct device *dev)
> +{
> + struct pasic3_leds_pdata *pdata = dev_get_platdata(dev);
> +
> + if (pdata->power_gpio)
> + gpio_set_value(pdata->power_gpio, 1);
> +
> + return 0;
> +}
> +#endif
> +
> +static SIMPLE_DEV_PM_OPS(pasic3_led_pm_ops, pasic3_led_suspend,
> + pasic3_led_resume);
> +
> +static struct platform_driver pasic3_led_driver = {
> + .probe = pasic3_led_probe,
> + .remove = pasic3_led_remove,
> + .driver = {
> + .name = "leds-pasic3",
> + .pm = &pasic3_led_pm_ops,
> + },
> +};
> +
> +module_platform_driver(pasic3_led_driver);
> +
> +MODULE_AUTHOR("Petr Cvek <petr.cvek@tul•cz>");
> +MODULE_DESCRIPTION("HTC Magician PASIC3 LED driver");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("platform:leds-pasic3");
>
--
Best Regards,
Jacek Anaszewski
next prev parent reply other threads:[~2015-08-18 10:27 UTC|newest]
Thread overview: 68+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <cover.1439843482.git.petr.cvek@tul.cz>
2015-08-17 21:56 ` [PATCH v2 01/21] ARM: pxa: magician: Fix Kconfig for magician to always include htc-egpio Petr Cvek
2015-08-18 18:31 ` Robert Jarzmik
2015-08-18 20:02 ` Petr Cvek
2015-08-19 7:29 ` Philipp Zabel
2015-08-24 3:39 ` Petr Cvek
2015-08-17 21:56 ` [PATCH v2 02/21] ARM: pxa: magician: Fix indentation and whitespaces Petr Cvek
2015-08-18 18:32 ` Robert Jarzmik
2015-08-19 7:42 ` Philipp Zabel
2015-08-17 21:57 ` [PATCH v2 03/21] ARM: pxa: magician: Fix comments, debug functions and print strings Petr Cvek
2015-08-18 18:39 ` Robert Jarzmik
2015-08-18 23:09 ` Petr Cvek
2015-08-19 7:57 ` Philipp Zabel
2015-08-17 21:58 ` [PATCH v2 04/21] ARM: pxa: magician: Add, fix and init (new) GPIOs Petr Cvek
2015-08-18 19:01 ` Robert Jarzmik
2015-08-18 22:46 ` Petr Cvek
2015-08-17 21:58 ` [PATCH v2 05/21] ARM: pxa: magician: Add support for ADS7846 touchscreen Petr Cvek
2015-08-19 18:41 ` Robert Jarzmik
2015-08-17 21:59 ` [PATCH v2 06/21] ARM: pxa: magician: Add normal and power I2C definition Petr Cvek
2015-08-19 8:02 ` Philipp Zabel
2015-08-19 18:41 ` Robert Jarzmik
2015-08-17 21:59 ` [PATCH v2 07/21] ARM: pxa: magician: Fix IrDA pdata and redundant GPIO request Petr Cvek
2015-08-19 18:41 ` Robert Jarzmik
2015-08-17 22:00 ` [PATCH v2 08/21] ARM: pxa: magician: Add StrataFlash Vpp GPIO and alternative driver Petr Cvek
2015-08-17 22:01 ` [PATCH v2 09/21] ARM: pxa: magician: Add OV9640 camera support Petr Cvek
2015-08-20 19:48 ` Robert Jarzmik
2015-08-20 20:26 ` Arnd Bergmann
2015-08-20 22:39 ` Petr Cvek
2015-08-21 13:45 ` Arnd Bergmann
2015-08-21 17:36 ` Robert Jarzmik
2015-08-21 22:09 ` Petr Cvek
2015-08-22 12:41 ` Robert Jarzmik
2015-08-24 6:53 ` Lee Jones
2015-08-17 22:01 ` [PATCH v2 10/21] ARM: pxa: magician: Add UDA1380 sound support Petr Cvek
2015-08-20 19:51 ` Robert Jarzmik
2015-08-20 23:01 ` Petr Cvek
2015-08-28 8:48 ` Robert Jarzmik
2015-08-17 22:01 ` [PATCH v2 11/21] ARM: pxa: magician: Add MAX1586 Vcore regulator support Petr Cvek
2015-08-20 19:32 ` Robert Jarzmik
2015-08-20 22:33 ` Petr Cvek
2015-08-28 8:49 ` Robert Jarzmik
2015-08-17 22:02 ` [PATCH v2 12/21] ARM: pxa: magician: Add PXA27x UDC support Petr Cvek
2015-08-20 17:23 ` Robert Jarzmik
2015-08-20 22:21 ` Petr Cvek
2015-08-28 9:58 ` Robert Jarzmik
2015-08-17 22:03 ` [PATCH v2 13/21] ARM: pxa: magician: Fix charging source and add NiCd backup charging Petr Cvek
2015-08-17 22:18 ` Petr Cvek
2015-08-17 22:03 ` [PATCH v2 14/21] ARM: pxa: magician: Fix PXA USB OHCI port enable flags Petr Cvek
2015-08-17 22:03 ` [PATCH v2 15/21] ARM: pxa: magician: Fix PWM backlight regulator Petr Cvek
2015-08-20 19:58 ` Robert Jarzmik
2015-08-17 22:04 ` [PATCH v2 16/21] ARM: pxa: magician: Add support for alternative LCD backlight Petr Cvek
2015-08-20 20:01 ` Robert Jarzmik
2015-08-23 20:55 ` Petr Cvek
2015-08-24 8:25 ` Robert Jarzmik
2015-08-17 22:04 ` [PATCH v2 17/21] ARM: pxa: magician: Remove (temporarily) pasic3 LED support Petr Cvek
2015-08-22 16:25 ` Robert Jarzmik
2015-08-17 22:05 ` [PATCH v2 18/21] mfd: htc-pasic3: Prepare driver for leds-pasic3 Petr Cvek
2015-08-18 6:52 ` Lee Jones
2015-08-18 21:01 ` Petr Cvek
2015-08-19 6:58 ` Lee Jones
2015-08-17 22:06 ` [PATCH v2 19/21] leds: leds-pasic3: Add support for PASIC3 LED controller Petr Cvek
2015-08-17 22:14 ` Petr Cvek
2015-08-18 10:27 ` Jacek Anaszewski [this message]
2015-08-18 21:48 ` Petr Cvek
2015-08-19 8:09 ` Philipp Zabel
2015-08-19 8:49 ` Jacek Anaszewski
2015-08-17 22:06 ` [PATCH v2 20/21] ARM: pxa: magician: Re-add pdata for new leds-pasic3 driver Petr Cvek
2015-08-17 22:07 ` [PATCH v2 21/21] ARM: pxa: magician: Move platform_add_devices() to the end of magician_init() Petr Cvek
2015-08-22 16:27 ` Robert Jarzmik
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=55D30875.3090103@samsung.com \
--to=j.anaszewski@samsung$(echo .)com \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox