[PATCH v8 0/2] Add Support for LTC3220 18 Channel LED Driver

[PATCH v8 0/2] Add Support for LTC3220 18 Channel LED Driver

[Edelweise Escala]

The LTC3220/LTC3220-1 is a multi-display LED driver, which contains a
high-efficiency, low-noise charge pump to provide power to up to
18 LED current sources. The LEDs are individually configurable to
64-step linear brightness control, blinking and gradation control
via 2-wire I2C interface. The blinking and gradation configuration
is shared across all LED.

LTC3220 has a quick write function which allows changing the brightness
on all LEDS simultaneously when the brightness is changed on led 1.
For this leds are aggregated in the device tree and on probe we check
if led-sources exist to enable quick write. 
We would like to know if this approach is alright?
Another way we might want to know is, is it alright to just make a
virtual led for the quick write function. Changing brightness on 
the virtual led will change the brightness for all.

Signed-off-by: Edelweise Escala <edelweise.escala@analog.com>
---
Changes in v8:
- Add '>' on descriptions with multiple paragraph
- Change led registers to hex
- Change cache type to REGCACHE_FLAT_S
- Link to v7: https://lore.kernel.org/r/20260508-ltc3220-driver-v7-0-0f092ba54f23@analog.com

Changes in v7:
- Add cahcetype on regmap
- Dropped struct ltc3220_state *ltc3220_state and use container_of()
- Rename ltc3220_state to ltc3220
- Dropped functions only used once and used inline instead.
- Loop variable changes
- Consistent decalaration for container_of
- Tabbing fixes
- Link to v6: https://lore.kernel.org/r/20260417-ltc3220-driver-v6-0-18157871eddd@analog.com

Changes in v6:
- Fix commit message
- Add manufacturer on Kconfig and improve description
- Rearrange register map and bitmask and improve naming
- Use regmap, also use update bits of regmap to avoid unnecessary
  structs
- Alignment and spacing fixes
- Use Define for magic naumbers
- Fix blink calculation
- Add comments on aggregated LED
- Fix variable name to something more understandable like i to led_index
- Link to v5: https://lore.kernel.org/r/20260126-ltc3220-driver-v5-0-152a30e98ab7@analog.com

Changes in v5:
- Missed rename on bindings filename in MAINTAINERS file
- Link to v4: https://lore.kernel.org/linux-leds/20260126-ltc3220-driver-v4-0-c59517206c24@analog.com

Changes in v4:
- Rename leds-ltc3220.yaml to adi,ltc3220.yaml
- Add Reviewed-by: Conor Dooley <conor.dooley@microchip.com> on
  adi,ltc3220.yaml
Other V1 comments I think already addressed
- Subject commit message was already changed to match hardware
- Fixed wrapping after description
- Dropped "Bindings for" in descriptions and improved description to match hardware
- Dropped adi,ltc3220-1
- Dropped redundant description on reset-gpios
- Dropped adi,force-cpo-level
- Dropped adi,quick-write in favor of aggregated LED
- Used consistent quotes ^led@([1-9]|1[0-8])$
- Fixed wrapping on error messages

- Link to v3: https://lore.kernel.org/r/20260120-ltc3220-driver-v3-0-fef612ec4faa@analog.com

Changes in v3:
- Dropped quick-write on bindings and added aggregated led instead.
- Add aggregated led example.
- Modify quick write to check if there is aggregated led, if there is
  aggregated led enable quick write.
- Use DEFINE_SIMPLE_DEV_PM_OPS instead of SIMPLE_DEV_PM_OPS.
- Link to v2: https://lore.kernel.org/r/20260112-ltc3220-driver-v2-0-d043058fc4df@analog.com

Changes in v2:
leds-ltc3220.yaml changes
- Fix wrapping on description
- Improve description and commit messge to describe hardware
- Drop ltc3220-1
- Drop charge pump
ltc3220.c changes
- Fix wrapping
- Drop ltc3220-1
- Drop devname_mandatory
- Link to v1: https://lore.kernel.org/r/20260106-ltc3220-driver-v1-0-73601d6f1649@analog.com

---
Edelweise Escala (2):
      dt-bindings: leds: Add LTC3220 18 channel LED Driver
      leds: ltc3220: Add Support for LTC3220 18 channel LED Driver

 .../devicetree/bindings/leds/adi,ltc3220.yaml      | 121 ++++++
 MAINTAINERS                                        |   8 +
 drivers/leds/Kconfig                               |  12 +
 drivers/leds/Makefile                              |   1 +
 drivers/leds/leds-ltc3220.c                        | 413 +++++++++++++++++++++
 5 files changed, 555 insertions(+)
---
base-commit: 8856d7fe1758937ac528770f552ec58c388c255b
change-id: 20260106-ltc3220-driver-f9ab6cc9d1e4

Best regards,
-- 
Edelweise Escala <edelweise.escala@analog.com>

[PATCH v8 1/2] dt-bindings: leds: Add LTC3220 18 channel LED Driver

[Edelweise Escala]

LTC3220 is a multi-display LED driver with I2C interface.
The LTC3220 provides individual brightness control (64-step),
blinking, and gradation features for up to 18 LED outputs.

Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Edelweise Escala <edelweise.escala@analog.com>
---
 .../devicetree/bindings/leds/adi,ltc3220.yaml      | 121 +++++++++++++++++++++
 MAINTAINERS                                        |   7 ++
 2 files changed, 128 insertions(+)

