Bug#677472: [3.1->3.2 regression] Immediate wake on suspend, associated with OHCI on MCP51
- To: Octavio Alvarez <alvarezp@alvarezp.com>
- Cc: Frank Schäfer <fschaefer.oss@googlemail.com>, Lan Tianyu <tianyu.lan@intel.com>, Lan Tianyu <lantianyu1986@gmail.com>, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, <677472@bugs.debian.org>, Ben Hutchings <ben@decadent.org.uk>, <linux-usb@vger.kernel.org>, Frank Schäfer <schaefer.frank@gmx.net>
- Subject: Bug#677472: [3.1->3.2 regression] Immediate wake on suspend, associated with OHCI on MCP51
- From: Alan Stern <stern@rowland.harvard.edu>
- Date: Mon, 17 Dec 2012 15:06:48 -0500 (EST)
- Message-id: <[🔎] Pine.LNX.4.44L0.1212171500020.1409-100000@iolanthe.rowland.org>
- Reply-to: Alan Stern <stern@rowland.harvard.edu>, 677472@bugs.debian.org
- In-reply-to: <[🔎] op.wpg0f4q36g6bxc@alvarezp-samsung>
On Mon, 17 Dec 2012, Octavio Alvarez wrote:
> On Thu, 13 Dec 2012 00:45:05 -0800, Lan Tianyu <tianyu.lan@intel.com>
> wrote:
>
> > diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
> > index f034716..9335f1b 100644
> > --- a/drivers/usb/core/hcd.c
> > +++ b/drivers/usb/core/hcd.c
> > @@ -2509,7 +2509,8 @@ int usb_add_hcd(struct usb_hcd *hcd,
> > * they only forward requests from the root hub. Therefore
> > * controllers should always be enabled for remote wakeup.
> > */
> > - device_wakeup_enable(hcd->self.controller);
> > + if (!usb_hcd_wakeup_quirks(hcd->self.controller))
> > + device_wakeup_enable(hcd->self.controller);
> > return retval;
> >
> > error_create_attr_group:
> > diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c
> > index fdefd9c..ba847d3 100644
> > --- a/drivers/usb/core/quirks.c
> > +++ b/drivers/usb/core/quirks.c
> > @@ -12,6 +12,7 @@
> > */
> >
> > #include <linux/usb.h>
> > +#include <linux/pci.h>
> > #include <linux/usb/quirks.h>
> > #include "usb.h"
> >
> > @@ -226,3 +227,33 @@ void usb_detect_interface_quirks(struct usb_device
> > *udev)
> > quirks);
> > udev->quirks |= quirks;
> > }
> > +
> > +struct pci_hcd {
> > + u32 vendor;
> > + u32 device;
> > +};
> > +
> > +static struct pci_hcd hcd_wakeup_qrk[] = {
> > + {PCI_VENDOR_ID_NVIDIA, 0x026d}, /* MCP51 OHCI */
> > + {PCI_VENDOR_ID_NVIDIA, 0x0aa5}, /* MCP79 OHCI */
> > + {PCI_VENDOR_ID_NVIDIA, 0x0aa7}, /* MCP79 OHCI */
> > + { }
> > +};
> > +
> > +int usb_hcd_wakeup_quirks(struct device *dev)
> > +{
> > + struct pci_dev *pdev;
> > + int i;
> > +
> > + if (dev->bus != (struct bus_type *)&pci_bus_type)
> > + return 0;
> > +
> > + pdev = to_pci_dev(dev);
> > + for (i = 0; hcd_wakeup_qrk[i].vendor || hcd_wakeup_qrk[i].device; i++)
> > + if ((hcd_wakeup_qrk[i].vendor == pdev->vendor) &&
> > + (hcd_wakeup_qrk[i].device == pdev->device)) {
> > + return 1;
> > + }
> > +
> > + return 0;
> > +}
>
> I would informing the user via dmesg output about the applied quirk
> and a point him to relevant documentation. Something like this:
>
> "Detected OHCI controller ID xxxx:yyyy, which requires no-wakeup quirk.
> See Documentation/quirks/ohci-no-wakeup.txt"
Incidentally, this patch should be written differently. Instead of a
quirks routine, there should simply be a bad_wakeup bitflag added to
the usb_hcd structure. The flag should be set in ohci-pci.c by
matching against nVidia's PCI vendor ID.
Alan Stern
Reply to: