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

2016-06-08 Thread Roger Quadros
On 07/06/16 18:05, Felipe Balbi wrote:
> 
> Hi,
> 
> Roger Quadros  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



signature.asc
Description: OpenPGP digital signature


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

2016-06-07 Thread Peter Chen
On Tue, Jun 07, 2016 at 06:05:25PM +0300, Felipe Balbi wrote:
> 
> Hi,
> 
> Roger Quadros  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


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

2016-06-07 Thread Jun Li
Hi, Baolu

From: Lu Baolu [mailto:baolu...@linux.intel.com] 
Sent: Wednesday, June 08, 2016 1:11 PM
To: Jun Li ; Felipe Balbi ; Roger 
Quadros ; Peter Chen 
Cc: Mathias Nyman ; Greg Kroah-Hartman 
; Lee Jones ; Heikki Krogerus 
; Liam Girdwood ; Mark 
Brown ; linux-...@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 
+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



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

2016-06-07 Thread Jun Li
Hi,
> -Original Message-
> From: Felipe Balbi [mailto:felipe.ba...@linux.intel.com]
> Sent: Tuesday, June 07, 2016 11:05 PM
> To: Roger Quadros ; Lu Baolu ;
> Jun Li ; Peter Chen 
> Cc: Mathias Nyman ; Greg Kroah-Hartman
> ; Lee Jones ; Heikki
> Krogerus ; Liam Girdwood
> ; Mark Brown ; linux-
> u...@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  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


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

2016-06-07 Thread Felipe Balbi

Hi,

Roger Quadros  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


signature.asc
Description: PGP signature


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

2016-06-07 Thread Roger Quadros
On 07/06/16 16:04, Felipe Balbi wrote:
> 
> Hi,
> 
> Roger Quadros  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



signature.asc
Description: OpenPGP digital signature


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

2016-06-07 Thread Felipe Balbi

Hi,

Roger Quadros  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


signature.asc
Description: PGP signature


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

2016-06-07 Thread Roger Quadros
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


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

2016-06-07 Thread Roger Quadros
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


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

2016-06-07 Thread Lu Baolu
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


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

2016-06-07 Thread Lu Baolu
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


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

2016-06-07 Thread Jun Li
Hi Baolu

> -Original Message-
> From: Lu Baolu [mailto:baolu...@linux.intel.com]
> Sent: Tuesday, June 07, 2016 2:27 PM
> To: Jun Li ; Roger Quadros ; Peter Chen
> 
> Cc: felipe.ba...@linux.intel.com; Mathias Nyman ;
> Greg Kroah-Hartman ; Lee Jones
> ; Heikki Krogerus ;
> Liam Girdwood ; Mark Brown ;
> linux-...@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
> >



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

2016-06-06 Thread Lu Baolu
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
>



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

2016-06-06 Thread Jun Li
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),
this is just an example for us to convince people to select a better
way:)

Li Jun


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

2016-06-06 Thread Lu Baolu
Hi Peter,

On 06/06/2016 03:02 PM, Peter Chen wrote:
 > >> But this code is better co-work with OTG/Dual-role framework, we'd
 > >> better have only interface that the user can know which role for the
 > >> current port.
 > >> OTG/Dual-role framework and portmux framework are not overlapped.
 > >> The sysfs interface shouldn't be overlapped as well. Say, I have a 
 > >> port
 > >> mux device and I have a driver for it. I am able to read the status 
 > >> of my
 > >> port mux device through sysfs. This is not part of OTG/Dual-role as 
 > >> far
 > >> as I can see.
 > >>
>>> > > Then how the user wants to switch the role through the mux driver's
>>> > > sysfs or dual-role switch sysfs?
>>> > >
>> > 
>> > It depends. If you have an OTG/DRD capable controllers, you need to
>> > do this through OTG sysfs; otherwise you only need to switch the port.
>> > 
> The user may not know the detail, they will do role switch according to
> sysfs documentation. Yes, in your role switch case, only port mux is enough,
> but for others, it needs other operations.

So we need to make it clear in Documentation/ABI/testing/sysfs-bus-platform.

>
> I agree with Roger that the dual-role switch part in your code is better
> to use OTG framework to reduce redundancy.

I agree that we should use dual-role framework for role switch. Actually,
my code doesn't do this work. It only adds a generic framework for port
mux device and two mux device drivers used in Intel platform.

Best regards,
Lu Baolu


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

2016-06-06 Thread Peter Chen
On Mon, Jun 06, 2016 at 11:04:48AM +0800, Lu Baolu wrote:
> Hi Peter,
> 
> On 06/06/2016 09:25 AM, Peter Chen wrote:
> > On Sun, Jun 05, 2016 at 02:55:56PM +0800, Lu Baolu wrote:
> >> Hi Peter,
> >>
> >> On 06/04/2016 10:28 AM, Peter Chen wrote:
> >>> On Sat, Jun 04, 2016 at 12:06:06AM +0800, Lu Baolu wrote:
> > from my point,it is a dual-role switch
> > driver too,
>  No, it's not a dual-role switch driver, but a driver for USB port 
>  multiplexing.
> 
>  One example of port multiplexing can be found in several Intel SOC and 
>  PCH
>  chips, inside of which, there are two independent USB controllers: host 
>  and
>  device. They might share a single port and this port could be configured 
>  to
>  route the line to one of these two controllers. This patch introduced a 
>  generic
>  framework for port mux drivers. It aids the drivers to handle port mux by
>  providing interfaces to 1) register/unregister a mux device; 2) lookup 
>  the
>  mux device; and 3) switch the port.
> 
> >>> For this case, I can't see it is different with dual-role switch.
> >> Port mux is part of dual role switch, but not the whole thing.
> >>
> >> Dual role switch includes at least below things:
> >>  - ID or type-C event detection
> >>  - port mux
> >>  - VBUS management
> >>  - start/stop host/device controllers
> >>
> >> An OTG/Dual-role framework can be used to keep all these
> >> things run together with an internal state machine. But it's
> >> not duplicated with a generic framework for port mux and
> >> the port mux drivers.
> > You have admitted port mux is one of the ports of dual-role switch,
> > Then, how they can co-work with each other? If can't, the dual-role
> > switch framework needs another input events management for switching.
> 
> My point is we need a generic framework for the port mux devices,
> just like we have that for PHY and regulator. OTG framework
> manages the port mux devices through the common interfaces
> provided by the port mux framework.
> 
> If we integrate the port mux device support into OTG itself, this  will
> force every use case of port mux to rely on the big OTG framework,
> although what it needs is only a single driver. That causes unnecessary
> software complexity.
> 



