[PATCH 0/2]: Make FSL EHCI drv as separate module

[PATCH 0/2]: Make FSL EHCI drv as separate module

[Ramneek Mehresh]

This patch set makes fsl ehci driver as a separate module 
from existing ehci-hcd driver. For this, following is done:
	- make ehci-fsl.c as a seperate entity by compiling
	  it separately
	- make use of module_init()/module_exit(), and 
	  makeing ehci-fsl.c as tristate
	- using overrides structure to override "reset" and
	  "extra_priv_size"

Ramneek Mehresh (2):
  drivers:usb:fsl:Make fsl ehci drv an independent module
  drivers:usb:fsl: Use overrides structure for fsl hc driver

 drivers/usb/host/Kconfig    |   2 +-
 drivers/usb/host/Makefile   |   1 +
 drivers/usb/host/ehci-fsl.c | 164 +++++++++++++++++++-------------------------
 drivers/usb/host/ehci-hcd.c |   5 --
 4 files changed, 72 insertions(+), 100 deletions(-)

-- 
1.8.3.1

[PATCH 1/2] drivers:usb:fsl:Make fsl ehci drv an independent module

[Ramneek Mehresh]

Make fsl ehci driver an independent entity from ehci-hcd driver

Signed-off-by: Ramneek Mehresh <ramneek.mehresh@freescale.com>
---
 drivers/usb/host/Kconfig    |   2 +-
 drivers/usb/host/Makefile   |   1 +
 drivers/usb/host/ehci-fsl.c | 116 ++++++++++++++++++++++++--------------------
 drivers/usb/host/ehci-hcd.c |   5 --
 4 files changed, 65 insertions(+), 59 deletions(-)

diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index 197a6a3..547cee8 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -137,7 +137,7 @@ config XPS_USB_HCD_XILINX
 		devices only.
 
 config USB_EHCI_FSL
-	bool "Support for Freescale PPC on-chip EHCI USB controller"
+	tristate "Support for Freescale PPC on-chip EHCI USB controller"
 	depends on FSL_SOC
 	select USB_EHCI_ROOT_HUB_TT
 	select USB_FSL_MPH_DR_OF if OF
diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
index 972a74a..754efaa 100644
--- a/drivers/usb/host/Makefile
+++ b/drivers/usb/host/Makefile
@@ -72,6 +72,7 @@ obj-$(CONFIG_USB_R8A66597_HCD)	+= r8a66597-hcd.o
 obj-$(CONFIG_USB_HWA_HCD)	+= hwa-hc.o
 obj-$(CONFIG_USB_IMX21_HCD)	+= imx21-hcd.o
 obj-$(CONFIG_USB_FSL_MPH_DR_OF)	+= fsl-mph-dr-of.o
+obj-$(CONFIG_USB_EHCI_FSL)	+= ehci-fsl.o
 obj-$(CONFIG_USB_HCD_BCMA)	+= bcma-hcd.o
 obj-$(CONFIG_USB_HCD_SSB)	+= ssb-hcd.o
 obj-$(CONFIG_USB_FUSBH200_HCD)	+= fusbh200-hcd.o
diff --git a/drivers/usb/host/ehci-fsl.c b/drivers/usb/host/ehci-fsl.c
index ab4eee3..42628e5 100644
--- a/drivers/usb/host/ehci-fsl.c
+++ b/drivers/usb/host/ehci-fsl.c
@@ -24,20 +24,29 @@
  */
 
 #include <linux/kernel.h>
+#include <linux/module.h>
 #include <linux/types.h>
 #include <linux/delay.h>
 #include <linux/pm.h>
 #include <linux/err.h>
+#include <linux/usb.h>
+#include <linux/usb/ehci_def.h>
+#include <linux/usb/hcd.h>
+#include <linux/usb/otg.h>
 #include <linux/platform_device.h>
 #include <linux/fsl_devices.h>
 
+#include "ehci.h"
 #include "ehci-fsl.h"
 
