Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 5 Jun 2012 19:06:31 GMT
From:      Brooks Davis <brooks@FreeBSD.org>
To:        Perforce Change Reviews <perforce@freebsd.org>
Subject:   PERFORCE change 212336 for review
Message-ID:  <201206051906.q55J6VKe018628@skunkworks.freebsd.org>

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

Change 212336 by brooks@brooks_ecr_current on 2012/06/05 19:06:28

	Track erase status at the device and erase block (128KB)
	block level.  Return EBUSY for writes during erase and allow reads
	of other blocks.
	
	Disallow writes that require an erase and return EIO if they are
	attempted.

Affected files ...

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

Differences ...

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

@@ -120,6 +120,7 @@
 #define ISF_SR_DWS	(1 << 7)	/* Device Write Status */
 #define ISF_SR_FSC_MASK	(ISF_SR_VPPS | ISF_SR_PS | ISF_SR_BLS)
 
+MALLOC_DEFINE(M_ISF, "isf_data", "Intel StrateFlash driver");
 static void	isf_task(void *arg);
 
 /* 
@@ -229,7 +230,16 @@
 	KASSERT(off % ISF_SECTORSIZE == 0,
 	    ("%s: invalid offset %ju\n", __func__, off));
 
-	bus_read_region_2(sc->isf_res, off, (uint16_t *)data, len / 2);
+	/*
+	 * It is not permitted to read blocks that are in the process of
+	 * being erased, but we know they will be all 1's after the
+	 * erase so just report that value if asked about a block that
+	 * is being erased.
+	 */
+	if (sc->isf_bstate[off / ISF_ERASE_BLOCK] == BS_ERASING)
+		memset(data, 0xFF, len);
+	else
+		bus_read_region_2(sc->isf_res, off, (uint16_t *)data, len / 2);
 }
 
 static int
@@ -246,6 +256,10 @@
 	    ("%s: invalid length %ju", __func__, len));
 	KASSERT(off % ISF_SECTORSIZE == 0,
 	    ("%s: invalid offset %ju\n", __func__, off));
+	KASSERT(!sc->isf_erasing,
+	    ("%s: trying to write while erasing\n", __func__));
+	KASSERT(sc->isf_bstate[off / ISF_ERASE_BLOCK] != BS_ERASING,
+	    ("%s: block being erased at %ju\n", __func__, off));
 
 	isf_unlock_block(sc, off);
 
@@ -302,12 +316,14 @@
 	uint16_t	w, status;
 	off_t		off;
 
-	ISF_LOCK(sc);
+	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;
 		
+		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);
@@ -315,14 +331,15 @@
 
 		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);
 		}
 
 		isf_write_cmd(sc, off, ISF_CMD_RA);
+		sc->isf_bstate[off / ISF_ERASE_BLOCK] = BS_STEADY;
 	}
-	ISF_UNLOCK(sc);
 }
 
 /*
@@ -347,7 +364,16 @@
 			error = EINVAL;
 			break;
 		}
+		ISF_LOCK(sc);
+		if (sc->isf_erasing) {
+			ISF_UNLOCK(sc);
+			error = EBUSY;
+			break;
+		}
+		sc->isf_erasing = 1;
 		isf_erase_range(sc, ir->ir_off, ir->ir_size);
+		sc->isf_erasing = 0;
+		ISF_UNLOCK(sc);
 		break;
 	default:
 		error = EINVAL;
@@ -380,7 +406,7 @@
 	struct isf_softc	*sc = arg;
 	struct bio		*bp;
 	int			ss = sc->isf_disk->d_sectorsize;
-	int			error;
+	int			error, i;
 
 	for (;;) {
 		ISF_LOCK(sc);
@@ -390,49 +416,59 @@
 				if (sc->isf_doomed)
 					kproc_exit(0);
 				else
-					ISF_SLEEP(sc, sc);
+					ISF_SLEEP(sc, sc, 0);
 			}
 		} while (bp == NULL);
 		bioq_remove(&sc->isf_bioq, bp);
 
+		error = 0;
 		switch (bp->bio_cmd) {
 		case BIO_READ:
 			isf_read(sc, bp->bio_pblkno * ss, bp->bio_data,
 			    bp->bio_bcount);
-			biodone(bp);
 			break;
 
 		case BIO_WRITE:
-			error = 0;
-#ifdef NOTYET
+			/*
+			 * In principle one could suspend the in-progress
+			 * erase, process any pending writes to other
+			 * blocks and then proceed, but that seems
+			 * overly complex for the likely usage modes.
+			 */
+			if (sc->isf_erasing) {
+				error = EBUSY;
+				break;
+			}
+
 			/*
 			 * Read in the block we want to write and check that
-			 * we're only setting bits to 0.
+			 * we're only setting bits to 0.  If an erase would
+			 * be required return an I/O error.
 			 */
 			isf_read(sc, bp->bio_pblkno * ss, sc->isf_rbuf,
 			    bp->bio_bcount);
 			for (i = 0; i < bp->bio_bcount / 2; i++)
