Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss

[PATCH] usb: host: tegra: Reset Tegra USB controller before init

46 views
Skip to first unread message

Venu Byravarasu

unread,
Feb 28, 2013, 1:40:01 AM2/28/13
to
To clear any configurations made by U-Boot on Tegra USB controller,
reset it before init in probe.

Signed-off-by: Venu Byravarasu <vbyra...@nvidia.com>
---
When U-Boot configures a Tegra USB controller in device mode and if the EHCI
driver of kernel tries to set it to HOST mode, message "irq 52: nobody cared"
appears and IRQ gets disabled.

This issue was initially reported with: http://marc.info/?l=linux-tegra&m=136110175423601&w=2

To avoid such issues, due to configurations made by U-Boot driver, reset the
Tegra USB controller, before configuring it by kernel.


drivers/usb/host/ehci-tegra.c | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c
index 568aecc..83d190a 100644
--- a/drivers/usb/host/ehci-tegra.c
+++ b/drivers/usb/host/ehci-tegra.c
@@ -28,6 +28,7 @@
#include <linux/pm_runtime.h>
#include <linux/usb/ehci_def.h>
#include <linux/usb/tegra_usb_phy.h>
+#include <linux/clk/tegra.h>

#define TEGRA_USB_BASE 0xC5000000
#define TEGRA_USB2_BASE 0xC5004000
@@ -691,6 +692,10 @@ static int tegra_ehci_probe(struct platform_device *pdev)
if (err)
goto fail_clk;

+ tegra_periph_reset_assert(tegra->clk);
+ udelay(1);
+ tegra_periph_reset_deassert(tegra->clk);
+
tegra->needs_double_reset = of_property_read_bool(pdev->dev.of_node,
"nvidia,needs-double-reset");

--
1.7.0.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Alan Stern

unread,
Feb 28, 2013, 10:20:02 AM2/28/13
to
On Thu, 28 Feb 2013, Venu Byravarasu wrote:

> To clear any configurations made by U-Boot on Tegra USB controller,
> reset it before init in probe.
>
> Signed-off-by: Venu Byravarasu <vbyra...@nvidia.com>
> ---
> When U-Boot configures a Tegra USB controller in device mode and if the EHCI
> driver of kernel tries to set it to HOST mode, message "irq 52: nobody cared"
> appears and IRQ gets disabled.
>
> This issue was initially reported with: http://marc.info/?l=linux-tegra&m=136110175423601&w=2
>
> To avoid such issues, due to configurations made by U-Boot driver, reset the
> Tegra USB controller, before configuring it by kernel.

Does the Tegra platform use shared interrupts? If it does, what
happens if the IRQ is enabled and in use by another device before
ehci-tegra resets the USB controller?

Does the unwanted interrupt occur only when the controller is switched
to host mode? If not, it seems to me this reset belongs in the
platform code, not in the glue layer. If yes, the reset belongs
somewher before the controller is switched -- where does that occur?

Alan Stern

Stephen Warren

unread,
Feb 28, 2013, 1:20:02 PM2/28/13
to
On 02/28/2013 08:09 AM, Alan Stern wrote:
> On Thu, 28 Feb 2013, Venu Byravarasu wrote:
>
>> To clear any configurations made by U-Boot on Tegra USB controller,
>> reset it before init in probe.
>>
>> Signed-off-by: Venu Byravarasu <vbyra...@nvidia.com>
>> ---
>> When U-Boot configures a Tegra USB controller in device mode and if the EHCI
>> driver of kernel tries to set it to HOST mode, message "irq 52: nobody cared"
>> appears and IRQ gets disabled.
>>
>> This issue was initially reported with: http://marc.info/?l=linux-tegra&m=136110175423601&w=2
>>
>> To avoid such issues, due to configurations made by U-Boot driver, reset the
>> Tegra USB controller, before configuring it by kernel.
>
> Does the Tegra platform use shared interrupts? If it does, what
> happens if the IRQ is enabled and in use by another device before
> ehci-tegra resets the USB controller?

I believe there's a dedicated interrupt just for each individual controller.

Stephen Warren

unread,
Feb 28, 2013, 1:20:03 PM2/28/13
to
On 02/27/2013 11:36 PM, Venu Byravarasu wrote:
> To clear any configurations made by U-Boot on Tegra USB controller,
> reset it before init in probe.

> diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c

> @@ -691,6 +692,10 @@ static int tegra_ehci_probe(struct platform_device *pdev)
> if (err)
> goto fail_clk;
>
> + tegra_periph_reset_assert(tegra->clk);
> + udelay(1);
> + tegra_periph_reset_deassert(tegra->clk);

I think this patch might cause unintended consequences.

