[V2 0/8] usb: ehci: more bus glues as separate modules

[V2 0/8] usb: ehci: more bus glues as separate modules

[Manjunath Goudar]

Separate the SOC On-Chip host controller driver from ehci-hcd
host code into its own driver module

V2:
Modified the patches, based on the first version review comments  

Manjunath Goudar (8):
  USB: EHCI: make ehci-spear a separate driver
  USB: EHCI: make ehci-atmel a separate driver
  USB: EHCI: make ehci-s5p a separate driver
  USB: EHCI: make ehci-mv as separate static driver
  USB: EHCI: make ehci-vt8500 a separate driver
  USB: EHCI: make ehci-msm a separate driver
  USB: EHCI: make ehci-w90X900 a separate driver
  USB: EHCI: make ehci-orion a separate driver

 drivers/usb/host/Kconfig        |   40 +++++++++++++++--
 drivers/usb/host/Makefile       |    9 +++-
 drivers/usb/host/ehci-atmel.c   |   76 ++++++++++++++++----------------
 drivers/usb/host/ehci-hcd.c     |   52 +++++-----------------
 drivers/usb/host/ehci-msm.c     |   85 ++++++++++++++++--------------------
 drivers/usb/host/ehci-mv.c      |   78 +++++++++++++++------------------
 drivers/usb/host/ehci-orion.c   |   90 ++++++++++++++++++--------------------
 drivers/usb/host/ehci-s5p.c     |   67 +++++++++++++++-------------
 drivers/usb/host/ehci-spear.c   |   77 +++++++++++++++++----------------
 drivers/usb/host/ehci-vt8500.c  |   73 ++++++++++++++-----------------
 drivers/usb/host/ehci-w90x900.c |   91 +++++++++++++++++----------------------
 drivers/usb/host/ehci.h         |    2 +-
 12 files changed, 358 insertions(+), 382 deletions(-)

-- 
1.7.9.5

[V2 0/8] usb: ehci: more bus glues as separate modules

[Arnd Bergmann]

On Friday 15 February 2013, Manjunath Goudar wrote:
> Separate the SOC On-Chip host controller driver from ehci-hcd
> host code into its own driver module
> 
> V2:
> Modified the patches, based on the first version review comments  
> 

The series looks good to me now, great work!

Please add my 'Acked-by: Arnd Bergmann <arnd@arndb.de>' for each patch.

Alan, Greg: I realize that it's too late to take the entire series for
3.9, but with the move of more ARM platforms to CONFIG_ARCH_MULTIPLATFORM,
it is now possible to enable OMAP, VT8500 and ORION concurrently, along
with the ones that are already moved out.

You should have a patch for OMAP, but that has not made it into linux-next
yet. It would be good to have at least two out the three I mentioned
merged into 3.9, in order to keep allyesconfig building, and take the
rest after the merge window, for integration into 3.10.

What I get today with linux-next ARM allyesconfig is this:

/home/arnd/arm-soc/drivers/usb/host/ehci-hcd.c:1277:0: warning: "PLATFORM_DRIVER" redefined [enabled by default]
 #define PLATFORM_DRIVER  ehci_orion_driver
 ^
/home/arnd/arm-soc/drivers/usb/host/ehci-hcd.c:1257:0: note: this is the location of the previous definition
 #define        PLATFORM_DRIVER         ehci_hcd_omap_driver
 ^
/home/arnd/arm-soc/drivers/usb/host/ehci-hcd.c:1297:0: warning: "PLATFORM_DRIVER" redefined [enabled by default]
 #define PLATFORM_DRIVER  vt8500_ehci_driver
 ^
/home/arnd/arm-soc/drivers/usb/host/ehci-hcd.c:1277:0: note: this is the location of the previous definition
 #define PLATFORM_DRIVER  ehci_orion_driver
 ^
/home/arnd/arm-soc/drivers/usb/host/ehci-hcd.c:1277:0: warning: "PLATFORM_DRIVER" redefined [enabled by default]
 #define PLATFORM_DRIVER  ehci_orion_driver
 ^
/home/arnd/arm-soc/drivers/usb/host/ehci-hcd.c:1257:0: note: this is the location of the previous definition
 #define        PLATFORM_DRIVER         ehci_hcd_omap_driver
 ^
/home/arnd/arm-soc/drivers/usb/host/ehci-hcd.c:1297:0: warning: "PLATFORM_DRIVER" redefined [enabled by default]
 #define PLATFORM_DRIVER  vt8500_ehci_driver
 ^
/home/arnd/arm-soc/drivers/usb/host/ehci-hcd.c:1277:0: note: this is the location of the previous definition
 #define PLATFORM_DRIVER  ehci_orion_driver
 ^

In 3.10, we are going to need spear and s5p as well, possibly also atmel,
mv and tegra, but Manjunath has done all the ARM platforms now (the tegra
patch still has issues).

	Arnd

[V2 0/8] usb: ehci: more bus glues as separate modules

[Greg KH]

On Fri, Feb 15, 2013 at 04:10:52PM +0000, Arnd Bergmann wrote:
> On Friday 15 February 2013, Manjunath Goudar wrote:
> > Separate the SOC On-Chip host controller driver from ehci-hcd
> > host code into its own driver module
> > 
> > V2:
> > Modified the patches, based on the first version review comments  
> > 
> 
> The series looks good to me now, great work!
> 
> Please add my 'Acked-by: Arnd Bergmann <arnd@arndb.de>' for each patch.
> 
> Alan, Greg: I realize that it's too late to take the entire series for
> 3.9, but with the move of more ARM platforms to CONFIG_ARCH_MULTIPLATFORM,
> it is now possible to enable OMAP, VT8500 and ORION concurrently, along
> with the ones that are already moved out.
> 
> You should have a patch for OMAP, but that has not made it into linux-next
> yet. It would be good to have at least two out the three I mentioned
> merged into 3.9, in order to keep allyesconfig building, and take the
> rest after the merge window, for integration into 3.10.

I have no idea what to do with _ANY_ of these patches as they are all
sent without proper changelog comments saying why they are needed, come
in random series without proper ordering, get sent multiple times in the
same series with no known differences between them, and probably other
issues that I just don't even know about.

So I can't take these this late in the development process, sorry, that
would be negligent of me.

Manjunath, please get this all working properly, as it is, I've deleted
all of your patches from my "to-apply" queue for all of the above
reasons.

greg k-h

[V2 0/8] usb: ehci: more bus glues as separate modules

[Arnd Bergmann]

On Friday 15 February 2013, Greg KH wrote:
> > Alan, Greg: I realize that it's too late to take the entire series for
> > 3.9, but with the move of more ARM platforms to CONFIG_ARCH_MULTIPLATFORM,
> > it is now possible to enable OMAP, VT8500 and ORION concurrently, along
> > with the ones that are already moved out.
> > 
> > You should have a patch for OMAP, but that has not made it into linux-next
> > yet. It would be good to have at least two out the three I mentioned
> > merged into 3.9, in order to keep allyesconfig building, and take the
> > rest after the merge window, for integration into 3.10.
> 
> I have no idea what to do with ANY of these patches as they are all
> sent without proper changelog comments saying why they are needed, come
> in random series without proper ordering, get sent multiple times in the
> same series with no known differences between them, and probably other
> issues that I just don't even know about.
> 
> So I can't take these this late in the development process, sorry, that
> would be negligent of me.

Yes, as I said above I think we should only merge two of them for 3.9
in order to keep the bus glues that are now conflicting (omap, orion
and vt8500) working when you enable all of the EHCI back-ends.

These three are actually all pretty popular targets (vt8500 not yet so
much, but it's used in some of the new sub-$100 tablets and HDMI
stick computers), so I would actually expect that distros like
Debin will want to enable them all together.

