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