On Tue, Feb 28, 2012 at 01:08:06AM -0800, Kuninori Morimoto wrote:
>
> Hi Felipe
>
> Thank you for checking patch.
>
> > > But, from 68d07f64b8a11a852d48d1b05b724c3e20c0d94b,
> > > (USB: Don't fail USB3 probe on missing legacy PCI IRQ.)
> > > current renesas_usbhs didn't call usb_hcd_request_irqs()
> >
> > renesas is not supposed to call that. usb_add_hcd() is calling that,
> > which makes me wonder if this patch is really needed.
>
> Sorry it mean
> renesas_usbhs :: usbhsh_start()
> - usb_add_hcd()
> - usb_hcd_request_irqs().
>
> renesas_usbhs's hc_driver (= usbhsh_driver) doesn't have .irq
> since mod_host/mod_gadet is sharing irq.
> So, it needs hcd->irq = -1.
>
> But current renesas_usbhs can't reach to usb_hcd_request_irqs().
> So, renesas_usbhs hcd->irq is 0.
> Old renesas_usbhs could reach to usb_hcd_request_irqs().
> So hcd->irq was -1
>
> > > /* add hcd */
> > > - ret = usb_add_hcd(hcd, 0, 0);
> > > + ret = usb_add_hcd(hcd, -1, 0);
> >
> > what will happen here is that usb_add_hcd() will call
> > usb_hcd_request_irqs() which will fall into the else branch:
> >
> > 2325 static int usb_hcd_request_irqs(struct usb_hcd *hcd,
> > 2326 unsigned int irqnum, unsigned long irqflags)
> > 2327 {
> > 2328 int retval;
> > 2329
> > 2330 if (hcd->driver->irq) {
> (snip)
> > 2354 } else {
> > 2355 hcd->irq = -1;
> > 2356 if (hcd->rsrc_start)
> > 2357 dev_info(hcd->self.controller, "%s 0x%08llx\n",
> > 2358 (hcd->driver->flags & HCD_MEMORY) ?
> > 2359 "io mem" : "io base",
> > 2360 (unsigned long long)hcd->rsrc_start);
> > 2361 }
> > 2362 return 0;
> > 2363 }
> >
> > Are you sure you never initialize driver->irq by mistake ??
>
> renesas_usbhs doesn't use hcd's driver->irq.
> it is using sharing irq between mod_host/mod_gadet.
> renesas_usbh/mod.c :: usbhs_mod_probe() call request_irq() for it.
>
> Current renesas_usbh issue is that
> hcd->irq = 0 even though it doesn't have/use driver->irq.
> Then, usb_remove_hcd() will call free_irq(), and get WARNING.
>
> void usb_remove_hcd(struct usb_hcd *hcd)
> {
> ...
> if (usb_hcd_is_primary_hcd(hcd)) {
> if (hcd->irq >= 0)
the bug is this check. IRQ 0 mean NO_IRQ. Alan, Greg, Sarah, would you
take the following:
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index eb19cba..cffbd2dc 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -2571,7 +2571,7 @@ void usb_remove_hcd(struct usb_hcd *hcd)
del_timer_sync(&hcd->rh_timer);
if (usb_hcd_is_primary_hcd(hcd)) {
- if (hcd->irq >= 0)
+ if (hcd->irq > 0)
free_irq(hcd->irq, hcd);
}
--
balbi
Attachment:
signature.asc
Description: Digital signature
