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

[Stephen Warren]

From: Manjunath Goudar 

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 
[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 
---
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(-)

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");
 #definePLATFORM_DRIVER ehci_hcd_msp_driver
 #endif
 
-#ifdef CONFIG_USB_EHCI_TEGRA
-#include "ehci-tegra.c"
-#define PLATFORM_DRIVERtegra_ehci_driver
-#endif
-
 #ifdef CONFIG_SPARC_LEON
 #include "ehci-grlib.c"
 #define PLATFORM_DRIVERehci_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 
+#include 
+#include 
 #include 
-#include 
-#include 
-#include 
-#include 
 #include 
+#include 
+#include 
+#include 
 #include 
 #include 
+#include 
+#include 
 #include 
+#include 
 #include 
 #include 
-#include 
+#include 
+#include 
+#include 
+
+#include "ehci.h"
 
 #define TEGRA_USB_BASE 0xC500
 #define TEGRA_USB2_BASE0xC5004000
 #define TEGRA_USB3_BASE0xC5008000
 
+#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_driv

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

[Alan Stern]

On Tue, 4 Jun 2013, Stephen Warren wrote:

> From: Manjunath Goudar 
> 
> 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 
> [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 
> ---
> 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-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html