Date: Sun, 25 Mar 2018 01:59:54 +0000 (UTC) From: Ian Lepore <ian@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-11@freebsd.org Subject: svn commit: r331523 - stable/11/sys/dev/flash Message-ID: <201803250159.w2P1xs2s011539@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
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:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201803250159.w2P1xs2s011539>