PERFORCE change 209388 for review
Simon Moore
Simon.Moore at cl.cam.ac.uk
Tue Apr 10 22:58:25 UTC 2012
Excellent news!
- Simon
(sent from phone, so please excuse the brevity)
[Simon Moore][http://www.cl.cam.ac.uk ]
On 10 Apr 2012, at 23:35, "Jonathan Woodruff" <jonwoodruff at gmail.com<mailto:jonwoodruff at gmail.com>> wrote:
Yay!
On Tue, Apr 10, 2012 at 11:00 PM, Robert Watson <rwatson at freebsd.org<mailto: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