> 
> >> But this code is better co-work with OTG/Dual-role framework, we'd
> >> better have only interface that the user can know which role for the
> >> current port.
> >> OTG/Dual-role framework and portmux framework are not overlapped.
> >> The sysfs interface shouldn't be overlapped as well. Say, I have a port
> >> mux device and I have a driver for it. I am able to read the status of my
> >> port mux device through sysfs. This is not part of OTG/Dual-role as far
> >> as I can see.
> >>
> > Then how the user wants to switch the role through the mux driver's
> > sysfs or dual-role switch sysfs?
> >
> 
> It depends. If you have an OTG/DRD capable controllers, you need to
> do this through OTG sysfs; otherwise you only need to switch the port.
> 

The user may not know the detail, they will do role switch according to
sysfs documentation. Yes, in your role switch case, only port mux is enough,
but for others, it needs other operations.

I agree with Roger that the dual-role switch part in your code is better
to use OTG framework to reduce redundancy.

-- 

Best Regards,
Peter Chen


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

2016-06-06 Thread Roger Quadros
Hi,

On 06/06/16 06:04, Lu Baolu wrote:
> Hi Peter,
> 
> On 06/06/2016 09:25 AM, Peter Chen wrote:
>> On Sun, Jun 05, 2016 at 02:55:56PM +0800, Lu Baolu wrote:
>>> Hi Peter,
>>>
>>> On 06/04/2016 10:28 AM, Peter Chen wrote:
 On Sat, Jun 04, 2016 at 12:06:06AM +0800, Lu Baolu wrote:
>> from my point,it is a dual-role switch
>> driver too,
> No, it's not a dual-role switch driver, but a driver for USB port 
> multiplexing.
>
> One example of port multiplexing can be found in several Intel SOC and PCH
> chips, inside of which, there are two independent USB controllers: host 
> and
> device. They might share a single port and this port could be configured 
> to
> route the line to one of these two controllers. This patch introduced a 
> generic
> framework for port mux drivers. It aids the drivers to handle port mux by
> providing interfaces to 1) register/unregister a mux device; 2) lookup the
> mux device; and 3) switch the port.
>
 For this case, I can't see it is different with dual-role switch.
>>> Port mux is part of dual role switch, but not the whole thing.
>>>
>>> Dual role switch includes at least below things:
>>>  - ID or type-C event detection
>>>  - port mux
>>>  - VBUS management
>>>  - start/stop host/device controllers
>>>
>>> An OTG/Dual-role framework can be used to keep all these
>>> things run together with an internal state machine. But it's
>>> not duplicated with a generic framework for port mux and
>>> the port mux drivers.
>> You have admitted port mux is one of the ports of dual-role switch,
>> Then, how they can co-work with each other? If can't, the dual-role
>> switch framework needs another input events management for switching.
> 
> My point is we need a generic framework for the port mux devices,
> just like we have that for PHY and regulator. OTG framework
> manages the port mux devices through the common interfaces
> provided by the port mux framework.
> 
> If we integrate the port mux device support into OTG itself, this  will
> force every use case of port mux to rely on the big OTG framework,
> although what it needs is only a single driver. That causes unnecessary
> software complexity.

I agree with Lu here.

Intel platforms seem to actually have a Mux device and we need a device driver 
for that.
OTG/dual-role core cannot directly handle the Mux device.

The Mux device can be used not only for dual-role but for other things so we 
can't
force it to use just OTG/dual-role.

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.

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.

> 
>>
 Your
 case is just like Renesas case, which uses two different drivers between
 peripheral and host[1].
>>> In my case, the port mux devices are physical devices and they
>>> can be controlled through GPIO pins or device registers. They
>>> are independent of both peripheral and host controllers.
>>>
>> Yes, it is the same. GPIO pin or device registers is like ID pin
>> event.
>>
> 
> 
> 
>>> But this code is better co-work with OTG/Dual-role framework, we'd
>>> better have only interface that the user can know which role for the
>>> current port.
>>> OTG/Dual-role framework and portmux framework are not overlapped.
>>> The sysfs interface shouldn't be overlapped as well. Say, I have a port
>>> mux device and I have a driver for it. I am able to read the status of my
>>> port mux device through sysfs. This is not part of OTG/Dual-role as far
>>> as I can see.
>>>
>> Then how the user wants to switch the role through the mux driver's
>> sysfs or dual-role switch sysfs?
>>
> 
> It depends. If you have an OTG/DRD capable controllers, you need to
> do this through OTG sysfs; otherwise you only need to switch the port.
> 

--
cheers,
-roger


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

2016-06-05 Thread Peter Chen
On Mon, Jun 06, 2016 at 10:45:34AM +0800, Lu Baolu wrote:
> Hi Peter,
> 
> On 06/06/2016 10:05 AM, Peter Chen wrote:
> > On Sun, Jun 05, 2016 at 04:46:55PM +0800, Lu Baolu wrote:
> >> Hi,
> >>
> >> On 06/05/2016 04:33 PM, Jun Li wrote:
>  Port mux is part of dual role switch, but not the whole thing.
> > Dual role switch includes at least below things:
> >  - ID or type-C event detection
> >  - port mux
> >  - VBUS management
> >  - start/stop host/device controllers
> >
> > An OTG/Dual-role framework can be used to keep all these things run
> > together with an internal state machine. But it's not duplicated with a
> > generic framework for port mux and the port mux drivers.
> >
> >>> Your
> >>> case is just like Renesas case, which uses two different drivers
> >>> between peripheral and host[1].
> > In my case, the port mux devices are physical devices and they can be
> > controlled through GPIO pins or device registers. They are independent 
> > of
> > both peripheral and host controllers.
> >
> >>> I also think current OTG/Dual role framework can support your case, if you
> >>> find there is any limitation of it which can't meet your requirement, we
> >>> should improve it, Roger also provide an example of dual role switch with
> >>> USB3 based on his OTG core.
> >> Why do we need an OTG framework to support a device driver?
> > Just like you said above, OTG framework can manage role switch, the
> > role switch may need to start or stop host/gadget driver according to
> > different hardware signals or user input.
> 
> We don't have any OTG or dual-role (reduced OTG) capable
> controllers. So we don't need to aid OTG framework to
> start/stop host/gadget drivers.
> 

In your case, the related APIs are NULL.

> >
> >> Is it something like a bus or class driver?
> > The DRD/OTG framework uses the same device structure with the caller,
> > the caller can be a dual-role controller driver (like dwc3, chipidea,
> > etc), or a separate switch driver which like your mux port driver.
> >
> 
> From my point of view, this isn't the right way to handle a port
> mux device.
> 
> We have many kinds of port mux devices across multiple archs,
> we should have a generic framework for them, so that consumers,
> (like OTG framework) can manipulate port mux devices through a
> common interfaces. Just like we already have frameworks for PHY,
> VBUS regulator and ...
> 

