drivers/leds/leds-gpio.c | 48 +++++++++++++++++++++++++++++----------- include/linux/leds.h | 2 ++ 2 files changed, 37 insertions(+), 13 deletions(-)
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.5On 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
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
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
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
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
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
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
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>
© 2016 - 2026 Red Hat, Inc.