[PATCH v2 0/4] Tegra USB probe order issue fix

[PATCH v2 0/4] Tegra USB probe order issue fix

[Tuomas Tynkkynen]

Hi all,

Here's a second version of the probe order issue series. This time I've
added the USB1 resets to the PHYs, thus replacing the really ugly parts
with something slightly better. Old device trees will still probe
successfully, but instead of this bugfix they'll get a dev_warn().

The reset control patch was dropped, as a similar patch from someone
else has landed in linux-next.

For a recap of the issue from the previous series:

Basically, the register area of the 1st USB controller contains some
registers that are global to all of the controllers, but that are also
cleared when reset is asserted to the 1st controller. So if (say) the
3rd controller would be the first one to finish probing successfully,
then the reset that happens during the 1st controller's probe would
result in broken USB. So the before doing anything with the USB HW,
we should reset the 1st controller once, and then never ever reset
it again.

While testing that the 1st USB controller still works without a reset
when the driver is unbound and bound again, I discovered an unbalanced
regulator_disable + clk_disable_unprepare in the PHY code if the EHCI
driver is unbound and rebound. This is fixed in patch 4.

Thanks,
Tuomas.

Tuomas Tynkkynen (4):
  USB: tegra: Add resets & has-utmi-pad-registers flag to the PHY
    binding
  ARM: tegra: Add resets & has-utmi-pad-registers flag to all USB PHYs
  USB: EHCI: tegra: Fix probe order issue leading to broken USB
  USB: PHY: tegra: Call tegra_usb_phy_close only on device removal

 .../bindings/usb/nvidia,tegra20-usb-phy.txt        |    8 +++
 arch/arm/boot/dts/tegra114.dtsi                    |    5 ++
 arch/arm/boot/dts/tegra124.dtsi                    |    7 +++
 arch/arm/boot/dts/tegra20.dtsi                     |    7 +++
 arch/arm/boot/dts/tegra30.dtsi                     |    7 +++
 drivers/usb/host/ehci-tegra.c                      |   62 +++++++++++++++++++-
 drivers/usb/phy/phy-tegra-usb.c                    |    8 +--
 7 files changed, 96 insertions(+), 8 deletions(-)

-- 
1.7.9.5

[PATCH v2 1/4] USB: tegra: Add resets & has-utmi-pad-registers flag to the PHY binding

[Tuomas Tynkkynen]

When Tegra was converted to use the standard reset bindings, the PHY was
forgotten, probably because all the resetting of the USB blocks were
done in the EHCI driver. What also went unnoticed is that resetting the
1st on-chip USB module also wipes some of the UTMI pad configuration
registers that are also used by the other USB blocks. So this fact needs
to be described in the device tree, and the driver modified not to reset
the 1st module at inappropriate times.

In order to stay compatible with old device trees, the USB drivers will
still function without these properties but with the old,
potentially buggy behaviour.

Signed-off-by: Tuomas Tynkkynen <ttynkkynen@nvidia.com>
---
v2 changes: new patch, didn't exist in v1
 .../bindings/usb/nvidia,tegra20-usb-phy.txt        |    8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/nvidia,tegra20-usb-phy.txt b/Documentation/devicetree/bindings/usb/nvidia,tegra20-usb-phy.txt
index ba797d3..c9205fb 100644
--- a/Documentation/devicetree/bindings/usb/nvidia,tegra20-usb-phy.txt
+++ b/Documentation/devicetree/bindings/usb/nvidia,tegra20-usb-phy.txt
@@ -20,6 +20,12 @@ Required properties :
      Present if phy_type == utmi.
    - ulpi-link: The clock Tegra provides to the ULPI PHY (cdev2).
      Present if phy_type == ulpi, and ULPI link mode is in use.
+ - resets : Must contain an entry for each entry in reset-names.
+   See ../reset/reset.txt for details.
+ - reset-names : Must include the following entries:
+   - usb: The PHY's own reset signal.
+   - utmi-pads: The reset of the PHY containing the chip-wide UTMI pad control
+     registers. Required even if phy_type == ulpi.
 
 Required properties for phy_type == ulpi:
   - nvidia,phy-reset-gpio : The GPIO used to reset the PHY.
@@ -56,6 +62,8 @@ Optional properties:
       host means this is a host controller
       peripheral means it is device controller
       otg means it can operate as either ("on the go")
