* [PATCH] USB: OHCI: make ohci-da8xx a separate driver
@ 2013-07-02 11:36 Manjunath Goudar
2013-07-02 14:50 ` Sergei Shtylyov
0 siblings, 1 reply; 4+ messages in thread
From: Manjunath Goudar @ 2013-07-02 11:36 UTC (permalink / raw)
To: linux-arm-kernel
Separate the Davinci OHCI host controller driver from ohci-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;
it would be nice to have in 3.11.
One preexisting error:
"da8xx_syscfg0_base" [drivers/usb/host/ohci-da8xx.ko] undefined!
Fixed eventually using below modification:
added EXPORT_SYMBOL_GPL(da8xx_syscfg0_base) in
arch/arm/mach-davinci/devices-da8xx.c.
Signed-off-by: Manjunath Goudar <manjunath.goudar@linaro.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: Sergei Shtylyov <sshtylyov@ru.mvista.com>
Cc: Kevin Hilman <kjh@hilman.org>
Cc: Greg KH <greg@kroah.com>
Cc: linux-usb at vger.kernel.org
---
drivers/usb/host/Kconfig | 8 +++
drivers/usb/host/Makefile | 1 +
drivers/usb/host/ohci-da8xx.c | 139 +++++++++++++++++++----------------------
drivers/usb/host/ohci-hcd.c | 18 ------
4 files changed, 75 insertions(+), 91 deletions(-)
diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index 002bf73..bf2689d 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -406,6 +406,14 @@ config USB_OHCI_HCD_LPC32XX
Enables support for the on-chip OHCI controller on
NXP chips.
+config USB_OHCI_HCD_DA8XX
+ tristate "Support for DAVINCI on-chip OHCI USB controller"
+ depends on USB_OHCI_HCD && ARCH_DAVINCI_DA8XX
+ default y
+ ---help---
+ Enables support for the on-chip OHCI controller on
+ Davinci chips.
+
config USB_OHCI_HCD_AT91
tristate "Support for Atmel on-chip OHCI USB controller"
depends on USB_OHCI_HCD && ARCH_AT91
diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
index edc0909..f8d59371 100644
--- a/drivers/usb/host/Makefile
+++ b/drivers/usb/host/Makefile
@@ -53,6 +53,7 @@ obj-$(CONFIG_USB_OHCI_HCD_SPEAR) += ohci-spear.o
obj-$(CONFIG_USB_OHCI_HCD_AT91) += ohci-at91.o
obj-$(CONFIG_USB_OHCI_HCD_S3CXXXX) += ohci-s3c2410.o
obj-$(CONFIG_USB_OHCI_HCD_LPC32XX) += ohci-nxp.o
+obj-$(CONFIG_USB_OHCI_HCD_DA8XX) += ohci-da8xx.o
obj-$(CONFIG_USB_UHCI_HCD) += uhci-hcd.o
obj-$(CONFIG_USB_FHCI_HCD) += fhci.o
diff --git a/drivers/usb/host/ohci-da8xx.c b/drivers/usb/host/ohci-da8xx.c
index c649a35..9b4d1e4 100644
--- a/drivers/usb/host/ohci-da8xx.c
+++ b/drivers/usb/host/ohci-da8xx.c
@@ -11,20 +11,35 @@
* kind, whether express or implied.
*/
+#include <linux/clk.h>
+#include <linux/io.h>
#include <linux/interrupt.h>
#include <linux/jiffies.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
#include <linux/platform_device.h>
-#include <linux/clk.h>
+#include <linux/platform_data/usb-davinci.h>
+#include <linux/usb.h>
+#include <linux/usb/hcd.h>
+
#include <mach/da8xx.h>
-#include <linux/platform_data/usb-davinci.h>
+#include <asm/unaligned.h>
-#ifndef CONFIG_ARCH_DAVINCI_DA8XX
-#error "This file is DA8xx bus glue. Define CONFIG_ARCH_DAVINCI_DA8XX."
-#endif
+#include "ohci.h"
#define CFGCHIP2 DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG)
+#define DRIVER_DESC "OHCI DA8XX driver"
+
+static const char hcd_name[] = "ohci-da8xx";
+
+static struct hc_driver __read_mostly ohci_da8xx_hc_driver;
+
+static int (*orig_ohci_hub_control)(struct usb_hcd *hcd, u16 typeReq,
+ u16 wValue, u16 wIndex, char *buf, u16 wLength);
+static int (*orig_ohci_hub_status_data)(struct usb_hcd *hcd, char *buf);
+
static struct clk *usb11_clk;
static struct clk *usb20_clk;
@@ -82,7 +97,7 @@ static void ohci_da8xx_ocic_handler(struct da8xx_ohci_root_hub *hub,
hub->set_power(port, 0);
}
-static int ohci_da8xx_init(struct usb_hcd *hcd)
+static int ohci_da8xx_reset(struct usb_hcd *hcd)
{
struct device *dev = hcd->self.controller;
struct da8xx_ohci_root_hub *hub = dev->platform_data;
@@ -100,7 +115,7 @@ static int ohci_da8xx_init(struct usb_hcd *hcd)
*/
ohci->num_ports = 1;
- result = ohci_init(ohci);
+ result = ohci_setup(hcd);
if (result < 0)
return result;
@@ -126,30 +141,12 @@ static int ohci_da8xx_init(struct usb_hcd *hcd)
return result;
}
-static void ohci_da8xx_stop(struct usb_hcd *hcd)
-{
- ohci_stop(hcd);
- ohci_da8xx_clock(0);
-}
-
-static int ohci_da8xx_start(struct usb_hcd *hcd)
-{
- struct ohci_hcd *ohci = hcd_to_ohci(hcd);
- int result;
-
- result = ohci_run(ohci);
- if (result < 0)
- ohci_da8xx_stop(hcd);
-
- return result;
-}
-
/*
* Update the status data from the hub with the over-current indicator change.
*/
static int ohci_da8xx_hub_status_data(struct usb_hcd *hcd, char *buf)
{
- int length = ohci_hub_status_data(hcd, buf);
+ int length = orig_ohci_hub_status_data(hcd, buf);
/* See if we have OCIC bit set on port 1 */
if (ocic_mask & (1 << 1)) {
@@ -231,53 +228,10 @@ check_port:
}
}
- return ohci_hub_control(hcd, typeReq, wValue, wIndex, buf, wLength);
+ return orig_ohci_hub_control(hcd, typeReq, wValue,
+ wIndex, buf, wLength);
}
-static const struct hc_driver ohci_da8xx_hc_driver = {
- .description = hcd_name,
- .product_desc = "DA8xx OHCI",
- .hcd_priv_size = sizeof(struct ohci_hcd),
-
- /*
- * generic hardware linkage
- */
- .irq = ohci_irq,
- .flags = HCD_USB11 | HCD_MEMORY,
-
- /*
- * basic lifecycle operations
- */
- .reset = ohci_da8xx_init,
- .start = ohci_da8xx_start,
- .stop = ohci_da8xx_stop,
- .shutdown = ohci_shutdown,
-
- /*
- * managing i/o requests and associated device resources
- */
- .urb_enqueue = ohci_urb_enqueue,
- .urb_dequeue = ohci_urb_dequeue,
- .endpoint_disable = ohci_endpoint_disable,
-
- /*
- * scheduling support
- */
- .get_frame_number = ohci_get_frame,
-
- /*
- * root hub support
- */
- .hub_status_data = ohci_da8xx_hub_status_data,
- .hub_control = ohci_da8xx_hub_control,
-
-#ifdef CONFIG_PM
- .bus_suspend = ohci_bus_suspend,
- .bus_resume = ohci_bus_resume,
-#endif
- .start_port_reset = ohci_start_port_reset,
-};
-
/*-------------------------------------------------------------------------*/
@@ -337,8 +291,6 @@ static int usb_hcd_da8xx_probe(const struct hc_driver *driver,
goto err3;
}
- ohci_hcd_init(hcd_to_ohci(hcd));
-
irq = platform_get_irq(pdev, 0);
if (irq < 0) {
error = -ENODEV;
@@ -383,6 +335,7 @@ usb_hcd_da8xx_remove(struct usb_hcd *hcd, struct platform_device *pdev)
struct da8xx_ohci_root_hub *hub = pdev->dev.platform_data;
hub->ocic_notify(NULL);
+ ohci_da8xx_clock(0);
usb_remove_hcd(hcd);
iounmap(hcd->regs);
release_mem_region(hcd->rsrc_start, hcd->rsrc_len);
@@ -444,6 +397,11 @@ static int ohci_da8xx_resume(struct platform_device *dev)
}
#endif
+static const struct ohci_driver_overrides da8xx_overrides __initconst = {
+ .reset = ohci_da8xx_reset
+};
+
+
/*
* Driver definition to register with platform structure.
*/
@@ -461,4 +419,39 @@ static struct platform_driver ohci_hcd_da8xx_driver = {
},
};
+static int __init ohci_da8xx_init(void)
+{
+ if (usb_disabled())
+ return -ENODEV;
+
+ pr_info("%s: " DRIVER_DESC "\n", hcd_name);
+ ohci_init_driver(&ohci_da8xx_hc_driver, &da8xx_overrides);
+
+ /*
+ * The Davinci da8xx HW has some unusual quirks, which require
+ * da8xx-specific workarounds. We override certain hc_driver
+ * functions here to achieve that. We explicitly do not enhance
+ * ohci_driver_overrides to allow this more easily, since this
+ * is an unusual case, and we don't want to encourage others to
+ * override these functions by making it too easy.
+ */
+
+ orig_ohci_hub_control = ohci_da8xx_hc_driver.hub_control;
+ orig_ohci_hub_status_data = ohci_da8xx_hc_driver.hub_status_data;
+
+ ohci_da8xx_hc_driver.hub_status_data = ohci_da8xx_hub_status_data;
+ ohci_da8xx_hc_driver.hub_control = ohci_da8xx_hub_control;
+
+ return platform_driver_register(&ohci_hcd_da8xx_driver);
+}
+module_init(ohci_da8xx_init);
+
+static void __exit ohci_da8xx_cleanup(void)
+{
+ platform_driver_unregister(&ohci_hcd_da8xx_driver);
+}
+module_exit(ohci_da8xx_cleanup);
+
+MODULE_DESCRIPTION(DRIVER_DESC);
+MODULE_LICENSE("GPL");
MODULE_ALIAS("platform:ohci");
diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c
index 9a0b023..8ae7a1b 100644
--- a/drivers/usb/host/ohci-hcd.c
+++ b/drivers/usb/host/ohci-hcd.c
@@ -1194,11 +1194,6 @@ MODULE_LICENSE ("GPL");
#define EP93XX_PLATFORM_DRIVER ohci_hcd_ep93xx_driver
#endif
-#ifdef CONFIG_ARCH_DAVINCI_DA8XX
-#include "ohci-da8xx.c"
-#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
@@ -1296,19 +1291,9 @@ static int __init ohci_hcd_mod_init(void)
goto error_ep93xx;
#endif
-#ifdef DAVINCI_PLATFORM_DRIVER
- retval = platform_driver_register(&DAVINCI_PLATFORM_DRIVER);
- if (retval < 0)
- goto error_davinci;
-#endif
-
return retval;
/* Error path */
-#ifdef DAVINCI_PLATFORM_DRIVER
- platform_driver_unregister(&DAVINCI_PLATFORM_DRIVER);
- error_davinci:
-#endif
#ifdef EP93XX_PLATFORM_DRIVER
platform_driver_unregister(&EP93XX_PLATFORM_DRIVER);
error_ep93xx:
@@ -1350,9 +1335,6 @@ module_init(ohci_hcd_mod_init);
static void __exit ohci_hcd_mod_exit(void)
{
-#ifdef DAVINCI_PLATFORM_DRIVER
- platform_driver_unregister(&DAVINCI_PLATFORM_DRIVER);
-#endif
#ifdef NXP_PLATFORM_DRIVER
platform_driver_unregister(&NXP_PLATFORM_DRIVER);
#endif
--
1.7.9.5
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH] USB: OHCI: make ohci-da8xx a separate driver
2013-07-02 11:36 [PATCH] USB: OHCI: make ohci-da8xx a separate driver Manjunath Goudar
@ 2013-07-02 14:50 ` Sergei Shtylyov
[not found] ` <CAJFYCKErW5hvgBvKiywHmf0eBYOix8Fkn9vQFa-TKHA63hX7aQ@mail.gmail.com>
0 siblings, 1 reply; 4+ messages in thread
From: Sergei Shtylyov @ 2013-07-02 14:50 UTC (permalink / raw)
To: linux-arm-kernel
Hello.
On 02-07-2013 15:36, Manjunath Goudar wrote:
> Separate the Davinci OHCI host controller driver from ohci-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;
> it would be nice to have in 3.11.
> One preexisting error:
> "da8xx_syscfg0_base" [drivers/usb/host/ohci-da8xx.ko] undefined!
> Fixed eventually using below modification:
> added EXPORT_SYMBOL_GPL(da8xx_syscfg0_base) in
> arch/arm/mach-davinci/devices-da8xx.c.
And you managed to get this fix into the DaVinci tree? I tried it
long ago and it was refused by then DaVinci maintainer Kevin Hilman.
> Signed-off-by: Manjunath Goudar <manjunath.goudar@linaro.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Alan Stern <stern@rowland.harvard.edu>
> Cc: Sergei Shtylyov <sshtylyov@ru.mvista.com>
> Cc: Kevin Hilman <kjh@hilman.org>
> Cc: Greg KH <greg@kroah.com>
> Cc: linux-usb at vger.kernel.org
> ---
> drivers/usb/host/Kconfig | 8 +++
> drivers/usb/host/Makefile | 1 +
> drivers/usb/host/ohci-da8xx.c | 139 +++++++++++++++++++----------------------
> drivers/usb/host/ohci-hcd.c | 18 ------
> 4 files changed, 75 insertions(+), 91 deletions(-)
> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
> index 002bf73..bf2689d 100644
> --- a/drivers/usb/host/Kconfig
> +++ b/drivers/usb/host/Kconfig
> @@ -406,6 +406,14 @@ config USB_OHCI_HCD_LPC32XX
> Enables support for the on-chip OHCI controller on
> NXP chips.
>
> +config USB_OHCI_HCD_DA8XX
> + tristate "Support for DAVINCI on-chip OHCI USB controller"
> + depends on USB_OHCI_HCD && ARCH_DAVINCI_DA8XX
> + default y
> + ---help---
> + Enables support for the on-chip OHCI controller on
> + Davinci chips.
DA8xx is not a real DaVinci chip. The real DaVincis don't have OHCI
at all. Please replace "Davinci" with "DA8xx/OMAP-L1x" in both the
prompt and help text.
> diff --git a/drivers/usb/host/ohci-da8xx.c b/drivers/usb/host/ohci-da8xx.c
> index c649a35..9b4d1e4 100644
> --- a/drivers/usb/host/ohci-da8xx.c
> +++ b/drivers/usb/host/ohci-da8xx.c
> @@ -11,20 +11,35 @@
p...]
> +#define DRIVER_DESC "OHCI DA8XX driver"
Better "DA8xx".
[...]
> @@ -444,6 +397,11 @@ static int ohci_da8xx_resume(struct platform_device *dev)
> }
> #endif
>
> +static const struct ohci_driver_overrides da8xx_overrides __initconst = {
> + .reset = ohci_da8xx_reset
Better terminate the line with comma.
> +};
> +
> +
Too many blank lines?
> @@ -461,4 +419,39 @@ static struct platform_driver ohci_hcd_da8xx_driver = {
> },
> };
>
> +static int __init ohci_da8xx_init(void)
> +{
> + if (usb_disabled())
> + return -ENODEV;
> +
> + pr_info("%s: " DRIVER_DESC "\n", hcd_name);
> + ohci_init_driver(&ohci_da8xx_hc_driver, &da8xx_overrides);
> +
> + /*
> + * The Davinci da8xx HW has some unusual quirks, which require
> + * da8xx-specific workarounds. We override certain hc_driver
> + * functions here to achieve that. We explicitly do not enhance
> + * ohci_driver_overrides to allow this more easily, since this
> + * is an unusual case, and we don't want to encourage others to
Not as unusual as it seems. IIRC Samsung OHCI has the same quirks.
WBR, Sergei
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] USB: OHCI: make ohci-da8xx a separate driver
[not found] ` <CAJFYCKErW5hvgBvKiywHmf0eBYOix8Fkn9vQFa-TKHA63hX7aQ@mail.gmail.com>
@ 2013-07-02 17:20 ` Kevin Hilman
2013-07-02 18:45 ` Sekhar Nori
0 siblings, 1 reply; 4+ messages in thread
From: Kevin Hilman @ 2013-07-02 17:20 UTC (permalink / raw)
To: linux-arm-kernel
On 07/02/2013 08:14 AM, Manjunath Goudar wrote:
>
>
> On 2 July 2013 20:20, Sergei Shtylyov
> <sergei.shtylyov@cogentembedded.com
> <mailto:sergei.shtylyov@cogentembedded.com>> wrote:
>
> Hello.
>
>
> On 02-07-2013 15:36, Manjunath Goudar wrote:
>
> Separate the Davinci OHCI host controller driver from ohci-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;
> it would be nice to have in 3.11.
>
>
> One preexisting error:
> "da8xx_syscfg0_base" [drivers/usb/host/ohci-da8xx.__ko] undefined!
>
>
> Fixed eventually using below modification:
> added EXPORT_SYMBOL_GPL(da8xx___syscfg0_base) in
> arch/arm/mach-davinci/devices-__da8xx.c.
>
>
> And you managed to get this fix into the DaVinci tree? I tried it
> long ago and it was refused by then DaVinci maintainer Kevin Hilman.
>
>
> Yes I saw your patch that is what I mentioned in patch description.
> We will wait for DaVinci maintainer response,what he will suggest.
Note that Sekhar Nori (now Cc'd) is the primary maintainer of davinci,
and I'll defer the final decision to him. However, the mach-davinci
change is not in this patch, so I'm not sure exactly how it relates
here, since that problem exists independently of this patch.
That being said, I will NAK the above EXPORT_SYMBOL change in
mach-davinci code because passing data between platform code and drivers
via global variables is still a bad idea. Some helper accessor
functions will need to be created to abstract those low-level accesses.
> Signed-off-by: Manjunath Goudar <manjunath.goudar@linaro.org
> <mailto:manjunath.goudar@linaro.org>>
> Cc: Arnd Bergmann <arnd at arndb.de <mailto:arnd@arndb.de>>
> Cc: Alan Stern <stern@rowland.harvard.edu
> <mailto:stern@rowland.harvard.edu>>
> Cc: Sergei Shtylyov <sshtylyov@ru.mvista.com
> <mailto:sshtylyov@ru.mvista.com>>
> Cc: Kevin Hilman <kjh at hilman.org <mailto:kjh@hilman.org>>
Also, please khilman at linaro.org instead of my personal address. Thanks,
Kevin
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] USB: OHCI: make ohci-da8xx a separate driver
2013-07-02 17:20 ` Kevin Hilman
@ 2013-07-02 18:45 ` Sekhar Nori
0 siblings, 0 replies; 4+ messages in thread
From: Sekhar Nori @ 2013-07-02 18:45 UTC (permalink / raw)
To: linux-arm-kernel
On 7/2/2013 10:50 PM, Kevin Hilman wrote:
> On 07/02/2013 08:14 AM, Manjunath Goudar wrote:
>>
>>
>> On 2 July 2013 20:20, Sergei Shtylyov
>> <sergei.shtylyov@cogentembedded.com
>> <mailto:sergei.shtylyov@cogentembedded.com>> wrote:
>>
>> Hello.
>>
>>
>> On 02-07-2013 15:36, Manjunath Goudar wrote:
>>
>> Separate the Davinci OHCI host controller driver from ohci-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;
>> it would be nice to have in 3.11.
>>
>>
>> One preexisting error:
>> "da8xx_syscfg0_base" [drivers/usb/host/ohci-da8xx.__ko] undefined!
>>
>>
>> Fixed eventually using below modification:
>> added EXPORT_SYMBOL_GPL(da8xx___syscfg0_base) in
>> arch/arm/mach-davinci/devices-__da8xx.c.
>>
>>
>> And you managed to get this fix into the DaVinci tree? I tried it
>> long ago and it was refused by then DaVinci maintainer Kevin Hilman.
>>
>>
>> Yes I saw your patch that is what I mentioned in patch description.
>> We will wait for DaVinci maintainer response,what he will suggest.
>
> Note that Sekhar Nori (now Cc'd) is the primary maintainer of davinci,
> and I'll defer the final decision to him. However, the mach-davinci
> change is not in this patch, so I'm not sure exactly how it relates
> here, since that problem exists independently of this patch.
>
> That being said, I will NAK the above EXPORT_SYMBOL change in
> mach-davinci code because passing data between platform code and drivers
> via global variables is still a bad idea. Some helper accessor
> functions will need to be created to abstract those low-level accesses.
Okay, I haven't seen the patch as well, but I agree with Kevin that
EXPORT_SYMBOL from platform code is a bad idea and wont help the
multi-platform build.
Right clean-up for this most probably requires creation of a PHY driver
to handle the USB 2.0 and USB 1.1 phy specifics on this chip. Its best
to start a mail thread on USB list for guidance. You can keep me in loop
too.
Thanks,
Sekhar
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-07-02 18:45 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-07-02 11:36 [PATCH] USB: OHCI: make ohci-da8xx a separate driver Manjunath Goudar
2013-07-02 14:50 ` Sergei Shtylyov
[not found] ` <CAJFYCKErW5hvgBvKiywHmf0eBYOix8Fkn9vQFa-TKHA63hX7aQ@mail.gmail.com>
2013-07-02 17:20 ` Kevin Hilman
2013-07-02 18:45 ` Sekhar Nori
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).