All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v5 0/2] Add support for LTC3208 multi-display driver
@ 2026-06-18 22:45 Jan Carlo Roleda
  2026-06-18 22:45 ` [PATCH v5 1/2] dt-bindings: leds: Document LTC3208 Multidisplay LED Driver Jan Carlo Roleda
  2026-06-18 22:45 ` [PATCH v5 2/2] leds: ltc3208: Add driver for " Jan Carlo Roleda
  0 siblings, 2 replies; 4+ messages in thread
From: Jan Carlo Roleda @ 2026-06-18 22:45 UTC (permalink / raw)
  To: Lee Jones, Pavel Machek, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: linux-kernel, linux-leds, devicetree, Jan Carlo Roleda,
	Krzysztof Kozlowski

The LTC3208 is a multi-display LED driver, using a high-efficiency, low
noise charge pump to provide power to 5 channels (MAIN, SUB, RGB, CAM,
AUX). Current for each LED is controlled by the I2C serial interface.
Four AUX current sources can be independently assigned via the I2C port
to the CAM, SUB, MAIN, or AUX DAC controlled displays

Signed-off-by: Jan Carlo Roleda <jancarlo.roleda@analog.com>
---
Changes in v5:
- Fixed MAINTAINERS commit ordering
- removed i2c_client from ltc3208_dev
- renamed ltc3208_dev struct to ltc3208
- refactored brightness_set function to use regmap_field
- updated leds attribute to use constant size array
- updated regmap_config to use REGCACHE_FLAT_S cache type
- added max_register to regmap_config
- renamed variables in probe function
- renamed map to regmap for regmap instances
- moved AUX channel configuration inline to probe
- Link to v4: https://lore.kernel.org/r/20260416-upstream-ltc3208-v4-0-3884ed3e49f5@analog.com

Changes in v4:
- Reordered commit order to match dependency order
- Updated Kconfig to be more descriptive of device
- Added led@0-7 with more complete example properties (function and
  color)
- Driver changes:
-- Removed unnecessary include headers
-- Formatted macros
-- Created helper `write_current_level` functions for LED current
  configuration, using `regmap_update_bits()`
-- Adjusted awkward tabbing issues
-- Updated variable names in probe to be more descriptive
-- Updated inline comment capitalization
-- Initialized `i` within the for loop in AUX configuration in probe
-- Refactored `update_aux_dac` function to use array pointer
-- Fixed error messages in probe 
- Link to v3: https://lore.kernel.org/r/20260406-upstream-ltc3208-v3-0-7f0b1d20ee7a@analog.com

Changes in v3:
- Edited device bindings descriptions
-- removed full stop in title
-- replaced quotes with double quotes for consistency
-- removed <dt-bindings/gpio/gpio.h> from example
-- removed led1-7 in example for brevity
- squashed maintainers commit to driver commit
- Link to v2: https://lore.kernel.org/r/20260326-upstream-ltc3208-v2-0-3dbc992b6098@analog.com

Changes in v2:
- Addressed DTSchema bot warnings and errors
-- removed extra blank lines
-- fixed $id to match current naming
- Addressed Kernel test warnings
-- fixed bounds for aux channel configurations
- Link to v0: https://lore.kernel.org/r/20260318-upstream-ltc3208-v1-0-015f1f1e9065@analog.com

---
Jan Carlo Roleda (2):
      dt-bindings: leds: Document LTC3208 Multidisplay LED Driver
      leds: ltc3208: Add driver for LTC3208 Multidisplay LED Driver

 .../devicetree/bindings/leds/adi,ltc3208.yaml      | 181 ++++++++++++++++
 MAINTAINERS                                        |   8 +
 drivers/leds/Kconfig                               |  12 ++
 drivers/leds/Makefile                              |   1 +
 drivers/leds/leds-ltc3208.c                        | 239 +++++++++++++++++++++
 5 files changed, 441 insertions(+)
