Re: [PATCH v2 4/4] usb: dwc3: Workaround for super-speed host on dra7 in dual-role mode

[Felipe Balbi <balbi@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 3466 bytes --]


Hi,

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 ;-)

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]