But, in your framework, it will finish the role switch like OTG
framework does.

-- 

Best Regards,
Peter Chen


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

2016-06-05 Thread Lu Baolu
Hi Peter,

On 06/06/2016 09:25 AM, Peter Chen wrote:
> On Sun, Jun 05, 2016 at 02:55:56PM +0800, Lu Baolu wrote:
>> Hi Peter,
>>
>> On 06/04/2016 10:28 AM, Peter Chen wrote:
>>> On Sat, Jun 04, 2016 at 12:06:06AM +0800, Lu Baolu wrote:
> from my point,it is a dual-role switch
> driver too,
 No, it's not a dual-role switch driver, but a driver for USB port 
 multiplexing.

 One example of port multiplexing can be found in several Intel SOC and PCH
 chips, inside of which, there are two independent USB controllers: host and
 device. They might share a single port and this port could be configured to
 route the line to one of these two controllers. This patch introduced a 
 generic
 framework for port mux drivers. It aids the drivers to handle port mux by
 providing interfaces to 1) register/unregister a mux device; 2) lookup the
 mux device; and 3) switch the port.

>>> For this case, I can't see it is different with dual-role switch.
>> Port mux is part of dual role switch, but not the whole thing.
>>
>> Dual role switch includes at least below things:
>>  - ID or type-C event detection
>>  - port mux
>>  - VBUS management
>>  - start/stop host/device controllers
>>
>> An OTG/Dual-role framework can be used to keep all these
>> things run together with an internal state machine. But it's
>> not duplicated with a generic framework for port mux and
>> the port mux drivers.
> You have admitted port mux is one of the ports of dual-role switch,
> Then, how they can co-work with each other? If can't, the dual-role
> switch framework needs another input events management for switching.

My point is we need a generic framework for the port mux devices,
just like we have that for PHY and regulator. OTG framework
manages the port mux devices through the common interfaces
provided by the port mux framework.

If we integrate the port mux device support into OTG itself, this  will
force every use case of port mux to rely on the big OTG framework,
although what it needs is only a single driver. That causes unnecessary
software complexity.

>
>>> Your
>>> case is just like Renesas case, which uses two different drivers between
>>> peripheral and host[1].
>> In my case, the port mux devices are physical devices and they
>> can be controlled through GPIO pins or device registers. They
>> are independent of both peripheral and host controllers.
>>
> Yes, it is the same. GPIO pin or device registers is like ID pin
> event.
>



>> But this code is better co-work with OTG/Dual-role framework, we'd
>> better have only interface that the user can know which role for the
>> current port.
>> OTG/Dual-role framework and portmux framework are not overlapped.
>> The sysfs interface shouldn't be overlapped as well. Say, I have a port
>> mux device and I have a driver for it. I am able to read the status of my
>> port mux device through sysfs. This is not part of OTG/Dual-role as far
>> as I can see.
>>
> Then how the user wants to switch the role through the mux driver's
> sysfs or dual-role switch sysfs?
>

It depends. If you have an OTG/DRD capable controllers, you need to
do this through OTG sysfs; otherwise you only need to switch the port.

Best regards,
Lu Baolu


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

2016-06-05 Thread Lu Baolu
Hi Peter,

On 06/06/2016 10:05 AM, Peter Chen wrote:
> On Sun, Jun 05, 2016 at 04:46:55PM +0800, Lu Baolu wrote:
>> Hi,
>>
>> On 06/05/2016 04:33 PM, Jun Li wrote:
 Port mux is part of dual role switch, but not the whole thing.
> Dual role switch includes at least below things:
>  - ID or type-C event detection
>  - port mux
>  - VBUS management
>  - start/stop host/device controllers
>
> An OTG/Dual-role framework can be used to keep all these things run
> together with an internal state machine. But it's not duplicated with a
> generic framework for port mux and the port mux drivers.
>
>>> Your
>>> case is just like Renesas case, which uses two different drivers
>>> between peripheral and host[1].
> In my case, the port mux devices are physical devices and they can be
> controlled through GPIO pins or device registers. They are independent of
> both peripheral and host controllers.
>
>>> I also think current OTG/Dual role framework can support your case, if you
>>> find there is any limitation of it which can't meet your requirement, we
>>> should improve it, Roger also provide an example of dual role switch with
>>> USB3 based on his OTG core.
>> Why do we need an OTG framework to support a device driver?
> Just like you said above, OTG framework can manage role switch, the
> role switch may need to start or stop host/gadget driver according to
> different hardware signals or user input.

We don't have any OTG or dual-role (reduced OTG) capable
controllers. So we don't need to aid OTG framework to
start/stop host/gadget drivers.

>
>> Is it something like a bus or class driver?
> The DRD/OTG framework uses the same device structure with the caller,
> the caller can be a dual-role controller driver (like dwc3, chipidea,
> etc), or a separate switch driver which like your mux port driver.
>

>From my point of view, this isn't the right way to handle a port
mux device.

We have many kinds of port mux devices across multiple archs,
we should have a generic framework for them, so that consumers,
(like OTG framework) can manipulate port mux devices through a
common interfaces. Just like we already have frameworks for PHY,
VBUS regulator and ...

Best regards,
Lu Baolu


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

2016-06-05 Thread Lu Baolu
Hi Jun,

On 06/06/2016 09:08 AM, Jun Li wrote:
>
>> -Original Message-
>> From: Lu Baolu [mailto:baolu...@linux.intel.com]
>> Sent: Sunday, June 05, 2016 4:47 PM
>> To: Jun Li ; Peter Chen 
>> Cc: felipe.ba...@linux.intel.com; Mathias Nyman ;
>> Greg Kroah-Hartman ; Lee Jones
>> ; Heikki Krogerus ;
>> Liam Girdwood ; Mark Brown ;
>> linux-...@vger.kernel.org; linux-kernel@vger.kernel.org; Roger Quadros
>> 
>> Subject: Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port
>> mux
>>
>> Hi,
>>
>> On 06/05/2016 04:33 PM, Jun Li wrote:
>>>> Port mux is part of dual role switch, but not the whole thing.
>>>>> Dual role switch includes at least below things:
>>>>>  - ID or type-C event detection
>>>>>  - port mux
>>>>>  - VBUS management
>>>>>  - start/stop host/device controllers
>>>>>
>>>>> An OTG/Dual-role framework can be used to keep all these things run
>>>>> together with an internal state machine. But it's not duplicated
>>>>> with a generic framework for port mux and the port mux drivers.
>>>>>
>>>>>>> Your
>>>>>>> case is just like Renesas case, which uses two different drivers
>>>>>>> between peripheral and host[1].
>>>>> In my case, the port mux devices are physical devices and they can
>>>>> be controlled through GPIO pins or device registers. They are
>>>>> independent of both peripheral and host controllers.
>>>>>
>>> I also think current OTG/Dual role framework can support your case, if
>>> you find there is any limitation of it which can't meet your
>>> requirement, we should improve it, Roger also provide an example of
>>> dual role switch with
>>> USB3 based on his OTG core.
>> Why do we need an OTG framework to support a device driver?
> Currently there are many controller drivers which are dual role
> capable, all has its specific approach/implementation, your case
> is another one, actually there are common part we can share and
> reuse, Roger is introducing a common framework which cooperates
> into usb core and udc-core. With that, each OTG/dual role user
> only need take care of its small specific part.

