diff mbox

[v7,10/14] usb: otg: add hcd companion support

Message ID 1462191537-10314-11-git-send-email-rogerq@ti.com
State Superseded, archived
Headers show

Commit Message

Roger Quadros May 2, 2016, 12:18 p.m. UTC
From: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>

Since some host controller (e.g. EHCI) needs a companion host controller
(e.g. OHCI), this patch adds such a configuration to use it in the OTG
core.

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 Documentation/devicetree/bindings/usb/generic.txt |  3 +++
 drivers/usb/common/usb-otg.c                      | 32 ++++++++++++++++-------
 include/linux/usb/otg.h                           |  7 ++++-
 3 files changed, 32 insertions(+), 10 deletions(-)

Comments

Rob Herring (Arm) May 4, 2016, 1:17 p.m. UTC | #1
On Mon, May 02, 2016 at 03:18:53PM +0300, Roger Quadros wrote:
> From: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> 
> Since some host controller (e.g. EHCI) needs a companion host controller
> (e.g. OHCI), this patch adds such a configuration to use it in the OTG
> core.
> 
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> Signed-off-by: Roger Quadros <rogerq@ti.com>
> ---
>  Documentation/devicetree/bindings/usb/generic.txt |  3 +++
>  drivers/usb/common/usb-otg.c                      | 32 ++++++++++++++++-------
>  include/linux/usb/otg.h                           |  7 ++++-
>  3 files changed, 32 insertions(+), 10 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/usb/generic.txt b/Documentation/devicetree/bindings/usb/generic.txt
> index f6866c1..1db1c33 100644
> --- a/Documentation/devicetree/bindings/usb/generic.txt
> +++ b/Documentation/devicetree/bindings/usb/generic.txt
> @@ -27,6 +27,9 @@ Optional properties:
>   - otg-controller: phandle to otg controller. Host or gadget controllers can
>  			contain this property to link it to a particular OTG
>  			controller.
> + - hcd-needs-companion: must be present if otg controller is dealing with
> +			EHCI host controller that needs a companion OHCI host
> +			controller.

Don't you need to have a link to the companion controller node?

>  
>  This is an attribute to a USB controller such as:
>  
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Roger Quadros May 4, 2016, 1:47 p.m. UTC | #2
On 04/05/16 16:17, Rob Herring wrote:
> On Mon, May 02, 2016 at 03:18:53PM +0300, Roger Quadros wrote:
>> From: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
>>
>> Since some host controller (e.g. EHCI) needs a companion host controller
>> (e.g. OHCI), this patch adds such a configuration to use it in the OTG
>> core.
>>
>> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>> ---
>>  Documentation/devicetree/bindings/usb/generic.txt |  3 +++
>>  drivers/usb/common/usb-otg.c                      | 32 ++++++++++++++++-------
>>  include/linux/usb/otg.h                           |  7 ++++-
>>  3 files changed, 32 insertions(+), 10 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/usb/generic.txt b/Documentation/devicetree/bindings/usb/generic.txt
>> index f6866c1..1db1c33 100644
>> --- a/Documentation/devicetree/bindings/usb/generic.txt
>> +++ b/Documentation/devicetree/bindings/usb/generic.txt
>> @@ -27,6 +27,9 @@ Optional properties:
>>   - otg-controller: phandle to otg controller. Host or gadget controllers can
>>  			contain this property to link it to a particular OTG
>>  			controller.
>> + - hcd-needs-companion: must be present if otg controller is dealing with
>> +			EHCI host controller that needs a companion OHCI host
>> +			controller.
> 
> Don't you need to have a link to the companion controller node?

primary and companion controllers are totally independent of each other
e.g. EHCI and OHCI. They are enabled by separate Kconfig options and
the system can operate with either or both of them enabled.

At the OTG layer we don't have information as to whether we should be waiting
for both of them to register or not and hence need this "hcd-needs-companion" flag.

> 
>>  
>>  This is an attribute to a USB controller such as:
>>  

--
cheers,
-roger
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Peter Chen May 11, 2016, 8:43 a.m. UTC | #3
On Mon, May 02, 2016 at 03:18:53PM +0300, Roger Quadros wrote:
> From: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> 
> Since some host controller (e.g. EHCI) needs a companion host controller
> (e.g. OHCI), this patch adds such a configuration to use it in the OTG
> core.
> 
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> Signed-off-by: Roger Quadros <rogerq@ti.com>

Acked-by: Peter Chen <peter.chen@nxp.com>

