[RFC] USB OHCI glue for PowerPC/Openfirmware

[RFC] USB OHCI glue for PowerPC/Openfirmware

[nd]

Theses patches add USB/OHCI HC glue for OpenFirmware. This allows to support any kind of USB/OHCI controller which is present in an OpenFirmware device tree.

By the way, I noticed that ohci-hcd.c module compilation is broken when multiple HC glues are selected. The glues sources files are directly include into ohci-hcd.c. Then, multiple module_init/exit in the same kmod cause compile issue. This news HC glue triggers this issue but is not the issue itself.
At least, this is how I understood the issue. I may be wrong but I just wanted to spot this issue.

Could you please review/comment this patch for submission in 2.6.20?

Regards 

[PATCH 1/2] Fixed compiled issue tu to new of_platform.h header

[nd]

From: Nicolas DET <nd@bplan-gmbh.de>

---
 drivers/usb/host/ohci-ppc-of.c |  285 ++++++++++++++++++++++++++++++++++++++++
 1 files changed, 285 insertions(+), 0 deletions(-)

diff --git a/drivers/usb/host/ohci-ppc-of.c b/drivers/usb/host/ohci-ppc-of.c
new file mode 100644
index 0000000..30ce520
--- /dev/null
+++ b/drivers/usb/host/ohci-ppc-of.c
@@ -0,0 +1,285 @@
+/*
+ * OHCI HCD (Host Controller Driver) for USB.
+ *
+ * (C) Copyright 1999 Roman Weissgaerber <weissg@vienna.at>
+ * (C) Copyright 2000-2002 David Brownell <dbrownell@users.sourceforge.net>
+ * (C) Copyright 2002 Hewlett-Packard Company
+ * (C) Copyright 2003-2005 MontaVista Software Inc.
+ *
+ * Probe and init OHCI Big endian HC from OpenFirmware device tree
+ * Tested on Efika 5k2
+ *
+ * Modified by Dale Farnsworth <dale@farnsworth.org> from ohci-sa1111.c
+ *
+ * This file is licenced under the GPL.
+ */
+
+#include <linux/signal.h>
+
+#include <asm/of_device.h>
+#include <asm/of_platform.h>
+#include <asm/prom.h>
+
+/* configure so an HC device and id are always provided */
+/* always called with process context; sleeping is OK */
+
+/*
+ * usb_hcd_ppc_of_probe - initialize On-Chip HCDs
+ * Context: !in_interrupt()
+ *
+ * Allocates basic resources for this USB host controller.
+ *
+ * Store this function in the HCD's struct pci_driver as probe().
+ */
+static int usb_hcd_ppc_of_probe(const struct hc_driver *driver,
+			  struct of_device *dev, int is_bigendian)
+{
+	int retval;
+	struct usb_hcd *hcd;
+	struct ohci_hcd	*ohci;
+	struct resource res;
+	int irq;
+	int ret;
+
+	pr_debug("initializing PPC-OF USB Controller\n");
+
+	if ((ret = of_address_to_resource(dev->node, 0, &res)) != 0)
+		return ret;
+
+	hcd = usb_create_hcd(driver, &dev->dev, "PPC-OF USB");
+	if (!hcd)
+		return -ENOMEM;
+
+	hcd->rsrc_start = res.start;
+	hcd->rsrc_len = res.end - res.start + 1;
+
+	if (!request_mem_region(hcd->rsrc_start, hcd->rsrc_len, hcd_name)) {
+		pr_debug(__FILE__ ": request_mem_region failed\n");
+		retval = -EBUSY;
+		goto err1;
+	}
+
+	irq = irq_of_parse_and_map(dev->node, 0);
+	if (irq == NO_IRQ) {
+		retval = -EBUSY;
+		goto err2;
+	}
+
+	hcd->regs = ioremap(hcd->rsrc_start, hcd->rsrc_len);
+	if (!hcd->regs) {
+		pr_debug(__FILE__ ": ioremap failed\n");
+		retval = -ENOMEM;
+		goto err2;
+	}
+
+	ohci = hcd_to_ohci(hcd);
+	if (is_bigendian)
+		ohci->flags |= OHCI_BIG_ENDIAN;
+
+	ohci_hcd_init(ohci);
+
+	retval = usb_add_hcd(hcd, irq, 0);
+	if (retval == 0)
+		return retval;
+
+	pr_debug("Removing PPC-OF USB Controller\n");
+
+	iounmap(hcd->regs);
+ err2:
+	release_mem_region(hcd->rsrc_start, hcd->rsrc_len);
+ err1:
+ 	usb_put_hcd(hcd);
+	return retval;
+}
+
+
+/* may be called without controller electrically present */
+/* may be called with controller, bus, and devices active */
+
+/*
+ * usb_hcd_ppc_of_remove - shutdown processing for On-Chip HCDs
+ * @pdev: USB Host Controller being removed
+ * Context: !in_interrupt()
+ *
+ * Reverses the effect of usb_hcd_ppc_of_probe().
+ * It is always called from a thread
+ * context, normally "rmmod", "apmd", or something similar.
+ *
+ */
+static void usb_hcd_ppc_of_remove(struct usb_hcd *hcd,
+		struct of_device *op)
+{
+	usb_remove_hcd(hcd);
+
+	pr_debug("stopping PPC-OF USB Controller\n");
+
+	iounmap(hcd->regs);
+	release_mem_region(hcd->rsrc_start, hcd->rsrc_len);
+	usb_put_hcd(hcd);
+}
+
+static int __devinit
+ohci_ppc_of_start(struct usb_hcd *hcd)
+{
+	struct ohci_hcd	*ohci = hcd_to_ohci(hcd);
+	int		ret;
+
+	if ((ret = ohci_init(ohci)) < 0)
+		return ret;
+
+	if ((ret = ohci_run(ohci)) < 0) {
+		err("can't start %s", ohci_to_hcd(ohci)->self.bus_name);
+		ohci_stop(hcd);
+		return ret;
+	}
+
+	return 0;
+}
+
+static const struct hc_driver ohci_ppc_of_hc_driver = {
+	.description =		hcd_name,
+	.hcd_priv_size =	sizeof(struct ohci_hcd),
+
+	/*
+	 * generic hardware linkage
+	 */
+	.irq =			ohci_irq,
+	.flags =		HCD_USB11 | HCD_MEMORY,
+
+	/*
+	 * basic lifecycle operations
+	 */
+	.start =		ohci_ppc_of_start,
+	.stop =			ohci_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_hub_status_data,
+	.hub_control =		ohci_hub_control,
+	.hub_irq_enable =	ohci_rhsc_enable,
+#ifdef	CONFIG_PM
+	.bus_suspend =		ohci_bus_suspend,
+	.bus_resume =		ohci_bus_resume,
+#endif
+	.start_port_reset =	ohci_start_port_reset,
+};
+
+
+
+static int ohci_hcd_ppc_of_drv_remove(struct of_device *op)
+{
+	struct usb_hcd *hcd = dev_get_drvdata(&op->dev);
+	dev_set_drvdata(&op->dev, NULL);
+
+	usb_hcd_ppc_of_remove(hcd, op);
+	return 0;
+}
+
+static int ohci_hcd_ppc_of_drv_shutdown(struct of_device *op)
+{
+	struct usb_hcd *hcd = dev_get_drvdata(&op->dev);
+
+        if (hcd->driver->shutdown)
+                hcd->driver->shutdown(hcd);
+
+	return 0;
+}
+
+/*
+ *
+*/
+
+static struct of_device_id ohci_hcd_ppc_of_match[] = {
+#ifdef CONFIG_USB_OHCI_HCD_PPC_OF_BE
+	{
+		.name = "usb",
+		.compatible = "ohci-bigendian",
+	},
+	{
+		.name = "usb",
+		.compatible = "ohci-be",
+	},
+#endif
+#ifdef CONFIG_USB_OHCI_HCD_PPC_OF_LE
+	{
+		.name = "usb",
+		.compatible = "ohci-littledian",
+	},
+	{
+		.name = "usb",
+		.compatible = "ohci-le",
+	},
+#endif
+	{},
+};
+
+static int __devinit
+ohci_hcd_ppc_of_drv_probe(struct of_device *op, const struct of_device_id *match)
+{
+	struct device_node *dev;
+	int ret;
+	int is_bigendian;
+
+	if (usb_disabled())
+		return -ENODEV;
+
+	dev = op->node;
+	is_bigendian = 0;
+
+	if ( device_is_compatible(dev, "ohci-bigendian") )
+		is_bigendian = 1;
+
+	if ( device_is_compatible(dev, "ohci-be") )
+		is_bigendian = 1;
+
+	ret = usb_hcd_ppc_of_probe(&ohci_ppc_of_hc_driver, op, is_bigendian);
+	return ret;
+}
+
+static struct of_platform_driver ohci_hcd_ppc_of_driver = {
+	.name		= "ppc-of-ohci",
+	.match_table	= ohci_hcd_ppc_of_match,
+	.probe		= ohci_hcd_ppc_of_drv_probe,
+	.remove		= ohci_hcd_ppc_of_drv_remove,
+	.shutdown 	= ohci_hcd_ppc_of_drv_shutdown,
+#ifdef	CONFIG_PM
+	/*.suspend	= ohci_hcd_ppc_soc_drv_suspend,*/
+	/*.resume	= ohci_hcd_ppc_soc_drv_resume,*/
+#endif
+	.driver		= {
+		.name	= "ppc-of-ohci",
+		.owner	= THIS_MODULE,
+	},
+};
+
+static int __init ohci_hcd_ppc_of_init(void)
+{
+	pr_debug(DRIVER_INFO " (PPC OF)\n");
+	pr_debug("block sizes: ed %d td %d\n", sizeof(struct ed),
+							sizeof(struct td));
+
+	return of_register_platform_driver(&ohci_hcd_ppc_of_driver);
+}
+
+static void __exit ohci_hcd_ppc_of_cleanup(void)
+{
+	of_unregister_platform_driver(&ohci_hcd_ppc_of_driver);
+}
+
+module_init(ohci_hcd_ppc_of_init);
+module_exit(ohci_hcd_ppc_of_cleanup);
+
-- 
1.4.3.2

[PATCH 2/2] USB OHCI OpenFirmware support

[nd]

From: Nicolas DET <nd@bplan-gmbh.de>

Signed-off-by: Nicolas DET <nd@bplan-gmbh.de>
---
 drivers/usb/host/Kconfig    |   19 +++++++++++++++++++
 drivers/usb/host/ohci-hcd.c |    6 ++++++
 2 files changed, 25 insertions(+), 0 deletions(-)

diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index cf10cbc..f5ef6f5 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -106,6 +106,25 @@ config USB_OHCI_HCD_PPC_SOC
 	  Enables support for the USB controller on the MPC52xx or
 	  STB03xxx processor chip.  If unsure, say Y.
 
+config USB_OHCI_HCD_PPC_OF
+	bool "OHCI support for PPC USB controller for OpenFirmware platform"
+	depends on USB_OHCI_HCD && PPC_OF
+	default y
+	---help---
+	  Enables support for the USB controller PowerPC OpenFirmware platform
+
+config USB_OHCI_HCD_PPC_OF_BE
+	bool "Support big endian HC"
+	depends on USB_OHCI_HCD_PPC_OF
+	default y
+	select USB_OHCI_BIG_ENDIAN
+
+config USB_OHCI_HCD_PPC_OF_LE
+	bool "Support little endian HC"
+	depends on USB_OHCI_HCD_PPC_OF
+	default n
+	select USB_OHCI_LITTLE_ENDIAN
+
 config USB_OHCI_HCD_PCI
 	bool "OHCI support for PCI-bus USB controllers"
 	depends on USB_OHCI_HCD && PCI && (STB03xxx || PPC_MPC52xx)
diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c
index 9be6b30..04413b6 100644
--- a/drivers/usb/host/ohci-hcd.c
+++ b/drivers/usb/host/ohci-hcd.c
@@ -930,6 +930,10 @@ #ifdef CONFIG_USB_OHCI_HCD_PPC_SOC
 #include "ohci-ppc-soc.c"
 #endif
 
+#ifdef CONFIG_USB_OHCI_HCD_PPC_OF
+#include "ohci-ppc-of.c"
+#endif
+
 #if defined(CONFIG_ARCH_AT91RM9200) || defined(CONFIG_ARCH_AT91SAM9261)
 #include "ohci-at91.c"
 #endif
@@ -950,6 +954,8 @@ #if !(defined(CONFIG_PCI) \
       || defined (CONFIG_ARCH_AT91RM9200) \
       || defined (CONFIG_ARCH_AT91SAM9261) \
       || defined (CONFIG_ARCH_PNX4008) \
+      || defined (CONFIG_USB_OHCI_HCD_PPC_OF_LE) \
+      || defined (CONFIG_USB_OHCI_HCD_PPC_OF_BE) \
 	)
 #error "missing bus glue for ohci-hcd"
 #endif
-- 
1.4.3.2

Re: [PATCH 1/2] Fixed compiled issue tu to new of_platform.h header

[Greg KH]

On Thu, Nov 23, 2006 at 05:12:00PM +0100, nd@bplan-gmbh.de wrote:
> From: Nicolas DET <nd@bplan-gmbh.de>

