[RFC PATCH 0/2] wii: add usb 2.0 support

[RFC PATCH 0/2] wii: add usb 2.0 support

[Albert Herranz]

The following patch series adds USB 2.0 support for the Wii powerpc
platform via the EHCI controller present in the "Hollywood" chipset
of the video game console.

Albert Herranz (2):
  USB: add HCD_BOUNCE_BUFFERS host controller driver flag
  wii: hollywood ehci controller support

 arch/powerpc/platforms/embedded6xx/Kconfig |    1 +
 drivers/usb/core/hcd.c                     |  337 +++++++++++++++++++++++-----
 drivers/usb/core/hcd.h                     |   13 +-
 drivers/usb/host/Kconfig                   |    8 +
 drivers/usb/host/ehci-hcd.c                |    5 +
 drivers/usb/host/ehci-hlwd.c               |  227 +++++++++++++++++++
 drivers/usb/host/ehci.h                    |   23 ++
 7 files changed, 550 insertions(+), 64 deletions(-)
 create mode 100644 drivers/usb/host/ehci-hlwd.c

[RFC PATCH 1/2] USB: add HCD_BOUNCE_BUFFERS host controller driver flag

[Albert Herranz]

The HCD_BOUNCE_BUFFERS USB host controller driver flag can be enabled
to instruct the USB stack to always bounce USB buffers to/from coherent
memory buffers _just_ before/after a host controller transmission.

This setting allows overcoming some platform-specific limitations.

For example, the Nintendo Wii video game console is a NOT_COHERENT_CACHE
platform that is unable to safely perform non-32 bit uncached writes
to RAM because the byte enables are not connected to the bus.
Thus, in that platform, "coherent" DMA buffers cannot be directly used
by the kernel code unless it guarantees that all write accesses
to said buffers are done in 32 bit chunks (which is not the case in the
USB subsystem).

To avoid this unwanted behaviour HCD_BOUNCE_BUFFERS can be enabled at
the HCD controller, causing buffer allocations to be satisfied from
normal memory and, only at the very last moment, before the actual
transfer, buffers get copied to/from their corresponding DMA coherent
bounce buffers.

Note that HCD_LOCAL_MEM doesn't help in solving this problem as in that
case buffers may be allocated from coherent memory in the first place
and thus potentially accessed in non-32 bit chuncks by USB drivers.

Signed-off-by: Albert Herranz <albert_herranz@yahoo.es>
---
 drivers/usb/core/hcd.c |  337 +++++++++++++++++++++++++++++++++++++++--------
 drivers/usb/core/hcd.h |   13 +-
 2 files changed, 286 insertions(+), 64 deletions(-)

diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 80995ef..befca85 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -1260,6 +1260,173 @@ static void hcd_free_coherent(struct usb_bus *bus, dma_addr_t *dma_handle,
 	*dma_handle = 0;
 }
 
