Skip to contentSkip to conversation

leds: gpio: Fix uninitialized gpio label for fwnode based probe

PATCH In review Jacek Anaszewski 3 participants 4 messages Dec 5, 2019 – Dec 21, 2019 LED

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 leds: gpio: Fix uninitialized gpio label for fwnode based probe

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 <[email protected]>
Cc Pavel Machek <[email protected]>
Cc Russell King <[email protected]>
---
 drivers/leds/leds-gpio.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)
drivers/leds/leds-gpio.c +9 -1
index a5c73f3d5f79..43eeed096d0d 100644
@@ -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++;
 	}
 
Signature · 1 line
2.11.0
Reviewed-by on v1 1/1
#1 lore raw
Quoted 10 lines · Jacek Anaszewski

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.

Quoted 8 lines

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 <[email protected]>
Cc Pavel Machek <[email protected]>
Cc Russell King <[email protected]>

Yours,
Linus Walleij

#2 lore raw
In reply to Jacek Anaszewski

Hi!

Quoted 10 lines

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.

Quoted 8 lines

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 <[email protected]>
Cc Pavel Machek <[email protected]>
Cc Russell King <[email protected]>

Patch looks good, except:

drivers/leds/leds-gpio.c static struct gpio_leds_priv *gpio_leds_create(struct platform_device *pdev) · line 161

Quoted 13 lines
drivers/leds/leds-gpio.c +6 -1
@@ -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

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
#3 lore raw
Quoted 27 lines · Pavel Machek

Hi!

Quoted 10 lines

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.

Quoted 8 lines

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 <[email protected]>
Cc Pavel Machek <[email protected]>
Cc Russell King <[email protected]>

Patch looks good, except:

Quoted 4 lines
drivers/leds/leds-gpio.c +0 -0
@@ -151,9 +151,14 @@ static struct gpio_leds_priv *gpio_leds_create(struct platform_device *pdev)
 		struct gpio_led led = {};
 		const char *state = NULL;
 
Quoted 11 lines

drivers/leds/leds-gpio.c static struct gpio_leds_priv *gpio_leds_create(struct platform_device *pdev) · line 161

Quoted 9 lines
drivers/leds/leds-gpio.c +6 -1
+		/**
+		 * 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.

Quoted 1 line

I'll probably hand-edit the patch.

Sure, thanks.

Signature · 2 lines
Best regards,
Jacek Anaszewski