Hm, is this really from you?  The copyright doesn't look like it.

I need a signed-off-by: please, along with a good description of what
the patch does.

thanks,

greg k-h

Re: [PATCH 1/2] Fixed compiled issue tu to new of_platform.h header

[Benjamin Herrenschmidt]

On Fri, 2006-11-24 at 00:57 -0800, Greg KH wrote:
> On Thu, Nov 23, 2006 at 05:12:00PM +0100, nd@bplan-gmbh.de wrote:
> > From: Nicolas DET <nd@bplan-gmbh.de>
> 
> Hm, is this really from you?  The copyright doesn't look like it.
> 
> I need a signed-off-by: please, along with a good description of what
> the patch does.

It's from him, though he copy pasted a pile of (c) that are getting a
bit stale here without adding his own :-)

Now, Nico, since the patch has obviously a problem (thouh the problem
might be generic to the way the USB code handles multiple versions of an
HCI), it cannot be merged as-is of course.

Alan, what is your approach there ? We can't have multiple module_init()
in a single file it seems... the ugly fix is to have one with a serie of
#ifdef calling into the various HCI's but that's a big horrid.

Ben.

Re: [PATCH 1/2] Fixed compiled issue tu to new of_platform.h header

[Alan Stern]

On Sat, 25 Nov 2006, Benjamin Herrenschmidt wrote:

> Now, Nico, since the patch has obviously a problem (thouh the problem
> might be generic to the way the USB code handles multiple versions of an
> HCI), it cannot be merged as-is of course.
> 
> Alan, what is your approach there ? We can't have multiple module_init()
> in a single file it seems... the ugly fix is to have one with a serie of
> #ifdef calling into the various HCI's but that's a big horrid.

I haven't been following this very closely.  The problem is that you have 
a system with two different kinds of OHCI controllers, thus requiring two 
different and incompatible versions of the driver?

If the only source of incompatibility is the module_init() and 
module_exit() routines, then your suggestion would be a simple fix.  
Ugly, yes, but I don't think anyone will really mind.

Is there anything else to prevent you from combining multiple HCIs into a
single driver?

Alan Stern

Re: [PATCH 1/2] Fixed compiled issue tu to new of_platform.h header

[Benjamin Herrenschmidt]

> I haven't been following this very closely.  The problem is that you have 
> a system with two different kinds of OHCI controllers, thus requiring two 
> different and incompatible versions of the driver?

Sort-of. Basically, PowerPC can (and must in some case) build a single
kernel image that can be booted on a variety of platforms. There are at
least 3 platforms coming in now (this Efika is one of them) where that
includes a non-PCI OHCI or EHCI (big endian even) while the same kernel
must also contain the normal PCI OHCI/EHCI driver.

It's generally not a problem as the endian thingy is a runtime bit, and
in general, the whole code allows for this just fine, with, afaik, the
exception of the multiple module_init() bits...

> If the only source of incompatibility is the module_init() and 
> module_exit() routines, then your suggestion would be a simple fix.  
> Ugly, yes, but I don't think anyone will really mind.
> 
> Is there anything else to prevent you from combining multiple HCIs into a
> single driver?

Not that I know. But we could have a cleaner approach. The #include one
dates a bit... we could have for example an ohci-core.ko with the core
bits, and ochi-pci.ko, ohci-patform.ko etc... be separate modules linked
on the first and instanciating it.

I'll have a look and will come up with either solution.

Ben.

Re: [PATCH 1/2] Fixed compiled issue tu to new of_platform.h header

[Sven Luther]

On Sat, Nov 25, 2006 at 08:19:05AM +1100, Benjamin Herrenschmidt wrote:
> 
> > I haven't been following this very closely.  The problem is that you have 
> > a system with two different kinds of OHCI controllers, thus requiring two 
> > different and incompatible versions of the driver?
> 
> Sort-of. Basically, PowerPC can (and must in some case) build a single
> kernel image that can be booted on a variety of platforms. There are at
> least 3 platforms coming in now (this Efika is one of them) where that
> includes a non-PCI OHCI or EHCI (big endian even) while the same kernel
> must also contain the normal PCI OHCI/EHCI driver.
> 
> It's generally not a problem as the endian thingy is a runtime bit, and
> in general, the whole code allows for this just fine, with, afaik, the
> exception of the multiple module_init() bits...
> 
> > If the only source of incompatibility is the module_init() and 
> > module_exit() routines, then your suggestion would be a simple fix.  
> > Ugly, yes, but I don't think anyone will really mind.
> > 
> > Is there anything else to prevent you from combining multiple HCIs into a
> > single driver?
> 
> Not that I know. But we could have a cleaner approach. The #include one
> dates a bit... we could have for example an ohci-core.ko with the core
> bits, and ochi-pci.ko, ohci-patform.ko etc... be separate modules linked
> on the first and instanciating it.
> 
> I'll have a look and will come up with either solution.