+  - nvidia,has-utmi-pad-registers : boolean indicates whether this controller
+    contains the UTMI pad control registers common to all USB controllers.
 
 VBUS control (required for dr_mode == otg, optional for dr_mode == host):
   - vbus-supply: regulator for VBUS
-- 
1.7.9.5

[PATCH v2 2/4] ARM: tegra: Add resets & has-utmi-pad-registers flag to all USB PHYs

[Tuomas Tynkkynen]

Add new properties to all of the Tegra PHYs that are now required
according to the binding.

In order to stay compatible with old device trees, the USB drivers
will still function without these reset properties but with the old,
potentially buggy behaviour.

Signed-off-by: Tuomas Tynkkynen <ttynkkynen@nvidia.com>
---
v2 changes: new patch, didn't exist in v1
 arch/arm/boot/dts/tegra114.dtsi |    5 +++++
 arch/arm/boot/dts/tegra124.dtsi |    7 +++++++
 arch/arm/boot/dts/tegra20.dtsi  |    7 +++++++
 arch/arm/boot/dts/tegra30.dtsi  |    7 +++++++
 4 files changed, 26 insertions(+)

diff --git a/arch/arm/boot/dts/tegra114.dtsi b/arch/arm/boot/dts/tegra114.dtsi
index 335a1d8..80b8edd 100644
--- a/arch/arm/boot/dts/tegra114.dtsi
+++ b/arch/arm/boot/dts/tegra114.dtsi
@@ -672,6 +672,8 @@
 			 <&tegra_car TEGRA114_CLK_PLL_U>,
 			 <&tegra_car TEGRA114_CLK_USBD>;
 		clock-names = "reg", "pll_u", "utmi-pads";
+		resets = <&tegra_car 22>, <&tegra_car 22>;
+		reset-names = "usb", "utmi-pads";
 		nvidia,hssync-start-delay = <0>;
 		nvidia,idle-wait-delay = <17>;
 		nvidia,elastic-limit = <16>;
@@ -682,6 +684,7 @@
 		nvidia,hssquelch-level = <2>;
 		nvidia,hsdiscon-level = <5>;
 		nvidia,xcvr-hsslew = <12>;
+		nvidia,has-utmi-pad-registers;
 		status = "disabled";
 	};
 
@@ -705,6 +708,8 @@
 			 <&tegra_car TEGRA114_CLK_PLL_U>,
 			 <&tegra_car TEGRA114_CLK_USBD>;
 		clock-names = "reg", "pll_u", "utmi-pads";
+		resets = <&tegra_car 59>, <&tegra_car 22>;
+		reset-names = "usb", "utmi-pads";
 		nvidia,hssync-start-delay = <0>;
 		nvidia,idle-wait-delay = <17>;
 		nvidia,elastic-limit = <16>;
diff --git a/arch/arm/boot/dts/tegra124.dtsi b/arch/arm/boot/dts/tegra124.dtsi
index d675186..5b14d79 100644
--- a/arch/arm/boot/dts/tegra124.dtsi
+++ b/arch/arm/boot/dts/tegra124.dtsi
@@ -643,6 +643,8 @@
 			 <&tegra_car TEGRA124_CLK_PLL_U>,
 			 <&tegra_car TEGRA124_CLK_USBD>;
 		clock-names = "reg", "pll_u", "utmi-pads";
+		resets = <&tegra_car 22>, <&tegra_car 22>;
+		reset-names = "usb", "utmi-pads";
 		nvidia,hssync-start-delay = <0>;
 		nvidia,idle-wait-delay = <17>;
 		nvidia,elastic-limit = <16>;
@@ -653,6 +655,7 @@
 		nvidia,hssquelch-level = <2>;
 		nvidia,hsdiscon-level = <5>;
 		nvidia,xcvr-hsslew = <12>;
+		nvidia,has-utmi-pad-registers;
 		status = "disabled";
 	};
 
@@ -677,6 +680,8 @@
 			 <&tegra_car TEGRA124_CLK_PLL_U>,
 			 <&tegra_car TEGRA124_CLK_USBD>;
 		clock-names = "reg", "pll_u", "utmi-pads";
+		resets = <&tegra_car 58>, <&tegra_car 22>;
+		reset-names = "usb", "utmi-pads";
 		nvidia,hssync-start-delay = <0>;
 		nvidia,idle-wait-delay = <17>;
 		nvidia,elastic-limit = <16>;
