[PATCH v2 0/6] leds: is31fl319x: Fix shutdown GPIO and update DT bindings

[PATCH v2 0/6] leds: is31fl319x: Fix shutdown GPIO and update DT bindings

[Jun Yan]

This series fixes incorrect shutdown GPIO polarity and improves
shutdown-gpios description in DT bindings.

Patch 1 adds detailed shutdown-gpios description and fixes example
in DT bindings.

Patch 2 fixes shutdown GPIO initial state and removes redundant
startup toggling.

Patch 3 adds fixup to force consumer active low for legacy device 
trees in gpiolib.

Patches 4-6 correct shutdown GPIO polarity in board DTS files.

Changes in v2:
- Add comments explaining the reason for this GPIO line level.
- Add fixup to force consumer active low for legacy device trees in gpiolib.
- Link to v1: https://lore.kernel.org/all/20260508152435.21389-1-jerrysteve1101@gmail.com

Jun Yan (6):
  dt-bindings: leds: issi,is31fl319x: Update description for the
    shutdown-gpios property
  leds: is31fl319x: Fix shutdown GPIO initial state and remove redundant
    startup pulse
  gpiolib: of: add quirk for IS31FL319X shutdown line
  arm64: dts: qcom: msm8916-alcatel-idol347: Fix sn3190 shutdown GPIO
    polarity
  ARM: dts: qcom: msm8974-oneplus-bacon: Fix sn3193 shutdown GPIO
    polarity
  arm64: dts: marvell: armada-7040-mochabin: Fix is31fl3199 shutdown
    GPIO polarity

 .../devicetree/bindings/leds/issi,is31fl319x.yaml     |  9 +++++++--
 .../boot/dts/qcom/qcom-msm8974pro-oneplus-bacon.dts   |  2 +-
 arch/arm64/boot/dts/marvell/armada-7040-mochabin.dts  |  2 +-
 arch/arm64/boot/dts/qcom/msm8916-alcatel-idol347.dts  |  2 +-
 drivers/gpio/gpiolib-of.c                             | 10 ++++++++++
 drivers/leds/leds-is31fl319x.c                        | 11 ++++-------
 6 files changed, 24 insertions(+), 12 deletions(-)

-- 
2.54.0

[PATCH v2 1/6] dt-bindings: leds: issi,is31fl319x: Update description for the shutdown-gpios property

[Jun Yan]

The IS31FL319X series features an SDB shutdown pin.
Driving it low (active low) places the chip into hardware shutdown
mode for power saving, while all register contents are preserved
and registers are not reset.

Update description for the shutdown down (SDB) pin and fix the example
device tree binding.

Fixes: dbc801b472c1 ("dt-bindings: leds: Convert is31fl319x to dtschema")
Signed-off-by: Jun Yan <jerrysteve1101@gmail.com>
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
---
 .../devicetree/bindings/leds/issi,is31fl319x.yaml        | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/leds/issi,is31fl319x.yaml b/Documentation/devicetree/bindings/leds/issi,is31fl319x.yaml
index 906735acfbaf..ebf2bed51e7f 100644
--- a/Documentation/devicetree/bindings/leds/issi,is31fl319x.yaml
+++ b/Documentation/devicetree/bindings/leds/issi,is31fl319x.yaml
@@ -39,7 +39,12 @@ properties:
 
   shutdown-gpios:
     maxItems: 1
-    description: GPIO attached to the SDB pin.
+    description: |
+      GPIO attached to the chip's SDB pin.
+      Driving this GPIO low places the chip into hardware shutdown mode
+      for power saving. All register contents are preserved and registers
+      are not reset during shutdown. The chip exits hardware shutdown mode
+      when the SDB pin is pulled high.
 
   audio-gain-db:
     default: 0
@@ -174,7 +179,7 @@ examples:
             #address-cells = <1>;
             #size-cells = <0>;
 
