From 09ca8adbe9f724a7e96f512c0039c4c4a1c5dcc0 Mon Sep 17 00:00:00 2001 From: Linus Torvalds Date: Sun, 6 Jul 2008 10:27:25 -0700 Subject: [PATCH] Revert "USB: don't explicitly reenable root-hub status interrupts" This reverts commit e872154921a6b5256a3c412dd69158ac0b135176. Andrey Borzenkov reports that it resulted in a totally hung machine for him when loading the OHCI driver. Extensive netconsole capture with SysRq output shows that modprobe gets stuck in ohci_hub_status_data() when probing and enabling the OHCI controller, see for example http://lkml.org/lkml/2008/7/5/236 for an analysis. The problem appears to be an interrupt flood triggered by the commit that gets reverted, and Andrey confirmed that the revert makes things work for him again. Reported-and-tested-by: Andrey Borzenkov Acked-by: Alan Stern Acked-by: David Brownell Cc: Greg Kroah-Hartman Signed-off-by: Linus Torvalds --- drivers/usb/core/hcd.c | 9 ++++++ drivers/usb/core/hcd.h | 2 ++ drivers/usb/core/hub.c | 9 ++++++ drivers/usb/host/ohci-at91.c | 1 + drivers/usb/host/ohci-au1xxx.c | 1 + drivers/usb/host/ohci-ep93xx.c | 1 + drivers/usb/host/ohci-hub.c | 53 ++++++++++++++------------------- drivers/usb/host/ohci-lh7a404.c | 1 + drivers/usb/host/ohci-omap.c | 1 + drivers/usb/host/ohci-pci.c | 1 + drivers/usb/host/ohci-pnx4008.c | 1 + drivers/usb/host/ohci-pnx8550.c | 1 + drivers/usb/host/ohci-ppc-of.c | 1 + drivers/usb/host/ohci-ppc-soc.c | 1 + drivers/usb/host/ohci-ps3.c | 1 + drivers/usb/host/ohci-pxa27x.c | 1 + drivers/usb/host/ohci-s3c2410.c | 1 + drivers/usb/host/ohci-sa1111.c | 1 + drivers/usb/host/ohci-sh.c | 1 + drivers/usb/host/ohci-sm501.c | 1 + drivers/usb/host/ohci-ssb.c | 1 + drivers/usb/host/u132-hcd.c | 11 +++++++ 22 files changed, 70 insertions(+), 31 deletions(-) diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c index 7158dbb6e4b..42a436478b7 100644 --- a/drivers/usb/core/hcd.c +++ b/drivers/usb/core/hcd.c @@ -924,6 +924,15 @@ static int register_root_hub(struct usb_hcd *hcd) return retval; } +void usb_enable_root_hub_irq (struct usb_bus *bus) +{ + struct usb_hcd *hcd; + + hcd = container_of (bus, struct usb_hcd, self); + if (hcd->driver->hub_irq_enable && hcd->state != HC_STATE_HALT) + hcd->driver->hub_irq_enable (hcd); +} + /*-------------------------------------------------------------------------*/ diff --git a/drivers/usb/core/hcd.h b/drivers/usb/core/hcd.h index a0bf5df6cb6..b9de1569b39 100644 --- a/drivers/usb/core/hcd.h +++ b/drivers/usb/core/hcd.h @@ -210,6 +210,8 @@ struct hc_driver { int (*bus_suspend)(struct usb_hcd *); int (*bus_resume)(struct usb_hcd *); int (*start_port_reset)(struct usb_hcd *, unsigned port_num); + void (*hub_irq_enable)(struct usb_hcd *); + /* Needed only if port-change IRQs are level-triggered */ /* force handover of high-speed port to full-speed companion */ void (*relinquish_port)(struct usb_hcd *, int); diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index 512d2d57d41..4cfe32a16c3 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -2073,6 +2073,8 @@ int usb_port_resume(struct usb_device *udev) } clear_bit(port1, hub->busy_bits); + if (!hub->hdev->parent && !hub->busy_bits[0]) + usb_enable_root_hub_irq(hub->hdev->bus); if (status == 0) status = finish_port_resume(udev); @@ -3002,6 +3004,11 @@ static void hub_events(void) hub->activating = 0; + /* If this is a root hub, tell the HCD it's okay to + * re-enable port-change interrupts now. */ + if (!hdev->parent && !hub->busy_bits[0]) + usb_enable_root_hub_irq(hdev->bus); + loop_autopm: /* Allow autosuspend if we're not going to run again */ if (list_empty(&hub->event_list)) @@ -3227,6 +3234,8 @@ int usb_reset_device(struct usb_device *udev) break; } clear_bit(port1, parent_hub->busy_bits); + if (!parent_hdev->parent && !parent_hub->busy_bits[0]) + usb_enable_root_hub_irq(parent_hdev->bus); if (ret < 0) goto re_enumerate; diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c index c96db1153dc..e534f9de0f0 100644 --- a/drivers/usb/host/ohci-at91.c +++ b/drivers/usb/host/ohci-at91.c @@ -261,6 +261,7 @@ static const struct hc_driver ohci_at91_hc_driver = { */ .hub_status_data = ohci_hub_status_data, .hub_control = ohci_hub_control, + .hub_irq_enable = ohci_rhsc_enable, #ifdef CONFIG_PM .bus_suspend = ohci_bus_suspend, .bus_resume = ohci_bus_resume, diff --git a/drivers/usb/host/ohci-au1xxx.c b/drivers/usb/host/ohci-au1xxx.c index 1b9abdba920..f90fe0c7373 100644 --- a/drivers/usb/host/ohci-au1xxx.c +++ b/drivers/usb/host/ohci-au1xxx.c @@ -288,6 +288,7 @@ static const struct hc_driver ohci_au1xxx_hc_driver = { */ .hub_status_data = ohci_hub_status_data, .hub_control = ohci_hub_control, + .hub_irq_enable = ohci_rhsc_enable, #ifdef CONFIG_PM .bus_suspend = ohci_bus_suspend, .bus_resume = ohci_bus_resume, diff --git a/drivers/usb/host/ohci-ep93xx.c b/drivers/usb/host/ohci-ep93xx.c index 06aadfb0ec2..5adaf36e47d 100644 --- a/drivers/usb/host/ohci-ep93xx.c +++ b/drivers/usb/host/ohci-ep93xx.c @@ -135,6 +135,7 @@ static struct hc_driver ohci_ep93xx_hc_driver = { .get_frame_number = ohci_get_frame, .hub_status_data = ohci_hub_status_data, .hub_control = ohci_hub_control, + .hub_irq_enable = ohci_rhsc_enable, #ifdef CONFIG_PM .bus_suspend = ohci_bus_suspend, .bus_resume = ohci_bus_resume, diff --git a/drivers/usb/host/ohci-hub.c b/drivers/usb/host/ohci-hub.c index 79a78029f89..b56739221d1 100644 --- a/drivers/usb/host/ohci-hub.c +++ b/drivers/usb/host/ohci-hub.c @@ -36,6 +36,18 @@ /*-------------------------------------------------------------------------*/ +/* hcd->hub_irq_enable() */ +static void ohci_rhsc_enable (struct usb_hcd *hcd) +{ + struct ohci_hcd *ohci = hcd_to_ohci (hcd); + + spin_lock_irq(&ohci->lock); + if (!ohci->autostop) + del_timer(&hcd->rh_timer); /* Prevent next poll */ + ohci_writel(ohci, OHCI_INTR_RHSC, &ohci->regs->intrenable); + spin_unlock_irq(&ohci->lock); +} + #define OHCI_SCHED_ENABLES \ (OHCI_CTRL_CLE|OHCI_CTRL_BLE|OHCI_CTRL_PLE|OHCI_CTRL_IE) @@ -362,28 +374,18 @@ static int ohci_root_hub_state_changes(struct ohci_hcd *ohci, int changed, int any_connected) { int poll_rh = 1; - int rhsc; - rhsc = ohci_readl(ohci, &ohci->regs->intrenable) & OHCI_INTR_RHSC; switch (ohci->hc_control & OHCI_CTRL_HCFS) { case OHCI_USB_OPER: - /* If no status changes are pending, enable status-change - * interrupts. - */ - if (!rhsc && !changed) { - rhsc = OHCI_INTR_RHSC; - ohci_writel(ohci, rhsc, &ohci->regs->intrenable); - } - - /* Keep on polling until we know a device is connected - * and RHSC is enabled, or until we autostop. - */ + /* keep on polling until we know a device is connected + * and RHSC is enabled */ if (!ohci->autostop) { if (any_connected || !device_may_wakeup(&ohci_to_hcd(ohci) ->self.root_hub->dev)) { - if (rhsc) + if (ohci_readl(ohci, &ohci->regs->intrenable) & + OHCI_INTR_RHSC) poll_rh = 0; } else { ohci->autostop = 1; @@ -396,13 +398,12 @@ static int ohci_root_hub_state_changes(struct ohci_hcd *ohci, int changed, ohci->autostop = 0; ohci->next_statechange = jiffies + STATECHANGE_DELAY; - } else if (rhsc && time_after_eq(jiffies, + } else if (time_after_eq(jiffies, ohci->next_statechange) && !ohci->ed_rm_list && !(ohci->hc_control & OHCI_SCHED_ENABLES)) { ohci_rh_suspend(ohci, 1); - poll_rh = 0; } } break; @@ -416,12 +417,6 @@ static int ohci_root_hub_state_changes(struct ohci_hcd *ohci, int changed, else usb_hcd_resume_root_hub(ohci_to_hcd(ohci)); } else { - if (!rhsc && (ohci->autostop || - ohci_to_hcd(ohci)->self.root_hub-> - do_remote_wakeup)) - ohci_writel(ohci, OHCI_INTR_RHSC, - &ohci->regs->intrenable); - /* everything is idle, no need for polling */ poll_rh = 0; } @@ -443,16 +438,12 @@ static inline int ohci_rh_resume(struct ohci_hcd *ohci) static int ohci_root_hub_state_changes(struct ohci_hcd *ohci, int changed, int any_connected) { - /* If RHSC is enabled, don't poll */ - if (ohci_readl(ohci, &ohci->regs->intrenable) & OHCI_INTR_RHSC) - return 0; + int poll_rh = 1; - /* If no status changes are pending, enable status-change interrupts */ - if (!changed) { - ohci_writel(ohci, OHCI_INTR_RHSC, &ohci->regs->intrenable); - return 0; - } - return 1; + /* keep on polling until RHSC is enabled */ + if (ohci_readl(ohci, &ohci->regs->intrenable) & OHCI_INTR_RHSC) + poll_rh = 0; + return poll_rh; } #endif /* CONFIG_PM */ diff --git a/drivers/usb/host/ohci-lh7a404.c b/drivers/usb/host/ohci-lh7a404.c index 96d14fa1d83..13c12ed2225 100644 --- a/drivers/usb/host/ohci-lh7a404.c +++ b/drivers/usb/host/ohci-lh7a404.c @@ -193,6 +193,7 @@ static const struct hc_driver ohci_lh7a404_hc_driver = { */ .hub_status_data = ohci_hub_status_data, .hub_control = ohci_hub_control, + .hub_irq_enable = ohci_rhsc_enable, #ifdef CONFIG_PM .bus_suspend = ohci_bus_suspend, .bus_resume = ohci_bus_resume, diff --git a/drivers/usb/host/ohci-omap.c b/drivers/usb/host/ohci-omap.c index 6859fb5f1d6..3a7c24c0367 100644 --- a/drivers/usb/host/ohci-omap.c +++ b/drivers/usb/host/ohci-omap.c @@ -466,6 +466,7 @@ static const struct hc_driver ohci_omap_hc_driver = { */ .hub_status_data = ohci_hub_status_data, .hub_control = ohci_hub_control, + .hub_irq_enable = ohci_rhsc_enable, #ifdef CONFIG_PM .bus_suspend = ohci_bus_suspend, .bus_resume = ohci_bus_resume, diff --git a/drivers/usb/host/ohci-pci.c b/drivers/usb/host/ohci-pci.c index 3bf175d95a2..4696cc912e1 100644 --- a/drivers/usb/host/ohci-pci.c +++ b/drivers/usb/host/ohci-pci.c @@ -327,6 +327,7 @@ static const struct hc_driver ohci_pci_hc_driver = { */ .hub_status_data = ohci_hub_status_data, .hub_control = ohci_hub_control, + .hub_irq_enable = ohci_rhsc_enable, #ifdef CONFIG_PM .bus_suspend = ohci_bus_suspend, .bus_resume = ohci_bus_resume, diff --git a/drivers/usb/host/ohci-pnx4008.c b/drivers/usb/host/ohci-pnx4008.c index 664f07ee873..28b458f20cc 100644 --- a/drivers/usb/host/ohci-pnx4008.c +++ b/drivers/usb/host/ohci-pnx4008.c @@ -280,6 +280,7 @@ static const struct hc_driver ohci_pnx4008_hc_driver = { */ .hub_status_data = ohci_hub_status_data, .hub_control = ohci_hub_control, + .hub_irq_enable = ohci_rhsc_enable, #ifdef CONFIG_PM .bus_suspend = ohci_bus_suspend, .bus_resume = ohci_bus_resume, diff --git a/drivers/usb/host/ohci-pnx8550.c b/drivers/usb/host/ohci-pnx8550.c index 28467e288a9..605d59cba28 100644 --- a/drivers/usb/host/ohci-pnx8550.c +++ b/drivers/usb/host/ohci-pnx8550.c @@ -201,6 +201,7 @@ static const struct hc_driver ohci_pnx8550_hc_driver = { */ .hub_status_data = ohci_hub_status_data, .hub_control = ohci_hub_control, + .hub_irq_enable = ohci_rhsc_enable, #ifdef CONFIG_PM .bus_suspend = ohci_bus_suspend, .bus_resume = ohci_bus_resume, diff --git a/drivers/usb/host/ohci-ppc-of.c b/drivers/usb/host/ohci-ppc-of.c index 50e55db1363..a6725279122 100644 --- a/drivers/usb/host/ohci-ppc-of.c +++ b/drivers/usb/host/ohci-ppc-of.c @@ -72,6 +72,7 @@ static const struct hc_driver ohci_ppc_of_hc_driver = { */ .hub_status_data = ohci_hub_status_data, .hub_control = ohci_hub_control, + .hub_irq_enable = ohci_rhsc_enable, #ifdef CONFIG_PM .bus_suspend = ohci_bus_suspend, .bus_resume = ohci_bus_resume, diff --git a/drivers/usb/host/ohci-ppc-soc.c b/drivers/usb/host/ohci-ppc-soc.c index cd3398b675b..523c3012557 100644 --- a/drivers/usb/host/ohci-ppc-soc.c +++ b/drivers/usb/host/ohci-ppc-soc.c @@ -172,6 +172,7 @@ static const struct hc_driver ohci_ppc_soc_hc_driver = { */ .hub_status_data = ohci_hub_status_data, .hub_control = ohci_hub_control, + .hub_irq_enable = ohci_rhsc_enable, #ifdef CONFIG_PM .bus_suspend = ohci_bus_suspend, .bus_resume = ohci_bus_resume, diff --git a/drivers/usb/host/ohci-ps3.c b/drivers/usb/host/ohci-ps3.c index bfdeb0d22d0..c1935ae537f 100644 --- a/drivers/usb/host/ohci-ps3.c +++ b/drivers/usb/host/ohci-ps3.c @@ -68,6 +68,7 @@ static const struct hc_driver ps3_ohci_hc_driver = { .get_frame_number = ohci_get_frame, .hub_status_data = ohci_hub_status_data, .hub_control = ohci_hub_control, + .hub_irq_enable = ohci_rhsc_enable, .start_port_reset = ohci_start_port_reset, #if defined(CONFIG_PM) .bus_suspend = ohci_bus_suspend, diff --git a/drivers/usb/host/ohci-pxa27x.c b/drivers/usb/host/ohci-pxa27x.c index 70b0d4b459e..d4ee27d92be 100644 --- a/drivers/usb/host/ohci-pxa27x.c +++ b/drivers/usb/host/ohci-pxa27x.c @@ -298,6 +298,7 @@ static const struct hc_driver ohci_pxa27x_hc_driver = { */ .hub_status_data = ohci_hub_status_data, .hub_control = ohci_hub_control, + .hub_irq_enable = ohci_rhsc_enable, #ifdef CONFIG_PM .bus_suspend = ohci_bus_suspend, .bus_resume = ohci_bus_resume, diff --git a/drivers/usb/host/ohci-s3c2410.c b/drivers/usb/host/ohci-s3c2410.c index a73d2ff322e..ead4772f0f2 100644 --- a/drivers/usb/host/ohci-s3c2410.c +++ b/drivers/usb/host/ohci-s3c2410.c @@ -466,6 +466,7 @@ static const struct hc_driver ohci_s3c2410_hc_driver = { */ .hub_status_data = ohci_s3c2410_hub_status_data, .hub_control = ohci_s3c2410_hub_control, + .hub_irq_enable = ohci_rhsc_enable, #ifdef CONFIG_PM .bus_suspend = ohci_bus_suspend, .bus_resume = ohci_bus_resume, diff --git a/drivers/usb/host/ohci-sa1111.c b/drivers/usb/host/ohci-sa1111.c index 99438c65981..0f48f2d9922 100644 --- a/drivers/usb/host/ohci-sa1111.c +++ b/drivers/usb/host/ohci-sa1111.c @@ -231,6 +231,7 @@ static const struct hc_driver ohci_sa1111_hc_driver = { */ .hub_status_data = ohci_hub_status_data, .hub_control = ohci_hub_control, + .hub_irq_enable = ohci_rhsc_enable, #ifdef CONFIG_PM .bus_suspend = ohci_bus_suspend, .bus_resume = ohci_bus_resume, diff --git a/drivers/usb/host/ohci-sh.c b/drivers/usb/host/ohci-sh.c index 60f03cc7ec4..e7ee607278f 100644 --- a/drivers/usb/host/ohci-sh.c +++ b/drivers/usb/host/ohci-sh.c @@ -68,6 +68,7 @@ static const struct hc_driver ohci_sh_hc_driver = { */ .hub_status_data = ohci_hub_status_data, .hub_control = ohci_hub_control, + .hub_irq_enable = ohci_rhsc_enable, #ifdef CONFIG_PM .bus_suspend = ohci_bus_suspend, .bus_resume = ohci_bus_resume, diff --git a/drivers/usb/host/ohci-sm501.c b/drivers/usb/host/ohci-sm501.c index e899a77dfb8..e610698c6b6 100644 --- a/drivers/usb/host/ohci-sm501.c +++ b/drivers/usb/host/ohci-sm501.c @@ -75,6 +75,7 @@ static const struct hc_driver ohci_sm501_hc_driver = { */ .hub_status_data = ohci_hub_status_data, .hub_control = ohci_hub_control, + .hub_irq_enable = ohci_rhsc_enable, #ifdef CONFIG_PM .bus_suspend = ohci_bus_suspend, .bus_resume = ohci_bus_resume, diff --git a/drivers/usb/host/ohci-ssb.c b/drivers/usb/host/ohci-ssb.c index c4265caec78..7275186db31 100644 --- a/drivers/usb/host/ohci-ssb.c +++ b/drivers/usb/host/ohci-ssb.c @@ -81,6 +81,7 @@ static const struct hc_driver ssb_ohci_hc_driver = { .hub_status_data = ohci_hub_status_data, .hub_control = ohci_hub_control, + .hub_irq_enable = ohci_rhsc_enable, #ifdef CONFIG_PM .bus_suspend = ohci_bus_suspend, .bus_resume = ohci_bus_resume, diff --git a/drivers/usb/host/u132-hcd.c b/drivers/usb/host/u132-hcd.c index f29307405bb..9b6323f768b 100644 --- a/drivers/usb/host/u132-hcd.c +++ b/drivers/usb/host/u132-hcd.c @@ -2934,6 +2934,16 @@ static int u132_start_port_reset(struct usb_hcd *hcd, unsigned port_num) return 0; } +static void u132_hub_irq_enable(struct usb_hcd *hcd) +{ + struct u132 *u132 = hcd_to_u132(hcd); + if (u132->going > 1) { + dev_err(&u132->platform_dev->dev, "device has been removed %d\n" + , u132->going); + } else if (u132->going > 0) + dev_err(&u132->platform_dev->dev, "device is being removed\n"); +} + #ifdef CONFIG_PM static int u132_bus_suspend(struct usb_hcd *hcd) @@ -2985,6 +2995,7 @@ static struct hc_driver u132_hc_driver = { .bus_suspend = u132_bus_suspend, .bus_resume = u132_bus_resume, .start_port_reset = u132_start_port_reset, + .hub_irq_enable = u132_hub_irq_enable, }; /* -- 2.41.1