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

[Scott Wood]

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.

> +	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?

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

For that matter, s/uint/u32/.

> -#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.

-Scott