2013-03-28 21:55:13

by Arnd Bergmann

[permalink] [raw]
Subject: [PATCH v3 0/7] USB EHCI multiplatform series again

Hi Alan,

These are the current patches from Manjunath, after I helped him address
the remaining review comments and found a few more in the process.

Unfortunately, Manjunath is currently on vacation and I will be away for
the next couple of days when he returns, so I took the liberty to send
the patches myself to have a chance of getting them merged in time for
3.10.

The patches are all based on today's usb-next branch, which includes
my earlier patch to remove the vt8500 backend. They are sorted by
priority: the first one obviously should have been in 3.9 but didn't
make it. The second one will be needed in 3.10, and the third one
quite likely as well. The atmel and msm patches can wait for 3.11
since we don't plan to have multiplatform support for those SoCs
in 3.10. Manjunath also did patches for the tegra, mv and w90X900
back-ends, but there are still known bugs in them so we don't submit
them yet. Tegra might become multiplatform in 3.10, but that will
still work as long as it's the only platform defining the
PLATFORM_DRIVER macro.

Please review the latest version so we can get at least the first
three patches merged for 3.10, or more if you are happy with them.
I'm not sure what to do about OHCI, the last patch in this series
is the best I could think of, given that nobody has worked on a
proper series.

Nicolas and David: the at91 and msm patches have no maintainer
Ack so far and are build-tested only. Please let us know if you
want to get them merged for 3.10, or if we should drop them for
now and let you pick them up when you get around to adding
multiplatform support for your SoCs. The at91 patch requires
"USB: EHCI: export ehci_shutdown", which will also be needed
for the upcoming Tegra patch.

Arnd

Arnd Bergmann (2):
USB: EHCI: export ehci_shutdown
USB: OHCI: avoid conflicting platform drivers

Manjunath Goudar (5):
USB: EHCI: make ehci-orion a separate driver
USB: EHCI: make ehci-spear a separate driver
USB: EHCI: make ehci-s5p a separate driver
USB: EHCI: make ehci-atmel a separate driver
USB: EHCI: make ehci-msm a separate driver

drivers/usb/host/Kconfig | 33 ++++++++-
drivers/usb/host/Makefile | 5 ++
drivers/usb/host/ehci-atmel.c | 85 +++++++++++------------
drivers/usb/host/ehci-hcd.c | 33 ++-------
drivers/usb/host/ehci-msm.c | 85 ++++++++++-------------
drivers/usb/host/ehci-orion.c | 82 ++++++++++------------
drivers/usb/host/ehci-s5p.c | 153 +++++++++++++++++++++---------------------
drivers/usb/host/ehci-spear.c | 88 +++++++++++-------------
drivers/usb/host/ehci.h | 1 +
drivers/usb/host/ohci-hcd.c | 136 ++++++++++++++++++++++++++++++++-----
10 files changed, 391 insertions(+), 310 deletions(-)

Cc: Alan Stern <[email protected]>
Cc: Bryan Huntsman <[email protected]>
Cc: David Brown <[email protected]>
Cc: Greg KH <[email protected]>
Cc: Jean-Christophe Plagniol-Villard <[email protected]>
Cc: Kukjin Kim <[email protected]>
Cc: Kyungmin Park <[email protected]>
Cc: Manjunath Goudar <[email protected]>
Cc: Nicolas Ferre <[email protected]>
Cc: Shiraz Hashim <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]


--
1.8.1.2


2013-03-28 21:54:57

by Arnd Bergmann

[permalink] [raw]
Subject: [PATCH v3 7/7] USB: OHCI: avoid conflicting platform drivers

Like the EHCI driver, OHCI supports a large number of different platform
glue drivers by directly including them, which causes problems with
conflicting macro definitions in some cases. As more ARM architecture
specific back-ends are required to coexist in a single build, we should
split those out into separate drivers. Unfortunately, the infrastructure
for that is still under development, so to give us more time, this uses
a separate *_PLATFORM_DRIVER macro for each ARM specific OHCI backend,
just like we already do on PowerPC and some of the other ARM platforms.

In linux-3.10, only the SPEAr and CNS3xxx back-ends would actually conflict
without this patch, but over time we would get more of them, so this
is a way to avoid having to patch the driver every time it breaks. We
should still split out all back-ends into separate loadable modules,
but that work is only needed to improve code size and cleanliness after
this patch, not for correctness.

While we're here, this fixes the incorrectly sorted error path
for the OMAP1 and OMAP3 backends to ensure we always unregister
the exact set of drivers that were registered before erroring out.

Signed-off-by: Arnd Bergmann <[email protected]>
Cc: Manjunath Goudar <[email protected]>
Cc: Greg KH <[email protected]>
Cc: Alan Stern <[email protected]>
Cc: [email protected]
---
drivers/usb/host/ohci-hcd.c | 136 ++++++++++++++++++++++++++++++++++++++------
1 file changed, 118 insertions(+), 18 deletions(-)

diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c
index 180a2b0..9e6de95 100644
--- a/drivers/usb/host/ohci-hcd.c
+++ b/drivers/usb/host/ohci-hcd.c
@@ -1102,12 +1102,12 @@ MODULE_LICENSE ("GPL");

#if defined(CONFIG_ARCH_S3C24XX) || defined(CONFIG_ARCH_S3C64XX)
#include "ohci-s3c2410.c"
-#define PLATFORM_DRIVER ohci_hcd_s3c2410_driver
+#define S3C2410_PLATFORM_DRIVER ohci_hcd_s3c2410_driver
#endif

#ifdef CONFIG_USB_OHCI_EXYNOS
#include "ohci-exynos.c"
-#define PLATFORM_DRIVER exynos_ohci_driver
+#define EXYNOS_PLATFORM_DRIVER exynos_ohci_driver
#endif

#ifdef CONFIG_USB_OHCI_HCD_OMAP1
@@ -1127,25 +1127,24 @@ MODULE_LICENSE ("GPL");

#ifdef CONFIG_ARCH_EP93XX
#include "ohci-ep93xx.c"
-#define PLATFORM_DRIVER ohci_hcd_ep93xx_driver
+#define EP93XX_PLATFORM_DRIVER ohci_hcd_ep93xx_driver
#endif

#ifdef CONFIG_ARCH_AT91
#include "ohci-at91.c"
-#define PLATFORM_DRIVER ohci_hcd_at91_driver
+#define AT91_PLATFORM_DRIVER ohci_hcd_at91_driver
#endif

#ifdef CONFIG_ARCH_LPC32XX
#include "ohci-nxp.c"
-#define PLATFORM_DRIVER usb_hcd_nxp_driver
+#define NXP_PLATFORM_DRIVER usb_hcd_nxp_driver
#endif

#ifdef CONFIG_ARCH_DAVINCI_DA8XX
#include "ohci-da8xx.c"
-#define PLATFORM_DRIVER ohci_hcd_da8xx_driver
+#define DAVINCI_PLATFORM_DRIVER ohci_hcd_da8xx_driver
#endif

-
#ifdef CONFIG_USB_OHCI_HCD_PPC_OF
#include "ohci-ppc-of.c"
#define OF_PLATFORM_DRIVER ohci_hcd_ppc_of_driver
@@ -1153,7 +1152,7 @@ MODULE_LICENSE ("GPL");

#ifdef CONFIG_PLAT_SPEAR
#include "ohci-spear.c"
-#define PLATFORM_DRIVER spear_ohci_hcd_driver
+#define SPEAR_PLATFORM_DRIVER spear_ohci_hcd_driver
#endif

#ifdef CONFIG_PPC_PS3
@@ -1199,7 +1198,14 @@ MODULE_LICENSE ("GPL");
!defined(SA1111_DRIVER) && \
!defined(PS3_SYSTEM_BUS_DRIVER) && \
!defined(SM501_OHCI_DRIVER) && \
- !defined(TMIO_OHCI_DRIVER)
+ !defined(TMIO_OHCI_DRIVER) && \
+ !defined(S3C2410_PLATFORM_DRIVER) && \
+ !defined(EXYNOS_PLATFORM_DRIVER) && \
+ !defined(EP93XX_PLATFORM_DRIVER) && \
+ !defined(AT91_PLATFORM_DRIVER) && \
+ !defined(NXP_PLATFORM_DRIVER) && \
+ !defined(DAVINCI_PLATFORM_DRIVER) && \
+ !defined(SPEAR_PLATFORM_DRIVER)
#error "missing bus glue for ohci-hcd"
#endif

@@ -1277,9 +1283,79 @@ static int __init ohci_hcd_mod_init(void)
goto error_tmio;
#endif

+#ifdef S3C2410_PLATFORM_DRIVER
+ retval = platform_driver_register(&S3C2410_PLATFORM_DRIVER);
+ if (retval < 0)
+ goto error_s3c2410;
+#endif
+
+#ifdef EXYNOS_PLATFORM_DRIVER
+ retval = platform_driver_register(&EXYNOS_PLATFORM_DRIVER);
+ if (retval < 0)
+ goto error_exynos;
+#endif
+
+#ifdef EP93XX_PLATFORM_DRIVER
+ retval = platform_driver_register(&EP93XX_PLATFORM_DRIVER);
+ if (retval < 0)
+ goto error_ep93xx;
+#endif
+
+#ifdef AT91_PLATFORM_DRIVER
+ retval = platform_driver_register(&AT91_PLATFORM_DRIVER);
+ if (retval < 0)
+ goto error_at91;
+#endif
+
+#ifdef NXP_PLATFORM_DRIVER
+ retval = platform_driver_register(&NXP_PLATFORM_DRIVER);
+ if (retval < 0)
+ goto error_nxp;
+#endif
+
+#ifdef DAVINCI_PLATFORM_DRIVER
+ retval = platform_driver_register(&DAVINCI_PLATFORM_DRIVER);
+ if (retval < 0)
+ goto error_davinci;
+#endif
+
+#ifdef SPEAR_PLATFORM_DRIVER
+ retval = platform_driver_register(&SPEAR_PLATFORM_DRIVER);
+ if (retval < 0)
+ goto error_spear;
+#endif
+
return retval;

