Re: [U-Boot] [PATCH] powerpc/85xx: Add P1023RDB board support

[Scott Wood]

On 04/16/2013 04:00:46 AM, Chunhe Lan wrote:
+struct fsl_e_tlb_entry tlb_table[] = {
+       /* TLB 0 - for temp stack in cache */
+	SET_TLB_ENTRY(0, CONFIG_SYS_INIT_RAM_ADDR,  
CONFIG_SYS_INIT_RAM_ADDR,
+                     MAS3_SX|MAS3_SW|MAS3_SR, 0,
+                     0, 0, BOOKE_PAGESZ_4K, 0),
+       SET_TLB_ENTRY(0, CONFIG_SYS_INIT_RAM_ADDR + 4 * 1024,
+                     CONFIG_SYS_INIT_RAM_ADDR + 4 * 1024,
+                     MAS3_SX|MAS3_SW|MAS3_SR, 0,
+                     0, 0, BOOKE_PAGESZ_4K, 0),
+       SET_TLB_ENTRY(0, CONFIG_SYS_INIT_RAM_ADDR + 8 * 1024,
+                     CONFIG_SYS_INIT_RAM_ADDR + 8 * 1024,
+                     MAS3_SX|MAS3_SW|MAS3_SR, 0,
+                     0, 0, BOOKE_PAGESZ_4K, 0),
+       SET_TLB_ENTRY(0, CONFIG_SYS_INIT_RAM_ADDR + 12 * 1024,
+                     CONFIG_SYS_INIT_RAM_ADDR + 12 * 1024,
+                     MAS3_SX|MAS3_SW|MAS3_SR, 0,
+                     0, 0, BOOKE_PAGESZ_4K, 0),
+
+       /* TLB 1 */
+       /* *I*** - Covers boot page */
+       SET_TLB_ENTRY(1, 0xfffff000, 0xfffff000,
+                     MAS3_SX|MAS3_SW|MAS3_SR, MAS2_I,
+                     0, 0, BOOKE_PAGESZ_4K, 1),
+
+       /* *I*G* - CCSRBAR */
+       SET_TLB_ENTRY(1, CONFIG_SYS_CCSRBAR, CONFIG_SYS_CCSRBAR_PHYS,
+                     MAS3_SX|MAS3_SW|MAS3_SR, MAS2_I|MAS2_G,
+                     0, 1, BOOKE_PAGESZ_4M, 1),
+
+       /* W**G* - Flash/promjet, localbus */
+       /* This will be changed to *I*G* after relocation to RAM. */
+	SET_TLB_ENTRY(1, CONFIG_SYS_FLASH_BASE,  
CONFIG_SYS_FLASH_BASE_PHYS,
+                     MAS3_SX|MAS3_SW|MAS3_SR, MAS2_W|MAS2_G,
+                     0, 2, BOOKE_PAGESZ_256M, 1),
+
+       /* *I*G* - PCI */
+	SET_TLB_ENTRY(1, CONFIG_SYS_PCIE3_MEM_VIRT,  
CONFIG_SYS_PCIE3_MEM_PHYS,
+                     MAS3_SX|MAS3_SW|MAS3_SR, MAS2_I|MAS2_G,
+                     0, 3, BOOKE_PAGESZ_1G, 1),
+
+       /* *I*G* - PCI */
+       SET_TLB_ENTRY(1, CONFIG_SYS_PCIE3_MEM_VIRT + 0x40000000,
+                     CONFIG_SYS_PCIE3_MEM_PHYS + 0x40000000,
+                     MAS3_SX|MAS3_SW|MAS3_SR, MAS2_I|MAS2_G,
+                     0, 4, BOOKE_PAGESZ_256M, 1),
+
+       SET_TLB_ENTRY(1, CONFIG_SYS_PCIE3_MEM_VIRT + 0x50000000,
+                     CONFIG_SYS_PCIE3_MEM_PHYS + 0x50000000,
+                     MAS3_SX|MAS3_SW|MAS3_SR, MAS2_I|MAS2_G,
+                     0, 5, BOOKE_PAGESZ_256M, 1),

Do not use MAS3_SX on I/O mappings.  The G bit does not prevent  
speculative instruction fetches.

+       /* *I*G - NAND */
+	SET_TLB_ENTRY(1, CONFIG_SYS_NAND_BASE,  
CONFIG_SYS_NAND_BASE_PHYS,
+                     MAS3_SX|MAS3_SW|MAS3_SR, MAS2_I|MAS2_G,
+                     0, 11, BOOKE_PAGESZ_1M, 1),

Why do you need 1M for NAND?  There's only 8K of stuff there.

+/*
+ * Memory map
+ *
+ * 0x0000_0000	0x1fff_ffff	DDR			500M  
Cacheable
+ * 0x8000_0000	0xbfff_ffff	PCI Express Mem		1G  
non-cacheable
+ * 0xc000_0000	0xdfff_ffff	PCI			512M  
non-cacheable
+ * 0xe100_0000	0xe3ff_ffff	PCI IO range		4M  
non-cacheable
+ *
+ * Localbus non-cacheable
+ *
+ * 0xec00_0000	0xefff_ffff	NOR flash		64M NOR  
flash
+ * 0xff00_0000 0xff3f_ffff     DPAA_QBMAN              4M
+ * 0xff60_0000	0xff7f_ffff	CCSR			2M  
non-cacheable
+ * 0xffa0_0000	0xffaf_ffff	NAND			1M  
non-cacheable
+ * 0xffd0_0000	0xffd0_3fff	L1 for stack		16K  
Cacheable TLB0
+ */

L1 for stack is neither "localbus" nor "non-cacheable".  If you don't  
want to distinguish that category, remove the comment.

[snip hundreds of lines of board config file]

Can we please refactor these board config headers to focus only on  
what's different from board to board?  These patches are very hard to  
effectively review as is.

-Scott
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot