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

[PATCH v13 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 v13:
- Fixed regcache_sync() broadcast issue in aggregated LED mode resume path
- Removed LED_CORE_SUSPENDRESUME flag
- Added bounds validation in second LED registration pass
- Fixed devicetree binding reg property validation
- Remove unused Headers
- Link to v12: https://lore.kernel.org/r/20260703-ltc3220-driver-v12-0-d4f0da2985e2@analog.com

Changes in v12:
- Add mutex when changing brightness
- Add regcache_mark_dirty on suspend
- Move aggreagted led moe hardware setting after validations
- Add clamps when changing brightness on various modes
- Link to v11: https://lore.kernel.org/linux-leds/20260616-ltc3220-driver-v11-0-6e51dd97ced6@analog.com

Changes in v11:
- Add guards to check for double reg
- Add led_classdev_suspend/resume
- Return -EOPNOTSUPP for repeat request not 1
- Single write instead of write+read for blink
- Link to v10: https://lore.kernel.org/r/20260615-ltc3220-driver-v10-0-916562c88598@analog.com

Changes in v10:
- Remove volatile reg
- Add regacache_sync on resume
- Add delays on reset
- Remove early return and let software rset even if there is reset
- Set to max brightness before blink
- Two pass validation approach to avoid premature sysfs exposure
- Use pointer arithmetic for container_of
- Remove unused macros
- Add missing property header
- Link to v9: https://lore.kernel.org/r/20260528-ltc3220-driver-v9-0-69450fc213cb@analog.com

Changes in v9:
- Add address-cells and size-cells in required
- Restrict to reg 1 if led-sources is present
- Make set brightness only change brighntess for nomal mode,
  blink and gradation is now handled only in its respective triggers.
- Fix blink to change mode on blink set
- Make pattern clear only change brightness and not change gradation
  mode so it won't affect other leds.
- Add checks if ever there is duplicate reg in the bindings.
- Add REGMAP_I2C on Kconfig
- Rebase on Latest next
- Link to v8: https://lore.kernel.org/r/20260519-ltc3220-driver-v8-0-e1771c1733f7@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      | 133 +++++
 MAINTAINERS                                        |   8 +
 drivers/leds/Kconfig                               |  13 +
 drivers/leds/Makefile                              |   1 +
 drivers/leds/leds-ltc3220.c                        | 586 +++++++++++++++++++++
 5 files changed, 741 insertions(+)
---
base-commit: 25025253476a64c186592d952c27f24bc3490e42
change-id: 20260106-ltc3220-driver-f9ab6cc9d1e4

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

[PATCH v13 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      | 133 +++++++++++++++++++++
 MAINTAINERS                                        |   7 ++
 2 files changed, 140 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..48215ad82d9a
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/adi,ltc3220.yaml
@@ -0,0 +1,133 @@
+# 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 = <0x1>
+          and use led-sources to list all controlled outputs. Only reg 1 should
+          be present when using led-sources.
+        items:
+          - minimum: 1
+            maximum: 18
+
+    required:
+      - reg
+
+    if:
+      required:
+        - led-sources
+    then:
+      properties:
+        reg:
+          items:
+            - const: 1
+
+required:
+  - compatible
+  - reg
+  - '#address-cells'
+  - '#size-cells'
+
+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 2fb1c75afd16..c8a242577d2f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15223,6 +15223,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

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] Incorrect Property Constraints for `reg` causing ineffective bounds validation
- [Medium] Missing array bounds and value constraints for `led-sources` property
- [Medium] Encoding driver software limitations into Device Tree schema for aggregated LEDs
--

--- Patch [1]: [PATCH v13 1/2] dt-bindings: leds: Add LTC3220 18 channel LED Driver ---
commit 4d1504d2ccfbbb8ac2eccd2728a2c5c976fd4d95
Author: Edelweise Escala <edelweise.escala@analog.com>

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

This commit adds device tree binding documentation for the Analog Devices
LTC3220 LED driver. The binding specifies properties for independent and
aggregated LED control across its 18 channels.

> diff --git a/Documentation/devicetree/bindings/leds/adi,ltc3220.yaml b/Documentation/devicetree/bindings/leds/adi,ltc3220.yaml
> new file mode 100644
> index 0000000000000..48215ad82d9a7
> --- /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:

[Severity: Medium]
Should led-sources be explicitly constrained here?

Since it is inherited from common.yaml without local bounds, a device tree
could potentially define an array of more than 18 channels or use
out-of-bounds indices and still pass validation.

> +      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 = <0x1>
> +          and use led-sources to list all controlled outputs. Only reg 1 should
> +          be present when using led-sources.

[Severity: Medium]
Does restricting reg to 1 when led-sources is present encode a software driver
limitation into the hardware description?

If the hardware allows grouping the 18 channels into multiple discrete
physical LEDs (e.g., LED A on channels 1+2, LED B on channels 3+4), this
schema restriction would prevent valid hardware topologies from being
described.

> +        items:
> +          - minimum: 1
> +            maximum: 18

[Severity: Medium]
Are these minimum and maximum constraints properly evaluated for reg arrays?

By nesting them inside an items array list, the schema applies the numeric
constraints to the array itself, which JSON Schema ignores, silently allowing
out-of-bounds values. Placing them directly under reg allows dt-schema to
correctly validate the integers.

> +
> +    required:
> +      - reg
> +
> +    if:
> +      required:
> +        - led-sources
> +    then:
> +      properties:
> +        reg:
> +          items:
> +            - const: 1

[Severity: Medium]
Does applying const: 1 inside an items list for the reg property fail
unconditionally on arrays?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-ltc3220-driver-v13-0-9b53237642eb@analog.com?part=1

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

[Escala, Edelweise]

> -----Original Message-----
> From: sashiko-bot@kernel.org <sashiko-bot@kernel.org>
> Sent: Thursday, July 9, 2026 8:54 AM
> To: Escala, Edelweise <Edelweise.Escala@analog.com>
> Cc: linux-leds@vger.kernel.org; conor+dt@kernel.org;
> devicetree@vger.kernel.org; lee@kernel.org; robh@kernel.org
> Subject: Re: [PATCH v13 1/2] dt-bindings: leds: Add LTC3220 18 channel LED
> Driver
> 
> [External]
> 
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to
> consider:
> - [Medium] Incorrect Property Constraints for `reg` causing ineffective bounds
> validation
> - [Medium] Missing array bounds and value constraints for `led-sources` property
> - [Medium] Encoding driver software limitations into Device Tree schema for
> aggregated LEDs
> --
> 
> --- Patch [1]: [PATCH v13 1/2] dt-bindings: leds: Add LTC3220 18 channel LED
> Driver --- commit 4d1504d2ccfbbb8ac2eccd2728a2c5c976fd4d95
> Author: Edelweise Escala <edelweise.escala@analog.com>
> 
> dt-bindings: leds: Add LTC3220 18 channel LED Driver
> 
> This commit adds device tree binding documentation for the Analog Devices
> LTC3220 LED driver. The binding specifies properties for independent and
> aggregated LED control across its 18 channels.
> 
> > diff --git a/Documentation/devicetree/bindings/leds/adi,ltc3220.yaml
> > b/Documentation/devicetree/bindings/leds/adi,ltc3220.yaml
> > new file mode 100644
> > index 0000000000000..48215ad82d9a7
> > --- /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:
> 
> [Severity: Medium]
> Should led-sources be explicitly constrained here?
> 
> Since it is inherited from common.yaml without local bounds, a device tree could
> potentially define an array of more than 18 channels or use out-of-bounds indices
> and still pass validation.
> 

The led-sources property inherits validation from common.yaml. While we could add 
explicit local constraints (minItems/maxItems: 18, items: {minimum: 1, maximum: 18}), 
the driver already validates this at probe time.

> > +      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 = <0x1>
> > +          and use led-sources to list all controlled outputs. Only reg 1 should
> > +          be present when using led-sources.
> 
> [Severity: Medium]
> Does restricting reg to 1 when led-sources is present encode a software driver
> limitation into the hardware description?
> 
> If the hardware allows grouping the 18 channels into multiple discrete physical
> LEDs (e.g., LED A on channels 1+2, LED B on channels 3+4), this schema
> restriction would prevent valid hardware topologies from being described.
> 

This is actually a **hardware limitation**, not software. The LTC3220's QUICK_WRITE 
feature broadcasts writes ONLY from register address 0x01 (LED1) to all 18 channels.

> > +        items:
> > +          - minimum: 1
> > +            maximum: 18
> 
> [Severity: Medium]
> Are these minimum and maximum constraints properly evaluated for reg arrays?
> 
> By nesting them inside an items array list, the schema applies the numeric
> constraints to the array itself, which JSON Schema ignores, silently allowing out-
> of-bounds values. Placing them directly under reg allows dt-schema to correctly
> validate the integers.
> 
> > +
> > +    required:
> > +      - reg
> > +
> > +    if:
> > +      required:
> > +        - led-sources
> > +    then:
> > +      properties:
> > +        reg:
> > +          items:
> > +            - const: 1
> 
> [Severity: Medium]
> Does applying const: 1 inside an items list for the reg property fail unconditionally
> on arrays?
> 

Previous review (v12) identified that scalar constraints don't validate 
uint32-matrix properties. The items array format was applied as the fix and 
dt_binding_check passes successfully.

> --
> Sashiko AI review ·
> https://urldefense.com/v3/__https://sashiko.dev/*/patchset/20260709-ltc3220-
> driver-v13-0-
> 9b53237642eb@analog.com?part=1__;Iw!!A3Ni8CS0y2Y!5nhG2a9dYPNgNlSmib
> dgQiCWjPO430OtScJ0L_ze_kPQrHgtlrxHWhJhUaT0YQzsBGC8Y-
> tMKvWqHcz_vArIfCsd7II$

[PATCH v13 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        |  13 +
 drivers/leds/Makefile       |   1 +
 drivers/leds/leds-ltc3220.c | 586 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 601 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index c8a242577d2f..0f553ada61d9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15229,6 +15229,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 f4a0a3c8c870..31b1e3ff094c 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -1000,6 +1000,19 @@ 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
+	select REGMAP_I2C
+	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 7db3768912ca..a68244bd50fb 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..5060d5042fe1
--- /dev/null
+++ b/drivers/leds/leds-ltc3220.c
@@ -0,0 +1,586 @@
+// 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/module.h>
+#include <linux/property.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_MAX_BRIGHTNESS				63
+
+#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_led_mode {
+	LTC3220_NORMAL_MODE,
+	LTC3220_BLINK_MODE,
+	LTC3220_GRADATION_MODE,
+};
+
+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 const struct regmap_config ltc3220_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.max_register = LTC3220_GRAD_BLINK_REG,
+	.cache_type = REGCACHE_FLAT_S,
+};
+
+struct ltc3220_uled_cfg {
+	struct led_classdev led_cdev;
+	u8 reg_value;
+	u8 led_index;
+	bool registered;
+};
+
+struct ltc3220 {
+	struct ltc3220_uled_cfg uled_cfg[LTC3220_NUM_LEDS];
+	struct regmap *regmap;
+	struct mutex lock;
+};
+
+/*
+ * Set LED brightness. Hardware supports 0-63 brightness levels.
+ * Mode switching (blink/gradation) is handled through dedicated callbacks.
+ *
+ * In aggregated mode only a single LED (reg = 1) is registered and the
+ * hardware quick-write feature propagates the write to all 18 channels, so
+ * there is no need to update the other registers explicitly.
+ */
+static int __ltc3220_set_led_data(struct ltc3220 *ltc3220,
+				  struct ltc3220_uled_cfg *uled_cfg,
+				  enum led_brightness brightness)
+{
+	int ret;
+
+	brightness &= LTC3220_LED_CURRENT_MASK;
+
+	ret = regmap_write(ltc3220->regmap, LTC3220_ULED_REG(uled_cfg->led_index),
+			   brightness);
+	if (ret)
+		return ret;
+
+	uled_cfg->reg_value = brightness;
+
+	return 0;
+}
+
+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 - uled_cfg->led_index, struct ltc3220,
+					       uled_cfg[0]);
+	int ret;
+
+	mutex_lock(&ltc3220->lock);
+	ret = __ltc3220_set_led_data(ltc3220, uled_cfg, brightness);
+	mutex_unlock(&ltc3220->lock);
+
+	return ret;
+}
+
+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 times (240ms, 480ms, 960ms)
+ * and can ramp up or down. The gradation period and direction are chip-global
+ * registers (LTC3220_GRAD_BLINK_REG), affecting all 18 channels simultaneously.
+ * This is a hardware limitation, not a driver bug.
+ *
+ * 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 - uled_cfg->led_index, struct ltc3220,
+					       uled_cfg[0]);
+	u8 gradation_period;
+	u8 start_brightness;
+	u8 end_brightness;
+	u8 gradation_val;
+	u8 led_mode;
+	bool is_increasing;
+	int ret;
+
+	if (len != 2)
+		return -EINVAL;
+
+	start_brightness = clamp_val(pattern[0].brightness, 0, LTC3220_LED_CURRENT_MASK);
+	end_brightness = clamp_val(pattern[1].brightness, 0, 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);
+
+	/*
+	 * With the ramp disabled (delta_t == 0) there is no gradation to run,
+	 * so apply the end brightness directly in NORMAL mode instead of
+	 * leaving the channel in gradation mode with a disabled ramp.
+	 */
+	led_mode = gradation_period == LTC3220_GRADATION_MODE_DISABLED ?
+		   LTC3220_NORMAL_MODE : LTC3220_GRADATION_MODE;
+
+	mutex_lock(&ltc3220->lock);
+
+	ret = regmap_update_bits(ltc3220->regmap, LTC3220_GRAD_BLINK_REG,
+				 LTC3220_GRADATION_MASK, gradation_val);
+	if (ret)
+		goto unlock;
+
+	if (led_mode == LTC3220_GRADATION_MODE) {
+		ret = regmap_write(ltc3220->regmap, LTC3220_ULED_REG(uled_cfg->led_index),
+				   start_brightness & LTC3220_LED_CURRENT_MASK);
+		if (ret)
+			goto unlock;
+
+		ret = regmap_write(ltc3220->regmap, LTC3220_ULED_REG(uled_cfg->led_index),
+				   FIELD_PREP(LTC3220_LED_MODE_MASK, led_mode) |
+				   (end_brightness & LTC3220_LED_CURRENT_MASK));
+		if (ret)
+			goto unlock;
+
+		uled_cfg->reg_value = end_brightness;
+	} else {
+		ret = __ltc3220_set_led_data(ltc3220, uled_cfg, end_brightness);
+		if (ret)
+			goto unlock;
+	}
+
+unlock:
+	mutex_unlock(&ltc3220->lock);
+	return ret;
+}
+
+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 - uled_cfg->led_index, struct ltc3220,
+					       uled_cfg[0]);
+	int ret;
+
+	mutex_lock(&ltc3220->lock);
+
+	ret = regmap_update_bits(ltc3220->regmap, LTC3220_ULED_REG(uled_cfg->led_index),
+				 LTC3220_LED_MODE_MASK, LTC3220_NORMAL_MODE);
+	if (ret)
+		goto unlock;
+
+	ret = __ltc3220_set_led_data(ltc3220, uled_cfg, LED_OFF);
+
+unlock:
+	mutex_unlock(&ltc3220->lock);
+	return ret;
+}
+
+/*
+ * LTC3220 has a global blink configuration that affects all LEDs.
+ * This implementation allows per-LED blink requests via sysfs, but setting
+ * blink on any LED reprograms the timing for all 18 channels simultaneously.
+ * The delay values are mapped to the hardware's discrete blink rates.
+ *
+ * HARDWARE LIMITATION: This is not a driver bug. Per-LED blink timing control
+ * is not possible with this hardware due to the global blink register.
+ */
+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 - uled_cfg->led_index, struct ltc3220,
+					       uled_cfg[0]);
+	u8 blink_brightness;
+	u8 blink_mode = 0;
+	int ret;
+
+	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;
+	}
+
+	mutex_lock(&ltc3220->lock);
+
+	ret = regmap_update_bits(ltc3220->regmap, LTC3220_GRAD_BLINK_REG,
+				 LTC3220_BLINK_MASK, FIELD_PREP(LTC3220_BLINK_MASK, blink_mode));
+	if (ret)
+		goto unlock;
+
+	blink_brightness = uled_cfg->reg_value ? : led_cdev->max_brightness;
+
+	ret = regmap_write(ltc3220->regmap, LTC3220_ULED_REG(uled_cfg->led_index),
+			    FIELD_PREP(LTC3220_LED_MODE_MASK, LTC3220_BLINK_MODE) |
+			    (blink_brightness & LTC3220_LED_CURRENT_MASK));
+	if (ret)
+		goto unlock;
+
+	uled_cfg->reg_value = blink_brightness;
+
+unlock:
+	mutex_unlock(&ltc3220->lock);
+	return ret;
+}
+
+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) {
+		usleep_range(10000, 12000);
+		gpiod_set_value_cansleep(reset_gpio, 0);
+		usleep_range(10000, 12000);
+
+		ret = devm_add_action_or_reset(&client->dev, ltc3220_reset_gpio_action,
+					       reset_gpio);
+		if (ret)
+			return ret;
+	}
+
+	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));
+	int ret;
+
+	ret = regmap_update_bits(ltc3220->regmap, LTC3220_COMMAND_REG,
+				 LTC3220_SHUTDOWN_MASK, LTC3220_SHUTDOWN_MASK);
+	if (ret)
+		return ret;
+
+	regcache_mark_dirty(ltc3220->regmap);
+
+	return 0;
+}
+
+static int ltc3220_resume(struct device *dev)
+{
+	struct ltc3220 *ltc3220 = i2c_get_clientdata(to_i2c_client(dev));
+	bool quick_write_enabled;
+	unsigned int command_reg;
+	int ret;
+
+	ret = regmap_read(ltc3220->regmap, LTC3220_COMMAND_REG, &command_reg);
+	if (ret)
+		return ret;
+
+	quick_write_enabled = command_reg & LTC3220_QUICK_WRITE_MASK;
+
+	if (quick_write_enabled) {
+		ret = regmap_update_bits(ltc3220->regmap, LTC3220_COMMAND_REG,
+					 LTC3220_QUICK_WRITE_MASK, 0);
+		if (ret)
+			return ret;
+	}
+
+	ret = regmap_update_bits(ltc3220->regmap, LTC3220_COMMAND_REG,
+				 LTC3220_SHUTDOWN_MASK, 0);
+	if (ret)
+		return ret;
+
+	usleep_range(10000, 12000);
+
+	ret = regcache_sync(ltc3220->regmap);
+	if (ret)
+		return ret;
+
+	if (quick_write_enabled) {
+		ret = regmap_update_bits(ltc3220->regmap, LTC3220_COMMAND_REG,
+					 LTC3220_QUICK_WRITE_MASK,
+					 LTC3220_QUICK_WRITE_MASK);
+		if (ret)
+			return ret;
+	}
+
+	return 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");
+
+	ret = devm_mutex_init(&client->dev, &ltc3220->lock);
+	if (ret)
+		return ret;
+
+	i2c_set_clientdata(client, ltc3220);
+
+	ret = ltc3220_reset(ltc3220, client);
+	if (ret)
+		return dev_err_probe(&client->dev, ret, "Failed to reset device\n");
+
+	/* First pass: validate configuration and set up LED structures */
+	device_for_each_child_node_scoped(&client->dev, child) {
+		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");
+
+		if (fwnode_property_present(child, "led-sources")) {
+			u32 led_sources[LTC3220_NUM_LEDS];
+			int count;
+
+			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");
+
+			count = fwnode_property_count_u32(child, "led-sources");
+			if (count != LTC3220_NUM_LEDS)
+				return dev_err_probe(&client->dev, -EINVAL,
+						     "Aggregated mode requires all %d outputs in led-sources, got %d\n",
+						     LTC3220_NUM_LEDS, count);
+
+			ret = fwnode_property_read_u32_array(child, "led-sources",
+							     led_sources, LTC3220_NUM_LEDS);
+			if (ret)
+				return dev_err_probe(&client->dev, ret,
+						     "Failed to read led-sources array\n");
+
+			/*
+			 * Validate array contents for DT correctness. The hardware
+			 * quick-write broadcasts to all 18 channels regardless of
+			 * array contents, but checking helps catch DT mistakes.
+			 */
+			for (int i = 0; i < LTC3220_NUM_LEDS; i++) {
+				if (led_sources[i] < 1 || led_sources[i] > LTC3220_NUM_LEDS)
+					return dev_err_probe(&client->dev, -EINVAL,
+							     "Invalid output %u in led-sources\n",
+							     led_sources[i]);
+			}
+
+			aggregated_led_found = true;
+		}
+
+		num_leds++;
+
+		/* LED node reg/index/address goes from 1 to 18 */
+		led_index = source - 1;
+		led = &ltc3220->uled_cfg[led_index];
+
+		if (led->registered)
+			return dev_err_probe(&client->dev, -EINVAL,
+					     "Duplicate LED reg %u found\n", source);
+
+		led->registered = true;
+		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 = LTC3220_MAX_BRIGHTNESS;
+		led->led_cdev.blink_set = ltc3220_blink_set;
+		led->led_cdev.pattern_set = ltc3220_pattern_set;
+		led->led_cdev.pattern_clear = ltc3220_pattern_clear;
+	}
+
+	/*
+	 * 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");
+
+	if (num_leds == 0)
+		return dev_err_probe(&client->dev, -EINVAL,
+				     "No LED nodes found in device tree\n");
+
+	if (aggregated_led_found) {
+		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");
+	}
+
+	/* Second pass: register LEDs after validation */
+	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 ret;
+
+		if (!source || source > LTC3220_NUM_LEDS)
+			return dev_err_probe(&client->dev, -EINVAL,
+					     "LED address out of range in second pass\n");
+
+		init_data.fwnode = child;
+		init_data.devicename = "ltc3220";
+
+		led_index = source - 1;
+		led = &ltc3220->uled_cfg[led_index];
+
+		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");
+	}
+
+	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 v13 2/2] leds: ltc3220: Add Support for LTC3220 18 channel LED Driver

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Aggregated LED mode breaks after suspend/resume because regcache_sync() overwrites LEDs 2-18 with zeros.
- [Medium] Missing LED_CORE_SUSPENDRESUME flag and missing locking in suspend/resume handlers allow race conditions.
--

