[PATCH] leds: gpio: Skip unavailable LEDs during shutdown

[PATCH] leds: gpio: Skip unavailable LEDs during shutdown

[Steve Dunnagan]

gpio_led_probe() leaves an error-valued GPIO descriptor in the LED
data when an unavailable platform-data LED is skipped.

The entry remains included in priv->num_leds, so gpio_led_shutdown()
later passes the error pointer to gpio_led_set(), producing:

gpiod_set_value: invalid GPIO (errorpointer: -ENOENT)

Skip entries with error-valued GPIO descriptors during shutdown.

Fixes: 45d4c6de4e49 ("leds: gpio: Try to lookup gpiod from device")
Assisted-by: ChatGPT:GPT-5.5-Thinking
Signed-off-by: Steve Dunnagan <sdunnaga@redhat.com>
---
 drivers/leds/leds-gpio.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c
index 8ae71c2e91e0..63cb517ef385 100644
--- a/drivers/leds/leds-gpio.c
+++ b/drivers/leds/leds-gpio.c
@@ -304,6 +304,9 @@ static void gpio_led_shutdown(struct platform_device *pdev)
 	for (i = 0; i < priv->num_leds; i++) {
 		struct gpio_led_data *led = &priv->leds[i];
 
+		if (IS_ERR(led->gpiod))
+			continue;
+
 		if (!(led->cdev.flags & LED_RETAIN_AT_SHUTDOWN))
 			gpio_led_set(&led->cdev, LED_OFF);
 	}
-- 
2.49.0

Re: [PATCH] leds: gpio: Skip unavailable LEDs during shutdown

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

Pre-existing issues:
- [High] The platform data probe path silently skips LEDs when GPIO lookup returns -EPROBE_DEFER, breaking deferred probing.
- [High] Teardown race: gpio_led_shutdown fails to disable software blink timers and active triggers, allowing concurrent LED updates during system shutdown.
- [Low] The driver does not use dev_err_probe() to handle and report probe failures.
- [Low] The driver uses generic names 'priv' and 'gpio_leds_priv' for its private data, violating subsystem naming conventions.
--

--- Patch [1]: [PATCH] leds: gpio: Skip unavailable LEDs during shutdown ---
commit 94da8744ab857bc87f8158e7c51602a324f2f403
Author: Steve Dunnagan <sdunnaga@redhat.com>
Subject: leds: gpio: Skip unavailable LEDs during shutdown

This commit fixes an issue in gpio_led_shutdown() where an error-valued
GPIO descriptor could be passed to gpio_led_set(). It avoids a potential
invalid GPIO operation by ensuring we skip unavailable LEDs during the
shutdown process.

> diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c
> index ce4e79939731d..6a2f2386978d7 100644
> --- a/drivers/leds/leds-gpio.c
> +++ b/drivers/leds/leds-gpio.c
> @@ -290,6 +290,9 @@ static void gpio_led_shutdown(struct platform_device *pdev)
>  	for (i = 0; i < priv->num_leds; i++) {

[Severity: Low]
This is a pre-existing issue, but does the use of the generic "priv" variable
name violate subsystem naming conventions?

The LED subsystem guidelines prefer using the device name and "ddata" for
private data structures and variables rather than generic names like "priv" or
"gpio_leds_priv".

>  		struct gpio_led_data *led = &priv->leds[i];
>  
> +		if (IS_ERR(led->gpiod))
> +			continue;
> +
>  		if (!(led->cdev.flags & LED_RETAIN_AT_SHUTDOWN))
>  			gpio_led_set(&led->cdev, LED_OFF);

[Severity: High]
This isn't a bug introduced by this patch, but does gpio_led_shutdown() fail
to disable software blink timers and active triggers?

While the physical GPIO is turned off here, if a heartbeat or timer trigger
fires during the device shutdown phase (while interrupts are still active), it
could queue a work to change the LED brightness. If the underlying GPIO
controller or sleepable bus is already shut down, could this concurrent access
cause a crash or bus lockup?

>  	}

While reviewing this file, I noticed a couple of other pre-existing issues in
gpio_led_probe().

[Severity: High]
This is a pre-existing issue, but does gpio_led_probe() silently swallow
probe deferrals?

drivers/leds/leds-gpio.c:gpio_led_probe() {
    ...
			if (IS_ERR(led_dat->gpiod)) {
				dev_info(&pdev->dev, "Skipping unavailable LED gpio %d (%s)\n",
					 template->gpio, template->name);
				continue;
			}
    ...
}

