[PATCH v2] ARM: tegra: fix ulpi regression on tegra20

Subsystems: generic phy framework, open firmware and flattened device tree bindings, the rest

STALE3104d

7 messages, 5 authors, 2018-03-08 · open the first message on its own page

[PATCH v2] ARM: tegra: fix ulpi regression on tegra20

From: Marcel Ziswiler <hidden>
Date: 2018-02-22 14:39:24

From: Marcel Ziswiler <redacted>

Since commit f8f8f1d04494 ("clk: Don't touch hardware when reparenting
during registration") ULPI has been broken on Tegra20 leading to the
following error message during boot:

[    1.974698] ulpi_phy_power_on: ulpi write failed
[    1.979384] tegra-ehci c5004000.usb: Failed to power on the phy
[    1.985434] tegra-ehci: probe of c5004000.usb failed with error -110

Debugging through the changes and finally also consulting the TRM
revealed that rather than the CDEV2 clock off OSC requiring such pin
muxing actually the PLL_P_OUT4 clock is in use. It looks like so far it
just worked by chance of that one having been enabled which Stephen's
commit now changed when reparenting sclk away from pll_p_out4 leaving
that one disabled. Fix this by properly assigning the PLL_P_OUT4 clock
as the ULPI PHY clock.

Signed-off-by: Marcel Ziswiler <redacted>

---

Changes in v2:
- Updated device tree binding documentation as well.
- CCing Dmitry as well.

 Documentation/devicetree/bindings/phy/nvidia,tegra20-usb-phy.txt | 4 +++-
 arch/arm/boot/dts/tegra20.dtsi                                   | 2 +-
 2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/phy/nvidia,tegra20-usb-phy.txt b/Documentation/devicetree/bindings/phy/nvidia,tegra20-usb-phy.txt
index a9aa79fb90ed..1aa6f2674af5 100644
--- a/Documentation/devicetree/bindings/phy/nvidia,tegra20-usb-phy.txt
+++ b/Documentation/devicetree/bindings/phy/nvidia,tegra20-usb-phy.txt
@@ -21,7 +21,9 @@ Required properties :
    - timer: The timeout clock (clk_m). Present if phy_type == utmi.
    - utmi-pads: The clock needed to access the UTMI pad control registers.
      Present if phy_type == utmi.
-   - ulpi-link: The clock Tegra provides to the ULPI PHY (cdev2).
+   - ulpi-link: The clock Tegra provides to the ULPI PHY (usually pad DAP_MCLK2
+     with pad group aka "nvidia,pins" cdev2 and pin mux option config aka
+     "nvidia,function" pllp_out4).
      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.
diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
index 864a95872b8d..e05b6bb2599f 100644
--- a/arch/arm/boot/dts/tegra20.dtsi
+++ b/arch/arm/boot/dts/tegra20.dtsi
@@ -741,7 +741,7 @@
 		phy_type = "ulpi";
 		clocks = <&tegra_car TEGRA20_CLK_USB2>,
 			 <&tegra_car TEGRA20_CLK_PLL_U>,
-			 <&tegra_car TEGRA20_CLK_CDEV2>;
+			 <&tegra_car TEGRA20_CLK_PLL_P_OUT4>;
 		clock-names = "reg", "pll_u", "ulpi-link";
 		resets = <&tegra_car 58>, <&tegra_car 22>;
 		reset-names = "usb", "utmi-pads";
-- 
2.14.3

Re: [PATCH v2] ARM: tegra: fix ulpi regression on tegra20

From: Dmitry Osipenko <digetx@gmail.com>
Date: 2018-02-22 20:08:52

On 22.02.2018 17:38, Marcel Ziswiler wrote:
From: Marcel Ziswiler <redacted>

Since commit f8f8f1d04494 ("clk: Don't touch hardware when reparenting
during registration") ULPI has been broken on Tegra20 leading to the
following error message during boot:

[    1.974698] ulpi_phy_power_on: ulpi write failed
[    1.979384] tegra-ehci c5004000.usb: Failed to power on the phy
[    1.985434] tegra-ehci: probe of c5004000.usb failed with error -110

Debugging through the changes and finally also consulting the TRM
revealed that rather than the CDEV2 clock off OSC requiring such pin
muxing actually the PLL_P_OUT4 clock is in use. It looks like so far it
just worked by chance of that one having been enabled which Stephen's
commit now changed when reparenting sclk away from pll_p_out4 leaving
that one disabled. Fix this by properly assigning the PLL_P_OUT4 clock
as the ULPI PHY clock.

Signed-off-by: Marcel Ziswiler <redacted>
Indeed CDEV2 clock doesn't exist on Tegra, but there is a CDEV2 pinmux pad
group. Looks like another clk-related relic from downstream driver that was
borrowed to upstream and stayed unnoticed for a long time.

I think in practice all T20 boards have CDEV2 configured to PLL_P_OUT4, so this
patch is correct to me. Probably it would also worth to set CDEV2 parent to
PLL_P_OUT4 in the clk driver for DT backwards compatibility or even make clk
driver to read the CDEV2 pinmux config and set the proper parent based on it.
CC'ed Peter.

Reviewed-by: Dmitry Osipenko <digetx@gmail.com>
quoted hunk
---

Changes in v2:
- Updated device tree binding documentation as well.
- CCing Dmitry as well.

 Documentation/devicetree/bindings/phy/nvidia,tegra20-usb-phy.txt | 4 +++-
 arch/arm/boot/dts/tegra20.dtsi                                   | 2 +-
 2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/phy/nvidia,tegra20-usb-phy.txt b/Documentation/devicetree/bindings/phy/nvidia,tegra20-usb-phy.txt
index a9aa79fb90ed..1aa6f2674af5 100644
--- a/Documentation/devicetree/bindings/phy/nvidia,tegra20-usb-phy.txt
+++ b/Documentation/devicetree/bindings/phy/nvidia,tegra20-usb-phy.txt
@@ -21,7 +21,9 @@ Required properties :
    - timer: The timeout clock (clk_m). Present if phy_type == utmi.
    - utmi-pads: The clock needed to access the UTMI pad control registers.
      Present if phy_type == utmi.
-   - ulpi-link: The clock Tegra provides to the ULPI PHY (cdev2).
+   - ulpi-link: The clock Tegra provides to the ULPI PHY (usually pad DAP_MCLK2
+     with pad group aka "nvidia,pins" cdev2 and pin mux option config aka
+     "nvidia,function" pllp_out4).
      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.
diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
index 864a95872b8d..e05b6bb2599f 100644
--- a/arch/arm/boot/dts/tegra20.dtsi
+++ b/arch/arm/boot/dts/tegra20.dtsi
@@ -741,7 +741,7 @@
 		phy_type = "ulpi";
 		clocks = <&tegra_car TEGRA20_CLK_USB2>,
 			 <&tegra_car TEGRA20_CLK_PLL_U>,
-			 <&tegra_car TEGRA20_CLK_CDEV2>;
+			 <&tegra_car TEGRA20_CLK_PLL_P_OUT4>;
 		clock-names = "reg", "pll_u", "ulpi-link";
 		resets = <&tegra_car 58>, <&tegra_car 22>;
 		reset-names = "usb", "utmi-pads";

Re: [PATCH v2] ARM: tegra: fix ulpi regression on tegra20

From: Marcel Ziswiler <hidden>
Date: 2018-02-22 21:38:33


On February 22, 2018 9:08:44 PM GMT+01:00, Dmitry Osipenko [off-list ref] wrote:
On 22.02.2018 17:38, Marcel Ziswiler wrote:
quoted
From: Marcel Ziswiler <redacted>

Since commit f8f8f1d04494 ("clk: Don't touch hardware when
reparenting
quoted
during registration") ULPI has been broken on Tegra20 leading to the
following error message during boot:

[    1.974698] ulpi_phy_power_on: ulpi write failed
[    1.979384] tegra-ehci c5004000.usb: Failed to power on the phy
[    1.985434] tegra-ehci: probe of c5004000.usb failed with error
-110
quoted
Debugging through the changes and finally also consulting the TRM
revealed that rather than the CDEV2 clock off OSC requiring such pin
muxing actually the PLL_P_OUT4 clock is in use. It looks like so far
it
quoted
just worked by chance of that one having been enabled which Stephen's
commit now changed when reparenting sclk away from pll_p_out4 leaving
that one disabled. Fix this by properly assigning the PLL_P_OUT4
clock
quoted
as the ULPI PHY clock.

Signed-off-by: Marcel Ziswiler <redacted>
Indeed CDEV2 clock doesn't exist on Tegra, but there is a CDEV2 pinmux
pad
group. Looks like another clk-related relic from downstream driver that
was
borrowed to upstream and stayed unnoticed for a long time.

I think in practice all T20 boards have CDEV2 configured to PLL_P_OUT4,
Exactly.
so this
patch is correct to me. Probably it would also worth to set CDEV2
parent to
PLL_P_OUT4 in the clk driver for DT backwards compatibility
I guess that might work but what the original designer(s) probably meant by that strange otherwise nonexistent cdev2 clock might be the other configuration of that pad group called OSC which would allow outputting the base Oscillator frequency even with a separate divider allowing by 2, 4 or even 8 as far as I remember from glancing at the TRM. However like you correctly noted no design ever made use of any such as far as I can tell.
or even
make clk
driver to read the CDEV2 pinmux config and set the proper parent based
on it.
Sounds a little too fancy to me but might work if device tree backwards compatibility is required at utmost importance.
CC'ed Peter.

Reviewed-by: Dmitry Osipenko <digetx@gmail.com>
Thanks Dmitry.
quoted
---

Changes in v2:
- Updated device tree binding documentation as well.
- CCing Dmitry as well.

 Documentation/devicetree/bindings/phy/nvidia,tegra20-usb-phy.txt | 4
+++-
quoted
 arch/arm/boot/dts/tegra20.dtsi                                   | 2
+-
quoted
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git
a/Documentation/devicetree/bindings/phy/nvidia,tegra20-usb-phy.txt
b/Documentation/devicetree/bindings/phy/nvidia,tegra20-usb-phy.txt
quoted
index a9aa79fb90ed..1aa6f2674af5 100644
---
a/Documentation/devicetree/bindings/phy/nvidia,tegra20-usb-phy.txt
quoted
+++
b/Documentation/devicetree/bindings/phy/nvidia,tegra20-usb-phy.txt
quoted
@@ -21,7 +21,9 @@ Required properties :
    - timer: The timeout clock (clk_m). Present if phy_type == utmi.
    - utmi-pads: The clock needed to access the UTMI pad control
registers.
quoted
      Present if phy_type == utmi.
-   - ulpi-link: The clock Tegra provides to the ULPI PHY (cdev2).
+   - ulpi-link: The clock Tegra provides to the ULPI PHY (usually
pad DAP_MCLK2
quoted
+     with pad group aka "nvidia,pins" cdev2 and pin mux option
config aka
quoted
+     "nvidia,function" pllp_out4).
      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.
diff --git a/arch/arm/boot/dts/tegra20.dtsi
b/arch/arm/boot/dts/tegra20.dtsi
quoted
index 864a95872b8d..e05b6bb2599f 100644
--- a/arch/arm/boot/dts/tegra20.dtsi
+++ b/arch/arm/boot/dts/tegra20.dtsi
@@ -741,7 +741,7 @@
 		phy_type = "ulpi";
 		clocks = <&tegra_car TEGRA20_CLK_USB2>,
 			 <&tegra_car TEGRA20_CLK_PLL_U>,
-			 <&tegra_car TEGRA20_CLK_CDEV2>;
+			 <&tegra_car TEGRA20_CLK_PLL_P_OUT4>;
 		clock-names = "reg", "pll_u", "ulpi-link";
 		resets = <&tegra_car 58>, <&tegra_car 22>;
 		reset-names = "usb", "utmi-pads";

Re: [PATCH v2] ARM: tegra: fix ulpi regression on tegra20

From: Rob Herring <robh@kernel.org>
Date: 2018-03-01 23:24:23

On Thu, Feb 22, 2018 at 03:38:25PM +0100, Marcel Ziswiler wrote:
From: Marcel Ziswiler <redacted>

Since commit f8f8f1d04494 ("clk: Don't touch hardware when reparenting
during registration") ULPI has been broken on Tegra20 leading to the
following error message during boot:

[    1.974698] ulpi_phy_power_on: ulpi write failed
[    1.979384] tegra-ehci c5004000.usb: Failed to power on the phy
[    1.985434] tegra-ehci: probe of c5004000.usb failed with error -110

Debugging through the changes and finally also consulting the TRM
revealed that rather than the CDEV2 clock off OSC requiring such pin
muxing actually the PLL_P_OUT4 clock is in use. It looks like so far it
just worked by chance of that one having been enabled which Stephen's
commit now changed when reparenting sclk away from pll_p_out4 leaving
that one disabled. Fix this by properly assigning the PLL_P_OUT4 clock
as the ULPI PHY clock.

Signed-off-by: Marcel Ziswiler <redacted>

---

Changes in v2:
- Updated device tree binding documentation as well.
- CCing Dmitry as well.

 Documentation/devicetree/bindings/phy/nvidia,tegra20-usb-phy.txt | 4 +++-
 arch/arm/boot/dts/tegra20.dtsi                                   | 2 +-
 2 files changed, 4 insertions(+), 2 deletions(-)
Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH v2] ARM: tegra: fix ulpi regression on tegra20

From: Thierry Reding <hidden>
Date: 2018-03-08 14:15:08

On Thu, Feb 22, 2018 at 03:38:25PM +0100, Marcel Ziswiler wrote:
From: Marcel Ziswiler <redacted>

Since commit f8f8f1d04494 ("clk: Don't touch hardware when reparenting
during registration") ULPI has been broken on Tegra20 leading to the
following error message during boot:

[    1.974698] ulpi_phy_power_on: ulpi write failed
[    1.979384] tegra-ehci c5004000.usb: Failed to power on the phy
[    1.985434] tegra-ehci: probe of c5004000.usb failed with error -110

Debugging through the changes and finally also consulting the TRM
revealed that rather than the CDEV2 clock off OSC requiring such pin
muxing actually the PLL_P_OUT4 clock is in use. It looks like so far it
just worked by chance of that one having been enabled which Stephen's
commit now changed when reparenting sclk away from pll_p_out4 leaving
that one disabled. Fix this by properly assigning the PLL_P_OUT4 clock
as the ULPI PHY clock.

Signed-off-by: Marcel Ziswiler <redacted>

---

Changes in v2:
- Updated device tree binding documentation as well.
- CCing Dmitry as well.

 Documentation/devicetree/bindings/phy/nvidia,tegra20-usb-phy.txt | 4 +++-
 arch/arm/boot/dts/tegra20.dtsi                                   | 2 +-
 2 files changed, 4 insertions(+), 2 deletions(-)
Applied, though I've split this into two separate patches, one going
into for-4.17/dt-bindings and the other going into for-4.17/arm/dt.

Thanks,
Thierry

Re: [PATCH v2] ARM: tegra: fix ulpi regression on tegra20

From: Marcel Ziswiler <hidden>
Date: 2018-03-08 14:31:57

On Thu, 2018-03-08 at 15:15 +0100, Thierry Reding wrote:
On Thu, Feb 22, 2018 at 03:38:25PM +0100, Marcel Ziswiler wrote:
quoted
From: Marcel Ziswiler <redacted>

Since commit f8f8f1d04494 ("clk: Don't touch hardware when
reparenting
during registration") ULPI has been broken on Tegra20 leading to
the
following error message during boot:

[    1.974698] ulpi_phy_power_on: ulpi write failed
[    1.979384] tegra-ehci c5004000.usb: Failed to power on the phy
[    1.985434] tegra-ehci: probe of c5004000.usb failed with error
-110

Debugging through the changes and finally also consulting the TRM
revealed that rather than the CDEV2 clock off OSC requiring such
pin
muxing actually the PLL_P_OUT4 clock is in use. It looks like so
far it
just worked by chance of that one having been enabled which
Stephen's
commit now changed when reparenting sclk away from pll_p_out4
leaving
that one disabled. Fix this by properly assigning the PLL_P_OUT4
clock
as the ULPI PHY clock.

Signed-off-by: Marcel Ziswiler <redacted>

---

Changes in v2:
- Updated device tree binding documentation as well.
- CCing Dmitry as well.

 Documentation/devicetree/bindings/phy/nvidia,tegra20-usb-phy.txt |
4 +++-
 arch/arm/boot/dts/tegra20.dtsi                                   |
2 +-
 2 files changed, 4 insertions(+), 2 deletions(-)
Applied, though I've split this into two separate patches, one going
into for-4.17/dt-bindings and the other going into for-4.17/arm/dt.
Thanks, Thierry!

BTW: Do you know what is happening with my T30 USB patch? Is there
anything I may do about it?
Thanks,
Thierry

Re: [PATCH v2] ARM: tegra: fix ulpi regression on tegra20

From: Thierry Reding <hidden>
Date: 2018-03-08 14:59:20

On Thu, Mar 08, 2018 at 02:31:26PM +0000, Marcel Ziswiler wrote:
On Thu, 2018-03-08 at 15:15 +0100, Thierry Reding wrote:
quoted
On Thu, Feb 22, 2018 at 03:38:25PM +0100, Marcel Ziswiler wrote:
quoted
From: Marcel Ziswiler <redacted>

Since commit f8f8f1d04494 ("clk: Don't touch hardware when
reparenting
during registration") ULPI has been broken on Tegra20 leading to
the
following error message during boot:

[    1.974698] ulpi_phy_power_on: ulpi write failed
[    1.979384] tegra-ehci c5004000.usb: Failed to power on the phy
[    1.985434] tegra-ehci: probe of c5004000.usb failed with error
-110

Debugging through the changes and finally also consulting the TRM
revealed that rather than the CDEV2 clock off OSC requiring such
pin
muxing actually the PLL_P_OUT4 clock is in use. It looks like so
far it
just worked by chance of that one having been enabled which
Stephen's
commit now changed when reparenting sclk away from pll_p_out4
leaving
that one disabled. Fix this by properly assigning the PLL_P_OUT4
clock
as the ULPI PHY clock.

Signed-off-by: Marcel Ziswiler <redacted>

---

Changes in v2:
- Updated device tree binding documentation as well.
- CCing Dmitry as well.

 Documentation/devicetree/bindings/phy/nvidia,tegra20-usb-phy.txt |
4 +++-
 arch/arm/boot/dts/tegra20.dtsi                                   |
2 +-
 2 files changed, 4 insertions(+), 2 deletions(-)
Applied, though I've split this into two separate patches, one going
into for-4.17/dt-bindings and the other going into for-4.17/arm/dt.
Thanks, Thierry!

BTW: Do you know what is happening with my T30 USB patch? Is there
anything I may do about it?
I just took care of that.

Thierry
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help