diff --git a/Documentation/devicetree/bindings/leds/adi,ltc3220.yaml b/Documentation/devicetree/bindings/leds/adi,ltc3220.yaml
new file mode 100644
index 000000000000..9e545442f3d9
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/adi,ltc3220.yaml
@@ -0,0 +1,121 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/leds/adi,ltc3220.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices LTC3220 LED Driver
+
+maintainers:
+  - Edelweise Escala <edelweise.escala@analog.com>
+
+description: >
+  The LTC3220 is a multi-display LED driver, which contains a high-efficiency,
+  low-noise charge pump to provide power to up to 18 LED current sources.
+  The LEDs are individually configurable to 64-step linear brightness control,
+  blinking and gradation control via 2-wire I2C interface.
+
+  For more product information please see the link below
+    https://www.analog.com/en/products/ltc3220.html
+
+properties:
+  compatible:
+    const: adi,ltc3220
+
+  reg:
+    maxItems: 1
+
+  '#address-cells':
+    const: 1
+
+  '#size-cells':
+    const: 0
+
+  reset-gpios:
+    maxItems: 1
+
+patternProperties:
+  '^led@([1-9a-f]|1[0-2])$':
+    type: object
+    $ref: /schemas/leds/common.yaml#
+    unevaluatedProperties: false
+    properties:
+      reg:
+        description:
+          Output channel for the LED (1-18 maps to LED outputs D1-D18).
+          Unit-address must be in hexadecimal (1-12 hex = 1-18 decimal).
+          For aggregated LED control, define only one LED node with reg = <1>
+          and use led-sources to list all controlled outputs. Only reg 1 should
+          be present when using led-sources.
+        minimum: 1
+        maximum: 18
+
+    required:
+      - reg
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    // Independent LEDs
+    #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/leds/common.h>
+
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        led-controller@1c {
+            compatible = "adi,ltc3220";
+            reg = <0x1c>;
+            #address-cells = <1>;
+            #size-cells = <0>;
+            reset-gpios = <&gpio 17 GPIO_ACTIVE_LOW>;
+
+            led@1 {
+                reg = <0x1>;
+                function = LED_FUNCTION_INDICATOR;
+                function-enumerator = <1>;
+            };
+
+            led@2 {
+                reg = <0x2>;
+                function = LED_FUNCTION_INDICATOR;
+                function-enumerator = <2>;
+            };
+
+            led@3 {
+                reg = <0x3>;
+                function = LED_FUNCTION_INDICATOR;
+                function-enumerator = <3>;
+            };
+        };
+    };
+
+  - |
+    // Aggregated LED
+    #include <dt-bindings/leds/common.h>
+
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        led-controller@1c {
+            compatible = "adi,ltc3220";
+            reg = <0x1c>;
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            led@1 {
+                reg = <0x1>;
+                led-sources = <0x1 0x2 0x3 0x4 0x5 0x6 0x7 0x8 0x9 0xa 0xb 0xc 0xd 0xe 0xf 0x10 0x11 0x12>;
+                function = LED_FUNCTION_BACKLIGHT;
+            };
+        };
+    };
+
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index 327d74ca7ecb..5c10cc3e3022 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14955,6 +14955,13 @@ W:	https://ez.analog.com/linux-software-drivers
 F:	Documentation/devicetree/bindings/iio/temperature/adi,ltc2983.yaml
 F:	drivers/iio/temperature/ltc2983.c
 
+LTC3220 LED DRIVER
+M:	Edelweise Escala <edelweise.escala@analog.com>
+L:	linux-leds@vger.kernel.org
+S:	Maintained
+W:	https://ez.analog.com/linux-software-drivers
+F:	Documentation/devicetree/bindings/leds/adi,ltc3220.yaml
+
 LTC4282 HARDWARE MONITOR DRIVER
 M:	Nuno Sa <nuno.sa@analog.com>
 L:	linux-hwmon@vger.kernel.org

-- 
2.43.0

[PATCH v8 2/2] leds: ltc3220: Add Support for LTC3220 18 channel LED Driver

[Edelweise Escala]

Add driver for the LTC3220 18-channel LED driver
with I2C interface, individual brightness control, and hardware-assisted
blink/gradation features.

Signed-off-by: Edelweise Escala <edelweise.escala@analog.com>
---
 MAINTAINERS                 |   1 +
 drivers/leds/Kconfig        |  12 ++
 drivers/leds/Makefile       |   1 +
 drivers/leds/leds-ltc3220.c | 413 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 427 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 5c10cc3e3022..7467537938bf 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14961,6 +14961,7 @@ L:	linux-leds@vger.kernel.org
 S:	Maintained
 W:	https://ez.analog.com/linux-software-drivers
 F:	Documentation/devicetree/bindings/leds/adi,ltc3220.yaml
+F:	drivers/leds/leds-ltc3220.c
 
 LTC4282 HARDWARE MONITOR DRIVER
 M:	Nuno Sa <nuno.sa@analog.com>
diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 597d7a79c988..f00cdc11c978 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -1001,6 +1001,18 @@ config LEDS_ST1202
 	  Say Y to enable support for LEDs connected to LED1202
 	  LED driver chips accessed via the I2C bus.
 
+config LEDS_LTC3220
+	tristate "LED Driver for Analog Devices Inc. LTC3220"
+	depends on I2C && LEDS_CLASS
+	help
+	  Say Y to enable support for the Analog Devices LTC3220
+	  18-channel LED controller with I2C interface.
+	  The driver supports individual LED brightness control (64 steps),
+	  hardware-assisted blinking and gradation effects.
+
+	  To compile this driver as a module, choose M here: the module will
+	  be called leds-ltc3220.
+
 config LEDS_TPS6105X
 	tristate "LED support for TI TPS6105X"
 	depends on LEDS_CLASS
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index 8fdb45d5b439..5301568d9e00 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -61,6 +61,7 @@ obj-$(CONFIG_LEDS_LP8788)		+= leds-lp8788.o
 obj-$(CONFIG_LEDS_LP8860)		+= leds-lp8860.o
 obj-$(CONFIG_LEDS_LP8864)		+= leds-lp8864.o
 obj-$(CONFIG_LEDS_LT3593)		+= leds-lt3593.o
+obj-$(CONFIG_LEDS_LTC3220)		+= leds-ltc3220.o
 obj-$(CONFIG_LEDS_MAX5970)		+= leds-max5970.o
 obj-$(CONFIG_LEDS_MAX77650)		+= leds-max77650.o
 obj-$(CONFIG_LEDS_MAX77705)		+= leds-max77705.o
