Hi Juerg,
On Thu, 24 Jan 2008 09:11:25 -0800, Juerg Haefliger wrote:
> This patch fixes a possible divide-by-0.
>
> Signed-off-by: Juerg Haefliger <juergh at gmail.com>
>
> Index: linux-2.6.24-rc8/drivers/hwmon/dme1737.c
> ===================================================================
> --- linux-2.6.24-rc8.orig/drivers/hwmon/dme1737.c 2008-01-24 09:04:24.000000000 -0800
> +++ linux-2.6.24-rc8/drivers/hwmon/dme1737.c 2008-01-24 09:05:05.000000000 -0800
> @@ -285,8 +285,8 @@
>
> static inline int FAN_TO_REG(int val, int tpc)
> {
> - return SENSORS_LIMIT((tpc == 0) ? 90000 * 60 / val : val / tpc,
> - 0, 0xffff);
> + return (val <= 0) ? 0xffff : SENSORS_LIMIT((tpc == 0) ?
> + 90000 * 60 / val : val / tpc, 0, 0xfffe);
> }
>
> /* Fan TPC (tach pulse count)
While I agree that it fixes the division by 0, and the fix looks OK in
legacy mode, I am not sure that it is correct in TPC mode. While
register value 0xffff corresponds to a stalled fan in legacy mode,
that's not the case in TPC mode according to the datasheet. In TPC
mode, a stalled fan leads to a register value of 0x0000. So I believe
that the driver should write 0x0000, not 0xffff, to the register if the
user sets the limit to 0 RPM in TPC mode. This would lead to the
following code (untested):
if (tpc)
return SENSORS_LIMIT(val / tpc, 0, 0xffff);
else
return (val <= 0) ? 0xffff :
SENSORS_LIMIT(90000 * 60 / val, 0, 0xfffe);
What do you think?
(I wish manufacturers could come up with more simple chips...)
--
Jean Delvare
