Hi,
Roger Quadros <rogerq@xxxxxx> writes:
>> Roger Quadros <rogerq@xxxxxx> writes:
>>>> Roger Quadros <rogerq@xxxxxx> 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.
> 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
> 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.
--
balbi
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
