I'm forwarding the questions to linux-usb emial list. Thanks a lot for everyone's help, hope we can get some feedback soon, the issue has been bothering us for quite a while now. Fei -----Original Message----- From: Hunter, Jon [mailto:jon-hunter@xxxxxx] Sent: Monday, June 29, 2009 9:07 AM To: Yang Fei-AFY095; Gadiyar, Anand; Pandita, Vikram; Ghongdemath, Girish Subject: RE: ehci->watchdog timer panic Hi Fei, I think that we should send the below email to the linux-usb (linux-usb@xxxxxxxxxxxxxxx) mailing list. Either I can send this email or you can. However, it was your patch so I thought that you should send this and take the credit if they would accept this. Let me know what you think. Cheers Jon _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ Hello All, I am looking for some advice on the correct method for utlising the ehci_bus_suspend() function on an OMAP3 device. On the OMAP3 device the usbhost controller is in a separate internal power-domain. So when the usbhost is inactive or suspend is called, we can disable clocks and power-down the usbhost to save power. Recently we found that after calling ehci_bus_suspend() and disabling the usbhost clocks we would see the ehci watchdog timer event fire. This was causing a kernel panic because the usbhost controllers clocks were disabled and inside the watchdog timer function the clocks were not being re-enabled, so when the ehci registers were accessed this resulted in a CPU data-abort. >From reviewing the ehci_bus_suspend() function, I do see that after deleting any pending watchdog timer events, there is a called made to ehci_work() (see below code snippet). From looking at the ehci_work() function it does appear to be possible that another watchdog timer event could be scheduled, but I have been unable to confirm this. Unfortunately, this failure is not easily reproduced. --- snip --- static int ehci_bus_suspend (struct usb_hcd *hcd) { struct ehci_hcd *ehci = hcd_to_ehci (hcd); int port; int mask; ehci_dbg(ehci, "suspend root hub\n"); if (time_before (jiffies, ehci->next_statechange)) msleep(5); del_timer_sync(&ehci->watchdog); del_timer_sync(&ehci->iaa_watchdog); port = HCS_N_PORTS (ehci->hcs_params); spin_lock_irq (&ehci->lock); /* stop schedules, clean any completed work */ if (HC_IS_RUNNING(hcd->state)) { ehci_quiesce (ehci); hcd->state = HC_STATE_QUIESCING; } ehci->command = ehci_readl(ehci, &ehci->regs->command); ehci_work(ehci); --- snip --- Hence, I wanted to ask the following questions: 1). Theoretically is it possible that a call to ehci_work() during the suspend could cause another watchdog timer event to be scheduled? 2). If there is a watchdog event pending after calling ehci_bus_suspend(), should we: a). Make the function, ehci_bus_suspend, return an error code -EBUSY to indicate there is still work pending? See below. diff --git a/drivers/usb/host/ehci-hub.c b/drivers/usb/host/ehci-hub.c index 003f65a..6c1bca1 100644 --- a/drivers/usb/host/ehci-hub.c +++ b/drivers/usb/host/ehci-hub.c @@ -131,6 +131,8 @@ static int ehci_bus_suspend (struct usb_hcd *hcd) } ehci->command = ehci_readl(ehci, &ehci->regs->command); ehci_work(ehci); + if (timer_pending(&ehci->watchdog)) + return -EBUSY; /* Unlike other USB host controller types, EHCI doesn't have * any notion of "global" or bus-wide suspend. The driver has b). Make the function calling the ehci_bus_suspend, returns an error to indicate that we could not disable the clocks and suspend failed? ________________________________________ From: Yang Fei-AFY095 [fei.yang@xxxxxxxxxxxx] Sent: Friday, June 26, 2009 5:38 PM To: Hunter, Jon; Gadiyar, Anand; Pandita, Vikram; Ghongdemath, Girish Subject: ehci->watchdog timer panic Jon, I had the attached change to fix the ehci->watchdog panic, there is no such panic observed after applying this patch so far. Can you please help take a look and provide your feedback? I think we will need to bring this up to the open source community as well. Let me know your opinion. Thanks, Fei -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
