From owner-p4-projects@FreeBSD.ORG Tue Apr 10 22:00:21 2012 Return-Path: Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id 09CE71065672; Tue, 10 Apr 2012 22:00:21 +0000 (UTC) Delivered-To: perforce@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id C0505106566B for ; Tue, 10 Apr 2012 22:00:20 +0000 (UTC) (envelope-from bb+lists.freebsd.perforce@cyrus.watson.org) Received: from skunkworks.freebsd.org (skunkworks.freebsd.org [IPv6:2001:4f8:fff6::2d]) by mx1.freebsd.org (Postfix) with ESMTP id 7F7D18FC15 for ; Tue, 10 Apr 2012 22:00:20 +0000 (UTC) Received: from skunkworks.freebsd.org (localhost [127.0.0.1]) by skunkworks.freebsd.org (8.14.4/8.14.4) with ESMTP id q3AM0KOv068590 for ; Tue, 10 Apr 2012 22:00:20 GMT (envelope-from bb+lists.freebsd.perforce@cyrus.watson.org) Received: (from perforce@localhost) by skunkworks.freebsd.org (8.14.4/8.14.4/Submit) id q3AM0KjU068586 for perforce@freebsd.org; Tue, 10 Apr 2012 22:00:20 GMT (envelope-from bb+lists.freebsd.perforce@cyrus.watson.org) Date: Tue, 10 Apr 2012 22:00:20 GMT Message-Id: <201204102200.q3AM0KjU068586@skunkworks.freebsd.org> X-Authentication-Warning: skunkworks.freebsd.org: perforce set sender to bb+lists.freebsd.perforce@cyrus.watson.org using -f From: Robert Watson To: Perforce Change Reviews Precedence: bulk Cc: Subject: PERFORCE change 209388 for review X-BeenThere: p4-projects@freebsd.org X-Mailman-Version: 2.1.5 List-Id: p4 projects tree changes List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 10 Apr 2012 22:00:21 -0000 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,