/* Error path */
+#ifdef SPEAR_PLATFORM_DRIVER
+ platform_driver_unregister(&SPEAR_PLATFORM_DRIVER);
+ error_spear:
+#endif
+#ifdef DAVINCI_PLATFORM_DRIVER
+ platform_driver_unregister(&DAVINCI_PLATFORM_DRIVER);
+ error_davinci:
+#endif
+#ifdef NXP_PLATFORM_DRIVER
+ platform_driver_unregister(&NXP_PLATFORM_DRIVER);
+ error_nxp:
+#endif
+#ifdef AT91_PLATFORM_DRIVER
+ platform_driver_unregister(&AT91_PLATFORM_DRIVER);
+ error_at91:
+#endif
+#ifdef EP93XX_PLATFORM_DRIVER
+ platform_driver_unregister(&EP93XX_PLATFORM_DRIVER);
+ error_ep93xx:
+#endif
+#ifdef EXYNOS_PLATFORM_DRIVER
+ platform_driver_unregister(&EXYNOS_PLATFORM_DRIVER);
+ error_exynos:
+#endif
+#ifdef S3C2410_PLATFORM_DRIVER
+ platform_driver_unregister(&S3C2410_PLATFORM_DRIVER);
+ error_s3c2410:
+#endif
#ifdef TMIO_OHCI_DRIVER
platform_driver_unregister(&TMIO_OHCI_DRIVER);
error_tmio:
@@ -1300,17 +1376,17 @@ static int __init ohci_hcd_mod_init(void)
platform_driver_unregister(&OF_PLATFORM_DRIVER);
error_of_platform:
#endif
-#ifdef PLATFORM_DRIVER
- platform_driver_unregister(&PLATFORM_DRIVER);
- error_platform:
+#ifdef OMAP3_PLATFORM_DRIVER
+ platform_driver_unregister(&OMAP3_PLATFORM_DRIVER);
+ error_omap3_platform:
#endif
#ifdef OMAP1_PLATFORM_DRIVER
platform_driver_unregister(&OMAP1_PLATFORM_DRIVER);
error_omap1_platform:
#endif
-#ifdef OMAP3_PLATFORM_DRIVER
- platform_driver_unregister(&OMAP3_PLATFORM_DRIVER);
- error_omap3_platform:
+#ifdef PLATFORM_DRIVER
+ platform_driver_unregister(&PLATFORM_DRIVER);
+ error_platform:
#endif
#ifdef PS3_SYSTEM_BUS_DRIVER
ps3_ohci_driver_unregister(&PS3_SYSTEM_BUS_DRIVER);
@@ -1329,6 +1405,27 @@ module_init(ohci_hcd_mod_init);

static void __exit ohci_hcd_mod_exit(void)
{
+#ifdef SPEAR_PLATFORM_DRIVER
+ platform_driver_unregister(&SPEAR_PLATFORM_DRIVER);
+#endif
+#ifdef DAVINCI_PLATFORM_DRIVER
+ platform_driver_unregister(&DAVINCI_PLATFORM_DRIVER);
+#endif
+#ifdef NXP_PLATFORM_DRIVER
+ platform_driver_unregister(&NXP_PLATFORM_DRIVER);
+#endif
+#ifdef AT91_PLATFORM_DRIVER
+ platform_driver_unregister(&AT91_PLATFORM_DRIVER);
+#endif
+#ifdef EP93XX_PLATFORM_DRIVER
+ platform_driver_unregister(&EP93XX_PLATFORM_DRIVER);
+#endif
+#ifdef EXYNOS_PLATFORM_DRIVER
+ platform_driver_unregister(&EXYNOS_PLATFORM_DRIVER);
+#endif
+#ifdef S3C2410_PLATFORM_DRIVER
+ platform_driver_unregister(&S3C2410_PLATFORM_DRIVER);
+#endif
#ifdef TMIO_OHCI_DRIVER
platform_driver_unregister(&TMIO_OHCI_DRIVER);
#endif
@@ -1344,12 +1441,15 @@ static void __exit ohci_hcd_mod_exit(void)
#ifdef OF_PLATFORM_DRIVER
platform_driver_unregister(&OF_PLATFORM_DRIVER);
#endif
-#ifdef PLATFORM_DRIVER
- platform_driver_unregister(&PLATFORM_DRIVER);
-#endif
#ifdef OMAP3_PLATFORM_DRIVER
platform_driver_unregister(&OMAP3_PLATFORM_DRIVER);
#endif
+#ifdef OMAP1_PLATFORM_DRIVER
+ platform_driver_unregister(&OMAP1_PLATFORM_DRIVER);
+#endif
+#ifdef PLATFORM_DRIVER
+ platform_driver_unregister(&PLATFORM_DRIVER);
+#endif
#ifdef PS3_SYSTEM_BUS_DRIVER
ps3_ohci_driver_unregister(&PS3_SYSTEM_BUS_DRIVER);
#endif
--
1.8.1.2

2013-03-28 21:54:52

by Arnd Bergmann

[permalink] [raw]
Subject: [PATCH v3 2/7] USB: EHCI: make ehci-spear a separate driver

From: Manjunath Goudar <[email protected]>

Separate the SPEAr 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;
however, note that other changes are still needed before SPEAr can be
booted with a multi-platform kernel, but they are queued in the
arm-soc tree for 3.10.

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 SPEAr bus glue.

In V3:
-Detailed commit message added here about why this patch is required.
-Eliminated ehci_spear_setup routine beacuse hcd registers
directly setting in spear_ehci_hcd_drv_probe function.
-spear_overrides struct initialized.
-Eliminate struct ehci_hcd ehci from struct spear_ehci,to enable SPEAr clock
uses directly usb_hcd *hcd in spear_start_ehci function.
-to_spear_ehci() macro modified for spear_ehci.

In V2:
Replaced spear as SPEAr everywhere, leaving functions/variables/config options.

Signed-off-by: Deepak Saxena <[email protected]>
Signed-off-by: Manjunath Goudar <[email protected]>
Signed-off-by: Arnd Bergmann <[email protected]>
Acked-by: Viresh Kumar <[email protected]>
Cc: Greg KH <[email protected]>
Cc: Alan Stern <[email protected]>
Cc: Shiraz Hashim <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
drivers/usb/host/Kconfig | 8 ++++
drivers/usb/host/Makefile | 1 +
drivers/usb/host/ehci-hcd.c | 6 +--
drivers/usb/host/ehci-spear.c | 88 ++++++++++++++++++++-----------------------
4 files changed, 51 insertions(+), 52 deletions(-)

diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index d89b7ad..12fb83e 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -173,6 +173,14 @@ config USB_EHCI_HCD_ORION
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_SPEAR
+ tristate "Support for ST SPEAr on-chip EHCI USB controller"
+ depends on USB_EHCI_HCD && PLAT_SPEAR
+ default y
+ ---help---
+ Enables support for the on-chip EHCI controller on
+ ST SPEAr 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 9492f50..3e02471 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_OMAP) += ehci-omap.o
obj-$(CONFIG_USB_EHCI_HCD_ORION) += ehci-orion.o
+obj-$(CONFIG_USB_EHCI_HCD_SPEAR) += ehci-spear.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 1f97268..c8c70a1 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -1264,11 +1264,6 @@ MODULE_LICENSE ("GPL");
#define PLATFORM_DRIVER ehci_octeon_driver
#endif

-#ifdef CONFIG_PLAT_SPEAR
-#include "ehci-spear.c"
-#define PLATFORM_DRIVER spear_ehci_hcd_driver
-#endif
-
#ifdef CONFIG_USB_EHCI_MSM
#include "ehci-msm.c"
#define PLATFORM_DRIVER ehci_msm_driver
@@ -1315,6 +1310,7 @@ MODULE_LICENSE ("GPL");
!IS_ENABLED(CONFIG_USB_EHCI_MXC) && \
!IS_ENABLED(CONFIG_USB_EHCI_HCD_OMAP) && \
!IS_ENABLED(CONFIG_USB_EHCI_HCD_ORION) && \
+ !IS_ENABLED(CONFIG_USB_EHCI_HCD_SPEAR) && \
!defined(PLATFORM_DRIVER) && \
!defined(PS3_SYSTEM_BUS_DRIVER) && \
!defined(OF_PLATFORM_DRIVER) && \
diff --git a/drivers/usb/host/ehci-spear.c b/drivers/usb/host/ehci-spear.c
index 210bb67..d3a5859 100644
--- a/drivers/usb/host/ehci-spear.c
+++ b/drivers/usb/host/ehci-spear.c
@@ -1,5 +1,5 @@
/*
-* Driver for EHCI HCD on SPEAR SOC
+* Driver for EHCI HCD on SPEAr SOC
*
* Copyright (C) 2010 ST Micro Electronics,
* Deepak Sikri <[email protected]>
@@ -12,17 +12,28 @@
*/

#include <linux/clk.h>
+#include <linux/dma-mapping.h>
+#include <linux/io.h>
#include <linux/jiffies.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
#include <linux/of.h>
#include <linux/platform_device.h>
#include <linux/pm.h>
+#include <linux/usb.h>
+#include <linux/usb/hcd.h>
+
+#include "ehci.h"
+
+#define DRIVER_DESC "EHCI SPEAr driver"
+
+static const char hcd_name[] = "SPEAr-ehci";

struct spear_ehci {
- struct ehci_hcd ehci;
struct clk *clk;
};

-#define to_spear_ehci(hcd) (struct spear_ehci *)hcd_to_ehci(hcd)
+#define to_spear_ehci(hcd) (struct spear_ehci *)(hcd_to_ehci(hcd)->priv)

static void spear_start_ehci(struct spear_ehci *ehci)
{
@@ -34,49 +45,7 @@ static void spear_stop_ehci(struct spear_ehci *ehci)
clk_disable_unprepare(ehci->clk);
}

