[PATCH v2] arm: dts: AM43x: Add usb_otg_hs node

[PATCH v2] arm: dts: AM43x: Add usb_otg_hs node

[George Cherian]

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 at 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 at 48390000 {
+				compatible = "snps,dwc3";
+				reg = <0x48390000 0xcfff>;
+				interrupts = <GIC_SPI  168 IRQ_TYPE_LEVEL_HIGH>;
+			};
+
+		};
+		
 	};
 };
-- 
1.8.1.4

[PATCH v2] arm: dts: AM43x: Add usb_otg_hs node

[Felipe Balbi]

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 at 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 at 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.

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130709/29ec9e9f/attachment.sig>

[PATCH v2] arm: dts: AM43x: Add usb_otg_hs node

[George Cherian]

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 at 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 at 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.
>


-- 
-George

[PATCH v2] arm: dts: AM43x: Add usb_otg_hs node

[Felipe Balbi]

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.

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130709/0b64e433/attachment.sig>

[PATCH v2] arm: dts: AM43x: Add usb_otg_hs node

[George Cherian]

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.



-- 
-George

[PATCH v2] arm: dts: AM43x: Add usb_otg_hs node

[Felipe Balbi]

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

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130709/1efa2643/attachment.sig>

[PATCH v2] arm: dts: AM43x: Add usb_otg_hs node

[Mohammed, Afzal]

Hi George,

On Tue, Jul 09, 2013 at 14:47:26, Cherian, George wrote:

> +		usb_otg_hs1: am4372_dwc3 at 48380000 {

Wouldn't "usb" a better node name ?

> +			compatible = "ti,am437x-dwc3";

Usage of wild card is discouraged per DT documentation.

Regards
Afzal

[PATCH v2] arm: dts: AM43x: Add usb_otg_hs node

[Sergei Shtylyov]

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 at 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 at 48390000 {

    The same comment.

> +				compatible = "snps,dwc3";
> +				reg = <0x48390000 0xcfff>;
> +				interrupts = <GIC_SPI  168 IRQ_TYPE_LEVEL_HIGH>;
> +			};
> +
> +		};
> +		

WBR, Sergei

[PATCH v2] arm: dts: AM43x: Add usb_otg_hs node

[Sergei Shtylyov]

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 at 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 at 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