[PATCH v4 0/2] leds: Add support for the is31fl3206 LED driver

[PATCH v4 0/2] leds: Add support for the is31fl3206 LED driver

[Flavio Amieiro]

This patch series adds support for the is31fl3206 LED driver to the
already existing is31fl32xx driver.

The main difference between this specific variant and the others in
the series is that the LED Control Register controls not only the
enable status of the channel, but also the maximum output current for
it.

This patch allows the chipdef to define at which offset the LED enable
bits are inside the LED Control Register, so when calculating the
value to write to the register we can offset it to the correct
position in the register for ICs like the IS31FL3206.

The patches keep the behavior of the drivers for the other ICs in the
family: initializing the LEDs by enabling all of them and setting
output current to maximum and then using only the PWM registers to
control the brightness of the LEDs.

Signed-off-by: Flavio Amieiro <flavio@amieiro.net>
---
Changes in v4:
- Changes approach from pointers to functions (used in v3) to
  describing the LED control register layout.
- Link to v3: https://patch.msgid.link/20261013-add-is31fl3206-support-v3-0-66a762a3c12f@amieiro.net

Changes in v3:
- Reorganizes the commits so the dt-bindings are documented before using
  them
- Link to v2: https://patch.msgid.link/20261013-add-is31fl3206-support-v2-0-ca0d2bf5783f@amieiro.net

Changes in v2:
- Avoid double initialization when enable_all_channels is non-NULL
- Add missing entry in the i2c_device_id table
- Link to v1: https://patch.msgid.link/20261013-add-is31fl3206-support-v1-0-08c066cc241d@amieiro.net

To: Lee Jones <lee@kernel.org>
To: Pavel Machek <pavel@kernel.org>
To: Griffin Kroah-Hartman <griffin@kroah.com>
To: Rob Herring <robh@kernel.org>
To: Krzysztof Kozlowski <krzk+dt@kernel.org>
To: Conor Dooley <conor+dt@kernel.org>
Cc: linux-leds@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org

---
Flavio Amieiro (2):
      dt-bindings: leds: Add issi,is31fl3206 to leds-is31fl32xx
      leds: is31f132xx: Add support for is31fl3206

 .../devicetree/bindings/leds/leds-is31fl32xx.txt   |  1 +
 drivers/leds/leds-is31fl32xx.c                     | 25 ++++++++++++++++++++++
 2 files changed, 26 insertions(+)
---
base-commit: 59cf91cf87abb67a49b1bb7b9d3b06486abde8cf
change-id: 20261012-add-is31fl3206-support-d557f863a97a

Best regards,
--  
Flavio Amieiro <flavio@amieiro.net>

[PATCH v4 1/2] dt-bindings: leds: Add issi,is31fl3206 to leds-is31fl32xx

[Flavio Amieiro]

This is a 12 channel variant of the is31fl32xx led driver series.

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Signed-off-by: Flavio Amieiro <flavio@amieiro.net>
---
 Documentation/devicetree/bindings/leds/leds-is31fl32xx.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/leds/leds-is31fl32xx.txt b/Documentation/devicetree/bindings/leds/leds-is31fl32xx.txt
index 7082ed186dd9..06f85a1c6aaf 100644
--- a/Documentation/devicetree/bindings/leds/leds-is31fl32xx.txt
+++ b/Documentation/devicetree/bindings/leds/leds-is31fl32xx.txt
@@ -11,6 +11,7 @@ Required properties:
 	issi,is31fl3218
 	issi,is31fl3216
 	issi,is31fl3293
+	issi,is31fl3206
 	si-en,sn3218
 	si-en,sn3216
 - reg: I2C slave address

-- 
2.53.0

[PATCH v4 2/2] leds: is31f132xx: Add support for is31fl3206

[Flavio Amieiro]

This chip has a different layout for the LED Control Register when
compared to the other variants. This register contains the enable bit
(in the highest nibble) but also the current control (in the lowest
nibble).

