Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1394412 > unrolled thread

[PATCH v3 1/2] usb: host: ehci-tegra: Grab the correct UTMI pads reset

Back to article view | Back to linux.kernel

Contents

  [PATCH v3 1/2] usb: host: ehci-tegra: Grab the correct UTMI pads reset Thierry Reding <thierry.reding@gmail.com> - 2016-05-04 16:50 +0200
    Re: [PATCH v3 1/2] usb: host: ehci-tegra: Grab the correct UTMI pads  reset Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-05-04 17:00 +0200
      Re: [PATCH v3 1/2] usb: host: ehci-tegra: Grab the correct UTMI pads  reset Thierry Reding <thierry.reding@gmail.com> - 2016-05-04 17:30 +0200
        Re: [PATCH v3 1/2] usb: host: ehci-tegra: Grab the correct UTMI pads  reset Tuomas Tynkkynen <tuomas.tynkkynen@iki.fi> - 2016-05-05 18:10 +0200
    Re: [PATCH v3 1/2] usb: host: ehci-tegra: Grab the correct UTMI pads  reset Stephen Warren <swarren@wwwdotorg.org> - 2016-05-04 19:20 +0200
      Re: [PATCH v3 1/2] usb: host: ehci-tegra: Grab the correct UTMI pads  reset Thierry Reding <thierry.reding@gmail.com> - 2016-05-04 22:40 +0200
    Re: [PATCH v3 1/2] usb: host: ehci-tegra: Grab the correct UTMI pads  reset Jon Hunter <jonathanh@nvidia.com> - 2016-05-05 09:40 +0200

#1394412 — [PATCH v3 1/2] usb: host: ehci-tegra: Grab the correct UTMI pads reset

From: Thierry Reding <treding@nvidia.com>

There are three EHCI controllers on Tegra SoCs, each with its own reset
line. However, the first controller contains a set of UTMI configuration
registers that are shared with its siblings. These registers will only
be reset as part of the first controller's reset. For proper operation
it must be ensured that the UTMI configuration registers are reset
before any of the EHCI controllers are enabled, irrespective of the
probe order.