Intel's USB controllers aren't dual role capable, and we don't
need any dual role capable drivers either. It's just two USB
controllers which shares a single port and it has a physical
port mux device which could control the route of lines. We
only need drivers for the port mux devices.

>   
>> Is it something like a bus or class driver?
> It's not actually a driver, instead, it's more like a lib or
> helper routines. You just need register your host and gadget
> into OTG core, and define the ops routines if required, OTG state
> machine will help you do the switch.

As I said above, we don't have any dual role capable controllers.
We don't need to have any dual role capable host or gadget drivers
to register into OTG core. We only provide drivers for port mux
devices, just like what we already have for phy or regulator
devices.

Best regards,
Lu Baolu


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

2016-06-05 Thread Peter Chen
On Sun, Jun 05, 2016 at 04:46:55PM +0800, Lu Baolu wrote:
> Hi,
> 
> On 06/05/2016 04:33 PM, Jun Li wrote:
> >> Port mux is part of dual role switch, but not the whole thing.
> >> > 
> >> > Dual role switch includes at least below things:
> >> >  - ID or type-C event detection
> >> >  - port mux
> >> >  - VBUS management
> >> >  - start/stop host/device controllers
> >> > 
> >> > An OTG/Dual-role framework can be used to keep all these things run
> >> > together with an internal state machine. But it's not duplicated with a
> >> > generic framework for port mux and the port mux drivers.
> >> > 
> >>> > > Your
> >>> > > case is just like Renesas case, which uses two different drivers
> >>> > > between peripheral and host[1].
> >> > 
> >> > In my case, the port mux devices are physical devices and they can be
> >> > controlled through GPIO pins or device registers. They are independent of
> >> > both peripheral and host controllers.
> >> > 
> > I also think current OTG/Dual role framework can support your case, if you
> > find there is any limitation of it which can't meet your requirement, we
> > should improve it, Roger also provide an example of dual role switch with
> > USB3 based on his OTG core.
> 
> Why do we need an OTG framework to support a device driver?

Just like you said above, OTG framework can manage role switch, the
role switch may need to start or stop host/gadget driver according to
different hardware signals or user input.

> Is it something like a bus or class driver?

The DRD/OTG framework uses the same device structure with the caller,
the caller can be a dual-role controller driver (like dwc3, chipidea,
etc), or a separate switch driver which like your mux port driver.

-- 

Best Regards,
Peter Chen


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

2016-06-05 Thread Peter Chen
On Sun, Jun 05, 2016 at 02:55:56PM +0800, Lu Baolu wrote:
> Hi Peter,
> 
> On 06/04/2016 10:28 AM, Peter Chen wrote:
> > On Sat, Jun 04, 2016 at 12:06:06AM +0800, Lu Baolu wrote:
> >>> from my point,it is a dual-role switch
> >>> driver too,
> >> No, it's not a dual-role switch driver, but a driver for USB port 
> >> multiplexing.
> >>
> >> One example of port multiplexing can be found in several Intel SOC and PCH
> >> chips, inside of which, there are two independent USB controllers: host and
> >> device. They might share a single port and this port could be configured to
> >> route the line to one of these two controllers. This patch introduced a 
> >> generic
> >> framework for port mux drivers. It aids the drivers to handle port mux by
> >> providing interfaces to 1) register/unregister a mux device; 2) lookup the
> >> mux device; and 3) switch the port.
> >>
> > For this case, I can't see it is different with dual-role switch.
> 
> Port mux is part of dual role switch, but not the whole thing.
> 
> Dual role switch includes at least below things:
>  - ID or type-C event detection
>  - port mux
>  - VBUS management
>  - start/stop host/device controllers
> 
> An OTG/Dual-role framework can be used to keep all these
> things run together with an internal state machine. But it's
> not duplicated with a generic framework for port mux and
> the port mux drivers.

You have admitted port mux is one of the ports of dual-role switch,
Then, how they can co-work with each other? If can't, the dual-role
switch framework needs another input events management for switching.

> 
> > Your
> > case is just like Renesas case, which uses two different drivers between
> > peripheral and host[1].
> 
> In my case, the port mux devices are physical devices and they
> can be controlled through GPIO pins or device registers. They
> are independent of both peripheral and host controllers.
> 

Yes, it is the same. GPIO pin or device registers is like ID pin
event.

> 
> >> Port multiplexing isn't equal to USB dual role. There are other cases in 
> >> today's
> >> systems. In several Intel PCH chips, there equips two USB host 
> >> controllers: ehci
> >> and xhci. The xhci USB2 ports are multiplexed with ehci. This guarantees 
> >> all
> >> USB ports work even running an old version of OS which lacks of USB3 
> >> support.
> >> In theory, we can create a driver for the port mux and switch the ports 
> >> between
> >> xhci and ehci, but that's silly, isn't it? Why not always USB3?:-)
> >>
> >> Another case is xHCI debug capability. The xHCI host controller might equip
> >> a unit for system debugging (refer to 7.6 of xHCI spec). The debugging 
> >> unit is
> >> independent of xhci host controller. But it shares its port with xhci. 
> >> Software
> >> could switch the port between xhci and the debugging unit through the 
> >> registers
> >> defined in xHCI spec.
> >>
> > Yes, above two are different with dual role switch. But in your code and
> > Kconfig, it seems this framework is dedicated for dual-role. Eg:
> >
> > +menuconfig USB_PORTMUX
> > +   bool "USB dual role port MUX support"
> > +   help
> > + Generic USB dual role port mux support.
> 
> Above two cases are examples for port multiplexing, but I don't think we
> need drivers for them. At this moment, this framework is only for dual
> role port mux devices.
> 
> >
> > I think a general dual role port mux is necessary, it can be used to
> > manage different dual-role switch method, eg
> 
> Yes, I agree.
> 
> > - ID pin
> > - External connector through GPIO
> > - SoC register
> > - sysfs
> > - type-C events
> 
> ID pin and type-C events are the *reasons* which trigger the port
> mux switch. Currently, on our platforms, gpio, registers and sysfs
> are methods to control the mux.
> 
> >
> > But this code is better co-work with OTG/Dual-role framework, we'd
> > better have only interface that the user can know which role for the
> > current port.
> 
> OTG/Dual-role framework and portmux framework are not overlapped.
> The sysfs interface shouldn't be overlapped as well. Say, I have a port
> mux device and I have a driver for it. I am able to read the status of my
> port mux device through sysfs. This is not part of OTG/Dual-role as far
> as I can see.
> 

