Bug 12646 - w1_therm overflows for temperatures greater that 32.767 Celsius
Summary: w1_therm overflows for temperatures greater that 32.767 Celsius
Status: CLOSED CODE_FIX
Alias: None
Product: Drivers
Classification: Unclassified
Component: W1 (show other bugs)
Hardware: All Linux
: P1 normal
Assignee: Evgeniy Polyakov
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-02-07 04:53 UTC by Ian Dall
Modified: 2012-05-30 12:55 UTC (History)
2 users (show)

See Also:
Kernel Version: 2.6.29-rc3
Tree: Mainline
Regression: No


Attachments
Patch to fix overflow in temperature conversion (421 bytes, patch)
2009-02-07 04:58 UTC, Ian Dall
Details | Diff
Fix problem with negative temperatures caused by previous patch. (500 bytes, patch)
2009-02-15 07:05 UTC, Ian Dall
Details | Diff
Take 2. Fix problem with negative temperatures caused by previous patch (502 bytes, patch)
2009-02-15 07:19 UTC, Ian Dall
Details | Diff

Description Ian Dall 2009-02-07 04:53:18 UTC
Latest working kernel version:Unknown
Earliest failing kernel version:2.6.29-rc3
Distribution:FC-8
Hardware Environment:i386, DS18B20 slave, DS9490R master.
Software Environment:
Problem Description:When the temperature exceeds 32767 milli-degrees the temperature overflows to -32768 millidegrees. These are bothe well within the -55 - +125 degree range for the sensor.

Steps to reproduce: Put your thumb on the sensor while periodically inspecting /sys/devices/w1\ bus\ master/*/w1_slave
Comment 1 Ian Dall 2009-02-07 04:58:47 UTC
Created attachment 20149 [details]
Patch to fix overflow in temperature conversion

Log:

w1_therm.c:w1_DS18B20_convert_temp(). t needs to be int, not s16, but must cast value to s16 before promoting to int so that sign is correct.

Signed-off-by: Ian Dall <ian@beware.dropbear.id.au>
Comment 2 Evgeniy Polyakov 2009-02-09 13:39:44 UTC
Looks good, I will push it upstream. Thank you.
Comment 3 Ian Dall 2009-02-15 07:05:49 UTC
Created attachment 20250 [details]
Fix problem with negative temperatures caused by previous patch.

The earlier patch attempted to preserve the sign extension, but I got it wrong. My only excuse is the high temperatures had affected my brain!

This is actually a synthesis following comments by Harvey Harrison <harvey.harrison@gmail.com> and Linus Torvalds <torvalds@linux-foundation.org>. I don't care if anyone wants to change the attribution.

Using le16_to_cpup actually generates faster code in little endian machines as well as being a bit more readable and perhaps portable to weird machines. It's a pity that there isn't a signed version (none of the approaches discussed work for negative numbers on non-twos complement machines if anyone cares).

This has been extensively tested with function level testing, so this time it should be right.

Log:
From: Ian Dall <ian@beware.dropbear.id.au>

Fix regression caused by commit 507e2fbaaacb6f164b4125b87c5002f95143174b, whereby negative temperatures for the DS18B20 are not converted properly.

Signed-of-by: Ian Dall <ian@beware.dropbear.id.au>
Comment 4 Ian Dall 2009-02-15 07:19:20 UTC
Created attachment 20252 [details]
Take 2. Fix problem with negative temperatures caused by previous patch

Sorry just ignore comment 3! I uploaded the wrong version of the patch. le16 is not defined in w1_therm.c.

The patch in comment 1 attempted to preserve the sign extension, but I got it wrong. My only excuse is the high temperatures had affected my brain!

This is actually a synthesis following comments by Harvey Harrison <harvey.harrison@gmail.com> and Linus Torvalds <torvalds@linux-foundation.org>. I don't care if anyone wants to change the attribution.

Using le16_to_cpup actually generates faster code in little endian machines as well as being a bit more readable and perhaps portable to weird machines. It's a pity that there isn't a signed version (none of the approaches discussed work for negative numbers on non-twos complement machines if anyone cares).

This has been extensively tested with function level testing, so this time it should be right.

Log:
From: Ian Dall <ian@beware.dropbear.id.au>

Fix regression caused by commit 507e2fbaaacb6f164b4125b87c5002f95143174b, whereby negative temperatures for the DS18B20 are not converted properly.

Signed-of-by: Ian Dall <ian@beware.dropbear.id.au>
Comment 5 Karsten Elfenbein 2010-03-05 11:28:47 UTC
the current mainline kernel still has a problem with negative temperatures (°C) the change in comment 4 fixes that

Is the w1 stuff going to be removed from the kernel in regard to userspace tools like http://owfs.org/ ?

Note You need to log in before you can comment on or make changes to this bug.