All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jerome Oufella <jerome.oufella@savoirfairelinux.com>
To: Jean Delvare <khali@linux-fr.org>
Cc: lm-sensors@lm-sensors.org, linux-kernel@vger.kernel.org,
	Jonathan Cameron <jic23@cam.ac.uk>
Subject: Re: [lm-sensors] [PATCH] hwmon: sht15: Fix sht15_calc_temp
Date: Thu, 01 Apr 2010 12:54:20 +0000	[thread overview]
Message-ID: <1732720040.861270126460533.JavaMail.root@mail.savoirfairelinux.com> (raw)
In-Reply-To: <20100401140152.3141b989@hyperion.delvare>
In-Reply-To: <1269639013-26029-1-git-send-email-jerome.oufella@savoirfairelinux.com>

----- "Jean Delvare" <khali@linux-fr.org> wrote :
> May I suggest the more simple fix below?
> 
> ---
>  drivers/hwmon/sht15.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> --- linux-2.6.34-rc3.orig/drivers/hwmon/sht15.c	2010-04-01
> 13:41:15.000000000 +0200
> +++ linux-2.6.34-rc3/drivers/hwmon/sht15.c	2010-04-01
> 13:41:55.000000000 +0200
> @@ -305,10 +305,10 @@ static inline int sht15_calc_temp(struct
>  	int d1 = 0;
>  	int i;
>  
> -	for (i = 1; i < ARRAY_SIZE(temppoints); i++)
> +	for (i = ARRAY_SIZE(temppoints) - 1; i > 0 ;i--)
>  		/* Find pointer to interpolate */
>  		if (data->supply_uV > temppoints[i - 1].vdd) {
> -			d1 = (data->supply_uV/1000 - temppoints[i - 1].vdd)
> +			d1 = (data->supply_uV - temppoints[i - 1].vdd)
>  				* (temppoints[i].d1 - temppoints[i - 1].d1)
>  				/ (temppoints[i].vdd - temppoints[i - 1].vdd)
>  				+ temppoints[i - 1].d1;
> 
> It leads to the same numbers as with Jerome's patch, with the
> advantages that 1* it is a much smaller change, more suitable for
> applying to stable kernels and 2* it avoids the magic constant number
> 10000.
> 
> The "/1000" seems to be a relict of former times when
> temppoints[*].vdd
> was probably expressed in millivolt instead of microvolt. And the
> inverted loop iteration is obviously a bug.
> 
> Note that in both cases, something should be done about values which
> are outside of the temppoints array. I don't know how likely these
> are,
> but they are seriously mishandled. For supply_uV values below
> temppoints[0].vdd, d1 defaults to 0, so no adjustment is done at all.
> temppoints[0].d1 would seem to be a much better default, if we don't
> want to do any interpolation in that case. For supply_uV values above
> temppoints[4].vdd, we do interpolate, which seems reasonable.
> 
> Opinions?
> 
> -- 
> Jean Delvare

That's fine, it does a good job for me, in the expected voltage range.

Jerome Oufella

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

WARNING: multiple messages have this Message-ID (diff)
From: Jerome Oufella <jerome.oufella@savoirfairelinux.com>
To: Jean Delvare <khali@linux-fr.org>
Cc: lm-sensors@lm-sensors.org, linux-kernel@vger.kernel.org,
	Jonathan Cameron <jic23@cam.ac.uk>
Subject: Re: [lm-sensors] [PATCH] hwmon: sht15: Fix sht15_calc_temp interpolation function
Date: Thu, 1 Apr 2010 08:54:20 -0400 (EDT)	[thread overview]
Message-ID: <1732720040.861270126460533.JavaMail.root@mail.savoirfairelinux.com> (raw)
In-Reply-To: <20100401140152.3141b989@hyperion.delvare>

----- "Jean Delvare" <khali@linux-fr.org> wrote :
> May I suggest the more simple fix below?
> 
> ---
>  drivers/hwmon/sht15.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> --- linux-2.6.34-rc3.orig/drivers/hwmon/sht15.c	2010-04-01
> 13:41:15.000000000 +0200
> +++ linux-2.6.34-rc3/drivers/hwmon/sht15.c	2010-04-01
> 13:41:55.000000000 +0200
> @@ -305,10 +305,10 @@ static inline int sht15_calc_temp(struct
>  	int d1 = 0;
>  	int i;
>  
> -	for (i = 1; i < ARRAY_SIZE(temppoints); i++)
> +	for (i = ARRAY_SIZE(temppoints) - 1; i > 0 ;i--)
>  		/* Find pointer to interpolate */
>  		if (data->supply_uV > temppoints[i - 1].vdd) {
> -			d1 = (data->supply_uV/1000 - temppoints[i - 1].vdd)
> +			d1 = (data->supply_uV - temppoints[i - 1].vdd)
>  				* (temppoints[i].d1 - temppoints[i - 1].d1)
>  				/ (temppoints[i].vdd - temppoints[i - 1].vdd)
>  				+ temppoints[i - 1].d1;
> 
> It leads to the same numbers as with Jerome's patch, with the
> advantages that 1* it is a much smaller change, more suitable for
> applying to stable kernels and 2* it avoids the magic constant number
> 10000.
> 
> The "/1000" seems to be a relict of former times when
> temppoints[*].vdd
> was probably expressed in millivolt instead of microvolt. And the
> inverted loop iteration is obviously a bug.
> 
> Note that in both cases, something should be done about values which
> are outside of the temppoints array. I don't know how likely these
> are,
> but they are seriously mishandled. For supply_uV values below
> temppoints[0].vdd, d1 defaults to 0, so no adjustment is done at all.
> temppoints[0].d1 would seem to be a much better default, if we don't
> want to do any interpolation in that case. For supply_uV values above
> temppoints[4].vdd, we do interpolate, which seems reasonable.
> 
> Opinions?
> 
> -- 
> Jean Delvare

That's fine, it does a good job for me, in the expected voltage range.

Jerome Oufella

  parent reply	other threads:[~2010-04-01 12:54 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-03-26 21:30 [lm-sensors] [PATCH] hwmon: sht15: Fix sht15_calc_temp Jerome Oufella
2010-03-29 16:23 ` Jerome Oufella
2010-03-29 18:42 ` Jonathan Cameron
2010-03-29 19:58 ` Jerome Oufella
2010-03-30 11:36 ` Jonathan Cameron
2010-03-30 14:00 ` Jerome Oufella
2010-03-30 14:00 ` Jerome Oufella
2010-04-01 12:01 ` Jean Delvare
2010-04-01 12:01   ` [lm-sensors] [PATCH] hwmon: sht15: Fix sht15_calc_temp interpolation function Jean Delvare
2010-04-01 12:54 ` Jerome Oufella [this message]
2010-04-01 12:54   ` Jerome Oufella
2010-04-01 13:49   ` [lm-sensors] [PATCH] hwmon: sht15: Fix sht15_calc_temp Jonathan Cameron
2010-04-01 13:49     ` [lm-sensors] [PATCH] hwmon: sht15: Fix sht15_calc_temp interpolation function Jonathan Cameron
  -- strict thread matches above, loose matches on Subject: below --
2010-04-01 15:02 [lm-sensors] [PATCH] hwmon: (sht15) Fix sht15_calc_temp Jean Delvare

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1732720040.861270126460533.JavaMail.root@mail.savoirfairelinux.com \
    --to=jerome.oufella@savoirfairelinux.com \
    --cc=jic23@cam.ac.uk \
    --cc=khali@linux-fr.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lm-sensors@lm-sensors.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.