[PATCH] memsize: Make get_ram_size() work with arbitary RAM size

[Bin Meng]

Hi Wolfgang,

On Tue, Jul 14, 2020 at 8:28 PM Wolfgang Denk <wd at denx.de> wrote:
>
> Dear Bin Meng,
>
> In message <CAEUhbmWBFvb_SGobk2f_JXhvAtyZkZ1jJ0HXiWdC0xU61Hn1jg at mail.gmail.com> you wrote:
> >
> > > I'm afraid I don't understand this change,  Can you please explain a
> > > bit more detailed what "any RAM size" means?
> >
> > I meant "any RAM size" that is not power of two.
>
> I was afraid you meant this.
>
> > > The existing code should work fine with any RAM size that is a power
> > > of two (and the algoithm used is based on this assumption, so the
> > > code would fail if it is not met).
> >
> > Unfortunately this limitation is not documented clearly.
>
> You are right - the code is completely undocumented, not even the
> fact that it should always operate on a single bank of memory at a
> time.
>
> > > For correct operation (as originally intended) you would always
> > > specify a maxsize twice as large as the maximum possible memory size
> > > of a bank of memory, and the function would return the real size it
> > > found.
> >
> > I don't think this function can work as expected. Even if I pass a
> > power-of-two RAM size to this function, it could still fail. Imagine
> > the target has 2GiB memory size, and I pass 8GiB as the maxsize to
> > this function. The function will hang when it tries to read/write
> > something at an address that is beyond 2GiB.
>
> It should not hang if used properly - but of course this depends a
> lot on the hardware properties, and you have to know what you are
> doing.
>
> When the code was written, all we had to deal with was Power
> Architecture systems with pretty flexible and easy to ptogram memory
> controllers.  A typical use case was a system where you could for
> example populate either 64 or 128 or 256 MB of RAM.  In such a case
> you would run the code with a max size of 512 MB, after configuring
> he memory controller to map such a size.  In this case, acceses to
> the whole 512 MB range will work without problems, the higher
> address ranges just mirroring the data of the actually populated
> RAM.  This mirroring will be detected, so you get the information
> how big the populated RAM really is.

Thanks for the details. Now I remember the days when I was playing
PowerPC boards :)

>
> If we would only check with a max size of 256 MB, we would not be
> able to detect the case when there is bigger RAM (say, 512 MB)
> populated.
>
> I am aware that there are situations where you can't program the
> memory controller so freely, so we often cannot use this complete
> testing and lose the ability to detect working, but bigger RAM.
>
> In any case we always test only the memory locations at the
> power-of-two borders - the main reason is that this test is supposed
> to run on every boot, so it must be fast.
>
> In your case this immediately shows the problem.  Assume you have a
> memory configuration which is supposed to have 2 GiB + 512 MiB,
> but by mistake wrong components were fit and instead of 512 MiB
> there are only 128 MiB actually present.
>
> U-Boot will test memory below the ... 128M, 256M, 512M, 1G, 2G (and
> ideally 4G) limits.  It will tell you that there are 2 GiB working
> memory.  It will NOT test any location between 2 GB and 4GB, so it
> has no chance to find out if there is 128 MB or 512 MB present, or
> nothing at all, or if this memory is not working at all and
> returning random data.
>
> This code works only for bank sizes that are a power of two!
>
> > Given what you mentioned the function limitation, we should update the
> > codes with the above power of two assumptions documented.
>
> That would indeed make sense.  On the other hand, I would have
> expected that this behaviour is kind of obvious from the fact, that
> we just bit shift the address location used for testing?
>

I will prepare a patch that improves the documentation of this API.

Regards,
Bin