hwmon: (tacho) Fix a condition in aspeed_get_fan_tach_ch_rpm()
Subsystems (3)
PATCH 1/2 hwmon: (tacho) Fix a condition in aspeed_get_fan_tach_ch_rpm()
What we want is for regmap_read() to return zero meaning success and for RESULT_STATUS_MASK which is BIT(31) to be set set.
The current code is buggy in two ways. It requires both failure conditions should be met before it fails. And if it times out then it returns zero instead of -EIO.
---
Static analysis. Not tested.
drivers/hwmon/aspeed-pwm-tacho.c +3 -3
@@ -512,11 +512,11 @@ static u32 aspeed_get_fan_tach_ch_rpm(struct aspeed_pwm_tacho_data *priv,msleep (sec); -while (!(regmap_read (priv->regmap,ASPEED_PTCR_RESULT , &val)) - & !(val &RESULT_STATUS_MASK )) { +while (regmap_read (priv->regmap,ASPEED_PTCR_RESULT , &val) || + !(val &RESULT_STATUS_MASK )) { timeout++;if (timeout >1 ) -return 0 ; +return -EIO ;msleep (sec); }
Guenter Roeck
maintainerHi Dan,
Quoted 11 lines · Dan Carpenter
What we want is for
regmap_read()to return zero meaning success and forRESULT_STATUS_MASKwhich is BIT(31) to be set set.The current code is buggy in two ways. It requires both failure conditions should be met before it fails. And if it times out then it returns zero instead of
-EIO.Fixes cc96f6f6d766 ("hwmon: Support for ASPEED PWM/Fan tach")Signed-off-by Dan Carpenter <[email protected]>---
Static analysis. Not tested.
Thanks for the report. My fault - I had asked for this and the problem in the next patch to be fixed, I got a new version of the patch with the fix, but I did not find the time to apply it. Sorry for the noise.
Guenter
Quoted 21 lines
drivers/hwmon/aspeed-pwm-tacho.c +3
-3@@ -512,11 +512,11 @@ static u32 aspeed_get_fan_tach_ch_rpm(struct aspeed_pwm_tacho_data *priv,msleep (sec); -while (!(regmap_read (priv->regmap,ASPEED_PTCR_RESULT , &val)) - & !(val &RESULT_STATUS_MASK )) { +while (regmap_read (priv->regmap,ASPEED_PTCR_RESULT , &val) || + !(val &RESULT_STATUS_MASK )) { timeout++;if (timeout >1 ) -return 0 ; +return -EIO ;msleep (sec); }