We may currently get unpaired regulator calls when configuring the LED
brightness via sysfs in case of regulator calls producing errors. Let's
fix this by maintaining local state for enabled.
Signed-off-by: Tony Lindgren <[email protected]>
---
drivers/leds/leds-lm3532.c | 26 ++++++++++++++++++++++++--
1 file changed, 24 insertions(+), 2 deletions(-)
diff --git a/drivers/leds/leds-lm3532.c b/drivers/leds/leds-lm3532.c
--- a/drivers/leds/leds-lm3532.c
+++ b/drivers/leds/leds-lm3532.c
@@ -127,6 +127,7 @@ struct lm3532_als_data {
* @num_leds - Number of LED strings are supported in this array
* @full_scale_current - The full-scale current setting for the current sink.
* @led_strings - The LED strings supported in this array
+ * @enabled - Enabled status
* @label - LED label
*/
struct lm3532_led {
@@ -138,6 +139,7 @@ struct lm3532_led {
int ctrl_brt_pointer;
int num_leds;
int full_scale_current;
+ int enabled:1;
u32 led_strings[LM3532_MAX_CONTROL_BANKS];
char label[LED_MAX_NAME_SIZE];
};
@@ -292,11 +294,15 @@ static int lm3532_get_ramp_index(int ramp_time)
ramp_time);
}
+/* Caller must take care of locking */
static int lm3532_led_enable(struct lm3532_led *led_data)
{
int ctrl_en_val = BIT(led_data->control_bank);
int ret;
+ if (led_data->enabled)
+ return 0;
+
ret = regmap_update_bits(led_data->priv->regmap, LM3532_REG_ENABLE,
ctrl_en_val, ctrl_en_val);
if (ret) {
@@ -304,14 +310,24 @@ static int lm3532_led_enable(struct lm3532_led *led_data)
return ret;
}
- return regulator_enable(led_data->priv->regulator);
+ ret = regulator_enable(led_data->priv->regulator);
+ if (ret < 0)
+ return ret;
+
+ led_data->enabled = 1;
+
+ return 0;
}
+/* Caller must take care of locking */
static int lm3532_led_disable(struct lm3532_led *led_data)
{
int ctrl_en_val = BIT(led_data->control_bank);
int ret;
+ if (!led_data->enabled)
+ return 0;
+
ret = regmap_update_bits(led_data->priv->regmap, LM3532_REG_ENABLE,
ctrl_en_val, 0);
if (ret) {
@@ -319,7 +335,13 @@ static int lm3532_led_disable(struct lm3532_led *led_data)
return ret;
}
- return regulator_disable(led_data->priv->regulator);
+ ret = regulator_disable(led_data->priv->regulator);
+ if (ret < 0)
+ return ret;
+
+ led_data->enabled = 0;
+
+ return 0;
}
static int lm3532_brightness_set(struct led_classdev *led_cdev,
--
2.23.0
Hi!
> We may currently get unpaired regulator calls when configuring the LED
> brightness via sysfs in case of regulator calls producing errors. Let's
> fix this by maintaining local state for enabled.
>
> Signed-off-by: Tony Lindgren <[email protected]>
Acked-by: Pavel Machek <[email protected]>
> +++ b/drivers/leds/leds-lm3532.c
> @@ -127,6 +127,7 @@ struct lm3532_als_data {
> * @num_leds - Number of LED strings are supported in this array
> * @full_scale_current - The full-scale current setting for the current sink.
> * @led_strings - The LED strings supported in this array
> + * @enabled - Enabled status
> * @label - LED label
> */
> struct lm3532_led {
> @@ -138,6 +139,7 @@ struct lm3532_led {
> int ctrl_brt_pointer;
> int num_leds;
> int full_scale_current;
> + int enabled:1;
> u32 led_strings[LM3532_MAX_CONTROL_BANKS];
> char label[LED_MAX_NAME_SIZE];
> };
I'd do bool enabled, but this version is good, too.
Best regards,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Hi!
Eventually, these will be needed.
Best regards,
Pavel
commit 38d956977a7d6cbdc811676f9b4033da7487e045
Author: Pavel <[email protected]>
Date: Wed Aug 7 12:43:52 2019 +0200
d4: lm3532 needs to use right register function for backlight to work.
diff --git a/drivers/leds/leds-lm3532.c b/drivers/leds/leds-lm3532.c
index 365a22a5..f98e657 100644
--- a/drivers/leds/leds-lm3532.c
+++ b/drivers/leds/leds-lm3532.c
@@ -629,7 +629,7 @@ static int lm3532_parse_node(struct lm3532_data *priv)
lm3532_init_registers(led);
- ret = devm_led_classdev_register(priv->dev, &led->led_dev);
+ ret = devm_of_led_classdev_register(priv->dev, to_of_node(child), &led->led_dev);
if (ret) {
dev_err(&priv->client->dev, "led register err: %d\n",
ret);
diff --git a/arch/arm/boot/dts/omap4-droid4-xt894.dts b/arch/arm/boot/dts/omap4-droid4-xt894.dts
index 4454449..64abe87 100644
--- a/arch/arm/boot/dts/omap4-droid4-xt894.dts
+++ b/arch/arm/boot/dts/omap4-droid4-xt894.dts
@@ -185,6 +185,14 @@
pwm-names = "enable", "direction";
direction-duty-cycle-ns = <10000000>;
};
+
+ backlight: backlight {
+ compatible = "led-backlight";
+
+ leds = <&backlight_led>;
+ brightness-levels = <0 4 8 16 32 64 128 255>;
+ default-brightness-level = <6>;
+ };
};
&dss {
@@ -208,6 +216,8 @@
vddi-supply = <&lcd_regulator>;
reset-gpios = <&gpio4 5 GPIO_ACTIVE_HIGH>; /* gpio101 */
+ backlight = <&backlight>;
+
width-mm = <50>;
height-mm = <89>;
@@ -389,12 +427,11 @@
ramp-up-us = <1024>;
ramp-down-us = <8193>;
- led@0 {
+ backlight_led: led@0 {
reg = <0>;
led-sources = <2>;
ti,led-mode = <0>;
label = ":backlight";
- linux,default-trigger = "backlight";
};
led@1 {
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Hi Tony,
Thank you for the patch.
On 8/27/19 11:52 PM, Tony Lindgren wrote:
> We may currently get unpaired regulator calls when configuring the LED
> brightness via sysfs in case of regulator calls producing errors. Let's
> fix this by maintaining local state for enabled.
>
> Signed-off-by: Tony Lindgren <[email protected]>
> ---
> drivers/leds/leds-lm3532.c | 26 ++++++++++++++++++++++++--
> 1 file changed, 24 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/leds/leds-lm3532.c b/drivers/leds/leds-lm3532.c
> --- a/drivers/leds/leds-lm3532.c
> +++ b/drivers/leds/leds-lm3532.c
> @@ -127,6 +127,7 @@ struct lm3532_als_data {
> * @num_leds - Number of LED strings are supported in this array
> * @full_scale_current - The full-scale current setting for the current sink.
> * @led_strings - The LED strings supported in this array
> + * @enabled - Enabled status
> * @label - LED label
> */
> struct lm3532_led {
> @@ -138,6 +139,7 @@ struct lm3532_led {
> int ctrl_brt_pointer;
> int num_leds;
> int full_scale_current;
> + int enabled:1;
> u32 led_strings[LM3532_MAX_CONTROL_BANKS];
> char label[LED_MAX_NAME_SIZE];
> };
> @@ -292,11 +294,15 @@ static int lm3532_get_ramp_index(int ramp_time)
> ramp_time);
> }
>
> +/* Caller must take care of locking */
> static int lm3532_led_enable(struct lm3532_led *led_data)
> {
> int ctrl_en_val = BIT(led_data->control_bank);
> int ret;
>
> + if (led_data->enabled)
> + return 0;
> +
> ret = regmap_update_bits(led_data->priv->regmap, LM3532_REG_ENABLE,
> ctrl_en_val, ctrl_en_val);
> if (ret) {
> @@ -304,14 +310,24 @@ static int lm3532_led_enable(struct lm3532_led *led_data)
> return ret;
> }
>
> - return regulator_enable(led_data->priv->regulator);
> + ret = regulator_enable(led_data->priv->regulator);
> + if (ret < 0)
> + return ret;
> +
> + led_data->enabled = 1;
> +
> + return 0;
> }
>
> +/* Caller must take care of locking */
> static int lm3532_led_disable(struct lm3532_led *led_data)
> {
> int ctrl_en_val = BIT(led_data->control_bank);
> int ret;
>
> + if (!led_data->enabled)
> + return 0;
> +
> ret = regmap_update_bits(led_data->priv->regmap, LM3532_REG_ENABLE,
> ctrl_en_val, 0);
> if (ret) {
> @@ -319,7 +335,13 @@ static int lm3532_led_disable(struct lm3532_led *led_data)
> return ret;
> }
>
> - return regulator_disable(led_data->priv->regulator);
> + ret = regulator_disable(led_data->priv->regulator);
> + if (ret < 0)
> + return ret;
> +
> + led_data->enabled = 0;
> +
> + return 0;
> }
>
> static int lm3532_brightness_set(struct led_classdev *led_cdev,
>
Applied.
--
Best regards,
Jacek Anaszewski
On 8/28/19 10:53 AM, Pavel Machek wrote:
> Hi!
>
> Eventually, these will be needed.
>
> Best regards,
> Pavel
>
> commit 38d956977a7d6cbdc811676f9b4033da7487e045
> Author: Pavel <[email protected]>
> Date: Wed Aug 7 12:43:52 2019 +0200
>
> d4: lm3532 needs to use right register function for backlight to work.
>
> diff --git a/drivers/leds/leds-lm3532.c b/drivers/leds/leds-lm3532.c
> index 365a22a5..f98e657 100644
> --- a/drivers/leds/leds-lm3532.c
> +++ b/drivers/leds/leds-lm3532.c
> @@ -629,7 +629,7 @@ static int lm3532_parse_node(struct lm3532_data *priv)
>
> lm3532_init_registers(led);
>
> - ret = devm_led_classdev_register(priv->dev, &led->led_dev);
> + ret = devm_of_led_classdev_register(priv->dev, to_of_node(child), &led->led_dev);
We no longer have devm_of_led_classdev_register(). You must use
devm_led_classdev_register_ext().
--
Best regards,
Jacek Anaszewski
On Wed 2019-08-28 22:32:57, Jacek Anaszewski wrote:
> On 8/28/19 10:53 AM, Pavel Machek wrote:
> > Hi!
> >
> > Eventually, these will be needed.
> >
> > Best regards,
> > Pavel
> >
> > commit 38d956977a7d6cbdc811676f9b4033da7487e045
> > Author: Pavel <[email protected]>
> > Date: Wed Aug 7 12:43:52 2019 +0200
> >
> > d4: lm3532 needs to use right register function for backlight to work.
> >
> > diff --git a/drivers/leds/leds-lm3532.c b/drivers/leds/leds-lm3532.c
> > index 365a22a5..f98e657 100644
> > --- a/drivers/leds/leds-lm3532.c
> > +++ b/drivers/leds/leds-lm3532.c
> > @@ -629,7 +629,7 @@ static int lm3532_parse_node(struct lm3532_data *priv)
> >
> > lm3532_init_registers(led);
> >
> > - ret = devm_led_classdev_register(priv->dev, &led->led_dev);
> > + ret = devm_of_led_classdev_register(priv->dev, to_of_node(child), &led->led_dev);
>
> We no longer have devm_of_led_classdev_register(). You must use
> devm_led_classdev_register_ext().
Something like this (untested)?
Pavel
diff --git a/drivers/leds/leds-lm3532.c b/drivers/leds/leds-lm3532.c
index 62ace66..6340d5b 100644
--- a/drivers/leds/leds-lm3532.c
+++ b/drivers/leds/leds-lm3532.c
@@ -577,6 +577,11 @@ static int lm3532_parse_node(struct lm3532_data *priv)
priv->runtime_ramp_down = lm3532_get_ramp_index(ramp_time);
device_for_each_child_node(priv->dev, child) {
+ struct led_init_data idata = {
+ .fwnode = child,
+ .default_label = "backlight",
+ };
+
led = &priv->leds[i];
ret = fwnode_property_read_u32(child, "reg", &control_bank);
@@ -648,7 +653,7 @@ static int lm3532_parse_node(struct lm3532_data *priv)
led->led_dev.name = led->label;
led->led_dev.brightness_set_blocking = lm3532_brightness_set;
- ret = devm_led_classdev_register(priv->dev, &led->led_dev);
+ ret = devm_led_classdev_register_ext(priv->dev, &led->led_dev, &idata);
if (ret) {
dev_err(&priv->client->dev, "led register err: %d\n",
ret);
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
On 9/8/19 10:03 AM, Pavel Machek wrote:
> On Wed 2019-08-28 22:32:57, Jacek Anaszewski wrote:
>> On 8/28/19 10:53 AM, Pavel Machek wrote:
>>> Hi!
>>>
>>> Eventually, these will be needed.
>>>
>>> Best regards,
>>> Pavel
>>>
>>> commit 38d956977a7d6cbdc811676f9b4033da7487e045
>>> Author: Pavel <[email protected]>
>>> Date: Wed Aug 7 12:43:52 2019 +0200
>>>
>>> d4: lm3532 needs to use right register function for backlight to work.
>>>
>>> diff --git a/drivers/leds/leds-lm3532.c b/drivers/leds/leds-lm3532.c
>>> index 365a22a5..f98e657 100644
>>> --- a/drivers/leds/leds-lm3532.c
>>> +++ b/drivers/leds/leds-lm3532.c
>>> @@ -629,7 +629,7 @@ static int lm3532_parse_node(struct lm3532_data *priv)
>>>
>>> lm3532_init_registers(led);
>>>
>>> - ret = devm_led_classdev_register(priv->dev, &led->led_dev);
>>> + ret = devm_of_led_classdev_register(priv->dev, to_of_node(child), &led->led_dev);
>>
>> We no longer have devm_of_led_classdev_register(). You must use
>> devm_led_classdev_register_ext().
>
> Something like this (untested)?
>
> Pavel
>
> diff --git a/drivers/leds/leds-lm3532.c b/drivers/leds/leds-lm3532.c
> index 62ace66..6340d5b 100644
> --- a/drivers/leds/leds-lm3532.c
> +++ b/drivers/leds/leds-lm3532.c
> @@ -577,6 +577,11 @@ static int lm3532_parse_node(struct lm3532_data *priv)
> priv->runtime_ramp_down = lm3532_get_ramp_index(ramp_time);
>
> device_for_each_child_node(priv->dev, child) {
> + struct led_init_data idata = {
> + .fwnode = child,
> + .default_label = "backlight",
> + };
> +
If you want to properly switch to the new extended LED registration
API, then you need:
.default_label = ":",
.devicename = led->client->name;
and in addition to that you need to remove old way of composing
the LED name. Something like patch [0] for leds-lm3692x.c.
And also patch for DT for consistency would be needed (like [1]).
However it will not change anything in LED naming in comparison
to the existing code, except that it will enable the possibility
of using 'function' and 'color' DT properties instead of deprecated
'label'.
I suppose that you expected some extra bonus by passing
DT node, but I'm not sure what exactly. Possibly you confused
this with the patch set [2] that allows for instantiating
backlight device on top of LED class device (it has been forgotten
btw and will miss 5.4).
And for lm3532 - it was just relying on the glue provided by
ledtrig-backlight, as pointed out by Tony in [3].
> led = &priv->leds[i];
>
> ret = fwnode_property_read_u32(child, "reg", &control_bank);
> @@ -648,7 +653,7 @@ static int lm3532_parse_node(struct lm3532_data *priv)
> led->led_dev.name = led->label;
> led->led_dev.brightness_set_blocking = lm3532_brightness_set;
>
> - ret = devm_led_classdev_register(priv->dev, &led->led_dev);
> + ret = devm_led_classdev_register_ext(priv->dev, &led->led_dev, &idata);
> if (ret) {
> dev_err(&priv->client->dev, "led register err: %d\n",
> ret);
>
>
[0]
https://git.kernel.org/pub/scm/linux/kernel/git/j.anaszewski/linux-leds.git/commit/?h=for-next&id=a50ff28348934913c46feb7945571329e46c70b3
[1]
https://git.kernel.org/pub/scm/linux/kernel/git/j.anaszewski/linux-leds.git/commit/?h=for-next&id=4dcbc8f8c59f4b618d651f5ba884ee5bf562c8de
[2]
https://lore.kernel.org/linux-leds/[email protected]/
[3] https://lore.kernel.org/linux-leds/[email protected]/
--
Best regards,
Jacek Anaszewski
Use new registration support, which will eventually be needed for
proper backlight support.
Signed-off-by: Pavel Machek <[email protected]>
diff --git a/drivers/leds/leds-lm3532.c b/drivers/leds/leds-lm3532.c
index 0507c65..23f49b6 100644
--- a/drivers/leds/leds-lm3532.c
+++ b/drivers/leds/leds-lm3532.c
@@ -577,6 +577,12 @@ static int lm3532_parse_node(struct lm3532_data *priv)
priv->runtime_ramp_down = lm3532_get_ramp_index(ramp_time);
device_for_each_child_node(priv->dev, child) {
+ struct led_init_data idata = {
+ .fwnode = child,
+ .default_label = ":",
+ .devicename = priv->client->name,
+ };
+
led = &priv->leds[i];
ret = fwnode_property_read_u32(child, "reg", &control_bank);
@@ -651,7 +657,7 @@ static int lm3532_parse_node(struct lm3532_data *priv)
led->led_dev.name = led->label;
led->led_dev.brightness_set_blocking = lm3532_brightness_set;
- ret = devm_led_classdev_register(priv->dev, &led->led_dev);
+ ret = devm_led_classdev_register_ext(priv->dev, &led->led_dev, &idata);
if (ret) {
dev_err(&priv->client->dev, "led register err: %d\n",
ret);
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Hi!
> >>> +++ b/drivers/leds/leds-lm3532.c
> >>> @@ -629,7 +629,7 @@ static int lm3532_parse_node(struct lm3532_data *priv)
> >>>
> >>> lm3532_init_registers(led);
> >>>
> >>> - ret = devm_led_classdev_register(priv->dev, &led->led_dev);
> >>> + ret = devm_of_led_classdev_register(priv->dev, to_of_node(child), &led->led_dev);
> >>
> >> We no longer have devm_of_led_classdev_register(). You must use
> >> devm_led_classdev_register_ext().
> >
> > Something like this (untested)?
> If you want to properly switch to the new extended LED registration
> API, then you need:
>
> .default_label = ":",
> .devicename = led->client->name;
>
> and in addition to that you need to remove old way of composing
> the LED name. Something like patch [0] for leds-lm3692x.c.
> And also patch for DT for consistency would be needed (like [1]).
>
> However it will not change anything in LED naming in comparison
> to the existing code, except that it will enable the possibility
> of using 'function' and 'color' DT properties instead of deprecated
> 'label'.
>
> I suppose that you expected some extra bonus by passing
> DT node, but I'm not sure what exactly. Possibly you confused
> this with the patch set [2] that allows for instantiating
> backlight device on top of LED class device (it has been forgotten
> btw and will miss 5.4).
Yes, it is for LED backlight. Thanks for hints, you have corrected
version in your inbox.
Best regards,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Hi Pavel,
I love your patch! Yet something to improve:
[auto build test ERROR on linus/master]
[cannot apply to v5.3 next-20190916]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Pavel-Machek/lm3532-right-registration-to-work-with-LED-backlight/20190917-205315
config: sparc64-allmodconfig (attached as .config)
compiler: sparc64-linux-gcc (GCC) 7.4.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.4.0 make.cross ARCH=sparc64
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <[email protected]>
All error/warnings (new ones prefixed by >>):
drivers/leds/leds-lm3532.c: In function 'lm3532_parse_node':
>> drivers/leds/leds-lm3532.c:520:10: error: variable 'idata' has initializer but incomplete type
struct led_init_data idata = {
^~~~~~~~~~~~~
>> drivers/leds/leds-lm3532.c:521:5: error: 'struct led_init_data' has no member named 'fwnode'
.fwnode = child,
^~~~~~
>> drivers/leds/leds-lm3532.c:521:14: warning: excess elements in struct initializer
.fwnode = child,
^~~~~
drivers/leds/leds-lm3532.c:521:14: note: (near initialization for 'idata')
>> drivers/leds/leds-lm3532.c:522:5: error: 'struct led_init_data' has no member named 'default_label'
.default_label = ":",
^~~~~~~~~~~~~
drivers/leds/leds-lm3532.c:522:21: warning: excess elements in struct initializer
.default_label = ":",
^~~
drivers/leds/leds-lm3532.c:522:21: note: (near initialization for 'idata')
>> drivers/leds/leds-lm3532.c:523:5: error: 'struct led_init_data' has no member named 'devicename'
.devicename = priv->client->name,
^~~~~~~~~~
drivers/leds/leds-lm3532.c:523:18: warning: excess elements in struct initializer
.devicename = priv->client->name,
^~~~
drivers/leds/leds-lm3532.c:523:18: note: (near initialization for 'idata')
>> drivers/leds/leds-lm3532.c:520:24: error: storage size of 'idata' isn't known
struct led_init_data idata = {
^~~~~
>> drivers/leds/leds-lm3532.c:591:9: error: implicit declaration of function 'devm_led_classdev_register_ext'; did you mean 'devm_led_classdev_register'? [-Werror=implicit-function-declaration]
ret = devm_led_classdev_register_ext(priv->dev, &led->led_dev, &idata);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
devm_led_classdev_register
drivers/leds/leds-lm3532.c:520:24: warning: unused variable 'idata' [-Wunused-variable]
struct led_init_data idata = {
^~~~~
cc1: some warnings being treated as errors
vim +/idata +520 drivers/leds/leds-lm3532.c
485
486 static int lm3532_parse_node(struct lm3532_data *priv)
487 {
488 struct fwnode_handle *child = NULL;
489 struct lm3532_led *led;
490 const char *name;
491 int control_bank;
492 u32 ramp_time;
493 size_t i = 0;
494 int ret;
495
496 priv->enable_gpio = devm_gpiod_get_optional(&priv->client->dev,
497 "enable", GPIOD_OUT_LOW);
498 if (IS_ERR(priv->enable_gpio))
499 priv->enable_gpio = NULL;
500
501 priv->regulator = devm_regulator_get(&priv->client->dev, "vin");
502 if (IS_ERR(priv->regulator))
503 priv->regulator = NULL;
504
505 ret = device_property_read_u32(&priv->client->dev, "ramp-up-us",
506 &ramp_time);
507 if (ret)
508 dev_info(&priv->client->dev, "ramp-up-ms property missing\n");
509 else
510 priv->runtime_ramp_up = lm3532_get_ramp_index(ramp_time);
511
512 ret = device_property_read_u32(&priv->client->dev, "ramp-down-us",
513 &ramp_time);
514 if (ret)
515 dev_info(&priv->client->dev, "ramp-down-ms property missing\n");
516 else
517 priv->runtime_ramp_down = lm3532_get_ramp_index(ramp_time);
518
519 device_for_each_child_node(priv->dev, child) {
> 520 struct led_init_data idata = {
> 521 .fwnode = child,
> 522 .default_label = ":",
> 523 .devicename = priv->client->name,
524 };
525
526 led = &priv->leds[i];
527
528 ret = fwnode_property_read_u32(child, "reg", &control_bank);
529 if (ret) {
530 dev_err(&priv->client->dev, "reg property missing\n");
531 fwnode_handle_put(child);
532 goto child_out;
533 }
534
535 if (control_bank > LM3532_CONTROL_C) {
536 dev_err(&priv->client->dev, "Control bank invalid\n");
537 continue;
538 }
539
540 led->control_bank = control_bank;
541
542 ret = fwnode_property_read_u32(child, "ti,led-mode",
543 &led->mode);
544 if (ret) {
545 dev_err(&priv->client->dev, "ti,led-mode property missing\n");
546 fwnode_handle_put(child);
547 goto child_out;
548 }
549
550 if (led->mode == LM3532_BL_MODE_ALS) {
551 ret = lm3532_parse_als(priv);
552 if (ret)
553 dev_err(&priv->client->dev, "Failed to parse als\n");
554 else
555 lm3532_als_configure(priv, led);
556 }
557
558 led->num_leds = fwnode_property_read_u32_array(child,
559 "led-sources",
560 NULL, 0);
561
562 if (led->num_leds > LM3532_MAX_LED_STRINGS) {
563 dev_err(&priv->client->dev, "To many LED string defined\n");
564 continue;
565 }
566
567 ret = fwnode_property_read_u32_array(child, "led-sources",
568 led->led_strings,
569 led->num_leds);
570 if (ret) {
571 dev_err(&priv->client->dev, "led-sources property missing\n");
572 fwnode_handle_put(child);
573 goto child_out;
574 }
575
576 fwnode_property_read_string(child, "linux,default-trigger",
577 &led->led_dev.default_trigger);
578
579 ret = fwnode_property_read_string(child, "label", &name);
580 if (ret)
581 snprintf(led->label, sizeof(led->label),
582 "%s::", priv->client->name);
583 else
584 snprintf(led->label, sizeof(led->label),
585 "%s:%s", priv->client->name, name);
586
587 led->priv = priv;
588 led->led_dev.name = led->label;
589 led->led_dev.brightness_set_blocking = lm3532_brightness_set;
590
> 591 ret = devm_led_classdev_register_ext(priv->dev, &led->led_dev, &idata);
592 if (ret) {
593 dev_err(&priv->client->dev, "led register err: %d\n",
594 ret);
595 fwnode_handle_put(child);
596 goto child_out;
597 }
598
599 lm3532_init_registers(led);
600
601 i++;
602 }
603
604 child_out:
605 return ret;
606 }
607
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Hi Pavel,
On 9/17/19 2:42 PM, Pavel Machek wrote:
> Hi!
>
>>>>> +++ b/drivers/leds/leds-lm3532.c
>>>>> @@ -629,7 +629,7 @@ static int lm3532_parse_node(struct lm3532_data *priv)
>>>>>
>>>>> lm3532_init_registers(led);
>>>>>
>>>>> - ret = devm_led_classdev_register(priv->dev, &led->led_dev);
>>>>> + ret = devm_of_led_classdev_register(priv->dev, to_of_node(child), &led->led_dev);
>>>>
>>>> We no longer have devm_of_led_classdev_register(). You must use
>>>> devm_led_classdev_register_ext().
>>>
>>> Something like this (untested)?
>
>> If you want to properly switch to the new extended LED registration
>> API, then you need:
>>
>> .default_label = ":",
>> .devicename = led->client->name;
>>
>> and in addition to that you need to remove old way of composing
>> the LED name. Something like patch [0] for leds-lm3692x.c.
>> And also patch for DT for consistency would be needed (like [1]).
>>
>> However it will not change anything in LED naming in comparison
>> to the existing code, except that it will enable the possibility
>> of using 'function' and 'color' DT properties instead of deprecated
>> 'label'.
>>
>> I suppose that you expected some extra bonus by passing
>> DT node, but I'm not sure what exactly. Possibly you confused
>> this with the patch set [2] that allows for instantiating
>> backlight device on top of LED class device (it has been forgotten
>> btw and will miss 5.4).
>
> Yes, it is for LED backlight. Thanks for hints, you have corrected
> version in your inbox.
You need also below cleanups. Please compare my patches reworking
existing drivers in the for-next branch.
diff --git a/drivers/leds/leds-lm3532.c b/drivers/leds/leds-lm3532.c
index 0507c6575c08..fc166f1a1789 100644
--- a/drivers/leds/leds-lm3532.c
+++ b/drivers/leds/leds-lm3532.c
@@ -9,7 +9,6 @@
#include <linux/types.h>
#include <linux/regulator/consumer.h>
#include <linux/module.h>
-#include <uapi/linux/uleds.h>
#include <linux/gpio/consumer.h>
#define LM3532_NAME "lm3532-led"
@@ -128,7 +127,6 @@ struct lm3532_als_data {
* @full_scale_current - The full-scale current setting for the current
sink.
* @led_strings - The LED strings supported in this array
* @enabled - Enabled status
- * @label - LED label
*/
struct lm3532_led {
struct led_classdev led_dev;
@@ -141,7 +139,6 @@ struct lm3532_led {
int full_scale_current;
int enabled:1;
u32 led_strings[LM3532_MAX_CONTROL_BANKS];
- char label[LED_MAX_NAME_SIZE];
};
/**
@@ -639,16 +636,7 @@ static int lm3532_parse_node(struct lm3532_data *priv)
fwnode_property_read_string(child, "linux,default-trigger",
&led->led_dev.default_trigger);
- ret = fwnode_property_read_string(child, "label", &name);
- if (ret)
- snprintf(led->label, sizeof(led->label),
- "%s::", priv->client->name);
- else
- snprintf(led->label, sizeof(led->label),
- "%s:%s", priv->client->name, name);
-
led->priv = priv;
- led->led_dev.name = led->label;
led->led_dev.brightness_set_blocking =
lm3532_brightness_set;
ret = devm_led_classdev_register(priv->dev, &led->led_dev);
--
Best regards,
Jacek Anaszewski