diff mbox series

hwmon: (lm90) Fix false alarm when writing convrate on max6658

Message ID 20210304153832.19275-1-pmenzel@molgen.mpg.de (mailing list archive)
State Rejected
Headers show
Series hwmon: (lm90) Fix false alarm when writing convrate on max6658 | expand

Commit Message

Paul Menzel March 4, 2021, 3:38 p.m. UTC 
From: Boyang Yu <byu@arista.com>

We found that the max6658 sometimes issues a false alarm when its
convrate is changed, with the current hwmon driver. This workaround
will fix it by stopping the conversion before setting the convrate.

Upstream the patch added added to the SONiC Linux kernel in merge/pull
request #82 [1][2].

[1]: https://github.com/Azure/sonic-linux-kernel/pull/82
[2]: https://github.com/Azure/sonic-linux-kernel/commit/d03f6167f64b2bfa1330ff7b33fe217f68ab7028

[pmenzel: Forward port patch from 4.19 to 5.12-rc1+]
Signed-off-by: Paul Menzel <pmenzel@molgen.mpg.de>
---
 drivers/hwmon/lm90.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

Comments

Guenter Roeck March 11, 2021, 10:15 p.m. UTC | #1 
On Thu, Mar 04, 2021 at 04:38:32PM +0100, Paul Menzel wrote:
> From: Boyang Yu <byu@arista.com>
> 
> We found that the max6658 sometimes issues a false alarm when its
> convrate is changed, with the current hwmon driver. This workaround
> will fix it by stopping the conversion before setting the convrate.
> 
> Upstream the patch added added to the SONiC Linux kernel in merge/pull
> request #82 [1][2].
> 
> [1]: https://github.com/Azure/sonic-linux-kernel/pull/82
> [2]: https://github.com/Azure/sonic-linux-kernel/commit/d03f6167f64b2bfa1330ff7b33fe217f68ab7028
> 
> [pmenzel: Forward port patch from 4.19 to 5.12-rc1+]
> Signed-off-by: Paul Menzel <pmenzel@molgen.mpg.de>
> ---
>  drivers/hwmon/lm90.c | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/hwmon/lm90.c b/drivers/hwmon/lm90.c
> index ebbfd5f352c0..0c1a91b715e8 100644
> --- a/drivers/hwmon/lm90.c
> +++ b/drivers/hwmon/lm90.c
> @@ -639,7 +639,11 @@ static int lm90_set_convrate(struct i2c_client *client, struct lm90_data *data,
>  		if (interval >= update_interval * 3 / 4)
>  			break;
>  
> -	err = lm90_write_convrate(data, i);
> +	if (data->kind == max6657)
> +		err = max6657_write_convrate(client, i);

There is no such function in the upstream kernel.
Anyway, this problem has already been already fixed
in the upstream kernel with commit 62456189f3292 ("hwmon:
(lm90) Fix max6658 sporadic wrong temperature reading").

Guenter

> +	else
> +		err = lm90_write_convrate(data, i);
> +
>  	data->update_interval = DIV_ROUND_CLOSEST(update_interval, 64);
>  	return err;
>  }
> @@ -1649,7 +1653,11 @@ static void lm90_restore_conf(void *_data)
>  	struct i2c_client *client = data->client;
>  
>  	/* Restore initial configuration */
> -	lm90_write_convrate(data, data->convrate_orig);
> +	if (data->kind == max6657)
> +		max6657_write_convrate(client, data->convrate_orig);
> +	else
> +		lm90_write_convrate(data, data->convrate_orig);
> +
>  	i2c_smbus_write_byte_data(client, LM90_REG_W_CONFIG1,
>  				  data->config_orig);
>  }
> @@ -1672,7 +1680,8 @@ static int lm90_init_client(struct i2c_client *client, struct lm90_data *data)
>  	data->config_orig = config;
>  	data->config = config;
>  
> -	lm90_set_convrate(client, data, 500); /* 500ms; 2Hz conversion rate */
> +	if (data->kind != max6657)
> +		lm90_set_convrate(client, data, 500); /* 500ms; 2Hz conversion rate */
>  
>  	/* Check Temperature Range Select */
>  	if (data->kind == adt7461 || data->kind == tmp451) {
Paul Menzel March 12, 2021, 7:48 a.m. UTC | #2 
Dear Guenter,


Thank you for your reply.

Am 11.03.21 um 23:15 schrieb Guenter Roeck:
> On Thu, Mar 04, 2021 at 04:38:32PM +0100, Paul Menzel wrote:
>> From: Boyang Yu <byu@arista.com>
>>
>> We found that the max6658 sometimes issues a false alarm when its
>> convrate is changed, with the current hwmon driver. This workaround
>> will fix it by stopping the conversion before setting the convrate.
>>
>> Upstream the patch added added to the SONiC Linux kernel in merge/pull
>> request #82 [1][2].
>>
>> [1]: https://github.com/Azure/sonic-linux-kernel/pull/82
>> [2]: https://github.com/Azure/sonic-linux-kernel/commit/d03f6167f64b2bfa1330ff7b33fe217f68ab7028
>>
>> [pmenzel: Forward port patch from 4.19 to 5.12-rc1+]
>> Signed-off-by: Paul Menzel <pmenzel@molgen.mpg.de>
>> ---
>>   drivers/hwmon/lm90.c | 15 ++++++++++++---
>>   1 file changed, 12 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/hwmon/lm90.c b/drivers/hwmon/lm90.c
>> index ebbfd5f352c0..0c1a91b715e8 100644
>> --- a/drivers/hwmon/lm90.c
>> +++ b/drivers/hwmon/lm90.c
>> @@ -639,7 +639,11 @@ static int lm90_set_convrate(struct i2c_client *client, struct lm90_data *data,
>>   		if (interval >= update_interval * 3 / 4)
>>   			break;
>>   
>> -	err = lm90_write_convrate(data, i);
>> +	if (data->kind == max6657)
>> +		err = max6657_write_convrate(client, i);
> 
> There is no such function in the upstream kernel.

Hmm, why did that hunk get lost when preparing the patch? I am sorry for 
this oversight.

> Anyway, this problem has already been already fixed
> in the upstream kernel with commit 62456189f3292 ("hwmon:
> (lm90) Fix max6658 sporadic wrong temperature reading").

Ah, indeed. Thank you for pointing that out. It’d be great to have that 
included in the 4.19 LTS series. Would it be alright, if I contacted the 
stable series maintainers?


Kind regards,

Paul


>> +	else
>> +		err = lm90_write_convrate(data, i);
>> +
>>   	data->update_interval = DIV_ROUND_CLOSEST(update_interval, 64);
>>   	return err;
>>   }
>> @@ -1649,7 +1653,11 @@ static void lm90_restore_conf(void *_data)
>>   	struct i2c_client *client = data->client;
>>   
>>   	/* Restore initial configuration */
>> -	lm90_write_convrate(data, data->convrate_orig);
>> +	if (data->kind == max6657)
>> +		max6657_write_convrate(client, data->convrate_orig);
>> +	else
>> +		lm90_write_convrate(data, data->convrate_orig);
>> +
>>   	i2c_smbus_write_byte_data(client, LM90_REG_W_CONFIG1,
>>   				  data->config_orig);
>>   }
>> @@ -1672,7 +1680,8 @@ static int lm90_init_client(struct i2c_client *client, struct lm90_data *data)
>>   	data->config_orig = config;
>>   	data->config = config;
>>   
>> -	lm90_set_convrate(client, data, 500); /* 500ms; 2Hz conversion rate */
>> +	if (data->kind != max6657)
>> +		lm90_set_convrate(client, data, 500); /* 500ms; 2Hz conversion rate */
>>   
>>   	/* Check Temperature Range Select */
>>   	if (data->kind == adt7461 || data->kind == tmp451) {
Guenter Roeck March 12, 2021, 2:48 p.m. UTC | #3 
On 3/11/21 11:48 PM, Paul Menzel wrote:
> Dear Guenter,
> 
> 
> Thank you for your reply.
> 
> Am 11.03.21 um 23:15 schrieb Guenter Roeck:
>> On Thu, Mar 04, 2021 at 04:38:32PM +0100, Paul Menzel wrote:
>>> From: Boyang Yu <byu@arista.com>
>>>
>>> We found that the max6658 sometimes issues a false alarm when its
>>> convrate is changed, with the current hwmon driver. This workaround
>>> will fix it by stopping the conversion before setting the convrate.
>>>
>>> Upstream the patch added added to the SONiC Linux kernel in merge/pull
>>> request #82 [1][2].
>>>
>>> [1]: https://github.com/Azure/sonic-linux-kernel/pull/82
>>> [2]: https://github.com/Azure/sonic-linux-kernel/commit/d03f6167f64b2bfa1330ff7b33fe217f68ab7028
>>>
>>> [pmenzel: Forward port patch from 4.19 to 5.12-rc1+]
>>> Signed-off-by: Paul Menzel <pmenzel@molgen.mpg.de>
>>> ---
>>>   drivers/hwmon/lm90.c | 15 ++++++++++++---
>>>   1 file changed, 12 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/hwmon/lm90.c b/drivers/hwmon/lm90.c
>>> index ebbfd5f352c0..0c1a91b715e8 100644
>>> --- a/drivers/hwmon/lm90.c
>>> +++ b/drivers/hwmon/lm90.c
>>> @@ -639,7 +639,11 @@ static int lm90_set_convrate(struct i2c_client *client, struct lm90_data *data,
>>>           if (interval >= update_interval * 3 / 4)
>>>               break;
>>>   -    err = lm90_write_convrate(data, i);
>>> +    if (data->kind == max6657)
>>> +        err = max6657_write_convrate(client, i);
>>
>> There is no such function in the upstream kernel.
> 
> Hmm, why did that hunk get lost when preparing the patch? I am sorry for this oversight.
> 
No worries.

>> Anyway, this problem has already been already fixed
>> in the upstream kernel with commit 62456189f3292 ("hwmon:
>> (lm90) Fix max6658 sporadic wrong temperature reading").
> 
> Ah, indeed. Thank you for pointing that out. It’d be great to have that included in the 4.19 LTS series. Would it be alright, if I contacted the stable series maintainers?
> 

Sure.

Guenter
Paul Menzel March 12, 2021, 4:53 p.m. UTC | #4 
Dear Linux folks,


Could you please apply commit 62456189f3292c62f87aef363f204886dc1d4b48 
to the Linux 4.19 stable series?


Kind regards,

Paul
Guenter Roeck March 12, 2021, 7:24 p.m. UTC | #5 
On Fri, Mar 12, 2021 at 05:53:49PM +0100, Paul Menzel wrote:
> Dear Linux folks,
> 
> 
> Could you please apply commit 62456189f3292c62f87aef363f204886dc1d4b48 to
> the Linux 4.19 stable series?
> 

v4.9.y..v1.19.y, actually, please.

Thanks,
Guenter
Greg KH March 15, 2021, 9:30 a.m. UTC | #6 
On Fri, Mar 12, 2021 at 11:24:57AM -0800, Guenter Roeck wrote:
> On Fri, Mar 12, 2021 at 05:53:49PM +0100, Paul Menzel wrote:
> > Dear Linux folks,
> > 
> > 
> > Could you please apply commit 62456189f3292c62f87aef363f204886dc1d4b48 to
> > the Linux 4.19 stable series?
> > 
> 
> v4.9.y..v1.19.y, actually, please.

Now done, thanks.

greg k-h
diff mbox series

Patch

diff --git a/drivers/hwmon/lm90.c b/drivers/hwmon/lm90.c
index ebbfd5f352c0..0c1a91b715e8 100644
--- a/drivers/hwmon/lm90.c
+++ b/drivers/hwmon/lm90.c
@@ -639,7 +639,11 @@  static int lm90_set_convrate(struct i2c_client *client, struct lm90_data *data,
 		if (interval >= update_interval * 3 / 4)
 			break;
 
-	err = lm90_write_convrate(data, i);
+	if (data->kind == max6657)
+		err = max6657_write_convrate(client, i);
+	else
+		err = lm90_write_convrate(data, i);
+
 	data->update_interval = DIV_ROUND_CLOSEST(update_interval, 64);
 	return err;
 }
@@ -1649,7 +1653,11 @@  static void lm90_restore_conf(void *_data)
 	struct i2c_client *client = data->client;
 
 	/* Restore initial configuration */
-	lm90_write_convrate(data, data->convrate_orig);
+	if (data->kind == max6657)
+		max6657_write_convrate(client, data->convrate_orig);
+	else
+		lm90_write_convrate(data, data->convrate_orig);
+
 	i2c_smbus_write_byte_data(client, LM90_REG_W_CONFIG1,
 				  data->config_orig);
 }
@@ -1672,7 +1680,8 @@  static int lm90_init_client(struct i2c_client *client, struct lm90_data *data)
 	data->config_orig = config;
 	data->config = config;
 
-	lm90_set_convrate(client, data, 500); /* 500ms; 2Hz conversion rate */
+	if (data->kind != max6657)
+		lm90_set_convrate(client, data, 500); /* 500ms; 2Hz conversion rate */
 
 	/* Check Temperature Range Select */
 	if (data->kind == adt7461 || data->kind == tmp451) {