Re: [PATCH v2 3/6] gpiolib: of: add quirk for IS31FL319X shutdown line

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Incomplete backward compatibility quirk for IS31FL319X shutdown GPIO polarity, risking breakage for out-of-tree platforms.
- [Low] Use of non-ASCII characters (non-breaking hyphens) in source code comments.
--

commit 8f0eb876e212314197458aa41b5be24457af3ea3
Author: Jun Yan <jerrysteve1101@gmail.com>

gpiolib: of: add quirk for IS31FL319X shutdown line

This commit adds a fixup to force the consumer active low for legacy
device trees that incorrectly configure the SDB pin as active-high.

> diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
> index 2c923d17541f2..f60291dcc90f7 100644
> --- a/drivers/gpio/gpiolib-of.c
> +++ b/drivers/gpio/gpiolib-of.c
[ ... ]
> @@ -194,6 +194,16 @@ static void of_gpio_try_fixup_polarity(const struct device_node *np,
>  		{ "himax,hx8357",	"gpios-reset",	false },
>  		{ "himax,hx8369",	"gpios-reset",	false },
>  #endif
> +#if IS_ENABLED(CONFIG_LEDS_IS31FL319X)
> +		/*
> +		 * According to the IS31FL319x datasheet, the SDB pin is active‑low.
> +		 * However, existing device tree incorrectly configure it
> +		 * as active‑high.

[Severity: Low]
This isn't a bug, but are there non-ASCII non-breaking hyphens (U+2011) used
here instead of standard ASCII hyphens?

Using standard ASCII hyphens (U+002D) for "active-low" and "active-high"
helps ensure the terms can be found using standard text search tools like
git grep.

> +		 */
> +		{ "issi,is31fl3199",	"shutdown-gpios",	false },
> +		{ "si-en,sn3190",	"shutdown-gpios",	false },
> +		{ "si-en,sn3193",	"shutdown-gpios",	false },

[Severity: Medium]
Will this omission break out-of-tree platforms using legacy device trees with
the other compatible strings for this driver?

The driver supports several other compatible strings (such as "issi,is31fl3190",
"issi,is31fl3196", "si-en,sn3196") which are not listed here.

If a legacy device tree uses one of the omitted compatibles, this quirk will be
bypassed. Because the updated driver now requests the GPIO with GPIOD_OUT_LOW,
a legacy device tree specifying GPIO_ACTIVE_HIGH would map to a physical LOW
signal, which would permanently hold the chip in shutdown.

Could we list all the compatible strings for this hardware family to ensure
ABI compatibility isn't broken for other legacy users?

> +#endif
>  #if IS_ENABLED(CONFIG_MTD_NAND_JZ4780)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260525144629.498630-1-jerrysteve1101@gmail.com?part=3