linux-kernel - Re: [lm-sensors] [PATCH] hwmon: sht15: Fix sht15_calc

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1732720040.861270126460533.JavaMail.root@mail.savoirfairelinux.com>
Date:	Thu, 1 Apr 2010 08:54:20 -0400 (EDT)
From:	Jerome Oufella <jerome.oufella@...oirfairelinux.com>
To:	Jean Delvare <khali@...ux-fr.org>
Cc:	lm-sensors@...sensors.org, linux-kernel@...r.kernel.org,
	Jonathan Cameron <jic23@....ac.uk>
Subject: Re: [lm-sensors] [PATCH] hwmon: sht15: Fix sht15_calc_temp
  interpolation function

----- "Jean Delvare" <khali@...ux-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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/