@@ -711,6 +716,8 @@
 			 <&tegra_car TEGRA124_CLK_PLL_U>,
 			 <&tegra_car TEGRA124_CLK_USBD>;
 		clock-names = "reg", "pll_u", "utmi-pads";
+		resets = <&tegra_car 59>, <&tegra_car 22>;
+		reset-names = "usb", "utmi-pads";
 		nvidia,hssync-start-delay = <0>;
 		nvidia,idle-wait-delay = <17>;
 		nvidia,elastic-limit = <16>;
diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
index 243d84c..1908f69 100644
--- a/arch/arm/boot/dts/tegra20.dtsi
+++ b/arch/arm/boot/dts/tegra20.dtsi
@@ -645,6 +645,8 @@
 			 <&tegra_car TEGRA20_CLK_CLK_M>,
 			 <&tegra_car TEGRA20_CLK_USBD>;
 		clock-names = "reg", "pll_u", "timer", "utmi-pads";
+		resets = <&tegra_car 22>, <&tegra_car 22>;
+		reset-names = "usb", "utmi-pads";
 		nvidia,has-legacy-mode;
 		nvidia,hssync-start-delay = <9>;
 		nvidia,idle-wait-delay = <17>;
@@ -653,6 +655,7 @@
 		nvidia,xcvr-setup = <9>;
 		nvidia,xcvr-lsfslew = <1>;
 		nvidia,xcvr-lsrslew = <1>;
+		nvidia,has-utmi-pad-registers;
 		status = "disabled";
 	};
 
@@ -676,6 +679,8 @@
 			 <&tegra_car TEGRA20_CLK_PLL_U>,
 			 <&tegra_car TEGRA20_CLK_CDEV2>;
 		clock-names = "reg", "pll_u", "ulpi-link";
+		resets = <&tegra_car 58>, <&tegra_car 22>;
+		reset-names = "usb", "utmi-pads";
 		status = "disabled";
 	};
 
@@ -700,6 +705,8 @@
 			 <&tegra_car TEGRA20_CLK_CLK_M>,
 			 <&tegra_car TEGRA20_CLK_USBD>;
 		clock-names = "reg", "pll_u", "timer", "utmi-pads";
+		resets = <&tegra_car 59>, <&tegra_car 22>;
+		reset-names = "usb", "utmi-pads";
 		nvidia,hssync-start-delay = <9>;
 		nvidia,idle-wait-delay = <17>;
 		nvidia,elastic-limit = <16>;
diff --git a/arch/arm/boot/dts/tegra30.dtsi b/arch/arm/boot/dts/tegra30.dtsi
index 0b1ede9..6b35c29 100644
--- a/arch/arm/boot/dts/tegra30.dtsi
+++ b/arch/arm/boot/dts/tegra30.dtsi
@@ -790,6 +790,8 @@
 			 <&tegra_car TEGRA30_CLK_PLL_U>,
 			 <&tegra_car TEGRA30_CLK_USBD>;
 		clock-names = "reg", "pll_u", "utmi-pads";
+		resets = <&tegra_car 22>, <&tegra_car 22>;
+		reset-names = "usb", "utmi-pads";
 		nvidia,hssync-start-delay = <9>;
 		nvidia,idle-wait-delay = <17>;
 		nvidia,elastic-limit = <16>;
@@ -801,6 +803,7 @@
 		nvidia,xcvr-hsslew = <32>;
 		nvidia,hssquelch-level = <2>;
 		nvidia,hsdiscon-level = <5>;
+		nvidia,has-utmi-pad-registers;
 		status = "disabled";
 	};
 
@@ -824,6 +827,8 @@
 			 <&tegra_car TEGRA30_CLK_PLL_U>,
 			 <&tegra_car TEGRA30_CLK_USBD>;
 		clock-names = "reg", "pll_u", "utmi-pads";
+		resets = <&tegra_car 58>, <&tegra_car 22>;
+		reset-names = "usb", "utmi-pads";
 		nvidia,hssync-start-delay = <9>;
 		nvidia,idle-wait-delay = <17>;
 		nvidia,elastic-limit = <16>;
@@ -858,6 +863,8 @@
 			 <&tegra_car TEGRA30_CLK_PLL_U>,
 			 <&tegra_car TEGRA30_CLK_USBD>;
 		clock-names = "reg", "pll_u", "utmi-pads";
+		resets = <&tegra_car 59>, <&tegra_car 22>;
+		reset-names = "usb", "utmi-pads";
 		nvidia,hssync-start-delay = <0>;
 		nvidia,idle-wait-delay = <17>;
 		nvidia,elastic-limit = <16>;