+#define DRIVER_DESC "Fsl EHCI driver"
+#define DRV_NAME "ehci-fsl"
+
 /* configure so an HC device and id are always provided */
 /* always called with process context; sleeping is OK */
 
-/**
- * usb_hcd_fsl_probe - initialize FSL-based HCDs
+/*
+ * fsl_ehci_drv_probe - initialize FSL-based HCDs
  * @drvier: Driver to be used for this HCD
  * @pdev: USB Host Controller being probed
  * Context: !in_interrupt()
@@ -45,8 +54,7 @@
  * Allocates basic resources for this USB host controller.
  *
  */
-static int usb_hcd_fsl_probe(const struct hc_driver *driver,
-			     struct platform_device *pdev)
+static int fsl_ehci_drv_probe(struct platform_device *pdev)
 {
 	struct fsl_usb2_platform_data *pdata;
 	struct usb_hcd *hcd;
@@ -86,7 +94,8 @@ static int usb_hcd_fsl_probe(const struct hc_driver *driver,
 	}
 	irq = res->start;
 
-	hcd = usb_create_hcd(driver, &pdev->dev, dev_name(&pdev->dev));
+	hcd = usb_create_hcd(&fsl_ehci_hc_driver, &pdev->dev,
+				dev_name(&pdev->dev));
 	if (!hcd) {
 		retval = -ENOMEM;
 		goto err1;
@@ -159,38 +168,6 @@ static int usb_hcd_fsl_probe(const struct hc_driver *driver,
 	return retval;
 }
 
-/* may be called without controller electrically present */
-/* may be called with controller, bus, and devices active */
-
-/**
- * usb_hcd_fsl_remove - shutdown processing for FSL-based HCDs
- * @dev: USB Host Controller being removed
- * Context: !in_interrupt()
- *
- * Reverses the effect of usb_hcd_fsl_probe().
- *
- */
-static void usb_hcd_fsl_remove(struct usb_hcd *hcd,
-			       struct platform_device *pdev)
-{
-	struct fsl_usb2_platform_data *pdata = dev_get_platdata(&pdev->dev);
-
-	if (!IS_ERR_OR_NULL(hcd->usb_phy)) {
-		otg_set_host(hcd->usb_phy->otg, NULL);
-		usb_put_phy(hcd->usb_phy);
-	}
-
-	usb_remove_hcd(hcd);
-
-	/*
-	 * do platform specific un-initialization:
-	 * release iomux pins, disable clock, etc.
-	 */
-	if (pdata->exit)
-		pdata->exit(pdev);
-	usb_put_hcd(hcd);
-}
-
 static int ehci_fsl_setup_phy(struct usb_hcd *hcd,
 			       enum fsl_usb2_phy_modes phy_mode,
 			       unsigned int port_offset)
@@ -637,7 +614,7 @@ static int ehci_start_port_reset(struct usb_hcd *hcd, unsigned port)
 #endif /* CONFIG_USB_OTG */
 
 
-static const struct hc_driver ehci_fsl_hc_driver = {
+static const struct hc_driver fsl_ehci_hc_driver = {
 	.description = hcd_name,
 	.product_desc = "Freescale On-Chip EHCI Host Controller",
 	.hcd_priv_size = sizeof(struct ehci_fsl),
@@ -683,32 +660,65 @@ static const struct hc_driver ehci_fsl_hc_driver = {
 	.clear_tt_buffer_complete = ehci_clear_tt_buffer_complete,
 };
 
-static int ehci_fsl_drv_probe(struct platform_device *pdev)
+/*
+ * usb_hcd_fsl_remove - shutdown processing for FSL-based HCDs
+ * @dev: USB Host Controller being removed
+ * Context: !in_interrupt()
+ *
+ * Reverses the effect of usb_hcd_fsl_probe().
+ *
+ */
+static int fsl_ehci_drv_remove(struct platform_device *pdev)
 {
-	if (usb_disabled())
-		return -ENODEV;
+	struct fsl_usb2_platform_data *pdata = dev_get_platdata(&pdev->dev);
+	struct usb_hcd *hcd = platform_get_drvdata(pdev);
 
-	/* FIXME we only want one one probe() not two */
-	return usb_hcd_fsl_probe(&ehci_fsl_hc_driver, pdev);
-}
+	if (!IS_ERR_OR_NULL(hcd->usb_phy)) {
+		otg_set_host(hcd->usb_phy->otg, NULL);
+		usb_put_phy(hcd->usb_phy);
+	}
 
-static int ehci_fsl_drv_remove(struct platform_device *pdev)
-{
-	struct usb_hcd *hcd = platform_get_drvdata(pdev);
+	usb_remove_hcd(hcd);
+
+	/*
+	 * do platform specific un-initialization:
+	 * release iomux pins, disable clock, etc.
+	 */
+	if (pdata->exit)
+		pdata->exit(pdev);
+
+	usb_put_hcd(hcd);
 
-	/* FIXME we only want one one remove() not two */
-	usb_hcd_fsl_remove(hcd, pdev);
 	return 0;
 }
 
-MODULE_ALIAS("platform:fsl-ehci");
-
 static struct platform_driver ehci_fsl_driver = {
-	.probe = ehci_fsl_drv_probe,
-	.remove = ehci_fsl_drv_remove,
+	.probe = fsl_ehci_drv_probe,
+	.remove = fsl_ehci_drv_remove,
 	.shutdown = usb_hcd_platform_shutdown,
 	.driver = {
 		.name = "fsl-ehci",
 		.pm = EHCI_FSL_PM_OPS,
 	},
 };
+
+static int __init ehci_fsl_init(void)
+{
+	if (usb_disabled())
+		return -ENODEV;
+
+	pr_info(DRV_NAME ": " DRIVER_DESC "\n");
+
+	return platform_driver_register(&ehci_fsl_driver);
+}
+module_init(ehci_fsl_init);
+
+static void __exit ehci_fsl_cleanup(void)
+{
+	platform_driver_unregister(&ehci_fsl_driver);
+}
+module_exit(ehci_fsl_cleanup);
+
+MODULE_DESCRIPTION(DRIVER_DESC);
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:" DRV_NAME);
diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index f4d88df..9dd161c 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -1250,11 +1250,6 @@ MODULE_DESCRIPTION(DRIVER_DESC);
 MODULE_AUTHOR (DRIVER_AUTHOR);
 MODULE_LICENSE ("GPL");
 
-#ifdef CONFIG_USB_EHCI_FSL
-#include "ehci-fsl.c"
-#define	PLATFORM_DRIVER		ehci_fsl_driver
-#endif
-
 #ifdef CONFIG_USB_EHCI_SH
 #include "ehci-sh.c"
 #define PLATFORM_DRIVER		ehci_hcd_sh_driver
-- 
1.8.3.1

[PATCH 2/2] drivers:usb:fsl: Use overrides structure for fsl hc driver

[Ramneek Mehresh]

Use overrides structure for fsl ehci host controller driver

Signed-off-by: Ramneek Mehresh <ramneek.mehresh@freescale.com>
---
 drivers/usb/host/ehci-fsl.c | 54 +++++++++------------------------------------
 1 file changed, 10 insertions(+), 44 deletions(-)

diff --git a/drivers/usb/host/ehci-fsl.c b/drivers/usb/host/ehci-fsl.c
index 42628e5..81c8df0 100644
--- a/drivers/usb/host/ehci-fsl.c
+++ b/drivers/usb/host/ehci-fsl.c
@@ -42,6 +42,8 @@
 #define DRIVER_DESC "Fsl EHCI driver"
 #define DRV_NAME "ehci-fsl"
 
+static struct hc_driver __read_mostly fsl_ehci_hc_driver;
+
 /* configure so an HC device and id are always provided */
 /* always called with process context; sleeping is OK */
 
@@ -613,51 +615,9 @@ static int ehci_start_port_reset(struct usb_hcd *hcd, unsigned port)
 #define ehci_start_port_reset	NULL
 #endif /* CONFIG_USB_OTG */
 
-
-static const struct hc_driver fsl_ehci_hc_driver = {
-	.description = hcd_name,
-	.product_desc = "Freescale On-Chip EHCI Host Controller",
-	.hcd_priv_size = sizeof(struct ehci_fsl),
-
-	/*
-	 * generic hardware linkage
-	 */
-	.irq = ehci_irq,
-	.flags = HCD_USB2 | HCD_MEMORY | HCD_BH,
-
-	/*
-	 * basic lifecycle operations
-	 */
+static struct ehci_driver_overrides ehci_fsl_overrides __initdata = {
+	.extra_priv_size = sizeof(struct ehci_fsl),
 	.reset = ehci_fsl_setup,
-	.start = ehci_run,
-	.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,
-	.start_port_reset = ehci_start_port_reset,
-	.relinquish_port = ehci_relinquish_port,
-	.port_handed_over = ehci_port_handed_over,
-
-	.clear_tt_buffer_complete = ehci_clear_tt_buffer_complete,
 };
 
 /*
@@ -709,6 +669,12 @@ static int __init ehci_fsl_init(void)
 
 	pr_info(DRV_NAME ": " DRIVER_DESC "\n");
 
+	ehci_init_driver(&fsl_ehci_hc_driver, &ehci_fsl_overrides);
+
+	fsl_ehci_hc_driver.product_desc =
+			"Freescale On-Chip EHCI Host Controller";
+	fsl_ehci_hc_driver.start_port_reset = ehci_start_port_reset;
+
 	return platform_driver_register(&ehci_fsl_driver);
 }
 module_init(ehci_fsl_init);
-- 
1.8.3.1

Re: [PATCH 0/2]: Make FSL EHCI drv as separate module

[Alan Stern]

On Tue, 12 May 2015, Ramneek Mehresh wrote:

> This patch set makes fsl ehci driver as a separate module 
> from existing ehci-hcd driver. For this, following is done:
> 	- make ehci-fsl.c as a seperate entity by compiling
> 	  it separately
> 	- make use of module_init()/module_exit(), and 
> 	  makeing ehci-fsl.c as tristate
> 	- using overrides structure to override "reset" and
> 	  "extra_priv_size"
> 
> Ramneek Mehresh (2):
>   drivers:usb:fsl:Make fsl ehci drv an independent module
>   drivers:usb:fsl: Use overrides structure for fsl hc driver

These need to be combined into a single patch.  As it stands now, if 
someone applies patch 1/2 but not 2/2 (which could happen in the middle 
of a bisection, for example), the driver will fail to build.


+#define DRIVER_DESC "Fsl EHCI driver"

...

-static const struct hc_driver fsl_ehci_hc_driver = {
-       .description = hcd_name,
-       .product_desc = "Freescale On-Chip EHCI Host Controller",

How is anybody supposed to know what "Fsl" means?  Your new DRIVER_DESC 
string should be more like the old .product_desc string.

-/**
- * usb_hcd_fsl_probe - initialize FSL-based HCDs
+/*
+ * fsl_ehci_drv_probe - initialize FSL-based HCDs
  * @drvier: Driver to be used for this HCD
  * @pdev: USB Host Controller being probed
  * Context: !in_interrupt()
@@ -45,8 +54,7 @@
  * Allocates basic resources for this USB host controller.
  *
  */
-static int usb_hcd_fsl_probe(const struct hc_driver *driver,
-                            struct platform_device *pdev)
+static int fsl_ehci_drv_probe(struct platform_device *pdev)
 {

Since you eliminated the const struct hc_driver *driver parameter from 
this function, you should also eliminate the corresponding line from 
the kerneldoc above the function.  (Note that the line even misspells 
"driver"!)

Alan Stern