[PATCH 2/2] ARM: dts: AM4372: add few nodes

On Mon, Aug 05, 2013 at 06:08:45AM +0100, Afzal Mohammed wrote:

Hi Muguthan,

On Saturday 03 August 2013 05:19 PM, Mugunthan V N wrote:

quoted

On 8/2/2013 7:16 PM, Afzal Mohammed wrote:

quoted

quoted

+		mac: ethernet at 4a100000 {
+			compatible = "ti,am4372-cpsw","ti,cpsw";

quoted

compatibility "ti,am4372-cpsw" is not needed as driver has only "ti,cpsw"
compatibility

No, please read device tree documentation [1].

DT is a pure hardware description, it does not depend on driver, 
dependency is only vice versa.

One point worth mentioning is that the "ti,am4372-cpsw" string isn't
documented. Will the "ti,am4372-cpsw" binding definitely be a superset
of the "ti,cpsw" binding, and if you were to take the DT as of this
patch, and attempt to use it with a future kernel, can you guarantee
it'll work? 

quoted

quoted

+			reg = <0x4a100000 0x800
+			       0x4a101200 0x100>;
+			interrupts = <GIC_SPI 40 IRQ_TYPE_LEVEL_HIGH
+				      GIC_SPI 41 IRQ_TYPE_LEVEL_HIGH
+				      GIC_SPI 42 IRQ_TYPE_LEVEL_HIGH
+				      GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>;
+			ti,hwmods = "cpgmac0";
+			status = "disabled";
+		};

quoted

There are many other parameters which are missed here.

Reason has been mentioned in the commit message, quoting relevant here 
again,

Actually, as you've marked the nodes disabled, it's probably fine. But
worth considering as properties are added...

Thanks,
Mark.

 >> For i2c, spi, cpsw & pwm - only the properties that were sure to be
 >> correct has been added (main intention is to make hwmod happy and
 >> avoid any later modification to here added properties).

I really wanted to avoid a later patch that has a line starting with 
minus on DTS.

Since you are working on cpsw support, can you help here with a patch 
for other properties.

Regards
Afzal

[1] 
http://devicetree.org/Device_Tree_Usage#Understanding_the_compatible_Property