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@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 > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html