[U-Boot] [PATCH] improved strmhz()

[U-Boot] [PATCH] improved strmhz()

[Ilko Iliev]

This patch prevents the displaying of results like this:
hz = 1999170000
buf = "200.-83"

Signed-off-by: Ilko Iliev <iliev@ronetix.at>

index 342cf2b..981a75a 100644
--- a/lib_generic/strmhz.c
+++ b/lib_generic/strmhz.c
@@ -28,10 +28,15 @@ char *strmhz (char *buf, long hz)
        long m;

        n = DIV_ROUND(hz, 1000000L);
-       l = sprintf (buf, "%ld", n);

        hz -= n * 1000000L;
        m = DIV_ROUND(hz, 1000L);
+       if ( m < 0 ) {
+               n--;
+               m += 1000L;
+       }
+
+       l = sprintf (buf, "%ld", n);
        if (m != 0)
                sprintf (buf + l, ".%03ld", m);
        return (buf);

[U-Boot] [PATCH] improved strmhz()

[Wolfgang Denk]

Dear Ilko Iliev,

In message <48FDCEB6.9040206@ronetix.at> you wrote:
> This patch prevents the displaying of results like this:
> hz = 1999170000
> buf = "200.-83"

I think you got one '0' too many in your example - the example would
just print "1999.170" which is correct.

> --- a/lib_generic/strmhz.c
> +++ b/lib_generic/strmhz.c
> @@ -28,10 +28,15 @@ char *strmhz (char *buf, long hz)
>         long m;
> 
>         n = DIV_ROUND(hz, 1000000L);
> -       l = sprintf (buf, "%ld", n);
> 
>         hz -= n * 1000000L;
>         m = DIV_ROUND(hz, 1000L);
> +       if ( m < 0 ) {
> +               n--;
> +               m += 1000L;
> +       }
> +
> +       l = sprintf (buf, "%ld", n);
>         if (m != 0)
>                 sprintf (buf + l, ".%03ld", m);
>         return (buf);

That looks overly complex to me. Can you please check if this patch
fixes the problem for your test cases, too:

[U-Boot] [PATCH] improved strmhz()

[Ilko Iliev]

Wolfgang Denk wrote:
> Dear Ilko Iliev,
>
> In message <48FDCEB6.9040206@ronetix.at> you wrote:
>   
>> This patch prevents the displaying of results like this:
>> hz = 1999170000
>> buf = "200.-83"
>>     
>
> I think you got one '0' too many in your example - the example would
> just print "1999.170" which is correct.
>
>   
>> --- a/lib_generic/strmhz.c
>> +++ b/lib_generic/strmhz.c
>> @@ -28,10 +28,15 @@ char *strmhz (char *buf, long hz)
>>         long m;
>>
>>         n = DIV_ROUND(hz, 1000000L);
>> -       l = sprintf (buf, "%ld", n);
>>
>>         hz -= n * 1000000L;
>>         m = DIV_ROUND(hz, 1000L);
>> +       if ( m < 0 ) {
>> +               n--;
>> +               m += 1000L;
>> +       }
>> +
>> +       l = sprintf (buf, "%ld", n);
>>         if (m != 0)
>>                 sprintf (buf + l, ".%03ld", m);
>>         return (buf);
>>     
>
> That looks overly complex to me. Can you please check if this patch
> fixes the problem for your test cases, too:
>   
Yes, it works.



-- 
Mit freundlichen Gr??en/With best regards,
Ilko Iliev
Ronetix Development Tools GmbH
Waidhausenstrasse 13/5
1140 Vienna, Austria
Tel: +43 1 956 3138 
Tel: +43 720 500 315
Fax: +43 1 8174 955 3464 
E-Mail: iliev at ronetix.at
Web: www.ronetix.at
VAT: ATU63916016
Ronetix GmbH - 1140 Vienna - Gesch?ftsf?hrer: Ilko Iliev
Registergericht: HG Vienna, FN 304979z

[U-Boot] [PATCH] improved strmhz()

[Kumar Gala]

>>
> That looks overly complex to me. Can you please check if this patch
> fixes the problem for your test cases, too:
>
>> From 963e7db81379225b78bfac0d7457300c86d6b4d6 Mon Sep 17 00:00:00  
>> 2001
> From: Wolfgang Denk <wd@denx.de>
> Date: Tue, 21 Oct 2008 15:53:51 +0200
> Subject: [PATCH] Fix strmhz(): avoid printing negative fractions
>
> Signed-off-by: Wolfgang Denk <wd@denx.de>
> ---
> lib_generic/strmhz.c |    2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/lib_generic/strmhz.c b/lib_generic/strmhz.c
> index 342cf2b..d6da1d1 100644
> --- a/lib_generic/strmhz.c
> +++ b/lib_generic/strmhz.c
> @@ -27,7 +27,7 @@ char *strmhz (char *buf, long hz)
> 	long l, n;
> 	long m;
>
> -	n = DIV_ROUND(hz, 1000000L);
> +	n = DIV_ROUND(hz, 1000) / 1000L;
> 	l = sprintf (buf, "%ld", n);
>
> 	hz -= n * 1000000L;
> -- 
> 1.5.5.1

I haven't been following this thread, but can we control the number of  
significant digits.  I'm starting to see output like:

Clock Configuration:
        CPU:1500.4294967282 MHz, CCB:600.4294967291 MHz,
        DDR:400.4294967293 MHz (800.4294967289 MT/s data rate)  
