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

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

[Pei Xiao]

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

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

[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?


thanks,

Cryolitia PukNgae

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

[Pei Xiao]

在 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.
 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

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

[Guenter Roeck]

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

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

[Guenter Roeck]

On Tue, Jun 09, 2026 at 03:41:35PM +0800, 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 */

Please do not change to comment format in an existing driver.
This driver uses the C++ comment style.

Thanks,
Guenter

>  	return DIV_ROUND_CLOSEST((var - 1) * 255, (drvdata->pwm_max - 1));
>  }
>