Re: [PATCH 1/2] hwmon: (tacho) Fix a condition in aspeed_get_fan_tach_ch_rpm()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Dan,

On 04/10/2017 06:46 AM, Dan Carpenter wrote:
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.

Fixes: cc96f6f6d766 ("hwmon: Support for ASPEED PWM/Fan tach")
Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
---
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


diff --git a/drivers/hwmon/aspeed-pwm-tacho.c b/drivers/hwmon/aspeed-pwm-tacho.c
index 29010ad94208..2f22add5c73b 100644
--- a/drivers/hwmon/aspeed-pwm-tacho.c
+++ b/drivers/hwmon/aspeed-pwm-tacho.c
@@ -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);
 	}



--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Kernel Development]     [Kernel Announce]     [Kernel Newbies]     [Linux Networking Development]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Device Mapper]

  Powered by Linux