-- 
1.7.9.5

Re: [PATCH v2 2/4] ARM: tegra: Add resets & has-utmi-pad-registers flag to all USB PHYs

[Tuomas Tynkkynen]

Thierry,

Since Stephen's on a vacation, I'd like to double-check with you that 
the DT changes looks good. Greg has applied these to the USB tree today.

Thanks,
Tuomas

On 04/07/14 04:09, Tuomas Tynkkynen wrote:
> Add new properties to all of the Tegra PHYs that are now required
> according to the binding.
>
> In order to stay compatible with old device trees, the USB drivers
> will still function without these reset properties but with the old,
> potentially buggy behaviour.
>
> Signed-off-by: Tuomas Tynkkynen <ttynkkynen-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> ---
> v2 changes: new patch, didn't exist in v1
>   arch/arm/boot/dts/tegra114.dtsi |    5 +++++
>   arch/arm/boot/dts/tegra124.dtsi |    7 +++++++
>   arch/arm/boot/dts/tegra20.dtsi  |    7 +++++++
>   arch/arm/boot/dts/tegra30.dtsi  |    7 +++++++
>   4 files changed, 26 insertions(+)
>
> diff --git a/arch/arm/boot/dts/tegra114.dtsi b/arch/arm/boot/dts/tegra114.dtsi
> index 335a1d8..80b8edd 100644
> --- a/arch/arm/boot/dts/tegra114.dtsi
> +++ b/arch/arm/boot/dts/tegra114.dtsi
> @@ -672,6 +672,8 @@
>   			 <&tegra_car TEGRA114_CLK_PLL_U>,
>   			 <&tegra_car TEGRA114_CLK_USBD>;
>   		clock-names = "reg", "pll_u", "utmi-pads";
> +		resets = <&tegra_car 22>, <&tegra_car 22>;
> +		reset-names = "usb", "utmi-pads";
>   		nvidia,hssync-start-delay = <0>;
>   		nvidia,idle-wait-delay = <17>;
>   		nvidia,elastic-limit = <16>;
> @@ -682,6 +684,7 @@
>   		nvidia,hssquelch-level = <2>;
>   		nvidia,hsdiscon-level = <5>;
>   		nvidia,xcvr-hsslew = <12>;
> +		nvidia,has-utmi-pad-registers;
>   		status = "disabled";
>   	};
>
> @@ -705,6 +708,8 @@
>   			 <&tegra_car TEGRA114_CLK_PLL_U>,
>   			 <&tegra_car TEGRA114_CLK_USBD>;
>   		clock-names = "reg", "pll_u", "utmi-pads";
> +		resets = <&tegra_car 59>, <&tegra_car 22>;
> +		reset-names = "usb", "utmi-pads";
>   		nvidia,hssync-start-delay = <0>;
>   		nvidia,idle-wait-delay = <17>;
>   		nvidia,elastic-limit = <16>;
> diff --git a/arch/arm/boot/dts/tegra124.dtsi b/arch/arm/boot/dts/tegra124.dtsi
> index d675186..5b14d79 100644
> --- a/arch/arm/boot/dts/tegra124.dtsi
> +++ b/arch/arm/boot/dts/tegra124.dtsi
> @@ -643,6 +643,8 @@
>   			 <&tegra_car TEGRA124_CLK_PLL_U>,
>   			 <&tegra_car TEGRA124_CLK_USBD>;
>   		clock-names = "reg", "pll_u", "utmi-pads";
> +		resets = <&tegra_car 22>, <&tegra_car 22>;
> +		reset-names = "usb", "utmi-pads";
>   		nvidia,hssync-start-delay = <0>;
>   		nvidia,idle-wait-delay = <17>;
>   		nvidia,elastic-limit = <16>;
> @@ -653,6 +655,7 @@
>   		nvidia,hssquelch-level = <2>;
>   		nvidia,hsdiscon-level = <5>;
>   		nvidia,xcvr-hsslew = <12>;
> +		nvidia,has-utmi-pad-registers;
>   		status = "disabled";
>   	};
>
> @@ -677,6 +680,8 @@
>   			 <&tegra_car TEGRA124_CLK_PLL_U>,
>   			 <&tegra_car TEGRA124_CLK_USBD>;
>   		clock-names = "reg", "pll_u", "utmi-pads";
> +		resets = <&tegra_car 58>, <&tegra_car 22>;
> +		reset-names = "usb", "utmi-pads";
>   		nvidia,hssync-start-delay = <0>;
>   		nvidia,idle-wait-delay = <17>;
>   		nvidia,elastic-limit = <16>;
> @@ -711,6 +716,8 @@
>   			 <&tegra_car TEGRA124_CLK_PLL_U>,
>   			 <&tegra_car TEGRA124_CLK_USBD>;
>   		clock-names = "reg", "pll_u", "utmi-pads";
> +		resets = <&tegra_car 59>, <&tegra_car 22>;
> +		reset-names = "usb", "utmi-pads";
>   		nvidia,hssync-start-delay = <0>;
>   		nvidia,idle-wait-delay = <17>;
>   		nvidia,elastic-limit = <16>;
> diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
> index 243d84c..1908f69 100644
> --- a/arch/arm/boot/dts/tegra20.dtsi
> +++ b/arch/arm/boot/dts/tegra20.dtsi
> @@ -645,6 +645,8 @@
>   			 <&tegra_car TEGRA20_CLK_CLK_M>,
>   			 <&tegra_car TEGRA20_CLK_USBD>;
>   		clock-names = "reg", "pll_u", "timer", "utmi-pads";
> +		resets = <&tegra_car 22>, <&tegra_car 22>;
> +		reset-names = "usb", "utmi-pads";
>   		nvidia,has-legacy-mode;
>   		nvidia,hssync-start-delay = <9>;
>   		nvidia,idle-wait-delay = <17>;
> @@ -653,6 +655,7 @@
>   		nvidia,xcvr-setup = <9>;
>   		nvidia,xcvr-lsfslew = <1>;
>   		nvidia,xcvr-lsrslew = <1>;
> +		nvidia,has-utmi-pad-registers;
>   		status = "disabled";
>   	};
>
> @@ -676,6 +679,8 @@
>   			 <&tegra_car TEGRA20_CLK_PLL_U>,
>   			 <&tegra_car TEGRA20_CLK_CDEV2>;
>   		clock-names = "reg", "pll_u", "ulpi-link";
> +		resets = <&tegra_car 58>, <&tegra_car 22>;
> +		reset-names = "usb", "utmi-pads";
>   		status = "disabled";
>   	};
>
> @@ -700,6 +705,8 @@
>   			 <&tegra_car TEGRA20_CLK_CLK_M>,
>   			 <&tegra_car TEGRA20_CLK_USBD>;
>   		clock-names = "reg", "pll_u", "timer", "utmi-pads";
> +		resets = <&tegra_car 59>, <&tegra_car 22>;
> +		reset-names = "usb", "utmi-pads";
>   		nvidia,hssync-start-delay = <9>;
>   		nvidia,idle-wait-delay = <17>;
>   		nvidia,elastic-limit = <16>;
> diff --git a/arch/arm/boot/dts/tegra30.dtsi b/arch/arm/boot/dts/tegra30.dtsi
> index 0b1ede9..6b35c29 100644
> --- a/arch/arm/boot/dts/tegra30.dtsi
> +++ b/arch/arm/boot/dts/tegra30.dtsi
> @@ -790,6 +790,8 @@
>   			 <&tegra_car TEGRA30_CLK_PLL_U>,
>   			 <&tegra_car TEGRA30_CLK_USBD>;
>   		clock-names = "reg", "pll_u", "utmi-pads";
> +		resets = <&tegra_car 22>, <&tegra_car 22>;
> +		reset-names = "usb", "utmi-pads";
>   		nvidia,hssync-start-delay = <9>;
>   		nvidia,idle-wait-delay = <17>;
>   		nvidia,elastic-limit = <16>;
> @@ -801,6 +803,7 @@
>   		nvidia,xcvr-hsslew = <32>;
>   		nvidia,hssquelch-level = <2>;
>   		nvidia,hsdiscon-level = <5>;
> +		nvidia,has-utmi-pad-registers;
>   		status = "disabled";
>   	};
>
> @@ -824,6 +827,8 @@
>   			 <&tegra_car TEGRA30_CLK_PLL_U>,
>   			 <&tegra_car TEGRA30_CLK_USBD>;
>   		clock-names = "reg", "pll_u", "utmi-pads";
> +		resets = <&tegra_car 58>, <&tegra_car 22>;
> +		reset-names = "usb", "utmi-pads";
>   		nvidia,hssync-start-delay = <9>;
>   		nvidia,idle-wait-delay = <17>;
>   		nvidia,elastic-limit = <16>;
> @@ -858,6 +863,8 @@
>   			 <&tegra_car TEGRA30_CLK_PLL_U>,
>   			 <&tegra_car TEGRA30_CLK_USBD>;
>   		clock-names = "reg", "pll_u", "utmi-pads";
> +		resets = <&tegra_car 59>, <&tegra_car 22>;
> +		reset-names = "usb", "utmi-pads";
>   		nvidia,hssync-start-delay = <0>;
>   		nvidia,idle-wait-delay = <17>;
>   		nvidia,elastic-limit = <16>;
>