diff --git a/drivers/leds/leds-ltc3220.c b/drivers/leds/leds-ltc3220.c
new file mode 100644
index 000000000000..0941acf39118
--- /dev/null
+++ b/drivers/leds/leds-ltc3220.c
@@ -0,0 +1,413 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * LTC3220 18-Channel LED Driver
+ *
+ * Copyright 2026 Analog Devices Inc.
+ *
+ * Author: Edelweise Escala <edelweise.escala@analog.com>
+ */
+
+#include <linux/bitfield.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/gpio/consumer.h>
+#include <linux/i2c.h>
+#include <linux/leds.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/types.h>
+
+/* LTC3220 Registers */
+#define LTC3220_COMMAND_REG				0x00
+#define   LTC3220_QUICK_WRITE_MASK			BIT(0)
+#define   LTC3220_SHUTDOWN_MASK				BIT(3)
+
+#define LTC3220_ULED_REG(x)				(0x01 + (x))
+#define   LTC3220_LED_CURRENT_MASK			GENMASK(5, 0)
+#define   LTC3220_LED_MODE_MASK				GENMASK(7, 6)
+
+#define LTC3220_GRAD_BLINK_REG				0x13
+#define   LTC3220_GRADATION_MASK			GENMASK(2, 0)
+#define   LTC3220_GRADATION_DIRECTION_MASK		BIT(0)
+#define   LTC3220_GRADATION_PERIOD_MASK			GENMASK(2, 1)
+#define   LTC3220_BLINK_MASK				GENMASK(4, 3)
+
+#define LTC3220_NUM_LEDS				18
+
+#define LTC3220_GRADATION_START_VALUE			128
+#define LTC3220_GRADATION_RAMP_TIME_240MS		240
+#define LTC3220_GRADATION_RAMP_TIME_480MS		480
+
+#define LTC3220_BLINK_ON_156MS				156
+#define LTC3220_BLINK_ON_625MS				625
+#define LTC3220_BLINK_PERIOD_1250MS			1250
+#define LTC3220_BLINK_PERIOD_2500MS			2500
+
+#define LTC3220_BLINK_SHORT_ON_TIME			BIT(0)
+#define LTC3220_BLINK_LONG_PERIOD			BIT(1)
+
+enum ltc3220_blink_mode {
+	LTC3220_BLINK_MODE_625MS_1250MS,
+	LTC3220_BLINK_MODE_156MS_1250MS,
+	LTC3220_BLINK_MODE_625MS_2500MS,
+	LTC3220_BLINK_MODE_156MS_2500MS
+};
+
+enum ltc3220_gradation_mode {
+	LTC3220_GRADATION_MODE_DISABLED,
+	LTC3220_GRADATION_MODE_240MS_RAMP_TIME,
+	LTC3220_GRADATION_MODE_480MS_RAMP_TIME,
+	LTC3220_GRADATION_MODE_960MS_RAMP_TIME
+};
+
+static bool ltc3220_volatile_reg(struct device *dev, unsigned int reg)
+{
+	return reg == LTC3220_GRAD_BLINK_REG;
+}
+
+static const struct regmap_config ltc3220_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.max_register = LTC3220_GRAD_BLINK_REG,
+	.cache_type = REGCACHE_FLAT_S,
+	.volatile_reg = ltc3220_volatile_reg,
+};
+
+struct ltc3220_uled_cfg {
+	struct led_classdev led_cdev;
+	u8 reg_value;
+	u8 led_index;
+};
+
+struct ltc3220 {
+	struct ltc3220_uled_cfg uled_cfg[LTC3220_NUM_LEDS];
+	struct regmap *regmap;
+	bool is_aggregated;
+};
+
+/*
+ * Set LED brightness and mode.
+ * The brightness value determines both the LED current and operating mode:
+ * 0-63:    Normal mode - LED current from 0-63 (off to full brightness)
+ * 64-127:  Blink mode - LED blinks with current level (brightness - 64)
+ * 128-191: Gradation mode - LED gradually changes brightness (brightness - 128)
+ * 192-255: GPO mode - LED operates as general purpose output (brightness - 192)
+ */
+static int ltc3220_set_led_data(struct led_classdev *led_cdev,
+				enum led_brightness brightness)
+{
+	struct ltc3220_uled_cfg *uled_cfg = container_of(led_cdev, struct ltc3220_uled_cfg,
+							 led_cdev);
+	struct ltc3220 *ltc3220 = container_of(uled_cfg, struct ltc3220,
+					       uled_cfg[uled_cfg->led_index]);
+	int ret;
+
+	ret = regmap_write(ltc3220->regmap, LTC3220_ULED_REG(uled_cfg->led_index),
+			   brightness);
+	if (ret)
+		return ret;
+
+	uled_cfg->reg_value = brightness;
+
+	/*
+	 * When aggregated LED mode is enabled, writing to LED 1 updates all
+	 * LEDs simultaneously via quick-write mode. Update cached values for
+	 * all LEDs to reflect the synchronized state.
+	 * See Documentation/devicetree/bindings/leds/adi,ltc3220.yaml for how
+	 * to configure aggregated LED mode.
+	 */
+	if (ltc3220->is_aggregated && uled_cfg->led_index == 0) {
+		for (int i = 0; i < LTC3220_NUM_LEDS; i++)
+			ltc3220->uled_cfg[i].reg_value = brightness;
+	}
+
+	return 0;
+}
+
+static enum led_brightness ltc3220_get_led_data(struct led_classdev *led_cdev)
+{
+	struct ltc3220_uled_cfg *uled_cfg = container_of(led_cdev, struct ltc3220_uled_cfg,
+							 led_cdev);
+
+	return uled_cfg->reg_value;
+}
+
+/*
+ * LTC3220 pattern support for hardware-assisted breathing/gradation.
+ * The hardware supports 3 gradation ramp time 240ms, 480ms, 960ms)
+ * and can ramp up or down.
+ *
+ * Pattern array interpretation:
+ *   pattern[0].brightness = start brightness (0-63)
+ *   pattern[0].delta_t = ramp time in milliseconds
+ *   pattern[1].brightness = end brightness (0-63)
+ *   pattern[1].delta_t = (optional, can be 0 or same as pattern[0].delta_t)
+ */
+static int ltc3220_pattern_set(struct led_classdev *led_cdev,
+			       struct led_pattern *pattern,
+			       u32 len, int repeat)
+{
+	struct ltc3220_uled_cfg *uled_cfg = container_of(led_cdev, struct ltc3220_uled_cfg,
+							 led_cdev);
+	struct ltc3220 *ltc3220 = container_of(uled_cfg, struct ltc3220,
+					       uled_cfg[uled_cfg->led_index]);
+	u8 gradation_period;
+	u8 start_brightness;
+	u8 end_brightness;
+	u8 gradation_val;
+	bool is_increasing;
+	int ret;
+
+	if (len != 2)
+		return -EINVAL;
+
+	start_brightness = pattern[0].brightness & LTC3220_LED_CURRENT_MASK;
+	end_brightness = pattern[1].brightness & LTC3220_LED_CURRENT_MASK;
+
+	is_increasing = end_brightness > start_brightness;
+
+	if (pattern[0].delta_t == 0)
+		gradation_period = LTC3220_GRADATION_MODE_DISABLED;
+	else if (pattern[0].delta_t <= LTC3220_GRADATION_RAMP_TIME_240MS)
+		gradation_period = LTC3220_GRADATION_MODE_240MS_RAMP_TIME;
+	else if (pattern[0].delta_t <= LTC3220_GRADATION_RAMP_TIME_480MS)
+		gradation_period = LTC3220_GRADATION_MODE_480MS_RAMP_TIME;
+	else
+		gradation_period = LTC3220_GRADATION_MODE_960MS_RAMP_TIME;
+
+	gradation_val = FIELD_PREP(LTC3220_GRADATION_PERIOD_MASK, gradation_period);
+	gradation_val |= FIELD_PREP(LTC3220_GRADATION_DIRECTION_MASK, is_increasing);
+
+	ret = regmap_update_bits(ltc3220->regmap, LTC3220_GRAD_BLINK_REG,
+				 LTC3220_GRADATION_MASK, gradation_val);
+	if (ret)
+		return ret;
+
+	ret = ltc3220_set_led_data(led_cdev, start_brightness);
+	if (ret)
+		return ret;
+
+	return ltc3220_set_led_data(led_cdev, LTC3220_GRADATION_START_VALUE + end_brightness);
+}
+
+static int ltc3220_pattern_clear(struct led_classdev *led_cdev)
+{
+	struct ltc3220_uled_cfg *uled_cfg = container_of(led_cdev, struct ltc3220_uled_cfg,
+							 led_cdev);
+	struct ltc3220 *ltc3220 = container_of(uled_cfg, struct ltc3220,
+					       uled_cfg[uled_cfg->led_index]);
+
+	return regmap_update_bits(ltc3220->regmap, LTC3220_GRAD_BLINK_REG,
+				  LTC3220_GRADATION_MASK, 0);
+}
+
+/*
+ * LTC3220 has a global blink configuration that affects all LEDs.
+ * This implementation allows per-LED blink requests, but the blink timing
+ * will be shared across all LEDs. The delay values are mapped to the
+ * hardware's discrete blink rates.
+ */
+static int ltc3220_blink_set(struct led_classdev *led_cdev,
+			     unsigned long *delay_on,
+			     unsigned long *delay_off)
+{
+	struct ltc3220_uled_cfg *uled_cfg = container_of(led_cdev, struct ltc3220_uled_cfg,
+							 led_cdev);
+	struct ltc3220 *ltc3220 = container_of(uled_cfg, struct ltc3220,
+					       uled_cfg[uled_cfg->led_index]);
+	u8 blink_mode = 0;
+
+	if (*delay_on <= LTC3220_BLINK_ON_156MS)
+		blink_mode = LTC3220_BLINK_SHORT_ON_TIME;
+
+	if (*delay_on + *delay_off > LTC3220_BLINK_PERIOD_1250MS)
+		blink_mode |= LTC3220_BLINK_LONG_PERIOD;
+
+	switch (blink_mode) {
+	case LTC3220_BLINK_MODE_625MS_1250MS:
+		*delay_on = LTC3220_BLINK_ON_625MS;
+		*delay_off = LTC3220_BLINK_PERIOD_1250MS - LTC3220_BLINK_ON_625MS;
+		break;
+	case LTC3220_BLINK_MODE_156MS_1250MS:
+		*delay_on = LTC3220_BLINK_ON_156MS;
+		*delay_off = LTC3220_BLINK_PERIOD_1250MS - LTC3220_BLINK_ON_156MS;
+		break;
+	case LTC3220_BLINK_MODE_625MS_2500MS:
+		*delay_on = LTC3220_BLINK_ON_625MS;
+		*delay_off = LTC3220_BLINK_PERIOD_2500MS - LTC3220_BLINK_ON_625MS;
+		break;
+	case LTC3220_BLINK_MODE_156MS_2500MS:
+		*delay_on = LTC3220_BLINK_ON_156MS;
+		*delay_off = LTC3220_BLINK_PERIOD_2500MS - LTC3220_BLINK_ON_156MS;
+		break;
+	}
+
+	return regmap_update_bits(ltc3220->regmap, LTC3220_GRAD_BLINK_REG,
+				  LTC3220_BLINK_MASK, blink_mode);
+}
+
+static void ltc3220_reset_gpio_action(void *data)
+{
+	struct gpio_desc *reset_gpio = data;
+
+	gpiod_set_value_cansleep(reset_gpio, 1);
+}
+
+static int ltc3220_reset(struct ltc3220 *ltc3220, struct i2c_client *client)
+{
+	struct gpio_desc *reset_gpio;
+	int ret;
+
+	reset_gpio = devm_gpiod_get_optional(&client->dev, "reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(reset_gpio))
+		return dev_err_probe(&client->dev, PTR_ERR(reset_gpio), "Failed on reset GPIO\n");
+
+	if (reset_gpio) {
+		gpiod_set_value_cansleep(reset_gpio, 0);
+
+		return devm_add_action_or_reset(&client->dev, ltc3220_reset_gpio_action,
+						reset_gpio);
+	}
+
+	ret = regmap_write(ltc3220->regmap, LTC3220_COMMAND_REG, 0);
+	if (ret)
+		return ret;
+
+	for (int i = 0; i < LTC3220_NUM_LEDS; i++) {
+		ret = regmap_write(ltc3220->regmap, LTC3220_ULED_REG(i), 0);
+		if (ret)
+			return ret;
+	}
+
+	return regmap_write(ltc3220->regmap, LTC3220_GRAD_BLINK_REG, 0);
+}
+
+static int ltc3220_suspend(struct device *dev)
+{
+	struct ltc3220 *ltc3220 = i2c_get_clientdata(to_i2c_client(dev));
+
+	return regmap_update_bits(ltc3220->regmap, LTC3220_COMMAND_REG,
+				  LTC3220_SHUTDOWN_MASK, LTC3220_SHUTDOWN_MASK);
+}
+
+static int ltc3220_resume(struct device *dev)
+{
+	struct ltc3220 *ltc3220 = i2c_get_clientdata(to_i2c_client(dev));
+
+	return regmap_update_bits(ltc3220->regmap, LTC3220_COMMAND_REG,
+				  LTC3220_SHUTDOWN_MASK, 0);
+}
+
+static DEFINE_SIMPLE_DEV_PM_OPS(ltc3220_pm_ops, ltc3220_suspend, ltc3220_resume);
+
+static int ltc3220_probe(struct i2c_client *client)
+{
+	struct ltc3220 *ltc3220;
+	bool aggregated_led_found = false;
+	int num_leds = 0;
+	u8 led_index = 0;
+	int ret;
+
+	ltc3220 = devm_kzalloc(&client->dev, sizeof(*ltc3220), GFP_KERNEL);
+	if (!ltc3220)
+		return -ENOMEM;
+
+	ltc3220->regmap = devm_regmap_init_i2c(client, &ltc3220_regmap_config);
+	if (IS_ERR(ltc3220->regmap))
+		return dev_err_probe(&client->dev, PTR_ERR(ltc3220->regmap),
+				     "Failed to initialize regmap\n");
+
+	i2c_set_clientdata(client, ltc3220);
+
+	ret = ltc3220_reset(ltc3220, client);
+	if (ret)
+		return dev_err_probe(&client->dev, ret, "Failed to reset device\n");
+
+	device_for_each_child_node_scoped(&client->dev, child) {
+		struct led_init_data init_data = {};
+		struct ltc3220_uled_cfg *led;
+		u32 source;
+
+		ret = fwnode_property_read_u32(child, "reg", &source);
+		if (ret)
+			return dev_err_probe(&client->dev, ret, "Couldn't read LED address\n");
+
+		if (!source || source > LTC3220_NUM_LEDS)
+			return dev_err_probe(&client->dev, -EINVAL, "LED address out of range\n");
+
+		init_data.fwnode = child;
+		init_data.devicename = "ltc3220";
+
+		if (fwnode_property_present(child, "led-sources")) {
+			if (source != 1)
+				return dev_err_probe(&client->dev, -EINVAL,
+						     "Aggregated LED out of range\n");
+
+			if (aggregated_led_found)
+				return dev_err_probe(&client->dev, -EINVAL,
+						     "One Aggregated LED only\n");
+
+			aggregated_led_found = true;
+			ltc3220->is_aggregated = true;
+
+			ret = regmap_update_bits(ltc3220->regmap,
+						 LTC3220_COMMAND_REG,
+						 LTC3220_QUICK_WRITE_MASK,
+						 LTC3220_QUICK_WRITE_MASK);
+			if (ret)
+				return dev_err_probe(&client->dev, ret,
+						     "Failed to set quick write mode\n");
+		}
+
+		num_leds++;
+
+		/* LED node reg/index/address goes from 1 to 18 */
+		led_index = source - 1;
+		led = &ltc3220->uled_cfg[led_index];
+		led->led_index = led_index;
+		led->reg_value = 0;
+		led->led_cdev.brightness_set_blocking = ltc3220_set_led_data;
+		led->led_cdev.brightness_get = ltc3220_get_led_data;
+		led->led_cdev.max_brightness = 255;
+		led->led_cdev.blink_set = ltc3220_blink_set;
+		led->led_cdev.pattern_set = ltc3220_pattern_set;
+		led->led_cdev.pattern_clear = ltc3220_pattern_clear;
+
+		ret = devm_led_classdev_register_ext(&client->dev, &led->led_cdev, &init_data);
+		if (ret)
+			return dev_err_probe(&client->dev, ret, "Failed to register LED class\n");
+	}
+
+	/*
+	 * Aggregated LED mode uses hardware quick-write to control all 18 LEDs
+	 * simultaneously. This is mutually exclusive with individual LED control.
+	 * See Documentation/devicetree/bindings/leds/adi,ltc3220.yaml for details
+	 * on how to configure aggregated LED mode.
+	 */
+	if (aggregated_led_found && num_leds > 1)
+		return dev_err_probe(&client->dev, -EINVAL,
+				     "Aggregated LED must be the only LED node\n");
+
+	return 0;
+}
+
+static const struct of_device_id ltc3220_of_match[] = {
+	{ .compatible = "adi,ltc3220" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ltc3220_of_match);
+
+static struct i2c_driver ltc3220_led_driver = {
+	.driver = {
+		.name = "ltc3220",
+		.of_match_table = ltc3220_of_match,
+		.pm = pm_sleep_ptr(&ltc3220_pm_ops),
+	},
+	.probe = ltc3220_probe,
+};
+module_i2c_driver(ltc3220_led_driver);
+
+MODULE_AUTHOR("Edelweise Escala <edelweise.escala@analog.com>");
+MODULE_DESCRIPTION("LED driver for LTC3220 controllers");
+MODULE_LICENSE("GPL");

-- 
2.43.0

Re: [PATCH v8 1/2] dt-bindings: leds: Add LTC3220 18 channel LED Driver

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] `#address-cells` and `#size-cells` are omitted from the top-level `required` list.
- [Low] The schema fails to programmatically enforce the constraint that `reg` must be 1 when `led-sources` is used.
--

commit 7595dbcc690fee63863037049c154e7a47df9973
Author: Edelweise Escala <edelweise.escala@analog.com>

dt-bindings: leds: Add LTC3220 18 channel LED Driver

This commit introduces the device tree binding for the Analog Devices LTC3220,
a multi-display LED driver with an I2C interface. It defines properties for
both individual and aggregated LED control.

> diff --git a/Documentation/devicetree/bindings/leds/adi,ltc3220.yaml b/Documentation/devicetree/bindings/leds/adi,ltc3220.yaml
> new file mode 100644
> index 0000000000000..9e545442f3d9d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/adi,ltc3220.yaml
[ ... ]
> patternProperties:
>   '^led@([1-9a-f]|1[0-2])$':
>     type: object
>     $ref: /schemas/leds/common.yaml#
>     unevaluatedProperties: false
>     properties:
>       reg:
>         description:
>           Output channel for the LED (1-18 maps to LED outputs D1-D18).
>           Unit-address must be in hexadecimal (1-12 hex = 1-18 decimal).
>           For aggregated LED control, define only one LED node with reg = <1>
>           and use led-sources to list all controlled outputs. Only reg 1 should
>           be present when using led-sources.
>         minimum: 1
>         maximum: 18
> 
>     required:
>       - reg

The description notes that only reg = <1> should be present when using
led-sources, but the schema doesn't appear to programmatically enforce this.

Would it make sense to add an if/then block or dependencies rule to enforce
that reg must be 1 when led-sources is present, so that invalid configurations
do not silently pass schema validation?

[ ... ]
> required:
>   - compatible
>   - reg

Since the schema defines child nodes with reg properties, does the top-level
node need to include #address-cells and #size-cells in this required list?

While dtc might issue warnings for missing cell properties, omitting them
from the schema's required list might leave the DT schema validation
incomplete for the container node.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260519-ltc3220-driver-v8-0-e1771c1733f7@analog.com?part=1

Re: [PATCH v8 2/2] leds: ltc3220: Add Support for LTC3220 18 channel LED Driver

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:
- [High] The driver abuses the `brightness` attribute to multiplex hardware modes, violating standard LED UAPI.
- [High] The `blink_mode` value is not shifted before being written via `regmap_update_bits()`, causing blink timing to fail.
- [High] `ltc3220_blink_set()` fails to enable blink mode on the target LED's control register.
- [High] `ltc3220_pattern_clear()` incorrectly halts the global gradation timer and fails to reset the individual LED mode.
- [High] Duplicate `reg` values in the device tree cause double-registration and list corruption.
- [High] Missing `select REGMAP_I2C` in Kconfig leads to silent build failures.
- [Medium] Missing `led_classdev_suspend()` and `led_classdev_resume()` calls during power management.
--

commit 0ef3ccbe351040b28172d9e382676a039d0118d4
Author: Edelweise Escala <edelweise.escala@analog.com>

leds: ltc3220: Add Support for LTC3220 18 channel LED Driver
    
Add driver for the LTC3220 18-channel LED driver with I2C interface,
individual brightness control, and hardware-assisted blink/gradation features.

> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> index 597d7a79c988b..f00cdc11c9782 100644
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -1001,6 +1001,18 @@ config LEDS_ST1202
>  	  Say Y to enable support for LEDs connected to LED1202
>  	  LED driver chips accessed via the I2C bus.
>  
> +config LEDS_LTC3220
> +	tristate "LED Driver for Analog Devices Inc. LTC3220"
> +	depends on I2C && LEDS_CLASS

Should this configuration select REGMAP_I2C? If the driver is built with
CONFIG_I2C enabled but no other module selects CONFIG_REGMAP_I2C, will the
compilation fail with an undefined reference to __devm_regmap_init_i2c?

> diff --git a/drivers/leds/leds-ltc3220.c b/drivers/leds/leds-ltc3220.c
> new file mode 100644
> index 0000000000000..0941acf39118a
> --- /dev/null
> +++ b/drivers/leds/leds-ltc3220.c
[ ... ]
> +static int ltc3220_pattern_clear(struct led_classdev *led_cdev)
> +{
> +	struct ltc3220_uled_cfg *uled_cfg = container_of(led_cdev, struct ltc3220_uled_cfg,
> +							 led_cdev);
> +	struct ltc3220 *ltc3220 = container_of(uled_cfg, struct ltc3220,
> +					       uled_cfg[uled_cfg->led_index]);
> +
> +	return regmap_update_bits(ltc3220->regmap, LTC3220_GRAD_BLINK_REG,
> +				  LTC3220_GRADATION_MASK, 0);
> +}

Since LTC3220_GRAD_BLINK_REG is a shared global register, will clearing the
gradation mask here abruptly break gradation patterns currently executing on
other active LEDs?

Also, should this function reset the target LED's individual mode bits back
to normal mode in LTC3220_ULED_REG to avoid leaving it in a stale state?

[ ... ]
> +static int ltc3220_blink_set(struct led_classdev *led_cdev,
> +			     unsigned long *delay_on,
> +			     unsigned long *delay_off)
> +{
[ ... ]
> +	return regmap_update_bits(ltc3220->regmap, LTC3220_GRAD_BLINK_REG,
> +				  LTC3220_BLINK_MASK, blink_mode);
> +}

Does this properly set the blink timing? Since LTC3220_BLINK_MASK is defined
as GENMASK(4, 3) and the regmap API applies it using bitwise AND, will
blink_mode & 0x18 always evaluate to 0? Should this use
FIELD_PREP(LTC3220_BLINK_MASK, blink_mode) instead?

Additionally, does this successfully enable blink mode on the target LED? It
sets the global blink timings, but doesn't appear to write to the individual
LED's configuration register (LTC3220_ULED_REG). Will the target LED remain
in its previous state instead of actually blinking?

[ ... ]
> +static int ltc3220_suspend(struct device *dev)
> +{
> +	struct ltc3220 *ltc3220 = i2c_get_clientdata(to_i2c_client(dev));
> +
> +	return regmap_update_bits(ltc3220->regmap, LTC3220_COMMAND_REG,
> +				  LTC3220_SHUTDOWN_MASK, LTC3220_SHUTDOWN_MASK);
> +}
> +
> +static int ltc3220_resume(struct device *dev)
> +{
> +	struct ltc3220 *ltc3220 = i2c_get_clientdata(to_i2c_client(dev));
> +
> +	return regmap_update_bits(ltc3220->regmap, LTC3220_COMMAND_REG,
> +				  LTC3220_SHUTDOWN_MASK, 0);
> +}

Should these callbacks invoke led_classdev_suspend() and
led_classdev_resume()? Without them, could active LED triggers fail to pause
and continue issuing concurrent I2C writes to the device while it is in
shutdown?

[ ... ]
> +static int ltc3220_probe(struct i2c_client *client)
> +{
[ ... ]
> +	device_for_each_child_node_scoped(&client->dev, child) {
[ ... ]
> +		/* LED node reg/index/address goes from 1 to 18 */
> +		led_index = source - 1;
> +		led = &ltc3220->uled_cfg[led_index];
> +		led->led_index = led_index;
> +		led->reg_value = 0;
> +		led->led_cdev.brightness_set_blocking = ltc3220_set_led_data;
> +		led->led_cdev.brightness_get = ltc3220_get_led_data;
> +		led->led_cdev.max_brightness = 255;

Does exposing a 0 to 255 scale cause standard userspace actions to
inadvertently switch hardware modes? If a user writes 255 to turn the LED
fully on, will it switch into GPO mode instead of setting maximum brightness?

Should max_brightness be restricted to 63 to keep standard triggers working,
with hardware mode switching handled internally?

> +		led->led_cdev.blink_set = ltc3220_blink_set;
> +		led->led_cdev.pattern_set = ltc3220_pattern_set;
> +		led->led_cdev.pattern_clear = ltc3220_pattern_clear;
> +
> +		ret = devm_led_classdev_register_ext(&client->dev, &led->led_cdev, &init_data);
> +		if (ret)
> +			return dev_err_probe(&client->dev, ret, "Failed to register LED class\n");
> +	}

What happens if the device tree contains multiple child nodes with the same
reg value?

Because the driver uses the source property to index into the pre-allocated
ltc3220->uled_cfg array without checking if the index is already initialized,
could it pass the exact same led_classdev structure to
devm_led_classdev_register_ext() multiple times?

Would this cause the core LED subsystem to call list_add_tail() on an
already-linked node, corrupting the global leds_list?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260519-ltc3220-driver-v8-0-e1771c1733f7@analog.com?part=2

Re: [PATCH v8 1/2] dt-bindings: leds: Add LTC3220 18 channel LED Driver

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2856 bytes --]

On Tue, May 19, 2026 at 01:23:06AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Low] `#address-cells` and `#size-cells` are omitted from the top-level `required` list.
> - [Low] The schema fails to programmatically enforce the constraint that `reg` must be 1 when `led-sources` is used.
> --
> 
> commit 7595dbcc690fee63863037049c154e7a47df9973
> Author: Edelweise Escala <edelweise.escala@analog.com>
> 
> dt-bindings: leds: Add LTC3220 18 channel LED Driver
> 
> This commit introduces the device tree binding for the Analog Devices LTC3220,
> a multi-display LED driver with an I2C interface. It defines properties for
> both individual and aggregated LED control.
> 
> > diff --git a/Documentation/devicetree/bindings/leds/adi,ltc3220.yaml b/Documentation/devicetree/bindings/leds/adi,ltc3220.yaml
> > new file mode 100644
> > index 0000000000000..9e545442f3d9d
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/leds/adi,ltc3220.yaml
> [ ... ]
> > patternProperties:
> >   '^led@([1-9a-f]|1[0-2])$':
> >     type: object
> >     $ref: /schemas/leds/common.yaml#
> >     unevaluatedProperties: false
> >     properties:
> >       reg:
> >         description:
> >           Output channel for the LED (1-18 maps to LED outputs D1-D18).
> >           Unit-address must be in hexadecimal (1-12 hex = 1-18 decimal).
> >           For aggregated LED control, define only one LED node with reg = <1>
> >           and use led-sources to list all controlled outputs. Only reg 1 should
> >           be present when using led-sources.
> >         minimum: 1
> >         maximum: 18
> > 
> >     required:
> >       - reg
> 
> The description notes that only reg = <1> should be present when using
> led-sources, but the schema doesn't appear to programmatically enforce this.
> 
> Would it make sense to add an if/then block or dependencies rule to enforce
> that reg must be 1 when led-sources is present, so that invalid configurations
> do not silently pass schema validation?

