On Thu, 12 Jul 2007 10:54:34 +0200 mb@xxxxxxxxx wrote: > This adds a driver for the Broadcom USB OHCI HCD device. > These devices are found in various embedded Broadcom-47xx machines. > The patch generates 2 checkpatch warnings. > +#ifdef CONFIG_PM > +static int ssb_ohci_hcd_suspend(struct usb_hcd *hcd, pm_message_t message) > +{ > + struct ssb_ohci_device *ohcidev = hcd_to_ssb_ohci(hcd); > + struct ohci_hcd *ohci = &ohcidev->ohci; > + unsigned long flags; > + > + spin_lock_irqsave(&ohci->lock, flags); > + > + ohci_writel(ohci, OHCI_INTR_MIE, &ohci->regs->intrdisable); > + ohci_readl(ohci, &ohci->regs->intrdisable); /* commit write */ > + > + /* make sure snapshot being resumed re-enumerates everything */ > + if (message.event == PM_EVENT_PRETHAW) > + ohci_usb_reset(ohci); > + > + clear_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags); > + > + spin_unlock_irqrestore(&ohci->lock, flags); > + > + return 0; > +} > + > +static int ssb_ohci_hcd_resume(struct usb_hcd *hcd) > +{ > + set_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags); > + usb_hcd_resume_root_hub(hcd); > + return 0; > +} Here, we'd normally do #else #define ssb_ohci_hcd_suspend NULL #define ssb_ohci_hcd_resume NULL > +#endif /* CONFIG_PM */ > + > +static const struct hc_driver ssb_ohci_hc_driver = { > + .description = "ssb-usb-ohci", > + .product_desc = "SSB OHCI Controller", > + .hcd_priv_size = sizeof(struct ssb_ohci_device), > + > + .irq = ohci_irq, > + .flags = HCD_MEMORY | HCD_USB11, > + > + .reset = ssb_ohci_reset, > + .start = ssb_ohci_start, > + .stop = ohci_stop, > + .shutdown = ohci_shutdown, > + > +#ifdef CONFIG_PM > + .suspend = ssb_ohci_hcd_suspend, > + .resume = ssb_ohci_hcd_resume, > +#endif and then remove these ifdefs. > + .urb_enqueue = ohci_urb_enqueue, > + .urb_dequeue = ohci_urb_dequeue, > + .endpoint_disable = ohci_endpoint_disable, > + > + .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, > +#endif Perhaps the same could be done here. > + .start_port_reset = ohci_start_port_reset, > +}; > + > > ... > > + > +#ifdef CONFIG_PM > +static int ssb_ohci_suspend(struct ssb_device *dev, pm_message_t state) > +{ > + ssb_device_disable(dev, 0); > + > + return 0; > +} > + > +static int ssb_ohci_resume(struct ssb_device *dev) > +{ > + struct usb_hcd *hcd = ssb_get_drvdata(dev); > + struct ssb_ohci_device *ohcidev = hcd_to_ssb_ohci(hcd); > + > + ssb_device_enable(dev, ohcidev->enable_flags); > + > + return 0; > +} > +#else /* CONFIG_PM */ > +# define ssb_ohci_suspend NULL > +# define ssb_ohci_resume NULL > +#endif /* CONFIG_PM */ > + > +static struct ssb_driver ssb_ohci_driver = { > + .name = KBUILD_MODNAME, > + .id_table = ssb_ohci_table, > + .probe = ssb_ohci_probe, > + .remove = ssb_ohci_remove, > + .suspend = ssb_ohci_suspend, > + .resume = ssb_ohci_resume, > +}; Like this. > Index: linux-2.6/drivers/usb/host/ohci-hcd.c > =================================================================== > --- linux-2.6.orig/drivers/usb/host/ohci-hcd.c 2007-07-12 10:51:46.000000000 +0200 > +++ linux-2.6/drivers/usb/host/ohci-hcd.c 2007-07-12 10:52:45.000000000 +0200 > @@ -920,11 +920,17 @@ MODULE_LICENSE ("GPL"); > #define PS3_SYSTEM_BUS_DRIVER ps3_ohci_sb_driver > #endif > > +#ifdef CONFIG_USB_OHCI_HCD_SSB > +#include "ohci-ssb.c" > +#define SSB_OHCI_DRIVER ssb_ohci_driver > +#endif argh. Why did USB do this? Sigh. > #if !defined(PCI_DRIVER) && \ > !defined(PLATFORM_DRIVER) && \ > !defined(OF_PLATFORM_DRIVER) && \ > !defined(SA1111_DRIVER) && \ > - !defined(PS3_SYSTEM_BUS_DRIVER) > + !defined(PS3_SYSTEM_BUS_DRIVER) && \ > + !defined(SSB_OHCI_DRIVER) > #error "missing bus glue for ohci-hcd" > #endif > > @@ -972,10 +978,20 @@ static int __init ohci_hcd_mod_init(void > goto error_pci; > #endif > > +#ifdef SSB_OHCI_DRIVER > + retval = ssb_driver_register(&SSB_OHCI_DRIVER); > + if (retval) > + goto error_ssb; > +#endif Why do we use SSB_OHCI_DRIVER here rather than ssb_ohci_driver? Seems odd. > + > return retval; > > /* Error path */ > +#ifdef SSB_OHCI_DRIVER > + error_ssb: > +#endif > #ifdef PCI_DRIVER > + pci_unregister_driver(&PCI_DRIVER); hm, are you sure about this? I don't see anywhere in here where PCI_DRIVER got newly registered, but we are newly unregistering it? - To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html