Re: [FYI] lm3532: right registration to work with LED-backlight

[Jacek Anaszewski <jacek.anaszewski@gmail.com>]

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 <pavel@ucw.cz>
>>> 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/20190717141514.21171-1-jjhiblot@ti.com/
[3] https://lore.kernel.org/linux-leds/20190403235500.GU49658@atomide.com/
-- 
Best regards,
Jacek Anaszewski