Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 8 Jun 2012 21:24:08 GMT
From:      Brooks Davis <brooks@FreeBSD.org>
To:        Perforce Change Reviews <perforce@freebsd.org>
Subject:   PERFORCE change 212492 for review
Message-ID:  <201206082124.q58LO8qX068127@skunkworks.freebsd.org>

next in thread | raw e-mail | index | archive | help
http://p4web.freebsd.org/@@212492?ac=10

Change 212492 by brooks@brooks_ecr_current on 2012/06/08 21:23:07

	Fix a pair of bugs the prevented writing and erasing on the last
	part of the chip from reporting completion.  (I was setting status
	mode in addition to reading status and I was reading status from
	an out of range location).
	
	Refactor the erase code as a result of debugging.  It now
	uncoditionally erases and knows that only the first or last 128K
	can be split into four 32K chunks.  In principle it should be easy
	to identify the correct ones.

Affected files ...

.. //depot/projects/ctsrd/beribsd/src/sys/dev/isf/isf.c#9 edit

Differences ...

==== //depot/projects/ctsrd/beribsd/src/sys/dev/isf/isf.c#9 (text+ko) ====

@@ -152,7 +152,7 @@
  */
 
 static uint16_t
-isf_read_reg(struct isf_softc *sc, uint32_t reg)
+isf_read_reg(struct isf_softc *sc, uint16_t reg)
 {
 
 	if (isf_debug)
@@ -161,7 +161,7 @@
 }
 
 static uint64_t
-isf_read_reg64(struct isf_softc *sc, uint32_t reg)
+isf_read_reg64(struct isf_softc *sc, uint16_t reg)
 {
 	uint64_t val;
 	uint16_t *val16 = (uint16_t *)&val;
@@ -176,6 +176,21 @@
 	return(le64toh(val));
 }
 
+static uint16_t
+isf_read_off(struct isf_softc *sc, off_t off)
+{
+
+	KASSERT(off >= 0, ("%s: negative offset\n", __func__));
+	KASSERT(off < sc->isf_disk->d_mediasize,
+	    ("%s: offset out side address space 0x%08jx \n", __func__,
+	    (intmax_t)off));
+
+	if (isf_debug)
+		device_printf(sc->isf_dev, "isf_read_off(0x%08jx)\n",
+		    (intmax_t)off);
+	return (le16toh(bus_read_2(sc->isf_res, off)));
+}
+
 static void
 isf_write_cmd(struct isf_softc *sc, off_t off, uint16_t cmd)
 {
@@ -190,8 +205,8 @@
 isf_read_status(struct isf_softc *sc, off_t off)
 {
 	
-	isf_write_cmd(sc, off, ISF_CMD_RSR);
-	return isf_read_reg(sc, off);
+	isf_write_cmd(sc, off/2, ISF_CMD_RSR);
+	return isf_read_off(sc, off);
 }
 
 static void
@@ -202,12 +217,12 @@
 }
 
 static int
-isf_full_status_check(struct isf_softc *sc)
+isf_full_status_check(struct isf_softc *sc, off_t off)
 {
 	int		error = 0;
 	uint16_t	status;
 	
-	status = isf_read_status(sc, 0);
+	status = isf_read_status(sc, off);
 	if (status & ISF_SR_VPPS) {
 		device_printf(sc->isf_dev, "Vpp Range Error\n");
 		error = EIO;
@@ -223,6 +238,31 @@
 	return(error);
 }
 
+static int
+isf_full_erase_status_check(struct isf_softc *sc, off_t off)
+{
+	int		error = 0;
+	uint16_t	status;
+	
+	status = isf_read_status(sc, off);
+	if (status & ISF_SR_VPPS) {
+		device_printf(sc->isf_dev, "Vpp Range Error\n");
+		error = EIO;
+	} else if (status & (ISF_SR_PS|ISF_SR_ES)) {
+		device_printf(sc->isf_dev, "Command Sequence Error\n");
+		error = EIO;
+	} else if (status & ISF_SR_ES) {
+		device_printf(sc->isf_dev, "Block Erase Error\n");
+		error = EIO;
+	} else if (status & ISF_SR_BLS) {
+		device_printf(sc->isf_dev, "Block Locked Error\n");
+		error = EIO;
+	}
+	isf_clear_status(sc);
+
+	return(error);
+}
+
 static void
 isf_unlock_block(struct isf_softc *sc, off_t off)
 {
@@ -291,9 +331,13 @@
 		isf_clear_status(sc);
 		isf_write_cmd(sc, coff, ISF_CMD_BPS);
 		cycles = 0xFFFF;
-		while ( !(isf_read_reg(sc, coff) & ISF_SR_DWS) ) {
-			if (cycles-- == 0)
+		while ( !(isf_read_off(sc, coff) & ISF_SR_DWS) ) {
+			if (cycles-- == 0) {
+				device_printf(sc->isf_dev, "timeout waiting"
+				    " for write to start at 0x08%jx\n",
+				    (intmax_t)coff);
 				return (EIO);
+			}
 			isf_write_cmd(sc, coff, ISF_CMD_BPS);
 		}
 
@@ -303,13 +347,19 @@
 
 		isf_write_cmd(sc, coff, ISF_CMD_BPC);
 
-		status = isf_read_reg(sc, coff);
+		status = isf_read_off(sc, coff);
+		cycles = 0xFFFFF;
 		while ( !(status & ISF_SR_DWS) ) {
-			if (status & ISF_SR_PSS)
-				panic("%s: suspend not supported", __func__);
-			status = isf_read_reg(sc, coff);
+			if (cycles-- == 0) {
+				device_printf(sc->isf_dev, "timeout waiting"
+				    " for write to complete at 0x08%jx\n",
+				    (intmax_t)coff);
+				error = EIO;
+				break;
+			}
+			status = isf_read_off(sc, coff);
 		}
-		isf_full_status_check(sc);
+		isf_full_status_check(sc, off);
 
 		isf_write_cmd(sc, coff, ISF_CMD_RA);
 	}
@@ -318,14 +368,21 @@
 	    dp++, coff += 2) {
 		isf_write_cmd(sc, coff, ISF_CMD_WPS);
 		bus_write_2(sc->isf_res, coff, *dp);
-		status = isf_read_reg(sc, coff);
+		status = isf_read_off(sc, coff);
+		cycles=0xFFFFF;
 		while ( !(status & ISF_SR_DWS) ) {
-			if (status & ISF_SR_PSS)
-				panic("%s: suspend not supported", __func__);
-			status = isf_read_reg(sc, coff);
+			if (cycles-- == 0) {
+				device_printf(sc->isf_dev, "timeout waiting"
+				    " for write to complete at 0x08%jx\n",
+				    (intmax_t)coff);
+				error = EIO;
+				break;
+			}
+			status = isf_read_off(sc, coff);
 		}
+
 	}
-	isf_full_status_check(sc);
+	isf_full_status_check(sc, off);
 	isf_write_cmd(sc, coff, ISF_CMD_RA);
 #endif
 
@@ -335,33 +392,65 @@
 }
 
 static void
