Re: [PATCH 1/2] Added support for PRTLVT based boards (MPC5121)

[Scott Wood <scottwood@freescale.com>]

David Jander wrote:
>  arch/powerpc/platforms/512x/mpc5121_ads.c     |  104 ----------
>  arch/powerpc/platforms/512x/mpc5121_generic.c |  105 ++++++++++

Pass -B -M -C to git-format-patch to detect renames, copies, etc.

> +	flash@fe000000 {
> +		device_type = "rom";
> +		compatible = "direct-mapped";
> +		probe-type = "CFI";
> +		reg = <0xfe000000 0x02000000>;
> +		bank-width = <2>;
> +		partitions = <0x00000000 0x01800000   // filesystem
> +		              0x01800000 0x00200000   // config0
> +		              0x01a00000 0x00200000   // config1
> +		              0x01c00000 0x002e0000   // kernel
> +		              0x01ee0000 0x00020000   // one sector for device tree
> +		              0x01f00000 0x00100000>; // u-boot
> +		partition-names = "filesystem", "config0", "config1","kernel", "device-tree", "u-boot";
> +	};

This is an old flash binding.  Please use the one currently described in 
Documentation/booting-without-of.txt.

> +	soc@80000000 {
> +		compatible = "fsl,mpc5121-immr";

Also simple-bus.

> +		//axe@2000 {
> +		//	compatible = "mpc512x-axe";
> +		//	reg = <2000 100>;
> +		//	interrupts = <2a 8>;
> +		//	interrupt-parent = < &ipic >;
> +		//};

Why is this here, but commented out?

> +		mdio@2800 {
> +			device_type = "mdio";

No device_type.

> +		// PSC4 in i2s mode
> +		i2s@11500 {
> +			device_type = "i2s";

No device_type.

> +		// PSC5 in SPI mode for touchscreen
> +		spi@11500 {
> +			device_type = "spi";

No device_type.

> +		dma2@14000 {
> +			compatible = "fsl,mpc5121-dma2";

Is this DMA controller compatible with drivers/dma/fsldma.c?  If so, 
follow the established binding.

> +config MPC5121_GENERIC
> +	bool
> +	default n
> +
>  config MPC5121_ADS
>  	bool "Freescale MPC5121E ADS"
>  	depends on PPC_MULTIPLATFORM && PPC32
>  	select DEFAULT_UIMAGE
>  	select PPC_MPC5121
> +	select MPC5121_GENERIC
>  	help
>  	  This option enables support for the MPC5121E ADS board.
>  	default n
> +
> +config PRTLVT
> +	bool "Protonic LVT family of MPC5121 based boards"
> +	depends on PPC_MULTIPLATFORM && PPC32
> +	select DEFAULT_UIMAGE
> +	select PPC_MPC5121
> +	select MPC5121_GENERIC
> +	help
> +	  This option enables support for the Protonic LVT family (ZANMCU and VICVT2).
> +	default n

Can we leave out the "default n" lines?  They're no-ops.

I don't think these configs should depend on an option 
(PPC_MULTIPLATFORM) that is described as "Generic desktop/server/laptop"...

-Scott