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

[Wolfgang Denk]

Dear Niklaus Giger,

In message <1253719369-26565-1-git-send-email-niklaus.giger at netstal.com> you wrote:
> Adds a HCU5 board specific cmd reghcu5 to dump about 140 internal
> register which define the HW configuration. Needed for documentation
> purposes and to compare different settings.
> 
> Signed-off-by: Niklaus Giger <niklaus.giger at netstal.com>
> ---
>  board/netstal/hcu5/Makefile      |    2 +-
>  board/netstal/hcu5/cmd_reghcu5.c |  283 ++++++++++++++++++++++++++++++++++++++
>  2 files changed, 284 insertions(+), 1 deletions(-)
>  create mode 100644 board/netstal/hcu5/cmd_reghcu5.c
> 
> diff --git a/board/netstal/hcu5/Makefile b/board/netstal/hcu5/Makefile
> index 4456771..670b100 100644
> --- a/board/netstal/hcu5/Makefile
> +++ b/board/netstal/hcu5/Makefile
> @@ -25,7 +25,7 @@ LIB	= $(obj)lib$(BOARD).a
>  
>  # NOBJS : Netstal common objects
>  NOBJS	= nm_bsp.o
> -COBJS	= $(BOARD).o sdram.o
> +COBJS	= $(BOARD).o sdram.o cmd_reghcu5.o

Please keep lists sorted. Consider using only one file entry per line.

>  SRCS	:= $(SOBJS:.o=.S) $(COBJS:.o=.c) $(addprefix ../common/,$(NOBJS:.o=.c))
> diff --git a/board/netstal/hcu5/cmd_reghcu5.c b/board/netstal/hcu5/cmd_reghcu5.c
> new file mode 100644
> index 0000000..ca330f7
> --- /dev/null
> +++ b/board/netstal/hcu5/cmd_reghcu5.c
...
> +/************************************************************************
> + * cmd_reghcu5.c - CPU Register Dump for HCU5 board with PPC440EPx
> + ***********************************************************************/

Incorrect multiline comment style.

> +#include <common.h>
> +#include <command.h>
> +#include <asm/processor.h>
> +
> +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 */

Lines too long. Please fix globally.

> +/* PPC440EPx registers ordered for output
> + * name           type    addr            size
> + * -------------------------------------------
> + */

Incorrect multiline comment style. Please fix globally.

> +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},
...

Instead of wasting memory for white space padded strings...

...
> +		printf("0x%08lx %16s: 0x%08lx\n", ppc440epx_reg[i].address,
> +		       ppc440epx_reg[i].name, value);

... you shoudl change this into

		printf("0x%08lx %-16s: 0x%08lx\n", ...

which will give you the same left-aligned output.


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"Science makes godlike -- it is all over with priests and  gods  when
man becomes scientific. Moral: science is the forbidden as such -- it
alone  is  forbidden. Science is the *first* sin, the *original* sin.
*This alone is  morality.*  ``Thou  shalt  not  know''  --  the  rest
follows."                                       - Friedrich Nietzsche