| Message ID | 1364507705-22012-7-git-send-email-arnd@arndb.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, 28 Mar 2013, Arnd Bergmann wrote: > From: Manjunath Goudar <manjunath.goudar@linaro.org> > > 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
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
Arnd Bergmann <arnd@arndb.de> 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 <davidb@codeaurora.org> 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
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");