-static int ehci_spear_setup(struct usb_hcd *hcd)
-{
- struct ehci_hcd *ehci = hcd_to_ehci(hcd);
-
- /* registers start at offset 0x0 */
- ehci->caps = hcd->regs;
-
- return ehci_setup(hcd);
-}
-
-static const struct hc_driver ehci_spear_hc_driver = {
- .description = hcd_name,
- .product_desc = "SPEAr EHCI",
- .hcd_priv_size = sizeof(struct spear_ehci),
-
- /* generic hardware linkage */
- .irq = ehci_irq,
- .flags = HCD_MEMORY | HCD_USB2,
-
- /* basic lifecycle operations */
- .reset = ehci_spear_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 struct hc_driver __read_mostly ehci_spear_hc_driver;

#ifdef CONFIG_PM_SLEEP
static int ehci_spear_drv_suspend(struct device *dev)
@@ -161,7 +130,7 @@ static int spear_ehci_hcd_drv_probe(struct platform_device *pdev)
goto err_put_hcd;
}

- ehci = (struct spear_ehci *)hcd_to_ehci(hcd);
+ ehci = to_spear_ehci(hcd);
ehci->clk = usbh_clk;

spear_start_ehci(ehci);
@@ -216,4 +185,29 @@ static struct platform_driver spear_ehci_hcd_driver = {
}
};

+static const struct ehci_driver_overrides spear_overrides __initdata = {
+ .extra_priv_size = sizeof(struct spear_ehci),
+};
+
+static int __init ehci_spear_init(void)
+{
+ if (usb_disabled())
+ return -ENODEV;
+
+ pr_info("%s: " DRIVER_DESC "\n", hcd_name);
+
+ ehci_init_driver(&ehci_spear_hc_driver, &spear_overrides);
+ return platform_driver_register(&spear_ehci_hcd_driver);
+}
+module_init(ehci_spear_init);
+
+static void __exit ehci_spear_cleanup(void)
+{
+ platform_driver_unregister(&spear_ehci_hcd_driver);
+}
+module_exit(ehci_spear_cleanup);
+
+MODULE_DESCRIPTION(DRIVER_DESC);
MODULE_ALIAS("platform:spear-ehci");
+MODULE_AUTHOR("Deepak Sikri");
+MODULE_LICENSE("GPL");
--
1.8.1.2

2013-03-28 21:55:10

by Arnd Bergmann

[permalink] [raw]
Subject: [PATCH v3 5/7] USB: EHCI: make ehci-atmel a separate driver

From: Manjunath Goudar <[email protected]>

Separate the Atmel 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;
however, note that other changes are still needed before Atmel can be
booted with a multi-platform kernel. This is currently planned for
Linux-3.11.

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 Atmel bus glue.

In V3:
-Detailed commit message added here about why this patch is required.
-Replaced hcd_name string "ehci-atmel" to "atmel-ehci".
-In Makefile inserted Blank line that separates the EHCI drivers from the following non-EHCI drivers.
-Export ehci_shutdown symbol as it is needed by the Atmel driver.
-Eliminated ehci_atmel_setup routine beacuse hcd registers
directly setting in ehci_atmel_drv_probe function.

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 <[email protected]>
Cc: Alan Stern <[email protected]>
Cc: Greg KH <[email protected]>
Cc: Andrew Victor <[email protected]>
Cc: Nicolas Ferre <[email protected]>
Cc: Jean-Christophe Plagniol-Villard <[email protected]>
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Arnd Bergmann <[email protected]>
---
drivers/usb/host/Kconfig | 8 ++++
drivers/usb/host/Makefile | 1 +
drivers/usb/host/ehci-atmel.c | 85 ++++++++++++++++++++-----------------------
drivers/usb/host/ehci-hcd.c | 6 +--
4 files changed, 50 insertions(+), 50 deletions(-)

diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index 01c1acb..8c564aa 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -181,6 +181,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 3d895b5..368d3eb 100644
--- a/drivers/usb/host/Makefile
+++ b/drivers/usb/host/Makefile
@@ -31,6 +31,7 @@ obj-$(CONFIG_USB_EHCI_HCD_OMAP) += ehci-omap.o
obj-$(CONFIG_USB_EHCI_HCD_ORION) += ehci-orion.o
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_OXU210HP_HCD) += oxu210hp-hcd.o
obj-$(CONFIG_USB_ISP116X_HCD) += isp116x-hcd.o
diff --git a/drivers/usb/host/ehci-atmel.c b/drivers/usb/host/ehci-atmel.c
index f3beac4..9ce1217 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;
@@ -50,51 +63,6 @@ static void atmel_stop_ehci(struct platform_device *pdev)

/*-------------------------------------------------------------------------*/

-static int ehci_atmel_setup(struct usb_hcd *hcd)
-{
- struct ehci_hcd *ehci = hcd_to_ehci(hcd);
-
- /* registers start at offset 0x0 */
- ehci->caps = hcd->regs;
-
- 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)
@@ -102,6 +70,7 @@ static int ehci_atmel_drv_probe(struct platform_device *pdev)
struct usb_hcd *hcd;
const struct hc_driver *driver = &ehci_atmel_hc_driver;
struct resource *res;
+ struct ehci_hcd *ehci;
int irq;
int retval;

@@ -162,6 +131,10 @@ static int ehci_atmel_drv_probe(struct platform_device *pdev)
goto fail_request_resource;
}

+ ehci = hcd_to_ehci(hcd);
+ /* registers start at offset 0x0 */
+ ehci->caps = hcd->regs;
+
atmel_start_ehci(pdev);

retval = usb_add_hcd(hcd, irq, IRQF_SHARED);
@@ -213,3 +186,25 @@ static struct platform_driver ehci_atmel_driver = {
.of_match_table = of_match_ptr(atmel_ehci_dt_ids),
},
};
+
+static int __init ehci_atmel_init(void)
+{
+ if (usb_disabled())
+ return -ENODEV;
+
+ pr_info("%s: " DRIVER_DESC "\n", hcd_name);
+ ehci_init_driver(&ehci_atmel_hc_driver, NULL);
+ return platform_driver_register(&ehci_atmel_driver);
+}
+module_init(ehci_atmel_init);
+
+static void __exit ehci_atmel_cleanup(void)
+{
+ platform_driver_unregister(&ehci_atmel_driver);
+}
+module_exit(ehci_atmel_cleanup);
+
+MODULE_DESCRIPTION(DRIVER_DESC);
+MODULE_ALIAS("platform:atmel-ehci");
+MODULE_AUTHOR("Nicolas Ferre");
+MODULE_LICENSE("GPL");
diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index 1c9aa17..efc641c 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -1255,11 +1255,6 @@ MODULE_LICENSE ("GPL");
#define PLATFORM_DRIVER ehci_hcd_w90x900_driver
#endif

-#ifdef CONFIG_ARCH_AT91
-#include "ehci-atmel.c"
-#define PLATFORM_DRIVER ehci_atmel_driver
-#endif
-
#ifdef CONFIG_USB_OCTEON_EHCI
#include "ehci-octeon.c"
#define PLATFORM_DRIVER ehci_octeon_driver
@@ -1308,6 +1303,7 @@ MODULE_LICENSE ("GPL");
!IS_ENABLED(CONFIG_USB_EHCI_HCD_ORION) && \
!IS_ENABLED(CONFIG_USB_EHCI_HCD_SPEAR) && \
!IS_ENABLED(CONFIG_USB_EHCI_S5P) && \
+ !IS_ENABLED(CONFIG_USB_EHCI_HCD_AT91) && \
!defined(PLATFORM_DRIVER) && \
!defined(PS3_SYSTEM_BUS_DRIVER) && \
!defined(OF_PLATFORM_DRIVER) && \
--
1.8.1.2

2013-03-28 21:55:58

by Arnd Bergmann

[permalink] [raw]
Subject: [PATCH v3 4/7] USB: EHCI: export ehci_shutdown

The ehci_shutdown function is used by the platform specific ehci backends
for at91, tegra and ps3. In order to turn any of these into separate
modules, we need to make this function globally visible and export it.

Signed-off-by: Arnd Bergmann <[email protected]>
Cc: Greg KH <[email protected]>
Cc: Alan Stern <[email protected]>
Cc: [email protected]
---
drivers/usb/host/ehci-hcd.c | 3 ++-
drivers/usb/host/ehci.h | 1 +
2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index 8f1f4b4..1c9aa17 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -353,7 +353,7 @@ static void ehci_silence_controller(struct ehci_hcd *ehci)
* This forcibly disables dma and IRQs, helping kexec and other cases
* where the next system software may expect clean state.
*/
-static void ehci_shutdown(struct usb_hcd *hcd)
+void ehci_shutdown(struct usb_hcd *hcd)
{
struct ehci_hcd *ehci = hcd_to_ehci(hcd);

@@ -367,6 +367,7 @@ static void ehci_shutdown(struct usb_hcd *hcd)

hrtimer_cancel(&ehci->hrtimer);
}
+EXPORT_SYMBOL_GPL(ehci_shutdown);

/*-------------------------------------------------------------------------*/

diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
index e666999..915739d 100644
--- a/drivers/usb/host/ehci.h
+++ b/drivers/usb/host/ehci.h
@@ -799,6 +799,7 @@ struct ehci_driver_overrides {
extern void ehci_init_driver(struct hc_driver *drv,
const struct ehci_driver_overrides *over);
extern int ehci_setup(struct usb_hcd *hcd);
+extern void ehci_shutdown(struct usb_hcd *hcd);

#ifdef CONFIG_PM
extern int ehci_suspend(struct usb_hcd *hcd, bool do_wakeup);
--
1.8.1.2

2013-03-28 21:54:49

by Arnd Bergmann

[permalink] [raw]
Subject: [PATCH v3 3/7] USB: EHCI: make ehci-s5p a separate driver

From: Manjunath Goudar <[email protected]>

Separate the Samsung S5P/EXYNOS 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;
however, note that other changes are still needed before S5P/EXYNOS can
be booted with a multi-platform kernel. We currently expect those
to get merged for 3.10.

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 s5p bus glue.

In V3:
-Detail commit message added here,why this patch is required.
-MODULE_LICENSE is GPL v2.
-Added .extra_priv_size to eliminate the separate allocation of the s5p_ehci_hcd structure
and removed .reset function pointer initialization.
-Arranged #include's in alphabetical order.
-After using extra_priv_size initialization,struct usb_hcd *hcd is redundant that's why removed
from the prob function.
-Eliminated s5p_ehci_phy_enable,contents of statements moved into the s5p_ehci_probe
-Eliminated s5p_ehci_phy_disable, contents of statements moved into the s5p_ehci_remove.

In V2:
Tegra patch related changes removed from this patch.

Signed-off-by: Manjunath Goudar <[email protected]>
Acked-by: Jingoo Han <[email protected]>
Cc: Greg KH <[email protected]>
Cc: Alan Stern <[email protected]>
Cc: Kukjin Kim <[email protected]>
Cc: Kyungmin Park <[email protected]>
Cc: Grant Likely <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: [email protected]
Signed-off-by: Arnd Bergmann <[email protected]>
---
drivers/usb/host/Kconfig | 5 +-
drivers/usb/host/Makefile | 1 +
drivers/usb/host/ehci-hcd.c | 6 +-
drivers/usb/host/ehci-s5p.c | 153 ++++++++++++++++++++++----------------------
4 files changed, 82 insertions(+), 83 deletions(-)

diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index 12fb83e..01c1acb 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -218,10 +218,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"
+ tristate "EHCI support for Samsung S5P/EXYNOS SoC Series"
depends on USB_EHCI_HCD && PLAT_S5P
help
- Enable support for the S5P SOC's on-chip EHCI controller.
+ 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"
diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
index 3e02471..3d895b5 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_EHCI_HCD_OMAP) += ehci-omap.o
obj-$(CONFIG_USB_EHCI_HCD_ORION) += ehci-orion.o
obj-$(CONFIG_USB_EHCI_HCD_SPEAR) += ehci-spear.o
+obj-$(CONFIG_USB_EHCI_S5P) += ehci-s5p.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 c8c70a1..8f1f4b4 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -1284,11 +1284,6 @@ MODULE_LICENSE ("GPL");
#define PLATFORM_DRIVER tegra_ehci_driver
#endif

-#ifdef CONFIG_USB_EHCI_S5P
-#include "ehci-s5p.c"
-#define PLATFORM_DRIVER s5p_ehci_driver
-#endif
-
#ifdef CONFIG_SPARC_LEON
#include "ehci-grlib.c"
#define PLATFORM_DRIVER ehci_grlib_driver
@@ -1311,6 +1306,7 @@ MODULE_LICENSE ("GPL");
!IS_ENABLED(CONFIG_USB_EHCI_HCD_OMAP) && \
!IS_ENABLED(CONFIG_USB_EHCI_HCD_ORION) && \
!IS_ENABLED(CONFIG_USB_EHCI_HCD_SPEAR) && \
+ !IS_ENABLED(CONFIG_USB_EHCI_S5P) && \
!defined(PLATFORM_DRIVER) && \
!defined(PS3_SYSTEM_BUS_DRIVER) && \
!defined(OF_PLATFORM_DRIVER) && \
diff --git a/drivers/usb/host/ehci-s5p.c b/drivers/usb/host/ehci-s5p.c
index 738490e..f32be6f 100644
--- a/drivers/usb/host/ehci-s5p.c
+++ b/drivers/usb/host/ehci-s5p.c
@@ -13,13 +13,24 @@
*/

#include <linux/clk.h>
+#include <linux/dma-mapping.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
#include <linux/of.h>
-#include <linux/platform_device.h>
#include <linux/of_gpio.h>
+#include <linux/platform_device.h>
#include <linux/platform_data/usb-ehci-s5p.h>
#include <linux/usb/phy.h>
#include <linux/usb/samsung_usb_phy.h>
#include <plat/usb-phy.h>
+#include <linux/usb.h>
+#include <linux/usb/hcd.h>
+#include <linux/usb/otg.h>
+
+#include "ehci.h"
+
+#define DRIVER_DESC "EHCI s5p driver"

#define EHCI_INSNREG00(base) (base + 0x90)
#define EHCI_INSNREG00_ENA_INCR16 (0x1 << 25)
@@ -30,65 +41,17 @@
(EHCI_INSNREG00_ENA_INCR16 | EHCI_INSNREG00_ENA_INCR8 | \
EHCI_INSNREG00_ENA_INCR4 | EHCI_INSNREG00_ENA_INCRX_ALIGN)

+static const char hcd_name[] = "ehci-s5p";
+static struct hc_driver __read_mostly s5p_ehci_hc_driver;
+
struct s5p_ehci_hcd {
- struct device *dev;
- struct usb_hcd *hcd;
struct clk *clk;
struct usb_phy *phy;
struct usb_otg *otg;
struct s5p_ehci_platdata *pdata;
};