The other patches are mainly there to solve the problem for all ARM
platforms so we don't have to deal with them one by one as we make
the remaining platforms multiplatform capable.

> Manjunath, please get this all working properly, as it is, I've deleted
> all of your patches from my "to-apply" queue for all of the above
> reasons.

Sorry for the time this is causing you, I'm still teaching Manjunath how
to follow all the processes and it took us a bit longer than expected
to get this far. I'll try to make this less painful for you with the rest
of the patches that should go into 3.10, and I'll send you the two
patches that I'd like to still see in 3.9 with a proper changelog
and in a form that applies on your usb-next branch.

	Arnd

[PATCH 1/2] USB: EHCI: make ehci-vt8500 a separate driver

[Arnd Bergmann]

From: Manjunath Goudar <manjunath.goudar@linaro.org>

With the multiplatform changes in arm-soc tree, it becomes
possible to enable the vt8500 platform at the same time
as other platforms that require a conflicting EHCI bus
glue. At the moment, this results in a warning like

drivers/usb/host/ehci-hcd.c:1277:0: warning: "PLATFORM_DRIVER" redefined [enabled by default]
drivers/usb/host/ehci-hcd.c:1257:0: note: this is the location of the previous definition
drivers/usb/host/ehci-omap.c:319:31: warning: 'ehci_hcd_omap_driver' defined but not used [-Wunused-variable]

and an ehci driver that only works on one of them.

With the infrastructure added by Alan Stern in patch 3e0232039
"USB: EHCI: prepare to make ehci-hcd a library module", we can
avoid this problem by turning a bus glue into a separate
module, as we do here for the vt8500 bus glue.

Signed-off-by: Manjunath Goudar <manjunath.goudar@linaro.org>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Tony Prisk <linux@prisktech.co.nz>
Cc: Alexey Charkov <alchark@gmail.com>
---
 drivers/usb/host/Kconfig       |  8 +++++
 drivers/usb/host/Makefile      |  1 +
 drivers/usb/host/ehci-hcd.c    |  6 +---
 drivers/usb/host/ehci-vt8500.c | 73 +++++++++++++++++++-----------------------
 4 files changed, 43 insertions(+), 45 deletions(-)

diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index c59a112..d77e028 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -162,6 +162,14 @@ config USB_EHCI_HCD_OMAP
 	  Enables support for the on-chip EHCI controller on
 	  OMAP3 and later chips.
 
+config USB_EHCI_HCD_VT8500
+        tristate "Support for VT8500 on-chip EHCI USB controller"
+        depends on USB_EHCI_HCD && ARCH_VT8500
+        default y
+        ---help---
+          Enables support for the on-chip EHCI controller on
+          VT8500 chips.
+
 config USB_EHCI_MSM
 	bool "Support for MSM on-chip EHCI USB controller"
 	depends on USB_EHCI_HCD && ARCH_MSM
diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
index 001fbff..b0da9cf 100644
--- a/drivers/usb/host/Makefile
+++ b/drivers/usb/host/Makefile
@@ -29,6 +29,7 @@ obj-$(CONFIG_USB_EHCI_HCD_PLATFORM)	+= ehci-platform.o
 obj-$(CONFIG_USB_EHCI_MXC)	+= ehci-mxc.o
 
 obj-$(CONFIG_USB_OXU210HP_HCD)	+= oxu210hp-hcd.o
+obj-$(CONFIG_USB_EHCI_HCD_VT8500)+= ehci-vt8500.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
diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index b416a3f..487ebb8 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -1292,11 +1292,6 @@ MODULE_LICENSE ("GPL");
 #define PLATFORM_DRIVER		ehci_octeon_driver
 #endif
 
-#ifdef CONFIG_ARCH_VT8500
-#include "ehci-vt8500.c"
-#define	PLATFORM_DRIVER		vt8500_ehci_driver
-#endif
-
 #ifdef CONFIG_PLAT_SPEAR
 #include "ehci-spear.c"
 #define PLATFORM_DRIVER		spear_ehci_hcd_driver
@@ -1347,6 +1342,7 @@ MODULE_LICENSE ("GPL");
 	!IS_ENABLED(CONFIG_USB_CHIPIDEA_HOST) && \
 	!IS_ENABLED(CONFIG_USB_EHCI_MXC) && \
 	!defined(PLATFORM_DRIVER) && \
+	!IS_ENABLED(CONFIG_ARCH_VT8500) && \
 	!defined(PS3_SYSTEM_BUS_DRIVER) && \
 	!defined(OF_PLATFORM_DRIVER) && \
 	!defined(XILINX_OF_PLATFORM_DRIVER)
diff --git a/drivers/usb/host/ehci-vt8500.c b/drivers/usb/host/ehci-vt8500.c
index 11695d5..98d65bd 100644
--- a/drivers/usb/host/ehci-vt8500.c
+++ b/drivers/usb/host/ehci-vt8500.c
@@ -16,52 +16,25 @@
  *
  */
 
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/usb.h>
+#include <linux/usb/hcd.h>
+#include <linux/io.h>
+#include <linux/dma-mapping.h>
 #include <linux/of.h>
 #include <linux/platform_device.h>
 