I actually dunno what the syntax is for restricting this from the parent
node, but it could be done in the led node I guess, using something like
if:
  required:
    - led-sources
then:
  properties:
    reg:
      maximum: 1

Probably.

> 
> [ ... ]
> > required:
> >   - compatible
> >   - reg
> 
> Since the schema defines child nodes with reg properties, does the top-level
> node need to include #address-cells and #size-cells in this required list?
> 
> While dtc might issue warnings for missing cell properties, omitting them
> from the schema's required list might leave the DT schema validation
> incomplete for the container node.

I think this second one is something you should implement.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

RE: [PATCH v8 1/2] dt-bindings: leds: Add LTC3220 18 channel LED Driver

[Escala, Edelweise]

Hi Conor,

> On Tue, May 19, 2026 at 01:23:06AM +0000, sashiko-bot@kernel.org wrote:
> > Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to
> consider:
> > - [Low] `#address-cells` and `#size-cells` are omitted from the top-level
> `required` list.
> > - [Low] The schema fails to programmatically enforce the constraint that `reg`
> must be 1 when `led-sources` is used.
> > --
> >
> > commit 7595dbcc690fee63863037049c154e7a47df9973
> > Author: Edelweise Escala <edelweise.escala@analog.com>
> >
> > dt-bindings: leds: Add LTC3220 18 channel LED Driver
> >
> > This commit introduces the device tree binding for the Analog Devices
> > LTC3220, a multi-display LED driver with an I2C interface. It defines
> > properties for both individual and aggregated LED control.
> >
> > > diff --git a/Documentation/devicetree/bindings/leds/adi,ltc3220.yaml
> > > b/Documentation/devicetree/bindings/leds/adi,ltc3220.yaml
> > > new file mode 100644
> > > index 0000000000000..9e545442f3d9d
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/leds/adi,ltc3220.yaml
> > [ ... ]
> > > patternProperties:
> > >   '^led@([1-9a-f]|1[0-2])$':
> > >     type: object
> > >     $ref: /schemas/leds/common.yaml#
> > >     unevaluatedProperties: false
> > >     properties:
> > >       reg:
> > >         description:
> > >           Output channel for the LED (1-18 maps to LED outputs D1-D18).
> > >           Unit-address must be in hexadecimal (1-12 hex = 1-18 decimal).
> > >           For aggregated LED control, define only one LED node with reg = <1>
> > >           and use led-sources to list all controlled outputs. Only reg 1 should
> > >           be present when using led-sources.
> > >         minimum: 1
> > >         maximum: 18
> > >
> > >     required:
> > >       - reg
> >
> > The description notes that only reg = <1> should be present when using
> > led-sources, but the schema doesn't appear to programmatically enforce this.
> >
> > Would it make sense to add an if/then block or dependencies rule to
> > enforce that reg must be 1 when led-sources is present, so that
> > invalid configurations do not silently pass schema validation?
> 
> I actually dunno what the syntax is for restricting this from the parent node, but it
> could be done in the led node I guess, using something like
> if:
>   required:
>     - led-sources
> then:
>   properties:
>     reg:
>       maximum: 1
> 
> Probably.

