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: Mon, 13 Jul 2020 19:18:37 +0200	[thread overview]
Message-ID: <20200713171837.802E6240702@gemini.denx.de> (raw)
In-Reply-To: <1594378641-26360-1-git-send-email-bmeng.cn@gmail.com>

Dear Bin Meng,

In message <1594378641-26360-1-git-send-email-bmeng.cn@gmail.com> you wrote:
>
> Currently get_ram_size() only works with certain RAM size like 1GiB,
> 2GiB, 4GiB, 8GiB, etc. Chanage the codes to work with any RAM size.

I'm afraid I don't understand this change,  Can you please explain a
bit more detailed what "any RAM size" means?

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

> -	long           save[BITS_PER_LONG - 1];
> -	long           save_base;
> -	long           cnt;
> -	long           val;
> -	long           size;
> -	int            i = 0;
> +	long save[BITS_PER_LONG - 1];
> +	long save_base;
> +	long cnt;
> +	long val;
> +	long size;
> +	int i = 0;

Why do you change the formatting here?  I can see no need for that?


> +	long n = maxsize / sizeof(long);
>  
> -	for (cnt = (maxsize / sizeof(long)) >> 1; cnt > 0; cnt >>= 1) {
> +	n = __ffs(n);
> +	n = BIT(n);
> +
> +	for (cnt = n >> 1; cnt > 0; cnt >>= 1) {

I can only speculate - but do you think that this will work for a
memory size of - for example - 2.5 GiB as might result from combining
two banks of 2 GiB resp. 512 MiB ?   I bet it doesn't.

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.

Any other use, especially not checking one bank of memory at a time,
will not work as intended.  And I have yet to see systems where
the size of a bank of memory is not a power of two.

So I feel what you are doing here is not an improvement, but a
"workaround" for some incorrect usage.

I don't think we should accept this patch.

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
Even if you aren't in doubt, consider the mental welfare of the  per-
son who has to maintain the code after you, and who will probably put
parens in the wrong place.          - Larry Wall in the perl man page

  reply	other threads:[~2020-07-13 17:18 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 [this message]
2020-07-14 10:35   ` Bin Meng
2020-07-14 12:28     ` Wolfgang Denk
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=20200713171837.802E6240702@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.