Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1411746 > unrolled thread

[PATCH v10 2/7] usb: mux: add generic code for dual role port mux

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v10 2/7] usb: mux: add generic code for dual role port mux Lu Baolu <baolu.lu@linux.intel.com> - 2016-06-02 03:40 +0200
    Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux Peter Chen <hzpeterchen@gmail.com> - 2016-06-03 09:50 +0200
      Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux Heikki Krogerus <heikki.krogerus@linux.intel.com> - 2016-06-03 10:20 +0200
        Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux Peter Chen <hzpeterchen@gmail.com> - 2016-06-03 11:30 +0200
      Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux Lu Baolu <baolu.lu@linux.intel.com> - 2016-06-03 18:10 +0200
        Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux Peter Chen <hzpeterchen@gmail.com> - 2016-06-04 04:40 +0200
          Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux Lu Baolu <baolu.lu@linux.intel.com> - 2016-06-05 09:00 +0200
            RE: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux Jun Li <jun.li@nxp.com> - 2016-06-05 10:40 +0200
              Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux Lu Baolu <baolu.lu@linux.intel.com> - 2016-06-05 10:50 +0200
                RE: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux Jun Li <jun.li@nxp.com> - 2016-06-06 03:10 +0200
                  Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux Lu Baolu <baolu.lu@linux.intel.com> - 2016-06-06 04:40 +0200
                Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux Peter Chen <hzpeterchen@gmail.com> - 2016-06-06 04:20 +0200
                  Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux Lu Baolu <baolu.lu@linux.intel.com> - 2016-06-06 04:50 +0200
                    Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux Peter Chen <hzpeterchen@gmail.com> - 2016-06-06 09:00 +0200
            Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux Peter Chen <hzpeterchen@gmail.com> - 2016-06-06 03:40 +0200
              Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux Lu Baolu <baolu.lu@linux.intel.com> - 2016-06-06 05:10 +0200
                Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux Peter Chen <hzpeterchen@gmail.com> - 2016-06-06 09:10 +0200
                  Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux Lu Baolu <baolu.lu@linux.intel.com> - 2016-06-06 09:40 +0200
                Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux Roger Quadros <rogerq@ti.com> - 2016-06-06 09:10 +0200
                  RE: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux Jun Li <jun.li@nxp.com> - 2016-06-07 05:20 +0200
                    Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux Lu Baolu <baolu.lu@linux.intel.com> - 2016-06-07 08:30 +0200
                      RE: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux Jun Li <jun.li@nxp.com> - 2016-06-07 09:10 +0200
                        Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux Lu Baolu <baolu.lu@linux.intel.com> - 2016-06-07 11:30 +0200
                          Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux Roger Quadros <rogerq@ti.com> - 2016-06-07 14:50 +0200
                    Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux Lu Baolu <baolu.lu@linux.intel.com> - 2016-06-07 12:00 +0200
                      Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux Roger Quadros <rogerq@ti.com> - 2016-06-07 15:00 +0200
                        Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux Felipe Balbi <felipe.balbi@linux.intel.com> - 2016-06-07 15:10 +0200
                          Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux Roger Quadros <rogerq@ti.com> - 2016-06-07 16:10 +0200
                            Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux Felipe Balbi <felipe.balbi@linux.intel.com> - 2016-06-07 17:10 +0200
                              RE: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux Jun Li <jun.li@nxp.com> - 2016-06-08 05:10 +0200
                                RE: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux Jun Li <jun.li@nxp.com> - 2016-06-08 08:30 +0200
                              Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux Peter Chen <hzpeterchen@gmail.com> - 2016-06-08 08:40 +0200
                              Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux Roger Quadros <rogerq@ti.com> - 2016-06-08 10:00 +0200

Page 2 of 2 — ← Prev page 1 [2]

#1415740

Hi Jun,

On 06/07/2016 11:03 AM, Jun Li wrote:
> Hi Roger
>
>>  
>> For Mux devices implementing dual-role, the mux device driver _must_ use
>> OTG/dual-role core API so that a common ABI is presented to user space for
>> OTG/dual-role.
> That's the only point we have concern, do dual role switch through
> OTG/dual-role core, not do it by itself.

That really depends on how do you define "dual role". Can you please
provide an unambiguous definition of "dual role" used in OTG/dual-role
framework?

Best regards,
Lu Baolu

