Discussion:
[PATCH][0/2] usb: sm501 ohci controller patches V3/V2
Magnus Damm
2008-01-22 03:20:02 UTC
Permalink
usb: sm501 ohci controller patches V3/V2

[1/2] usb: dma bounce buffer support V3
[2/2] usb: ohci-sm501 driver V2

The sm501 usb host controller is a regular ohci implementation but with the
exception that it can only do dma to a certain area of memory. This memory is
sm501 specific and the mfd driver for sm501 currently divides this memory
between the framebuffer and the usb host controller.

To support the sm501 usb host controller we need to allocate all usb data from
this sm501 memory. The function dma_declare_coherent_memory() helps us with
that - all dma_alloc_coherent() calls will end up allocating such memory.

The USB core does however allocate data using kmalloc, so to fully support
such a behaviour we need dma bounce buffer support. These bounce buffers
can be requested using the flag HCD_LOCAL_MEM.

Signed-off-by: Magnus Damm <***@igel.co.jp>
---

drivers/usb/core/buffer.c | 9 -
drivers/usb/core/hcd.c | 134 +++++++++++++++++---
drivers/usb/core/hcd.h | 1
drivers/usb/host/ohci-hcd.c | 5
drivers/usb/host/ohci-sm501.c | 264 +++++++++++++++++++++++++++++++++++++++++
5 files changed, 391 insertions(+), 22 deletions(-)

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
linux-usb-***@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
Magnus Damm
2008-01-22 03:20:10 UTC
Permalink
usb: dma bounce buffer support V3

This patch adds dma bounce buffer support to the usb core. These buffers
can be enabled with the HCD_LOCAL_MEM flag, and they make sure that all data
passed to the host controller is allocated using dma_alloc_coherent().

Signed-off-by: Magnus Damm <***@igel.co.jp>
Acked-by: Alan Stern <***@rowland.harvard.edu>
---

drivers/usb/core/buffer.c | 9 ++-
drivers/usb/core/hcd.c | 134 ++++++++++++++++++++++++++++++++++++++-------
drivers/usb/core/hcd.h | 1
3 files changed, 122 insertions(+), 22 deletions(-)

