Skip to contentSkip to conversation

USB: OHCI: make ohci-da8xx a separate driver

PATCH Superseded Manjunath Goudar 4 participants 4 messages Jul 2, 2013 1 message not in the archive

Subsystems (2)
USB USB ohci driver

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Manjunath Goudar

author
#0 lore raw

PATCH 1/1 USB: OHCI: make ohci-da8xx a separate driver

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 <[email protected]>
Cc Arnd Bergmann <[email protected]>
Cc Alan Stern <[email protected]>
Cc Sergei Shtylyov <[email protected]>
Cc Kevin Hilman <[email protected]>
Cc Greg KH <[email protected]>
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(-)
drivers/usb/host/Kconfig +8 -0
index 002bf73..bf2689d 100644
@@ -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
drivers/usb/host/Makefile +1 -0
index edc0909..f8d59371 100644
@@ -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
drivers/usb/host/ohci-da8xx.c +66 -73

207 lines not on this page · raw message

drivers/usb/host/ohci-hcd.c +0 -18
index 9a0b023..8ae7a1b 100644
@@ -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
Signature · 1 line
1.7.9.5
#1 lore raw

PATCH 1/1 USB: OHCI: make ohci-da8xx a separate driver

Hello.

Quoted 11 lines · Manjunath Goudar

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.

Quoted 19 lines
Signed-off-by Manjunath Goudar <[email protected]>
Cc Arnd Bergmann <[email protected]>
Cc Alan Stern <[email protected]>
Cc Sergei Shtylyov <[email protected]>
Cc Kevin Hilman <[email protected]>
Cc Greg KH <[email protected]>
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(-)
drivers/usb/host/Kconfig +0 -0
index 002bf73..bf2689d 100644

drivers/usb/host/Kconfig config USB_OHCI_HCD_LPC32XX · line 415

Quoted 10 lines
drivers/usb/host/Kconfig +7 -0
@@ -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.

drivers/usb/host/ohci-da8xx.c

Quoted 5 lines
drivers/usb/host/ohci-da8xx.c +0 -0
index c649a35..9b4d1e4 100644
@@ -11,20 +11,35 @@

p...]

drivers/usb/host/ohci-da8xx.c line 33

Quoted 1 line
drivers/usb/host/ohci-da8xx.c +1 -0
+#define DRIVER_DESC "OHCI DA8XX driver"

    Better "DA8xx".

[...]

drivers/usb/host/ohci-da8xx.c static int ohci_da8xx_resume(struct platform_device *dev) · line 401

Quoted 6 lines
drivers/usb/host/ohci-da8xx.c +2 -0
@@ -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.

drivers/usb/host/ohci-da8xx.c static int ohci_da8xx_resume(struct platform_device *dev) · line 404

Quoted 3 lines
drivers/usb/host/ohci-da8xx.c +3 -0
+};
+
+

Too many blank lines?

Quoted 8 lines
drivers/usb/host/ohci-da8xx.c +4 -0
@@ -461,4 +419,39 @@ static struct platform_driver ohci_hcd_da8xx_driver = {
  	},
  };
 
+static int __init ohci_da8xx_init(void)
+{
+	if (usb_disabled())
+		return -ENODEV;

drivers/usb/host/ohci-da8xx.c static struct platform_driver ohci_hcd_da8xx_driver = { · line 435

Quoted 10 lines
drivers/usb/host/ohci-da8xx.c +10 -0
+
+	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

A message that is not in the archive (sent off-list or not yet received): CAJFYCKErW5hvgBvKiywHmf0eBYOix8Fkn9vQFa-TKHA63hX7aQ@mail.gmail.com

#3 lore raw

PATCH 1/1 USB: OHCI: make ohci-da8xx a separate driver

Quoted 26 lines · Manjunath Goudar

On 2 July 2013 20:20, Sergei Shtylyov <[email protected] <mailto:[email protected]>> 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.

Quoted 6 lines

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.

Quoted 8 lines
Signed-off-by Manjunath Goudar <[email protected]

<mailto:[email protected]>>
Cc: Arnd Bergmann <arnd at arndb.de <mailto:[email protected]>>
Cc: Alan Stern <[email protected] <mailto:[email protected]>>
Cc: Sergei Shtylyov <[email protected] <mailto:[email protected]>>
Cc: Kevin Hilman <kjh at hilman.org <mailto:[email protected]>>

Also, please khilman at linaro.org instead of my personal address. Thanks,

Kevin

#4 lore raw

PATCH 1/1 USB: OHCI: make ohci-da8xx a separate driver

Quoted 39 lines · Kevin Hilman
Quoted 26 lines · Manjunath Goudar

On 2 July 2013 20:20, Sergei Shtylyov <[email protected] <mailto:[email protected]>> 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.

Quoted 6 lines

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.

Quoted 4 lines

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