[U-Boot] [PATCH] common/memsize.c: restore content of the base address

[Patrick DELAUNAY]

Dear Wolfgang,

> From: Wolfgang Denk [mailto:wd at denx.de]
> 
> Dear Patrick,
> 
> In message <885aaa3abdfe440591ea271f92ab42ff at SFHDAG6NODE3.st.com>
> you wrote:
>>
> > But it is recommended in ./doc/README.arm-relocation:
> 
> 
> Where do  you read that this allows runing the code from the same memory
> which is sized/tested?  I cannot see any such claim.  It is always supposed that
> you run from a totally different, independent memory type - in the olden days
> usually parallel NOR flash, nor some on-chip-memory, but never ever the RAM
> you are testing!

Fine, so I misunderstood the sentence.
 
> > And before to setup by board I check on many device with ARM architecture.
> > Most the time U-Boot loaded at beginning of the RAM, relocated at the
> > end of the RAM and dram_init use get_ram_size
> 
> But definitely not in this order!  The sequence should always be:
> initialize the RAM and determine it's size; load U-Boot and relocate it to the
> (now known) end of RAM (resp. to some lower address, depending on any
> reservations made for shared display buffer, shared log buffer, PRAM, etc.).

OK.
I will modified my board to have
- SPL => get_ram_size on DDR / chacke size of DDR is correct (value from DTB) 
               + load U-boot at beginning of DDR
- U-Boot => relocated at the end of the DDR (get_ram_size no more use), size found in DTB

> > Example1 :
> >   ./board/toradex/colibri_imx6/colibri_imx6.c
> 
> What do you mean to show by this?  It just means that dram_init() must be
> running from some other memory (OCM), too.

I thought that it is an example of ARM platform which do the same sequence 
=>  dram_init is called in U-Boot before relocation (common/board_f.c), 
      it is using get_ram_size() and U-Boot link address (CONFIG_SYS_TEXT_BASE = 0x17800000) is the same value than
      the base address used in get_ram_size
      (CONFIG_SYS_SDRAM_BASE = PHYS_SDRAM = MMDC0_ARB_BASE_ADDR = 0x10000000)

But perhaps I make a error in the code analysis (I have no other board),
 sorry  for that....
I will no more argue with other ARM target, aas I accept that use get_ram_size in DDR is a really bad idea.

> > So I just do the same for my arm/armv7 platform.
> 
> No, apparently not.

So I will remove it
 
> > On ARM device device, I think we don't have the banks...
> 
> So you must run this (typically int he SPL) fom some on-chip memory.

Yes

 
> > Ok, I will remove the call of the code on my board, even if over arm platform
> use this function in the same context.
> 
> You should keep the functionality, but move it to where it belongs, i. e. to the
> SPL running from OCM.
> 

I remove it in U-Boot and I call it only in SPL,
executed in onchip RAM, juste after DDR controller initalisation

So for my board, I have no more issue with the function.

> get_ram_size() is supposed to be non-destructive, i. e. _all_ memory locations
> should be the same before and after running this code. In our tests this was the
> case (but I admit that this has not been tested for a long time - probably not
> since the last discussion about this "fix").

For the last case (return (maxsize)),
when no overlap is detect, the content of base is not restored.
So for me, the function get_ram_size is not safe for the DDR content.

Do you think, that I need to continue with the patchset,
(I need a v2 to remove the uncessary restore in the second loop after code analysis)
even it is no more usefull for me.

Or do you prefer that I drop the patchset to avoid regression ?

> Best regards,
> 
> Wolfgang Denk
> 

Best Regards

Patrick Delaunay