[U-Boot-Users] Bug in DTT for temperatures below zero degress !!!

[U-Boot-Users] Bug in DTT for temperatures below zero degress !!!

[Steven Scholz]

Hi there,

there's a terrible bug in the current U-Boot CVS concerning DDT:

For temperature below zero the function dtt_get_temp() (at least for lm75) 
returns wrong values!

...
DTT:   1 is 235 C
LCD:    Hitachi TX14D11 5,7" 320x240 TFT (235?C)
WDT:    disabled

This is because of the use of an "int" although the lm75 only returns two 
byte ("signed short").

This trivial fix is needed:

Index: u-boot/dtt/lm75.c
===================================================================
RCS file: /cvsroot/u-boot/u-boot/dtt/lm75.c,v
retrieving revision 1.2
diff -u -r1.2 lm75.c
--- u-boot/dtt/lm75.c   27 Jun 2003 21:32:38 -0000      1.2
+++ u-boot/dtt/lm75.c   15 Jun 2005 08:44:21 -0000
@@ -174,7 +170,7 @@

  int dtt_get_temp(int sensor)
  {
-    return (dtt_read(sensor, DTT_READ_TEMP) / 256);
+    return ( ((signed short) dtt_read(sensor, DTT_READ_TEMP)) / 256);
  } /* dtt_get_temp() */

  #endif /* CONFIG_DTT_LM75 */

Then the output is correct:

...
DTT:   1 is -28 C
LCD:    Hitachi TX14D11 5,7" 320x240 TFT (-28?C)
CCFL:   disabled (too cold!)
WDT:    disabled

I am afraid this is an issues on all DTT in U-Boot. Some even only use "uchar".

Comments?

--
Steven

[U-Boot-Users] Bug in DTT for temperatures below zero degress !!!

[Wolfgang Denk]

In message <42AFEBD3.3050008@imc-berlin.de> you wrote:
> 
> there's a terrible bug in the current U-Boot CVS concerning DDT:
> 
> For temperature below zero the function dtt_get_temp() (at least for lm75) 
> returns wrong values!
> 
> ...
> DTT:   1 is 235 C
> LCD:    Hitachi TX14D11 5,7" 320x240 TFT (235?C)
> WDT:    disabled
> 
> This is because of the use of an "int" although the lm75 only returns two
> byte ("signed short").

Ummm...

> This trivial fix is needed:

Wait a moment...

> --- u-boot/dtt/lm75.c   27 Jun 2003 21:32:38 -0000      1.2
> +++ u-boot/dtt/lm75.c   15 Jun 2005 08:44:21 -0000
> @@ -174,7 +170,7 @@
> 
>   int dtt_get_temp(int sensor)
>   {
> -    return (dtt_read(sensor, DTT_READ_TEMP) / 256);
> +    return ( ((signed short) dtt_read(sensor, DTT_READ_TEMP)) / 256);

This is not the right way to fix it. I reject this patch.

dtt_read() is declared to return "int", so no  such  cast  should  be
needed, i. e. you fix the symptoms instead of the cause.


I think the patch must be done in dtt_read(); IMO the line

	return ((int)((short)data[1] + (((short)data[0]) << 8)));

is wrong; it should read

	return ((short)((short)data[1] + (((short)data[0]) << 8)));

or, even better,

	short rc_s;
	...
	rc_s = (short)data[1] + (((short)data[0]) << 8);
	return (rc_s);

And the same problem is with dlen == 1 - it should read

	return ((signed char)data[0]);


Do you agree with this assessment? And can you please confirm that my
suggested patch fixes your problem, too?

Best regards,

Wolfgang Denk

-- 
Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
Of all possible committee reactions to any  given  agenda  item,  the
reaction  that will occur is the one which will liberate the greatest
amount of hot air.                                -- Thomas L. Martin

[U-Boot-Users] Bug in DTT for temperatures below zero degress !!!

[Steven Scholz]

Wolfgang Denk wrote:

>> -    return (dtt_read(sensor, DTT_READ_TEMP) / 256);
>> +    return ( ((signed short) dtt_read(sensor, DTT_READ_TEMP)) / 256);
> 
> This is not the right way to fix it. I reject this patch.
> 
> dtt_read() is declared to return "int", so no  such  cast  should  be
> needed, i. e. you fix the symptoms instead of the cause.

You're probably right. But at the moment I have no access to suitable
hardware and I am off for a two weeks vacations today.
I promise to test this when I am back.

Thansk.

--
Steven