Hi, ...

I am just wondering if i got dropped from the CCs or if there was no further
discussion on this.

The 2.6.19 kernel doesn't build when the efika patches are applied and the
ohci modules are built. We need a solution for this. Even disabling the patch
for ohci fails to build the ohci driver, but this could be a bad manipulation.

Is there any hint on the right way to go for this ? 

Friendly,

Sven Luther

Re: [PATCH 1/2] Fixed compiled issue tu to new of_platform.h header

[Nicolas DET]

On Sun, 3 Dec 2006 09:23:03 +0100
S0ven Luther <sven@genesi-usa.com> wrote:


> > Not that I know. But we could have a cleaner approach. The #include one
> > dates a bit... we could have for example an ohci-core.ko with the core
> > bits, and ochi-pci.ko, ohci-patform.ko etc... be separate modules linked
> > on the first and instanciating it.
> > 
> > I'll have a look and will come up with either solution.
> 

In my mind, this is the correct approch.

> Hi, ...
> 
> I am just wondering if i got dropped from the CCs or if there was no further
> discussion on this.
> 
> The 2.6.19 kernel doesn't build when the efika patches are applied and the
> ohci modules are built. We need a solution for this. Even disabling the patch
> for ohci fails to build the ohci driver, but this could be a bad manipulation.
> 

"Efika OHCI patches" does not exist. There is a new OHCI glue for OpenFrimware.

> Is there any hint on the right way to go for this ? 
> 

I'm currently running on 2.6.19 (the whole USB in kernel not kmod). It compiles and works fine.

Regards.

Re: [PATCH 1/2] Fixed compiled issue tu to new of_platform.h header

[Sven Luther]

On Sun, Dec 03, 2006 at 10:13:41AM +0100, Nicolas DET wrote:
> On Sun, 3 Dec 2006 09:23:03 +0100
> S0ven Luther <sven@genesi-usa.com> wrote:
> 
> 
> > > Not that I know. But we could have a cleaner approach. The #include one
> > > dates a bit... we could have for example an ohci-core.ko with the core
> > > bits, and ochi-pci.ko, ohci-patform.ko etc... be separate modules linked
> > > on the first and instanciating it.
> > > 
> > > I'll have a look and will come up with either solution.
> > 
> 
> In my mind, this is the correct approch.
> 
> > Hi, ...
> > 
> > I am just wondering if i got dropped from the CCs or if there was no further
> > discussion on this.
> > 
> > The 2.6.19 kernel doesn't build when the efika patches are applied and the
> > ohci modules are built. We need a solution for this. Even disabling the patch
> > for ohci fails to build the ohci driver, but this could be a bad manipulation.
> > 
> 
> "Efika OHCI patches" does not exist. There is a new OHCI glue for OpenFrimware.

Whatever.

> > Is there any hint on the right way to go for this ? 
> > 
> 
> I'm currently running on 2.6.19 (the whole USB in kernel not kmod). It compiles and works fine.

Ok, the day you are able to run the same ohci driver binary on both pegasos
with ohci usb card, and efika, then we can speak.

Friendly,

Sven Luther

Re: [linux-usb-devel] [PATCH 1/2] Fixed compiled issue tu to new of_platform.h header

[David Brownell]

On Thursday 23 November 2006 8:12 am, nd@bplan-gmbh.de wrote:
>  drivers/usb/host/ohci-ppc-of.c |  285 ++++++++++++++++++++++++++++++++++++++++

You've split one "add new bus glue" patch into two ... please
do it the normal way, as one patch!

(Oh, I see you did that the next day.  The next two comments
still apply.)


> + * Store this function in the HCD's struct pci_driver as probe().

You should remove irrelevant/ancient comments like this.  Looks
like you cloned the PCI bus glue and didn't update key parts...


> + */
> +static int usb_hcd_ppc_of_probe(const struct hc_driver *driver,
> +			  struct of_device *dev, int is_bigendian)

I'm not sure why you didn't just have ohci_hcd_ppc_of_drv_probe()
do all this stuff; there's no point in splitting that code into
two chunks.

Likewise, ohci_hcd_ppc_of_drv_remove() shouldn't be split in two.