Re: [PATCH v6 1/9] leds: st1202: stop pattern sequence before reprogramming

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Medium] Clearing a pattern on one LED overwrites the global pattern duration registers, breaking patterns on other LEDs.
--

commit 386064b373f70c3a399e565edba1ae99f2fd73ed
Author: Manuel Fombuena <fombuena@outlook.com>

leds: st1202: stop pattern sequence before reprogramming

The LED1202 ignores modifications to pattern registers if the sequencer is in
infinite loop mode. This patch clears the PATS bit in the Configuration
register to stop the running sequence before reprogramming it.

> diff --git a/drivers/leds/leds-st1202.c b/drivers/leds/leds-st1202.c
> index 7f68d956f6948..316ed8eb054f9 100644
> --- a/drivers/leds/leds-st1202.c
> +++ b/drivers/leds/leds-st1202.c
> @@ -200,6 +200,10 @@ static int st1202_led_pattern_clear(struct led_classdev *ldev)
>  
>  	guard(mutex)(&chip->lock);
>  
> +	ret = st1202_write_reg(chip, ST1202_CONFIG_REG, ST1202_CONFIG_REG_SHFT);
> +	if (ret != 0)
> +		return ret;
> +
>  	for (int patt = 0; patt < ST1202_MAX_PATTERNS; patt++) {
>  		ret = st1202_pwm_pattern_write(chip, led->led_num, patt, LED_OFF);
>  		if (ret != 0)
>  			return ret;
> 
>  		ret = st1202_write_reg(chip, ST1202_PATTERN_DUR + patt, 0);

[Severity: Medium]
This is a pre-existing issue, but does clearing a pattern on one LED
inadvertently break patterns on other LEDs?

When st1202_led_pattern_clear() is called to clear a single LED's pattern,
it zeros out the pattern duration registers for all 8 pattern steps as shown
in the loop above.

Because these duration registers are shared chip-wide among all LEDs,
overwriting them corrupts the timing for any other LEDs that are currently
running a pattern concurrently.

This patch makes the behavior actively destructive because adding the
sequencer stop ensures these duration writes are fully applied by the
hardware.

While the commit message notes that halting the sequencer is a hardware
constraint, is there a hardware requirement to also overwrite the shared
duration registers when clearing a single LED's pattern?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/GV1PR08MB84972A16ED557BFB96D63F57C5F52@GV1PR08MB8497.eurprd08.prod.outlook.com?part=1