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

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

On Tue, May 05, 2026 at 05:58:48PM +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.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
There is a couple of nit-picks, but I'm not objecting if they are not
going to be addressed.

...

>  	gpiod = devm_gpiod_get_index_optional(dev, NULL, idx, GPIOD_OUT_LOW);
> -	if (IS_ERR(gpiod))
> -		return gpiod;
> -	if (gpiod) {
> +	if (!IS_ERR(gpiod))
>  		gpiod_set_consumer_name(gpiod, template->name);
> -		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;

Okay, let's stick with this form.

...

> -			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);

Why not keep the style as before:

				led_dat->gpiod =
					gpio_led_get_legacy_gpiod(dev, i, template);

? (Yes, it's a bit longer than 80, but I think in this case it's justified for
readability).

> +

This blank like is not needed.

>  			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;
>  			}

-- 
With Best Regards,
Andy Shevchenko