Patchwork [U-Boot] arm/omap3: limit chip select iteration based on board config

login
register
mail settings
Submitter Grant Erickson
Date Dec. 22, 2011, 7:28 p.m.
Message ID <1324582093-27718-1-git-send-email-marathon96@gmail.com>
Download mbox | patch
Permalink /patch/132890/
State Superseded
Delegated to: Tom Rini
Headers show

Comments

Grant Erickson - Dec. 22, 2011, 7:28 p.m.
Only attempt to configure and add DRAM at chip select 1 if the board has configured more than one bank of DRAM.

This prevents boards that have CONFIG_NR_DRAM_BANKS set to 1 from getting an incorrect DRAM size.

Signed-off-by: Grant Erickson <marathon96@gmail.com>
Cc: Tom Rini <trini@ti.com>
---
 arch/arm/cpu/armv7/omap3/sdrc.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
Tom Rini - Jan. 3, 2012, 2:31 p.m.
On 12/22/2011 12:28 PM, Grant Erickson wrote:
> Only attempt to configure and add DRAM at chip select 1 if the board has configured more than one bank of DRAM.
> 
> This prevents boards that have CONFIG_NR_DRAM_BANKS set to 1 from getting an incorrect DRAM size.
> 
> Signed-off-by: Grant Erickson <marathon96@gmail.com>
> Cc: Tom Rini <trini@ti.com>

OK, what problem (and on what board) are you seeing?  Many boards only
have CS0 populated with DRAM but when we try and configure CS1 the
mem_ok() call fails and we don't try and use it, so we don't get a wrong
amount of memory.  Thanks!
Grant Erickson - Jan. 4, 2012, 4:10 p.m.
On Jan 3, 2012, at 6:31 AM, Tom Rini wrote:
> On 12/22/2011 12:28 PM, Grant Erickson wrote:
>> Only attempt to configure and add DRAM at chip select 1 if the board has configured more than one bank of DRAM.
>> 
>> This prevents boards that have CONFIG_NR_DRAM_BANKS set to 1 from getting an incorrect DRAM size.
>> 
>> Signed-off-by: Grant Erickson <marathon96@gmail.com>
>> Cc: Tom Rini <trini@ti.com>
> 
> OK, what problem (and on what board) are you seeing?  Many boards only
> have CS0 populated with DRAM but when we try and configure CS1 the
> mem_ok() call fails and we don't try and use it, so we don't get a wrong
> amount of memory.  Thanks!

Tom:

This is on an OMAP3EVM-derived board with 64 MiB of fixed Micron MDDR DRAM.