---
base-commit: d43f1d792902ba0a53fd311bff2cf96095c7606d
change-id: 20260318-upstream-ltc3208-7cc8968bf69e

Best regards,
-- 
Jan Carlo Roleda <jancarlo.roleda@analog.com>


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v5 1/2] dt-bindings: leds: Document LTC3208 Multidisplay LED Driver
  2026-06-18 22:45 [PATCH v5 0/2] Add support for LTC3208 multi-display driver Jan Carlo Roleda
@ 2026-06-18 22:45 ` Jan Carlo Roleda
  2026-06-18 22:45 ` [PATCH v5 2/2] leds: ltc3208: Add driver for " Jan Carlo Roleda
  1 sibling, 0 replies; 4+ messages in thread
From: Jan Carlo Roleda @ 2026-06-18 22:45 UTC (permalink / raw)
  To: Lee Jones, Pavel Machek, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: linux-kernel, linux-leds, devicetree, Jan Carlo Roleda,
	Krzysztof Kozlowski

Add Devicetree Documentation for LTC3208 Multidisplay LED Driver.

Signed-off-by: Jan Carlo Roleda <jancarlo.roleda@analog.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
---
 .../devicetree/bindings/leds/adi,ltc3208.yaml      | 181 +++++++++++++++++++++
 MAINTAINERS                                        |   7 +
 2 files changed, 188 insertions(+)

