Patchwork [v2,2/7] ARM: tegra: update device trees for USB binding rework

login
register
mail settings
Submitter Venu Byravarasu
Date April 3, 2013, 8:41 a.m.
Message ID <1364978502-22887-3-git-send-email-vbyravarasu@nvidia.com>
Download mbox | patch
Permalink /patch/233313/
State Superseded, archived
Headers show

Comments

Venu Byravarasu - April 3, 2013, 8:41 a.m.
This patch updates all Tegra board files so that they contain all the
properties required by the updated USB DT binding. Note that this patch
only adds the new properties and does not yet remove the old properties,
in order to maintain bisectability. The old properties will be removed
once the driver has been updated to assume the new bindings.

Signed-off-by: Venu Byravarasu <vbyravarasu@nvidia.com>
---
delta from v1:
1. Fixed voltage regulators were used for vbus-supply
2. Added UTMI PHY timing Parameters to DT.

 arch/arm/boot/dts/tegra20-colibri-512.dtsi |    4 ++
 arch/arm/boot/dts/tegra20-harmony.dts      |    8 ++--
 arch/arm/boot/dts/tegra20-iris-512.dts     |    4 ++
 arch/arm/boot/dts/tegra20-paz00.dts        |    8 ++--
 arch/arm/boot/dts/tegra20-seaboard.dts     |   22 +++++++++++--
 arch/arm/boot/dts/tegra20-trimslice.dts    |   21 ++++++++++--
 arch/arm/boot/dts/tegra20-ventana.dts      |    7 ++--
 arch/arm/boot/dts/tegra20.dtsi             |   46 ++++++++++++++++++++-------
 8 files changed, 89 insertions(+), 31 deletions(-)
Stephen Warren - April 3, 2013, 7:16 p.m.
On 04/03/2013 02:41 AM, Venu Byravarasu wrote:
> This patch updates all Tegra board files so that they contain all the
> properties required by the updated USB DT binding. Note that this patch
> only adds the new properties and does not yet remove the old properties,
> in order to maintain bisectability. The old properties will be removed
> once the driver has been updated to assume the new bindings.

The binding documentation says that the vbus-supply property is required
in all cases. However, many of the DT files still don't have that
property even after this patch. That's inconsistent.

> diff --git a/arch/arm/boot/dts/tegra20-iris-512.dts b/arch/arm/boot/dts/tegra20-iris-512.dts
> index 52f1103..c99eccc 100644
> --- a/arch/arm/boot/dts/tegra20-iris-512.dts
> +++ b/arch/arm/boot/dts/tegra20-iris-512.dts
> @@ -41,6 +41,10 @@
>  		dr_mode = "otg";
>  	};
>  
> +	usb-phy@c5000000 {
> +		dr_mode = "otg";
> +	};

Since this port claims to be OTG-capable, presumably a vbus-supply
property is mandatory here? If you don't know enough about the board to
correctly set up such a regulator, lets just mark this port as host-only
for now; we can switch it back to OTG-mode later once someone implements
the required regulator. This won't lose any functionality, since we
don't support OTG-mode yet anyway. However, it will allow the device
tree to be correct/consistent in the mean time.

> diff --git a/arch/arm/boot/dts/tegra20-seaboard.dts b/arch/arm/boot/dts/tegra20-seaboard.dts

> +	usb-phy@c5000000 {
> +		vbus-supply = <&vbus_reg>;
> +		dr_mode = "otg";
> +	};

Where is the nvidia,vbus-gpio property? Since your code changes don't
actually implement use of the vbus-supply property yet, they will expect
the vbus-gpio property to exist in the PHY node.

> diff --git a/arch/arm/boot/dts/tegra20-trimslice.dts b/arch/arm/boot/dts/tegra20-trimslice.dts
> index 955bf49..8f2bb9b 100644
> --- a/arch/arm/boot/dts/tegra20-trimslice.dts
> +++ b/arch/arm/boot/dts/tegra20-trimslice.dts
> @@ -305,17 +305,21 @@
>  		nvidia,vbus-gpio = <&gpio 170 0>; /* gpio PV2 */
>  	};
>  
> +	usb-phy@c5000000 {
> +		vbus-supply = <&vbus_reg>;
> +	};

Same here; isn't nvidia,vbus-gpio needed in the PHY node until the
driver is converted to use vbus-supply?

I guess this is a case of a host-only port where there is SW-control
over VBUS...
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stephen Warren - April 3, 2013, 7:33 p.m.
On 04/03/2013 02:41 AM, Venu Byravarasu wrote:
> This patch updates all Tegra board files so that they contain all the
> properties required by the updated USB DT binding. Note that this patch
> only adds the new properties and does not yet remove the old properties,
> in order to maintain bisectability. The old properties will be removed
> once the driver has been updated to assume the new bindings.

> diff --git a/arch/arm/boot/dts/tegra20-colibri-512.dtsi b/arch/arm/boot/dts/tegra20-colibri-512.dtsi
> index cb73e62..af5a7ae 100644
> --- a/arch/arm/boot/dts/tegra20-colibri-512.dtsi
> +++ b/arch/arm/boot/dts/tegra20-colibri-512.dtsi
> @@ -443,6 +443,10 @@
>  		nvidia,phy-reset-gpio = <&gpio 169 0>; /* gpio PV1 */

Another problem here: The code pulses this GPIO low to reset the PHY, so
in other words, the GPIO is active low.

Can you please prepare a separate patch to fix this; you need to change
all existing instances from:

nvidia,phy-reset-gpio = <&gpio 169 0>; /* gpio PV1 */

to:

nvidia,phy-reset-gpio = <&gpio 169 1>; /* gpio PV1, active low */

Insert that new patch before this patch in the series, and then fix this
patch so that any new copies of that property have the correct content
from the start.

Note: I pointed out this problem in my review of V1, but V2 didn't
include a fix for it:-(
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Venu Byravarasu - April 4, 2013, 1:01 p.m.
> -----Original Message-----
> From: Stephen Warren [mailto:swarren@wwwdotorg.org]
> Sent: Thursday, April 04, 2013 12:47 AM
> To: Venu Byravarasu
> Cc: gregkh@linuxfoundation.org; balbi@ti.com;
> stern@rowland.harvard.edu; linux-tegra@vger.kernel.org; linux-
> usb@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v2 2/7] ARM: tegra: update device trees for USB binding
> rework
> 
> On 04/03/2013 02:41 AM, Venu Byravarasu wrote:
> > This patch updates all Tegra board files so that they contain all the
> 
> The binding documentation says that the vbus-supply property is required
> in all cases. However, many of the DT files still don't have that
> property even after this patch. That's inconsistent.

Will edit the binding to have the vbus-supply only for otg cases. 

> 
> > diff --git a/arch/arm/boot/dts/tegra20-iris-512.dts
> b/arch/arm/boot/dts/tegra20-iris-512.dts
> > index 52f1103..c99eccc 100644
> > --- a/arch/arm/boot/dts/tegra20-iris-512.dts
> > +++ b/arch/arm/boot/dts/tegra20-iris-512.dts
> > @@ -41,6 +41,10 @@
> >  		dr_mode = "otg";
> >  	};
> >
> > +	usb-phy@c5000000 {
> > +		dr_mode = "otg";
> > +	};
> 
> Since this port claims to be OTG-capable, presumably a vbus-supply
> property is mandatory here? If you don't know enough about the board to
> correctly set up such a regulator, lets just mark this port as host-only
> for now; we can switch it back to OTG-mode later once someone implements
> the required regulator. This won't lose any functionality, since we
> don't support OTG-mode yet anyway. However, it will allow the device
> tree to be correct/consistent in the mean time.
 
Agree completely.
Will update the patch accordingly and send for review.

> 
> > diff --git a/arch/arm/boot/dts/tegra20-seaboard.dts
> b/arch/arm/boot/dts/tegra20-seaboard.dts
> 
> > +	usb-phy@c5000000 {
> > +		vbus-supply = <&vbus_reg>;
> > +		dr_mode = "otg";
> > +	};
> 
> Where is the nvidia,vbus-gpio property? Since your code changes don't
> actually implement use of the vbus-supply property yet, they will expect
> the vbus-gpio property to exist in the PHY node.

As of now Vbus-gpio is being used by EHCI driver only.
As anyways we wanted to use vbus-supply, will implement its
support in PHY driver and remove vbus-gpio from EHCI node. 
Hence did not add vbus-gpio to PHY DT nodes.

