Hi, On Mon, Oct 01, 2012 at 04:21:37PM -0400, Alan Stern wrote: > Felipe: > > This makes ehci-pci a separate module that depends on ehci-hcd. It's > very straightforward; the only awkward thing is the way the driver > entries have to be overridden by hand. I couldn't think of any better > way of doing it. This is the penalty we pay for not using an > object-oriented programming language. :-) heh :-) > Index: usb-3.6/drivers/usb/host/Kconfig > =================================================================== > --- usb-3.6.orig/drivers/usb/host/Kconfig > +++ usb-3.6/drivers/usb/host/Kconfig > @@ -95,6 +95,11 @@ config USB_EHCI_TT_NEWSCHED > > If unsure, say Y. > > +config USB_EHCI_PCI > + tristate > + depends on USB_EHCI_HCD && PCI > + default y > + > config USB_EHCI_HCD_PMC_MSP > tristate "EHCI support for on-chip PMC MSP71xx USB controller" > depends on USB_EHCI_HCD && MSP_HAS_USB > Index: usb-3.6/drivers/usb/host/Makefile > =================================================================== > --- usb-3.6.orig/drivers/usb/host/Makefile > +++ usb-3.6/drivers/usb/host/Makefile > @@ -24,6 +24,8 @@ obj-$(CONFIG_USB_WHCI_HCD) += whci/ > obj-$(CONFIG_PCI) += pci-quirks.o > > obj-$(CONFIG_USB_EHCI_HCD) += ehci-hcd.o > +obj-$(CONFIG_USB_EHCI_PCI) += ehci-pci.o > + > obj-$(CONFIG_USB_OXU210HP_HCD) += oxu210hp-hcd.o > obj-$(CONFIG_USB_ISP116X_HCD) += isp116x-hcd.o > obj-$(CONFIG_USB_ISP1362_HCD) += isp1362-hcd.o > Index: usb-3.6/drivers/usb/host/ehci-hcd.c > =================================================================== > --- usb-3.6.orig/drivers/usb/host/ehci-hcd.c > +++ usb-3.6/drivers/usb/host/ehci-hcd.c > @@ -1276,11 +1276,6 @@ MODULE_DESCRIPTION(DRIVER_DESC); > MODULE_AUTHOR (DRIVER_AUTHOR); > MODULE_LICENSE ("GPL"); > > -#ifdef CONFIG_PCI > -#include "ehci-pci.c" > -#define PCI_DRIVER ehci_pci_driver > -#endif > - > #ifdef CONFIG_USB_EHCI_FSL > #include "ehci-fsl.c" > #define PLATFORM_DRIVER ehci_fsl_driver > @@ -1416,7 +1411,7 @@ MODULE_LICENSE ("GPL"); > #define PLATFORM_DRIVER ehci_platform_driver > #endif > > -#if !defined(PCI_DRIVER) && !defined(PLATFORM_DRIVER) && \ > +#if !defined(CONFIG_USB_EHCI_PCI) && !defined(PLATFORM_DRIVER) && \ > !defined(PS3_SYSTEM_BUS_DRIVER) && !defined(OF_PLATFORM_DRIVER) && \ > !defined(XILINX_OF_PLATFORM_DRIVER) > #error "missing bus glue for ehci-hcd" > @@ -1455,12 +1450,6 @@ static int __init ehci_hcd_init(void) > goto clean0; > #endif > > -#ifdef PCI_DRIVER > - retval = pci_register_driver(&PCI_DRIVER); > - if (retval < 0) > - goto clean1; > -#endif > - > #ifdef PS3_SYSTEM_BUS_DRIVER > retval = ps3_ehci_driver_register(&PS3_SYSTEM_BUS_DRIVER); > if (retval < 0) > @@ -1492,10 +1481,6 @@ clean3: > ps3_ehci_driver_unregister(&PS3_SYSTEM_BUS_DRIVER); > clean2: > #endif > -#ifdef PCI_DRIVER > - pci_unregister_driver(&PCI_DRIVER); > -clean1: > -#endif > #ifdef PLATFORM_DRIVER > platform_driver_unregister(&PLATFORM_DRIVER); > clean0: > @@ -1521,9 +1506,6 @@ static void __exit ehci_hcd_cleanup(void > #ifdef PLATFORM_DRIVER > platform_driver_unregister(&PLATFORM_DRIVER); > #endif > -#ifdef PCI_DRIVER > - pci_unregister_driver(&PCI_DRIVER); > -#endif > #ifdef PS3_SYSTEM_BUS_DRIVER > ps3_ehci_driver_unregister(&PS3_SYSTEM_BUS_DRIVER); > #endif > Index: usb-3.6/drivers/usb/host/ehci-pci.c > =================================================================== > --- usb-3.6.orig/drivers/usb/host/ehci-pci.c > +++ usb-3.6/drivers/usb/host/ehci-pci.c > @@ -18,9 +18,18 @@ > * Inc., 675 Mass Ave, Cambridge, MA 02139, USA. > */ > > -#ifndef CONFIG_PCI > -#error "This file is PCI bus glue. CONFIG_PCI must be defined." > -#endif > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/pci.h> > +#include <linux/usb.h> > +#include <linux/usb/hcd.h> > + > +#include "ehci.h" > +#include "pci-quirks.h" > + > +#define DRIVER_DESC "EHCI PCI platform driver" > + > +static const char hcd_name[] = "ehci-pci"; > > /* defined here to avoid adding to pci_ids.h for single instance use */ > #define PCI_DEVICE_ID_INTEL_CE4100_USB 0x2e70 > @@ -377,7 +386,11 @@ static int ehci_pci_resume(struct usb_hc > (void) ehci_pci_reinit(ehci, pdev); > return 0; > } > -#endif > +#else > + > +#define ehci_pci_suspend NULL > +#define ehci_pci_resume NULL > +#endif /* CONFIG_PM */ > > static int ehci_update_device(struct usb_hcd *hcd, struct usb_device *udev) > { > @@ -395,59 +408,7 @@ static int ehci_update_device(struct usb > return rc; > } > > -static const struct hc_driver ehci_pci_hc_driver = { > - .description = hcd_name, > - .product_desc = "EHCI Host Controller", > - .hcd_priv_size = sizeof(struct ehci_hcd), > - > - /* > - * generic hardware linkage > - */ > - .irq = ehci_irq, > - .flags = HCD_MEMORY | HCD_USB2, > - > - /* > - * basic lifecycle operations > - */ > - .reset = ehci_pci_setup, > - .start = ehci_run, > -#ifdef CONFIG_PM > - .pci_suspend = ehci_pci_suspend, > - .pci_resume = ehci_pci_resume, > -#endif > - .stop = ehci_stop, > - .shutdown = ehci_shutdown, > - > - /* > - * managing i/o requests and associated device resources > - */ > - .urb_enqueue = ehci_urb_enqueue, > - .urb_dequeue = ehci_urb_dequeue, > - .endpoint_disable = ehci_endpoint_disable, > - .endpoint_reset = ehci_endpoint_reset, > - > - /* > - * scheduling support > - */ > - .get_frame_number = ehci_get_frame, > - > - /* > - * root hub support > - */ > - .hub_status_data = ehci_hub_status_data, > - .hub_control = ehci_hub_control, > - .bus_suspend = ehci_bus_suspend, > - .bus_resume = ehci_bus_resume, > - .relinquish_port = ehci_relinquish_port, > - .port_handed_over = ehci_port_handed_over, > - > - /* > - * call back when device connected and addressed > - */ > - .update_device = ehci_update_device, > - > - .clear_tt_buffer_complete = ehci_clear_tt_buffer_complete, > -}; > +static struct hc_driver ehci_pci_hc_driver; > > /*-------------------------------------------------------------------------*/ > > @@ -466,7 +427,7 @@ MODULE_DEVICE_TABLE(pci, pci_ids); > > /* pci driver glue; this is a "new style" PCI driver module */ > static struct pci_driver ehci_pci_driver = { > - .name = (char *) hcd_name, > + .name = hcd_name, > .id_table = pci_ids, > > .probe = usb_hcd_pci_probe, > @@ -479,3 +440,33 @@ static struct pci_driver ehci_pci_driver > }, > #endif > }; > + > +static int __init ehci_pci_init(void) > +{ > + if (usb_disabled()) > + return -ENODEV; > + > + pr_info("%s: " DRIVER_DESC "\n", hcd_name); > + > + /* Copy the generic driver, then add our own functions and overrides */ > + ehci_pci_hc_driver = ehci_hc_driver; > + ehci_pci_hc_driver.description = hcd_name; > + ehci_pci_hc_driver.reset = ehci_pci_setup; > + ehci_pci_hc_driver.pci_suspend = ehci_pci_suspend; > + ehci_pci_hc_driver.pci_resume = ehci_pci_resume; > + ehci_pci_hc_driver.update_device = ehci_update_device; I would rather see ehci-pci.c passing a "ops" structure with a set of function pointers to the core layer. PCI suspend and resume shouldn't be exposed to ehci core layer, btw. If we make ehci-pci's device a parent to ehci core's device (should we add one), PM core will already make sure PCI's suspend is only called after EHCI is suspended, so we can properly split EHCI-suspend from PCI-suspend. In fact, looking at the code, there's nothing PCI-specific about ehci_pci_suspend(). It just calls ehci_suspend(), which shouldn't be exported. In that case, we don't need pci-suspend/pci-resume at all. ps: as a bonus to changing as suggested above, we would be able to use module_pci_driver() :-p -- balbi
Attachment:
signature.asc
Description: Digital signature