>
>> I haven't yet looked at the mux framework but if we take care of the above
>> point then we are not introducing any redundancy.
>>
> Roger, actually this is my worry on OTG core: those dual role switch
> users just tends to do it simply by itself(straightforward and easy),
> not through the OTG core(some complicated in first look),
> this is just an example for us to convince people to select a better
> way:)
>
> Li Jun
>

[toc] | [prev] | [next] | [standalone]

#1415774

Hi Baolu

> -----Original Message-----
> From: Lu Baolu [mailto:baolu.lu@linux.intel.com]
> Sent: Tuesday, June 07, 2016 2:27 PM
> To: Jun Li <jun.li@nxp.com>; Roger Quadros <rogerq@ti.com>; Peter Chen
> <hzpeterchen@gmail.com>
> Cc: felipe.balbi@linux.intel.com; Mathias Nyman <mathias.nyman@intel.com>;
> Greg Kroah-Hartman <gregkh@linuxfoundation.org>; Lee Jones
> <lee.jones@linaro.org>; Heikki Krogerus <heikki.krogerus@linux.intel.com>;
> Liam Girdwood <lgirdwood@gmail.com>; Mark Brown <broonie@kernel.org>;
> linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port
> mux
> 
> Hi Jun,
> 
> On 06/07/2016 11:03 AM, Jun Li wrote:
> > Hi Roger
> >
> >>
> >> For Mux devices implementing dual-role, the mux device driver _must_
> >> use OTG/dual-role core API so that a common ABI is presented to user
> >> space for OTG/dual-role.
> > That's the only point we have concern, do dual role switch through
> > OTG/dual-role core, not do it by itself.
> 
> That really depends on how do you define "dual role". Can you please
> provide an unambiguous definition of "dual role" used in OTG/dual-role
> framework?

Host and peripheral.

> 
> Best regards,
> Lu Baolu
> 
> >
> >> I haven't yet looked at the mux framework but if we take care of the
> >> above point then we are not introducing any redundancy.
> >>
> > Roger, actually this is my worry on OTG core: those dual role switch
> > users just tends to do it simply by itself(straightforward and easy),
> > not through the OTG core(some complicated in first look), this is just
> > an example for us to convince people to select a better
> > way:)
> >
> > Li Jun
> >

[toc] | [prev] | [next] | [standalone]

#1415917

Hi,

On 06/07/2016 02:34 PM, Jun Li wrote:
>> > On 06/07/2016 11:03 AM, Jun Li wrote:
>>> > > Hi Roger
>>> > >
>>>> > >>
>>>> > >> For Mux devices implementing dual-role, the mux device driver _must_
>>>> > >> use OTG/dual-role core API so that a common ABI is presented to user
>>>> > >> space for OTG/dual-role.
>>> > > That's the only point we have concern, do dual role switch through
>>> > > OTG/dual-role core, not do it by itself.
>> > 
>> > That really depends on how do you define "dual role". Can you please
>> > provide an unambiguous definition of "dual role" used in OTG/dual-role
>> > framework?
> Host and peripheral.
>

This is definitely ambiguous.

By reading OTG/dual-role code, my understanding is that "dual-role" is a
"reduced OTG" which is for DRD devices lacking of some OTG negotiation
protocols.

We really can't say "it's the scope of OTG/dual-role" whenever it comes to
"host and peripheral".

Best regards,
Lu Baolu

[toc] | [prev] | [next] | [standalone]

#1416134

On 07/06/16 12:27, Lu Baolu wrote:
> Hi,
> 
> On 06/07/2016 02:34 PM, Jun Li wrote:
>>>> On 06/07/2016 11:03 AM, Jun Li wrote:
>>>>>> Hi Roger
>>>>>>
>>>>>>>>
>>>>>>>> For Mux devices implementing dual-role, the mux device driver _must_
>>>>>>>> use OTG/dual-role core API so that a common ABI is presented to user
>>>>>>>> space for OTG/dual-role.
>>>>>> That's the only point we have concern, do dual role switch through
>>>>>> OTG/dual-role core, not do it by itself.
>>>>
>>>> That really depends on how do you define "dual role". Can you please
>>>> provide an unambiguous definition of "dual role" used in OTG/dual-role
>>>> framework?
>> Host and peripheral.
>>
> 
> This is definitely ambiguous.
> 
> By reading OTG/dual-role code, my understanding is that "dual-role" is a
> "reduced OTG" which is for DRD devices lacking of some OTG negotiation
> protocols.

DRD means dual role with zero OTG features, which is similar to just host and
peripheral mode.

