[PATCH] musb: move usb_add_hcd to the core init code from gadget code (v2)

[PATCH] musb: move usb_add_hcd to the core init code from gadget code (v2)

[Bryan Wu]

BugLink: http://bugs.launchpad.net/bugs/608312

v2:
fix the building error on latest 2.6.35-rc kernel, since v1 was generated in
2.6.33 kernel.

v1:
usb_add_hcd was only called when we insmod the gadget class module or built-in
that gadget class driver. If musb is configured as OTG controller, we need to
insmod or built-in gadget class driver to make our Host mode fucntion works.

In our Ubuntu system, normally we compiled all the gadget class drivers as
modules. Then users can insmod the gadget modules as they want. But without the
gadget class driver running, we needs host function to support common USB
devices.

This patch fix this issue and tested on omap3 beagle board and Gumstix board.

Signed-off-by: Bryan Wu <bryan.wu@canonical.com>
---
 drivers/usb/musb/musb_core.c   |   13 +++++--------
 drivers/usb/musb/musb_gadget.c |   18 ------------------
 2 files changed, 5 insertions(+), 26 deletions(-)

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index 3b795c5..1b6d74c 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -1583,14 +1583,6 @@ irqreturn_t musb_interrupt(struct musb *musb)
 		(devctl & MUSB_DEVCTL_HM) ? "host" : "peripheral",
 		musb->int_usb, musb->int_tx, musb->int_rx);
 
-#ifdef CONFIG_USB_GADGET_MUSB_HDRC
-	if (is_otg_enabled(musb) || is_peripheral_enabled(musb))
-		if (!musb->gadget_driver) {
-			DBG(5, "No gadget driver loaded\n");
-			return IRQ_HANDLED;
-		}
-#endif
-
 	/* the core can interrupt us for multiple reasons; docs have
 	 * a generic interrupt flowchart to follow
 	 */
@@ -2128,6 +2120,11 @@ bad_config:
 
 		status = musb_gadget_setup(musb);
 
+		if (is_otg_enabled(musb)) {
+			status = usb_add_hcd(musb_to_hcd(musb), -1, 0);
+			musb_start(musb);
+		}
+
 		DBG(1, "%s mode, status %d, dev%02x\n",
 			is_otg_enabled(musb) ? "OTG" : "PERIPHERAL",
 			status,
diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
index 6fca870..9e55534 100644
--- a/drivers/usb/musb/musb_gadget.c
+++ b/drivers/usb/musb/musb_gadget.c
@@ -1761,24 +1761,6 @@ int usb_gadget_register_driver(struct usb_gadget_driver *driver)
 		otg_set_peripheral(musb->xceiv, &musb->g);
 
 		spin_unlock_irqrestore(&musb->lock, flags);
-
-		if (is_otg_enabled(musb)) {
-			DBG(3, "OTG startup...\n");
-
-			/* REVISIT:  funcall to other code, which also
-			 * handles power budgeting ... this way also
-			 * ensures HdrcStart is indirectly called.
-			 */
-			retval = usb_add_hcd(musb_to_hcd(musb), -1, 0);
-			if (retval < 0) {
-				DBG(1, "add_hcd failed, %d\n", retval);
-				spin_lock_irqsave(&musb->lock, flags);
-				otg_set_peripheral(musb->xceiv, NULL);
-				musb->gadget_driver = NULL;
-				musb->g.dev.driver = NULL;
-				spin_unlock_irqrestore(&musb->lock, flags);
-			}
-		}
 	}
 
 	return retval;
-- 
1.7.0.4

Re: [PATCH] musb: move usb_add_hcd to the core init code from gadget code (v2)

[Sergei Shtylyov]

Hello.

Bryan Wu wrote:

> BugLink: http://bugs.launchpad.net/bugs/608312

> v2:
> fix the building error on latest 2.6.35-rc kernel, since v1 was generated in
> 2.6.33 kernel.

    The remarks about patch changes should follow the --- tear line.

> v1:
> usb_add_hcd was only called when we insmod the gadget class module or built-in
> that gadget class driver. If musb is configured as OTG controller, we need to
> insmod or built-in gadget class driver to make our Host mode fucntion works.

> In our Ubuntu system, normally we compiled all the gadget class drivers as
> modules. Then users can insmod the gadget modules as they want. But without the
> gadget class driver running, we needs host function to support common USB
> devices.

> This patch fix this issue and tested on omap3 beagle board and Gumstix board.

> Signed-off-by: Bryan Wu <bryan.wu@canonical.com>

WBR, Sergei