(Asynchronous), LBC:37.500 MHz

(it use to look like)

Clock Configuration:
        CPU:1500 MHz, CCB: 600 MHz,
        DDR: 401 MHz (801 MT/s data rate) (Asynchronous), LBC:  37 MHz

- k

[U-Boot] [PATCH] improved strmhz()

[Jerry Van Baren]

Kumar Gala wrote:
>> That looks overly complex to me. Can you please check if this patch
>> fixes the problem for your test cases, too:
>>
>>> From 963e7db81379225b78bfac0d7457300c86d6b4d6 Mon Sep 17 00:00:00  
>>> 2001
>> From: Wolfgang Denk <wd@denx.de>
>> Date: Tue, 21 Oct 2008 15:53:51 +0200
>> Subject: [PATCH] Fix strmhz(): avoid printing negative fractions
>>
>> Signed-off-by: Wolfgang Denk <wd@denx.de>
>> ---
>> lib_generic/strmhz.c |    2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/lib_generic/strmhz.c b/lib_generic/strmhz.c
>> index 342cf2b..d6da1d1 100644
>> --- a/lib_generic/strmhz.c
>> +++ b/lib_generic/strmhz.c
>> @@ -27,7 +27,7 @@ char *strmhz (char *buf, long hz)
>> 	long l, n;
>> 	long m;
>>
>> -	n = DIV_ROUND(hz, 1000000L);
>> +	n = DIV_ROUND(hz, 1000) / 1000L;
>> 	l = sprintf (buf, "%ld", n);
>>
>> 	hz -= n * 1000000L;
>> -- 
>> 1.5.5.1
> 
> I haven't been following this thread, but can we control the number of  
> significant digits.  I'm starting to see output like:
> 
> Clock Configuration:
>         CPU:1500.4294967282 MHz, CCB:600.4294967291 MHz,
>         DDR:400.4294967293 MHz (800.4294967289 MT/s data rate)  

(unsigned) 4294967289 = (signed) -7 (if I did my math right).  This is 
indicating Wolfgang's patch still does negative remainders, but masks 
that by printing them as unsigned numbers.

Best regards,
gvb

[U-Boot] [PATCH] improved strmhz()

[Mike Frysinger]

On Tuesday 21 October 2008, Kumar Gala wrote:
> I haven't been following this thread, but can we control the number of
> significant digits.

customizable # of digits is always going to differ according to taste.  just 
lock everyone to like .3 and be done.  no point in wasting overhead on this.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 835 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20081021/447b3b38/attachment.pgp 

[U-Boot] [PATCH] improved strmhz()

[Wolfgang Denk]

Dear Kumar Gala,

In message <0E4961A3-232C-4307-91E3-B2F7CB216BFA@kernel.crashing.org> you wrote:
>
> > --- a/lib_generic/strmhz.c
> > +++ b/lib_generic/strmhz.c
> > @@ -27,7 +27,7 @@ char *strmhz (char *buf, long hz)
> > 	long l, n;
> > 	long m;
> >
> > -	n = DIV_ROUND(hz, 1000000L);
> > +	n = DIV_ROUND(hz, 1000) / 1000L;
> > 	l = sprintf (buf, "%ld", n);
> >
> > 	hz -= n * 1000000L;
> > -- 
> > 1.5.5.1
> 
> I haven't been following this thread, but can we control the number of  
> significant digits.  I'm starting to see output like:
> 
> Clock Configuration:
>         CPU:1500.4294967282 MHz, CCB:600.4294967291 MHz,
>         DDR:400.4294967293 MHz (800.4294967289 MT/s data rate)  
> (Asynchronous), LBC:37.500 MHz
> 
> (it use to look like)
> 
> Clock Configuration:
>         CPU:1500 MHz, CCB: 600 MHz,
>         DDR: 401 MHz (801 MT/s data rate) (Asynchronous), LBC:  37 MHz

Can you please provide the input to the strmhz() function  (the  "hz"
parameter) that is causing such output?

I am aware that we will have a problem when "hz" exceeds the 2.147 GHz
limit (as that's the limit for a "long" data type), but that's another
problem, and requires many changes.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
We are all agreed that your  theory  is  crazy.  The  question  which
divides  us  is  whether it is crazy enough to have a chance of being
correct. My own feeling is that it is not crazy enough.  - Niels Bohr

[U-Boot] [PATCH] improved strmhz()

[Stefan Roese]

Hi Wolfgang,

On Tuesday 21 October 2008, Wolfgang Denk wrote:
> > > --- a/lib_generic/strmhz.c
> > > +++ b/lib_generic/strmhz.c
> > > @@ -27,7 +27,7 @@ char *strmhz (char *buf, long hz)
> > > 	long l, n;
> > > 	long m;
> > >
> > > -	n = DIV_ROUND(hz, 1000000L);
> > > +	n = DIV_ROUND(hz, 1000) / 1000L;
> > > 	l = sprintf (buf, "%ld", n);
> > >
> > > 	hz -= n * 1000000L;

<snip>

> Can you please provide the input to the strmhz() function  (the  "hz"
> parameter) that is causing such output?

This patch fixes a problem I have seen on the 1.066GHz Canyonlands. Without it 
I see this:

CPU:   AMCC PowerPC 460EX Rev. A at 1067.4294966964 MHz

with it it's this:

CPU:   AMCC PowerPC 460EX Rev. A at 1066.667 MHz

Wolfgang, can you check in the new patch version?

Thanks.

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================