Date: Tue, 10 Apr 2012 22:00:20 GMT From: Robert Watson <rwatson@FreeBSD.org> To: Perforce Change Reviews <perforce@freebsd.org> Subject: PERFORCE change 209388 for review Message-ID: <201204102200.q3AM0KjU068586@skunkworks.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://p4web.freebsd.org/@@209388?ac=10 Change 209388 by rwatson@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,
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201204102200.q3AM0KjU068586>