[U-Boot] [PATCH] powerpc/85xx: Add TWR-P10xx board support

[Scott Wood <scottwood@freescale.com>]

On 04/11/2013 05:30:07 AM, Xie Xiaobo wrote:
> +int checkboard(void)
> +{
> +	ccsr_gur_t *gur = (void __iomem  
> *)(CONFIG_SYS_MPC85xx_GUTS_ADDR);
> +	u8 boot_status;
> +
> +	printf("Board: %s\n", CONFIG_BOARDNAME);
> +
> +#ifdef CONFIG_PHYS_64BIT
> +	puts("(36-bit addrmap) ");
> +#endif

Is there really a need for supporting both 32-bit and 36-bit address  
maps here?  How much RAM does the board have, and is it pluggable or  
soldered?  If it's pluggable, you really should use SPD...

> +	boot_status = ((gur->porbmsr) & MPC85xx_PORBMSR_ROMLOC)
> +			>> MPC85xx_PORBMSR_ROMLOC_SHIFT;
> +	puts("rom_loc: ");
> +	if (boot_status == 0x0F || boot_status == 0x0E)
> +		puts("nor flash");
> +	else if (boot_status == 0x07)
> +		puts("sd");
> +	else
> +		puts("unknow");

unknown

> +int board_early_init_r(void)
> +{
> +	const unsigned int flashbase = CONFIG_SYS_FLASH_BASE;
> +	const u8 flash_esel = find_tlb_idx((void *)flashbase, 1);
> +
> +	/*
> +	 * Remap Boot flash region to caching-inhibited
> +	 * so that flash can be erased properly.
> +	 */
> +
> +	/* Flush d-cache and invalidate i-cache of any FLASH data */
> +	flush_dcache();
> +	invalidate_icache();
> +
> +	/* invalidate existing TLB entry for flash */
> +	disable_tlb(flash_esel);
> +
> +	set_tlb(1, flashbase, CONFIG_SYS_FLASH_BASE_PHYS, /* tlb, epn,  
> rpn */
> +		MAS3_SX|MAS3_SW|MAS3_SR, MAS2_I|MAS2_G,/* perms, wimge  
> */
> +		0, flash_esel, BOOKE_PAGESZ_64M, 1);/* ts, esel, tsize,  
> iprot */

You should be done executing from flash here, so remove SX.  The  
guarded bit won't prevent speculative instruction fetches.

> +#if defined(CONFIG_TWR_P1025)
> +static void fdt_board_fixup_etsec(void *blob)
> +{
> +	int node;
> +
> +	/* In TWR-P1025 board, the eTSEC2 don't
> +	 * be used, remove the node from dt blob.
> +	 */
> +	node = fdt_path_offset(blob, "/soc/ethernet at b1000");
> +	if (node >= 0)
> +		fdt_del_node(blob, node);
> +
> +	return;
> +}
> +#endif

Why is it in the dts (and not disabled) in the first place?  Do you not  
have a separate dts for twr_p1025?

> +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_PHYS,
> +			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_PHYS + 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_PHYS + 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_PHYS + 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_1M, 1),
> +
> +#ifndef CONFIG_NAND_SPL
> +	/* W**G* - Flash, 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_SR, MAS2_W|MAS2_G,
> +			0, 2, BOOKE_PAGESZ_64M, 1),
> +
> +	/* W**G* - Flash, localbus */
> +	/* This will be changed to *I*G* after relocation to RAM. */
> +	SET_TLB_ENTRY(1, CONFIG_SYS_SSD_BASE, CONFIG_SYS_SSD_BASE_PHYS,
> +			MAS3_SX|MAS3_SW|MAS3_SR, MAS2_I|MAS2_G,
> +			0, 5, BOOKE_PAGESZ_1M, 1),
> +
> +#ifdef CONFIG_PCI
> +	/* *I*G* - PCI memory 1.5G */
> +	SET_TLB_ENTRY(1, CONFIG_SYS_PCIE1_MEM_VIRT,  
> CONFIG_SYS_PCIE1_MEM_PHYS,
> +			MAS3_SX|MAS3_SW|MAS3_SR, MAS2_I|MAS2_G,
> +			0, 3, BOOKE_PAGESZ_1G, 1),
> +
> +	/* *I*G* - PCI I/O effective: 192K  */
> +	SET_TLB_ENTRY(1, CONFIG_SYS_PCIE1_IO_VIRT,  
> CONFIG_SYS_PCIE1_IO_PHYS,
> +			MAS3_SX|MAS3_SW|MAS3_SR, MAS2_I|MAS2_G,
> +			0, 4, BOOKE_PAGESZ_256K, 1),
> +#endif

As above, don't put MAS3_SX on I/O TLB entries.

> diff --git a/boards.cfg b/boards.cfg
> index 35f38f3..fab6f67 100644
> --- a/boards.cfg
> +++ b/boards.cfg
> @@ -798,6 +798,8 @@ P1025RDB_36BIT               powerpc      
> mpc85xx     p1_p2_rdb_pc        freesca
>  P1025RDB_NAND                powerpc     mpc85xx      
> p1_p2_rdb_pc        freescale      -            
> p1_p2_rdb_pc:P1025RDB,NAND
>  P1025RDB_SDCARD              powerpc     mpc85xx      
> p1_p2_rdb_pc        freescale      -            
> p1_p2_rdb_pc:P1025RDB,SDCARD
>  P1025RDB_SPIFLASH            powerpc     mpc85xx      
> p1_p2_rdb_pc        freescale      -            
> p1_p2_rdb_pc:P1025RDB,SPIFLASH
> +TWR-P1025                    powerpc     mpc85xx      
> p1_twr	         freescale      -           p1_twr:TWR_P1025
> +TWR-P1025_36BIT              powerpc     mpc85xx      
> p1_twr	         freescale      -            
> p1_twr:TWR_P1025,36BIT

Fix column alignment

> +#undef CONFIG_CLOCKS_IN_MHZ

When was this defined in the first place?

> +
> +/*
> + * Memory map
> + *
> + * 0x0000_0000 0x1fff_ffff	DDR	Up to 512MB cacheable
> + * 0x8000_0000 0xdfff_ffff	PCI Express Mem	1.5G non-cacheable(PCIe  
> * 3)
> + * 0xffc0_0000 0xffc3_ffff	PCI IO range	256k non-cacheable
> + *
> + * Localbus cacheable (TBD)
> + * 0xXXXX_XXXX 0xXXXX_XXXX	SRAM	YZ M Cacheable
> + *
> + * Localbus non-cacheable
> + * 0xec00_0000 0xefff_ffff	FLASH	Up to 64M non-cacheable
> + * 0xff90_0000 0xff97_ffff	L2 SDRAM(REV.)  512K cacheable(optional)

L2 SRAM, not L2 SDRAM.  Move it to the cacheable section (or remove the  
cacheable/non-cacheable split), and remove the nonsense "SRAM YZ M"  
entry and the TBD.

> + * 0xffd0_0000 0xffd0_3fff	L1 for stack	16K Cacheable TLB0
> + * 0xffe0_0000 0xffef_ffff	CCSR	1M non-cacheable
> + */
[snip]
> +#define CONFIG_SYS_SSD_BASE	0xe0000000

This isn't mentioned in the address space comment above.

> +#define CONFIG_SYS_SSD_BASE_PHYS	CONFIG_SYS_SSD_BASE

What about 36-bit?

> +#define CONFIG_SYS_MONITOR_LEN	(256 * 1024)/* Reserve 256 kB  
> for Mon */

Only 256 KiB?  We tend to exceed that, and it's easier to reserve 512  
KiB now than to change it later when people want to turn on more  
features.

How large is the U-Boot binary now?

> +#define	CONFIG_EXTRA_ENV_SETTINGS	\
> +"netdev=eth0\0"	\
> +"uboot=" __stringify(CONFIG_UBOOTPATH) "\0"	\
> +"loadaddr=1000000\0"	\
> +"bootfile=uImage\0"	\
> +"dtbfile=twr-p1025_32b.dtb\0"	\

Shouldn't "_32b" depend on whether this is a 36-bit U-boot?

> +"consoledev=ttyS0\0"	\
> +"ramdiskaddr=2000000\0"	\
> +"ramdiskfile=rootfs.ext2.gz.uboot\0"	\
> +"fdtaddr=c00000\0"	\
> +"bdev=sda1\0" \
> +"jffs2nor=mtdblock3\0"	\
> +"norbootaddr=ef080000\0"	\
> +"norfdtaddr=ef040000\0"	\
> +"jffs2nand=mtdblock9\0"	\

Use mtdparts if you want to specify partition information here.

> +#define CONFIG_NFSBOOTCOMMAND	\
> +"setenv bootargs root=/dev/nfs rw "	\
> +"nfsroot=$serverip:$rootpath "	\
> +"ip=$ipaddr:$serverip:$gatewayip:$netmask:$hostname:$netdev:off " \
> +"console=$consoledev,$baudrate $othbootargs;" \
> +"tftp $loadaddr $bootfile;"	\
> +"tftp $fdtaddr $fdtfile;"	\
> +"bootm $loadaddr - $fdtaddr"

Use && rather than ; so that we don't attempt to boot if tftp fails.

-Scott