[RFT PATCH 1/2] usb: dwc2: Add a 10 ms delay to dwc2_core_reset()

[RFT PATCH 1/2] usb: dwc2: Add a 10 ms delay to dwc2_core_reset()

[Douglas Anderson]

>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

[RFT PATCH 2/2] Revert "usb: dwc2: Fix probe problem on bcm2835"

[Douglas Anderson]

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

Re: [RFT PATCH 1/2] usb: dwc2: Add a 10 ms delay to dwc2_core_reset()

[Michael Niewoehner]

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

Re: [RFT PATCH 2/2] Revert "usb: dwc2: Fix probe problem on bcm2835"

[Michael Niewoehner]

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
> 

Re: [RFT PATCH 1/2] usb: dwc2: Add a 10 ms delay to dwc2_core_reset()

[Karl Palsson]

[-- 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-----

Re: [RFT PATCH 1/2] usb: dwc2: Add a 10 ms delay to dwc2_core_reset()

[Sergei Shtylyov]

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

Re: [RFT PATCH 1/2] usb: dwc2: Add a 10 ms delay to dwc2_core_reset()

[Michael Niewoehner]

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

Re: [RFT PATCH 1/2] usb: dwc2: Add a 10 ms delay to dwc2_core_reset()

[Doug Anderson]

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);

Re: [RFT PATCH 1/2] usb: dwc2: Add a 10 ms delay to dwc2_core_reset()

[Doug Anderson]

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

Re: [RFT PATCH 1/2] usb: dwc2: Add a 10 ms delay to dwc2_core_reset()

[Doug Anderson]

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

Re: [RFT PATCH 1/2] usb: dwc2: Add a 10 ms delay to dwc2_core_reset()

[John Youn]

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

Re: [RFT PATCH 1/2] usb: dwc2: Add a 10 ms delay to dwc2_core_reset()

[Michael Niewoehner]

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

Re: [RFT PATCH 1/2] usb: dwc2: Add a 10 ms delay to dwc2_core_reset()

[Doug Anderson]

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

Re: [RFT PATCH 2/2] Revert "usb: dwc2: Fix probe problem on bcm2835"

[Stefan Wahren]

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

Re: [RFT PATCH 2/2] Revert "usb: dwc2: Fix probe problem on bcm2835"

[Doug Anderson]

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

Re: [RFT PATCH 2/2] Revert "usb: dwc2: Fix probe problem on bcm2835"

[Stefan Wahren]

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

Re: [RFT PATCH 2/2] Revert "usb: dwc2: Fix probe problem on bcm2835"

[Stefan Wahren]

> 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

Re: [RFT PATCH 2/2] Revert "usb: dwc2: Fix probe problem on bcm2835"

[Doug Anderson]

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?

Re: [RFT PATCH 2/2] Revert "usb: dwc2: Fix probe problem on bcm2835"

[John Youn]

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

Re: [RFT PATCH 2/2] Revert "usb: dwc2: Fix probe problem on bcm2835"

[John Youn]

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

Re: [RFT PATCH 2/2] Revert "usb: dwc2: Fix probe problem on bcm2835"

[Stefan Wahren]

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

Re: [RFT PATCH 2/2] Revert "usb: dwc2: Fix probe problem on bcm2835"

[Eric Anholt]

[-- 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 --]

Re: [RFT PATCH 2/2] Revert "usb: dwc2: Fix probe problem on bcm2835"

[Doug Anderson]

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

Re: [RFT PATCH 2/2] Revert "usb: dwc2: Fix probe problem on bcm2835"

[Martin Sperl]

> 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

Re: [RFT PATCH 2/2] Revert "usb: dwc2: Fix probe problem on bcm2835"

[Stefan Wahren]

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.

Re: [RFT PATCH 2/2] Revert "usb: dwc2: Fix probe problem on bcm2835"

[Stefan Wahren]

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
>
>

Re: [RFT PATCH 2/2] Revert "usb: dwc2: Fix probe problem on bcm2835"

[Martin Sperl]

> 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

Re: [RFT PATCH 2/2] Revert "usb: dwc2: Fix probe problem on bcm2835"

[Stefan Wahren]

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

Re: [RFT PATCH 2/2] Revert "usb: dwc2: Fix probe problem on bcm2835"

[John Youn]

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

Re: [RFT PATCH 2/2] Revert "usb: dwc2: Fix probe problem on bcm2835"

[Doug Anderson]

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

Re: [RFT PATCH 2/2] Revert "usb: dwc2: Fix probe problem on bcm2835"

[John Youn]

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

Re: [RFT PATCH 2/2] Revert "usb: dwc2: Fix probe problem on bcm2835"

[John Youn]

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