-static const struct hc_driver s5p_ehci_hc_driver = {
- .description = hcd_name,
- .product_desc = "S5P EHCI Host Controller",
- .hcd_priv_size = sizeof(struct ehci_hcd),
-
- .irq = ehci_irq,
- .flags = HCD_MEMORY | HCD_USB2,
-
- .reset = ehci_setup,
- .start = ehci_run,
- .stop = ehci_stop,
- .shutdown = ehci_shutdown,
-
- .get_frame_number = ehci_get_frame,
-
- .urb_enqueue = ehci_urb_enqueue,
- .urb_dequeue = ehci_urb_dequeue,
- .endpoint_disable = ehci_endpoint_disable,
- .endpoint_reset = ehci_endpoint_reset,
-
- .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 s5p_ehci_phy_enable(struct s5p_ehci_hcd *s5p_ehci)
-{
- struct platform_device *pdev = to_platform_device(s5p_ehci->dev);
-
- if (s5p_ehci->phy)
- usb_phy_init(s5p_ehci->phy);
- else if (s5p_ehci->pdata->phy_init)
- s5p_ehci->pdata->phy_init(pdev, USB_PHY_TYPE_HOST);
-}
-
-static void s5p_ehci_phy_disable(struct s5p_ehci_hcd *s5p_ehci)
-{
- struct platform_device *pdev = to_platform_device(s5p_ehci->dev);
-
- if (s5p_ehci->phy)
- usb_phy_shutdown(s5p_ehci->phy);
- else if (s5p_ehci->pdata->phy_exit)
- s5p_ehci->pdata->phy_exit(pdev, USB_PHY_TYPE_HOST);
-}
+#define to_s5p_ehci(hcd) (struct s5p_ehci_hcd *)(hcd_to_ehci(hcd)->priv)

static void s5p_setup_vbus_gpio(struct platform_device *pdev)
{
@@ -113,9 +76,10 @@ static u64 ehci_s5p_dma_mask = DMA_BIT_MASK(32);

static int s5p_ehci_probe(struct platform_device *pdev)
{
+ struct usb_hcd *hcd ;
struct s5p_ehci_platdata *pdata = pdev->dev.platform_data;
+ const struct hc_driver *driver = &s5p_ehci_hc_driver;
struct s5p_ehci_hcd *s5p_ehci;
- struct usb_hcd *hcd;
struct ehci_hcd *ehci;
struct resource *res;
struct usb_phy *phy;
@@ -153,16 +117,12 @@ static int s5p_ehci_probe(struct platform_device *pdev)
s5p_ehci->otg = phy->otg;
}

- s5p_ehci->dev = &pdev->dev;
-
- hcd = usb_create_hcd(&s5p_ehci_hc_driver, &pdev->dev,
- dev_name(&pdev->dev));
+ hcd = usb_create_hcd(driver, &pdev->dev, dev_name(&pdev->dev));
if (!hcd) {
dev_err(&pdev->dev, "Unable to create HCD\n");
return -ENOMEM;
}

- s5p_ehci->hcd = hcd;
s5p_ehci->clk = devm_clk_get(&pdev->dev, "usbhost");

if (IS_ERR(s5p_ehci->clk)) {
@@ -199,9 +159,12 @@ static int s5p_ehci_probe(struct platform_device *pdev)
}

if (s5p_ehci->otg)
- s5p_ehci->otg->set_host(s5p_ehci->otg, &s5p_ehci->hcd->self);
+ s5p_ehci->otg->set_host(s5p_ehci->otg, &hcd->self);

- s5p_ehci_phy_enable(s5p_ehci);
+ if (s5p_ehci->phy)
+ usb_phy_init(s5p_ehci->phy);
+ else if (s5p_ehci->pdata->phy_init)
+ s5p_ehci->pdata->phy_init(pdev, USB_PHY_TYPE_HOST);

ehci = hcd_to_ehci(hcd);
ehci->caps = hcd->regs;
@@ -220,7 +183,10 @@ static int s5p_ehci_probe(struct platform_device *pdev)
return 0;

fail_add_hcd:
- s5p_ehci_phy_disable(s5p_ehci);
+ if (s5p_ehci->phy)
+ usb_phy_shutdown(s5p_ehci->phy);
+ else if (s5p_ehci->pdata->phy_exit)
+ s5p_ehci->pdata->phy_exit(pdev, USB_PHY_TYPE_HOST);
fail_io:
clk_disable_unprepare(s5p_ehci->clk);
fail_clk:
@@ -230,15 +196,18 @@ fail_clk:

static int s5p_ehci_remove(struct platform_device *pdev)
{
- struct s5p_ehci_hcd *s5p_ehci = platform_get_drvdata(pdev);
- struct usb_hcd *hcd = s5p_ehci->hcd;
+ struct usb_hcd *hcd = platform_get_drvdata(pdev);
+ struct s5p_ehci_hcd *s5p_ehci = to_s5p_ehci(hcd);

usb_remove_hcd(hcd);

if (s5p_ehci->otg)
- s5p_ehci->otg->set_host(s5p_ehci->otg, &s5p_ehci->hcd->self);
+ s5p_ehci->otg->set_host(s5p_ehci->otg, &hcd->self);

- s5p_ehci_phy_disable(s5p_ehci);
+ if (s5p_ehci->phy)
+ usb_phy_shutdown(s5p_ehci->phy);
+ else if (s5p_ehci->pdata->phy_exit)
+ s5p_ehci->pdata->phy_exit(pdev, USB_PHY_TYPE_HOST);

clk_disable_unprepare(s5p_ehci->clk);

@@ -249,8 +218,7 @@ static int s5p_ehci_remove(struct platform_device *pdev)

static void s5p_ehci_shutdown(struct platform_device *pdev)
{
- struct s5p_ehci_hcd *s5p_ehci = platform_get_drvdata(pdev);
- struct usb_hcd *hcd = s5p_ehci->hcd;
+ struct usb_hcd *hcd = platform_get_drvdata(pdev);

if (hcd->driver->shutdown)
hcd->driver->shutdown(hcd);
@@ -259,17 +227,22 @@ static void s5p_ehci_shutdown(struct platform_device *pdev)
#ifdef CONFIG_PM
static int s5p_ehci_suspend(struct device *dev)
{
- struct s5p_ehci_hcd *s5p_ehci = dev_get_drvdata(dev);
- struct usb_hcd *hcd = s5p_ehci->hcd;
+ struct usb_hcd *hcd = dev_get_drvdata(dev);
+ struct s5p_ehci_hcd *s5p_ehci = to_s5p_ehci(hcd);
+ struct platform_device *pdev = to_platform_device(dev);
+
bool do_wakeup = device_may_wakeup(dev);
int rc;

rc = ehci_suspend(hcd, do_wakeup);

if (s5p_ehci->otg)
- s5p_ehci->otg->set_host(s5p_ehci->otg, &s5p_ehci->hcd->self);
+ s5p_ehci->otg->set_host(s5p_ehci->otg, &hcd->self);

- s5p_ehci_phy_disable(s5p_ehci);
+ if (s5p_ehci->phy)
+ usb_phy_shutdown(s5p_ehci->phy);
+ else if (s5p_ehci->pdata->phy_exit)
+ s5p_ehci->pdata->phy_exit(pdev, USB_PHY_TYPE_HOST);

clk_disable_unprepare(s5p_ehci->clk);

@@ -278,15 +251,19 @@ static int s5p_ehci_suspend(struct device *dev)

static int s5p_ehci_resume(struct device *dev)
{
- struct s5p_ehci_hcd *s5p_ehci = dev_get_drvdata(dev);
- struct usb_hcd *hcd = s5p_ehci->hcd;
+ struct usb_hcd *hcd = dev_get_drvdata(dev);
+ struct s5p_ehci_hcd *s5p_ehci = to_s5p_ehci(hcd);
+ struct platform_device *pdev = to_platform_device(dev);

clk_prepare_enable(s5p_ehci->clk);

if (s5p_ehci->otg)
- s5p_ehci->otg->set_host(s5p_ehci->otg, &s5p_ehci->hcd->self);
+ s5p_ehci->otg->set_host(s5p_ehci->otg, &hcd->self);

- s5p_ehci_phy_enable(s5p_ehci);
+ if (s5p_ehci->phy)
+ usb_phy_init(s5p_ehci->phy);
+ else if (s5p_ehci->pdata->phy_init)
+ s5p_ehci->pdata->phy_init(pdev, USB_PHY_TYPE_HOST);

/* DMA burst Enable */
writel(EHCI_INSNREG00_ENABLE_DMA_BURST, EHCI_INSNREG00(hcd->regs));
@@ -323,5 +300,29 @@ static struct platform_driver s5p_ehci_driver = {
.of_match_table = of_match_ptr(exynos_ehci_match),
}
};
+static const struct ehci_driver_overrides s5p_overrides __initdata = {
+ .extra_priv_size = sizeof(struct s5p_ehci_hcd),
+};
+
+static int __init ehci_s5p_init(void)
+{
+ if (usb_disabled())
+ return -ENODEV;
+
+ pr_info("%s: " DRIVER_DESC "\n", hcd_name);
+ ehci_init_driver(&s5p_ehci_hc_driver, &s5p_overrides);
+ return platform_driver_register(&s5p_ehci_driver);
+}
+module_init(ehci_s5p_init);
+
+static void __exit ehci_s5p_cleanup(void)
+{
+ platform_driver_unregister(&s5p_ehci_driver);
+}
+module_exit(ehci_s5p_cleanup);

+MODULE_DESCRIPTION(DRIVER_DESC);
MODULE_ALIAS("platform:s5p-ehci");
+MODULE_AUTHOR("Jingoo Han");
+MODULE_AUTHOR("Joonyoung Shim");
+MODULE_LICENSE("GPL v2");
--
1.8.1.2

2013-03-28 21:56:42

by Arnd Bergmann

[permalink] [raw]
Subject: [PATCH v3 6/7] USB: EHCI: make ehci-msm a separate driver

From: Manjunath Goudar <[email protected]>

Separate the Qualcomm QSD/MSM 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;
however, note that other changes are still needed before Qualcomm QSD/MSM
can be booted with a multi-platform kernel, which is not expected before
3.11.

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 msm bus glue.

In V3:
1.Detail commit message added here,why this patch is required.
2.Arranged #include's in alphabetical order.
3.drive.name initialized hcd_name[] = "ehci-msm" in platform_driver structure initialization
instead of "msm-ehci",that is reason it was breaking in EHCI USB testing.this one fixed in V3.

In V2:
Tegra patch related changes removed from this patch.

Signed-off-by: Manjunath Goudar <[email protected]>
Cc: Greg KH <[email protected]>
Cc: Alan Stern <[email protected]>
Cc: David Brown <[email protected]>
Cc: Daniel Walker <[email protected]>
Cc: Bryan Huntsman <[email protected]>
Cc: Brian Swetland <[email protected]>
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Arnd Bergmann <[email protected]>
---
drivers/usb/host/Kconfig | 2 +-
drivers/usb/host/Makefile | 1 +
drivers/usb/host/ehci-hcd.c | 6 +---
drivers/usb/host/ehci-msm.c | 85 ++++++++++++++++++++-------------------------
4 files changed, 40 insertions(+), 54 deletions(-)

diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index 8c564aa..35a5d3b 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -190,7 +190,7 @@ config USB_EHCI_HCD_AT91
Atmel chips.

config USB_EHCI_MSM
- bool "Support for MSM on-chip EHCI USB controller"
+ tristate "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
diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
index 368d3eb..4fb73c1 100644
--- a/drivers/usb/host/Makefile
+++ b/drivers/usb/host/Makefile
@@ -32,6 +32,7 @@ obj-$(CONFIG_USB_EHCI_HCD_ORION) += ehci-orion.o
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_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 efc641c..d5c4ae8 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -1260,11 +1260,6 @@ MODULE_LICENSE ("GPL");
#define PLATFORM_DRIVER ehci_octeon_driver
#endif

-#ifdef CONFIG_USB_EHCI_MSM
-#include "ehci-msm.c"
-#define PLATFORM_DRIVER ehci_msm_driver
-#endif
-
#ifdef CONFIG_TILE_USB
#include "ehci-tilegx.c"
#define PLATFORM_DRIVER ehci_hcd_tilegx_driver
@@ -1304,6 +1299,7 @@ MODULE_LICENSE ("GPL");
!IS_ENABLED(CONFIG_USB_EHCI_HCD_SPEAR) && \
!IS_ENABLED(CONFIG_USB_EHCI_S5P) && \
!IS_ENABLED(CONFIG_USB_EHCI_HCD_AT91) && \
+ !IS_ENABLED(CONFIG_USB_EHCI_MSM) && \
!defined(PLATFORM_DRIVER) && \
!defined(PS3_SYSTEM_BUS_DRIVER) && \
!defined(OF_PLATFORM_DRIVER) && \
diff --git a/drivers/usb/host/ehci-msm.c b/drivers/usb/host/ehci-msm.c
index 88a49c8..3b45f0c 100644
--- a/drivers/usb/host/ehci-msm.c
+++ b/drivers/usb/host/ehci-msm.c
@@ -22,16 +22,26 @@
* along with this program; if not, you can find it at http://www.fsf.org
*/

-#include <linux/platform_device.h>
#include <linux/clk.h>
#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
#include <linux/pm_runtime.h>
-
#include <linux/usb/otg.h>
#include <linux/usb/msm_hsusb_hw.h>
+#include <linux/usb.h>
+#include <linux/usb/hcd.h>
+
+#include "ehci.h"

#define MSM_USB_BASE (hcd->regs)

+#define DRIVER_DESC "Qualcomm On-Chip EHCI Host Controller"
+
+static const char hcd_name[] = "ehci-msm";
+static struct hc_driver __read_mostly msm_hc_driver;
static struct usb_phy *phy;

static int ehci_msm_reset(struct usb_hcd *hcd)
@@ -56,52 +66,6 @@ static int ehci_msm_reset(struct usb_hcd *hcd)
return 0;
}

-static struct hc_driver msm_hc_driver = {
- .description = hcd_name,
- .product_desc = "Qualcomm On-Chip EHCI Host Controller",
- .hcd_priv_size = sizeof(struct ehci_hcd),
-
- /*
- * generic hardware linkage
- */
- .irq = ehci_irq,
- .flags = HCD_USB2 | HCD_MEMORY,
-
- .reset = ehci_msm_reset,
- .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,
- .clear_tt_buffer_complete = ehci_clear_tt_buffer_complete,
-
- /*
- * scheduling support
- */
- .get_frame_number = ehci_get_frame,
-
- /*
- * root hub support
- */
- .hub_status_data = ehci_hub_status_data,
- .hub_control = ehci_hub_control,
- .relinquish_port = ehci_relinquish_port,
- .port_handed_over = ehci_port_handed_over,
-
- /*
- * PM support
- */
- .bus_suspend = ehci_bus_suspend,
- .bus_resume = ehci_bus_resume,
-};
-
static int ehci_msm_probe(struct platform_device *pdev)
{
struct usb_hcd *hcd;
@@ -226,3 +190,28 @@ static struct platform_driver ehci_msm_driver = {
.pm = &ehci_msm_dev_pm_ops,
},
};
+
+static const struct ehci_driver_overrides msm_overrides __initdata = {
+ .reset = ehci_msm_reset,
+};
+
+static int __init ehci_msm_init(void)
+{
+ if (usb_disabled())
+ return -ENODEV;
+
+ pr_info("%s: " DRIVER_DESC "\n", hcd_name);
+ ehci_init_driver(&msm_hc_driver, &msm_overrides);
+ return platform_driver_register(&ehci_msm_driver);
+}
+module_init(ehci_msm_init);
+
+static void __exit ehci_msm_cleanup(void)
+{
+ platform_driver_unregister(&ehci_msm_driver);
+}
+module_exit(ehci_msm_cleanup);
+
+MODULE_DESCRIPTION(DRIVER_DESC);
+MODULE_ALIAS("platform:msm-ehci");
+MODULE_LICENSE("GPL");
--
1.8.1.2

2013-03-28 21:54:48

by Arnd Bergmann

[permalink] [raw]
Subject: [PATCH v3 1/7] USB: EHCI: make ehci-orion a separate driver

From: Manjunath Goudar <[email protected]>

Separate the Orion host controller driver from ehci-hcd host
code into its own driver module because of following reason.

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.

An earlier version of this patch was included in 3.9 but caused
a regression there, which has subsequently been fixed.

In V3:
1.More detail provided in commit message regarding this patch.
2.Replaced hcd_name string "ehci-orion" into "orion-ehci".
3.MODULE_LICENSE is GPL v2.
4.In ehci_init_driver calling second argument passed as NULL instead of
ehci_orion_overrides because ehci_orion_overrides is removed.

In V2:
Tegra patch related changes removed from this patch.

Signed-off-by: Manjunath Goudar <[email protected]>
Signed-off-by: Arnd Bergmann <[email protected]>
Acked-by: Jason Cooper <[email protected]>
Tested-by: Andrew Lunn <[email protected]>
Cc: Greg KH <[email protected]>
Cc: Alan Stern <[email protected]>
Cc: Russell King <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
drivers/usb/host/Kconfig | 10 ++++++
drivers/usb/host/Makefile | 1 +
drivers/usb/host/ehci-hcd.c | 6 +---
drivers/usb/host/ehci-orion.c | 82 ++++++++++++++++++-------------------------
4 files changed, 47 insertions(+), 52 deletions(-)

diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index 2f68221..d89b7ad 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -163,6 +163,16 @@ 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 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 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_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 56de410..9492f50 100644
--- a/drivers/usb/host/Makefile
+++ b/drivers/usb/host/Makefile
@@ -28,6 +28,7 @@ obj-$(CONFIG_USB_EHCI_PCI) += ehci-pci.o
obj-$(CONFIG_USB_EHCI_HCD_PLATFORM) += ehci-platform.o
obj-$(CONFIG_USB_EHCI_MXC) += ehci-mxc.o
obj-$(CONFIG_USB_EHCI_HCD_OMAP) += ehci-omap.o
+obj-$(CONFIG_USB_EHCI_HCD_ORION) += ehci-orion.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 b12b97d..1f97268 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -1249,11 +1249,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
@@ -1319,6 +1314,7 @@ MODULE_LICENSE ("GPL");
!IS_ENABLED(CONFIG_USB_CHIPIDEA_HOST) && \
!IS_ENABLED(CONFIG_USB_EHCI_MXC) && \
!IS_ENABLED(CONFIG_USB_EHCI_HCD_OMAP) && \
+ !IS_ENABLED(CONFIG_USB_EHCI_HCD_ORION) && \
!defined(PLATFORM_DRIVER) && \
!defined(PS3_SYSTEM_BUS_DRIVER) && \
!defined(OF_PLATFORM_DRIVER) && \
diff --git a/drivers/usb/host/ehci-orion.c b/drivers/usb/host/ehci-orion.c
index 38c45fb..54c5794 100644
--- a/drivers/usb/host/ehci-orion.c
+++ b/drivers/usb/host/ehci-orion.c
@@ -17,6 +17,12 @@
#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 +40,12 @@
#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;
+
/*
* Implement Orion USB controller specification guidelines
*/
@@ -104,51 +116,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 +290,6 @@ static int 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", },
{},
@@ -341,3 +306,26 @@ static struct platform_driver ehci_orion_driver = {
.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, NULL);
+ 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:orion-ehci");
+MODULE_AUTHOR("Tzachi Perelstein");
+MODULE_LICENSE("GPL v2");
--
1.8.1.2

2013-03-29 02:56:43

by Viresh Kumar

[permalink] [raw]
Subject: Re: [PATCH v3 2/7] USB: EHCI: make ehci-spear a separate driver

On Fri, Mar 29, 2013 at 3:25 AM, Arnd Bergmann <[email protected]> wrote:
> From: Manjunath Goudar <[email protected]>
>
> Separate the SPEAr 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;
> however, note that other changes are still needed before SPEAr can be
> booted with a multi-platform kernel, but they are queued in the
> arm-soc tree for 3.10.
>
> 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 SPEAr bus glue.
>
> In V3:
> -Detailed commit message added here about why this patch is required.
> -Eliminated ehci_spear_setup routine beacuse hcd registers
> directly setting in spear_ehci_hcd_drv_probe function.
> -spear_overrides struct initialized.
> -Eliminate struct ehci_hcd ehci from struct spear_ehci,to enable SPEAr clock
> uses directly usb_hcd *hcd in spear_start_ehci function.
> -to_spear_ehci() macro modified for spear_ehci.
>
> In V2:
> Replaced spear as SPEAr everywhere, leaving functions/variables/config options.
>
> Signed-off-by: Deepak Saxena <[email protected]>
> Signed-off-by: Manjunath Goudar <[email protected]>
> Signed-off-by: Arnd Bergmann <[email protected]>
> Acked-by: Viresh Kumar <[email protected]>

This version has changed from what i Acked, but it still looks fine.

Thanks.

Viresh

2013-03-29 17:49:17

by Alan Stern

[permalink] [raw]
Subject: Re: [PATCH v3 1/7] USB: EHCI: make ehci-orion a separate driver

On Thu, 28 Mar 2013, Arnd Bergmann wrote:

> From: Manjunath Goudar <[email protected]>
>
> Separate the Orion host controller driver from ehci-hcd host
> code into its own driver module because of following reason.
...

On the whole this patch is good.

> --- a/drivers/usb/host/Kconfig
> +++ b/drivers/usb/host/Kconfig
> @@ -163,6 +163,16 @@ 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 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 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.

I don't know about this last phrase. When someone is running "make
menuconfig", for example, what shows up is the symbol's description,
not the symbol's name. That person would see "EHCI support for Marvell
on-chip controller", not "USB_EHCI_MV".

In fact, shouldn't the description for USB_EHCI_MV be changed too, to
make it more distinct from this one?

All the code changes are fine.

Alan Stern

2013-03-29 17:59:12

by Alan Stern

[permalink] [raw]
Subject: Re: [PATCH v3 2/7] USB: EHCI: make ehci-spear a separate driver

On Thu, 28 Mar 2013, Arnd Bergmann wrote:

> From: Manjunath Goudar <[email protected]>
>
> Separate the SPEAr 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;
> however, note that other changes are still needed before SPEAr can be
> booted with a multi-platform kernel, but they are queued in the
> arm-soc tree for 3.10.
>
> 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 SPEAr bus glue.
>
> In V3:
> -Detailed commit message added here about why this patch is required.
> -Eliminated ehci_spear_setup routine beacuse hcd registers
> directly setting in spear_ehci_hcd_drv_probe function.

Fix the grammar, please.

> -spear_overrides struct initialized.
> -Eliminate struct ehci_hcd ehci from struct spear_ehci,to enable SPEAr clock
> uses directly usb_hcd *hcd in spear_start_ehci function.
> -to_spear_ehci() macro modified for spear_ehci.
>
> In V2:
> Replaced spear as SPEAr everywhere, leaving functions/variables/config options.

...

> @@ -34,49 +45,7 @@ static void spear_stop_ehci(struct spear_ehci *ehci)
> clk_disable_unprepare(ehci->clk);
> }
>
> -static int ehci_spear_setup(struct usb_hcd *hcd)
> -{
> - struct ehci_hcd *ehci = hcd_to_ehci(hcd);
> -
> - /* registers start at offset 0x0 */
> - ehci->caps = hcd->regs;

This line never got moved into spear_ehci_hcd_drv_probe().

> @@ -161,7 +130,7 @@ static int spear_ehci_hcd_drv_probe(struct platform_device *pdev)
> goto err_put_hcd;
> }
>
> - ehci = (struct spear_ehci *)hcd_to_ehci(hcd);
> + ehci = to_spear_ehci(hcd);
> ehci->clk = usbh_clk;

I strongly believe that the name "ehci" should be reserved for
variables of type struct ehci_hcd. Here and in the start, stop, and
remove routines, please use "spear_ehci" as the name for a variable of
type struct spear_ehci. Or whatever else you want -- just don't call
it "ehci" or "ehci_p".

Alan Stern

2013-03-29 19:41:44

by Alan Stern

[permalink] [raw]
Subject: Re: [PATCH v3 3/7] USB: EHCI: make ehci-s5p a separate driver

On Thu, 28 Mar 2013, Arnd Bergmann wrote:

> From: Manjunath Goudar <[email protected]>
>
> Separate the Samsung S5P/EXYNOS 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;
> however, note that other changes are still needed before S5P/EXYNOS can
> be booted with a multi-platform kernel. We currently expect those
> to get merged for 3.10.
>
> 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 s5p bus glue.
>
> In V3:
> -Detail commit message added here,why this patch is required.
> -MODULE_LICENSE is GPL v2.
> -Added .extra_priv_size to eliminate the separate allocation of the s5p_ehci_hcd structure
> and removed .reset function pointer initialization.
> -Arranged #include's in alphabetical order.
> -After using extra_priv_size initialization,struct usb_hcd *hcd is redundant that's why removed
> from the prob function.
> -Eliminated s5p_ehci_phy_enable,contents of statements moved into the s5p_ehci_probe
> -Eliminated s5p_ehci_phy_disable, contents of statements moved into the s5p_ehci_remove.

And several other places as well.

Personally, I would have left these two functions the way they were and
relied on the compiler to inline them when appropriate. Eliminating
them just makes the code more complicated.

...

> @@ -113,9 +76,10 @@ static u64 ehci_s5p_dma_mask = DMA_BIT_MASK(32);
>
> static int s5p_ehci_probe(struct platform_device *pdev)
> {
> + struct usb_hcd *hcd ;
> struct s5p_ehci_platdata *pdata = pdev->dev.platform_data;
> + const struct hc_driver *driver = &s5p_ehci_hc_driver;
> struct s5p_ehci_hcd *s5p_ehci;
> - struct usb_hcd *hcd;

What's the reason for these changes? There's no need for the "driver"
variable, and improper whitespace was added to the declaration of
"hcd".

> @@ -153,16 +117,12 @@ static int s5p_ehci_probe(struct platform_device *pdev)
> s5p_ehci->otg = phy->otg;
> }
>
> - s5p_ehci->dev = &pdev->dev;
> -
> - hcd = usb_create_hcd(&s5p_ehci_hc_driver, &pdev->dev,
> - dev_name(&pdev->dev));
> + hcd = usb_create_hcd(driver, &pdev->dev, dev_name(&pdev->dev));

s5p_ehci is not initialized correctly. The devm_kzalloc() call was
left in and to_s5p_ehci() was not called. Was anybody able to test
this patch?

Alan Stern

2013-03-29 19:57:00

by Alan Stern

[permalink] [raw]
Subject: Re: [PATCH v3 4/7] USB: EHCI: export ehci_shutdown

On Thu, 28 Mar 2013, Arnd Bergmann wrote:

> The ehci_shutdown function is used by the platform specific ehci backends
> for at91, tegra and ps3. In order to turn any of these into separate
> modules, we need to make this function globally visible and export it.

Actually, I think this is not necessary. Instead those three glue
files ought to be changed. They should not need to call
ehci_shutdown() directly.

The references to ehci_shutdown() in ehci-atmel.c and ehci-ps3.c seem
totally unnecesary. That routine does a hard shutdown -- but the calls
are immediately followed by usb_remove_hcd(), which calls ehci_stop(),
which does an orderly shutdown followed by a reset. While it certainly
would be good to check with the authors of these drivers, it would be
surprising if removing those calls led to any trouble.

The reference in ehci-tegra.c is there only so that
tegra_ehci_shutdown() can call tegra_ehci_power_up() before doing its
real work. Instead, tegra_ehci_power_up() should be called by
tegra_ehci_hcd_shutdown(). Then there would be no need to have the
tegra_ehci_shutdown() routine at all.

Alan Stern

2013-03-29 20:02:08

by Alan Stern

[permalink] [raw]
Subject: Re: [PATCH v3 5/7] USB: EHCI: make ehci-atmel a separate driver

On Thu, 28 Mar 2013, Arnd Bergmann wrote:

> From: Manjunath Goudar <[email protected]>
>
> Separate the Atmel 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;
> however, note that other changes are still needed before Atmel can be
> booted with a multi-platform kernel. This is currently planned for
> Linux-3.11.
>
> 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 Atmel bus glue.

Generally okay.

> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
> index 01c1acb..8c564aa 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>

While not absolutely necessary, it would be nice to have the #include
files in alphabetical order.

> +
> +#include "ehci.h"
> +
> +#define DRIVER_DESC "EHCI atmel driver"

"atmel" should have a capital 'A'.

Alan Stern

2013-03-29 20:12:32

by Alan Stern

[permalink] [raw]
Subject: Re: [PATCH v3 6/7] USB: EHCI: make ehci-msm a separate driver

On Thu, 28 Mar 2013, Arnd Bergmann wrote:

> From: Manjunath Goudar <[email protected]>
>
> Separate the Qualcomm QSD/MSM 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;
> however, note that other changes are still needed before Qualcomm QSD/MSM
> can be booted with a multi-platform kernel, which is not expected before
> 3.11.
>
> 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 msm bus glue.

This patch is good. However the ehci-msm driver itself is not. While
checking through the code, I was struck by the fact that it never calls
usb_add_hcd() or usb_remove_hcd(). Obviously the driver cannot work
properly.

In addition, it stores the PHY pointer in a global variable.
(ehci-atmel does much the same thing for its clocks.) This means the
driver cannot be used on a system having more than one EHCI controller.
Maybe this doesn't matter, though.

Maybe somebody would like to fix and test it...

Alan Stern

2013-03-29 20:15:57

by Alan Stern

[permalink] [raw]
Subject: Re: [PATCH v3 7/7] USB: OHCI: avoid conflicting platform drivers

On Thu, 28 Mar 2013, Arnd Bergmann wrote:

> Like the EHCI driver, OHCI supports a large number of different platform
> glue drivers by directly including them, which causes problems with
> conflicting macro definitions in some cases. As more ARM architecture
> specific back-ends are required to coexist in a single build, we should
> split those out into separate drivers. Unfortunately, the infrastructure
> for that is still under development, so to give us more time, this uses
> a separate *_PLATFORM_DRIVER macro for each ARM specific OHCI backend,
> just like we already do on PowerPC and some of the other ARM platforms.
>
> In linux-3.10, only the SPEAr and CNS3xxx back-ends would actually conflict
> without this patch, but over time we would get more of them, so this
> is a way to avoid having to patch the driver every time it breaks. We
> should still split out all back-ends into separate loadable modules,
> but that work is only needed to improve code size and cleanliness after
> this patch, not for correctness.
>
> While we're here, this fixes the incorrectly sorted error path
> for the OMAP1 and OMAP3 backends to ensure we always unregister
> the exact set of drivers that were registered before erroring out.

I have not checked the details of all the changes; however, the basic
idea is okay as a stop-gap measure.

I guess this means the onus is now on me to split up ohci-hcd into a
central library and separate bus drivers, like ehci-hcd...

Alan Stern

2013-03-30 00:29:55

by Geoff Levand

[permalink] [raw]
Subject: Re: [PATCH v3 4/7] USB: EHCI: export ehci_shutdown

Hi Alan,

> Actually, I think this is not necessary. Instead those three glue
> files ought to be changed. They should not need to call
> ehci_shutdown() directly.

I sent out a separate patch that removes the ehci_shutdown()
call in ps3_ehci_remove(). I tested it by removing and
installing the ehci_hcd module and it seems to work OK.

-Geoff

2013-03-30 01:36:27

by Alan Stern

[permalink] [raw]
Subject: Re: [PATCH v3 4/7] USB: EHCI: export ehci_shutdown

On Fri, 29 Mar 2013, Geoff Levand wrote:

> Hi Alan,
>
> > Actually, I think this is not necessary. Instead those three glue
> > files ought to be changed. They should not need to call
> > ehci_shutdown() directly.
>
> I sent out a separate patch that removes the ehci_shutdown()
> call in ps3_ehci_remove(). I tested it by removing and
> installing the ehci_hcd module and it seems to work OK.

Thanks! I'll send it on up.

Alan Stern

2013-03-30 07:16:00

by Nicolas Ferre

[permalink] [raw]
Subject: Re: [PATCH v3 5/7] USB: EHCI: make ehci-atmel a separate driver

On 03/29/2013 09:02 PM, Alan Stern :
> On Thu, 28 Mar 2013, Arnd Bergmann wrote:
>
>> From: Manjunath Goudar <[email protected]>
>>
>> Separate the Atmel 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;
>> however, note that other changes are still needed before Atmel can be
>> booted with a multi-platform kernel. This is currently planned for
>> Linux-3.11.
>>
>> 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 Atmel bus glue.
>
> Generally okay.
>
>> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
>> index 01c1acb..8c564aa 100644
>> --- a/drivers/usb/host/ehci-atmel.c
>> +++ b/drivers/usb/host/ehci-atmel.c


I missed the patch itself but I reviewed it on the mailing-list archive.
So, here is my:

Acked-by: Nicolas Ferre <[email protected]>

Thanks a lot for having taking care of this driver.

Best regards,


>> @@ -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>
>
> While not absolutely necessary, it would be nice to have the #include
> files in alphabetical order.
>
>> +
>> +#include "ehci.h"
>> +
>> +#define DRIVER_DESC "EHCI atmel driver"
>
> "atmel" should have a capital 'A'.
>
> Alan Stern
>
>
>


--
Nicolas Ferre

2013-03-30 11:37:28

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH v3 1/7] USB: EHCI: make ehci-orion a separate driver

On Friday 29 March 2013, Alan Stern wrote:
> I don't know about this last phrase. When someone is running "make
> menuconfig", for example, what shows up is the symbol's description,
> not the symbol's name. That person would see "EHCI support for Marvell
> on-chip controller", not "USB_EHCI_MV".
>
> In fact, shouldn't the description for USB_EHCI_MV be changed too, to
> make it more distinct from this one?
>
> All the code changes are fine.

Ok, I've included these Kconfig changes. Originally, the text change
for PXA/MMP was included in the patch for ehci-mv, but that one is
deferred to 3.11 now because of dependencies for the platform code that
we want to first merge in 3.10.

Arnd

2013-03-30 12:03:14

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH v3 2/7] USB: EHCI: make ehci-spear a separate driver

On Friday 29 March 2013, Alan Stern wrote:
> On Thu, 28 Mar 2013, Arnd Bergmann wrote:
>
> > From: Manjunath Goudar <[email protected]>
> >
> > Separate the SPEAr 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;
> > however, note that other changes are still needed before SPEAr can be
> > booted with a multi-platform kernel, but they are queued in the
> > arm-soc tree for 3.10.
> >
> > 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 SPEAr bus glue.
> >
> > In V3:
> > -Detailed commit message added here about why this patch is required.
> > -Eliminated ehci_spear_setup routine beacuse hcd registers
> > directly setting in spear_ehci_hcd_drv_probe function.
>
> Fix the grammar, please.

Done. I agree some of this is hardly legible.

Manjunath, I can teach you about device drivers and submission
procedures, but I cannot teach you basic English. If necessary, find
someone to proofread your emails.
> > -static int ehci_spear_setup(struct usb_hcd *hcd)
> > -{
> > - struct ehci_hcd *ehci = hcd_to_ehci(hcd);
> > -
> > - /* registers start at offset 0x0 */
> > - ehci->caps = hcd->regs;
>
> This line never got moved into spear_ehci_hcd_drv_probe().

Ah, I missed it. Thanks for looking at this more carefully than
I did.

> > @@ -161,7 +130,7 @@ static int spear_ehci_hcd_drv_probe(struct platform_device *pdev)
> > goto err_put_hcd;
> > }
> >
> > - ehci = (struct spear_ehci *)hcd_to_ehci(hcd);
> > + ehci = to_spear_ehci(hcd);
> > ehci->clk = usbh_clk;
>
> I strongly believe that the name "ehci" should be reserved for
> variables of type struct ehci_hcd. Here and in the start, stop, and
> remove routines, please use "spear_ehci" as the name for a variable of
> type struct spear_ehci. Or whatever else you want -- just don't call
> it "ehci" or "ehci_p".

Ok, renamed to "sehci" in lack of a better idea. I noticed this before,
but I did not ask Manjunath to fix it because it was a preexisting mistake
in the driver.

Arnd

2013-03-30 12:23:14

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH v3 3/7] USB: EHCI: make ehci-s5p a separate driver

