Bug 12646
| Summary: | w1_therm overflows for temperatures greater that 32.767 Celsius | ||
|---|---|---|---|
| Product: | Drivers | Reporter: | Ian Dall (ian) |
| Component: | W1 | Assignee: | Evgeniy Polyakov (johnpol) |
| Status: | CLOSED CODE_FIX | ||
| Severity: | normal | CC: | alan, kelfe |
| Priority: | P1 | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Kernel Version: | 2.6.29-rc3 | Subsystem: | |
| Regression: | No | Bisected commit-id: | |
| Attachments: |
Patch to fix overflow in temperature conversion
Fix problem with negative temperatures caused by previous patch. Take 2. Fix problem with negative temperatures caused by previous patch |
||
|
Description
Ian Dall
2009-02-07 04:53:18 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> Looks good, I will push it upstream. Thank you. 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> 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> 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/ ? |