[RFC,2/2] USB: OHCI: make ohci-pci a separate driver

Message ID 516d2f23.62ae440a.26b5.ffff86f0@mx.google.com
State New
Headers show

Commit Message

manjunath.goudar@linaro.org April 16, 2013, 10:58 a.m.
From: Manjunath Goudar <manjunath.goudar@linaro.org>

This patch splits the PCI portion of ohci-hcd out into its
own separate driver module, called ohci-pci.  Consistently with the
current practice, the decision whether to build this module is not
user-configurable.  If OHCI_PCI are enabled then the module will
be built, always.

Signed-off-by: Manjunath Goudar <manjunath.goudar@linaro.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Greg KH <greg@kroah.com>
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: linux-usb@vger.kernel.org
---
 drivers/usb/host/Kconfig    |    5 +++
 drivers/usb/host/Makefile   |    1 +
 drivers/usb/host/ohci-hcd.c |   31 +--------------
 drivers/usb/host/ohci-mem.c |    1 -
 drivers/usb/host/ohci-pci.c |   93 ++++++++++++++++++-------------------------
 drivers/usb/host/ohci.h     |    6 ++-
 6 files changed, 51 insertions(+), 86 deletions(-)

Comments

Arnd Bergmann April 17, 2013, 12:09 p.m. | #1
On Tuesday 16 April 2013, manjunath.goudar@linaro.org wrote:
>  
> -
>  /*
>   * On architectures with edge-triggered interrupts we must never return
>   * IRQ_NONE.
> @@ -585,7 +574,6 @@ int ohci_init(struct ohci_hcd *ohci)
>  	return ret;
>  }
>  EXPORT_SYMBOL_GPL(ohci_init);
> -
>  /*-------------------------------------------------------------------------*/
> diff --git a/drivers/usb/host/ohci-mem.c b/drivers/usb/host/ohci-mem.c
> index 1a4dede..f98727f 100644
> --- a/drivers/usb/host/ohci-mem.c
> +++ b/drivers/usb/host/ohci-mem.c
> @@ -30,7 +30,6 @@ void ohci_hcd_init (struct ohci_hcd *ohci)
>  	INIT_LIST_HEAD (&ohci->pending);
>  }
>  EXPORT_SYMBOL_GPL(ohci_hcd_init);
> -
>  /*-------------------------------------------------------------------------*/
>  
>  static int ohci_mem_init (struct ohci_hcd *ohci)

No need to remove the empty lines here.

> @@ -175,7 +182,7 @@ static int ohci_quirk_amd700(struct usb_hcd *hcd)
>  	return 0;
>  }
>  
> -static void sb800_prefetch(struct ohci_hcd *ohci, int on)
> +void sb800_prefetch(struct ohci_hcd *ohci, int on)
>  {
>  	struct pci_dev *pdev;
>  	u16 misc;
> @@ -187,6 +194,7 @@ static void sb800_prefetch(struct ohci_hcd *ohci, int on)
>  	else
>  		pci_write_config_word(pdev, 0x50, misc | 0x0300);
>  }
> +EXPORT_SYMBOL_GPL(sb800_prefetch);
>  
>  /* List of quirks for OHCI */

I think this won't work when both ehci-hcd and ehci-pci are loadable modules,
since you cannot export symbols both ways. Since the function is used in
drivers/usb/host/ohci-q.c but does not actually require code from ehci-pci.c,
the easiest solution I see would be to make it a static function in ohci-q.c.

Alternatively, it could be an exported function in drivers/usb/host/pci-quirks.c.

	Arnd
manjunath.goudar@linaro.org April 18, 2013, 6:35 a.m. | #2
On 17 April 2013 17:39, Arnd Bergmann <arnd@arndb.de> wrote:

> On Tuesday 16 April 2013, manjunath.goudar@linaro.org wrote:
> >
> > -
> >  /*
> >   * On architectures with edge-triggered interrupts we must never return
> >   * IRQ_NONE.
> > @@ -585,7 +574,6 @@ int ohci_init(struct ohci_hcd *ohci)
> >       return ret;
> >  }
> >  EXPORT_SYMBOL_GPL(ohci_init);
> > -
> >
>  /*-------------------------------------------------------------------------*/
> > diff --git a/drivers/usb/host/ohci-mem.c b/drivers/usb/host/ohci-mem.c
> > index 1a4dede..f98727f 100644
> > --- a/drivers/usb/host/ohci-mem.c
> > +++ b/drivers/usb/host/ohci-mem.c
> > @@ -30,7 +30,6 @@ void ohci_hcd_init (struct ohci_hcd *ohci)
> >       INIT_LIST_HEAD (&ohci->pending);
> >  }
> >  EXPORT_SYMBOL_GPL(ohci_hcd_init);
> > -
> >
>  /*-------------------------------------------------------------------------*/
> >
> >  static int ohci_mem_init (struct ohci_hcd *ohci)
>
> No need to remove the empty lines here.
>

reverted empty line.

>
> > @@ -175,7 +182,7 @@ static int ohci_quirk_amd700(struct usb_hcd *hcd)
> >       return 0;
> >  }
> >
> > -static void sb800_prefetch(struct ohci_hcd *ohci, int on)
> > +void sb800_prefetch(struct ohci_hcd *ohci, int on)
> >  {
> >       struct pci_dev *pdev;
> >       u16 misc;
> > @@ -187,6 +194,7 @@ static void sb800_prefetch(struct ohci_hcd *ohci,
> int on)
> >       else
> >               pci_write_config_word(pdev, 0x50, misc | 0x0300);
> >  }
> > +EXPORT_SYMBOL_GPL(sb800_prefetch);
> >
> >  /* List of quirks for OHCI */
>
> I think this won't work when both ehci-hcd and ehci-pci are loadable
> modules,
> since you cannot export symbols both ways. Since the function is used in
> drivers/usb/host/ohci-q.c but does not actually require code from
> ehci-pci.c,
> the easiest solution I see would be to make it a static function in
> ohci-q.c.
>
> Alternatively, it could be an exported function in
> drivers/usb/host/pci-quirks.c.
>
>         Arnd
>

Moved sb800_prefetch to ohci-q.c and made static function.

Thanks
Manjunath Goudar

Patch

diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index 7a1a248..8bd53e2 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -333,6 +333,11 @@  config USB_ISP1362_HCD
 	  To compile this driver as a module, choose M here: the
 	  module will be called isp1362-hcd.
 
+config USB_OHCI_PCI
+	tristate "OHCI PCI support"
+	depends on USB_OHCI_HCD
+	default y
+
 config USB_OHCI_HCD
 	tristate "OHCI HCD support"
 	depends on USB && USB_ARCH_HAS_OHCI
diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
index 58c14c1..39ddcca 100644
--- a/drivers/usb/host/Makefile
+++ b/drivers/usb/host/Makefile
@@ -41,6 +41,7 @@  obj-$(CONFIG_USB_OXU210HP_HCD)	+= oxu210hp-hcd.o
 obj-$(CONFIG_USB_ISP116X_HCD)	+= isp116x-hcd.o
 obj-$(CONFIG_USB_ISP1362_HCD)	+= isp1362-hcd.o
 obj-$(CONFIG_USB_OHCI_HCD)	+= ohci-hcd.o
+obj-$(CONFIG_USB_OHCI_PCI)	+= ohci-pci.o
 obj-$(CONFIG_USB_UHCI_HCD)	+= uhci-hcd.o
 obj-$(CONFIG_USB_FHCI_HCD)	+= fhci.o
 obj-$(CONFIG_USB_XHCI_HCD)	+= xhci-hcd.o
diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c
index fb869fc..3259ff9 100644
--- a/drivers/usb/host/ohci-hcd.c
+++ b/drivers/usb/host/ohci-hcd.c
@@ -78,24 +78,13 @@  static const char	hcd_name [] = "ohci_hcd";
 #include "ohci.h"
 #include "pci-quirks.h"
 
-static void ohci_dump (struct ohci_hcd *ohci, int verbose);
-
-#ifdef CONFIG_PCI
-static void sb800_prefetch(struct ohci_hcd *ohci, int on);
-#else
-static inline void sb800_prefetch(struct ohci_hcd *ohci, int on)
-{
-	return;
-}
-#endif
-
+static void ohci_dump(struct ohci_hcd *ohci, int verbose);
 
 #include "ohci-hub.c"
 #include "ohci-dbg.c"
 #include "ohci-mem.c"
 #include "ohci-q.c"
 