Then how the user wants to switch the role through the mux driver's
sysfs or dual-role switch sysfs?

-- 

Best Regards,
Peter Chen


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

2016-06-05 Thread Jun Li


> -Original Message-
> From: Lu Baolu [mailto:baolu...@linux.intel.com]
> Sent: Sunday, June 05, 2016 4:47 PM
> To: Jun Li ; Peter Chen 
> Cc: felipe.ba...@linux.intel.com; Mathias Nyman ;
> Greg Kroah-Hartman ; Lee Jones
> ; Heikki Krogerus ;
> Liam Girdwood ; Mark Brown ;
> linux-...@vger.kernel.org; linux-kernel@vger.kernel.org; Roger Quadros
> 
> Subject: Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port
> mux
> 
> Hi,
> 
> On 06/05/2016 04:33 PM, Jun Li wrote:
> >> Port mux is part of dual role switch, but not the whole thing.
> >> >
> >> > Dual role switch includes at least below things:
> >> >  - ID or type-C event detection
> >> >  - port mux
> >> >  - VBUS management
> >> >  - start/stop host/device controllers
> >> >
> >> > An OTG/Dual-role framework can be used to keep all these things run
> >> > together with an internal state machine. But it's not duplicated
> >> > with a generic framework for port mux and the port mux drivers.
> >> >
> >>> > > Your
> >>> > > case is just like Renesas case, which uses two different drivers
> >>> > > between peripheral and host[1].
> >> >
> >> > In my case, the port mux devices are physical devices and they can
> >> > be controlled through GPIO pins or device registers. They are
> >> > independent of both peripheral and host controllers.
> >> >
> > I also think current OTG/Dual role framework can support your case, if
> > you find there is any limitation of it which can't meet your
> > requirement, we should improve it, Roger also provide an example of
> > dual role switch with
> > USB3 based on his OTG core.
> 
> Why do we need an OTG framework to support a device driver?

Currently there are many controller drivers which are dual role
capable, all has its specific approach/implementation, your case
is another one, actually there are common part we can share and
reuse, Roger is introducing a common framework which cooperates
into usb core and udc-core. With that, each OTG/dual role user
only need take care of its small specific part.
  
> Is it something like a bus or class driver?

It's not actually a driver, instead, it's more like a lib or
helper routines. You just need register your host and gadget
into OTG core, and define the ops routines if required, OTG state
machine will help you do the switch.

Li Jun
 
> 
> Best regards,
> Lu Baolu


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

2016-06-05 Thread Lu Baolu
Hi,

On 06/05/2016 04:33 PM, Jun Li wrote:
>> Port mux is part of dual role switch, but not the whole thing.
>> > 
>> > Dual role switch includes at least below things:
>> >  - ID or type-C event detection
>> >  - port mux
>> >  - VBUS management
>> >  - start/stop host/device controllers
>> > 
>> > An OTG/Dual-role framework can be used to keep all these things run
>> > together with an internal state machine. But it's not duplicated with a
>> > generic framework for port mux and the port mux drivers.
>> > 
>>> > > Your
>>> > > case is just like Renesas case, which uses two different drivers
>>> > > between peripheral and host[1].
>> > 
>> > In my case, the port mux devices are physical devices and they can be
>> > controlled through GPIO pins or device registers. They are independent of
>> > both peripheral and host controllers.
>> > 
> I also think current OTG/Dual role framework can support your case, if you
> find there is any limitation of it which can't meet your requirement, we
> should improve it, Roger also provide an example of dual role switch with
> USB3 based on his OTG core.

Why do we need an OTG framework to support a device driver?
Is it something like a bus or class driver?

Best regards,
Lu Baolu


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

2016-06-05 Thread Jun Li
Hi

> -Original Message-
> From: linux-usb-ow...@vger.kernel.org [mailto:linux-usb-
> ow...@vger.kernel.org] On Behalf Of Lu Baolu
> Sent: Sunday, June 05, 2016 2:56 PM
> To: Peter Chen 
> Cc: felipe.ba...@linux.intel.com; Mathias Nyman ;
> Greg Kroah-Hartman ; Lee Jones
> ; Heikki Krogerus ;
> Liam Girdwood ; Mark Brown ;
> linux-...@vger.kernel.org; linux-kernel@vger.kernel.org; Roger Quadros
> 
> Subject: Re: [PATCH v10 2/7] usb: mux: add generic code for dual role port
> mux
> 
> Hi Peter,
> 
> On 06/04/2016 10:28 AM, Peter Chen wrote:
> > On Sat, Jun 04, 2016 at 12:06:06AM +0800, Lu Baolu wrote:
> >>> from my point,it is a dual-role switch driver too,
> >> No, it's not a dual-role switch driver, but a driver for USB port
> multiplexing.
> >>
> >> One example of port multiplexing can be found in several Intel SOC
> >> and PCH chips, inside of which, there are two independent USB
> >> controllers: host and device. They might share a single port and this
> >> port could be configured to route the line to one of these two
> >> controllers. This patch introduced a generic framework for port mux
> >> drivers. It aids the drivers to handle port mux by providing
> >> interfaces to 1) register/unregister a mux device; 2) lookup the mux
> device; and 3) switch the port.
> >>
> > For this case, I can't see it is different with dual-role switch.
> 
> Port mux is part of dual role switch, but not the whole thing.
> 
> Dual role switch includes at least below things:
>  - ID or type-C event detection
>  - port mux
>  - VBUS management
>  - start/stop host/device controllers
> 
> An OTG/Dual-role framework can be used to keep all these things run
> together with an internal state machine. But it's not duplicated with a
> generic framework for port mux and the port mux drivers.
> 
> > Your
> > case is just like Renesas case, which uses two different drivers
> > between peripheral and host[1].
> 
> In my case, the port mux devices are physical devices and they can be
> controlled through GPIO pins or device registers. They are independent of
> both peripheral and host controllers.
> 

I also think current OTG/Dual role framework can support your case, if you
find there is any limitation of it which can't meet your requirement, we
should improve it, Roger also provide an example of dual role switch with
USB3 based on his OTG core.