> 
> We really can't say "it's the scope of OTG/dual-role" whenever it comes to
> "host and peripheral".

What other combination you foresee?

cheers,
-roger

[toc] | [prev] | [next] | [standalone]

#1415953

Hi,

On 06/07/2016 11:03 AM, Jun Li wrote:
> Hi Roger
>
>>  
>> For Mux devices implementing dual-role, the mux device driver _must_ use
>> OTG/dual-role core API so that a common ABI is presented to user space for
>> OTG/dual-role.
> That's the only point we have concern, do dual role switch through
> OTG/dual-role core, not do it by itself.
>
>> I haven't yet looked at the mux framework but if we take care of the above
>> point then we are not introducing any redundancy.
>>
> Roger, actually this is my worry on OTG core: those dual role switch
> users just tends to do it simply by itself(straightforward and easy),
> not through the OTG core(some complicated in first look),

I'm sorry, but I'm really confused.

Why do we need to drop "straightforward and easy", but have to run
an *unnecessary* OTG state machine? Don't you think that will (1) add
*unnecessary* software complexity; (2) increase *unnecessary* memory
footprint; and (3) increase the debugging efforts?

> this is just an example for us to convince people to select a better
> way:)

Sure. Let's take my case for an example.

My system has a third-party port mux, which is not part any USB controllers.
Also, my system doesn't have any DRD capable devices. I need a
"straightforward and easy" driver for it. Otherwise, the system could not be
waken up from system suspend.

But you said I must run an unnecessary OTG state machine, even thought it
has nothing to do with my system, only because the two sides of my port
mux device is a host and peripheral controller.

Why?

Best regards,
Lu Baolu

[toc] | [prev] | [next] | [standalone]

#1416139

On 07/06/16 12:53, Lu Baolu wrote:
> Hi,
> 
> On 06/07/2016 11:03 AM, Jun Li wrote:
>> Hi Roger
>>
>>>  
>>> For Mux devices implementing dual-role, the mux device driver _must_ use
>>> OTG/dual-role core API so that a common ABI is presented to user space for
>>> OTG/dual-role.
>> That's the only point we have concern, do dual role switch through
>> OTG/dual-role core, not do it by itself.
>>
>>> I haven't yet looked at the mux framework but if we take care of the above
>>> point then we are not introducing any redundancy.
>>>
>> Roger, actually this is my worry on OTG core: those dual role switch
>> users just tends to do it simply by itself(straightforward and easy),
>> not through the OTG core(some complicated in first look),
> 
> I'm sorry, but I'm really confused.
> 
> Why do we need to drop "straightforward and easy", but have to run
> an *unnecessary* OTG state machine? Don't you think that will (1) add
> *unnecessary* software complexity; (2) increase *unnecessary* memory
> footprint; and (3) increase the debugging efforts?
> 
>> this is just an example for us to convince people to select a better
>> way:)
> 
> Sure. Let's take my case for an example.
> 
> My system has a third-party port mux, which is not part any USB controllers.
> Also, my system doesn't have any DRD capable devices. I need a
> "straightforward and easy" driver for it. Otherwise, the system could not be
> waken up from system suspend.
> 
> But you said I must run an unnecessary OTG state machine, even thought it
> has nothing to do with my system, only because the two sides of my port
> mux device is a host and peripheral controller.

We have a minimal dual-role state machine that just looks at ID pin and toggles
the port role.

How are you switching the port mux between host and peripheral? Only by sysfs
or do you have a GPIO for ID pin as well?

What happens to the gadget controller when the port is muxed to the host controller?
Is it stopped or it continues to run?

cheers,
-roger

[toc] | [prev] | [next] | [standalone]

#1416149

[Multipart message — attachments visible in raw view] — view raw

Hi,

Roger Quadros <rogerq@ti.com> writes:
>> But you said I must run an unnecessary OTG state machine, even thought it
>> has nothing to do with my system, only because the two sides of my port
>> mux device is a host and peripheral controller.
>
> We have a minimal dual-role state machine that just looks at ID pin
> and toggles the port role.

I don't know if we want to bring all that extra baggage just to write a
few bits in a single register. Even for DWC3-only dual-role (what
Synopsys licenses as part of some DWC3 instantiations), the OTG/DRD
layer is a bit overkill.

If you take my testing/next, for example, we have everything we need for
dual-role; except for OTG/DRD IRQ handler. Just look at how we implement
->suspend()/->resume() and it's be clear that we're just missing one
step.

I might be able to find some time to implement a proof of concept which
would allow your platforms to get dual-role with code we already have,
but I need DWC3's OTG support which, I'm assuming, you already have :-)

If you wanna try something offline, just ping me ;-) I'll be happy to
help.

> How are you switching the port mux between host and peripheral? Only
> by sysfs or do you have a GPIO for ID pin as well?

depends. Some SoCs have GPIO-controller muxes while some just have mux's
select signals (one for ID, one for VBUS) mapped on xHCI's address
space.

> What happens to the gadget controller when the port is muxed to the
> host controller?  Is it stopped or it continues to run?

it continues running, but that's pretty irrelevant for Intel's dual-role
setup. We have an actual physical (inside the die, though) mux which
muxes USB signals to XHCI (not DWC3's XHCI) or to a peripheral-only
DWC3.

-- 
balbi

[toc] | [prev] | [next] | [standalone]

#1416226

[Multipart message — attachments visible in raw view] — view raw

On 07/06/16 16:04, Felipe Balbi wrote:
> 
> Hi,
> 
> Roger Quadros <rogerq@ti.com> writes:
>>> But you said I must run an unnecessary OTG state machine, even thought it
>>> has nothing to do with my system, only because the two sides of my port
>>> mux device is a host and peripheral controller.
>>
>> We have a minimal dual-role state machine that just looks at ID pin
>> and toggles the port role.
> 
> I don't know if we want to bring all that extra baggage just to write a
> few bits in a single register. Even for DWC3-only dual-role (what
> Synopsys licenses as part of some DWC3 instantiations), the OTG/DRD
> layer is a bit overkill.
> 
> If you take my testing/next, for example, we have everything we need for
> dual-role; except for OTG/DRD IRQ handler. Just look at how we implement
> ->suspend()/->resume() and it's be clear that we're just missing one
> step.
> 
> I might be able to find some time to implement a proof of concept which
> would allow your platforms to get dual-role with code we already have,
> but I need DWC3's OTG support which, I'm assuming, you already have :-)
> 
> If you wanna try something offline, just ping me ;-) I'll be happy to
> help.

What you are proposing is a dwc3 only solution. With the otg/dual-role
series we are trying to be generic as much as possible.
Whether controller drivers want to use it or not is upto the driver maintainers
but we should at least ensure that user space ABI if any, is consistent
across different implementations.

> 
>> How are you switching the port mux between host and peripheral? Only
>> by sysfs or do you have a GPIO for ID pin as well?
> 
> depends. Some SoCs have GPIO-controller muxes while some just have mux's
> select signals (one for ID, one for VBUS) mapped on xHCI's address
> space.
> 
>> What happens to the gadget controller when the port is muxed to the
>> host controller?  Is it stopped or it continues to run?
> 
> it continues running, but that's pretty irrelevant for Intel's dual-role

Isn't that unnecessary waste of power? Or you have firmware assisted
low power mode?

> setup. We have an actual physical (inside the die, though) mux which
> muxes USB signals to XHCI (not DWC3's XHCI) or to a peripheral-only
> DWC3.
> 

Probably irrelevant for Intel's dual-role but many platforms that share
the port can't have device controller running when port is in host mode and vice versa. 
So there has to be a central point of control where the respective controllers
are started/stopped.
That is the other point we are trying to address with the common
otg/dual-role code.

Even in the TI dwc3 implementation we use dwc3's XHCI so I guess we need
to stop the host controller for device mode, right?

If so then who will deal with start/stop of the controllers then?

So for Intel port-mux case it seems that OTG/dual-role is overcomplicated
and I wouldn't force you to use it. It is upto Peter to decide how he wants
dual-role users to behave.

cheers,
-roger

[toc] | [prev] | [next] | [standalone]

#1416286

[Multipart message — attachments visible in raw view] — view raw

Hi,

Roger Quadros <rogerq@ti.com> writes:
>> I might be able to find some time to implement a proof of concept which
>> would allow your platforms to get dual-role with code we already have,
>> but I need DWC3's OTG support which, I'm assuming, you already have :-)
>> 
>> If you wanna try something offline, just ping me ;-) I'll be happy to
>> help.
>
> What you are proposing is a dwc3 only solution. With the otg/dual-role
> series we are trying to be generic as much as possible.

Well, if there is a need for that, sure. Take MUSB for instance. It
makes use of nothing of the sorts, because it doesn't have to.

> Whether controller drivers want to use it or not is upto the driver
> maintainers but we should at least ensure that user space ABI if any,
> is consistent across different implementations.

