lkml.org 
[lkml]   [2019]   [Apr]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 5/5] leds: lm3697: Introduce the lm3697 driver
From
Date
Pavel

On 4/13/19 3:06 PM, Pavel Machek wrote:
> On Mon 2019-03-25 09:24:03, Dan Murphy wrote:
>> Introduce the lm3697 LED driver for
>> backlighting and display.
>>
>> Datasheet location:
>> http://www.ti.com/lit/ds/symlink/lm3697.pdf
>>
>> Signed-off-by: Dan Murphy <dmurphy@ti.com>
>> ---
>> drivers/leds/Kconfig | 8 +-
>> drivers/leds/Makefile | 1 +
>> drivers/leds/leds-lm3697.c | 401 +++++++++++++++++++++++++++++++++++++
>> 3 files changed, 409 insertions(+), 1 deletion(-)
>> create mode 100644 drivers/leds/leds-lm3697.c
>>
>> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
>> index 735009e73414..688bb9a6f275 100644
>> --- a/drivers/leds/Kconfig
>> +++ b/drivers/leds/Kconfig
>> @@ -776,9 +776,15 @@ config LEDS_NIC78BX
>> To compile this driver as a module, choose M here: the module
>> will be called leds-nic78bx.
>>
>> +config LEDS_LM3697
>> + tristate "LED driver for LM3697"
>> + depends on LEDS_TI_LMU_COMMON
>> + help
>> + Say Y to enable the LM3697 LED driver for TI LMU devices.
>> + This supports the LED device LM3697.
>> +
>> config LEDS_TI_LMU_COMMON
>> tristate "LED driver for TI LMU"
>> - depends on REGMAP
>> help
>> Say Y to enable the LED driver for TI LMU devices.
>> This supports common features between the TI LM3532, LM3631, LM3632,
>
> Is deleting "depends on REGMAP" intentional? AFAICT you are using it.
>

Thanks for pointing that out. I don't know how that was in there.

> Plus we'd normally expect "COMMON" first and then specific driver. Not
> sure if Kconfig can handle it out-of-order...
>
>

OK. Should I rename the ti_lmu file to leds-common-ti-lmu?

This keeps the naming convention the same in the leds directory as well.

FYI I will not add your acked-by on the LMU patch that introduced the code unless you approve.
Since you found issues with the kconfig

Refererence
https://lore.kernel.org/patchwork/patch/1054500/

>> +static int lm3697_init(struct lm3697 *priv)
>> +{
>> + struct lm3697_led *led;
>> + int i, ret;
>> +
>> + if (priv->enable_gpio) {
>> + gpiod_direction_output(priv->enable_gpio, 1);
>> + } else {
>> + ret = regmap_write(priv->regmap, LM3697_RESET, LM3697_SW_RESET);
>> + if (ret) {
>> + dev_err(&priv->client->dev, "Cannot reset the device\n");
>> + goto out;
>> + }
>> + }
>> +
>> + ret = regmap_write(priv->regmap, LM3697_CTRL_ENABLE, 0x0);
>> + if (ret) {
>> + dev_err(&priv->client->dev, "Cannot write ctrl enable\n");
>> + goto out;
>> + }
>> +
>> + ret = regmap_write(priv->regmap, LM3697_OUTPUT_CONFIG, priv->bank_cfg);
>> + if (ret)
>> + dev_err(&priv->client->dev, "Cannot write OUTPUT config\n");
>
> Missing goto out?

Ack

>
>> + for (i = 0; i < LM3697_MAX_CONTROL_BANKS; i++) {
>> + led = &priv->leds[i];
>> + ret = ti_lmu_common_set_ramp(&led->lmu_data);
>> + if (ret)
>> + dev_err(&priv->client->dev, "Setting the ramp rate failed\n");
>> + }
>> +out:
>> + return ret;
>> +}
> Pavel
>

\
 
 \ /
  Last update: 2019-04-15 14:51    [W:0.059 / U:9.216 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site