svn commit: r331523 - stable/11/sys/dev/flash

Ian Lepore ian at FreeBSD.org
Sun Mar 25 01:59:55 UTC 2018


Author: ian
Date: Sun Mar 25 01:59:54 2018
New Revision: 331523
URL: https://svnweb.freebsd.org/changeset/base/331523

Log:
  MFC r331123, r331126, r331129, r331132, r331136, r331138-r331139, r331141
  
  r331123:
  Do not overwrite the contents of BIO_WRITE buffers.  SPI inherently
  transfers data in both directions at once.  When writing to the device,
  use a dummy buffer for the incoming data, not the same buffer as the
  outgoing data.  Writes are done in FLASH_PAGE_SIZE chunks, which is only
  256 bytes, so just put the dummy buffer into the softc.
  
  r331126:
  Remove a pointless KASSERT and reword a comment a bit.  The KASSERT tested
  for the same condition that the preceeding lines checked for and would have
  returned EIO, so the assert could never possibly trigger (sc_sectorsize must
  inherently be an integer multiple of FLASH_PAGE_SIZE).
  
  r331129:
  Eliminate some unneeded intermediate variables.  Eliminate some redundant
  parens in shift-and-mask expressions.  Reword and reflow some comments.
  
  r331132:
  Bugfix: wait for writes/erases to complete after starting them, instead of
  before starting them.
  
  Using the wait-before logic would make sense if there was useful time-
  consuming work that could be done between the end of one write and the
  beginning of the next, but it also requires doing the wait-for-ready before
  reading, because a prior write or erase could still be in progress.  Reading
  is the far more common case, so adding a whole extra bus transaction to
  check for ready before each read would soak up any small gains that might be
  had from doing async writes.
  
  r331136:
  Add sc_parent to the softc and use it in place of device_get_parent() calls
  all over the place.  Also pass the softc as the arg to all the internal
  functions instead of passing a device_t and calling device_get_softc() in
  each function.
  
  r331138:
  Make all internal routines return an int error status, and check the
  status at all call points.  Combine the get_status and wait_for_ready
  routines, since waiting for ready is the only reason to ever get status.
  
  r331139:
  Add support for 4K and 32K erase block sizes.  Many of the supported chips
  have these flags set in the ident table, but there was no code to support
  using the smaller erase sizes.
  
  r331141:
  Add the device/chip type to the disk d_descr field, and print more info
  about the chip including the erase block size at attach time.
  
  Also add myself to the copyrights since at this point svn blame would point
  to me as the culprit for much of this.

Modified:
  stable/11/sys/dev/flash/mx25l.c
Directory Properties:
  stable/11/   (props changed)

Modified: stable/11/sys/dev/flash/mx25l.c
==============================================================================
--- stable/11/sys/dev/flash/mx25l.c	Sun Mar 25 01:55:17 2018	(r331522)
+++ stable/11/sys/dev/flash/mx25l.c	Sun Mar 25 01:59:54 2018	(r331523)
@@ -3,6 +3,7 @@
  *
  * Copyright (c) 2006 M. Warner Losh.  All rights reserved.
  * Copyright (c) 2009 Oleksandr Tymoshenko.  All rights reserved.
+ * Copyright (c) 2018 Ian Lepore.  All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -81,15 +82,17 @@ struct mx25l_flash_ident
 struct mx25l_softc 
 {
 	device_t	sc_dev;
+	device_t	sc_parent;
 	uint8_t		sc_manufacturer_id;
 	uint16_t	sc_device_id;
-	unsigned int	sc_sectorsize;
+	unsigned int	sc_erasesize;
 	struct mtx	sc_mtx;
 	struct disk	*sc_disk;
 	struct proc	*sc_p;
 	struct bio_queue_head sc_bio_queue;
 	unsigned int	sc_flags;
 	unsigned int	sc_taskstate;
+	uint8_t		sc_dummybuf[FLASH_PAGE_SIZE];
 };
 
 #define	TSTATE_STOPPED	0
@@ -148,37 +151,30 @@ struct mx25l_flash_ident flash_devices[] = {
 	{ "gd25q64",	0xc8, 0x4017, 64 * 1024, 128, FL_ERASE_4K },
 };
 