On Friday 29 March 2013, Alan Stern wrote:
> On Thu, 28 Mar 2013, Arnd Bergmann wrote:

>
> Personally, I would have left these two functions the way they were and
> relied on the compiler to inline them when appropriate. Eliminating
> them just makes the code more complicated.

Yes, makes sense. I'm leaving the change in now, because I don't
see a strong reason to undo the change, and the two functions
will likely get collapsed into a single line each after the move
to the phy subsystem is complete.

> > static int s5p_ehci_probe(struct platform_device *pdev)
> > {
> > + struct usb_hcd *hcd ;
> > struct s5p_ehci_platdata *pdata = pdev->dev.platform_data;
> > + const struct hc_driver *driver = &s5p_ehci_hc_driver;
> > struct s5p_ehci_hcd *s5p_ehci;
> > - struct usb_hcd *hcd;
>
> What's the reason for these changes? There's no need for the "driver"
> variable, and improper whitespace was added to the declaration of
> "hcd".

I'll let Manjunath answer this, I have no idea. My suspicion is that
it was a misguided attempt to work around a checkpatch warning for
an overly long line.

I've reverted the above changes now for v4.

> > @@ -153,16 +117,12 @@ static int s5p_ehci_probe(struct platform_device *pdev)
> > s5p_ehci->otg = phy->otg;
> > }
> >
> > - s5p_ehci->dev = &pdev->dev;
> > -
> > - hcd = usb_create_hcd(&s5p_ehci_hc_driver, &pdev->dev,
> > - dev_name(&pdev->dev));
> > + hcd = usb_create_hcd(driver, &pdev->dev, dev_name(&pdev->dev));
>
> s5p_ehci is not initialized correctly. The devm_kzalloc() call was
> left in and to_s5p_ehci() was not called.

