On Thu, 31 Oct 2013, Peter Chen wrote: > Individual controller driver has different requirement for wakeup > setting, so move it from core to itself. In order to align with > current etting the default wakeup setting is enabled (except for > chipidea host). > > Pass compile test with below commands: > make O=outout/all allmodconfig > make -j$CPU_NUM O=outout/all drivers/usb > > Signed-off-by: Peter Chen <peter.chen@xxxxxxxxxxxxx> This is basically right. I have only a couple of minor comments... > diff --git a/drivers/staging/usbip/vhci_hcd.c b/drivers/staging/usbip/vhci_hcd.c > index d7974cb..d618a19 100644 > --- a/drivers/staging/usbip/vhci_hcd.c > +++ b/drivers/staging/usbip/vhci_hcd.c > @@ -1030,6 +1030,7 @@ static int vhci_hcd_probe(struct platform_device *pdev) > the_controller = NULL; > return ret; > } > + device_wakeup_enable(hcd->self.controller); > > usbip_dbg_vhci_hc("bye\n"); > return 0; The host controller in usbip, like the one in dummy_hcd, is a virtual device. It can't generate interrupts or wakeup events. So this change isn't needed. > diff --git a/drivers/usb/core/hcd-pci.c b/drivers/usb/core/hcd-pci.c > index dfe9d0f..3ad2205 100644 > --- a/drivers/usb/core/hcd-pci.c > +++ b/drivers/usb/core/hcd-pci.c > @@ -267,6 +267,7 @@ int usb_hcd_pci_probe(struct pci_dev *dev, const struct pci_device_id *id) > retval = usb_add_hcd(hcd, hcd_irq, IRQF_SHARED); > if (retval != 0) > dev_set_drvdata(&dev->dev, NULL); > + device_wakeup_enable(hcd->self.controller); > for_each_companion(dev, hcd, ehci_post_add); > up_write(&companions_rwsem); > } else { > @@ -277,6 +278,7 @@ int usb_hcd_pci_probe(struct pci_dev *dev, const struct pci_device_id *id) > dev_set_drvdata(&dev->dev, NULL); > else > for_each_companion(dev, hcd, non_ehci_add); > + device_wakeup_enable(hcd->self.controller); > up_read(&companions_rwsem); > } It would be better to put a single call to device_wakeup_enable() a few lines farther down, after the "goto unmap_registers" line. > diff --git a/drivers/usb/host/ehci-fsl.c b/drivers/usb/host/ehci-fsl.c > index a06d501..bf405fd 100644 > --- a/drivers/usb/host/ehci-fsl.c > +++ b/drivers/usb/host/ehci-fsl.c > @@ -139,6 +139,7 @@ static int usb_hcd_fsl_probe(const struct hc_driver *driver, > if (retval != 0) > goto err4; > > + device_wakeup_enable(hcd->self.controller); > #ifdef CONFIG_USB_OTG > if (pdata->operating_mode == FSL_USB2_DR_OTG) { > struct ehci_hcd *ehci = hcd_to_ehci(hcd); Here and in some other places, the patch displays a bad sense of style. The device_wakeup_enable() call belongs along with the other hcd-related statements; it has nothing to do with CONFIG_USB_OTG. Therefore you should put a blank line before the "#ifdef". (You could also consider removing the blank line that precedes device_wakeup_enable().) > diff --git a/drivers/usb/host/ohci-sm501.c b/drivers/usb/host/ohci-sm501.c > index 2a5de5f..2a01c24 100644 > --- a/drivers/usb/host/ohci-sm501.c > +++ b/drivers/usb/host/ohci-sm501.c > @@ -169,6 +169,7 @@ static int ohci_hcd_sm501_drv_probe(struct platform_device *pdev) > if (retval) > goto err5; > > + device_wakeup_enable(hcd->self.controller); > /* enable power and unmask interrupts */ > > sm501_unit_power(dev->parent, SM501_GATE_USB_HOST, 1); This is another example of bad style. > diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c > index b8dffd5..bd4213b 100644 > --- a/drivers/usb/host/xhci-pci.c > +++ b/drivers/usb/host/xhci-pci.c > @@ -210,6 +210,7 @@ static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id) > IRQF_SHARED); > if (retval) > goto put_usb3_hcd; > + device_wakeup_enable(xhci->shared_hcd->self.controller); > /* Roothub already marked as USB 3.0 speed */ > > /* We know the LPM timeout algorithms for this host, let the USB core xhci-hcd is tricky. It registers two hcd structures, but they have the same parent controller. In xhci-pci.c, the first hcd is registered by the call to usb_hcd_pci_probe(). That call enables wakeup, so you don't need to enable it again here. > diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c > index d9c169f..a24e406 100644 > --- a/drivers/usb/host/xhci-plat.c > +++ b/drivers/usb/host/xhci-plat.c > @@ -140,6 +140,7 @@ static int xhci_plat_probe(struct platform_device *pdev) > if (ret) > goto unmap_registers; > > + device_wakeup_enable(hcd->self.controller); > /* USB 2.0 roothub is stored in the platform_device now. */ > hcd = platform_get_drvdata(pdev); > xhci = hcd_to_xhci(hcd); > @@ -160,6 +161,7 @@ static int xhci_plat_probe(struct platform_device *pdev) > if (ret) > goto put_usb3_hcd; > > + device_wakeup_enable(xhci->shared_hcd->self.controller); > return 0; > > put_usb3_hcd: A similar comment applies to xhci-plat.c. The first device_wakeup_enable() call is sufficient; you don't need to add the second one. > diff --git a/drivers/usb/renesas_usbhs/mod_host.c b/drivers/usb/renesas_usbhs/mod_host.c > index e40f565..e564ec5 100644 > --- a/drivers/usb/renesas_usbhs/mod_host.c > +++ b/drivers/usb/renesas_usbhs/mod_host.c > @@ -1470,6 +1470,7 @@ static int usbhsh_start(struct usbhs_priv *priv) > if (ret < 0) > return 0; > > + device_wakeup_enable(hcd->self.controller); > /* > * pipe initialize and enable DCP > */ Here is another example of bad style. After you fix up these things, you can add my Acked-by: to the patch. Alan Stern -- 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