--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stephen Warren - April 4, 2013, 6:01 p.m.
On 04/04/2013 07:01 AM, Venu Byravarasu wrote:
>> -----Original Message-----
>> From: Stephen Warren [mailto:swarren@wwwdotorg.org]
>> Sent: Thursday, April 04, 2013 12:47 AM
>> To: Venu Byravarasu
>> Cc: gregkh@linuxfoundation.org; balbi@ti.com;
>> stern@rowland.harvard.edu; linux-tegra@vger.kernel.org; linux-
>> usb@vger.kernel.org; linux-kernel@vger.kernel.org
>> Subject: Re: [PATCH v2 2/7] ARM: tegra: update device trees for USB binding
>> rework
>>
>> On 04/03/2013 02:41 AM, Venu Byravarasu wrote:
>>> This patch updates all Tegra board files so that they contain all the
>>
>> The binding documentation says that the vbus-supply property is required
>> in all cases. However, many of the DT files still don't have that
>> property even after this patch. That's inconsistent.
> 
> Will edit the binding to have the vbus-supply only for otg cases. 

>>> diff --git a/arch/arm/boot/dts/tegra20-seaboard.dts b/arch/arm/boot/dts/tegra20-seaboard.dts
>>
>>> +	usb-phy@c5000000 {
>>> +		vbus-supply = <&vbus_reg>;
>>> +		dr_mode = "otg";
>>> +	};
>>
>> Where is the nvidia,vbus-gpio property? Since your code changes don't
>> actually implement use of the vbus-supply property yet, they will expect
>> the vbus-gpio property to exist in the PHY node.
> 
> As of now Vbus-gpio is being used by EHCI driver only.
> As anyways we wanted to use vbus-supply, will implement its
> support in PHY driver and remove vbus-gpio from EHCI node. 
> Hence did not add vbus-gpio to PHY DT nodes.

Ah right. I thought that this patch series moved the use of vbus-gpio
from the EHCI to the PHY driver, just like it moved the handling of
other properties such as dr_mode. I guess that's not actually true.

Just so we're clear, I'd expect the following then:

* Update binding to remove references to vbus-gpio from EHCI binding,
add references to vbus-supply to PHY binding.

* Update DT to add vbus-supply to PHY nodes, but don't remove vbus-gpio
from EHCI nodes.

* The code in this series doesn't actually change the vbus-* usage yet.

...

* Once this series is finalized, create another follow-on patch that
adds vbus-supply handling in to the PHY driver, and removes vbus-gpio
handling from the EHCI driver.

* Finally, remove vbus-gpio from the DT files.

I think we're in agreement on this now, and this series mostly already
does exactly that. I just wanted to spell it out explicitly to make sure.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/arch/arm/boot/dts/tegra20-colibri-512.dtsi b/arch/arm/boot/dts/tegra20-colibri-512.dtsi
index cb73e62..af5a7ae 100644
--- a/arch/arm/boot/dts/tegra20-colibri-512.dtsi
+++ b/arch/arm/boot/dts/tegra20-colibri-512.dtsi
@@ -443,6 +443,10 @@ 
 		nvidia,phy-reset-gpio = <&gpio 169 0>; /* gpio PV1 */
 	};
 
+	usb-phy@c5004000 {
+		nvidia,phy-reset-gpio = <&gpio 169 0>; /* gpio PV1 */
+	};
+
 	sdhci@c8000600 {
 		cd-gpios = <&gpio 23 1>; /* gpio PC7 */
 	};
diff --git a/arch/arm/boot/dts/tegra20-harmony.dts b/arch/arm/boot/dts/tegra20-harmony.dts
index 5fb0888..3454ce2 100644
--- a/arch/arm/boot/dts/tegra20-harmony.dts
+++ b/arch/arm/boot/dts/tegra20-harmony.dts
@@ -427,12 +427,12 @@ 
 		nvidia,phy-reset-gpio = <&gpio 169 0>; /* gpio PV1 */
 	};
 