Re: [PATCH] musb: move usb_add_hcd to the core init code from gadget code (v2)

[Bryan Wu]

On 07/23/2010 11:28 PM, Sergei Shtylyov wrote:
> Hello.
> 
> Bryan Wu wrote:
> 
>> BugLink: http://bugs.launchpad.net/bugs/608312
> 
>> v2:
>> fix the building error on latest 2.6.35-rc kernel, since v1 was
>> generated in
>> 2.6.33 kernel.
> 
>    The remarks about patch changes should follow the --- tear line.
> 

Thanks Sergei, I will take care of it in the future.

Any other comments about this patch?

Felipe, could you please take a look? or need I ping you in the Facebook, -:))

-Bryan

>> v1:
>> usb_add_hcd was only called when we insmod the gadget class module or
>> built-in
>> that gadget class driver. If musb is configured as OTG controller, we
>> need to
>> insmod or built-in gadget class driver to make our Host mode fucntion
>> works.
> 
>> In our Ubuntu system, normally we compiled all the gadget class
>> drivers as
>> modules. Then users can insmod the gadget modules as they want. But
>> without the
>> gadget class driver running, we needs host function to support common USB
>> devices.
> 
>> This patch fix this issue and tested on omap3 beagle board and Gumstix
>> board.
> 
>> Signed-off-by: Bryan Wu <bryan.wu@canonical.com>
> 
> WBR, Sergei

Re: [PATCH] musb: move usb_add_hcd to the core init code from gadget code (v2)

[Bryan Wu]

Felipe,

Any comments on this patch?

Thanks a lot,
-Bryan


On 07/23/2010 10:36 PM, Bryan Wu wrote:
> BugLink: http://bugs.launchpad.net/bugs/608312
>
> v2:
> fix the building error on latest 2.6.35-rc kernel, since v1 was generated in
> 2.6.33 kernel.
>
> v1:
> usb_add_hcd was only called when we insmod the gadget class module or built-in
> that gadget class driver. If musb is configured as OTG controller, we need to
> insmod or built-in gadget class driver to make our Host mode fucntion works.
>
> In our Ubuntu system, normally we compiled all the gadget class drivers as
> modules. Then users can insmod the gadget modules as they want. But without the
> gadget class driver running, we needs host function to support common USB
> devices.
>
> This patch fix this issue and tested on omap3 beagle board and Gumstix board.
>
> Signed-off-by: Bryan Wu<bryan.wu@canonical.com>
> ---
>   drivers/usb/musb/musb_core.c   |   13 +++++--------
>   drivers/usb/musb/musb_gadget.c |   18 ------------------
>   2 files changed, 5 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
> index 3b795c5..1b6d74c 100644
> --- a/drivers/usb/musb/musb_core.c
> +++ b/drivers/usb/musb/musb_core.c
> @@ -1583,14 +1583,6 @@ irqreturn_t musb_interrupt(struct musb *musb)
>   		(devctl&  MUSB_DEVCTL_HM) ? "host" : "peripheral",
>   		musb->int_usb, musb->int_tx, musb->int_rx);
>
> -#ifdef CONFIG_USB_GADGET_MUSB_HDRC
> -	if (is_otg_enabled(musb) || is_peripheral_enabled(musb))
> -		if (!musb->gadget_driver) {
> -			DBG(5, "No gadget driver loaded\n");
> -			return IRQ_HANDLED;
> -		}
> -#endif
> -
>   	/* the core can interrupt us for multiple reasons; docs have
>   	 * a generic interrupt flowchart to follow
>   	 */
> @@ -2128,6 +2120,11 @@ bad_config:
>
>   		status = musb_gadget_setup(musb);
>
> +		if (is_otg_enabled(musb)) {
> +			status = usb_add_hcd(musb_to_hcd(musb), -1, 0);
> +			musb_start(musb);
> +		}
> +
>   		DBG(1, "%s mode, status %d, dev%02x\n",
>   			is_otg_enabled(musb) ? "OTG" : "PERIPHERAL",
>   			status,
> diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
> index 6fca870..9e55534 100644
> --- a/drivers/usb/musb/musb_gadget.c
> +++ b/drivers/usb/musb/musb_gadget.c
> @@ -1761,24 +1761,6 @@ int usb_gadget_register_driver(struct usb_gadget_driver *driver)
>   		otg_set_peripheral(musb->xceiv,&musb->g);
>
>   		spin_unlock_irqrestore(&musb->lock, flags);
> -
> -		if (is_otg_enabled(musb)) {
> -			DBG(3, "OTG startup...\n");
> -
> -			/* REVISIT:  funcall to other code, which also
> -			 * handles power budgeting ... this way also
> -			 * ensures HdrcStart is indirectly called.
> -			 */
> -			retval = usb_add_hcd(musb_to_hcd(musb), -1, 0);
> -			if (retval<  0) {
> -				DBG(1, "add_hcd failed, %d\n", retval);
> -				spin_lock_irqsave(&musb->lock, flags);
> -				otg_set_peripheral(musb->xceiv, NULL);
> -				musb->gadget_driver = NULL;
> -				musb->g.dev.driver = NULL;
> -				spin_unlock_irqrestore(&musb->lock, flags);
> -			}
> -		}
>   	}
>
>   	return retval;

