diff mbox

[V2,6/6] USB: EHCI: make ehci-tegra a separate driver

Message ID 1370390014-25452-7-git-send-email-swarren@wwwdotorg.org
State Not Applicable, archived
Headers show

Commit Message

Stephen Warren June 4, 2013, 11:53 p.m. UTC 
From: Manjunath Goudar <manjunath.goudar@linaro.org>

Separate the Tegra on-chip host controller driver from
ehci-hcd host code so that it can be built as a separate driver module.
This work is part of enabling multi-platform kernels on ARM.

Signed-off-by: Manjunath Goudar <manjunath.goudar@linaro.org>
[swarren, reworked Manjunath's patches to split them more logically,
minor re-order of added lines to better match layout of other split-up
HCD drivers and existing code, add MODULE_DEVICE_TABLE, fix
MODULE_LICENSE.]
Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
v2:
* Set non-standard fields in tegra_ehci_hc_driver manually, rather than
  relying on an expanded struct ehci_driver_overrides.
* Save orig_hub_control rather than relying on ehci_hub_control being
  exported.
* Rebased on new versions of earlier patches.
---
 drivers/usb/host/Kconfig      |   2 +-
 drivers/usb/host/Makefile     |   1 +
 drivers/usb/host/ehci-hcd.c   |   5 --
 drivers/usb/host/ehci-tegra.c | 132 +++++++++++++++++++++++-------------------
 4 files changed, 73 insertions(+), 67 deletions(-)

Comments

Alan Stern June 5, 2013, 2:41 p.m. UTC | #1 
On Tue, 4 Jun 2013, Stephen Warren wrote:

> From: Manjunath Goudar <manjunath.goudar@linaro.org>
> 
> Separate the Tegra on-chip host controller driver from
> ehci-hcd host code so that it can be built as a separate driver module.
> This work is part of enabling multi-platform kernels on ARM.
> 
> Signed-off-by: Manjunath Goudar <manjunath.goudar@linaro.org>
> [swarren, reworked Manjunath's patches to split them more logically,
> minor re-order of added lines to better match layout of other split-up
> HCD drivers and existing code, add MODULE_DEVICE_TABLE, fix
> MODULE_LICENSE.]
> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> ---
> v2:
> * Set non-standard fields in tegra_ehci_hc_driver manually, rather than
>   relying on an expanded struct ehci_driver_overrides.
> * Save orig_hub_control rather than relying on ehci_hub_control being
>   exported.
> * Rebased on new versions of earlier patches.

Considering the changes recommended for the 4/6 patch, this needs a few 
updates too.

> diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c
> index c8dc687..b164757 100644
> --- a/drivers/usb/host/ehci-tegra.c
> +++ b/drivers/usb/host/ehci-tegra.c

>  struct tegra_ehci_hcd {
>  	struct ehci_hcd *ehci;
>  	struct tegra_usb_phy *phy;

Since you're doing the conversion, it makes sense also to convert this
structure from being separately allocated to using the private area at
the end of ehci_hcd.  (As part of that, the platform_set/get_drvdata
calls would store/retrieve the address of the ehci_hcd structure
instead of the tegra_ehci_hcd.)  Manjunath has already done this for
other drivers.

I suppose this could be added on in a separate patch.

> -static void tegra_ehci_shutdown(struct usb_hcd *hcd)
> -{
> -	struct tegra_ehci_hcd *tegra = dev_get_drvdata(hcd->self.controller);
> -
> -	/* ehci_shutdown touches the USB controller registers, make sure
> -	 * controller has clocks to it */
> -	if (!tegra->host_resumed)
> -		tegra_ehci_power_up(hcd);
> -
> -	ehci_shutdown(hcd);
> -}

Of course, this routine should already be gone.

> @@ -462,6 +429,9 @@ static int tegra_ehci_probe(struct platform_device *pdev)
>  		err = -ENOMEM;
>  		goto cleanup_clk;
>  	}
> +	tegra->ehci = hcd_to_ehci(hcd);