Oh crap.

I checked that Manjunath had fixed this bug in the other drivers, but
missed it here. I updated it for v4 now.

> Was anybody able to test this patch?

I thought that Manjunath had received hardware for it now, but it's pretty
evident that the patch was not tested. The Ack from Jingoo Han was for an
older version that did not contain the change to .extra_priv_size.

Arnd

2013-03-30 12:32:45

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH v3 5/7] USB: EHCI: make ehci-atmel a separate driver

On Friday 29 March 2013, Alan Stern wrote:

> While not absolutely necessary, it would be nice to have the #include
> files in alphabetical order.
>
> > +
> > +#include "ehci.h"
> > +
> > +#define DRIVER_DESC "EHCI atmel driver"
>
> "atmel" should have a capital 'A'.
>

Ok, added these changes for v4 along with Nicolas' Ack,
and removed the call to ehci_shutdown.

Arnd

2013-03-30 12:49:30

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH v3 6/7] USB: EHCI: make ehci-msm a separate driver

On Friday 29 March 2013, Alan Stern wrote:
> On Thu, 28 Mar 2013, Arnd Bergmann wrote:

> This patch is good. However the ehci-msm driver itself is not. While
> checking through the code, I was struck by the fact that it never calls
> usb_add_hcd() or usb_remove_hcd(). Obviously the driver cannot work
> properly.
>
> In addition, it stores the PHY pointer in a global variable.
> (ehci-atmel does much the same thing for its clocks.) This means the
> driver cannot be used on a system having more than one EHCI controller.
> Maybe this doesn't matter, though.
>
> Maybe somebody would like to fix and test it...

