From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stephen Warren Date: Thu, 02 Oct 2014 17:29:32 -0600 Subject: [U-Boot] [PATCH] ARM: tegra: Use mem size from MC in combination with get_ram_size() In-Reply-To: <8fcc0d74f8ce13bceef4c1002d3dddb43cb1e448.1412262683.git.marcel@ziswiler.com> References: <8fcc0d74f8ce13bceef4c1002d3dddb43cb1e448.1412262683.git.marcel@ziswiler.com> Message-ID: <542DDFDC.4030906@wwwdotorg.org> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 10/02/2014 09:38 AM, Marcel Ziswiler wrote: > On popular request this now completes the Warren's work started for > TK1: > > aeb3fcb35956461077804720b8a252d50758d7e0 Please mention the commit description too, so people can still find the commit if it's been cherry-picked to a different branch with a different commit ID: aeb3fcb35956 ARM: tegra: Use mem size from MC rather than ODMDATA > In addition to the move of using the Tegra memory controller (MC) > register rather than ODMDATA for T20, T30 and T114 as well it further > uses the generic get_ram_size() function (see "common/memsize.c") > TM. I still think calling get_ram_size() is completely pointless given how Tegra works and that this code is already running from RAM. Equally, there doesn't appear to be any protection in the implementation against trashing the code that implements get_ram_size() with the RAM writes the code performs. I'd rather not call get_ram_size() at all. > Added benefit is > that it should reproducible) memory errors> as well. That's *extremely* unlikely. Perhaps it will catch a very tiny percentage of HW problems, and some number of SW configuration problems (i.e. BCT EmemCfg field mismatch with the board's actual RAM size). get_ram_size() doesn't perform anything remotely like a complete RAM test; it just writes to each power-of-two address, which skips a lot of RAM cells! Even then, I'm not sure that it's robust. One issue I immediately saw: it writes to *base then reads it back with only a CPU synchronization between and no bus clear operation (e.g. writing a different pattern to a different address). I've such tests pass that operation when there's nothing connected to the bus at all, due to capacitance in bus drivers/input buffers. Aside from that, this change looks reasonable at a quick glance, although I do hope your testing was very thorough. I personally don't have time to go through and test every board we support with this change. (As an aside, I learned something from this patch; I'd thought that our earlier SoCs didn't have a register in the EMC that indicated the RAM size, and so code *had* to use the ODMDATA to determine the RAM size. Evidently that wasn't the case, and we should have been using an EMC register all along).