-				if (sc->isf_rbuf[i] &
-				    (unit16_t *)bp->bio_data[i] !=
-				    (unit16_t *)bp->bio_data[i]) {
+				if ((sc->isf_rbuf[i] &
+				    ((uint16_t *)bp->bio_data)[i]) !=
+				    ((uint16_t *)bp->bio_data)[i]) {
 					error = EIO;
 					break;
 				}
-#endif
-				
-			if (error == 0)
-				error = isf_write(sc, bp->bio_pblkno * ss,
-				    bp->bio_data, bp->bio_bcount);
-			if (error == 0)
-				biodone(bp);
-			else
-				biofinish(bp, NULL, error);
+			if (error != 0)
+				break;
+
+			error = isf_write(sc, bp->bio_pblkno * ss,
+			    bp->bio_data, bp->bio_bcount);
 			break;
 
 		default:
 			panic("%s: unsupported I/O operation %d", __func__,
 			    bp->bio_cmd);
 		}
+		if (error == 0)
+			biodone(bp);
+		else
+			biofinish(bp, NULL, error);
 		ISF_UNLOCK(sc);
 	}
 }
@@ -474,11 +510,14 @@
 }
 
 static void
-isf_disk_insert(struct isf_softc *sc)
+isf_disk_insert(struct isf_softc *sc, off_t mediasize)
 {
 	struct disk *disk;
 
 	sc->isf_doomed = 0;
+	sc->isf_erasing = 0;
+	sc->isf_bstate = malloc(sizeof(*sc->isf_bstate) *
+	    (mediasize / ISF_ERASE_BLOCK), M_ISF, M_ZERO | M_WAITOK);
 	kproc_create(&isf_task, sc, &sc->isf_proc, 0, 0, "isf");
 
 	disk = disk_alloc();
@@ -488,7 +527,7 @@
 	disk->d_strategy = isf_disk_strategy;
 	disk->d_ioctl = isf_disk_ioctl;
 	disk->d_sectorsize = ISF_SECTORSIZE;
-	disk->d_mediasize = ISF_MEDIASIZE;
+	disk->d_mediasize = mediasize;
 	disk->d_maxsize = ISF_SECTORSIZE;
 	sc->isf_disk = disk;
 
@@ -509,9 +548,9 @@
 	ISF_LOCK_ASSERT(sc);
 	KASSERT(sc->isf_disk != NULL, ("%s: isf_disk NULL", __func__));
 
-	sc->isf_doomed = 0;
+	sc->isf_doomed = 1;
 	ISF_WAKEUP(sc);
-	ISF_SLEEP(sc, sc->isf_proc);
+	ISF_SLEEP(sc, sc->isf_proc, 0);
 
 	/*
 	 * XXXRW: Is it OK to call disk_destroy() under the mutex, or should
@@ -521,6 +560,7 @@
 	disk_gone(disk);
 	disk_destroy(disk);
 	sc->isf_disk = NULL;
+	free(sc->isf_bstate, M_ISF);
 	device_printf(sc->isf_dev, "flash device removed\n");
 }
 
@@ -555,7 +595,7 @@
 	bioq_init(&sc->isf_bioq);
 	ISF_LOCK_INIT(sc);
 	sc->isf_disk = NULL;
-	isf_disk_insert(sc);
+	isf_disk_insert(sc, ISF_MEDIASIZE);
 	return(0);
 }
 

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

@@ -52,6 +52,13 @@
 #define ISF_ERASE_BLOCK	(128 * 1024)
 
 #ifdef _KERNEL
+MALLOC_DECLARE(M_ISF);
+
+enum bstate {
+	BS_STEADY = 0,
+	BS_ERASING
+};
+
 struct isf_softc {
 	device_t		 isf_dev;
 	int			 isf_unit;
@@ -67,6 +74,8 @@
 	 */
 	struct bio_queue_head	 isf_bioq;
 	uint16_t		 isf_rbuf[ISF_SECTORSIZE / 2];
+	int			 isf_erasing;
+	enum bstate		*isf_bstate;
 };
 
 #define	ISF_LOCK(sc)		mtx_lock(&(sc)->isf_lock)
@@ -74,10 +83,11 @@
 #define	ISF_LOCK_DESTROY(sc)	mtx_destroy(&(sc)->isf_lock)
 #define	ISF_LOCK_INIT(sc)	mtx_init(&(sc)->isf_lock, "isf", NULL,	\
 				    MTX_DEF)
-#define	ISF_SLEEP(sc, wait)	mtx_sleep((wait), &(sc)->isf_lock, PRIBIO, \
-				    "isf", 0)
-#define	ISF_UNLOCK(sc)		mtx_unlock(&(sc)->isf_lock)
-#define	ISF_WAKEUP(sc)		wakeup((sc))
+#define ISF_SLEEP(sc, wait, timo)	mtx_sleep((wait), 		\
+					    &(sc)->isf_lock, PRIBIO, 	\
+					    "isf", (timo))
+#define	ISF_UNLOCK(sc)			mtx_unlock(&(sc)->isf_lock)
+#define	ISF_WAKEUP(sc)			wakeup((sc))
 
 int	isf_attach(struct isf_softc *sc);
 void	isf_detach(struct isf_softc *sc);



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