Role decisions should not be exposed to userspace unless as debug
feature (using e.g. DebugFS). That should be done either by the HW or
within the kernel.

If we're discussing userspace ABI here, there's something very wrong
with OTG/DRD layer design.

>>> How are you switching the port mux between host and peripheral? Only
>>> by sysfs or do you have a GPIO for ID pin as well?
>> 
>> depends. Some SoCs have GPIO-controller muxes while some just have mux's
>> select signals (one for ID, one for VBUS) mapped on xHCI's address
>> space.
>> 
>>> What happens to the gadget controller when the port is muxed to the
>>> host controller?  Is it stopped or it continues to run?
>> 
>> it continues running, but that's pretty irrelevant for Intel's dual-role
>
> Isn't that unnecessary waste of power? Or you have firmware assisted
> low power mode?

that's an implementation detail which brings nothing to this discussion,
right? :-)

We can, certainly, put the other side to D3.

>> setup. We have an actual physical (inside the die, though) mux which
>> muxes USB signals to XHCI (not DWC3's XHCI) or to a peripheral-only
>> DWC3.
>> 
>
> Probably irrelevant for Intel's dual-role but many platforms that
> share the port can't have device controller running when port is in
> host mode and vice versa.

but that doesn't mean we need an entire new layer added to the kernel
;-)

DWC3 already gives us all the information necessary to make a decision
on which role we should assume. Just consider your options. Here's how
things would look like without any OTG/DRD layer:

-> DWC3 OTG IRQ
 -> readl(OSTS);
  -> if (OSTS & BIT(4))
   -> dwc3_host_exit(); __dwc3_gadget_start();
  -> else
   -> __dwc3_gadget_stop(); dwc3_host_init();

Can you draw something similar for your proposed OTG/DRD layer?

I remember there were at least two schedule_work(). IIRC it looked
something like below:

-> DWC3 OTG IRQ
 -> readl(OSTS);
  -> if (OSTS & BIT(4))
   -> otg_set_mode(PERIPHERAL);
    -> schedule_work();
     -> otg_ops->stop_host();
      -> usb_del_hcd();
     -> otg_ops->start_peripheral();
      -> usb_gadget_add_udc();
  -> else
   -> otg_set_mode(HOST);
    -> schedule_work();
     -> otg_ops->stop_peripheral();
      -> usb_gadget_del_udc();
     -> otg_ops->start_host();
      -> usb_add_hcd();

I'm probably missing some steps there.    

> So there has to be a central point of control where the respective
> controllers are started/stopped.

some implementations might need this, yes. DWC3 and MUSB don't seem to
be this type of system.

> That is the other point we are trying to address with the common
> otg/dual-role code.
>
> Even in the TI dwc3 implementation we use dwc3's XHCI so I guess we need
> to stop the host controller for device mode, right?

yes, see above. We already have that code.

> If so then who will deal with start/stop of the controllers then?

dwc3 itself.

-- 
balbi

[toc] | [prev] | [next] | [standalone]

#1416844

Hi,
> -----Original Message-----
> From: Felipe Balbi [mailto:felipe.balbi@linux.intel.com]
> Sent: Tuesday, June 07, 2016 11:05 PM
> To: Roger Quadros <rogerq@ti.com>; Lu Baolu <baolu.lu@linux.intel.com>;
> Jun Li <jun.li@nxp.com>; Peter Chen <hzpeterchen@gmail.com>
> Cc: Mathias Nyman <mathias.nyman@intel.com>; Greg Kroah-Hartman
> <gregkh@linuxfoundation.org>; Lee Jones <lee.jones@linaro.org>; Heikki
> Krogerus <heikki.krogerus@linux.intel.com>; Liam Girdwood
> <lgirdwood@gmail.com>; Mark Brown <broonie@kernel.org>; linux-
> usb@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port
> mux
> 
> 
> Hi,
> 
> Roger Quadros <rogerq@ti.com> writes:
> >> I might be able to find some time to implement a proof of concept
> >> which would allow your platforms to get dual-role with code we
> >> already have, but I need DWC3's OTG support which, I'm assuming, you
> >> already have :-)
> >>
> >> If you wanna try something offline, just ping me ;-) I'll be happy to
> >> help.
> >
> > What you are proposing is a dwc3 only solution. With the otg/dual-role
> > series we are trying to be generic as much as possible.
> 
> Well, if there is a need for that, sure. Take MUSB for instance. It makes
> use of nothing of the sorts, because it doesn't have to.
> 
> > Whether controller drivers want to use it or not is upto the driver
> > maintainers but we should at least ensure that user space ABI if any,
> > is consistent across different implementations.
> 
> Role decisions should not be exposed to userspace unless as debug feature
> (using e.g. DebugFS). That should be done either by the HW or within the
> kernel.