+/*
+ * The HCD_BOUNCE_BUFFERS USB host controller driver flag can be enabled
+ * to instruct the USB stack to always bounce USB buffers to/from coherent
+ * memory buffers _just_ before/after a host controller transmission.
+ *
+ * This setting allows overcoming some platform-specific limitations.
+ *
+ * For example, the Nintendo Wii video game console is a NOT_COHERENT_CACHE
+ * platform that is unable to safely perform non-32 bit uncached writes
+ * to RAM because the byte enables are not connected to the bus.
+ * Thus, in that platform, "coherent" DMA buffers cannot be directly used
+ * by the kernel code unless it guarantees that all write accesses
+ * to said buffers are done in 32 bit chunks (which is not the case in the
+ * USB subsystem).
+ *
+ * To avoid this unwanted behaviour HCD_BOUNCE_BUFFERS can be enabled at
+ * the HCD controller, causing buffer allocations to be satisfied from
+ * normal memory and, only at the very last moment, before the actual
+ * transfer, buffers get copied to/from their corresponding DMA coherent
+ * bounce buffers.
+ *
+ * Note that HCD_LOCAL_MEM doesn't help in solving this problem as in that
+ * case buffers may be allocated from coherent memory in the first place
+ * and thus potentially accessed in non-32 bit chuncks by USB drivers.
+ *
+ */
+
+#define HCD_BOUNCE_ALIGN	4
+
+#define hcd_align_up(addr, size)	(((addr)+((size)-1))&(~((size)-1)))
+
+struct hcd_coherent_buffer_ctx {
+	unsigned char *vaddr;
+	dma_addr_t dma_handle;
+};
+
+/**
+ * hcd_memcpy32_to_coherent - copy data to a bounce buffer
+ * @dst: destination dma bounce buffer
+ * @src: source buffer
+ * @len: number of bytes to copy
+ *
+ * This function copies @len bytes from @src to @dst in 32 bit chunks.
+ * The caller must guarantee that @dst length is 4 byte aligned and
+ * that @dst length is greater than or equal to @src length.
+ */
+static void *hcd_memcpy32_to_coherent(void *dst, const void *src, size_t len)
+{
+	u32 *q = dst, *p = (void *)src;
+	u8 *s;
+
+	while (len >= 4) {
+		*q++ = *p++;
+		len -= 4;
+	}
+	s = (u8 *)p;
+	switch (len) {
+	case 3:
+		*q = s[0] << 24 | s[1] << 16 | s[2] << 8;
+		break;
+	case 2:
+		*q = s[0] << 24 | s[1] << 16;
+		break;
+	case 1:
+		*q = s[0] << 24;
+		break;
+	default:
+		break;
+	}
+	return dst;
+}
+
+/**
+ * hcd_memcpy32_from_coherent - copy data from a bounce buffer
+ * @dst: destination buffer
+ * @src: source dma bounce buffer
+ * @len: number of bytes to copy
+ *
+ * This function copies @len bytes from @src to @dst in 32 bit chunks.
+ * The caller must guarantee that @src length is 4 byte aligned and
+ * that @src length is greater than or equal to @dst length.
+ */
+static void *hcd_memcpy32_from_coherent(void *dst, const void *src, size_t len)
+{
+	u32 *q = dst, *p = (void *)src;
+	u32 v;
+	u8 *d;
+
+	while (len >= 4) {
+		*q++ = *p++;
+		len -= 4;
+	}
+	if (len) {
+		d = (u8 *)q;
+		v = p[0];
+		switch (len) {
+		case 3:
+			d[2] = (v >> 8) & 0xff;
+			/* FALL THROUGH */
+		case 2:
+			d[1] = (v >> 16) & 0xff;
+			/* FALL THROUGH */
+		case 1:
+			d[0] = (v >> 24) & 0xff;
+			break;
+		default:
+			break;
+		}
+	}
+	return dst;
+}
+
+static int hcd_bounce_to_coherent(struct device *dev,
+				  gfp_t mem_flags, dma_addr_t *dma_handle,
+				  void **vaddr_handle, size_t size,
+				  enum dma_data_direction dir)
+{
+	struct hcd_coherent_buffer_ctx ctx;
+	unsigned char *vaddr;
+	size_t up_size = hcd_align_up(size + sizeof(ctx), HCD_BOUNCE_ALIGN);
+
+	BUILD_BUG_ON(sizeof(ctx) & (HCD_BOUNCE_ALIGN-1));
+
+	if (!*vaddr_handle) {
+		WARN_ON(1);
+		return -EINVAL;
+	}
+
+	ctx.vaddr = *vaddr_handle;
+	ctx.dma_handle = *dma_handle;
+
+	vaddr = dma_alloc_coherent(dev, up_size, dma_handle, mem_flags);
+	if (!vaddr)
+		return -ENOMEM;
+
+	hcd_memcpy32_to_coherent(vaddr + up_size - sizeof(ctx), &ctx,
+				 sizeof(ctx));
+	if (dir == DMA_TO_DEVICE)
+		hcd_memcpy32_to_coherent(vaddr, *vaddr_handle, size);
+
+	*vaddr_handle = vaddr;
+	return 0;
+}
+
+static void hcd_bounce_from_coherent(struct device *dev, dma_addr_t *dma_handle,
+				     void **vaddr_handle, size_t size,
+				     enum dma_data_direction dir)
+{
+	struct hcd_coherent_buffer_ctx ctx;
+	unsigned char *vaddr = *vaddr_handle;
+	size_t up_size = hcd_align_up(size + sizeof(ctx), HCD_BOUNCE_ALIGN);
+
+	if (!*vaddr_handle)
+		return;
+
+	hcd_memcpy32_from_coherent(&ctx, vaddr + up_size - sizeof(ctx),
+				   sizeof(ctx));
+	BUG_ON(!ctx.vaddr);
+	if (dir == DMA_FROM_DEVICE)
+		hcd_memcpy32_from_coherent(ctx.vaddr, vaddr, size);
+
+	dma_free_coherent(dev, up_size, vaddr, *dma_handle);
+
+	*vaddr_handle = ctx.vaddr;
+	*dma_handle = ctx.dma_handle;
+}
+
 static int map_urb_for_dma(struct usb_hcd *hcd, struct urb *urb,
 			   gfp_t mem_flags)
 {
@@ -1274,53 +1441,80 @@ static int map_urb_for_dma(struct usb_hcd *hcd, struct urb *urb,
 	if (is_root_hub(urb->dev))
 		return 0;
 
-	if (usb_endpoint_xfer_control(&urb->ep->desc)
-	    && !(urb->transfer_flags & URB_NO_SETUP_DMA_MAP)) {
-		if (hcd->self.uses_dma) {
-			urb->setup_dma = dma_map_single(
-					hcd->self.controller,
-					urb->setup_packet,
-					sizeof(struct usb_ctrlrequest),
-					DMA_TO_DEVICE);
-			if (dma_mapping_error(hcd->self.controller,
-						urb->setup_dma))
-				return -EAGAIN;
-		} else if (hcd->driver->flags & HCD_LOCAL_MEM)
-			ret = hcd_alloc_coherent(
-					urb->dev->bus, mem_flags,
+	if (usb_endpoint_xfer_control(&urb->ep->desc)) {
+		if (hcd->driver->flags & HCD_BOUNCE_BUFFERS) {
+			if (!(urb->transfer_flags & URB_NO_SETUP_DMA_MAP))
+				urb->setup_dma = 0;
+			ret = hcd_bounce_to_coherent(
+					hcd->self.controller, mem_flags,
 					&urb->setup_dma,
 					(void **)&urb->setup_packet,
 					sizeof(struct usb_ctrlrequest),
 					DMA_TO_DEVICE);
+		} else if (!(urb->transfer_flags & URB_NO_SETUP_DMA_MAP)) {
+			if (hcd->self.uses_dma) {
+				urb->setup_dma = dma_map_single(
+						hcd->self.controller,
+						urb->setup_packet,
+						sizeof(struct usb_ctrlrequest),
+						DMA_TO_DEVICE);
+				if (dma_mapping_error(hcd->self.controller,
+						      urb->setup_dma))
+					return -EAGAIN;
+			} else if (hcd->driver->flags & HCD_LOCAL_MEM)
+				ret = hcd_alloc_coherent(
+						urb->dev->bus, mem_flags,
+						&urb->setup_dma,
+						(void **)&urb->setup_packet,
+						sizeof(struct usb_ctrlrequest),
+						DMA_TO_DEVICE);
+		}
 	}
 
 	dir = usb_urb_dir_in(urb) ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
-	if (ret == 0 && urb->transfer_buffer_length != 0
-	    && !(urb->transfer_flags & URB_NO_TRANSFER_DMA_MAP)) {
-		if (hcd->self.uses_dma) {
-			urb->transfer_dma = dma_map_single (
-					hcd->self.controller,
-					urb->transfer_buffer,
-					urb->transfer_buffer_length,
-					dir);
-			if (dma_mapping_error(hcd->self.controller,
-						urb->transfer_dma))
-				return -EAGAIN;
-		} else if (hcd->driver->flags & HCD_LOCAL_MEM) {
-			ret = hcd_alloc_coherent(
-					urb->dev->bus, mem_flags,
+	if (ret == 0 && urb->transfer_buffer_length != 0) {
+		if (hcd->driver->flags & HCD_BOUNCE_BUFFERS) {
+			if (!(urb->transfer_flags & URB_NO_TRANSFER_DMA_MAP))
+				urb->transfer_dma = 0;
+			ret = hcd_bounce_to_coherent(
+					hcd->self.controller, mem_flags,
 					&urb->transfer_dma,
 					&urb->transfer_buffer,
 					urb->transfer_buffer_length,
 					dir);
 
-			if (ret && usb_endpoint_xfer_control(&urb->ep->desc)
-			    && !(urb->transfer_flags & URB_NO_SETUP_DMA_MAP))
-				hcd_free_coherent(urb->dev->bus,
-					&urb->setup_dma,
-					(void **)&urb->setup_packet,
-					sizeof(struct usb_ctrlrequest),
-					DMA_TO_DEVICE);
+			if (ret && usb_endpoint_xfer_control(&urb->ep->desc))
+				hcd_bounce_from_coherent(hcd->self.controller,
+						&urb->setup_dma,
+						(void **)&urb->setup_packet,
+						sizeof(struct usb_ctrlrequest),
+						DMA_TO_DEVICE);
+		} else if (!(urb->transfer_flags & URB_NO_TRANSFER_DMA_MAP)) {
+			if (hcd->self.uses_dma) {
+				urb->transfer_dma = dma_map_single(
+						hcd->self.controller,
+						urb->transfer_buffer,
+						urb->transfer_buffer_length,
+						dir);
+				if (dma_mapping_error(hcd->self.controller,
+						      urb->transfer_dma))
+					return -EAGAIN;
+			} else if (hcd->driver->flags & HCD_LOCAL_MEM) {
+				ret = hcd_alloc_coherent(
+						urb->dev->bus, mem_flags,
+						&urb->transfer_dma,
+						&urb->transfer_buffer,
+						urb->transfer_buffer_length,
+						dir);
+
+				if (ret &&
+				    usb_endpoint_xfer_control(&urb->ep->desc))
+					hcd_free_coherent(urb->dev->bus,
+						&urb->setup_dma,
+						(void **)&urb->setup_packet,
+						sizeof(struct usb_ctrlrequest),
+						DMA_TO_DEVICE);
+			}
 		}
 	}
 	return ret;
@@ -1333,32 +1527,49 @@ static void unmap_urb_for_dma(struct usb_hcd *hcd, struct urb *urb)
 	if (is_root_hub(urb->dev))
 		return;
 
-	if (usb_endpoint_xfer_control(&urb->ep->desc)
-	    && !(urb->transfer_flags & URB_NO_SETUP_DMA_MAP)) {
-		if (hcd->self.uses_dma)
-			dma_unmap_single(hcd->self.controller, urb->setup_dma,
-					sizeof(struct usb_ctrlrequest),
-					DMA_TO_DEVICE);
-		else if (hcd->driver->flags & HCD_LOCAL_MEM)
-			hcd_free_coherent(urb->dev->bus, &urb->setup_dma,
-					(void **)&urb->setup_packet,
-					sizeof(struct usb_ctrlrequest),
-					DMA_TO_DEVICE);
+	if (usb_endpoint_xfer_control(&urb->ep->desc)) {
+		if (hcd->driver->flags & HCD_BOUNCE_BUFFERS)
+			hcd_bounce_from_coherent(hcd->self.controller,
+						&urb->setup_dma,
+						(void **)&urb->setup_packet,
+						sizeof(struct usb_ctrlrequest),
+						DMA_TO_DEVICE);
+		else if (!(urb->transfer_flags & URB_NO_SETUP_DMA_MAP)) {
+			if (hcd->self.uses_dma)
+				dma_unmap_single(hcd->self.controller,
+						 urb->setup_dma,
+						 sizeof(struct usb_ctrlrequest),
+						 DMA_TO_DEVICE);
+			else if (hcd->driver->flags & HCD_LOCAL_MEM)
+				hcd_free_coherent(urb->dev->bus,
+						&urb->setup_dma,
+						(void **)&urb->setup_packet,
+						sizeof(struct usb_ctrlrequest),
+						DMA_TO_DEVICE);
+		}
 	}
 
 	dir = usb_urb_dir_in(urb) ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
-	if (urb->transfer_buffer_length != 0
-	    && !(urb->transfer_flags & URB_NO_TRANSFER_DMA_MAP)) {
-		if (hcd->self.uses_dma)
-			dma_unmap_single(hcd->self.controller,
-					urb->transfer_dma,
-					urb->transfer_buffer_length,
-					dir);
-		else if (hcd->driver->flags & HCD_LOCAL_MEM)
-			hcd_free_coherent(urb->dev->bus, &urb->transfer_dma,
-					&urb->transfer_buffer,
-					urb->transfer_buffer_length,
-					dir);
+	if (urb->transfer_buffer_length != 0) {
+		if (hcd->driver->flags & HCD_BOUNCE_BUFFERS)
+			hcd_bounce_from_coherent(hcd->self.controller,
+						&urb->transfer_dma,
+						&urb->transfer_buffer,
+						urb->transfer_buffer_length,
+						dir);
+		else if (!(urb->transfer_flags & URB_NO_TRANSFER_DMA_MAP)) {
+			if (hcd->self.uses_dma)
+				dma_unmap_single(hcd->self.controller,
+						urb->transfer_dma,
+						urb->transfer_buffer_length,
+						dir);
+			else if (hcd->driver->flags & HCD_LOCAL_MEM)
+				hcd_free_coherent(urb->dev->bus,
+						&urb->transfer_dma,
+						&urb->transfer_buffer,
+						urb->transfer_buffer_length,
+						dir);
+		}
 	}
 }
 
@@ -2022,6 +2233,16 @@ struct usb_hcd *usb_create_hcd (const struct hc_driver *driver,
 	dev_set_drvdata(dev, hcd);
 	kref_init(&hcd->kref);
 
+	if (driver->flags & HCD_BOUNCE_BUFFERS) {
+		/*
+		 * This will force allocation of HCD buffers in normal kernel
+		 * memory via kmalloc().
+		 * Note that this is fine as we always bounce them later
+		 * to coherent memory.
+		 */
+		dev->dma_mask = NULL;
+	}
+
 	usb_bus_init(&hcd->self);
 	hcd->self.controller = dev;
 	hcd->self.bus_name = bus_name;
diff --git a/drivers/usb/core/hcd.h b/drivers/usb/core/hcd.h
index bbe2b92..e0b8179 100644
--- a/drivers/usb/core/hcd.h
+++ b/drivers/usb/core/hcd.h
@@ -183,12 +183,13 @@ struct hc_driver {
 	irqreturn_t	(*irq) (struct usb_hcd *hcd);
 
 	int	flags;
-#define	HCD_MEMORY	0x0001		/* HC regs use memory (else I/O) */
-#define	HCD_LOCAL_MEM	0x0002		/* HC needs local memory */
-#define	HCD_USB11	0x0010		/* USB 1.1 */
-#define	HCD_USB2	0x0020		/* USB 2.0 */
-#define	HCD_USB3	0x0040		/* USB 3.0 */
-#define	HCD_MASK	0x0070
+#define	HCD_MEMORY		0x0001	/* HC regs use memory (else I/O) */
+#define	HCD_LOCAL_MEM		0x0002	/* HC needs local memory */
+#define HCD_BOUNCE_BUFFERS	0x0004	/* HC needs to bounce buffers */
+#define	HCD_USB11		0x0010	/* USB 1.1 */
+#define	HCD_USB2		0x0020	/* USB 2.0 */
+#define	HCD_USB3		0x0040	/* USB 3.0 */
+#define	HCD_MASK		0x0070
 
 	/* called to init HCD and root hub */
 	int	(*reset) (struct usb_hcd *hcd);
-- 
1.6.3.3

[RFC PATCH 2/2] wii: hollywood ehci controller support

[Albert Herranz]

Add support for the USB Enhanced Host Controller Interface included
in the "Hollywood" chipset of the Nintendo Wii video game console.

Signed-off-by: Albert Herranz <albert_herranz@yahoo.es>
---
 arch/powerpc/platforms/embedded6xx/Kconfig |    1 +
 drivers/usb/host/Kconfig                   |    8 +
 drivers/usb/host/ehci-hcd.c                |    5 +
 drivers/usb/host/ehci-hlwd.c               |  227 ++++++++++++++++++++++++++++
 drivers/usb/host/ehci.h                    |   23 +++
 5 files changed, 264 insertions(+), 0 deletions(-)
 create mode 100644 drivers/usb/host/ehci-hlwd.c

diff --git a/arch/powerpc/platforms/embedded6xx/Kconfig b/arch/powerpc/platforms/embedded6xx/Kconfig
index 524d971..34dbb79 100644
--- a/arch/powerpc/platforms/embedded6xx/Kconfig
+++ b/arch/powerpc/platforms/embedded6xx/Kconfig
@@ -119,6 +119,7 @@ config WII
 	bool "Nintendo-Wii"
 	depends on EMBEDDED6xx
 	select GAMECUBE_COMMON
+	select USB_ARCH_HAS_EHCI
 	help
 	  Select WII if configuring for the Nintendo Wii.
 	  More information at: <http://gc-linux.sourceforge.net/>
diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index 2678a16..342954f 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -131,6 +131,14 @@ config USB_EHCI_HCD_PPC_OF
 	  Enables support for the USB controller present on the PowerPC
 	  OpenFirmware platform bus.
 
+config USB_EHCI_HCD_HLWD
+	bool "Nintendo Wii (Hollywood) EHCI USB controller support"
+	depends on USB_EHCI_HCD && WII
+	default y
+	---help---
+	  Say Y here to support the EHCI USB controller found in the
+	  Hollywood chipset of the Nintendo Wii video game console.
+
 config USB_W90X900_EHCI
 	bool "W90X900(W90P910) EHCI support"
 	depends on USB_EHCI_HCD && ARCH_W90X900
diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index 1ec3857..395c6a1 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -1133,6 +1133,11 @@ MODULE_LICENSE ("GPL");
 #define OF_PLATFORM_DRIVER	ehci_hcd_ppc_of_driver
 #endif
 
+#ifdef CONFIG_USB_EHCI_HCD_HLWD
+#include "ehci-hlwd.c"
+#define OF_PLATFORM_DRIVER	ehci_hcd_hlwd_driver
+#endif
+
 #ifdef CONFIG_XPS_USB_HCD_XILINX
 #include "ehci-xilinx-of.c"
 #define OF_PLATFORM_DRIVER	ehci_hcd_xilinx_of_driver
diff --git a/drivers/usb/host/ehci-hlwd.c b/drivers/usb/host/ehci-hlwd.c
new file mode 100644
index 0000000..19812db
--- /dev/null
+++ b/drivers/usb/host/ehci-hlwd.c
@@ -0,0 +1,227 @@
+/*
+ * drivers/usb/host/ehci-hlwd.c
+ *
+ * Nintendo Wii (Hollywood) USB Enhanced Host Controller Interface.
+ * Copyright (C) 2009-2010 The GameCube Linux Team
+ * Copyright (C) 2009,2010 Albert Herranz
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or (at
+ * your option) any later version.
+ *
+ * Based on ehci-ppc-of.c
+ *
+ * EHCI HCD (Host Controller Driver) for USB.
+ *
+ * Bus Glue for PPC On-Chip EHCI driver on the of_platform bus
+ * Tested on AMCC PPC 440EPx
+ *
+ * Valentine Barshak <vbarshak@ru.mvista.com>
+ *
+ * Based on "ehci-ppc-soc.c" by Stefan Roese <sr@denx.de>
+ * and "ohci-ppc-of.c" by Sylvain Munaut <tnt@246tNt.com>
+ *
+ * This file is licenced under the GPL.
+ */
+
+#include <linux/signal.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+
+#define DRV_MODULE_NAME "ehci-hlwd"
+#define DRV_DESCRIPTION "Nintendo Wii EHCI Host Controller"
+#define DRV_AUTHOR      "Albert Herranz"
+
+/*
+ * Non-standard registers.
+ */
+#define HLWD_EHCI_CTL		0x00cc	/* Controller Control */
+#define HLWD_EHCI_CTL_INTE	(1<<15)	/* Notify EHCI interrupts */
+
+/* called during probe() after chip reset completes */
+static int ehci_hlwd_reset(struct usb_hcd *hcd)
+{
+	struct ehci_hcd	*ehci = hcd_to_ehci(hcd);
+	int error;
+
+	dbg_hcs_params(ehci, "reset");
+	dbg_hcc_params(ehci, "reset");
+
+	error = ehci_halt(ehci);
+	if (error)
+		goto out;
+
+	error = ehci_init(hcd);
+	if (error)
+		goto out;
+	hcd->self.sg_tablesize = 0;
+
+	/* enable notification of EHCI interrupts */
+	setbits32(hcd->regs + HLWD_EHCI_CTL, HLWD_EHCI_CTL_INTE);
+
+	ehci->sbrn = 0x20;
+	error = ehci_reset(ehci);
+	ehci_port_power(ehci, 0);
+out:
+	return error;
+}
+
+static const struct hc_driver ehci_hlwd_hc_driver = {
+	.description		= hcd_name,
+	.product_desc		= "Nintendo Wii EHCI Host Controller",
+	.hcd_priv_size		= sizeof(struct ehci_hcd),
+
+	/*
+	 * generic hardware linkage
+	 */
+	.irq			= ehci_irq,
+	.flags			= HCD_USB2 | HCD_BOUNCE_BUFFERS,
+
+	/*
+	 * basic lifecycle operations
+	 */
+	.reset			= ehci_hlwd_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,
+
+	/*
+	 * scheduling support
+	 */
+	.get_frame_number	= ehci_get_frame,
+
+	/*
+	 * root hub support
+	 */
+	.hub_status_data	= ehci_hub_status_data,
+	.hub_control		= ehci_hub_control,
+#ifdef	CONFIG_PM
+	.bus_suspend		= ehci_bus_suspend,
+	.bus_resume		= ehci_bus_resume,
+#endif
+	.relinquish_port	= ehci_relinquish_port,
+	.port_handed_over	= ehci_port_handed_over,
+
+	.clear_tt_buffer_complete	= ehci_clear_tt_buffer_complete,
+};
+
+
+static int __devinit
+ehci_hcd_hlwd_probe(struct of_device *op, const struct of_device_id *match)
+{
+	struct device_node *dn = op->node;
+	struct usb_hcd *hcd;
+	struct ehci_hcd	*ehci = NULL;
+	struct resource res;
+	int irq;
+	int error = -ENODEV;
+
+	if (usb_disabled())
+		goto out;
+
+	dev_dbg(&op->dev, "initializing " DRV_MODULE_NAME " USB Controller\n");
+
+	error = of_address_to_resource(dn, 0, &res);
+	if (error)
+		goto out;
+
+	hcd = usb_create_hcd(&ehci_hlwd_hc_driver, &op->dev, DRV_MODULE_NAME);
+	if (!hcd) {
+		error = -ENOMEM;
+		goto out;
+	}
+
+	hcd->rsrc_start = res.start;
+	hcd->rsrc_len = resource_size(&res);
+
+	irq = irq_of_parse_and_map(dn, 0);
+	if (irq == NO_IRQ) {
+		printk(KERN_ERR __FILE__ ": irq_of_parse_and_map failed\n");
+		error = -EBUSY;
+		goto err_irq;
+	}
+
+	hcd->regs = ioremap(hcd->rsrc_start, hcd->rsrc_len);
+	if (!hcd->regs) {
+		printk(KERN_ERR __FILE__ ": ioremap failed\n");
+		error = -EBUSY;
+		goto err_ioremap;
+	}
+
+	ehci = hcd_to_ehci(hcd);
+	ehci->caps = hcd->regs;
+	ehci->regs = hcd->regs +
+			HC_LENGTH(ehci_readl(ehci, &ehci->caps->hc_capbase));
+
+	/* cache this readonly data; minimize chip reads */
+	ehci->hcs_params = ehci_readl(ehci, &ehci->caps->hcs_params);
+
+	error = usb_add_hcd(hcd, irq, 0);
+	if (error == 0)
+		return 0;
+
+	iounmap(hcd->regs);
+err_ioremap:
+	irq_dispose_mapping(irq);
+err_irq:
+	usb_put_hcd(hcd);
+out:
+	return error;
+}
+
+
+static int ehci_hcd_hlwd_remove(struct of_device *op)
+{
+	struct usb_hcd *hcd = dev_get_drvdata(&op->dev);
+
+	dev_set_drvdata(&op->dev, NULL);
+
+	dev_dbg(&op->dev, "stopping " DRV_MODULE_NAME " USB Controller\n");
+
+	usb_remove_hcd(hcd);
+	iounmap(hcd->regs);
+	irq_dispose_mapping(hcd->irq);
+	usb_put_hcd(hcd);
+
+	return 0;
+}
+
+
+static int ehci_hcd_hlwd_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 ehci_hcd_hlwd_match[] = {
+	{ .compatible = "nintendo,hollywood-usb-ehci", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, ehci_hcd_hlwd_match);
+
+static struct of_platform_driver ehci_hcd_hlwd_driver = {
+	.name		= DRV_MODULE_NAME,
+	.match_table	= ehci_hcd_hlwd_match,
+	.probe		= ehci_hcd_hlwd_probe,
+	.remove		= ehci_hcd_hlwd_remove,
+	.shutdown	= ehci_hcd_hlwd_shutdown,
+	.driver		= {
+		.name	= DRV_MODULE_NAME,
+		.owner	= THIS_MODULE,
+	},
+};
+
diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
index 2d85e21..8fb519b 100644
--- a/drivers/usb/host/ehci.h
+++ b/drivers/usb/host/ehci.h
@@ -599,6 +599,27 @@ ehci_port_speed(struct ehci_hcd *ehci, unsigned int portsc)
 #define ehci_big_endian_mmio(e)		0
 #endif
 
+#ifdef CONFIG_USB_EHCI_HCD_HLWD
+
+/*
+ * The Nintendo Wii video game console has no PCI hardware.
+ * The USB controllers are part of the "Hollywood" chipset and are
+ * accessed via the platform internal I/O accessors.
+ */
+static inline unsigned int ehci_readl(const struct ehci_hcd *ehci,
+				      __u32 __iomem *regs)
+{
+	return in_be32(regs);
+}
+
+static inline void ehci_writel(const struct ehci_hcd *ehci,
+			       const unsigned int val, __u32 __iomem *regs)
+{
+	out_be32(regs, val);
+}
+
+#else
+
 /*
  * Big-endian read/write functions are arch-specific.
  * Other arches can be added if/when they're needed.
@@ -632,6 +653,8 @@ static inline void ehci_writel(const struct ehci_hcd *ehci,
 #endif
 }
 
+#endif /* CONFIG_USB_EHCI_HCD_HLWD */
+
 /*
  * On certain ppc-44x SoC there is a HW issue, that could only worked around with
  * explicit suspend/operate of OHCI. This function hereby makes sense only on that arch.
-- 
1.6.3.3

Re: [RFC PATCH 1/2] USB: add HCD_BOUNCE_BUFFERS host controller driver flag

[Greg KH]

On Wed, Feb 03, 2010 at 07:30:39PM +0100, Albert Herranz wrote:
> +/**
> + * hcd_memcpy32_to_coherent - copy data to a bounce buffer
> + * @dst: destination dma bounce buffer
> + * @src: source buffer
> + * @len: number of bytes to copy
> + *
> + * This function copies @len bytes from @src to @dst in 32 bit chunks.
> + * The caller must guarantee that @dst length is 4 byte aligned and
> + * that @dst length is greater than or equal to @src length.
> + */
> +static void *hcd_memcpy32_to_coherent(void *dst, const void *src, size_t len)

Why isn't there platform-specific functions for this already?  It seems
a bit odd to bury them in the USB hcd core, when I'm sure that other
people need these, if they haven't already created them.

thanks,

greg k-h

Re: [RFC PATCH 1/2] USB: add HCD_BOUNCE_BUFFERS host controller driver flag

[Alan Stern]

On Wed, 3 Feb 2010, Albert Herranz wrote:

> The HCD_BOUNCE_BUFFERS USB host controller driver flag can be enabled
> to instruct the USB stack to always bounce USB buffers to/from coherent
> memory buffers _just_ before/after a host controller transmission.
> 
> This setting allows overcoming some platform-specific limitations.
> 
> For example, the Nintendo Wii video game console is a NOT_COHERENT_CACHE
> platform that is unable to safely perform non-32 bit uncached writes
> to RAM because the byte enables are not connected to the bus.
> Thus, in that platform, "coherent" DMA buffers cannot be directly used
> by the kernel code unless it guarantees that all write accesses
> to said buffers are done in 32 bit chunks (which is not the case in the
> USB subsystem).
> 
> To avoid this unwanted behaviour HCD_BOUNCE_BUFFERS can be enabled at
> the HCD controller, causing buffer allocations to be satisfied from
> normal memory and, only at the very last moment, before the actual
> transfer, buffers get copied to/from their corresponding DMA coherent
> bounce buffers.
> 
> Note that HCD_LOCAL_MEM doesn't help in solving this problem as in that
> case buffers may be allocated from coherent memory in the first place
> and thus potentially accessed in non-32 bit chuncks by USB drivers.

This description sounds hopelessly confused.  Maybe you're just
misusing the term "coherent".  The patch itself doesn't affect the
coherent DMA mappings anyway; it affects the streaming mappings.  Or to
put it another way, what's the justification for replacing a call to
dma_map_single() with a call to dma_alloc_coherent()?

Since the patch doesn't affect any of the coherent mappings (see for 
example the calls to dma_pool_create() in ehci-mem.c), I don't see how 
it can possibly do what you claim.

> +/**
> + * hcd_memcpy32_to_coherent - copy data to a bounce buffer
> + * @dst: destination dma bounce buffer
> + * @src: source buffer
> + * @len: number of bytes to copy
> + *
> + * This function copies @len bytes from @src to @dst in 32 bit chunks.
> + * The caller must guarantee that @dst length is 4 byte aligned and
> + * that @dst length is greater than or equal to @src length.
> + */
> +static void *hcd_memcpy32_to_coherent(void *dst, const void *src, size_t len)
> +{
> +	u32 *q = dst, *p = (void *)src;
> +	u8 *s;
> +
> +	while (len >= 4) {
> +		*q++ = *p++;
> +		len -= 4;
> +	}
> +	s = (u8 *)p;
> +	switch (len) {
> +	case 3:
> +		*q = s[0] << 24 | s[1] << 16 | s[2] << 8;
> +		break;
> +	case 2:
> +		*q = s[0] << 24 | s[1] << 16;
> +		break;
> +	case 1:
> +		*q = s[0] << 24;
> +		break;
> +	default:
> +		break;
> +	}
> +	return dst;
> +}

What happens if somebody tries to use this code on a little-endian CPU?

>  static int map_urb_for_dma(struct usb_hcd *hcd, struct urb *urb,
>  			   gfp_t mem_flags)
>  {
> @@ -1274,53 +1441,80 @@ static int map_urb_for_dma(struct usb_hcd *hcd, struct urb *urb,
>  	if (is_root_hub(urb->dev))
>  		return 0;
>  
> -	if (usb_endpoint_xfer_control(&urb->ep->desc)
> -	    && !(urb->transfer_flags & URB_NO_SETUP_DMA_MAP)) {
> -		if (hcd->self.uses_dma) {
> -			urb->setup_dma = dma_map_single(
> -					hcd->self.controller,
> -					urb->setup_packet,
> -					sizeof(struct usb_ctrlrequest),
> -					DMA_TO_DEVICE);
> -			if (dma_mapping_error(hcd->self.controller,
> -						urb->setup_dma))
> -				return -EAGAIN;
> -		} else if (hcd->driver->flags & HCD_LOCAL_MEM)
> -			ret = hcd_alloc_coherent(
> -					urb->dev->bus, mem_flags,
> +	if (usb_endpoint_xfer_control(&urb->ep->desc)) {
> +		if (hcd->driver->flags & HCD_BOUNCE_BUFFERS) {
> +			if (!(urb->transfer_flags & URB_NO_SETUP_DMA_MAP))
> +				urb->setup_dma = 0;
> +			ret = hcd_bounce_to_coherent(
> +					hcd->self.controller, mem_flags,
>  					&urb->setup_dma,
>  					(void **)&urb->setup_packet,
>  					sizeof(struct usb_ctrlrequest),
>  					DMA_TO_DEVICE);
> +		} else if (!(urb->transfer_flags & URB_NO_SETUP_DMA_MAP)) {
> +			if (hcd->self.uses_dma) {
> +				urb->setup_dma = dma_map_single(
> +						hcd->self.controller,
> +						urb->setup_packet,
> +						sizeof(struct usb_ctrlrequest),
> +						DMA_TO_DEVICE);
> +				if (dma_mapping_error(hcd->self.controller,
> +						      urb->setup_dma))
> +					return -EAGAIN;
> +			} else if (hcd->driver->flags & HCD_LOCAL_MEM)
> +				ret = hcd_alloc_coherent(
> +						urb->dev->bus, mem_flags,
> +						&urb->setup_dma,
> +						(void **)&urb->setup_packet,
> +						sizeof(struct usb_ctrlrequest),
> +						DMA_TO_DEVICE);
> +		}
>  	}
>  
>  	dir = usb_urb_dir_in(urb) ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
> -	if (ret == 0 && urb->transfer_buffer_length != 0
> -	    && !(urb->transfer_flags & URB_NO_TRANSFER_DMA_MAP)) {
> -		if (hcd->self.uses_dma) {
> -			urb->transfer_dma = dma_map_single (
> -					hcd->self.controller,
> -					urb->transfer_buffer,
> -					urb->transfer_buffer_length,
> -					dir);
> -			if (dma_mapping_error(hcd->self.controller,
> -						urb->transfer_dma))
> -				return -EAGAIN;
> -		} else if (hcd->driver->flags & HCD_LOCAL_MEM) {
> -			ret = hcd_alloc_coherent(
> -					urb->dev->bus, mem_flags,
> +	if (ret == 0 && urb->transfer_buffer_length != 0) {
> +		if (hcd->driver->flags & HCD_BOUNCE_BUFFERS) {
> +			if (!(urb->transfer_flags & URB_NO_TRANSFER_DMA_MAP))
> +				urb->transfer_dma = 0;
> +			ret = hcd_bounce_to_coherent(
> +					hcd->self.controller, mem_flags,
>  					&urb->transfer_dma,
>  					&urb->transfer_buffer,
>  					urb->transfer_buffer_length,
>  					dir);
>  
> -			if (ret && usb_endpoint_xfer_control(&urb->ep->desc)
> -			    && !(urb->transfer_flags & URB_NO_SETUP_DMA_MAP))
> -				hcd_free_coherent(urb->dev->bus,
> -					&urb->setup_dma,
> -					(void **)&urb->setup_packet,
> -					sizeof(struct usb_ctrlrequest),
> -					DMA_TO_DEVICE);
> +			if (ret && usb_endpoint_xfer_control(&urb->ep->desc))
> +				hcd_bounce_from_coherent(hcd->self.controller,
> +						&urb->setup_dma,
> +						(void **)&urb->setup_packet,
> +						sizeof(struct usb_ctrlrequest),
> +						DMA_TO_DEVICE);
> +		} else if (!(urb->transfer_flags & URB_NO_TRANSFER_DMA_MAP)) {
> +			if (hcd->self.uses_dma) {
> +				urb->transfer_dma = dma_map_single(
> +						hcd->self.controller,
> +						urb->transfer_buffer,
> +						urb->transfer_buffer_length,
> +						dir);
> +				if (dma_mapping_error(hcd->self.controller,
> +						      urb->transfer_dma))
> +					return -EAGAIN;
> +			} else if (hcd->driver->flags & HCD_LOCAL_MEM) {
> +				ret = hcd_alloc_coherent(
> +						urb->dev->bus, mem_flags,
> +						&urb->transfer_dma,
> +						&urb->transfer_buffer,
> +						urb->transfer_buffer_length,
> +						dir);
> +
> +				if (ret &&
> +				    usb_endpoint_xfer_control(&urb->ep->desc))
> +					hcd_free_coherent(urb->dev->bus,
> +						&urb->setup_dma,
> +						(void **)&urb->setup_packet,
> +						sizeof(struct usb_ctrlrequest),
> +						DMA_TO_DEVICE);
> +			}
>  		}
>  	}
>  	return ret;

It seems that every time somebody comes up with a new kind of 
memory-access restriction, this function grows by a factor of 2.  After 
a few more iterations it will be larger than the rest of the kernel!

There must be a better way to structure the requirements here.

Alan Stern

Re: [RFC PATCH 1/2] USB: add HCD_BOUNCE_BUFFERS host controller driver flag

[Albert Herranz]

Hi Alan,

Alan Stern wrote:
> This description sounds hopelessly confused.  Maybe you're just
> misusing the term "coherent".  The patch itself doesn't affect the
> coherent DMA mappings anyway; it affects the streaming mappings.  Or to
> put it another way, what's the justification for replacing a call to
> dma_map_single() with a call to dma_alloc_coherent()?
> 
> Since the patch doesn't affect any of the coherent mappings (see for 
> example the calls to dma_pool_create() in ehci-mem.c), I don't see how 
> it can possibly do what you claim.
> 

Thanks for your comments. Let's try to hopefully clarify this a bit.

I've used the term "coherent" as described in Documentation/DMA-API.txt (aka "consistent" as used in PCI-related functions).
I've tried to describe first the limitations of the platform that I'm working on. Basically, one of the annoying things of that platform is that writes to uncached memory (as used in "coherent" memory) can only be reliably performed in 32-bit accesses.

The USB subsystem ends up using "coherent" memory for buffers and/or other structures in different ways.

The "coherent" memory allocated in dma_pool_create() in ehci-mem.c that you report is not a problem at all because it is always accessed in 32-bit chunks (it hasn't been always like that but since commit 3807e26d69b9ad3864fe03224ebebc9610d5802e "USB: EHCI: split ehci_qh into hw and sw parts" this got addressed as a side effect, so I didn't need to post another patch for that).

Other possible interactions with "coherent" memory are those involving buffers used in USB transactions, which may be allocated via the USB subsystem (at usb_buffer_alloc() or when bounced via hcd_alloc_coherent()) or which may come already allocated and ready for use (URB_NO_{SETUP,TRANSFER}_DMA_MAP).

The patch, as posted, allocates normal memory for USB buffers _within_ the USB subsystem and invariably bounces all buffers to new "coherent" buffers.
So, basically, what the patch claims (avoid 32-bit writes for "coherent" memory within the USB subsystem) is "done" (hey, it actually works ;-).

But I think you have raised valid points here :)

If the "coherent" memory is already allocated and passed (as already dma-mapped) to the USB subsystem then there is no gain in bouncing the buffer:
- if a non-32 bit write was done to that "coherent" memory the damage is already done
- if the "coherent" memory was written always in 32-bit accesses then we can just safely use it
So bouncing here should be avoided as it is unneeded.

On the other hand, we can control USB buffers managed by the USB subsystem itself.
That's what the patch "does". It avoids access restrictions to USB buffers by allocating them from normal memory (leaving USB drivers free to access those buffers in whatever bus width they need, as they do today) ... and bouncing them.
The thing here is that it makes no sense to bounce them to "coherent" memory if they can be dma-mapped directly (as you point in your dma_map_single-vs-dma_alloc_coherent comment).

So... that's what RFCs are for :)
I'll take a look again at the patch.

>> +/**
>> + * hcd_memcpy32_to_coherent - copy data to a bounce buffer
>> + * @dst: destination dma bounce buffer
>> + * @src: source buffer
>> + * @len: number of bytes to copy
>> + *
>> + * This function copies @len bytes from @src to @dst in 32 bit chunks.
>> + * The caller must guarantee that @dst length is 4 byte aligned and
>> + * that @dst length is greater than or equal to @src length.
>> + */
>> +static void *hcd_memcpy32_to_coherent(void *dst, const void *src, size_t len)
>> +{
>> +	u32 *q = dst, *p = (void *)src;
>> +	u8 *s;
>> +
>> +	while (len >= 4) {
>> +		*q++ = *p++;
>> +		len -= 4;
>> +	}
>> +	s = (u8 *)p;
>> +	switch (len) {
>> +	case 3:
>> +		*q = s[0] << 24 | s[1] << 16 | s[2] << 8;
>> +		break;
>> +	case 2:
>> +		*q = s[0] << 24 | s[1] << 16;
>> +		break;
>> +	case 1:
>> +		*q = s[0] << 24;
>> +		break;
>> +	default:
>> +		break;
>> +	}
>> +	return dst;
>> +}
> 
> What happens if somebody tries to use this code on a little-endian CPU?
> 

It will fail.


> It seems that every time somebody comes up with a new kind of 
> memory-access restriction, this function grows by a factor of 2.  After 
> a few more iterations it will be larger than the rest of the kernel!
> 
> There must be a better way to structure the requirements here.
> 

Hopefully I didn't miss any of your concerns and managed to explain the problem.

> Alan Stern
> 
> 

Thanks,
Albert

Re: [RFC PATCH 1/2] USB: add HCD_BOUNCE_BUFFERS host controller driver flag

[Alan Stern]

On Thu, 4 Feb 2010, Albert Herranz wrote:

> Hi Alan,
> 
> Alan Stern wrote:
> > This description sounds hopelessly confused.  Maybe you're just
> > misusing the term "coherent".  The patch itself doesn't affect the
> > coherent DMA mappings anyway; it affects the streaming mappings.  Or to
> > put it another way, what's the justification for replacing a call to
> > dma_map_single() with a call to dma_alloc_coherent()?
> > 
> > Since the patch doesn't affect any of the coherent mappings (see for 
> > example the calls to dma_pool_create() in ehci-mem.c), I don't see how 
> > it can possibly do what you claim.
> > 
> 
> Thanks for your comments. Let's try to hopefully clarify this a bit.
> 
> I've used the term "coherent" as described in Documentation/DMA-API.txt (aka "consistent" as used in PCI-related functions).
> I've tried to describe first the limitations of the platform that I'm working on. Basically, one of the annoying things of that platform is that writes to uncached memory (as used in "coherent" memory) can only be reliably performed in 32-bit accesses.
> 
> The USB subsystem ends up using "coherent" memory for buffers and/or other structures in different ways.
> 
> The "coherent" memory allocated in dma_pool_create() in ehci-mem.c that you report is not a problem at all because it is always accessed in 32-bit chunks (it hasn't been always like that but since commit 3807e26d69b9ad3864fe03224ebebc9610d5802e "USB: EHCI: split ehci_qh into hw and sw parts" this got addressed as a side effect, so I didn't need to post another patch for that).

On a 64-bit processor, some of the accesses will be 64 bits wide
instead of 32.  Does that matter for your purposes?

What about ohci-hcd and uhci-hcd?  They both use non-32-bit accesses to 
structures in coherent memory.

> Other possible interactions with "coherent" memory are those involving buffers used in USB transactions, which may be allocated via the USB subsystem (at usb_buffer_alloc() or when bounced via hcd_alloc_coherent()) or which may come already allocated and ready for use (URB_NO_{SETUP,TRANSFER}_DMA_MAP).

Ah yes, quite correct.  And this indicates that you need to concentrate
on usb_buffer_alloc().  On your system (or rather, whenever the
HCD_NO_COHERENT_MEM flag is set) it should allocate normal memory and
set the DMA pointer to NULL.

Then map_urb_for_dma() should check the urb->setup_dma and
urb->transfer_dma pointers; if a pointer is NULL then the
corresponding urb->URB_NO_SETUP_DMA_MAP or urb->NO_TRANSFER_DMA_MAP
flag should be ignored (and probably should be cleared so as to 
avoid confusing unmap_urb_for_dma()).

> The patch, as posted, allocates normal memory for USB buffers _within_ the USB subsystem and invariably bounces all buffers to new "coherent" buffers.
> So, basically, what the patch claims (avoid 32-bit writes for "coherent" memory within the USB subsystem) is "done" (hey, it actually works ;-).
> 
> But I think you have raised valid points here :)
> 
> If the "coherent" memory is already allocated and passed (as already dma-mapped) to the USB subsystem then there is no gain in bouncing the buffer:
> - if a non-32 bit write was done to that "coherent" memory the damage is already done
> - if the "coherent" memory was written always in 32-bit accesses then we can just safely use it
> So bouncing here should be avoided as it is unneeded.
> 
> On the other hand, we can control USB buffers managed by the USB subsystem itself.
> That's what the patch "does". It avoids access restrictions to USB buffers by allocating them from normal memory (leaving USB drivers free to access those buffers in whatever bus width they need, as they do today) ... and bouncing them.
> The thing here is that it makes no sense to bounce them to "coherent" memory if they can be dma-mapped directly (as you point in your dma_map_single-vs-dma_alloc_coherent comment).
> 
> So... that's what RFCs are for :)

If you do it as described above then the buffers you're worried about
won't be allocated in coherent memory to begin with, so no problems 
will arise.

Alan Stern

Re: [RFC PATCH 1/2] USB: add HCD_BOUNCE_BUFFERS host controller driver flag

[Albert Herranz]

Alan Stern wrote:
>On a 64-bit processor, some of the accesses will be 64 bits wide
>instead of 32.  Does that matter for your purposes?
>

The wii uses a 32-bit processor, so this is safe in this case.

>What about ohci-hcd and uhci-hcd?  They both use non-32-bit accesses to 
>structures in coherent memory.
>

The wii has no uhci, but has 2 ohci controllers.
For ohci we need a similar approach as done for ehci.

>If you do it as described above then the buffers you're worried about
>won't be allocated in coherent memory to begin with, so no problems 
>will arise.

It turns out that we have more limitations.
The wii has 2 discontiguous memory areas (usually called MEM1 and MEM2). I have checked that the ehci controller doesn't work properly when performing dma to buffers allocated in MEM1 (it corrupts part of the data) but has no problems if the buffers sit within MEM2.
So usb buffers will need to be bounced anyway if they are part of MEM1.

This worked in the original patch as buffers were always bounced to MEM2 buffers. Sigh.

>Alan Stern
>

Thanks,
Albert

PS: Your reply didn't get to me. I looked at the ML and found it (I'm not subscribed). Sorry for the late answer.

Re: [RFC PATCH 1/2] USB: add HCD_BOUNCE_BUFFERS host controller driver flag

[Alan Stern]

On Sun, 7 Feb 2010, Albert Herranz wrote:

> The wii has no uhci, but has 2 ohci controllers.
> For ohci we need a similar approach as done for ehci.

So you'll need to write a patch splitting up the OHCI data structures 
in the same way the EHCI qh was split up.

> >If you do it as described above then the buffers you're worried about
> >won't be allocated in coherent memory to begin with, so no problems 
> >will arise.
> 
> It turns out that we have more limitations.
> The wii has 2 discontiguous memory areas (usually called MEM1 and MEM2). I have checked that the ehci controller doesn't work properly when performing dma to buffers allocated in MEM1 (it corrupts part of the data) but has no problems if the buffers sit within MEM2.
> So usb buffers will need to be bounced anyway if they are part of MEM1.

This sounds like the sort of restriction that dma_map_single() should 
be capable of handling.

Alan Stern

Re: [RFC PATCH 1/2] USB: add HCD_BOUNCE_BUFFERS host controller driver flag

[Albert Herranz]

Alan Stern wrote:
> On Sun, 7 Feb 2010, Albert Herranz wrote:
> 
>> The wii has no uhci, but has 2 ohci controllers.
>> For ohci we need a similar approach as done for ehci.
> 
> So you'll need to write a patch splitting up the OHCI data structures 
> in the same way the EHCI qh was split up.
> 

Yes.

>> It turns out that we have more limitations.
>> The wii has 2 discontiguous memory areas (usually called MEM1 and MEM2). I have checked that the ehci controller doesn't work properly when performing dma to buffers allocated in MEM1 (it corrupts part of the data) but has no problems if the buffers sit within MEM2.
>> So usb buffers will need to be bounced anyway if they are part of MEM1.
> 
> This sounds like the sort of restriction that dma_map_single() should 
> be capable of handling.
> 

On powerpc you can have per-device specific dma ops.
I'll work on that direction and create a special dma ops set for devices which need their dma buffers on mem2, and then use those for ehci-hlwd.

> Alan Stern
> 

Thanks,
Albert