[U-Boot] [PATCH v2] memsize: Fix for bug in memory sizing code

Wolfgang Denk wd at denx.de
Tue Oct 21 22:14:29 CEST 2014


Dear Gerd Hoffmann,

In message <1413910153-5907-1-git-send-email-kraxel at 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: wd at denx.de
Q:  Why do mountain climbers rope themselves together?
A:  To prevent the sensible ones from going home.


More information about the U-Boot mailing list