When the Tegra PHY code is converted to a driver (i.e. has its own
probe), the initial order of execution of the PHY and EHCI driver probes
will not be guaranteed.

In particular, since the EHCI probe will attempt to "find" the PHY
device, and defer the EHCI probe until it can do so, this guarantees
that the PHY's probe() will have completed before EHCI's probe()
completes (although EHCI's probe may start running first some number of
times, and be retried with -EPROBE_DEFERRED for a variety of reasons).

Now, if the PHY driver's probe() actually touches HW and sets up some
registers, isn't this reset call going to trash any of that register
setup? Or, will PHY probe() not touch registers, but only do so during
the standard PHY open/init "op"/API calls?

I think the way to solve this is to put the reset call into the PHY
driver. I assume it has access to the appropriate clock object. This may
also address Alan's query re: when the unexpected interrupt occurs; it's
triggered by (or correlated with at least) the PHY (or USB port in
general) being in device mode due to the boot ROM setting it up this
way, then switching to host mode via the Linux driver. I /think/ that
device/host mode switching is more related to the PHY than EHCI driver,
although I could well be wrong here.

Alan Stern

unread,
Feb 28, 2013, 2:00:02 PM2/28/13
to
On Thu, 28 Feb 2013, Stephen Warren wrote:

> I think this patch might cause unintended consequences.
>
> When the Tegra PHY code is converted to a driver (i.e. has its own
> probe), the initial order of execution of the PHY and EHCI driver probes
> will not be guaranteed.
>
> In particular, since the EHCI probe will attempt to "find" the PHY
> device, and defer the EHCI probe until it can do so, this guarantees
> that the PHY's probe() will have completed before EHCI's probe()
> completes (although EHCI's probe may start running first some number of
> times, and be retried with -EPROBE_DEFERRED for a variety of reasons).
>
> Now, if the PHY driver's probe() actually touches HW and sets up some
> registers, isn't this reset call going to trash any of that register
> setup? Or, will PHY probe() not touch registers, but only do so during
> the standard PHY open/init "op"/API calls?
>
> I think the way to solve this is to put the reset call into the PHY
> driver. I assume it has access to the appropriate clock object. This may
> also address Alan's query re: when the unexpected interrupt occurs; it's
> triggered by (or correlated with at least) the PHY (or USB port in
> general) being in device mode due to the boot ROM setting it up this
> way, then switching to host mode via the Linux driver. I /think/ that
> device/host mode switching is more related to the PHY than EHCI driver,
> although I could well be wrong here.

With the PCI platform driver, the handoff from the firmware (we can
categorize U-Boot as firmware for this discussion) is handled as soon
as the controller is discovered by the platform-specific code.
There's a special pci-quirks.c file to take care of it. It is not
handled by the driver or the glue layer.

In general I think that's what needs to be done. Errant interrupt
sources should be disabled as quickly as possible.

In this case I don't know exactly when the earliest opportunity is. I
assume that the EHCI driver and/or the PHY driver gets probed because
some platform-layer code has registered the device. If this is so then
that platform-layer code is the right place to do the reset.

Alan Stern

Stephen Warren

unread,
Feb 28, 2013, 2:20:01 PM2/28/13
to
The first platform-specific code that is executed in this case is the
(struct platform_driver) probe() functions for the PHY and/or EHCI device.

On Tegra, all devices are listed in device tree, and core kernel code
instantiates platform devices based on the information in device tree.
The first non-core code that runs on a platform device is the driver's
probe() method. That probe() method is for a very specific piece of HW,
and hence seems to be the best place to put any quirks for it. Putting
the quirks outside the driver would mean some piece of the core DT code
would need to have a quirk list, and be able to map the registers for
the device, acquire clocks, etc. etc., implement the quirk, and tear it
all down again. All of which would just be duplicated with the driver's
own probe() function.

Venu Byravarasu

unread,
Mar 4, 2013, 3:00:02 AM3/4/13
to
Yes, PHY driver probe does not touch any registers. It just sets up the PHY API hooks.
These APIs will be called from ehci-tegra.c as part of ehci tegra probe function, after
getting PHY handle, which in turn happens after issuing above reset.

Thanks to Stephen & Alan, for the review comments.

>
> I think the way to solve this is to put the reset call into the PHY
> driver. I assume it has access to the appropriate clock object.

Stephen Warren

unread,
Mar 4, 2013, 12:00:01 PM3/4/13
to
On 03/04/2013 12:55 AM, Venu Byravarasu wrote:
OK, in that case I have no objection to this patch.