-static const struct hc_driver vt8500_ehci_hc_driver = {
-	.description		= hcd_name,
-	.product_desc		= "VT8500 EHCI",
-	.hcd_priv_size		= sizeof(struct ehci_hcd),
+#include "ehci.h"
 
-	/*
-	 * generic hardware linkage
-	 */
-	.irq			= ehci_irq,
-	.flags			= HCD_MEMORY | HCD_USB2,
-
-	/*
-	 * basic lifecycle operations
-	 */
-	.reset			= ehci_setup,
-	.start			= ehci_run,
-	.stop			= ehci_stop,
-	.shutdown		= ehci_shutdown,
+#define DRIVER_DESC "vt8500 On-Chip EHCI Host driver"
 
-	/*
-	 * 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,
+static const char hcd_name[] = "ehci-vt8500";
 
-	/*
-	 * scheduling support
-	 */
-	.get_frame_number	= ehci_get_frame,
+static struct hc_driver __read_mostly vt8500_ehci_hc_driver;
 
-	/*
-	 * 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,
-
-	.clear_tt_buffer_complete	= ehci_clear_tt_buffer_complete,
+static const struct ehci_driver_overrides ehci_vt8500_overrides __initdata = {
+	.reset = ehci_setup,
 };
 
 static u64 vt8500_ehci_dma_mask = DMA_BIT_MASK(32);
@@ -140,11 +113,31 @@ static struct platform_driver vt8500_ehci_driver = {
 	.remove		= vt8500_ehci_drv_remove,
 	.shutdown	= usb_hcd_platform_shutdown,
 	.driver = {
-		.name	= "vt8500-ehci",
+		.name	= hcd_name,
 		.owner	= THIS_MODULE,
 		.of_match_table = of_match_ptr(vt8500_ehci_ids),
 	}
 };
 
+static int __init ehci_vt8500_init(void)
+{
+	if (usb_disabled())
+		return -ENODEV;
+
+	pr_info("%s: " DRIVER_DESC "\n", hcd_name);
+	ehci_init_driver(&vt8500_ehci_hc_driver, &ehci_vt8500_overrides);
+	return platform_driver_register(&vt8500_ehci_driver);
+}
+module_init(ehci_vt8500_init);
+
+static void __exit ehci_vt8500_cleanup(void)
+{
+	platform_driver_unregister(&vt8500_ehci_driver);
+}
+module_exit(ehci_vt8500_cleanup);
+
+MODULE_DESCRIPTION(DRIVER_DESC);
+MODULE_AUTHOR("Alexey Charkov");
+MODULE_LICENSE("GPL v2");
 MODULE_ALIAS("platform:vt8500-ehci");
 MODULE_DEVICE_TABLE(of, vt8500_ehci_ids);
-- 
1.8.1.2

[PATCH 2/2] USB: EHCI: make ehci-orion a separate driver

[Arnd Bergmann]

From: Manjunath Goudar <manjunath.goudar@linaro.org>

With the multiplatform changes in arm-soc tree, it becomes
possible to enable the mvebu platform (which uses
ehci-orion) at the same time as other platforms that require
a conflicting EHCI bus glue. At the moment, this results
in a warning like

drivers/usb/host/ehci-hcd.c:1297:0: warning: "PLATFORM_DRIVER" redefined [enabled by default]
drivers/usb/host/ehci-hcd.c:1277:0: note: this is the location of the previous definition
drivers/usb/host/ehci-orion.c:334:31: warning: 'ehci_orion_driver' defined but not used [-Wunused-variable]

and an ehci driver that only works on one of them.

With the infrastructure added by Alan Stern in patch 3e0232039
"USB: EHCI: prepare to make ehci-hcd a library module", we can
avoid this problem by turning a bus glue into a separate
module, as we do here for the orion bus glue.

Signed-off-by: Manjunath Goudar <manjunath.goudar@linaro.org>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Jason Cooper <jason@lakedaemon.net>
Cc: Andrew Lunn <andrew@lunn.ch>
---
 drivers/usb/host/Kconfig      |  8 ++++
 drivers/usb/host/Makefile     |  1 +
 drivers/usb/host/ehci-hcd.c   |  6 +--
 drivers/usb/host/ehci-orion.c | 90 ++++++++++++++++++++-----------------------
 4 files changed, 52 insertions(+), 53 deletions(-)

diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index d77e028..7ac6f48 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -162,6 +162,14 @@ config USB_EHCI_HCD_OMAP
 	  Enables support for the on-chip EHCI controller on
 	  OMAP3 and later chips.
 
+config USB_EHCI_HCD_ORION
+        tristate  "Support for Marvell Orion on-chip EHCI USB controller"
+        depends on USB_EHCI_HCD && PLAT_ORION
+        default y
+        ---help---
+          Enables support for the on-chip EHCI controller on
+          Morvell Orion chips.
+
 config USB_EHCI_HCD_VT8500
         tristate "Support for VT8500 on-chip EHCI USB controller"
         depends on USB_EHCI_HCD && ARCH_VT8500
diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
index b0da9cf..4db3f01 100644
--- a/drivers/usb/host/Makefile
+++ b/drivers/usb/host/Makefile
@@ -30,6 +30,7 @@ obj-$(CONFIG_USB_EHCI_MXC)	+= ehci-mxc.o
 
 obj-$(CONFIG_USB_OXU210HP_HCD)	+= oxu210hp-hcd.o
 obj-$(CONFIG_USB_EHCI_HCD_VT8500)+= ehci-vt8500.o
+obj-$(CONFIG_USB_EHCI_HCD_ORION)+= ehci-orion.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
diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index 487ebb8..26154f0 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -1272,11 +1272,6 @@ MODULE_LICENSE ("GPL");
 #define XILINX_OF_PLATFORM_DRIVER	ehci_hcd_xilinx_of_driver
 #endif
 
-#ifdef CONFIG_PLAT_ORION
-#include "ehci-orion.c"
-#define	PLATFORM_DRIVER		ehci_orion_driver
-#endif
-
 #ifdef CONFIG_USB_W90X900_EHCI
 #include "ehci-w90x900.c"
 #define	PLATFORM_DRIVER		ehci_hcd_w90x900_driver
@@ -1343,6 +1338,7 @@ MODULE_LICENSE ("GPL");
 	!IS_ENABLED(CONFIG_USB_EHCI_MXC) && \
 	!defined(PLATFORM_DRIVER) && \
 	!IS_ENABLED(CONFIG_ARCH_VT8500) && \
+	!IS_ENABLED(CONFIG_PLAT_ORION) && \
 	!defined(PS3_SYSTEM_BUS_DRIVER) && \
 	!defined(OF_PLATFORM_DRIVER) && \
 	!defined(XILINX_OF_PLATFORM_DRIVER)
diff --git a/drivers/usb/host/ehci-orion.c b/drivers/usb/host/ehci-orion.c
index 914a3ec..cfc4803 100644
--- a/drivers/usb/host/ehci-orion.c
+++ b/drivers/usb/host/ehci-orion.c
@@ -17,6 +17,13 @@
 #include <linux/of.h>
 #include <linux/of_device.h>
 #include <linux/of_irq.h>
+#include <linux/usb.h>
+#include <linux/usb/hcd.h>
+#include <linux/io.h>
+#include <linux/dma-mapping.h>
+
+#include "ehci.h"
+
 
 #define rdl(off)	__raw_readl(hcd->regs + (off))
 #define wrl(off, val)	__raw_writel((val), hcd->regs + (off))
@@ -34,6 +41,17 @@
 #define USB_PHY_IVREF_CTRL	0x440
 #define USB_PHY_TST_GRP_CTRL	0x450
 
+#define DRIVER_DESC "EHCI orion driver"
+
+static const char hcd_name[] = "ehci-orion";
+
+static struct hc_driver __read_mostly ehci_orion_hc_driver;
+
+static const struct ehci_driver_overrides orion_overrides __initdata = {
+	.reset = ehci_setup,
+};
+
+
 /*
  * Implement Orion USB controller specification guidelines
  */
@@ -104,51 +122,6 @@ static void orion_usb_phy_v1_setup(struct usb_hcd *hcd)
 	wrl(USB_MODE, 0x13);
 }
 
-static const struct hc_driver ehci_orion_hc_driver = {
-	.description = hcd_name,
-	.product_desc = "Marvell Orion EHCI",
-	.hcd_priv_size = sizeof(struct ehci_hcd),
-
-	/*
-	 * generic hardware linkage
-	 */
-	.irq = ehci_irq,
-	.flags = HCD_MEMORY | HCD_USB2,
-
-	/*
-	 * basic lifecycle operations
-	 */
-	.reset = ehci_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,
-	.relinquish_port = ehci_relinquish_port,
-	.port_handed_over = ehci_port_handed_over,
-
-	.clear_tt_buffer_complete = ehci_clear_tt_buffer_complete,
-};
-
 static void
 ehci_orion_conf_mbus_windows(struct usb_hcd *hcd,
 			     const struct mbus_dram_target_info *dram)
@@ -323,8 +296,6 @@ static int __exit ehci_orion_drv_remove(struct platform_device *pdev)
 	return 0;
 }
 
