[U-Boot] [PATCH] [RFC] memsize.c: adapt get_ram_size() for address spaces >32 bit

Wolfgang Denk wd at denx.de
Thu May 27 21:44:42 CEST 2010


Dear Timur Tabi,

In message <AANLkTilrFhTwnQgQX9NVmplaBl8HpISZY97HKK73-ncz at mail.gmail.com> you wrote:
> On Thu, May 27, 2010 at 1:11 PM, Wolfgang Denk <wd at denx.de> wrote:
> > get_ram_size() used to use "long" data types for addresses and data,
> > which in limited it to systems with less than 4 GiB memory. As more
> > and more systems are coming up with bigger memory resources, we adapt
> > the code to use phys_addr_t / phys_size_t data types instead.
> >
> > Signed-off-by: Wolfgang Denk <wd at denx.de>
> > Cc: Timur Tabi <timur at freescale.com>
> > ---
> 
> The problem is that on all of our PowerPC boards, the TLBs only map
> the lower 2GB of memory, regardless as to how much is present.  So we
> still can't use get_ram_size() to determine how much memory is in the
> system, because any attempt to access memory higher than 2GB will
> fail.

Now this is your problem, then, and you should kno how to fix it.

> And even if we did have TLBs for all of memory, an attempt to access
> RAM that doesn't exist will cause a machine check, which will hang
> U-Boot.  So we still couldn't use get_ram_size() to determine how much
> RAM actually exists.

Please see how it's done on all other PowerPC systems, and do similar.

> > -long get_ram_size(volatile long *base, long maxsize)
> > +phys_size_t get_ram_size(volatile phys_addr_t *base, phys_size_t maxsize)
> 
> I don't think you want 'base' to be a pointer to phys_addr_t, because
> the pointer type determines how much is read/written in a single
> operation.  I don't think you want to be doing 64-bit reads and
> writes.

I don't know your mnemory bus. This is an RFC patch.

> Plus, you still have this:
> 
> 	for (cnt = (maxsize / sizeof (long)) >> 1; cnt > 0; cnt >>= 1) {
> 		addr = base + cnt;	/* pointer arith! */
> 		sync ();
> 		save[i++] = *addr;
> 		sync ();
> 		*addr = ~cnt;
> 	}
> 
> I'm not quite sure what this loop is doing, but I have a suspicion the
> "sizeof(long)" does not match with "phys_addr_t *base".

Right. This needs to be fixed.

Thanks for pointing out.

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
"... freedom ... is a worship word..."
"It is our worship word too."
	-- Cloud William and Kirk, "The Omega Glory", stardate unknown


More information about the U-Boot mailing list