Add Support for LTC3208 #3022

jcroleda · 2025-11-20T03:56:45Z

PR Description

This PR adds driver support for LTC3208 Multi-Display Driver
Datasheet: LTC3208
Tested on Raspberry Pi 4B using the following evaluation board: DC824A

PR Type

Bug fix (a change that fixes an issue)
New feature (a change that adds new functionality)
Breaking change (a change that affects other repos or cause CIs to fail)

PR Checklist

I have conducted a self-review of my own code changes
I have compiled my changes, including the documentation
I have tested the changes on the relevant hardware
I have updated the documentation outside this repo accordingly
I have provided links for the relevant upstream lore

edelweiseescala

Hello, just sharing what I've learned so far

drivers/leds/leds-ltc3208.c

Documentation/ABI/testing/sysfs-class-led-driver-ltc3208

Documentation/devicetree/bindings/leds/leds-ltc3208.yaml

drivers/leds/leds-ltc3208.c

jcroleda · 2025-11-24T05:20:21Z

Changelog V2:

removed ltc3208_get_brightness
fixed indentations
replaced device attributes for AUX configurations in favor of devicetree properties
cleaned define macros (removed ltc3208_low_byte_data and ltc3208_byte, used field_prep for ltc3208_high_byte_data)
updated GPL license on leds-ltc3208.c
updated commit message for devicetree bindings

drivers/leds/leds-ltc3208.c

jcroleda · 2025-12-01T06:40:59Z

Changelog V3:

replaced device_property_match_string with device_property_read_string
updated LTC3208_NUM_AUX_LEDS to 4

Documentation/devicetree/bindings/leds/leds-ltc3208.yaml

drivers/leds/leds-ltc3208.c

nunojsa · 2025-12-01T10:02:07Z

drivers/leds/leds-ltc3208.c

+	if (ret) {
+		dev_err(&client->dev, "Error Writing brightness to register %u\n", reg);
+		return ret;
+	}


Again, I would just return regmap()

This is exposed to user space by brightness set and is not used in the probe function, if so should I still remove this dev_err?

jcroleda · 2025-12-02T23:11:30Z

Changelog V4:
leds-ltc3208.yaml

Removed common.yaml ref from led patternproperty
updated descriptions for leds and AUX channel configurations for clarity

leds-ltc3208.c

fixed formatting for multiline function calls
Adjusted defines (removed name, updated option count/limits)
fixed error message locations to focus within probe()
used FIELD_PREP() in update options for consistency
replaced device_property_read_string with device_property_match_property_string to shorten aux pin configuration code in probe

jcroleda · 2025-12-02T23:49:51Z

Changelog V4.1:

Adjusted tab spacing

machschmitt

Hello @jcroleda,

Some additional comments. Take these with a grain of salt (specially the one about led_trigger) as I'm not experienced with the LEDs subsystem.

Besides those, I suggest to drop the "LTC3208:" part of the
"dt-bindings: leds: LTC3208: Document LTC3208 Multidisplay LED Driver"
commit title. LTC3208 has not been added to the kernel yet.

Documentation/devicetree/bindings/leds/leds-ltc3208.yaml

drivers/leds/leds-ltc3208.c

machschmitt · 2025-12-05T17:59:20Z

drivers/leds/leds-ltc3208.c

+	ret = ltc3208_update_options(data, en_rgbs, dis_camhl,
+		dropdis_rgb_aux4, cpo_mode);


I'm not familiar with the LED subsystem. Though, I wonder if we can have the ENRGBS pin functionality better supported as a led_trigger. Main idea is, we add/register a struct led_trigger, setting its name, activate/deactivate callbacks, and any other required fields. With the proper configuration, we should be able to get /sys/class/leds/<led>/shot provided to user space (see Documentation/ABI/testing/sysfs-class-led-trigger-oneshot and Documentation/ABI/testing/sysfs-class-led). There could be one trigger for RGB toggle and one for SUB toggle. The trigger activate/deactive handlers would write the proper REGG bit G1, then pulse the connected GPIO (see the comment about the dt property).

