| Message ID | 20250206063600.321650-1-Jerry_C_Chen@wiwynn.com (mailing list archive) |
|---|---|
| State | Rejected |
| Headers | show |
| Series | [v1,1/1] hwmon: (adc128d818): filter out 0x1ff reading | expand |
On 2/5/25 22:35, Jerry C Chen wrote: > ASPEED i2c driver doesn't support SMBUS timeout thus we will get dirty > data while SCL hold time over 35ms, filter out default register value > 0x1ff to avoid abnormal data reading. If the Aspeed i2c driver has a problem, it should be fixed there, not in drivers utilizing it. > > Signed-off-by: Jerry C Chen <Jerry_C_Chen@wiwynn.com> > --- > drivers/hwmon/adc128d818.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/hwmon/adc128d818.c b/drivers/hwmon/adc128d818.c > index 5e805d4ee76a..8197d3f14ea7 100644 > --- a/drivers/hwmon/adc128d818.c > +++ b/drivers/hwmon/adc128d818.c > @@ -88,7 +88,7 @@ static struct adc128_data *adc128_update_device(struct device *dev) > for (i = 0; i < num_inputs[data->mode]; i++) { > rv = i2c_smbus_read_word_swapped(client, > ADC128_REG_IN(i)); > - if (rv < 0) > + if (rv < 0 || rv == 511) That would return a pointer to 511 to the caller, which is most definitely not useful and would very likely result in a crash. That makes me wonder you have actually tested this patch. On top of that, 511 or 0x1ff is a perfectly valid return value. I find zero evidence in the datasheet suggesting that a returned value of 0x1ff would suggest an invalid value. More specifically, the datasheet does not say if bit 0..3 are returned as 0 or as 1. > goto abort; > data->in[0][i] = rv >> 4; > > -- > 2.25.1 > > WIWYNN PROPRIETARY > This email (and any attachments) contains proprietary or confidential information and is for the sole use of its intended recipient. Any unauthorized review, use, copying or distribution of this email or the content of this email is strictly prohibited. If you are not the intended recipient, please notify the sender and delete this email immediately. That means that I would not be able to apply the patch anyway, even if it was perfect. Guenter
diff --git a/drivers/hwmon/adc128d818.c b/drivers/hwmon/adc128d818.c index 5e805d4ee76a..8197d3f14ea7 100644 --- a/drivers/hwmon/adc128d818.c +++ b/drivers/hwmon/adc128d818.c @@ -88,7 +88,7 @@ static struct adc128_data *adc128_update_device(struct device *dev) for (i = 0; i < num_inputs[data->mode]; i++) { rv = i2c_smbus_read_word_swapped(client, ADC128_REG_IN(i)); - if (rv < 0) + if (rv < 0 || rv == 511) goto abort; data->in[0][i] = rv >> 4;
ASPEED i2c driver doesn't support SMBUS timeout thus we will get dirty data while SCL hold time over 35ms, filter out default register value 0x1ff to avoid abnormal data reading. Signed-off-by: Jerry C Chen <Jerry_C_Chen@wiwynn.com> --- drivers/hwmon/adc128d818.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- 2.25.1 WIWYNN PROPRIETARY This email (and any attachments) contains proprietary or confidential information and is for the sole use of its intended recipient. Any unauthorized review, use, copying or distribution of this email or the content of this email is strictly prohibited. If you are not the intended recipient, please notify the sender and delete this email immediately.