Re: [PATCH 3/6] [v6] leds: gpio: make legacy gpiolib interface optional

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] Disabling CONFIG_GPIOLIB_LEGACY breaks compilation for unrelated non-legacy drivers that set `struct gpio_led::active_low`.
- [Low] The commit message description is not capitalized, violating the LED subsystem guidelines.
--

--- Patch [3]: [PATCH 3/6] [v6] leds: gpio: make legacy gpiolib interface optional ---
commit ad55bf4fa40ddbff55ee69fcb15b56da1f063668
Author: Arnd Bergmann <[email protected]>

> leds: gpio: make legacy gpiolib interface optional

[Severity: Low]
This isn't a bug, but should the commit description be capitalized after the
subsystem prefix to match the LED subsystem guidelines? For example, 
"leds: gpio: Make legacy..."

This commit makes the legacy GPIO library interface optional in the leds-gpio
driver by guarding the platform_data handling with CONFIG_GPIOLIB_LEGACY.

Link: https://lore.kernel.org/all/[email protected]/

[ ... ]

> diff --git a/include/linux/leds.h b/include/linux/leds.h
> index b16b803cc1ac5..e646bffcd8e74 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

[Severity: Low]
Does hiding active_low behind CONFIG_GPIOLIB_LEGACY break the build for
unrelated non-legacy drivers (such as ath10k) that still rely on setting this
field in struct gpio_led?

>  	unsigned	retain_state_suspended : 1;
>  	unsigned	panic_indicator : 1;
>  	unsigned	default_state : 2;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3