Re: [PATCH v2] leds: add new lp8788 led driver
On Thu, Jul 26, 2012 at 10:51:18AM +0800, Bryan Wu wrote: > OK, thanks, I've merge this patch through my tree and can I add your ack to > it? Sure, Acked-by: Mark Brown -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] leds: add new lp8788 led driver
On Thu, Jul 26, 2012 at 10:51:18AM +0800, Bryan Wu wrote: OK, thanks, I've merge this patch through my tree and can I add your ack to it? Sure, Acked-by: Mark Brown broo...@opensource.wolfsonmicro.com -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] leds: add new lp8788 led driver
On Thu, Jul 26, 2012 at 2:43 AM, Mark Brown wrote: > On Wed, Jul 25, 2012 at 12:46:56PM +0800, Bryan Wu wrote: > >> I'm going to Ack this driver and Mark will you merge this as whole patchset? > >> Acked-by: Bryan Wu > > It's an MFD so Samuel would normally apply if it were going via the MFD > tree, though if the dependencies are correct there should be no harm > merging via LEDs (until the core is merged it won't be possible to > enable the build in Kconfig). OK, thanks, I've merge this patch through my tree and can I add your ack to it? -- Bryan Wu Kernel Developer+86.186-168-78255 Mobile Canonical Ltd. www.canonical.com Ubuntu - Linux for human beings | www.ubuntu.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] leds: add new lp8788 led driver
On Wed, Jul 25, 2012 at 12:46:56PM +0800, Bryan Wu wrote: > I'm going to Ack this driver and Mark will you merge this as whole patchset? > Acked-by: Bryan Wu It's an MFD so Samuel would normally apply if it were going via the MFD tree, though if the dependencies are correct there should be no harm merging via LEDs (until the core is merged it won't be possible to enable the build in Kconfig). signature.asc Description: Digital signature
Re: [PATCH v2] leds: add new lp8788 led driver
On Wed, Jul 25, 2012 at 12:46:56PM +0800, Bryan Wu wrote: I'm going to Ack this driver and Mark will you merge this as whole patchset? Acked-by: Bryan Wu bryan...@canonical.com It's an MFD so Samuel would normally apply if it were going via the MFD tree, though if the dependencies are correct there should be no harm merging via LEDs (until the core is merged it won't be possible to enable the build in Kconfig). signature.asc Description: Digital signature
Re: [PATCH v2] leds: add new lp8788 led driver
On Thu, Jul 26, 2012 at 2:43 AM, Mark Brown broo...@opensource.wolfsonmicro.com wrote: On Wed, Jul 25, 2012 at 12:46:56PM +0800, Bryan Wu wrote: I'm going to Ack this driver and Mark will you merge this as whole patchset? Acked-by: Bryan Wu bryan...@canonical.com It's an MFD so Samuel would normally apply if it were going via the MFD tree, though if the dependencies are correct there should be no harm merging via LEDs (until the core is merged it won't be possible to enable the build in Kconfig). OK, thanks, I've merge this patch through my tree and can I add your ack to it? -- Bryan Wu bryan...@canonical.com Kernel Developer+86.186-168-78255 Mobile Canonical Ltd. www.canonical.com Ubuntu - Linux for human beings | www.ubuntu.com -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] leds: add new lp8788 led driver
On Tue, Jul 24, 2012 at 8:55 PM, Mark Brown wrote: > On Tue, Jul 24, 2012 at 08:23:00AM +0800, Bryan Wu wrote: >> On Mon, Jul 23, 2012 at 2:19 AM, Mark Brown > >> > If the work is flushed then the state that userspace thought was set >> > when the driver is removed will actually be set before the driver is >> > removed. This is fairly minor but might be useful. > >> So what's kind of state you mentioned here that is cared by user >> space. I find these 2 functions are quite confused for use right now. > > Any state - none of the drivers with sleeping I/O can do anything > directly in their callbacks so they defer everything to work (we really > should have that in the core but it was too annoying to implement last > time I looked). > >> Literally, canceling normally will remove pending work item and wait >> for running work item to finish. flushing will wait for both pending >> and running work item to finish. > > Right, so if we flush it means we know that any scheduled work actually > ran and implemented whatever change was requested. Thanks Mark for clarifying this. I'm going to Ack this driver and Mark will you merge this as whole patchset? Acked-by: Bryan Wu Thanks, -Bryan -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] leds: add new lp8788 led driver
On Tue, Jul 24, 2012 at 08:23:00AM +0800, Bryan Wu wrote: > On Mon, Jul 23, 2012 at 2:19 AM, Mark Brown > > If the work is flushed then the state that userspace thought was set > > when the driver is removed will actually be set before the driver is > > removed. This is fairly minor but might be useful. > So what's kind of state you mentioned here that is cared by user > space. I find these 2 functions are quite confused for use right now. Any state - none of the drivers with sleeping I/O can do anything directly in their callbacks so they defer everything to work (we really should have that in the core but it was too annoying to implement last time I looked). > Literally, canceling normally will remove pending work item and wait > for running work item to finish. flushing will wait for both pending > and running work item to finish. Right, so if we flush it means we know that any scheduled work actually ran and implemented whatever change was requested. signature.asc Description: Digital signature
Re: [PATCH v2] leds: add new lp8788 led driver
On Tue, Jul 24, 2012 at 08:23:00AM +0800, Bryan Wu wrote: On Mon, Jul 23, 2012 at 2:19 AM, Mark Brown If the work is flushed then the state that userspace thought was set when the driver is removed will actually be set before the driver is removed. This is fairly minor but might be useful. So what's kind of state you mentioned here that is cared by user space. I find these 2 functions are quite confused for use right now. Any state - none of the drivers with sleeping I/O can do anything directly in their callbacks so they defer everything to work (we really should have that in the core but it was too annoying to implement last time I looked). Literally, canceling normally will remove pending work item and wait for running work item to finish. flushing will wait for both pending and running work item to finish. Right, so if we flush it means we know that any scheduled work actually ran and implemented whatever change was requested. signature.asc Description: Digital signature
Re: [PATCH v2] leds: add new lp8788 led driver
On Tue, Jul 24, 2012 at 8:55 PM, Mark Brown broo...@opensource.wolfsonmicro.com wrote: On Tue, Jul 24, 2012 at 08:23:00AM +0800, Bryan Wu wrote: On Mon, Jul 23, 2012 at 2:19 AM, Mark Brown If the work is flushed then the state that userspace thought was set when the driver is removed will actually be set before the driver is removed. This is fairly minor but might be useful. So what's kind of state you mentioned here that is cared by user space. I find these 2 functions are quite confused for use right now. Any state - none of the drivers with sleeping I/O can do anything directly in their callbacks so they defer everything to work (we really should have that in the core but it was too annoying to implement last time I looked). Literally, canceling normally will remove pending work item and wait for running work item to finish. flushing will wait for both pending and running work item to finish. Right, so if we flush it means we know that any scheduled work actually ran and implemented whatever change was requested. Thanks Mark for clarifying this. I'm going to Ack this driver and Mark will you merge this as whole patchset? Acked-by: Bryan Wu bryan...@canonical.com Thanks, -Bryan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] leds: add new lp8788 led driver
On Mon, Jul 23, 2012 at 2:19 AM, Mark Brown wrote: > On Sat, Jul 21, 2012 at 02:48:49AM +0800, Bryan Wu wrote: > >> Actually cancel_work_sync() is quite similar to flush_work_sync() >> here. For the timer thing, in fact it is NULL when cancel_work_sync() >> call __cancel_work_timer(). > >> And Mark, do you have any advice about the flush_work_sync() and >> cancel_work_sync(). I saw you use flush in the >> drivers/leds/leds-wm8350.c. > > If the work is flushed then the state that userspace thought was set > when the driver is removed will actually be set before the driver is > removed. This is fairly minor but might be useful. So what's kind of state you mentioned here that is cared by user space. I find these 2 functions are quite confused for use right now. Literally, canceling normally will remove pending work item and wait for running work item to finish. flushing will wait for both pending and running work item to finish. Thanks, -- Bryan Wu Kernel Developer+86.186-168-78255 Mobile Canonical Ltd. www.canonical.com Ubuntu - Linux for human beings | www.ubuntu.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] leds: add new lp8788 led driver
On Mon, Jul 23, 2012 at 2:19 AM, Mark Brown broo...@opensource.wolfsonmicro.com wrote: On Sat, Jul 21, 2012 at 02:48:49AM +0800, Bryan Wu wrote: Actually cancel_work_sync() is quite similar to flush_work_sync() here. For the timer thing, in fact it is NULL when cancel_work_sync() call __cancel_work_timer(). And Mark, do you have any advice about the flush_work_sync() and cancel_work_sync(). I saw you use flush in the drivers/leds/leds-wm8350.c. If the work is flushed then the state that userspace thought was set when the driver is removed will actually be set before the driver is removed. This is fairly minor but might be useful. So what's kind of state you mentioned here that is cared by user space. I find these 2 functions are quite confused for use right now. Literally, canceling normally will remove pending work item and wait for running work item to finish. flushing will wait for both pending and running work item to finish. Thanks, -- Bryan Wu bryan...@canonical.com Kernel Developer+86.186-168-78255 Mobile Canonical Ltd. www.canonical.com Ubuntu - Linux for human beings | www.ubuntu.com -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] leds: add new lp8788 led driver
On Sat, Jul 21, 2012 at 02:48:49AM +0800, Bryan Wu wrote: > Actually cancel_work_sync() is quite similar to flush_work_sync() > here. For the timer thing, in fact it is NULL when cancel_work_sync() > call __cancel_work_timer(). > And Mark, do you have any advice about the flush_work_sync() and > cancel_work_sync(). I saw you use flush in the > drivers/leds/leds-wm8350.c. If the work is flushed then the state that userspace thought was set when the driver is removed will actually be set before the driver is removed. This is fairly minor but might be useful. signature.asc Description: Digital signature
Re: [PATCH v2] leds: add new lp8788 led driver
On Sat, Jul 21, 2012 at 02:48:49AM +0800, Bryan Wu wrote: Actually cancel_work_sync() is quite similar to flush_work_sync() here. For the timer thing, in fact it is NULL when cancel_work_sync() call __cancel_work_timer(). And Mark, do you have any advice about the flush_work_sync() and cancel_work_sync(). I saw you use flush in the drivers/leds/leds-wm8350.c. If the work is flushed then the state that userspace thought was set when the driver is removed will actually be set before the driver is removed. This is fairly minor but might be useful. signature.asc Description: Digital signature
Re: [PATCH v2] leds: add new lp8788 led driver
On Fri, Jul 20, 2012 at 11:49 PM, Shuah Khan wrote:
> On Fri, 2012-07-20 at 08:43 +, Kim, Milo wrote:
>> TI LP8788 PMU has the current sink as the keyboard led driver.
>> The brightness is controlled by the i2c commands.
>> Configurable parameters can be defined in the platform side.
>>
>> Patch v2.
>> (a) use workqueue on changing the brightness
>>
>> (b) use mutex_lock/unlock when the brightness is set
>> and the led block of lp8788 device is enabled
>>
>> (c) remove err_dev on _probe()
>> : just return as returned value if any errors
>>
>> (d) replace module_init/exit() with module_platform_driver()
>>
>> (e) add led configuration structure and loading them by default
>> if platform data is null
>>
>> Signed-off-by: Milo(Woogyom) Kim
>> ---
>> drivers/leds/Kconfig |7 ++
>> drivers/leds/Makefile |1 +
>> drivers/leds/leds-lp8788.c | 192
>>
>> 3 files changed, 200 insertions(+), 0 deletions(-)
>> create mode 100644 drivers/leds/leds-lp8788.c
>>
>> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
>> index f028f03..a498deb 100644
>> --- a/drivers/leds/Kconfig
>> +++ b/drivers/leds/Kconfig
>> @@ -200,6 +200,13 @@ config LEDS_LP5523
>> Driver provides direct control via LED class and interface for
>> programming the engines.
>>
>> +config LEDS_LP8788
>> + tristate "LED support for the TI LP8788 PMIC"
>> + depends on LEDS_CLASS
>> + depends on MFD_LP8788
>> + help
>> + This option enables support for the Keyboard LEDs on the LP8788 PMIC.
>> +
>> config LEDS_CLEVO_MAIL
>> tristate "Mail LED on Clevo notebook"
>> depends on LEDS_CLASS
>> diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
>> index 5eebd7b..f156193 100644
>> --- a/drivers/leds/Makefile
>> +++ b/drivers/leds/Makefile
>> @@ -24,6 +24,7 @@ obj-$(CONFIG_LEDS_GPIO) += leds-gpio.o
>> obj-$(CONFIG_LEDS_LP3944)+= leds-lp3944.o
>> obj-$(CONFIG_LEDS_LP5521)+= leds-lp5521.o
>> obj-$(CONFIG_LEDS_LP5523)+= leds-lp5523.o
>> +obj-$(CONFIG_LEDS_LP8788)+= leds-lp8788.o
>> obj-$(CONFIG_LEDS_TCA6507) += leds-tca6507.o
>> obj-$(CONFIG_LEDS_CLEVO_MAIL)+= leds-clevo-mail.o
>> obj-$(CONFIG_LEDS_HP6XX) += leds-hp6xx.o
>> diff --git a/drivers/leds/leds-lp8788.c b/drivers/leds/leds-lp8788.c
>> new file mode 100644
>> index 000..574b49f
>> --- /dev/null
>> +++ b/drivers/leds/leds-lp8788.c
>> @@ -0,0 +1,192 @@
>> +/*
>> + * TI LP8788 MFD - keyled driver
>> + *
>> + * Copyright 2012 Texas Instruments
>> + *
>> + * Author: Milo(Woogyom) Kim
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + *
>> + */
>> +
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +
>> +#define MAX_BRIGHTNESS LP8788_ISINK_MAX_PWM
>> +#define DEFAULT_LED_NAME "keyboard-backlight"
>> +
>> +struct lp8788_led {
>> + struct lp8788 *lp;
>> + struct mutex lock;
>> + struct work_struct work;
>> + struct led_classdev led_dev;
>> + enum lp8788_isink_number isink_num;
>> + enum led_brightness brightness;
>> + int on;
>> +};
>> +
>> +struct lp8788_led_config {
>> + enum lp8788_isink_scale scale;
>> + enum lp8788_isink_number num;
>> + int iout;
>> +};
>> +
>> +static struct lp8788_led_config default_led_config = {
>> + .scale = LP8788_ISINK_SCALE_100mA,
>> + .num = LP8788_ISINK_3,
>> + .iout = 0,
>> +};
>> +
>> +static int lp8788_led_init_device(struct lp8788_led *led,
>> + struct lp8788_led_platform_data *pdata)
>> +{
>> + struct lp8788_led_config *cfg = _led_config;
>> + u8 addr, mask, val;
>> + int ret;
>> +
>> + if (pdata) {
>> + cfg->scale = pdata->scale;
>> + cfg->num = pdata->num;
>> + cfg->iout = pdata->iout_code;
>> + }
>> +
>> + led->isink_num = cfg->num;
>> +
>> + /* scale configuration */
>> + addr = LP8788_ISINK_CTRL;
>> + mask = 1 << (cfg->num + LP8788_ISINK_SCALE_OFFSET);
>> + val = cfg->scale << cfg->num;
>> + ret = lp8788_update_bits(led->lp, addr, mask, val);
>> + if (ret)
>> + return ret;
>> +
>> + /* current configuration */
>> + addr = lp8788_iout_addr[cfg->num];
>> + mask = lp8788_iout_mask[cfg->num];
>> + val = cfg->iout;
>> +
>> + return lp8788_update_bits(led->lp, addr, mask, val);
>> +}
>> +
>> +static void lp8788_led_enable(struct lp8788_led *led,
>> + enum lp8788_isink_number num, int on)
>> +{
>> + u8 mask = 1 << num;
>> + u8 val = on << num;
>> +
>> + if (lp8788_update_bits(led->lp, LP8788_ISINK_CTRL, mask, val))
>> + return;
Re: [PATCH v2] leds: add new lp8788 led driver
Hi,
I think a mutex_unlock is missed out,
On Fri, Jul 20, 2012 at 2:28 PM, Kim, Milo wrote:
> +
> +static void lp8788_led_work(struct work_struct *work)
> +{
> + struct lp8788_led *led = container_of(work, struct lp8788_led, work);
> + enum lp8788_isink_number num = led->isink_num;
> + int enable;
> + u8 val = led->brightness;
> +
> + mutex_lock(>lock);
> +
> + switch (num) {
> + case LP8788_ISINK_1:
> + case LP8788_ISINK_2:
> + case LP8788_ISINK_3:
> + lp8788_write_byte(led->lp, lp8788_pwm_addr[num], val);
> + break;
> + default:
missed mutex_unlock
> + return;
> + }
> +
> + enable = (val > 0) ? 1 : 0;
> + if (enable != led->on)
> + lp8788_led_enable(led, num, enable);
> +
> + mutex_unlock(>lock);
> +}
> +
Thanks,
Devendra
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] leds: add new lp8788 led driver
On Fri, 2012-07-20 at 08:43 +, Kim, Milo wrote:
> TI LP8788 PMU has the current sink as the keyboard led driver.
> The brightness is controlled by the i2c commands.
> Configurable parameters can be defined in the platform side.
>
> Patch v2.
> (a) use workqueue on changing the brightness
>
> (b) use mutex_lock/unlock when the brightness is set
> and the led block of lp8788 device is enabled
>
> (c) remove err_dev on _probe()
> : just return as returned value if any errors
>
> (d) replace module_init/exit() with module_platform_driver()
>
> (e) add led configuration structure and loading them by default
> if platform data is null
>
> Signed-off-by: Milo(Woogyom) Kim
> ---
> drivers/leds/Kconfig |7 ++
> drivers/leds/Makefile |1 +
> drivers/leds/leds-lp8788.c | 192
>
> 3 files changed, 200 insertions(+), 0 deletions(-)
> create mode 100644 drivers/leds/leds-lp8788.c
>
> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> index f028f03..a498deb 100644
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -200,6 +200,13 @@ config LEDS_LP5523
> Driver provides direct control via LED class and interface for
> programming the engines.
>
> +config LEDS_LP8788
> + tristate "LED support for the TI LP8788 PMIC"
> + depends on LEDS_CLASS
> + depends on MFD_LP8788
> + help
> + This option enables support for the Keyboard LEDs on the LP8788 PMIC.
> +
> config LEDS_CLEVO_MAIL
> tristate "Mail LED on Clevo notebook"
> depends on LEDS_CLASS
> diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
> index 5eebd7b..f156193 100644
> --- a/drivers/leds/Makefile
> +++ b/drivers/leds/Makefile
> @@ -24,6 +24,7 @@ obj-$(CONFIG_LEDS_GPIO) += leds-gpio.o
> obj-$(CONFIG_LEDS_LP3944)+= leds-lp3944.o
> obj-$(CONFIG_LEDS_LP5521)+= leds-lp5521.o
> obj-$(CONFIG_LEDS_LP5523)+= leds-lp5523.o
> +obj-$(CONFIG_LEDS_LP8788)+= leds-lp8788.o
> obj-$(CONFIG_LEDS_TCA6507) += leds-tca6507.o
> obj-$(CONFIG_LEDS_CLEVO_MAIL)+= leds-clevo-mail.o
> obj-$(CONFIG_LEDS_HP6XX) += leds-hp6xx.o
> diff --git a/drivers/leds/leds-lp8788.c b/drivers/leds/leds-lp8788.c
> new file mode 100644
> index 000..574b49f
> --- /dev/null
> +++ b/drivers/leds/leds-lp8788.c
> @@ -0,0 +1,192 @@
> +/*
> + * TI LP8788 MFD - keyled driver
> + *
> + * Copyright 2012 Texas Instruments
> + *
> + * Author: Milo(Woogyom) Kim
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#define MAX_BRIGHTNESS LP8788_ISINK_MAX_PWM
> +#define DEFAULT_LED_NAME "keyboard-backlight"
> +
> +struct lp8788_led {
> + struct lp8788 *lp;
> + struct mutex lock;
> + struct work_struct work;
> + struct led_classdev led_dev;
> + enum lp8788_isink_number isink_num;
> + enum led_brightness brightness;
> + int on;
> +};
> +
> +struct lp8788_led_config {
> + enum lp8788_isink_scale scale;
> + enum lp8788_isink_number num;
> + int iout;
> +};
> +
> +static struct lp8788_led_config default_led_config = {
> + .scale = LP8788_ISINK_SCALE_100mA,
> + .num = LP8788_ISINK_3,
> + .iout = 0,
> +};
> +
> +static int lp8788_led_init_device(struct lp8788_led *led,
> + struct lp8788_led_platform_data *pdata)
> +{
> + struct lp8788_led_config *cfg = _led_config;
> + u8 addr, mask, val;
> + int ret;
> +
> + if (pdata) {
> + cfg->scale = pdata->scale;
> + cfg->num = pdata->num;
> + cfg->iout = pdata->iout_code;
> + }
> +
> + led->isink_num = cfg->num;
> +
> + /* scale configuration */
> + addr = LP8788_ISINK_CTRL;
> + mask = 1 << (cfg->num + LP8788_ISINK_SCALE_OFFSET);
> + val = cfg->scale << cfg->num;
> + ret = lp8788_update_bits(led->lp, addr, mask, val);
> + if (ret)
> + return ret;
> +
> + /* current configuration */
> + addr = lp8788_iout_addr[cfg->num];
> + mask = lp8788_iout_mask[cfg->num];
> + val = cfg->iout;
> +
> + return lp8788_update_bits(led->lp, addr, mask, val);
> +}
> +
> +static void lp8788_led_enable(struct lp8788_led *led,
> + enum lp8788_isink_number num, int on)
> +{
> + u8 mask = 1 << num;
> + u8 val = on << num;
> +
> + if (lp8788_update_bits(led->lp, LP8788_ISINK_CTRL, mask, val))
> + return;
> +
> + led->on = on;
> +}
> +
> +static void lp8788_led_work(struct work_struct *work)
> +{
> + struct lp8788_led *led = container_of(work, struct lp8788_led, work);
> + enum
Re: [PATCH v2] leds: add new lp8788 led driver
On Fri, 2012-07-20 at 08:43 +, Kim, Milo wrote:
TI LP8788 PMU has the current sink as the keyboard led driver.
The brightness is controlled by the i2c commands.
Configurable parameters can be defined in the platform side.
Patch v2.
(a) use workqueue on changing the brightness
(b) use mutex_lock/unlock when the brightness is set
and the led block of lp8788 device is enabled
(c) remove err_dev on _probe()
: just return as returned value if any errors
(d) replace module_init/exit() with module_platform_driver()
(e) add led configuration structure and loading them by default
if platform data is null
Signed-off-by: Milo(Woogyom) Kim milo@ti.com
---
drivers/leds/Kconfig |7 ++
drivers/leds/Makefile |1 +
drivers/leds/leds-lp8788.c | 192
3 files changed, 200 insertions(+), 0 deletions(-)
create mode 100644 drivers/leds/leds-lp8788.c
diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index f028f03..a498deb 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -200,6 +200,13 @@ config LEDS_LP5523
Driver provides direct control via LED class and interface for
programming the engines.
+config LEDS_LP8788
+ tristate LED support for the TI LP8788 PMIC
+ depends on LEDS_CLASS
+ depends on MFD_LP8788
+ help
+ This option enables support for the Keyboard LEDs on the LP8788 PMIC.
+
config LEDS_CLEVO_MAIL
tristate Mail LED on Clevo notebook
depends on LEDS_CLASS
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index 5eebd7b..f156193 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -24,6 +24,7 @@ obj-$(CONFIG_LEDS_GPIO) += leds-gpio.o
obj-$(CONFIG_LEDS_LP3944)+= leds-lp3944.o
obj-$(CONFIG_LEDS_LP5521)+= leds-lp5521.o
obj-$(CONFIG_LEDS_LP5523)+= leds-lp5523.o
+obj-$(CONFIG_LEDS_LP8788)+= leds-lp8788.o
obj-$(CONFIG_LEDS_TCA6507) += leds-tca6507.o
obj-$(CONFIG_LEDS_CLEVO_MAIL)+= leds-clevo-mail.o
obj-$(CONFIG_LEDS_HP6XX) += leds-hp6xx.o
diff --git a/drivers/leds/leds-lp8788.c b/drivers/leds/leds-lp8788.c
new file mode 100644
index 000..574b49f
--- /dev/null
+++ b/drivers/leds/leds-lp8788.c
@@ -0,0 +1,192 @@
+/*
+ * TI LP8788 MFD - keyled driver
+ *
+ * Copyright 2012 Texas Instruments
+ *
+ * Author: Milo(Woogyom) Kim milo@ti.com
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#include linux/module.h
+#include linux/slab.h
+#include linux/err.h
+#include linux/platform_device.h
+#include linux/leds.h
+#include linux/mutex.h
+#include linux/mfd/lp8788.h
+#include linux/mfd/lp8788-isink.h
+
+#define MAX_BRIGHTNESS LP8788_ISINK_MAX_PWM
+#define DEFAULT_LED_NAME keyboard-backlight
+
+struct lp8788_led {
+ struct lp8788 *lp;
+ struct mutex lock;
+ struct work_struct work;
+ struct led_classdev led_dev;
+ enum lp8788_isink_number isink_num;
+ enum led_brightness brightness;
+ int on;
+};
+
+struct lp8788_led_config {
+ enum lp8788_isink_scale scale;
+ enum lp8788_isink_number num;
+ int iout;
+};
+
+static struct lp8788_led_config default_led_config = {
+ .scale = LP8788_ISINK_SCALE_100mA,
+ .num = LP8788_ISINK_3,
+ .iout = 0,
+};
+
+static int lp8788_led_init_device(struct lp8788_led *led,
+ struct lp8788_led_platform_data *pdata)
+{
+ struct lp8788_led_config *cfg = default_led_config;
+ u8 addr, mask, val;
+ int ret;
+
+ if (pdata) {
+ cfg-scale = pdata-scale;
+ cfg-num = pdata-num;
+ cfg-iout = pdata-iout_code;
+ }
+
+ led-isink_num = cfg-num;
+
+ /* scale configuration */
+ addr = LP8788_ISINK_CTRL;
+ mask = 1 (cfg-num + LP8788_ISINK_SCALE_OFFSET);
+ val = cfg-scale cfg-num;
+ ret = lp8788_update_bits(led-lp, addr, mask, val);
+ if (ret)
+ return ret;
+
+ /* current configuration */
+ addr = lp8788_iout_addr[cfg-num];
+ mask = lp8788_iout_mask[cfg-num];
+ val = cfg-iout;
+
+ return lp8788_update_bits(led-lp, addr, mask, val);
+}
+
+static void lp8788_led_enable(struct lp8788_led *led,
+ enum lp8788_isink_number num, int on)
+{
+ u8 mask = 1 num;
+ u8 val = on num;
+
+ if (lp8788_update_bits(led-lp, LP8788_ISINK_CTRL, mask, val))
+ return;
+
+ led-on = on;
+}
+
+static void lp8788_led_work(struct work_struct *work)
+{
+ struct lp8788_led *led = container_of(work, struct lp8788_led, work);
+ enum lp8788_isink_number num =
Re: [PATCH v2] leds: add new lp8788 led driver
Hi,
I think a mutex_unlock is missed out,
On Fri, Jul 20, 2012 at 2:28 PM, Kim, Milo milo@ti.com wrote:
+
+static void lp8788_led_work(struct work_struct *work)
+{
+ struct lp8788_led *led = container_of(work, struct lp8788_led, work);
+ enum lp8788_isink_number num = led-isink_num;
+ int enable;
+ u8 val = led-brightness;
+
+ mutex_lock(led-lock);
+
+ switch (num) {
+ case LP8788_ISINK_1:
+ case LP8788_ISINK_2:
+ case LP8788_ISINK_3:
+ lp8788_write_byte(led-lp, lp8788_pwm_addr[num], val);
+ break;
+ default:
missed mutex_unlock
+ return;
+ }
+
+ enable = (val 0) ? 1 : 0;
+ if (enable != led-on)
+ lp8788_led_enable(led, num, enable);
+
+ mutex_unlock(led-lock);
+}
+
Thanks,
Devendra
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] leds: add new lp8788 led driver
On Fri, Jul 20, 2012 at 11:49 PM, Shuah Khan shuahk...@gmail.com wrote:
On Fri, 2012-07-20 at 08:43 +, Kim, Milo wrote:
TI LP8788 PMU has the current sink as the keyboard led driver.
The brightness is controlled by the i2c commands.
Configurable parameters can be defined in the platform side.
Patch v2.
(a) use workqueue on changing the brightness
(b) use mutex_lock/unlock when the brightness is set
and the led block of lp8788 device is enabled
(c) remove err_dev on _probe()
: just return as returned value if any errors
(d) replace module_init/exit() with module_platform_driver()
(e) add led configuration structure and loading them by default
if platform data is null
Signed-off-by: Milo(Woogyom) Kim milo@ti.com
---
drivers/leds/Kconfig |7 ++
drivers/leds/Makefile |1 +
drivers/leds/leds-lp8788.c | 192
3 files changed, 200 insertions(+), 0 deletions(-)
create mode 100644 drivers/leds/leds-lp8788.c
diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index f028f03..a498deb 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -200,6 +200,13 @@ config LEDS_LP5523
Driver provides direct control via LED class and interface for
programming the engines.
+config LEDS_LP8788
+ tristate LED support for the TI LP8788 PMIC
+ depends on LEDS_CLASS
+ depends on MFD_LP8788
+ help
+ This option enables support for the Keyboard LEDs on the LP8788 PMIC.
+
config LEDS_CLEVO_MAIL
tristate Mail LED on Clevo notebook
depends on LEDS_CLASS
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index 5eebd7b..f156193 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -24,6 +24,7 @@ obj-$(CONFIG_LEDS_GPIO) += leds-gpio.o
obj-$(CONFIG_LEDS_LP3944)+= leds-lp3944.o
obj-$(CONFIG_LEDS_LP5521)+= leds-lp5521.o
obj-$(CONFIG_LEDS_LP5523)+= leds-lp5523.o
+obj-$(CONFIG_LEDS_LP8788)+= leds-lp8788.o
obj-$(CONFIG_LEDS_TCA6507) += leds-tca6507.o
obj-$(CONFIG_LEDS_CLEVO_MAIL)+= leds-clevo-mail.o
obj-$(CONFIG_LEDS_HP6XX) += leds-hp6xx.o
diff --git a/drivers/leds/leds-lp8788.c b/drivers/leds/leds-lp8788.c
new file mode 100644
index 000..574b49f
--- /dev/null
+++ b/drivers/leds/leds-lp8788.c
@@ -0,0 +1,192 @@
+/*
+ * TI LP8788 MFD - keyled driver
+ *
+ * Copyright 2012 Texas Instruments
+ *
+ * Author: Milo(Woogyom) Kim milo@ti.com
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#include linux/module.h
+#include linux/slab.h
+#include linux/err.h
+#include linux/platform_device.h
+#include linux/leds.h
+#include linux/mutex.h
+#include linux/mfd/lp8788.h
+#include linux/mfd/lp8788-isink.h
+
+#define MAX_BRIGHTNESS LP8788_ISINK_MAX_PWM
+#define DEFAULT_LED_NAME keyboard-backlight
+
+struct lp8788_led {
+ struct lp8788 *lp;
+ struct mutex lock;
+ struct work_struct work;
+ struct led_classdev led_dev;
+ enum lp8788_isink_number isink_num;
+ enum led_brightness brightness;
+ int on;
+};
+
+struct lp8788_led_config {
+ enum lp8788_isink_scale scale;
+ enum lp8788_isink_number num;
+ int iout;
+};
+
+static struct lp8788_led_config default_led_config = {
+ .scale = LP8788_ISINK_SCALE_100mA,
+ .num = LP8788_ISINK_3,
+ .iout = 0,
+};
+
+static int lp8788_led_init_device(struct lp8788_led *led,
+ struct lp8788_led_platform_data *pdata)
+{
+ struct lp8788_led_config *cfg = default_led_config;
+ u8 addr, mask, val;
+ int ret;
+
+ if (pdata) {
+ cfg-scale = pdata-scale;
+ cfg-num = pdata-num;
+ cfg-iout = pdata-iout_code;
+ }
+
+ led-isink_num = cfg-num;
+
+ /* scale configuration */
+ addr = LP8788_ISINK_CTRL;
+ mask = 1 (cfg-num + LP8788_ISINK_SCALE_OFFSET);
+ val = cfg-scale cfg-num;
+ ret = lp8788_update_bits(led-lp, addr, mask, val);
+ if (ret)
+ return ret;
+
+ /* current configuration */
+ addr = lp8788_iout_addr[cfg-num];
+ mask = lp8788_iout_mask[cfg-num];
+ val = cfg-iout;
+
+ return lp8788_update_bits(led-lp, addr, mask, val);
+}
+
+static void lp8788_led_enable(struct lp8788_led *led,
+ enum lp8788_isink_number num, int on)
+{
+ u8 mask = 1 num;
+ u8 val = on num;
+
+ if (lp8788_update_bits(led-lp, LP8788_ISINK_CTRL, mask, val))
+ return;
+
+ led-on = on;
+}
+
+static void lp8788_led_work(struct work_struct *work)
+{
+ struct lp8788_led *led = container_of(work,
