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

Dear Gerd Hoffmann,

In message 1413910153-5907-1-git-send-email-kraxel@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