Re: [PATCH v1 3/3] leds: aat1290: Add support for AAT1270 Current Regulator for Flash LEDs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] leds: aat1270: silently ignores flash timeout while exposing sysfs attribute
--

commit 74189882b25371dce0e983ebf15bb883544c3cfd
Author: Svyatoslav Ryhel <clamor95@gmail.com>

leds: aat1290: Add support for AAT1270 Current Regulator for Flash LEDs

This commit adds support for the AAT1270 flash LED controller. It
differentiates it from the AAT1290 by introducing a chip description structure
that flags whether the Advanced Simple Serial Control (AS2C) interface is
supported, and gates AS2C-specific features accordingly.

> diff --git a/drivers/leds/flash/leds-aat1290.c b/drivers/leds/flash/leds-aat1290.c
> index 49251cfd33509..773cdcc73a9e1 100644
> --- a/drivers/leds/flash/leds-aat1290.c
> +++ b/drivers/leds/flash/leds-aat1290.c
[ ... ]
> @@ -178,7 +190,8 @@ static int aat1290_led_flash_strobe_set(struct led_classdev_flash *fled_cdev,
>  	mutex_lock(&led->lock);
>  
>  	if (state) {
> -		aat1290_set_flash_safety_timer(led, timeout->val);
> +		if (led->info->as2c)
> +			aat1290_set_flash_safety_timer(led, timeout->val);

[Severity: Medium]
Can this regression silently ignore the flash timeout for the AAT1270 chip
while still exposing the sysfs attribute?

The patch bypasses programming the hardware flash safety timer here for chips
without AS2C support (like the AAT1270). However, the driver unconditionally
populates the fled_cdev->timeout bounds for all chips in
aat1290_init_flash_timeout():

drivers/leds/flash/leds-aat1290.c:aat1290_init_flash_timeout() {
	...
	/* Init flash timeout setting */
	setting = &fled_cdev->timeout;
	setting->min = cfg->max_flash_tm / AAT1290_FLASH_TM_NUM_LEVELS;
	setting->max = cfg->max_flash_tm;
	setting->step = setting->min;
	setting->val = setting->max;
}

As a result, the LED subsystem will expose a writable flash_timeout sysfs
attribute to userspace. Userspace can configure a timeout value, believing
it provides safety bounds, but the driver will silently discard it during
the actual flash strobe for the AAT1270.

Could this violate the API contract and mislead userspace into overestimating
thermal safety margins?

>  		gpiod_direction_output(led->gpio_fl_en, 1);
>  	} else {
>  		gpiod_direction_output(led->gpio_fl_en, 0);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261007085457.98762-1-clamor95@gmail.com?part=3