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

From: Doug Anderson
Date: Sat Mar 05 2016 - 20:39:13 EST


Hi,

On Sat, Mar 5, 2016 at 12:41 PM, Michael Niewoehner
<linux@xxxxxxxxxxxxxx> wrote:
> Hi Douglas,
> Hi John,
>
> Am 05.03.2016 um 01:33 schrieb Doug Anderson <dianders@xxxxxxxxxxxx>:
>
>> Michael,
>>
>> On Fri, Mar 4, 2016 at 4:09 PM, Michael Niewoehner <linux@xxxxxxxxxxxxxx> 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@xxxxxxxxxxxx>
>>>>
>>>> Tested-by: Michael Niewoehner <linux@xxxxxxxxxxxxxx>
>>
>> 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