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

From: John Youn
Date: Tue Mar 22 2016 - 16:37:36 EST


On 3/22/2016 12:44 PM, Doug Anderson wrote:
> John,
>
> On Tue, Mar 22, 2016 at 12:26 PM, John Youn <John.Youn@xxxxxxxxxxxx> 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