* [PATCH v5 1/5] dt-bindings: leds: is31fl32xx: convert the binding to yaml
2026-05-08 13:11 [PATCH v5 0/5] leds: Add powerdown gpio for is31fl32xx Jun Yan
@ 2026-05-08 13:11 ` Jun Yan
2026-05-08 20:49 ` sashiko-bot
2026-05-08 13:11 ` [PATCH v5 2/5] dt-bindings: leds: leds-is31fl32xx: add support for is31fl3236a Jun Yan
` (3 subsequent siblings)
4 siblings, 1 reply; 9+ messages in thread
From: Jun Yan @ 2026-05-08 13:11 UTC (permalink / raw)
To: linux-kernel, devicetree, linux-leds
Cc: lee, robh, krzk+dt, conor+dt, luccafachinetti, pzalewski, daniel,
Jun Yan
Convert leds-is31fl32xx to DT schema format.
Co-developed-by: Lucca Fachinetti <luccafachinetti@gmail.com>
Signed-off-by: Lucca Fachinetti <luccafachinetti@gmail.com>
Co-developed-by: Pawel Zalewski <pzalewski@thegoodpenguin.co.uk>
Signed-off-by: Pawel Zalewski <pzalewski@thegoodpenguin.co.uk>
Signed-off-by: Jun Yan <jerrysteve1101@gmail.com>
---
.../bindings/leds/issl,is31fl32xx.yaml | 151 ++++++++++++++++++
.../bindings/leds/leds-is31fl32xx.txt | 53 ------
2 files changed, 151 insertions(+), 53 deletions(-)
create mode 100644 Documentation/devicetree/bindings/leds/issl,is31fl32xx.yaml
delete mode 100644 Documentation/devicetree/bindings/leds/leds-is31fl32xx.txt
diff --git a/Documentation/devicetree/bindings/leds/issl,is31fl32xx.yaml b/Documentation/devicetree/bindings/leds/issl,is31fl32xx.yaml
new file mode 100644
index 000000000000..f4a6ef99b477
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/issl,is31fl32xx.yaml
@@ -0,0 +1,151 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/leds/issl,is31fl32xx.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: IS31FL32xx and Si-En SN32xx LED controller
+
+maintainers:
+ - Lucca Fachinetti <luccafachinetti@gmail.com>
+ - Pavel Machek <pavel@ucw.cz>
+ - Jun Yan <jerrysteve1101@gmail.com>
+
+description: |
+ The IS31FL32xx/SN32xx family of LED drivers are I2C devices with multiple
+ constant-current channels, each with independent 256-level PWM control.
+ Each LED is represented as a sub-node of the device.
+
+ For more product information please see the links below:
+ https://www.lumissil.com/assets/pdf/core/IS31FL3216_DS.pdf
+ https://www.lumissil.com/assets/pdf/core/IS31FL3218_DS.pdf
+ https://www.lumissil.com/assets/pdf/core/IS31FL3235_DS.pdf
+ https://www.lumissil.com/assets/pdf/core/IS31FL3236_DS.pdf
+ https://www.lumissil.com/assets/pdf/core/IS31FL3293_DS.pdf
+
+properties:
+ compatible:
+ enum:
+ - issi,is31fl3216
+ - issi,is31fl3218
+ - issi,is31fl3235
+ - issi,is31fl3236
+ - issi,is31fl3293
+ - si-en,sn3216
+ - si-en,sn3218
+
+ reg:
+ maxItems: 1
+
+ "#address-cells":
+ const: 1
+
+ "#size-cells":
+ const: 0
+
+patternProperties:
+ "^led@([1-9a-f]|1[0-9a-f]|2[0-4])$":
+ type: object
+ $ref: common.yaml#
+ unevaluatedProperties: false
+
+ properties:
+ reg:
+ description:
+ LED channel number (1..N)
+ minimum: 1
+ maximum: 36
+
+ required:
+ - reg
+
+allOf:
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - issi,is31fl3293
+ then:
+ patternProperties:
+ "^led@":
+ properties:
+ reg:
+ maximum: 3
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - issi,is31fl3216
+ - si-en,sn3216
+ then:
+ patternProperties:
+ "^led@":
+ properties:
+ reg:
+ maximum: 16
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - issi,is31fl3218
+ - si-en,sn3218
+ then:
+ patternProperties:
+ "^led@":
+ properties:
+ reg:
+ maximum: 18
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - issi,is31fl3235
+ then:
+ patternProperties:
+ "^led@":
+ properties:
+ reg:
+ maximum: 28
+
+required:
+ - compatible
+ - reg
+ - "#address-cells"
+ - "#size-cells"
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/leds/common.h>
+
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ led-controller@3c {
+ compatible = "issi,is31fl3236";
+ reg = <0x3c>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ led@1 {
+ reg = <1>;
+ color = <LED_COLOR_ID_RED>;
+ function = LED_FUNCTION_STATUS;
+ };
+
+ led@5 {
+ reg = <5>;
+ color = <LED_COLOR_ID_GREEN>;
+ function = LED_FUNCTION_POWER;
+ linux,default-trigger = "default-on";
+ };
+ };
+ };
+...
+
diff --git a/Documentation/devicetree/bindings/leds/leds-is31fl32xx.txt b/Documentation/devicetree/bindings/leds/leds-is31fl32xx.txt
deleted file mode 100644
index 7082ed186dd9..000000000000
--- a/Documentation/devicetree/bindings/leds/leds-is31fl32xx.txt
+++ /dev/null
@@ -1,53 +0,0 @@
-Binding for ISSI IS31FL32xx and Si-En SN32xx LED Drivers
-
-The IS31FL32xx/SN32xx family of LED drivers are I2C devices with multiple
-constant-current channels, each with independent 256-level PWM control.
-Each LED is represented as a sub-node of the device.
-
-Required properties:
-- compatible: one of
- issi,is31fl3236
- issi,is31fl3235
- issi,is31fl3218
- issi,is31fl3216
- issi,is31fl3293
- si-en,sn3218
- si-en,sn3216
-- reg: I2C slave address
-- address-cells : must be 1
-- size-cells : must be 0
-
-LED sub-node properties:
-- reg : LED channel number (1..N)
-- label : (optional)
- see Documentation/devicetree/bindings/leds/common.txt
-- linux,default-trigger : (optional)
- see Documentation/devicetree/bindings/leds/common.txt
-
-
-Example:
-
-is31fl3236: led-controller@3c {
- compatible = "issi,is31fl3236";
- reg = <0x3c>;
- #address-cells = <1>;
- #size-cells = <0>;
-
- led@1 {
- reg = <1>;
- label = "EB:blue:usr0";
- };
- led@2 {
- reg = <2>;
- label = "EB:blue:usr1";
- };
- ...
- led@36 {
- reg = <36>;
- label = "EB:blue:usr35";
- };
-};
-
-For more product information please see the links below:
-http://www.issi.com/US/product-analog-fxled-driver.shtml
-http://www.si-en.com/product.asp?parentid=890
--
2.53.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v5 1/5] dt-bindings: leds: is31fl32xx: convert the binding to yaml
2026-05-08 13:11 ` [PATCH v5 1/5] dt-bindings: leds: is31fl32xx: convert the binding to yaml Jun Yan
@ 2026-05-08 20:49 ` sashiko-bot
0 siblings, 0 replies; 9+ messages in thread
From: sashiko-bot @ 2026-05-08 20:49 UTC (permalink / raw)
To: Jun Yan; +Cc: devicetree, krzk+dt, conor+dt, robh
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The YAML binding filename and its `$id` use an incorrect vendor prefix (`issl` instead of `issi`).
--
commit e7636bd58d531019b888ef291c9c0d2fe4fa5e0c
Author: Jun Yan <jerrysteve1101@gmail.com>
dt-bindings: leds: is31fl32xx: convert the binding to yaml
Convert leds-is31fl32xx to DT schema format.
> diff --git a/Documentation/devicetree/bindings/leds/issl,is31fl32xx.yaml b/Documentation/devicetree/bindings/leds/issl,is31fl32xx.yaml
> new file mode 100644
> index 0000000000000..f4a6ef99b4777
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/issl,is31fl32xx.yaml
This isn't a bug, but there appears to be a typo in the new filename. The
correct vendor prefix for Integrated Silicon Solution, Inc. is issi, but the
filename uses issl.
> @@ -0,0 +1,151 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/leds/issl,is31fl32xx.yaml#
Should this $id URI also be updated to use issi instead of issl to match the
correct vendor prefix used in the compatible strings below?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260508131139.1523597-1-jerrysteve1101@gmail.com?part=1
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v5 2/5] dt-bindings: leds: leds-is31fl32xx: add support for is31fl3236a
2026-05-08 13:11 [PATCH v5 0/5] leds: Add powerdown gpio for is31fl32xx Jun Yan
2026-05-08 13:11 ` [PATCH v5 1/5] dt-bindings: leds: is31fl32xx: convert the binding to yaml Jun Yan
@ 2026-05-08 13:11 ` Jun Yan
2026-05-08 13:11 ` [PATCH v5 3/5] dt-bindings: leds: leds-is31fl32xx: Add powerdown-gpios property Jun Yan
` (2 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Jun Yan @ 2026-05-08 13:11 UTC (permalink / raw)
To: linux-kernel, devicetree, linux-leds
Cc: lee, robh, krzk+dt, conor+dt, luccafachinetti, pzalewski, daniel,
Jun Yan
Add an additional and optional control property for setting
the output PWM frequency to 22kHz that exists on is31fl3236a.
The default is 3kHz and this option puts the operational frequency
outside of the audible range.
Co-developed-by: Pawel Zalewski <pzalewski@thegoodpenguin.co.uk>
Signed-off-by: Pawel Zalewski <pzalewski@thegoodpenguin.co.uk>
Signed-off-by: Jun Yan <jerrysteve1101@gmail.com>
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
---
.../bindings/leds/issl,is31fl32xx.yaml | 24 +++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/Documentation/devicetree/bindings/leds/issl,is31fl32xx.yaml b/Documentation/devicetree/bindings/leds/issl,is31fl32xx.yaml
index f4a6ef99b477..a8ed62fd2f35 100644
--- a/Documentation/devicetree/bindings/leds/issl,is31fl32xx.yaml
+++ b/Documentation/devicetree/bindings/leds/issl,is31fl32xx.yaml
@@ -21,6 +21,7 @@ description: |
https://www.lumissil.com/assets/pdf/core/IS31FL3218_DS.pdf
https://www.lumissil.com/assets/pdf/core/IS31FL3235_DS.pdf
https://www.lumissil.com/assets/pdf/core/IS31FL3236_DS.pdf
+ https://www.lumissil.com/assets/pdf/core/IS31FL3236A_DS.pdf
https://www.lumissil.com/assets/pdf/core/IS31FL3293_DS.pdf
properties:
@@ -30,6 +31,7 @@ properties:
- issi,is31fl3218
- issi,is31fl3235
- issi,is31fl3236
+ - issi,is31fl3236a
- issi,is31fl3293
- si-en,sn3216
- si-en,sn3218
@@ -37,6 +39,12 @@ properties:
reg:
maxItems: 1
+ issi,22khz-pwm:
+ type: boolean
+ description:
+ When present, the chip's PWM will operate at ~22kHz as opposed
+ to ~3kHz to move the operating frequency out of the audible range.
+
"#address-cells":
const: 1
@@ -60,6 +68,22 @@ patternProperties:
- reg
allOf:
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - issi,is31fl3216
+ - issi,is31fl3218
+ - issi,is31fl3235
+ - issi,is31fl3236
+ - issi,is31fl3293
+ - si-en,sn3216
+ - si-en,sn3218
+ then:
+ properties:
+ issi,22khz-pwm: false
+
- if:
properties:
compatible:
--
2.53.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v5 3/5] dt-bindings: leds: leds-is31fl32xx: Add powerdown-gpios property
2026-05-08 13:11 [PATCH v5 0/5] leds: Add powerdown gpio for is31fl32xx Jun Yan
2026-05-08 13:11 ` [PATCH v5 1/5] dt-bindings: leds: is31fl32xx: convert the binding to yaml Jun Yan
2026-05-08 13:11 ` [PATCH v5 2/5] dt-bindings: leds: leds-is31fl32xx: add support for is31fl3236a Jun Yan
@ 2026-05-08 13:11 ` Jun Yan
2026-05-08 13:11 ` [PATCH v5 4/5] leds: is31fl32xx: Add powerdown pin to exit hardware shutdown mode Jun Yan
2026-05-08 13:11 ` [PATCH v5 5/5] leds: is31f132xx: Fix missing brightness_steps for is31f13236 Jun Yan
4 siblings, 0 replies; 9+ messages in thread
From: Jun Yan @ 2026-05-08 13:11 UTC (permalink / raw)
To: linux-kernel, devicetree, linux-leds
Cc: lee, robh, krzk+dt, conor+dt, luccafachinetti, pzalewski, daniel,
Jun Yan
The IS31FL32XX 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.
Add powerdown-gpios property to describe the GPIO connected to the
SDB pin of IS31FL32XX series LED controllers.
Signed-off-by: Jun Yan <jerrysteve1101@gmail.com>
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
---
.../devicetree/bindings/leds/issl,is31fl32xx.yaml | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/Documentation/devicetree/bindings/leds/issl,is31fl32xx.yaml b/Documentation/devicetree/bindings/leds/issl,is31fl32xx.yaml
index a8ed62fd2f35..1763d3a17168 100644
--- a/Documentation/devicetree/bindings/leds/issl,is31fl32xx.yaml
+++ b/Documentation/devicetree/bindings/leds/issl,is31fl32xx.yaml
@@ -45,6 +45,15 @@ properties:
When present, the chip's PWM will operate at ~22kHz as opposed
to ~3kHz to move the operating frequency out of the audible range.
+ powerdown-gpios:
+ maxItems: 1
+ description:
+ GPIO connected 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.
+
"#address-cells":
const: 1
@@ -145,6 +154,7 @@ additionalProperties: false
examples:
- |
+ #include <dt-bindings/gpio/gpio.h>
#include <dt-bindings/leds/common.h>
i2c {
@@ -157,6 +167,8 @@ examples:
#address-cells = <1>;
#size-cells = <0>;
+ powerdown-gpios = <&gpio0 11 GPIO_ACTIVE_LOW>;
+
led@1 {
reg = <1>;
color = <LED_COLOR_ID_RED>;
--
2.53.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v5 4/5] leds: is31fl32xx: Add powerdown pin to exit hardware shutdown mode
2026-05-08 13:11 [PATCH v5 0/5] leds: Add powerdown gpio for is31fl32xx Jun Yan
` (2 preceding siblings ...)
2026-05-08 13:11 ` [PATCH v5 3/5] dt-bindings: leds: leds-is31fl32xx: Add powerdown-gpios property Jun Yan
@ 2026-05-08 13:11 ` Jun Yan
2026-05-08 21:15 ` sashiko-bot
2026-05-08 13:11 ` [PATCH v5 5/5] leds: is31f132xx: Fix missing brightness_steps for is31f13236 Jun Yan
4 siblings, 1 reply; 9+ messages in thread
From: Jun Yan @ 2026-05-08 13:11 UTC (permalink / raw)
To: linux-kernel, devicetree, linux-leds
Cc: lee, robh, krzk+dt, conor+dt, luccafachinetti, pzalewski, daniel,
Jun Yan
The IS31FL32XX 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.
Drive the pin logic low during probe() to exit hardware shutdown
mode.
Signed-off-by: Jun Yan <jerrysteve1101@gmail.com>
---
drivers/leds/leds-is31fl32xx.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/leds/leds-is31fl32xx.c b/drivers/leds/leds-is31fl32xx.c
index fe07acbb103a..57d779e07a62 100644
--- a/drivers/leds/leds-is31fl32xx.c
+++ b/drivers/leds/leds-is31fl32xx.c
@@ -10,6 +10,7 @@
*/
#include <linux/device.h>
+#include <linux/gpio.h>
#include <linux/i2c.h>
#include <linux/kernel.h>
#include <linux/leds.h>
@@ -60,6 +61,7 @@ struct is31fl32xx_led_data {
struct is31fl32xx_priv {
const struct is31fl32xx_chipdef *cdef;
struct i2c_client *client;
+ struct gpio_desc *powerdown_gpio;
unsigned int num_leds;
struct is31fl32xx_led_data leds[];
};
@@ -421,6 +423,11 @@ static int is31fl32xx_parse_dt(struct device *dev,
const struct is31fl32xx_chipdef *cdef = priv->cdef;
int ret = 0;
+ priv->powerdown_gpio = devm_gpiod_get_optional(dev, "powerdown", GPIOD_OUT_LOW);
+ if (IS_ERR(priv->powerdown_gpio))
+ return dev_err_probe(dev, PTR_ERR(priv->powerdown_gpio),
+ "Failed to get powerdown gpio\n");
+
if ((cdef->output_frequency_setting_reg != IS31FL32XX_REG_NONE) &&
of_property_read_bool(dev_of_node(dev), "issi,22khz-pwm")) {
--
2.53.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v5 4/5] leds: is31fl32xx: Add powerdown pin to exit hardware shutdown mode
2026-05-08 13:11 ` [PATCH v5 4/5] leds: is31fl32xx: Add powerdown pin to exit hardware shutdown mode Jun Yan
@ 2026-05-08 21:15 ` sashiko-bot
0 siblings, 0 replies; 9+ messages in thread
From: sashiko-bot @ 2026-05-08 21:15 UTC (permalink / raw)
To: Jun Yan; +Cc: robh, krzk+dt, conor+dt, devicetree
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Missing stabilization delay after exiting hardware shutdown.
- [Medium] Device is not placed back into hardware shutdown on driver removal.
- [Low] Incorrect header included for the descriptor-based GPIO API.
--
commit 0d566feaa3d3c1a7340af532307bd3b794d71799
Author: Jun Yan <jerrysteve1101@gmail.com>
leds: is31fl32xx: Add powerdown pin to exit hardware shutdown mode
This commit adds support for the SDB shutdown pin on the IS31FL32XX series
LED controllers. It requests the "powerdown" GPIO during probe and drives it
low to exit hardware shutdown mode so the chip can be initialized.
> diff --git a/drivers/leds/leds-is31fl32xx.c b/drivers/leds/leds-is31fl32xx.c
> index fe07acbb103a8..57d779e07a624 100644
> --- a/drivers/leds/leds-is31fl32xx.c
> +++ b/drivers/leds/leds-is31fl32xx.c
> @@ -10,6 +10,7 @@
> */
>
> #include <linux/device.h>
> +#include <linux/gpio.h>
This isn't a bug, but since this patch uses the modern descriptor-based GPIO
API (devm_gpiod_get_optional and GPIOD_OUT_LOW), should this include
<linux/gpio/consumer.h> instead of the legacy <linux/gpio.h>?
[ ... ]
> @@ -421,6 +423,11 @@ static int is31fl32xx_parse_dt(struct device *dev,
> const struct is31fl32xx_chipdef *cdef = priv->cdef;
> int ret = 0;
>
> + priv->powerdown_gpio = devm_gpiod_get_optional(dev, "powerdown", GPIOD_OUT_LOW);
> + if (IS_ERR(priv->powerdown_gpio))
> + return dev_err_probe(dev, PTR_ERR(priv->powerdown_gpio),
> + "Failed to get powerdown gpio\n");
> +
> if ((cdef->output_frequency_setting_reg != IS31FL32XX_REG_NONE) &&
> of_property_read_bool(dev_of_node(dev), "issi,22khz-pwm")) {
>
> ret = is31fl32xx_write(priv, cdef->output_frequency_setting_reg,
Does the hardware require a stabilization delay after the powerdown pin is
driven low?
Hardware LED controllers typically require some time, often around 400us to
1ms, for their internal oscillator to stabilize before the I2C interface
becomes responsive.
If we initiate I2C communications immediately after asserting the GPIO, could
the initial I2C transaction fail with a NACK and cause the probe to abort on
physical hardware?
Also, a general question about the lifecycle of the powerdown pin:
The commit message notes this feature is used to exit hardware shutdown mode
for power saving. Does the device need to be placed back into hardware shutdown
during driver removal?
While the devres framework automatically frees the GPIO descriptor upon driver
unbind, it only releases the reference and does not alter the physical state
of the pin. Would it make sense to explicitly assert the shutdown state in the
remove function to ensure the device actually powers down when the driver is
unloaded?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260508131139.1523597-1-jerrysteve1101@gmail.com?part=4
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v5 5/5] leds: is31f132xx: Fix missing brightness_steps for is31f13236
2026-05-08 13:11 [PATCH v5 0/5] leds: Add powerdown gpio for is31fl32xx Jun Yan
` (3 preceding siblings ...)
2026-05-08 13:11 ` [PATCH v5 4/5] leds: is31fl32xx: Add powerdown pin to exit hardware shutdown mode Jun Yan
@ 2026-05-08 13:11 ` Jun Yan
2026-05-08 21:28 ` sashiko-bot
4 siblings, 1 reply; 9+ messages in thread
From: Jun Yan @ 2026-05-08 13:11 UTC (permalink / raw)
To: linux-kernel, devicetree, linux-leds
Cc: lee, robh, krzk+dt, conor+dt, luccafachinetti, pzalewski, daniel,
Jun Yan
Add missing brightness_steps for is31fl3236 to fix brightness control.
Fixes: a18983b95a61 ("leds: is31f132xx: Add support for is31fl3293")
Signed-off-by: Jun Yan <jerrysteve1101@gmail.com>
---
drivers/leds/leds-is31fl32xx.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/leds/leds-is31fl32xx.c b/drivers/leds/leds-is31fl32xx.c
index 57d779e07a62..750f8d899513 100644
--- a/drivers/leds/leds-is31fl32xx.c
+++ b/drivers/leds/leds-is31fl32xx.c
@@ -488,6 +488,7 @@ static const struct is31fl32xx_chipdef is31fl3236_cdef = {
.pwm_register_base = 0x01,
.led_control_register_base = 0x26,
.enable_bits_per_led_control_register = 1,
+ .brightness_steps = 256,
};
static const struct is31fl32xx_chipdef is31fl3236a_cdef = {
--
2.53.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v5 5/5] leds: is31f132xx: Fix missing brightness_steps for is31f13236
2026-05-08 13:11 ` [PATCH v5 5/5] leds: is31f132xx: Fix missing brightness_steps for is31f13236 Jun Yan
@ 2026-05-08 21:28 ` sashiko-bot
0 siblings, 0 replies; 9+ messages in thread
From: sashiko-bot @ 2026-05-08 21:28 UTC (permalink / raw)
To: Jun Yan; +Cc: conor+dt, robh, devicetree, krzk+dt
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The commit being fixed (a18983b95a61) broke the driver's lockless design by introducing multi-register PWM updates.
- [High] The commit being fixed (a18983b95a61) introduced an initialization order regression where hardware is reset after LED class devices are registered, wiping their default states.
- [Low] The commit subject contains typos in both the driver and chip names ('1' instead of 'l').
--
commit 368d92722167efb41b11021742d19656c04d1740
Author: Jun Yan <jerrysteve1101@gmail.com>
leds: is31f132xx: Fix missing brightness_steps for is31f13236
This commit adds the missing brightness_steps configuration for the
is31fl3236 chip to restore proper brightness control.
> leds: is31f132xx: Fix missing brightness_steps for is31f13236
This isn't a bug, but there's a typo in the subject line where the number
'1' is used instead of the letter 'l' in the driver and chip names
(is31f132xx and is31f13236).
> Add missing brightness_steps for is31fl3236 to fix brightness control.
>
> Fixes: a18983b95a61 ("leds: is31f132xx: Add support for is31fl3293")
Since this patch fixes commit a18983b95a61, are there a couple of other
regressions introduced by that commit that also need addressing?
The driver lacks a lock for brightness updates, relying on the assumption
that intervening update register writes from other channels are harmless.
While this was true for older 8-bit chips that use a single PWM write, the
IS31FL3293 requires two writes (LSB and MSB) per brightness update.
If two LED channels update concurrently from different per-LED workqueue
threads, an update register write from one channel could interleave between
the LSB and MSB writes of the other channel.
Could this cause the hardware to latch a torn brightness value?
By swapping the order of is31fl32xx_parse_dt() and is31fl32xx_init_regs()
in the probe function in commit a18983b95a61, the driver now registers the
LED class devices before initializing the hardware.
During registration, the subsystem evaluates properties like default-state
and sets the initial brightness via I2C writes. However, immediately after
device tree parsing, is31fl32xx_init_regs() is invoked and triggers a
hardware reset, which silently wipes out the PWM values applied during
registration.
Does this initialization order wipe out the default states entirely?
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260508131139.1523597-1-jerrysteve1101@gmail.com?part=5
^ permalink raw reply [flat|nested] 9+ messages in thread