[PATCH] [v2] leds: gpio: make legacy gpiolib interface optional

[PATCH] [v2] leds: gpio: make legacy gpiolib interface optional

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

There are still a handful of ancient mips/armv5/sh boards that use the
gpio_led:gpio member to pass an old-style gpio number, but all modern
users have been converted to gpio descriptors.

While the CONFIG_GPIOLIB_LEGACY option that guards devm_gpio_request_one()
and related helpers is currently turned on in all kernel builds,
the plan is to only enable it on the few platforms that actually
pass gpio numbers in any platform_data.

Split out the legacy portion of the platform_data handling into a custom
helper function that is guarded with in #ifdef block, to allow the
the leds-gpio driver to compile cleanly when CONFIG_GPIOLIB_LEGACY
gets turned off. Once the last user is converted, this function can
be removed.

Link: https://lore.kernel.org/all/e9252384-a55c-4a91-9c61-06e05a0b2ce4@app.fastmail.com/
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
v2: rework a little bit to keep the legacy code path more separate,
    extend changelog description

Related to this, we may also want to remove support for passing
a gpio descriptor in the ->gpiod flag. The only user doing this
at the moment was introduced in commit 1892e87a3e91 ("powerpc/warp:
switch to using gpiod API").

Linus Walleij previously gave a Reviewed-by tag, but I dropped
it again during the rework.
---
 drivers/leds/leds-gpio.c | 48 +++++++++++++++++++++++++++++-----------
 include/linux/leds.h     |  2 ++
 2 files changed, 37 insertions(+), 13 deletions(-)

diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c
index 961acc18d0ac..788d9ef7720a 100644
--- a/drivers/leds/leds-gpio.c
+++ b/drivers/leds/leds-gpio.c
@@ -212,7 +212,6 @@ static struct gpio_desc *gpio_led_get_gpiod(struct device *dev, int idx,
 					    const struct gpio_led *template)
 {
 	struct gpio_desc *gpiod;
-	int ret;
 
 	/*
 	 * This means the LED does not come from the device tree
@@ -228,11 +227,27 @@ static struct gpio_desc *gpio_led_get_gpiod(struct device *dev, int idx,
 		return gpiod;
 	}
 
-	/*
-	 * This is the legacy code path for platform code that
-	 * still uses GPIO numbers. Ultimately we would like to get
-	 * rid of this block completely.
-	 */
+	return gpiod;
+}
+
+#ifdef CONFIG_GPIOLIB_LEGACY
+/*
+ * This is the legacy code path for platform code that still uses
+ * GPIO numbers, mainly MIPS and SuperH board files.
+ * Ultimately we would like to get rid of this block completely.
+ *
+ * ppc44x-warp sets the template->gpiod directly instead of
+ * adding a lookup table or device properties. This is not
+ * much better.
+ */
+static struct gpio_desc *gpio_led_get_legacy_gpiod(struct device *dev, int idx,
+						   const struct gpio_led *template)
+{
+	struct gpio_desc *gpiod;
+	int ret;
+
+	if (template->gpiod)
+		return template->gpiod;
 
 	/* skip leds that aren't available */
 	if (!gpio_is_valid(template->gpio))
@@ -252,6 +267,13 @@ static struct gpio_desc *gpio_led_get_gpiod(struct device *dev, int idx,
 
 	return gpiod;
 }
+#else
+static struct gpio_desc *gpio_led_get_legacy_gpiod(struct device *dev, int idx,
+						   const struct gpio_led *template)
+{
+	return template->gpiod;
+}
+#endif
 
 static int gpio_led_probe(struct platform_device *pdev)
 {
@@ -270,14 +292,14 @@ static int gpio_led_probe(struct platform_device *pdev)
 			const struct gpio_led *template = &pdata->leds[i];
 			struct gpio_led_data *led_dat = &priv->leds[i];
 
-			if (template->gpiod)
-				led_dat->gpiod = template->gpiod;
-			else
-				led_dat->gpiod =
-					gpio_led_get_gpiod(dev, i, template);
+			led_dat->gpiod = gpio_led_get_gpiod(dev, i, template);
+			if (!led_dat->gpiod)
+				led_dat->gpiod = gpio_led_get_legacy_gpiod(dev,
+								  i, template);
+
 			if (IS_ERR(led_dat->gpiod)) {
-				dev_info(dev, "Skipping unavailable LED gpio %d (%s)\n",
-					 template->gpio, template->name);
+				dev_info(dev, "Skipping unavailable LED gpio %s\n",
+					 template->name);
 				continue;
 			}
 
diff --git a/include/linux/leds.h b/include/linux/leds.h
index b16b803cc1ac..e646bffcd8e7 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -676,8 +676,10 @@ typedef int (*gpio_blink_set_t)(struct gpio_desc *desc, int state,
 struct gpio_led {
 	const char *name;
 	const char *default_trigger;
+#ifdef CONFIG_GPIOLIB_LEGACY
 	unsigned 	gpio;
 	unsigned	active_low : 1;
+#endif
 	unsigned	retain_state_suspended : 1;
 	unsigned	panic_indicator : 1;
 	unsigned	default_state : 2;
-- 
2.39.5

Re: [PATCH] [v2] leds: gpio: make legacy gpiolib interface optional

[Bartosz Golaszewski]

On Thu, 30 Apr 2026 11:11:55 +0200, Arnd Bergmann <arnd@kernel.org> said:
> From: Arnd Bergmann <arnd@arndb.de>
>
> There are still a handful of ancient mips/armv5/sh boards that use the
> gpio_led:gpio member to pass an old-style gpio number, but all modern
> users have been converted to gpio descriptors.
>
> While the CONFIG_GPIOLIB_LEGACY option that guards devm_gpio_request_one()
> and related helpers is currently turned on in all kernel builds,
> the plan is to only enable it on the few platforms that actually
> pass gpio numbers in any platform_data.
>
> Split out the legacy portion of the platform_data handling into a custom
> helper function that is guarded with in #ifdef block, to allow the
> the leds-gpio driver to compile cleanly when CONFIG_GPIOLIB_LEGACY
> gets turned off. Once the last user is converted, this function can
> be removed.
>
> Link: https://lore.kernel.org/all/e9252384-a55c-4a91-9c61-06e05a0b2ce4@app.fastmail.com/
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---

Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>

Re: [PATCH] [v2] leds: gpio: make legacy gpiolib interface optional

[Andy Shevchenko]

On Thu, Apr 30, 2026 at 11:11:55AM +0200, Arnd Bergmann wrote:

> There are still a handful of ancient mips/armv5/sh boards that use the
> gpio_led:gpio member to pass an old-style gpio number, but all modern
> users have been converted to gpio descriptors.
> 
> While the CONFIG_GPIOLIB_LEGACY option that guards devm_gpio_request_one()
> and related helpers is currently turned on in all kernel builds,
> the plan is to only enable it on the few platforms that actually
> pass gpio numbers in any platform_data.
> 
> Split out the legacy portion of the platform_data handling into a custom
> helper function that is guarded with in #ifdef block, to allow the
> the leds-gpio driver to compile cleanly when CONFIG_GPIOLIB_LEGACY
> gets turned off. Once the last user is converted, this function can
> be removed.

...

>  		return gpiod;
>  	}
>  
> -	/*
> -	 * This is the legacy code path for platform code that
> -	 * still uses GPIO numbers. Ultimately we would like to get
> -	 * rid of this block completely.
> -	 */
> +	return gpiod;

Do we need to repeat the upper `return gpiod;` statement? With this split
I don't see that we need to have two repetitive return statements.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] [v2] leds: gpio: make legacy gpiolib interface optional

[Arnd Bergmann]

On Mon, May 4, 2026, at 09:31, Andy Shevchenko wrote:
> On Thu, Apr 30, 2026 at 11:11:55AM +0200, Arnd Bergmann wrote:
>> -	/*
>> -	 * This is the legacy code path for platform code that
>> -	 * still uses GPIO numbers. Ultimately we would like to get
>> -	 * rid of this block completely.
>> -	 */
>> +	return gpiod;
>
> Do we need to repeat the upper `return gpiod;` statement? With this split
> I don't see that we need to have two repetitive return statements.

Right, I've simplified this now to


static struct gpio_desc *gpio_led_get_gpiod(struct device *dev, int idx,
                                           const struct gpio_led *template)
{
       struct gpio_desc *gpiod;

       gpiod = devm_gpiod_get_index_optional(dev, NULL, idx, GPIOD_OUT_LOW);
       if (gpiod && !IS_ERR(gpiod));
               gpiod_set_consumer_name(gpiod, template->name);

       return gpiod;
}

which still keeps the existing behavior but is a bit more compact.

I think we can actually just remove that function altogether
and just pass the name into devm_gpiod_get_index_optional()
from the caller like

  gpiod = devm_gpiod_get_index_optional(dev, template->name, i, GPIOD_OUT_LOW);

Did I get that right? If so, I'll fold that in as another
simplification.

     Arnd

Re: [PATCH] [v2] leds: gpio: make legacy gpiolib interface optional

[Andy Shevchenko]

On Tue, May 05, 2026 at 03:10:28PM +0200, Arnd Bergmann wrote:
> On Mon, May 4, 2026, at 09:31, Andy Shevchenko wrote:
> > On Thu, Apr 30, 2026 at 11:11:55AM +0200, Arnd Bergmann wrote:
> >> -	/*
> >> -	 * This is the legacy code path for platform code that
> >> -	 * still uses GPIO numbers. Ultimately we would like to get
> >> -	 * rid of this block completely.
> >> -	 */
> >> +	return gpiod;
> >
> > Do we need to repeat the upper `return gpiod;` statement? With this split
> > I don't see that we need to have two repetitive return statements.
> 
> Right, I've simplified this now to
> 
> static struct gpio_desc *gpio_led_get_gpiod(struct device *dev, int idx,
>                                            const struct gpio_led *template)
> {
>        struct gpio_desc *gpiod;
> 
>        gpiod = devm_gpiod_get_index_optional(dev, NULL, idx, GPIOD_OUT_LOW);

>        if (gpiod && !IS_ERR(gpiod));

And this is not needed. The below is NULL-aware.

>                gpiod_set_consumer_name(gpiod, template->name);
> 
>        return gpiod;
> }
> 
> which still keeps the existing behavior but is a bit more compact.
> 
> I think we can actually just remove that function altogether
> and just pass the name into devm_gpiod_get_index_optional()
> from the caller like
> 
>   gpiod = devm_gpiod_get_index_optional(dev, template->name, i, GPIOD_OUT_LOW);
> 
> Did I get that right? If so, I'll fold that in as another
> simplification.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] [v2] leds: gpio: make legacy gpiolib interface optional

[Andy Shevchenko]

On Tue, May 05, 2026 at 04:19:36PM +0300, Andy Shevchenko wrote:
> On Tue, May 05, 2026 at 03:10:28PM +0200, Arnd Bergmann wrote:
> > On Mon, May 4, 2026, at 09:31, Andy Shevchenko wrote:
> > > On Thu, Apr 30, 2026 at 11:11:55AM +0200, Arnd Bergmann wrote:

...

> > >> +	return gpiod;
> > >
> > > Do we need to repeat the upper `return gpiod;` statement? With this split
> > > I don't see that we need to have two repetitive return statements.
> > 
> > Right, I've simplified this now to
> > 
> > static struct gpio_desc *gpio_led_get_gpiod(struct device *dev, int idx,
> >                                            const struct gpio_led *template)
> > {
> >        struct gpio_desc *gpiod;
> > 
> >        gpiod = devm_gpiod_get_index_optional(dev, NULL, idx, GPIOD_OUT_LOW);
> 
> >        if (gpiod && !IS_ERR(gpiod));
> 
> And this is not needed. The below is NULL-aware.
> 
> >                gpiod_set_consumer_name(gpiod, template->name);
> > 
> >        return gpiod;
> > }

To be clear

        struct gpio_desc *gpiod;

        gpiod = devm_gpiod_get_index_optional(dev, NULL, idx, GPIOD_OUT_LOW);
        if (!IS_ERR(gpiod))
                gpiod_set_consumer_name(gpiod, template->name);

        return gpiod;

But looking at the original code, I would leave another return, so

	gpiod = devm_gpiod_get_index_optional(dev, NULL, idx, GPIOD_OUT_LOW);
	if (IS_ERR(gpiod))
		return gpiod;

	gpiod_set_consumer_name(gpiod, template->name);
	return gpiod;

> > which still keeps the existing behavior but is a bit more compact.
> > 
> > I think we can actually just remove that function altogether
> > and just pass the name into devm_gpiod_get_index_optional()
> > from the caller like
> > 
> >   gpiod = devm_gpiod_get_index_optional(dev, template->name, i, GPIOD_OUT_LOW);
> > 
> > Did I get that right? If so, I'll fold that in as another

Nope, the con_id != consumer name. Can't be done this way.

> > simplification.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] [v2] leds: gpio: make legacy gpiolib interface optional

[Arnd Bergmann]

On Tue, May 5, 2026, at 15:25, Andy Shevchenko wrote:
> On Tue, May 05, 2026 at 04:19:36PM +0300, Andy Shevchenko wrote:
>> On Tue, May 05, 2026 at 03:10:28PM +0200, Arnd Bergmann wrote:
>> > On Mon, May 4, 2026, at 09:31, Andy Shevchenko wrote:
>> >        struct gpio_desc *gpiod;
>> > 
>> >        gpiod = devm_gpiod_get_index_optional(dev, NULL, idx, GPIOD_OUT_LOW);
>> 
>> >        if (gpiod && !IS_ERR(gpiod));
>> 
>> And this is not needed. The below is NULL-aware.
>
> To be clear
>
>         struct gpio_desc *gpiod;
>
>         gpiod = devm_gpiod_get_index_optional(dev, NULL, idx, GPIOD_OUT_LOW);
>         if (!IS_ERR(gpiod))
>                 gpiod_set_consumer_name(gpiod, template->name);
>
>         return gpiod;
>
> But looking at the original code, I would leave another return, so

Right, I actually had this at first, but decided to keep the NULL
check because that was used in the driver already.

I've dropped it again now.

>> >   gpiod = devm_gpiod_get_index_optional(dev, template->name, i, GPIOD_OUT_LOW);
>> > 
>> > Did I get that right? If so, I'll fold that in as another
>
> Nope, the con_id != consumer name. Can't be done this way.

I see. I had tried to find an existing interface that sets
the consumer name and saw that gpiod_get_index() internally
uses con_id as the label if set, but I missed that this would
break the actual lookup in gpio_desc_table_match() because the
13 lookup tables are of course all defined by index rather than
by name.

      Arnd

Re: [PATCH] [v2] leds: gpio: make legacy gpiolib interface optional

[Linus Walleij]

On Tue, May 5, 2026 at 4:21 PM Arnd Bergmann <arnd@arndb.de> wrote:

> >> >   gpiod = devm_gpiod_get_index_optional(dev, template->name, i, GPIOD_OUT_LOW);
> >> >
> >> > Did I get that right? If so, I'll fold that in as another
> >
> > Nope, the con_id != consumer name. Can't be done this way.
>
> I see. I had tried to find an existing interface that sets
> the consumer name and saw that gpiod_get_index() internally
> uses con_id as the label if set, but I missed that this would
> break the actual lookup in gpio_desc_table_match() because the
> 13 lookup tables are of course all defined by index rather than
> by name.

Oh there is
gpiod_set_consumer_name(gpiod, "name");
that you can call unconditionally to set the consumer name.
It's NULL-tolerant.

Yours,
Linus Walleij

Re: [PATCH] [v2] leds: gpio: make legacy gpiolib interface optional

[Linus Walleij]

On Thu, Apr 30, 2026 at 11:12 AM Arnd Bergmann <arnd@kernel.org> wrote:

> From: Arnd Bergmann <arnd@arndb.de>
>
> There are still a handful of ancient mips/armv5/sh boards that use the
> gpio_led:gpio member to pass an old-style gpio number, but all modern
> users have been converted to gpio descriptors.
>
> While the CONFIG_GPIOLIB_LEGACY option that guards devm_gpio_request_one()
> and related helpers is currently turned on in all kernel builds,
> the plan is to only enable it on the few platforms that actually
> pass gpio numbers in any platform_data.
>
> Split out the legacy portion of the platform_data handling into a custom
> helper function that is guarded with in #ifdef block, to allow the
> the leds-gpio driver to compile cleanly when CONFIG_GPIOLIB_LEGACY
> gets turned off. Once the last user is converted, this function can
> be removed.
>
> Link: https://lore.kernel.org/all/e9252384-a55c-4a91-9c61-06e05a0b2ce4@app.fastmail.com/
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> v2: rework a little bit to keep the legacy code path more separate,
>     extend changelog description
>
> Related to this, we may also want to remove support for passing
> a gpio descriptor in the ->gpiod flag. The only user doing this
> at the moment was introduced in commit 1892e87a3e91 ("powerpc/warp:
> switch to using gpiod API").
>
> Linus Walleij previously gave a Reviewed-by tag, but I dropped
> it again during the rework.

You can have it back :)
Reviewed-by: Linus Walleij <linusw@kernel.org>

Yours,
Linus Walleij