| Message ID | 1373361446-30050-1-git-send-email-george.cherian@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Jul 09, 2013 at 02:47:26PM +0530, George Cherian wrote: > Adds device node for HS USB Host module for AM437x > > changes from v1 > > renamed synopsis to snps > removed flag tx-fifo-resize the patch revision changes don't need to go to the commit log, they should be placed after the tearline (---) and before the diffstat. > Signed-off-by: George Cherian <george.cherian@ti.com> > --- > arch/arm/boot/dts/am4372.dtsi | 18 ++++++++++++++++++ > 1 file changed, 18 insertions(+) > > diff --git a/arch/arm/boot/dts/am4372.dtsi b/arch/arm/boot/dts/am4372.dtsi > index ddc1df7..c9e0da8 100644 > --- a/arch/arm/boot/dts/am4372.dtsi > +++ b/arch/arm/boot/dts/am4372.dtsi > @@ -64,5 +64,23 @@ > compatible = "ti,am4372-counter32k","ti,omap-counter32k"; > reg = <0x44e86000 0x40>; > }; > + > + usb_otg_hs1: am4372_dwc3@48380000 { dtsi should always have status = "disabled"; no ? > + compatible = "ti,am437x-dwc3"; > + reg = <0x48380000 0x1ff>; weird size, shouldn't this be 0x200 ? > + interrupts = <GIC_SPI 172 IRQ_TYPE_LEVEL_HIGH>; > + #address-cells = <1>; > + #size-cells = <1>; > + utmi-mode = <1>; > + ranges; > + > + dwc3@48390000 { dtsi should always have status = "disabled"; no ? > + compatible = "snps,dwc3"; > + reg = <0x48390000 0xcfff>; weird size, shouldn't this be 0xd000 then the size would be exactly 52KiB > + interrupts = <GIC_SPI 168 IRQ_TYPE_LEVEL_HIGH>; > + }; > + > + }; > + there two trailing tabs on this line. Another thing: am437x has 4 instances of this IP, why are you adding only one ? And why aren't you pasing the PHY nodes here ? The device won't work without its PHYs.
Hi George, On Tue, Jul 09, 2013 at 14:47:26, Cherian, George wrote: > + usb_otg_hs1: am4372_dwc3@48380000 { Wouldn't "usb" a better node name ? > + compatible = "ti,am437x-dwc3"; Usage of wild card is discouraged per DT documentation. Regards Afzal
On 7/9/2013 3:50 PM, Felipe Balbi wrote: > On Tue, Jul 09, 2013 at 02:47:26PM +0530, George Cherian wrote: >> Adds device node for HS USB Host module for AM437x >> >> changes from v1 >> >> renamed synopsis to snps >> removed flag tx-fifo-resize > the patch revision changes don't need to go to the commit log, > they should be placed after the tearline (---) and before the diffstat. > >> Signed-off-by: George Cherian <george.cherian@ti.com> >> --- >> arch/arm/boot/dts/am4372.dtsi | 18 ++++++++++++++++++ >> 1 file changed, 18 insertions(+) >> >> diff --git a/arch/arm/boot/dts/am4372.dtsi b/arch/arm/boot/dts/am4372.dtsi >> index ddc1df7..c9e0da8 100644 >> --- a/arch/arm/boot/dts/am4372.dtsi >> +++ b/arch/arm/boot/dts/am4372.dtsi >> @@ -64,5 +64,23 @@ >> compatible = "ti,am4372-counter32k","ti,omap-counter32k"; >> reg = <0x44e86000 0x40>; >> }; >> + >> + usb_otg_hs1: am4372_dwc3@48380000 { > dtsi should always have status = "disabled"; no ? > >> + compatible = "ti,am437x-dwc3"; >> + reg = <0x48380000 0x1ff>; > weird size, shouldn't this be 0x200 ? okay >> + interrupts = <GIC_SPI 172 IRQ_TYPE_LEVEL_HIGH>; >> + #address-cells = <1>; >> + #size-cells = <1>; >> + utmi-mode = <1>; >> + ranges; >> + >> + dwc3@48390000 { > dtsi should always have status = "disabled"; no ? okay > >> + compatible = "snps,dwc3"; >> + reg = <0x48390000 0xcfff>; > weird size, shouldn't this be 0xd000 then the size would be exactly > 52KiB okay >> + interrupts = <GIC_SPI 168 IRQ_TYPE_LEVEL_HIGH>; >> + }; >> + >> + }; >> + > there two trailing tabs on this line. > > Another thing: am437x has 4 instances of this IP, why are you adding > only one ? AM437x has got only 2 instances. I have verified only one on HAPS so adding only one. > And why aren't you pasing the PHY nodes here ? The device > won't work without its PHYs. Yes true, again in HAPS I didnt have any PHY configuration to be done. >
Hi, On Tue, Jul 09, 2013 at 04:51:35PM +0530, George Cherian wrote: > >>+ compatible = "snps,dwc3"; > >>+ reg = <0x48390000 0xcfff>; > >weird size, shouldn't this be 0xd000 then the size would be exactly > >52KiB > > okay btw, the reason here is that when you call devm_ioremap_resource(), that will call resource_size() which does: size = res->end - res->start - 1; so you need this extra 1 on the size when passing it via DT. > >Another thing: am437x has 4 instances of this IP, why are you adding > >only one ? > > AM437x has got only 2 instances. I have verified only one on HAPS so > adding only one. weird, on my TRM I see for dwc3 but 2 PHYs. > >And why aren't you pasing the PHY nodes here ? The device > >won't work without its PHYs. > > Yes true, again in HAPS I didnt have any PHY configuration to be done. alright, but we should still pass the PHY right ? once silicon comes, we want this to work without any further changers.
On 7/9/2013 4:57 PM, Felipe Balbi wrote: > Hi, > > On Tue, Jul 09, 2013 at 04:51:35PM +0530, George Cherian wrote: >>>> + compatible = "snps,dwc3"; >>>> + reg = <0x48390000 0xcfff>; >>> weird size, shouldn't this be 0xd000 then the size would be exactly >>> 52KiB >> okay > btw, the reason here is that when you call devm_ioremap_resource(), that > will call resource_size() which does: > > size = res->end - res->start - 1; > > so you need this extra 1 on the size when passing it via DT. agreed. >>> Another thing: am437x has 4 instances of this IP, why are you adding >>> only one ? >> AM437x has got only 2 instances. I have verified only one on HAPS so >> adding only one. > weird, on my TRM I see for dwc3 but 2 PHYs. Please confirm whether you are looking at am437x TRM or dra7x TRM? dra7x has 4 dwc3 and 2 internal phys and 2 external phys >>> And why aren't you pasing the PHY nodes here ? The device >>> won't work without its PHYs. >> Yes true, again in HAPS I didnt have any PHY configuration to be done. > alright, but we should still pass the PHY right ? once silicon comes, > we want this to work without any further changers. okay will add phy nodes and 2 instances of dwc3.
Hi, On Tue, Jul 09, 2013 at 05:22:43PM +0530, George Cherian wrote: > >>>Another thing: am437x has 4 instances of this IP, why are you adding > >>>only one ? > >>AM437x has got only 2 instances. I have verified only one on HAPS so > >>adding only one. > >weird, on my TRM I see for dwc3 but 2 PHYs. > > Please confirm whether you are looking at am437x TRM or dra7x TRM? > dra7x has 4 dwc3 and 2 internal phys and 2 external phys guilty as charged :-) > >>>And why aren't you pasing the PHY nodes here ? The device > >>>won't work without its PHYs. > >>Yes true, again in HAPS I didnt have any PHY configuration to be done. > >alright, but we should still pass the PHY right ? once silicon comes, > >we want this to work without any further changers. > > okay will add phy nodes and 2 instances of dwc3. thanks
Hello. On 09-07-2013 13:17, George Cherian wrote: > Adds device node for HS USB Host module for AM437x > changes from v1 > renamed synopsis to snps > removed flag tx-fifo-resize > Signed-off-by: George Cherian <george.cherian@ti.com> > --- > arch/arm/boot/dts/am4372.dtsi | 18 ++++++++++++++++++ > 1 file changed, 18 insertions(+) > diff --git a/arch/arm/boot/dts/am4372.dtsi b/arch/arm/boot/dts/am4372.dtsi > index ddc1df7..c9e0da8 100644 > --- a/arch/arm/boot/dts/am4372.dtsi > +++ b/arch/arm/boot/dts/am4372.dtsi > @@ -64,5 +64,23 @@ > compatible = "ti,am4372-counter32k","ti,omap-counter32k"; > reg = <0x44e86000 0x40>; > }; > + > + usb_otg_hs1: am4372_dwc3@48380000 { See http://www.devicetree.org/Device_Tree_Usage, "Node Names" section. > + compatible = "ti,am437x-dwc3"; > + reg = <0x48380000 0x1ff>; > + interrupts = <GIC_SPI 172 IRQ_TYPE_LEVEL_HIGH>; > + #address-cells = <1>; > + #size-cells = <1>; > + utmi-mode = <1>; > + ranges; > + > + dwc3@48390000 { The same comment. > + compatible = "snps,dwc3"; > + reg = <0x48390000 0xcfff>; > + interrupts = <GIC_SPI 168 IRQ_TYPE_LEVEL_HIGH>; > + }; > + > + }; > + WBR, Sergei
Hello. On 07/09/2013 06:07 PM, Sergei Shtylyov wrote: >> Adds device node for HS USB Host module for AM437x >> changes from v1 >> renamed synopsis to snps >> removed flag tx-fifo-resize >> Signed-off-by: George Cherian <george.cherian@ti.com> >> --- >> arch/arm/boot/dts/am4372.dtsi | 18 ++++++++++++++++++ >> 1 file changed, 18 insertions(+) >> diff --git a/arch/arm/boot/dts/am4372.dtsi >> b/arch/arm/boot/dts/am4372.dtsi >> index ddc1df7..c9e0da8 100644 >> --- a/arch/arm/boot/dts/am4372.dtsi >> +++ b/arch/arm/boot/dts/am4372.dtsi >> @@ -64,5 +64,23 @@ >> compatible = "ti,am4372-counter32k","ti,omap-counter32k"; >> reg = <0x44e86000 0x40>; >> }; >> + >> + usb_otg_hs1: am4372_dwc3@48380000 { > > See http://www.devicetree.org/Device_Tree_Usage, "Node Names" section. > >> + compatible = "ti,am437x-dwc3"; >> + reg = <0x48380000 0x1ff>; And I bet this should be 0x200, not 0x1ff. This is length, not upper limit. >> + interrupts = <GIC_SPI 172 IRQ_TYPE_LEVEL_HIGH>; >> + #address-cells = <1>; >> + #size-cells = <1>; >> + utmi-mode = <1>; >> + ranges; >> + >> + dwc3@48390000 { > The same comment. >> + compatible = "snps,dwc3"; >> + reg = <0x48390000 0xcfff>; The same, this should be 0xd000, not 0xcfff. >> + interrupts = <GIC_SPI 168 IRQ_TYPE_LEVEL_HIGH>; >> + }; >> + >> + }; >> + WBR, Sergei
diff --git a/arch/arm/boot/dts/am4372.dtsi b/arch/arm/boot/dts/am4372.dtsi index ddc1df7..c9e0da8 100644 --- a/arch/arm/boot/dts/am4372.dtsi +++ b/arch/arm/boot/dts/am4372.dtsi @@ -64,5 +64,23 @@ compatible = "ti,am4372-counter32k","ti,omap-counter32k"; reg = <0x44e86000 0x40>; }; + + usb_otg_hs1: am4372_dwc3@48380000 { + compatible = "ti,am437x-dwc3"; + reg = <0x48380000 0x1ff>; + 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 0xcfff>; + interrupts = <GIC_SPI 168 IRQ_TYPE_LEVEL_HIGH>; + }; + + }; + }; };
Adds device node for HS USB Host module for AM437x changes from v1 renamed synopsis to snps removed flag tx-fifo-resize Signed-off-by: George Cherian <george.cherian@ti.com> --- arch/arm/boot/dts/am4372.dtsi | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)