--- Patch [2]: [PATCH v13 2/2] leds: ltc3220: Add Support for LTC3220 18 channel LED Driver ---
commit 530249002ce164e8156c89d4dcd1994b3713c0c8
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/leds-ltc3220.c b/drivers/leds/leds-ltc3220.c
> --- /dev/null
> +++ b/drivers/leds/leds-ltc3220.c

[ ... ]

> @@ -371,20 +371,11 @@ static int ltc3220_suspend(struct device *dev)
>  {
>  	struct ltc3220 *ltc3220 = i2c_get_clientdata(to_i2c_client(dev));
>  	int ret;
>  

[Severity: Medium]
Should ltc3220->lock be held here?

Because LED_CORE_SUSPENDRESUME is not set on the led_classdev, triggers and
sysfs writes can still fire during suspend, potentially racing with these
state changes and executing I2C writes while the device is in shutdown.

>  	ret = regmap_update_bits(ltc3220->regmap, LTC3220_COMMAND_REG,
>  				 LTC3220_SHUTDOWN_MASK, LTC3220_SHUTDOWN_MASK);
>  	if (ret)
>  		return ret;

[ ... ]

> @@ -391,24 +382,15 @@ static int ltc3220_resume(struct device *dev)
>  {
>  	struct ltc3220 *ltc3220 = i2c_get_clientdata(to_i2c_client(dev));
>  	bool quick_write_enabled;
>  	unsigned int command_reg;
>  	int ret;

[ ... ]

>  	if (quick_write_enabled) {
>  		ret = regmap_update_bits(ltc3220->regmap, LTC3220_COMMAND_REG,
>  					 LTC3220_QUICK_WRITE_MASK, 0);
>  		if (ret)
>  			return ret;
>  	}
>  
>  	ret = regmap_update_bits(ltc3220->regmap, LTC3220_COMMAND_REG,
>  				 LTC3220_SHUTDOWN_MASK, 0);
>  	if (ret)
>  		return ret;
>  
>  	usleep_range(10000, 12000);
>  

[Severity: Medium]
Because the driver does not set LED_CORE_SUSPENDRESUME or hold ltc3220->lock
during resume, can an LED trigger fire during this sleep?

If a trigger calls ltc3220_set_led_data() here while QUICK_WRITE is temporarily
disabled, the write would only apply to a single LED instead of broadcasting
to all channels.

>  	ret = regcache_sync(ltc3220->regmap);

[Severity: High]
Does this regcache_sync() break aggregated LED mode?

Since the cache for LEDs 2-18 contains zeros from their initial state, and
QUICK_WRITE is disabled during the sync, this looks like it will explicitly
write zeros to those channels, leaving most LEDs permanently off after resume.

>  	if (ret)
>  		return ret;

[ ... ]

> @@ -499,19 +481,10 @@ static int ltc3220_probe(struct i2c_client *client)

[ ... ]

>  	/* Second pass: register LEDs after validation */
>  	device_for_each_child_node_scoped(&client->dev, child) {
>  		struct led_init_data init_data = {};
>  		struct ltc3220_uled_cfg *led;
>  		u32 source;

[ ... ]

>  		init_data.fwnode = child;
>  		init_data.devicename = "ltc3220";
>  
>  		led_index = source - 1;
>  		led = &ltc3220->uled_cfg[led_index];
>  

[Severity: Medium]
Does this led_cdev need the LED_CORE_SUSPENDRESUME flag set in its flags
field so the LED core knows to suspend triggers during system suspend?

>  		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");
>  	}
>  
>  	return 0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-ltc3220-driver-v13-0-9b53237642eb@analog.com?part=2

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

[Escala, Edelweise]

> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to
> consider:
> - [High] Aggregated LED mode breaks after suspend/resume because
> regcache_sync() overwrites LEDs 2-18 with zeros.
> - [Medium] Missing LED_CORE_SUSPENDRESUME flag and missing locking in
> suspend/resume handlers allow race conditions.
> --
> 
> --- Patch [2]: [PATCH v13 2/2] leds: ltc3220: Add Support for LTC3220 18 channel
> LED Driver --- commit 530249002ce164e8156c89d4dcd1994b3713c0c8
> 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/leds-ltc3220.c b/drivers/leds/leds-ltc3220.c
> > --- /dev/null
> > +++ b/drivers/leds/leds-ltc3220.c
> 
> [ ... ]
> 
> > @@ -371,20 +371,11 @@ static int ltc3220_suspend(struct device *dev)
> > {
> >  	struct ltc3220 *ltc3220 = i2c_get_clientdata(to_i2c_client(dev));
> >  	int ret;
> >
> 
> [Severity: Medium]
> Should ltc3220->lock be held here?
> 
> Because LED_CORE_SUSPENDRESUME is not set on the led_classdev, triggers
> and sysfs writes can still fire during suspend, potentially racing with these state
> changes and executing I2C writes while the device is in shutdown.
> 
> >  	ret = regmap_update_bits(ltc3220->regmap, LTC3220_COMMAND_REG,
> >  				 LTC3220_SHUTDOWN_MASK,
> LTC3220_SHUTDOWN_MASK);
> >  	if (ret)
> >  		return ret;
> 
> [ ... ]

This is already handled correctly. The resume function:
1. Reads COMMAND_REG to detect if QUICK_WRITE is enabled
2. Temporarily disables QUICK_WRITE before regcache_sync()
3. Performs regcache_sync() with QUICK_WRITE disabled
4. Re-enables QUICK_WRITE after sync completes

With QUICK_WRITE disabled during regcache_sync(), writes to LEDs 2-18 apply 
only to those specific channels, not broadcast to all. The cached zeros correctly 
restore each LED's individual off state. When QUICK_WRITE is re-enabled afterward, 
subsequent brightness writes work correctly.


> 
> > @@ -391,24 +382,15 @@ static int ltc3220_resume(struct device *dev)  {
> >  	struct ltc3220 *ltc3220 = i2c_get_clientdata(to_i2c_client(dev));
> >  	bool quick_write_enabled;
> >  	unsigned int command_reg;
> >  	int ret;
> 
> [ ... ]
> 
> >  	if (quick_write_enabled) {
> >  		ret = regmap_update_bits(ltc3220->regmap,
> LTC3220_COMMAND_REG,
> >  					 LTC3220_QUICK_WRITE_MASK, 0);
> >  		if (ret)
> >  			return ret;
> >  	}
> >
> >  	ret = regmap_update_bits(ltc3220->regmap, LTC3220_COMMAND_REG,
> >  				 LTC3220_SHUTDOWN_MASK, 0);
> >  	if (ret)
> >  		return ret;
> >
> >  	usleep_range(10000, 12000);
> >
> 
> [Severity: Medium]
> Because the driver does not set LED_CORE_SUSPENDRESUME or hold ltc3220-
> >lock during resume, can an LED trigger fire during this sleep?
> 
> If a trigger calls ltc3220_set_led_data() here while QUICK_WRITE is temporarily
> disabled, the write would only apply to a single LED instead of broadcasting to all
> channels.
> 
> >  	ret = regcache_sync(ltc3220->regmap);
> 
> [Severity: High]
> Does this regcache_sync() break aggregated LED mode?
> 
> Since the cache for LEDs 2-18 contains zeros from their initial state, and
> QUICK_WRITE is disabled during the sync, this looks like it will explicitly write
> zeros to those channels, leaving most LEDs permanently off after resume.
> 
> >  	if (ret)
> >  		return ret;
> 
> [ ... ]
> 
> > @@ -499,19 +481,10 @@ static int ltc3220_probe(struct i2c_client
> > *client)
> 
> [ ... ]
> 
> >  	/* Second pass: register LEDs after validation */
> >  	device_for_each_child_node_scoped(&client->dev, child) {
> >  		struct led_init_data init_data = {};
> >  		struct ltc3220_uled_cfg *led;
> >  		u32 source;
> 
> [ ... ]
> 
> >  		init_data.fwnode = child;
> >  		init_data.devicename = "ltc3220";
> >
> >  		led_index = source - 1;
> >  		led = &ltc3220->uled_cfg[led_index];
> >
> 
> [Severity: Medium]
> Does this led_cdev need the LED_CORE_SUSPENDRESUME flag set in its flags
> field so the LED core knows to suspend triggers during system suspend?
> 

The driver intentionally omits LED_CORE_SUSPENDRESUME because:
1. Hardware power management is handled via the SHUTDOWN bit
2. The LED core's forced brightness-to-zero on suspend would interfere with 
   hardware blink/gradation state restoration


> >  		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");
> >  	}
> >
> >  	return 0;
> >  }
> 
> --
> Sashiko AI review ·
> https://urldefense.com/v3/__https://sashiko.dev/*/patchset/20260709-ltc3220-
> driver-v13-0-
> 9b53237642eb@analog.com?part=2__;Iw!!A3Ni8CS0y2Y!5sRuO2kidckEwZZPnnr
> M2kxEPjgElsbpjq_JYWKopaoBxsxjSoGN4OBNJg9BlPKQcBKNHsRD1Lmz-
> 9bG73HqJXnacOE$