> 
> >> Port multiplexing isn't equal to USB dual role. There are other cases
> >> in today's systems. In several Intel PCH chips, there equips two USB
> >> host controllers: ehci and xhci. The xhci USB2 ports are multiplexed
> >> with ehci. This guarantees all USB ports work even running an old
> version of OS which lacks of USB3 support.
> >> In theory, we can create a driver for the port mux and switch the
> >> ports between xhci and ehci, but that's silly, isn't it? Why not
> >> always USB3?:-)
> >>
> >> Another case is xHCI debug capability. The xHCI host controller might
> >> equip a unit for system debugging (refer to 7.6 of xHCI spec). The
> >> debugging unit is independent of xhci host controller. But it shares
> >> its port with xhci. Software could switch the port between xhci and
> >> the debugging unit through the registers defined in xHCI spec.
> >>
> > Yes, above two are different with dual role switch. But in your code
> > and Kconfig, it seems this framework is dedicated for dual-role. Eg:
> >
> > +menuconfig USB_PORTMUX
> > +   bool "USB dual role port MUX support"
> > +   help
> > + Generic USB dual role port mux support.
> 
> Above two cases are examples for port multiplexing, but I don't think we
> need drivers for them. At this moment, this framework is only for dual
> role port mux devices.
> 
> >
> > I think a general dual role port mux is necessary, it can be used to
> > manage different dual-role switch method, eg
> 
> Yes, I agree.
> 
> > - ID pin
> > - External connector through GPIO
> > - SoC register
> > - sysfs
> > - type-C events
> 
> ID pin and type-C events are the *reasons* which trigger the port mux
> switch. Currently, on our platforms, gpio, registers and sysfs are methods
> to control the mux.

Those methods also can be mapped/added into OTG/dual role core framework.

> 
> >
> > But this code is better co-work with OTG/Dual-role framework, we'd
> > better have only interface that the user can know which role for the
> > current port.
> 
>

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

2016-06-04 Thread Lu Baolu
Hi Peter,

On 06/04/2016 10:28 AM, Peter Chen wrote:
> On Sat, Jun 04, 2016 at 12:06:06AM +0800, Lu Baolu wrote:
>>> from my point,it is a dual-role switch
>>> driver too,
>> No, it's not a dual-role switch driver, but a driver for USB port 
>> multiplexing.
>>
>> One example of port multiplexing can be found in several Intel SOC and PCH
>> chips, inside of which, there are two independent USB controllers: host and
>> device. They might share a single port and this port could be configured to
>> route the line to one of these two controllers. This patch introduced a 
>> generic
>> framework for port mux drivers. It aids the drivers to handle port mux by
>> providing interfaces to 1) register/unregister a mux device; 2) lookup the
>> mux device; and 3) switch the port.
>>
> For this case, I can't see it is different with dual-role switch.

Port mux is part of dual role switch, but not the whole thing.

Dual role switch includes at least below things:
 - ID or type-C event detection
 - port mux
 - VBUS management
 - start/stop host/device controllers

An OTG/Dual-role framework can be used to keep all these
things run together with an internal state machine. But it's
not duplicated with a generic framework for port mux and
the port mux drivers.

> Your
> case is just like Renesas case, which uses two different drivers between
> peripheral and host[1].

In my case, the port mux devices are physical devices and they
can be controlled through GPIO pins or device registers. They
are independent of both peripheral and host controllers.


>> Port multiplexing isn't equal to USB dual role. There are other cases in 
>> today's
>> systems. In several Intel PCH chips, there equips two USB host controllers: 
>> ehci
>> and xhci. The xhci USB2 ports are multiplexed with ehci. This guarantees all
>> USB ports work even running an old version of OS which lacks of USB3 support.
>> In theory, we can create a driver for the port mux and switch the ports 
>> between
>> xhci and ehci, but that's silly, isn't it? Why not always USB3?:-)
>>
>> Another case is xHCI debug capability. The xHCI host controller might equip
>> a unit for system debugging (refer to 7.6 of xHCI spec). The debugging unit 
>> is
>> independent of xhci host controller. But it shares its port with xhci. 
>> Software
>> could switch the port between xhci and the debugging unit through the 
>> registers
>> defined in xHCI spec.
>>
> Yes, above two are different with dual role switch. But in your code and
> Kconfig, it seems this framework is dedicated for dual-role. Eg:
>
> +menuconfig USB_PORTMUX
> +   bool "USB dual role port MUX support"
> +   help
> + Generic USB dual role port mux support.

Above two cases are examples for port multiplexing, but I don't think we
need drivers for them. At this moment, this framework is only for dual
role port mux devices.

>
> I think a general dual role port mux is necessary, it can be used to
> manage different dual-role switch method, eg

Yes, I agree.

> - ID pin
> - External connector through GPIO
> - SoC register
> - sysfs
> - type-C events

ID pin and type-C events are the *reasons* which trigger the port
mux switch. Currently, on our platforms, gpio, registers and sysfs
are methods to control the mux.

>
> But this code is better co-work with OTG/Dual-role framework, we'd
> better have only interface that the user can know which role for the
> current port.

OTG/Dual-role framework and portmux framework are not overlapped.
The sysfs interface shouldn't be overlapped as well. Say, I have a port
mux device and I have a driver for it. I am able to read the status of my
port mux device through sysfs. This is not part of OTG/Dual-role as far
as I can see.

Best regards,
Lu Baolu


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

2016-06-03 Thread Peter Chen
On Sat, Jun 04, 2016 at 12:06:06AM +0800, Lu Baolu wrote:
> Hi Peter,
> 
> On 06/03/2016 03:41 PM, Peter Chen wrote:
> > On Thu, Jun 02, 2016 at 09:37:24AM +0800, Lu Baolu wrote:
> >> > Several Intel platforms implement USB dual role by having completely
> >> > separate xHCI and dwc3 IPs in PCH or SOC silicons. These two IPs share
> >> > a single USB port. There is another external port mux which controls
> >> > where the data lines should go. While the USB controllers are part of
> >> > the silicon, the port mux design are platform specific.
> >> > 
> >> > This patch adds the generic code to handle such multiple roles of a
> >> > usb port. It exports the necessary interfaces for other components to
> >> > register or unregister a usb mux device, and to control its role.
> >> > It registers the mux device with sysfs as well, so that users are able
> >> > to control the port role from user space.
> >> > 
> >> > Some other archs (e.g. Renesas R-Car gen2 SoCs) need an external mux to
> >> > swap usb roles as well. This code could also be leveraged for those 
> >> > archs.
> >> > 
> > Sorry to review this so late,
> 
> It doesn't matter. Thanks for review. Comments are always welcome.:-)
> 
> > from my point,it is a dual-role switch
> > driver too,
> 
> No, it's not a dual-role switch driver, but a driver for USB port 
> multiplexing.
> 
> One example of port multiplexing can be found in several Intel SOC and PCH
> chips, inside of which, there are two independent USB controllers: host and
> device. They might share a single port and this port could be configured to
> route the line to one of these two controllers. This patch introduced a 
> generic
> framework for port mux drivers. It aids the drivers to handle port mux by
> providing interfaces to 1) register/unregister a mux device; 2) lookup the
> mux device; and 3) switch the port.
> 

