Re: [linux-usb-devel] Re: 2.6.13-mm2

Daniel Ritz
2005-09-29T19:39:31Z

On Thursday 29 September 2005 17.36, Linus Torvalds wrote:
>
> On Wed, 28 Sep 2005, David Brownell wrote:
> >
> > You could try adding
> >
> > ohci_writel(ohci, OHCI_INTR_MIE, &ohci->regs->intrdisable);
> >
> > near the end of ohci_pci_suspend().

that's what i meant with "i think ohci-hcd does not fully disable
interrupts in it's suspend callback"...

>
> Give it up.

i'd say: yes put that line there to shut up the controller during suspend
and nuke the free_irq() in suspend to avoid problems during restore (attached).

>
> The right thing is to not free and re-aquire the damn interrupt in the
> first place. It was a MISTAKE. We undid the ACPI braindamage that made it
> be required a month ago, because sane people REALIZED it was a mistake.
>
> It's not just "random luck" that not releasing the interrupt over suspend
> fixes the problem. The problem is _due_ to drivers releasing the
> interrupt in the first place.
>
> IT DOESN'T MATTER what we do before the suspend, because we don't control
> the wakeup sequence. If the BIOS wakeup enables the devices again, the
> fact that we disabled them on suspend makes zero difference.
>
> And yes, we can always "fix" things by selecting the right order to
> re-aquire the interrupts, but the thing is, the "right order" will be
> machine-dependent and in general depend on the phase of the moon and BIOS
> version, and ACPI quirks.
>
> The _only_ sane thing to do is to not drop the interrupts in the first
> place. So that if you start getting interrupts before you expect them, you
> can still handle them.
>

fully agreed.

> Linus
>

attached the patch that kills the free_irq()/request_irq() pair in USB.
one round in -mm?

rgds
-daniel

----

[PATCH] usb/core/hcd-pci.c: don't free_irq() on suspend

the free_irq() in USB suspend breaks resume on some setups where USB
(ohci/ehci) shares the interrupt with an other device.

Signed-off-by: Daniel Ritz <daniel.ritz@gmx.ch>

diff --git a/drivers/usb/core/hcd-pci.c b/drivers/usb/core/hcd-pci.c
--- a/drivers/usb/core/hcd-pci.c
+++ b/drivers/usb/core/hcd-pci.c
@@ -242,7 +242,6 @@ int usb_hcd_pci_suspend (struct pci_dev
case HC_STATE_SUSPENDED:
/* no DMA or IRQs except when HC is active */
if (dev->current_state == PCI_D0) {
- free_irq (hcd->irq, hcd);
pci_save_state (dev);
pci_disable_device (dev);
}
@@ -374,14 +373,6 @@ int usb_hcd_pci_resume (struct pci_dev *

hcd->state = HC_STATE_RESUMING;
hcd->saw_irq = 0;
- retval = request_irq (dev->irq, usb_hcd_irq, SA_SHIRQ,
- hcd->irq_descr, hcd);
- if (retval < 0) {
- dev_err (hcd->self.controller,
- "can't restore IRQ after resume!\n");
- usb_hc_died (hcd);
- return retval;
- }

retval = hcd->driver->resume (hcd);
if (!HC_IS_RUNNING (hcd->state)) {