PERFORCE change 209388 for review
Jonathan Woodruff
jonwoodruff at gmail.com
Tue Apr 10 22:32:50 UTC 2012
Yay!
On Tue, Apr 10, 2012 at 11:00 PM, Robert Watson <rwatson at freebsd.org> wrote:
> http://p4web.freebsd.org/@@209388?ac=10
>
> Change 209388 by rwatson at rwatson_svr_ctsrd_mipsbuild on 2012/04/10
> 22:00:07
>
> Modify the Altera SD Card IP core driver to ignore the presence of
> ALTERA_SDCARD_RR1_COMMANDCRCFAILED in the RR1 register when a read
> error is reported by the core. In my (limited) tests, the error
> bit does not reflect data corruption in the I/O. With this change
> I am able to mount and use UFS file systems reliably on 2GB CSD
> structure 0 SD cards from FreeBSD/BERI.
>
> Affected files ...
>
> ..
> //depot/projects/ctsrd/beribsd/src/sys/dev/altera/sdcard/altera_sdcard.h#8
> edit
> ..
> //depot/projects/ctsrd/beribsd/src/sys/dev/altera/sdcard/altera_sdcard_io.c#10
> edit
>
> Differences ...
>
> ====
> //depot/projects/ctsrd/beribsd/src/sys/dev/altera/sdcard/altera_sdcard.h#8
> (text+ko) ====
>
> @@ -115,7 +115,6 @@
> */
> uint16_t altera_sdcard_read_asr(struct altera_sdcard_softc *sc);
> int altera_sdcard_read_csd(struct altera_sdcard_softc *sc);
> -uint16_t altera_sdcard_read_rr1(struct altera_sdcard_softc *sc);
>
> int altera_sdcard_io_complete(struct altera_sdcard_softc *sc,
> uint16_t asr);
> @@ -185,14 +184,28 @@
> #define ALTERA_SDCARD_ASR_CMDDATAERROR 0x0020
>
> /*
> - * The Altera IP Core provides a 16-bit "Response R1" regster (RR1) beyond
> - * those described in the SD Card specification that holds additional
> - * information on the most recently completed command sent to the unit.
> + * The Altera IP Core claims to provide a 16-bit "Response R1" register
> (RR1)
> + * to provide more detailed error reporting when a read or write fails.
> *
> * XXXRW: The specification claims that this field is 16-bit, but then
> - * proceeds to define values as though it is 32-bit. Which piece of the
> - * documentation is correct?
> + * proceeds to define values as though it is 32-bit. In practice, 16-bit
> + * seems more likely as the register is not 32-bit aligned.
> + */
> +#define ALTERA_SDCARD_RR1_INITPROCRUNNING 0x0100
> +#define ALTERA_SDCARD_RR1_ERASEINTERRUPTED 0x0200
> +#define ALTERA_SDCARD_RR1_ILLEGALCOMMAND 0x0400
> +#define ALTERA_SDCARD_RR1_COMMANDCRCFAILED 0x0800
> +#define ALTERA_SDCARD_RR1_ADDRESSMISALIGNED 0x1000
> +#define ALTERA_SDCARD_RR1_ADDRBLOCKRANGE 0x2000
> +
> +/*
> + * Not all RR1 values are "errors" per se -- check only for the ones that
> are
> + * when performing error handling.
> */
> +#define ALTERA_SDCARD_RR1_ERRORMASK
> \
> + (ALTERA_SDCARD_RR1_ERASEINTERRUPTED |
> ALTERA_SDCARD_RR1_ILLEGALCOMMAND | \
> + ALTERA_SDCARD_RR1_COMMANDCRCFAILED |
> ALTERA_SDCARD_RR1_ADDRESSMISALIGNED |\
> + ALTERA_SDCARD_RR1_ADDRBLOCKRANGE)
>
> /*
> * Although SD Cards may have various sector sizes, the Altera IP Core
>
> ====
> //depot/projects/ctsrd/beribsd/src/sys/dev/altera/sdcard/altera_sdcard_io.c#10
> (text+ko) ====
>
> @@ -188,7 +188,7 @@
> * register, but all bits it identifies are >16 bit. Most likely, RR1 is a
> * 32-bit register?
> */
> -uint16_t
> +static uint16_t
> altera_sdcard_read_rr1(struct altera_sdcard_softc *sc)
> {
>
> @@ -287,7 +287,7 @@
> altera_sdcard_io_complete(struct altera_sdcard_softc *sc, uint16_t asr)
> {
> struct bio *bp;
> - uint16_t rr1;
> + uint16_t rr1, mask;
> int error;
>
> ALTERA_SDCARD_LOCK_ASSERT(sc);
> @@ -298,12 +298,42 @@
>
> bp = sc->as_currentbio;
>
> - /*
> - * Handle I/O retries if an error is returned by the device.
> + /*-
> + * Handle I/O retries if an error is returned by the device.
> Various
> + * quirks handled in the process:
> + *
> + * 1. ALTERA_SDCARD_ASR_CMDDATAERROR is ignored for BIO_WRITE.
> + * 2. ALTERA_SDCARD_RR1_COMMANDCRCFAILED is ignored for BIO_READ.
> */
> - if ((asr & ALTERA_SDCARD_ASR_CMDTIMEOUT) || (bp->bio_cmd ==
> BIO_READ
> - && (asr & ALTERA_SDCARD_ASR_CMDDATAERROR))) {
> - rr1 = altera_sdcard_read_rr1(sc);
> + error = 0;
> + rr1 = altera_sdcard_read_rr1(sc);
> + switch (bp->bio_cmd) {
> + case BIO_READ:
> + mask = ALTERA_SDCARD_RR1_ERRORMASK;
> + mask &= ~ALTERA_SDCARD_RR1_COMMANDCRCFAILED;
> + if (asr & ALTERA_SDCARD_ASR_CMDTIMEOUT)
> + error = EIO;
> + else if ((asr & ALTERA_SDCARD_ASR_CMDDATAERROR) &&
> + (rr1 & mask))
> + error = EIO;
> + else
> + error = 0;
> + break;
> +
> + case BIO_WRITE:
> + if (asr & ALTERA_SDCARD_ASR_CMDTIMEOUT)
> + error = EIO;
> + else
> + error = 0;
> + break;
> +
> + default:
> + break;
> + }
> + if (error) {
> + /*
> + * This attempt experienced an error; possibly retry.
> + */
> sc->as_retriesleft--;
> if (sc->as_retriesleft != 0) {
> sc->as_flags |= ALTERA_SDCARD_FLAG_IOERROR;
> @@ -316,8 +346,10 @@
> (bp->bio_cmd == BIO_WRITE ? "BIO_WRITE" : "unknown"),
> bp->bio_pblkno, bp->bio_bcount, asr, rr1);
> sc->as_flags &= ~ALTERA_SDCARD_FLAG_IOERROR;
> - error = EIO;
> } else {
> + /*
> + * Successful I/O completion path.
> + */
> if (sc->as_flags & ALTERA_SDCARD_FLAG_IOERROR) {
> device_printf(sc->as_dev, "%s: %s operation block
> %ju"
> " length %ju succeeded after %d retries\n",
> @@ -327,10 +359,6 @@
> ALTERA_SDCARD_RETRY_LIMIT - sc->as_retriesleft);
> sc->as_flags &= ~ALTERA_SDCARD_FLAG_IOERROR;
> }
> -
> - /*
> - * Successful I/O completion path.
> - */
> switch (bp->bio_cmd) {
> case BIO_READ:
> altera_sdcard_read_rxtx_buffer(sc, bp->bio_data,
>
>
More information about the p4-projects
mailing list