-- 
nvpublic

Re: [PATCH v2 2/4] ARM: tegra: Add resets & has-utmi-pad-registers flag to all USB PHYs

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 449 bytes --]

On Thu, Jul 10, 2014 at 06:12:48PM +0300, Tuomas Tynkkynen wrote:
> Thierry,
> 
> Since Stephen's on a vacation, I'd like to double-check with you that the DT
> changes looks good. Greg has applied these to the USB tree today.

Yes, looks sane to me. Not sure how much people will like to see the DTS
changes go in through the USB tree, but I don't see a lot of potential
for conflicts, so chances are nobody will notice anyway.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

[PATCH v2 3/4] USB: EHCI: tegra: Fix probe order issue leading to broken USB

[Tuomas Tynkkynen]

The Tegra USB complex has a particularly annoying misdesign: some of the
UTMI pad configuration registers are global for all the 3 USB controllers
on the chip, but those registers are located in the first controller's
register space and will be cleared when the reset to the first
controller is asserted. Currently, this means that if the 1st controller
were to finish probing after the 2nd or 3rd controller, USB would not
work at all.

Fix this situation by always resetting the 1st controller before doing
any other setup to any of the controllers, and then never ever reset the
first controller again. As the UTMI registers are related to the PHY,
the PHY driver should probably reset the Tegra controllers instead,
but since old device trees only have reset phandles in the EHCI nodes,
do it here, which means a bit of device tree groveling. Those old DTs
also won't get the reset fix from this commit, so we'll dev_warn() them,
but the driver will still keep probing successfully.

