Re: [PATCH v3 6/7] USB: EHCI: make ehci-msm a separate driver
Arnd Bergmann 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 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 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 6/7] USB: EHCI: make ehci-msm a separate driver
On Thu, 28 Mar 2013, Arnd Bergmann wrote: > From: Manjunath Goudar > > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 6/7] USB: EHCI: make ehci-msm a separate driver
From: Manjunath Goudar
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
Cc: Greg KH
Cc: Alan Stern
Cc: David Brown
Cc: Daniel Walker
Cc: Bryan Huntsman
Cc: Brian Swetland
Cc: linux-usb@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
Signed-off-by: Arnd Bergmann
---
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_DRIVERehci_octeon_driver
#endif
-#ifdef CONFIG_USB_EHCI_MSM
-#include "ehci-msm.c"
-#define PLATFORM_DRIVERehci_msm_driver
-#endif
-
#ifdef CONFIG_TILE_USB
#include "ehci-tilegx.c"
#definePLATFORM_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
#include
#include
+#include
+#include
+#include
+#include
#include
-
#include
#include
+#include
+#include
+
+#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_re
