Subject: Re: 答复: [PATCH 2/2] powerpc/85xx: add the P1020RDB-PD DTS support

From: Scott Wood <[email-masked]>
Date: Mon, 8 Jul 2013 12:08:58 -0500

On 07/03/2013 09:04:40 PM, Zhang Haijun-B42677 wrote:
> 
> 
> Regards & Thanks
> 
> Haijun.
> 
> ________________________________________
> 发件人: Wood Scott-B07421
> 发送时间: 2013年7月3日 19:09
> 收件人: Zhang Haijun-B42677
> Cc: [email-masked]; [email-masked]; Zhang  
> Haijun-B42677; Huang Changming-R66093
> 主题: Re: [PATCH 2/2] powerpc/85xx: add the P1020RDB-PD DTS support
> 
> On 06/30/2013 11:12:23 PM, Haijun Zhang wrote:
> > From: "Haijun.Zhang" <[email-masked]>
> >
> > Overview of P1020RDB-PD device:
> > - DDR3 2GB
> > - NOR flash 64MB
> > - NAND flash 128MB
> > - SPI flash 16MB
> > - I2C EEPROM 256Kb
> > - eTSEC1 (RGMII PHY) connected to VSC7385 L2 switch
> > - eTSEC2 (SGMII PHY)
> > - eTSEC3 (RGMII PHY)
> > - SDHC
> > - 2 USB ports
> > - 4 TDM ports
> > - PCIe
> >
> > Signed-off-by: Haijun Zhang <[email-masked]>
> > Signed-off-by: Jerry Huang <[email-masked]>
> > CC: Scott Wood <[email-masked]>
> > ---
> >  arch/powerpc/boot/dts/p1020rdb-pd.dtsi    | 257
> > ++++++++++++++++++++++++++++++
> >  arch/powerpc/boot/dts/p1020rdb-pd_32b.dts |  90 +++++++++++
> >  2 files changed, 347 insertions(+)
> >  create mode 100644 arch/powerpc/boot/dts/p1020rdb-pd.dtsi
> >  create mode 100644 arch/powerpc/boot/dts/p1020rdb-pd_32b.dts
> 
> What about 36b?
> 
> Haijun: 2G DDR was fix on P1020RDB-PD board. No need 36bit support.  
> Also no 36bit uboot support.

So then why not just have one "p1020rdb-pd.dts"?

Or, can you have a common p1020rdb.dtsi that all the variants use, with  
the .dts file only handling the differences?

> > +     cpld@2,0 {
> > +             #address-cells = <1>;
> > +             #size-cells = <1>;
> > +             compatible = "cpld";
> > +             reg = <0x2 0x0 0x20000>;
> > +             read-only;
> > +     };
> 
> Where does "cpld" as a compatible come from (it's way too vague)?   
> What
> is read-only supposed to mean here?
> 
> Haijun: In fact almost all board has its special CPLD. This node just  
> copy from p1020rdb-pc board.
> 
> So, change it to :
> 
> cpld@2,0 {
>           compatible = "fsl, p1020rdb-cpld";
>           reg = <0x2 0x0 0x20000>;
>           read-only;
> };

Again, what is read-only supposed to mean here?  I don't care that it's  
copied from the PC version.  It doesn't make sense, and is not  
documented in a binding.

Is the -pd CPLD 100% identical to the -pc CPLD?  If not, shouldn't the  
compatible note the difference between the two?

-Scott