In many cases the role decision is made by usersapce, this also should be
covered.
This patchset also expose it to userspace but I think it isn't for debug:
/sys/bus/platform/devices/.../portmux.N/state

Li Jun
> 
> If we're discussing userspace ABI here, there's something very wrong with
> OTG/DRD layer design.
> 
> >>> How are you switching the port mux between host and peripheral? Only
> >>> by sysfs or do you have a GPIO for ID pin as well?
> >>
> >> depends. Some SoCs have GPIO-controller muxes while some just have
> >> mux's select signals (one for ID, one for VBUS) mapped on xHCI's
> >> address space.
> >>
> >>> What happens to the gadget controller when the port is muxed to the
> >>> host controller?  Is it stopped or it continues to run?
> >>
> >> it continues running, but that's pretty irrelevant for Intel's
> >> dual-role
> >
> > Isn't that unnecessary waste of power? Or you have firmware assisted
> > low power mode?
> 
> that's an implementation detail which brings nothing to this discussion,
> right? :-)
> 
> We can, certainly, put the other side to D3.
> 
> >> setup. We have an actual physical (inside the die, though) mux which
> >> muxes USB signals to XHCI (not DWC3's XHCI) or to a peripheral-only
> >> DWC3.
> >>
> >
> > Probably irrelevant for Intel's dual-role but many platforms that
> > share the port can't have device controller running when port is in
> > host mode and vice versa.
> 
> but that doesn't mean we need an entire new layer added to the kernel
> ;-)
> 
> DWC3 already gives us all the information necessary to make a decision on
> which role we should assume. Just consider your options. Here's how things
> would look like without any OTG/DRD layer:
> 
> -> DWC3 OTG IRQ
>  -> readl(OSTS);
>   -> if (OSTS & BIT(4))
>    -> dwc3_host_exit(); __dwc3_gadget_start();
>   -> else
>    -> __dwc3_gadget_stop(); dwc3_host_init();
> 
> Can you draw something similar for your proposed OTG/DRD layer?
> 
> I remember there were at least two schedule_work(). IIRC it looked
> something like below:
> 
> -> DWC3 OTG IRQ
>  -> readl(OSTS);
>   -> if (OSTS & BIT(4))
>    -> otg_set_mode(PERIPHERAL);
>     -> schedule_work();
>      -> otg_ops->stop_host();
>       -> usb_del_hcd();
>      -> otg_ops->start_peripheral();
>       -> usb_gadget_add_udc();
>   -> else
>    -> otg_set_mode(HOST);
>     -> schedule_work();
>      -> otg_ops->stop_peripheral();
>       -> usb_gadget_del_udc();
>      -> otg_ops->start_host();
>       -> usb_add_hcd();
> 
> I'm probably missing some steps there.
> 
> > So there has to be a central point of control where the respective
> > controllers are started/stopped.
> 
> some implementations might need this, yes. DWC3 and MUSB don't seem to be
> this type of system.
> 
> > That is the other point we are trying to address with the common
> > otg/dual-role code.
> >
> > Even in the TI dwc3 implementation we use dwc3's XHCI so I guess we
> > need to stop the host controller for device mode, right?
> 
> yes, see above. We already have that code.
> 
> > If so then who will deal with start/stop of the controllers then?
> 
> dwc3 itself.
> 
> --
> balbi

[toc] | [prev] | [next] | [standalone]

#1416897

Hi, Baolu

From: Lu Baolu [mailto:baolu.lu@linux.intel.com] 
Sent: Wednesday, June 08, 2016 1:11 PM
To: Jun Li <jun.li@nxp.com>; Felipe Balbi <felipe.balbi@linux.intel.com>; Roger Quadros <rogerq@ti.com>; Peter Chen <hzpeterchen@gmail.com>
Cc: Mathias Nyman <mathias.nyman@intel.com>; Greg Kroah-Hartman <gregkh@linuxfoundation.org>; Lee Jones <lee.jones@linaro.org>; Heikki Krogerus <heikki.krogerus@linux.intel.com>; Liam Girdwood <lgirdwood@gmail.com>; Mark Brown <broonie@kernel.org>; linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port mux

Hi,

