[Fwd: patch usb-ehci-ensure-all-watchdog-timer-events-are-deleted-when-suspending-usb.patch added to gregkh-2.6 tree]
Subsystems (2)
Jon Hunter
AppliedHi All,
I just thought I would send out this patch as an FYI. We recently ran into an issue where we were getting a kernel panic in the ehci driver when entering suspend on the omap3. We discussed this with the linux-usb folks and they recommended the below fix. This problem is hard to reproduce, but has not been seen since adding this fix. Anyway the details are below.
Cheers
Jon
-------- Original Message --------
Subject: patch usb-ehci-ensure-all-watchdog-timer-events-are-deleted-when-suspending-usb.patch added to gregkh-2.6 tree
Date: Fri, 14 Aug 2009 11:48:45 -0500
From: [email protected] <[email protected]>
To: Hunter, Jon <[email protected]>, [email protected] <[email protected]>, [email protected] <[email protected]>, [email protected] <[email protected]>, [email protected] <[email protected]>
This is a note to let you know that I've just added the patch titled
Subject: USB: EHCI: ensure all watchdog timer events are deleted
when suspending usb
to my gregkh-2.6 tree. Its filename is
usb-ehci-ensure-all-watchdog-timer-events-are-deleted-when-suspending-usb.patch
This tree can be found at
http://www.kernel.org/pub/linux/kernel/people/gregkh/gregkh-2.6/patches/
From [email protected] Thu Aug 13 16:37:16 2009
From: Jon Hunter <[email protected]>
Date: Wed, 12 Aug 2009 11:57:59 -0400 (EDT)
Subject: USB: EHCI: ensure all watchdog timer events are deleted when suspending usb
To: Greg KH <[email protected]>, Jon Hunter <[email protected]>
Cc: Fei Yang <[email protected]>
Message-ID: <[email protected]>
From: Jon Hunter <[email protected]>
This patch was previously discussed in the following thread:
http://thread.gmane.org/gmane.linux.usb.general/19472/focus=19484
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.
To avoid this panic, per recommendation from Alan Stern (see above thread), we make sure any pending timer events (that may have been scheduled by calling ehci_work within the ehci_bus_suspend() function) are deleted before returning.
--- drivers/usb/host/ehci-hub.c | 5 +++++ 1 file changed, 5 insertions(+)
drivers/usb/host/ehci-hub.c +5 -0
@@ -209,6 +209,11 @@ static int ehci_bus_suspend (struct usb_ ehci->next_statechange = jiffies +msecs_to_jiffies (10 );spin_unlock_irq (&ehci->lock); + +/* ehci_work() may have re-enabled the watchdog timer, which we do not +* want, and so we must delete any pending watchdog timer events. +*/ +del_timer_sync (&ehci->watchdog);return 0 ; }
Quoted 9 lines · Jon Hunter
Hi All,
I just thought I would send out this patch as an FYI. We recently ran into an issue where we were getting a kernel panic in the ehci driver when entering suspend on the omap3. We discussed this with the linux-usb folks and they recommended the below fix. This problem is hard to reproduce, but has not been seen since adding this fix. Anyway the details are below.
Jon,
Do you know if this will be queued as a fix for .31? or if it will wait for the .32 merge window.
Kevin
Quoted 89 lines
-------- Original Message --------
Subject: patch usb-ehci-ensure-all-watchdog-timer-events-are-deleted-when-suspending-usb.patch added to gregkh-2.6 tree
Date: Fri, 14 Aug 2009 11:48:45 -0500
From: [email protected] <[email protected]>
To: Hunter, Jon <[email protected]>, [email protected] <[email protected]>, [email protected] <[email protected]>,
[email protected] <[email protected]>, [email protected]
<[email protected]>This is a note to let you know that I've just added the patch titled
Subject: USB: EHCI: ensure all watchdog timer events are deleted
when suspending usbto my gregkh-2.6 tree. Its filename is
usb-ehci-ensure-all-watchdog-timer-events-are-deleted-when-suspending-usb.patch
This tree can be found at
http://www.kernel.org/pub/linux/kernel/people/gregkh/gregkh-2.6/patches/
From [email protected] Thu Aug 13 16:37:16 2009
From: Jon Hunter <[email protected]>
Date: Wed, 12 Aug 2009 11:57:59 -0400 (EDT)
Subject: USB: EHCI: ensure all watchdog timer events are deleted when suspending usb
To: Greg KH <[email protected]>, Jon Hunter <[email protected]>
Cc: Fei Yang <[email protected]>
Message-ID: <[email protected]>From: Jon Hunter <[email protected]>
This patch was previously discussed in the following thread:
http://thread.gmane.org/gmane.linux.usb.general/19472/focus=19484On 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.To avoid this panic, per recommendation from Alan Stern (see above thread), we
make sure any pending timer events (that may have been scheduled by callingehci_workwithin theehci_bus_suspend()function) are deleted before returning.Signed-off-by Fei Yang <[email protected]>Signed-off-by Jon Hunter <[email protected]>Acked-by Alan Stern <[email protected]>Signed-off-by Greg Kroah-Hartman <[email protected]>--- drivers/usb/host/ehci-hub.c | 5 +++++ 1 file changed, 5 insertions(+)drivers/usb/host/ehci-hub.c +5
-1@@ -209,6 +209,11 @@ static int ehci_bus_suspend (struct usb_ ehci->next_statechange = jiffies +msecs_to_jiffies (10 );spin_unlock_irq (&ehci->lock); + +/* ehci_work() may have re-enabled the watchdog timer, which we do not +* want, and so we must delete any pending watchdog timer events. +*/ +del_timer_sync (&ehci->watchdog);return 0 ; } --To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 6 lines · Kevin Hilman
Jon,
Do you know if this will be queued as a fix for .31? or if it will wait for the .32 merge window.
Kevin
I am not sure. I was just browsing Greg's page and I found the following text file.
http://www.kernel.org/pub/linux/kernel/people/gregkh/gregkh-2.6/patches/series
Here Greg has a list of patches for .31. This patch is not part of what he has planned for .31. The patch is listed under "USB stuff (after 2.6.30 is out)", however, I believe that this is a typo, as this is under the main heading "Stuff to be merged after 2.6.31 is out". So I would presume this would come after .31 in .32 if there are no issues.
Cheers
Jon