+isf_erase_at(struct isf_softc *sc, off_t off)
+{
+	int		cycles;
+	uint16_t	status;
+
+	isf_unlock_block(sc, off);
+	isf_clear_status(sc);
+
+	isf_write_cmd(sc, off, ISF_CMD_BES);
+	isf_write_cmd(sc, off, ISF_CMD_BEC);
+
+	cycles=0xFFFFFF;
+	status = isf_read_off(sc, off);
+	while ( !(status & ISF_SR_DWS) ) {
+#ifdef NOTYET
+		ISF_SLEEP(sc, sc, hz);
+#endif
+		if (cycles-- == 0) {
+			device_printf(sc->isf_dev,
+			    "Giving up on erase\n");
+			break;
+		}
+		status = isf_read_off(sc, off);
+	}
+
+	isf_full_erase_status_check(sc, off);
+
+	isf_lock_block(sc, off);
+
+	isf_write_cmd(sc, off, ISF_CMD_RA);
+}
+
+static void
 isf_erase_range(struct isf_softc *sc, off_t blk_off, size_t size)
 {
-	uint16_t	w, status;
 	off_t		off;
+	off_t		ms = sc->isf_disk->d_mediasize;
 
+	KASSERT(blk_off % ISF_ERASE_BLOCK == 0,
+	    ("%s: invalid offset %ju\n", __func__, blk_off));
+
 	ISF_LOCK_ASSERT(sc);
 
-	for (off = blk_off; off < blk_off + size; off += 2) {
-		w = bus_read_2(sc->isf_res, off);
-		if (w == 0xFFFF)
-			continue;
-		
+	for (off = blk_off; off < blk_off + size; off += ISF_ERASE_BLOCK) {
 		sc->isf_bstate[off / ISF_ERASE_BLOCK] = BS_ERASING;
-		isf_clear_status(sc);
-		isf_unlock_block(sc, off);
-		isf_write_cmd(sc, off, ISF_CMD_BES);
-		isf_write_cmd(sc, off, ISF_CMD_BEC);
 
-		status = isf_read_status(sc, off);
-		while ( !(status & ISF_SR_DWS) ) {
-			ISF_SLEEP(sc, sc, hz);
-			if (status & ISF_SR_PSS)
-				panic("%s: suspend not supported", __func__);
-			status = isf_read_status(sc, off);
-		}
+		/*
+		 * The first or last 128K is four blocks depending which
+		 * part this is.  For now, just assume both are and
+		 * erase four times.
+		 */
+		if (off == 0 || ms - off == ISF_ERASE_BLOCK) {
+			isf_erase_at(sc, off);
+			isf_erase_at(sc, off + 0x08000);
+			isf_erase_at(sc, off + 0x10000);
+			isf_erase_at(sc, off + 0x18000);
+		} else
+			isf_erase_at(sc, off);
 
-		isf_write_cmd(sc, off, ISF_CMD_RA);
 		sc->isf_bstate[off / ISF_ERASE_BLOCK] = BS_STEADY;
 	}
 }
@@ -475,6 +564,9 @@
 				if ((sc->isf_rbuf[i] &
 				    ((uint16_t *)bp->bio_data)[i]) !=
 				    ((uint16_t *)bp->bio_data)[i]) {
+					device_printf(sc->isf_dev, "write"
+					    " requires erase at 0x08%jx\n",
+					    bp->bio_pblkno * ss);
 					error = EIO;
 					break;
 				}



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201206082124.q58LO8qX068127>