Thank you for the recommendation I will be applying

> >
> > [ ... ]
> > > required:
> > >   - compatible
> > >   - reg
> >
> > Since the schema defines child nodes with reg properties, does the
> > top-level node need to include #address-cells and #size-cells in this required
> list?
> >
> > While dtc might issue warnings for missing cell properties, omitting
> > them from the schema's required list might leave the DT schema
> > validation incomplete for the container node.
> 
> I think this second one is something you should implement.
I'll Implement this.

Thank You!

Best Regards,
Edelweise Escala

RE: [PATCH v8 2/2] leds: ltc3220: Add Support for LTC3220 18 channel LED Driver

[Escala, Edelweise]

> -----Original Message-----
> From: sashiko-bot@kernel.org <sashiko-bot@kernel.org>
> Sent: Tuesday, May 19, 2026 9:50 AM
> To: Escala, Edelweise <Edelweise.Escala@analog.com>
> Cc: robh@kernel.org; devicetree@vger.kernel.org; conor+dt@kernel.org
> Subject: Re: [PATCH v8 2/2] leds: ltc3220: Add Support for LTC3220 18 channel
> LED Driver
> 
> [External]
> 
> Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to
> consider:
> - [High] The driver abuses the `brightness` attribute to multiplex hardware modes,
> violating standard LED UAPI.
> - [High] The `blink_mode` value is not shifted before being written via
> `regmap_update_bits()`, causing blink timing to fail.
> - [High] `ltc3220_blink_set()` fails to enable blink mode on the target LED's
> control register.
> - [High] `ltc3220_pattern_clear()` incorrectly halts the global gradation timer and
> fails to reset the individual LED mode.
> - [High] Duplicate `reg` values in the device tree cause double-registration and list
> corruption.
> - [High] Missing `select REGMAP_I2C` in Kconfig leads to silent build failures.
> - [Medium] Missing `led_classdev_suspend()` and `led_classdev_resume()` calls
> during power management.
> --
> 
> commit 0ef3ccbe351040b28172d9e382676a039d0118d4
> Author: Edelweise Escala <edelweise.escala@analog.com>
> 
> leds: ltc3220: Add Support for LTC3220 18 channel LED Driver
> 
> Add driver for the LTC3220 18-channel LED driver with I2C interface, individual
> brightness control, and hardware-assisted blink/gradation features.
> 
> > diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig index
> > 597d7a79c988b..f00cdc11c9782 100644
> > --- a/drivers/leds/Kconfig
> > +++ b/drivers/leds/Kconfig
> > @@ -1001,6 +1001,18 @@ config LEDS_ST1202
> >  	  Say Y to enable support for LEDs connected to LED1202
> >  	  LED driver chips accessed via the I2C bus.
> >
> > +config LEDS_LTC3220
> > +	tristate "LED Driver for Analog Devices Inc. LTC3220"
> > +	depends on I2C && LEDS_CLASS
> 
> Should this configuration select REGMAP_I2C? If the driver is built with
> CONFIG_I2C enabled but no other module selects CONFIG_REGMAP_I2C, will the
> compilation fail with an undefined reference to __devm_regmap_init_i2c?
> 

