2010-07-23 09:36:14

by Bryan Wu

[permalink] [raw]
Subject: [PATCH] musb: move usb_add_hcd to the core init code from gadget code

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

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 boards.

Signed-off-by: Bryan Wu <[email protected]>
---
drivers/usb/musb/musb_core.c | 15 +++++++--------
drivers/usb/musb/musb_gadget.c | 18 ------------------
2 files changed, 7 insertions(+), 26 deletions(-)

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index 5eb9318..6b30a6d 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -1521,14 +1521,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
*/
@@ -2071,6 +2063,13 @@ bad_config:
if (status)
goto fail;

+ if (is_otg_enabled(musb)) {
+ status = usb_add_hcd(musb_to_hcd(musb), -1, 0);
+ if (status)
+ goto fail;
+ 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 cbcf14a..ddeb9d8 100644
--- a/drivers/usb/musb/musb_gadget.c
+++ b/drivers/usb/musb/musb_gadget.c
@@ -1748,24 +1748,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


2010-07-23 09:53:33

by David Brownell

[permalink] [raw]
Subject: Re: [PATCH] musb: move usb_add_hcd to the core init code from gadget code

Are you sure this doesn't break the hardware
initialization sequencing on some chips?

I distinctly call losing over a month of development
time on this issue, because the hardware has some
undocumented constraints in this area. The entire
reeason the host init is so "late" is that doing
it earlier (at more logically sensible moments, (in
terms of USB specs vs Mentor silicon) broke things.

- Dave

2010-07-23 10:50:54

by Bryan Wu

[permalink] [raw]
Subject: Re: [PATCH] musb: move usb_add_hcd to the core init code from gadget code

On 07/23/2010 05:53 PM, David Brownell wrote:
> Are you sure this doesn't break the hardware
> initialization sequencing on some chips?
>

We tested on our Beagle board C3/C4 board and will test that on Beagle XM board
and OMAP4 board. Since I still got a Blackfin BF527 board, I will help to test.

For other silicons, I'm not sure about that.

> I distinctly call losing over a month of development
> time on this issue, because the hardware has some
> undocumented constraints in this area. The entire
> reeason the host init is so "late" is that doing
> it earlier (at more logically sensible moments, (in
> terms of USB specs vs Mentor silicon) broke things.
>

Yeah, that's frustrated. I also spent several days here to find a solution to
this. I saw the comments in the code about that undocumented constraints. But
that maybe depends on different silicon.

Thanks, Dave.
-Bryan