--- 0001/drivers/usb/core/buffer.c
+++ work/drivers/usb/core/buffer.c 2008-01-21 17:48:52.000000000 +0900
@@ -53,7 +53,8 @@ int hcd_buffer_create(struct usb_hcd *hc
char name[16];
int i, size;

- if (!hcd->self.controller->dma_mask)
+ if (!hcd->self.controller->dma_mask &&
+ !(hcd->driver->flags & HCD_LOCAL_MEM))
return 0;

for (i = 0; i < HCD_BUFFER_POOLS; i++) {
@@ -107,7 +108,8 @@ void *hcd_buffer_alloc(
int i;

/* some USB hosts just use PIO */
- if (!bus->controller->dma_mask) {
+ if (!bus->controller->dma_mask &&
+ !(hcd->driver->flags & HCD_LOCAL_MEM)) {
*dma = ~(dma_addr_t) 0;
return kmalloc(size, mem_flags);
}
@@ -132,7 +134,8 @@ void hcd_buffer_free(
if (!addr)
return;

- if (!bus->controller->dma_mask) {
+ if (!bus->controller->dma_mask &&
+ !(hcd->driver->flags & HCD_LOCAL_MEM)) {
kfree(addr);
return;
}
--- 0001/drivers/usb/core/hcd.c
+++ work/drivers/usb/core/hcd.c 2008-01-21 17:55:15.000000000 +0900
@@ -35,6 +35,7 @@
#include <linux/mutex.h>
#include <asm/irq.h>
#include <asm/byteorder.h>
+#include <asm/unaligned.h>
#include <linux/platform_device.h>
#include <linux/workqueue.h>

@@ -1112,48 +1113,137 @@ void usb_hcd_unlink_urb_from_ep(struct u
}
EXPORT_SYMBOL_GPL(usb_hcd_unlink_urb_from_ep);

-static void map_urb_for_dma(struct usb_hcd *hcd, struct urb *urb)
+static int hcd_alloc_coherent(struct usb_bus *bus,
+ gfp_t mem_flags, dma_addr_t *dma_handle,
+ void **vaddr_handle, size_t size,
+ enum dma_data_direction dir)
{
+ unsigned char *vaddr;
+
+ vaddr = hcd_buffer_alloc(bus, size + sizeof(vaddr),
+ mem_flags, dma_handle);
+ if (!vaddr)
+ return -ENOMEM;
+
+ put_unaligned((unsigned long)*vaddr_handle,
+ (unsigned long *)(vaddr + size));
+
+ if (dir == DMA_TO_DEVICE)
+ memcpy(vaddr, *vaddr_handle, size);
+
+ *vaddr_handle = vaddr;
+ return 0;
+}
+
+static void hcd_free_coherent(struct usb_bus *bus, dma_addr_t *dma_handle,
+ void **vaddr_handle, size_t size,
+ enum dma_data_direction dir)
+{
+ unsigned char *vaddr = *vaddr_handle;
+
+ vaddr = (void *)get_unaligned((unsigned long *)(vaddr + size));
+
+ if (dir == DMA_FROM_DEVICE)
+ memcpy(vaddr, *vaddr_handle, size);
+
+ hcd_buffer_free(bus, size + sizeof(vaddr), *vaddr_handle, *dma_handle);
+
+ *vaddr_handle = vaddr;
+ *dma_handle = 0;
+}
+
+static int map_urb_for_dma(struct usb_hcd *hcd, struct urb *urb,
+ gfp_t mem_flags)
+{
+ enum dma_data_direction dir;
+ int ret = 0;
+
/* Map the URB's buffers for DMA access.
* Lower level HCD code should use *_dma exclusively,
* unless it uses pio or talks to another transport.
*/
- if (hcd->self.uses_dma && !is_root_hub(urb->dev)) {
- if (usb_endpoint_xfer_control(&urb->ep->desc)
- && !(urb->transfer_flags & URB_NO_SETUP_DMA_MAP))
- urb->setup_dma = dma_map_single (
+ 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 (urb->transfer_buffer_length != 0
- && !(urb->transfer_flags & URB_NO_TRANSFER_DMA_MAP))
+ 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,
- usb_urb_dir_in(urb)
- ? DMA_FROM_DEVICE
- : DMA_TO_DEVICE);
+ dir);
+ 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)
+ && !(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);
+ }
}
+ return ret;
}

static void unmap_urb_for_dma(struct usb_hcd *hcd, struct urb *urb)
{
- if (hcd->self.uses_dma && !is_root_hub(urb->dev)) {
- if (usb_endpoint_xfer_control(&urb->ep->desc)
- && !(urb->transfer_flags & URB_NO_SETUP_DMA_MAP))
+ enum dma_data_direction dir;
+
+ 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);
- if (urb->transfer_buffer_length != 0
- && !(urb->transfer_flags & URB_NO_TRANSFER_DMA_MAP))
+ 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,
- usb_urb_dir_in(urb)
- ? DMA_FROM_DEVICE
- : DMA_TO_DEVICE);
+ 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);
}
}

