All of lore.kernel.org
 help / color / mirror / Atom feed
From: Wolfgang Denk <wd@denx.de>
To: u-boot@lists.denx.de
Subject: [PATCH] memsize: Make get_ram_size() work with arbitary RAM size
Date: Tue, 14 Jul 2020 14:28:17 +0200	[thread overview]
Message-ID: <20200714122817.3A29D240702@gemini.denx.de> (raw)
In-Reply-To: <CAEUhbmWBFvb_SGobk2f_JXhvAtyZkZ1jJ0HXiWdC0xU61Hn1jg@mail.gmail.com>

Dear Bin Meng,

In message <CAEUhbmWBFvb_SGobk2f_JXhvAtyZkZ1jJ0HXiWdC0xU61Hn1jg@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.

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?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
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 number you have dialed is imaginary. Please divide by 0  and  try
again.

  reply	other threads:[~2020-07-14 12:28 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-10 10:57 [PATCH] memsize: Make get_ram_size() work with arbitary RAM size Bin Meng
2020-07-13 17:18 ` Wolfgang Denk
2020-07-14 10:35   ` Bin Meng
2020-07-14 12:28     ` Wolfgang Denk [this message]
2020-07-16  9:36       ` Bin Meng
2020-07-14 15:53     ` Heinrich Schuchardt
2020-07-14 16:09       ` Wolfgang Denk
2020-07-16 21:04         ` Heinrich Schuchardt
2020-07-21  6:51           ` Wolfgang Denk

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200714122817.3A29D240702@gemini.denx.de \
    --to=wd@denx.de \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.