Re: [v4,1/1] powerpc/86xx: Add support for Emerson/Artesyn MVME7100

From: Scott Wood
Date: Mon May 16 2016 - 19:41:58 EST


On Wed, Apr 27, 2016 at 10:35:25AM +0200, Alessio Igor Bogani wrote:
> + bcsr@4,0 {
> + compatible = "artesyn,mvme7100-bcsr";
> + reg = <4 0 0x10000>;
> + };
> +
> + serial@5,1000 {
> + cell-index = <2>;
> + device_type = "serial";
> + compatible = "ns16550";
> + reg = <5 0x1000 0x100>;
> + clock-frequency = <1843200>;
> + interrupts = <11 1 0 0>;
> + };

The "serial@5,1000" line has spaces where there should be tabs. There
are several other instances of this in the patch.

Where did these cell-index values come from? Why are they needed?

> + };
> +
> +};

No blank line here.

> + platform_ops.fixups = mvme7100_fixups;
> +
> +}

No blank line here.

> diff --git a/arch/powerpc/boot/ppcboot.h b/arch/powerpc/boot/ppcboot.h
> index 6ae6f90..7b758be 100644
> --- a/arch/powerpc/boot/ppcboot.h
> +++ b/arch/powerpc/boot/ppcboot.h
> @@ -43,7 +43,7 @@ typedef struct bd_info {
> unsigned long bi_sramstart; /* start of SRAM memory */
> unsigned long bi_sramsize; /* size of SRAM memory */
> #if defined(TARGET_8xx) || defined(TARGET_CPM2) || defined(TARGET_85xx) ||\
> - defined(TARGET_83xx)
> + defined(TARGET_83xx) || defined(TARGET_MVME7100)
> unsigned long bi_immr_base; /* base of IMMR register */
> #endif

Again, please use TARGET_86xx here rather than TARGET_MVME7100.

> @@ -69,7 +74,7 @@ config MPC8641
> select FSL_PCI if PCI
> select PPC_UDBG_16550
> select MPIC
> - default y if MPC8641_HPCN || SBC8641D || GEF_SBC610 || GEF_SBC310 || GEF_PPC9A
> + default y if MPC8641_HPCN || SBC8641D || GEF_SBC610 || GEF_SBC310 || GEF_PPC9A || MVME7100

Please wrap this long line.

-Scott