Re: [PATCH net-next v5 2/7] leds: is32fl3207: Add controller driver

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] `is32fl3207_clear_retained_scaling` fails to latch the zeroed scaling registers, leading to a brief LED flash during hardware initialization.
--

--- Patch [2]: [PATCH net-next v5 2/7] leds: is32fl3207: Add controller driver ---
commit 63a2ff2e96d1f984d184b4b7a9733188419e971d
Author: Ahmad Byagowi <[email protected]>

leds: is32fl3207: Add controller driver

Add an I2C driver for the Lumissil IS32FL3207 18-channel LED controller.
Individual and multicolor LEDs are exposed through the LED class, with
support for brightness and channel scaling.

> diff --git a/drivers/leds/rgb/leds-is32fl3207.c b/drivers/leds/rgb/leds-is32fl3207.c
> new file mode 100644
> index 0000000000000..6a46f97ba50a5
> --- /dev/null
> +++ b/drivers/leds/rgb/leds-is32fl3207.c
[ ... ]
> +static int is32fl3207_clear_retained_scaling(struct is32fl3207 *chip)
> +{
> +	u8 scaling[IS32FL3207_NUM_CHANNELS] = { };
> +	int ret;
> +
> +	ret = regmap_write(chip->regmap, IS32FL3207_REG_CONTROL, 0);
> +	if (ret)
> +		return ret;
> +
> +	return regmap_bulk_write(chip->regmap,
> +				 IS32FL3207_REG_SCALING(0), scaling,
> +				 sizeof(scaling));
> +}

[Severity: Medium]
Will this fail to latch the zeroed scaling registers, leading to a brief LED
flash during hardware initialization?

The zeroing of the scaling registers is performed in software shutdown without
entering normal operation or issuing a PWM_UPDATE, leaving the active hardware
registers at their old values.

> +
> +static int is32fl3207_hw_init(struct is32fl3207 *chip)
> +{
> +	u8 scaling[IS32FL3207_NUM_CHANNELS] = { };
> +	u8 pwm[2 * IS32FL3207_NUM_CHANNELS] = { };
> +	int disable_ret;
> +	int ret;
> +
> +	/* Software reset requires normal operation (SSD = 1). */
> +	ret = regmap_write(chip->regmap, IS32FL3207_REG_CONTROL,
> +			   IS32FL3207_CONTROL_ENABLE);

[Severity: Medium]
When normal operation is enabled here to issue the software reset, will the
unlatched active registers drive the LEDs using their retained pre-boot values
and cause a flash?

The controller requires normal operation and a write to the PWM_UPDATE
register to latch new values into the active output stages. Because the
zeroed scaling values remain unlatched in shadow registers, this write to
enable normal operation causes the active registers to hold their pre-reboot
retained values, briefly illuminating the LEDs until the reset is processed.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2