Will add select REGMAP_I2C

> > diff --git a/drivers/leds/leds-ltc3220.c b/drivers/leds/leds-ltc3220.c
> > new file mode 100644 index 0000000000000..0941acf39118a
> > --- /dev/null
> > +++ b/drivers/leds/leds-ltc3220.c
> [ ... ]
> > +static int ltc3220_pattern_clear(struct led_classdev *led_cdev) {
> > +	struct ltc3220_uled_cfg *uled_cfg = container_of(led_cdev, struct
> ltc3220_uled_cfg,
> > +							 led_cdev);
> > +	struct ltc3220 *ltc3220 = container_of(uled_cfg, struct ltc3220,
> > +					       uled_cfg[uled_cfg->led_index]);
> > +
> > +	return regmap_update_bits(ltc3220->regmap,
> LTC3220_GRAD_BLINK_REG,
> > +				  LTC3220_GRADATION_MASK, 0);
> > +}
> 
> Since LTC3220_GRAD_BLINK_REG is a shared global register, will clearing the
> gradation mask here abruptly break gradation patterns currently executing on
> other active LEDs?
> 
> Also, should this function reset the target LED's individual mode bits back to
> normal mode in LTC3220_ULED_REG to avoid leaving it in a stale state?

It might abruptly change gradation in other leds, will change this to change mode to normal and brightness to 0