If the GPIO lookup returns -EPROBE_DEFER (for example, if the controller
probes later), this catch-all check logs a generic message and skips the LED
instead of propagating the error. Will this cause the LED to be permanently
missing since the kernel won't re-attempt probing?

[Severity: Low]
This is another pre-existing issue, but should gpio_led_probe() use
dev_err_probe() to handle failures?

drivers/leds/leds-gpio.c:gpio_led_probe() {
    ...
		priv = gpio_leds_create(pdev);
		if (IS_ERR(priv))
			return PTR_ERR(priv);
    ...
}

Errors originating from gpio_leds_create() are returned directly. Subsystem
guidelines recommend always using dev_err_probe() to report probe failures
and correctly handle -EPROBE_DEFER.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260710150841.19669-1-sdunnaga@redhat.com?part=1

Re: [PATCH] leds: gpio: Skip unavailable LEDs during shutdown

[Lee Jones]

On Fri, 10 Jul 2026, Steve Dunnagan wrote:

> gpio_led_probe() leaves an error-valued GPIO descriptor in the LED
> data when an unavailable platform-data LED is skipped.

So why not nip that in the bud and fix that instead?

> The entry remains included in priv->num_leds, so gpio_led_shutdown()
> later passes the error pointer to gpio_led_set(), producing:
> 
> gpiod_set_value: invalid GPIO (errorpointer: -ENOENT)
> 
> Skip entries with error-valued GPIO descriptors during shutdown.
> 
> Fixes: 45d4c6de4e49 ("leds: gpio: Try to lookup gpiod from device")
> Assisted-by: ChatGPT:GPT-5.5-Thinking
> Signed-off-by: Steve Dunnagan <sdunnaga@redhat.com>
> ---
>  drivers/leds/leds-gpio.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c
> index 8ae71c2e91e0..63cb517ef385 100644
> --- a/drivers/leds/leds-gpio.c
> +++ b/drivers/leds/leds-gpio.c
> @@ -304,6 +304,9 @@ static void gpio_led_shutdown(struct platform_device *pdev)
>  	for (i = 0; i < priv->num_leds; i++) {
>  		struct gpio_led_data *led = &priv->leds[i];
>  
> +		if (IS_ERR(led->gpiod))
> +			continue;
> +
>  		if (!(led->cdev.flags & LED_RETAIN_AT_SHUTDOWN))
>  			gpio_led_set(&led->cdev, LED_OFF);
>  	}
> -- 
> 2.49.0
> 

-- 
Lee Jones

[PATCH v2] leds: gpio: Clear error pointers for skipped LEDs

[Steve Dunnagan]

gpio_led_get_gpiod() returns an error pointer when a platform-data
LED's GPIO is unavailable. gpio_led_probe() skips registration in that
case, but leaves the error pointer in led_dat->gpiod.

The skipped entry remains included in priv->num_leds. During shutdown,
gpio_led_shutdown() walks those entries and passes the error pointer to
gpio_led_set(), producing:

  gpiod_set_value: invalid GPIO (errorpointer: -ENOENT)

Clear led_dat->gpiod before skipping the LED so skipped entries do not
retain error-valued descriptors.

Fixes: 45d4c6de4e49 ("leds: gpio: Try to lookup gpiod from device")
Suggested-by: Lee Jones <lee@kernel.org>
Assisted-by: ChatGPT:GPT-5.5-Thinking
Signed-off-by: Steve Dunnagan <sdunnaga@redhat.com>
---
Changes in v2:
- Clear led_dat->gpiod in the probe skip path instead of checking for
  error-valued descriptors during shutdown, as suggested by Lee.

Tested on an Orange Pi 5 Plus running Fedora 42 with a test platform
device containing an unavailable GPIO LED. Before the fix, reboot
logged:

  gpiod_set_value: invalid GPIO (errorpointer: -ENOENT)

After the fix, the LED was still skipped during probe, but reboot
completed without the invalid-GPIO warning.

 drivers/leds/leds-gpio.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c
index 8ae71c2e91e0..8810fdcf2d77 100644
--- a/drivers/leds/leds-gpio.c
+++ b/drivers/leds/leds-gpio.c
@@ -277,6 +277,7 @@ static int gpio_led_probe(struct platform_device *pdev)
 			if (IS_ERR(led_dat->gpiod)) {
 				dev_info(dev, "Skipping unavailable LED gpio %d (%s)\n",
 					 template->gpio, template->name);
+				led_dat->gpiod = NULL;
 				continue;
 			}
 