diff --git a/Documentation/devicetree/bindings/leds/adi,ltc3208.yaml b/Documentation/devicetree/bindings/leds/adi,ltc3208.yaml
new file mode 100644
index 000000000000..0a01e07e0ab7
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/adi,ltc3208.yaml
@@ -0,0 +1,181 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (c) 2026 Analog Devices, Inc.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/leds/adi,ltc3208.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: LTC3208 Multidisplay LED Controller from Linear Technologies (Now Analog Devices)
+
+maintainers:
+  - Jan Carlo Roleda <jancarlo.roleda@analog.com>
+
+description:
+  The LTC3208 is a multidisplay LED controller that can support up to 1A to all
+  connected LEDs.
+
+  The datasheet for this device can be found in
+  https://www.analog.com/en/products/ltc3208.html
+
+properties:
+  compatible:
+    const: adi,ltc3208
+
+  reg:
+    maxItems: 1
+
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 0
+
+  adi,disable-camhl-pin:
+    type: boolean
+    description:
+      Configures whether the external CAMHL pin is disabled.
+      If disabled then the output pins associated with CAM will always select
+      the CAM register's high half-byte brightness.
+
+  adi,cfg-enrgbs-pin:
+    type: boolean
+    description:
+      Configures which channel the ENRGBS pin toggles when it receives a signal.
+      ENRGBS pin controls the SUB channel's output pins if this is set,
+      or RGB channel's output pins if this is unset.
+
+  adi,disable-rgb-aux4-dropout:
+    type: boolean
+    description:
+      Configures the RGB and AUX4 dropout signals to be disabled.
+
+  adi,aux1-channel:
+    $ref: /schemas/types.yaml#/definitions/string
+    description:
+      LED Channel that the AUX1 output pin mirrors its brightness level from.
+    enum: [aux, main, sub, cam]
+    default: aux
+
+  adi,aux2-channel:
+    $ref: /schemas/types.yaml#/definitions/string
+    description:
+      LED Channel that the AUX2 output pin mirrors its brightness level from.
+    enum: [aux, main, sub, cam]
+    default: aux
+
+  adi,aux3-channel:
+    $ref: /schemas/types.yaml#/definitions/string
+    description:
+      LED Channel that the AUX3 output pin mirrors its brightness level from.
+    enum: [aux, main, sub, cam]
+    default: aux
+
+  adi,aux4-channel:
+    $ref: /schemas/types.yaml#/definitions/string
+    description:
+      LED Channel that the AUX4 output pin mirrors its brightness level from.
+    enum: [aux, main, sub, cam]
+    default: aux
+
+patternProperties:
+  "^led@[0-7]$":
+    type: object
+    $ref: /schemas/leds/common.yaml#
+    unevaluatedProperties: false
+    properties:
+      reg:
+        description:
+          LED Channel Number. each channel maps to a specific channel group used
+          to configure the brightness level of the output pins corresponding to
+          the channel.
+        enum:
+          - 0 # Main Channel (8-bit brightness)
+          - 1 # Sub Channel (8-bit brightness)
+          - 2 # AUX Channel (4-bit brightness)
+          - 3 # Camera Channel, Low-side byte (4-bit brightness)
+          - 4 # Camera Channel, High-side byte (4-bit brightness)
+          - 5 # Red Channel (4-bit brightness)
+          - 6 # Blue Channel (4-bit brightness)
+          - 7 # Green Channel (4-bit brightness)
+    required:
+      - reg
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/leds/common.h>
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      led-controller@1b {
+        compatible = "adi,ltc3208";
+        reg = <0x1b>;
+        #address-cells = <1>;
+        #size-cells = <0>;
+        adi,disable-camhl-pin;
+        adi,cfg-enrgbs-pin;
+        adi,disable-rgb-aux4-dropout;
+
+        /* MAIN */
+        led@0 {
+          reg = <0>;
+          function = LED_FUNCTION_ACTIVITY;
+          color = <LED_COLOR_ID_WHITE>;
+        };
+
+        /* SUB */
+        led@1 {
+          reg = <1>;
+          function = LED_FUNCTION_ACTIVITY;
+          color = <LED_COLOR_ID_WHITE>;
+        };
+
+        /* AUX */
+        led@2 {
+          reg = <2>;
+          function = LED_FUNCTION_ACTIVITY;
+          color = <LED_COLOR_ID_WHITE>;
+        };
+
+        /* CAMLO */
+        led@3 {
+          reg = <3>;
+          function = LED_FUNCTION_FLASH;
+          color = <LED_COLOR_ID_WHITE>;
+        };
+
+        /* CAMHI */
+        led@4 {
+          reg = <4>;
+          function = LED_FUNCTION_FLASH;
+          color = <LED_COLOR_ID_WHITE>;
+        };
+
+        /* RED */
+        led@5 {
+          reg = <5>;
+          function = LED_FUNCTION_INDICATOR;
+          color = <LED_COLOR_ID_RED>;
+        };
+
+        /* BLUE */
+        led@6 {
+          reg = <6>;
+          function = LED_FUNCTION_INDICATOR;
+          color = <LED_COLOR_ID_BLUE>;
+        };
+
+        /* GREEN */
+        led@7 {
+          reg = <7>;
+          function = LED_FUNCTION_INDICATOR;
+          color = <LED_COLOR_ID_GREEN>;
+        };
+      };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index 2fb1c75afd16..2fd6ffdaaf04 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
 
+LTC3208 LED DRIVER
+M:	Jan Carlo Roleda <jancarlo.roleda@analog.com>
+L:	linux-leds@vger.kernel.org
+S:	Maintained
+W:	https://ez.analog.com/linux-software-drivers
+F:	Documentation/devicetree/bindings/leds/adi,ltc3208.yaml
+
 LTC4282 HARDWARE MONITOR DRIVER
 M:	Nuno Sa <nuno.sa@analog.com>
 L:	linux-hwmon@vger.kernel.org