-            shutdown-gpios = <&gpio0 11 GPIO_ACTIVE_HIGH>;
+            shutdown-gpios = <&gpio0 11 GPIO_ACTIVE_LOW>;
 
             led@1 {
                 reg = <1>;
-- 
2.54.0

[PATCH v2 2/6] leds: is31fl319x: Fix shutdown GPIO initial state and remove redundant startup pulse

[Jun Yan]

1. Per IS31FL319x datasheet [1], the hardware shutdown pin (SDB) is
active-low.

Fix incorrect initial GPIO level to properly release the chip from shutdown
state.

2. According to datasheet [1] definition:
  Shutdown mode can either be used as a means of
  reducing power consumption or generating a flashing
  display (repeatedly entering and leaving shutdown
  mode). During shutdown mode all registers retain their
  data.
shutdown mode does NOT perform chip reset.A dedicated software reset is
already implemented in driver by writing 0 to reset chip.

Remove redundant unnecessary toggling of the shutdown GPIO.

[1] https://lumissil.com/assets/pdf/core/IS31FL3193_DS.pdf

Fixes: dddb4e38c6ba ("leds: is31fl319x: Add shutdown pin and generate a 5ms low pulse when startup")
Signed-off-by: Jun Yan <jerrysteve1101@gmail.com>
---
 drivers/leds/leds-is31fl319x.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/leds/leds-is31fl319x.c b/drivers/leds/leds-is31fl319x.c
index e411cee06dab..41087ed262cb 100644
--- a/drivers/leds/leds-is31fl319x.c
+++ b/drivers/leds/leds-is31fl319x.c
@@ -396,7 +396,10 @@ static int is31fl319x_parse_fw(struct device *dev, struct is31fl319x_chip *is31)
 	int count;
 	int ret;
 
-	is31->shutdown_gpio = devm_gpiod_get_optional(dev, "shutdown", GPIOD_OUT_HIGH);
+	/* Driving this GPIO line low (in fact high) takes the chip out of shutdown,
+	 * as it is flagged as GPIO_ACTIVE_LOW in provider (such as the device tree).
+	 */
+	is31->shutdown_gpio = devm_gpiod_get_optional(dev, "shutdown", GPIOD_OUT_LOW);
 	if (IS_ERR(is31->shutdown_gpio))
 		return dev_err_probe(dev, PTR_ERR(is31->shutdown_gpio),
 				     "Failed to get shutdown gpio\n");
@@ -506,12 +509,6 @@ static int is31fl319x_probe(struct i2c_client *client)
 	if (err)
 		return err;
 
-	if (is31->shutdown_gpio) {
-		gpiod_direction_output(is31->shutdown_gpio, 0);
-		mdelay(5);
-		gpiod_direction_output(is31->shutdown_gpio, 1);
-	}
-
 	is31->client = client;
 	is31->regmap = devm_regmap_init_i2c(client, is31->cdef->is31fl319x_regmap_config);
 	if (IS_ERR(is31->regmap))
-- 
2.54.0

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

[Jun Yan]

According to the IS31FL319x datasheet[1], the SDB pin is active‑low.
However, existing device tree incorrectly configure it as active‑high.

Add a fixup to force the consumer active low for legacy device trees.

[1] https://lumissil.com/assets/pdf/core/IS31FL3196_DS.pdf

Signed-off-by: Jun Yan <jerrysteve1101@gmail.com>
---
 drivers/gpio/gpiolib-of.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index ef1ac68b94b7..8a79aaadc9ea 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.
+		 */
+		{ "issi,is31fl3199",	"shutdown-gpios",	false },
+		{ "si-en,sn3190",	"shutdown-gpios",	false },
+		{ "si-en,sn3193",	"shutdown-gpios",	false },
+#endif
 #if IS_ENABLED(CONFIG_MTD_NAND_JZ4780)
 		/*
 		 * The rb-gpios semantics was undocumented and qi,lb60 (along with
-- 
2.54.0

[PATCH v2 4/6] arm64: dts: qcom: msm8916-alcatel-idol347: Fix sn3190 shutdown GPIO polarity

[Jun Yan]

The sn3190 shutdown pin is active-low [1]. Correct the GPIO flags
from GPIO_ACTIVE_HIGH to GPIO_ACTIVE_LOW to match the hardware.

[1] https://lumissil.com/assets/pdf/core/IS31FL3190_DS.pdf

Fixes: 1c8cc183d070 ("arm64: dts: qcom: msm8916-alcatel-idol347: add LED indicator")
Signed-off-by: Jun Yan <jerrysteve1101@gmail.com>
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
---
 arch/arm64/boot/dts/qcom/msm8916-alcatel-idol347.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/msm8916-alcatel-idol347.dts b/arch/arm64/boot/dts/qcom/msm8916-alcatel-idol347.dts
index 2de8b6f9531b..6d51d6efcbac 100644
--- a/arch/arm64/boot/dts/qcom/msm8916-alcatel-idol347.dts
+++ b/arch/arm64/boot/dts/qcom/msm8916-alcatel-idol347.dts
@@ -197,7 +197,7 @@ &blsp_i2c6 {
 	led-controller@68 {
 		compatible = "si-en,sn3190";
 		reg = <0x68>;
-		shutdown-gpios = <&tlmm 89 GPIO_ACTIVE_HIGH>;
+		shutdown-gpios = <&tlmm 89 GPIO_ACTIVE_LOW>;
 		pinctrl-names = "default";
 		pinctrl-0 = <&led_enable_default &led_shutdown_default>;
 		#address-cells = <1>;
-- 
2.54.0

[PATCH v2 5/6] ARM: dts: qcom: msm8974-oneplus-bacon: Fix sn3193 shutdown GPIO polarity

[Jun Yan]

The sn3193 shutdown pin is active-low[1]. Correct the GPIO flags
from GPIO_ACTIVE_HIGH to GPIO_ACTIVE_LOW to match the hardware.

[1] https://lumissil.com/assets/pdf/core/IS31FL3193_DS.pdf

Fixes: 724ba6751532 ("ARM: dts: Move .dts files to vendor sub-directories")
Signed-off-by: Jun Yan <jerrysteve1101@gmail.com>
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
---
 arch/arm/boot/dts/qcom/qcom-msm8974pro-oneplus-bacon.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/qcom/qcom-msm8974pro-oneplus-bacon.dts b/arch/arm/boot/dts/qcom/qcom-msm8974pro-oneplus-bacon.dts
index 258bbbecd927..c528d42bade5 100644
--- a/arch/arm/boot/dts/qcom/qcom-msm8974pro-oneplus-bacon.dts
+++ b/arch/arm/boot/dts/qcom/qcom-msm8974pro-oneplus-bacon.dts
@@ -147,7 +147,7 @@ led-controller@68 {
 		compatible = "si-en,sn3193";
 		reg = <0x68>;
 
-		shutdown-gpios = <&tlmm 45 GPIO_ACTIVE_HIGH>;
+		shutdown-gpios = <&tlmm 45 GPIO_ACTIVE_LOW>;
 
 		#address-cells = <1>;
 		#size-cells = <0>;
-- 
2.54.0

[PATCH v2 6/6] arm64: dts: marvell: armada-7040-mochabin: Fix is31fl3199 shutdown GPIO polarity

[Jun Yan]

The is31fl3199 shutdown pin is active-low [1]. Correct the GPIO flags
from GPIO_ACTIVE_HIGH to GPIO_ACTIVE_LOW to match the hardware.

[1] https://lumissil.com/assets/pdf/core/IS31FL3199_DS.pdf

Fixes: 737929191283 ("arm64: dts: marvell: add Globalscale MOCHAbin")
Signed-off-by: Jun Yan <jerrysteve1101@gmail.com>
---
 arch/arm64/boot/dts/marvell/armada-7040-mochabin.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/marvell/armada-7040-mochabin.dts b/arch/arm64/boot/dts/marvell/armada-7040-mochabin.dts
index 6bdc4f1e6939..cf690a85cc2a 100644
--- a/arch/arm64/boot/dts/marvell/armada-7040-mochabin.dts
+++ b/arch/arm64/boot/dts/marvell/armada-7040-mochabin.dts
@@ -236,7 +236,7 @@ leds@64 {
 		#size-cells = <0>;
 		pinctrl-names = "default";
 		pinctrl-0 = <&is31_sdb_pins>;
-		shutdown-gpios = <&cp0_gpio1 30 GPIO_ACTIVE_HIGH>;
+		shutdown-gpios = <&cp0_gpio1 30 GPIO_ACTIVE_LOW>;
 		reg = <0x64>;
 
 		led1_red: led@1 {
-- 
2.54.0

Re: [PATCH v2 2/6] leds: is31fl319x: Fix shutdown GPIO initial state and remove redundant startup pulse

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Changing the shutdown GPIO initialization to `GPIOD_OUT_LOW` breaks backwards compatibility with existing device trees that use `GPIO_ACTIVE_HIGH`.
- [High] Removing the 5ms startup pulse on the shutdown GPIO eliminates a critical hardware stability workaround necessary for I2C recovery.
--

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

leds: is31fl319x: Fix shutdown GPIO initial state and remove redundant startup pulse

This commit alters the initial state of the shutdown GPIO from OUT_HIGH
to OUT_LOW. It also removes the 5ms toggle of the shutdown GPIO during
driver probe, relying instead on the existing software reset via I2C.

Link: https://lumissil.com/assets/pdf/core/IS31FL3193_DS.pdf

> diff --git a/drivers/leds/leds-is31fl319x.c b/drivers/leds/leds-is31fl319x.c
> index e411cee06dabd..41087ed262cb4 100644
> --- a/drivers/leds/leds-is31fl319x.c
> +++ b/drivers/leds/leds-is31fl319x.c
> @@ -396,7 +396,10 @@ static int is31fl319x_parse_fw(struct device *dev, struct is31fl319x_chip *is31)
>  	int count;
>  	int ret;
>  
> -	is31->shutdown_gpio = devm_gpiod_get_optional(dev, "shutdown", GPIOD_OUT_HIGH);
> +	/* Driving this GPIO line low (in fact high) takes the chip out of shutdown,
> +	 * as it is flagged as GPIO_ACTIVE_LOW in provider (such as the device tree).
> +	 */
> +	is31->shutdown_gpio = devm_gpiod_get_optional(dev, "shutdown", GPIOD_OUT_LOW);

[Severity: High]
Does this break backwards compatibility with existing device trees that
configured the shutdown pin as GPIO_ACTIVE_HIGH?

Legacy device trees mapping logical 1 (GPIOD_OUT_HIGH) to a physical HIGH
signal will now drive the physical pin LOW when GPIOD_OUT_LOW is used. This
keeps the chip permanently in shutdown.

While a subsequent gpiolib commit in this series attempts to add a polarity
quirk, that quirk only covers 3 specific compatibles (issi,is31fl3199,
si-en,sn3190, si-en,sn3193). The driver supports 10 distinct compatibles,
so out-of-tree device trees using the other 7 (like issi,is31fl3193 or
si-en,sn3196) will bypass the quirk and fail to probe.

[ ... ]

> @@ -506,12 +509,6 @@ static int is31fl319x_probe(struct i2c_client *client)
>  	if (err)
>  		return err;
>  
> -	if (is31->shutdown_gpio) {
> -		gpiod_direction_output(is31->shutdown_gpio, 0);
> -		mdelay(5);
> -		gpiod_direction_output(is31->shutdown_gpio, 1);
> -	}
> -

[Severity: High]
Does removing this hardware toggle prevent recovery of a wedged I2C bus?

The commit message states the toggle is redundant because a software reset
is performed via I2C later. However, the original commit adding this pulse
(dddb4e38c6ba) noted it was to make the chip more stable in complex EM
environments.

If EMI causes the chip's I2C state machine to become stuck (e.g., holding
SDA low), the subsequent software reset command via regmap_write() will
fail because the bus is unresponsive. Toggling the hardware shutdown pin
acts as a hardware reset for the I2C interface. Without this pulse, the
driver loses its ability to recover a hung I2C interface, which could lead
to probe failures with a no response from chip write error.

