Dear Bin Meng,
In message <[email protected]> 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: [email protected]
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
