[PATCH 10/21] leds: gpio: make legacy gpiolib interface optional

Arnd Bergmann posted 21 patches 1 month, 3 weeks ago
[PATCH 10/21] leds: gpio: make legacy gpiolib interface optional
Posted by Arnd Bergmann 1 month, 3 weeks ago
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.

Make the code that deals with this optional so the legacy interfaces
can be left out for all normal builds.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/leds/leds-gpio.c | 8 ++++++--
 include/linux/leds.h     | 2 ++
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c
index a3428b22de3a..e43accfa78e9 100644
--- a/drivers/leds/leds-gpio.c
+++ b/drivers/leds/leds-gpio.c
@@ -212,7 +212,9 @@ static struct gpio_desc *gpio_led_get_gpiod(struct device *dev, int idx,
 					    const struct gpio_led *template)
 {
 	struct gpio_desc *gpiod;
+#ifdef CONFIG_GPIOLIB_LEGACY
 	int ret;
+#endif
 
 	/*
 	 * This means the LED does not come from the device tree
@@ -228,6 +230,7 @@ static struct gpio_desc *gpio_led_get_gpiod(struct device *dev, int idx,
 		return gpiod;
 	}
 
+#ifdef CONFIG_GPIOLIB_LEGACY
 	/*
 	 * This is the legacy code path for platform code that
 	 * still uses GPIO numbers. Ultimately we would like to get
@@ -244,6 +247,7 @@ static struct gpio_desc *gpio_led_get_gpiod(struct device *dev, int idx,
 		return ERR_PTR(ret);
 
 	gpiod = gpio_to_desc(template->gpio);
+#endif
 	if (!gpiod)
 		return ERR_PTR(-EINVAL);
 
@@ -276,8 +280,8 @@ static int gpio_led_probe(struct platform_device *pdev)
 				led_dat->gpiod =
 					gpio_led_get_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..034643f40152 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -676,7 +676,9 @@ 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;
+#endif
 	unsigned	active_low : 1;
 	unsigned	retain_state_suspended : 1;
 	unsigned	panic_indicator : 1;
-- 
2.39.5
Re: [PATCH 10/21] leds: gpio: make legacy gpiolib interface optional
Posted by Lee Jones 1 month, 2 weeks ago
On Fri, 08 Aug 2025, Arnd Bergmann 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.
> 
> Make the code that deals with this optional so the legacy interfaces
> can be left out for all normal builds.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/leds/leds-gpio.c | 8 ++++++--
>  include/linux/leds.h     | 2 ++
>  2 files changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c
> index a3428b22de3a..e43accfa78e9 100644
> --- a/drivers/leds/leds-gpio.c
> +++ b/drivers/leds/leds-gpio.c
> @@ -212,7 +212,9 @@ static struct gpio_desc *gpio_led_get_gpiod(struct device *dev, int idx,
>  					    const struct gpio_led *template)
>  {
>  	struct gpio_desc *gpiod;
> +#ifdef CONFIG_GPIOLIB_LEGACY
>  	int ret;
> +#endif

Isn't there another way to do his that doesn't entail sprinkling #ifery
around C-files?

>  	/*
>  	 * This means the LED does not come from the device tree
> @@ -228,6 +230,7 @@ static struct gpio_desc *gpio_led_get_gpiod(struct device *dev, int idx,
>  		return gpiod;
>  	}
>  
> +#ifdef CONFIG_GPIOLIB_LEGACY
>  	/*
>  	 * This is the legacy code path for platform code that
>  	 * still uses GPIO numbers. Ultimately we would like to get
> @@ -244,6 +247,7 @@ static struct gpio_desc *gpio_led_get_gpiod(struct device *dev, int idx,
>  		return ERR_PTR(ret);
>  
>  	gpiod = gpio_to_desc(template->gpio);
> +#endif
>  	if (!gpiod)
>  		return ERR_PTR(-EINVAL);
>  
> @@ -276,8 +280,8 @@ static int gpio_led_probe(struct platform_device *pdev)
>  				led_dat->gpiod =
>  					gpio_led_get_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..034643f40152 100644
> --- a/include/linux/leds.h
> +++ b/include/linux/leds.h
> @@ -676,7 +676,9 @@ 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;
> +#endif
>  	unsigned	active_low : 1;
>  	unsigned	retain_state_suspended : 1;
>  	unsigned	panic_indicator : 1;
> -- 
> 2.39.5
> 

-- 
Lee Jones [李琼斯]
Re: [PATCH 10/21] leds: gpio: make legacy gpiolib interface optional
Posted by Arnd Bergmann 1 month, 2 weeks ago
On Tue, Aug 19, 2025, at 14:19, Lee Jones wrote:
> On Fri, 08 Aug 2025, Arnd Bergmann wrote:
>>  {
>>  	struct gpio_desc *gpiod;
>> +#ifdef CONFIG_GPIOLIB_LEGACY
>>  	int ret;
>> +#endif
>
> Isn't there another way to do his that doesn't entail sprinkling #ifery
> around C-files?
>

An alternativew would be to duplicate the driver and have
one modern variant and an additional legacy variant that
is only used on the few remaining platforms that select CONFIG_GPIOLIB_LEGACY and define platform data. See below
for the list of files that reference struct gpio_led.

There are already patches to convert some of those to
software nodes, and a lot of the others can probably be
removed, in particular the orion5x ones.

The leds-gpio driver with just the legacy interfaces left
would be a really small driver, and removing those bits from
the normal one would make that a bit simpler as well, but
there would be some amount of duplication.

     Arnd

$ git grep -wl struct.gpio_led
arch/arm/mach-omap1/board-ams-delta.c
arch/arm/mach-omap1/board-osk.c
arch/arm/mach-orion5x/board-d2net.c
arch/arm/mach-orion5x/dns323-setup.c
arch/arm/mach-orion5x/mv2120-setup.c
arch/arm/mach-orion5x/net2big-setup.c
arch/arm/mach-orion5x/ts409-setup.c
arch/arm/mach-s3c/mach-crag6410.c
arch/arm/mach-sa1100/assabet.c
arch/mips/alchemy/board-gpr.c
arch/mips/alchemy/board-mtx1.c
arch/mips/bcm47xx/leds.c
arch/mips/include/asm/mach-bcm63xx/board_bcm963xx.h
arch/mips/loongson32/ls1b/board.c
arch/mips/txx9/generic/setup.c
arch/mips/txx9/rbtx4927/setup.c
arch/powerpc/platforms/44x/warp.c
arch/sh/boards/mach-rsk/devices-rsk7203.c
drivers/leds/leds-gpio.c
drivers/leds/simatic/simatic-ipc-leds-gpio-core.c
drivers/net/wireless/ath/ath10k/core.h
drivers/net/wireless/ath/ath10k/leds.c
drivers/platform/x86/barco-p50-gpio.c
drivers/platform/x86/intel/atomisp2/led.c
drivers/platform/x86/meraki-mx100.c
drivers/platform/x86/pcengines-apuv2.c
drivers/platform/x86/sel3350-platform.c
Re: [PATCH 10/21] leds: gpio: make legacy gpiolib interface optional
Posted by Lee Jones 1 month, 2 weeks ago
On Tue, 19 Aug 2025, Arnd Bergmann wrote:

> On Tue, Aug 19, 2025, at 14:19, Lee Jones wrote:
> > On Fri, 08 Aug 2025, Arnd Bergmann wrote:
> >>  {
> >>  	struct gpio_desc *gpiod;
> >> +#ifdef CONFIG_GPIOLIB_LEGACY
> >>  	int ret;
> >> +#endif
> >
> > Isn't there another way to do his that doesn't entail sprinkling #ifery
> > around C-files?
> >
> 
> An alternativew would be to duplicate the driver and have
> one modern variant and an additional legacy variant that
> is only used on the few remaining platforms that select CONFIG_GPIOLIB_LEGACY and define platform data. See below
> for the list of files that reference struct gpio_led.
> 
> There are already patches to convert some of those to
> software nodes, and a lot of the others can probably be
> removed, in particular the orion5x ones.
> 
> The leds-gpio driver with just the legacy interfaces left
> would be a really small driver, and removing those bits from
> the normal one would make that a bit simpler as well, but
> there would be some amount of duplication.

Sounds like we're between a rock and a hard place with this.

Will the legacy parts be removed at some point or do you foresee us
supporting this forever?

-- 
Lee Jones [李琼斯]
Re: [PATCH 10/21] leds: gpio: make legacy gpiolib interface optional
Posted by Arnd Bergmann 1 month, 2 weeks ago
On Wed, Aug 20, 2025, at 09:16, Lee Jones wrote:
> On Tue, 19 Aug 2025, Arnd Bergmann wrote:
>
> Sounds like we're between a rock and a hard place with this.

I don't think either variant is that bad to be honest, as it
gets us a long way towards removing the legacy interface from
default builds without having to update or remove the holdouts
immediately. It's mainly led-gpio and gpio-keys that need
a change like this.

Splitting out the entire gpio_led_platform_data handling
into a single #ifdef function block would be a little cleaner,
but that would in turn require changing over a couple of
files that got converted from legacy gpio numbers to passing
gpio descriptors or lookup tables (ppc44x/warp, x86/sel3350,
arm/omap1), making them use device properties instead.

> Will the legacy parts be removed at some point or do you foresee us
> supporting this forever?

It's hard to predict an timeline here, there is certainly a lot of
interest in minimizing the legacy users as much as possible and
patches are getting written for x86 and arm, but I don't see much
movement on the mips and sh platforms. These are also the ones
that have been holdouts for CONFIG_LEGACY_CLK for years, and I
hope we can eventually drop support for those boards.

That said, I first need to get my own act together and refresh
my patches to drop the old arm board files so we can merge that.

      Arnd
Re: [PATCH 10/21] leds: gpio: make legacy gpiolib interface optional
Posted by Andy Shevchenko 1 month, 2 weeks ago
On Wed, Aug 20, 2025 at 02:00:56PM +0200, Arnd Bergmann wrote:
> On Wed, Aug 20, 2025, at 09:16, Lee Jones wrote:
> > On Tue, 19 Aug 2025, Arnd Bergmann wrote:
> >
> > Sounds like we're between a rock and a hard place with this.
> 
> I don't think either variant is that bad to be honest, as it
> gets us a long way towards removing the legacy interface from
> default builds without having to update or remove the holdouts
> immediately. It's mainly led-gpio and gpio-keys that need
> a change like this.

And I believe Dmitry is working on gpio-keys this cycle to get rid of legacy
GPIO APIs.


-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH 10/21] leds: gpio: make legacy gpiolib interface optional
Posted by Andy Shevchenko 1 month, 2 weeks ago
On Wed, Aug 20, 2025 at 04:15:05PM +0300, Andy Shevchenko wrote:
> On Wed, Aug 20, 2025 at 02:00:56PM +0200, Arnd Bergmann wrote:
> > On Wed, Aug 20, 2025, at 09:16, Lee Jones wrote:
> > > On Tue, 19 Aug 2025, Arnd Bergmann wrote:
> > >
> > > Sounds like we're between a rock and a hard place with this.
> > 
> > I don't think either variant is that bad to be honest, as it
> > gets us a long way towards removing the legacy interface from
> > default builds without having to update or remove the holdouts
> > immediately. It's mainly led-gpio and gpio-keys that need
> > a change like this.
> 
> And I believe Dmitry is working on gpio-keys this cycle to get rid of legacy
> GPIO APIs.

I just realize that this might be odd, I meant Dmitry Torokhov in this context.

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH 10/21] leds: gpio: make legacy gpiolib interface optional
Posted by Linus Walleij 1 month, 2 weeks ago
On Fri, Aug 8, 2025 at 5:22 PM 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.
>
> Make the code that deals with this optional so the legacy interfaces
> can be left out for all normal builds.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

I like this, it cleans up things for current systems so they do not need
to carry around so much legacy.

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

Yours,
Linus Walleij