Re: [PATCH v8 2/2] leds: ltc3208: Add driver for LTC3208 Multidisplay LED Driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Regcache desynchronization leads to inability to turn off LEDs left on by bootloader
--

--- Patch [2]: [PATCH v8 2/2] leds: ltc3208: Add driver for LTC3208 Multidisplay LED Driver ---
commit 438e39dda8c60e73fe40abad6d35331fd8bb4b80
Author: Jan Carlo Roleda <jancarlo.roleda@analog.com>

leds: ltc3208: Add driver for LTC3208 Multidisplay LED Driver

This commit adds support for the Analog Devices LTC3208 Multidisplay LED
driver. It supports 7 distinct LED channels and uses I2C for configuration,
utilizing regmap for register access and caching.

> diff --git a/drivers/leds/leds-ltc3208.c b/drivers/leds/leds-ltc3208.c
> new file mode 100644
> --- /dev/null
> +++ b/drivers/leds/leds-ltc3208.c
[ ... ]
> +static int ltc3208_probe(struct i2c_client *client)
> +{
[ ... ]
> +	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.");

[Severity: Medium]
Does this code need to synchronize the register cache with the hardware
during initialization, for instance by calling regcache_sync() or explicitly
clearing the LED DAC registers?

If the bootloader or firmware leaves any LEDs physically turned on,
the regmap cache is still initialized with a default value of 0 for the
LED DAC registers (via ltc3208_reg_defaults), but the hardware itself
is not reset to 0.

If userspace later tries to turn off the LED by setting a brightness of 0,
will regmap_field_write() see that the requested value matches the
cached value (0) and optimize away the I2C transaction? This would leave
the LED physically on while the kernel and sysfs report it as off,
requiring the user to first set a non-zero brightness to force a hardware
update before they can successfully turn it off.

> +
> +	disable_camhl_pin = device_property_read_bool(&client->dev, "adi,disable-camhl-pin");
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260730-upstream-ltc3208-v8-0-6bdcc35e65d7@analog.com?part=2