-	usb@c5008000 {
-		status = "okay";
+	usb-phy@c5004000 {
+		nvidia,phy-reset-gpio = <&gpio 169 0>; /* gpio PV1 */
 	};
 
-	usb-phy@c5004400 {
-		nvidia,phy-reset-gpio = <&gpio 169 0>; /* gpio PV1 */
+	usb@c5008000 {
+		status = "okay";
 	};
 
 	sdhci@c8000200 {
diff --git a/arch/arm/boot/dts/tegra20-iris-512.dts b/arch/arm/boot/dts/tegra20-iris-512.dts
index 52f1103..c99eccc 100644
--- a/arch/arm/boot/dts/tegra20-iris-512.dts
+++ b/arch/arm/boot/dts/tegra20-iris-512.dts
@@ -41,6 +41,10 @@ 
 		dr_mode = "otg";
 	};
 
+	usb-phy@c5000000 {
+		dr_mode = "otg";
+	};
+
 	usb@c5008000 {
 		status = "okay";
 	};
diff --git a/arch/arm/boot/dts/tegra20-paz00.dts b/arch/arm/boot/dts/tegra20-paz00.dts
index 43fd28b..cc76129 100644
--- a/arch/arm/boot/dts/tegra20-paz00.dts
+++ b/arch/arm/boot/dts/tegra20-paz00.dts
@@ -426,12 +426,12 @@ 
 		nvidia,phy-reset-gpio = <&gpio 168 0>; /* gpio PV0 */
 	};
 
-	usb@c5008000 {
-		status = "okay";
+	usb-phy@c5004000 {
+		nvidia,phy-reset-gpio = <&gpio 168 0>; /* gpio PV0 */
 	};
 
-	usb-phy@c5004400 {
-		nvidia,phy-reset-gpio = <&gpio 168 0>; /* gpio PV0 */
+	usb@c5008000 {
+		status = "okay";
 	};
 
 	sdhci@c8000000 {
diff --git a/arch/arm/boot/dts/tegra20-seaboard.dts b/arch/arm/boot/dts/tegra20-seaboard.dts
index 4f810a5..d234766 100644
--- a/arch/arm/boot/dts/tegra20-seaboard.dts
+++ b/arch/arm/boot/dts/tegra20-seaboard.dts
@@ -563,17 +563,22 @@ 
 		dr_mode = "otg";
 	};
 
+	usb-phy@c5000000 {
+		vbus-supply = <&vbus_reg>;
+		dr_mode = "otg";
+	};
+
 	usb@c5004000 {
 		status = "okay";
 		nvidia,phy-reset-gpio = <&gpio 169 0>; /* gpio PV1 */
 	};
 
-	usb@c5008000 {
-		status = "okay";
+	usb-phy@c5004000 {
+		nvidia,phy-reset-gpio = <&gpio 169 0>; /* gpio PV1 */
 	};
 
-	usb-phy@c5004400 {
-		nvidia,phy-reset-gpio = <&gpio 169 0>; /* gpio PV1 */
+	usb@c5008000 {
+		status = "okay";
 	};
 
 	sdhci@c8000000 {
@@ -786,6 +791,15 @@ 
 			gpio = <&pmic 1 0>;
 			enable-active-high;
 		};
+
+		vbus_reg: regulator@3 {
+			compatible = "regulator-fixed";
+			reg = <3>;
+			regulator-name = "vdd_5v0";
+			regulator-min-microvolt = <5000000>;
+			regulator-max-microvolt = <5000000>;
+			gpio = <&gpio 24 0>; /* PD0 */
+		};
 	};
 
 	sound {
diff --git a/arch/arm/boot/dts/tegra20-trimslice.dts b/arch/arm/boot/dts/tegra20-trimslice.dts
index 955bf49..8f2bb9b 100644
--- a/arch/arm/boot/dts/tegra20-trimslice.dts
+++ b/arch/arm/boot/dts/tegra20-trimslice.dts
@@ -305,17 +305,21 @@ 
 		nvidia,vbus-gpio = <&gpio 170 0>; /* gpio PV2 */
 	};
 
+	usb-phy@c5000000 {
+		vbus-supply = <&vbus_reg>;
+	};
+
 	usb@c5004000 {
 		status = "okay";
 		nvidia,phy-reset-gpio = <&gpio 168 0>; /* gpio PV0 */
 	};
 
-	usb@c5008000 {
-		status = "okay";
+	usb-phy@c5004000 {
+		nvidia,phy-reset-gpio = <&gpio 168 0>; /* gpio PV0 */
 	};
 
-	usb-phy@c5004400 {
-		nvidia,phy-reset-gpio = <&gpio 168 0>; /* gpio PV0 */
+	usb@c5008000 {
+		status = "okay";
 	};
 
 	sdhci@c8000000 {
@@ -357,6 +361,15 @@ 
 			regulator-max-microvolt = <1800000>;
 			regulator-always-on;
 		};
+
+		vbus_reg: regulator@2 {
+			compatible = "regulator-fixed";
+			reg = <2>;
+			regulator-name = "vdd_5v0";
+			regulator-min-microvolt = <5000000>;
+			regulator-max-microvolt = <5000000>;
+			gpio = <&gpio 170 0>; /* PV2 */
+		};
 	};
 
 	sound {
diff --git a/arch/arm/boot/dts/tegra20-ventana.dts b/arch/arm/boot/dts/tegra20-ventana.dts
index 3f8ae10..4c88b72 100644
--- a/arch/arm/boot/dts/tegra20-ventana.dts
+++ b/arch/arm/boot/dts/tegra20-ventana.dts
@@ -504,13 +504,14 @@ 
 		nvidia,phy-reset-gpio = <&gpio 169 0>; /* gpio PV1 */
 	};
 
+	usb-phy@c5004000 {
+		nvidia,phy-reset-gpio = <&gpio 169 0>; /* gpio PV1 */
+	};
+
 	usb@c5008000 {
 		status = "okay";
 	};
 
-	usb-phy@c5004400 {
-		nvidia,phy-reset-gpio = <&gpio 169 0>; /* gpio PV1 */
-	};
 
 	sdhci@c8000000 {
 		status = "okay";
diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
index 26c1134..9169d64 100644
--- a/arch/arm/boot/dts/tegra20.dtsi
+++ b/arch/arm/boot/dts/tegra20.dtsi
@@ -453,13 +453,23 @@ 
 		status = "disabled";
 	};
 
-	phy1: usb-phy@c5000400 {
+	phy1: usb-phy@c5000000 {
 		compatible = "nvidia,tegra20-usb-phy";
-		reg = <0xc5000400 0x3c00>;
+		reg = <0xc5000000 0x4000 0xc5000000 0x4000>;
 		phy_type = "utmi";
+		clocks = <&tegra_car 22>,
+			 <&tegra_car 127>,
+			 <&tegra_car 106>,
+			 <&tegra_car 22>;
+		clock-names = "reg", "pll_u", "timer", "utmi-pads";
 		nvidia,has-legacy-mode;
-		clocks = <&tegra_car 22>, <&tegra_car 127>;
-		clock-names = "phy", "pll_u";
+		hssync_start_delay = <9>;
+		idle_wait_delay = <17>;
+		elastic_limit = <16>;
+		term_range_adj = <6>;
+		xcvr_setup = <9>;
+		xcvr_lsfslew = <1>;
+		xcvr_lsrslew = <1>;
 	};
 
 	usb@c5004000 {
@@ -472,12 +482,14 @@ 
 		status = "disabled";
 	};
 
-	phy2: usb-phy@c5004400 {
+	phy2: usb-phy@c5004000 {
 		compatible = "nvidia,tegra20-usb-phy";
-		reg = <0xc5004400 0x3c00>;
+		reg = <0xc5004000 0x4000>;
 		phy_type = "ulpi";
-		clocks = <&tegra_car 94>, <&tegra_car 127>;
-		clock-names = "phy", "pll_u";
+		clocks = <&tegra_car 58>,
+			 <&tegra_car 127>,
+			 <&tegra_car 94>;
+		clock-names = "reg", "pll_u", "ulpi-link";
 	};
 
 	usb@c5008000 {
@@ -490,12 +502,22 @@ 
 		status = "disabled";
 	};
 
-	phy3: usb-phy@c5008400 {
+	phy3: usb-phy@c5008000 {
 		compatible = "nvidia,tegra20-usb-phy";
-		reg = <0xc5008400 0x3c00>;
+		reg = <0xc5008000 0x4000 0xc5000000 0x4000>;
 		phy_type = "utmi";
-		clocks = <&tegra_car 22>, <&tegra_car 127>;
-		clock-names = "phy", "pll_u";
+		clocks = <&tegra_car 59>,
+			 <&tegra_car 127>,
+			 <&tegra_car 106>,
+			 <&tegra_car 22>;
+		clock-names = "reg", "pll_u", "timer", "utmi-pads";
+		hssync_start_delay = <9>;
+		idle_wait_delay = <17>;
+		elastic_limit = <16>;
+		term_range_adj = <6>;
+		xcvr_setup = <9>;
+		xcvr_lsfslew = <2>;
+		xcvr_lsrslew = <2>;
 	};
 
 	sdhci@c8000000 {