> ---
>  Documentation/devicetree/bindings/usb/generic.txt |  3 +++
>  drivers/usb/common/usb-otg.c                      | 32 ++++++++++++++++-------
>  include/linux/usb/otg.h                           |  7 ++++-
>  3 files changed, 32 insertions(+), 10 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/usb/generic.txt b/Documentation/devicetree/bindings/usb/generic.txt
> index f6866c1..1db1c33 100644
> --- a/Documentation/devicetree/bindings/usb/generic.txt
> +++ b/Documentation/devicetree/bindings/usb/generic.txt
> @@ -27,6 +27,9 @@ Optional properties:
>   - otg-controller: phandle to otg controller. Host or gadget controllers can
>  			contain this property to link it to a particular OTG
>  			controller.
> + - hcd-needs-companion: must be present if otg controller is dealing with
> +			EHCI host controller that needs a companion OHCI host
> +			controller.
>  
>  This is an attribute to a USB controller such as:
>  
> diff --git a/drivers/usb/common/usb-otg.c b/drivers/usb/common/usb-otg.c
> index 702bca8..77048aa 100644
> --- a/drivers/usb/common/usb-otg.c
> +++ b/drivers/usb/common/usb-otg.c
> @@ -20,6 +20,7 @@
>  #include <linux/list.h>
>  #include <linux/of.h>
>  #include <linux/of_platform.h>
> +#include <linux/usb/of.h>
>  #include <linux/usb/otg.h>
>  #include <linux/usb/gadget.h>
>  #include <linux/workqueue.h>
> @@ -582,6 +583,10 @@ struct usb_otg *usb_otg_register(struct device *dev,
>  	else
>  		INIT_WORK(&otg->work, usb_drd_work);
>  
> +	if (of_find_property(dev->of_node, "hcd-needs-companion", NULL) ||
> +	    config->hcd_needs_companion)	/* needs companion ? */
> +		otg->flags |= OTG_FLAG_HCD_NEEDS_COMPANION;
> +
>  	otg->wq = create_singlethread_workqueue("usb_otg");
>  	if (!otg->wq) {
>  		dev_err(dev, "otg: %s: can't create workqueue\n",
> @@ -805,15 +810,18 @@ int usb_otg_register_hcd(struct usb_hcd *hcd, unsigned int irqnum,
>  	/* HCD will be started by OTG fsm when needed */
>  	mutex_lock(&otg->fsm.lock);
>  	if (otg->primary_hcd.hcd) {
> -		/* probably a shared HCD ? */
> -		if (usb_otg_hcd_is_primary_hcd(hcd)) {
> +		/* probably a shared HCD or a companion OHCI HCD ? */
> +		if (!(otg->flags & OTG_FLAG_HCD_NEEDS_COMPANION) &&
> +		    usb_otg_hcd_is_primary_hcd(hcd)) {
>  			dev_err(otg_dev, "otg: primary host already registered\n");
>  			goto err;
>  		}
>  
> -		if (hcd->shared_hcd == otg->primary_hcd.hcd) {
> +		if (otg->flags & OTG_FLAG_HCD_NEEDS_COMPANION ||
> +		    (hcd->shared_hcd == otg->primary_hcd.hcd)) {
>  			if (otg->shared_hcd.hcd) {
> -				dev_err(otg_dev, "otg: shared host already registered\n");
> +				dev_err(otg_dev,
> +					"otg: shared/companion host already registered\n");
>  				goto err;
>  			}
>  
> @@ -821,10 +829,12 @@ int usb_otg_register_hcd(struct usb_hcd *hcd, unsigned int irqnum,
>  			otg->shared_hcd.irqnum = irqnum;
>  			otg->shared_hcd.irqflags = irqflags;
>  			otg->shared_hcd.ops = ops;
> -			dev_info(otg_dev, "otg: shared host %s registered\n",
> +			dev_info(otg_dev,
> +				 "otg: shared/companion host %s registered\n",
>  				 dev_name(hcd->self.controller));
>  		} else {
> -			dev_err(otg_dev, "otg: invalid shared host %s\n",
> +			dev_err(otg_dev,
> +				"otg: invalid shared/companion host %s\n",
>  				dev_name(hcd->self.controller));
>  			goto err;
>  		}
> @@ -847,14 +857,17 @@ int usb_otg_register_hcd(struct usb_hcd *hcd, unsigned int irqnum,
>  	 * we're ready only if we have shared HCD
>  	 * or we don't need shared HCD.
>  	 */
> -	if (otg->shared_hcd.hcd || !otg->primary_hcd.hcd->shared_hcd) {
> +	if (otg->shared_hcd.hcd ||
> +	    (!(otg->flags & OTG_FLAG_HCD_NEEDS_COMPANION) &&
> +	     !otg->primary_hcd.hcd->shared_hcd)) {
>  		otg->host = hcd_to_bus(hcd);
>  		/* FIXME: set bus->otg_port if this is true OTG port with HNP */
>  
>  		/* start FSM */
>  		usb_otg_start_fsm(otg);
>  	} else {
> -		dev_dbg(otg_dev, "otg: can't start till shared host registers\n");
> +		dev_dbg(otg_dev,
> +			"otg: can't start till shared/companion host registers\n");
>  	}
>  
>  	mutex_unlock(&otg->fsm.lock);
> @@ -905,7 +918,8 @@ int usb_otg_unregister_hcd(struct usb_hcd *hcd)
>  			 dev_name(hcd_dev));
>  	} else if (hcd == otg->shared_hcd.hcd) {
>  		otg->shared_hcd.hcd = NULL;
> -		dev_info(otg_dev, "otg: shared host %s unregistered\n",
> +		dev_info(otg_dev,
> +			 "otg: shared/companion host %s unregistered\n",
>  			 dev_name(hcd_dev));
>  	} else {
>  		dev_err(otg_dev, "otg: host %s wasn't registered with otg\n",
> diff --git a/include/linux/usb/otg.h b/include/linux/usb/otg.h
> index b094352..6f4ca77 100644
> --- a/include/linux/usb/otg.h
> +++ b/include/linux/usb/otg.h
> @@ -57,7 +57,8 @@ struct otg_hcd {
>   * @list: list of otg controllers
>   * @work: otg state machine work
>   * @wq: otg state machine work queue
> - * @flags: to track if host/gadget is running
> + * @flags: to track if host/gadget is running, or to indicate if hcd needs
> + *	   companion
>   */
>  struct usb_otg {
>  	u8			default_a;
> @@ -84,6 +85,7 @@ struct usb_otg {
>  	u32 flags;
>  #define OTG_FLAG_GADGET_RUNNING (1 << 0)
>  #define OTG_FLAG_HOST_RUNNING (1 << 1)
> +#define OTG_FLAG_HCD_NEEDS_COMPANION (1 << 2)
>  	/* use otg->fsm.lock for serializing access */
>  
>  /*------------- deprecated interface -----------------------------*/
> @@ -125,11 +127,14 @@ struct usb_otg_caps {
>   * @caps: otg capabilities of the controller
>   * @ops: otg fsm operations
>   * @otg_work: optional custom otg state machine work function
> + * @hcd_needs_companion: Indicates if host controller needs a companion
> + *			 controller
>   */
>  struct usb_otg_config {
>  	struct usb_otg_caps *otg_caps;
>  	struct otg_fsm_ops *fsm_ops;
>  	void (*otg_work)(struct work_struct *work);
> +	bool hcd_needs_companion;
>  };
>  
>  extern const char *usb_otg_state_string(enum usb_otg_state state);
> -- 
> 2.7.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rob Herring (Arm) May 11, 2016, 1:54 p.m. UTC | #4
On Wed, May 04, 2016 at 04:47:18PM +0300, Roger Quadros wrote:
> On 04/05/16 16:17, Rob Herring wrote:
> > On Mon, May 02, 2016 at 03:18:53PM +0300, Roger Quadros wrote:
> >> From: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> >>
> >> Since some host controller (e.g. EHCI) needs a companion host controller
> >> (e.g. OHCI), this patch adds such a configuration to use it in the OTG
> >> core.
> >>
> >> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> >> Signed-off-by: Roger Quadros <rogerq@ti.com>
> >> ---
> >>  Documentation/devicetree/bindings/usb/generic.txt |  3 +++
> >>  drivers/usb/common/usb-otg.c                      | 32 ++++++++++++++++-------
> >>  include/linux/usb/otg.h                           |  7 ++++-
> >>  3 files changed, 32 insertions(+), 10 deletions(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/usb/generic.txt b/Documentation/devicetree/bindings/usb/generic.txt
> >> index f6866c1..1db1c33 100644
> >> --- a/Documentation/devicetree/bindings/usb/generic.txt
> >> +++ b/Documentation/devicetree/bindings/usb/generic.txt
> >> @@ -27,6 +27,9 @@ Optional properties:
> >>   - otg-controller: phandle to otg controller. Host or gadget controllers can
> >>  			contain this property to link it to a particular OTG
> >>  			controller.
> >> + - hcd-needs-companion: must be present if otg controller is dealing with
> >> +			EHCI host controller that needs a companion OHCI host
> >> +			controller.
> > 
> > Don't you need to have a link to the companion controller node?
> 
> primary and companion controllers are totally independent of each other
> e.g. EHCI and OHCI. They are enabled by separate Kconfig options and
> the system can operate with either or both of them enabled.
> 
> At the OTG layer we don't have information as to whether we should be waiting
> for both of them to register or not and hence need this "hcd-needs-companion" flag.

What I mean is if you have 2 EHCI controllers with 2 companion 
controllers, don't you need to know which companion goes with which EHCI 
controller? Just like you do for the otg-controller property.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Roger Quadros May 11, 2016, 2:13 p.m. UTC | #5
On 11/05/16 16:54, Rob Herring wrote:
> On Wed, May 04, 2016 at 04:47:18PM +0300, Roger Quadros wrote:
>> On 04/05/16 16:17, Rob Herring wrote:
>>> On Mon, May 02, 2016 at 03:18:53PM +0300, Roger Quadros wrote:
>>>> From: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
>>>>
>>>> Since some host controller (e.g. EHCI) needs a companion host controller
>>>> (e.g. OHCI), this patch adds such a configuration to use it in the OTG
>>>> core.
>>>>
>>>> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
>>>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>>>> ---
>>>>  Documentation/devicetree/bindings/usb/generic.txt |  3 +++
>>>>  drivers/usb/common/usb-otg.c                      | 32 ++++++++++++++++-------
>>>>  include/linux/usb/otg.h                           |  7 ++++-
>>>>  3 files changed, 32 insertions(+), 10 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/usb/generic.txt b/Documentation/devicetree/bindings/usb/generic.txt
>>>> index f6866c1..1db1c33 100644
>>>> --- a/Documentation/devicetree/bindings/usb/generic.txt
>>>> +++ b/Documentation/devicetree/bindings/usb/generic.txt
>>>> @@ -27,6 +27,9 @@ Optional properties:
>>>>   - otg-controller: phandle to otg controller. Host or gadget controllers can
>>>>  			contain this property to link it to a particular OTG
>>>>  			controller.
>>>> + - hcd-needs-companion: must be present if otg controller is dealing with
>>>> +			EHCI host controller that needs a companion OHCI host
>>>> +			controller.
>>>
>>> Don't you need to have a link to the companion controller node?
>>
>> primary and companion controllers are totally independent of each other
>> e.g. EHCI and OHCI. They are enabled by separate Kconfig options and
>> the system can operate with either or both of them enabled.
>>
>> At the OTG layer we don't have information as to whether we should be waiting
>> for both of them to register or not and hence need this "hcd-needs-companion" flag.
> 
> What I mean is if you have 2 EHCI controllers with 2 companion 
> controllers, don't you need to know which companion goes with which EHCI 
> controller? Just like you do for the otg-controller property.
> 

That is a very good point. I'm not very sure and it seems that current code won't work
with multiple EHCI + companion instances.

Alan, does USB core even know which EHCI and OHCI are linked to the same port
or the handoff is software transparent?

cheers,
-roger
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alan Stern May 11, 2016, 2:47 p.m. UTC | #6
On Wed, 11 May 2016, Roger Quadros wrote:

> > What I mean is if you have 2 EHCI controllers with 2 companion 
> > controllers, don't you need to know which companion goes with which EHCI 
> > controller? Just like you do for the otg-controller property.
> > 
> 
> That is a very good point. I'm not very sure and it seems that current code won't work
> with multiple EHCI + companion instances.
> 
> Alan, does USB core even know which EHCI and OHCI are linked to the same port
> or the handoff is software transparent?

The core knows.  It doesn't use the information for a whole lot of 
things, but it does use it in a couple of places.  Search for 
"companion" in core/hcd-pci.c and you'll see.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Yoshihiro Shimoda May 12, 2016, 4 a.m. UTC | #7
Hi,

> From: Alan Stern
> Sent: Wednesday, May 11, 2016 11:47 PM
> 
> On Wed, 11 May 2016, Roger Quadros wrote:
> 
> > > What I mean is if you have 2 EHCI controllers with 2 companion
> > > controllers, don't you need to know which companion goes with which EHCI
> > > controller? Just like you do for the otg-controller property.
> > >
> >
> > That is a very good point. I'm not very sure and it seems that current code won't work
> > with multiple EHCI + companion instances.

I may misunderstand this topic, but if I use the following environment, it works correctly.

< My environment >
- an otg controller: Sets hcd-needs-companion.
- ehci0 and ohci0 and a function: They connect to the otg controller using "otg-controller" property.
- ehci1 and ohci1: No "otg-controller" property.
- ehci2 and ohci2: No "otg-controller" property.

In this environment, all hosts works correctly.
Also I think if we have 2 otg controlelrs, it should be work because otg_dev instance differs.
Or, does this topic assume an otg controller handles 2 EHCI controllers?
I'm not sure such environment actually exists.

> > Alan, does USB core even know which EHCI and OHCI are linked to the same port
> > or the handoff is software transparent?
> 
> The core knows.  It doesn't use the information for a whole lot of
> things, but it does use it in a couple of places.  Search for
> "companion" in core/hcd-pci.c and you'll see.

Thank you for the information. I didn't know this code.
If my understanding is correct, the core/hcd-pci.c code will not be used by non-PCI devices.
In other words, nobody sets "hcd->self.hs_companion" if we use such a device.
So, I will try to add such a code if needed.

Best regards,
Yoshihiro Shimoda

> Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Roger Quadros May 12, 2016, 8:34 a.m. UTC | #8
On 12/05/16 07:00, Yoshihiro Shimoda wrote:
> Hi,
> 
>> From: Alan Stern
>> Sent: Wednesday, May 11, 2016 11:47 PM
>>
>> On Wed, 11 May 2016, Roger Quadros wrote:
>>
>>>> What I mean is if you have 2 EHCI controllers with 2 companion
>>>> controllers, don't you need to know which companion goes with which EHCI
>>>> controller? Just like you do for the otg-controller property.
>>>>
>>>
>>> That is a very good point. I'm not very sure and it seems that current code won't work
>>> with multiple EHCI + companion instances.
> 
> I may misunderstand this topic, but if I use the following environment, it works correctly.
> 
> < My environment >
> - an otg controller: Sets hcd-needs-companion.
> - ehci0 and ohci0 and a function: They connect to the otg controller using "otg-controller" property.
> - ehci1 and ohci1: No "otg-controller" property.
> - ehci2 and ohci2: No "otg-controller" property.
> 
> In this environment, all hosts works correctly.
> Also I think if we have 2 otg controlelrs, it should be work because otg_dev instance differs.

The topic is about more than one otg controllers and how to tie the right ehci and ohci
to the correct otg_dev instance especially in cases where we can't depend on probe order.

> Or, does this topic assume an otg controller handles 2 EHCI controllers?
> I'm not sure such environment actually exists.

No it is not about that.

> 
>>> Alan, does USB core even know which EHCI and OHCI are linked to the same port
>>> or the handoff is software transparent?
>>
>> The core knows.  It doesn't use the information for a whole lot of
>> things, but it does use it in a couple of places.  Search for
>> "companion" in core/hcd-pci.c and you'll see.
> 
> Thank you for the information. I didn't know this code.
> If my understanding is correct, the core/hcd-pci.c code will not be used by non-PCI devices.

That is correct.

> In other words, nobody sets "hcd->self.hs_companion" if we use such a device.
> So, I will try to add such a code if needed.

I think OTG core would have to rely on USB core in providing the right companion device,
just like we rely on it for the primary vs shared HCD case.

cheers,
-roger
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Roger Quadros May 12, 2016, 9:31 a.m. UTC | #9
Hi,

On 12/05/16 11:34, Roger Quadros wrote:
> On 12/05/16 07:00, Yoshihiro Shimoda wrote:
>> Hi,
>>
>>> From: Alan Stern
>>> Sent: Wednesday, May 11, 2016 11:47 PM
>>>
>>> On Wed, 11 May 2016, Roger Quadros wrote:
>>>
>>>>> What I mean is if you have 2 EHCI controllers with 2 companion
>>>>> controllers, don't you need to know which companion goes with which EHCI
>>>>> controller? Just like you do for the otg-controller property.
>>>>>
>>>>
>>>> That is a very good point. I'm not very sure and it seems that current code won't work
>>>> with multiple EHCI + companion instances.
>>
>> I may misunderstand this topic, but if I use the following environment, it works correctly.
>>
>> < My environment >
>> - an otg controller: Sets hcd-needs-companion.
>> - ehci0 and ohci0 and a function: They connect to the otg controller using "otg-controller" property.
>> - ehci1 and ohci1: No "otg-controller" property.
>> - ehci2 and ohci2: No "otg-controller" property.
>>
>> In this environment, all hosts works correctly.
>> Also I think if we have 2 otg controlelrs, it should be work because otg_dev instance differs.
> 
> The topic is about more than one otg controllers and how to tie the right ehci and ohci
> to the correct otg_dev instance especially in cases where we can't depend on probe order.
> 
>> Or, does this topic assume an otg controller handles 2 EHCI controllers?
>> I'm not sure such environment actually exists.
> 
> No it is not about that.
> 
>>
>>>> Alan, does USB core even know which EHCI and OHCI are linked to the same port
>>>> or the handoff is software transparent?
>>>
>>> The core knows.  It doesn't use the information for a whole lot of
>>> things, but it does use it in a couple of places.  Search for
>>> "companion" in core/hcd-pci.c and you'll see.
>>
>> Thank you for the information. I didn't know this code.
>> If my understanding is correct, the core/hcd-pci.c code will not be used by non-PCI devices.
> 
> That is correct.
> 
>> In other words, nobody sets "hcd->self.hs_companion" if we use such a device.
>> So, I will try to add such a code if needed.
> 
> I think OTG core would have to rely on USB core in providing the right companion device,
> just like we rely on it for the primary vs shared HCD case.
> 

OK, it is not so simple.

EHCI and companion port handoff is really meant to be software transparent.

non-PCI devices really don't have knowledge of which OHCI instance is companion to the EHCI.
With device tree we could provide this mapping but for non-device tree case we can't do
anything.

So my suggestion would be to keep dual role implementation limited to one instance for
EHCI + companion case for non-DT.
For PCI case I don't see how dual role can be implemented. I don't think we have any
dual-role PCI cards.
For DT case we could have a DT binding to tie the EHCI and companion and use that
in the OTG framework.

Any objections?

cheers,
-roger
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Yoshihiro Shimoda May 12, 2016, 10:31 a.m. UTC | #10
Hi,

> From: Roger Quadros
> Sent: Thursday, May 12, 2016 6:32 PM
> 
> Hi,
> 
> On 12/05/16 11:34, Roger Quadros wrote:
> > On 12/05/16 07:00, Yoshihiro Shimoda wrote:
> >> Hi,
> >>
> >>> From: Alan Stern
> >>> Sent: Wednesday, May 11, 2016 11:47 PM
> >>>
> >>> On Wed, 11 May 2016, Roger Quadros wrote:
> >>>
> >>>>> What I mean is if you have 2 EHCI controllers with 2 companion
> >>>>> controllers, don't you need to know which companion goes with which EHCI
> >>>>> controller? Just like you do for the otg-controller property.
> >>>>>
> >>>>
> >>>> That is a very good point. I'm not very sure and it seems that current code won't work
> >>>> with multiple EHCI + companion instances.
> >>
> >> I may misunderstand this topic, but if I use the following environment, it works correctly.
> >>
> >> < My environment >
> >> - an otg controller: Sets hcd-needs-companion.
> >> - ehci0 and ohci0 and a function: They connect to the otg controller using "otg-controller" property.
> >> - ehci1 and ohci1: No "otg-controller" property.
> >> - ehci2 and ohci2: No "otg-controller" property.
> >>
> >> In this environment, all hosts works correctly.
> >> Also I think if we have 2 otg controlelrs, it should be work because otg_dev instance differs.
> >
> > The topic is about more than one otg controllers and how to tie the right ehci and ohci
> > to the correct otg_dev instance especially in cases where we can't depend on probe order.
> >
> >> Or, does this topic assume an otg controller handles 2 EHCI controllers?
> >> I'm not sure such environment actually exists.
> >
> > No it is not about that.

Thank you for the reply. I understood it.

> >>>> Alan, does USB core even know which EHCI and OHCI are linked to the same port
> >>>> or the handoff is software transparent?
> >>>
> >>> The core knows.  It doesn't use the information for a whole lot of
> >>> things, but it does use it in a couple of places.  Search for
> >>> "companion" in core/hcd-pci.c and you'll see.
> >>
> >> Thank you for the information. I didn't know this code.
> >> If my understanding is correct, the core/hcd-pci.c code will not be used by non-PCI devices.
> >
> > That is correct.
> >
> >> In other words, nobody sets "hcd->self.hs_companion" if we use such a device.
> >> So, I will try to add such a code if needed.
> >
> > I think OTG core would have to rely on USB core in providing the right companion device,
> > just like we rely on it for the primary vs shared HCD case.
> >
> 
> OK, it is not so simple.
> 
> EHCI and companion port handoff is really meant to be software transparent.
> 
> non-PCI devices really don't have knowledge of which OHCI instance is companion to the EHCI.
> With device tree we could provide this mapping but for non-device tree case we can't do
> anything.
> 
> So my suggestion would be to keep dual role implementation limited to one instance for
> EHCI + companion case for non-DT.
> For PCI case I don't see how dual role can be implemented. I don't think we have any
> dual-role PCI cards.

R-Car Gen2 SoCs (r8a779[0134] / arm32) has USB 2.0 host controllers via PCI bus and
one high speed function controller via AXI bus.
One of channel can be used as host or function.

> For DT case we could have a DT binding to tie the EHCI and companion and use that
> in the OTG framework.

R-Car Gen3 SoC (r8a7795 / arm64) will be this type.
(Both USB 2.0 host/function controllers connect to AXI bus.)

> Any objections?

I don't have any objections because I'm just focus on R-Car Gen3 SoC for now.
If someone needs for PCI case, I think it is possible to add such a code somehow later.

Best regards,
Yoshihiro Shimoda

> cheers,
> -roger
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Roger Quadros May 12, 2016, 12:13 p.m. UTC | #11
Hi,

On 12/05/16 13:31, Yoshihiro Shimoda wrote:
> Hi,
> 
>> From: Roger Quadros
>> Sent: Thursday, May 12, 2016 6:32 PM
>>
>> Hi,
>>
>> On 12/05/16 11:34, Roger Quadros wrote:
>>> On 12/05/16 07:00, Yoshihiro Shimoda wrote:
>>>> Hi,
>>>>
>>>>> From: Alan Stern
>>>>> Sent: Wednesday, May 11, 2016 11:47 PM
>>>>>
>>>>> On Wed, 11 May 2016, Roger Quadros wrote:
>>>>>
>>>>>>> What I mean is if you have 2 EHCI controllers with 2 companion
>>>>>>> controllers, don't you need to know which companion goes with which EHCI
>>>>>>> controller? Just like you do for the otg-controller property.
>>>>>>>
>>>>>>
>>>>>> That is a very good point. I'm not very sure and it seems that current code won't work
>>>>>> with multiple EHCI + companion instances.
>>>>
>>>> I may misunderstand this topic, but if I use the following environment, it works correctly.
>>>>
>>>> < My environment >
>>>> - an otg controller: Sets hcd-needs-companion.
>>>> - ehci0 and ohci0 and a function: They connect to the otg controller using "otg-controller" property.
>>>> - ehci1 and ohci1: No "otg-controller" property.
>>>> - ehci2 and ohci2: No "otg-controller" property.
>>>>
>>>> In this environment, all hosts works correctly.
>>>> Also I think if we have 2 otg controlelrs, it should be work because otg_dev instance differs.
>>>
>>> The topic is about more than one otg controllers and how to tie the right ehci and ohci
>>> to the correct otg_dev instance especially in cases where we can't depend on probe order.
>>>
>>>> Or, does this topic assume an otg controller handles 2 EHCI controllers?
>>>> I'm not sure such environment actually exists.
>>>
>>> No it is not about that.
> 
> Thank you for the reply. I understood it.
> 
>>>>>> Alan, does USB core even know which EHCI and OHCI are linked to the same port
>>>>>> or the handoff is software transparent?
>>>>>
>>>>> The core knows.  It doesn't use the information for a whole lot of
>>>>> things, but it does use it in a couple of places.  Search for
>>>>> "companion" in core/hcd-pci.c and you'll see.
>>>>
>>>> Thank you for the information. I didn't know this code.
>>>> If my understanding is correct, the core/hcd-pci.c code will not be used by non-PCI devices.
>>>
>>> That is correct.
>>>
>>>> In other words, nobody sets "hcd->self.hs_companion" if we use such a device.
>>>> So, I will try to add such a code if needed.
>>>
>>> I think OTG core would have to rely on USB core in providing the right companion device,
>>> just like we rely on it for the primary vs shared HCD case.
>>>
>>
>> OK, it is not so simple.
>>
>> EHCI and companion port handoff is really meant to be software transparent.
>>
>> non-PCI devices really don't have knowledge of which OHCI instance is companion to the EHCI.
>> With device tree we could provide this mapping but for non-device tree case we can't do
>> anything.
>>
>> So my suggestion would be to keep dual role implementation limited to one instance for
>> EHCI + companion case for non-DT.
>> For PCI case I don't see how dual role can be implemented. I don't think we have any
>> dual-role PCI cards.
> 
> R-Car Gen2 SoCs (r8a779[0134] / arm32) has USB 2.0 host controllers via PCI bus and
> one high speed function controller via AXI bus.
> One of channel can be used as host or function.
> 
>> For DT case we could have a DT binding to tie the EHCI and companion and use that
>> in the OTG framework.

After looking at the code it seems we don't need this special binding as we are already
linking the EHCI controller and companion controller to the single otg controller instance
using the otg-controller property.

So all is good as of now.

For non DT case, it is the responsibility of platform support code to ensure that
it calls usb_otg_add_hcd() with the correct otg controller instance for both EHCI and
companion controller and things should work fine there as well.

--
cheers,
-roger

> 
> R-Car Gen3 SoC (r8a7795 / arm64) will be this type.
> (Both USB 2.0 host/function controllers connect to AXI bus.)
> 
>> Any objections?
> 
> I don't have any objections because I'm just focus on R-Car Gen3 SoC for now.
> If someone needs for PCI case, I think it is possible to add such a code somehow later.
> 
> Best regards,
> Yoshihiro Shimoda
> 
>> cheers,
>> -roger
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alan Stern May 12, 2016, 6:16 p.m. UTC | #12
On Thu, 12 May 2016, Yoshihiro Shimoda wrote:

> > > Alan, does USB core even know which EHCI and OHCI are linked to the same port
> > > or the handoff is software transparent?
> > 
> > The core knows.  It doesn't use the information for a whole lot of
> > things, but it does use it in a couple of places.  Search for
> > "companion" in core/hcd-pci.c and you'll see.
> 
> Thank you for the information. I didn't know this code.
> If my understanding is correct, the core/hcd-pci.c code will not be used by non-PCI devices.
> In other words, nobody sets "hcd->self.hs_companion" if we use such a device.

That's right.

> So, I will try to add such a code if needed.

The main thing to watch out for is during system resume.  The EHCI
controller must not be resumed until all of its companion controllers
have been resumed.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Peter Chen May 16, 2016, 2:13 a.m. UTC | #13
On Thu, May 12, 2016 at 03:13:48PM +0300, Roger Quadros wrote:
> Hi,
> 
> On 12/05/16 13:31, Yoshihiro Shimoda wrote:
> > Hi,
> > 
> >> From: Roger Quadros
> >> Sent: Thursday, May 12, 2016 6:32 PM
> >>
> >> Hi,
> >>
> >> On 12/05/16 11:34, Roger Quadros wrote:
> >>> On 12/05/16 07:00, Yoshihiro Shimoda wrote:
> >>>> Hi,
> >>>>
> >>>>> From: Alan Stern
> >>>>> Sent: Wednesday, May 11, 2016 11:47 PM
> >>>>>
> >>>>> On Wed, 11 May 2016, Roger Quadros wrote:
> >>>>>
> >>>>>>> What I mean is if you have 2 EHCI controllers with 2 companion
> >>>>>>> controllers, don't you need to know which companion goes with which EHCI
> >>>>>>> controller? Just like you do for the otg-controller property.
> >>>>>>>
> >>>>>>
> >>>>>> That is a very good point. I'm not very sure and it seems that current code won't work
> >>>>>> with multiple EHCI + companion instances.
> >>>>
> >>>> I may misunderstand this topic, but if I use the following environment, it works correctly.
> >>>>
> >>>> < My environment >
> >>>> - an otg controller: Sets hcd-needs-companion.
> >>>> - ehci0 and ohci0 and a function: They connect to the otg controller using "otg-controller" property.
> >>>> - ehci1 and ohci1: No "otg-controller" property.
> >>>> - ehci2 and ohci2: No "otg-controller" property.
> >>>>
> >>>> In this environment, all hosts works correctly.
> >>>> Also I think if we have 2 otg controlelrs, it should be work because otg_dev instance differs.
> >>>
> >>> The topic is about more than one otg controllers and how to tie the right ehci and ohci
> >>> to the correct otg_dev instance especially in cases where we can't depend on probe order.
> >>>
> >>>> Or, does this topic assume an otg controller handles 2 EHCI controllers?
> >>>> I'm not sure such environment actually exists.
> >>>
> >>> No it is not about that.
> > 
> > Thank you for the reply. I understood it.
> > 
> >>>>>> Alan, does USB core even know which EHCI and OHCI are linked to the same port
> >>>>>> or the handoff is software transparent?
> >>>>>
> >>>>> The core knows.  It doesn't use the information for a whole lot of
> >>>>> things, but it does use it in a couple of places.  Search for
> >>>>> "companion" in core/hcd-pci.c and you'll see.
> >>>>
> >>>> Thank you for the information. I didn't know this code.
> >>>> If my understanding is correct, the core/hcd-pci.c code will not be used by non-PCI devices.
> >>>
> >>> That is correct.
> >>>
> >>>> In other words, nobody sets "hcd->self.hs_companion" if we use such a device.
> >>>> So, I will try to add such a code if needed.
> >>>
> >>> I think OTG core would have to rely on USB core in providing the right companion device,
> >>> just like we rely on it for the primary vs shared HCD case.
> >>>
> >>
> >> OK, it is not so simple.
> >>
> >> EHCI and companion port handoff is really meant to be software transparent.
> >>
> >> non-PCI devices really don't have knowledge of which OHCI instance is companion to the EHCI.
> >> With device tree we could provide this mapping but for non-device tree case we can't do
> >> anything.
> >>
> >> So my suggestion would be to keep dual role implementation limited to one instance for
> >> EHCI + companion case for non-DT.
> >> For PCI case I don't see how dual role can be implemented. I don't think we have any
> >> dual-role PCI cards.
> > 
> > R-Car Gen2 SoCs (r8a779[0134] / arm32) has USB 2.0 host controllers via PCI bus and
> > one high speed function controller via AXI bus.
> > One of channel can be used as host or function.
> > 
> >> For DT case we could have a DT binding to tie the EHCI and companion and use that
> >> in the OTG framework.
> 
> After looking at the code it seems we don't need this special binding as we are already
> linking the EHCI controller and companion controller to the single otg controller instance
> using the otg-controller property.
> 

Then, how you know this EHCI + companion controller special case during otg adds
hcd, it needs special handling, right?

Peter

> So all is good as of now.
> 
> For non DT case, it is the responsibility of platform support code to ensure that
> it calls usb_otg_add_hcd() with the correct otg controller instance for both EHCI and
> companion controller and things should work fine there as well.
> 
> --
> cheers,
> -roger
> 
> > 
> > R-Car Gen3 SoC (r8a7795 / arm64) will be this type.
> > (Both USB 2.0 host/function controllers connect to AXI bus.)
> > 
> >> Any objections?
> > 
> > I don't have any objections because I'm just focus on R-Car Gen3 SoC for now.
> > If someone needs for PCI case, I think it is possible to add such a code somehow later.
> > 
> > Best regards,
> > Yoshihiro Shimoda
> > 
> >> cheers,
> >> -roger
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
Roger Quadros May 16, 2016, 8:01 a.m. UTC | #14
On 16/05/16 05:13, Peter Chen wrote:
> On Thu, May 12, 2016 at 03:13:48PM +0300, Roger Quadros wrote:
>> Hi,
>>
>> On 12/05/16 13:31, Yoshihiro Shimoda wrote:
>>> Hi,
>>>
>>>> From: Roger Quadros
>>>> Sent: Thursday, May 12, 2016 6:32 PM
>>>>
>>>> Hi,
>>>>
>>>> On 12/05/16 11:34, Roger Quadros wrote:
>>>>> On 12/05/16 07:00, Yoshihiro Shimoda wrote:
>>>>>> Hi,
>>>>>>
>>>>>>> From: Alan Stern
>>>>>>> Sent: Wednesday, May 11, 2016 11:47 PM
>>>>>>>
>>>>>>> On Wed, 11 May 2016, Roger Quadros wrote:
>>>>>>>
>>>>>>>>> What I mean is if you have 2 EHCI controllers with 2 companion
>>>>>>>>> controllers, don't you need to know which companion goes with which EHCI
>>>>>>>>> controller? Just like you do for the otg-controller property.
>>>>>>>>>
>>>>>>>>
>>>>>>>> That is a very good point. I'm not very sure and it seems that current code won't work
>>>>>>>> with multiple EHCI + companion instances.
>>>>>>
>>>>>> I may misunderstand this topic, but if I use the following environment, it works correctly.
>>>>>>
>>>>>> < My environment >
>>>>>> - an otg controller: Sets hcd-needs-companion.
>>>>>> - ehci0 and ohci0 and a function: They connect to the otg controller using "otg-controller" property.
>>>>>> - ehci1 and ohci1: No "otg-controller" property.
>>>>>> - ehci2 and ohci2: No "otg-controller" property.
>>>>>>
>>>>>> In this environment, all hosts works correctly.
>>>>>> Also I think if we have 2 otg controlelrs, it should be work because otg_dev instance differs.
>>>>>
>>>>> The topic is about more than one otg controllers and how to tie the right ehci and ohci
>>>>> to the correct otg_dev instance especially in cases where we can't depend on probe order.
>>>>>
>>>>>> Or, does this topic assume an otg controller handles 2 EHCI controllers?
>>>>>> I'm not sure such environment actually exists.
>>>>>
>>>>> No it is not about that.
>>>
>>> Thank you for the reply. I understood it.
>>>
>>>>>>>> Alan, does USB core even know which EHCI and OHCI are linked to the same port
>>>>>>>> or the handoff is software transparent?
>>>>>>>
>>>>>>> The core knows.  It doesn't use the information for a whole lot of
>>>>>>> things, but it does use it in a couple of places.  Search for
>>>>>>> "companion" in core/hcd-pci.c and you'll see.
>>>>>>
>>>>>> Thank you for the information. I didn't know this code.
>>>>>> If my understanding is correct, the core/hcd-pci.c code will not be used by non-PCI devices.
>>>>>
>>>>> That is correct.
>>>>>
>>>>>> In other words, nobody sets "hcd->self.hs_companion" if we use such a device.
>>>>>> So, I will try to add such a code if needed.
>>>>>
>>>>> I think OTG core would have to rely on USB core in providing the right companion device,
>>>>> just like we rely on it for the primary vs shared HCD case.
>>>>>
>>>>
>>>> OK, it is not so simple.
>>>>
>>>> EHCI and companion port handoff is really meant to be software transparent.
>>>>
>>>> non-PCI devices really don't have knowledge of which OHCI instance is companion to the EHCI.
>>>> With device tree we could provide this mapping but for non-device tree case we can't do
>>>> anything.
>>>>
>>>> So my suggestion would be to keep dual role implementation limited to one instance for
>>>> EHCI + companion case for non-DT.
>>>> For PCI case I don't see how dual role can be implemented. I don't think we have any
>>>> dual-role PCI cards.
>>>
>>> R-Car Gen2 SoCs (r8a779[0134] / arm32) has USB 2.0 host controllers via PCI bus and
>>> one high speed function controller via AXI bus.
>>> One of channel can be used as host or function.
>>>
>>>> For DT case we could have a DT binding to tie the EHCI and companion and use that
>>>> in the OTG framework.
>>
>> After looking at the code it seems we don't need this special binding as we are already
>> linking the EHCI controller and companion controller to the single otg controller instance
>> using the otg-controller property.
>>
> 
> Then, how you know this EHCI + companion controller special case during otg adds
> hcd, it needs special handling, right?

We know the special case by using the hcd_needs_companion flag.

cheers,
-roger

> 
> Peter
> 
>> So all is good as of now.
>>
>> For non DT case, it is the responsibility of platform support code to ensure that
>> it calls usb_otg_add_hcd() with the correct otg controller instance for both EHCI and
>> companion controller and things should work fine there as well.
>>
>> --
>> cheers,
>> -roger
>>
>>>
>>> R-Car Gen3 SoC (r8a7795 / arm64) will be this type.
>>> (Both USB 2.0 host/function controllers connect to AXI bus.)
>>>
>>>> Any objections?
>>>
>>> I don't have any objections because I'm just focus on R-Car Gen3 SoC for now.
>>> If someone needs for PCI case, I think it is possible to add such a code somehow later.
>>>
>>> Best regards,
>>> Yoshihiro Shimoda
>>>
>>>> cheers,
>>>> -roger
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Peter Chen May 16, 2016, 8:13 a.m. UTC | #15
On Mon, May 16, 2016 at 11:01:27AM +0300, Roger Quadros wrote:
> On 16/05/16 05:13, Peter Chen wrote:
> > On Thu, May 12, 2016 at 03:13:48PM +0300, Roger Quadros wrote:
> >> Hi,
> >>
> >> On 12/05/16 13:31, Yoshihiro Shimoda wrote:
> >>> Hi,
> >>>
> >>>> From: Roger Quadros
> >>>> Sent: Thursday, May 12, 2016 6:32 PM
> >>>>
> >>>> Hi,
> >>>>
> >>>> On 12/05/16 11:34, Roger Quadros wrote:
> >>>>> On 12/05/16 07:00, Yoshihiro Shimoda wrote:
> >>>>>> Hi,
> >>>>>>
> >>>>>>> From: Alan Stern
> >>>>>>> Sent: Wednesday, May 11, 2016 11:47 PM
> >>>>>>>
> >>>>>>> On Wed, 11 May 2016, Roger Quadros wrote:
> >>>>>>>
> >>>>>>>>> What I mean is if you have 2 EHCI controllers with 2 companion
> >>>>>>>>> controllers, don't you need to know which companion goes with which EHCI
> >>>>>>>>> controller? Just like you do for the otg-controller property.
> >>>>>>>>>
> >>>>>>>>
> >>>>>>>> That is a very good point. I'm not very sure and it seems that current code won't work
> >>>>>>>> with multiple EHCI + companion instances.
> >>>>>>
> >>>>>> I may misunderstand this topic, but if I use the following environment, it works correctly.
> >>>>>>
> >>>>>> < My environment >
> >>>>>> - an otg controller: Sets hcd-needs-companion.
> >>>>>> - ehci0 and ohci0 and a function: They connect to the otg controller using "otg-controller" property.
> >>>>>> - ehci1 and ohci1: No "otg-controller" property.
> >>>>>> - ehci2 and ohci2: No "otg-controller" property.
> >>>>>>
> >>>>>> In this environment, all hosts works correctly.
> >>>>>> Also I think if we have 2 otg controlelrs, it should be work because otg_dev instance differs.
> >>>>>
> >>>>> The topic is about more than one otg controllers and how to tie the right ehci and ohci
> >>>>> to the correct otg_dev instance especially in cases where we can't depend on probe order.
> >>>>>
> >>>>>> Or, does this topic assume an otg controller handles 2 EHCI controllers?
> >>>>>> I'm not sure such environment actually exists.
> >>>>>
> >>>>> No it is not about that.
> >>>
> >>> Thank you for the reply. I understood it.
> >>>
> >>>>>>>> Alan, does USB core even know which EHCI and OHCI are linked to the same port
> >>>>>>>> or the handoff is software transparent?
> >>>>>>>
> >>>>>>> The core knows.  It doesn't use the information for a whole lot of
> >>>>>>> things, but it does use it in a couple of places.  Search for
> >>>>>>> "companion" in core/hcd-pci.c and you'll see.
> >>>>>>
> >>>>>> Thank you for the information. I didn't know this code.
> >>>>>> If my understanding is correct, the core/hcd-pci.c code will not be used by non-PCI devices.
> >>>>>
> >>>>> That is correct.
> >>>>>
> >>>>>> In other words, nobody sets "hcd->self.hs_companion" if we use such a device.
> >>>>>> So, I will try to add such a code if needed.
> >>>>>
> >>>>> I think OTG core would have to rely on USB core in providing the right companion device,
> >>>>> just like we rely on it for the primary vs shared HCD case.
> >>>>>
> >>>>
> >>>> OK, it is not so simple.
> >>>>
> >>>> EHCI and companion port handoff is really meant to be software transparent.
> >>>>
> >>>> non-PCI devices really don't have knowledge of which OHCI instance is companion to the EHCI.
> >>>> With device tree we could provide this mapping but for non-device tree case we can't do
> >>>> anything.
> >>>>
> >>>> So my suggestion would be to keep dual role implementation limited to one instance for
> >>>> EHCI + companion case for non-DT.
> >>>> For PCI case I don't see how dual role can be implemented. I don't think we have any
> >>>> dual-role PCI cards.
> >>>
> >>> R-Car Gen2 SoCs (r8a779[0134] / arm32) has USB 2.0 host controllers via PCI bus and
> >>> one high speed function controller via AXI bus.
> >>> One of channel can be used as host or function.
> >>>
> >>>> For DT case we could have a DT binding to tie the EHCI and companion and use that
> >>>> in the OTG framework.
> >>
> >> After looking at the code it seems we don't need this special binding as we are already
> >> linking the EHCI controller and companion controller to the single otg controller instance
> >> using the otg-controller property.
> >>

[...]
> > 
> > Then, how you know this EHCI + companion controller special case during otg adds
> > hcd, it needs special handling, right?
> 
> We know the special case by using the hcd_needs_companion flag.
> 

You had said "we don't need this..", ok, yes, we do need it.
Roger Quadros May 16, 2016, 8:35 a.m. UTC | #16
On 16/05/16 11:13, Peter Chen wrote:
> On Mon, May 16, 2016 at 11:01:27AM +0300, Roger Quadros wrote:
>> On 16/05/16 05:13, Peter Chen wrote:
>>> On Thu, May 12, 2016 at 03:13:48PM +0300, Roger Quadros wrote:
>>>> Hi,
>>>>
>>>> On 12/05/16 13:31, Yoshihiro Shimoda wrote:
>>>>> Hi,
>>>>>
>>>>>> From: Roger Quadros
>>>>>> Sent: Thursday, May 12, 2016 6:32 PM
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> On 12/05/16 11:34, Roger Quadros wrote:
>>>>>>> On 12/05/16 07:00, Yoshihiro Shimoda wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>>> From: Alan Stern
>>>>>>>>> Sent: Wednesday, May 11, 2016 11:47 PM
>>>>>>>>>
>>>>>>>>> On Wed, 11 May 2016, Roger Quadros wrote:
>>>>>>>>>
>>>>>>>>>>> What I mean is if you have 2 EHCI controllers with 2 companion
>>>>>>>>>>> controllers, don't you need to know which companion goes with which EHCI
>>>>>>>>>>> controller? Just like you do for the otg-controller property.
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> That is a very good point. I'm not very sure and it seems that current code won't work
>>>>>>>>>> with multiple EHCI + companion instances.
>>>>>>>>
>>>>>>>> I may misunderstand this topic, but if I use the following environment, it works correctly.
>>>>>>>>
>>>>>>>> < My environment >
>>>>>>>> - an otg controller: Sets hcd-needs-companion.
>>>>>>>> - ehci0 and ohci0 and a function: They connect to the otg controller using "otg-controller" property.
>>>>>>>> - ehci1 and ohci1: No "otg-controller" property.
>>>>>>>> - ehci2 and ohci2: No "otg-controller" property.
>>>>>>>>
>>>>>>>> In this environment, all hosts works correctly.
>>>>>>>> Also I think if we have 2 otg controlelrs, it should be work because otg_dev instance differs.
>>>>>>>
>>>>>>> The topic is about more than one otg controllers and how to tie the right ehci and ohci
>>>>>>> to the correct otg_dev instance especially in cases where we can't depend on probe order.
>>>>>>>
>>>>>>>> Or, does this topic assume an otg controller handles 2 EHCI controllers?
>>>>>>>> I'm not sure such environment actually exists.
>>>>>>>
>>>>>>> No it is not about that.
>>>>>
>>>>> Thank you for the reply. I understood it.
>>>>>
>>>>>>>>>> Alan, does USB core even know which EHCI and OHCI are linked to the same port
>>>>>>>>>> or the handoff is software transparent?
>>>>>>>>>
>>>>>>>>> The core knows.  It doesn't use the information for a whole lot of
>>>>>>>>> things, but it does use it in a couple of places.  Search for
>>>>>>>>> "companion" in core/hcd-pci.c and you'll see.
>>>>>>>>
>>>>>>>> Thank you for the information. I didn't know this code.
>>>>>>>> If my understanding is correct, the core/hcd-pci.c code will not be used by non-PCI devices.
>>>>>>>
>>>>>>> That is correct.
>>>>>>>
>>>>>>>> In other words, nobody sets "hcd->self.hs_companion" if we use such a device.
>>>>>>>> So, I will try to add such a code if needed.
>>>>>>>
>>>>>>> I think OTG core would have to rely on USB core in providing the right companion device,
>>>>>>> just like we rely on it for the primary vs shared HCD case.
>>>>>>>
>>>>>>
>>>>>> OK, it is not so simple.
>>>>>>
>>>>>> EHCI and companion port handoff is really meant to be software transparent.
>>>>>>
>>>>>> non-PCI devices really don't have knowledge of which OHCI instance is companion to the EHCI.
>>>>>> With device tree we could provide this mapping but for non-device tree case we can't do
>>>>>> anything.
>>>>>>
>>>>>> So my suggestion would be to keep dual role implementation limited to one instance for
>>>>>> EHCI + companion case for non-DT.
>>>>>> For PCI case I don't see how dual role can be implemented. I don't think we have any
>>>>>> dual-role PCI cards.
>>>>>
>>>>> R-Car Gen2 SoCs (r8a779[0134] / arm32) has USB 2.0 host controllers via PCI bus and
>>>>> one high speed function controller via AXI bus.
>>>>> One of channel can be used as host or function.
>>>>>
>>>>>> For DT case we could have a DT binding to tie the EHCI and companion and use that
>>>>>> in the OTG framework.
>>>>
>>>> After looking at the code it seems we don't need this special binding as we are already
>>>> linking the EHCI controller and companion controller to the single otg controller instance
>>>> using the otg-controller property.
>>>>
> 
> [...]
>>>
>>> Then, how you know this EHCI + companion controller special case during otg adds
>>> hcd, it needs special handling, right?
>>
>> We know the special case by using the hcd_needs_companion flag.
>>
> 
> You had said "we don't need this..", ok, yes, we do need it.
> 
I'm sorry for the confusion. What I meant by "we don't need this special binding" was that
we don't need additional binding to link the HCD and companion HCD.

cheers,
-roger
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/usb/generic.txt b/Documentation/devicetree/bindings/usb/generic.txt
index f6866c1..1db1c33 100644
--- a/Documentation/devicetree/bindings/usb/generic.txt
+++ b/Documentation/devicetree/bindings/usb/generic.txt
@@ -27,6 +27,9 @@  Optional properties:
  - otg-controller: phandle to otg controller. Host or gadget controllers can
 			contain this property to link it to a particular OTG
 			controller.
+ - hcd-needs-companion: must be present if otg controller is dealing with
+			EHCI host controller that needs a companion OHCI host
+			controller.
 
 This is an attribute to a USB controller such as:
 
diff --git a/drivers/usb/common/usb-otg.c b/drivers/usb/common/usb-otg.c
index 702bca8..77048aa 100644
--- a/drivers/usb/common/usb-otg.c
+++ b/drivers/usb/common/usb-otg.c
@@ -20,6 +20,7 @@ 
 #include <linux/list.h>
 #include <linux/of.h>
 #include <linux/of_platform.h>
+#include <linux/usb/of.h>
 #include <linux/usb/otg.h>
 #include <linux/usb/gadget.h>
 #include <linux/workqueue.h>
@@ -582,6 +583,10 @@  struct usb_otg *usb_otg_register(struct device *dev,
 	else
 		INIT_WORK(&otg->work, usb_drd_work);
 
+	if (of_find_property(dev->of_node, "hcd-needs-companion", NULL) ||
+	    config->hcd_needs_companion)	/* needs companion ? */
+		otg->flags |= OTG_FLAG_HCD_NEEDS_COMPANION;
+
 	otg->wq = create_singlethread_workqueue("usb_otg");
 	if (!otg->wq) {
 		dev_err(dev, "otg: %s: can't create workqueue\n",
@@ -805,15 +810,18 @@  int usb_otg_register_hcd(struct usb_hcd *hcd, unsigned int irqnum,
 	/* HCD will be started by OTG fsm when needed */
 	mutex_lock(&otg->fsm.lock);
 	if (otg->primary_hcd.hcd) {
-		/* probably a shared HCD ? */
-		if (usb_otg_hcd_is_primary_hcd(hcd)) {
+		/* probably a shared HCD or a companion OHCI HCD ? */
+		if (!(otg->flags & OTG_FLAG_HCD_NEEDS_COMPANION) &&
+		    usb_otg_hcd_is_primary_hcd(hcd)) {
 			dev_err(otg_dev, "otg: primary host already registered\n");
 			goto err;
 		}
 
-		if (hcd->shared_hcd == otg->primary_hcd.hcd) {
+		if (otg->flags & OTG_FLAG_HCD_NEEDS_COMPANION ||
+		    (hcd->shared_hcd == otg->primary_hcd.hcd)) {
 			if (otg->shared_hcd.hcd) {
-				dev_err(otg_dev, "otg: shared host already registered\n");
+				dev_err(otg_dev,
+					"otg: shared/companion host already registered\n");
 				goto err;
 			}
 
@@ -821,10 +829,12 @@  int usb_otg_register_hcd(struct usb_hcd *hcd, unsigned int irqnum,
 			otg->shared_hcd.irqnum = irqnum;
 			otg->shared_hcd.irqflags = irqflags;
 			otg->shared_hcd.ops = ops;
-			dev_info(otg_dev, "otg: shared host %s registered\n",
+			dev_info(otg_dev,
+				 "otg: shared/companion host %s registered\n",
 				 dev_name(hcd->self.controller));
 		} else {
-			dev_err(otg_dev, "otg: invalid shared host %s\n",
+			dev_err(otg_dev,
+				"otg: invalid shared/companion host %s\n",
 				dev_name(hcd->self.controller));
 			goto err;
 		}
@@ -847,14 +857,17 @@  int usb_otg_register_hcd(struct usb_hcd *hcd, unsigned int irqnum,
 	 * we're ready only if we have shared HCD
 	 * or we don't need shared HCD.
 	 */
-	if (otg->shared_hcd.hcd || !otg->primary_hcd.hcd->shared_hcd) {
+	if (otg->shared_hcd.hcd ||
+	    (!(otg->flags & OTG_FLAG_HCD_NEEDS_COMPANION) &&
+	     !otg->primary_hcd.hcd->shared_hcd)) {
 		otg->host = hcd_to_bus(hcd);
 		/* FIXME: set bus->otg_port if this is true OTG port with HNP */
 
 		/* start FSM */
 		usb_otg_start_fsm(otg);
 	} else {
-		dev_dbg(otg_dev, "otg: can't start till shared host registers\n");
+		dev_dbg(otg_dev,
+			"otg: can't start till shared/companion host registers\n");
 	}
 
 	mutex_unlock(&otg->fsm.lock);
@@ -905,7 +918,8 @@  int usb_otg_unregister_hcd(struct usb_hcd *hcd)
 			 dev_name(hcd_dev));
 	} else if (hcd == otg->shared_hcd.hcd) {
 		otg->shared_hcd.hcd = NULL;
-		dev_info(otg_dev, "otg: shared host %s unregistered\n",
+		dev_info(otg_dev,
+			 "otg: shared/companion host %s unregistered\n",
 			 dev_name(hcd_dev));
 	} else {
 		dev_err(otg_dev, "otg: host %s wasn't registered with otg\n",
diff --git a/include/linux/usb/otg.h b/include/linux/usb/otg.h
index b094352..6f4ca77 100644
--- a/include/linux/usb/otg.h
+++ b/include/linux/usb/otg.h
@@ -57,7 +57,8 @@  struct otg_hcd {
  * @list: list of otg controllers
  * @work: otg state machine work
  * @wq: otg state machine work queue
- * @flags: to track if host/gadget is running
+ * @flags: to track if host/gadget is running, or to indicate if hcd needs
+ *	   companion
  */
 struct usb_otg {
 	u8			default_a;
@@ -84,6 +85,7 @@  struct usb_otg {
 	u32 flags;
 #define OTG_FLAG_GADGET_RUNNING (1 << 0)
 #define OTG_FLAG_HOST_RUNNING (1 << 1)
+#define OTG_FLAG_HCD_NEEDS_COMPANION (1 << 2)
 	/* use otg->fsm.lock for serializing access */
 
 /*------------- deprecated interface -----------------------------*/
@@ -125,11 +127,14 @@  struct usb_otg_caps {
  * @caps: otg capabilities of the controller
  * @ops: otg fsm operations
  * @otg_work: optional custom otg state machine work function
+ * @hcd_needs_companion: Indicates if host controller needs a companion
+ *			 controller
  */
 struct usb_otg_config {
 	struct usb_otg_caps *otg_caps;
 	struct otg_fsm_ops *fsm_ops;
 	void (*otg_work)(struct work_struct *work);
+	bool hcd_needs_companion;
 };
 
 extern const char *usb_otg_state_string(enum usb_otg_state state);