@@ -1185,7 +1275,12 @@ int usb_hcd_submit_urb (struct urb *urb,
* URBs must be submitted in process context with interrupts
* enabled.
*/
- map_urb_for_dma(hcd, urb);
+ status = map_urb_for_dma(hcd, urb, mem_flags);
+ if (unlikely(status)) {
+ usbmon_urb_submit_error(&hcd->self, urb, status);
+ goto error;
+ }
+
if (is_root_hub(urb->dev))
status = rh_urb_enqueue(hcd, urb);
else
@@ -1194,6 +1289,7 @@ int usb_hcd_submit_urb (struct urb *urb,
if (unlikely(status)) {
usbmon_urb_submit_error(&hcd->self, urb, status);
unmap_urb_for_dma(hcd, urb);
+ error:
urb->hcpriv = NULL;
INIT_LIST_HEAD(&urb->urb_list);
atomic_dec(&urb->use_count);
--- 0001/drivers/usb/core/hcd.h
+++ work/drivers/usb/core/hcd.h 2008-01-21 17:48:52.000000000 +0900
@@ -165,6 +165,7 @@ struct hc_driver {

int flags;
#define HCD_MEMORY 0x0001 /* HC regs use memory (else I/O) */
+#define HCD_LOCAL_MEM 0x0002 /* HC needs dma_alloc_coherent() */
#define HCD_USB11 0x0010 /* USB 1.1 */
#define HCD_USB2 0x0020 /* USB 2.0 */


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
linux-usb-***@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
Andrew Morton
2008-01-22 05:01:23 UTC
Permalink
Post by Magnus Damm
usb: dma bounce buffer support V3
This patch adds dma bounce buffer support to the usb core. These buffers
can be enabled with the HCD_LOCAL_MEM flag, and they make sure that all data
passed to the host controller is allocated using dma_alloc_coherent().
It is unobvious (to me) why this code uses put_unaligned() and
get_unaligned() in the places where it does. So I'll assume that it will
be unobvious to other reviewers and readers.

Addition of suitable comments would help here. Unless this is all
blindingly obvious to anyone who has a useful knowledge of USB core?



-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
linux-usb-***@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
David Brownell
2008-01-22 05:17:57 UTC
Permalink
Post by Andrew Morton
It is unobvious (to me) why this code uses put_unaligned() and
get_unaligned() in the places where it does.  So I'll assume that it will
be unobvious to other reviewers and readers.  
It was unobvious to me too. "Shouldn't be needed" was one
of my as-yet-unsent comments on this round.
Post by Andrew Morton
Addition of suitable comments would help here.  Unless this is all
blindingly obvious to anyone who has a useful knowledge of USB core?
It's all a bit of a hack to _portably_ support controllers that
require their buffers to live in a particular (local) memory
block. There's some dmabounce code in the ARM stack, but that
won't work for SH and doesn't easily accomodate the "local"
semantics.

And again, one of my as-yet-unsent comments was about this very
issue. (The comments that are there are a bit sparse and don't
touch on the real issues.) Very optimistically, this means one
of us can reduce (or at least shift!) our load of code reviews. ;)

- Dave




-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
linux-usb-***@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
Magnus Damm
2008-01-22 06:12:53 UTC
Permalink
Post by Andrew Morton
Post by Magnus Damm
usb: dma bounce buffer support V3
This patch adds dma bounce buffer support to the usb core. These buffers
can be enabled with the HCD_LOCAL_MEM flag, and they make sure that all data
passed to the host controller is allocated using dma_alloc_coherent().
It is unobvious (to me) why this code uses put_unaligned() and
get_unaligned() in the places where it does. So I'll assume that it will
be unobvious to other reviewers and readers.
The HCD_LOCAL_MEM dma bounce buffer code is using unaligned functions
to temporarily save a pointer at the end of the dma data buffer.
hcd_alloc_coherent() saves the pointer which is needed by
hcd_free_coherent(). The unaligned functions are used because the size
of the buffer may be uneven.

It's of course possible to round up the size instead and use aligned
access if you prefer that. The memory area used for dma may be limited
though so I felt it was natural to optimize for small memory footprint
over access speed.

Thanks,

/ magnus

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
linux-usb-***@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
Andrew Morton
2008-01-22 06:25:35 UTC
Permalink
Post by Magnus Damm
Post by Andrew Morton
Post by Magnus Damm
usb: dma bounce buffer support V3
This patch adds dma bounce buffer support to the usb core. These buffers
can be enabled with the HCD_LOCAL_MEM flag, and they make sure that all data
passed to the host controller is allocated using dma_alloc_coherent().
It is unobvious (to me) why this code uses put_unaligned() and
get_unaligned() in the places where it does. So I'll assume that it will
be unobvious to other reviewers and readers.
The HCD_LOCAL_MEM dma bounce buffer code is using unaligned functions
to temporarily save a pointer at the end of the dma data buffer.
hcd_alloc_coherent() saves the pointer which is needed by
hcd_free_coherent(). The unaligned functions are used because the size
of the buffer may be uneven.
It's of course possible to round up the size instead and use aligned
access if you prefer that. The memory area used for dma may be limited
though so I felt it was natural to optimize for small memory footprint
over access speed.
that wasn't a patch ;)

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
linux-usb-***@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
Magnus Damm
2008-01-22 06:50:58 UTC
Permalink
Post by Andrew Morton
Post by Magnus Damm
Post by Andrew Morton
Post by Magnus Damm
usb: dma bounce buffer support V3
This patch adds dma bounce buffer support to the usb core. These buffers
can be enabled with the HCD_LOCAL_MEM flag, and they make sure that all data
passed to the host controller is allocated using dma_alloc_coherent().
It is unobvious (to me) why this code uses put_unaligned() and
get_unaligned() in the places where it does. So I'll assume that it will
be unobvious to other reviewers and readers.
The HCD_LOCAL_MEM dma bounce buffer code is using unaligned functions
to temporarily save a pointer at the end of the dma data buffer.
hcd_alloc_coherent() saves the pointer which is needed by
hcd_free_coherent(). The unaligned functions are used because the size
of the buffer may be uneven.
It's of course possible to round up the size instead and use aligned
access if you prefer that. The memory area used for dma may be limited
though so I felt it was natural to optimize for small memory footprint
over access speed.
that wasn't a patch ;)
Gotcha. Sorry about the attachment - I have no idea how to reply
inline and not mangle patches using gmail...

/ magnus
David Brownell
2008-01-22 08:19:35 UTC
Permalink
Post by Magnus Damm
usb: dma bounce buffer support V3
This patch adds dma bounce buffer support to the usb core. These buffers
can be enabled with the HCD_LOCAL_MEM flag, and they make sure that all data
passed to the host controller is allocated using dma_alloc_coherent().
And FWIW, after the updates:

Acked-by: David Brownell <***@users.sourceforge.net>

You already got most of my first-round comments addressed (in V2)
but one more is the "needs comments" issue. See below.
Post by Magnus Damm
--- 0001/drivers/usb/core/hcd.c
+++ work/drivers/usb/core/hcd.c 2008-01-21 17:55:15.000000000 +0900
...
+static int map_urb_for_dma(struct usb_hcd *hcd, struct urb *urb,
+ gfp_t mem_flags)
+{
+ enum dma_data_direction dir;
+ int ret = 0;
+
/* Map the URB's buffers for DMA access.
* Lower level HCD code should use *_dma exclusively,
* unless it uses pio or talks to another transport.
*/
- if (hcd->self.uses_dma && !is_root_hub(urb->dev)) {
- if (usb_endpoint_xfer_control(&urb->ep->desc)
- && !(urb->transfer_flags & URB_NO_SETUP_DMA_MAP))
- urb->setup_dma = dma_map_single (
+ 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 (urb->transfer_buffer_length != 0
- && !(urb->transfer_flags & URB_NO_TRANSFER_DMA_MAP))
Right about here, I think, is where a comment is needed to explain
what's really going on here. Key points:

- We need "local" memory, canonical example being
a small SRAM on a discrete controller being the
only memory that the controller can read ...
(a) "normal" kernel memory is no good, and
(b) there's not enough to share

- The only *portable* hook for such stuff in the
DMA framework is dma_declare_coherent_memory()

- So we use that, even though the primary requirement
is that the memory be "local" (hence addressible
by that device), not "coherent".

Someday DMA memory management may be smart enough to handle this
stuff without such workarounds. Having a clear description of
this particular sub-problem should help. Having that knowledge
living outside the source tree ... won't.
Post by Magnus Damm
+ 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,
...
--- 0001/drivers/usb/core/hcd.h
+++ work/drivers/usb/core/hcd.h 2008-01-21 17:48:52.000000000 +0900
@@ -165,6 +165,7 @@ struct hc_driver {
int flags;
#define HCD_MEMORY 0x0001 /* HC regs use memory (else I/O) */
+#define HCD_LOCAL_MEM 0x0002 /* HC needs dma_alloc_coherent() */
And again here. "HC needs local memory" is the issue.
The fact that we can only achieve it with dma_alloc_coherent()
for now is both an implementation hack, and too long to fit
onto the tail end of a #define line. ;)
Post by Magnus Damm
#define HCD_USB11 0x0010 /* USB 1.1 */
#define HCD_USB2 0x0020 /* USB 2.0 */
-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
linux-usb-***@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
Magnus Damm
2008-01-22 10:18:29 UTC
Permalink
Post by David Brownell
Post by Magnus Damm
usb: dma bounce buffer support V3
This patch adds dma bounce buffer support to the usb core. These buffers
can be enabled with the HCD_LOCAL_MEM flag, and they make sure that all data
passed to the host controller is allocated using dma_alloc_coherent().
You already got most of my first-round comments addressed (in V2)
but one more is the "needs comments" issue. See below.
Thanks for your additional comments. If the are no objections then
I'll incorporate akpm's checkpatch fix and the add more comments and
repost V4 some time tomorrow.

/ magnus

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
linux-usb-***@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
Alan Stern
2008-01-22 15:10:02 UTC
Permalink
Post by David Brownell
Post by Magnus Damm
+static int map_urb_for_dma(struct usb_hcd *hcd, struct urb *urb,
+ gfp_t mem_flags)
+{
...
Post by David Brownell
Right about here, I think, is where a comment is needed to explain
- We need "local" memory, canonical example being
a small SRAM on a discrete controller being the
only memory that the controller can read ...
(a) "normal" kernel memory is no good, and
(b) there's not enough to share
- The only *portable* hook for such stuff in the
DMA framework is dma_declare_coherent_memory()
- So we use that, even though the primary requirement
is that the memory be "local" (hence addressible
by that device), not "coherent".
Someday DMA memory management may be smart enough to handle this
stuff without such workarounds. Having a clear description of
this particular sub-problem should help. Having that knowledge
living outside the source tree ... won't.
Post by Magnus Damm
+ 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);
+ }
Isn't the logical location for these comments at the beginning of
hcd_alloc_coherent()'s definition, rather than the place where it is
called?

Alan Stern


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
linux-usb-***@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
David Brownell
2008-01-22 19:23:57 UTC
Permalink
Post by Alan Stern
Isn't the logical location for these comments at the beginning of
hcd_alloc_coherent()'s definition, rather than the place where it is
called?
I'll leave that up to Magnus ... but that's the first point where
the LOCAL_MEM flag is coupled to the "coherent dma" concept , and
I'd expect that "coherent dma" is better understood.

- Dave


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
linux-usb-***@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

Magnus Damm
2008-01-22 03:20:19 UTC
Permalink
usb: ohci-sm501 driver V2

This patch adds sm501 ohci support. It's all very straightforward with the
exception of dma_declare_coherent_memory() and HCD_LOCAL_MEM. Together they
are used to ensure that usb data is allocated using dma_alloc_coherent(),
and that only valid dma memory is used to allocate from. This driver is
a platform device, and the mfd driver sm501.c is already creating one
usb host controller instance per sm501.

Signed-off-by: Magnus Damm <***@igel.co.jp>
---

drivers/usb/host/ohci-hcd.c | 5
drivers/usb/host/ohci-sm501.c | 264 +++++++++++++++++++++++++++++++++++++++++
2 files changed, 269 insertions(+)

--- 0001/drivers/usb/host/ohci-hcd.c
+++ work/drivers/usb/host/ohci-hcd.c 2008-01-17 15:29:48.000000000 +0900
@@ -1048,6 +1048,11 @@ MODULE_LICENSE ("GPL");
#define SSB_OHCI_DRIVER ssb_ohci_driver
#endif

+#ifdef CONFIG_MFD_SM501
+#include "ohci-sm501.c"
+#define PLATFORM_DRIVER ohci_hcd_sm501_driver
+#endif
+
#if !defined(PCI_DRIVER) && \
!defined(PLATFORM_DRIVER) && \
!defined(OF_PLATFORM_DRIVER) && \
--- /dev/null
+++ work/drivers/usb/host/ohci-sm501.c 2008-01-17 15:32:26.000000000 +0900
@@ -0,0 +1,264 @@
+/*
+ * OHCI HCD (Host Controller Driver) for USB.
+ *
+ * (C) Copyright 1999 Roman Weissgaerber <***@vienna.at>
+ * (C) Copyright 2000-2005 David Brownell
+ * (C) Copyright 2002 Hewlett-Packard Company
+ * (C) Copyright 2008 Magnus Damm
+ *
+ * SM501 Bus Glue - based on ohci-omap.c
+ *
+ * This file is licenced under the GPL.
+ */
+
+#include <linux/interrupt.h>
+#include <linux/jiffies.h>
+#include <linux/platform_device.h>
+#include <linux/dma-mapping.h>
+#include <linux/sm501.h>
+#include <linux/sm501-regs.h>
+
+static int ohci_sm501_init(struct usb_hcd *hcd)
+{
+ return ohci_init(hcd_to_ohci(hcd));
+}
+
+static int ohci_sm501_start(struct usb_hcd *hcd)
+{
+ struct device *dev = hcd->self.controller;
+ int ret;
+
+ ret = ohci_run(hcd_to_ohci(hcd));
+ if (ret < 0) {
+ dev_err(dev, "can't start %s", hcd->self.bus_name);
+ ohci_stop(hcd);
+ }
+
+ return ret;
+}
+
+/*-------------------------------------------------------------------------*/
+
+static const struct hc_driver ohci_sm501_hc_driver = {
+ .description = hcd_name,
+ .product_desc = "SM501 OHCI",
+ .hcd_priv_size = sizeof(struct ohci_hcd),
+
+ /*
+ * generic hardware linkage
+ */
+ .irq = ohci_irq,
+ .flags = HCD_USB11 | HCD_MEMORY | HCD_LOCAL_MEM,
+
+ /*
+ * basic lifecycle operations
+ */
+ .reset = ohci_sm501_init,
+ .start = ohci_sm501_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_sm501_drv_probe(struct platform_device *pdev)
+{
+ const struct hc_driver *driver = &ohci_sm501_hc_driver;
+ struct device *dev = &pdev->dev;
+ struct resource *res, *mem;
+ int retval, irq;
+ struct usb_hcd *hcd = 0;
+
+ irq = retval = platform_get_irq(pdev, 0);
+ if (retval < 0)
+ goto err0;
+
+ mem = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+ if (mem == NULL) {
+ dev_err(dev, "no resource definition for memory\n");
+ retval = -ENOENT;
+ goto err0;
+ }
+
+ if (!request_mem_region(mem->start, mem->end - mem->start + 1,
+ pdev->name)) {
+ dev_err(dev, "request_mem_region failed\n");
+ retval = -EBUSY;
+ goto err0;
+ }
+
+ /* The sm501 chip is equipped with local memory that may be used
+ * by on-chip devices such as the video controller and the usb host.
+ * This driver uses dma_declare_coherent_memory() to make sure
+ * usb allocations with dma_alloc_coherent() allocate from
+ * this local memory. The dma_handle returned by dma_alloc_coherent()
+ * will be an offset starting from 0 for the first local memory byte.
+ *
+ * So as long as data is allocated using dma_alloc_coherent() all is
+ * fine. This is however not always the case - buffers may be allocated
+ * using kmalloc() - so the usb core needs to be told that it must copy
+ * data into our local memory if the buffers happen to be placed in
+ * regular memory. The HCD_BOUNCE flag does just that.
+ */
+
+ if (!dma_declare_coherent_memory(dev, mem->start,
+ mem->start - mem->parent->start,
+ (mem->end - mem->start) + 1,
+ DMA_MEMORY_MAP |
+ DMA_MEMORY_EXCLUSIVE)) {
+ dev_err(dev, "cannot declare coherent memory\n");
+ retval = -ENXIO;
+ goto err1;
+ }
+
+ /* allocate, reserve and remap resources for registers */
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (res == NULL) {
+ dev_err(dev, "no resource definition for registers\n");
+ retval = -ENOENT;
+ goto err2;
+ }
+
+ hcd = usb_create_hcd(driver, &pdev->dev, pdev->dev.bus_id);
+ if (!hcd) {
+ retval = -ENOMEM;
+ goto err2;
+ }
+
+ hcd->rsrc_start = res->start;
+ hcd->rsrc_len = res->end - res->start + 1;
+
+ if (!request_mem_region(hcd->rsrc_start, hcd->rsrc_len, pdev->name)) {
+ dev_err(dev, "request_mem_region failed\n");
+ retval = -EBUSY;
+ goto err3;
+ }
+
+ hcd->regs = ioremap(hcd->rsrc_start, hcd->rsrc_len);
+ if (hcd->regs == NULL) {
+ dev_err(dev, "cannot remap registers\n");
+ retval = -ENXIO;
+ goto err4;
+ }
+
+ ohci_hcd_init(hcd_to_ohci(hcd));
+
+ retval = usb_add_hcd(hcd, irq, IRQF_DISABLED | IRQF_SHARED);
+ if (retval)
+ goto err4;
+
+ /* enable power and unmask interrupts */
+
+ sm501_unit_power(dev->parent, SM501_GATE_USB_HOST, 1);
+ sm501_modify_reg(dev->parent, SM501_IRQ_MASK, 1 << 6, 0);
+
+ return 0;
+err4:
+ release_mem_region(hcd->rsrc_start, hcd->rsrc_len);
+err3:
+ usb_put_hcd(hcd);
+err2:
+ dma_release_declared_memory(dev);
+err1:
+ release_mem_region(mem->start, mem->end - mem->start + 1);
+err0:
+ return retval;
+}
+
+static int ohci_hcd_sm501_drv_remove(struct platform_device *pdev)
+{
+ struct usb_hcd *hcd = platform_get_drvdata(pdev);
+ struct resource *mem;
+
+ usb_remove_hcd(hcd);
+ release_mem_region(hcd->rsrc_start, hcd->rsrc_len);
+ usb_put_hcd(hcd);
+ dma_release_declared_memory(&pdev->dev);
+ mem = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+ release_mem_region(mem->start, mem->end - mem->start + 1);
+
+ /* mask interrupts and disable power */
+
+ sm501_modify_reg(pdev->dev.parent, SM501_IRQ_MASK, 0, 1 << 6);
+ sm501_unit_power(pdev->dev.parent, SM501_GATE_USB_HOST, 0);
+
+ platform_set_drvdata(pdev, NULL);
+ return 0;
+}
+
+/*-------------------------------------------------------------------------*/
+
+#ifdef CONFIG_PM
+static int ohci_sm501_suspend(struct platform_device *pdev, pm_message_t msg)
+{
+ struct device *dev = &pdev->dev;
+ struct ohci_hcd *ohci = hcd_to_ohci(platform_get_drvdata(pdev));
+
+ if (time_before(jiffies, ohci->next_statechange))
+ msleep(5);
+ ohci->next_statechange = jiffies;
+
+ sm501_unit_power(dev->parent, SM501_GATE_USB_HOST, 0);
+ ohci_to_hcd(ohci)->state = HC_STATE_SUSPENDED;
+ dev->power.power_state = PMSG_SUSPEND;
+ return 0;
+}
+
+static int ohci_sm501_resume(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct ohci_hcd *ohci = hcd_to_ohci(platform_get_drvdata(pdev));
+
+ if (time_before(jiffies, ohci->next_statechange))
+ msleep(5);
+ ohci->next_statechange = jiffies;
+
+ sm501_unit_power(dev->parent, SM501_GATE_USB_HOST, 1);
+ dev->power.power_state = PMSG_ON;
+ usb_hcd_resume_root_hub(platform_get_drvdata(pdev));
+ return 0;
+}
+#endif
+
+/*-------------------------------------------------------------------------*/
+
+/*
+ * Driver definition to register with the SM501 bus
+ */
+static struct platform_driver ohci_hcd_sm501_driver = {
+ .probe = ohci_hcd_sm501_drv_probe,
+ .remove = ohci_hcd_sm501_drv_remove,
+ .shutdown = usb_hcd_platform_shutdown,
+#ifdef CONFIG_PM
+ .suspend = ohci_sm501_suspend,
+ .resume = ohci_sm501_resume,
+#endif
+ .driver = {
+ .owner = THIS_MODULE,
+ .name = "sm501-usb",
+ },
+};

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
linux-usb-***@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
Loading...