Message-ID: <3c95b592d78aa569de33d420c4c93018@codeaurora.org>
Date:   Thu, 19 Jan 2017 20:45:50 +0530
From:   vivek.gautam@...eaurora.org
To:     Roger Quadros <rogerq@...com>
Cc:     Felipe Balbi <balbi@...nel.org>,
        Peter Chen <hzpeterchen@...il.com>,
        Yoshihiro Shimoda <yoshihiro.shimoda.uh@...esas.com>,
        peter.chen@...escale.com, Tony Lindgren <tony@...mide.com>,
        Greg KH <gregkh@...uxfoundation.org>, dan.j.williams@...el.com,
        Mathias Nyman <mathias.nyman@...ux.intel.com>,
        Joao.Pinto@...opsys.com,
        Sergei Shtylyov <sergei.shtylyov@...entembedded.com>,
        jun.li@...escale.com, grygorii.strashko@...com,
        Rob Herring <robh@...nel.org>, nsekhar@...com, b-liu@...com,
        Joe Perches <joe@...ches.com>,
        Linux USB Mailing List <linux-usb@...r.kernel.org>,
        linux-omap@...r.kernel.org, linux-kernel@...r.kernel.org,
        devicetree@...r.kernel.org
Subject: Re: [PATCH v11 08/14] usb: otg: add OTG/dual-role core

Hi Roger,

On 2017-01-19 17:45, Roger Quadros wrote:
> Vivek,
> 
> On 19/01/17 13:56, Vivek Gautam wrote:
>> Hi,
>> 
>> 
>> On Wed, Jun 22, 2016 at 2:00 PM, Roger Quadros <rogerq@...com> wrote:
>> 
>> Luckily hit this thread while checking about DRD role functionality 
>> for DWC3.
>> 
>>> On 22/06/16 11:14, Felipe Balbi wrote:
>>>> 
>>>> Hi,
>>>> 
>>>> Roger Quadros <rogerq@...com> 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.
>> 
>> I will talk about the msm platforms using dwc3 hardware.
>> DWC3 controller on msm doesn't seem to have full otg functionality,
>> and the driver makes use of switching between host and device
>> using PRTCAPDIR register in of the core [1].
>> test
>> We plan to support this DRD role switching (swapping host and device
>> functionality based on id/vbus interrupts) in upstream.
>> 
>> Do we see a valid case to have this framework?
> 
> Felipe wanted to have a minimal dual-role logic inside dwc3 which is
> independent of any DRD/OTG framework.
> 
> I have implemented this and will send out patches today for review.

Okay, good to know that. I will be happy to take a look at the
patches and test them for msm.

Thanks for sharing the info.

> 
>> Or, may be add a 'drd' layer for dwc3 that handles
>> role switching (using PRTCAPDIR) based on the id/vbus extcon 
>> notifications.
>> 
>> 
>> [1] 
>> https://source.codeaurora.org/quic/la/kernel/msm-3.18/tree/drivers/usb/dwc3/dwc3-msm.c?h=msm-3.18
>>      "dwc3_otg_start_host()"
>>      "dwc3_otg_start_peripheral()"
>> 
>> 
> 
> regards,
> -roger


Regards
Vivek

Powered by blists - more mailing lists