spi: spi-mem: Use 2 SPI messages instead of a single full-duplex one
PATCH v3 1/4 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.
---
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(-)
drivers/spi/spi-mem.c +43 -42
@@ -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 ;
Signature · 1 line
2.18.0
PATCH v3 2/4 cmd: mtd: Don't abort erase operation when a bad block is detected
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.
---
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(-)
cmd/mtd.c +15 -1
@@ -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 ; }
Signature · 1 line
2.18.0
PATCH v3 3/4 cmd: mtd: Don't use with negative return codes for shell commands
In reply to Stefan RoeseWhen 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.
---
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(-)
cmd/mtd.c +10 -8
@@ -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[] =
Signature · 1 line
2.18.0
PATCH v3 4/4 cmd: mtd: Add info text to mtd erase subcommand
In reply to Stefan RoeseAdding 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
---
v3:
- No changes
v2:
- Print number of eraseblocks instead of pages as suggested by Boris
cmd/mtd.c | 3 +++ 1 file changed, 3 insertions(+)
cmd/mtd.c +3 -0
@@ -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;
Signature · 1 line
2.18.0