-- 
2.43.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH v5 2/2] leds: ltc3208: Add driver for LTC3208 Multidisplay LED Driver
  2026-06-18 22:45 [PATCH v5 0/2] Add support for LTC3208 multi-display driver Jan Carlo Roleda
  2026-06-18 22:45 ` [PATCH v5 1/2] dt-bindings: leds: Document LTC3208 Multidisplay LED Driver Jan Carlo Roleda
@ 2026-06-18 22:45 ` Jan Carlo Roleda
  2026-06-18 22:56   ` sashiko-bot
  1 sibling, 1 reply; 4+ messages in thread
From: Jan Carlo Roleda @ 2026-06-18 22:45 UTC (permalink / raw)
  To: Lee Jones, Pavel Machek, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: linux-kernel, linux-leds, devicetree, Jan Carlo Roleda

Kernel driver implementation for LTC3208 Multidisplay LED Driver.

The LTC3208 is a Multi-display LED driver, designed to control up to
7 distinct LED channels (MAIN, SUB, AUX, CAMHI, CAMLO, RED, GREEN, BLUE),
each configurable with its own current level that is equally set to its
respective output current source pins for external LEDs.

It is programmed via the I2C serial interface.
MAIN and SUB support 8-bit current level resolution,
while AUX, CAMHI/LO, RED, GREEN, and BLUE support 4-bit levels.

The AUX LED channel can be configured to mirror the CAM, SUB, and MAIN
channel current levels, or as its own independent AUX channel.

The CAM LED channel is configured as 2 separate CAMHI and CAMLO register
sub-channels, which currnet is selected via the CAMHL pin, or set to
CAMHI register only via setting the S_CAMHILO bit high in register G (0x7).

Signed-off-by: Jan Carlo Roleda <jancarlo.roleda@analog.com>
---
 MAINTAINERS                 |   1 +
 drivers/leds/Kconfig        |  12 +++
 drivers/leds/Makefile       |   1 +
 drivers/leds/leds-ltc3208.c | 239 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 253 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 2fd6ffdaaf04..e3b59485ecb3 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,ltc3208.yaml
+F:	drivers/leds/leds-ltc3208.c
 
 LTC4282 HARDWARE MONITOR DRIVER
 M:	Nuno Sa <nuno.sa@analog.com>
diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index f4a0a3c8c870..d917ce3b72f4 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -1028,6 +1028,18 @@ config LEDS_ACER_A500
 	  This option enables support for the Power Button LED of
 	  Acer Iconia Tab A500.
 
+config LEDS_LTC3208
+	tristate "LED Driver for Analog Devices LTC3208"
+	depends on LEDS_CLASS && I2C
+	select REGMAP_I2C
+	help
+	  Say Y to enable the LTC3208 LED driver.
+	  This enables the LED device LTC3208, a 7-channel, 17-current source
+	  multidisplay high-current LED driver, configured via I2C.
+
+	  To compile this driver as a module, choose M here: the module will
+	  be called ltc3208.
+
 source "drivers/leds/blink/Kconfig"
 
 comment "Flash and Torch LED drivers"
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index 7db3768912ca..0148b87e16ba 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_LTC3208)		+= leds-ltc3208.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-ltc3208.c b/drivers/leds/leds-ltc3208.c
new file mode 100644
index 000000000000..fa2b6c6acc71
--- /dev/null
+++ b/drivers/leds/leds-ltc3208.c
@@ -0,0 +1,239 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * LED driver for Analog Devices LTC3208 Multi-Display Driver
+ *
+ * Copyright 2026 Analog Devices Inc.
+ *
+ * Author: Jan Carlo Roleda <jancarlo.roleda@analog.com>
+ */
+#include <linux/bitfield.h>
+#include <linux/errno.h>
+#include <linux/i2c.h>
+#include <linux/leds.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/property.h>
+#include <linux/regmap.h>
+#include <linux/types.h>
+
+/* Registers */
+#define LTC3208_REG_A_GRNRED 0x1 /* Green and Red current DAC*/
+#define LTC3208_REG_B_AUXBLU 0x2 /* AUX and Blue current DAC*/
+#define LTC3208_REG_C_MAIN 0x3 /* Main current DAC */
+#define LTC3208_REG_D_SUB 0x4 /* Sub current DAC */
+#define LTC3208_REG_E_AUX_SELECT 0x5 /* AUX DAC Select */
+#define  LTC3208_AUX1_MASK GENMASK(1, 0)
+#define  LTC3208_AUX2_MASK GENMASK(3, 2)
+#define  LTC3208_AUX3_MASK GENMASK(5, 4)
+#define  LTC3208_AUX4_MASK GENMASK(7, 6)
+#define LTC3208_REG_F_CAM 0x6 /* CAM (High and Low) current DAC*/
+#define LTC3208_REG_G_OPT 0x7 /* Device Options */
+#define  LTC3208_OPT_CPO_MASK GENMASK(7, 6)
+#define  LTC3208_OPT_DIS_RGBDROP BIT(3)
+#define  LTC3208_OPT_DIS_CAMHILO BIT(2)
+#define  LTC3208_OPT_EN_RGBS BIT(1)
+
+#define LTC3208_MAX_BRIGHTNESS_4BIT 0xF
+#define LTC3208_MAX_BRIGHTNESS_8BIT 0xFF
+
+#define LTC3208_NUM_LED_GRPS 8
+#define LTC3208_NUM_AUX_LEDS 4
+
+#define LTC3208_NUM_AUX_OPT 4
+#define LTC3208_MAX_CPO_OPT 3
+
+enum ltc3208_aux_channel {
+	LTC3208_AUX_CHAN_AUX = 0,
+	LTC3208_AUX_CHAN_MAIN,
+	LTC3208_AUX_CHAN_SUB,
+	LTC3208_AUX_CHAN_CAM
+};
+
+enum ltc3208_channel {
+	LTC3208_CHAN_MAIN = 0,
+	LTC3208_CHAN_SUB,
+	LTC3208_CHAN_AUX,
+	LTC3208_CHAN_CAML,
+	LTC3208_CHAN_CAMH,
+	LTC3208_CHAN_RED,
+	LTC3208_CHAN_BLUE,
+	LTC3208_CHAN_GREEN,
+	LTC3208_CHAN_N_COUNT,
+};
+
+static const char *const ltc3208_dt_aux_channels[] = { "adi,aux1-channel",
+						       "adi,aux2-channel",
+						       "adi,aux3-channel",
+						       "adi,aux4-channel" };
+
+static const char *const ltc3208_aux_opt[] = { "aux", "main", "sub", "cam" };
+
+struct ltc3208_led {
+	struct led_classdev cdev;
+	struct i2c_client *client;
+	struct regmap_field *rfield;
+	enum ltc3208_channel channel;
+};
+
+struct ltc3208 {
+	struct ltc3208_led leds[LTC3208_NUM_LED_GRPS];
+	struct regmap *regmap;
+};
+
+static const struct regmap_config ltc3208_regmap_cfg = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.max_register = LTC3208_REG_G_OPT,
+	.cache_type = REGCACHE_FLAT_S,
+};
+
+static const struct reg_field ltc3208_led_reg_field[LTC3208_CHAN_N_COUNT] = {
+	[LTC3208_CHAN_MAIN] =  REG_FIELD(LTC3208_REG_C_MAIN, 0, 7),
+	[LTC3208_CHAN_SUB] =   REG_FIELD(LTC3208_REG_D_SUB, 0, 7),
+	[LTC3208_CHAN_BLUE] =  REG_FIELD(LTC3208_REG_B_AUXBLU, 0, 3),
+	[LTC3208_CHAN_AUX] =   REG_FIELD(LTC3208_REG_B_AUXBLU, 4, 7),
+	[LTC3208_CHAN_CAML] =  REG_FIELD(LTC3208_REG_F_CAM, 0, 3),
+	[LTC3208_CHAN_CAMH] =  REG_FIELD(LTC3208_REG_F_CAM, 4, 7),
+	[LTC3208_CHAN_RED] =   REG_FIELD(LTC3208_REG_A_GRNRED, 0, 3),
+	[LTC3208_CHAN_GREEN] = REG_FIELD(LTC3208_REG_A_GRNRED, 4, 7),
+};
+
+static int ltc3208_led_set_brightness(struct led_classdev *led_cdev,
+				      enum led_brightness brightness)
+{
+	struct ltc3208_led *led =
+		container_of(led_cdev, struct ltc3208_led, cdev);
+	u8 current_level = brightness;
+
+	return regmap_field_write(led->rfield, current_level);
+}
+
+static int ltc3208_probe(struct i2c_client *client)
+{
+	enum ltc3208_aux_channel aux_channels[LTC3208_NUM_AUX_LEDS];
+	struct ltc3208 *ddata;
+	struct regmap *regmap;
+	bool disable_rgb_aux4_dropout_signal;
+	bool disable_camhl_pin;
+	bool set_sub_control_pin;
+	int ret;
+	u8 reg_val;
+
+	regmap = devm_regmap_init_i2c(client, &ltc3208_regmap_cfg);
+	if (IS_ERR(regmap))
+		return dev_err_probe(&client->dev, PTR_ERR(regmap),
+				     "Failed to initialize regmap\n");
+
+	ddata = devm_kzalloc(&client->dev, sizeof(*ddata), GFP_KERNEL);
+	if (!ddata)
+		return -ENOMEM;
+
+	ddata->regmap = regmap;
+
+	disable_camhl_pin = device_property_read_bool(&client->dev,
+						      "adi,disable-camhl-pin");
+	set_sub_control_pin =
+		device_property_read_bool(&client->dev, "adi,cfg-enrgbs-pin");
+	disable_rgb_aux4_dropout_signal = device_property_read_bool(
+		&client->dev, "adi,disable-rgb-aux4-dropout");
+
+	reg_val = FIELD_PREP(LTC3208_OPT_EN_RGBS, set_sub_control_pin) |
+		  FIELD_PREP(LTC3208_OPT_DIS_CAMHILO, disable_camhl_pin) |
+		  FIELD_PREP(LTC3208_OPT_DIS_RGBDROP,
+			     disable_rgb_aux4_dropout_signal);
+
+	ret = regmap_write(regmap, LTC3208_REG_G_OPT, reg_val);
+	if (ret)
+		return dev_err_probe(&client->dev, ret,
+				     "error writing to options register\n");
+
+	/* Initialize aux channel configurations */
+	for (int i = 0; i < LTC3208_NUM_AUX_LEDS; i++) {
+		ret = device_property_match_property_string(
+			&client->dev, ltc3208_dt_aux_channels[i],
+			ltc3208_aux_opt, LTC3208_NUM_AUX_OPT);
+		/* Fallback to default value (AUX) if not found */
+		if (ret == -EINVAL)
+			aux_channels[i] = LTC3208_AUX_CHAN_AUX;
+		else if (ret >= 0)
+			aux_channels[i] = ret;
+	}
+
+	reg_val = FIELD_PREP(LTC3208_AUX1_MASK, aux_channels[0]) |
+		  FIELD_PREP(LTC3208_AUX2_MASK, aux_channels[1]) |
+		  FIELD_PREP(LTC3208_AUX3_MASK, aux_channels[2]) |
+		  FIELD_PREP(LTC3208_AUX4_MASK, aux_channels[3]);
+
+	ret = regmap_write(regmap, LTC3208_REG_E_AUX_SELECT, reg_val);
+	if (ret)
+		return dev_err_probe(&client->dev, ret,
+			"error writing to aux channel register.\n");
+
+	i2c_set_clientdata(client, ddata);
+
+	device_for_each_child_node_scoped(&client->dev, child) {
+		struct ltc3208_led *led;
+		struct led_init_data init_data = {};
+		u32 chan;
+
+		ret = fwnode_property_read_u32(child, "reg", &chan);
+		if (ret)
+			return dev_err_probe(&client->dev, ret,
+					    "Failed to get reg value of LED\n");
+		else if (chan >= LTC3208_NUM_LED_GRPS)
+			return dev_err_probe(&client->dev, ret,
+					     "%d is an invalid LED ID\n", chan);
+
+		led = &ddata->leds[chan];
+
+		led->rfield =
+			devm_regmap_field_alloc(&client->dev, ddata->regmap,
+						ltc3208_led_reg_field[chan]);
+		if (IS_ERR(led->rfield))
+			return dev_err_probe(&client->dev, PTR_ERR(led->rfield),
+					     "cannot allocate regmap field\n");
+		led->client = client;
+		led->channel = chan;
+		led->cdev.brightness_set_blocking = ltc3208_led_set_brightness;
+		led->cdev.max_brightness = LTC3208_MAX_BRIGHTNESS_4BIT;
+
+		if (chan == LTC3208_CHAN_MAIN || chan == LTC3208_CHAN_SUB)
+			led->cdev.max_brightness = LTC3208_MAX_BRIGHTNESS_8BIT;
+
+		init_data.fwnode = child;
+
+		ret = devm_led_classdev_register_ext(&client->dev, &led->cdev,
+						     &init_data);
+		if (ret)
+			return dev_err_probe(&client->dev, ret,
+					     "LED %u Register failed.\n", chan);
+	}
+
+	return 0;
+}
+
+static const struct of_device_id ltc3208_match_table[] = {
+	{.compatible = "adi,ltc3208"},
+	{}
+};
+MODULE_DEVICE_TABLE(of, ltc3208_match_table);
+
+static const struct i2c_device_id ltc3208_idtable[] = {
+	{ "ltc3208" },
+	{}
+};
+MODULE_DEVICE_TABLE(i2c, ltc3208_idtable);
+
+static struct i2c_driver ltc3208_driver = {
+	.driver = {
+		.name = "ltc3208",
+		.of_match_table = ltc3208_match_table,
+	},
+	.id_table = ltc3208_idtable,
+	.probe = ltc3208_probe,
+};
+module_i2c_driver(ltc3208_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Jan Carlo Roleda <jancarlo.roleda@analog.com>");
+MODULE_DESCRIPTION("LTC3208 LED Driver");

-- 
2.43.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v5 2/2] leds: ltc3208: Add driver for LTC3208 Multidisplay LED Driver
  2026-06-18 22:45 ` [PATCH v5 2/2] leds: ltc3208: Add driver for " Jan Carlo Roleda
@ 2026-06-18 22:56   ` sashiko-bot
  0 siblings, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-06-18 22:56 UTC (permalink / raw)
  To: Jan Carlo Roleda; +Cc: devicetree, robh, conor+dt

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Duplicate `reg` property values in the device tree cause global `leds_list` corruption due to registering the exact same `led_classdev` object multiple times.
- [Medium] The `aux_channels` stack array can be left uninitialized if device tree properties fail to match due to errors other than `-EINVAL` (e.g., `-ENOENT` or `-ENODATA`), leading to uninitialized memory being used to configure the register.
- [Medium] Falsely successful probe and premature loop abortion when an invalid LED ID is encountered because `ret` is strictly 0.
--

commit b568fbfe98ac0a2fda2a035cd233d75353b8718c
Author: Jan Carlo Roleda <jancarlo.roleda@analog.com>
Subject: leds: ltc3208: Add driver for LTC3208 Multidisplay LED Driver

Kernel driver implementation for LTC3208 Multidisplay LED Driver.
The LTC3208 is a Multi-display LED driver, designed to control up to
7 distinct LED channels.

> diff --git a/drivers/leds/leds-ltc3208.c b/drivers/leds/leds-ltc3208.c
> new file mode 100644
> index 0000000000000..fa2b6c6acc713
> --- /dev/null
> +++ b/drivers/leds/leds-ltc3208.c

[ ... ]

> +	/* Initialize aux channel configurations */
> +	for (int i = 0; i < LTC3208_NUM_AUX_LEDS; i++) {
> +		ret = device_property_match_property_string(
> +			&client->dev, ltc3208_dt_aux_channels[i],
> +			ltc3208_aux_opt, LTC3208_NUM_AUX_OPT);
> +		/* Fallback to default value (AUX) if not found */
> +		if (ret == -EINVAL)
> +			aux_channels[i] = LTC3208_AUX_CHAN_AUX;
> +		else if (ret >= 0)
> +			aux_channels[i] = ret;
> +	}
> +
> +	reg_val = FIELD_PREP(LTC3208_AUX1_MASK, aux_channels[0]) |
> +		  FIELD_PREP(LTC3208_AUX2_MASK, aux_channels[1]) |
> +		  FIELD_PREP(LTC3208_AUX3_MASK, aux_channels[2]) |
> +		  FIELD_PREP(LTC3208_AUX4_MASK, aux_channels[3]);

[Severity: Medium]
What happens to aux_channels[i] if device_property_match_property_string()
returns an error code other than -EINVAL, such as -ENODATA?

It appears aux_channels[i] would remain uninitialized on the stack, and
this uninitialized memory would then be passed to FIELD_PREP() and
written to the LTC3208_REG_E_AUX_SELECT register.

Would it be safer to have a default catch-all else branch, or to fully
initialize the aux_channels array when it is declared?

[ ... ]

> +	device_for_each_child_node_scoped(&client->dev, child) {
> +		struct ltc3208_led *led;
> +		struct led_init_data init_data = {};
> +		u32 chan;
> +
> +		ret = fwnode_property_read_u32(child, "reg", &chan);
> +		if (ret)
> +			return dev_err_probe(&client->dev, ret,
> +					    "Failed to get reg value of LED\n");
> +		else if (chan >= LTC3208_NUM_LED_GRPS)
> +			return dev_err_probe(&client->dev, ret,
> +					     "%d is an invalid LED ID\n", chan);

[Severity: Medium]
Does this return a success code of 0 instead of an error?

Because the preceding fwnode_property_read_u32() call succeeded, ret is
exactly 0 in this else if branch.

Calling dev_err_probe() with a 0 return code will cause the probe function
to abort iterating through the children but return a success status to the
driver core, masking the probe failure.

Should this return an explicit error like -EINVAL instead?

> +
> +		led = &ddata->leds[chan];
> +
> +		led->rfield =
> +			devm_regmap_field_alloc(&client->dev, ddata->regmap,
> +						ltc3208_led_reg_field[chan]);

[ ... ]

> +		ret = devm_led_classdev_register_ext(&client->dev, &led->cdev,
> +						     &init_data);

[Severity: High]
Can duplicate reg properties in the device tree corrupt the led_classdev
list?

If a malformed device tree provides multiple children with the same reg
value, the driver will reuse the exact same struct ltc3208_led object
from the ddata->leds array without checking if it is already in use.

Calling devm_led_classdev_register_ext() multiple times with the identical
&led->cdev pointer would cause the LED core to add the same node to the
global leds_list twice, corrupting the doubly-linked list.

Could we add a check to verify if the channel has already been populated
before proceeding with registration?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260619-upstream-ltc3208-v5-0-075d18060606@analog.com?part=2

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-06-18 22:56 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-18 22:45 [PATCH v5 0/2] Add support for LTC3208 multi-display driver Jan Carlo Roleda
2026-06-18 22:45 ` [PATCH v5 1/2] dt-bindings: leds: Document LTC3208 Multidisplay LED Driver Jan Carlo Roleda
2026-06-18 22:45 ` [PATCH v5 2/2] leds: ltc3208: Add driver for " Jan Carlo Roleda
2026-06-18 22:56   ` sashiko-bot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.