Linux-LEDs Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] leds: gpio: Fix uninitialized gpio label for fwnode based probe
@ 2019-12-05 21:25 Jacek Anaszewski
  2019-12-11  0:44 ` Linus Walleij
  2019-12-21 18:49 ` Pavel Machek
  0 siblings, 2 replies; 4+ messages in thread
From: Jacek Anaszewski @ 2019-12-05 21:25 UTC (permalink / raw)
  To: linux-leds
  Cc: linux-kernel, jacek.anaszewski, Linus Walleij, Pavel Machek,
	Russell King

When switching to using generic LED name composition mechanism via
devm_led_classdev_register_ext() API the part of code initializing
struct gpio_led's template name property was removed alongside.
It was however overlooked that the property was also passed to
devm_fwnode_get_gpiod_from_child() in place of "label" parameter,
which when set to NULL, results in gpio label being initialized to '?'.

It could be observed in debugfs and failed to properly identify
gpio association with LED consumer.

Fix this shortcoming by updating the GPIO label after the LED is
registered and its final name is known.

Fixes: d7235f5feaa0 ("leds: gpio: Use generic support for composing LED names")
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Pavel Machek <pavel@ucw.cz>
Cc: Russell King <linux@armlinux.org.uk>
Signed-off-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
---
 drivers/leds/leds-gpio.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c
index a5c73f3d5f79..43eeed096d0d 100644
--- a/drivers/leds/leds-gpio.c
+++ b/drivers/leds/leds-gpio.c
@@ -151,9 +151,14 @@ static struct gpio_leds_priv *gpio_leds_create(struct platform_device *pdev)
 		struct gpio_led led = {};
 		const char *state = NULL;
 
+		/**
+		 * Acquire gpiod from DT with uninitialized label, which
+		 * will be updated after LED class device is registered,
+		 * Only then the final LED name is known.
+		 */
 		led.gpiod = devm_fwnode_get_gpiod_from_child(dev, NULL, child,
 							     GPIOD_ASIS,
-							     led.name);
+							     NULL);
 		if (IS_ERR(led.gpiod)) {
 			fwnode_handle_put(child);
 			return ERR_CAST(led.gpiod);
@@ -186,6 +191,9 @@ static struct gpio_leds_priv *gpio_leds_create(struct platform_device *pdev)
 			fwnode_handle_put(child);
 			return ERR_PTR(ret);
 		}
+		/* Set gpiod label to match the corresponding LED name. */
+		gpiod_set_consumer_name(led_dat->gpiod,
+					led_dat->cdev.dev->kobj.name);
 		priv->num_leds++;
 	}
 
-- 
2.11.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] leds: gpio: Fix uninitialized gpio label for fwnode based probe
  2019-12-05 21:25 [PATCH] leds: gpio: Fix uninitialized gpio label for fwnode based probe Jacek Anaszewski
@ 2019-12-11  0:44 ` Linus Walleij
  2019-12-21 18:49 ` Pavel Machek
  1 sibling, 0 replies; 4+ messages in thread
From: Linus Walleij @ 2019-12-11  0:44 UTC (permalink / raw)
  To: Jacek Anaszewski
  Cc: Linux LED Subsystem, linux-kernel, Pavel Machek, Russell King

On Thu, Dec 5, 2019 at 10:25 PM Jacek Anaszewski
<jacek.anaszewski@gmail.com> wrote:

> When switching to using generic LED name composition mechanism via
> devm_led_classdev_register_ext() API the part of code initializing
> struct gpio_led's template name property was removed alongside.
> It was however overlooked that the property was also passed to
> devm_fwnode_get_gpiod_from_child() in place of "label" parameter,
> which when set to NULL, results in gpio label being initialized to '?'.
>
> It could be observed in debugfs and failed to properly identify
> gpio association with LED consumer.
>
> Fix this shortcoming by updating the GPIO label after the LED is
> registered and its final name is known.
>
> Fixes: d7235f5feaa0 ("leds: gpio: Use generic support for composing LED names")
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Pavel Machek <pavel@ucw.cz>
> Cc: Russell King <linux@armlinux.org.uk>
> Signed-off-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] leds: gpio: Fix uninitialized gpio label for fwnode based probe
  2019-12-05 21:25 [PATCH] leds: gpio: Fix uninitialized gpio label for fwnode based probe Jacek Anaszewski
  2019-12-11  0:44 ` Linus Walleij
@ 2019-12-21 18:49 ` Pavel Machek
  2019-12-21 20:17   ` Jacek Anaszewski
  1 sibling, 1 reply; 4+ messages in thread
From: Pavel Machek @ 2019-12-21 18:49 UTC (permalink / raw)
  To: Jacek Anaszewski; +Cc: linux-leds, linux-kernel, Linus Walleij, Russell King

[-- Attachment #1: Type: text/plain, Size: 1745 bytes --]

Hi!

> When switching to using generic LED name composition mechanism via
> devm_led_classdev_register_ext() API the part of code initializing
> struct gpio_led's template name property was removed alongside.
> It was however overlooked that the property was also passed to
> devm_fwnode_get_gpiod_from_child() in place of "label" parameter,
> which when set to NULL, results in gpio label being initialized to '?'.
> 
> It could be observed in debugfs and failed to properly identify
> gpio association with LED consumer.
> 
> Fix this shortcoming by updating the GPIO label after the LED is
> registered and its final name is known.
> 
> Fixes: d7235f5feaa0 ("leds: gpio: Use generic support for composing LED names")
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Pavel Machek <pavel@ucw.cz>
> Cc: Russell King <linux@armlinux.org.uk>
> Signed-off-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>

Patch looks good, except:

> @@ -151,9 +151,14 @@ static struct gpio_leds_priv *gpio_leds_create(struct platform_device *pdev)
>  		struct gpio_led led = {};
>  		const char *state = NULL;
>  
> +		/**
> +		 * Acquire gpiod from DT with uninitialized label, which
> +		 * will be updated after LED class device is registered,
> +		 * Only then the final LED name is known.
> +		 */
>  		led.gpiod = devm_fwnode_get_gpiod_from_child(dev, NULL, child,
>  							     GPIOD_ASIS,
> -							     led.name);
> +							     NULL);

This is not linuxdoc, so comment should beging with /* AFAICT.

I'll probably hand-edit the patch.

Best regards,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] leds: gpio: Fix uninitialized gpio label for fwnode based probe
  2019-12-21 18:49 ` Pavel Machek
@ 2019-12-21 20:17   ` Jacek Anaszewski
  0 siblings, 0 replies; 4+ messages in thread
From: Jacek Anaszewski @ 2019-12-21 20:17 UTC (permalink / raw)
  To: Pavel Machek; +Cc: linux-leds, linux-kernel, Linus Walleij, Russell King

On 12/21/19 7:49 PM, Pavel Machek wrote:
> Hi!
> 
>> When switching to using generic LED name composition mechanism via
>> devm_led_classdev_register_ext() API the part of code initializing
>> struct gpio_led's template name property was removed alongside.
>> It was however overlooked that the property was also passed to
>> devm_fwnode_get_gpiod_from_child() in place of "label" parameter,
>> which when set to NULL, results in gpio label being initialized to '?'.
>>
>> It could be observed in debugfs and failed to properly identify
>> gpio association with LED consumer.
>>
>> Fix this shortcoming by updating the GPIO label after the LED is
>> registered and its final name is known.
>>
>> Fixes: d7235f5feaa0 ("leds: gpio: Use generic support for composing LED names")
>> Cc: Linus Walleij <linus.walleij@linaro.org>
>> Cc: Pavel Machek <pavel@ucw.cz>
>> Cc: Russell King <linux@armlinux.org.uk>
>> Signed-off-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
> 
> Patch looks good, except:
> 
>> @@ -151,9 +151,14 @@ static struct gpio_leds_priv *gpio_leds_create(struct platform_device *pdev)
>>  		struct gpio_led led = {};
>>  		const char *state = NULL;
>>  
>> +		/**
>> +		 * Acquire gpiod from DT with uninitialized label, which
>> +		 * will be updated after LED class device is registered,
>> +		 * Only then the final LED name is known.
>> +		 */
>>  		led.gpiod = devm_fwnode_get_gpiod_from_child(dev, NULL, child,
>>  							     GPIOD_ASIS,
>> -							     led.name);
>> +							     NULL);
> 
> This is not linuxdoc, so comment should beging with /* AFAICT.

Right.

> I'll probably hand-edit the patch.

Sure, thanks.

-- 
Best regards,
Jacek Anaszewski

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-05 21:25 [PATCH] leds: gpio: Fix uninitialized gpio label for fwnode based probe Jacek Anaszewski
2019-12-11  0:44 ` Linus Walleij
2019-12-21 18:49 ` Pavel Machek
2019-12-21 20:17   ` Jacek Anaszewski

Linux-LEDs Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-leds/0 linux-leds/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-leds linux-leds/ https://lore.kernel.org/linux-leds \
		linux-leds@vger.kernel.org
	public-inbox-index linux-leds

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-leds


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git