Commit a47cc24cd1e5 ("USB: EHCI: tegra: Fix probe order issue leading to
broken USB") introduced code that ensures the first controller is always
reset before setting up any of the controllers, and is never again reset
afterwards.

This code, however, grabs the wrong reset. Each EHCI controller has two
reset controls attached: 1) the USB controller reset and 2) the UTMI
pads reset (really the first controller's reset). In order to reset the
UTMI pads registers the code must grab the second reset, but instead it
grabbing the first.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
Stephen, Alex, Jon, have you ever encountered cases where UTMI might not
have worked correctly? It seems that this code was pulsing the wrong
reset line and therefore the UTMI pads would never be reset unless the
first USB controller was probed before all others. I've never seen any
such problems myself, so I'm unsure about whether it's worth Cc'ing the
patch to stable@vger.kernel.org.

 drivers/usb/host/ehci-tegra.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c
index 4031b372008e..c1c1024a054c 100644
--- a/drivers/usb/host/ehci-tegra.c
+++ b/drivers/usb/host/ehci-tegra.c
@@ -89,7 +89,7 @@ static int tegra_reset_usb_controller(struct platform_device *pdev)
 	if (!usb1_reset_attempted) {
 		struct reset_control *usb1_reset;
 
-		usb1_reset = of_reset_control_get(phy_np, "usb");
+		usb1_reset = of_reset_control_get(phy_np, "utmi-pads");
 		if (IS_ERR(usb1_reset)) {
 			dev_warn(&pdev->dev,
 				 "can't get utmi-pads reset from the PHY\n");
-- 
2.8.0

[toc] | [next] | [standalone]

#1394419 — Re: [PATCH v3 1/2] usb: host: ehci-tegra: Grab the correct UTMI pads reset

On Wed, May 04, 2016 at 04:39:59PM +0200, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
> 
> There are three EHCI controllers on Tegra SoCs, each with its own reset
> line. However, the first controller contains a set of UTMI configuration
> registers that are shared with its siblings. These registers will only
> be reset as part of the first controller's reset. For proper operation
> it must be ensured that the UTMI configuration registers are reset
> before any of the EHCI controllers are enabled, irrespective of the
> probe order.
> 
> Commit a47cc24cd1e5 ("USB: EHCI: tegra: Fix probe order issue leading to
> broken USB") introduced code that ensures the first controller is always
> reset before setting up any of the controllers, and is never again reset
> afterwards.
> 
> This code, however, grabs the wrong reset. Each EHCI controller has two
> reset controls attached: 1) the USB controller reset and 2) the UTMI
> pads reset (really the first controller's reset). In order to reset the
> UTMI pads registers the code must grab the second reset, but instead it
> grabbing the first.
> 
> Signed-off-by: Thierry Reding <treding@nvidia.com>

Any reason you don't want this backported to stable kernels?

[toc] | [prev] | [next] | [standalone]

#1394467 — Re: [PATCH v3 1/2] usb: host: ehci-tegra: Grab the correct UTMI pads reset

[Multipart message — attachments visible in raw view] — view raw

On Wed, May 04, 2016 at 07:57:10AM -0700, Greg Kroah-Hartman wrote:
> On Wed, May 04, 2016 at 04:39:59PM +0200, Thierry Reding wrote:
> > From: Thierry Reding <treding@nvidia.com>
> > 
> > There are three EHCI controllers on Tegra SoCs, each with its own reset
> > line. However, the first controller contains a set of UTMI configuration
> > registers that are shared with its siblings. These registers will only
> > be reset as part of the first controller's reset. For proper operation
> > it must be ensured that the UTMI configuration registers are reset
> > before any of the EHCI controllers are enabled, irrespective of the
> > probe order.
> > 
> > Commit a47cc24cd1e5 ("USB: EHCI: tegra: Fix probe order issue leading to
> > broken USB") introduced code that ensures the first controller is always
> > reset before setting up any of the controllers, and is never again reset
> > afterwards.
> > 
> > This code, however, grabs the wrong reset. Each EHCI controller has two
> > reset controls attached: 1) the USB controller reset and 2) the UTMI
> > pads reset (really the first controller's reset). In order to reset the
> > UTMI pads registers the code must grab the second reset, but instead it
> > grabbing the first.
> > 
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> 
> Any reason you don't want this backported to stable kernels?

There's a brief note below the commit message that explains why I'm not
sure if there's a need to backport:

> Stephen, Alex, Jon, have you ever encountered cases where UTMI might not
> have worked correctly? It seems that this code was pulsing the wrong
> reset line and therefore the UTMI pads would never be reset unless the
> first USB controller was probed before all others. I've never seen any
> such problems myself, so I'm unsure about whether it's worth Cc'ing the
> patch to stable@vger.kernel.org.

So the bottom line is that I have no evidence that this fixes any real
issue, hence I'm not sure if it's worth bothering the stable kernel
maintainers with it.

While at it, adding Tuomas who wrote the original probe order fix.
Tuomas, does this patch look correct to you? Here's the patch in full if
you don't have it in your inbox:

	http://patchwork.ozlabs.org/patch/618488/

Thanks,
Thierry

[toc] | [prev] | [next] | [standalone]

#1395204 — Re: [PATCH v3 1/2] usb: host: ehci-tegra: Grab the correct UTMI pads reset

On 05/04/2016 06:26 PM, Thierry Reding wrote:
> On Wed, May 04, 2016 at 07:57:10AM -0700, Greg Kroah-Hartman wrote:
>> On Wed, May 04, 2016 at 04:39:59PM +0200, Thierry Reding wrote:
>>> From: Thierry Reding <treding@nvidia.com>
>>>
>>> There are three EHCI controllers on Tegra SoCs, each with its own reset
>>> line. However, the first controller contains a set of UTMI configuration
>>> registers that are shared with its siblings. These registers will only
>>> be reset as part of the first controller's reset. For proper operation
>>> it must be ensured that the UTMI configuration registers are reset
>>> before any of the EHCI controllers are enabled, irrespective of the
>>> probe order.
>>>
>>> Commit a47cc24cd1e5 ("USB: EHCI: tegra: Fix probe order issue leading to
>>> broken USB") introduced code that ensures the first controller is always
>>> reset before setting up any of the controllers, and is never again reset
>>> afterwards.
>>>
>>> This code, however, grabs the wrong reset. Each EHCI controller has two
>>> reset controls attached: 1) the USB controller reset and 2) the UTMI
>>> pads reset (really the first controller's reset). In order to reset the
>>> UTMI pads registers the code must grab the second reset, but instead it
>>> grabbing the first.
>>>
>>> Signed-off-by: Thierry Reding <treding@nvidia.com>
...snip ...
>
> While at it, adding Tuomas who wrote the original probe order fix.
> Tuomas, does this patch look correct to you? Here's the patch in full if
> you don't have it in your inbox:
>
> 	http://patchwork.ozlabs.org/patch/618488/
>

D'oh! Yes, that patch looks correct.

- Tuomas

[toc] | [prev] | [next] | [standalone]

#1394561 — Re: [PATCH v3 1/2] usb: host: ehci-tegra: Grab the correct UTMI pads reset

On 05/04/2016 08:39 AM, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
>
> There are three EHCI controllers on Tegra SoCs, each with its own reset
> line. However, the first controller contains a set of UTMI configuration
> registers that are shared with its siblings. These registers will only
> be reset as part of the first controller's reset. For proper operation
> it must be ensured that the UTMI configuration registers are reset
> before any of the EHCI controllers are enabled, irrespective of the
> probe order.
>
> Commit a47cc24cd1e5 ("USB: EHCI: tegra: Fix probe order issue leading to
> broken USB") introduced code that ensures the first controller is always
> reset before setting up any of the controllers, and is never again reset
> afterwards.
>
> This code, however, grabs the wrong reset. Each EHCI controller has two
> reset controls attached: 1) the USB controller reset and 2) the UTMI
> pads reset (really the first controller's reset). In order to reset the
> UTMI pads registers the code must grab the second reset, but instead it
> grabbing the first.
>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
> Stephen, Alex, Jon, have you ever encountered cases where UTMI might not
> have worked correctly? It seems that this code was pulsing the wrong
> reset line and therefore the UTMI pads would never be reset unless the
> first USB controller was probed before all others. I've never seen any
> such problems myself, so I'm unsure about whether it's worth Cc'ing the
> patch to stable@vger.kernel.org.

I don't think I recall seeing USB issues like that, although I don't use 
USB a huge amount. Perhaps the issue just never happens because we 
always have USB1 enabled, and it's physically present in the DTB first, 
so it always happens to get probed first?

[toc] | [prev] | [next] | [standalone]

#1394707 — Re: [PATCH v3 1/2] usb: host: ehci-tegra: Grab the correct UTMI pads reset

[Multipart message — attachments visible in raw view] — view raw

On Wed, May 04, 2016 at 11:14:50AM -0600, Stephen Warren wrote:
> On 05/04/2016 08:39 AM, Thierry Reding wrote:
> > From: Thierry Reding <treding@nvidia.com>
> > 
> > There are three EHCI controllers on Tegra SoCs, each with its own reset
> > line. However, the first controller contains a set of UTMI configuration
> > registers that are shared with its siblings. These registers will only
> > be reset as part of the first controller's reset. For proper operation
> > it must be ensured that the UTMI configuration registers are reset
> > before any of the EHCI controllers are enabled, irrespective of the
> > probe order.
> > 
> > Commit a47cc24cd1e5 ("USB: EHCI: tegra: Fix probe order issue leading to
> > broken USB") introduced code that ensures the first controller is always
> > reset before setting up any of the controllers, and is never again reset
> > afterwards.
> > 
> > This code, however, grabs the wrong reset. Each EHCI controller has two
> > reset controls attached: 1) the USB controller reset and 2) the UTMI
> > pads reset (really the first controller's reset). In order to reset the
> > UTMI pads registers the code must grab the second reset, but instead it
> > grabbing the first.
> > 
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> > Stephen, Alex, Jon, have you ever encountered cases where UTMI might not
> > have worked correctly? It seems that this code was pulsing the wrong
> > reset line and therefore the UTMI pads would never be reset unless the
> > first USB controller was probed before all others. I've never seen any
> > such problems myself, so I'm unsure about whether it's worth Cc'ing the
> > patch to stable@vger.kernel.org.
> 
> I don't think I recall seeing USB issues like that, although I don't use USB
> a huge amount. Perhaps the issue just never happens because we always have
> USB1 enabled, and it's physically present in the DTB first, so it always
> happens to get probed first?

Actually for Jetson TK1 we don't enable USB1. It's interesting because
v1 of patch 2/2 only fixed the issue for device where USB1 was indeed
enabled and probed first (I tested on TrimSlice). Running these fixes
through EIMT I noticed that it didn't fix it in the general case and I
was still seeing the warning on Jetson TK1 for example.

That's in fact what tipped me off about the consumer name, because I was
seeing reset 58 (USB2) being requested twice.

Thierry

[toc] | [prev] | [next] | [standalone]

#1394908 — Re: [PATCH v3 1/2] usb: host: ehci-tegra: Grab the correct UTMI pads reset

On 04/05/16 15:39, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
> 
> There are three EHCI controllers on Tegra SoCs, each with its own reset
> line. However, the first controller contains a set of UTMI configuration
> registers that are shared with its siblings. These registers will only
> be reset as part of the first controller's reset. For proper operation
> it must be ensured that the UTMI configuration registers are reset
> before any of the EHCI controllers are enabled, irrespective of the
> probe order.
> 
> Commit a47cc24cd1e5 ("USB: EHCI: tegra: Fix probe order issue leading to
> broken USB") introduced code that ensures the first controller is always
> reset before setting up any of the controllers, and is never again reset
> afterwards.
> 
> This code, however, grabs the wrong reset. Each EHCI controller has two
> reset controls attached: 1) the USB controller reset and 2) the UTMI
> pads reset (really the first controller's reset). In order to reset the
> UTMI pads registers the code must grab the second reset, but instead it
> grabbing the first.
> 
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
> Stephen, Alex, Jon, have you ever encountered cases where UTMI might not
> have worked correctly? It seems that this code was pulsing the wrong
> reset line and therefore the UTMI pads would never be reset unless the
> first USB controller was probed before all others. I've never seen any
> such problems myself, so I'm unsure about whether it's worth Cc'ing the
> patch to stable@vger.kernel.org.
> 
>  drivers/usb/host/ehci-tegra.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c
> index 4031b372008e..c1c1024a054c 100644
> --- a/drivers/usb/host/ehci-tegra.c
> +++ b/drivers/usb/host/ehci-tegra.c
> @@ -89,7 +89,7 @@ static int tegra_reset_usb_controller(struct platform_device *pdev)
>  	if (!usb1_reset_attempted) {
>  		struct reset_control *usb1_reset;
>  
> -		usb1_reset = of_reset_control_get(phy_np, "usb");
> +		usb1_reset = of_reset_control_get(phy_np, "utmi-pads");
>  		if (IS_ERR(usb1_reset)) {
>  			dev_warn(&pdev->dev,
>  				 "can't get utmi-pads reset from the PHY\n");
> 

I have not seen any issues either, but may be we were getting lucky. The
change makes sense to me.

Acked-by: Jon Hunter <jonathanh@nvidia.com>

Cheers
Jon

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web