From mboxrd@z Thu Jan 1 00:00:00 1970 From: Wolfgang Denk Date: Mon, 13 Jul 2020 19:18:37 +0200 Subject: [PATCH] memsize: Make get_ram_size() work with arbitary RAM size In-Reply-To: <1594378641-26360-1-git-send-email-bmeng.cn@gmail.com> References: <1594378641-26360-1-git-send-email-bmeng.cn@gmail.com> Message-ID: <20200713171837.802E6240702@gemini.denx.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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