lkml.org 
[lkml]   [2017]   [Mar]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v2 4/4] usb: dwc3: Workaround for super-speed host on dra7 in dual-role mode
From
Date


On 29/03/17 16:21, Felipe Balbi wrote:
>
> Hi,
>
> Roger Quadros <rogerq@ti.com> writes:
>>> Roger Quadros <rogerq@ti.com> writes:
>>>>> Roger Quadros <rogerq@ti.com> writes:
>>>>>> dra7 OTG core limits the host controller to USB2.0 (high-speed) mode
>>>>>> when we're operating in dual-role.
>>>>>
>>>>> yeah, that's not a quirk. DRA7 supports OTGv2, not OTGv3. There was no
>>>>> USB3 when OTGv2 was written.
>>>>>
>>>>> DRA7 just shouldn't use OTG core altogether. In fact, this is the very
>>>>> thing I've been saying for a long time. Make the simplest implementation
>>>>> possible. The dead simple, does-one-thing-only sort of implementation.
>>>>>
>>>>> All we need for Dual-Role (without OTG extras) is some input for ID and
>>>>> VBUS, then we add/remove HCD/UDC conditionally and set PRTCAPDIR.
>>>>>
>>>>
>>>> The catch is that on AM437x there is no way to get ID and VBUS events other
>>>> than the OTG controller so we have to rely on the OTG controller for that. :(
>>>
>>> okay, so AM437x can get OTG interrupts properly. That's fine. We can
>>> still do everything we need using code that's already existing in dwc3
>>> if we refactor it a bit and hook it up to the OTG IRQ handler.
>>>
>>> Here's what we do:
>>>
>>> * First we re-factor all necessary code around so the API for OTG/DRD
>>> is resumed to calling:
>>>
>>> dwc3_add_udc(dwc);
>>> dwc3_del_udc(dwc);
>>> dwc3_add_hcd(dwc);
>>> dwc3_del_hcd(dwc);
>>>
>>> the semantics of these should be easy to understand and you can
>>> implement each in their respective host.c/gadget.c files.
>>>
>>> * Second step is to modify our dwc3_init_mode() (or whatever that
>>> function was called, sorry, didn't check) to make sure we have
>>> something like:
>>>
>>> case OTG:
>>> dwc3_add_udc(dwc);
>>> break;
>>>
>>> We should *not* add HCD in this case yet.
>>>
>>> * After that we add otg.c (or drd.c, no preference) and make that call
>>> dwc3_add_udc(dwc) and, also, provide
>>> dwc3_add_otg(dwc)/dwc3_del_otg(dwc) calls. Then patch the switch
>>> statement above to:
>>>
>>> case OTG:
>>> dwc3_add_otg(dwc);
>>> break;
>>>
>>> Note that at this point, this is simply a direct replacement of
>>> dwc3_add_udc() to dwc3_add_otg(). This should maintain current behavior
>>> (which is starting with peripheral mode by default), but it should also
>>> add support for OTG interrupts to change the mode (from an interrupt
>>> thead)
>>>
>>> otg_isr()
>>> {
>>>
>>> /* don't forget to remove preivous mode if necessary */
>>> if (perimode)
>>> dwc3_add_udc(dwc);
>>> else
>>> dwc3_add_hcd(dwc);
>>> }
>>>
>>> * The next patch would be to choose default conditionally based on
>>> PERIMODE or whatever.
>>>
>>> Of course, this is an oversimplified view of reality. You still need to
>>> poke around at PRTCAPDIR, etc. But all this can, actually, be prototyped
>>> using our "mode" debugfs file. Just make that call
>>> dwc3_add/del_udc/hcd() apart from fiddling with PRTCAPDIR in GCTL.
>>>
>>> Your first implementation could be just that. Refactoring what needs to
>>> be refactored, then patching "mode" debugfs to work properly in that
>>> case. Only add otg.c/drd.c after "mode" debugfs file is stable, because
>>> then you know what needs to be taken into consideration.
>>>
>>> Just to be clear, I'm not saying we should *ONLY* get the debugfs
>>> interface for v4.12, I'm saying you should start with that and get that
>>> stable and working properly (make an infinite loop constantly changing
>>> modes and keep it running over the weekend) before you add support for
>>> OTG interrupts, which could come in the same series ;-)
>>>
>>
>> Agree with you. Moreover I could get rid of OTG controller related code
>> and have just debugfs and extcon implementation. We can add the OTG controller
>> bits later.
>>
>> I agree with you on everything you said except using add/del_gadget_udc. :)
>> I've explained why we can't use del_gadget_udc in the other thread
>> but I'll explain it here again.
>>
>> 1) If we start in host role, usb_add_gadget_udc() won't be called. That means
>> no UDC and user can't load a gadget driver. Typical applications need to have
>> a gadget driver ready *before* the peripheral mode starts so that it can
>> enumerate immediately.
>
> that has changed since you started writing this series :-) gadget
> drivers are kept in pending list until a UDC is around. I'll get
> information on that tomorrow, if you require.

"until a UDC is around" is the key point. If we never call usb_add_gadget_udc()
or we call usb_del_gadget_udc() then the UDC is not around right?
>
>> 2) If we use usb_del_gadget_udc() when switching to host mode and
>> usb_add_gadget_udc() when switching back to peripheral mode, the previously
>> loaded gadget driver will not be assigned to this UDC. User has to unload
>> and reload the gadget driver.
>
> that should not be the case anymore, if it is we have a bug in udc-core

OK. good to know.
>
>> 3) All this becomes even more complex for configfs based gadget driver.
>>
>> So using stop/start gadget is a much simpler solution really as UDC software
>> side of things remain unchanged and the gadget driver can persist between
>> role switches.
>
> I hadn't considered configfs, I'll try this out tomorrow as well.
>
Al-right, thanks.

cheers,
-roger

\
 
 \ /
  Last update: 2017-03-29 16:01    [W:0.068 / U:1.432 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site