[U-Boot] [PATCH] ARM: tegra: Use mem size from MC rather than ODMDATA
From: Stephen Warren swarren@nvidia.com
In at least Tegra124, the Tegra memory controller (MC) has a register that controls the memory size. Read this to determine the memory size rather than requiring this to be redundantly encoded into the ODMDATA. This way, changes to the BCT (i.e. MC configuration) automatically updated SW's view of the memory size, without requiring manual changes to the ODMDATA.
Future work potentially required: * Clip the memory size to architectural limits; U-Boot probably doesn't and won't support either LPAE or Tegra's "swiss cheese" memory layout, at least one of which would be required for >2GB RAM. * Subtract out any carveout required by firmware on future SoCs.
Based-on-work-by: Tom Warren twarren@nvidia.com Signed-off-by: Stephen Warren swarren@nvidia.com --- Note: this patch depends on Bryan's/Alex's "ARM: tegra: Disable VPR", since that introduces mc.h, which this patch relies on. --- arch/arm/cpu/tegra-common/board.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/arch/arm/cpu/tegra-common/board.c b/arch/arm/cpu/tegra-common/board.c index 6a6faf4b2760..433da09d10c2 100644 --- a/arch/arm/cpu/tegra-common/board.c +++ b/arch/arm/cpu/tegra-common/board.c @@ -27,11 +27,12 @@ enum { UART_COUNT = 5, };
+#if defined(CONFIG_TEGRA20) || defined(CONFIG_TEGRA30) || \ + defined(CONFIG_TEGRA114) /* * Boot ROM initializes the odmdata in APBDEV_PMC_SCRATCH20_0, * so we are using this value to identify memory size. */ - unsigned int query_sdram_size(void) { struct pmc_ctlr *const pmc = (struct pmc_ctlr *)NV_PA_PMC_BASE; @@ -72,6 +73,21 @@ unsigned int query_sdram_size(void) } #endif } +#else +#include <asm/arch/mc.h> + +/* Read the RAM size directly from the memory controller */ +unsigned int query_sdram_size(void) +{ + struct mc_ctlr *const mc = (struct mc_ctlr *)NV_PA_MC_BASE; + u32 size_mb; + + size_mb = readl(&mc->mc_emem_cfg); + debug("mc->mc_emem_cfg (MEM_SIZE_MB) = 0x%08x\n", size_mb); + + return size_mb * 1024 * 1024; +} +#endif
int dram_init(void) {
Dear Stephen Warren,
In message 1404331950-4916-1-git-send-email-swarren@wwwdotorg.org you wrote:
In at least Tegra124, the Tegra memory controller (MC) has a register that controls the memory size. Read this to determine the memory size rather than requiring this to be redundantly encoded into the ODMDATA. This way, changes to the BCT (i.e. MC configuration) automatically updated SW's view of the memory size, without requiring manual changes to the ODMDATA.
Is there a specific reason for not using get_ram_size()?
Best regards,
Wolfgang Denk
On 07/02/2014 03:18 PM, Wolfgang Denk wrote:
Dear Stephen Warren,
In message 1404331950-4916-1-git-send-email-swarren@wwwdotorg.org you wrote:
In at least Tegra124, the Tegra memory controller (MC) has a register that controls the memory size. Read this to determine the memory size rather than requiring this to be redundantly encoded into the ODMDATA. This way, changes to the BCT (i.e. MC configuration) automatically updated SW's view of the memory size, without requiring manual changes to the ODMDATA.
Is there a specific reason for not using get_ram_size()?
Since we know the exact RAM size, we may as well simply use it directly rather than "probing" for it.
I know that if non-existent peripheral addresses are accessed by the CPU, the CPU or some bus hangs. I'm not sure if the same applies to addresses within the memory window where there is not actually RAM present on a particular board, but I'd rather not risk it by touching them during probing.
BTW, I'm out on vacation starting tomorrow, so may not respond soon.
Dear Stephen,
In message 53B47F6F.1090405@wwwdotorg.org you wrote:
Is there a specific reason for not using get_ram_size()?
Since we know the exact RAM size, we may as well simply use it directly rather than "probing" for it.
You _think_ you know the size, but you can never be sure that all this RAM is actually present and working. There has been many discussions before why using get_ram_size() makes a lot of sense even in fixed size RAM configurations.
Best regards,
Wolfgang Denk
On Thu, Jul 03, 2014 at 09:45:52AM +0200, Wolfgang Denk wrote:
Dear Stephen,
In message 53B47F6F.1090405@wwwdotorg.org you wrote:
Is there a specific reason for not using get_ram_size()?
Since we know the exact RAM size, we may as well simply use it directly rather than "probing" for it.
You _think_ you know the size, but you can never be sure that all this RAM is actually present and working. There has been many discussions before why using get_ram_size() makes a lot of sense even in fixed size RAM configurations.
Right which is why the flow in this case is: 1) Read the place that "knows" 2) Pass that size to get_ram_size(), use returned value as what we really know the size to be.
On 07/03/2014 05:01 AM, Tom Rini wrote:
On Thu, Jul 03, 2014 at 09:45:52AM +0200, Wolfgang Denk wrote:
Dear Stephen,
In message 53B47F6F.1090405@wwwdotorg.org you wrote:
Is there a specific reason for not using get_ram_size()?
Since we know the exact RAM size, we may as well simply use it directly rather than "probing" for it.
You _think_ you know the size, but you can never be sure that all this RAM is actually present and working. There has been many discussions before why using get_ram_size() makes a lot of sense even in fixed size RAM configurations.
Right which is why the flow in this case is:
- Read the place that "knows"
- Pass that size to get_ram_size(), use returned value as what we
really know the size to be.
Wolfgang, given Tom's explanation, are you now OK with this patch? TomW would like clarification?
But to address your points: We really do know that the RAM size is equal to what this register says, since at this point in the code, U-Boot is already running in RAM (since our HW's boot ROM initializes RAM and copies U-Boot to it). Any issues with the RAM itself, either bad HW or incorrect configuration, would already have caused a problem just executing any code.
On 07/02/2014 02:12 PM, Stephen Warren wrote:
From: Stephen Warren swarren@nvidia.com
In at least Tegra124, the Tegra memory controller (MC) has a register that controls the memory size. Read this to determine the memory size rather than requiring this to be redundantly encoded into the ODMDATA. This way, changes to the BCT (i.e. MC configuration) automatically updated SW's view of the memory size, without requiring manual changes to the ODMDATA.
Future work potentially required:
- Clip the memory size to architectural limits; U-Boot probably doesn't and won't support either LPAE or Tegra's "swiss cheese" memory layout, at least one of which would be required for >2GB RAM.
- Subtract out any carveout required by firmware on future SoCs.
I noticed that this patch triggers a bug in DT relocation during bootm/bootz. It'd be best if the following were applied somewhere before this patch:
http://patchwork.ozlabs.org/patch/375408/ lib: lmb: fix overflow in __lmb_alloc_base w/ large RAM
That's because this patch changes the RAM size on Tegra124 from 2GB-1MB to 2GB. I believe that change is correct, it just triggers an overflow in U-Boot's RAM handling which needs to be fixed first.
In practice, I suspect even if this dependency is ignored, there will be no user-visible impact, because the default environment in Tegra U-Boot disables DT relocation completely. This problem would only affect someone who flashed/ran a U-Boot with this patch without resetting their environment to the default that's embedded into the new U-Boot, and their environment in flash was older than when we disabled DT relocation. Since tegra-uboot-flasher always does reset the environment, I expect most people would never notice this.
participants (3)
-
Stephen Warren -
Tom Rini -
Wolfgang Denk