>  	is31->client = client;
>  	is31->regmap = devm_regmap_init_i2c(client, is31->cdef->is31fl319x_regmap_config);
>  	if (IS_ERR(is31->regmap))

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

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

[sashiko-bot]

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

Re: [PATCH v2 5/6] ARM: dts: qcom: msm8974-oneplus-bacon: Fix sn3193 shutdown GPIO polarity

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The `Fixes` tag in the commit message incorrectly points to the commit that moved the DTS file rather than the one that introduced the issue.
--

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

ARM: dts: qcom: msm8974-oneplus-bacon: Fix sn3193 shutdown GPIO polarity

This commit corrects the sn3193 shutdown GPIO polarity from GPIO_ACTIVE_HIGH
to GPIO_ACTIVE_LOW to match the hardware specification.

> The sn3193 shutdown pin is active-low[1]. Correct the GPIO flags
> from GPIO_ACTIVE_HIGH to GPIO_ACTIVE_LOW to match the hardware.
> 
> [1] https://lumissil.com/assets/pdf/core/IS31FL3193_DS.pdf
> 
> Fixes: 724ba6751532 ("ARM: dts: Move .dts files to vendor sub-directories")

[Severity: Medium]
Does this Fixes tag point to the correct commit? Commit 724ba6751532 appears
to be a structural tree reorganization that just moved the DTS file. 

The led-controller@68 node with the faulty GPIO_ACTIVE_HIGH flag seems to
have been originally introduced in commit 3835f9a0cf56 ("ARM: dts: qcom:
msm8974-oneplus-bacon: Add notification LED").

Pointing to a file-move commit might break automated backporting to stable
kernels.

> Signed-off-by: Jun Yan <jerrysteve1101@gmail.com>
> Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

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

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

[Bartosz Golaszewski]

On Mon, 25 May 2026 16:46:09 +0200, Jun Yan <jerrysteve1101@gmail.com> said:
> According to the IS31FL319x datasheet[1], the SDB pin is active‑low.
> However, existing device tree incorrectly configure it as active‑high.
>
> Add a fixup to force the consumer active low for legacy device trees.
>
> [1] https://lumissil.com/assets/pdf/core/IS31FL3196_DS.pdf
>
> Signed-off-by: Jun Yan <jerrysteve1101@gmail.com>
> ---
>  drivers/gpio/gpiolib-of.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
> index ef1ac68b94b7..8a79aaadc9ea 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.
> +		 */
> +		{ "issi,is31fl3199",	"shutdown-gpios",	false },
> +		{ "si-en,sn3190",	"shutdown-gpios",	false },
> +		{ "si-en,sn3193",	"shutdown-gpios",	false },
> +#endif
>  #if IS_ENABLED(CONFIG_MTD_NAND_JZ4780)
>  		/*
>  		 * The rb-gpios semantics was undocumented and qi,lb60 (along with
> --
> 2.54.0
>
>

Acked-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>

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

[Linus Walleij]

On Mon, May 25, 2026 at 4:47 PM Jun Yan <jerrysteve1101@gmail.com> wrote:

> According to the IS31FL319x datasheet[1], the SDB pin is active‑low.
> However, existing device tree incorrectly configure it as active‑high.
>
> Add a fixup to force the consumer active low for legacy device trees.
>
> [1] https://lumissil.com/assets/pdf/core/IS31FL3196_DS.pdf
>
> Signed-off-by: Jun Yan <jerrysteve1101@gmail.com>

Reviewed-by: Linus Walleij <linusw@kernel.org>

Yours,
Linus Walleij

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

[Linus Walleij]

On Mon, May 25, 2026 at 5:42 PM <sashiko-bot@kernel.org> wrote:

> 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.

Wrong: you did not look at the whole patch series, just this one in
isolation.

> - [Low] Use of non-ASCII characters (non-breaking hyphens) in source code comments.

I wonder if that is because the commit message was created
using AI...

Jun, please use Assisted-by tags, because these help to evolve
the LLM kernel reviews if the commit message was assisted
by AI.
https://github.com/torvalds/linux/blob/master/Documentation/process/coding-assistants.rst

Yours,
Linus Walleij

Re: [PATCH v2 6/6] arm64: dts: marvell: armada-7040-mochabin: Fix is31fl3199 shutdown GPIO polarity

[Gregory CLEMENT]

Hello,

> The is31fl3199 shutdown pin is active-low [1]. Correct the GPIO flags
> from GPIO_ACTIVE_HIGH to GPIO_ACTIVE_LOW to match the hardware.
>
> [1] https://lumissil.com/assets/pdf/core/IS31FL3199_DS.pdf
>
> Fixes: 737929191283 ("arm64: dts: marvell: add Globalscale MOCHAbin")
> Signed-off-by: Jun Yan <jerrysteve1101@gmail.com>

What happens if this patch is merged before the driver LED patch? Will
it break something?

Gregory

> ---
>  arch/arm64/boot/dts/marvell/armada-7040-mochabin.dts | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/marvell/armada-7040-mochabin.dts b/arch/arm64/boot/dts/marvell/armada-7040-mochabin.dts
> index 6bdc4f1e6939..cf690a85cc2a 100644
> --- a/arch/arm64/boot/dts/marvell/armada-7040-mochabin.dts
> +++ b/arch/arm64/boot/dts/marvell/armada-7040-mochabin.dts
> @@ -236,7 +236,7 @@ leds@64 {
>  		#size-cells = <0>;
>  		pinctrl-names = "default";
>  		pinctrl-0 = <&is31_sdb_pins>;
> -		shutdown-gpios = <&cp0_gpio1 30 GPIO_ACTIVE_HIGH>;
> +		shutdown-gpios = <&cp0_gpio1 30 GPIO_ACTIVE_LOW>;
>  		reg = <0x64>;
>  
>  		led1_red: led@1 {
> -- 
> 2.54.0
>

-- 
Grégory CLEMENT, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH v2 2/6] leds: is31fl319x: Fix shutdown GPIO initial state and remove redundant startup pulse

[Krzysztof Kozlowski]

On 25/05/2026 16:46, Jun Yan wrote:
> 1. Per IS31FL319x datasheet [1], the hardware shutdown pin (SDB) is
> active-low.
> 
> Fix incorrect initial GPIO level to properly release the chip from shutdown
> state.
> 
> 2. According to datasheet [1] definition:
>   Shutdown mode can either be used as a means of
>   reducing power consumption or generating a flashing
>   display (repeatedly entering and leaving shutdown
>   mode). During shutdown mode all registers retain their
>   data.
> shutdown mode does NOT perform chip reset.A dedicated software reset is
> already implemented in driver by writing 0 to reset chip.
> 
> Remove redundant unnecessary toggling of the shutdown GPIO.
> 
> [1] https://lumissil.com/assets/pdf/core/IS31FL3193_DS.pdf
> 
> Fixes: dddb4e38c6ba ("leds: is31fl319x: Add shutdown pin and generate a 5ms low pulse when startup")
> Signed-off-by: Jun Yan <jerrysteve1101@gmail.com>
> ---
>  drivers/leds/leds-is31fl319x.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/leds/leds-is31fl319x.c b/drivers/leds/leds-is31fl319x.c
> index e411cee06dab..41087ed262cb 100644
> --- a/drivers/leds/leds-is31fl319x.c
> +++ b/drivers/leds/leds-is31fl319x.c
> @@ -396,7 +396,10 @@ static int is31fl319x_parse_fw(struct device *dev, struct is31fl319x_chip *is31)
>  	int count;
>  	int ret;
>  
> -	is31->shutdown_gpio = devm_gpiod_get_optional(dev, "shutdown", GPIOD_OUT_HIGH);
> +	/* Driving this GPIO line low (in fact high) takes the chip out of shutdown,
> +	 * as it is flagged as GPIO_ACTIVE_LOW in provider (such as the device tree).
> +	 */
> +	is31->shutdown_gpio = devm_gpiod_get_optional(dev, "shutdown", GPIOD_OUT_LOW);

That's ABI break.

Best regards,
Krzysztof

Re: [PATCH v2 6/6] arm64: dts: marvell: armada-7040-mochabin: Fix is31fl3199 shutdown GPIO polarity

[Krzysztof Kozlowski]

On 01/06/2026 08:58, Gregory CLEMENT wrote:
> Hello,
> 
>> The is31fl3199 shutdown pin is active-low [1]. Correct the GPIO flags
>> from GPIO_ACTIVE_HIGH to GPIO_ACTIVE_LOW to match the hardware.
>>
>> [1] https://lumissil.com/assets/pdf/core/IS31FL3199_DS.pdf
>>
>> Fixes: 737929191283 ("arm64: dts: marvell: add Globalscale MOCHAbin")
>> Signed-off-by: Jun Yan <jerrysteve1101@gmail.com>
> 
> What happens if this patch is merged before the driver LED patch? Will
> it break something?
> 

What if this patch is merged by a different maintainer, not LED? What if
this  patch is merged by the same LED maintainer (not allowed, but let's
discuss) but is not squashed into LED driver?

IOW, entire set like breaking users.

Best regards,
Krzysztof