On Mon, 2015-03-30 at 22:32 -0500, Liu Shengzhou-B36685 wrote:

quoted

quoted

quoted

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.

quoted

quoted

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.

quoted

quoted

Both t1023 and t1024 support sleep, so label both chips as having t1024 rcpm.

That's not how it works.

quoted

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