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

[Mark Rutland]

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:
> > On 8/2/2013 7:16 PM, Afzal Mohammed wrote:
> 
> >> +		mac: ethernet at 4a100000 {
> >> +			compatible = "ti,am4372-cpsw","ti,cpsw";
> 
> > 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? 

> 
> >> +			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";
> >> +		};
> 
> > 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
>