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

[Sander Vanheule <sander@svanheule.net>]

Hi Edelweise,

On Fri, 2026-05-08 at 12:09 +0800, Edelweise Escala wrote:
> +static const struct regmap_config ltc3220_regmap_config = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +	.max_register = LTC3220_GRAD_BLINK_REG,
> +	.cache_type = REGCACHE_FLAT,
> +	.volatile_reg = ltc3220_volatile_reg,
> +};

With this config, you are assuming that all register values will default to 0,
otherwise the cache will not work properly. While the datasheet seems to
indicate this is the case, I also suspect you are now seeing the warning

    "using zero-initialized flat cache, this may cause unexpected behavior"

I would suggest to use REGCACHE_FLAT_S instead.

[...]

> +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);
> +}

regmap_write() will always go to hardware, so this feels a bit like you're
manually forcing the hardware to match the cache state. The implied all-0 cache
could otherwise prevent later regmap_update_bits() calls from actually
performing a write. 

> +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");

Up to you if you want to clear the device configuration (maybe this causes LED
flickering?), but with REGCACHE_FLAT_S, you should be able to maintain the boot
state of the device.

Best,
Sander