Re: [PATCH] hwmon: (gpd-fan) Reject EC PWM value 0 as invalid

[Guenter Roeck <linux@roeck-us.net>]

On 6/9/26 18:10, Pei Xiao wrote:
> 
> 
> 在 2026/6/9 23:41, Cryolitia PukNgae 写道:
>>
>> On 6/9/26 07:41, Pei Xiao wrote:
>>> The EC firmware is expected to return values in [1, pwm_max]. A read of 0
>>> is illegal and would cause underflow in the conversion formula. Explicitly
>>> check for 0 and return -EIO.
>>>
>>> Signed-off-by: Pei Xiao <xiaopei01@kylinos.cn>
>>> ---
>>>    drivers/hwmon/gpd-fan.c | 6 +++++-
>>>    1 file changed, 5 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/hwmon/gpd-fan.c b/drivers/hwmon/gpd-fan.c
>>> index d1993cd645cb..decb61936b95 100644
>>> --- a/drivers/hwmon/gpd-fan.c
>>> +++ b/drivers/hwmon/gpd-fan.c
>>> @@ -341,7 +341,11 @@ static int gpd_wm2_read_pwm(struct gpd_fan_data *data)
>>>          gpd_ecram_read(drvdata, drvdata->pwm_write, &var);
>>>    -    // Match gpd_generic_write_pwm(u8) below
>>> +    /* EC PWM register valid range is 1 ~ pwm_max; 0 is an invalid
>>> state */
>>> +    if (unlikely(!var))
>>> +        return -EIO;
>>> +
>>> +    /* Match gpd_generic_write_pwm() below */
>>>        return DIV_ROUND_CLOSEST((var - 1) * 255, (drvdata->pwm_max - 1));
>>>    }
>>>    
>>
>> Have you ever tested the behavior on a real device? When it returns 0,
>> what state is the device in?
>>
> Hi Cryolitia,
> 
> Thanks for your  messages!
> Yes, this is not an issue, but this problem was reported during the Sashiko
> AI review.

I disagree: it is an issue. We can not assume that the EC never returns 0,
and the case must be handled, either by returning an error as implemented above
or by returning 0.

Guenter

>   The maintainer suggested I take a look. After checking, I think it might
> be better to add a check:
> if it's 0, just return directly. In case some EC returns 0 someday, causing
> an underflow,
> returning an error code would be more intuitive than returning an incorrect
> value.
> 
> https://sashiko.dev/#/patchset/cover.1780880972.git.xiaopei01@kylinos.cn?part=1
> 
> Pei.
>>
>> thanks,
>>
>> Cryolitia PukNgae
> 
>