[U-Boot] [PATCH 1/4 v3] spi: spi-mem: Use 2 SPI messages instead of a single full-duplex one
Some SPI controller do not support full-duplex SPI transfers. This patch changes the SPI transfer into 2 separate transfers - or 1, if no data is to be transmitted.
With this change, only a small buffer for the command, address and dummy bytes needs to be allocated. We use the TX and RX buffers that are passed to spi_mem_exec_op() directly.
Signed-off-by: Stefan Roese sr@denx.de Suggested-by: Boris Brezillon boris.brezillon@bootlin.com Cc: Miquel Raynal miquel.raynal@bootlin.com Cc: Boris Brezillon boris.brezillon@bootlin.com Cc: Jagan Teki jagan@openedev.com Reviewed-by: Boris Brezillon boris.brezillon@bootlin.com --- v3: - Allocate buffer for cmd, addr and dummy bytes - Return code for spi_xfer is checked - Drop rx_data and tx_data and use rx_buf / tx_buf instead as suggested by Boris - Minor coding style changes
v2: - Replaces patch "spi: spi-mem: Add optional half-duplex SPI transfer mode" from first patchset version - No compile-time option but the default to use 2 separate SPI messages to transfer the command and data
drivers/spi/spi-mem.c | 85 ++++++++++++++++++++++--------------------- 1 file changed, 43 insertions(+), 42 deletions(-)
diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c index 07ce799170..af9aef009a 100644 --- a/drivers/spi/spi-mem.c +++ b/drivers/spi/spi-mem.c @@ -196,12 +196,14 @@ EXPORT_SYMBOL_GPL(spi_mem_supports_op); */ int spi_mem_exec_op(struct spi_slave *slave, const struct spi_mem_op *op) { - bool rx_data = op->data.nbytes && (op->data.dir == SPI_MEM_DATA_IN); - bool tx_data = op->data.nbytes && (op->data.dir == SPI_MEM_DATA_OUT); struct udevice *bus = slave->dev->parent; struct dm_spi_ops *ops = spi_get_ops(bus); - unsigned int xfer_len, pos = 0; - u8 *tx_buf, *rx_buf = NULL; + unsigned int pos = 0; + const u8 *tx_buf = NULL; + u8 *rx_buf = NULL; + u8 *op_buf; + int op_len; + u32 flag; int ret; int i;
@@ -330,67 +332,66 @@ int spi_mem_exec_op(struct spi_slave *slave, const struct spi_mem_op *op) return -ENOTSUPP; }
- xfer_len = sizeof(op->cmd.opcode) + op->addr.nbytes + - op->dummy.nbytes + op->data.nbytes; - - /* - * Allocate a buffer to transmit the CMD, ADDR cycles with kmalloc() so - * we're guaranteed that this buffer is DMA-able, as required by the - * SPI layer. - */ - tx_buf = kzalloc(xfer_len, GFP_KERNEL); - if (!tx_buf) - return -ENOMEM; - - if (rx_data) { - rx_buf = kzalloc(xfer_len, GFP_KERNEL); - if (!rx_buf) - return -ENOMEM; + if (op->data.nbytes) { + if (op->data.dir == SPI_MEM_DATA_IN) + rx_buf = op->data.buf.in; + else + tx_buf = op->data.buf.out; }
+ op_len = sizeof(op->cmd.opcode) + op->addr.nbytes + op->dummy.nbytes; + op_buf = calloc(1, op_len); + ret = spi_claim_bus(slave); if (ret < 0) return ret;
- tx_buf[pos++] = op->cmd.opcode; + op_buf[pos++] = op->cmd.opcode;
if (op->addr.nbytes) { for (i = 0; i < op->addr.nbytes; i++) - tx_buf[pos + i] = op->addr.val >> - (8 * (op->addr.nbytes - i - 1)); + op_buf[pos + i] = op->addr.val >> + (8 * (op->addr.nbytes - i - 1));
pos += op->addr.nbytes; }
- if (op->dummy.nbytes) { - memset(tx_buf + pos, 0xff, op->dummy.nbytes); - pos += op->dummy.nbytes; - } + if (op->dummy.nbytes) + memset(op_buf + pos, 0xff, op->dummy.nbytes); + + /* 1st transfer: opcode + address + dummy cycles */ + flag = SPI_XFER_BEGIN; + /* Make sure to set END bit if no tx or rx data messages follow */ + if (!tx_buf && !rx_buf) + flag |= SPI_XFER_END; + + ret = spi_xfer(slave, op_len * 8, op_buf, NULL, flag); + if (ret) + return ret;
- if (tx_data) - memcpy(tx_buf + pos, op->data.buf.out, op->data.nbytes); + /* 2nd transfer: rx or tx data path */ + if (tx_buf || rx_buf) { + ret = spi_xfer(slave, op->data.nbytes * 8, tx_buf, + rx_buf, SPI_XFER_END); + if (ret) + return ret; + }
- ret = spi_xfer(slave, xfer_len * 8, tx_buf, rx_buf, - SPI_XFER_BEGIN | SPI_XFER_END); spi_release_bus(slave);
for (i = 0; i < pos; i++) - debug("%02x ", tx_buf[i]); + debug("%02x ", op_buf[i]); debug("| [%dB %s] ", - tx_data || rx_data ? op->data.nbytes : 0, - tx_data || rx_data ? (tx_data ? "out" : "in") : "-"); - for (; i < xfer_len; i++) - debug("%02x ", tx_data ? tx_buf[i] : rx_buf[i]); + tx_buf || rx_buf ? op->data.nbytes : 0, + tx_buf || rx_buf ? (tx_buf ? "out" : "in") : "-"); + for (i = 0; i < op->data.nbytes; i++) + debug("%02x ", tx_buf ? tx_buf[i] : rx_buf[i]); debug("[ret %d]\n", ret);
+ free(op_buf); + if (ret < 0) return ret; - - if (rx_data) - memcpy(op->data.buf.in, rx_buf + pos, op->data.nbytes); - - kfree(tx_buf); - kfree(rx_buf); #endif /* __UBOOT__ */
return 0;
It was noticed, that the erase command (mtd erase spi-nand0) aborts upon the first bad block. With this change, bad blocks are now skipped and the erase operation will continue.
Signed-off-by: Stefan Roese sr@denx.de Cc: Miquel Raynal miquel.raynal@bootlin.com Cc: Boris Brezillon boris.brezillon@bootlin.com Cc: Jagan Teki jagan@openedev.com --- v3: - Handle bad-block skipping completely in cmd/mtd. as suggested by Boris - Add message upon bad-block detection
v2: - Use an U-Boot "mtd" command specific option to skip the bad block upon erase so that other MTD users are not affected by this change
cmd/mtd.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/cmd/mtd.c b/cmd/mtd.c index 221b12500f..9f552e7c4a 100644 --- a/cmd/mtd.c +++ b/cmd/mtd.c @@ -360,7 +360,21 @@ static int do_mtd(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) erase_op.len = len; erase_op.scrub = scrub;
- ret = mtd_erase(mtd, &erase_op); + while (erase_op.len) { + ret = mtd_erase(mtd, &erase_op); + + /* Abort if its not an bad block error */ + if (ret != -EIO) + break; + + printf("Skipping bad block at 0x%08llx\n", + erase_op.fail_addr); + + /* Skip bad block and continue behind it */ + erase_op.len -= erase_op.fail_addr - erase_op.addr; + erase_op.len -= mtd->erasesize; + erase_op.addr = erase_op.fail_addr + mtd->erasesize; + } } else { return CMD_RET_USAGE; }
When negative return codes are used in commands (do_foo()), the shell prints these messages:
exit not allowed from main input shell.
Change the return codes in the new mtd commands to use only positive values and these annoying warnings are gone.
Signed-off-by: Stefan Roese sr@denx.de Cc: Miquel Raynal miquel.raynal@bootlin.com Cc: Boris Brezillon boris.brezillon@bootlin.com Cc: Jagan Teki jagan@openedev.com Reviewed-by: Boris Brezillon boris.brezillon@bootlin.com --- v3: - No changes
v2: - Use CMD_RET_FAILURE as return value as suggested by Boris
cmd/mtd.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/cmd/mtd.c b/cmd/mtd.c index 9f552e7c4a..b0ad0616cc 100644 --- a/cmd/mtd.c +++ b/cmd/mtd.c @@ -188,7 +188,7 @@ static int do_mtd_list(void)
if (!dev_nb) { printf("No MTD device found\n"); - return -EINVAL; + return CMD_RET_FAILURE; }
return 0; @@ -269,13 +269,13 @@ static int do_mtd(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) if (mtd_is_aligned_with_min_io_size(mtd, off)) { printf("Offset not aligned with a page (0x%x)\n", mtd->writesize); - return -EINVAL; + return CMD_RET_FAILURE; }
if (mtd_is_aligned_with_min_io_size(mtd, len)) { printf("Size not a multiple of a page (0x%x)\n", mtd->writesize); - return -EINVAL; + return CMD_RET_FAILURE; }
if (dump) @@ -285,7 +285,7 @@ static int do_mtd(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
if (!buf) { printf("Could not map/allocate the user buffer\n"); - return -ENOMEM; + return CMD_RET_FAILURE; }
printf("%s %lldB (%d page(s)) at offset 0x%08llx%s%s\n", @@ -306,7 +306,7 @@ static int do_mtd(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) if (ret) { printf("%s on %s failed with error %d\n", read ? "Read" : "Write", mtd->name, ret); - return ret; + return CMD_RET_FAILURE; }
if (dump) { @@ -346,13 +346,13 @@ static int do_mtd(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) if (mtd_is_aligned_with_block_size(mtd, off)) { printf("Offset not aligned with a block (0x%x)\n", mtd->erasesize); - return -EINVAL; + return CMD_RET_FAILURE; }
if (mtd_is_aligned_with_block_size(mtd, len)) { printf("Size not a multiple of a block (0x%x)\n", mtd->erasesize); - return -EINVAL; + return CMD_RET_FAILURE; }
erase_op.mtd = mtd; @@ -379,7 +379,9 @@ static int do_mtd(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) return CMD_RET_USAGE; }
- return ret; + if (ret) + return CMD_RET_FAILURE; + return 0; }
static char mtd_help_text[] =
Adding this info helps seeing, what really is being erased - especially if no arguments are passed for offset and size. Now this is the output:
=> mtd erase spi-nand0 Erasing 0x00000000 ... 0x07ffffff (1024 eraseblock(s)) nand: attempt to erase a bad/reserved block @6000000 nand: attempt to erase a bad/reserved block @7fe0000
Signed-off-by: Stefan Roese sr@denx.de Cc: Miquel Raynal miquel.raynal@bootlin.com Cc: Boris Brezillon boris.brezillon@bootlin.com Cc: Jagan Teki jagan@openedev.com --- v3: - No changes
v2: - Print number of eraseblocks instead of pages as suggested by Boris
cmd/mtd.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/cmd/mtd.c b/cmd/mtd.c index b0ad0616cc..03a48e7e22 100644 --- a/cmd/mtd.c +++ b/cmd/mtd.c @@ -355,6 +355,9 @@ static int do_mtd(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) return CMD_RET_FAILURE; }
+ printf("Erasing 0x%08llx ... 0x%08llx (%d eraseblock(s))\n", + off, off + len - 1, mtd_div_by_eb(len, mtd)); + erase_op.mtd = mtd; erase_op.addr = off; erase_op.len = len;
participants (1)
-
Stefan Roese