* [PATCH v3] arm: dts: AM43x: Add usb DT nodes for AM4372 @ 2013-07-10 8:14 ` George Cherian 0 siblings, 0 replies; 16+ messages in thread From: George Cherian @ 2013-07-10 8:14 UTC (permalink / raw) To: linux-arm-kernel, linux-omap Cc: linux, benoit.cousson, a-kishore, afzal, linux-kernel, balbi, George Cherian This patch adds - HS USB nodes - phy nodes - usb control module nodes. Signed-off-by: George Cherian <george.cherian@ti.com> --- changes from v2 change synopsis to snps use simple node names add both USB and PHY instances add usbctrl node changes from v1 renamed synopsis to snps removed flag tx-fifo-resize arch/arm/boot/dts/am4372.dtsi | 58 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/arch/arm/boot/dts/am4372.dtsi b/arch/arm/boot/dts/am4372.dtsi index ddc1df7..37f196f 100644 --- a/arch/arm/boot/dts/am4372.dtsi +++ b/arch/arm/boot/dts/am4372.dtsi @@ -64,5 +64,63 @@ compatible = "ti,am4372-counter32k","ti,omap-counter32k"; reg = <0x44e86000 0x40>; }; + + phy1: usbphy1 { + compatible = "ti,am4372-usb2"; + #phy-cells = <0>; + id = <0>; + status = "disabled"; + }; + + phy2: usbphy2 { + compatible = "ti,am4372-usb2"; + #phy-cells = <0>; + id = <1>; + status = "disabled"; + }; + + usbctrl: omap-control-usb@44e10620 { + compatible = "ti,omap-control-usb"; + reg = <0x44e10620 0x10>; + reg-names = "control_dev_conf"; + ti,type = <3>; + status = "disabled"; + }; + + usb1: am4372_dwc3@48380000 { + status = "disabled"; + compatible = "ti,am4372-dwc3"; + reg = <0x48380000 0x200>; + interrupts = <GIC_SPI 172 IRQ_TYPE_LEVEL_HIGH>; + #address-cells = <1>; + #size-cells = <1>; + utmi-mode = <1>; + ranges; + dwc3@48390000 { + compatible = "snps,dwc3"; + reg = <0x48390000 0xd000>; + interrupts = <GIC_SPI 168 IRQ_TYPE_LEVEL_HIGH>; + phys = <&phy1>; + phy-names = "am4372-usb1"; + }; + }; + + usb2: am4372_dwc3@483c0000 { + status = "disabled"; + compatible = "ti,am4372-dwc3"; + reg = <0x483c0000 0x200>; + interrupts = <GIC_SPI 178 IRQ_TYPE_LEVEL_HIGH>; + #address-cells = <1>; + #size-cells = <1>; + utmi-mode = <1>; + ranges; + dwc3@483d0000 { + compatible = "snps,dwc3"; + reg = <0x483d0000 0xd000>; + interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>; + phys = <&phy2>; + phy-names = "am4372-usb2"; + }; + }; }; }; -- 1.8.1.4 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v3] arm: dts: AM43x: Add usb DT nodes for AM4372 @ 2013-07-10 8:14 ` George Cherian 0 siblings, 0 replies; 16+ messages in thread From: George Cherian @ 2013-07-10 8:14 UTC (permalink / raw) To: linux-arm-kernel This patch adds - HS USB nodes - phy nodes - usb control module nodes. Signed-off-by: George Cherian <george.cherian@ti.com> --- changes from v2 change synopsis to snps use simple node names add both USB and PHY instances add usbctrl node changes from v1 renamed synopsis to snps removed flag tx-fifo-resize arch/arm/boot/dts/am4372.dtsi | 58 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/arch/arm/boot/dts/am4372.dtsi b/arch/arm/boot/dts/am4372.dtsi index ddc1df7..37f196f 100644 --- a/arch/arm/boot/dts/am4372.dtsi +++ b/arch/arm/boot/dts/am4372.dtsi @@ -64,5 +64,63 @@ compatible = "ti,am4372-counter32k","ti,omap-counter32k"; reg = <0x44e86000 0x40>; }; + + phy1: usbphy1 { + compatible = "ti,am4372-usb2"; + #phy-cells = <0>; + id = <0>; + status = "disabled"; + }; + + phy2: usbphy2 { + compatible = "ti,am4372-usb2"; + #phy-cells = <0>; + id = <1>; + status = "disabled"; + }; + + usbctrl: omap-control-usb at 44e10620 { + compatible = "ti,omap-control-usb"; + reg = <0x44e10620 0x10>; + reg-names = "control_dev_conf"; + ti,type = <3>; + status = "disabled"; + }; + + usb1: am4372_dwc3 at 48380000 { + status = "disabled"; + compatible = "ti,am4372-dwc3"; + reg = <0x48380000 0x200>; + interrupts = <GIC_SPI 172 IRQ_TYPE_LEVEL_HIGH>; + #address-cells = <1>; + #size-cells = <1>; + utmi-mode = <1>; + ranges; + dwc3 at 48390000 { + compatible = "snps,dwc3"; + reg = <0x48390000 0xd000>; + interrupts = <GIC_SPI 168 IRQ_TYPE_LEVEL_HIGH>; + phys = <&phy1>; + phy-names = "am4372-usb1"; + }; + }; + + usb2: am4372_dwc3 at 483c0000 { + status = "disabled"; + compatible = "ti,am4372-dwc3"; + reg = <0x483c0000 0x200>; + interrupts = <GIC_SPI 178 IRQ_TYPE_LEVEL_HIGH>; + #address-cells = <1>; + #size-cells = <1>; + utmi-mode = <1>; + ranges; + dwc3 at 483d0000 { + compatible = "snps,dwc3"; + reg = <0x483d0000 0xd000>; + interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>; + phys = <&phy2>; + phy-names = "am4372-usb2"; + }; + }; }; }; -- 1.8.1.4 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v3] arm: dts: AM43x: Add usb DT nodes for AM4372 @ 2013-07-10 8:14 ` George Cherian 0 siblings, 0 replies; 16+ messages in thread From: George Cherian @ 2013-07-10 8:14 UTC (permalink / raw) To: linux-arm-kernel, linux-omap Cc: linux, benoit.cousson, a-kishore, afzal, linux-kernel, balbi, George Cherian This patch adds - HS USB nodes - phy nodes - usb control module nodes. Signed-off-by: George Cherian <george.cherian@ti.com> --- changes from v2 change synopsis to snps use simple node names add both USB and PHY instances add usbctrl node changes from v1 renamed synopsis to snps removed flag tx-fifo-resize arch/arm/boot/dts/am4372.dtsi | 58 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/arch/arm/boot/dts/am4372.dtsi b/arch/arm/boot/dts/am4372.dtsi index ddc1df7..37f196f 100644 --- a/arch/arm/boot/dts/am4372.dtsi +++ b/arch/arm/boot/dts/am4372.dtsi @@ -64,5 +64,63 @@ compatible = "ti,am4372-counter32k","ti,omap-counter32k"; reg = <0x44e86000 0x40>; }; + + phy1: usbphy1 { + compatible = "ti,am4372-usb2"; + #phy-cells = <0>; + id = <0>; + status = "disabled"; + }; + + phy2: usbphy2 { + compatible = "ti,am4372-usb2"; + #phy-cells = <0>; + id = <1>; + status = "disabled"; + }; + + usbctrl: omap-control-usb@44e10620 { + compatible = "ti,omap-control-usb"; + reg = <0x44e10620 0x10>; + reg-names = "control_dev_conf"; + ti,type = <3>; + status = "disabled"; + }; + + usb1: am4372_dwc3@48380000 { + status = "disabled"; + compatible = "ti,am4372-dwc3"; + reg = <0x48380000 0x200>; + interrupts = <GIC_SPI 172 IRQ_TYPE_LEVEL_HIGH>; + #address-cells = <1>; + #size-cells = <1>; + utmi-mode = <1>; + ranges; + dwc3@48390000 { + compatible = "snps,dwc3"; + reg = <0x48390000 0xd000>; + interrupts = <GIC_SPI 168 IRQ_TYPE_LEVEL_HIGH>; + phys = <&phy1>; + phy-names = "am4372-usb1"; + }; + }; + + usb2: am4372_dwc3@483c0000 { + status = "disabled"; + compatible = "ti,am4372-dwc3"; + reg = <0x483c0000 0x200>; + interrupts = <GIC_SPI 178 IRQ_TYPE_LEVEL_HIGH>; + #address-cells = <1>; + #size-cells = <1>; + utmi-mode = <1>; + ranges; + dwc3@483d0000 { + compatible = "snps,dwc3"; + reg = <0x483d0000 0xd000>; + interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>; + phys = <&phy2>; + phy-names = "am4372-usb2"; + }; + }; }; }; -- 1.8.1.4 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v3] arm: dts: AM43x: Add usb DT nodes for AM4372 2013-07-10 8:14 ` George Cherian (?) @ 2013-08-14 14:16 ` George Cherian -1 siblings, 0 replies; 16+ messages in thread From: George Cherian @ 2013-08-14 14:16 UTC (permalink / raw) To: George Cherian Cc: linux-arm-kernel, linux-omap, linux, benoit.cousson, a-kishore, afzal, linux-kernel, balbi, bcousson +Benoit If you dont have any comments, can you take this for 3.12? Regards -George On 7/10/2013 1:44 PM, George Cherian wrote: > This patch adds > - HS USB nodes > - phy nodes > - usb control module nodes. > > Signed-off-by: George Cherian <george.cherian@ti.com> > --- > > changes from v2 > change synopsis to snps > use simple node names > add both USB and PHY instances > add usbctrl node > > changes from v1 > renamed synopsis to snps > removed flag tx-fifo-resize > > arch/arm/boot/dts/am4372.dtsi | 58 +++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 58 insertions(+) > > diff --git a/arch/arm/boot/dts/am4372.dtsi b/arch/arm/boot/dts/am4372.dtsi > index ddc1df7..37f196f 100644 > --- a/arch/arm/boot/dts/am4372.dtsi > +++ b/arch/arm/boot/dts/am4372.dtsi > @@ -64,5 +64,63 @@ > compatible = "ti,am4372-counter32k","ti,omap-counter32k"; > reg = <0x44e86000 0x40>; > }; > + > + phy1: usbphy1 { > + compatible = "ti,am4372-usb2"; > + #phy-cells = <0>; > + id = <0>; > + status = "disabled"; > + }; > + > + phy2: usbphy2 { > + compatible = "ti,am4372-usb2"; > + #phy-cells = <0>; > + id = <1>; > + status = "disabled"; > + }; > + > + usbctrl: omap-control-usb@44e10620 { > + compatible = "ti,omap-control-usb"; > + reg = <0x44e10620 0x10>; > + reg-names = "control_dev_conf"; > + ti,type = <3>; > + status = "disabled"; > + }; > + > + usb1: am4372_dwc3@48380000 { > + status = "disabled"; > + compatible = "ti,am4372-dwc3"; > + reg = <0x48380000 0x200>; > + interrupts = <GIC_SPI 172 IRQ_TYPE_LEVEL_HIGH>; > + #address-cells = <1>; > + #size-cells = <1>; > + utmi-mode = <1>; > + ranges; > + dwc3@48390000 { > + compatible = "snps,dwc3"; > + reg = <0x48390000 0xd000>; > + interrupts = <GIC_SPI 168 IRQ_TYPE_LEVEL_HIGH>; > + phys = <&phy1>; > + phy-names = "am4372-usb1"; > + }; > + }; > + > + usb2: am4372_dwc3@483c0000 { > + status = "disabled"; > + compatible = "ti,am4372-dwc3"; > + reg = <0x483c0000 0x200>; > + interrupts = <GIC_SPI 178 IRQ_TYPE_LEVEL_HIGH>; > + #address-cells = <1>; > + #size-cells = <1>; > + utmi-mode = <1>; > + ranges; > + dwc3@483d0000 { > + compatible = "snps,dwc3"; > + reg = <0x483d0000 0xd000>; > + interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>; > + phys = <&phy2>; > + phy-names = "am4372-usb2"; > + }; > + }; > }; > }; -- -George ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v3] arm: dts: AM43x: Add usb DT nodes for AM4372 @ 2013-08-14 14:16 ` George Cherian 0 siblings, 0 replies; 16+ messages in thread From: George Cherian @ 2013-08-14 14:16 UTC (permalink / raw) To: linux-arm-kernel +Benoit If you dont have any comments, can you take this for 3.12? Regards -George On 7/10/2013 1:44 PM, George Cherian wrote: > This patch adds > - HS USB nodes > - phy nodes > - usb control module nodes. > > Signed-off-by: George Cherian <george.cherian@ti.com> > --- > > changes from v2 > change synopsis to snps > use simple node names > add both USB and PHY instances > add usbctrl node > > changes from v1 > renamed synopsis to snps > removed flag tx-fifo-resize > > arch/arm/boot/dts/am4372.dtsi | 58 +++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 58 insertions(+) > > diff --git a/arch/arm/boot/dts/am4372.dtsi b/arch/arm/boot/dts/am4372.dtsi > index ddc1df7..37f196f 100644 > --- a/arch/arm/boot/dts/am4372.dtsi > +++ b/arch/arm/boot/dts/am4372.dtsi > @@ -64,5 +64,63 @@ > compatible = "ti,am4372-counter32k","ti,omap-counter32k"; > reg = <0x44e86000 0x40>; > }; > + > + phy1: usbphy1 { > + compatible = "ti,am4372-usb2"; > + #phy-cells = <0>; > + id = <0>; > + status = "disabled"; > + }; > + > + phy2: usbphy2 { > + compatible = "ti,am4372-usb2"; > + #phy-cells = <0>; > + id = <1>; > + status = "disabled"; > + }; > + > + usbctrl: omap-control-usb at 44e10620 { > + compatible = "ti,omap-control-usb"; > + reg = <0x44e10620 0x10>; > + reg-names = "control_dev_conf"; > + ti,type = <3>; > + status = "disabled"; > + }; > + > + usb1: am4372_dwc3 at 48380000 { > + status = "disabled"; > + compatible = "ti,am4372-dwc3"; > + reg = <0x48380000 0x200>; > + interrupts = <GIC_SPI 172 IRQ_TYPE_LEVEL_HIGH>; > + #address-cells = <1>; > + #size-cells = <1>; > + utmi-mode = <1>; > + ranges; > + dwc3 at 48390000 { > + compatible = "snps,dwc3"; > + reg = <0x48390000 0xd000>; > + interrupts = <GIC_SPI 168 IRQ_TYPE_LEVEL_HIGH>; > + phys = <&phy1>; > + phy-names = "am4372-usb1"; > + }; > + }; > + > + usb2: am4372_dwc3 at 483c0000 { > + status = "disabled"; > + compatible = "ti,am4372-dwc3"; > + reg = <0x483c0000 0x200>; > + interrupts = <GIC_SPI 178 IRQ_TYPE_LEVEL_HIGH>; > + #address-cells = <1>; > + #size-cells = <1>; > + utmi-mode = <1>; > + ranges; > + dwc3 at 483d0000 { > + compatible = "snps,dwc3"; > + reg = <0x483d0000 0xd000>; > + interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>; > + phys = <&phy2>; > + phy-names = "am4372-usb2"; > + }; > + }; > }; > }; -- -George ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3] arm: dts: AM43x: Add usb DT nodes for AM4372 @ 2013-08-14 14:16 ` George Cherian 0 siblings, 0 replies; 16+ messages in thread From: George Cherian @ 2013-08-14 14:16 UTC (permalink / raw) To: George Cherian Cc: linux-arm-kernel, linux-omap, linux, benoit.cousson, a-kishore, afzal, linux-kernel, balbi, bcousson +Benoit If you dont have any comments, can you take this for 3.12? Regards -George On 7/10/2013 1:44 PM, George Cherian wrote: > This patch adds > - HS USB nodes > - phy nodes > - usb control module nodes. > > Signed-off-by: George Cherian <george.cherian@ti.com> > --- > > changes from v2 > change synopsis to snps > use simple node names > add both USB and PHY instances > add usbctrl node > > changes from v1 > renamed synopsis to snps > removed flag tx-fifo-resize > > arch/arm/boot/dts/am4372.dtsi | 58 +++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 58 insertions(+) > > diff --git a/arch/arm/boot/dts/am4372.dtsi b/arch/arm/boot/dts/am4372.dtsi > index ddc1df7..37f196f 100644 > --- a/arch/arm/boot/dts/am4372.dtsi > +++ b/arch/arm/boot/dts/am4372.dtsi > @@ -64,5 +64,63 @@ > compatible = "ti,am4372-counter32k","ti,omap-counter32k"; > reg = <0x44e86000 0x40>; > }; > + > + phy1: usbphy1 { > + compatible = "ti,am4372-usb2"; > + #phy-cells = <0>; > + id = <0>; > + status = "disabled"; > + }; > + > + phy2: usbphy2 { > + compatible = "ti,am4372-usb2"; > + #phy-cells = <0>; > + id = <1>; > + status = "disabled"; > + }; > + > + usbctrl: omap-control-usb@44e10620 { > + compatible = "ti,omap-control-usb"; > + reg = <0x44e10620 0x10>; > + reg-names = "control_dev_conf"; > + ti,type = <3>; > + status = "disabled"; > + }; > + > + usb1: am4372_dwc3@48380000 { > + status = "disabled"; > + compatible = "ti,am4372-dwc3"; > + reg = <0x48380000 0x200>; > + interrupts = <GIC_SPI 172 IRQ_TYPE_LEVEL_HIGH>; > + #address-cells = <1>; > + #size-cells = <1>; > + utmi-mode = <1>; > + ranges; > + dwc3@48390000 { > + compatible = "snps,dwc3"; > + reg = <0x48390000 0xd000>; > + interrupts = <GIC_SPI 168 IRQ_TYPE_LEVEL_HIGH>; > + phys = <&phy1>; > + phy-names = "am4372-usb1"; > + }; > + }; > + > + usb2: am4372_dwc3@483c0000 { > + status = "disabled"; > + compatible = "ti,am4372-dwc3"; > + reg = <0x483c0000 0x200>; > + interrupts = <GIC_SPI 178 IRQ_TYPE_LEVEL_HIGH>; > + #address-cells = <1>; > + #size-cells = <1>; > + utmi-mode = <1>; > + ranges; > + dwc3@483d0000 { > + compatible = "snps,dwc3"; > + reg = <0x483d0000 0xd000>; > + interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>; > + phys = <&phy2>; > + phy-names = "am4372-usb2"; > + }; > + }; > }; > }; -- -George ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3] arm: dts: AM43x: Add usb DT nodes for AM4372 2013-08-14 14:16 ` George Cherian @ 2013-08-14 14:34 ` Benoit Cousson -1 siblings, 0 replies; 16+ messages in thread From: Benoit Cousson @ 2013-08-14 14:34 UTC (permalink / raw) To: George Cherian Cc: linux-arm-kernel, linux-omap, linux, a-kishore, afzal, linux-kernel, balbi, Roger Quadros + Roger Hi George, Yes, I had some comment about the "ti'type" in Roger's series that will be applicable here as well. On 14/08/2013 16:16, George Cherian wrote: > +Benoit > If you dont have any comments, can you take this for 3.12? > > Regards > -George > > On 7/10/2013 1:44 PM, George Cherian wrote: > >> This patch adds >> - HS USB nodes >> - phy nodes >> - usb control module nodes. >> >> Signed-off-by: George Cherian <george.cherian@ti.com> >> --- >> >> changes from v2 >> change synopsis to snps >> use simple node names >> add both USB and PHY instances >> add usbctrl node >> >> changes from v1 >> renamed synopsis to snps >> removed flag tx-fifo-resize >> >> arch/arm/boot/dts/am4372.dtsi | 58 >> +++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 58 insertions(+) >> >> diff --git a/arch/arm/boot/dts/am4372.dtsi >> b/arch/arm/boot/dts/am4372.dtsi >> index ddc1df7..37f196f 100644 >> --- a/arch/arm/boot/dts/am4372.dtsi >> +++ b/arch/arm/boot/dts/am4372.dtsi >> @@ -64,5 +64,63 @@ >> compatible = "ti,am4372-counter32k","ti,omap-counter32k"; >> reg = <0x44e86000 0x40>; >> }; >> + >> + phy1: usbphy1 { >> + compatible = "ti,am4372-usb2"; That's not a very good name for a phy? It looks like a usb module. The bindings specify only ti,omap-usb2 or ti,omap-usb3 for USB2 or USB3. I guess it should be one of them and potentially the binding should be updated with ti,omap-usb2-phy and ti,omap-usb3-phy names while we can still do it. >> + #phy-cells = <0>; >> + id = <0>; >> + status = "disabled"; >> + }; >> + >> + phy2: usbphy2 { Why do you need a different node name here? It should be a generic name to identify the function, so usbphy or usb-phy seems good enough. >> + compatible = "ti,am4372-usb2"; >> + #phy-cells = <0>; >> + id = <1>; >> + status = "disabled"; >> + }; >> + >> + usbctrl: omap-control-usb@44e10620 { >> + compatible = "ti,omap-control-usb"; >> + reg = <0x44e10620 0x10>; >> + reg-names = "control_dev_conf"; >> + ti,type = <3>; >> + status = "disabled"; >> + }; >> + >> + usb1: am4372_dwc3@48380000 { >> + status = "disabled"; >> + compatible = "ti,am4372-dwc3"; >> + reg = <0x48380000 0x200>; >> + interrupts = <GIC_SPI 172 IRQ_TYPE_LEVEL_HIGH>; >> + #address-cells = <1>; >> + #size-cells = <1>; >> + utmi-mode = <1>; >> + ranges; A blank line here will be nice. >> + dwc3@48390000 { >> + compatible = "snps,dwc3"; >> + reg = <0x48390000 0xd000>; >> + interrupts = <GIC_SPI 168 IRQ_TYPE_LEVEL_HIGH>; >> + phys = <&phy1>; >> + phy-names = "am4372-usb1"; What is the purpose of the phy-names? Is it relevant to add the SoC name in something that usually, at least for the clocks and IRQs, is IP specific? The current documentation does not explain it and does not support phys property either. synopsys DWC3 CORE DWC3- USB3 CONTROLLER Required properties: - compatible: must be "synopsys,dwc3" - reg : Address and length of the register set for the device - interrupts: Interrupts used by the dwc3 controller. - usb-phy : array of phandle for the PHY device Is there some binding update ongoing for 3.12? >> + }; >> + }; >> + >> + usb2: am4372_dwc3@483c0000 { >> + status = "disabled"; >> + compatible = "ti,am4372-dwc3"; >> + reg = <0x483c0000 0x200>; >> + interrupts = <GIC_SPI 178 IRQ_TYPE_LEVEL_HIGH>; >> + #address-cells = <1>; >> + #size-cells = <1>; >> + utmi-mode = <1>; >> + ranges; A blank line here will be nice. >> + dwc3@483d0000 { >> + compatible = "snps,dwc3"; >> + reg = <0x483d0000 0xd000>; >> + interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>; >> + phys = <&phy2>; >> + phy-names = "am4372-usb2"; >> + }; >> + }; >> }; >> }; > > Regards, Benoit ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v3] arm: dts: AM43x: Add usb DT nodes for AM4372 @ 2013-08-14 14:34 ` Benoit Cousson 0 siblings, 0 replies; 16+ messages in thread From: Benoit Cousson @ 2013-08-14 14:34 UTC (permalink / raw) To: linux-arm-kernel + Roger Hi George, Yes, I had some comment about the "ti'type" in Roger's series that will be applicable here as well. On 14/08/2013 16:16, George Cherian wrote: > +Benoit > If you dont have any comments, can you take this for 3.12? > > Regards > -George > > On 7/10/2013 1:44 PM, George Cherian wrote: > >> This patch adds >> - HS USB nodes >> - phy nodes >> - usb control module nodes. >> >> Signed-off-by: George Cherian <george.cherian@ti.com> >> --- >> >> changes from v2 >> change synopsis to snps >> use simple node names >> add both USB and PHY instances >> add usbctrl node >> >> changes from v1 >> renamed synopsis to snps >> removed flag tx-fifo-resize >> >> arch/arm/boot/dts/am4372.dtsi | 58 >> +++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 58 insertions(+) >> >> diff --git a/arch/arm/boot/dts/am4372.dtsi >> b/arch/arm/boot/dts/am4372.dtsi >> index ddc1df7..37f196f 100644 >> --- a/arch/arm/boot/dts/am4372.dtsi >> +++ b/arch/arm/boot/dts/am4372.dtsi >> @@ -64,5 +64,63 @@ >> compatible = "ti,am4372-counter32k","ti,omap-counter32k"; >> reg = <0x44e86000 0x40>; >> }; >> + >> + phy1: usbphy1 { >> + compatible = "ti,am4372-usb2"; That's not a very good name for a phy? It looks like a usb module. The bindings specify only ti,omap-usb2 or ti,omap-usb3 for USB2 or USB3. I guess it should be one of them and potentially the binding should be updated with ti,omap-usb2-phy and ti,omap-usb3-phy names while we can still do it. >> + #phy-cells = <0>; >> + id = <0>; >> + status = "disabled"; >> + }; >> + >> + phy2: usbphy2 { Why do you need a different node name here? It should be a generic name to identify the function, so usbphy or usb-phy seems good enough. >> + compatible = "ti,am4372-usb2"; >> + #phy-cells = <0>; >> + id = <1>; >> + status = "disabled"; >> + }; >> + >> + usbctrl: omap-control-usb at 44e10620 { >> + compatible = "ti,omap-control-usb"; >> + reg = <0x44e10620 0x10>; >> + reg-names = "control_dev_conf"; >> + ti,type = <3>; >> + status = "disabled"; >> + }; >> + >> + usb1: am4372_dwc3 at 48380000 { >> + status = "disabled"; >> + compatible = "ti,am4372-dwc3"; >> + reg = <0x48380000 0x200>; >> + interrupts = <GIC_SPI 172 IRQ_TYPE_LEVEL_HIGH>; >> + #address-cells = <1>; >> + #size-cells = <1>; >> + utmi-mode = <1>; >> + ranges; A blank line here will be nice. >> + dwc3 at 48390000 { >> + compatible = "snps,dwc3"; >> + reg = <0x48390000 0xd000>; >> + interrupts = <GIC_SPI 168 IRQ_TYPE_LEVEL_HIGH>; >> + phys = <&phy1>; >> + phy-names = "am4372-usb1"; What is the purpose of the phy-names? Is it relevant to add the SoC name in something that usually, at least for the clocks and IRQs, is IP specific? The current documentation does not explain it and does not support phys property either. synopsys DWC3 CORE DWC3- USB3 CONTROLLER Required properties: - compatible: must be "synopsys,dwc3" - reg : Address and length of the register set for the device - interrupts: Interrupts used by the dwc3 controller. - usb-phy : array of phandle for the PHY device Is there some binding update ongoing for 3.12? >> + }; >> + }; >> + >> + usb2: am4372_dwc3 at 483c0000 { >> + status = "disabled"; >> + compatible = "ti,am4372-dwc3"; >> + reg = <0x483c0000 0x200>; >> + interrupts = <GIC_SPI 178 IRQ_TYPE_LEVEL_HIGH>; >> + #address-cells = <1>; >> + #size-cells = <1>; >> + utmi-mode = <1>; >> + ranges; A blank line here will be nice. >> + dwc3 at 483d0000 { >> + compatible = "snps,dwc3"; >> + reg = <0x483d0000 0xd000>; >> + interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>; >> + phys = <&phy2>; >> + phy-names = "am4372-usb2"; >> + }; >> + }; >> }; >> }; > > Regards, Benoit ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3] arm: dts: AM43x: Add usb DT nodes for AM4372 2013-08-14 14:34 ` Benoit Cousson (?) @ 2013-08-16 3:19 ` George Cherian -1 siblings, 0 replies; 16+ messages in thread From: George Cherian @ 2013-08-16 3:19 UTC (permalink / raw) To: Benoit Cousson Cc: linux-arm-kernel, linux-omap, linux, a-kishore, afzal, linux-kernel, balbi, Roger Quadros Hi Benoit , Thanks for your review. On 8/14/2013 8:04 PM, Benoit Cousson wrote: > + Roger > > Hi George, > > Yes, I had some comment about the "ti'type" in Roger's series that > will be applicable here as well. > > > On 14/08/2013 16:16, George Cherian wrote: >> +Benoit >> If you dont have any comments, can you take this for 3.12? >> >> Regards >> -George >> >> On 7/10/2013 1:44 PM, George Cherian wrote: >> >>> This patch adds >>> - HS USB nodes >>> - phy nodes >>> - usb control module nodes. >>> >>> Signed-off-by: George Cherian <george.cherian@ti.com> >>> --- >>> >>> changes from v2 >>> change synopsis to snps >>> use simple node names >>> add both USB and PHY instances >>> add usbctrl node >>> >>> changes from v1 >>> renamed synopsis to snps >>> removed flag tx-fifo-resize >>> >>> arch/arm/boot/dts/am4372.dtsi | 58 >>> +++++++++++++++++++++++++++++++++++++++++++ >>> 1 file changed, 58 insertions(+) >>> >>> diff --git a/arch/arm/boot/dts/am4372.dtsi >>> b/arch/arm/boot/dts/am4372.dtsi >>> index ddc1df7..37f196f 100644 >>> --- a/arch/arm/boot/dts/am4372.dtsi >>> +++ b/arch/arm/boot/dts/am4372.dtsi >>> @@ -64,5 +64,63 @@ >>> compatible = "ti,am4372-counter32k","ti,omap-counter32k"; >>> reg = <0x44e86000 0x40>; >>> }; >>> + >>> + phy1: usbphy1 { >>> + compatible = "ti,am4372-usb2"; > > That's not a very good name for a phy? It looks like a usb module. > > The bindings specify only ti,omap-usb2 or ti,omap-usb3 for USB2 or USB3. > I guess it should be one of them and potentially the binding should be > updated with ti,omap-usb2-phy and ti,omap-usb3-phy names while we can > still do it. > >>> + #phy-cells = <0>; >>> + id = <0>; >>> + status = "disabled"; >>> + }; >>> + >>> + phy2: usbphy2 { > > Why do you need a different node name here? It should be a generic > name to identify the function, so usbphy or usb-phy seems good enough. There are 2 instances of usb controllers and these instances for those 2 phys, which essentially dont have any reg property. So I named it as usbphy1 and usbphy2. > >>> + compatible = "ti,am4372-usb2"; >>> + #phy-cells = <0>; >>> + id = <1>; >>> + status = "disabled"; >>> + }; >>> + >>> + usbctrl: omap-control-usb@44e10620 { >>> + compatible = "ti,omap-control-usb"; >>> + reg = <0x44e10620 0x10>; >>> + reg-names = "control_dev_conf"; >>> + ti,type = <3>; >>> + status = "disabled"; >>> + }; >>> + >>> + usb1: am4372_dwc3@48380000 { >>> + status = "disabled"; >>> + compatible = "ti,am4372-dwc3"; >>> + reg = <0x48380000 0x200>; >>> + interrupts = <GIC_SPI 172 IRQ_TYPE_LEVEL_HIGH>; >>> + #address-cells = <1>; >>> + #size-cells = <1>; >>> + utmi-mode = <1>; >>> + ranges; > > A blank line here will be nice. okay > >>> + dwc3@48390000 { >>> + compatible = "snps,dwc3"; >>> + reg = <0x48390000 0xd000>; >>> + interrupts = <GIC_SPI 168 IRQ_TYPE_LEVEL_HIGH>; >>> + phys = <&phy1>; >>> + phy-names = "am4372-usb1"; > > What is the purpose of the phy-names? Is it relevant to add the SoC > name in something that usually, at least for the clocks and IRQs, is > IP specific? > > The current documentation does not explain it and does not support > phys property either. > > synopsys DWC3 CORE > > DWC3- USB3 CONTROLLER > > Required properties: > - compatible: must be "synopsys,dwc3" > - reg : Address and length of the register set for the device > - interrupts: Interrupts used by the dwc3 controller. > - usb-phy : array of phandle for the PHY device > > Is there some binding update ongoing for 3.12? The phy part, especially was added with the generic phy framework in mind. The generic phy framework uses phys and phy-names instead of usb-phy. Also all synopsys references for compatible are being replaced with snps. > >>> + }; >>> + }; >>> + >>> + usb2: am4372_dwc3@483c0000 { >>> + status = "disabled"; >>> + compatible = "ti,am4372-dwc3"; >>> + reg = <0x483c0000 0x200>; >>> + interrupts = <GIC_SPI 178 IRQ_TYPE_LEVEL_HIGH>; >>> + #address-cells = <1>; >>> + #size-cells = <1>; >>> + utmi-mode = <1>; >>> + ranges; > > A blank line here will be nice. > okay >>> + dwc3@483d0000 { >>> + compatible = "snps,dwc3"; >>> + reg = <0x483d0000 0xd000>; >>> + interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>; >>> + phys = <&phy2>; >>> + phy-names = "am4372-usb2"; >>> + }; >>> + }; >>> }; >>> }; >> >> > > Regards, > Benoit > -- -George ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v3] arm: dts: AM43x: Add usb DT nodes for AM4372 @ 2013-08-16 3:19 ` George Cherian 0 siblings, 0 replies; 16+ messages in thread From: George Cherian @ 2013-08-16 3:19 UTC (permalink / raw) To: linux-arm-kernel Hi Benoit , Thanks for your review. On 8/14/2013 8:04 PM, Benoit Cousson wrote: > + Roger > > Hi George, > > Yes, I had some comment about the "ti'type" in Roger's series that > will be applicable here as well. > > > On 14/08/2013 16:16, George Cherian wrote: >> +Benoit >> If you dont have any comments, can you take this for 3.12? >> >> Regards >> -George >> >> On 7/10/2013 1:44 PM, George Cherian wrote: >> >>> This patch adds >>> - HS USB nodes >>> - phy nodes >>> - usb control module nodes. >>> >>> Signed-off-by: George Cherian <george.cherian@ti.com> >>> --- >>> >>> changes from v2 >>> change synopsis to snps >>> use simple node names >>> add both USB and PHY instances >>> add usbctrl node >>> >>> changes from v1 >>> renamed synopsis to snps >>> removed flag tx-fifo-resize >>> >>> arch/arm/boot/dts/am4372.dtsi | 58 >>> +++++++++++++++++++++++++++++++++++++++++++ >>> 1 file changed, 58 insertions(+) >>> >>> diff --git a/arch/arm/boot/dts/am4372.dtsi >>> b/arch/arm/boot/dts/am4372.dtsi >>> index ddc1df7..37f196f 100644 >>> --- a/arch/arm/boot/dts/am4372.dtsi >>> +++ b/arch/arm/boot/dts/am4372.dtsi >>> @@ -64,5 +64,63 @@ >>> compatible = "ti,am4372-counter32k","ti,omap-counter32k"; >>> reg = <0x44e86000 0x40>; >>> }; >>> + >>> + phy1: usbphy1 { >>> + compatible = "ti,am4372-usb2"; > > That's not a very good name for a phy? It looks like a usb module. > > The bindings specify only ti,omap-usb2 or ti,omap-usb3 for USB2 or USB3. > I guess it should be one of them and potentially the binding should be > updated with ti,omap-usb2-phy and ti,omap-usb3-phy names while we can > still do it. > >>> + #phy-cells = <0>; >>> + id = <0>; >>> + status = "disabled"; >>> + }; >>> + >>> + phy2: usbphy2 { > > Why do you need a different node name here? It should be a generic > name to identify the function, so usbphy or usb-phy seems good enough. There are 2 instances of usb controllers and these instances for those 2 phys, which essentially dont have any reg property. So I named it as usbphy1 and usbphy2. > >>> + compatible = "ti,am4372-usb2"; >>> + #phy-cells = <0>; >>> + id = <1>; >>> + status = "disabled"; >>> + }; >>> + >>> + usbctrl: omap-control-usb at 44e10620 { >>> + compatible = "ti,omap-control-usb"; >>> + reg = <0x44e10620 0x10>; >>> + reg-names = "control_dev_conf"; >>> + ti,type = <3>; >>> + status = "disabled"; >>> + }; >>> + >>> + usb1: am4372_dwc3 at 48380000 { >>> + status = "disabled"; >>> + compatible = "ti,am4372-dwc3"; >>> + reg = <0x48380000 0x200>; >>> + interrupts = <GIC_SPI 172 IRQ_TYPE_LEVEL_HIGH>; >>> + #address-cells = <1>; >>> + #size-cells = <1>; >>> + utmi-mode = <1>; >>> + ranges; > > A blank line here will be nice. okay > >>> + dwc3 at 48390000 { >>> + compatible = "snps,dwc3"; >>> + reg = <0x48390000 0xd000>; >>> + interrupts = <GIC_SPI 168 IRQ_TYPE_LEVEL_HIGH>; >>> + phys = <&phy1>; >>> + phy-names = "am4372-usb1"; > > What is the purpose of the phy-names? Is it relevant to add the SoC > name in something that usually, at least for the clocks and IRQs, is > IP specific? > > The current documentation does not explain it and does not support > phys property either. > > synopsys DWC3 CORE > > DWC3- USB3 CONTROLLER > > Required properties: > - compatible: must be "synopsys,dwc3" > - reg : Address and length of the register set for the device > - interrupts: Interrupts used by the dwc3 controller. > - usb-phy : array of phandle for the PHY device > > Is there some binding update ongoing for 3.12? The phy part, especially was added with the generic phy framework in mind. The generic phy framework uses phys and phy-names instead of usb-phy. Also all synopsys references for compatible are being replaced with snps. > >>> + }; >>> + }; >>> + >>> + usb2: am4372_dwc3 at 483c0000 { >>> + status = "disabled"; >>> + compatible = "ti,am4372-dwc3"; >>> + reg = <0x483c0000 0x200>; >>> + interrupts = <GIC_SPI 178 IRQ_TYPE_LEVEL_HIGH>; >>> + #address-cells = <1>; >>> + #size-cells = <1>; >>> + utmi-mode = <1>; >>> + ranges; > > A blank line here will be nice. > okay >>> + dwc3 at 483d0000 { >>> + compatible = "snps,dwc3"; >>> + reg = <0x483d0000 0xd000>; >>> + interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>; >>> + phys = <&phy2>; >>> + phy-names = "am4372-usb2"; >>> + }; >>> + }; >>> }; >>> }; >> >> > > Regards, > Benoit > -- -George ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3] arm: dts: AM43x: Add usb DT nodes for AM4372 @ 2013-08-16 3:19 ` George Cherian 0 siblings, 0 replies; 16+ messages in thread From: George Cherian @ 2013-08-16 3:19 UTC (permalink / raw) To: Benoit Cousson Cc: afzal, linux, linux-kernel, balbi, a-kishore, linux-omap, linux-arm-kernel, Roger Quadros Hi Benoit , Thanks for your review. On 8/14/2013 8:04 PM, Benoit Cousson wrote: > + Roger > > Hi George, > > Yes, I had some comment about the "ti'type" in Roger's series that > will be applicable here as well. > > > On 14/08/2013 16:16, George Cherian wrote: >> +Benoit >> If you dont have any comments, can you take this for 3.12? >> >> Regards >> -George >> >> On 7/10/2013 1:44 PM, George Cherian wrote: >> >>> This patch adds >>> - HS USB nodes >>> - phy nodes >>> - usb control module nodes. >>> >>> Signed-off-by: George Cherian <george.cherian@ti.com> >>> --- >>> >>> changes from v2 >>> change synopsis to snps >>> use simple node names >>> add both USB and PHY instances >>> add usbctrl node >>> >>> changes from v1 >>> renamed synopsis to snps >>> removed flag tx-fifo-resize >>> >>> arch/arm/boot/dts/am4372.dtsi | 58 >>> +++++++++++++++++++++++++++++++++++++++++++ >>> 1 file changed, 58 insertions(+) >>> >>> diff --git a/arch/arm/boot/dts/am4372.dtsi >>> b/arch/arm/boot/dts/am4372.dtsi >>> index ddc1df7..37f196f 100644 >>> --- a/arch/arm/boot/dts/am4372.dtsi >>> +++ b/arch/arm/boot/dts/am4372.dtsi >>> @@ -64,5 +64,63 @@ >>> compatible = "ti,am4372-counter32k","ti,omap-counter32k"; >>> reg = <0x44e86000 0x40>; >>> }; >>> + >>> + phy1: usbphy1 { >>> + compatible = "ti,am4372-usb2"; > > That's not a very good name for a phy? It looks like a usb module. > > The bindings specify only ti,omap-usb2 or ti,omap-usb3 for USB2 or USB3. > I guess it should be one of them and potentially the binding should be > updated with ti,omap-usb2-phy and ti,omap-usb3-phy names while we can > still do it. > >>> + #phy-cells = <0>; >>> + id = <0>; >>> + status = "disabled"; >>> + }; >>> + >>> + phy2: usbphy2 { > > Why do you need a different node name here? It should be a generic > name to identify the function, so usbphy or usb-phy seems good enough. There are 2 instances of usb controllers and these instances for those 2 phys, which essentially dont have any reg property. So I named it as usbphy1 and usbphy2. > >>> + compatible = "ti,am4372-usb2"; >>> + #phy-cells = <0>; >>> + id = <1>; >>> + status = "disabled"; >>> + }; >>> + >>> + usbctrl: omap-control-usb@44e10620 { >>> + compatible = "ti,omap-control-usb"; >>> + reg = <0x44e10620 0x10>; >>> + reg-names = "control_dev_conf"; >>> + ti,type = <3>; >>> + status = "disabled"; >>> + }; >>> + >>> + usb1: am4372_dwc3@48380000 { >>> + status = "disabled"; >>> + compatible = "ti,am4372-dwc3"; >>> + reg = <0x48380000 0x200>; >>> + interrupts = <GIC_SPI 172 IRQ_TYPE_LEVEL_HIGH>; >>> + #address-cells = <1>; >>> + #size-cells = <1>; >>> + utmi-mode = <1>; >>> + ranges; > > A blank line here will be nice. okay > >>> + dwc3@48390000 { >>> + compatible = "snps,dwc3"; >>> + reg = <0x48390000 0xd000>; >>> + interrupts = <GIC_SPI 168 IRQ_TYPE_LEVEL_HIGH>; >>> + phys = <&phy1>; >>> + phy-names = "am4372-usb1"; > > What is the purpose of the phy-names? Is it relevant to add the SoC > name in something that usually, at least for the clocks and IRQs, is > IP specific? > > The current documentation does not explain it and does not support > phys property either. > > synopsys DWC3 CORE > > DWC3- USB3 CONTROLLER > > Required properties: > - compatible: must be "synopsys,dwc3" > - reg : Address and length of the register set for the device > - interrupts: Interrupts used by the dwc3 controller. > - usb-phy : array of phandle for the PHY device > > Is there some binding update ongoing for 3.12? The phy part, especially was added with the generic phy framework in mind. The generic phy framework uses phys and phy-names instead of usb-phy. Also all synopsys references for compatible are being replaced with snps. > >>> + }; >>> + }; >>> + >>> + usb2: am4372_dwc3@483c0000 { >>> + status = "disabled"; >>> + compatible = "ti,am4372-dwc3"; >>> + reg = <0x483c0000 0x200>; >>> + interrupts = <GIC_SPI 178 IRQ_TYPE_LEVEL_HIGH>; >>> + #address-cells = <1>; >>> + #size-cells = <1>; >>> + utmi-mode = <1>; >>> + ranges; > > A blank line here will be nice. > okay >>> + dwc3@483d0000 { >>> + compatible = "snps,dwc3"; >>> + reg = <0x483d0000 0xd000>; >>> + interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>; >>> + phys = <&phy2>; >>> + phy-names = "am4372-usb2"; >>> + }; >>> + }; >>> }; >>> }; >> >> > > Regards, > Benoit > -- -George ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3] arm: dts: AM43x: Add usb DT nodes for AM4372 2013-08-16 3:19 ` George Cherian @ 2013-08-16 9:08 ` Benoit Cousson -1 siblings, 0 replies; 16+ messages in thread From: Benoit Cousson @ 2013-08-16 9:08 UTC (permalink / raw) To: George Cherian Cc: linux-arm-kernel, linux-omap, linux, a-kishore, afzal, linux-kernel, balbi, Roger Quadros On 16/08/2013 05:19, George Cherian wrote: > Hi Benoit , > > Thanks for your review. > > On 8/14/2013 8:04 PM, Benoit Cousson wrote: >> + Roger >> >> Hi George, >> >> Yes, I had some comment about the "ti'type" in Roger's series that >> will be applicable here as well. >> > > >> >> On 14/08/2013 16:16, George Cherian wrote: >>> +Benoit >>> If you dont have any comments, can you take this for 3.12? >>> >>> Regards >>> -George >>> >>> On 7/10/2013 1:44 PM, George Cherian wrote: >>> >>>> This patch adds >>>> - HS USB nodes >>>> - phy nodes >>>> - usb control module nodes. >>>> >>>> Signed-off-by: George Cherian <george.cherian@ti.com> >>>> --- >>>> >>>> changes from v2 >>>> change synopsis to snps >>>> use simple node names >>>> add both USB and PHY instances >>>> add usbctrl node >>>> >>>> changes from v1 >>>> renamed synopsis to snps >>>> removed flag tx-fifo-resize >>>> >>>> arch/arm/boot/dts/am4372.dtsi | 58 >>>> +++++++++++++++++++++++++++++++++++++++++++ >>>> 1 file changed, 58 insertions(+) >>>> >>>> diff --git a/arch/arm/boot/dts/am4372.dtsi >>>> b/arch/arm/boot/dts/am4372.dtsi >>>> index ddc1df7..37f196f 100644 >>>> --- a/arch/arm/boot/dts/am4372.dtsi >>>> +++ b/arch/arm/boot/dts/am4372.dtsi >>>> @@ -64,5 +64,63 @@ >>>> compatible = "ti,am4372-counter32k","ti,omap-counter32k"; >>>> reg = <0x44e86000 0x40>; >>>> }; >>>> + >>>> + phy1: usbphy1 { >>>> + compatible = "ti,am4372-usb2"; >> >> That's not a very good name for a phy? It looks like a usb module. >> >> The bindings specify only ti,omap-usb2 or ti,omap-usb3 for USB2 or USB3. >> I guess it should be one of them and potentially the binding should be >> updated with ti,omap-usb2-phy and ti,omap-usb3-phy names while we can >> still do it. >> >>>> + #phy-cells = <0>; >>>> + id = <0>; >>>> + status = "disabled"; >>>> + }; >>>> + >>>> + phy2: usbphy2 { >> >> Why do you need a different node name here? It should be a generic >> name to identify the function, so usbphy or usb-phy seems good enough. > > There are 2 instances of usb controllers and these instances for those 2 > phys, which essentially dont have any reg property. So I named it as > usbphy1 and usbphy2. >> >>>> + compatible = "ti,am4372-usb2"; >>>> + #phy-cells = <0>; >>>> + id = <1>; >>>> + status = "disabled"; >>>> + }; >>>> + >>>> + usbctrl: omap-control-usb@44e10620 { >>>> + compatible = "ti,omap-control-usb"; >>>> + reg = <0x44e10620 0x10>; >>>> + reg-names = "control_dev_conf"; >>>> + ti,type = <3>; >>>> + status = "disabled"; >>>> + }; >>>> + >>>> + usb1: am4372_dwc3@48380000 { >>>> + status = "disabled"; >>>> + compatible = "ti,am4372-dwc3"; >>>> + reg = <0x48380000 0x200>; >>>> + interrupts = <GIC_SPI 172 IRQ_TYPE_LEVEL_HIGH>; >>>> + #address-cells = <1>; >>>> + #size-cells = <1>; >>>> + utmi-mode = <1>; >>>> + ranges; >> >> A blank line here will be nice. > > okay >> >>>> + dwc3@48390000 { >>>> + compatible = "snps,dwc3"; >>>> + reg = <0x48390000 0xd000>; >>>> + interrupts = <GIC_SPI 168 IRQ_TYPE_LEVEL_HIGH>; >>>> + phys = <&phy1>; >>>> + phy-names = "am4372-usb1"; >> >> What is the purpose of the phy-names? Is it relevant to add the SoC >> name in something that usually, at least for the clocks and IRQs, is >> IP specific? >> >> The current documentation does not explain it and does not support >> phys property either. >> >> synopsys DWC3 CORE >> >> DWC3- USB3 CONTROLLER >> >> Required properties: >> - compatible: must be "synopsys,dwc3" >> - reg : Address and length of the register set for the device >> - interrupts: Interrupts used by the dwc3 controller. >> - usb-phy : array of phandle for the PHY device >> >> Is there some binding update ongoing for 3.12? > > The phy part, especially was added with the generic phy framework in > mind. The generic phy framework uses phys and phy-names instead of usb-phy. > Also all synopsys references for compatible are being replaced with snps. That's fine, but that was not my question. Is there any already bindings that defined phys and phy-names in the kernel? I couldn't find them in 3.11-rc5. And anyway the current bindings for dwc3 is not aligned with what you are doing here. So you have to change the binding before using it in this DTS. Regards, Benoit ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v3] arm: dts: AM43x: Add usb DT nodes for AM4372 @ 2013-08-16 9:08 ` Benoit Cousson 0 siblings, 0 replies; 16+ messages in thread From: Benoit Cousson @ 2013-08-16 9:08 UTC (permalink / raw) To: linux-arm-kernel On 16/08/2013 05:19, George Cherian wrote: > Hi Benoit , > > Thanks for your review. > > On 8/14/2013 8:04 PM, Benoit Cousson wrote: >> + Roger >> >> Hi George, >> >> Yes, I had some comment about the "ti'type" in Roger's series that >> will be applicable here as well. >> > > >> >> On 14/08/2013 16:16, George Cherian wrote: >>> +Benoit >>> If you dont have any comments, can you take this for 3.12? >>> >>> Regards >>> -George >>> >>> On 7/10/2013 1:44 PM, George Cherian wrote: >>> >>>> This patch adds >>>> - HS USB nodes >>>> - phy nodes >>>> - usb control module nodes. >>>> >>>> Signed-off-by: George Cherian <george.cherian@ti.com> >>>> --- >>>> >>>> changes from v2 >>>> change synopsis to snps >>>> use simple node names >>>> add both USB and PHY instances >>>> add usbctrl node >>>> >>>> changes from v1 >>>> renamed synopsis to snps >>>> removed flag tx-fifo-resize >>>> >>>> arch/arm/boot/dts/am4372.dtsi | 58 >>>> +++++++++++++++++++++++++++++++++++++++++++ >>>> 1 file changed, 58 insertions(+) >>>> >>>> diff --git a/arch/arm/boot/dts/am4372.dtsi >>>> b/arch/arm/boot/dts/am4372.dtsi >>>> index ddc1df7..37f196f 100644 >>>> --- a/arch/arm/boot/dts/am4372.dtsi >>>> +++ b/arch/arm/boot/dts/am4372.dtsi >>>> @@ -64,5 +64,63 @@ >>>> compatible = "ti,am4372-counter32k","ti,omap-counter32k"; >>>> reg = <0x44e86000 0x40>; >>>> }; >>>> + >>>> + phy1: usbphy1 { >>>> + compatible = "ti,am4372-usb2"; >> >> That's not a very good name for a phy? It looks like a usb module. >> >> The bindings specify only ti,omap-usb2 or ti,omap-usb3 for USB2 or USB3. >> I guess it should be one of them and potentially the binding should be >> updated with ti,omap-usb2-phy and ti,omap-usb3-phy names while we can >> still do it. >> >>>> + #phy-cells = <0>; >>>> + id = <0>; >>>> + status = "disabled"; >>>> + }; >>>> + >>>> + phy2: usbphy2 { >> >> Why do you need a different node name here? It should be a generic >> name to identify the function, so usbphy or usb-phy seems good enough. > > There are 2 instances of usb controllers and these instances for those 2 > phys, which essentially dont have any reg property. So I named it as > usbphy1 and usbphy2. >> >>>> + compatible = "ti,am4372-usb2"; >>>> + #phy-cells = <0>; >>>> + id = <1>; >>>> + status = "disabled"; >>>> + }; >>>> + >>>> + usbctrl: omap-control-usb at 44e10620 { >>>> + compatible = "ti,omap-control-usb"; >>>> + reg = <0x44e10620 0x10>; >>>> + reg-names = "control_dev_conf"; >>>> + ti,type = <3>; >>>> + status = "disabled"; >>>> + }; >>>> + >>>> + usb1: am4372_dwc3 at 48380000 { >>>> + status = "disabled"; >>>> + compatible = "ti,am4372-dwc3"; >>>> + reg = <0x48380000 0x200>; >>>> + interrupts = <GIC_SPI 172 IRQ_TYPE_LEVEL_HIGH>; >>>> + #address-cells = <1>; >>>> + #size-cells = <1>; >>>> + utmi-mode = <1>; >>>> + ranges; >> >> A blank line here will be nice. > > okay >> >>>> + dwc3 at 48390000 { >>>> + compatible = "snps,dwc3"; >>>> + reg = <0x48390000 0xd000>; >>>> + interrupts = <GIC_SPI 168 IRQ_TYPE_LEVEL_HIGH>; >>>> + phys = <&phy1>; >>>> + phy-names = "am4372-usb1"; >> >> What is the purpose of the phy-names? Is it relevant to add the SoC >> name in something that usually, at least for the clocks and IRQs, is >> IP specific? >> >> The current documentation does not explain it and does not support >> phys property either. >> >> synopsys DWC3 CORE >> >> DWC3- USB3 CONTROLLER >> >> Required properties: >> - compatible: must be "synopsys,dwc3" >> - reg : Address and length of the register set for the device >> - interrupts: Interrupts used by the dwc3 controller. >> - usb-phy : array of phandle for the PHY device >> >> Is there some binding update ongoing for 3.12? > > The phy part, especially was added with the generic phy framework in > mind. The generic phy framework uses phys and phy-names instead of usb-phy. > Also all synopsys references for compatible are being replaced with snps. That's fine, but that was not my question. Is there any already bindings that defined phys and phy-names in the kernel? I couldn't find them in 3.11-rc5. And anyway the current bindings for dwc3 is not aligned with what you are doing here. So you have to change the binding before using it in this DTS. Regards, Benoit ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3] arm: dts: AM43x: Add usb DT nodes for AM4372 2013-08-16 9:08 ` Benoit Cousson (?) @ 2013-08-16 10:21 ` George Cherian -1 siblings, 0 replies; 16+ messages in thread From: George Cherian @ 2013-08-16 10:21 UTC (permalink / raw) To: Benoit Cousson Cc: linux-arm-kernel, linux-omap, linux, a-kishore, afzal, linux-kernel, balbi, Roger Quadros On 8/16/2013 2:38 PM, Benoit Cousson wrote: > That's fine, but that was not my question. Is there any already bindings > that defined phys and phy-names in the kernel? No > > I couldn't find them in 3.11-rc5. I will drop the phy reference and send an updated patch adding dwc nodes. > And anyway the current bindings for dwc3 is not aligned with what you > are doing here. > So you have to change the binding before using it in this DTS. > > Regards, > Benoit -- -George ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v3] arm: dts: AM43x: Add usb DT nodes for AM4372 @ 2013-08-16 10:21 ` George Cherian 0 siblings, 0 replies; 16+ messages in thread From: George Cherian @ 2013-08-16 10:21 UTC (permalink / raw) To: linux-arm-kernel On 8/16/2013 2:38 PM, Benoit Cousson wrote: > That's fine, but that was not my question. Is there any already bindings > that defined phys and phy-names in the kernel? No > > I couldn't find them in 3.11-rc5. I will drop the phy reference and send an updated patch adding dwc nodes. > And anyway the current bindings for dwc3 is not aligned with what you > are doing here. > So you have to change the binding before using it in this DTS. > > Regards, > Benoit -- -George ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3] arm: dts: AM43x: Add usb DT nodes for AM4372 @ 2013-08-16 10:21 ` George Cherian 0 siblings, 0 replies; 16+ messages in thread From: George Cherian @ 2013-08-16 10:21 UTC (permalink / raw) To: Benoit Cousson Cc: afzal, linux, linux-kernel, balbi, a-kishore, linux-omap, linux-arm-kernel, Roger Quadros On 8/16/2013 2:38 PM, Benoit Cousson wrote: > That's fine, but that was not my question. Is there any already bindings > that defined phys and phy-names in the kernel? No > > I couldn't find them in 3.11-rc5. I will drop the phy reference and send an updated patch adding dwc nodes. > And anyway the current bindings for dwc3 is not aligned with what you > are doing here. > So you have to change the binding before using it in this DTS. > > Regards, > Benoit -- -George ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2013-08-16 10:22 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2013-07-10 8:14 [PATCH v3] arm: dts: AM43x: Add usb DT nodes for AM4372 George Cherian 2013-07-10 8:14 ` George Cherian 2013-07-10 8:14 ` George Cherian 2013-08-14 14:16 ` George Cherian 2013-08-14 14:16 ` George Cherian 2013-08-14 14:16 ` George Cherian 2013-08-14 14:34 ` Benoit Cousson 2013-08-14 14:34 ` Benoit Cousson 2013-08-16 3:19 ` George Cherian 2013-08-16 3:19 ` George Cherian 2013-08-16 3:19 ` George Cherian 2013-08-16 9:08 ` Benoit Cousson 2013-08-16 9:08 ` Benoit Cousson 2013-08-16 10:21 ` George Cherian 2013-08-16 10:21 ` George Cherian 2013-08-16 10:21 ` George Cherian
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.