[I have to resend my reply. The previous reply was failed to deliver
to usb mailing list. Sorry for inconvenience.]
On 06/08/2016 11:04 AM, Jun Li wrote:
Whether controller drivers want to use it or not is upto the driver
> > maintainers but we should at least ensure that user space ABI if any,
> > is consistent across different implementations.
> 
> Role decisions should not be exposed to userspace unless as debug feature
> (using e.g. DebugFS). That should be done either by the HW or within the
> kernel.
> In many cases the role decision is made by usersapce, this also should be
> covered.
> This patchset also expose it to userspace but I think it isn't for debug:
> /sys/bus/platform/devices/.../portmux.N/state

> Please don't use this interface for host/gadget role switch, and the
> document doesn't tell you to do so as well. This is only designed to
> put the port mux device to a right direction. Host/gadget dual
> role switch includes other elements, like ID pin detection, type-c
> events, VBUS management and so on.

Confused, then what's the purpose of it? How to use it?
Below is all about it in document, it's seems telling me can do that,
but you say no:)

+What:          /sys/bus/platform/devices/.../portmux.N/name
+               /sys/bus/platform/devices/.../portmux.N/state
+Date:          April 2016
+Contact:       Lu Baolu <baolu.lu@linux.intel.com>
+Description:
+               In some platforms, a single USB port is shared between a USB host
+               controller and a device controller. A USB mux driver is needed to
+               handle the port mux. Read-only attribute "name" shows the name of
+               the port mux device. "state" attribute shows and stores the mux
+               state.
+               For read:
+               'unknown'    - the mux hasn't been set yet;
+               'peripheral' - mux has been switched to PERIPHERAL controller;
+               'host'       - mux has been switched to HOST controller.
+               For write:
+               'peripheral' - mux will be switched to PERIPHERAL controller;
+               'host'       - mux will be switched to HOST controller.

> Best regards,
> Lu Baolu

[toc] | [prev] | [next] | [standalone]

#1416910

On Tue, Jun 07, 2016 at 06:05:25PM +0300, Felipe Balbi wrote:
> 
> Hi,
> 
> Roger Quadros <rogerq@ti.com> writes:
> >> I might be able to find some time to implement a proof of concept which
> >> would allow your platforms to get dual-role with code we already have,
> >> but I need DWC3's OTG support which, I'm assuming, you already have :-)
> >> 
> >> If you wanna try something offline, just ping me ;-) I'll be happy to
> >> help.
> >
> > What you are proposing is a dwc3 only solution. With the otg/dual-role
> > series we are trying to be generic as much as possible.
> 
> Well, if there is a need for that, sure. Take MUSB for instance. It
> makes use of nothing of the sorts, because it doesn't have to.
> 

Indeed, some centralized IP drivers like MUSB, chipidea, dwc3 do not
need this framework for role switch. But there are some common stuffs,
like OTG FSM (fully/simplified), manage roles and sysfs for role switch,
these things can be in a framework, the purpose of this framework is
easy for dual-role switch function.

Besides, when the host and device driver are in different folders for
platform, eg host/ and gadget/udc/, a role switch driver is needed if 
we need dual role function.

Recently, the dual-role function is more and more common for USB, a
framework can avoid duplicated work and let switch be standardized.

> > Whether controller drivers want to use it or not is upto the driver
> > maintainers but we should at least ensure that user space ABI if any,
> > is consistent across different implementations.
> 
> Role decisions should not be exposed to userspace unless as debug
> feature (using e.g. DebugFS). That should be done either by the HW or
> within the kernel.
> 
> If we're discussing userspace ABI here, there's something very wrong
> with OTG/DRD layer design.

Currently, there are some use cases which need to switch role on the
fly (will be more for type-c in future), a sysfs for role switch is
necessary.

-- 

Best Regards,
Peter Chen

[toc] | [prev] | [next] | [standalone]

#1417006

[Multipart message — attachments visible in raw view] — view raw