Not sure how much of the above actually makes sense so you might look for somebody more experienced with the LED subsystem if things don't fit well.

Or, if this gets too complicated or time consuming, it might be reasonable to assume ENRGBS is always high and not mess around with that on the initial support for LTC3208.

I think having seperate oneshot triggers for each channel might introduce specific conflicts on the use of the pin (ex. Sub trigger -> RGB trigger before oneshot period is over, will override the Sub trigger as it will switch control of ENRGBS).

However, we could add a general GPIO pin support to provide sysfs access of the pin, and possibly support the same for the CAMHL pin as it serves a similar toggle function.

I think that can be avoided by locking a mutex on LTC3208 driver, no? Lock a mutex at the beginning of SUB trigger handler (before writing the ENRGBS config), unlock the mutex at the end of the handler. Same logic (and lock) for the RGB trigger handler.

Documentation/devicetree/bindings/leds/leds-ltc3208.yaml

Add Documentation for LTC3208 Multidisplay LED Driver. Signed-off-by: Jan Carlo Roleda <jancarlo.roleda@analog.com>

Kernel driver implementation for LTC3208 Multidisplay LED Driver Signed-off-by: Jan Carlo Roleda <jancarlo.roleda@analog.com>

Add entry to Kconfig for LTC3208 driver Signed-off-by: Jan Carlo Roleda <jancarlo.roleda@analog.com>

jcroleda · 2025-12-10T07:12:41Z

Changelog V5:

Updated dtbindings name and description for ENRGBS for clarity
removed redundant struct in ltc3208_dev
removed prints in -ENOMEM returns
moved regmap_write to inside switch case of set_brightness and added invalid default
added -ENOMEM error check in kcalloc of LED groups
added GPIO support for ENRGBS and CAMHL device pins
aligned with @edelweiseescala on the naming of adi,force-cpo-level on both ltc3208 and ltc3220 device drivers

machschmitt

Hi @jcroleda,

Third round of review.

machschmitt · 2025-12-10T14:06:29Z

Documentation/devicetree/bindings/leds/leds-ltc3208.yaml

+    const: 0
+
+  enrgbs-gpios:
+    $ref: /schemas/types.yaml#/definitions/phandle-array


Usually not needed to reference the phandle-array type. I'd just drop that.
Also, add maxItems: 1

machschmitt · 2025-12-10T14:06:50Z

Documentation/devicetree/bindings/leds/leds-ltc3208.yaml

+      the SUB channel (if adi,cfg-enrgbs-pin is set).
+
+  camhl-gpios:
+    $ref: /schemas/types.yaml#/definitions/phandle-array


machschmitt · 2025-12-10T15:10:56Z

drivers/leds/leds-ltc3208.c

+	ret = gpiod_get_value(data->enrgbs_gpio);
+	return sysfs_emit(buf, "%d\n", ret);
+}
+static DEVICE_ATTR_RW(enrgbs);


I still think this can be better supported as some sort of triggerable led. Note this is adding a new interface between the kernel and user space and, if accepted, will very likely need to be documented (e.g. Documentation/ABI/testing/sysfs-class-led-driver-lm3533).

machschmitt · 2025-12-10T15:11:29Z

drivers/leds/leds-ltc3208.c

+	ret = gpiod_get_value(data->camhl_gpio);
+	return sysfs_emit(buf, "%d\n", ret);
+}
+static DEVICE_ATTR_RW(camhl);


same for this one

machschmitt · 2025-12-10T15:14:06Z

drivers/leds/leds-ltc3208.c