-
 /*
  * On architectures with edge-triggered interrupts we must never return
  * IRQ_NONE.
@@ -585,7 +574,6 @@  int ohci_init(struct ohci_hcd *ohci)
 	return ret;
 }
 EXPORT_SYMBOL_GPL(ohci_init);
-
 /*-------------------------------------------------------------------------*/
 
 /* Start an OHCI controller, set the BUS operational
@@ -1151,11 +1139,6 @@  MODULE_AUTHOR (DRIVER_AUTHOR);
 MODULE_DESCRIPTION(DRIVER_DESC);
 MODULE_LICENSE ("GPL");
 
-#ifdef CONFIG_PCI
-#include "ohci-pci.c"
-#define PCI_DRIVER		ohci_pci_driver
-#endif
-
 #if defined(CONFIG_ARCH_SA1100) && defined(CONFIG_SA1111)
 #include "ohci-sa1111.c"
 #define SA1111_DRIVER		ohci_hcd_sa1111_driver
@@ -1251,7 +1234,7 @@  MODULE_LICENSE ("GPL");
 #define PLATFORM_DRIVER		ohci_platform_driver
 #endif
 
-#if	!defined(PCI_DRIVER) &&		\
+#if	!IS_ENABLED(CONFIG_USB_OHCI_PCI) &&	\
 	!defined(PLATFORM_DRIVER) &&	\
 	!defined(OMAP1_PLATFORM_DRIVER) &&	\
 	!defined(OMAP3_PLATFORM_DRIVER) &&	\
@@ -1326,12 +1309,6 @@  static int __init ohci_hcd_mod_init(void)
 		goto error_sa1111;
 #endif
 
-#ifdef PCI_DRIVER
-	retval = pci_register_driver(&PCI_DRIVER);
-	if (retval < 0)
-		goto error_pci;
-#endif
-
 #ifdef SM501_OHCI_DRIVER
 	retval = platform_driver_register(&SM501_OHCI_DRIVER);
 	if (retval < 0)
@@ -1425,10 +1402,6 @@  static int __init ohci_hcd_mod_init(void)
 	platform_driver_unregister(&SM501_OHCI_DRIVER);
  error_sm501:
 #endif
-#ifdef PCI_DRIVER
-	pci_unregister_driver(&PCI_DRIVER);
- error_pci:
-#endif
 #ifdef SA1111_DRIVER
 	sa1111_driver_unregister(&SA1111_DRIVER);
  error_sa1111:
diff --git a/drivers/usb/host/ohci-mem.c b/drivers/usb/host/ohci-mem.c
index 1a4dede..f98727f 100644
--- a/drivers/usb/host/ohci-mem.c
+++ b/drivers/usb/host/ohci-mem.c
@@ -30,7 +30,6 @@  void ohci_hcd_init (struct ohci_hcd *ohci)
 	INIT_LIST_HEAD (&ohci->pending);
 }
 EXPORT_SYMBOL_GPL(ohci_hcd_init);
-
 /*-------------------------------------------------------------------------*/
 
 static int ohci_mem_init (struct ohci_hcd *ohci)
diff --git a/drivers/usb/host/ohci-pci.c b/drivers/usb/host/ohci-pci.c
index 951514e..f9c6c2e 100644
--- a/drivers/usb/host/ohci-pci.c
+++ b/drivers/usb/host/ohci-pci.c
@@ -14,12 +14,19 @@ 
  * This file is licenced under the GPL.
  */
 
-#ifndef CONFIG_PCI
-#error "This file is PCI bus glue.  CONFIG_PCI must be defined."
-#endif
-
-#include <linux/pci.h>
 #include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/pci.h>
+#include <linux/usb.h>
+#include <linux/usb/hcd.h>
+
+#include "ohci.h"
+#include "pci-quirks.h"
+
+#define DRIVER_DESC "OHCI PCI platform driver"
+
+static const char hcd_name[] = "ohci-pci";
 
 
 /*-------------------------------------------------------------------------*/
@@ -175,7 +182,7 @@  static int ohci_quirk_amd700(struct usb_hcd *hcd)
 	return 0;
 }
 
-static void sb800_prefetch(struct ohci_hcd *ohci, int on)
+void sb800_prefetch(struct ohci_hcd *ohci, int on)
 {
 	struct pci_dev *pdev;
 	u16 misc;
@@ -187,6 +194,7 @@  static void sb800_prefetch(struct ohci_hcd *ohci, int on)
 	else
 		pci_write_config_word(pdev, 0x50, misc | 0x0300);
 }
