Re: [PATCH v11 08/14] usb: otg: add OTG/dual-role core

[Roger Quadros]

On 22/06/16 11:14, Felipe Balbi wrote:
> 
> Hi,
> 
> Roger Quadros <rogerq@xxxxxx> writes:
>>>>>>>>>> For the real use case, some Carplay platforms need it.
>>>>>>>>>
>>>>>>>>> Carplay does *NOT* rely on OTG. Apple has its own proprietary and closed
>>>>>>>>> specification which is not OTG-compliant.
>>>>>>>>>
>>>>>>>>
>>>>>>>> Yes, it is not OTG-compliant, but it can co-work with some standard OTG FSM
>>>>>>>> states to finish role swap.
>>>>>>>
>>>>>>> What are you referring to as "finish role swap"? I don't get that.
>>>>>>
>>>>>> Change current role from host to peripheral.
>>>>>
>>>>> Okay, we have two scenarios here:
>>>>>
>>>>> 1. You need full OTG compliance
>>>>>
>>>>> 	For this, granted, you need the state machine if your HW doesn't
>>>>> 	track it. This is a given. With only one user, however, perhaps
>>>>> 	we don't need a generic layer. There are not enough different
>>>>> 	setups to design a good enough generic layer. We will end up
>>>>> 	with a pseudo-generic framework which is coupled with its only
>>>>> 	user.
>>>>>
>>>>> 2. Dual-role support, without OTG compliance
>>>>>
>>>>> 	In this case, you don't need a stack. All you need is a signal
>>>>> 	to tell you state of ID pin and another to tell you state of
>>>>> 	VBUS level. If you have those, you don't need to walk an OTG
>>>>> 	state machine at all. You don't need any of those quirky OTG
>>>>> 	timers, agreed?
>>>>>
>>>>> 	Given the above, why would you even want to use a subset of OTG
>>>>> 	state machine to implement something that's _usually_ as simple
>>>>> 	as:
>>>>>
>>>>> 8<----------------------------------------------------------------------
>>>>> 	vbus = read(VBUS_STATE); /* could be a gpio_get_value() */
>>>>>         id = read(ID_STATE); /* could be a gpio_get_value() */
>>>>>
>>>>>         set_role(id);
>>>>>         set_vbus(vbus);
>>>>> ------------------------------------------------------------------------
>>>>>
>>>>
>>>> In fact, the individual driver can do it by itself. The chipidea driver
>>>> handles OTG and dual-role well currently. By considering this OTG/DRD
>>>> framework is worthwhile or not, we would like to see if it can
>>>> simplify DRD design for each driver, and can benefit the platforms which
>>>> has different drivers for host and peripheral to finish the role switch
>>>> well.
>>>
>>> simplify how?  By adding unnecessary workqueues and a level indirection
>>> that just goes back to the same driver?
>>
>> What do you mean by same driver?
> 
> dwc3 registers to OTG layer. dwc3 also registers as UDC to UDC
> layer. When dwc3 OTG IRQ fires, dwc3 tells OTG layer about it and OTG
> layer jumps to a callback that goes back to dwc3 to e.g. start
> peripheral side.
> 
> See ?!? Starts on dwc3, goes to OTG layer, goes back to DWC3.
> 
>> Gadget driver, host driver and PHY (or MUX) driver (for ID/VBUS) can
>> be 3 totally independent drivers unlike dwc3 where you have a single
>> driver in control of both host and gadget.
> 
> That's a totally different issue and one not being tackled by OTG
> layer, because there are no such users yet. We can't design anything
> based solely on speculation of what might happen.
> 
> If there aren't enough users, there is no way to design a good generic
> layer.
> 
>> Questions not clear to me are:
>>
>> 1) Which driver handles ID/VBUS events and makes a decision to do the
>> role swap?  Probably the PHY/MUX driver?
> 
> This is implementation dependent. For TI's USB subsystem, we have PMIC
> sampling VBUS/ID that and using EXTCON to tell dwc3-omap to program UTMI
> mailbox. The same mailbox can be used in HW-mode (see AM437x) where SW
> has no intervention.
> 
> For Intel's USB subsystem, we have PMIC sampling VBUS/ID with an
> internal mux (much like TI's UTMI mailbox, but slightly different) to
> switch between a separate XHCI or a separate dwc3. The same mux can be
> put in HW-mode where SW has no intervention.
> 
> In any case, for Intel's stuff most of the magic happens in ASL. Our PHY
> driver just detects role (at least for Type-C based plats) and executes
> _DSM with correct arguments [1]. _DSM will program internal MUX, toggle
> VBUS and, for type-C, toggle VCONN when needed.
> 
>> 2) How does it perform the role swap? Probably a register write to the
>> PHY/MUX without needing to stop/start controllers? Easy case is both
>> controllers can run in co-existence without interference. Is there any
>> platform other than dwc3 where this is not the case?
> 
> Again speculation. But to answer your question, only dwc3 is in such a
> case today. But even for dwc3 we can have DRD with a much, much simpler
> setup as I have already explained.
> 
>> 3) Even if host and gadget controllers can operate in coexistence,
>> there is no need for both to be running for embedded applications
>> which are usually power conservative.  How can we achieve that?
> 
> Now you're also speculating that you're running on embedded applications
> and that we _can_ power off parts of the IP. I happen to know that we
> can't power off XHCI part of dwc3 in TI's SoC because that's fed by same
> Clocks and power rails as the peripheral side.
> 
> [1] https://lkml.org/lkml/2016/6/21/658
> 
For TI's case it is dwc3 and you are implementing the role swap in the dwc3
driver where you do intend to remove the XHCI platform device. So I'm not
much concerned about that.

I was concerned about other platforms. I guess I'll let the other platform
people speak up as to what they need.

--
cheers,
-roger

Attachment: signature.asc
Description: OpenPGP digital signature