I'd like to hold off on applying this though; I suspect I'll want to
take the Tegra USB patches through the Tegra tree rather than the USB
tree again for the 3.10 kernel cycle. I think I may have screwed the
pooch on the DT binding I set up for the USB controller clocks, and
fixing this may require some Tegra DT changes, which would be easiest
taken through the Tegra tree, and so to reduce conflicts in the USB
code, taking the rest through there migth just be easiest.

Alan, Greg, if you're OK with this patch now, or for any revised
version, an Ack so I can take it through the Tegra tree would be great,
thanks.

Alan Stern

unread,
Mar 4, 2013, 12:40:02 PM3/4/13
to
On Mon, 4 Mar 2013, Stephen Warren wrote:

> On 03/04/2013 12:55 AM, Venu Byravarasu wrote:
> > Stephen Warren wrote at Thursday, February 28, 2013 11:47 PM:
> >> On 02/27/2013 11:36 PM, Venu Byravarasu wrote:
> >>> To clear any configurations made by U-Boot on Tegra USB controller,
> >>> reset it before init in probe.
> >>
> >>> diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c
> >>
> >>> @@ -691,6 +692,10 @@ static int tegra_ehci_probe(struct platform_device
> >> *pdev)
> >>> if (err)
> >>> goto fail_clk;
> >>>
> >>> + tegra_periph_reset_assert(tegra->clk);
> >>> + udelay(1);
> >>> + tegra_periph_reset_deassert(tegra->clk);

> OK, in that case I have no objection to this patch.
>
> I'd like to hold off on applying this though; I suspect I'll want to
> take the Tegra USB patches through the Tegra tree rather than the USB
> tree again for the 3.10 kernel cycle. I think I may have screwed the
> pooch on the DT binding I set up for the USB controller clocks, and
> fixing this may require some Tegra DT changes, which would be easiest
> taken through the Tegra tree, and so to reduce conflicts in the USB
> code, taking the rest through there migth just be easiest.
>
> Alan, Greg, if you're OK with this patch now, or for any revised
> version, an Ack so I can take it through the Tegra tree would be great,
> thanks.

I have no other objections.

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

Alan Stern

gre...@linuxfoundation.org

unread,
Mar 4, 2013, 7:40:01 PM3/4/13
to
Acked-by: Greg Kroah-Hartman <gre...@linuxfoundation.org>

Venu Byravarasu

unread,
Apr 2, 2013, 3:20:03 AM4/2/13
to
> -----Original Message-----
> From: gre...@linuxfoundation.org [mailto:gre...@linuxfoundation.org]
> Sent: Tuesday, March 05, 2013 6:04 AM
> To: Stephen Warren
> Cc: Venu Byravarasu; st...@rowland.harvard.edu; linux-
> u...@vger.kernel.org; linux-...@vger.kernel.org
> Subject: Re: [PATCH] usb: host: tegra: Reset Tegra USB controller before init
>
> On Mon, Mar 04, 2013 at 09:55:44AM -0700, Stephen Warren wrote:
> > On 03/04/2013 12:55 AM, Venu Byravarasu wrote:
> > > Stephen Warren wrote at Thursday, February 28, 2013 11:47 PM:
> > >> On 02/27/2013 11:36 PM, Venu Byravarasu wrote:
> > >>> To clear any configurations made by U-Boot on Tegra USB controller,
> > >>> reset it before init in probe.
> > >>
> > >>> diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-
> tegra.c
> > >>
> > >>> @@ -691,6 +692,10 @@ static int tegra_ehci_probe(struct
> platform_device
> > >> *pdev)
> > >>> if (err)
> > >>> goto fail_clk;
> > >>>
> > >>> + tegra_periph_reset_assert(tegra->clk);
> > >>> + udelay(1);
> > >>> + tegra_periph_reset_deassert(tegra->clk);
> > >>

> > Alan, Greg, if you're OK with this patch now, or for any revised
> > version, an Ack so I can take it through the Tegra tree would be great,
> > thanks.
>
> Acked-by: Greg Kroah-Hartman <gre...@linuxfoundation.org>

Hi Greg,

Stephen initially thought that the patch can be taken through tegra tree, along with
other patches posted at http://marc.info/?l=linux-tegra&m=136361016003625&w=2 .
As some other issues are blocking that patch series, can this change be merged
independent of that patch series via USB tree?

The original change can be seen at http://marc.info/?l=linux-usb&m=136203353205291&w=2

Thanks,
Venu

Stephen Warren