Signed-off-by: Tuomas Tynkkynen <ttynkkynen@nvidia.com>
---
v2 changes: assume we can find the usb1 reset from the PHY DT node,
            don't fail if it's not found but just issue a warning
 drivers/usb/host/ehci-tegra.c |   62 +++++++++++++++++++++++++++++++++++++++--
 1 file changed, 59 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c
index 5590567..f495775 100644
--- a/drivers/usb/host/ehci-tegra.c
+++ b/drivers/usb/host/ehci-tegra.c
@@ -46,6 +46,7 @@
 #define DRV_NAME "tegra-ehci"
 
 static struct hc_driver __read_mostly tegra_ehci_hc_driver;
+static bool usb1_reset_attempted;
 
 struct tegra_ehci_soc_config {
 	bool has_hostpc;
@@ -60,6 +61,61 @@ struct tegra_ehci_hcd {
 	enum tegra_usb_phy_port_speed port_speed;
 };
 
+/*
+ * The 1st USB controller contains some UTMI pad registers that are global for
+ * all the controllers on the chip. Those registers are also cleared when
+ * reset is asserted to the 1st controller. This means that the 1st controller
+ * can only be reset when no other controlled has finished probing. So we'll
+ * reset the 1st controller before doing any other setup on any of the
+ * controllers, and then never again.
+ *
+ * Since this is a PHY issue, the Tegra PHY driver should probably be doing
+ * the resetting of the USB controllers. But to keep compatibility with old
+ * device trees that don't have reset phandles in the PHYs, do it here.
+ * Those old DTs will be vulnerable to total USB breakage if the 1st EHCI
+ * device isn't the first one to finish probing, so warn them.
+ */
+static int tegra_reset_usb_controller(struct platform_device *pdev)
+{
+	struct device_node *phy_np;
+	struct usb_hcd *hcd = platform_get_drvdata(pdev);
+	struct tegra_ehci_hcd *tegra =
+		(struct tegra_ehci_hcd *)hcd_to_ehci(hcd)->priv;
+
+	phy_np = of_parse_phandle(pdev->dev.of_node, "nvidia,phy", 0);
+	if (!phy_np)
+		return -ENOENT;
+
+	if (!usb1_reset_attempted) {
+		struct reset_control *usb1_reset;
+
+		usb1_reset = of_reset_control_get(phy_np, "usb");
+		if (IS_ERR(usb1_reset)) {
+			dev_warn(&pdev->dev,
+				 "can't get utmi-pads reset from the PHY\n");
+			dev_warn(&pdev->dev,
+				 "continuing, but please update your DT\n");
+		} else {
+			reset_control_assert(usb1_reset);
+			udelay(1);
+			reset_control_deassert(usb1_reset);
+		}
+
+		reset_control_put(usb1_reset);
+		usb1_reset_attempted = true;
+	}
+
+	if (!of_property_read_bool(phy_np, "nvidia,has-utmi-pad-registers")) {
+		reset_control_assert(tegra->rst);
+		udelay(1);
+		reset_control_deassert(tegra->rst);
+	}
+
+	of_node_put(phy_np);
+
+	return 0;
+}
+
 static int tegra_ehci_internal_port_reset(
 	struct ehci_hcd	*ehci,
 	u32 __iomem	*portsc_reg
@@ -389,9 +445,9 @@ static int tegra_ehci_probe(struct platform_device *pdev)
 	if (err)
 		goto cleanup_hcd_create;
 
-	reset_control_assert(tegra->rst);
-	udelay(1);
-	reset_control_deassert(tegra->rst);
+	err = tegra_reset_usb_controller(pdev);
+	if (err)
+		goto cleanup_clk_en;
 
 	u_phy = devm_usb_get_phy_by_phandle(&pdev->dev, "nvidia,phy", 0);
 	if (IS_ERR(u_phy)) {
-- 
1.7.9.5

Re: [PATCH v2 3/4] USB: EHCI: tegra: Fix probe order issue leading to broken USB

[Alan Stern]

On Fri, 4 Jul 2014, Tuomas Tynkkynen wrote:

> The Tegra USB complex has a particularly annoying misdesign: some of the
> UTMI pad configuration registers are global for all the 3 USB controllers
> on the chip, but those registers are located in the first controller's
> register space and will be cleared when the reset to the first
> controller is asserted. Currently, this means that if the 1st controller
> were to finish probing after the 2nd or 3rd controller, USB would not
> work at all.
> 
> Fix this situation by always resetting the 1st controller before doing
> any other setup to any of the controllers, and then never ever reset the
> first controller again. As the UTMI registers are related to the PHY,
> the PHY driver should probably reset the Tegra controllers instead,
> but since old device trees only have reset phandles in the EHCI nodes,
> do it here, which means a bit of device tree groveling. Those old DTs
> also won't get the reset fix from this commit, so we'll dev_warn() them,
> but the driver will still keep probing successfully.
> 
> Signed-off-by: Tuomas Tynkkynen <ttynkkynen@nvidia.com>
> ---
> v2 changes: assume we can find the usb1 reset from the PHY DT node,
>             don't fail if it's not found but just issue a warning
>  drivers/usb/host/ehci-tegra.c |   62 +++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 59 insertions(+), 3 deletions(-)

Acked-by: Alan Stern <stern@rowland.harvard.edu>

[PATCH v2 4/4] USB: PHY: tegra: Call tegra_usb_phy_close only on device removal

[Tuomas Tynkkynen]

tegra_usb_phy_close() is supposed to undo the effects of
tegra_usb_phy_init(). It is also currently added as the USB PHY shutdown
callback, which is wrong, since tegra_usb_phy_init() is only called
during probing wheras the shutdown callback can get called multiple
times. This then leads to warnings about unbalanced regulator_disable if
the EHCI driver is unbound and bound again at runtime.

Signed-off-by: Tuomas Tynkkynen <ttynkkynen@nvidia.com>
---
v2 changes: none
 drivers/usb/phy/phy-tegra-usb.c |    8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/phy/phy-tegra-usb.c b/drivers/usb/phy/phy-tegra-usb.c
index bbe4f8e..72e04a9 100644
--- a/drivers/usb/phy/phy-tegra-usb.c
+++ b/drivers/usb/phy/phy-tegra-usb.c
@@ -686,10 +686,8 @@ static int ulpi_phy_power_off(struct tegra_usb_phy *phy)
 	return gpio_direction_output(phy->reset_gpio, 0);
 }
 
-static void tegra_usb_phy_close(struct usb_phy *x)
+static void tegra_usb_phy_close(struct tegra_usb_phy *phy)
 {
-	struct tegra_usb_phy *phy = container_of(x, struct tegra_usb_phy, u_phy);
-
 	if (!IS_ERR(phy->vbus))
 		regulator_disable(phy->vbus);
 
@@ -1061,14 +1059,13 @@ static int tegra_usb_phy_probe(struct platform_device *pdev)
 	if (err < 0)
 		return err;
 
-	tegra_phy->u_phy.shutdown = tegra_usb_phy_close;
 	tegra_phy->u_phy.set_suspend = tegra_usb_phy_suspend;
 
 	platform_set_drvdata(pdev, tegra_phy);
 
 	err = usb_add_phy_dev(&tegra_phy->u_phy);
 	if (err < 0) {
-		tegra_usb_phy_close(&tegra_phy->u_phy);
+		tegra_usb_phy_close(tegra_phy);
 		return err;
 	}
 
@@ -1080,6 +1077,7 @@ static int tegra_usb_phy_remove(struct platform_device *pdev)
 	struct tegra_usb_phy *tegra_phy = platform_get_drvdata(pdev);
 
 	usb_remove_phy(&tegra_phy->u_phy);
+	tegra_usb_phy_close(tegra_phy);
 
 	return 0;
 }
-- 
1.7.9.5

Re: [PATCH v2 4/4] USB: PHY: tegra: Call tegra_usb_phy_close only on device removal

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 919 bytes --]

Hi,

On Fri, Jul 04, 2014 at 04:09:39AM +0300, Tuomas Tynkkynen wrote:
> tegra_usb_phy_close() is supposed to undo the effects of
> tegra_usb_phy_init(). It is also currently added as the USB PHY shutdown
> callback, which is wrong, since tegra_usb_phy_init() is only called

you could just make tegra_usb_phy_init() be called as u_phy->init().
That way you even delay enabling clocks and regulators to the point
where they are more likely to be needed. Also, if EHCI is never loaded,
you won't power up the PHY for no reason.

> during probing wheras the shutdown callback can get called multiple
> times. This then leads to warnings about unbalanced regulator_disable if
> the EHCI driver is unbound and bound again at runtime.
> 
> Signed-off-by: Tuomas Tynkkynen <ttynkkynen-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

I suppose this has no dependencies with the rest of the series ?

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v2 4/4] USB: PHY: tegra: Call tegra_usb_phy_close only on device removal