I'm not too surprised. The driver was added the last time that the MSM
maintainers started a serious attempt to get a lot of their code mainlined,
a little over two years ago. While there is some activity from Qualcomm
in specific areas of the code every now and then, they literally have
thousands of patches on top of the kernel that they use in actual
products and I would not expect a mainline kernel to actually work on
any recent Qualcomm hardware.

Arnd

2013-03-30 12:56:23

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH v3 7/7] USB: OHCI: avoid conflicting platform drivers

On Friday 29 March 2013, Alan Stern wrote:
> I have not checked the details of all the changes; however, the basic
> idea is okay as a stop-gap measure.

Ok, thanks.

> I guess this means the onus is now on me to split up ohci-hcd into a
> central library and separate bus drivers, like ehci-hcd...

The original plan in my teams was that Manjunath would do that after
he was done with the simple conversion of the EHCI drivers. I think we
can all agree now that it's better if you at least the groundwork instead.

Please let us know if you would like Manjunath to continue with splitting
out the OHCI back-ends into separate drivers, or if you think that there
is no point given the quality of the earlier patches.

Arnd

2013-03-31 18:30:24

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH v3 2/7] USB: EHCI: make ehci-spear a separate driver

On Saturday 30 March 2013, Arnd Bergmann wrote:
> > > In V3:
> > > -Detailed commit message added here about why this patch is required.
> > > -Eliminated ehci_spear_setup routine beacuse hcd registers
> > > directly setting in spear_ehci_hcd_drv_probe function.
> >
> > Fix the grammar, please.
>
> Done. I agree some of this is hardly legible.
>

I realized later that the other patches have similarly screwed up
changelogs. I'll send a new version once you've commented on v4,
so in case there is anything else I missed I'm not spamming everyone
another time.

Arnd

2013-04-01 15:27:16

by Alan Stern

[permalink] [raw]
Subject: Re: [PATCH v3 2/7] USB: EHCI: make ehci-spear a separate driver

On Sun, 31 Mar 2013, Arnd Bergmann wrote:

> On Saturday 30 March 2013, Arnd Bergmann wrote:
> > > > In V3:
> > > > -Detailed commit message added here about why this patch is required.
> > > > -Eliminated ehci_spear_setup routine beacuse hcd registers
> > > > directly setting in spear_ehci_hcd_drv_probe function.
> > >
> > > Fix the grammar, please.
> >
> > Done. I agree some of this is hardly legible.
> >
>
> I realized later that the other patches have similarly screwed up
> changelogs. I'll send a new version once you've commented on v4,
> so in case there is anything else I missed I'm not spamming everyone
> another time.

I still haven't checked the details of the last patch in the series
(the OHCI changes). However, for patches 1/6 - 5/6:

Acked-by: Alan Stern <[email protected]>

2013-04-01 15:29:37

by Alan Stern

[permalink] [raw]
Subject: Re: [PATCH v3 7/7] USB: OHCI: avoid conflicting platform drivers

On Sat, 30 Mar 2013, Arnd Bergmann wrote:

> > I guess this means the onus is now on me to split up ohci-hcd into a
> > central library and separate bus drivers, like ehci-hcd...
>
> The original plan in my teams was that Manjunath would do that after
> he was done with the simple conversion of the EHCI drivers. I think we
> can all agree now that it's better if you at least the groundwork instead.

Yes, it would be better for me to prepare for the general split-up.

> Please let us know if you would like Manjunath to continue with splitting
> out the OHCI back-ends into separate drivers, or if you think that there
> is no point given the quality of the earlier patches.

This is how people learn. After I have taken care of the initial steps
and converted a couple of the drivers, Manjunath can work on the rest.

Alan Stern

2013-04-01 22:17:52

by David Brown

[permalink] [raw]
Subject: Re: [PATCH v3 6/7] USB: EHCI: make ehci-msm a separate driver

Arnd Bergmann <[email protected]> writes:

> On Friday 29 March 2013, Alan Stern wrote:
>> On Thu, 28 Mar 2013, Arnd Bergmann wrote:
>
>> This patch is good. However the ehci-msm driver itself is not. While
>> checking through the code, I was struck by the fact that it never calls
>> usb_add_hcd() or usb_remove_hcd(). Obviously the driver cannot work
>> properly.
>>
>> In addition, it stores the PHY pointer in a global variable.
>> (ehci-atmel does much the same thing for its clocks.) This means the
>> driver cannot be used on a system having more than one EHCI controller.
>> Maybe this doesn't matter, though.
>>
>> Maybe somebody would like to fix and test it...
>
> I'm not too surprised. The driver was added the last time that the MSM
> maintainers started a serious attempt to get a lot of their code mainlined,
> a little over two years ago. While there is some activity from Qualcomm
> in specific areas of the code every now and then, they literally have
> thousands of patches on top of the kernel that they use in actual
> products and I would not expect a mainline kernel to actually work on
> any recent Qualcomm hardware.

As far as this patch goes, you can have an
Acked-by: David Brown <[email protected]>
for it.

I'm hoping things are going to get better as far as people working on
things. EHCI is definitely an important one to get working (as is SD),
but we've got to get clocks and regulators working first.

David


--
sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation