Re: [1/4] powerpc/fsl-booke: Add device tree support for T1024/T1023 SoC

[Scott Wood <scottwood@freescale.com>]

On Mon, 2015-03-30 at 22:32 -0500, Liu Shengzhou-B36685 wrote:
> > > > There are other differences between t1023 an t1024.  Where do you
> > > > describe t1024's QE?  Where do you describe the DDR and IFC differences?
> > > > can they be detected at runtime?  t1024 supports deep sleep, but
> > > > t1023 doesn't -- yet you label both chips as having t1024 rcpm.
> > > >
> > > As QE IP block has not been upstream yet, 
> > Huh? 
> > arch/powerpc/sysdev/qe_lib/
> 
> arch/powerpc/boot/dts/fsl/qoriq-tdm1.0.dtsi has not been upstream by TDM owner.
> Ok, I will first send qoriq-tdm1.0.dtsi upstream in order to include QE in t1024 dts.

Thanks, but make sure there's also a binding for it.

> > > DDR and IFC differences are in u-boot, not in dts. 
> > The differences are in hardware, which is what the dts is supposed to describe.
> 
> Theoretically I think so, but not all hardware details must be described in dts

No, but all hardware should be properly identified.

> as current IP driver doesn't take care of it from dts.

The device tree describes the hardware, not the driver.

> If so, IP owners will have to update drivers, for now let's keep as it's.    

Please don't use the phrase "IP owner" in upstream discussions.  Besides
being a bad name for "maintainer", SDK maintainership isn't relevant
here.

> > > Both t1023 and t1024 support sleep, so label both chips as having t1024 rcpm.
> > 
> > That's not how it works.
> > 
> > > Only t1024 has deep sleep, the difference is identified in *.c not in dts (confirmed with deep sleep owner).
> > 
> > Even if the C code chooses to use SVR to identify the difference (why?),
> > that doesn't mean it's OK for the device tree to contain wrong information.
>  
> Where is the wrong information?
> 
>         rcpm: global-utilities@e2000 {
>                 compatible = "fsl,t1024-rcpm", "fsl,qoriq-rcpm-2.0";
>                 reg = <0xe2000 0x1000>;
>         };
> 
>         sdhc@114000 {
>                 compatible = "fsl,t1024-esdhc", "fsl,esdhc";
>                 fsl,iommu-parent = <&pamu0>;
>                 fsl,liodn-reg = <&guts 0x530>; /* eSDHCLIODNR */
>                 sdhci,auto-cmd12;
>                 no-1-8-v;
>                 sleep = <&rcpm 0x00000080>;
>         };
> t1023 also supports sleep(not deep sleep), it needs the info above.

The part that's wrong is where it says "t1024".  It's not t1024 and for
rcpm it's not 100% compatible with t1024.

-Scott