From owner-p4-projects@FreeBSD.ORG Thu Jan 11 04:49:06 2007 Return-Path: X-Original-To: p4-projects@freebsd.org Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id 4785C16A412; Thu, 11 Jan 2007 04:49:06 +0000 (UTC) X-Original-To: perforce@freebsd.org Delivered-To: perforce@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 1F8C416A403 for ; Thu, 11 Jan 2007 04:49:06 +0000 (UTC) (envelope-from imp@bsdimp.com) Received: from harmony.bsdimp.com (vc4-2-0-87.dsl.netrack.net [199.45.160.85]) by mx1.freebsd.org (Postfix) with ESMTP id B451613C428 for ; Thu, 11 Jan 2007 04:49:05 +0000 (UTC) (envelope-from imp@bsdimp.com) Received: from localhost (localhost [127.0.0.1]) by harmony.bsdimp.com (8.13.4/8.13.4) with ESMTP id l0B4mgBf061814; Wed, 10 Jan 2007 21:48:43 -0700 (MST) (envelope-from imp@bsdimp.com) Date: Wed, 10 Jan 2007 21:48:55 -0700 (MST) Message-Id: <20070110.214855.-267228456.imp@bsdimp.com> To: ticso@cicely.de, ticso@cicely12.cicely.de From: "M. Warner Losh" In-Reply-To: <20070110092251.GG80390@cicely12.cicely.de> References: <200701100703.l0A734xi068010@repoman.freebsd.org> <20070110092251.GG80390@cicely12.cicely.de> X-Mailer: Mew version 4.2 on Emacs 21.3 / Mule 5.0 (SAKAKI) Mime-Version: 1.0 Content-Type: Text/Plain; charset=us-ascii Content-Transfer-Encoding: 7bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-2.0 (harmony.bsdimp.com [127.0.0.1]); Wed, 10 Jan 2007 21:48:43 -0700 (MST) Cc: perforce@freebsd.org Subject: Re: PERFORCE change 112701 for review X-BeenThere: p4-projects@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: p4 projects tree changes List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 11 Jan 2007 04:49:06 -0000 In message: <20070110092251.GG80390@cicely12.cicely.de> Bernd Walter writes: : On Wed, Jan 10, 2007 at 07:03:04AM +0000, Warner Losh wrote: : > http://perforce.freebsd.org/chv.cgi?CH=112701 : > : > Change 112701 by imp@imp_lighthouse on 2007/01/10 07:02:04 : > : > Add a comment about why we need to do the dance we do with enabling : > the PDC, along with a simplified version of the code. : > : > Affected files ... : > : > .. //depot/projects/arm/src/sys/arm/at91/at91_mci.c#30 edit : > : > Differences ... : > : > ==== //depot/projects/arm/src/sys/arm/at91/at91_mci.c#30 (text+ko) ==== : > : > @@ -402,16 +402,27 @@ : > } : > } : > // printf("CMDR %x ARGR %x with data\n", cmdr, cmd->arg); : > + /* : > + * For Reads, we need to enable the DMA buffer before we send : > + * the command. For writes, alas, it has to be enabled after : > + * we send the command. If enabled after the CMDR write for : > + * reads, fast SD parts could win the race that's present and : > + * the result would be corrupted data because the ENDRX bit : > + * would be set, but the dma wouldn't have started yet. When : > + * that interrupt returned, we'd enable DMA. We'd then get a : > + * RXBUFF interrupt and then a CMDRDY interrupt. We'd process : > + * things int he ISR. But since the DMA started after we got : > + * the ENDRX and RXBUFF interrupts, when we got the CMDRDY : > + * interrupt the data would still be in flight, leading to : > + * corruption. This race was 'hard' to trigger for slow parts, : > + * but easy to trigger for faster ones. : > + */ : > WR4(sc, MCI_ARGR, cmd->arg); : > - if (cmdr & MCI_CMDR_TRCMD_START) { : > - if (cmdr & MCI_CMDR_TRDIR) { : > - WR4(sc, PDC_PTCR, PDC_PTCR_RXTEN); : > - WR4(sc, MCI_CMDR, cmdr); : > - } else { : > - WR4(sc, MCI_CMDR, cmdr); : > - WR4(sc, PDC_PTCR, PDC_PTCR_TXTEN); : > - } : > - } : > + if ((cmdr & MCI_CMDR_TRCMD_START) && (cmdr & MCI_CMDR_TRDIR)) : > + WR4(sc, PDC_PTCR, PDC_PTCR_RXTEN); : > + WR4(sc, MCI_CMDR, cmdr); : > + if ((cmdr & MCI_CMDR_TRCMD_START) && !(cmdr & MCI_CMDR_TRDIR)) : > + WR4(sc, PDC_PTCR, PDC_PTCR_TXTEN); : > WR4(sc, MCI_IER, MCI_SR_ERROR | ier); : > } : > : : This moves MCI_CMDR writing out of the (cmdr & MCI_CMDR_TRCMD_START) : check. why would we write the ARGR if we weren't going to also write the CMDR? You moved the CMDR write under the start in the last round of commits, iirc. Warner