Date: Wed, 10 Jan 2007 21:48:55 -0700 (MST) From: "M. Warner Losh" <imp@bsdimp.com> To: ticso@cicely.de, ticso@cicely12.cicely.de Cc: perforce@freebsd.org Subject: Re: PERFORCE change 112701 for review Message-ID: <20070110.214855.-267228456.imp@bsdimp.com> In-Reply-To: <20070110092251.GG80390@cicely12.cicely.de> References: <200701100703.l0A734xi068010@repoman.freebsd.org> <20070110092251.GG80390@cicely12.cicely.de>
next in thread | previous in thread | raw e-mail | index | archive | help
In message: <20070110092251.GG80390@cicely12.cicely.de>
Bernd Walter <ticso@cicely12.cicely.de> 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20070110.214855.-267228456.imp>