PERFORCE change 112701 for review

[M. Warner Losh]

In message: <20070110092251.GG80390 at cicely12.cicely.de>
            Bernd Walter <ticso at 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 at 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