+EXPORT_SYMBOL_GPL(sb800_prefetch);
 
 /* List of quirks for OHCI */
 static const struct pci_device_id ohci_pci_quirks[] = {
@@ -296,60 +304,14 @@  static int ohci_pci_start (struct usb_hcd *hcd)
 	return ret;
 }
 
+static struct hc_driver __read_mostly ohci_pci_hc_driver;
 
-/*-------------------------------------------------------------------------*/
-
-static const struct hc_driver ohci_pci_hc_driver = {
-	.description =		hcd_name,
-	.product_desc =		"OHCI Host Controller",
-	.hcd_priv_size =	sizeof(struct ohci_hcd),
-
-	/*
-	 * generic hardware linkage
-	 */
-	.irq =			ohci_irq,
-	.flags =		HCD_MEMORY | HCD_USB11,
-
-	/*
-	 * basic lifecycle operations
-	 */
+static const struct ohci_driver_overrides overrides = {
+	.product_desc =		"OHCI PCI host controller",
 	.reset =		ohci_pci_reset,
 	.start =		ohci_pci_start,
-	.stop =			ohci_stop,
-	.shutdown =		ohci_shutdown,
-
-#ifdef	CONFIG_PM
-	.pci_suspend =		ohci_suspend,
-	.pci_resume =		ohci_resume,
-#endif
-
-	/*
-	 * managing i/o requests and associated device resources
-	 */
-	.urb_enqueue =		ohci_urb_enqueue,
-	.urb_dequeue =		ohci_urb_dequeue,
-	.endpoint_disable =	ohci_endpoint_disable,
-
-	/*
-	 * scheduling support
-	 */
-	.get_frame_number =	ohci_get_frame,
-
-	/*
-	 * root hub support
-	 */
-	.hub_status_data =	ohci_hub_status_data,
-	.hub_control =		ohci_hub_control,
-#ifdef	CONFIG_PM
-	.bus_suspend =		ohci_bus_suspend,
-	.bus_resume =		ohci_bus_resume,
-#endif
-	.start_port_reset =	ohci_start_port_reset,
 };
 
-/*-------------------------------------------------------------------------*/
-
-
 static const struct pci_device_id pci_ids [] = { {
 	/* handle any USB OHCI controller */
 	PCI_DEVICE_CLASS(PCI_CLASS_SERIAL_USB_OHCI, ~0),
@@ -377,3 +339,24 @@  static struct pci_driver ohci_pci_driver = {
 	},
 #endif
 };
+
+static int __init ohci_pci_init(void)
+{
+	if (usb_disabled())
+		return -ENODEV;
+
+	pr_info("%s: " DRIVER_DESC "\n", hcd_name);
+
+	ohci_init_driver(&ohci_pci_hc_driver, &overrides);
+	return pci_register_driver(&ohci_pci_driver);
+}
+module_init(ohci_pci_init);
+
+static void __exit ohci_pci_cleanup(void)
+{
+	pci_unregister_driver(&ohci_pci_driver);
+}
+module_exit(ohci_pci_cleanup);
+
+MODULE_DESCRIPTION(DRIVER_DESC);
+MODULE_LICENSE("GPL");
diff --git a/drivers/usb/host/ohci.h b/drivers/usb/host/ohci.h
index 12c750a..7a775849 100644
--- a/drivers/usb/host/ohci.h
+++ b/drivers/usb/host/ohci.h
@@ -440,6 +440,7 @@  static inline int quirk_amdprefetch(struct ohci_hcd *ohci)
 {
 	return ohci->flags & OHCI_QUIRK_AMD_PREFETCH;
 }
+extern void sb800_prefetch(struct ohci_hcd *ohci, int on);
 #else
 static inline int quirk_nec(struct ohci_hcd *ohci)
 {
@@ -457,6 +458,10 @@  static inline int quirk_amdprefetch(struct ohci_hcd *ohci)
 {
 	return 0;
 }
+static void sb800_prefetch(struct ohci_hcd *ohci, int on)
+{
+	return;
+}
 #endif
 
 /* convert between an hcd pointer and the corresponding ohci_hcd */
@@ -734,7 +739,6 @@  extern int ohci_init (struct ohci_hcd *ohci);
 extern void ohci_stop (struct usb_hcd *hcd);
 extern void ohci_hcd_init (struct ohci_hcd *ohci);
 extern int ohci_run (struct ohci_hcd *ohci);
-
 #if defined(CONFIG_PM) || defined(CONFIG_PCI)
 extern int ohci_restart (struct ohci_hcd *ohci);
 #else