| Message ID | 20140407160410.GC20228@saruman.home |
|---|---|
| State | Accepted |
| Commit | 8b2bc2c9351b4c09bc3d9096e2a7af3988565dbf |
| Headers | show |
On 07.04.2014 18:04, Felipe Balbi wrote: <snip> > that's not caused by my patch, it's a previously existing bug. This > should sort it out: > > commit e7f69404a878b5345ad07bf06d78559ecd31db79 > Author: Felipe Balbi <balbi@ti.com> > Date: Mon Apr 7 10:58:01 2014 -0500 > > usb: musb: omap2430: make sure clocks are enabled when running mailbox > > on early initialization we could fall into > a situation where the mailbox is called before > MUSB's clocks are running, in order to avoid > that, make sure mailbox is always wrapped with > pm_runtime calls. > > Reported-by: Stefan Roese <sr@denx.de> > Signed-off-by: Felipe Balbi <balbi@ti.com> > > diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c > index 819a7cd..d369bf1 100644 > --- a/drivers/usb/musb/omap2430.c > +++ b/drivers/usb/musb/omap2430.c > @@ -316,7 +316,13 @@ static void omap_musb_mailbox_work(struct work_struct *mailbox_work) > { > struct omap2430_glue *glue = container_of(mailbox_work, > struct omap2430_glue, omap_musb_mailbox_work); > + struct musb *musb = glue_to_musb(glue); > + struct device *dev = musb->controller; > + > + pm_runtime_get_sync(dev); > omap_musb_set_mailbox(glue); > + pm_runtime_mark_last_busy(dev); > + pm_runtime_put_autosuspend(dev); > } > > static irqreturn_t omap2430_musb_interrupt(int irq, void *__hci) > > let me know The crash doesn't appear anymore with this patch. But the ethernet gadget is not started. This happens also with a plugged cable upon driver startup. Now I see an error in the log: [ 2.793121] musb-hdrc musb-hdrc.0.auto: VBUS_ERROR in b_idle (80, <SessEnd), retry #0, port1 00000100 Thanks, Stefan -- 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
Hi, On Tue, Apr 08, 2014 at 09:24:09AM +0200, Stefan Roese wrote: > On 07.04.2014 18:04, Felipe Balbi wrote: > > <snip> > > > that's not caused by my patch, it's a previously existing bug. This > > should sort it out: > > > > commit e7f69404a878b5345ad07bf06d78559ecd31db79 > > Author: Felipe Balbi <balbi@ti.com> > > Date: Mon Apr 7 10:58:01 2014 -0500 > > > > usb: musb: omap2430: make sure clocks are enabled when running mailbox > > > > on early initialization we could fall into > > a situation where the mailbox is called before > > MUSB's clocks are running, in order to avoid > > that, make sure mailbox is always wrapped with > > pm_runtime calls. > > > > Reported-by: Stefan Roese <sr@denx.de> > > Signed-off-by: Felipe Balbi <balbi@ti.com> > > > > diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c > > index 819a7cd..d369bf1 100644 > > --- a/drivers/usb/musb/omap2430.c > > +++ b/drivers/usb/musb/omap2430.c > > @@ -316,7 +316,13 @@ static void omap_musb_mailbox_work(struct work_struct *mailbox_work) > > { > > struct omap2430_glue *glue = container_of(mailbox_work, > > struct omap2430_glue, omap_musb_mailbox_work); > > + struct musb *musb = glue_to_musb(glue); > > + struct device *dev = musb->controller; > > + > > + pm_runtime_get_sync(dev); > > omap_musb_set_mailbox(glue); > > + pm_runtime_mark_last_busy(dev); > > + pm_runtime_put_autosuspend(dev); > > } > > > > static irqreturn_t omap2430_musb_interrupt(int irq, void *__hci) > > > > let me know > > The crash doesn't appear anymore with this patch. But the ethernet > gadget is not started. This happens also with a plugged cable upon > driver startup. Now I see an error in the log: > > [ 2.793121] musb-hdrc musb-hdrc.0.auto: VBUS_ERROR in b_idle (80, <SessEnd), retry #0, port1 00000100 that shouldn't happen... Can you add some extra debugging prints and try to figure out what's going on ?
Hi Felipe, On 08.04.2014 16:31, Felipe Balbi wrote: >>> diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c >>> index 819a7cd..d369bf1 100644 >>> --- a/drivers/usb/musb/omap2430.c >>> +++ b/drivers/usb/musb/omap2430.c >>> @@ -316,7 +316,13 @@ static void omap_musb_mailbox_work(struct work_struct *mailbox_work) >>> { >>> struct omap2430_glue *glue = container_of(mailbox_work, >>> struct omap2430_glue, omap_musb_mailbox_work); >>> + struct musb *musb = glue_to_musb(glue); >>> + struct device *dev = musb->controller; >>> + >>> + pm_runtime_get_sync(dev); >>> omap_musb_set_mailbox(glue); >>> + pm_runtime_mark_last_busy(dev); >>> + pm_runtime_put_autosuspend(dev); >>> } >>> >>> static irqreturn_t omap2430_musb_interrupt(int irq, void *__hci) >>> >>> let me know >> >> The crash doesn't appear anymore with this patch. But the ethernet >> gadget is not started. This happens also with a plugged cable upon >> driver startup. Now I see an error in the log: >> >> [ 2.793121] musb-hdrc musb-hdrc.0.auto: VBUS_ERROR in b_idle (80, <SessEnd), retry #0, port1 00000100 > > that shouldn't happen... Can you add some extra debugging prints and try > to figure out what's going on ? Unfortunately I can't reproduce this error. And the crash is also not happening right now. Seems to be very timing sensitive. Still with both patches applied, the ethernet gadget is only started correctly (with cable connected after driver loading) when I add some printk's. This one helps for example: static void musb_do_idle(unsigned long _musb) { struct musb *musb = (void *)_musb; unsigned long flags; u8 power; u8 devctl; #if 1 // test-only: this helps as well with gadget to connect printk("*** %s (%d): state=%d\n", __func__, __LINE__, musb->xceiv->state); // test-only #endif Without this printk the gadget is not started upon cable insertion. No MUSB interrupts occur when the USB cable is connected. There seems to be some timing dependency which I fail to solve quickly. The original locations (removed with [1]) for phy_power_on() and phy_power_off() don't suffer these timing / printk dependencies. Any ideas? Thanks, Stefan [1] 30a70b02 (usb: musb: fix obex in g_nokia.ko causing kernel panic) -- 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
On Thu, Apr 10, 2014 at 03:41:16PM +0200, Stefan Roese wrote: > Hi Felipe, > > On 08.04.2014 16:31, Felipe Balbi wrote: > >>> diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c > >>> index 819a7cd..d369bf1 100644 > >>> --- a/drivers/usb/musb/omap2430.c > >>> +++ b/drivers/usb/musb/omap2430.c > >>> @@ -316,7 +316,13 @@ static void omap_musb_mailbox_work(struct work_struct *mailbox_work) > >>> { > >>> struct omap2430_glue *glue = container_of(mailbox_work, > >>> struct omap2430_glue, omap_musb_mailbox_work); > >>> + struct musb *musb = glue_to_musb(glue); > >>> + struct device *dev = musb->controller; > >>> + > >>> + pm_runtime_get_sync(dev); > >>> omap_musb_set_mailbox(glue); > >>> + pm_runtime_mark_last_busy(dev); > >>> + pm_runtime_put_autosuspend(dev); > >>> } > >>> > >>> static irqreturn_t omap2430_musb_interrupt(int irq, void *__hci) > >>> > >>> let me know > >> > >> The crash doesn't appear anymore with this patch. But the ethernet > >> gadget is not started. This happens also with a plugged cable upon > >> driver startup. Now I see an error in the log: > >> > >> [ 2.793121] musb-hdrc musb-hdrc.0.auto: VBUS_ERROR in b_idle (80, <SessEnd), retry #0, port1 00000100 > > > > that shouldn't happen... Can you add some extra debugging prints and try > > to figure out what's going on ? > > Unfortunately I can't reproduce this error. And the crash is also not > happening right now. Seems to be very timing sensitive. > > Still with both patches applied, the ethernet gadget is only started > correctly (with cable connected after driver loading) when I add some > printk's. This one helps for example: > > static void musb_do_idle(unsigned long _musb) > { > struct musb *musb = (void *)_musb; > unsigned long flags; > u8 power; > u8 devctl; > > #if 1 // test-only: this helps as well with gadget to connect > printk("*** %s (%d): state=%d\n", __func__, __LINE__, musb->xceiv->state); // test-only > #endif > > Without this printk the gadget is not started upon cable insertion. > No MUSB interrupts occur when the USB cable is connected. > > There seems to be some timing dependency which I fail to solve > quickly. The original locations (removed with [1]) for > phy_power_on() and phy_power_off() don't suffer these timing / > printk dependencies. > > Any ideas? no ideas, I'm guessing there's a race somewhere, but no time to deal with that right now. Patches are welcome though.
diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c index 819a7cd..d369bf1 100644 --- a/drivers/usb/musb/omap2430.c +++ b/drivers/usb/musb/omap2430.c @@ -316,7 +316,13 @@ static void omap_musb_mailbox_work(struct work_struct *mailbox_work) { struct omap2430_glue *glue = container_of(mailbox_work, struct omap2430_glue, omap_musb_mailbox_work); + struct musb *musb = glue_to_musb(glue); + struct device *dev = musb->controller; + + pm_runtime_get_sync(dev); omap_musb_set_mailbox(glue); + pm_runtime_mark_last_busy(dev); + pm_runtime_put_autosuspend(dev); } static irqreturn_t omap2430_musb_interrupt(int irq, void *__hci)