unread,
Apr 2, 2013, 11:40:01 AM4/2/13
to
On 04/02/2013 01:12 AM, Venu Byravarasu wrote:
>> gre...@linuxfoundation.org wrote at Tuesday, March 05, 2013 6:04 AM:
>> On Mon, Mar 04, 2013 at 09:55:44AM -0700, Stephen Warren wrote:
>>> On 03/04/2013 12:55 AM, Venu Byravarasu wrote:
>>>> Stephen Warren wrote at Thursday, February 28, 2013 11:47 PM:
>>>>> On 02/27/2013 11:36 PM, Venu Byravarasu wrote:
>>>>>> To clear any configurations made by U-Boot on Tegra USB controller,
>>>>>> reset it before init in probe.
...
>> Acked-by: Greg Kroah-Hartman <gre...@linuxfoundation.org>
>
> Hi Greg,
>
> Stephen initially thought that the patch can be taken through tegra tree, along with
> other patches posted at http://marc.info/?l=linux-tegra&m=136361016003625&w=2 .
> As some other issues are blocking that patch series, can this change be merged
> independent of that patch series via USB tree?
>
> The original change can be seen at http://marc.info/?l=linux-usb&m=136203353205291&w=2

Venu, you should simply repost the patch; I'm sure its not in anyone's
email inbox any more.

Venu Byravarasu

unread,
Apr 3, 2013, 6:50:02 AM4/3/13
to
> -----Original Message-----
> From: Stephen Warren [mailto:swa...@wwwdotorg.org]
> Sent: Tuesday, April 02, 2013 9:02 PM
> To: Venu Byravarasu
> Cc: gre...@linuxfoundation.org; st...@rowland.harvard.edu; linux-
> u...@vger.kernel.org; linux-...@vger.kernel.org
> Subject: Re: [PATCH] usb: host: tegra: Reset Tegra USB controller before init
>
> On 04/02/2013 01:12 AM, Venu Byravarasu wrote:
> >> gre...@linuxfoundation.org wrote at Tuesday, March 05, 2013 6:04 AM:
> >> On Mon, Mar 04, 2013 at 09:55:44AM -0700, Stephen Warren wrote:
> >>> On 03/04/2013 12:55 AM, Venu Byravarasu wrote:
> >>>> Stephen Warren wrote at Thursday, February 28, 2013 11:47 PM:
> >>>>> On 02/27/2013 11:36 PM, Venu Byravarasu wrote:
> >>>>>> To clear any configurations made by U-Boot on Tegra USB controller,
> >>>>>> reset it before init in probe.
> ...
> >> Acked-by: Greg Kroah-Hartman <gre...@linuxfoundation.org>
> >
> > Hi Greg,
> >
> > Stephen initially thought that the patch can be taken through tegra tree,
> along with
> > other patches posted at http://marc.info/?l=linux-
> tegra&m=136361016003625&w=2 .
> > As some other issues are blocking that patch series, can this change be
> merged
> > independent of that patch series via USB tree?
> >
> > The original change can be seen at http://marc.info/?l=linux-
> usb&m=136203353205291&w=2
>
> Venu, you should simply repost the patch; I'm sure its not in anyone's
> email inbox any more.

Resent the patch, with ACKs added.
Latest patch can be seen at: http://marc.info/?l=linux-tegra&m=136498598329081&w=2

Thanks,
Venu

Venu Byravarasu

unread,
Apr 3, 2013, 6:50:02 AM4/3/13
to
To clear any configurations made by U-Boot on Tegra USB controller,
reset it before init in probe.

Signed-off-by: Venu Byravarasu <vbyra...@nvidia.com>
Acked-by: Alan Stern <st...@rowland.harvard.edu>
Acked-by: Greg Kroah-Hartman <gre...@linuxfoundation.org>
reviewed-by: Stephen Warren <swa...@nvidia.com>
---
This patch was already reviewed at:
http://marc.info/?l=linux-usb&m=136203353205291&w=2

drivers/usb/host/ehci-tegra.c | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c
index 1d2488c..60ed5d1 100644
--- a/drivers/usb/host/ehci-tegra.c
+++ b/drivers/usb/host/ehci-tegra.c
@@ -28,6 +28,7 @@
#include <linux/pm_runtime.h>
#include <linux/usb/ehci_def.h>
#include <linux/usb/tegra_usb_phy.h>
+#include <linux/clk/tegra.h>

#define TEGRA_USB_BASE 0xC5000000
#define TEGRA_USB2_BASE 0xC5004000
@@ -691,6 +692,10 @@ static int tegra_ehci_probe(struct platform_device *pdev)
if (err)
goto fail_clk;

+ tegra_periph_reset_assert(tegra->clk);
+ udelay(1);
+ tegra_periph_reset_deassert(tegra->clk);
+
tegra->needs_double_reset = of_property_read_bool(pdev->dev.of_node,
"nvidia,needs-double-reset");

--
1.7.0.4

0 new messages