Dear Gerd Hoffmann,

In message <1413910153-5907-1-git-send-email-kra...@redhat.com> you wrote:
> The original memory sizing code in get_ram_size clobbers the word
> at the base address, but forgets to restore it.

The funny thing is that it avtually works on some boards.  Do you have
an explanation for that?

> -     long           save[32];
> +     long           save[32], save_base;

Why do you need another variable? The original code stores the value
as last entry in save[] - what's wrong with that?

> +     save_base = base[0];
> +     sync ();

You add this code here, but...

>       for (cnt = (maxsize / sizeof (long)) >> 1; cnt > 0; cnt >>= 1) {
>               addr = base + cnt;      /* pointer arith! */
>               sync ();
> @@ -43,8 +46,6 @@ long get_ram_size(long *base, long maxsize)
>  
>       addr = base;
>       sync ();
> -     save[i] = *addr;
> -     sync ();

...remove it's equivalent here.  Why would your code be any better
than the existing one?

> -             *addr = save[i];
>               for (cnt = 1; cnt < maxsize / sizeof(long); cnt <<= 1) {
>                       addr  = base + cnt;
>                       sync ();
>                       *addr = save[--i];
>               }
> +             base[0] = save_base;

Same here - the line you removed above does the very same as the one
you add here.  In which way is the new code supposed to be different
or even better?

> @@ -73,10 +74,12 @@ long get_ram_size(long *base, long maxsize)
>                               addr  = base + cnt;
>                               *addr = save[--i];
>                       }
> +                     base[0] = save_base;
>                       return (size);
>               }
>       }
>  
> +     base[0] = save_base;
>       return (maxsize);

Agreed here. These two make sense to me. I still wonder why it works
on the boards I used for testing, while it's failing on yours.

Which exit path are you taking? The one at the end of the function,
i. e. "return (maxsize)" ?  What happens if you double the memory
size to be checked?


I'll resend a slightly reworked patch in a minute; could you please
test if this works for you, too?

Thanks.

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: w...@denx.de
Q:  Why do mountain climbers rope themselves together?
A:  To prevent the sensible ones from going home.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to