-- 
2.55.0

Re: [PATCH v2] leds: gpio: Clear error pointers for skipped LEDs

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The driver masks -EPROBE_DEFER from GPIO fetching functions and incorrectly skips the LED instead of deferring the probe.
--

--- Patch [1]: [PATCH v2] leds: gpio: Clear error pointers for skipped LEDs ---
commit 2d09674f074f178fbc54110548c9a0816564760c
Author: Steve Dunnagan <sdunnaga@redhat.com>

leds: gpio: Clear error pointers for skipped LEDs

When an LED's GPIO is unavailable, the error pointer is left in led_dat->gpiod.
During shutdown this error pointer is passed to gpio_led_set(), producing an
invalid GPIO warning. This commit clears the error pointer to prevent the
warning.

> diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c
> index 8ae71c2e91e01..8810fdcf2d774 100644
> --- a/drivers/leds/leds-gpio.c
> +++ b/drivers/leds/leds-gpio.c
> @@ -277,6 +277,7 @@ static int gpio_led_probe(struct platform_device *pdev)
>  			if (IS_ERR(led_dat->gpiod)) {
>  				dev_info(dev, "Skipping unavailable LED gpio %d (%s)\n",
>  					 template->gpio, template->name);

[Severity: High]
This isn't a bug introduced by this patch, but does this unconditionally skip
the LED even if gpio_led_get_gpiod() returns -EPROBE_DEFER?

If the GPIO controller providing the LED pins hasn't probed yet,
gpio_led_get_gpiod() returns ERR_PTR(-EPROBE_DEFER) to gpio_led_probe(). By
catching all error pointers with IS_ERR() here and continuing, the driver
completes the probe without the deferred LED instead of actually deferring,
permanently breaking the LED's functionality.

Would it be better to use dev_err_probe() here to correctly handle
-EPROBE_DEFER?

> +				led_dat->gpiod = NULL;
>  				continue;
>  			}
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260724180412.43150-1-sdunnaga@redhat.com?part=1

Re: [PATCH v2] leds: gpio: Clear error pointers for skipped LEDs

[Linus Walleij]

On Fri, Jul 24, 2026 at 8:04 PM Steve Dunnagan <sdunnaga@redhat.com> wrote:

> gpio_led_get_gpiod() returns an error pointer when a platform-data
> LED's GPIO is unavailable. gpio_led_probe() skips registration in that
> case, but leaves the error pointer in led_dat->gpiod.
>
> The skipped entry remains included in priv->num_leds. During shutdown,
> gpio_led_shutdown() walks those entries and passes the error pointer to
> gpio_led_set(), producing:
>
>   gpiod_set_value: invalid GPIO (errorpointer: -ENOENT)
>
> Clear led_dat->gpiod before skipping the LED so skipped entries do not
> retain error-valued descriptors.
>
> Fixes: 45d4c6de4e49 ("leds: gpio: Try to lookup gpiod from device")
> Suggested-by: Lee Jones <lee@kernel.org>
> Assisted-by: ChatGPT:GPT-5.5-Thinking
> Signed-off-by: Steve Dunnagan <sdunnaga@redhat.com>

Reviewed-by: Linus Walleij <linusw@kernel.org>

Yours,
Linus Walleij

Re: (subset) [PATCH v2] leds: gpio: Clear error pointers for skipped LEDs

[Lee Jones]

On Fri, 24 Jul 2026 14:04:12 -0400, Steve Dunnagan wrote:
> gpio_led_get_gpiod() returns an error pointer when a platform-data
> LED's GPIO is unavailable. gpio_led_probe() skips registration in that
> case, but leaves the error pointer in led_dat->gpiod.
> 
> The skipped entry remains included in priv->num_leds. During shutdown,
> gpio_led_shutdown() walks those entries and passes the error pointer to
> gpio_led_set(), producing:
> 
> [...]

Applied, thanks!

[1/1] leds: gpio: Clear error pointers for skipped LEDs
      commit: 942901eeda934f1bebf2605a781155e9d6bc7f6e

--
Lee Jones [李琼斯]

Re: [PATCH v2] leds: gpio: Clear error pointers for skipped LEDs

[Steve Dunnagan]

Hi Lee, Pavel,

Just checking whether anything else is needed for this v2.

Linus Walleij replied with:

  Reviewed-by: Linus Walleij <linusw@kernel.org>

Thanks,
Steve