According to the datasheet, a value of 010000 (0x10) in the register
enables an LED with the output current set to maximum.

This commit adds support for this IC allowing the chipdef to define an
offset for the enable bits inside the LED Control Register.

The limitation that already existed for other variants is still in
this one: all LEDs are enabled and set with maximum current, letting
brightness be controlled by the PWM registers.

Signed-off-by: Flavio Amieiro <flavio@amieiro.net>
---
 drivers/leds/leds-is31fl32xx.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/drivers/leds/leds-is31fl32xx.c b/drivers/leds/leds-is31fl32xx.c
index 285085d27453..586713ff7d98 100644
--- a/drivers/leds/leds-is31fl32xx.c
+++ b/drivers/leds/leds-is31fl32xx.c
@@ -77,6 +77,7 @@ struct is31fl32xx_priv {
  * @pwm_registers_reversed: : true if PWM registers count down instead of up
  * @led_control_register_base : address of first LED control register (optional)
  * @enable_bits_per_led_control_register: number of LEDs enable bits in each
+ * @enable_bits_offset_in_led_control_reg: offset of the enable bit in the LED Control Register
  * @brightness_steps    : number of brightness steps supported by the chip
  * @reset_func          : pointer to reset function
  * @sw_shutdown_func    : pointer to software shutdown function
@@ -88,6 +89,12 @@ struct is31fl32xx_priv {
  * necessary registers to a known initialization state. This is needed
  * for chips that do not have a @reset_reg.
  *
+ * @enable_bit_offset_in_led_control_reg determines the offset of the
+ * enable bits from the least significant bit of the LED control
+ * register. This is needed for chips that have a LED Control Register
+ * that control more than simply the enable/disable state of the
+ * channel such as the IS31FL3206.
+ *
  * @enable_bits_per_led_control_register must be >=1 if
  * @led_control_register_base != %IS31FL32XX_REG_NONE.
  */
@@ -103,6 +110,7 @@ struct is31fl32xx_chipdef {
 	bool	pwm_registers_reversed;
 	u8	led_control_register_base;
 	u8	enable_bits_per_led_control_register;
+	u8	enable_bits_offset_in_led_control_reg;
 	u16	brightness_steps;
 	int (*reset_func)(struct is31fl32xx_priv *priv);
 	int (*sw_shutdown_func)(struct is31fl32xx_priv *priv, bool enable);
@@ -351,6 +359,7 @@ static int is31fl32xx_init_regs(struct is31fl32xx_priv *priv)
 	if (cdef->led_control_register_base != IS31FL32XX_REG_NONE) {
 		u8 value =
 		    GENMASK(cdef->enable_bits_per_led_control_register-1, 0);
+		value = value << cdef->enable_bits_offset_in_led_control_reg;
 		u8 num_regs = cdef->channels /
 				cdef->enable_bits_per_led_control_register;
 		int i;
@@ -552,7 +561,22 @@ static const struct is31fl32xx_chipdef is31fl3293_cdef = {
 	.sw_shutdown_func			= is31fl3293_software_shutdown,
 };
 
+static const struct is31fl32xx_chipdef is31fl3206_cdef = {
+	.channels				= 12,
+	.shutdown_reg				= 0x00,
+	.pwm_update_reg			= 0x13,
+	.global_control_reg			= 0x26,
+	.reset_reg				= 0x2f,
+	.output_frequency_setting_reg		= 0x27,
+	.pwm_register_base			= 0x04,
+	.led_control_register_base		= 0x17,
+	.enable_bits_per_led_control_register	= 1,
+	.enable_bits_offset_in_led_control_reg	= 4,
+	.brightness_steps			= 256,
+};
+
 static const struct of_device_id of_is31fl32xx_match[] = {
+	{ .compatible = "issi,is31fl3206",  .data = &is31fl3206_cdef, },
 	{ .compatible = "issi,is31fl3293",  .data = &is31fl3293_cdef, },
 	{ .compatible = "issi,is31fl3236",  .data = &is31fl3236_cdef, },
 	{ .compatible = "issi,is31fl3236a", .data = &is31fl3236a_cdef, },
@@ -616,6 +640,7 @@ static void is31fl32xx_remove(struct i2c_client *client)
  * even though it is not used for DeviceTree based instantiation.
  */
 static const struct i2c_device_id is31fl32xx_id[] = {
+	{ .name = "is31fl3206" },
 	{ .name = "is31fl3293" },
 	{ .name = "is31fl3236" },
 	{ .name = "is31fl3236a" },

-- 
2.53.0

Re: [PATCH v4 1/2] dt-bindings: leds: Add issi,is31fl3206 to leds-is31fl32xx

[sashiko-bot]

> This is a 12 channel variant of the is31fl32xx led driver series.
> 
> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
> Signed-off-by: Flavio Amieiro <flavio@amieiro.net>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261028-add-is31fl3206-support-v4-0-e331cee9fcb3@amieiro.net?part=1

Re: [PATCH v4 2/2] leds: is31f132xx: Add support for is31fl3206

[sashiko-bot]

> This chip has a different layout for the LED Control Register when
> compared to the other variants. This register contains the enable bit
> (in the highest nibble) but also the current control (in the lowest
> nibble).
> 
> According to the datasheet, a value of 010000 (0x10) in the register
> enables an LED with the output current set to maximum.
> 
> This commit adds support for this IC allowing the chipdef to define an
> offset for the enable bits inside the LED Control Register.
> 
> The limitation that already existed for other variants is still in
> this one: all LEDs are enabled and set with maximum current, letting
> brightness be controlled by the PWM registers.
> 
> Signed-off-by: Flavio Amieiro <flavio@amieiro.net>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261028-add-is31fl3206-support-v4-0-e331cee9fcb3@amieiro.net?part=2

Re: [PATCH v4 2/2] leds: is31f132xx: Add support for is31fl3206

[Griffin Kroah-Hartman]

On Wed, 28 Oct 2026 18:31:02 -0300, Flavio Amieiro <flavio@amieiro.net> wrote:
> This chip has a different layout for the LED Control Register when
> compared to the other variants. This register contains the enable bit
> (in the highest nibble) but also the current control (in the lowest
> nibble).
> 
> According to the datasheet, a value of 010000 (0x10) in the register
> enables an LED with the output current set to maximum.
> 
> [...]

Reviewed-by: Griffin Kroah-Hartman <griffin@kroah.com>

-- 
 

Re: [PATCH v4 2/2] leds: is31f132xx: Add support for is31fl3206

[Lee Jones]

--- checkpatch.pl (1 issue) ---
Line 83: WARNING: DT compatible string "issi,is31fl3206" appears un-documented -- check ./Documentation/devicetree/bindings/
----------------------------------

On Wed, 28 Oct 2026, Flavio Amieiro wrote:

> This chip has a different layout for the LED Control Register when
> compared to the other variants. This register contains the enable bit
> (in the highest nibble) but also the current control (in the lowest
> nibble).
>
> According to the datasheet, a value of 010000 (0x10) in the register
> enables an LED with the output current set to maximum.
>
> This commit adds support for this IC allowing the chipdef to define an
> offset for the enable bits inside the LED Control Register.
>
> The limitation that already existed for other variants is still in
> this one: all LEDs are enabled and set with maximum current, letting
> brightness be controlled by the PWM registers.
>
> Signed-off-by: Flavio Amieiro <flavio@amieiro.net>
> ---
>  drivers/leds/leds-is31fl32xx.c | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
>
> diff --git a/drivers/leds/leds-is31fl32xx.c b/drivers/leds/leds-is31fl32xx.c
> index 285085d27453..586713ff7d98 100644
> --- a/drivers/leds/leds-is31fl32xx.c
> +++ b/drivers/leds/leds-is31fl32xx.c
> @@ -77,6 +77,7 @@ struct is31fl32xx_priv {
>   * @pwm_registers_reversed: : true if PWM registers count down instead of up
>   * @led_control_register_base : address of first LED control register (optional)
>   * @enable_bits_per_led_control_register: number of LEDs enable bits in each
> + * @enable_bits_offset_in_led_control_reg: offset of the enable bit in the LED Control Register
>   * @brightness_steps    : number of brightness steps supported by the chip
>   * @reset_func          : pointer to reset function
>   * @sw_shutdown_func    : pointer to software shutdown function
> @@ -88,6 +89,12 @@ struct is31fl32xx_priv {
>   * necessary registers to a known initialization state. This is needed
>   * for chips that do not have a @reset_reg.
>   *
> + * @enable_bit_offset_in_led_control_reg determines the offset of the
> + * enable bits from the least significant bit of the LED control

Make sure you run W=1, particularly when changing Kerneldoc.

> + * register. This is needed for chips that have a LED Control Register
> + * that control more than simply the enable/disable state of the
> + * channel such as the IS31FL3206.
> + *
>   * @enable_bits_per_led_control_register must be >=1 if
>   * @led_control_register_base != %IS31FL32XX_REG_NONE.
>   */
> @@ -103,6 +110,7 @@ struct is31fl32xx_chipdef {
>  	bool	pwm_registers_reversed;
>  	u8	led_control_register_base;
>  	u8	enable_bits_per_led_control_register;
> +	u8	enable_bits_offset_in_led_control_reg;
>  	u16	brightness_steps;
>  	int (*reset_func)(struct is31fl32xx_priv *priv);
>  	int (*sw_shutdown_func)(struct is31fl32xx_priv *priv, bool enable);
> @@ -351,6 +359,7 @@ static int is31fl32xx_init_regs(struct is31fl32xx_priv *priv)
>  	if (cdef->led_control_register_base != IS31FL32XX_REG_NONE) {
>  		u8 value =
>  		    GENMASK(cdef->enable_bits_per_led_control_register-1, 0);

Feel free to unwrap this - you can use up to 100-chars to avoid this.

> +		value = value << cdef->enable_bits_offset_in_led_control_reg;

Could we avoid mixing declarations and statements here?

Also, instead of shifting afterwards, could we just pass the offset
directly to 'GENMASK()', or use '<<='?

>  		u8 num_regs = cdef->channels /
>  				cdef->enable_bits_per_led_control_register;
>  		int i;
> @@ -552,7 +561,22 @@ static const struct is31fl32xx_chipdef is31fl3293_cdef = {
>  	.sw_shutdown_func			= is31fl3293_software_shutdown,
>  };
>
> +static const struct is31fl32xx_chipdef is31fl3206_cdef = {
> +	.channels				= 12,
> +	.shutdown_reg				= 0x00,
> +	.pwm_update_reg			= 0x13,
> +	.global_control_reg			= 0x26,
> +	.reset_reg				= 0x2f,
> +	.output_frequency_setting_reg		= 0x27,
> +	.pwm_register_base			= 0x04,
> +	.led_control_register_base		= 0x17,
> +	.enable_bits_per_led_control_register	= 1,
> +	.enable_bits_offset_in_led_control_reg	= 4,
> +	.brightness_steps			= 256,
> +};
> +
>  static const struct of_device_id of_is31fl32xx_match[] = {
> +	{ .compatible = "issi,is31fl3206",  .data = &is31fl3206_cdef, },
>  	{ .compatible = "issi,is31fl3293",  .data = &is31fl3293_cdef, },
>  	{ .compatible = "issi,is31fl3236",  .data = &is31fl3236_cdef, },
>  	{ .compatible = "issi,is31fl3236a", .data = &is31fl3236a_cdef, },
> @@ -616,6 +640,7 @@ static void is31fl32xx_remove(struct i2c_client *client)
>   * even though it is not used for DeviceTree based instantiation.
>   */
>  static const struct i2c_device_id is31fl32xx_id[] = {
> +	{ .name = "is31fl3206" },

Looks like this is going to need rebasing on -next to apply.

>  	{ .name = "is31fl3293" },
>  	{ .name = "is31fl3236" },
>  	{ .name = "is31fl3236a" },
>
> -- 
> 2.53.0
>

-- 
Lee Jones

Re: [PATCH v4 2/2] leds: is31f132xx: Add support for is31fl3206

[Flávio Amieiro]

Thank you very much for the review.

> Lee Jones <lee@kernel.org> writes:
> --- checkpatch.pl (1 issue) ---
> Line 83: WARNING: DT compatible string "issi,is31fl3206" appears un-documented -- check ./Documentation/devicetree/bindings/
> ----------------------------------
>

This is documented in the first patch of the series, as requested as
part of the review for v1[1].

> On Wed, 28 Oct 2026, Flavio Amieiro wrote:
>
>> + * @enable_bit_offset_in_led_control_reg determines the offset of the
>> + * enable bits from the least significant bit of the LED control
>
> Make sure you run W=1, particularly when changing Kerneldoc.
>

I checked and get no warnings with `make W=1
./drivers/leds/leds-is31fl32xx.o`, `./scripts/checkpatch.pl --strict`,
and `./scripts/kernel-doc -N drivers/leds/leds-is31fl32xx.c`. Am I
missing some other check here?

Again, thank you very much for the review. I will address the other
points and send a v5 as soon as possible.

Best regards,
Flávio Amieiro


[1]: https://lore.kernel.org/all/20261013-jolly-salamander-of-tact-7cf46e@quoll/

Re: [PATCH v4 2/2] leds: is31f132xx: Add support for is31fl3206

[Lee Jones]

On Fri, 13 Nov 2026, Flávio Amieiro wrote:

> 
> Thank you very much for the review.
> 
> > Lee Jones <lee@kernel.org> writes:
> > --- checkpatch.pl (1 issue) ---
> > Line 83: WARNING: DT compatible string "issi,is31fl3206" appears un-documented -- check ./Documentation/devicetree/bindings/
> > ----------------------------------
> >
> 
> This is documented in the first patch of the series, as requested as
> part of the review for v1[1].
> 
> > On Wed, 28 Oct 2026, Flavio Amieiro wrote:
> >
> >> + * @enable_bit_offset_in_led_control_reg determines the offset of the
> >> + * enable bits from the least significant bit of the LED control
> >
> > Make sure you run W=1, particularly when changing Kerneldoc.
> >
> 
> I checked and get no warnings with `make W=1
> ./drivers/leds/leds-is31fl32xx.o`, `./scripts/checkpatch.pl --strict`,
> and `./scripts/kernel-doc -N drivers/leds/leds-is31fl32xx.c`. Am I
> missing some other check here?

Ah, it's because the first one is spelt correctly.

This mention however, is not:

@enable_bit_offset_in_led_control_reg

> Again, thank you very much for the review. I will address the other
> points and send a v5 as soon as possible.
> 
> Best regards,
> Flávio Amieiro
> 
> 
> [1]: https://lore.kernel.org/all/20261013-jolly-salamander-of-tact-7cf46e@quoll/

-- 
Lee Jones

Re: [PATCH v4 2/2] leds: is31f132xx: Add support for is31fl3206

[Flávio Amieiro]

Lee Jones <lee@kernel.org> writes:
> Ah, it's because the first one is spelt correctly.
>
> This mention however, is not:
>
> @enable_bit_offset_in_led_control_reg
>

I'm sorry I missed that. I'll also fix it in v5.

Thank you!

--
Flávio Amieiro