* [RFT PATCH 1/2] usb: dwc2: Add a 10 ms delay to dwc2_core_reset()
@ 2016-03-04 18:23 Douglas Anderson
2016-03-04 18:23 ` [RFT PATCH 2/2] Revert "usb: dwc2: Fix probe problem on bcm2835" Douglas Anderson
` (4 more replies)
0 siblings, 5 replies; 32+ messages in thread
From: Douglas Anderson @ 2016-03-04 18:23 UTC (permalink / raw)
To: johnyoun, balbi, Heiko Stuebner
Cc: linux, caesar.upstream, huangtao, repk, stefan.wahren,
Julius Werner, Douglas Anderson, gregkh, linux-usb, linux-kernel
>From testing and trying to make sense of the documentation, it appears
that a 10 ms delay is needed after resetting the core to make sure that
everything is stable and consistent. Let's add it.
In my testing (on rk3288) this allows us to revert commit
192cb07f7928 ("usb: dwc2: Fix probe problem on bcm2835"). Though I
could never reproduce the problems on my board, this might also allow us
to revert commit bd84f4ae9986 ("usb: dwc2: Add extra delay when forcing
dr_mode").
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
drivers/usb/dwc2/core.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c
index 5e5a0f135b5a..8710b2d3e770 100644
--- a/drivers/usb/dwc2/core.c
+++ b/drivers/usb/dwc2/core.c
@@ -277,6 +277,26 @@ int dwc2_core_reset(struct dwc2_hsotg *hsotg)
}
} while (!(greset & GRSTCTL_AHBIDLE));
+ /*
+ * Sleep for 10-15 ms after the reset to let it finish.
+ *
+ * It's been confirmed on at least one version of the controller
+ * that this is a requirement that this is a requirement in order for
+ * everything to settle. Specifically if you:
+ * - change GNPTXFSIZ or HPTXFSIZ before the reset
+ * - do the reset
+ * - read GNPTXFSIZ or HPTXFSIZ in a loop
+ * ...you'll find that it takes almost exactly 10 ms for the registers
+ * to return to their reset defaults.
+ *
+ * Note that it's possible that this 10 ms is the time referred to
+ * in "Host Initialization" where it says to "Wait at least 10 ms for
+ * the reset process to complete". In "Device Initialization" there
+ * is also talk of a reset lasting 10 ms. That may be the source of
+ * this delay.
+ */
+ usleep_range(10000, 15000);
+
return 0;
}
--
2.7.0.rc3.207.g0ac5344
^ permalink raw reply related [flat|nested] 32+ messages in thread* [RFT PATCH 2/2] Revert "usb: dwc2: Fix probe problem on bcm2835" 2016-03-04 18:23 [RFT PATCH 1/2] usb: dwc2: Add a 10 ms delay to dwc2_core_reset() Douglas Anderson @ 2016-03-04 18:23 ` Douglas Anderson 2016-03-04 22:54 ` Michael Niewoehner 2016-03-07 18:40 ` Stefan Wahren 2016-03-04 22:54 ` [RFT PATCH 1/2] usb: dwc2: Add a 10 ms delay to dwc2_core_reset() Michael Niewoehner ` (3 subsequent siblings) 4 siblings, 2 replies; 32+ messages in thread From: Douglas Anderson @ 2016-03-04 18:23 UTC (permalink / raw) To: johnyoun, balbi, Heiko Stuebner Cc: linux, caesar.upstream, huangtao, repk, stefan.wahren, Julius Werner, Douglas Anderson, gregkh, linux-usb, linux-kernel This reverts commit 192cb07f7928 ("usb: dwc2: Fix probe problem on bcm2835") now that we've found the root cause. See the change titled ("usb: dwc2: Add a 10 ms delay to dwc2_core_reset()"). Signed-off-by: Douglas Anderson <dianders@chromium.org> --- drivers/usb/dwc2/core.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c index 8710b2d3e770..7c4a6cf4c73a 100644 --- a/drivers/usb/dwc2/core.c +++ b/drivers/usb/dwc2/core.c @@ -353,6 +353,12 @@ static bool dwc2_force_mode(struct dwc2_hsotg *hsotg, bool host) set = host ? GUSBCFG_FORCEHOSTMODE : GUSBCFG_FORCEDEVMODE; clear = host ? GUSBCFG_FORCEDEVMODE : GUSBCFG_FORCEHOSTMODE; + /* + * If the force mode bit is already set, don't set it. + */ + if ((gusbcfg & set) && !(gusbcfg & clear)) + return false; + gusbcfg &= ~clear; gusbcfg |= set; dwc2_writel(gusbcfg, hsotg->regs + GUSBCFG); -- 2.7.0.rc3.207.g0ac5344 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [RFT PATCH 2/2] Revert "usb: dwc2: Fix probe problem on bcm2835" 2016-03-04 18:23 ` [RFT PATCH 2/2] Revert "usb: dwc2: Fix probe problem on bcm2835" Douglas Anderson @ 2016-03-04 22:54 ` Michael Niewoehner 2016-03-07 18:40 ` Stefan Wahren 1 sibling, 0 replies; 32+ messages in thread From: Michael Niewoehner @ 2016-03-04 22:54 UTC (permalink / raw) To: Douglas Anderson Cc: johnyoun, balbi, Heiko Stuebner, caesar.upstream, huangtao, repk, stefan.wahren, Julius Werner, gregkh, linux-usb, linux-kernel Am 04.03.2016 um 19:23 schrieb Douglas Anderson <dianders@chromium.org>: > This reverts commit 192cb07f7928 ("usb: dwc2: Fix probe problem on > bcm2835") now that we've found the root cause. See the change > titled ("usb: dwc2: Add a 10 ms delay to dwc2_core_reset()"). > > Signed-off-by: Douglas Anderson <dianders@chromium.org> Tested-by: Michael Niewoehner <linux@mniewoehner.de> > --- > drivers/usb/dwc2/core.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c > index 8710b2d3e770..7c4a6cf4c73a 100644 > --- a/drivers/usb/dwc2/core.c > +++ b/drivers/usb/dwc2/core.c > @@ -353,6 +353,12 @@ static bool dwc2_force_mode(struct dwc2_hsotg *hsotg, bool host) > set = host ? GUSBCFG_FORCEHOSTMODE : GUSBCFG_FORCEDEVMODE; > clear = host ? GUSBCFG_FORCEDEVMODE : GUSBCFG_FORCEHOSTMODE; > > + /* > + * If the force mode bit is already set, don't set it. > + */ > + if ((gusbcfg & set) && !(gusbcfg & clear)) > + return false; > + > gusbcfg &= ~clear; > gusbcfg |= set; > dwc2_writel(gusbcfg, hsotg->regs + GUSBCFG); > -- > 2.7.0.rc3.207.g0ac5344 > ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFT PATCH 2/2] Revert "usb: dwc2: Fix probe problem on bcm2835" 2016-03-04 18:23 ` [RFT PATCH 2/2] Revert "usb: dwc2: Fix probe problem on bcm2835" Douglas Anderson 2016-03-04 22:54 ` Michael Niewoehner @ 2016-03-07 18:40 ` Stefan Wahren 2016-03-07 21:30 ` Doug Anderson 1 sibling, 1 reply; 32+ messages in thread From: Stefan Wahren @ 2016-03-07 18:40 UTC (permalink / raw) To: johnyoun, Douglas Anderson, Heiko Stuebner, balbi Cc: linux, huangtao, Julius Werner, gregkh, linux-kernel, linux-usb, caesar.upstream, repk Hi Doug, > Douglas Anderson <dianders@chromium.org> hat am 4. März 2016 um 19:23 > geschrieben: > > > This reverts commit 192cb07f7928 ("usb: dwc2: Fix probe problem on > bcm2835") now that we've found the root cause. See the change > titled ("usb: dwc2: Add a 10 ms delay to dwc2_core_reset()"). adding a delay of 10 ms after a core reset might be a idea, but applying both patches breaks USB support on RPi :-( I'm getting the wrong register values ... Stefan ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFT PATCH 2/2] Revert "usb: dwc2: Fix probe problem on bcm2835" 2016-03-07 18:40 ` Stefan Wahren @ 2016-03-07 21:30 ` Doug Anderson 2016-03-08 17:49 ` Stefan Wahren 2016-03-09 19:01 ` Stefan Wahren 0 siblings, 2 replies; 32+ messages in thread From: Doug Anderson @ 2016-03-07 21:30 UTC (permalink / raw) To: Stefan Wahren Cc: John Youn, Heiko Stuebner, Felipe Balbi, Michael Niewoehner, Tao Huang, Julius Werner, Greg Kroah-Hartman, linux-kernel, linux-usb, Caesar Wang, Remi Pommarel Stefan, On Mon, Mar 7, 2016 at 10:40 AM, Stefan Wahren <stefan.wahren@i2se.com> wrote: > Hi Doug, > >> Douglas Anderson <dianders@chromium.org> hat am 4. März 2016 um 19:23 >> geschrieben: >> >> >> This reverts commit 192cb07f7928 ("usb: dwc2: Fix probe problem on >> bcm2835") now that we've found the root cause. See the change >> titled ("usb: dwc2: Add a 10 ms delay to dwc2_core_reset()"). > > adding a delay of 10 ms after a core reset might be a idea, but applying both > patches breaks USB support on RPi :-( > > I'm getting the wrong register values ... Ugh. :( Just out of curiosity, if you loop and time long it takes for the registers to get to the right state after reset, what do you get? AKA, pick: https://chromium-review.googlesource.com/331260 ...and let me know what it prints out. On my system I see: [ 1.990743] dwc2 ff540000.usb: Waited 300001 us, 0x04000400 => 0x04000400, 0x02000800 => 0x02000800 [ 2.119677] dwc2 ff580000.usb: Waited 9997 us, 0x00100400 => 0x04000400, 0x00000000 => 0x02000800 I believe the difference in behavior is because of the two different types of USB controllers (one is OTG and the other is host only). -Doug ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFT PATCH 2/2] Revert "usb: dwc2: Fix probe problem on bcm2835" 2016-03-07 21:30 ` Doug Anderson @ 2016-03-08 17:49 ` Stefan Wahren 2016-03-09 19:01 ` Stefan Wahren 1 sibling, 0 replies; 32+ messages in thread From: Stefan Wahren @ 2016-03-08 17:49 UTC (permalink / raw) To: Doug Anderson Cc: Michael Niewoehner, Tao Huang, Julius Werner, Greg Kroah-Hartman, linux-kernel, linux-usb, John Youn, Caesar Wang, Heiko Stuebner, Felipe Balbi, Remi Pommarel Hi Doug, > Doug Anderson <dianders@chromium.org> hat am 7. März 2016 um 22:30 > geschrieben: > > > Stefan, > > On Mon, Mar 7, 2016 at 10:40 AM, Stefan Wahren <stefan.wahren@i2se.com> wrote: > > Hi Doug, > > > >> Douglas Anderson <dianders@chromium.org> hat am 4. März 2016 um 19:23 > >> geschrieben: > >> > >> > >> This reverts commit 192cb07f7928 ("usb: dwc2: Fix probe problem on > >> bcm2835") now that we've found the root cause. See the change > >> titled ("usb: dwc2: Add a 10 ms delay to dwc2_core_reset()"). > > > > adding a delay of 10 ms after a core reset might be a idea, but applying > > both > > patches breaks USB support on RPi :-( > > > > I'm getting the wrong register values ... > > Ugh. :( > > Just out of curiosity, if you loop and time long it takes for the > registers to get to the right state after reset, what do you get? > AKA, pick: > > https://chromium-review.googlesource.com/331260 > > ...and let me know what it prints out. On my system I see: > > [ 1.990743] dwc2 ff540000.usb: Waited 300001 us, 0x04000400 => > 0x04000400, 0x02000800 => 0x02000800 > [ 2.119677] dwc2 ff580000.usb: Waited 9997 us, 0x00100400 => > 0x04000400, 0x00000000 => 0x02000800 sure, but this will take some time (weekend). > > I believe the difference in behavior is because of the two different > types of USB controllers (one is OTG and the other is host only). > > > -Doug ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFT PATCH 2/2] Revert "usb: dwc2: Fix probe problem on bcm2835" 2016-03-07 21:30 ` Doug Anderson 2016-03-08 17:49 ` Stefan Wahren @ 2016-03-09 19:01 ` Stefan Wahren 2016-03-09 19:06 ` Doug Anderson 1 sibling, 1 reply; 32+ messages in thread From: Stefan Wahren @ 2016-03-09 19:01 UTC (permalink / raw) To: Doug Anderson Cc: Michael Niewoehner, Tao Huang, Julius Werner, Greg Kroah-Hartman, linux-kernel, linux-usb, John Youn, Caesar Wang, Heiko Stuebner, Felipe Balbi, Remi Pommarel > Doug Anderson <dianders@chromium.org> hat am 7. März 2016 um 22:30 > geschrieben: > > > Stefan, > > On Mon, Mar 7, 2016 at 10:40 AM, Stefan Wahren <stefan.wahren@i2se.com> wrote: > > Hi Doug, > > > >> Douglas Anderson <dianders@chromium.org> hat am 4. März 2016 um 19:23 > >> geschrieben: > >> > >> > >> This reverts commit 192cb07f7928 ("usb: dwc2: Fix probe problem on > >> bcm2835") now that we've found the root cause. See the change > >> titled ("usb: dwc2: Add a 10 ms delay to dwc2_core_reset()"). > > > > adding a delay of 10 ms after a core reset might be a idea, but applying > > both > > patches breaks USB support on RPi :-( > > > > I'm getting the wrong register values ... > > Ugh. :( > > Just out of curiosity, if you loop and time long it takes for the > registers to get to the right state after reset, what do you get? > AKA, pick: > > https://chromium-review.googlesource.com/331260 > > ...and let me know what it prints out. On my Raspberry Pi B i get the following: [ 2.084411] dwc2 20980000.usb: mapped PA 20980000 to VA cc880000 [ 2.084461] dwc2 20980000.usb: cannot get otg clock [ 2.084549] dwc2 20980000.usb: registering common handler for irq33 [ 2.084713] dwc2 20980000.usb: Configuration mismatch. dr_mode forced to host [ 2.153965] dwc2 20980000.usb: Waited 49996 us, 0x00201000 => 0x01001000, 0x00000000 => 0x02002000 [ 2.174930] dwc2 20980000.usb: Forcing mode to host So i changed the delay in patch #1 to msleep(50) and then both patches work like a charm. Stefan ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFT PATCH 2/2] Revert "usb: dwc2: Fix probe problem on bcm2835" 2016-03-09 19:01 ` Stefan Wahren @ 2016-03-09 19:06 ` Doug Anderson 2016-03-10 19:14 ` John Youn 0 siblings, 1 reply; 32+ messages in thread From: Doug Anderson @ 2016-03-09 19:06 UTC (permalink / raw) To: Stefan Wahren Cc: Michael Niewoehner, Tao Huang, Julius Werner, Greg Kroah-Hartman, linux-kernel, linux-usb, John Youn, Caesar Wang, Heiko Stuebner, Felipe Balbi, Remi Pommarel Stefan, On Wed, Mar 9, 2016 at 11:01 AM, Stefan Wahren <stefan.wahren@i2se.com> wrote: > >> Doug Anderson <dianders@chromium.org> hat am 7. März 2016 um 22:30 >> geschrieben: >> >> >> Stefan, >> >> On Mon, Mar 7, 2016 at 10:40 AM, Stefan Wahren <stefan.wahren@i2se.com> wrote: >> > Hi Doug, >> > >> >> Douglas Anderson <dianders@chromium.org> hat am 4. März 2016 um 19:23 >> >> geschrieben: >> >> >> >> >> >> This reverts commit 192cb07f7928 ("usb: dwc2: Fix probe problem on >> >> bcm2835") now that we've found the root cause. See the change >> >> titled ("usb: dwc2: Add a 10 ms delay to dwc2_core_reset()"). >> > >> > adding a delay of 10 ms after a core reset might be a idea, but applying >> > both >> > patches breaks USB support on RPi :-( >> > >> > I'm getting the wrong register values ... >> >> Ugh. :( >> >> Just out of curiosity, if you loop and time long it takes for the >> registers to get to the right state after reset, what do you get? >> AKA, pick: >> >> https://chromium-review.googlesource.com/331260 >> >> ...and let me know what it prints out. > > On my Raspberry Pi B i get the following: > > [ 2.084411] dwc2 20980000.usb: mapped PA 20980000 to VA cc880000 > [ 2.084461] dwc2 20980000.usb: cannot get otg clock > [ 2.084549] dwc2 20980000.usb: registering common handler for irq33 > [ 2.084713] dwc2 20980000.usb: Configuration mismatch. dr_mode forced to host > [ 2.153965] dwc2 20980000.usb: Waited 49996 us, 0x00201000 => 0x01001000, > 0x00000000 => 0x02002000 > [ 2.174930] dwc2 20980000.usb: Forcing mode to host > > So i changed the delay in patch #1 to msleep(50) and then both patches work like > a charm. Great news! :-) John: it's pretty clear that there's something taking almost exactly 10ms on my system and almost exactly 50ms on Stefan's system. Is there some register we could poll to see when this process is done? ...or can we look at the dwc2 revision number / feature register and detect how long to delay? ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFT PATCH 2/2] Revert "usb: dwc2: Fix probe problem on bcm2835" 2016-03-09 19:06 ` Doug Anderson @ 2016-03-10 19:14 ` John Youn 2016-03-16 18:28 ` John Youn 0 siblings, 1 reply; 32+ messages in thread From: John Youn @ 2016-03-10 19:14 UTC (permalink / raw) To: Doug Anderson, Stefan Wahren Cc: Michael Niewoehner, Tao Huang, Julius Werner, Greg Kroah-Hartman, linux-kernel, linux-usb, John Youn, Caesar Wang, Heiko Stuebner, Felipe Balbi, Remi Pommarel On 3/9/2016 11:06 AM, Doug Anderson wrote: > Stefan, > > On Wed, Mar 9, 2016 at 11:01 AM, Stefan Wahren <stefan.wahren@i2se.com> wrote: >> >>> Doug Anderson <dianders@chromium.org> hat am 7. März 2016 um 22:30 >>> geschrieben: >>> >>> >>> Stefan, >>> >>> On Mon, Mar 7, 2016 at 10:40 AM, Stefan Wahren <stefan.wahren@i2se.com> wrote: >>>> Hi Doug, >>>> >>>>> Douglas Anderson <dianders@chromium.org> hat am 4. März 2016 um 19:23 >>>>> geschrieben: >>>>> >>>>> >>>>> This reverts commit 192cb07f7928 ("usb: dwc2: Fix probe problem on >>>>> bcm2835") now that we've found the root cause. See the change >>>>> titled ("usb: dwc2: Add a 10 ms delay to dwc2_core_reset()"). >>>> >>>> adding a delay of 10 ms after a core reset might be a idea, but applying >>>> both >>>> patches breaks USB support on RPi :-( >>>> >>>> I'm getting the wrong register values ... >>> >>> Ugh. :( >>> >>> Just out of curiosity, if you loop and time long it takes for the >>> registers to get to the right state after reset, what do you get? >>> AKA, pick: >>> >>> https://chromium-review.googlesource.com/331260 >>> >>> ...and let me know what it prints out. >> >> On my Raspberry Pi B i get the following: >> >> [ 2.084411] dwc2 20980000.usb: mapped PA 20980000 to VA cc880000 >> [ 2.084461] dwc2 20980000.usb: cannot get otg clock >> [ 2.084549] dwc2 20980000.usb: registering common handler for irq33 >> [ 2.084713] dwc2 20980000.usb: Configuration mismatch. dr_mode forced to host >> [ 2.153965] dwc2 20980000.usb: Waited 49996 us, 0x00201000 => 0x01001000, >> 0x00000000 => 0x02002000 >> [ 2.174930] dwc2 20980000.usb: Forcing mode to host >> >> So i changed the delay in patch #1 to msleep(50) and then both patches work like >> a charm. > > Great news! :-) > > John: it's pretty clear that there's something taking almost exactly > 10ms on my system and almost exactly 50ms on Stefan's system. Is > there some register we could poll to see when this process is done? > ...or can we look at the dwc2 revision number / feature register and > detect how long to delay? > Hi Doug, I'll have to ask around to see if anyone knows about this. And I'll run some tests on the platforms I have available to me as well. Regards, John ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFT PATCH 2/2] Revert "usb: dwc2: Fix probe problem on bcm2835" 2016-03-10 19:14 ` John Youn @ 2016-03-16 18:28 ` John Youn 2016-03-18 23:14 ` Stefan Wahren 2016-03-19 5:21 ` Doug Anderson 0 siblings, 2 replies; 32+ messages in thread From: John Youn @ 2016-03-16 18:28 UTC (permalink / raw) To: Doug Anderson, Stefan Wahren Cc: Michael Niewoehner, Tao Huang, Julius Werner, Greg Kroah-Hartman, linux-kernel, linux-usb, John Youn, Caesar Wang, Heiko Stuebner, Felipe Balbi, Remi Pommarel On 3/10/2016 11:14 AM, John Youn wrote: > On 3/9/2016 11:06 AM, Doug Anderson wrote: >> Stefan, >> >> On Wed, Mar 9, 2016 at 11:01 AM, Stefan Wahren <stefan.wahren@i2se.com> wrote: >>> >>>> Doug Anderson <dianders@chromium.org> hat am 7. März 2016 um 22:30 >>>> geschrieben: >>>> >>>> >>>> Stefan, >>>> >>>> On Mon, Mar 7, 2016 at 10:40 AM, Stefan Wahren <stefan.wahren@i2se.com> wrote: >>>>> Hi Doug, >>>>> >>>>>> Douglas Anderson <dianders@chromium.org> hat am 4. März 2016 um 19:23 >>>>>> geschrieben: >>>>>> >>>>>> >>>>>> This reverts commit 192cb07f7928 ("usb: dwc2: Fix probe problem on >>>>>> bcm2835") now that we've found the root cause. See the change >>>>>> titled ("usb: dwc2: Add a 10 ms delay to dwc2_core_reset()"). >>>>> >>>>> adding a delay of 10 ms after a core reset might be a idea, but applying >>>>> both >>>>> patches breaks USB support on RPi :-( >>>>> >>>>> I'm getting the wrong register values ... >>>> >>>> Ugh. :( >>>> >>>> Just out of curiosity, if you loop and time long it takes for the >>>> registers to get to the right state after reset, what do you get? >>>> AKA, pick: >>>> >>>> https://chromium-review.googlesource.com/331260 >>>> >>>> ...and let me know what it prints out. >>> >>> On my Raspberry Pi B i get the following: >>> >>> [ 2.084411] dwc2 20980000.usb: mapped PA 20980000 to VA cc880000 >>> [ 2.084461] dwc2 20980000.usb: cannot get otg clock >>> [ 2.084549] dwc2 20980000.usb: registering common handler for irq33 >>> [ 2.084713] dwc2 20980000.usb: Configuration mismatch. dr_mode forced to host >>> [ 2.153965] dwc2 20980000.usb: Waited 49996 us, 0x00201000 => 0x01001000, >>> 0x00000000 => 0x02002000 >>> [ 2.174930] dwc2 20980000.usb: Forcing mode to host >>> >>> So i changed the delay in patch #1 to msleep(50) and then both patches work like >>> a charm. >> >> Great news! :-) >> >> John: it's pretty clear that there's something taking almost exactly >> 10ms on my system and almost exactly 50ms on Stefan's system. Is >> there some register we could poll to see when this process is done? >> ...or can we look at the dwc2 revision number / feature register and >> detect how long to delay? >> > > Hi Doug, > > I'll have to ask around to see if anyone knows about this. And I'll > run some tests on the platforms I have available to me as well. > There's still nothing definitive on our end as to why this is happening. Also I don't think there is any other way to poll the reset. Our hardware engineers asked for some more information to look into it further. Doug, Stefan, Caesar, and anyone else with a related platform, do you know the answers to the following: 1. What is the AHB Clock frequency? Is the AHB Clock gated during Reset? 2. Also is the PHY clock stopped during the reset or is the PHY PLL lock times high in the order of ms? 3. In these cases, is the PHY actually an FS Transceiver and not a UTMI/ULPI PHY? 4. Which version of the controller is being used in these cases? Regards, John ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFT PATCH 2/2] Revert "usb: dwc2: Fix probe problem on bcm2835" 2016-03-16 18:28 ` John Youn @ 2016-03-18 23:14 ` Stefan Wahren 2016-03-19 2:17 ` Eric Anholt 2016-03-19 5:21 ` Doug Anderson 1 sibling, 1 reply; 32+ messages in thread From: Stefan Wahren @ 2016-03-18 23:14 UTC (permalink / raw) To: John Youn, Doug Anderson, Eric Anholt, Martin Sperl Cc: Michael Niewoehner, Tao Huang, Julius Werner, Greg Kroah-Hartman, linux-kernel, linux-usb, Caesar Wang, Heiko Stuebner, Felipe Balbi, Remi Pommarel Hi Eric, hi Martin, > John Youn <John.Youn@synopsys.com> hat am 16. März 2016 um 19:28 geschrieben: > > > On 3/10/2016 11:14 AM, John Youn wrote: > > On 3/9/2016 11:06 AM, Doug Anderson wrote: > >> Stefan, > >> > >> On Wed, Mar 9, 2016 at 11:01 AM, Stefan Wahren <stefan.wahren@i2se.com> > >> wrote: > >>> > >>>> Doug Anderson <dianders@chromium.org> hat am 7. März 2016 um 22:30 > >>>> geschrieben: > >>>> > >>>> > >>>> Stefan, > >>>> > >>>> On Mon, Mar 7, 2016 at 10:40 AM, Stefan Wahren <stefan.wahren@i2se.com> > >>>> wrote: > >>>>> Hi Doug, > >>>>> > >>>>>> Douglas Anderson <dianders@chromium.org> hat am 4. März 2016 um 19:23 > >>>>>> geschrieben: > >>>>>> > >>>>>> > >>>>>> This reverts commit 192cb07f7928 ("usb: dwc2: Fix probe problem on > >>>>>> bcm2835") now that we've found the root cause. See the change > >>>>>> titled ("usb: dwc2: Add a 10 ms delay to dwc2_core_reset()"). > >>>>> > >>>>> adding a delay of 10 ms after a core reset might be a idea, but applying > >>>>> both > >>>>> patches breaks USB support on RPi :-( > >>>>> > >>>>> I'm getting the wrong register values ... > >>>> > >>>> Ugh. :( > >>>> > >>>> Just out of curiosity, if you loop and time long it takes for the > >>>> registers to get to the right state after reset, what do you get? > >>>> AKA, pick: > >>>> > >>>> https://chromium-review.googlesource.com/331260 > >>>> > >>>> ...and let me know what it prints out. > >>> > >>> On my Raspberry Pi B i get the following: > >>> > >>> [ 2.084411] dwc2 20980000.usb: mapped PA 20980000 to VA cc880000 > >>> [ 2.084461] dwc2 20980000.usb: cannot get otg clock > >>> [ 2.084549] dwc2 20980000.usb: registering common handler for irq33 > >>> [ 2.084713] dwc2 20980000.usb: Configuration mismatch. dr_mode forced to > >>> host > >>> [ 2.153965] dwc2 20980000.usb: Waited 49996 us, 0x00201000 => 0x01001000, > >>> 0x00000000 => 0x02002000 > >>> [ 2.174930] dwc2 20980000.usb: Forcing mode to host > >>> > >>> So i changed the delay in patch #1 to msleep(50) and then both patches > >>> work like > >>> a charm. > >> > >> Great news! :-) > >> > >> John: it's pretty clear that there's something taking almost exactly > >> 10ms on my system and almost exactly 50ms on Stefan's system. Is > >> there some register we could poll to see when this process is done? > >> ...or can we look at the dwc2 revision number / feature register and > >> detect how long to delay? > >> > > > > Hi Doug, > > > > I'll have to ask around to see if anyone knows about this. And I'll > > run some tests on the platforms I have available to me as well. > > > > There's still nothing definitive on our end as to why this is > happening. Also I don't think there is any other way to poll the > reset. Our hardware engineers asked for some more information to look > into it further. Doug, Stefan, Caesar, and anyone else with a related > platform, do you know the answers to the following: > > 1. What is the AHB Clock frequency? Is the AHB Clock gated during > Reset? > > 2. Also is the PHY clock stopped during the reset or is the PHY PLL > lock times high in the order of ms? > > 3. In these cases, is the PHY actually an FS Transceiver and not a > UTMI/ULPI PHY? are you able to answer these questions 1 - 3 for BCM2835? I don't want to say something wrong here. > > 4. Which version of the controller is being used in these cases? @John: The BCM2835 has version 2.80a Regards Stefan > > Regards, > John ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFT PATCH 2/2] Revert "usb: dwc2: Fix probe problem on bcm2835" 2016-03-18 23:14 ` Stefan Wahren @ 2016-03-19 2:17 ` Eric Anholt 2016-03-19 7:44 ` Martin Sperl 2016-03-19 9:45 ` Stefan Wahren 0 siblings, 2 replies; 32+ messages in thread From: Eric Anholt @ 2016-03-19 2:17 UTC (permalink / raw) To: Stefan Wahren, John Youn, Doug Anderson, Martin Sperl Cc: Michael Niewoehner, Tao Huang, Julius Werner, Greg Kroah-Hartman, linux-kernel, linux-usb, Caesar Wang, Heiko Stuebner, Felipe Balbi, Remi Pommarel [-- Attachment #1: Type: text/plain, Size: 4087 bytes --] Stefan Wahren <stefan.wahren@i2se.com> writes: > Hi Eric, > hi Martin, > >> John Youn <John.Youn@synopsys.com> hat am 16. März 2016 um 19:28 geschrieben: >> >> >> On 3/10/2016 11:14 AM, John Youn wrote: >> > On 3/9/2016 11:06 AM, Doug Anderson wrote: >> >> Stefan, >> >> >> >> On Wed, Mar 9, 2016 at 11:01 AM, Stefan Wahren <stefan.wahren@i2se.com> >> >> wrote: >> >>> >> >>>> Doug Anderson <dianders@chromium.org> hat am 7. März 2016 um 22:30 >> >>>> geschrieben: >> >>>> >> >>>> >> >>>> Stefan, >> >>>> >> >>>> On Mon, Mar 7, 2016 at 10:40 AM, Stefan Wahren <stefan.wahren@i2se.com> >> >>>> wrote: >> >>>>> Hi Doug, >> >>>>> >> >>>>>> Douglas Anderson <dianders@chromium.org> hat am 4. März 2016 um 19:23 >> >>>>>> geschrieben: >> >>>>>> >> >>>>>> >> >>>>>> This reverts commit 192cb07f7928 ("usb: dwc2: Fix probe problem on >> >>>>>> bcm2835") now that we've found the root cause. See the change >> >>>>>> titled ("usb: dwc2: Add a 10 ms delay to dwc2_core_reset()"). >> >>>>> >> >>>>> adding a delay of 10 ms after a core reset might be a idea, but applying >> >>>>> both >> >>>>> patches breaks USB support on RPi :-( >> >>>>> >> >>>>> I'm getting the wrong register values ... >> >>>> >> >>>> Ugh. :( >> >>>> >> >>>> Just out of curiosity, if you loop and time long it takes for the >> >>>> registers to get to the right state after reset, what do you get? >> >>>> AKA, pick: >> >>>> >> >>>> https://chromium-review.googlesource.com/331260 >> >>>> >> >>>> ...and let me know what it prints out. >> >>> >> >>> On my Raspberry Pi B i get the following: >> >>> >> >>> [ 2.084411] dwc2 20980000.usb: mapped PA 20980000 to VA cc880000 >> >>> [ 2.084461] dwc2 20980000.usb: cannot get otg clock >> >>> [ 2.084549] dwc2 20980000.usb: registering common handler for irq33 >> >>> [ 2.084713] dwc2 20980000.usb: Configuration mismatch. dr_mode forced to >> >>> host >> >>> [ 2.153965] dwc2 20980000.usb: Waited 49996 us, 0x00201000 => 0x01001000, >> >>> 0x00000000 => 0x02002000 >> >>> [ 2.174930] dwc2 20980000.usb: Forcing mode to host >> >>> >> >>> So i changed the delay in patch #1 to msleep(50) and then both patches >> >>> work like >> >>> a charm. >> >> >> >> Great news! :-) >> >> >> >> John: it's pretty clear that there's something taking almost exactly >> >> 10ms on my system and almost exactly 50ms on Stefan's system. Is >> >> there some register we could poll to see when this process is done? >> >> ...or can we look at the dwc2 revision number / feature register and >> >> detect how long to delay? >> >> >> > >> > Hi Doug, >> > >> > I'll have to ask around to see if anyone knows about this. And I'll >> > run some tests on the platforms I have available to me as well. >> > >> >> There's still nothing definitive on our end as to why this is >> happening. Also I don't think there is any other way to poll the >> reset. Our hardware engineers asked for some more information to look >> into it further. Doug, Stefan, Caesar, and anyone else with a related >> platform, do you know the answers to the following: >> >> 1. What is the AHB Clock frequency? Is the AHB Clock gated during >> Reset? Low confidence here as I'm tracing lines across a ton of modules, but it looks like it comes from the USB AXI clock in peri_image, which is a gate on the normal 250Mhz APB clock, but nothing should be touching that gate register as part of USB reset as far as I know. >> 2. Also is the PHY clock stopped during the reset or is the PHY PLL >> lock times high in the order of ms? PHY PLL lock times should be about 40 us, and reset needs to be high for 40us. I haven't traced where GRSTCTL_CSFTRST (the reset I assume you're talking about here) goes, so I can't say if it's an input to PHY reset. >> 3. In these cases, is the PHY actually an FS Transceiver and not a >> UTMI/ULPI PHY? The PHY docs say it's UTMI+ compatible. >> 4. Which version of the controller is being used in these cases? > > @John: The BCM2835 has version 2.80a Yeah, that looks right. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 818 bytes --] ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFT PATCH 2/2] Revert "usb: dwc2: Fix probe problem on bcm2835" 2016-03-19 2:17 ` Eric Anholt @ 2016-03-19 7:44 ` Martin Sperl 2016-03-19 9:52 ` Stefan Wahren 2016-03-19 9:45 ` Stefan Wahren 1 sibling, 1 reply; 32+ messages in thread From: Martin Sperl @ 2016-03-19 7:44 UTC (permalink / raw) To: Eric Anholt Cc: Stefan Wahren, John Youn, Doug Anderson, Michael Niewoehner, Tao Huang, Julius Werner, Greg Kroah-Hartman, linux-kernel, linux-usb, Caesar Wang, Heiko Stuebner, Felipe Balbi, Remi Pommarel > On 19.03.2016, at 03:17, Eric Anholt <eric@anholt.net> wrote: > > Stefan Wahren <stefan.wahren@i2se.com> writes: > >> Hi Eric, >> hi Martin, >> >>> John Youn <John.Youn@synopsys.com> hat am 16. März 2016 um 19:28 geschrieben: >>> >>> >>> On 3/10/2016 11:14 AM, John Youn wrote: >>>> On 3/9/2016 11:06 AM, Doug Anderson wrote: >>>>> >>>>> John: it's pretty clear that there's something taking almost exactly >>>>> 10ms on my system and almost exactly 50ms on Stefan's system. Is >>>>> there some register we could poll to see when this process is done? >>>>> ...or can we look at the dwc2 revision number / feature register and >>>>> detect how long to delay? >>>>> Maybe this difference is related to overclocking settings in the firmware? >>> >>> 1. What is the AHB Clock frequency? Is the AHB Clock gated during >>> Reset? > > Low confidence here as I'm tracing lines across a ton of modules, but it > looks like it comes from the USB AXI clock in peri_image, which is a > gate on the normal 250Mhz APB clock, but nothing should be touching that > gate register as part of USB reset as far as I know. > Isn’t it possible that this clock (probably BCM2835_CLOCK_VPU) is changed by the firmware due to overclocking settings in /boot/config.txt? Martin ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFT PATCH 2/2] Revert "usb: dwc2: Fix probe problem on bcm2835" 2016-03-19 7:44 ` Martin Sperl @ 2016-03-19 9:52 ` Stefan Wahren 2016-03-19 10:10 ` Martin Sperl 0 siblings, 1 reply; 32+ messages in thread From: Stefan Wahren @ 2016-03-19 9:52 UTC (permalink / raw) To: Martin Sperl, Eric Anholt Cc: Michael Niewoehner, Tao Huang, John Youn, Julius Werner, Greg Kroah-Hartman, linux-kernel, linux-usb, Caesar Wang, Doug Anderson, Heiko Stuebner, Felipe Balbi, Remi Pommarel Hi, > Martin Sperl <kernel@martin.sperl.org> hat am 19. März 2016 um 08:44 > geschrieben: > > > > > On 19.03.2016, at 03:17, Eric Anholt <eric@anholt.net> wrote: > > > > Stefan Wahren <stefan.wahren@i2se.com> writes: > > > >> Hi Eric, > >> hi Martin, > >> > >>> John Youn <John.Youn@synopsys.com> hat am 16. März 2016 um 19:28 > >>> geschrieben: > >>> > >>> > >>> On 3/10/2016 11:14 AM, John Youn wrote: > >>>> On 3/9/2016 11:06 AM, Doug Anderson wrote: > >>>>> > >>>>> John: it's pretty clear that there's something taking almost exactly > >>>>> 10ms on my system and almost exactly 50ms on Stefan's system. Is > >>>>> there some register we could poll to see when this process is done? > >>>>> ...or can we look at the dwc2 revision number / feature register and > >>>>> detect how long to delay? > >>>>> > > Maybe this difference is related to overclocking settings in the firmware? > > >>> > >>> 1. What is the AHB Clock frequency? Is the AHB Clock gated during > >>> Reset? > > > > Low confidence here as I'm tracing lines across a ton of modules, but it > > looks like it comes from the USB AXI clock in peri_image, which is a > > gate on the normal 250Mhz APB clock, but nothing should be touching that > > gate register as part of USB reset as far as I know. > > > Isn’t it possible that this clock (probably BCM2835_CLOCK_VPU) is > changed by the firmware due to overclocking settings in /boot/config.txt? i don't use any overclocking settings. Are you able to reproduce the behavior on your Pi? Stefan > > Martin > > ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFT PATCH 2/2] Revert "usb: dwc2: Fix probe problem on bcm2835" 2016-03-19 9:52 ` Stefan Wahren @ 2016-03-19 10:10 ` Martin Sperl 2016-03-19 12:09 ` Stefan Wahren 0 siblings, 1 reply; 32+ messages in thread From: Martin Sperl @ 2016-03-19 10:10 UTC (permalink / raw) To: Stefan Wahren Cc: Eric Anholt, Michael Niewoehner, Tao Huang, John Youn, Julius Werner, Greg Kroah-Hartman, linux-kernel, linux-usb, Caesar Wang, Doug Anderson, Heiko Stuebner, Felipe Balbi, Remi Pommarel > On 19.03.2016, at 10:52, Stefan Wahren <stefan.wahren@i2se.com> wrote: > > Hi, > >> Martin Sperl <kernel@martin.sperl.org> hat am 19. März 2016 um 08:44 >> geschrieben: >> >> >> >>> On 19.03.2016, at 03:17, Eric Anholt <eric@anholt.net> wrote: >>> >>> Stefan Wahren <stefan.wahren@i2se.com> writes: >>> >>>> Hi Eric, >>>> hi Martin, >>>> >>>>> John Youn <John.Youn@synopsys.com> hat am 16. März 2016 um 19:28 >>>>> geschrieben: >>>>> >>>>> >>>>> On 3/10/2016 11:14 AM, John Youn wrote: >>>>>> On 3/9/2016 11:06 AM, Doug Anderson wrote: >>>>>>> >>>>>>> John: it's pretty clear that there's something taking almost exactly >>>>>>> 10ms on my system and almost exactly 50ms on Stefan's system. Is >>>>>>> there some register we could poll to see when this process is done? >>>>>>> ...or can we look at the dwc2 revision number / feature register and >>>>>>> detect how long to delay? >>>>>>> >> >> Maybe this difference is related to overclocking settings in the firmware? >> >>>>> >>>>> 1. What is the AHB Clock frequency? Is the AHB Clock gated during >>>>> Reset? >>> >>> Low confidence here as I'm tracing lines across a ton of modules, but it >>> looks like it comes from the USB AXI clock in peri_image, which is a >>> gate on the normal 250Mhz APB clock, but nothing should be touching that >>> gate register as part of USB reset as far as I know. >>> >> Isn’t it possible that this clock (probably BCM2835_CLOCK_VPU) is >> changed by the firmware due to overclocking settings in /boot/config.txt? > > i don't use any overclocking settings. > > Are you able to reproduce the behavior on your Pi? I did not have any problems with USB recently (using 4.5), so I would not have any idea how to reproduce it. Note that I am using it with an AXIS USB-ethernet dongle plus USB-stick all the time on my Compute Module connected via a tiny hub, so I wonder if that qualifies as being able to trigger the issue... Martin ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFT PATCH 2/2] Revert "usb: dwc2: Fix probe problem on bcm2835" 2016-03-19 10:10 ` Martin Sperl @ 2016-03-19 12:09 ` Stefan Wahren 0 siblings, 0 replies; 32+ messages in thread From: Stefan Wahren @ 2016-03-19 12:09 UTC (permalink / raw) To: Martin Sperl Cc: Michael Niewoehner, Tao Huang, John Youn, Julius Werner, Eric Anholt, Greg Kroah-Hartman, linux-kernel, linux-usb, Caesar Wang, Doug Anderson, Heiko Stuebner, Felipe Balbi, Remi Pommarel Hi, > Martin Sperl <kernel@martin.sperl.org> hat am 19. März 2016 um 11:10 > geschrieben: > > > > On 19.03.2016, at 10:52, Stefan Wahren <stefan.wahren@i2se.com> wrote: > > > > Hi, > > > >> Martin Sperl <kernel@martin.sperl.org> hat am 19. März 2016 um 08:44 > >> geschrieben: > >> > >> > >> > >>> On 19.03.2016, at 03:17, Eric Anholt <eric@anholt.net> wrote: > >>> > >>> Stefan Wahren <stefan.wahren@i2se.com> writes: > >>> > >>>> Hi Eric, > >>>> hi Martin, > >>>> > >>>>> John Youn <John.Youn@synopsys.com> hat am 16. März 2016 um 19:28 > >>>>> geschrieben: > >>>>> > >>>>> > >>>>> On 3/10/2016 11:14 AM, John Youn wrote: > >>>>>> On 3/9/2016 11:06 AM, Doug Anderson wrote: > >>>>>>> > >>>>>>> John: it's pretty clear that there's something taking almost exactly > >>>>>>> 10ms on my system and almost exactly 50ms on Stefan's system. Is > >>>>>>> there some register we could poll to see when this process is done? > >>>>>>> ...or can we look at the dwc2 revision number / feature register and > >>>>>>> detect how long to delay? > >>>>>>> > >> > >> Maybe this difference is related to overclocking settings in the firmware? > >> > >>>>> > >>>>> 1. What is the AHB Clock frequency? Is the AHB Clock gated during > >>>>> Reset? > >>> > >>> Low confidence here as I'm tracing lines across a ton of modules, but it > >>> looks like it comes from the USB AXI clock in peri_image, which is a > >>> gate on the normal 250Mhz APB clock, but nothing should be touching that > >>> gate register as part of USB reset as far as I know. > >>> > >> Isn’t it possible that this clock (probably BCM2835_CLOCK_VPU) is > >> changed by the firmware due to overclocking settings in /boot/config.txt? > > > > i don't use any overclocking settings. > > > > Are you able to reproduce the behavior on your Pi? > > I did not have any problems with USB recently (using 4.5), > so I would not have any idea how to reproduce it. > > Note that I am using it with an AXIS USB-ethernet dongle plus USB-stick > all the time on my Compute Module connected via a tiny hub, > so I wonder if that qualifies as being able to trigger the issue... the probe problem appears after applying John's patch series (it's not in 4.5): [RFT PATCH 1/2] usb: dwc2: Add a 10 ms delay to dwc2_core_reset() [RFT PATCH 2/2] Revert "usb: dwc2: Fix probe problem on bcm2835" Please follow the complete discussion here [1]. [1] - http://marc.info/?l=linux-kernel&m=145711582915801&w=2 > > Martin ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFT PATCH 2/2] Revert "usb: dwc2: Fix probe problem on bcm2835" 2016-03-19 2:17 ` Eric Anholt 2016-03-19 7:44 ` Martin Sperl @ 2016-03-19 9:45 ` Stefan Wahren 1 sibling, 0 replies; 32+ messages in thread From: Stefan Wahren @ 2016-03-19 9:45 UTC (permalink / raw) To: John Youn, Martin Sperl, Eric Anholt, Doug Anderson Cc: Michael Niewoehner, Tao Huang, Julius Werner, Greg Kroah-Hartman, linux-kernel, linux-usb, Caesar Wang, Heiko Stuebner, Felipe Balbi, Remi Pommarel Hi, > Eric Anholt <eric@anholt.net> hat am 19. März 2016 um 03:17 geschrieben: > > > Stefan Wahren <stefan.wahren@i2se.com> writes: > > > Hi Eric, > > hi Martin, > > > >> John Youn <John.Youn@synopsys.com> hat am 16. März 2016 um 19:28 > >> geschrieben: > >> > >> > >> On 3/10/2016 11:14 AM, John Youn wrote: > >> > On 3/9/2016 11:06 AM, Doug Anderson wrote: > >> >> Stefan, > >> >> > >> >> On Wed, Mar 9, 2016 at 11:01 AM, Stefan Wahren <stefan.wahren@i2se.com> > >> >> wrote: > >> >>> > >> >>>> Doug Anderson <dianders@chromium.org> hat am 7. März 2016 um 22:30 > >> >>>> geschrieben: > >> >>>> > >> >>>> > >> >>>> Stefan, > >> >>>> > >> >>>> On Mon, Mar 7, 2016 at 10:40 AM, Stefan Wahren > >> >>>> <stefan.wahren@i2se.com> > >> >>>> wrote: > >> >>>>> Hi Doug, > >> >>>>> > >> >>>>>> Douglas Anderson <dianders@chromium.org> hat am 4. März 2016 um > >> >>>>>> 19:23 > >> >>>>>> geschrieben: > >> >>>>>> > >> >>>>>> > >> >>>>>> This reverts commit 192cb07f7928 ("usb: dwc2: Fix probe problem on > >> >>>>>> bcm2835") now that we've found the root cause. See the change > >> >>>>>> titled ("usb: dwc2: Add a 10 ms delay to dwc2_core_reset()"). > >> >>>>> > >> >>>>> adding a delay of 10 ms after a core reset might be a idea, but > >> >>>>> applying > >> >>>>> both > >> >>>>> patches breaks USB support on RPi :-( > >> >>>>> > >> >>>>> I'm getting the wrong register values ... > >> >>>> > >> >>>> Ugh. :( > >> >>>> > >> >>>> Just out of curiosity, if you loop and time long it takes for the > >> >>>> registers to get to the right state after reset, what do you get? > >> >>>> AKA, pick: > >> >>>> > >> >>>> https://chromium-review.googlesource.com/331260 > >> >>>> > >> >>>> ...and let me know what it prints out. > >> >>> > >> >>> On my Raspberry Pi B i get the following: > >> >>> > >> >>> [ 2.084411] dwc2 20980000.usb: mapped PA 20980000 to VA cc880000 > >> >>> [ 2.084461] dwc2 20980000.usb: cannot get otg clock > >> >>> [ 2.084549] dwc2 20980000.usb: registering common handler for irq33 > >> >>> [ 2.084713] dwc2 20980000.usb: Configuration mismatch. dr_mode forced > >> >>> to > >> >>> host > >> >>> [ 2.153965] dwc2 20980000.usb: Waited 49996 us, 0x00201000 => > >> >>> 0x01001000, > >> >>> 0x00000000 => 0x02002000 > >> >>> [ 2.174930] dwc2 20980000.usb: Forcing mode to host > >> >>> > >> >>> So i changed the delay in patch #1 to msleep(50) and then both patches > >> >>> work like > >> >>> a charm. > >> >> > >> >> Great news! :-) > >> >> > >> >> John: it's pretty clear that there's something taking almost exactly > >> >> 10ms on my system and almost exactly 50ms on Stefan's system. Is > >> >> there some register we could poll to see when this process is done? > >> >> ...or can we look at the dwc2 revision number / feature register and > >> >> detect how long to delay? > >> >> > >> > > >> > Hi Doug, > >> > > >> > I'll have to ask around to see if anyone knows about this. And I'll > >> > run some tests on the platforms I have available to me as well. > >> > > >> > >> There's still nothing definitive on our end as to why this is > >> happening. Also I don't think there is any other way to poll the > >> reset. Our hardware engineers asked for some more information to look > >> into it further. Doug, Stefan, Caesar, and anyone else with a related > >> platform, do you know the answers to the following: > >> > >> 1. What is the AHB Clock frequency? Is the AHB Clock gated during > >> Reset? > > Low confidence here as I'm tracing lines across a ton of modules, but it > looks like it comes from the USB AXI clock in peri_image, which is a > gate on the normal 250Mhz APB clock, but nothing should be touching that > gate register as part of USB reset as far as I know. > > >> 2. Also is the PHY clock stopped during the reset or is the PHY PLL > >> lock times high in the order of ms? > > PHY PLL lock times should be about 40 us, and reset needs to be high for > 40us. I haven't traced where GRSTCTL_CSFTRST (the reset I assume you're > talking about here) goes, so I can't say if it's an input to PHY reset. > > >> 3. In these cases, is the PHY actually an FS Transceiver and not a > >> UTMI/ULPI PHY? > > The PHY docs say it's UTMI+ compatible. please correct me if i am wrong, but according to the datasheet [1] there should be 2 PHYs: Feature/Parameter | Selected value High-Speed PHY Interfaces | 1: UTMI+ USB 1.1 Full-Speed Serial Transceiver Interface | 1: Dedicated FS But non of them is in the BCM2835 devicetree and i don't know if there is any PHY driver we could use. Also there is no OTG clock decribed for dwc2. Btw the USB_GAHBCFG register is modified for BCM2835 following p. 204 in the datasheet. I don't know if it's important for this issue. [1] - https://www.raspberrypi.org/wp-content/uploads/2012/02/BCM2835-ARM-Peripherals.pdf Thanks Stefan > > >> 4. Which version of the controller is being used in these cases? > > > > @John: The BCM2835 has version 2.80a > > Yeah, that looks right. ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFT PATCH 2/2] Revert "usb: dwc2: Fix probe problem on bcm2835" 2016-03-16 18:28 ` John Youn 2016-03-18 23:14 ` Stefan Wahren @ 2016-03-19 5:21 ` Doug Anderson 2016-03-22 19:26 ` John Youn 1 sibling, 1 reply; 32+ messages in thread From: Doug Anderson @ 2016-03-19 5:21 UTC (permalink / raw) To: John Youn Cc: Stefan Wahren, Michael Niewoehner, Tao Huang, Julius Werner, Greg Kroah-Hartman, linux-kernel, linux-usb, Caesar Wang, Heiko Stuebner, Felipe Balbi, Remi Pommarel, Kever Yang Hi, On Wed, Mar 16, 2016 at 11:28 AM, John Youn <John.Youn@synopsys.com> wrote: > On 3/10/2016 11:14 AM, John Youn wrote: >> On 3/9/2016 11:06 AM, Doug Anderson wrote: >>> Stefan, >>> >>> On Wed, Mar 9, 2016 at 11:01 AM, Stefan Wahren <stefan.wahren@i2se.com> wrote: >>>> >>>>> Doug Anderson <dianders@chromium.org> hat am 7. März 2016 um 22:30 >>>>> geschrieben: >>>>> >>>>> >>>>> Stefan, >>>>> >>>>> On Mon, Mar 7, 2016 at 10:40 AM, Stefan Wahren <stefan.wahren@i2se.com> wrote: >>>>>> Hi Doug, >>>>>> >>>>>>> Douglas Anderson <dianders@chromium.org> hat am 4. März 2016 um 19:23 >>>>>>> geschrieben: >>>>>>> >>>>>>> >>>>>>> This reverts commit 192cb07f7928 ("usb: dwc2: Fix probe problem on >>>>>>> bcm2835") now that we've found the root cause. See the change >>>>>>> titled ("usb: dwc2: Add a 10 ms delay to dwc2_core_reset()"). >>>>>> >>>>>> adding a delay of 10 ms after a core reset might be a idea, but applying >>>>>> both >>>>>> patches breaks USB support on RPi :-( >>>>>> >>>>>> I'm getting the wrong register values ... >>>>> >>>>> Ugh. :( >>>>> >>>>> Just out of curiosity, if you loop and time long it takes for the >>>>> registers to get to the right state after reset, what do you get? >>>>> AKA, pick: >>>>> >>>>> https://chromium-review.googlesource.com/331260 >>>>> >>>>> ...and let me know what it prints out. >>>> >>>> On my Raspberry Pi B i get the following: >>>> >>>> [ 2.084411] dwc2 20980000.usb: mapped PA 20980000 to VA cc880000 >>>> [ 2.084461] dwc2 20980000.usb: cannot get otg clock >>>> [ 2.084549] dwc2 20980000.usb: registering common handler for irq33 >>>> [ 2.084713] dwc2 20980000.usb: Configuration mismatch. dr_mode forced to host >>>> [ 2.153965] dwc2 20980000.usb: Waited 49996 us, 0x00201000 => 0x01001000, >>>> 0x00000000 => 0x02002000 >>>> [ 2.174930] dwc2 20980000.usb: Forcing mode to host >>>> >>>> So i changed the delay in patch #1 to msleep(50) and then both patches work like >>>> a charm. >>> >>> Great news! :-) >>> >>> John: it's pretty clear that there's something taking almost exactly >>> 10ms on my system and almost exactly 50ms on Stefan's system. Is >>> there some register we could poll to see when this process is done? >>> ...or can we look at the dwc2 revision number / feature register and >>> detect how long to delay? >>> >> >> Hi Doug, >> >> I'll have to ask around to see if anyone knows about this. And I'll >> run some tests on the platforms I have available to me as well. >> > > There's still nothing definitive on our end as to why this is > happening. Also I don't think there is any other way to poll the > reset. One thing I noticed is that the delay was only needed on my OTG port (not my host-only port). ...I also noticed that while waiting the HPTXFSIZ was temporarily 0x00000000 while I was delaying. That seems to match Stephan. I wonder if perhaps the delay has to do with how long it took to detect that it needed to go into host mode? Ah, interesting. It looks as if "GOTGCTL" changes during this time too. To summarize: HOST-only (ff540000.usb): needs no delay: GNPTXFSIZ: 0x04000400 HPTXFSIZ: 0x02000800 GOTGCTL: 0x002d0000 OTG (ff580000): needs 10ms delay. Before delay: GNPTXFSIZ: 0x00100400 HPTXFSIZ: 0x00000000 GOTGCTL: 0x00010000 After delay: GNPTXFSIZ: 0x04000400 HPTXFSIZ: 0x02000800 GOTGCTL: 0x002c0000 Could we loop until either BSesVld or ASesVld is set? Don't know much about OTG, but seems like that might be something sane? > Our hardware engineers asked for some more information to look > into it further. Doug, Stefan, Caesar, and anyone else with a related > platform, do you know the answers to the following: > > 1. What is the AHB Clock frequency? Is the AHB Clock gated during > Reset? According to commit 20bde643434d ("usb: dwc2: reduce dwc2 driver probe time"), our AHB clock is 150MHz. I'm nearly certain it is not gated. > 2. Also is the PHY clock stopped during the reset or is the PHY PLL > lock times high in the order of ms? I don't think so. > 3. In these cases, is the PHY actually an FS Transceiver and not a > UTMI/ULPI PHY? I don't think so. Should be answered by debug info spew below... > 4. Which version of the controller is being used in these cases? There are two controllers in my case and they behave differently. OTG takes 10ms and the host-only 0ms. Here is debug info: [ 1.752005] dwc2 ff540000.usb: Core Release: 3.10a (snpsid=4f54310a) [ 1.758350] dwc2 ff540000.usb: hwcfg1=00000000 [ 1.762807] dwc2 ff540000.usb: hwcfg2=228fc856 [ 1.767245] dwc2 ff540000.usb: hwcfg3=03c0c068 [ 1.771693] dwc2 ff540000.usb: hwcfg4=c8004030 [ 1.776149] dwc2 ff540000.usb: grxfsiz=00000400 [ 1.780687] dwc2 ff540000.usb: gnptxfsiz=04000400 [ 1.785402] dwc2 ff540000.usb: hptxfsiz=02000800 [ 1.790024] dwc2 ff540000.usb: Detected values from hardware: [ 1.795781] dwc2 ff540000.usb: op_mode=6 [ 1.799872] dwc2 ff540000.usb: arch=2 [ 1.817620] dwc2 ff540000.usb: dma_desc_enable=1 [ 1.955925] dwc2 ff540000.usb: power_optimized=1 [ 2.022862] dwc2 ff540000.usb: i2c_enable=0 [ 2.027220] dwc2 ff540000.usb: hs_phy_type=1 [ 2.031657] dwc2 ff540000.usb: fs_phy_type=0 [ 2.036101] dwc2 ff540000.usb: utmi_phy_data_wdith=1 [ 2.041231] dwc2 ff540000.usb: num_dev_ep=2 [ 2.045586] dwc2 ff540000.usb: num_dev_perio_in_ep=0 [ 2.050717] dwc2 ff540000.usb: host_channels=16 [ 2.055420] dwc2 ff540000.usb: max_transfer_size=524287 [ 2.060811] dwc2 ff540000.usb: max_packet_count=1023 [ 2.065946] dwc2 ff540000.usb: nperio_tx_q_depth=0x4 [ 2.071076] dwc2 ff540000.usb: host_perio_tx_q_depth=0x4 [ 2.076556] dwc2 ff540000.usb: dev_token_q_depth=0x8 [ 2.081686] dwc2 ff540000.usb: enable_dynamic_fifo=1 [ 2.086824] dwc2 ff540000.usb: en_multiple_tx_fifo=0 [ 2.099436] dwc2 ff540000.usb: total_fifo_size=960 [ 2.204560] dwc2 ff540000.usb: host_rx_fifo_size=1024 [ 2.209780] dwc2 ff540000.usb: host_nperio_tx_fifo_size=1024 [ 2.712045] dwc2 ff540000.usb: host_perio_tx_fifo_size=512 [ 2.872149] dwc2 ff580000.usb: Core Release: 3.10a (snpsid=4f54310a) [ 2.872153] dwc2 ff580000.usb: hwcfg1=00006664 [ 2.872156] dwc2 ff580000.usb: hwcfg2=228e2450 [ 2.872158] dwc2 ff580000.usb: hwcfg3=03cc90e8 [ 2.872160] dwc2 ff580000.usb: hwcfg4=dbf04030 [ 2.872163] dwc2 ff580000.usb: grxfsiz=00000400 [ 2.872166] dwc2 ff580000.usb: gnptxfsiz=04000400 [ 2.872168] dwc2 ff580000.usb: hptxfsiz=02000800 [ 2.872171] dwc2 ff580000.usb: Detected values from hardware: [ 2.872173] dwc2 ff580000.usb: op_mode=0 [ 2.872175] dwc2 ff580000.usb: arch=2 [ 2.872177] dwc2 ff580000.usb: dma_desc_enable=1 [ 2.872179] dwc2 ff580000.usb: power_optimized=1 [ 2.872181] dwc2 ff580000.usb: i2c_enable=0 [ 2.872183] dwc2 ff580000.usb: hs_phy_type=1 [ 2.872186] dwc2 ff580000.usb: fs_phy_type=0 [ 2.872188] dwc2 ff580000.usb: utmi_phy_data_wdith=1 [ 2.872190] dwc2 ff580000.usb: num_dev_ep=9 [ 2.872192] dwc2 ff580000.usb: num_dev_perio_in_ep=0 [ 2.872194] dwc2 ff580000.usb: host_channels=9 [ 2.872196] dwc2 ff580000.usb: max_transfer_size=524287 [ 2.872198] dwc2 ff580000.usb: max_packet_count=1023 [ 2.872201] dwc2 ff580000.usb: nperio_tx_q_depth=0x4 [ 2.872203] dwc2 ff580000.usb: host_perio_tx_q_depth=0x4 [ 2.872205] dwc2 ff580000.usb: dev_token_q_depth=0x8 [ 2.872207] dwc2 ff580000.usb: enable_dynamic_fifo=1 [ 2.872209] dwc2 ff580000.usb: en_multiple_tx_fifo=1 [ 2.872211] dwc2 ff580000.usb: total_fifo_size=972 [ 2.872213] dwc2 ff580000.usb: host_rx_fifo_size=1024 [ 2.872215] dwc2 ff580000.usb: host_nperio_tx_fifo_size=1024 [ 2.872217] dwc2 ff580000.usb: host_perio_tx_fifo_size=512 ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFT PATCH 2/2] Revert "usb: dwc2: Fix probe problem on bcm2835" 2016-03-19 5:21 ` Doug Anderson @ 2016-03-22 19:26 ` John Youn 2016-03-22 19:44 ` Doug Anderson 0 siblings, 1 reply; 32+ messages in thread From: John Youn @ 2016-03-22 19:26 UTC (permalink / raw) To: Doug Anderson, John Youn Cc: Stefan Wahren, Michael Niewoehner, Tao Huang, Julius Werner, Greg Kroah-Hartman, linux-kernel, linux-usb, Caesar Wang, Heiko Stuebner, Felipe Balbi, Remi Pommarel, Kever Yang On 3/18/2016 10:21 PM, Doug Anderson wrote: > Hi, > > On Wed, Mar 16, 2016 at 11:28 AM, John Youn <John.Youn@synopsys.com> wrote: >> On 3/10/2016 11:14 AM, John Youn wrote: >>> On 3/9/2016 11:06 AM, Doug Anderson wrote: >>>> Stefan, >>>> >>>> On Wed, Mar 9, 2016 at 11:01 AM, Stefan Wahren <stefan.wahren@i2se.com> wrote: >>>>> >>>>>> Doug Anderson <dianders@chromium.org> hat am 7. März 2016 um 22:30 >>>>>> geschrieben: >>>>>> >>>>>> >>>>>> Stefan, >>>>>> >>>>>> On Mon, Mar 7, 2016 at 10:40 AM, Stefan Wahren <stefan.wahren@i2se.com> wrote: >>>>>>> Hi Doug, >>>>>>> >>>>>>>> Douglas Anderson <dianders@chromium.org> hat am 4. März 2016 um 19:23 >>>>>>>> geschrieben: >>>>>>>> >>>>>>>> >>>>>>>> This reverts commit 192cb07f7928 ("usb: dwc2: Fix probe problem on >>>>>>>> bcm2835") now that we've found the root cause. See the change >>>>>>>> titled ("usb: dwc2: Add a 10 ms delay to dwc2_core_reset()"). >>>>>>> >>>>>>> adding a delay of 10 ms after a core reset might be a idea, but applying >>>>>>> both >>>>>>> patches breaks USB support on RPi :-( >>>>>>> >>>>>>> I'm getting the wrong register values ... >>>>>> >>>>>> Ugh. :( >>>>>> >>>>>> Just out of curiosity, if you loop and time long it takes for the >>>>>> registers to get to the right state after reset, what do you get? >>>>>> AKA, pick: >>>>>> >>>>>> https://chromium-review.googlesource.com/331260 >>>>>> >>>>>> ...and let me know what it prints out. >>>>> >>>>> On my Raspberry Pi B i get the following: >>>>> >>>>> [ 2.084411] dwc2 20980000.usb: mapped PA 20980000 to VA cc880000 >>>>> [ 2.084461] dwc2 20980000.usb: cannot get otg clock >>>>> [ 2.084549] dwc2 20980000.usb: registering common handler for irq33 >>>>> [ 2.084713] dwc2 20980000.usb: Configuration mismatch. dr_mode forced to host >>>>> [ 2.153965] dwc2 20980000.usb: Waited 49996 us, 0x00201000 => 0x01001000, >>>>> 0x00000000 => 0x02002000 >>>>> [ 2.174930] dwc2 20980000.usb: Forcing mode to host >>>>> >>>>> So i changed the delay in patch #1 to msleep(50) and then both patches work like >>>>> a charm. >>>> >>>> Great news! :-) >>>> >>>> John: it's pretty clear that there's something taking almost exactly >>>> 10ms on my system and almost exactly 50ms on Stefan's system. Is >>>> there some register we could poll to see when this process is done? >>>> ...or can we look at the dwc2 revision number / feature register and >>>> detect how long to delay? >>>> >>> >>> Hi Doug, >>> >>> I'll have to ask around to see if anyone knows about this. And I'll >>> run some tests on the platforms I have available to me as well. >>> >> >> There's still nothing definitive on our end as to why this is >> happening. Also I don't think there is any other way to poll the >> reset. > > One thing I noticed is that the delay was only needed on my OTG port > (not my host-only port). ...I also noticed that while waiting the > HPTXFSIZ was temporarily 0x00000000 while I was delaying. That seems > to match Stephan. > > I wonder if perhaps the delay has to do with how long it took to > detect that it needed to go into host mode? > > Ah, interesting. It looks as if "GOTGCTL" changes during this time > too. To summarize: > > HOST-only (ff540000.usb): needs no delay: > GNPTXFSIZ: 0x04000400 > HPTXFSIZ: 0x02000800 > GOTGCTL: 0x002d0000 > > OTG (ff580000): needs 10ms delay. Before delay: > GNPTXFSIZ: 0x00100400 > HPTXFSIZ: 0x00000000 > GOTGCTL: 0x00010000 > After delay: > GNPTXFSIZ: 0x04000400 > HPTXFSIZ: 0x02000800 > GOTGCTL: 0x002c0000 > > Could we loop until either BSesVld or ASesVld is set? Don't know much > about OTG, but seems like that might be something sane? > >> Our hardware engineers asked for some more information to look >> into it further. Doug, Stefan, Caesar, and anyone else with a related >> platform, do you know the answers to the following: >> >> 1. What is the AHB Clock frequency? Is the AHB Clock gated during >> Reset? > > According to commit 20bde643434d ("usb: dwc2: reduce dwc2 driver probe > time"), our AHB clock is 150MHz. I'm nearly certain it is not gated. > > >> 2. Also is the PHY clock stopped during the reset or is the PHY PLL >> lock times high in the order of ms? > > I don't think so. > > >> 3. In these cases, is the PHY actually an FS Transceiver and not a >> UTMI/ULPI PHY? > > I don't think so. Should be answered by debug info spew below... > > >> 4. Which version of the controller is being used in these cases? > > There are two controllers in my case and they behave differently. OTG > takes 10ms and the host-only 0ms. > > Here is debug info: > > [ 1.752005] dwc2 ff540000.usb: Core Release: 3.10a (snpsid=4f54310a) > [ 1.758350] dwc2 ff540000.usb: hwcfg1=00000000 > [ 1.762807] dwc2 ff540000.usb: hwcfg2=228fc856 > [ 1.767245] dwc2 ff540000.usb: hwcfg3=03c0c068 > [ 1.771693] dwc2 ff540000.usb: hwcfg4=c8004030 > [ 1.776149] dwc2 ff540000.usb: grxfsiz=00000400 > [ 1.780687] dwc2 ff540000.usb: gnptxfsiz=04000400 > [ 1.785402] dwc2 ff540000.usb: hptxfsiz=02000800 > [ 1.790024] dwc2 ff540000.usb: Detected values from hardware: > [ 1.795781] dwc2 ff540000.usb: op_mode=6 > [ 1.799872] dwc2 ff540000.usb: arch=2 > [ 1.817620] dwc2 ff540000.usb: dma_desc_enable=1 > [ 1.955925] dwc2 ff540000.usb: power_optimized=1 > [ 2.022862] dwc2 ff540000.usb: i2c_enable=0 > [ 2.027220] dwc2 ff540000.usb: hs_phy_type=1 > [ 2.031657] dwc2 ff540000.usb: fs_phy_type=0 > [ 2.036101] dwc2 ff540000.usb: utmi_phy_data_wdith=1 > [ 2.041231] dwc2 ff540000.usb: num_dev_ep=2 > [ 2.045586] dwc2 ff540000.usb: num_dev_perio_in_ep=0 > [ 2.050717] dwc2 ff540000.usb: host_channels=16 > [ 2.055420] dwc2 ff540000.usb: max_transfer_size=524287 > [ 2.060811] dwc2 ff540000.usb: max_packet_count=1023 > [ 2.065946] dwc2 ff540000.usb: nperio_tx_q_depth=0x4 > [ 2.071076] dwc2 ff540000.usb: host_perio_tx_q_depth=0x4 > [ 2.076556] dwc2 ff540000.usb: dev_token_q_depth=0x8 > [ 2.081686] dwc2 ff540000.usb: enable_dynamic_fifo=1 > [ 2.086824] dwc2 ff540000.usb: en_multiple_tx_fifo=0 > [ 2.099436] dwc2 ff540000.usb: total_fifo_size=960 > [ 2.204560] dwc2 ff540000.usb: host_rx_fifo_size=1024 > [ 2.209780] dwc2 ff540000.usb: host_nperio_tx_fifo_size=1024 > [ 2.712045] dwc2 ff540000.usb: host_perio_tx_fifo_size=512 > > [ 2.872149] dwc2 ff580000.usb: Core Release: 3.10a (snpsid=4f54310a) > [ 2.872153] dwc2 ff580000.usb: hwcfg1=00006664 > [ 2.872156] dwc2 ff580000.usb: hwcfg2=228e2450 > [ 2.872158] dwc2 ff580000.usb: hwcfg3=03cc90e8 > [ 2.872160] dwc2 ff580000.usb: hwcfg4=dbf04030 > [ 2.872163] dwc2 ff580000.usb: grxfsiz=00000400 > [ 2.872166] dwc2 ff580000.usb: gnptxfsiz=04000400 > [ 2.872168] dwc2 ff580000.usb: hptxfsiz=02000800 > [ 2.872171] dwc2 ff580000.usb: Detected values from hardware: > [ 2.872173] dwc2 ff580000.usb: op_mode=0 > [ 2.872175] dwc2 ff580000.usb: arch=2 > [ 2.872177] dwc2 ff580000.usb: dma_desc_enable=1 > [ 2.872179] dwc2 ff580000.usb: power_optimized=1 > [ 2.872181] dwc2 ff580000.usb: i2c_enable=0 > [ 2.872183] dwc2 ff580000.usb: hs_phy_type=1 > [ 2.872186] dwc2 ff580000.usb: fs_phy_type=0 > [ 2.872188] dwc2 ff580000.usb: utmi_phy_data_wdith=1 > [ 2.872190] dwc2 ff580000.usb: num_dev_ep=9 > [ 2.872192] dwc2 ff580000.usb: num_dev_perio_in_ep=0 > [ 2.872194] dwc2 ff580000.usb: host_channels=9 > [ 2.872196] dwc2 ff580000.usb: max_transfer_size=524287 > [ 2.872198] dwc2 ff580000.usb: max_packet_count=1023 > [ 2.872201] dwc2 ff580000.usb: nperio_tx_q_depth=0x4 > [ 2.872203] dwc2 ff580000.usb: host_perio_tx_q_depth=0x4 > [ 2.872205] dwc2 ff580000.usb: dev_token_q_depth=0x8 > [ 2.872207] dwc2 ff580000.usb: enable_dynamic_fifo=1 > [ 2.872209] dwc2 ff580000.usb: en_multiple_tx_fifo=1 > [ 2.872211] dwc2 ff580000.usb: total_fifo_size=972 > [ 2.872213] dwc2 ff580000.usb: host_rx_fifo_size=1024 > [ 2.872215] dwc2 ff580000.usb: host_nperio_tx_fifo_size=1024 > [ 2.872217] dwc2 ff580000.usb: host_perio_tx_fifo_size=512 > Thanks for the debug logs and everyones help. After reviewing with our hardware engineers, it seems this is likely to do with the IDDIG debounce filtering when switching between modes. You can see if this is enabled in your core by checking GHWCFG4.IddgFltr. >From the databook: OTG_EN_IDDIG_FILTER Specifies whether to add a filter on the "iddig" input from the PHY. If your PHY already has a filter on iddig for de-bounce, then it is not necessary to enable the one in the DWC_otg. The filter is implemented in the DWC_otg_wpc module as a 20-bit counter that works on the PHY clock. In the case of the UTMI+ PHY, this pin is from the PHY. In the case of the ULPI PHY, this signal is generated by the ULPI Wrapper inside the core. This only affects OTG cores and the delay is 10ms for 30MHz PHY clock and 50ms with a 6MHz PHY clock. The reason we see this after a reset is that by default the IDDIG indicates device mode. But if the id pin is set to host the core will immediately detect it after the reset and trigger the filtering delay before changing to host mode. This delay is also triggered by force mode. This is the origin of the 25 ms delay specified in the databook. The databook did not take into account that there might be a 6MHz clock so this delay could actually be up to 50 ms. So we aren't delaying enough time for those cases. I think this explains all the problems and platform differences we are seeing related to this. I think we can just add an IDDIG delay after a force mode, a clear force mode, and any time after reset where we don't do either a force or clear force mode immediately afterwards. Maybe set the delay time via a core parameter or measure it once on probe. Or we can probably just poll for the mode change in all of the above conditions. Doug, Are you able to continue looking into this? If not, I can take it up. Regards, John ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFT PATCH 2/2] Revert "usb: dwc2: Fix probe problem on bcm2835" 2016-03-22 19:26 ` John Youn @ 2016-03-22 19:44 ` Doug Anderson 2016-03-22 20:37 ` John Youn 2016-03-24 6:18 ` John Youn 0 siblings, 2 replies; 32+ messages in thread From: Doug Anderson @ 2016-03-22 19:44 UTC (permalink / raw) To: John Youn Cc: Stefan Wahren, Michael Niewoehner, Tao Huang, Julius Werner, Greg Kroah-Hartman, linux-kernel, linux-usb, Caesar Wang, Heiko Stuebner, Felipe Balbi, Remi Pommarel, Kever Yang John, On Tue, Mar 22, 2016 at 12:26 PM, John Youn <John.Youn@synopsys.com> wrote: > Thanks for the debug logs and everyones help. > > After reviewing with our hardware engineers, it seems this is likely > to do with the IDDIG debounce filtering when switching between > modes. You can see if this is enabled in your core by checking > GHWCFG4.IddgFltr. > > From the databook: > > OTG_EN_IDDIG_FILTER > > Specifies whether to add a filter on the "iddig" input from the > PHY. If your PHY already has a filter on iddig for de-bounce, then > it is not necessary to enable the one in the DWC_otg. The filter > is implemented in the DWC_otg_wpc module as a 20-bit counter that > works on the PHY clock. In the case of the UTMI+ PHY, this pin is > from the PHY. In the case of the ULPI PHY, this signal is > generated by the ULPI Wrapper inside the core. > > > This only affects OTG cores and the delay is 10ms for 30MHz PHY clock > and 50ms with a 6MHz PHY clock. Wow, nice to have it so perfectly explained! :) > The reason we see this after a reset is that by default the IDDIG > indicates device mode. But if the id pin is set to host the core will > immediately detect it after the reset and trigger the filtering delay > before changing to host mode. > > This delay is also triggered by force mode. This is the origin of the > 25 ms delay specified in the databook. The databook did not take into > account that there might be a 6MHz clock so this delay could actually > be up to 50 ms. So we aren't delaying enough time for those cases. > > I think this explains all the problems and platform differences we are > seeing related to this. > > I think we can just add an IDDIG delay after a force mode, a clear > force mode, and any time after reset where we don't do either a force > or clear force mode immediately afterwards. Maybe set the delay time > via a core parameter or measure it once on probe. Or we can probably > just poll for the mode change in all of the above conditions. The driver seems to be able to figure out the PHY clock in most cases in dwc2_calc_frame_interval(). It doesn't seem to handle 6MHz there, though. I wonder if that's another bug? Polling seems fine too, though. > Are you able to continue looking into this? If not, I can take it up. I'm pretty much out of time at this point and it sounds like you've though through all of the corner cases. If you can take it up that would be wonderful! :) I'm happy to give the patches a test, though! :) -Doug ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFT PATCH 2/2] Revert "usb: dwc2: Fix probe problem on bcm2835" 2016-03-22 19:44 ` Doug Anderson @ 2016-03-22 20:37 ` John Youn 2016-03-24 6:18 ` John Youn 1 sibling, 0 replies; 32+ messages in thread From: John Youn @ 2016-03-22 20:37 UTC (permalink / raw) To: Doug Anderson, John Youn Cc: Stefan Wahren, Michael Niewoehner, Tao Huang, Julius Werner, Greg Kroah-Hartman, linux-kernel, linux-usb, Caesar Wang, Heiko Stuebner, Felipe Balbi, Remi Pommarel, Kever Yang On 3/22/2016 12:44 PM, Doug Anderson wrote: > John, > > On Tue, Mar 22, 2016 at 12:26 PM, John Youn <John.Youn@synopsys.com> wrote: >> Thanks for the debug logs and everyones help. >> >> After reviewing with our hardware engineers, it seems this is likely >> to do with the IDDIG debounce filtering when switching between >> modes. You can see if this is enabled in your core by checking >> GHWCFG4.IddgFltr. >> >> From the databook: >> >> OTG_EN_IDDIG_FILTER >> >> Specifies whether to add a filter on the "iddig" input from the >> PHY. If your PHY already has a filter on iddig for de-bounce, then >> it is not necessary to enable the one in the DWC_otg. The filter >> is implemented in the DWC_otg_wpc module as a 20-bit counter that >> works on the PHY clock. In the case of the UTMI+ PHY, this pin is >> from the PHY. In the case of the ULPI PHY, this signal is >> generated by the ULPI Wrapper inside the core. >> >> >> This only affects OTG cores and the delay is 10ms for 30MHz PHY clock >> and 50ms with a 6MHz PHY clock. > > Wow, nice to have it so perfectly explained! :) > > >> The reason we see this after a reset is that by default the IDDIG >> indicates device mode. But if the id pin is set to host the core will >> immediately detect it after the reset and trigger the filtering delay >> before changing to host mode. >> >> This delay is also triggered by force mode. This is the origin of the >> 25 ms delay specified in the databook. The databook did not take into >> account that there might be a 6MHz clock so this delay could actually >> be up to 50 ms. So we aren't delaying enough time for those cases. >> >> I think this explains all the problems and platform differences we are >> seeing related to this. >> >> I think we can just add an IDDIG delay after a force mode, a clear >> force mode, and any time after reset where we don't do either a force >> or clear force mode immediately afterwards. Maybe set the delay time >> via a core parameter or measure it once on probe. Or we can probably >> just poll for the mode change in all of the above conditions. > > The driver seems to be able to figure out the PHY clock in most cases > in dwc2_calc_frame_interval(). It doesn't seem to handle 6MHz there, > though. I wonder if that's another bug? > > Polling seems fine too, though. > > >> Are you able to continue looking into this? If not, I can take it up. > > I'm pretty much out of time at this point and it sounds like you've > though through all of the corner cases. If you can take it up that > would be wonderful! :) I'm happy to give the patches a test, though! > :) > Ok sure. I'll try to get something testable within a few days. Regards, John ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFT PATCH 2/2] Revert "usb: dwc2: Fix probe problem on bcm2835" 2016-03-22 19:44 ` Doug Anderson 2016-03-22 20:37 ` John Youn @ 2016-03-24 6:18 ` John Youn 1 sibling, 0 replies; 32+ messages in thread From: John Youn @ 2016-03-24 6:18 UTC (permalink / raw) To: Doug Anderson, John Youn Cc: Stefan Wahren, Michael Niewoehner, Tao Huang, Julius Werner, Greg Kroah-Hartman, linux-kernel, linux-usb, Caesar Wang, Heiko Stuebner, Felipe Balbi, Remi Pommarel, Kever Yang On 3/22/2016 12:44 PM, Doug Anderson wrote: > John, > > On Tue, Mar 22, 2016 at 12:26 PM, John Youn <John.Youn@synopsys.com> wrote: >> Thanks for the debug logs and everyones help. >> >> After reviewing with our hardware engineers, it seems this is likely >> to do with the IDDIG debounce filtering when switching between >> modes. You can see if this is enabled in your core by checking >> GHWCFG4.IddgFltr. >> >> From the databook: >> >> OTG_EN_IDDIG_FILTER >> >> Specifies whether to add a filter on the "iddig" input from the >> PHY. If your PHY already has a filter on iddig for de-bounce, then >> it is not necessary to enable the one in the DWC_otg. The filter >> is implemented in the DWC_otg_wpc module as a 20-bit counter that >> works on the PHY clock. In the case of the UTMI+ PHY, this pin is >> from the PHY. In the case of the ULPI PHY, this signal is >> generated by the ULPI Wrapper inside the core. >> >> >> This only affects OTG cores and the delay is 10ms for 30MHz PHY clock >> and 50ms with a 6MHz PHY clock. > > Wow, nice to have it so perfectly explained! :) > > >> The reason we see this after a reset is that by default the IDDIG >> indicates device mode. But if the id pin is set to host the core will >> immediately detect it after the reset and trigger the filtering delay >> before changing to host mode. >> >> This delay is also triggered by force mode. This is the origin of the >> 25 ms delay specified in the databook. The databook did not take into >> account that there might be a 6MHz clock so this delay could actually >> be up to 50 ms. So we aren't delaying enough time for those cases. >> >> I think this explains all the problems and platform differences we are >> seeing related to this. >> >> I think we can just add an IDDIG delay after a force mode, a clear >> force mode, and any time after reset where we don't do either a force >> or clear force mode immediately afterwards. Maybe set the delay time >> via a core parameter or measure it once on probe. Or we can probably >> just poll for the mode change in all of the above conditions. > > The driver seems to be able to figure out the PHY clock in most cases > in dwc2_calc_frame_interval(). It doesn't seem to handle 6MHz there, > though. I wonder if that's another bug? > The 6 MHz is from a dedicated FS PHY operating at low speed. So that must be the case for the platforms showing 50 ms delay. > Polling seems fine too, though. I'm thinking that's the way to go as it can be troublesome to figure out the correct PHY clock speed. Regards, John ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFT PATCH 1/2] usb: dwc2: Add a 10 ms delay to dwc2_core_reset() 2016-03-04 18:23 [RFT PATCH 1/2] usb: dwc2: Add a 10 ms delay to dwc2_core_reset() Douglas Anderson 2016-03-04 18:23 ` [RFT PATCH 2/2] Revert "usb: dwc2: Fix probe problem on bcm2835" Douglas Anderson @ 2016-03-04 22:54 ` Michael Niewoehner 2016-03-05 0:09 ` Michael Niewoehner 2016-03-04 23:46 ` Karl Palsson ` (2 subsequent siblings) 4 siblings, 1 reply; 32+ messages in thread From: Michael Niewoehner @ 2016-03-04 22:54 UTC (permalink / raw) To: Douglas Anderson Cc: johnyoun, balbi, Heiko Stuebner, caesar.upstream, huangtao, repk, stefan.wahren, Julius Werner, gregkh, linux-usb, linux-kernel Hi Douglas, Am 04.03.2016 um 19:23 schrieb Douglas Anderson <dianders@chromium.org>: > From testing and trying to make sense of the documentation, it appears > that a 10 ms delay is needed after resetting the core to make sure that > everything is stable and consistent. Let's add it. > > In my testing (on rk3288) this allows us to revert commit > 192cb07f7928 ("usb: dwc2: Fix probe problem on bcm2835"). Though I > could never reproduce the problems on my board, this might also allow us > to revert commit bd84f4ae9986 ("usb: dwc2: Add extra delay when forcing > dr_mode"). > > Signed-off-by: Douglas Anderson <dianders@chromium.org> Tested-by: Michael Niewoehner <linux@mniewoehner.de> > --- > drivers/usb/dwc2/core.c | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > > diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c > index 5e5a0f135b5a..8710b2d3e770 100644 > --- a/drivers/usb/dwc2/core.c > +++ b/drivers/usb/dwc2/core.c > @@ -277,6 +277,26 @@ int dwc2_core_reset(struct dwc2_hsotg *hsotg) > } > } while (!(greset & GRSTCTL_AHBIDLE)); > > + /* > + * Sleep for 10-15 ms after the reset to let it finish. > + * > + * It's been confirmed on at least one version of the controller > + * that this is a requirement that this is a requirement in order for > + * everything to settle. Specifically if you: > + * - change GNPTXFSIZ or HPTXFSIZ before the reset > + * - do the reset > + * - read GNPTXFSIZ or HPTXFSIZ in a loop > + * ...you'll find that it takes almost exactly 10 ms for the registers > + * to return to their reset defaults. > + * > + * Note that it's possible that this 10 ms is the time referred to > + * in "Host Initialization" where it says to "Wait at least 10 ms for > + * the reset process to complete". In "Device Initialization" there > + * is also talk of a reset lasting 10 ms. That may be the source of > + * this delay. > + */ > + usleep_range(10000, 15000); > + > return 0; > } > > -- > 2.7.0.rc3.207.g0ac5344 > I’m a bit confused since git log says bd84f4ae9986 has been merged in 62718e304aa6 but looking at drivers/usb/dwc2/core.c it seems the patch has not been applied anyways ... However, I tested you your two patches with „magically reverted“ bd84f4ae9986 (msleep 50) on rk3188. The sdcard keeps being detected and boots just fine. Best regards Michael ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFT PATCH 1/2] usb: dwc2: Add a 10 ms delay to dwc2_core_reset() 2016-03-04 22:54 ` [RFT PATCH 1/2] usb: dwc2: Add a 10 ms delay to dwc2_core_reset() Michael Niewoehner @ 2016-03-05 0:09 ` Michael Niewoehner 2016-03-05 0:33 ` Doug Anderson 0 siblings, 1 reply; 32+ messages in thread From: Michael Niewoehner @ 2016-03-05 0:09 UTC (permalink / raw) To: Douglas Anderson Cc: johnyoun, balbi, Heiko Stuebner, caesar.upstream, huangtao, repk, stefan.wahren, Julius Werner, gregkh, linux-usb, linux-kernel Hi Douglas, Am 04.03.2016 um 23:54 schrieb Michael Niewoehner <linux@mniewoehner.de>: > Hi Douglas, > > Am 04.03.2016 um 19:23 schrieb Douglas Anderson <dianders@chromium.org>: > >> From testing and trying to make sense of the documentation, it appears >> that a 10 ms delay is needed after resetting the core to make sure that >> everything is stable and consistent. Let's add it. >> >> In my testing (on rk3288) this allows us to revert commit >> 192cb07f7928 ("usb: dwc2: Fix probe problem on bcm2835"). Though I >> could never reproduce the problems on my board, this might also allow us >> to revert commit bd84f4ae9986 ("usb: dwc2: Add extra delay when forcing >> dr_mode"). >> >> Signed-off-by: Douglas Anderson <dianders@chromium.org> > > Tested-by: Michael Niewoehner <linux@mniewoehner.de> > >> --- >> drivers/usb/dwc2/core.c | 20 ++++++++++++++++++++ >> 1 file changed, 20 insertions(+) >> >> diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c >> index 5e5a0f135b5a..8710b2d3e770 100644 >> --- a/drivers/usb/dwc2/core.c >> +++ b/drivers/usb/dwc2/core.c >> @@ -277,6 +277,26 @@ int dwc2_core_reset(struct dwc2_hsotg *hsotg) >> } >> } while (!(greset & GRSTCTL_AHBIDLE)); >> >> + /* >> + * Sleep for 10-15 ms after the reset to let it finish. >> + * >> + * It's been confirmed on at least one version of the controller >> + * that this is a requirement that this is a requirement in order for >> + * everything to settle. Specifically if you: >> + * - change GNPTXFSIZ or HPTXFSIZ before the reset >> + * - do the reset >> + * - read GNPTXFSIZ or HPTXFSIZ in a loop >> + * ...you'll find that it takes almost exactly 10 ms for the registers >> + * to return to their reset defaults. >> + * >> + * Note that it's possible that this 10 ms is the time referred to >> + * in "Host Initialization" where it says to "Wait at least 10 ms for >> + * the reset process to complete". In "Device Initialization" there >> + * is also talk of a reset lasting 10 ms. That may be the source of >> + * this delay. >> + */ >> + usleep_range(10000, 15000); >> + >> return 0; >> } >> >> -- >> 2.7.0.rc3.207.g0ac5344 >> > > I’m a bit confused since git log says bd84f4ae9986 has been merged in 62718e304aa6 but looking at drivers/usb/dwc2/core.c it seems the patch has not been applied anyways ... > However, I tested you your two patches with „magically reverted“ bd84f4ae9986 (msleep 50) on rk3188. > The sdcard keeps being detected and boots just fine. > > Best regards > Michael I meant usb stick of course… too much sdcards in my head today \o/. Regards Michael ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFT PATCH 1/2] usb: dwc2: Add a 10 ms delay to dwc2_core_reset() 2016-03-05 0:09 ` Michael Niewoehner @ 2016-03-05 0:33 ` Doug Anderson 2016-03-05 1:09 ` Doug Anderson 2016-03-05 20:41 ` Michael Niewoehner 0 siblings, 2 replies; 32+ messages in thread From: Doug Anderson @ 2016-03-05 0:33 UTC (permalink / raw) To: Michael Niewoehner Cc: John Youn, Felipe Balbi, Heiko Stuebner, Caesar Wang, Tao Huang, Remi Pommarel, Stefan Wahren, Julius Werner, Greg Kroah-Hartman, linux-usb, linux-kernel Michael, On Fri, Mar 4, 2016 at 4:09 PM, Michael Niewoehner <linux@mniewoehner.de> wrote: >>> From testing and trying to make sense of the documentation, it appears >>> that a 10 ms delay is needed after resetting the core to make sure that >>> everything is stable and consistent. Let's add it. >>> >>> In my testing (on rk3288) this allows us to revert commit >>> 192cb07f7928 ("usb: dwc2: Fix probe problem on bcm2835"). Though I >>> could never reproduce the problems on my board, this might also allow us >>> to revert commit bd84f4ae9986 ("usb: dwc2: Add extra delay when forcing >>> dr_mode"). >>> >>> Signed-off-by: Douglas Anderson <dianders@chromium.org> >> >> Tested-by: Michael Niewoehner <linux@mniewoehner.de> Thanks! That's great news! >> I’m a bit confused since git log says bd84f4ae9986 has been merged in 62718e304aa6 but looking at drivers/usb/dwc2/core.c it seems the patch has not been applied anyways ... >> However, I tested you your two patches with „magically reverted“ bd84f4ae9986 (msleep 50) on rk3188. >> The sdcard keeps being detected and boots just fine. > I meant usb stick of course… too much sdcards in my head today \o/. Odd. It looks to be there for me... $ git checkout 62718e304aa6 HEAD is now at 62718e304aa6... Merge tag 'usb-4.5-rc6' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb $ git grep -C3 "NOTE: This is required" -- drivers/usb/dwc2/core.c drivers/usb/dwc2/core.c- } drivers/usb/dwc2/core.c- drivers/usb/dwc2/core.c- /* drivers/usb/dwc2/core.c: * NOTE: This is required for some rockchip soc based drivers/usb/dwc2/core.c- * platforms. drivers/usb/dwc2/core.c- */ drivers/usb/dwc2/core.c- msleep(50); ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFT PATCH 1/2] usb: dwc2: Add a 10 ms delay to dwc2_core_reset() 2016-03-05 0:33 ` Doug Anderson @ 2016-03-05 1:09 ` Doug Anderson 2016-03-05 20:41 ` Michael Niewoehner 1 sibling, 0 replies; 32+ messages in thread From: Doug Anderson @ 2016-03-05 1:09 UTC (permalink / raw) To: Michael Niewoehner Cc: John Youn, Felipe Balbi, Heiko Stuebner, Caesar Wang, Tao Huang, Remi Pommarel, Stefan Wahren, Julius Werner, Greg Kroah-Hartman, linux-usb, linux-kernel Hi, On Fri, Mar 4, 2016 at 4:33 PM, Doug Anderson <dianders@chromium.org> wrote: > Michael, > > On Fri, Mar 4, 2016 at 4:09 PM, Michael Niewoehner <linux@mniewoehner.de> wrote: >>>> From testing and trying to make sense of the documentation, it appears >>>> that a 10 ms delay is needed after resetting the core to make sure that >>>> everything is stable and consistent. Let's add it. >>>> >>>> In my testing (on rk3288) this allows us to revert commit >>>> 192cb07f7928 ("usb: dwc2: Fix probe problem on bcm2835"). Though I >>>> could never reproduce the problems on my board, this might also allow us >>>> to revert commit bd84f4ae9986 ("usb: dwc2: Add extra delay when forcing >>>> dr_mode"). >>>> >>>> Signed-off-by: Douglas Anderson <dianders@chromium.org> >>> >>> Tested-by: Michael Niewoehner <linux@mniewoehner.de> > > Thanks! That's great news! > > >>> I’m a bit confused since git log says bd84f4ae9986 has been merged in 62718e304aa6 but looking at drivers/usb/dwc2/core.c it seems the patch has not been applied anyways ... >>> However, I tested you your two patches with „magically reverted“ bd84f4ae9986 (msleep 50) on rk3188. >>> The sdcard keeps being detected and boots just fine. >> I meant usb stick of course… too much sdcards in my head today \o/. > > Odd. It looks to be there for me... > > $ git checkout 62718e304aa6 > HEAD is now at 62718e304aa6... Merge tag 'usb-4.5-rc6' of > git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb > > $ git grep -C3 "NOTE: This is required" -- drivers/usb/dwc2/core.c > drivers/usb/dwc2/core.c- } > drivers/usb/dwc2/core.c- > drivers/usb/dwc2/core.c- /* > drivers/usb/dwc2/core.c: * NOTE: This is required for some > rockchip soc based > drivers/usb/dwc2/core.c- * platforms. > drivers/usb/dwc2/core.c- */ > drivers/usb/dwc2/core.c- msleep(50); For anyone playing along at home, please see <http://article.gmane.org/gmane.linux.usb.general/138355>. -Doug ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFT PATCH 1/2] usb: dwc2: Add a 10 ms delay to dwc2_core_reset() 2016-03-05 0:33 ` Doug Anderson 2016-03-05 1:09 ` Doug Anderson @ 2016-03-05 20:41 ` Michael Niewoehner 2016-03-06 1:38 ` Doug Anderson 1 sibling, 1 reply; 32+ messages in thread From: Michael Niewoehner @ 2016-03-05 20:41 UTC (permalink / raw) To: Doug Anderson, John Youn Cc: Felipe Balbi, Heiko Stuebner, Caesar Wang, Tao Huang, Remi Pommarel, Stefan Wahren, Julius Werner, Greg Kroah-Hartman, linux-usb, linux-kernel Hi Douglas, Hi John, Am 05.03.2016 um 01:33 schrieb Doug Anderson <dianders@chromium.org>: > Michael, > > On Fri, Mar 4, 2016 at 4:09 PM, Michael Niewoehner <linux@mniewoehner.de> wrote: >>>> From testing and trying to make sense of the documentation, it appears >>>> that a 10 ms delay is needed after resetting the core to make sure that >>>> everything is stable and consistent. Let's add it. >>>> >>>> In my testing (on rk3288) this allows us to revert commit >>>> 192cb07f7928 ("usb: dwc2: Fix probe problem on bcm2835"). Though I >>>> could never reproduce the problems on my board, this might also allow us >>>> to revert commit bd84f4ae9986 ("usb: dwc2: Add extra delay when forcing >>>> dr_mode"). >>>> >>>> Signed-off-by: Douglas Anderson <dianders@chromium.org> >>> >>> Tested-by: Michael Niewoehner <linux@mniewoehner.de> > > Thanks! That's great news! > > >>> I’m a bit confused since git log says bd84f4ae9986 has been merged in 62718e304aa6 but looking at drivers/usb/dwc2/core.c it seems the patch has not been applied anyways ... >>> However, I tested you your two patches with „magically reverted“ bd84f4ae9986 (msleep 50) on rk3188. >>> The sdcard keeps being detected and boots just fine. >> I meant usb stick of course… too much sdcards in my head today \o/. > > Odd. It looks to be there for me... > > $ git checkout 62718e304aa6 > HEAD is now at 62718e304aa6... Merge tag 'usb-4.5-rc6' of > git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb > > $ git grep -C3 "NOTE: This is required" -- drivers/usb/dwc2/core.c > drivers/usb/dwc2/core.c- } > drivers/usb/dwc2/core.c- > drivers/usb/dwc2/core.c- /* > drivers/usb/dwc2/core.c: * NOTE: This is required for some > rockchip soc based > drivers/usb/dwc2/core.c- * platforms. > drivers/usb/dwc2/core.c- */ > drivers/usb/dwc2/core.c- msleep(50); I unfortunately have bad news. After some more testing it turned out that usb does NOT work (always) on rk3188 when reverting bd84f4ae9986. It looks like that is dependent on which device / vendor is plugged in. The usb stick I tested yesterday worked once but today just blinks shortly and then stops working. Another usb stick I tested today doesn’t blink or work at all. Maybe I should have tested booting some more times :-( Best regards Michael ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFT PATCH 1/2] usb: dwc2: Add a 10 ms delay to dwc2_core_reset() 2016-03-05 20:41 ` Michael Niewoehner @ 2016-03-06 1:38 ` Doug Anderson 0 siblings, 0 replies; 32+ messages in thread From: Doug Anderson @ 2016-03-06 1:38 UTC (permalink / raw) To: Michael Niewoehner Cc: John Youn, Felipe Balbi, Heiko Stuebner, Caesar Wang, Tao Huang, Remi Pommarel, Stefan Wahren, Julius Werner, Greg Kroah-Hartman, linux-usb, linux-kernel Hi, On Sat, Mar 5, 2016 at 12:41 PM, Michael Niewoehner <linux@mniewoehner.de> wrote: > Hi Douglas, > Hi John, > > Am 05.03.2016 um 01:33 schrieb Doug Anderson <dianders@chromium.org>: > >> Michael, >> >> On Fri, Mar 4, 2016 at 4:09 PM, Michael Niewoehner <linux@mniewoehner.de> wrote: >>>>> From testing and trying to make sense of the documentation, it appears >>>>> that a 10 ms delay is needed after resetting the core to make sure that >>>>> everything is stable and consistent. Let's add it. >>>>> >>>>> In my testing (on rk3288) this allows us to revert commit >>>>> 192cb07f7928 ("usb: dwc2: Fix probe problem on bcm2835"). Though I >>>>> could never reproduce the problems on my board, this might also allow us >>>>> to revert commit bd84f4ae9986 ("usb: dwc2: Add extra delay when forcing >>>>> dr_mode"). >>>>> >>>>> Signed-off-by: Douglas Anderson <dianders@chromium.org> >>>> >>>> Tested-by: Michael Niewoehner <linux@mniewoehner.de> >> >> Thanks! That's great news! >> >> >>>> I’m a bit confused since git log says bd84f4ae9986 has been merged in 62718e304aa6 but looking at drivers/usb/dwc2/core.c it seems the patch has not been applied anyways ... >>>> However, I tested you your two patches with „magically reverted“ bd84f4ae9986 (msleep 50) on rk3188. >>>> The sdcard keeps being detected and boots just fine. >>> I meant usb stick of course… too much sdcards in my head today \o/. >> >> Odd. It looks to be there for me... >> >> $ git checkout 62718e304aa6 >> HEAD is now at 62718e304aa6... Merge tag 'usb-4.5-rc6' of >> git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb >> >> $ git grep -C3 "NOTE: This is required" -- drivers/usb/dwc2/core.c >> drivers/usb/dwc2/core.c- } >> drivers/usb/dwc2/core.c- >> drivers/usb/dwc2/core.c- /* >> drivers/usb/dwc2/core.c: * NOTE: This is required for some >> rockchip soc based >> drivers/usb/dwc2/core.c- * platforms. >> drivers/usb/dwc2/core.c- */ >> drivers/usb/dwc2/core.c- msleep(50); > > I unfortunately have bad news. > After some more testing it turned out that usb does NOT work (always) on rk3188 when reverting bd84f4ae9986. > It looks like that is dependent on which device / vendor is plugged in. > The usb stick I tested yesterday worked once but today just blinks shortly and then stops working. > Another usb stick I tested today doesn’t blink or work at all. Maybe I should have tested booting some more times :-( Just to clarify based on IRC conversation on #linux-rockchip: * My two patches work fine as per Michael (c0d3z3r0) and another person (mrjay). * My two patches _don't_ also allow us to revert to "50 ms" commit bd84f4ae9986 ("usb: dwc2: Add extra delay when forcing dr_mode"). This is Michael's "bad news". That means we could apply my two patches and the continue to work separately to figure out how to revert commit bd84f4ae9986 ("usb: dwc2: Add extra delay when forcing dr_mode"). Thanks! -Doug ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFT PATCH 1/2] usb: dwc2: Add a 10 ms delay to dwc2_core_reset() 2016-03-04 18:23 [RFT PATCH 1/2] usb: dwc2: Add a 10 ms delay to dwc2_core_reset() Douglas Anderson 2016-03-04 18:23 ` [RFT PATCH 2/2] Revert "usb: dwc2: Fix probe problem on bcm2835" Douglas Anderson 2016-03-04 22:54 ` [RFT PATCH 1/2] usb: dwc2: Add a 10 ms delay to dwc2_core_reset() Michael Niewoehner @ 2016-03-04 23:46 ` Karl Palsson 2016-03-05 0:36 ` Doug Anderson 2016-03-04 23:52 ` Sergei Shtylyov 2016-03-05 2:23 ` John Youn 4 siblings, 1 reply; 32+ messages in thread From: Karl Palsson @ 2016-03-04 23:46 UTC (permalink / raw) To: Douglas Anderson Cc: Heiko Stuebner, Julius Werner, Felipe Balbi, caesar.upstream, Greg Kroah-Hartman, huangtao, johnyoun, Linux Kernel Mailing List, Linux USB Mailing List, linux, repk, stefan.wahren [-- Attachment #1: Type: text/plain, Size: 1198 bytes --] -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Douglas Anderson <dianders@chromium.org> wrote: > > + /* > + * Sleep for 10-15 ms after the reset to let it finish. > + * > + * It's been confirmed on at least one version of the controller > + * that this is a requirement that this is a requirement in order for ^^ duplicate wording here. Cheers, Karl P -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (GNU/Linux) iQIcBAEBAgAGBQJW2h5AAAoJEBmotQ/U1cr2izQQAJ26Rnz1arSPnccUXyOilD9g vkf6wEEZNHsQUsnrzLbY9+x7rUAWgK6Wq+LYFv7vsOL63ogKpf6O52bZVE0/KQkT IABKZB312AecotpbSdsZBXaK1SYFUXSRd0CDHqnL9o7SBE2sNRVVd+e/h2z45hUg H2KkHZbzJA5btZveR+kkYX8PzV3QTBAmgqZ4YjI3uFllQtcRyJflYg9lJNm/GzpG +ckG/JDlfSfv8Y4C7CrvCot0iTktLXFgzYO8ftI2z8ZAbV2IP3kOf2sc+8b+TX34 yI3odnpf+N0lNQhJESQaYAbOLF45SLGbFK5cqi1zi0AuHJA2eTISOOZWo5Zl/nhE vneDG6zkS2q0YQRJlBIq23KxTdT9WJjW6qNs+OhVFo5k1900GWtuibfKr43g7JeF l2d5uVeL9trwDUNmMvyGelSRXL12DhJ/k3IX1TgVMPsfACbGFWS74nzWfdHYjTUS 48ou5a9QED632Na1ZsxhSa1Ce4IOn7Uhaa13WIjKqo8IZM5TXEWwTAczF/9lLpBM kz4Gb1tII5lQt0KpTOMHs/rXs0/k9iq0x0zuSUVNQEYJrAPhcJ/r+SBRJMqQb5Zy jzsMzWiuYrL7hpAjQv9s9vyJdQT+/IlIhgM3g+MiQat+LO3uUO10xIspK1+hIglJ A9VTP1o6Il8hRlQGrqLl =p21r -----END PGP SIGNATURE----- ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFT PATCH 1/2] usb: dwc2: Add a 10 ms delay to dwc2_core_reset() 2016-03-04 23:46 ` Karl Palsson @ 2016-03-05 0:36 ` Doug Anderson 0 siblings, 0 replies; 32+ messages in thread From: Doug Anderson @ 2016-03-05 0:36 UTC (permalink / raw) To: Karl Palsson Cc: Heiko Stuebner, Julius Werner, Felipe Balbi, caesar.upstream, Greg Kroah-Hartman, huangtao, johnyoun, Linux Kernel Mailing List, Linux USB Mailing List, linux, repk, stefan.wahren Hi, On Fri, Mar 4, 2016 at 3:46 PM, Karl Palsson <karlp@tweak.net.au> wrote: >> + /* >> + * Sleep for 10-15 ms after the reset to let it finish. >> + * >> + * It's been confirmed on at least one version of the controller >> + * that this is a requirement that this is a requirement in order for > > ^^ duplicate wording here. Thanks for catching. I'm happy to re-post with fixed wording or have a maintainer adjust it to this if/when it is applied: * It's been confirmed on at least one version of the * controller that this is a requirement in order for * everything to settle. Specifically if you: Please let me know if you'd like it re-posted. -Doug ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFT PATCH 1/2] usb: dwc2: Add a 10 ms delay to dwc2_core_reset() 2016-03-04 18:23 [RFT PATCH 1/2] usb: dwc2: Add a 10 ms delay to dwc2_core_reset() Douglas Anderson ` (2 preceding siblings ...) 2016-03-04 23:46 ` Karl Palsson @ 2016-03-04 23:52 ` Sergei Shtylyov 2016-03-05 2:23 ` John Youn 4 siblings, 0 replies; 32+ messages in thread From: Sergei Shtylyov @ 2016-03-04 23:52 UTC (permalink / raw) To: Douglas Anderson, johnyoun, balbi, Heiko Stuebner Cc: linux, caesar.upstream, huangtao, repk, stefan.wahren, Julius Werner, gregkh, linux-usb, linux-kernel Hello. On 03/04/2016 09:23 PM, Douglas Anderson wrote: > From testing and trying to make sense of the documentation, it appears > that a 10 ms delay is needed after resetting the core to make sure that > everything is stable and consistent. Let's add it. > > In my testing (on rk3288) this allows us to revert commit > 192cb07f7928 ("usb: dwc2: Fix probe problem on bcm2835"). Though I > could never reproduce the problems on my board, this might also allow us > to revert commit bd84f4ae9986 ("usb: dwc2: Add extra delay when forcing > dr_mode"). > > Signed-off-by: Douglas Anderson <dianders@chromium.org> > --- > drivers/usb/dwc2/core.c | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > > diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c > index 5e5a0f135b5a..8710b2d3e770 100644 > --- a/drivers/usb/dwc2/core.c > +++ b/drivers/usb/dwc2/core.c > @@ -277,6 +277,26 @@ int dwc2_core_reset(struct dwc2_hsotg *hsotg) > } > } while (!(greset & GRSTCTL_AHBIDLE)); > > + /* > + * Sleep for 10-15 ms after the reset to let it finish. > + * > + * It's been confirmed on at least one version of the controller > + * that this is a requirement that this is a requirement in order for Saying it once is enough. :-) [...] MBR, Sergei ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFT PATCH 1/2] usb: dwc2: Add a 10 ms delay to dwc2_core_reset() 2016-03-04 18:23 [RFT PATCH 1/2] usb: dwc2: Add a 10 ms delay to dwc2_core_reset() Douglas Anderson ` (3 preceding siblings ...) 2016-03-04 23:52 ` Sergei Shtylyov @ 2016-03-05 2:23 ` John Youn 4 siblings, 0 replies; 32+ messages in thread From: John Youn @ 2016-03-05 2:23 UTC (permalink / raw) To: Douglas Anderson, John.Youn, balbi, Heiko Stuebner Cc: linux, caesar.upstream, huangtao, repk, stefan.wahren, Julius Werner, gregkh, linux-usb, linux-kernel On 3/4/2016 10:23 AM, Douglas Anderson wrote: > From testing and trying to make sense of the documentation, it appears > that a 10 ms delay is needed after resetting the core to make sure that > everything is stable and consistent. Let's add it. > > In my testing (on rk3288) this allows us to revert commit > 192cb07f7928 ("usb: dwc2: Fix probe problem on bcm2835"). Though I > could never reproduce the problems on my board, this might also allow us > to revert commit bd84f4ae9986 ("usb: dwc2: Add extra delay when forcing > dr_mode"). > > Signed-off-by: Douglas Anderson <dianders@chromium.org> > --- > drivers/usb/dwc2/core.c | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > > diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c > index 5e5a0f135b5a..8710b2d3e770 100644 > --- a/drivers/usb/dwc2/core.c > +++ b/drivers/usb/dwc2/core.c > @@ -277,6 +277,26 @@ int dwc2_core_reset(struct dwc2_hsotg *hsotg) > } > } while (!(greset & GRSTCTL_AHBIDLE)); > > + /* > + * Sleep for 10-15 ms after the reset to let it finish. > + * > + * It's been confirmed on at least one version of the controller > + * that this is a requirement that this is a requirement in order for > + * everything to settle. Specifically if you: > + * - change GNPTXFSIZ or HPTXFSIZ before the reset > + * - do the reset > + * - read GNPTXFSIZ or HPTXFSIZ in a loop > + * ...you'll find that it takes almost exactly 10 ms for the registers > + * to return to their reset defaults. > + * > + * Note that it's possible that this 10 ms is the time referred to > + * in "Host Initialization" where it says to "Wait at least 10 ms for > + * the reset process to complete". In "Device Initialization" there > + * is also talk of a reset lasting 10 ms. That may be the source of > + * this delay. > + */ > + usleep_range(10000, 15000); > + > return 0; > } > > Hi Doug, Thanks for tracking this down. Caesar, Could you test these two commits on your rk3066 platform? And also see if it works after reverting bd84f4ae9986? Thanks, John ^ permalink raw reply [flat|nested] 32+ messages in thread
end of thread, other threads:[~2016-03-24 6:18 UTC | newest] Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-03-04 18:23 [RFT PATCH 1/2] usb: dwc2: Add a 10 ms delay to dwc2_core_reset() Douglas Anderson 2016-03-04 18:23 ` [RFT PATCH 2/2] Revert "usb: dwc2: Fix probe problem on bcm2835" Douglas Anderson 2016-03-04 22:54 ` Michael Niewoehner 2016-03-07 18:40 ` Stefan Wahren 2016-03-07 21:30 ` Doug Anderson 2016-03-08 17:49 ` Stefan Wahren 2016-03-09 19:01 ` Stefan Wahren 2016-03-09 19:06 ` Doug Anderson 2016-03-10 19:14 ` John Youn 2016-03-16 18:28 ` John Youn 2016-03-18 23:14 ` Stefan Wahren 2016-03-19 2:17 ` Eric Anholt 2016-03-19 7:44 ` Martin Sperl 2016-03-19 9:52 ` Stefan Wahren 2016-03-19 10:10 ` Martin Sperl 2016-03-19 12:09 ` Stefan Wahren 2016-03-19 9:45 ` Stefan Wahren 2016-03-19 5:21 ` Doug Anderson 2016-03-22 19:26 ` John Youn 2016-03-22 19:44 ` Doug Anderson 2016-03-22 20:37 ` John Youn 2016-03-24 6:18 ` John Youn 2016-03-04 22:54 ` [RFT PATCH 1/2] usb: dwc2: Add a 10 ms delay to dwc2_core_reset() Michael Niewoehner 2016-03-05 0:09 ` Michael Niewoehner 2016-03-05 0:33 ` Doug Anderson 2016-03-05 1:09 ` Doug Anderson 2016-03-05 20:41 ` Michael Niewoehner 2016-03-06 1:38 ` Doug Anderson 2016-03-04 23:46 ` Karl Palsson 2016-03-05 0:36 ` Doug Anderson 2016-03-04 23:52 ` Sergei Shtylyov 2016-03-05 2:23 ` John Youn
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).