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

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

[Wolfgang Denk]

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@denx.de>
Cc: Timur Tabi <timur@freescale.com>
---
Note: this is only minimally tested - I just compiled a dozen of ppc
boards with it without appearent problems. Please review and test
carefully.

 common/memsize.c |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/common/memsize.c b/common/memsize.c
index 6c275c9..b99e51b 100644
--- a/common/memsize.c
+++ b/common/memsize.c
@@ -37,14 +37,14 @@
  * the actually available RAM size between addresses `base' and
  * `base + maxsize'.
  */
-long get_ram_size(volatile long *base, long maxsize)
+phys_size_t get_ram_size(volatile phys_addr_t *base, phys_size_t maxsize)
 {
-	volatile long *addr;
-	long           save[32];
-	long           cnt;
-	long           val;
-	long           size;
-	int            i = 0;
+	volatile phys_addr_t *addr;
+	phys_size_t	save[32];
+	phys_size_t	cnt;
+	phys_size_t	val;
+	phys_size_t	size;
+	int		i = 0;
 
 	for (cnt = (maxsize / sizeof (long)) >> 1; cnt > 0; cnt >>= 1) {
 		addr = base + cnt;	/* pointer arith! */
-- 
1.6.6.1

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

[Wolfgang Denk]

get_ram_size() used to use "long" data types for addresses and data,
which 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@denx.de>
Cc: Timur Tabi <timur@freescale.com>
---
Note: this is only minimally tested - I just compiled a dozen of ppc
boards with it without appearent problems. Please review and test
carefully.

v2: - fix commit message
    - change function prototype, too.

 common/memsize.c |   14 +++++++-------
 include/common.h |    2 +-
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/common/memsize.c b/common/memsize.c
index 6c275c9..b99e51b 100644
--- a/common/memsize.c
+++ b/common/memsize.c
@@ -37,14 +37,14 @@
  * the actually available RAM size between addresses `base' and
  * `base + maxsize'.
  */
-long get_ram_size(volatile long *base, long maxsize)
+phys_size_t get_ram_size(volatile phys_addr_t *base, phys_size_t maxsize)
 {
-	volatile long *addr;
-	long           save[32];
-	long           cnt;
-	long           val;
-	long           size;
-	int            i = 0;
+	volatile phys_addr_t *addr;
+	phys_size_t	save[32];
+	phys_size_t	cnt;
+	phys_size_t	val;
+	phys_size_t	size;
+	int		i = 0;
 
 	for (cnt = (maxsize / sizeof (long)) >> 1; cnt > 0; cnt >>= 1) {
 		addr = base + cnt;	/* pointer arith! */
diff --git a/include/common.h b/include/common.h
index 8bca04f..648e00f 100644
--- a/include/common.h
+++ b/include/common.h
@@ -316,7 +316,7 @@ const char *symbol_lookup(unsigned long addr, unsigned long *caddr);
 void	api_init (void);
 
 /* common/memsize.c */
-long	get_ram_size  (volatile long *, long);
+phys_size_t get_ram_size(volatile phys_addr_t *, phys_size_t);
 
 /* $(BOARD)/$(BOARD).c */
 void	reset_phy     (void);
-- 
1.6.6.1

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

[Scott Wood]

On Thu, May 27, 2010 at 08:16:28PM +0200, Wolfgang Denk wrote:
> get_ram_size() used to use "long" data types for addresses and data,
> which 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.

This cannot work as is.  The only systems where this makes a difference are
where physical addresses are larger than virtual pointers -- but you try to
shove the 64-bit physical offset into a 32-bit pointer.

You need to create temporary mappings, if you really want to do this.

-Scott

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

[Wolfgang Denk]

Dear Scott Wood,

In message <20100527194618.GC5915@schlenkerla.am.freescale.net> you wrote:
> On Thu, May 27, 2010 at 08:16:28PM +0200, Wolfgang Denk wrote:
> > get_ram_size() used to use "long" data types for addresses and data,
> > which 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.
> 
> This cannot work as is.  The only systems where this makes a difference are
> where physical addresses are larger than virtual pointers -- but you try to
> shove the 64-bit physical offset into a 32-bit pointer.
> 
> You need to create temporary mappings, if you really want to do this.

?

Isn't phys_addr_t assumed to be the right data type to hold a
physical address?

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
"And it should be the law: If you use  the  word  `paradigm'  without
knowing  what  the  dictionary  says  it  means,  you  go to jail. No
exceptions."                     - David Jones @ Megatest Corporation

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

[Scott Wood]

On 05/27/2010 02:57 PM, Wolfgang Denk wrote:
> Dear Scott Wood,
>
> In message<20100527194618.GC5915@schlenkerla.am.freescale.net>  you wrote:
>> On Thu, May 27, 2010 at 08:16:28PM +0200, Wolfgang Denk wrote:
>>> get_ram_size() used to use "long" data types for addresses and data,
>>> which 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.
>>
>> This cannot work as is.  The only systems where this makes a difference are
>> where physical addresses are larger than virtual pointers -- but you try to
>> shove the 64-bit physical offset into a 32-bit pointer.
>>
>> You need to create temporary mappings, if you really want to do this.
>
> ?
>
> Isn't phys_addr_t assumed to be the right data type to hold a
> physical address?

Yes.  But you can't dereference a physical address directly.

When you do "addr = base + cnt", you're throwing away the upper 32 bits.

"phys_addr_t *" is not a 64-bit pointer, it is a 32-bit pointer to a 
64-bit quantity.

-Scott

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

[Wolfgang Denk]

Dear Scott Wood,

In message <4BFECF43.40600@freescale.com> you wrote:
>
> > Isn't phys_addr_t assumed to be the right data type to hold a
> > physical address?
> 
> Yes.  But you can't dereference a physical address directly.

Ah, right.  OK, so this needs more work...

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
The most difficult thing in the world is to know how to  do  a  thing
and to watch someone else doing it wrong, without commenting.
                                                        -- T.H. White

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

[Timur Tabi]

On Thu, May 27, 2010 at 1:11 PM, Wolfgang Denk <wd@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@denx.de>
> Cc: Timur Tabi <timur@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.

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.

> -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.

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".

-- 
Timur Tabi
Linux kernel developer at Freescale

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

[Wolfgang Denk]

Dear Timur Tabi,

In message <AANLkTilrFhTwnQgQX9NVmplaBl8HpISZY97HKK73-ncz@mail.gmail.com> you wrote:
> On Thu, May 27, 2010 at 1:11 PM, Wolfgang Denk <wd@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@denx.de>
> > Cc: Timur Tabi <timur@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

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

[Timur Tabi]

Wolfgang Denk wrote:

>> 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.

Scott pointed out that writing/reading memory to determine how much memory
actually exists is dangerous.  I'm not convinced that I should be using
get_ram_size().  I still believe that I shouldn't.

>> 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.

I have not been able to find any other PowerPC system in U-boot that
supports more memory than is mapped.  If you know of one, please tell me.
Otherwise, I would say that there are no other comparable PowerPC systems
that I can use as an example.

>>> -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.

My point is that sizeof(phys_addr_t) has got nothing to do with the size of
the read/write operation, so I think it's wrong on all platforms.

-- 
Timur Tabi
Linux kernel developer at Freescale

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

[Wolfgang Denk]

Dear Timur Tabi,

In message <4BFECF82.60901@freescale.com> you wrote:
> 
> Scott pointed out that writing/reading memory to determine how much memory
> actually exists is dangerous.  I'm not convinced that I should be using
> get_ram_size().  I still believe that I shouldn't.

And I point out that we have been doing this for a decade on tens of
different board configurations with millions of devices in the field.

> I have not been able to find any other PowerPC system in U-boot that
> supports more memory than is mapped.  If you know of one, please tell me.

The systems I know are the opposite - initially they map more memory
than they support, then they determine the real size, then they
adjust the mapping to the real size.

> My point is that sizeof(phys_addr_t) has got nothing to do with the size of
> the read/write operation, so I think it's wrong on all platforms.

Actually it will only be wrong on systems where phys_addr_t != ulong,
but you are right.

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
"The two most common things in the universe are hydrogen  and  stupi-
dity."

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

[Timur Tabi]

Wolfgang Denk wrote:
> The systems I know are the opposite - initially they map more memory
> than they support, then they determine the real size, then they
> adjust the mapping to the real size.

But we don't ever do that, at least not on systems that use SPD.  We query
the DIMMs directly via SPD and calculate how much memory is in the system.

Most of our boards support both SPD and "fixed" DDR programming.  In fixed
mode, the actual values to be programmed in the controller are hard-coded in
the board header file, like this:

#define CONFIG_SYS_DDR_CS0_BNDS		0x0000003F
#define CONFIG_SYS_DDR_CS1_BNDS		0x00000000
#define CONFIG_SYS_DDR_CS0_CONFIG	0x80014202
#define CONFIG_SYS_DDR_CS1_CONFIG	0x00000000
...

So in this case, as long as there's <= 2GB of DDR expected, then
get_ram_size() can work.

But on the P1022DS, I don't support fixed DDR mode.  Only SPD is supported,
so I have no idea at compile-time how much memory is in the system.  That's
why I don't think calling get_ram_size() is appropriate for the P1022DS board.

-- 
Timur Tabi
Linux kernel developer@Freescale

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

[Wolfgang Denk]

Dear Timur Tabi,

In message <4BFEDE90.6070802@freescale.com> you wrote:
> Wolfgang Denk wrote:
> > The systems I know are the opposite - initially they map more memory
> > than they support, then they determine the real size, then they
> > adjust the mapping to the real size.
> 
> But we don't ever do that, at least not on systems that use SPD.  We query
> the DIMMs directly via SPD and calculate how much memory is in the system.
> 
> Most of our boards support both SPD and "fixed" DDR programming.  In fixed
> mode, the actual values to be programmed in the controller are hard-coded in
> the board header file, like this:

When you have SPD information you can of course use this for size
information. In the "fixed" case you get a max size from the #defines.

> But on the P1022DS, I don't support fixed DDR mode.  Only SPD is supported,
> so I have no idea at compile-time how much memory is in the system.  That's
> why I don't think calling get_ram_size() is appropriate for the P1022DS board.

You do not need to know this at compile time - it is sufficient to
know it when calling get_ram_size().

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
"This is a test of the Emergency Broadcast System. If this had been an
actual emergency, do you really think we'd stick around to tell you?"

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

[Kumar Gala]

On May 27, 2010, at 3:57 PM, Wolfgang Denk wrote:

> Dear Timur Tabi,
> 
> In message <4BFECF82.60901@freescale.com> you wrote:
>> 
>> Scott pointed out that writing/reading memory to determine how much memory
>> actually exists is dangerous.  I'm not convinced that I should be using
>> get_ram_size().  I still believe that I shouldn't.
> 
> And I point out that we have been doing this for a decade on tens of
> different board configurations with millions of devices in the field.
> 
>> I have not been able to find any other PowerPC system in U-boot that
>> supports more memory than is mapped.  If you know of one, please tell me.
> 
> The systems I know are the opposite - initially they map more memory
> than they support, then they determine the real size, then they
> adjust the mapping to the real size.

Is the point of get_ram_size() to deal with having the same firmware image (binary) be able to be agnostic of memory size?

I'm not sure I understand what utility one gets out of it if we have SPD eeprom information?  I can see some purpose in the soldered memory case if you dont want to tweak binaries between different memory size configs.

- k

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

[Wolfgang Denk]

Dear Kumar Gala,

In message <48A9B08D-9237-4837-91CA-0D23CE1E56D8@kernel.crashing.org> you wrote:
> 
> > The systems I know are the opposite - initially they map more memory
> > than they support, then they determine the real size, then they
> > adjust the mapping to the real size.
> 
> Is the point of get_ram_size() to deal with having the same firmware
> image (binary) be able to be agnostic of memory size?

Yes, this is the key point of it.

> I'm not sure I understand what utility one gets out of it if we have SPD
> eeprom information?  I can see some purpose in the soldered memory case
> if you dont want to tweak binaries between different memory size
> configs.

With SPD information the benefits go down to the (limited) testing it
does, so you might still detect hardware issues - if you try and
insert a 2 GiB DIMM and U-Boot sees only 512 MiB then you can be damn
sure that something is not working as expected.

Especially on systems where the end user can plug in any types of
memory modules that seems to be a pretty useful feature to me.

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
miracle:  an  extremely  outstanding  or  unusual  event,  thing,  or
accomplishment.                                - Webster's Dictionary

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

[Scott Wood]

On 05/27/2010 02:44 PM, Wolfgang Denk wrote:
> Dear Timur Tabi,
>
> In message<AANLkTilrFhTwnQgQX9NVmplaBl8HpISZY97HKK73-ncz@mail.gmail.com>  you wrote:
>> On Thu, May 27, 2010 at 1:11 PM, Wolfgang Denk<wd@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@denx.de>
>>> Cc: Timur Tabi<timur@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.

Not using get_ram_size(), which is of minimal utility when you know the 
actual size (and is an unsafe approach to the problem when you don't, 
unless you know a lot about where I/O is mapped and how the system 
responds to accessing memory that doesn't exist -- not exactly the sort 
of thing you want to make a mandate for any board to be accepted), seems 
like a good way 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.

"All other PowerPC systems" is a ridiculous overstatement.  I see 
exactly *one* Freescale board that uses this thing, and it's an ARM board.

-Scott

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

[Wolfgang Denk]

Dear Scott Wood,

In message <4BFED0CD.5010301@freescale.com> you wrote:
>
> Not using get_ram_size(), which is of minimal utility when you know the 
> actual size (and is an unsafe approach to the problem when you don't, 
> unless you know a lot about where I/O is mapped and how the system 
> responds to accessing memory that doesn't exist -- not exactly the sort 
> of thing you want to make a mandate for any board to be accepted), seems 
> like a good way to "fix it".

Come on. Anybody who does NOT know the exact memory map of a system
(and is able to define it as needed) is obviously not in a position to
port U-Boot to this hardware.

This is basic knowledge.

> "All other PowerPC systems" is a ridiculous overstatement.  I see 
> exactly *one* Freescale board that uses this thing, and it's an ARM board.

And this uses it incorrectly, like all ARM boards, if I'm not wrong.

Well, there is a ton of other boards besides the ones manufactured by
Freescale, including many that use Freescale chips.

I am not to blame if Freescale did not pick up a number of design
concepts.

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
I object to intellect without discipline;  I object to power without
constructive purpose.
	-- Spock, "The Squire of Gothos", stardate 2124.5

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

[Wolfgang Wegner]

Dear Wolfgang Denk,

as the systems I was working on always had far less memory, I can
not comment much on the extension you propose here, but as Timur
Tabi's comments seem to go in a direction which could lead to a
bigger overhaul/discussion, I would like to add 2C from my point
of view on coldfire...

- MCF53xx/MCF5445x both simply lock up if non-existent memory is
  accessed. So if the SDRAM controller is set up for a too big size
  of memory, get_ram_size() will fail. I assume this applies to
  most coldfire devices.
- How about systems/configurations using CONFIG_MONITOR_IS_IN_RAM?
  I could not see special precautions for this, but in case an address
  to be tested by accident is occupied by a part of get_ram_size()
  itself, the result would be ... interesting. ;-) Of course, this is
  a rare thing (both using CONFIG_MONITOR_IS_IN_RAM and the chance
  to have get_ram_size() at such a crucial location), but still I
  fear it might have an impact if get_ram_size() gets "mandatory".
- at least in the coldfire world, CONFIG_SYS_SDRAM_SIZE is quite
  often used for cache setup in the assembly code. This contradicts
  changing/setting the SDRAM size at runtime...

(Please don't see this as a vote against using and promoting
get_ram_size() - I just see some problems that I am not aware of an
easy solution for.)

Best regards,
Wolfgang Wegner

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

[Wolfgang Denk]

Dear Wolfgang Wegner,

In message <20100527185909.GG31271@leila.ping.de> you wrote:
> 
> as the systems I was working on always had far less memory, I can
> not comment much on the extension you propose here, but as Timur
> Tabi's comments seem to go in a direction which could lead to a
> bigger overhaul/discussion, I would like to add 2C from my point
> of view on coldfire...
> 
> - MCF53xx/MCF5445x both simply lock up if non-existent memory is
>   accessed. So if the SDRAM controller is set up for a too big size
>   of memory, get_ram_size() will fail. I assume this applies to
>   most coldfire devices.

The design of get_ram_size() is based on the assumption that you start
with a "big enough" mapping. Usually we map twice the maximum possible
size that actually can be fit.

> - How about systems/configurations using CONFIG_MONITOR_IS_IN_RAM?

Here you obviously cannot use this. What some ARM systems are doing
right now is plain wrong. get_ram_size() must always and only be
called _before_ the code is running from RAM.

> - at least in the coldfire world, CONFIG_SYS_SDRAM_SIZE is quite
>   often used for cache setup in the assembly code. This contradicts
>   changing/setting the SDRAM size at runtime...

You may want to change this :-)


Note that we have plans underway to rework the ARM architecture to do
proper relocation including auto-sizing of the RAM.  Other
architectures are invited to follow.

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
Never underestimate the bandwidth of a station wagon full of tapes.
                                -- Dr. Warren Jackson, Director, UTCS