From owner-freebsd-usb@FreeBSD.ORG Wed Aug 5 13:17:20 2009 Return-Path: Delivered-To: usb@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id BB2A1106566C; Wed, 5 Aug 2009 13:17:20 +0000 (UTC) (envelope-from gjb@semihalf.com) Received: from smtp.semihalf.com (smtp.semihalf.com [213.17.239.109]) by mx1.freebsd.org (Postfix) with ESMTP id 6559F8FC17; Wed, 5 Aug 2009 13:17:20 +0000 (UTC) (envelope-from gjb@semihalf.com) Received: from [10.0.0.75] (cardhu.semihalf.com [213.17.239.108]) by smtp.semihalf.com (Postfix) with ESMTPA id 1A0FFC426C; Wed, 5 Aug 2009 15:14:53 +0200 (CEST) Message-ID: <4A79865E.3060206@semihalf.com> Date: Wed, 05 Aug 2009 15:17:18 +0200 From: Grzegorz Bernacki User-Agent: Thunderbird 2.0.0.16 (X11/20090618) MIME-Version: 1.0 To: Hans Petter Selasky References: <3E1658AF-67C6-4E61-B6E7-BEF528C3FF4D@mac.com> <200908031759.46491.hselasky@c2i.net> <4A7848A0.4080905@semihalf.com> <200908041754.50244.hselasky@c2i.net> In-Reply-To: <200908041754.50244.hselasky@c2i.net> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: arm@freebsd.org, Rafal Jaworowski , freebsd-current@freebsd.org, usb@freebsd.org Subject: Re: About the "USB Cache and busdma usage in USB" thread X-BeenThere: freebsd-usb@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: FreeBSD support for USB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 05 Aug 2009 13:17:21 -0000 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); } /*------------------------------------------------------------------------*