When the private data structure is moved to the private area, there 
will be no need for this back-pointer.  The "ehci" field can be 
eliminated.

> @@ -563,6 +533,12 @@ static void tegra_ehci_hcd_shutdown(struct platform_device *pdev)
>  	struct tegra_ehci_hcd *tegra = platform_get_drvdata(pdev);
>  	struct usb_hcd *hcd = ehci_to_hcd(tegra->ehci);
>  
> +	/*
> +	 * ehci_shutdown touches the USB controller registers, make sure
> +	 * controller has clocks to it
> +	 */
> +	if (!tegra->host_resumed)
> +		tegra_ehci_power_up(hcd);

This doesn't need to go here.  Since all the power management has been 
removed, the controller will never be suspended or powered down.  Hence 
there's no need to check or to power it back up.

> +static struct ehci_driver_overrides tegra_overrides __initdata = {
> +	.extra_priv_size	= sizeof(struct tegra_ehci_hcd),
> +};

The annotation should be __initconst rather than __initdata.

> +
> +static int __init ehci_tegra_init(void)
> +{
> +	if (usb_disabled())
> +		return -ENODEV;
> +
> +	pr_info(DRV_NAME ": " DRIVER_DESC "\n");
> +
> +	ehci_init_driver(&tegra_ehci_hc_driver, &tegra_overrides);
> +
> +	orig_hub_control = tegra_ehci_hc_driver.hub_control;
> +
> +	tegra_ehci_hc_driver.map_urb_for_dma = tegra_ehci_map_urb_for_dma;
> +	tegra_ehci_hc_driver.unmap_urb_for_dma = tegra_ehci_unmap_urb_for_dma;
> +	tegra_ehci_hc_driver.hub_control = tegra_ehci_hub_control;

You might want to add a comment explaining the reason for these manual 
overrides.  It's up to you.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index fcb20fd..6c9347c 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -200,7 +200,7 @@  config USB_EHCI_MSM
 	  has an external PHY.
 
 config USB_EHCI_TEGRA
-       boolean "NVIDIA Tegra HCD support"
+       tristate "NVIDIA Tegra HCD support"
        depends on ARCH_TEGRA
        select USB_EHCI_ROOT_HUB_TT
        select USB_PHY
diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
index dbc785d..c170383 100644
--- a/drivers/usb/host/Makefile
+++ b/drivers/usb/host/Makefile
@@ -33,6 +33,7 @@  obj-$(CONFIG_USB_EHCI_HCD_SPEAR)	+= ehci-spear.o
 obj-$(CONFIG_USB_EHCI_S5P)	+= ehci-s5p.o
 obj-$(CONFIG_USB_EHCI_HCD_AT91) += ehci-atmel.o
 obj-$(CONFIG_USB_EHCI_MSM)	+= ehci-msm.o
+obj-$(CONFIG_USB_EHCI_TEGRA)	+= ehci-tegra.o
 
 obj-$(CONFIG_USB_OXU210HP_HCD)	+= oxu210hp-hcd.o
 obj-$(CONFIG_USB_ISP116X_HCD)	+= isp116x-hcd.o
diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index e8a6f3d..7abf1ce 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -1269,11 +1269,6 @@  MODULE_LICENSE ("GPL");
 #define	PLATFORM_DRIVER		ehci_hcd_msp_driver
 #endif
 
-#ifdef CONFIG_USB_EHCI_TEGRA
-#include "ehci-tegra.c"
-#define PLATFORM_DRIVER		tegra_ehci_driver
-#endif
-
 #ifdef CONFIG_SPARC_LEON
 #include "ehci-grlib.c"
 #define PLATFORM_DRIVER		ehci_grlib_driver
diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c
index c8dc687..b164757 100644
--- a/drivers/usb/host/ehci-tegra.c
+++ b/drivers/usb/host/ehci-tegra.c
@@ -17,25 +17,44 @@ 
  */
 
 #include <linux/clk.h>
+#include <linux/clk/tegra.h>
+#include <linux/dma-mapping.h>
 #include <linux/err.h>
-#include <linux/platform_device.h>
-#include <linux/platform_data/tegra_usb.h>
-#include <linux/irq.h>
-#include <linux/usb/otg.h>
 #include <linux/gpio.h>
+#include <linux/io.h>
+#include <linux/irq.h>
+#include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_gpio.h>
+#include <linux/platform_device.h>
+#include <linux/platform_data/tegra_usb.h>
 #include <linux/pm_runtime.h>
+#include <linux/slab.h>
 #include <linux/usb/ehci_def.h>
 #include <linux/usb/tegra_usb_phy.h>
-#include <linux/clk/tegra.h>
+#include <linux/usb.h>
+#include <linux/usb/hcd.h>
+#include <linux/usb/otg.h>
+
+#include "ehci.h"
 
 #define TEGRA_USB_BASE			0xC5000000
 #define TEGRA_USB2_BASE			0xC5004000
 #define TEGRA_USB3_BASE			0xC5008000
 
+#define PORT_WAKE_BITS (PORT_WKOC_E|PORT_WKDISC_E|PORT_WKCONN_E)
+
 #define TEGRA_USB_DMA_ALIGN 32
 
+#define DRIVER_DESC "Tegra EHCI driver"
+#define DRV_NAME "tegra-ehci"
+
+static struct hc_driver __read_mostly tegra_ehci_hc_driver;
+
+static int (*orig_hub_control)(struct usb_hcd *hcd,
+				u16 typeReq, u16 wValue, u16 wIndex,
+				char *buf, u16 wLength);
+
 struct tegra_ehci_hcd {
 	struct ehci_hcd *ehci;
 	struct tegra_usb_phy *phy;
@@ -228,37 +247,13 @@  static int tegra_ehci_hub_control(
 	spin_unlock_irqrestore(&ehci->lock, flags);
 
 	/* Handle the hub control events here */
-	return ehci_hub_control(hcd, typeReq, wValue, wIndex, buf, wLength);
+	return orig_hub_control(hcd, typeReq, wValue, wIndex, buf, wLength);
+
 done:
 	spin_unlock_irqrestore(&ehci->lock, flags);
 	return retval;
 }
 
-static void tegra_ehci_shutdown(struct usb_hcd *hcd)
-{
-	struct tegra_ehci_hcd *tegra = dev_get_drvdata(hcd->self.controller);
-
-	/* ehci_shutdown touches the USB controller registers, make sure
-	 * controller has clocks to it */
-	if (!tegra->host_resumed)
-		tegra_ehci_power_up(hcd);
-
-	ehci_shutdown(hcd);
-}
-
-static int tegra_ehci_setup(struct usb_hcd *hcd)
-{
-	struct ehci_hcd *ehci = hcd_to_ehci(hcd);
-
-	/* EHCI registers start at offset 0x100 */
-	ehci->caps = hcd->regs + 0x100;
-
-	/* switch to host mode */
-	hcd->has_tt = 1;
-
-	return ehci_setup(hcd);
-}
-
 struct dma_aligned_buffer {
 	void *kmalloc_ptr;
 	void *old_xfer_buffer;
@@ -338,34 +333,6 @@  static void tegra_ehci_unmap_urb_for_dma(struct usb_hcd *hcd, struct urb *urb)
 	free_dma_aligned_buffer(urb);
 }
 
-static const struct hc_driver tegra_ehci_hc_driver = {
-	.description		= hcd_name,
-	.product_desc		= "Tegra EHCI Host Controller",
-	.hcd_priv_size		= sizeof(struct ehci_hcd),
-	.flags			= HCD_USB2 | HCD_MEMORY,
-
-	/* standard ehci functions */
-	.irq			= ehci_irq,
-	.start			= ehci_run,
-	.stop			= ehci_stop,
-	.urb_enqueue		= ehci_urb_enqueue,
-	.urb_dequeue		= ehci_urb_dequeue,
-	.endpoint_disable	= ehci_endpoint_disable,
-	.endpoint_reset		= ehci_endpoint_reset,
-	.get_frame_number	= ehci_get_frame,
-	.hub_status_data	= ehci_hub_status_data,
-	.clear_tt_buffer_complete = ehci_clear_tt_buffer_complete,
-	.relinquish_port	= ehci_relinquish_port,
-	.port_handed_over	= ehci_port_handed_over,
-
-	/* modified ehci functions for tegra */
-	.reset			= tegra_ehci_setup,
-	.shutdown		= tegra_ehci_shutdown,
-	.map_urb_for_dma	= tegra_ehci_map_urb_for_dma,
-	.unmap_urb_for_dma	= tegra_ehci_unmap_urb_for_dma,
-	.hub_control		= tegra_ehci_hub_control,
-};
-
 static int setup_vbus_gpio(struct platform_device *pdev,
 			   struct tegra_ehci_platform_data *pdata)
 {
@@ -462,6 +429,9 @@  static int tegra_ehci_probe(struct platform_device *pdev)
 		err = -ENOMEM;
 		goto cleanup_clk;
 	}
+	tegra->ehci = hcd_to_ehci(hcd);
+
+	hcd->has_tt = 1;
 	hcd->phy = u_phy;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -478,6 +448,7 @@  static int tegra_ehci_probe(struct platform_device *pdev)
 		err = -ENOMEM;
 		goto cleanup_hcd_create;
 	}
+	tegra->ehci->caps = hcd->regs + 0x100;
 
 	err = usb_phy_init(hcd->phy);
 	if (err) {
@@ -501,7 +472,6 @@  static int tegra_ehci_probe(struct platform_device *pdev)
 	}
 
 	tegra->host_resumed = 1;
-	tegra->ehci = hcd_to_ehci(hcd);
 
 	irq = platform_get_irq(pdev, 0);
 	if (!irq) {
@@ -563,6 +533,12 @@  static void tegra_ehci_hcd_shutdown(struct platform_device *pdev)
 	struct tegra_ehci_hcd *tegra = platform_get_drvdata(pdev);
 	struct usb_hcd *hcd = ehci_to_hcd(tegra->ehci);
 
+	/*
+	 * ehci_shutdown touches the USB controller registers, make sure
+	 * controller has clocks to it
+	 */
+	if (!tegra->host_resumed)
+		tegra_ehci_power_up(hcd);
 	if (hcd->driver->shutdown)
 		hcd->driver->shutdown(hcd);
 }
@@ -577,7 +553,41 @@  static struct platform_driver tegra_ehci_driver = {
 	.remove		= tegra_ehci_remove,
 	.shutdown	= tegra_ehci_hcd_shutdown,
 	.driver		= {
-		.name	= "tegra-ehci",
+		.name	= DRV_NAME,
 		.of_match_table = tegra_ehci_of_match,
 	}
 };
+
+static struct ehci_driver_overrides tegra_overrides __initdata = {
+	.extra_priv_size	= sizeof(struct tegra_ehci_hcd),
+};
+
+static int __init ehci_tegra_init(void)
+{
+	if (usb_disabled())
+		return -ENODEV;
+
+	pr_info(DRV_NAME ": " DRIVER_DESC "\n");
+
+	ehci_init_driver(&tegra_ehci_hc_driver, &tegra_overrides);
+
+	orig_hub_control = tegra_ehci_hc_driver.hub_control;
+
+	tegra_ehci_hc_driver.map_urb_for_dma = tegra_ehci_map_urb_for_dma;
+	tegra_ehci_hc_driver.unmap_urb_for_dma = tegra_ehci_unmap_urb_for_dma;
+	tegra_ehci_hc_driver.hub_control = tegra_ehci_hub_control;
+
+	return platform_driver_register(&tegra_ehci_driver);
+}
+module_init(ehci_tegra_init);
+
+static void __exit ehci_tegra_cleanup(void)
+{
+	platform_driver_unregister(&tegra_ehci_driver);
+}
+module_exit(ehci_tegra_cleanup);
+
+MODULE_DESCRIPTION(DRIVER_DESC);
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:" DRV_NAME);
+MODULE_DEVICE_TABLE(of, tegra_ehci_of_match);