> [ ... ]
> > +static int ltc3220_blink_set(struct led_classdev *led_cdev,
> > +			     unsigned long *delay_on,
> > +			     unsigned long *delay_off)
> > +{
> [ ... ]
> > +	return regmap_update_bits(ltc3220->regmap,
> LTC3220_GRAD_BLINK_REG,
> > +				  LTC3220_BLINK_MASK, blink_mode); }
> 
> Does this properly set the blink timing? Since LTC3220_BLINK_MASK is defined as
> GENMASK(4, 3) and the regmap API applies it using bitwise AND, will blink_mode
> & 0x18 always evaluate to 0? Should this use FIELD_PREP(LTC3220_BLINK_MASK,
> blink_mode) instead?
> 
> Additionally, does this successfully enable blink mode on the target LED? It sets
> the global blink timings, but doesn't appear to write to the individual LED's
> configuration register (LTC3220_ULED_REG). Will the target LED remain in its
> previous state instead of actually blinking?

Will fix data shift. 
In my testing, I did observe blinking behavior, but I now realize it was the 
LED subsystem's software fallback mechanism, not the hardware blink feature.

> [ ... ]
> > +static int ltc3220_suspend(struct device *dev) {
> > +	struct ltc3220 *ltc3220 = i2c_get_clientdata(to_i2c_client(dev));
> > +
> > +	return regmap_update_bits(ltc3220->regmap,
> LTC3220_COMMAND_REG,
> > +				  LTC3220_SHUTDOWN_MASK,
> LTC3220_SHUTDOWN_MASK); }
> > +
> > +static int ltc3220_resume(struct device *dev) {
> > +	struct ltc3220 *ltc3220 = i2c_get_clientdata(to_i2c_client(dev));
> > +
> > +	return regmap_update_bits(ltc3220->regmap,
> LTC3220_COMMAND_REG,
> > +				  LTC3220_SHUTDOWN_MASK, 0);
> > +}
> 
> Should these callbacks invoke led_classdev_suspend() and
> led_classdev_resume()? Without them, could active LED triggers fail to pause and
> continue issuing concurrent I2C writes to the device while it is in shutdown?