-MODULE_ALIAS("platform:orion-ehci");
-
 static const struct of_device_id ehci_orion_dt_ids[] = {
 	{ .compatible = "marvell,orion-ehci", },
 	{},
@@ -336,8 +307,31 @@ static struct platform_driver ehci_orion_driver = {
 	.remove		= __exit_p(ehci_orion_drv_remove),
 	.shutdown	= usb_hcd_platform_shutdown,
 	.driver = {
-		.name	= "orion-ehci",
+		.name	= hcd_name,
 		.owner  = THIS_MODULE,
 		.of_match_table = of_match_ptr(ehci_orion_dt_ids),
 	},
 };
+
+static int __init ehci_orion_init(void)
+{
+	if (usb_disabled())
+		return -ENODEV;
+
+	pr_info("%s: " DRIVER_DESC "\n", hcd_name);
+
+	ehci_init_driver(&ehci_orion_hc_driver, &orion_overrides);
+	return platform_driver_register(&ehci_orion_driver);
+}
+module_init(ehci_orion_init);
+
+static void __exit ehci_orion_cleanup(void)
+{
+	platform_driver_unregister(&ehci_orion_driver);
+}
+module_exit(ehci_orion_cleanup);
+
+MODULE_DESCRIPTION(DRIVER_DESC);
+MODULE_ALIAS("platform:ehci-orion");
+MODULE_AUTHOR("Tzachi Perelstein");
+MODULE_LICENSE("GPL");
-- 
1.8.1.2

[PATCH 2/2] USB: EHCI: make ehci-orion a separate driver

[Jason Cooper]

On Fri, Feb 15, 2013 at 11:12:29PM +0100, Arnd Bergmann wrote:
> From: Manjunath Goudar <manjunath.goudar@linaro.org>
> 
> With the multiplatform changes in arm-soc tree, it becomes
> possible to enable the mvebu platform (which uses
> ehci-orion) at the same time as other platforms that require
> a conflicting EHCI bus glue. At the moment, this results
> in a warning like
> 
> drivers/usb/host/ehci-hcd.c:1297:0: warning: "PLATFORM_DRIVER" redefined [enabled by default]
> drivers/usb/host/ehci-hcd.c:1277:0: note: this is the location of the previous definition
> drivers/usb/host/ehci-orion.c:334:31: warning: 'ehci_orion_driver' defined but not used [-Wunused-variable]
> 
> and an ehci driver that only works on one of them.
> 
> With the infrastructure added by Alan Stern in patch 3e0232039
> "USB: EHCI: prepare to make ehci-hcd a library module", we can
> avoid this problem by turning a bus glue into a separate
> module, as we do here for the orion bus glue.
> 
> Signed-off-by: Manjunath Goudar <manjunath.goudar@linaro.org>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Cc: Jason Cooper <jason@lakedaemon.net>
> Cc: Andrew Lunn <andrew@lunn.ch>
> ---
>  drivers/usb/host/Kconfig      |  8 ++++
>  drivers/usb/host/Makefile     |  1 +
>  drivers/usb/host/ehci-hcd.c   |  6 +--
>  drivers/usb/host/ehci-orion.c | 90 ++++++++++++++++++++-----------------------
>  4 files changed, 52 insertions(+), 53 deletions(-)

This looks sane to me, but I unfortunately don't have time to test atm.
So if Arnd has been working with you:

Acked-by: Jason Cooper <jason@lakedaemon.net>

thx,

Jason.

[PATCH 2/2] USB: EHCI: make ehci-orion a separate driver

[Andrew Lunn]

On Fri, Feb 15, 2013 at 11:12:29PM +0100, Arnd Bergmann wrote:
> From: Manjunath Goudar <manjunath.goudar@linaro.org>
> 
> With the multiplatform changes in arm-soc tree, it becomes
> possible to enable the mvebu platform (which uses
> ehci-orion) at the same time as other platforms that require
> a conflicting EHCI bus glue. At the moment, this results
> in a warning like
> 
> drivers/usb/host/ehci-hcd.c:1297:0: warning: "PLATFORM_DRIVER" redefined [enabled by default]
> drivers/usb/host/ehci-hcd.c:1277:0: note: this is the location of the previous definition
> drivers/usb/host/ehci-orion.c:334:31: warning: 'ehci_orion_driver' defined but not used [-Wunused-variable]
> 
> and an ehci driver that only works on one of them.
> 
> With the infrastructure added by Alan Stern in patch 3e0232039
> "USB: EHCI: prepare to make ehci-hcd a library module", we can
> avoid this problem by turning a bus glue into a separate
> module, as we do here for the orion bus glue.
> 
> Signed-off-by: Manjunath Goudar <manjunath.goudar@linaro.org>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Cc: Jason Cooper <jason@lakedaemon.net>
> Cc: Andrew Lunn <andrew@lunn.ch>
> ---
>  drivers/usb/host/Kconfig      |  8 ++++
>  drivers/usb/host/Makefile     |  1 +
>  drivers/usb/host/ehci-hcd.c   |  6 +--
>  drivers/usb/host/ehci-orion.c | 90 ++++++++++++++++++++-----------------------
>  4 files changed, 52 insertions(+), 53 deletions(-)

Tested-by: Andrew Lunn <andrew@lunn.ch>

I booted with this patch on a Kirkwood based QNAP NAS box. The USB bus
enumerated as expected.

	   Andrew

[PATCH 2/2] USB: EHCI: make ehci-orion a separate driver

[Ezequiel Garcia]

Hi Arnd,

On Fri, Feb 15, 2013 at 11:12:29PM +0100, Arnd Bergmann wrote:
> From: Manjunath Goudar <manjunath.goudar@linaro.org>
> 
> With the multiplatform changes in arm-soc tree, it becomes
> possible to enable the mvebu platform (which uses
> ehci-orion) at the same time as other platforms that require
> a conflicting EHCI bus glue. At the moment, this results
> in a warning like
> 
> drivers/usb/host/ehci-hcd.c:1297:0: warning: "PLATFORM_DRIVER" redefined [enabled by default]
> drivers/usb/host/ehci-hcd.c:1277:0: note: this is the location of the previous definition
> drivers/usb/host/ehci-orion.c:334:31: warning: 'ehci_orion_driver' defined but not used [-Wunused-variable]
> 
> and an ehci driver that only works on one of them.
> 
> With the infrastructure added by Alan Stern in patch 3e0232039
> "USB: EHCI: prepare to make ehci-hcd a library module", we can
> avoid this problem by turning a bus glue into a separate
> module, as we do here for the orion bus glue.
> 
> Signed-off-by: Manjunath Goudar <manjunath.goudar@linaro.org>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Cc: Jason Cooper <jason@lakedaemon.net>
> Cc: Andrew Lunn <andrew@lunn.ch>
> ---
>  drivers/usb/host/Kconfig      |  8 ++++
>  drivers/usb/host/Makefile     |  1 +
>  drivers/usb/host/ehci-hcd.c   |  6 +--
>  drivers/usb/host/ehci-orion.c | 90 ++++++++++++++++++++-----------------------
>  4 files changed, 52 insertions(+), 53 deletions(-)
> 
> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
> index d77e028..7ac6f48 100644
> --- a/drivers/usb/host/Kconfig
> +++ b/drivers/usb/host/Kconfig
> @@ -162,6 +162,14 @@ config USB_EHCI_HCD_OMAP
>  	  Enables support for the on-chip EHCI controller on
>  	  OMAP3 and later chips.
>  
> +config USB_EHCI_HCD_ORION
> +        tristate  "Support for Marvell Orion on-chip EHCI USB controller"
> +        depends on USB_EHCI_HCD && PLAT_ORION
> +        default y
> +        ---help---
> +          Enables support for the on-chip EHCI controller on
> +          Morvell Orion chips.

s/Morvell/Marvell

Just this tiny nitpick.

Thanks,

-- 
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

[PATCH] USB: update host controller Kconfig entries

[Arnd Bergmann]

On Saturday 16 February 2013, Ezequiel Garcia wrote:
> 
> s/Morvell/Marvell
> 
> Just this tiny nitpick.

Thanks for pointing this out! Tony Prisk had another comment
about the VT8500 patch, so I figured it would be good to
update both here, and I found a few more outdated changeset
comments about drivers that have become used on more platforms
now. Greg, please apply to whichever release you see fit. This
is based on your current usb-next branch.

	Arnd

8<-----------
Subject: USB: update host controller Kconfig entries

The recent patches from Manjunath Goudar introduced two small
mistakes in the Kconfig help text for the new options. Let's
fix those and the other entries that have become stale over time.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/usb/host/Kconfig | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index 7ac6f48..245da35 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -163,23 +163,28 @@ config USB_EHCI_HCD_OMAP
 	  OMAP3 and later chips.
 
 config USB_EHCI_HCD_ORION
-        tristate  "Support for Marvell Orion on-chip EHCI USB controller"
+        tristate  "Support for Marvell EBU on-chip EHCI USB controller"
         depends on USB_EHCI_HCD && PLAT_ORION
         default y
         ---help---
           Enables support for the on-chip EHCI controller on
-          Morvell Orion chips.
+          Marvell's embedded ARM SoCs, including Orion,
+	  Kirkwood, Dove, Armada XP, Armada 370.
+	  This is different from the EHCI implementation on
+	  Marvell's mobile PXA and MMP SoC, see USB_EHCI_MV
+	  for those.
+	
 
 config USB_EHCI_HCD_VT8500
-        tristate "Support for VT8500 on-chip EHCI USB controller"
+        tristate "Support for VIA/Wondermedia on-chip EHCI USB controller"
         depends on USB_EHCI_HCD && ARCH_VT8500
         default y
         ---help---
           Enables support for the on-chip EHCI controller on
-          VT8500 chips.
+          VIA VT8500 and Wondermedia WM8x50 chips.
 
 config USB_EHCI_MSM
-	bool "Support for MSM on-chip EHCI USB controller"
+	bool "Support for Qualcomm QSD/MSM on-chip EHCI USB controller"
 	depends on USB_EHCI_HCD && ARCH_MSM
 	select USB_EHCI_ROOT_HUB_TT
 	select USB_MSM_OTG
@@ -215,10 +220,11 @@ config USB_EHCI_SH
 	  If you use the PCI EHCI controller, this option is not necessary.
 
 config USB_EHCI_S5P
-       boolean "S5P EHCI support"
-       depends on USB_EHCI_HCD && PLAT_S5P
-       help
-	 Enable support for the S5P SOC's on-chip EHCI controller.
+	boolean "EHCI support for Samsung S5P/EXYNOS SoC Series"
+	depends on USB_EHCI_HCD && PLAT_S5P
+	help
+	 Enable support for the Samsung S5Pxxxx and Exynos3/4/5 SOC's
+	 on-chip EHCI controller.
 
 config USB_EHCI_MV
 	bool "EHCI support for Marvell on-chip controller"
@@ -228,6 +234,8 @@ config USB_EHCI_MV
 	  Enables support for Marvell (including PXA and MMP series) on-chip
 	  USB SPH and OTG controller. SPH is a single port host, and it can
 	  only be EHCI host. OTG is controller that can switch to host mode.
+	  Note that there is a separate driver for Marvell's embedded ARM
+	  SoCs, see USB_EHCI_HCD_ORION for those.
 
 config USB_W90X900_EHCI
 	bool "W90X900(W90P910) EHCI support"

[PATCH 2/2] USB: EHCI: make ehci-orion a separate driver

[Alan Stern]

On Fri, 15 Feb 2013, Arnd Bergmann wrote:

> From: Manjunath Goudar <manjunath.goudar@linaro.org>
> 
> With the multiplatform changes in arm-soc tree, it becomes
> possible to enable the mvebu platform (which uses
> ehci-orion) at the same time as other platforms that require
> a conflicting EHCI bus glue. At the moment, this results
> in a warning like
> 
> drivers/usb/host/ehci-hcd.c:1297:0: warning: "PLATFORM_DRIVER" redefined [enabled by default]
> drivers/usb/host/ehci-hcd.c:1277:0: note: this is the location of the previous definition
> drivers/usb/host/ehci-orion.c:334:31: warning: 'ehci_orion_driver' defined but not used [-Wunused-variable]
> 
> and an ehci driver that only works on one of them.
> 
> With the infrastructure added by Alan Stern in patch 3e0232039
> "USB: EHCI: prepare to make ehci-hcd a library module", we can
> avoid this problem by turning a bus glue into a separate
> module, as we do here for the orion bus glue.

> --- a/drivers/usb/host/Makefile
> +++ b/drivers/usb/host/Makefile
> @@ -30,6 +30,7 @@ obj-$(CONFIG_USB_EHCI_MXC)	+= ehci-mxc.o
>  
>  obj-$(CONFIG_USB_OXU210HP_HCD)	+= oxu210hp-hcd.o
>  obj-$(CONFIG_USB_EHCI_HCD_VT8500)+= ehci-vt8500.o
> +obj-$(CONFIG_USB_EHCI_HCD_ORION)+= ehci-orion.o

Both of these two new lines should be formatted like the other lines in
this file (i.e., with tabs at the corresponding places), and they
should come before the OXU210HP_HCD entry so that they are next to the
other EHCI-related lines.

> --- a/drivers/usb/host/ehci-orion.c
> +++ b/drivers/usb/host/ehci-orion.c
> @@ -17,6 +17,13 @@
>  #include <linux/of.h>
>  #include <linux/of_device.h>
>  #include <linux/of_irq.h>
> +#include <linux/usb.h>
> +#include <linux/usb/hcd.h>
> +#include <linux/io.h>
> +#include <linux/dma-mapping.h>

Is this line really needed?

> @@ -34,6 +41,17 @@
>  #define USB_PHY_IVREF_CTRL	0x440
>  #define USB_PHY_TST_GRP_CTRL	0x450
>  
> +#define DRIVER_DESC "EHCI orion driver"
> +
> +static const char hcd_name[] = "ehci-orion";
> +
> +static struct hc_driver __read_mostly ehci_orion_hc_driver;
> +
> +static const struct ehci_driver_overrides orion_overrides __initdata = {
> +	.reset = ehci_setup,
> +};

This is not necessary; ehci_setup is the default value anyway.  This 
structure can be omitted.

> @@ -323,8 +296,6 @@ static int __exit ehci_orion_drv_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> -MODULE_ALIAS("platform:orion-ehci");
> -
>  static const struct of_device_id ehci_orion_dt_ids[] = {
>  	{ .compatible = "marvell,orion-ehci", },
>  	{},
> @@ -336,8 +307,31 @@ static struct platform_driver ehci_orion_driver = {
>  	.remove		= __exit_p(ehci_orion_drv_remove),
>  	.shutdown	= usb_hcd_platform_shutdown,
>  	.driver = {
> -		.name	= "orion-ehci",
> +		.name	= hcd_name,

Is this really what you want -- changing the driver name from 
"orion-ehci" to "ehci-orion"?  Is that liable to cause trouble?

> +MODULE_DESCRIPTION(DRIVER_DESC);
> +MODULE_ALIAS("platform:ehci-orion");

And is this really what you want -- changing the alias from 
"platform:orion-ehci" to "platform:ehci-orion"?

Alan Stern

[PATCH 2/2] USB: EHCI: make ehci-orion a separate driver

[Andrew Lunn]

On Mon, Feb 18, 2013 at 05:34:35PM -0500, Alan Stern wrote:
> On Fri, 15 Feb 2013, Arnd Bergmann wrote:
> 
> > From: Manjunath Goudar <manjunath.goudar@linaro.org>
> > 
> > With the multiplatform changes in arm-soc tree, it becomes
> > possible to enable the mvebu platform (which uses
> > ehci-orion) at the same time as other platforms that require
> > a conflicting EHCI bus glue. At the moment, this results
> > in a warning like
> > 
> > drivers/usb/host/ehci-hcd.c:1297:0: warning: "PLATFORM_DRIVER" redefined [enabled by default]
> > drivers/usb/host/ehci-hcd.c:1277:0: note: this is the location of the previous definition
> > drivers/usb/host/ehci-orion.c:334:31: warning: 'ehci_orion_driver' defined but not used [-Wunused-variable]
> > 
> > and an ehci driver that only works on one of them.
> > 
> > With the infrastructure added by Alan Stern in patch 3e0232039
> > "USB: EHCI: prepare to make ehci-hcd a library module", we can
> > avoid this problem by turning a bus glue into a separate
> > module, as we do here for the orion bus glue.
> 
> > --- a/drivers/usb/host/Makefile
> > +++ b/drivers/usb/host/Makefile
> > @@ -30,6 +30,7 @@ obj-$(CONFIG_USB_EHCI_MXC)	+= ehci-mxc.o
> >  
> >  obj-$(CONFIG_USB_OXU210HP_HCD)	+= oxu210hp-hcd.o
> >  obj-$(CONFIG_USB_EHCI_HCD_VT8500)+= ehci-vt8500.o
> > +obj-$(CONFIG_USB_EHCI_HCD_ORION)+= ehci-orion.o
> 
> Both of these two new lines should be formatted like the other lines in
> this file (i.e., with tabs at the corresponding places), and they
> should come before the OXU210HP_HCD entry so that they are next to the
> other EHCI-related lines.
> 
> > --- a/drivers/usb/host/ehci-orion.c
> > +++ b/drivers/usb/host/ehci-orion.c
> > @@ -17,6 +17,13 @@
> >  #include <linux/of.h>
> >  #include <linux/of_device.h>
> >  #include <linux/of_irq.h>
> > +#include <linux/usb.h>
> > +#include <linux/usb/hcd.h>
> > +#include <linux/io.h>
> > +#include <linux/dma-mapping.h>
> 
> Is this line really needed?
> 
> > @@ -34,6 +41,17 @@
> >  #define USB_PHY_IVREF_CTRL	0x440
> >  #define USB_PHY_TST_GRP_CTRL	0x450
> >  
> > +#define DRIVER_DESC "EHCI orion driver"
> > +
> > +static const char hcd_name[] = "ehci-orion";
> > +
> > +static struct hc_driver __read_mostly ehci_orion_hc_driver;
> > +
> > +static const struct ehci_driver_overrides orion_overrides __initdata = {
> > +	.reset = ehci_setup,
> > +};
> 
> This is not necessary; ehci_setup is the default value anyway.  This 
> structure can be omitted.
> 
> > @@ -323,8 +296,6 @@ static int __exit ehci_orion_drv_remove(struct platform_device *pdev)
> >  	return 0;
> >  }
> >  
> > -MODULE_ALIAS("platform:orion-ehci");
> > -
> >  static const struct of_device_id ehci_orion_dt_ids[] = {
> >  	{ .compatible = "marvell,orion-ehci", },
> >  	{},
> > @@ -336,8 +307,31 @@ static struct platform_driver ehci_orion_driver = {
> >  	.remove		= __exit_p(ehci_orion_drv_remove),
> >  	.shutdown	= usb_hcd_platform_shutdown,
> >  	.driver = {
> > -		.name	= "orion-ehci",
> > +		.name	= hcd_name,
> 
> Is this really what you want -- changing the driver name from 
> "orion-ehci" to "ehci-orion"?  Is that liable to cause trouble?
> 
> > +MODULE_DESCRIPTION(DRIVER_DESC);
> > +MODULE_ALIAS("platform:ehci-orion");
> 
> And is this really what you want -- changing the alias from 
> "platform:orion-ehci" to "platform:ehci-orion"?

Hi Manjunath

I can confirm that this breaks non DT based kirkwood systems. The
driver does not get loaded.

Sorry for not testing and finding this case earlier, i just tested a
DT based system.

GregKH: Please can you drop this patch from usb-next. It breaks more
than it fixes.

   Andrew

[PATCH 2/2] USB: EHCI: make ehci-orion a separate driver

[Arnaud Patard (Rtp)]

Andrew Lunn <andrew@lunn.ch> writes:
Hi,

[...]

>> > +
>> > +static const char hcd_name[] = "ehci-orion";

[...]

>> >  }
>> >  
>> > -MODULE_ALIAS("platform:orion-ehci");
>> > -
>> >  static const struct of_device_id ehci_orion_dt_ids[] = {
>> >  	{ .compatible = "marvell,orion-ehci", },

orion-ehci here ...

>> >  	{},
>> > @@ -336,8 +307,31 @@ static struct platform_driver ehci_orion_driver = {
>> >  	.remove		= __exit_p(ehci_orion_drv_remove),
>> >  	.shutdown	= usb_hcd_platform_shutdown,
>> >  	.driver = {
>> > -		.name	= "orion-ehci",
>> > +		.name	= hcd_name,

... and ehci-orion here. This would explain why only DT case seems to
work. I'm wondering why it has not been changed given that it has been
changed everywhere else, breaking stuff.

>> 
>> Is this really what you want -- changing the driver name from 
>> "orion-ehci" to "ehci-orion"?  Is that liable to cause trouble?
>> 
>> > +MODULE_DESCRIPTION(DRIVER_DESC);
>> > +MODULE_ALIAS("platform:ehci-orion");
>> 
>> And is this really what you want -- changing the alias from 
>> "platform:orion-ehci" to "platform:ehci-orion"?
>
> Hi Manjunath
>
> I can confirm that this breaks non DT based kirkwood systems. The
> driver does not get loaded.
>
> Sorry for not testing and finding this case earlier, i just tested a
> DT based system.

Maybe doing a mass renaming from orion-ehci to ehci-orion in
arch/arm/* files would be enough ? [ well, not necessary everywhere, I'm
not sure if changing the clock name in mach-kirkwood/board-dt.c would be
a good idea given the of node didn't change its name ]

Arnaud

[PATCH 2/2] USB: EHCI: make ehci-orion a separate driver

[Greg KH]

On Tue, Feb 19, 2013 at 08:24:38AM +0100, Andrew Lunn wrote:
> 
> GregKH: Please can you drop this patch from usb-next. It breaks more
> than it fixes.

I've now reverted both of these, and the follow-on patch that Arnd sent
in.

Arnd, sorry, but please be more careful, especially when you ask for
patches to be rushed into the tree...

greg k-h

[PATCH 2/2] USB: EHCI: make ehci-orion a separate driver

[Arnd Bergmann]

On Wednesday 20 February 2013, Greg KH wrote:
> On Tue, Feb 19, 2013 at 08:24:38AM +0100, Andrew Lunn wrote:
> > 
> > GregKH: Please can you drop this patch from usb-next. It breaks more
> > than it fixes.
> 
> I've now reverted both of these, and the follow-on patch that Arnd sent
> in.
> 
> Arnd, sorry, but please be more careful, especially when you ask for
> patches to be rushed into the tree...

Yes, I understand. I'll try to come up with a less invasive way to
deal with the build error in 3.9 once the ARM multiplatform branch
gets pulled.

I guess we could either forbid the combinations that are now broken
using Kconfig dependencies, or add just add separate registriation
paths to ehci_hcd_init() and ehci_hcd_cleanup() as we have for
powerpc, although Alan did not like that too much last time I
suggested it.

	Arnd

[PATCH 2/2] USB: EHCI: make ehci-orion a separate driver

[Arnd Bergmann]

On Tuesday 19 February 2013, Manjunath Goudar wrote:
> > This is not necessary; ehci_setup is the default value anyway.  This
> > structure can be omitted.
> >
> 
> ehci_init_driver function we are passing orion_overrides,if we are removing
> above struct initialization, what should we need to pass in
> the  ehci_init_driver function.
> 

ehci_init_driver already checks if you pass an override in there, and does
not override anything in that case, so you can simply pass NULL as the
argument.

	Arnd

[PATCH 2/2] USB: EHCI: make ehci-orion a separate driver

[Alan Stern]

On Tue, 19 Feb 2013, Arnd Bergmann wrote:

> On Tuesday 19 February 2013, Manjunath Goudar wrote:
> > > This is not necessary; ehci_setup is the default value anyway.  This
> > > structure can be omitted.
> > >
> > 
> > ehci_init_driver function we are passing orion_overrides,if we are removing
> > above struct initialization, what should we need to pass in
> > the  ehci_init_driver function.
> > 
> 
> ehci_init_driver already checks if you pass an override in there, and does
> not override anything in that case, so you can simply pass NULL as the
> argument.

That's right.  Manjunath, you can answer at lot of questions like this 
for yourself very easily, simply by reading the source code.

Alan Stern

[PATCH 2/2] USB: EHCI: make ehci-orion a separate driver

[Alan Stern]

On Fri, 15 Feb 2013, Arnd Bergmann wrote:

> From: Manjunath Goudar <manjunath.goudar@linaro.org>
> 
> With the multiplatform changes in arm-soc tree, it becomes
> possible to enable the mvebu platform (which uses
> ehci-orion) at the same time as other platforms that require
> a conflicting EHCI bus glue. At the moment, this results
> in a warning like
> 
> drivers/usb/host/ehci-hcd.c:1297:0: warning: "PLATFORM_DRIVER" redefined [enabled by default]
> drivers/usb/host/ehci-hcd.c:1277:0: note: this is the location of the previous definition
> drivers/usb/host/ehci-orion.c:334:31: warning: 'ehci_orion_driver' defined but not used [-Wunused-variable]
> 
> and an ehci driver that only works on one of them.
> 
> With the infrastructure added by Alan Stern in patch 3e0232039
> "USB: EHCI: prepare to make ehci-hcd a library module", we can
> avoid this problem by turning a bus glue into a separate
> module, as we do here for the orion bus glue.

One more comment on this patch...

> --- a/drivers/usb/host/Kconfig
> +++ b/drivers/usb/host/Kconfig
> @@ -162,6 +162,14 @@ config USB_EHCI_HCD_OMAP
>  	  Enables support for the on-chip EHCI controller on
>  	  OMAP3 and later chips.
>  
> +config USB_EHCI_HCD_ORION
> +        tristate  "Support for Marvell Orion on-chip EHCI USB controller"
> +        depends on USB_EHCI_HCD && PLAT_ORION
> +        default y
> +        ---help---
> +          Enables support for the on-chip EHCI controller on
> +          Morvell Orion chips.

Currently there is no Kconfig option to control specifically whether
the ehci-orion driver gets built; it always gets built whenever
CONFIG_PLAT_ORION and CONFIG_USB_EHCI_HCD are both enabled.

Do you think it is a good idea to add an option for this?  Should it at
least be non-interactive, so that the driver always gets built under
the same conditions as currently?  A later patch can make it 
interactive, if desired.

Alan Stern

[PATCH 2/2] USB: EHCI: make ehci-orion a separate driver

[Arnd Bergmann]

On Wednesday 20 February 2013, Alan Stern wrote:
> Currently there is no Kconfig option to control specifically whether
> the ehci-orion driver gets built; it always gets built whenever
> CONFIG_PLAT_ORION and CONFIG_USB_EHCI_HCD are both enabled.
> 
> Do you think it is a good idea to add an option for this?  Should it at
> least be non-interactive, so that the driver always gets built under
> the same conditions as currently?  A later patch can make it 
> interactive, if desired.

I think it's good to have it interactive, because we are now building for
multiplatform with Orion being one of many. It's useful to be able to
build EHCI_HCD=y AND EHCD_HCD_ORION=m.

However, you are right that this is a change that was not mentioned in
the patch description and would better have been kept separate or
at least explicitly spelled out.

	Arnd

[PATCH 2/2] USB: EHCI: make ehci-orion a separate driver

[Jason Cooper]

Alan,

On Wed, Feb 20, 2013 at 04:27:04PM +0000, Arnd Bergmann wrote:
> On Wednesday 20 February 2013, Alan Stern wrote:
> > Currently there is no Kconfig option to control specifically whether
> > the ehci-orion driver gets built; it always gets built whenever
> > CONFIG_PLAT_ORION and CONFIG_USB_EHCI_HCD are both enabled.
> > 
> > Do you think it is a good idea to add an option for this?  Should it at
> > least be non-interactive, so that the driver always gets built under
> > the same conditions as currently?  A later patch can make it 
> > interactive, if desired.
> 
> I think it's good to have it interactive, because we are now building for
> multiplatform with Orion being one of many. It's useful to be able to
> build EHCI_HCD=y AND EHCD_HCD_ORION=m.

Yes, explicit is preferable.  On kirkwood usb has it's own gateable
clock.  The user should be able to conserve power by unloading the
module or building without it.

thx,

Jason.

[PATCH 1/2] USB: EHCI: make ehci-vt8500 a separate driver

[Tony Prisk]

On Fri, 2013-02-15 at 23:12 +0100, Arnd Bergmann wrote:
> From: Manjunath Goudar <manjunath.goudar@linaro.org>
> 
> With the multiplatform changes in arm-soc tree, it becomes
> possible to enable the vt8500 platform at the same time
> as other platforms that require a conflicting EHCI bus
> glue. At the moment, this results in a warning like
> 
> drivers/usb/host/ehci-hcd.c:1277:0: warning: "PLATFORM_DRIVER" redefined [enabled by default]
> drivers/usb/host/ehci-hcd.c:1257:0: note: this is the location of the previous definition
> drivers/usb/host/ehci-omap.c:319:31: warning: 'ehci_hcd_omap_driver' defined but not used [-Wunused-variable]
> 
> and an ehci driver that only works on one of them.
> 
> With the infrastructure added by Alan Stern in patch 3e0232039
> "USB: EHCI: prepare to make ehci-hcd a library module", we can
> avoid this problem by turning a bus glue into a separate
> module, as we do here for the vt8500 bus glue.
> 
> Signed-off-by: Manjunath Goudar <manjunath.goudar@linaro.org>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Cc: Tony Prisk <linux@prisktech.co.nz>
> Cc: Alexey Charkov <alchark@gmail.com>
> ---
>  drivers/usb/host/Kconfig       |  8 +++++
>  drivers/usb/host/Makefile      |  1 +
>  drivers/usb/host/ehci-hcd.c    |  6 +---
>  drivers/usb/host/ehci-vt8500.c | 73 +++++++++++++++++++-----------------------
>  4 files changed, 43 insertions(+), 45 deletions(-)
> 
> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
> index c59a112..d77e028 100644
> --- a/drivers/usb/host/Kconfig
> +++ b/drivers/usb/host/Kconfig
> @@ -162,6 +162,14 @@ config USB_EHCI_HCD_OMAP
>  	  Enables support for the on-chip EHCI controller on
>  	  OMAP3 and later chips.
>  
> +config USB_EHCI_HCD_VT8500
> +        tristate "Support for VT8500 on-chip EHCI USB controller"
> +        depends on USB_EHCI_HCD && ARCH_VT8500
> +        default y
> +        ---help---
> +          Enables support for the on-chip EHCI controller on
> +          VT8500 chips.
> +

This should also mention Wondermedia SoCs I guess since the same driver
is used for VT8500, WM8505, WM8650 etc etc.

Other than that:

Acked-by: Tony Prisk <linux@prisktech.co.nz>

[V2 0/8] usb: ehci: more bus glues as separate modules

[Ezequiel Garcia]

Hi Manjunath,

Nice job!

On Fri, Feb 15, 2013 at 03:54:05PM +0530, Manjunath Goudar wrote:
> Separate the SOC On-Chip host controller driver from ehci-hcd
> host code into its own driver module
> 
> V2:
> Modified the patches, based on the first version review comments  
> 

If at all possible, I'd still like to see a better commit message
explaining why we need this at all, as it has been previously requested.

I guess there has been some discussion about this rework somewhere.
It would be nice to add a link to that discussion in the commit message
or in the cover letter so reviewers can have better context.


> Manjunath Goudar (8):
>   USB: EHCI: make ehci-spear a separate driver
>   USB: EHCI: make ehci-atmel a separate driver
>   USB: EHCI: make ehci-s5p a separate driver
>   USB: EHCI: make ehci-mv as separate static driver
>   USB: EHCI: make ehci-vt8500 a separate driver
>   USB: EHCI: make ehci-msm a separate driver
>   USB: EHCI: make ehci-w90X900 a separate driver
>   USB: EHCI: make ehci-orion a separate driver
> 
>  drivers/usb/host/Kconfig        |   40 +++++++++++++++--
>  drivers/usb/host/Makefile       |    9 +++-
>  drivers/usb/host/ehci-atmel.c   |   76 ++++++++++++++++----------------
>  drivers/usb/host/ehci-hcd.c     |   52 +++++-----------------
>  drivers/usb/host/ehci-msm.c     |   85 ++++++++++++++++--------------------
>  drivers/usb/host/ehci-mv.c      |   78 +++++++++++++++------------------
>  drivers/usb/host/ehci-orion.c   |   90 ++++++++++++++++++--------------------
>  drivers/usb/host/ehci-s5p.c     |   67 +++++++++++++++-------------
>  drivers/usb/host/ehci-spear.c   |   77 +++++++++++++++++----------------
>  drivers/usb/host/ehci-vt8500.c  |   73 ++++++++++++++-----------------
>  drivers/usb/host/ehci-w90x900.c |   91 +++++++++++++++++----------------------
>  drivers/usb/host/ehci.h         |    2 +-
>  12 files changed, 358 insertions(+), 382 deletions(-)
> 
> -- 
> 1.7.9.5
> 

Thanks a lot,

-- 
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

[V2 2/8] USB: EHCI: make ehci-atmel a separate driver

[Bo Shen]

On 02/15/2013 06:24 PM, Manjunath Goudar wrote:
> Separate the Atmel host controller driver from ehci-hcd host code
> into its own driver module.
>
> In V2:
> Resolved below compiler error.
> drivers/usb/host/ehci-atmel.c: In function 'ehci_atmel_drv_remove':
> drivers/usb/host/ehci-atmel.c:167: error: implicit declaration of function 'ehci_shutdown'
>
> Signed-off-by: Manjunath Goudar<manjunath.goudar@linaro.org>
> Cc: Alan Stern<stern@rowland.harvard.edu>
> Cc: Greg KH<greg@kroah.com>
> Cc: Grant Likely<grant.likely@secretlab.ca>
> Cc: Rob Herring<rob.herring@calxeda.com>
> Cc: Andrew Victor<linux@maxim.org.za>
> Cc: Nicolas Ferre<nicolas.ferre@atmel.com>
> Cc: Jean-Christophe Plagniol-Villard<plagnioj@jcrosoft.com>
> Cc: linux-usb at vger.kernel.org
> Cc: linux-kernel at vger.kernel.org
> ---
>   drivers/usb/host/Kconfig      |    8 +++++
>   drivers/usb/host/Makefile     |    1 +
>   drivers/usb/host/ehci-atmel.c |   76 ++++++++++++++++++++++-------------------
>   drivers/usb/host/ehci-hcd.c   |   10 ++----
>   drivers/usb/host/ehci.h       |    2 +-
>   5 files changed, 53 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
> index 15e8032..1ef37d7 100644
> --- a/drivers/usb/host/Kconfig
> +++ b/drivers/usb/host/Kconfig
> @@ -170,6 +170,14 @@ config USB_EHCI_HCD_SPEAR
>             Enables support for the on-chip EHCI controller on
>             ST SPEAr chips.
>
> +config USB_EHCI_HCD_AT91
> +        tristate  "Support for Atmel on-chip EHCI USB controller"
> +        depends on USB_EHCI_HCD&&  ARCH_AT91
> +        default y
> +        ---help---
> +          Enables support for the on-chip EHCI controller on
> +          Atmel chips.
> +
>   config USB_EHCI_MSM
>   	bool "Support for MSM on-chip EHCI USB controller"
>   	depends on USB_EHCI_HCD&&  ARCH_MSM
> diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
> index c8fcde9..b301243 100644
> --- a/drivers/usb/host/Makefile
> +++ b/drivers/usb/host/Makefile
> @@ -29,6 +29,7 @@ obj-$(CONFIG_USB_EHCI_HCD_PLATFORM)	+= ehci-platform.o
>   obj-$(CONFIG_USB_EHCI_MXC)	+= ehci-mxc.o
>   obj-$(CONFIG_USB_EHCI_HCD_SPEAR)+= ehci-spear.o
>   obj-$(CONFIG_USB_OXU210HP_HCD)	+= oxu210hp-hcd.o
> +obj-$(CONFIG_USB_EHCI_HCD_AT91) += ehci-atmel.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
> diff --git a/drivers/usb/host/ehci-atmel.c b/drivers/usb/host/ehci-atmel.c
> index 27639487..3cd0dd4 100644
> --- a/drivers/usb/host/ehci-atmel.c
> +++ b/drivers/usb/host/ehci-atmel.c
> @@ -15,6 +15,19 @@
>   #include<linux/platform_device.h>
>   #include<linux/of.h>
>   #include<linux/of_platform.h>
> +#include<linux/kernel.h>
> +#include<linux/module.h>
> +#include<linux/usb.h>
> +#include<linux/usb/hcd.h>
> +#include<linux/io.h>
> +#include<linux/dma-mapping.h>
> +
> +#include "ehci.h"
> +
> +#define DRIVER_DESC "EHCI atmel driver"
> +
> +static const char hcd_name[] = "ehci-atmel";
> +static struct hc_driver __read_mostly ehci_atmel_hc_driver;
>
>   /* interface and function clocks */
>   static struct clk *iclk, *fclk;
> @@ -60,41 +73,6 @@ static int ehci_atmel_setup(struct usb_hcd *hcd)
>   	return ehci_setup(hcd);
>   }
>
> -static const struct hc_driver ehci_atmel_hc_driver = {
> -	.description		= hcd_name,
> -	.product_desc		= "Atmel EHCI UHP HS",
> -	.hcd_priv_size		= sizeof(struct ehci_hcd),
> -
> -	/* generic hardware linkage */
> -	.irq			= ehci_irq,
> -	.flags			= HCD_MEMORY | HCD_USB2,
> -
> -	/* basic lifecycle operations */
> -	.reset			= ehci_atmel_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,
> -	.relinquish_port	= ehci_relinquish_port,
> -	.port_handed_over	= ehci_port_handed_over,
> -
> -	.clear_tt_buffer_complete	= ehci_clear_tt_buffer_complete,
> -};
> -
>   static u64 at91_ehci_dma_mask = DMA_BIT_MASK(32);
>
>   static int ehci_atmel_drv_probe(struct platform_device *pdev)
> @@ -210,7 +188,33 @@ static struct platform_driver ehci_atmel_driver = {
>   	.remove		= ehci_atmel_drv_remove,
>   	.shutdown	= usb_hcd_platform_shutdown,
>   	.driver		= {
> -		.name	= "atmel-ehci",
> +		.name	= hcd_name,

This change will cause atmel ehci won't work with the none device tree 
kernel.

it can be fixed with replace other places using "atmel-ehci" with 
<hcd_name>, that means replace "atmel-ehci" with "ehci-atmel" at other 
places.

Best Regards,
Bo Shen

[V2 2/8] USB: EHCI: make ehci-atmel a separate driver

[Arnd Bergmann]

On Monday 18 February 2013, Bo Shen wrote:
> > -             .name   = "atmel-ehci",
> > +             .name   = hcd_name,
> 
> This change will cause atmel ehci won't work with the none device tree 
> kernel.
> 
> it can be fixed with replace other places using "atmel-ehci" with 
> <hcd_name>, that means replace "atmel-ehci" with "ehci-atmel" at other 
> places.

Ah, that is right. Thanks for pointing it out!

Manjunath, please revert this change and ensure you are not changing
the contents of the 'platform_driver->name' field in any of the drivers.

	Arnd