RE: [PATCH] usb: host: tegra: Reset Tegra USB controller before init
> -Original Message- > From: Stephen Warren [mailto:swar...@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-kernel@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 > > > > 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=136361016003625=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=136203353205291=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=136498598329081=2 Thanks, Venu -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] usb: host: tegra: Reset Tegra USB controller before init
-Original Message- From: Stephen Warren [mailto:swar...@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-kernel@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- tegram=136361016003625w=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- usbm=136203353205291w=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-tegram=136498598329081w=2 Thanks, Venu -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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 > > 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=136361016003625=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=136203353205291=2 Venu, you should simply repost the patch; I'm sure its not in anyone's email inbox any more. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] usb: host: tegra: Reset Tegra USB controller before init
> -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-kernel@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 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=136361016003625=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=136203353205291=2 Thanks, Venu -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] usb: host: tegra: Reset Tegra USB controller before init
-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-kernel@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-tegram=136361016003625w=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-usbm=136203353205291w=2 Thanks, Venu -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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-tegram=136361016003625w=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-usbm=136203353205291w=2 Venu, you should simply repost the patch; I'm sure its not in anyone's email inbox any more. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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); > >> > >> 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? > > > > 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. > > 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. Acked-by: Greg Kroah-Hartman -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] usb: host: tegra: Reset Tegra USB controller before init
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 Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] usb: host: tegra: Reset Tegra USB controller before init
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); >> >> 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? > > 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. 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. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] usb: host: tegra: Reset Tegra USB controller before init
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); 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? 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. 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. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] usb: host: tegra: Reset Tegra USB controller before init
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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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); 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? 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. 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. Acked-by: Greg Kroah-Hartman gre...@linuxfoundation.org -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] usb: host: tegra: Reset Tegra USB controller before init
> -Original Message- > From: Stephen Warren [mailto:swar...@wwwdotorg.org] > Sent: Thursday, February 28, 2013 11:47 PM > To: Venu Byravarasu > Cc: gre...@linuxfoundation.org; st...@rowland.harvard.edu; linux- > u...@vger.kernel.org; linux-kernel@vger.kernel.org > Subject: Re: [PATCH] usb: host: tegra: Reset Tegra USB controller before init > > 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? 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. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] usb: host: tegra: Reset Tegra USB controller before init
-Original Message- From: Stephen Warren [mailto:swar...@wwwdotorg.org] Sent: Thursday, February 28, 2013 11:47 PM To: Venu Byravarasu Cc: gre...@linuxfoundation.org; st...@rowland.harvard.edu; linux- u...@vger.kernel.org; linux-kernel@vger.kernel.org Subject: Re: [PATCH] usb: host: tegra: Reset Tegra USB controller before init 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? 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. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] usb: host: tegra: Reset Tegra USB controller before init
On 02/28/2013 11:52 AM, Alan Stern wrote: > 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. 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. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] usb: host: tegra: Reset Tegra USB controller before init
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] usb: host: tegra: Reset Tegra USB controller before init
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. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] usb: host: tegra: Reset Tegra USB controller before init
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 >> --- >> 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=136110175423601=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. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] usb: host: tegra: Reset Tegra USB controller before init
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 > --- > 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=136110175423601=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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] usb: host: tegra: Reset Tegra USB controller before init
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 vbyravar...@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-tegram=136110175423601w=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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] usb: host: tegra: Reset Tegra USB controller before init
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 vbyravar...@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-tegram=136110175423601w=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. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] usb: host: tegra: Reset Tegra USB controller before init
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. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] usb: host: tegra: Reset Tegra USB controller before init
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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] usb: host: tegra: Reset Tegra USB controller before init
On 02/28/2013 11:52 AM, Alan Stern wrote: 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. 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. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
