Date: Wed, 05 Aug 2009 15:17:18 +0200 From: Grzegorz Bernacki <gjb@semihalf.com> To: Hans Petter Selasky <hselasky@c2i.net> Cc: arm@freebsd.org, freebsd-current@freebsd.org, usb@freebsd.org Subject: Re: About the "USB Cache and busdma usage in USB" thread Message-ID: <4A79865E.3060206@semihalf.com> In-Reply-To: <200908041754.50244.hselasky@c2i.net> References: <3E1658AF-67C6-4E61-B6E7-BEF528C3FF4D@mac.com> <200908031759.46491.hselasky@c2i.net> <4A7848A0.4080905@semihalf.com> <200908041754.50244.hselasky@c2i.net>
next in thread | previous in thread | raw e-mail | index | archive | help
Hans Petter Selasky wrote:
> There are two kinds of DMA memory in USB regard:
> 1) Transfer descriptors are allocated in coherent DMA memory.
> Operation logic:
>
> 1.a) Write to descriptor.
> 1.b.0) Call usb_pc_cpu_flush() to write data to RAM.
> 1.b.1) Write more fields to descriptor.
> 1.b.2) Call usb_pc_cpu_flush() to write data to RAM.
> 1.c) Call usb_pc_cpu_invalidate() to clear cache.
> 1.d) Read status field. If not complete goto 1.c)
>
> 2) Any kernel virtual memory (which might not be coherent)
>
> 2.a.0) CPU read case:
> 2.a.1) Before transfer start usb_pc_cpu_invalidate() is called to clear any
> data in cache for this buffer.
> 2.a.2) After transfer completion usb_pc_cpu_invalidate() is called again.
>
> 2.b.0) CPU write case:
> 2.b.1) Before transfer start usb_pc_cpu_flush() is called to to flush any data
> in cache to RAM for this buffer.
> 2.b.2) After transfer completion there is no cache operation.
>
The best solution is to use bus_dmamap_sync() in in conventional way. I
mean call bus_dmamap_sync(..., BUS_DMASYNC_PREREAD) in case 2.a.1 and
bus_dmamap_sync(..., BUS_DMASYNC_POSTREAD) in cases 2.a.2 and 1.c. But
this is quite a big change and it's risky to put in into -current now,
so below is another solution which I believe is simple and safe.
I understand that usb_pc_cpu_flush() is called *before* write
transfer. So I think that we can just call bus_dmamap_sync(pc->tag,
pc->map, BUS_DMASYNC_PREWRITE) there.
usb_pc_cpu_invalidate() is called before and after each read transfer
and to invalidate cache before reading status field.
So I think that simplest fix is to call following sequence of functions
in it:
bus_dmamap_sync(pc->tag, pc->map, BUS_DMASYNC_POSTREAD);
bus_dmamap_sync(pc->tag, pc->map, BUS_DMASYNC_PREREAD);
Below is the patch with that solution. I tested it on ARM and PowerPC
and it fixes the problem. Please test it on other platforms you have to
see if there is no regression.
diff --git a/sys/dev/usb/usb_busdma.c b/sys/dev/usb/usb_busdma.c
index 82d18a1..c57f51d 100644
--- a/sys/dev/usb/usb_busdma.c
+++ b/sys/dev/usb/usb_busdma.c
@@ -678,8 +678,8 @@ usb_pc_cpu_invalidate(struct usb_page_cache *pc)
/* nothing has been loaded into this page cache! */
return;
}
- bus_dmamap_sync(pc->tag, pc->map,
- BUS_DMASYNC_POSTWRITE | BUS_DMASYNC_POSTREAD);
+ bus_dmamap_sync(pc->tag, pc->map, BUS_DMASYNC_POSTREAD);
+ bus_dmamap_sync(pc->tag, pc->map, BUS_DMASYNC_PREREAD);
}
/*------------------------------------------------------------------------*
@@ -692,8 +692,7 @@ usb_pc_cpu_flush(struct usb_page_cache *pc)
/* nothing has been loaded into this page cache! */
return;
}
- bus_dmamap_sync(pc->tag, pc->map,
- BUS_DMASYNC_PREWRITE | BUS_DMASYNC_PREREAD);
+ bus_dmamap_sync(pc->tag, pc->map, BUS_DMASYNC_PREWRITE);
}
/*------------------------------------------------------------------------*
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4A79865E.3060206>