| Message ID | 1364978502-22887-3-git-send-email-vbyravarasu@nvidia.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
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
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
> -----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
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
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 {
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(-)