The LED class core already provides PM infrastructure through its own device PM 
ops (see led_suspend/led_resume in drivers/leds/led-class.c)

> [ ... ]
> > +static int ltc3220_probe(struct i2c_client *client) {
> [ ... ]
> > +	device_for_each_child_node_scoped(&client->dev, child) {
> [ ... ]
> > +		/* LED node reg/index/address goes from 1 to 18 */
> > +		led_index = source - 1;
> > +		led = &ltc3220->uled_cfg[led_index];
> > +		led->led_index = led_index;
> > +		led->reg_value = 0;
> > +		led->led_cdev.brightness_set_blocking = ltc3220_set_led_data;
> > +		led->led_cdev.brightness_get = ltc3220_get_led_data;
> > +		led->led_cdev.max_brightness = 255;
> 
> Does exposing a 0 to 255 scale cause standard userspace actions to inadvertently
> switch hardware modes? If a user writes 255 to turn the LED fully on, will it
> switch into GPO mode instead of setting maximum brightness?
> 
> Should max_brightness be restricted to 63 to keep standard triggers working,
> with hardware mode switching handled internally?

Will update this to just change brightness and rely on pattern and blink trigger

> > +		led->led_cdev.blink_set = ltc3220_blink_set;
> > +		led->led_cdev.pattern_set = ltc3220_pattern_set;
> > +		led->led_cdev.pattern_clear = ltc3220_pattern_clear;
> > +
> > +		ret = devm_led_classdev_register_ext(&client->dev, &led-
> >led_cdev, &init_data);
> > +		if (ret)
> > +			return dev_err_probe(&client->dev, ret, "Failed to
> register LED class\n");
> > +	}
> 
> What happens if the device tree contains multiple child nodes with the same reg
> value?
> 
> Because the driver uses the source property to index into the pre-allocated
> ltc3220->uled_cfg array without checking if the index is already
> ltc3220->initialized,
> could it pass the exact same led_classdev structure to
> devm_led_classdev_register_ext() multiple times?
> 
> Would this cause the core LED subsystem to call list_add_tail() on an already-
> linked node, corrupting the global leds_list?

I'll add a check using led_cdev.dev 
(which is set during registration) to detect and reject duplicate reg values 
before attempting registration.

Re: [PATCH v8 2/2] leds: ltc3220: Add Support for LTC3220 18 channel LED Driver

[Lee Jones]

On Wed, 20 May 2026, Escala, Edelweise wrote:

> 
> > -----Original Message-----
> > From: sashiko-bot@kernel.org <sashiko-bot@kernel.org>

Do I need to sign-up to receive these?

> > Sent: Tuesday, May 19, 2026 9:50 AM
> > To: Escala, Edelweise <Edelweise.Escala@analog.com>
> > Cc: robh@kernel.org; devicetree@vger.kernel.org; conor+dt@kernel.org

Working why only these email addresses were Cc:ed.

> > Subject: Re: [PATCH v8 2/2] leds: ltc3220: Add Support for LTC3220 18 channel
> > LED Driver
> > 
> > [External]
> > 
> > Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to
> > consider:
> > - [High] The driver abuses the `brightness` attribute to multiplex hardware modes,
> > violating standard LED UAPI.
> > - [High] The `blink_mode` value is not shifted before being written via
> > `regmap_update_bits()`, causing blink timing to fail.
> > - [High] `ltc3220_blink_set()` fails to enable blink mode on the target LED's
> > control register.
> > - [High] `ltc3220_pattern_clear()` incorrectly halts the global gradation timer and
> > fails to reset the individual LED mode.
> > - [High] Duplicate `reg` values in the device tree cause double-registration and list
> > corruption.
> > - [High] Missing `select REGMAP_I2C` in Kconfig leads to silent build failures.
> > - [Medium] Missing `led_classdev_suspend()` and `led_classdev_resume()` calls
> > during power management.
> > --
> > 
> > commit 0ef3ccbe351040b28172d9e382676a039d0118d4
> > Author: Edelweise Escala <edelweise.escala@analog.com>



-- 
Lee Jones