-static uint8_t
-mx25l_get_status(device_t dev)
+static int
+mx25l_wait_for_device_ready(struct mx25l_softc *sc)
 {
 	uint8_t txBuf[2], rxBuf[2];
 	struct spi_command cmd;
 	int err;
 
 	memset(&cmd, 0, sizeof(cmd));
-	memset(txBuf, 0, sizeof(txBuf));
-	memset(rxBuf, 0, sizeof(rxBuf));
 
-	txBuf[0] = CMD_READ_STATUS;
-	cmd.tx_cmd = txBuf;
-	cmd.rx_cmd = rxBuf;
-	cmd.rx_cmd_sz = 2;
-	cmd.tx_cmd_sz = 2;
-	err = SPIBUS_TRANSFER(device_get_parent(dev), dev, &cmd);
-	return (rxBuf[1]);
-}
+	do {
+		txBuf[0] = CMD_READ_STATUS;
+		cmd.tx_cmd = txBuf;
+		cmd.rx_cmd = rxBuf;
+		cmd.rx_cmd_sz = 2;
+		cmd.tx_cmd_sz = 2;
+		err = SPIBUS_TRANSFER(sc->sc_parent, sc->sc_dev, &cmd);
+	} while (err == 0 && (rxBuf[1] & STATUS_WIP));
 
