Re: [PATCH v2 12/13] leds: st1202: Cap the channel current in st1202_led_set()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] leds: broken brightness scaling and saturation when max-brightness > 255
--

commit 1bc7b66ecae1ce3e4dd27e771fe90fae2aa2ecc9
Author: Manuel Fombuena <fombuena@outlook.com>
Subject: leds: st1202: Cap the channel current in st1202_led_set()

This commit limits the channel current register value written in
st1202_led_set to U8_MAX to prevent 8-bit wrap-around when the device
tree declares a max-brightness greater than 255.

> diff --git a/drivers/leds/leds-st1202.c b/drivers/leds/leds-st1202.c
> index 39129206b78d6..5e4a8b80cfe47 100644
> --- a/drivers/leds/leds-st1202.c
> +++ b/drivers/leds/leds-st1202.c
> @@ -207,7 +207,8 @@ static int st1202_led_set(struct led_classdev *ldev, enum led_brightness value)
>  			return ret;
>  	}
>  
> -	ret = st1202_write_reg(chip, ST1202_ILED_REG0 + led->led_num, value);
> +	ret = st1202_write_reg(chip, ST1202_ILED_REG0 + led->led_num,
> +				min_t(unsigned int, value, U8_MAX));

[Severity: Medium]
Does capping the value here instead of scaling it cause broken dimming
curves when max-brightness is configured to a value greater than 255 in
the device tree?

If a value like 4095 is used for max-brightness, capping instead of
linearly scaling causes all requests above 255 to fully saturate the
output at max hardware current rather than scaling proportionately.

Additionally, does this clamping affect the digital PWM scaler in
st1202_brightness_to_pwm() when called from st1202_led_pattern_set()?

If the clamped analog limit of 255 is passed to st1202_brightness_to_pwm()
as both the clamp limit and the scaling denominator instead of the actual
ldev->max_brightness, the function appears to calculate:

255 * 4095 / 255 = 4095

This would result in a 100% PWM duty cycle for any pattern brightness
above 255.

Would the lack of linear scaling also cause confusion during sysfs reads
if st1202_brightness_get() returns the raw 8-bit register value instead
of a scaled representation up to ldev->max_brightness?

>  	if (ret)
>  		return ret;

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