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

[PATCH v3 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.

Because the layout of the register is very different from the previously
supported variants, this patch follows a similar pattern present in the
driver for paths that deviated from the norm: the chipdef can define a
pointer to an extra function that (if non-NULL) will be called during
probing. This was already done for reset_func and for sw_shutdown_func
in order to support other chips in the same family but here, if the
function is defined, the original initialization code will not run. This
is because, in the previously existing cases, the lack of a register
meant the generic code could be skipped, while here the registers exist,
but differ in function.

The patches keep the previous behavior of using only the PWM registers
to control the state of the LEDs. For that reason, the current output
is always set to the maximum value.

Signed-off-by: Flavio Amieiro <flavio@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: 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: linux-kernel@vger.kernel.org
Cc: devicetree@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                     | 95 ++++++++++++++++++----
 2 files changed, 80 insertions(+), 16 deletions(-)
---
base-commit: 05b4738b0078f7d6f154f68068a11c8a0635e9df
change-id: 20261012-add-is31fl3206-support-d557f863a97a

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

[PATCH v3 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.

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

Re: [PATCH v3 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.
> 
> Signed-off-by: Flavio Amieiro <flavio@amieiro.net>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261013-add-is31fl3206-support-v3-0-66a762a3c12f@amieiro.net?part=1

[PATCH v3 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.

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 following the previous limitation
of the other variants: 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 | 95 +++++++++++++++++++++++++++++++++++-------
 1 file changed, 79 insertions(+), 16 deletions(-)

diff --git a/drivers/leds/leds-is31fl32xx.c b/drivers/leds/leds-is31fl32xx.c
index 285085d27453..a22d814be4b0 100644
--- a/drivers/leds/leds-is31fl32xx.c
+++ b/drivers/leds/leds-is31fl32xx.c
@@ -49,6 +49,9 @@
 #define IS31FL3293_RESET_MAGIC 0xc5
 #define IS31FL3293_MAX_MICROAMP 20000
 
+/* Custom LED Control enable value for IS31FL3206 */
+#define IS31FL3206_CONTROL_ENABLE_WITH_IMAX BIT(4)
+
 struct is31fl32xx_priv;
 struct is31fl32xx_led_data {
 	struct led_classdev cdev;
@@ -79,6 +82,7 @@ struct is31fl32xx_priv {
  * @enable_bits_per_led_control_register: number of LEDs enable bits in each
  * @brightness_steps    : number of brightness steps supported by the chip
  * @reset_func          : pointer to reset function
+ * @enable_all_channels_func  : pointer to enable all channels function
  * @sw_shutdown_func    : pointer to software shutdown function
  *
  * For all optional register addresses, the sentinel value %IS31FL32XX_REG_NONE
@@ -88,6 +92,11 @@ struct is31fl32xx_priv {
  * necessary registers to a known initialization state. This is needed
  * for chips that do not have a @reset_reg.
  *
+ * If non-NULL, @enable_all_channels_func will be used in place of the
+ * generic channel enable function. This is needed for chips that have
+ * a LED Control Register that control more than simply the
+ * enable/disable state of the channel.
+ *
  * @enable_bits_per_led_control_register must be >=1 if
  * @led_control_register_base != %IS31FL32XX_REG_NONE.
  */
@@ -105,6 +114,7 @@ struct is31fl32xx_chipdef {
 	u8	enable_bits_per_led_control_register;
 	u16	brightness_steps;
 	int (*reset_func)(struct is31fl32xx_priv *priv);
+	int (*enable_all_channels_func)(struct is31fl32xx_priv *priv);
 	int (*sw_shutdown_func)(struct is31fl32xx_priv *priv, bool enable);
 };
 
@@ -228,6 +238,28 @@ static int is31fl3293_software_shutdown(struct is31fl32xx_priv *priv,
 	return is31fl32xx_write(priv, IS31FL3293_SHUTDOWN_REG, value);
 }
 
+/*
+ * Custom enable_all_channels function for IS31FL3206 because this
+ * variant has only one bit in the higher nibble for enable, and the
+ * lower nibble controls output current.
+ */
+static int is31fl3206_enable_all_channels(struct is31fl32xx_priv *priv)
+{
+	const struct is31fl32xx_chipdef *cdef = priv->cdef;
+	u8 num_regs = cdef->channels / cdef->enable_bits_per_led_control_register;
+	int i, ret;
+
+	for (i = 0; i < num_regs; i++) {
+		ret = is31fl32xx_write(priv,
+				       cdef->led_control_register_base+i,
+				       IS31FL3206_CONTROL_ENABLE_WITH_IMAX);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
 /*
  * NOTE: A mutex is not needed in this function because:
  * - All referenced data is read-only after probe()
@@ -315,6 +347,33 @@ static int is31fl32xx_reset_regs(struct is31fl32xx_priv *priv)
 	return 0;
 }
 
+static int is31fl32xx_enable_all_channels(struct is31fl32xx_priv *priv)
+{
+	const struct is31fl32xx_chipdef *cdef = priv->cdef;
+	int ret;
+
+	if (cdef->enable_all_channels_func)
+		return cdef->enable_all_channels_func(priv);
+
+	if (cdef->led_control_register_base != IS31FL32XX_REG_NONE) {
+		u8 value =
+			GENMASK(cdef->enable_bits_per_led_control_register-1, 0);
+		u8 num_regs = cdef->channels /
+			cdef->enable_bits_per_led_control_register;
+		int i;
+
+		for (i = 0; i < num_regs; i++) {
+			ret = is31fl32xx_write(priv,
+					       cdef->led_control_register_base+i,
+					       value);
+			if (ret)
+				return ret;
+		}
+	}
+
+	return 0;
+}
+
 static int is31fl32xx_software_shutdown(struct is31fl32xx_priv *priv,
 					bool enable)
 {
@@ -345,24 +404,12 @@ static int is31fl32xx_init_regs(struct is31fl32xx_priv *priv)
 		return ret;
 
 	/*
-	 * Set enable bit for all channels.
+	 * Enable all channels.
 	 * We will control state with PWM registers alone.
 	 */
-	if (cdef->led_control_register_base != IS31FL32XX_REG_NONE) {
-		u8 value =
-		    GENMASK(cdef->enable_bits_per_led_control_register-1, 0);
-		u8 num_regs = cdef->channels /
-				cdef->enable_bits_per_led_control_register;
-		int i;
-
-		for (i = 0; i < num_regs; i++) {
-			ret = is31fl32xx_write(priv,
-					       cdef->led_control_register_base+i,
-					       value);
-			if (ret)
-				return ret;
-		}
-	}
+	ret = is31fl32xx_enable_all_channels(priv);
+	if (ret)
+		return ret;
 
 	ret = is31fl32xx_software_shutdown(priv, false);
 	if (ret)
@@ -552,7 +599,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,
+	.brightness_steps			= 256,
+	.enable_all_channels_func		= is31fl3206_enable_all_channels,
+};
+
 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 +678,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 v3 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.
> 
> 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 following the previous limitation
> of the other variants: 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. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261013-add-is31fl3206-support-v3-0-66a762a3c12f@amieiro.net?part=2