Re: [PATCH v7 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:
- [High] The driver assigns a sleepable function to `led_cdev.brightness_set`, causing panics when called from atomic context.
--

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

leds: st1202: stop pattern sequence before reprogramming

This commit updates st1202_led_pattern_clear() and st1202_led_pattern_set()
to clear the PATS bit in the Configuration register before modifying pattern
registers. This stops the sequence, ensuring the hardware accepts the new
values immediately instead of ignoring them during infinite loop mode.

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

[Severity: High]
This is a pre-existing issue, but I noticed that the driver assigns a
sleepable function to led_cdev.brightness_set in st1202_dt_init():

	led->led_cdev.brightness_set = st1202_brightness_set;

Because st1202_brightness_set() acquires a mutex and performs I2C writes,
it can sleep and must not be called from atomic context. When a user
configures an atomic LED trigger (such as 'timer' or 'heartbeat') via
sysfs, led_set_brightness() can be called in softirq/timer context.

Since this is an I2C device, should the driver only implement
brightness_set_blocking and leave brightness_set as NULL to prevent
kernel panics when called from atomic contexts?

> +	ret = st1202_write_reg(chip, ST1202_CONFIG_REG, ST1202_CONFIG_REG_SHFT);
> +	if (ret != 0)
> +		return ret;
> +
[ ... ]

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