[U-Boot] [PATCH 1/7] ppc/p4080: Add p4080 platform immap definitions

[Kumar Gala]

On Sep 18, 2009, at 4:20 PM, Scott Wood wrote:

> Kumar Gala wrote:
>> +	u32	lawbarh0;	/* 0xc00 - LAW0 base address register high */
>> +	u32	lawbarl0;	/* 0xc04 - LAW0 base address register low */
>> +	u32	lawar0;		/* 0xc08 - LAW0 attributes register */
>> +	u8	res4[4];
>> +	u32	lawbarh1;	/* 0xc10 - LAW1 base address register high */
>> +	u32	lawbarl1;	/* 0xc14 - LAW1 base address register low */
>> +	u32	lawar1;		/* 0xc18 - LAW1 attributes register */
>> +	u8	res5[4];
>> +	u32	lawbarh2;	/* 0xc20 - LAW2 base address register high */
>> +	u32	lawbarl2;	/* 0xc24 - LAW2 base address register low */
>> +	u32	lawar2;		/* 0xc28 - LAW2 attributes register */
>> +	u8	res6[4];
>> +	u32	lawbarh3;	/* 0xc30 - LAW3 base address register high */
>> +	u32	lawbarl3;	/* 0xc34 - LAW3 base address register low */
>> +	u32	lawar3;		/* 0xc38 - LAW3 attributes register */
>> +	u8	res7[4];
>> +	u32	lawbarh4;	/* 0xc40 - LAW4 base address register high */
>> +	u32	lawbarl4;	/* 0xc44 - LAW4 base address register low */
>> +	u32	lawar4;		/* 0xc48 - LAW4 attributes register */
>> +	u8	res8[4];
>> +	u32	lawbarh5;	/* 0xc50 - LAW5 base address register high */
>> +	u32	lawbarl5;	/* 0xc54 - LAW5 base address register low */
>> +	u32	lawar5;		/* 0xc58 - LAW5 attributes register */
>
> Can we use an array for this?  Likewise many other parts.

we could, but we didn't do this on PQ3 and I've got some other code  
that depends on lawbar{h,l}0.

>> +	char	res7[12];
>> +	uint	powmgtcsr;	/* 0xe0080 - Power management status and control  
>> register */
>> +	char	res8[12];
>> +	uint	coredisru;	/* 0xe0090 - uppper portion for support of 64  
>> cores */
>> +	uint	coredisrl;	/* 0xe0094 - lower portion for support of 64  
>> cores */
>> +	char	res9[8];
>> +	uint	pvr;		/* 0xe00a0 - Processor version register */
>> +	uint	svr;		/* 0xe00a4 - System version register */
>> +	char	res10[8];
>> +	uint	rstcr;		/* 0xe00b0 - Reset control register */
>> +	uint	rstrqpblsr;	/* 0xe00b4 - Reset request preboot loader status  
>> register */
>> +	char	res11[8];
>> +	uint	rstrqmr1;	/* 0xe00c0 - Reset request mask register */
>> +	char	res12[4];	/* Reserved: RSTRQMR2 */
>> +	uint	rstrqsr1;	/* 0xe00c8 - Reset request status register */
>> +	char	res13[4];	/* Reserved: RSTRQSR2 */
>> +	char	res14[4];	/* Reserved: RSTRQWDTMRU */
>> +	uint	rstrqwdtmrl;	/* 0xe00d4 - Reset request WDT mask register */
>> +	char	res15[4];	/* Reserved: RSTRQWDTSRU */
>> +	uint	rstrqwdtsrl;	/* 0xe00dc - Reset request WDT status register */
>> +	char	res16[4];	/* Reserved: BRRU max total of  2 for up to 64  
>> cores */
>
> If those fields have a name, why not use the name instead of "res13"  
> etc?

For right now I'm leaving this alone.  These registers are architected  
but we just dont implement them in p4080.  To catch any issues I'll  
stick with it as is for now.

> If all these fields are 32 bit, why are the reserved fields char[4]  
> rather than u32?  It's very visually distracting.

I'll change char -> u8.  However I'm not making u8[4] -> u32.  For  
historically reasons we used u8 since we autogen'd these in the past.   
I'm sticking with u8 for reserved fields.

> For that matter, s/uint/u32/.

I'll make this change.

>
>> -#define CONFIG_SYS_MPC85xx_GUTS_OFFSET	(0xE0000)
>> +#ifdef CONFIG_FSL_CORENET
>> +#define CONFIG_SYS_FSL_CORENET_CCM_OFFSET	(0x0000)
>> +#define CONFIG_SYS_MPC85xx_DDR_OFFSET		(0x8000)
>> +#define CONFIG_SYS_MPC85xx_DDR2_OFFSET		(0x9000)
>> +#define CONFIG_SYS_FSL_CORENET_CLK_OFFSET	(0xE1000)
>> +#define CONFIG_SYS_FSL_CORENET_RCPM_OFFSET	(0xE2000)
>> +#define CONFIG_SYS_MPC85xx_DMA_OFFSET		(0x100000)
>> +#define CONFIG_SYS_MPC85xx_ESPI_OFFSET		(0x110000)
>> +#define CONFIG_SYS_MPC85xx_ESDHC_OFFSET		(0x114000)
>> +#define CONFIG_SYS_MPC85xx_LBC_OFFSET		(0x124000)
>> +#define CONFIG_SYS_MPC85xx_GPIO_OFFSET		(0x130000)
>> +#define CONFIG_SYS_MPC85xx_QMAN_OFFSET		(0x318000)
>> +#define CONFIG_SYS_MPC85xx_BMAN_OFFSET		(0x31a000)
>> +#else
>> +#define CONFIG_SYS_MPC85xx_ECM_OFFSET		(0x0000)
>> +#define CONFIG_SYS_MPC85xx_DDR_OFFSET		(0x2000)
>> +#define CONFIG_SYS_MPC85xx_LBC_OFFSET		(0x5000)
>> +#define CONFIG_SYS_MPC85xx_DDR2_OFFSET		(0x6000)
>> +#define CONFIG_SYS_MPC85xx_ESPI_OFFSET		(0x7000)
>> +#define CONFIG_SYS_MPC85xx_PCIX_OFFSET		(0x8000)
>> +#define CONFIG_SYS_MPC85xx_PCIX2_OFFSET		(0x9000)
>> +#define CONFIG_SYS_MPC85xx_GPIO_OFFSET		(0xF000)
>> +#define CONFIG_SYS_MPC85xx_SATA1_OFFSET		(0x18000)
>> +#define CONFIG_SYS_MPC85xx_SATA2_OFFSET		(0x19000)
>> +#define CONFIG_SYS_MPC85xx_L2_OFFSET		(0x20000)
>> +#define CONFIG_SYS_MPC85xx_DMA_OFFSET		(0x21000)
>> +#define CONFIG_SYS_MPC85xx_ESDHC_OFFSET		(0x2e000)
>> +#define CONFIG_SYS_MPC85xx_SERDES2_OFFSET	(0xE3100)
>> +#define CONFIG_SYS_MPC85xx_SERDES1_OFFSET	(0xE3000)
>> +#define CONFIG_SYS_MPC85xx_CPM_OFFSET		(0x80000)
>> +#endif
>> +
>> +#define CONFIG_SYS_MPC85xx_PIC_OFFSET		(0x40000)
>> +#define CONFIG_SYS_MPC85xx_GUTS_OFFSET		(0xE0000)
>
> Unnecessary parens.

will fix.

- k