+	enum ltc3208_aux_channel aux_3, enum ltc3208_aux_channel aux_4)
+{
+	struct regmap *map = dev->map;
+	u8 val = 0;


no need to initialize val to zero if it is going to be assigned right below

machschmitt · 2025-12-10T15:15:11Z

drivers/leds/leds-ltc3208.c

+	struct ltc3208_dev *data;
+	struct regmap *map;
+	struct ltc3208_led *leds;
+	enum ltc3208_aux_channel aux_channels[LTC3208_NUM_AUX_LEDS];
+	enum ltc3208_cpo_mode cpo_mode;
+	int ret, i;
+	u32 val;
+	bool en_rgbs, dis_camhl, dropdis_rgb_aux4;


can the local variable heres follow reverse xmas tree ordering?

machschmitt · 2025-12-10T15:21:59Z

drivers/leds/leds-ltc3208.c

+	if (!device_property_read_u32(&client->dev,
+			"adi,force-cpo-level", &val)) {


Is adi,force-cpo-level not a string? Use device_property_match_property_string() 1. See usage examples within the IIO subsystem (e.g. 2 3).

jcroleda requested review from CiprianRegus, amiclaus, btogorean, buha, chegbeli, danmois, dbogdan, machschmitt, mhennerich, nunojsa and rbolboac as code owners November 20, 2025 03:56

jcroleda force-pushed the staging/ltc3208 branch 3 times, most recently from 47e5181 to d2fada2 Compare November 20, 2025 10:25

edelweiseescala reviewed Nov 21, 2025

View reviewed changes

jcroleda force-pushed the staging/ltc3208 branch from d2fada2 to 57ca070 Compare November 24, 2025 05:19

jcroleda force-pushed the staging/ltc3208 branch from 57ca070 to 999b8f4 Compare November 25, 2025 00:36

UtsavAgarwalADI reviewed Nov 28, 2025

View reviewed changes

drivers/leds/leds-ltc3208.c Outdated Show resolved Hide resolved

UtsavAgarwalADI reviewed Nov 28, 2025

View reviewed changes

drivers/leds/leds-ltc3208.c Outdated Show resolved Hide resolved

jcroleda force-pushed the staging/ltc3208 branch from 999b8f4 to 20cc126 Compare December 1, 2025 06:38

nunojsa reviewed Dec 1, 2025

View reviewed changes

jcroleda force-pushed the staging/ltc3208 branch from 20cc126 to e3af1f6 Compare December 2, 2025 23:09

jcroleda force-pushed the staging/ltc3208 branch 2 times, most recently from 5d8624c to 6e8165c Compare December 2, 2025 23:48

machschmitt reviewed Dec 5, 2025

View reviewed changes

dt-bindings: leds: LTC3208: Document LTC3208 Multidisplay LED Driver

8b3e145

Add Documentation for LTC3208 Multidisplay LED Driver. Signed-off-by: Jan Carlo Roleda <jancarlo.roleda@analog.com>

jcroleda added 2 commits December 10, 2025 15:11

leds: ltc3208: add driver

a144ded

Kernel driver implementation for LTC3208 Multidisplay LED Driver Signed-off-by: Jan Carlo Roleda <jancarlo.roleda@analog.com>

leds: Kconfig.adi: imply LTC3208

0a738c7

Add entry to Kconfig for LTC3208 driver Signed-off-by: Jan Carlo Roleda <jancarlo.roleda@analog.com>

jcroleda force-pushed the staging/ltc3208 branch from 6e8165c to 0a738c7 Compare December 10, 2025 07:11

machschmitt reviewed Dec 10, 2025

View reviewed changes

		ret = ltc3208_update_options(data, en_rgbs, dis_camhl,
		dropdis_rgb_aux4, cpo_mode);

		if (!device_property_read_u32(&client->dev,
		"adi,force-cpo-level", &val)) {

Add Support for LTC3208 #3022

Are you sure you want to change the base?

Add Support for LTC3208 #3022

Conversation

jcroleda commented Nov 20, 2025

PR Description

PR Type

PR Checklist

Uh oh!

edelweiseescala left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jcroleda commented Nov 24, 2025

Uh oh!

Uh oh!

Uh oh!

jcroleda commented Dec 1, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jcroleda commented Dec 2, 2025

Uh oh!

jcroleda commented Dec 2, 2025

Uh oh!

machschmitt left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jcroleda Dec 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jcroleda commented Dec 10, 2025

Uh oh!

machschmitt left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

jcroleda Dec 9, 2025 •

edited

Loading