[Tuomas Tynkkynen]

On 10/07/14 16:48, Felipe Balbi wrote:
> * PGP Signed by an unknown key
>
> Hi,
>
> On Fri, Jul 04, 2014 at 04:09:39AM +0300, Tuomas Tynkkynen wrote:
>> tegra_usb_phy_close() is supposed to undo the effects of
>> tegra_usb_phy_init(). It is also currently added as the USB PHY shutdown
>> callback, which is wrong, since tegra_usb_phy_init() is only called
>
> you could just make tegra_usb_phy_init() be called as u_phy->init().

Apart from enabling pll_u and vbus, that function mostly fetches clocks 
etc. from the device tree and as such should preferably fail the probe() 
and not when the EHCI driver enables the PHY. (Renaming it would 
probably be a good idea.)

> That way you even delay enabling clocks and regulators to the point
> where they are more likely to be needed. Also, if EHCI is never loaded,
> you won't power up the PHY for no reason.
>

That's true, but due to the above that'd be a bigger refactoring.

>> during probing wheras the shutdown callback can get called multiple
>> times. This then leads to warnings about unbalanced regulator_disable if
>> the EHCI driver is unbound and bound again at runtime.
>>
>> Signed-off-by: Tuomas Tynkkynen <ttynkkynen-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>
> I suppose this has no dependencies with the rest of the series ?
>

No. But Greg apparently applied these to his tree earlier today.

Thanks,
Tuomas.

-- 
nvpublic

Re: [PATCH v2 0/4] Tegra USB probe order issue fix

[Mark Rutland]

On Fri, Jul 04, 2014 at 02:09:35AM +0100, Tuomas Tynkkynen wrote:
> Hi all,
> 
> Here's a second version of the probe order issue series. This time I've
> added the USB1 resets to the PHYs, thus replacing the really ugly parts
> with something slightly better. Old device trees will still probe
> successfully, but instead of this bugfix they'll get a dev_warn().

Given the binding looks sane and doesn't unnecessarily break existing
DTBs, for patches 1-3:

Acked-by: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>

Thanks,
Mark.