Re: [PATCH 07/15] HID: hid-oxp: select brightness policy for the new RGB effect

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Incomplete hardware state rollback in effect_store() leaves the hardware brightness out of sync with software.
--

commit 75feb6ea7d7e2f2f49294cf7e04996cd656c16bc
Author: Andrei Aldea <andrei1998@gmail.com>

HID: hid-oxp: select brightness policy for the new RGB effect

This commit updates the effect_store() function to set the requested effect
before constructing its status command to handle monocolor software intensity
scaling. It also adds logic to restore the cached software effect if either
output command fails.

> diff --git a/drivers/hid/hid-oxp.c b/drivers/hid/hid-oxp.c
> index 8d07816132166..9c22c8e0703a7 100644
> --- a/drivers/hid/hid-oxp.c
> +++ b/drivers/hid/hid-oxp.c
> @@ -1250,15 +1251,21 @@ static ssize_t effect_store(struct device *dev, struct device_attribute *attr,
>  	val = ret;
>  
>  	guard(mutex)(&drvdata.rgb_mutex);
> +	old_effect = drvdata.rgb_effect;
> +	drvdata.rgb_effect = val;
>  
>  	ret = oxp_rgb_status_store(drvdata.rgb_en, drvdata.rgb_speed,
>  				   drvdata.rgb_brightness);
> -	if (ret)
> +	if (ret) {
> +		drvdata.rgb_effect = old_effect;
>  		return ret;
> +	}
>  
>  	ret = oxp_rgb_effect_set(val);
> -	if (ret)
> +	if (ret) {
> +		drvdata.rgb_effect = old_effect;

[Severity: Medium]
Does this error path leave the hardware desynchronized from the software state?

If oxp_rgb_effect_set() fails, the software state (drvdata.rgb_effect) is 
rolled back to old_effect, but the hardware has already successfully processed 
the oxp_rgb_status_store() command using the new effect's policy. 

Should this error path also send a corrective command via 
oxp_rgb_status_store() to restore the original hardware status?

>  		return ret;
> +	}
>  
>  	return count;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260910032115.28669-1-andrei1998@gmail.com?part=7