> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Tuesday, July 23, 2013 6:59 AM
> To: Liu Po-B43644
> Cc: linuxppc-...@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...@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...@freescale.com>
> > Singed-off-by: Po Liu <po....@freescale.com>
>
> Signed
>
> > + partition@900000 {
> > + /* 33MB for rootfs */
> > + reg = <0x00900000 0x02100000>;
> > + label = "NOR Rootfs Image";
> > + };
> > +
> > + partition@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@600000 {
> > + /* 4MB for Compressed Root file System Image */
> > + reg = <0x00600000 0x00400000>;
> > + label = "NAND Compressed RFS Image";
> > + };
> > +
> > + partition@a00000 {
> > + /* 15MB for JFFS2 based Root file System */
> > + reg = <0x00a00000 0x00f00000>;
> > + label = "NAND JFFS2 Root File System";
> > + };
>
> Likewise.
>
> > + partition@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@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@580000 {
> > + /* 4MB for Compressed RFS Image */
> > + reg = <0x00580000 0x00400000>;
> > + label = "SPI Flash Compressed RFSImage";
> > + };
> > +
> > + partition@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
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
