On 9/15/2016 12:08 AM, Stefan Wahren wrote:
> Hi John,
>
> [add Felipe and Greg]
>
> Am 14.09.2016 um 23:10 schrieb John Youn:
>> On 9/14/2016 7:05 AM, Heiko St?bner wrote:
>>> Am Dienstag, 13. September 2016, 12:04:12 schrieb John Youn:
>>>> On 9/13/2016 11:39 AM, Heiko St?bner wrote:
>>>>> Am Dienstag, 13. September 2016, 20:07:25 schrieb Stefan Wahren:
>>>>>> Hi Heiko,
>>>>>>
>>>>>>> Heiko Stuebner <heiko at sntech.de> hat am 12. September 2016 um 13:05
>>>>>>> geschrieben:
>>>>>>>
>>>>>>>
>>>>>>> Hi Stefan,
>>>>>>>
>>>>>>> Am Montag, 12. September 2016, 07:20:44 CEST schrieb Stefan Wahren:
>>>>>>>>> Heiko Stuebner <heiko at sntech.de> hat am 11. September 2016 um 23:19
>>>>>>>>> geschrieben:
>>>>>>>>>
>>>>>>>>> Am Mittwoch, 7. September 2016, 19:39:43 CEST schrieb John Youn:
>>>>>>>>>> When a force mode bit is set and the IDDIG debounce filter is
>>>>>>>>>> enabled,
>>>>>>>>>> there is a delay for the forced mode to take effect. This delay is
>>>>>>>>>> due
>>>>>>>>>> to the IDDIG debounce filter and is variable depending on the
>>>>>>>>>> platform's
>>>>>>>>>> PHY clock speed. To account for this delay we can poll for the
>>>>>>>>>> expected
>>>>>>>>>> mode.
>>>>>>>>>>
>>>>>>>>>> On a clear force mode, since we don't know what mode to poll for,
>>>>>>>>>> delay
>>>>>>>>>> for a fixed 100 ms. This is the maximum delay based on the slowest
>>>>>>>>>> PHY
>>>>>>>>>> clock speed.
>>>>>>>>>>
>>>>>>>>>> Tested-by: Stefan Wahren <stefan.wahren at i2se.com>
>>>>>>>>>> Signed-off-by: John Youn <johnyoun at synopsys.com>
>>>>>>>>>> ---
>>>>>>>>> [...]
>>>>>>>>>
>>>>>>>>>> @@ -475,12 +478,6 @@ void dwc2_force_dr_mode(struct dwc2_hsotg
>>>>>>>>>> *hsotg)
>>>>>>>>>>
>>>>>>>>>> __func__, hsotg->dr_mode);
>>>>>>>>>>
>>>>>>>>>> break;
>>>>>>>>>>
>>>>>>>>>> }
>>>>>>>>>>
>>>>>>>>>> -
>>>>>>>>>> - /*
>>>>>>>>>> - * NOTE: This is required for some rockchip soc based
>>>>>>>>>> - * platforms.
>>>>>>>>>> - */
>>>>>>>>>> - msleep(50);
>>>>>>>>>>
>>>>>>>>>> }
>>>>>>>>> sorry for not finding the time to test your subsequent versions, but
>>>>>>>>> this
>>>>>>>>> still
>>>>>>>>> acts up on my Rockchip boards, as I'm still running into errors like
>>>>>>>>>
>>>>>>>>> [ 4.875570] usb usb2-port1: connect-debounce failed
>>>>>>>> could you please name the relevant DTS file of the affected boards?
>>>>>>> So far I've been able to see that on
>>>>>>>
>>>>>>> rk3188-radxarock
>>>>>>> rk3036-kylin
>>>>>>>
>>>>>>> both on host-only dwc2 controllers.
>>>>>> thanks, does patch 1 & 2 already have a negative effect on these
>>>>>> controllers?
>>>>> nope, patches 1+2 alone are ok and only the msleep(50) going away causes
>>>>> problems. I'm back home now, so I'll hopefully have time to check
>>>>> host-only
>>>>> vs. otg dwc2 controllers tomorrow as well.
>>>> Ok, thanks for testing Heiko.
>>>>
>>>> Given that, let's try to fix this in the next -rc (or before) because
>>>> I think the Raspberry Pi needs the other parts of that last patch. We
>>>> can just revert the msleep(50) if needed.
>>> I could nicely confirm, that only the host-only controller seems affected by
>>> this. Same kernel image that fails on the host-only one with the "connect-
>>> debounce failed" message seems to work once I switch that over to the otg
>>> controller and the system comes up nicely again.
>>>
>>> So if we don't find the root cause in the short term, maybe we could at least
>>> limit the delay for host-only variants, like
>>>
>>> ---------------------- 8< -------------------
>>> diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c
>>> index fa9b26b..4c0fa0b 100644
>>> --- a/drivers/usb/dwc2/core.c
>>> +++ b/drivers/usb/dwc2/core.c
>>> @@ -463,9 +463,18 @@ static void dwc2_clear_force_mode(struct dwc2_hsotg *hsotg)
>>> */
>>> void dwc2_force_dr_mode(struct dwc2_hsotg *hsotg)
>>> {
>>> + bool ret;
>>> +
>>> switch (hsotg->dr_mode) {
>>> case USB_DR_MODE_HOST:
>>> - dwc2_force_mode(hsotg, true);
>>> + ret = dwc2_force_mode(hsotg, true);
>>> + /*
>>> + * NOTE: This is required for some rockchip soc based
>>> + * platforms on their host-only dwc2.
>>> + */
>>> + if (!ret)
>>> + msleep(50);
>>> +
>>> break;
>>> case USB_DR_MODE_PERIPHERAL:
>>> dwc2_force_mode(hsotg, false);
>>> ---------------------- 8< -------------------
>>>
>>> I've been running with that change for some time now on both the
>>> rk3036-kylin and rk3188-radxarock. Plugged and unplugged different
>>> combinations of usb-devices on the usb otg and host ports, so far
>>> seems to work.
>>>
>> Ok great. We'll use that for the -rc if we can't find the root cause.
>>
>> Then hopefully we can apply the final dropped patch for 4.10.
>
> unfortunately i noticed too late that Felipe and Greg didn't get the
> whole conversation. The series v5 has been merged in Greg's USB tree.
>
Yeah, we'll have to fix in -rc. Either revert the last patch or add
the msleep.
Regards,
John
