Re: [PATCH v2 6/6] DTS: add SPI flash(s25fl128p01) support on p4080ds and mpc8536ds board

2010-08-10 Thread Grant Likely
Hi Mingkai,

one comment below.  Otherwise this patch looks good, and so does patch 5.

g.

On Mon, Aug 2, 2010 at 1:52 AM, Mingkai Hu  wrote:
> Signed-off-by: Mingkai Hu 
> ---
>
> v2:
>  - Remove the whitespace inconsitencies
>
>  arch/powerpc/boot/dts/mpc8536ds.dts |   52 
> +++
>  arch/powerpc/boot/dts/p4080ds.dts   |   11 +++-
>  2 files changed, 56 insertions(+), 7 deletions(-)
>
> diff --git a/arch/powerpc/boot/dts/mpc8536ds.dts 
> b/arch/powerpc/boot/dts/mpc8536ds.dts
> index 815cebb..a75c10e 100644
> --- a/arch/powerpc/boot/dts/mpc8536ds.dts
> +++ b/arch/powerpc/boot/dts/mpc8536ds.dts
> @@ -108,6 +108,58 @@
>                        };
>                };
>
> +               s...@7000 {
> +                       #address-cells = <1>;
> +                       #size-cells = <0>;
> +                       compatible = "fsl,mpc8536-espi";
> +                       reg = <0x7000 0x1000>;
> +                       interrupts = <59 0x2>;
> +                       interrupt-parent = <&mpic>;
> +                       fsl,espi-num-chipselects = <4>;
> +
> +                       fl...@0 {
> +                               #address-cells = <1>;
> +                               #size-cells = <1>;
> +                               compatible = "spansion,s25sl12801";
> +                               reg = <0>;
> +                               spi-max-frequency = <4000>;
> +                               partit...@u-boot {
> +                                       label = "u-boot";
> +                                       reg = <0x 0x0010>;
> +                                       read-only;
> +                               };
> +                               partit...@kernel {
> +                                       label = "kernel";
> +                                       reg = <0x0010 0x0050>;
> +                                       read-only;
> +                               };
> +                               partit...@dtb {
> +                                       label = "dtb";
> +                                       reg = <0x0060 0x0010>;
> +                                       read-only;
> +                               };
> +                               partit...@fs {
> +                                       label = "file system";
> +                                       reg = <0x0070 0x0090>;
> +                               };
> +                       };
> +                       fl...@1 {
> +                               compatible = "spansion,s25sl12801";
> +                               reg = <1>;
> +                               spi-max-frequency = <4000>;
> +                       };
> +                       fl...@2 {
> +                               compatible = "spansion,s25sl12801";
> +                               reg = <2>;
> +                               spi-max-frequency = <4000>;
> +                       };
> +                       fl...@3 {
> +                               compatible = "spansion,s25sl12801";
> +                               reg = <3>;
> +                               spi-max-frequency = <4000>;
> +                       };
> +               };
> +
>                ...@21300 {
>                        #address-cells = <1>;
>                        #size-cells = <1>;
> diff --git a/arch/powerpc/boot/dts/p4080ds.dts 
> b/arch/powerpc/boot/dts/p4080ds.dts
> index 6b29eab..48437ad 100644
> --- a/arch/powerpc/boot/dts/p4080ds.dts
> +++ b/arch/powerpc/boot/dts/p4080ds.dts
> @@ -236,22 +236,19 @@
>                };
>
>                ...@11 {
> -                       cell-index = <0>;
>                        #address-cells = <1>;
>                        #size-cells = <0>;
> -                       compatible = "fsl,espi";
> +                       compatible = "fsl,mpc8536-espi";

Should be more specific here by specifying the exact device; plus a
list of what it is compatible with.  For example:

compatible = "fsl,p4080-espi", "fsl,mpc5836-espi";

the reason for this is that the driver for the existing part is still
able to bind against the node, but if it ever needs it, then
information about the specific device is available which can be used
to (for example) figure out when to enable silicon bug workarounds.

>                        reg = <0x11 0x1000>;
>                        interrupts = <53 0x2>;
>                        interrupt-parent = <&mpic>;
> -                       espi,num-ss-bits = <4>;
> -                       mode = "cpu";
> +                       fsl,espi-num-chipselects = <4>;
>
> -                       fsl_m25...@0 {
> +                       fl...@0 {
>                                #address-cells = <1>;
>                                #size-cells = <1>;
> -                               compatible = "fsl,espi-flash";
> +                               compatible = "spansion,s25sl12801";
>                      

RE: [PATCH v2 6/6] DTS: add SPI flash(s25fl128p01) support on p4080ds and mpc8536ds board

2010-09-02 Thread Hu Mingkai-B21284


> -Original Message-
> From: glik...@secretlab.ca [mailto:glik...@secretlab.ca] On Behalf Of Grant
> Likely
> Sent: Tuesday, August 10, 2010 3:11 PM
> To: Hu Mingkai-B21284
> Cc: linuxppc-...@ozlabs.org; spi-devel-gene...@lists.sourceforge.net; Gala
> Kumar-B11780; Zang Roy-R61911
> Subject: Re: [PATCH v2 6/6] DTS: add SPI flash(s25fl128p01) support on p4080ds
> and mpc8536ds board
> 
> Hi Mingkai,
> 
> one comment below.  Otherwise this patch looks good, and so does patch 5.
> 
> g.
> 
> On Mon, Aug 2, 2010 at 1:52 AM, Mingkai Hu  wrote:
> > Signed-off-by: Mingkai Hu 
> > ---
> >
> > v2:
> >  - Remove the whitespace inconsitencies
> >
> >  arch/powerpc/boot/dts/mpc8536ds.dts |   52
> > +++
> >  arch/powerpc/boot/dts/p4080ds.dts   |   11 +++-
> >  2 files changed, 56 insertions(+), 7 deletions(-)
> >
> > diff --git a/arch/powerpc/boot/dts/mpc8536ds.dts
> > b/arch/powerpc/boot/dts/mpc8536ds.dts
> > index 815cebb..a75c10e 100644
> > --- a/arch/powerpc/boot/dts/mpc8536ds.dts
> > +++ b/arch/powerpc/boot/dts/mpc8536ds.dts
> > @@ -108,6 +108,58 @@
> >                        };
> >                };
> >
> > +               s...@7000 {
> > +                       #address-cells = <1>;
> > +                       #size-cells = <0>;
> > +                       compatible = "fsl,mpc8536-espi";
> > +                       reg = <0x7000 0x1000>;
> > +                       interrupts = <59 0x2>;
> > +                       interrupt-parent = <&mpic>;
> > +                       fsl,espi-num-chipselects = <4>;
> > +
> > +                       fl...@0 {
> > +                               #address-cells = <1>;
> > +                               #size-cells = <1>;
> > +                               compatible = "spansion,s25sl12801";
> > +                               reg = <0>;
> > +                               spi-max-frequency = <4000>;
> > +                               partit...@u-boot {
> > +                                       label = "u-boot";
> > +                                       reg = <0x 0x0010>;
> > +                                       read-only;
> > +                               };
> > +                               partit...@kernel {
> > +                                       label = "kernel";
> > +                                       reg = <0x0010 0x0050>;
> > +                                       read-only;
> > +                               };
> > +                               partit...@dtb {
> > +                                       label = "dtb";
> > +                                       reg = <0x0060 0x0010>;
> > +                                       read-only;
> > +                               };
> > +                               partit...@fs {
> > +                                       label = "file system";
> > +                                       reg = <0x0070 0x0090>;
> > +                               };
> > +                       };
> > +                       fl...@1 {
> > +                               compatible = "spansion,s25sl12801";
> > +                               reg = <1>;
> > +                               spi-max-frequency = <4000>;
> > +                       };
> > +                       fl...@2 {
> > +                               compatible = "spansion,s25sl12801";
> > +                               reg = <2>;
> > +                               spi-max-frequency = <4000>;
> > +                       };
> > +                       fl...@3 {
> > +                               compatible = "spansion,s25sl12801";
> > +                               reg = <3>;
> > +                               spi-max-frequency = <4000>;
> > +                       };
> > +               };
> > +
> >                ...@21300 {
> >                        #address-cells = <1>;
> >                        #size-cells = <1>; diff --git
> > a/arch/powerpc/boot/dts/p4080ds.dts
> > b/arch/powerpc/boot/dts/p4080ds.dts
> > index 6b29eab..48437ad 100644
> > --- a/arch/powerpc/boot/dts/p4080ds.dts
> > +++ b/arch/powerpc/boot/dts/p4080ds.dts
> > @@ -236,22 +236,19 @@
> >                };
> >
>