-static void
-mx25l_wait_for_device_ready(device_t dev)
-{
-	while ((mx25l_get_status(dev) & STATUS_WIP))
-		continue;
+	return (err);
 }
 
 static struct mx25l_flash_ident*
 mx25l_get_device_ident(struct mx25l_softc *sc)
 {
-	device_t dev = sc->sc_dev;
 	uint8_t txBuf[8], rxBuf[8];
 	struct spi_command cmd;
 	uint8_t manufacturer_id;
@@ -198,27 +194,27 @@ mx25l_get_device_ident(struct mx25l_softc *sc)
 	 */
 	cmd.tx_cmd_sz = 4;
 	cmd.rx_cmd_sz = 4;
-	err = SPIBUS_TRANSFER(device_get_parent(dev), dev, &cmd);
+	err = SPIBUS_TRANSFER(sc->sc_parent, sc->sc_dev, &cmd);
 	if (err)
 		return (NULL);
 
 	manufacturer_id = rxBuf[1];
 	dev_id = (rxBuf[2] << 8) | (rxBuf[3]);
 
-	for (i = 0; 
-	    i < nitems(flash_devices); i++) {
+	for (i = 0; i < nitems(flash_devices); i++) {
 		if ((flash_devices[i].manufacturer_id == manufacturer_id) &&
 		    (flash_devices[i].device_id == dev_id))
 			return &flash_devices[i];
 	}
 
-	printf("Unknown SPI flash device. Vendor: %02x, device id: %04x\n",
+	device_printf(sc->sc_dev,
+	    "Unknown SPI flash device. Vendor: %02x, device id: %04x\n",
 	    manufacturer_id, dev_id);
 	return (NULL);
 }
 
-static void
-mx25l_set_writable(device_t dev, int writable)
+static int
+mx25l_set_writable(struct mx25l_softc *sc, int writable)
 {
 	uint8_t txBuf[1], rxBuf[1];
 	struct spi_command cmd;
@@ -233,29 +229,34 @@ mx25l_set_writable(device_t dev, int writable)
 	cmd.rx_cmd = rxBuf;
 	cmd.rx_cmd_sz = 1;
 	cmd.tx_cmd_sz = 1;
-	err = SPIBUS_TRANSFER(device_get_parent(dev), dev, &cmd);
+	err = SPIBUS_TRANSFER(sc->sc_parent, sc->sc_dev, &cmd);
+	return (err);
 }
 
-static void
-mx25l_erase_cmd(device_t dev, off_t sector, uint8_t ecmd)
+static int
+mx25l_erase_cmd(struct mx25l_softc *sc, off_t sector)
 {
-	struct mx25l_softc *sc;
 	uint8_t txBuf[5], rxBuf[5];
 	struct spi_command cmd;
 	int err;
 
-	sc = device_get_softc(dev);
+	if ((err = mx25l_set_writable(sc, 1)) != 0)
+		return (err);
 
-	mx25l_wait_for_device_ready(dev);
-	mx25l_set_writable(dev, 1);
-
 	memset(&cmd, 0, sizeof(cmd));
 	memset(txBuf, 0, sizeof(txBuf));
 	memset(rxBuf, 0, sizeof(rxBuf));
 
-	txBuf[0] = ecmd;
 	cmd.tx_cmd = txBuf;
 	cmd.rx_cmd = rxBuf;
+
+	if (sc->sc_flags & FL_ERASE_4K)
+		txBuf[0] = CMD_BLOCK_4K_ERASE;
+	else if (sc->sc_flags & FL_ERASE_32K)
+		txBuf[0] = CMD_BLOCK_32K_ERASE;
+	else
+		txBuf[0] = CMD_SECTOR_ERASE;
+
 	if (sc->sc_flags & FL_ENABLE_4B_ADDR) {
 		cmd.rx_cmd_sz = 5;
 		cmd.tx_cmd_sz = 5;
@@ -270,23 +271,20 @@ mx25l_erase_cmd(device_t dev, off_t sector, uint8_t ec
 		txBuf[2] = ((sector >> 8) & 0xff);
 		txBuf[3] = (sector & 0xff);
 	}
-	err = SPIBUS_TRANSFER(device_get_parent(dev), dev, &cmd);
+	if ((err = SPIBUS_TRANSFER(sc->sc_parent, sc->sc_dev, &cmd)) != 0)
+		return (err);
+	err = mx25l_wait_for_device_ready(sc);
+	return (err);
 }
 
 static int
-mx25l_write(device_t dev, off_t offset, caddr_t data, off_t count)
+mx25l_write(struct mx25l_softc *sc, off_t offset, caddr_t data, off_t count)
 {
-	struct mx25l_softc *sc;
 	uint8_t txBuf[8], rxBuf[8];
 	struct spi_command cmd;
-	off_t write_offset;
-	long bytes_to_write, bytes_writen;
-	device_t pdev;
+	off_t bytes_to_write;
 	int err = 0;
 
-	pdev = device_get_parent(dev);
-	sc = device_get_softc(dev);
-
 	if (sc->sc_flags & FL_ENABLE_4B_ADDR) {
 		cmd.tx_cmd_sz = 5;
 		cmd.rx_cmd_sz = 5;
@@ -295,96 +293,83 @@ mx25l_write(device_t dev, off_t offset, caddr_t data, 
 		cmd.rx_cmd_sz = 4;
 	}
 
-	bytes_writen = 0;
-	write_offset = offset;
-
 	/*
-	 * Use the erase sectorsize here since blocks are fully erased
-	 * first before they're written to.
+	 * Writes must be aligned to the erase sectorsize, since blocks are
+	 * fully erased before they're written to.
 	 */
-	if (count % sc->sc_sectorsize != 0 || offset % sc->sc_sectorsize != 0)
+	if (count % sc->sc_erasesize != 0 || offset % sc->sc_erasesize != 0)
 		return (EIO);
 
 	/*
-	 * Assume here that we write per-sector only 
-	 * and sector size should be 256 bytes aligned
+	 * Maximum write size for CMD_PAGE_PROGRAM is FLASH_PAGE_SIZE, so loop
+	 * to write chunks of FLASH_PAGE_SIZE bytes each.
 	 */
-	KASSERT(write_offset % FLASH_PAGE_SIZE == 0,
-	    ("offset for BIO_WRITE is not page size (%d bytes) aligned",
-		FLASH_PAGE_SIZE));
+	while (count != 0) {
+		/* If we crossed a sector boundary, erase the next sector. */
+		if (((offset) % sc->sc_erasesize) == 0) {
+			err = mx25l_erase_cmd(sc, offset);
+			if (err)
+				break;
+		}
 
-	/*
-	 * Maximum write size for CMD_PAGE_PROGRAM is 
-	 * FLASH_PAGE_SIZE, so split data to chunks 
-	 * FLASH_PAGE_SIZE bytes eash and write them
-	 * one by one
-	 */
-	while (bytes_writen < count) {
-		/*
-		 * If we crossed sector boundary - erase next sector
-		 */
-		if (((offset + bytes_writen) % sc->sc_sectorsize) == 0)
-			mx25l_erase_cmd(dev, offset + bytes_writen, CMD_SECTOR_ERASE);
-
 		txBuf[0] = CMD_PAGE_PROGRAM;
 		if (sc->sc_flags & FL_ENABLE_4B_ADDR) {
-			txBuf[1] = ((write_offset >> 24) & 0xff);
-			txBuf[2] = ((write_offset >> 16) & 0xff);
-			txBuf[3] = ((write_offset >> 8) & 0xff);
-			txBuf[4] = (write_offset & 0xff);
+			txBuf[1] = (offset >> 24) & 0xff;
+			txBuf[2] = (offset >> 16) & 0xff;
+			txBuf[3] = (offset >> 8) & 0xff;
+			txBuf[4] = offset & 0xff;
 		} else {
-			txBuf[1] = ((write_offset >> 16) & 0xff);
-			txBuf[2] = ((write_offset >> 8) & 0xff);
-			txBuf[3] = (write_offset & 0xff);
+			txBuf[1] = (offset >> 16) & 0xff;
+			txBuf[2] = (offset >> 8) & 0xff;
+			txBuf[3] = offset & 0xff;
 		}
 
-		bytes_to_write = MIN(FLASH_PAGE_SIZE,
-		    count - bytes_writen);
+		bytes_to_write = MIN(FLASH_PAGE_SIZE, count);
 		cmd.tx_cmd = txBuf;
 		cmd.rx_cmd = rxBuf;
-		cmd.tx_data = data + bytes_writen;
-		cmd.tx_data_sz = bytes_to_write;
-		cmd.rx_data = data + bytes_writen;
-		cmd.rx_data_sz = bytes_to_write;
+		cmd.tx_data = data;
+		cmd.rx_data = sc->sc_dummybuf;
+		cmd.tx_data_sz = (uint32_t)bytes_to_write;
+		cmd.rx_data_sz = (uint32_t)bytes_to_write;
 
 		/*
-		 * Eash completed write operation resets WEL 
-		 * (write enable latch) to disabled state,
-		 * so we re-enable it here 
+		 * Each completed write operation resets WEL (write enable
+		 * latch) to disabled state, so we re-enable it here.
 		 */
-		mx25l_wait_for_device_ready(dev);
-		mx25l_set_writable(dev, 1);
+		if ((err = mx25l_wait_for_device_ready(sc)) != 0)
+			break;
+		if ((err = mx25l_set_writable(sc, 1)) != 0)
+			break;
 
-		err = SPIBUS_TRANSFER(pdev, dev, &cmd);
+		err = SPIBUS_TRANSFER(sc->sc_parent, sc->sc_dev, &cmd);
+		if (err != 0)
+			break;
+		err = mx25l_wait_for_device_ready(sc);
 		if (err)
 			break;
 
-		bytes_writen += bytes_to_write;
-		write_offset += bytes_to_write;
+		data   += bytes_to_write;
+		offset += bytes_to_write;
+		count  -= bytes_to_write;
 	}
 
 	return (err);
 }
 
 static int
-mx25l_read(device_t dev, off_t offset, caddr_t data, off_t count)
+mx25l_read(struct mx25l_softc *sc, off_t offset, caddr_t data, off_t count)
 {
-	struct mx25l_softc *sc;
 	uint8_t txBuf[8], rxBuf[8];
 	struct spi_command cmd;
-	device_t pdev;
 	int err = 0;
 
-	pdev = device_get_parent(dev);
-	sc = device_get_softc(dev);
-
 	/*
-	 * Enforce the disk read sectorsize not the erase sectorsize.
-	 * In this way, smaller read IO is possible,dramatically
-	 * speeding up filesystem/geom_compress access.
+	 * Enforce that reads are aligned to the disk sectorsize, not the
+	 * erase sectorsize.  In this way, smaller read IO is possible,
+	 * dramatically speeding up filesystem/geom_compress access.
 	 */
-	if (count % sc->sc_disk->d_sectorsize != 0
-	    || offset % sc->sc_disk->d_sectorsize != 0)
+	if (count % sc->sc_disk->d_sectorsize != 0 ||
+	    offset % sc->sc_disk->d_sectorsize != 0)
 		return (EIO);
 
 	txBuf[0] = CMD_FAST_READ;
@@ -392,19 +377,19 @@ mx25l_read(device_t dev, off_t offset, caddr_t data, o
 		cmd.tx_cmd_sz = 6;
 		cmd.rx_cmd_sz = 6;
 
-		txBuf[1] = ((offset >> 24) & 0xff);
-		txBuf[2] = ((offset >> 16) & 0xff);
-		txBuf[3] = ((offset >> 8) & 0xff);
-		txBuf[4] = (offset & 0xff);
+		txBuf[1] = (offset >> 24) & 0xff;
+		txBuf[2] = (offset >> 16) & 0xff;
+		txBuf[3] = (offset >> 8) & 0xff;
+		txBuf[4] = offset & 0xff;
 		/* Dummy byte */
 		txBuf[5] = 0;
 	} else {
 		cmd.tx_cmd_sz = 5;
 		cmd.rx_cmd_sz = 5;
 
-		txBuf[1] = ((offset >> 16) & 0xff);
-		txBuf[2] = ((offset >> 8) & 0xff);
-		txBuf[3] = (offset & 0xff);
+		txBuf[1] = (offset >> 16) & 0xff;
+		txBuf[2] = (offset >> 8) & 0xff;
+		txBuf[3] = offset & 0xff;
 		/* Dummy byte */
 		txBuf[4] = 0;
 	}
@@ -412,29 +397,25 @@ mx25l_read(device_t dev, off_t offset, caddr_t data, o
 	cmd.tx_cmd = txBuf;
 	cmd.rx_cmd = rxBuf;
 	cmd.tx_data = data;
-	cmd.tx_data_sz = count;
 	cmd.rx_data = data;
+	cmd.tx_data_sz = count;
 	cmd.rx_data_sz = count;
 
-	err = SPIBUS_TRANSFER(pdev, dev, &cmd);
-
+	err = SPIBUS_TRANSFER(sc->sc_parent, sc->sc_dev, &cmd);
 	return (err);
 }
 
 static int
-mx25l_set_4b_mode(device_t dev, uint8_t command)
+mx25l_set_4b_mode(struct mx25l_softc *sc, uint8_t command)
 {
 	uint8_t txBuf[1], rxBuf[1];
 	struct spi_command cmd;
-	device_t pdev;
 	int err;
 
 	memset(&cmd, 0, sizeof(cmd));
 	memset(txBuf, 0, sizeof(txBuf));
 	memset(rxBuf, 0, sizeof(rxBuf));
 
-	pdev = device_get_parent(dev);
-
 	cmd.tx_cmd_sz = cmd.rx_cmd_sz = 1;
 
 	cmd.tx_cmd = txBuf;
@@ -442,10 +423,9 @@ mx25l_set_4b_mode(device_t dev, uint8_t command)
 
 	txBuf[0] = command;
 
-	err = SPIBUS_TRANSFER(pdev, dev, &cmd);
+	if ((err = SPIBUS_TRANSFER(sc->sc_parent, sc->sc_dev, &cmd)) == 0)
+		err = mx25l_wait_for_device_ready(sc);
 
-	mx25l_wait_for_device_ready(dev);
-
 	return (err);
 }
 
@@ -491,17 +471,38 @@ mx25l_attach(device_t dev)
 {
 	struct mx25l_softc *sc;
 	struct mx25l_flash_ident *ident;
+	int err;
 
 	sc = device_get_softc(dev);
 	sc->sc_dev = dev;
+	sc->sc_parent = device_get_parent(sc->sc_dev);
+
 	M25PXX_LOCK_INIT(sc);
 
 	ident = mx25l_get_device_ident(sc);
 	if (ident == NULL)
 		return (ENXIO);
 
-	mx25l_wait_for_device_ready(sc->sc_dev);
+	if ((err = mx25l_wait_for_device_ready(sc)) != 0)
+		return (err);
 
+	sc->sc_flags = ident->flags;
+
+	if (sc->sc_flags & FL_ERASE_4K)
+		sc->sc_erasesize = 4 * 1024;
+	else if (sc->sc_flags & FL_ERASE_32K)
+		sc->sc_erasesize = 32 * 1024;
+	else
+		sc->sc_erasesize = ident->sectorsize;
+
+	if (sc->sc_flags & FL_ENABLE_4B_ADDR) {
+		if ((err = mx25l_set_4b_mode(sc, CMD_ENTER_4B_MODE)) != 0)
+			return (err);
+	} else if (sc->sc_flags & FL_DISABLE_4B_ADDR) {
+		if ((err = mx25l_set_4b_mode(sc, CMD_EXIT_4B_MODE)) != 0)
+			return (err);
+	}
+
 	sc->sc_disk = disk_alloc();
 	sc->sc_disk->d_open = mx25l_open;
 	sc->sc_disk->d_close = mx25l_close;
@@ -513,29 +514,24 @@ mx25l_attach(device_t dev)
 	sc->sc_disk->d_maxsize = DFLTPHYS;
 	sc->sc_disk->d_sectorsize = MX25L_SECTORSIZE;
 	sc->sc_disk->d_mediasize = ident->sectorsize * ident->sectorcount;
+	sc->sc_disk->d_stripesize = sc->sc_erasesize;
 	sc->sc_disk->d_unit = device_get_unit(sc->sc_dev);
 	sc->sc_disk->d_dump = NULL;		/* NB: no dumps */
-	/* Sectorsize for erase operations */
-	sc->sc_sectorsize =  ident->sectorsize;
-	sc->sc_flags = ident->flags;
+	strlcpy(sc->sc_disk->d_descr, ident->name,
+	    sizeof(sc->sc_disk->d_descr));
 
-	if (sc->sc_flags & FL_ENABLE_4B_ADDR)
-		mx25l_set_4b_mode(dev, CMD_ENTER_4B_MODE);
-
-	if (sc->sc_flags & FL_DISABLE_4B_ADDR)
-		mx25l_set_4b_mode(dev, CMD_EXIT_4B_MODE);
-
-        /* NB: use stripesize to hold the erase/region size for RedBoot */
-	sc->sc_disk->d_stripesize = ident->sectorsize;
-
 	disk_create(sc->sc_disk, DISK_VERSION);
 	bioq_init(&sc->sc_bio_queue);
 
 	kproc_create(&mx25l_task, sc, &sc->sc_p, 0, 0, "task: mx25l flash");
 	sc->sc_taskstate = TSTATE_RUNNING;
 
-	device_printf(sc->sc_dev, "%s, sector %d bytes, %d sectors\n", 
-	    ident->name, ident->sectorsize, ident->sectorcount);
+	device_printf(sc->sc_dev, 
+	    "device type %s, size %dK in %d sectors of %dK, erase size %dK\n",
+	    ident->name,
+	    ident->sectorcount * ident->sectorsize / 1024,
+	    ident->sectorcount, ident->sectorsize / 1024,
+	    sc->sc_erasesize / 1024);
 
 	return (0);
 }
@@ -557,7 +553,7 @@ mx25l_detach(device_t dev)
 			err = msleep(sc, &sc->sc_mtx, 0, "mx25dt", hz * 3);
 			if (err != 0) {
 				sc->sc_taskstate = TSTATE_RUNNING;
-				device_printf(dev,
+				device_printf(sc->sc_dev,
 				    "Failed to stop queue task\n");
 			}
 		}
@@ -652,11 +648,11 @@ mx25l_task(void *arg)
 
 		switch (bp->bio_cmd) {
 		case BIO_READ:
-			bp->bio_error = mx25l_read(dev, bp->bio_offset, 
+			bp->bio_error = mx25l_read(sc, bp->bio_offset, 
 			    bp->bio_data, bp->bio_bcount);
 			break;
 		case BIO_WRITE:
-			bp->bio_error = mx25l_write(dev, bp->bio_offset, 
+			bp->bio_error = mx25l_write(sc, bp->bio_offset, 
 			    bp->bio_data, bp->bio_bcount);
 			break;
 		default:


More information about the svn-src-all mailing list