On 07/06/16 18:05, Felipe Balbi wrote:
> 
> Hi,
> 
> Roger Quadros <rogerq@ti.com> writes:
>>> I might be able to find some time to implement a proof of concept which
>>> would allow your platforms to get dual-role with code we already have,
>>> but I need DWC3's OTG support which, I'm assuming, you already have :-)
>>>
>>> If you wanna try something offline, just ping me ;-) I'll be happy to
>>> help.
>>
>> What you are proposing is a dwc3 only solution. With the otg/dual-role
>> series we are trying to be generic as much as possible.
> 
> Well, if there is a need for that, sure. Take MUSB for instance. It
> makes use of nothing of the sorts, because it doesn't have to.
> 
>> Whether controller drivers want to use it or not is upto the driver
>> maintainers but we should at least ensure that user space ABI if any,
>> is consistent across different implementations.
> 
> Role decisions should not be exposed to userspace unless as debug
> feature (using e.g. DebugFS). That should be done either by the HW or
> within the kernel.
> 
> If we're discussing userspace ABI here, there's something very wrong
> with OTG/DRD layer design.

Not really. There can be a need for user space application to control the
port role. Consider Apple carplay for instance or even full OTG support
which has user selectable role.

> 
>>>> How are you switching the port mux between host and peripheral? Only
>>>> by sysfs or do you have a GPIO for ID pin as well?
>>>
>>> depends. Some SoCs have GPIO-controller muxes while some just have mux's
>>> select signals (one for ID, one for VBUS) mapped on xHCI's address
>>> space.
>>>
>>>> What happens to the gadget controller when the port is muxed to the
>>>> host controller?  Is it stopped or it continues to run?
>>>
>>> it continues running, but that's pretty irrelevant for Intel's dual-role
>>
>> Isn't that unnecessary waste of power? Or you have firmware assisted
>> low power mode?
> 
> that's an implementation detail which brings nothing to this discussion,
> right? :-)
> 
> We can, certainly, put the other side to D3.
> 
>>> setup. We have an actual physical (inside the die, though) mux which
>>> muxes USB signals to XHCI (not DWC3's XHCI) or to a peripheral-only
>>> DWC3.
>>>
>>
>> Probably irrelevant for Intel's dual-role but many platforms that
>> share the port can't have device controller running when port is in
>> host mode and vice versa.
> 
> but that doesn't mean we need an entire new layer added to the kernel
> ;-)
> 
> DWC3 already gives us all the information necessary to make a decision
> on which role we should assume. Just consider your options. Here's how
> things would look like without any OTG/DRD layer:
> 
> -> DWC3 OTG IRQ
>  -> readl(OSTS);
>   -> if (OSTS & BIT(4))
>    -> dwc3_host_exit(); __dwc3_gadget_start();
>   -> else
>    -> __dwc3_gadget_stop(); dwc3_host_init();
> 
> Can you draw something similar for your proposed OTG/DRD layer?

What about B_IDLE state? We don't want either peripheral or host to run when no cable
is inserted.

Have you tested if it works? I'd be happy to test if you can prepare a patch
to get dual-role working on dwc3 without the OTG/DRD layer :).

> 
> I remember there were at least two schedule_work(). IIRC it looked
> something like below:
> 
> -> DWC3 OTG IRQ
>  -> readl(OSTS);
>   -> if (OSTS & BIT(4))
>    -> otg_set_mode(PERIPHERAL);
>     -> schedule_work();
>      -> otg_ops->stop_host();
>       -> usb_del_hcd();
>      -> otg_ops->start_peripheral();
>       -> usb_gadget_add_udc();
>   -> else
>    -> otg_set_mode(HOST);
>     -> schedule_work();
>      -> otg_ops->stop_peripheral();
>       -> usb_gadget_del_udc();
>      -> otg_ops->start_host();
>       -> usb_add_hcd();
> 
> I'm probably missing some steps there.    

As a user you just need to do this

        reg = read(OSTS);
        dwc->otg->fsm.id = !!(reg & STS_ID);
        dwc->otg->fsm.b_sess_vld = !!(reg &STS_BSESVLD);
        usb_otg_sync_inputs(dwc->otg);

And the layer does the rest.

But as I said earlier. I have absolutely no issues if dwc3 doesn't use that layer
as long as dual-role works on our platforms.

> 
>> So there has to be a central point of control where the respective
>> controllers are started/stopped.
> 
> some implementations might need this, yes. DWC3 and MUSB don't seem to
> be this type of system.

OK.
> 
>> That is the other point we are trying to address with the common
>> otg/dual-role code.
>>
>> Even in the TI dwc3 implementation we use dwc3's XHCI so I guess we need
>> to stop the host controller for device mode, right?
> 
> yes, see above. We already have that code.

Just code is not enough. We need to know if it works :).

> 
>> If so then who will deal with start/stop of the controllers then?
> 
> dwc3 itself.
> 
OK.

cheers,
-roger

[toc] | [prev] | [standalone]

Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | linux.kernel

csiph-web