On 2012年12月18日 04:06, Alan Stern wrote: > On Mon, 17 Dec 2012, Octavio Alvarez wrote: > >> On Thu, 13 Dec 2012 00:45:05 -0800, Lan Tianyu <tianyu.lan@xxxxxxxxx> >> 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. Hi Alan: How about this patch? Index: linux-pm/drivers/usb/host/ohci-pci.c =================================================================== --- linux-pm.orig/drivers/usb/host/ohci-pci.c 2012-11-01 18:21:33.604460469 +0800 +++ linux-pm/drivers/usb/host/ohci-pci.c 2012-12-19 14:39:07.081601806 +0800 @@ -188,6 +188,15 @@ pci_write_config_word(pdev, 0x50, misc | 0x0300); } +static int ohci_quirk_bad_wakeup(struct usb_hcd *hcd) +{ + struct ohci_hcd *ohci = hcd_to_ohci (hcd); + + ohci_dbg(ohci, "marked as bad wakeup.\n"); + hcd->bad_wakeup = true; + return 0; +} + /* List of quirks for OHCI */ static const struct pci_device_id ohci_pci_quirks[] = { { @@ -238,6 +247,31 @@ PCI_DEVICE(PCI_VENDOR_ID_ATI, 0x4399), .driver_data = (unsigned long)ohci_quirk_amd700, }, + { + /* MCP51 OHCI */ + PCI_DEVICE(PCI_VENDOR_ID_NVIDIA, 0x026d), + .driver_data = (unsigned long)ohci_quirk_bad_wakeup, + }, + { + /* MCP61 OHCI */ + PCI_DEVICE(PCI_VENDOR_ID_NVIDIA, 0x03f1), + .driver_data = (unsigned long)ohci_quirk_bad_wakeup, + }, + { + /* MCP79 OHCI */ + PCI_DEVICE(PCI_VENDOR_ID_NVIDIA, 0x0aa5), + .driver_data = (unsigned long)ohci_quirk_bad_wakeup, + }, + { + /* MCP79 OHCI */ + PCI_DEVICE(PCI_VENDOR_ID_NVIDIA, 0x0aa7), + .driver_data = (unsigned long)ohci_quirk_bad_wakeup, + }, + { + /* SiS OHCI */ + PCI_DEVICE(PCI_VENDOR_ID_SI, 7001), + .driver_data = (unsigned long)ohci_quirk_bad_wakeup, + }, /* FIXME for some of the early AMD 760 southbridges, OHCI * won't work at all. blacklist them. Index: linux-pm/include/linux/usb/hcd.h =================================================================== --- linux-pm.orig/include/linux/usb/hcd.h 2012-11-01 18:21:34.732460451 +0800 +++ linux-pm/include/linux/usb/hcd.h 2012-12-19 10:48:43.305822774 +0800 @@ -138,6 +138,7 @@ resource_size_t rsrc_start; /* memory/io resource start */ resource_size_t rsrc_len; /* memory/io resource length */ unsigned power_budget; /* in mA, 0 = no limit */ + bool bad_wakeup; /* bandwidth_mutex should be taken before adding or removing * any new bus bandwidth constraints: Index: linux-pm/drivers/usb/core/hcd.c =================================================================== --- linux-pm.orig/drivers/usb/core/hcd.c 2012-12-18 10:21:38.287229824 +0800 +++ linux-pm/drivers/usb/core/hcd.c 2012-12-19 13:51:57.085647043 +0800 @@ -2509,7 +2509,8 @@ * they only forward requests from the root hub. Therefore * controllers should always be enabled for remote wakeup. */ - device_wakeup_enable(hcd->self.controller); + if (!hcd->bad_wakeup) + device_wakeup_enable(hcd->self.controller); return retval; error_create_attr_group: > > Alan Stern > -- Best regards Tianyu Lan -- 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