[U-Boot] [PATCH] ppc4xx: HCU5 board: add register dump

[Peter Tyser]

Hi Niklaus,

> +enum REGISTER_TYPE {
> +	DCR,			/* Directly Accessed DCR's */
> +	IDCR1,			/* Indirectly Accessed DCR to SDRAM0_CFGADDR and SDRAM0_CFGDATA */
> +	IDCR2,			/* Indirectly Accessed DCR to EBC0_CFGADDR and EBC0_CFGDATA */
> +	IDCR3,			/* Indirectly Accessed DCR to EBM0_CFGADDR and EBM0_CFGDATA */
> +	IDCR4,			/* Indirectly Accessed DCR to PPM0_CFGADDR and PPM0_CFGDATA */
> +	IDCR5,			/* Indirectly Accessed DCR to CPR0_CFGADDR and CPR0_CFGDATA */
> +	IDCR6,			/* Indirectly Accessed DCR to SDR0_CFGADDR and SDR0_CFGDATA */
> +	MM			/* Directly Accessed MMIO Register */
> +};

The lines above are well over 80 lines.

> +struct cpu_register {
> +	char *name;
> +	enum REGISTER_TYPE type;
> +	unsigned long address;
> +};
> +
> +/* PPC440EPx registers ordered for output
> + * name           type    addr            size
> + * -------------------------------------------
> + */
> +const struct cpu_register ppc440epx_reg[] = {
> +	{"EBC0_B0CR      ", IDCR2, PB0CR},
> +	{"EBC0_B1CR      ", IDCR2, PB1CR},
> +	{"EBC0_B2CR      ", IDCR2, PB2CR},
> +	{"EBC0_B3CR      ", IDCR2, PB3CR},
> +	{"EBC0_B4CR      ", IDCR2, PB4CR},
> +	{"EBC0_B5CR      ", IDCR2, PB5CR},
> +	{"EBC0_B0AP      ", IDCR2, PB0AP},
> +	{"EBC0_B1AP      ", IDCR2, PB1AP},
> +	{"EBC0_B2AP      ", IDCR2, PB2AP},
> +	{"EBC0_B3AP      ", IDCR2, PB3AP},

You should be able to remove all those empty spaces in the strings above
and use a fancy printf format to get your alignment right.

<snip>

> +/*
> + * CPU Register dump of PPC440EPx
> + * Output in order of struct ppc440epx_reg
> + */
> +int do_reghcu5(cmd_tbl_t * cmdtp, int flag, int argc, char *argv[])
> +{
> +	unsigned int i;
> +	unsigned int n;
> +	unsigned long value;
> +	enum REGISTER_TYPE type;
> +
> +	printf
> +	    ("\nRegister Dump PPC440EPx for comparison with document A0001492\n\n");
> +	n = sizeof(ppc440epx_reg) / sizeof(ppc440epx_reg[0]);
> +	for (i = 0; i < n; i++) {
> +		value = 0;
> +		type = ppc440epx_reg[i].type;
> +		switch (type) {
> +		case DCR:	/* Directly Accessed DCR's */
> +			switch (ppc440epx_reg[i].address) {
> +				/* following list includes only registers included in struct */
> +			case 0x0b0:
> +				value = mfdcr(0x0b0);
> +				break;
> +			case 0x0f0:
> +				value = mfdcr(0x0f0);
> +				break;
> +			case 0x0180:
> +				value = mfdcr(0x0180);
> +				break;
> +			case 0x081:
> +				value = mfdcr(0x081);
> +				break;
> +			case 0x089:
> +				value = mfdcr(0x089);
> +				break;
> +			case 0x077:
> +				value = mfdcr(0x077);
> +				break;
> +			case 0x350:
> +				value = mfdcr(0x350);
> +				break;
> +			case 0x026:
> +				value = mfdcr(0x026);
> +				break;

Replace all the above "value = ...; break;" with 1 "value =
mfdcr(ppc440epx_reg[i].address); break;"?

<snip>

> +
> +/* define do_reghcu5 as u-boot command */
> +U_BOOT_CMD(reghcu5, 2, 1, do_reghcu5,
> +	   "reghcu5 - print register information for HCU5\n",);

This command's help message won't be printed correctly.

Best,
Peter