The board is bootstrapped in second stage boot using X-Loader 1.46 (it'll move to U-Boot SPL really soon now). The original X-Loader 1.46 + U-Boot 2010.09 went successfully as follows:

	Texas Instruments X-Loader 1.46 (Dec 21 2011 - 21:23:39)
	
	Reading boot sector
	
	229792 bytes read
	Loading from MMC0, file "u-boot.img"
	   Image Name:   U-Boot 2010.09
	   Created:      2011-12-22   5:12:46 UTC
	   Image Type:   ARM U-Boot Firmware (uncompressed)
	   Data Size:    229728 Bytes = 224.3 KiB
	   Load Address: 80e80000
	   Entry Point:  00000000
	   Verifying Checksum ... OK
	   Loading Firmware ... OK
	
	Starting Firmware ...
	
	U-Boot 2010.09 (Dec 21 2011 - 21:11:09)
	
	U-Boot code: 80E80000 -> 80EB8160  BSS: -> 80EFA604
	OMAP36XX/37XX-GP ES2.1, CPU-OPP2, L3-165MHz, Max CPU Clock 1 Ghz
	I2C:   ready
	RAM Configuration:
	Bank #0: 80000000 64 MiB
	Bank #1: a0000000 0 Bytes
	NAND:  256 MiB
	In:    serial
	Out:   serial
	Err:   serial
	Board: Revision 6, Serial 109212560100008
	Build: development
	Die ID #6f4400029e3800000160a7451000f006
	Net:   No ethernet found.
	### main_loop entered: bootdelay=1
	
	### main_loop: bootcmd="run nandboot0 || run nandboot1 || run mmcboot || reset"
	Hit any key to stop autoboot:  0 

With the same X-Loader but using U-Boot 2011.12-rc2 without the proposed patch, things failed as follows:

	Texas Instruments X-Loader 1.46 (Dec 21 2011 - 21:23:39)
	
	Reading boot sector

	[ ... ]

	U-Boot 2011.12-rc2 (Dec 21 2011 - 22:47:56)
	
	U-Boot code: 80E80000 -> 80EC00E4  BSS: -> 80F02800
	OMAP36XX/37XX-GP ES1.2, CPU-OPP2, L3-165MHz, Max CPU Clock 1 Ghz
	I2C:   ready
	--> dram_init
		--> get_sdr_cs_size
		<-- get_sdr_cs_size size 67108864
		size0 67108864
		--> get_sdr_cs_size
		<-- get_sdr_cs_size size 67108864
		--> get_sdr_cs_size
		<-- get_sdr_cs_size size 67108864
		size1 67108864
		gd->ram_size 134217728
	<-- dram_init
	monitor len: 00082800
	ramsize: 08000000
	TLB table at: 87ff0000
	Top of RAM usable for U-Boot at: 87ff0000
	Reserving 522k for U-Boot at: 87f6d000
	Reserving 512k for malloc() at: 87eed000
	Reserving 32 Bytes for Board Info at: 87eecfe0
	Reserving 120 Bytes for Global Data at: 87eecf68
	New Stack Pointer is: 87eecf58
	--> dram_init_banksize
		--> get_sdr_cs_size
		<-- get_sdr_cs_size size 67108864
		--> get_sdr_cs_size
		<-- get_sdr_cs_size size 67108864
		size0 67108864
		size1 67108864
	<-- dram_init_banksize
	RAM Configuration:
	Bank #0: 7461000a 1.1 GiB
	Bank #1: 4806a000 1.1 GiB
	relocation Offset is: 070ed000

	[ Hangs ]

with the patch in place, things went successfully as follows:

	Texas Instruments X-Loader 1.46 (Dec 21 2011 - 21:23:39)
	
	Reading boot sector
	
	[ ... ]
	
	U-Boot 2011.12-rc2 (Dec 21 2011 - 23:54:24)
	
	U-Boot code: 80E80000 -> 80EC018C  BSS: -> 80F028A8
	OMAP36XX/37XX-GP ES1.2, CPU-OPP2, L3-165MHz, Max CPU Clock 1 Ghz
	I2C:   ready
	--> dram_init
		--> get_sdr_cs_size
		<-- get_sdr_cs_size size 67108864
		size0 67108864
		size1 67108864
		gd->ram_size 67108864
	<-- dram_init
	monitor len: 000828A8
	ramsize: 04000000
	TLB table at: 83ff0000
	Top of RAM usable for U-Boot at: 83ff0000
	Reserving 522k for U-Boot at: 83f6d000
	Reserving 512k for malloc() at: 83eed000
	Reserving 24 Bytes for Board Info at: 83eecfe8
	Reserving 120 Bytes for Global Data at: 83eecf70
	New Stack Pointer is: 83eecf60
	--> dram_init_banksize
		--> get_sdr_cs_size
		<-- get_sdr_cs_size size 67108864
		--> get_sdr_cs_size
		<-- get_sdr_cs_size size 0
		size0 67108864
		size1 0
	<-- dram_init_banksize
	RAM Configuration:
	Bank #0: 80000000 64 MiB
	relocation Offset is: 030ed000
	dram_bank_mmu_setup: bank: 0
	monitor flash len: 000470E4
	Now running in RAM - U-Boot at: 83f6d000
	NAND:  256 MiB
	MMC:   OMAP SD/MMC: 0
	In:    serial
	Out:   serial
	Err:   serial
	Board: Revision 6, Serial 109212560100008
	Build: development
	Die ID #6f4400029e3800000160a7451000f006
	Net:   No ethernet found.
	### main_loop entered: bootdelay=1
	
	### main_loop: bootcmd="run nandboot0 || run nandboot1 || run mmcboot || reset"
	Hit any key to stop autoboot:  0 
	
	Loading from NAND 256MiB 1,8V 16-bit, offset 0x400000
	   Image Name:   Linux-2.6.32
	   Image Type:   ARM Linux Kernel Image (uncompressed)
	   Data Size:    2615228 Bytes = 2.5 MiB
	   Load Address: 80008000
	   Entry Point:  80008000
	## Current stack ends at 0x83eecc40 *  kernel: cmdline image address = 0x81000000
	## Booting kernel from Legacy Image at 81000000 ...
	   Image Name:   Linux-2.6.32
	   Image Type:   ARM Linux Kernel Image (uncompressed)
	   Data Size:    2615228 Bytes = 2.5 MiB
	   Load Address: 80008000
	   Entry Point:  80008000
	   Verifying Checksum ... OK
	   kernel data at 0x81000040, len = 0x0027e7bc (2615228)
	## No init Ramdisk
	   ramdisk start = 0x00000000, ramdisk end = 0x00000000
	   Loading Kernel Image ... OK
	OK
	   kernel loaded at 0x80008000, end = 0x802867bc
	## Transferring control to Linux (at address 80008000) ...
	
	Starting kernel ...

In past PPC boards, I've seen routines like mem_ok fail because of memory aliasing; however, I didn't dive into mem_ok in this instance to see if that what was happening as the proposed patch seems simpler: for this board, simply don't poke at or try to use memory we already know isn't there.

Best,

Grant
Tom Rini - Jan. 4, 2012, 4:14 p.m.
On 01/04/2012 09:10 AM, Grant Erickson wrote:
> On Jan 3, 2012, at 6:31 AM, Tom Rini wrote:
>> On 12/22/2011 12:28 PM, Grant Erickson wrote:
>>> Only attempt to configure and add DRAM at chip select 1 if the board has configured more than one bank of DRAM.
>>>
>>> This prevents boards that have CONFIG_NR_DRAM_BANKS set to 1 from getting an incorrect DRAM size.
>>>
>>> Signed-off-by: Grant Erickson <marathon96@gmail.com>
>>> Cc: Tom Rini <trini@ti.com>
>>
>> OK, what problem (and on what board) are you seeing?  Many boards only
>> have CS0 populated with DRAM but when we try and configure CS1 the
>> mem_ok() call fails and we don't try and use it, so we don't get a wrong
>> amount of memory.  Thanks!
> 
> Tom:
> 
> This is on an OMAP3EVM-derived board with 64 MiB of fixed Micron MDDR DRAM.
> 
> The board is bootstrapped in second stage boot using X-Loader 1.46 (it'll move to U-Boot SPL really soon now). The original X-Loader 1.46 + U-Boot 2010.09 went successfully as follows:

X-Loader is misbehaving (and misconfiguring) here.  Actual OMAP3EVMs
also had this problem until the SPL switch.
Grant Erickson - Jan. 4, 2012, 4:28 p.m.
On Jan 4, 2012, at 8:14 AM, Tom Rini wrote:
> On 01/04/2012 09:10 AM, Grant Erickson wrote:
>> On Jan 3, 2012, at 6:31 AM, Tom Rini wrote:
>>> On 12/22/2011 12:28 PM, Grant Erickson wrote:
>>>> Only attempt to configure and add DRAM at chip select 1 if the board has configured more than one bank of DRAM.
>>>> 
>>>> This prevents boards that have CONFIG_NR_DRAM_BANKS set to 1 from getting an incorrect DRAM size.
>>>> 
>>>> Signed-off-by: Grant Erickson <marathon96@gmail.com>
>>>> Cc: Tom Rini <trini@ti.com>
>>> 
>>> OK, what problem (and on what board) are you seeing?  Many boards only
>>> have CS0 populated with DRAM but when we try and configure CS1 the
>>> mem_ok() call fails and we don't try and use it, so we don't get a wrong
>>> amount of memory.  Thanks!
>> 
>> Tom:
>> 
>> This is on an OMAP3EVM-derived board with 64 MiB of fixed Micron MDDR DRAM.
>> 
>> The board is bootstrapped in second stage boot using X-Loader 1.46 (it'll move to U-Boot SPL really soon now). The original X-Loader 1.46 + U-Boot 2010.09 went successfully as follows:
> 
> X-Loader is misbehaving (and misconfiguring) here.  Actual OMAP3EVM also had this problem until the SPL switch.

Tom:

That makes sense; however, I have deployed boards in the field for which an in-place X-Loader update isn't trivial and for which interoperability is key. In light of that, are you willing to accept the patch?

	* For CONFIG_NR_DRAM_BANKS set to 1, we get interop with old second stage boot such as X-Loader.

	* For CONFIG_NR_DRAM_BANKS set to 2 (most boards) the behavior is exactly as it is without the patch.

What do you think?

Thanks,

Grant
Wolfgang Denk - Jan. 5, 2012, 8:44 a.m.
Dear Grant Erickson,

In message <02A515F3-CCD2-4635-AD38-07DE542A46CE@gmail.com> you wrote:
>
> That makes sense; however, I have deployed boards in the field for
> which an in-place X-Loader update isn't trivial and for which
> interoperability is key. In light of that, are you willing to accept
> the patch?
> 
> 	* For CONFIG_NR_DRAM_BANKS set to 1, we get interop with old second stage boot such as X-Loader.
> 
> 	* For CONFIG_NR_DRAM_BANKS set to 2 (most boards) the behavior is exactly as it is without the patch.

This an ugly hack which I do not want to see in mainline.  If you
really think you need that, then please maintain it yourself out of
tree.  Thanks.

Best regards,

Wolfgang Denk

Patch

diff --git a/arch/arm/cpu/armv7/omap3/sdrc.c b/arch/arm/cpu/armv7/omap3/sdrc.c
index a27b4b1..4c02214 100644
--- a/arch/arm/cpu/armv7/omap3/sdrc.c
+++ b/arch/arm/cpu/armv7/omap3/sdrc.c
@@ -213,6 +213,7 @@  int dram_init(void)
 	unsigned int size0 = 0, size1 = 0;
 
 	size0 = get_sdr_cs_size(CS0);
+#if CONFIG_NR_DRAM_BANKS > 1
 	/*
 	 * We always need to have cs_cfg point at where the second
 	 * bank would be, if present.  Failure to do so can lead to
@@ -223,6 +224,7 @@  int dram_init(void)
 	make_cs1_contiguous();
 	do_sdrc_init(CS1, NOT_EARLY);
 	size1 = get_sdr_cs_size(CS1);
+#endif
 
 	gd->ram_size = size0 + size1;