Re: [PATCH] musb: move usb_add_hcd to the core init code from gadget code (v2)

[Brad Parker]

I know this is a top post, and a comment on a post from August, but I'd like
to point out that without this patch I can't get musb host mode to work
on the beagle board xm (3630).

With this patch, host mode works.  If I disconnect and reconnect the port
goes into "a_idle", which is almost right, and will go into host mode if
I "echo >/sys/devices/platform/musb_hdrc/mode".

(my expectation is that the mode should toggle from "b_idle" to "a_host"
when I plug in an a connector with the id pin tied correctly, and back 
to "b_idle"
when I disconnect)

It seems like few people people test host mode on the omap twl otg port,
but I have applications which use it...

should this patch get more attention?  (there was no response to the 
initial post I could find)

-brad

Bryan Wu wrote:
> Felipe,
>
> Any comments on this patch?
>
> Thanks a lot,
> -Bryan
>
>
> On 07/23/2010 10:36 PM, Bryan Wu wrote:
>> BugLink: http://bugs.launchpad.net/bugs/608312
>>
>> v2:
>> fix the building error on latest 2.6.35-rc kernel, since v1 was 
>> generated in
>> 2.6.33 kernel.
>>
>> v1:
>> usb_add_hcd was only called when we insmod the gadget class module or 
>> built-in
>> that gadget class driver. If musb is configured as OTG controller, we 
>> need to
>> insmod or built-in gadget class driver to make our Host mode fucntion 
>> works.
>>
>> In our Ubuntu system, normally we compiled all the gadget class 
>> drivers as
>> modules. Then users can insmod the gadget modules as they want. But 
>> without the
>> gadget class driver running, we needs host function to support common 
>> USB
>> devices.
>>
>> This patch fix this issue and tested on omap3 beagle board and 
>> Gumstix board.
>>
>> Signed-off-by: Bryan Wu<bryan.wu@canonical.com>
>> ---
>>   drivers/usb/musb/musb_core.c   |   13 +++++--------
>>   drivers/usb/musb/musb_gadget.c |   18 ------------------
>>   2 files changed, 5 insertions(+), 26 deletions(-)
>>
>> diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
>> index 3b795c5..1b6d74c 100644
>> --- a/drivers/usb/musb/musb_core.c
>> +++ b/drivers/usb/musb/musb_core.c
>> @@ -1583,14 +1583,6 @@ irqreturn_t musb_interrupt(struct musb *musb)
>>           (devctl&  MUSB_DEVCTL_HM) ? "host" : "peripheral",
>>           musb->int_usb, musb->int_tx, musb->int_rx);
>>
>> -#ifdef CONFIG_USB_GADGET_MUSB_HDRC
>> -    if (is_otg_enabled(musb) || is_peripheral_enabled(musb))
>> -        if (!musb->gadget_driver) {
>> -            DBG(5, "No gadget driver loaded\n");
>> -            return IRQ_HANDLED;
>> -        }
>> -#endif
>> -
>>       /* the core can interrupt us for multiple reasons; docs have
>>        * a generic interrupt flowchart to follow
>>        */
>> @@ -2128,6 +2120,11 @@ bad_config:
>>
>>           status = musb_gadget_setup(musb);
>>
>> +        if (is_otg_enabled(musb)) {
>> +            status = usb_add_hcd(musb_to_hcd(musb), -1, 0);
>> +            musb_start(musb);
>> +        }
>> +
>>           DBG(1, "%s mode, status %d, dev%02x\n",
>>               is_otg_enabled(musb) ? "OTG" : "PERIPHERAL",
>>               status,
>> diff --git a/drivers/usb/musb/musb_gadget.c 
>> b/drivers/usb/musb/musb_gadget.c
>> index 6fca870..9e55534 100644
>> --- a/drivers/usb/musb/musb_gadget.c
>> +++ b/drivers/usb/musb/musb_gadget.c
>> @@ -1761,24 +1761,6 @@ int usb_gadget_register_driver(struct 
>> usb_gadget_driver *driver)
>>           otg_set_peripheral(musb->xceiv,&musb->g);
>>
>>           spin_unlock_irqrestore(&musb->lock, flags);
>> -
>> -        if (is_otg_enabled(musb)) {
>> -            DBG(3, "OTG startup...\n");
>> -
>> -            /* REVISIT:  funcall to other code, which also
>> -             * handles power budgeting ... this way also
>> -             * ensures HdrcStart is indirectly called.
>> -             */
>> -            retval = usb_add_hcd(musb_to_hcd(musb), -1, 0);
>> -            if (retval<  0) {
>> -                DBG(1, "add_hcd failed, %d\n", retval);
>> -                spin_lock_irqsave(&musb->lock, flags);
>> -                otg_set_peripheral(musb->xceiv, NULL);
>> -                musb->gadget_driver = NULL;
>> -                musb->g.dev.driver = NULL;
>> -                spin_unlock_irqrestore(&musb->lock, flags);
>> -            }
>> -        }
>>       }
>>
>>       return retval;
>
> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] musb: move usb_add_hcd to the core init code from gadget code (v2)

[Felipe Balbi]

Hi,

On Mon, Jan 10, 2011 at 06:47:58AM -0500, Brad Parker wrote:
> I know this is a top post, and a comment on a post from August, but I'd like
> to point out that without this patch I can't get musb host mode to work
> on the beagle board xm (3630).
> 
> With this patch, host mode works.  If I disconnect and reconnect the port
> goes into "a_idle", which is almost right, and will go into host mode if
> I "echo >/sys/devices/platform/musb_hdrc/mode".
> 
> (my expectation is that the mode should toggle from "b_idle" to "a_host"
> when I plug in an a connector with the id pin tied correctly, and
> back to "b_idle"
> when I disconnect)
> 
> It seems like few people people test host mode on the omap twl otg port,
> but I have applications which use it...
> 
> should this patch get more attention?  (there was no response to the
> initial post I could find)

What's the problem in simply insmoding a gadget driver ? If you have OTG
enabled, you _need_ a gadget driver to have a fully working setup. If
you want host only, then change Kconfig.

-- 
balbi

Re: [PATCH] musb: move usb_add_hcd to the core init code from gadget code (v2)

[David Brownell]

> With this patch, host mode works.

What about OTG and gadget modes?

The MUSB hardware is flakey with respect to
those init issues.  Re-ordering host vs gadget
init has been a good way to waste a few months,
for anyone who has a few to waste.  (And a
better way to waste them for folk who do NOT
but just need to stay on schedule.)  One learns
quickly that the Mentor IP follows some other
state machine than the USB-OTG says, but even their
docs don't help uncover whatever init model they
are trying  to follow.

Re: [PATCH] musb: move usb_add_hcd to the core init code from gadget code (v2)

[David Brownell]

> With this patch, host mode works.

What about OTG and gadget modes?

The MUSB hardware is flakey with respect to
those init issues.  Re-ordering host vs gadget
init has been a good way to waste a few months,
for anyone who has a few to waste.  (And a
better way to waste them for folk who do NOT
but just need to stay on schedule.)  One learns
quickly that the Mentor IP follows some other
state machine than the USB-OTG says, but even their
docs don't help uncover whatever init model they
are trying  to follow.


--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: UNS: Re: [PATCH] musb: move usb_add_hcd to the core init code from gadget code (v2)

[Brad Parker]

Felipe Balbi wrote:
> Hi,
>
> On Mon, Jan 10, 2011 at 06:47:58AM -0500, Brad Parker wrote:
>   
>> ...
>> (my expectation is that the mode should toggle from "b_idle" to "a_host"
>> when I plug in an a connector with the id pin tied correctly, and
>> back to "b_idle"
>> when I disconnect)
>>     
> What's the problem in simply insmoding a gadget driver ? If you have OTG
> enabled, you _need_ a gadget driver to have a fully working setup. If
> you want host only, then change Kconfig
I guess there are multiple "modalities".

- some users don't use a gadget driver and expect host mode to work 
(they fail)
- some users use a gadget driver and host mode (apparently that works)
- some users use a gadget driver only (they work)

I'd  like to avoid having multiple kernels.  (and to be honest, I'm not 
convinced
just convince I can get the right behaviors by changing the .config; I 
assume
you mean .config and not Kconfig...  I don't think it works but I'll try 
it out)

-brad