[3/4] powerpc/85xx: Add C293PCIE board support

[Liu Po-B43644]

>  -----Original Message-----
>  From: Wood Scott-B07421
>  Sent: Tuesday, July 23, 2013 6:59 AM
>  To: Liu Po-B43644
>  Cc: linuxppc-dev at ozlabs.org; Hu Mingkai-B21284
>  Subject: Re: [3/4] powerpc/85xx: Add C293PCIE board support
>  
>  On Thu, Apr 25, 2013 at 09:54:16AM +0800, Po Liu wrote:
>  > From: Mingkai Hu <Mingkai.Hu at freescale.com>
>  >
>  > C293PCIE board is a series of Freescale PCIe add-in cards to perform
>  > as public key crypto accelerator or secure key management module.
>  >
>  >  - 512KB platform SRAM in addition to 512K L2 Cache/SRAM
>  >  - 512MB soldered DDR3 32bit memory
>  >  - CPLD System Logic
>  >  - 64MB x16 NOR flash and 4GB x8 NAND flash
>  >  - 16MB SPI flash
>  >
>  > Signed-off-by: Mingkai Hu <Mingkai.Hu at freescale.com>
>  > Singed-off-by: Po Liu <Po.Liu at freescale.com>
>  
>  Signed
>  
>  > +		partition at 900000 {
>  > +			/* 33MB for rootfs */
>  > +			reg = <0x00900000 0x02100000>;
>  > +			label = "NOR Rootfs Image";
>  > +		};
>  > +
>  > +		partition at 2a00000 {
>  > +			/* 20MB for JFFS2 based Root file System */
>  > +			reg = <0x02a00000 0x01400000>;
>  > +			label = "NOR JFFS2 Root File System";
>  > +		};
>  
>  Don't specify JFFS2.  Combine these two partitions into one.
Ok, I'll merge up two partition.
>  
>  > +		partition at 600000 {
>  > +			/* 4MB for Compressed Root file System Image */
>  > +			reg = <0x00600000 0x00400000>;
>  > +			label = "NAND Compressed RFS Image";
>  > +		};
>  > +
>  > +		partition at a00000 {
>  > +			/* 15MB for JFFS2 based Root file System */
>  > +			reg = <0x00a00000 0x00f00000>;
>  > +			label = "NAND JFFS2 Root File System";
>  > +		};
>  
>  Likewise.
>  
>  > +		partition at 1900000 {
>  > +			/* 7MB for User Area */
>  > +			reg = <0x01900000 0x00700000>;
>  > +			label = "NAND User area";
>  > +		};
>  
>  Above you say there's 4 GiB of NAND, but here you define partitions that
>  only cover 32 MiB.
Can I set one partion include all other space(4GB- 32MB) with label name "Others"?
>  
>  > +	};
>  > +
>  > +	cpld at 2,0 {
>  > +		#address-cells = <1>;
>  > +		#size-cells = <1>;
>  > +		compatible = "fsl,c293pcie-cpld";
>  > +		reg = <0x2 0x0 0x0000020>;
>  > +		bank-width = <1>;
>  > +		device-width = <1>;
>  > +	};
>  
>  What do bank-width and device-width mean here?
I will remove these two lines? I thought I copy from other platform.
>  
>  Why all the leading zeroes in 0x0000020?
I'll change to 0x20 from 0x0000020.
>  
>  > +			partition at 580000 {
>  > +				/* 4MB for Compressed RFS Image */
>  > +				reg = <0x00580000 0x00400000>;
>  > +				label = "SPI Flash Compressed RFSImage";
>  > +			};
>  > +
>  > +			partition at 980000 {
>  > +				/* 6.5MB for JFFS2 based RFS */
>  > +				reg = <0x00980000 0x00680000>;
>  > +				label = "SPI Flash JFFS2 RFS";
>  > +			};
>  
>  Again, merge these two and don't specify JFFS2.
Ok, thanks
>  
>  > diff --git a/arch/powerpc/platforms/85xx/Kconfig
>  > b/arch/powerpc/platforms/85xx/Kconfig
>  > index a0dcd57..df26b21 100644
>  > --- a/arch/powerpc/platforms/85xx/Kconfig
>  > +++ b/arch/powerpc/platforms/85xx/Kconfig
>  > @@ -32,6 +32,13 @@ config BSC9131_RDB
>  >  	  StarCore SC3850 DSP
>  >  	  Manufacturer : Freescale Semiconductor, Inc
>  >
>  > +config C293_PCIE
>  > +	  bool "Freescale C293PCIE"
>  > +	  select DEFAULT_UIMAGE
>  > +	  select SWIOTLB
>  > +	  help
>  > +	  This option enables support for the C293PCIE board
>  
>  Why do you need SWIOTLB if the board has 512 MiB soldered RAM?
I'll remove it.
>  
>  > diff --git a/arch/powerpc/platforms/85xx/c293pcie.c
>  > b/arch/powerpc/platforms/85xx/c293pcie.c
>  > new file mode 100644
>  > index 0000000..75dda12
>  > --- /dev/null
>  > +++ b/arch/powerpc/platforms/85xx/c293pcie.c
>  > @@ -0,0 +1,82 @@
>  > +/*
>  > + * C293PCIE Board Setup
>  > + *
>  > + * Copyright 2013 Freescale Semiconductor Inc.
>  > + *
>  > + * This program is free software; you can redistribute  it and/or
>  > +modify it
>  > + * under  the terms of  the GNU General  Public License as published
>  > +by the
>  > + * Free Software Foundation;  either version 2 of the  License, or
>  > +(at your
>  > + * option) any later version.
>  > + */
>  > +
>  > +#include <linux/stddef.h>
>  > +#include <linux/kernel.h>
>  > +#include <linux/pci.h>
>  > +#include <linux/delay.h>
>  > +#include <linux/interrupt.h>
>  > +#include <linux/of_platform.h>
>  > +
>  > +#include <asm/time.h>
>  > +#include <asm/machdep.h>
>  > +#include <asm/pci-bridge.h>
>  > +#include <mm/mmu_decl.h>
>  > +#include <asm/prom.h>
>  > +#include <asm/udbg.h>
>  > +#include <asm/mpic.h>
>  > +
>  > +#include <sysdev/fsl_soc.h>
>  > +#include <sysdev/fsl_pci.h>
>  > +
>  > +#include "mpc85xx.h"
>  
>  Are you sure you need all of these?  I don't see any delays, for example.
Thanks, I'll test and remove redundant includes.
>  
>  -Scott