For this case, I can't see it is different with dual-role switch. Your
case is just like Renesas case, which uses two different drivers between
peripheral and host[1].

> Port multiplexing isn't equal to USB dual role. There are other cases in 
> today's
> systems. In several Intel PCH chips, there equips two USB host controllers: 
> ehci
> and xhci. The xhci USB2 ports are multiplexed with ehci. This guarantees all
> USB ports work even running an old version of OS which lacks of USB3 support.
> In theory, we can create a driver for the port mux and switch the ports 
> between
> xhci and ehci, but that's silly, isn't it? Why not always USB3?:-)
> 
> Another case is xHCI debug capability. The xHCI host controller might equip
> a unit for system debugging (refer to 7.6 of xHCI spec). The debugging unit is
> independent of xhci host controller. But it shares its port with xhci. 
> Software
> could switch the port between xhci and the debugging unit through the 
> registers
> defined in xHCI spec.
> 

Yes, above two are different with dual role switch. But in your code and
Kconfig, it seems this framework is dedicated for dual-role. Eg:

+menuconfig USB_PORTMUX
+   bool "USB dual role port MUX support"
+   help
+ Generic USB dual role port mux support.

I think a general dual role port mux is necessary, it can be used to
manage different dual-role switch method, eg
- ID pin
- External connector through GPIO
- SoC register
- sysfs
- type-C events

But this code is better co-work with OTG/Dual-role framework, we'd
better have only interface that the user can know which role for the
current port.

[1] https://lkml.org/lkml/2016/4/7/115
-- 

Best Regards,
Peter Chen


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

2016-06-03 Thread Lu Baolu
Hi Peter,

On 06/03/2016 03:41 PM, Peter Chen wrote:
> On Thu, Jun 02, 2016 at 09:37:24AM +0800, Lu Baolu wrote:
>> > Several Intel platforms implement USB dual role by having completely
>> > separate xHCI and dwc3 IPs in PCH or SOC silicons. These two IPs share
>> > a single USB port. There is another external port mux which controls
>> > where the data lines should go. While the USB controllers are part of
>> > the silicon, the port mux design are platform specific.
>> > 
>> > This patch adds the generic code to handle such multiple roles of a
>> > usb port. It exports the necessary interfaces for other components to
>> > register or unregister a usb mux device, and to control its role.
>> > It registers the mux device with sysfs as well, so that users are able
>> > to control the port role from user space.
>> > 
>> > Some other archs (e.g. Renesas R-Car gen2 SoCs) need an external mux to
>> > swap usb roles as well. This code could also be leveraged for those archs.
>> > 
> Sorry to review this so late,

It doesn't matter. Thanks for review. Comments are always welcome.:-)

> from my point,it is a dual-role switch
> driver too,

No, it's not a dual-role switch driver, but a driver for USB port multiplexing.

One example of port multiplexing can be found in several Intel SOC and PCH
chips, inside of which, there are two independent USB controllers: host and
device. They might share a single port and this port could be configured to
route the line to one of these two controllers. This patch introduced a generic
framework for port mux drivers. It aids the drivers to handle port mux by
providing interfaces to 1) register/unregister a mux device; 2) lookup the
mux device; and 3) switch the port.

Port multiplexing isn't equal to USB dual role. There are other cases in today's
systems. In several Intel PCH chips, there equips two USB host controllers: ehci
and xhci. The xhci USB2 ports are multiplexed with ehci. This guarantees all
USB ports work even running an old version of OS which lacks of USB3 support.
In theory, we can create a driver for the port mux and switch the ports between
xhci and ehci, but that's silly, isn't it? Why not always USB3?:-)

Another case is xHCI debug capability. The xHCI host controller might equip
a unit for system debugging (refer to 7.6 of xHCI spec). The debugging unit is
independent of xhci host controller. But it shares its port with xhci. Software
could switch the port between xhci and the debugging unit through the registers
defined in xHCI spec.

Best regards,
Lu Baolu

> we are reviewing USB OTG/dual-role framework [1], it is
> not necessary to create another framework to do it. And USB OTG framework
> has already tested at Renesas's platform [2].
>
> [1] http://www.spinics.net/lists/linux-usb/msg140835.html
> [2] http://www.spinics.net/lists/linux-usb/msg140827.html
>
> Peter
>



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

2016-06-03 Thread Peter Chen
On Fri, Jun 03, 2016 at 11:16:32AM +0300, Heikki Krogerus wrote:
> On Fri, Jun 03, 2016 at 03:41:13PM +0800, Peter Chen wrote:
> > On Thu, Jun 02, 2016 at 09:37:24AM +0800, Lu Baolu wrote:
> > > Several Intel platforms implement USB dual role by having completely
> > > separate xHCI and dwc3 IPs in PCH or SOC silicons. These two IPs share
> > > a single USB port. There is another external port mux which controls
> > > where the data lines should go. While the USB controllers are part of
> > > the silicon, the port mux design are platform specific.
> > > 
> > > This patch adds the generic code to handle such multiple roles of a
> > > usb port. It exports the necessary interfaces for other components to
> > > register or unregister a usb mux device, and to control its role.
> > > It registers the mux device with sysfs as well, so that users are able
> > > to control the port role from user space.
> > > 
> > > Some other archs (e.g. Renesas R-Car gen2 SoCs) need an external mux to
> > > swap usb roles as well. This code could also be leveraged for those archs.
> > > 
> > 
> > Sorry to review this so late, from my point, it is a dual-role switch
> > driver too, we are reviewing USB OTG/dual-role framework [1], it is
> > not necessary to create another framework to do it. And USB OTG framework
> > has already tested at Renesas's platform [2].
> > 
> > [1] http://www.spinics.net/lists/linux-usb/msg140835.html
> > [2] http://www.spinics.net/lists/linux-usb/msg140827.html
> 
> We really can't marry dual-role capability with OTG. That OTG
> framework can be used be when the hardware actually supports the
> protocols defined in the OTG spec starting from SRP. In other cases it
> must not be used.
> 
> OTG relies heavily on existence of the ID pin, but with Type-C
> connectors we do not have it. Therefore USB Type-C defines competing
> support for example for the role swapping. With USB Type-C connectors
> OTG will never be supported.
> 
> So let's not mix USB dual-role capability with OTG.
> 

Well, DRD/OTG framework is mainly used for dual-role switch, no
matter what input is, you can use id pin, sysfs, or Type-C events.
It is long term target, currently, it only supports id pin.
In future, we can expend it to support more input events.
Microsoft also has similar framework for it:

https://msdn.microsoft.com/en-us/library/windows/hardware/dn957036(v=vs.85).aspx

-- 

Best Regards,
Peter Chen


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

2016-06-03 Thread Heikki Krogerus
On Fri, Jun 03, 2016 at 03:41:13PM +0800, Peter Chen wrote:
> On Thu, Jun 02, 2016 at 09:37:24AM +0800, Lu Baolu wrote:
> > Several Intel platforms implement USB dual role by having completely
> > separate xHCI and dwc3 IPs in PCH or SOC silicons. These two IPs share
> > a single USB port. There is another external port mux which controls
> > where the data lines should go. While the USB controllers are part of
> > the silicon, the port mux design are platform specific.
> > 
> > This patch adds the generic code to handle such multiple roles of a
> > usb port. It exports the necessary interfaces for other components to
> > register or unregister a usb mux device, and to control its role.
> > It registers the mux device with sysfs as well, so that users are able
> > to control the port role from user space.
> > 
> > Some other archs (e.g. Renesas R-Car gen2 SoCs) need an external mux to
> > swap usb roles as well. This code could also be leveraged for those archs.
> > 
> 
> Sorry to review this so late, from my point, it is a dual-role switch
> driver too, we are reviewing USB OTG/dual-role framework [1], it is
> not necessary to create another framework to do it. And USB OTG framework
> has already tested at Renesas's platform [2].
> 
> [1] http://www.spinics.net/lists/linux-usb/msg140835.html
> [2] http://www.spinics.net/lists/linux-usb/msg140827.html

We really can't marry dual-role capability with OTG. That OTG
framework can be used be when the hardware actually supports the
protocols defined in the OTG spec starting from SRP. In other cases it
must not be used.

OTG relies heavily on existence of the ID pin, but with Type-C
connectors we do not have it. Therefore USB Type-C defines competing
support for example for the role swapping. With USB Type-C connectors
OTG will never be supported.

So let's not mix USB dual-role capability with OTG.


Thanks,

-- 
heikki


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

2016-06-03 Thread Peter Chen
On Thu, Jun 02, 2016 at 09:37:24AM +0800, Lu Baolu wrote:
> Several Intel platforms implement USB dual role by having completely
> separate xHCI and dwc3 IPs in PCH or SOC silicons. These two IPs share
> a single USB port. There is another external port mux which controls
> where the data lines should go. While the USB controllers are part of
> the silicon, the port mux design are platform specific.
> 
> This patch adds the generic code to handle such multiple roles of a
> usb port. It exports the necessary interfaces for other components to
> register or unregister a usb mux device, and to control its role.
> It registers the mux device with sysfs as well, so that users are able
> to control the port role from user space.
> 
> Some other archs (e.g. Renesas R-Car gen2 SoCs) need an external mux to
> swap usb roles as well. This code could also be leveraged for those archs.
> 

Sorry to review this so late, from my point, it is a dual-role switch
driver too, we are reviewing USB OTG/dual-role framework [1], it is
not necessary to create another framework to do it. And USB OTG framework
has already tested at Renesas's platform [2].

[1] http://www.spinics.net/lists/linux-usb/msg140835.html
[2] http://www.spinics.net/lists/linux-usb/msg140827.html

Peter

> Signed-off-by: Lu Baolu 
> Reviewed-by: Heikki Krogerus 
> Reviewed-by: Felipe Balbi 
> ---
>  Documentation/ABI/testing/sysfs-bus-platform |  18 +++
>  drivers/usb/Kconfig  |   2 +
>  drivers/usb/Makefile |   1 +
>  drivers/usb/mux/Kconfig  |   8 ++
>  drivers/usb/mux/Makefile |   4 +
>  drivers/usb/mux/portmux-core.c   | 202 
> +++
>  include/linux/usb/portmux.h  |  90 
>  7 files changed, 325 insertions(+)
>  create mode 100644 drivers/usb/mux/Kconfig
>  create mode 100644 drivers/usb/mux/Makefile
>  create mode 100644 drivers/usb/mux/portmux-core.c
>  create mode 100644 include/linux/usb/portmux.h
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-platform 
> b/Documentation/ABI/testing/sysfs-bus-platform
> index 5172a61..b994e4e 100644
> --- a/Documentation/ABI/testing/sysfs-bus-platform
> +++ b/Documentation/ABI/testing/sysfs-bus-platform
> @@ -18,3 +18,21 @@ Description:
>   devices to opt-out of driver binding using a driver_override
>   name such as "none".  Only a single driver may be specified in
>   the override, there is no support for parsing delimiters.
> +
> +What:/sys/bus/platform/devices/.../portmux.N/name
> + /sys/bus/platform/devices/.../portmux.N/state
> +Date:April 2016
> +Contact: Lu Baolu 
> +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.
> diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
> index 8689dcb..328916e 100644
> --- a/drivers/usb/Kconfig
> +++ b/drivers/usb/Kconfig
> @@ -148,6 +148,8 @@ endif # USB
>  
>  source "drivers/usb/phy/Kconfig"
>  
> +source "drivers/usb/mux/Kconfig"
> +
>  source "drivers/usb/gadget/Kconfig"
>  
>  config USB_LED_TRIG
> diff --git a/drivers/usb/Makefile b/drivers/usb/Makefile
> index dca7856..9a92338 100644
> --- a/drivers/usb/Makefile
> +++ b/drivers/usb/Makefile
> @@ -6,6 +6,7 @@
>  
>  obj-$(CONFIG_USB)+= core/
>  obj-$(CONFIG_USB_SUPPORT)+= phy/
> +obj-$(CONFIG_USB_SUPPORT)+= mux/
>  
>  obj-$(CONFIG_USB_DWC3)   += dwc3/
>  obj-$(CONFIG_USB_DWC2)   += dwc2/
> diff --git a/drivers/usb/mux/Kconfig b/drivers/usb/mux/Kconfig
> new file mode 100644
> index 000..ba778f2
> --- /dev/null
> +++ b/drivers/usb/mux/Kconfig
> @@ -0,0 +1,8 @@
> +#
> +# USB port mux driver configuration
> +#
> +
> +menuconfig USB_PORTMUX
> + bool "USB dual role port MUX support"
> + help
> +   Generic USB dual role port mux support.
> diff --git a/drivers/usb/mux/Makefile b/drivers/usb/mux/Makefile
> new file mode 100644
> index 000..f85df92
> --- /dev/null
> +++ b/drivers/usb/mux/Makefile
> @@ -0,0 +1,4 @@
> +#
> +# Makefile for USB port mux drivers
> +#
> +obj-$(CONFIG_USB_PORTMUX)+= portmux-core.o
> diff --git a/drivers/usb/mux/portmux-core.c b/drivers/usb/mux/portmux-core.c
> ne