[U-Boot] [PATCH 1/4] spi: spi-mem: Add optional half-duplex SPI transfer mode
Some SPI controller might now support full-duplex SPI transfers. This option can be enabled to use half-duplex operation mode for such SPI controllers.
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 --- drivers/spi/Kconfig | 8 ++++++++ drivers/spi/spi-mem.c | 31 ++++++++++++++++++++++++++++--- 2 files changed, 36 insertions(+), 3 deletions(-)
diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig index 9fbd26740d..5bd8289284 100644 --- a/drivers/spi/Kconfig +++ b/drivers/spi/Kconfig @@ -25,6 +25,14 @@ config SPI_MEM This extension is meant to simplify interaction with SPI memories by providing an high-level interface to send memory-like commands.
+config SPI_MEM_HALF_DUPLEX + bool "Use half-duplex SPI transfer" + depends on SPI_MEM + help + Some SPI controller might not support full-duplex SPI transfers. + This option can be enabled to use half-duplex operation mode for + such SPI controllers. + config ALTERA_SPI bool "Altera SPI driver" help diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c index 07ce799170..617fbbfe09 100644 --- a/drivers/spi/spi-mem.c +++ b/drivers/spi/spi-mem.c @@ -202,6 +202,10 @@ int spi_mem_exec_op(struct spi_slave *slave, const struct spi_mem_op *op) struct dm_spi_ops *ops = spi_get_ops(bus); unsigned int xfer_len, pos = 0; u8 *tx_buf, *rx_buf = NULL; + unsigned int pos2; + int tx_len; + int rx_len; + u32 flag; int ret; int i;
@@ -370,8 +374,29 @@ int spi_mem_exec_op(struct spi_slave *slave, const struct spi_mem_op *op) if (tx_data) memcpy(tx_buf + pos, op->data.buf.out, op->data.nbytes);
- ret = spi_xfer(slave, xfer_len * 8, tx_buf, rx_buf, - SPI_XFER_BEGIN | SPI_XFER_END); + pos2 = pos; + if (CONFIG_IS_ENABLED(SPI_MEM_HALF_DUPLEX)) { + if (rx_data) { + tx_len = (sizeof(op->cmd.opcode) + + op->addr.nbytes + op->dummy.nbytes); + rx_len = op->data.nbytes; + flag = SPI_XFER_BEGIN; + pos2 = 0; + } else { + tx_len = xfer_len; + rx_len = 0; + flag = SPI_XFER_BEGIN | SPI_XFER_END; + } + ret = spi_xfer(slave, tx_len * 8, tx_buf, NULL, flag); + if (rx_data) { + ret = spi_xfer(slave, rx_len * 8, NULL, + rx_buf, SPI_XFER_END); + } + } else { + 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++) @@ -387,7 +412,7 @@ int spi_mem_exec_op(struct spi_slave *slave, const struct spi_mem_op *op) return ret;
if (rx_data) - memcpy(op->data.buf.in, rx_buf + pos, op->data.nbytes); + memcpy(op->data.buf.in, rx_buf + pos2, op->data.nbytes);
kfree(tx_buf); kfree(rx_buf);
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 --- drivers/mtd/nand/core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mtd/nand/core.c b/drivers/mtd/nand/core.c index 0b793695cc..888f765b90 100644 --- a/drivers/mtd/nand/core.c +++ b/drivers/mtd/nand/core.c @@ -162,7 +162,7 @@ int nanddev_mtd_erase(struct mtd_info *mtd, struct erase_info *einfo) nanddev_offs_to_pos(nand, einfo->addr + einfo->len - 1, &last); while (nanddev_pos_cmp(&pos, &last) <= 0) { ret = nanddev_erase(nand, &pos); - if (ret) { + if (ret && ret != -EIO) { einfo->fail_addr = nanddev_pos_to_offs(nand, &pos);
return ret;
Hi Stefan,
On Mon, 6 Aug 2018 17:12:51 +0200 Stefan Roese sr@denx.de wrote:
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.
That's not what the raw NAND framework does [1], and I'm almost sure MTD users expect erase ops to stop when a bad block is found and "skip bad block" was not explicitly requested.
I'd suggest moving to an approach where cmd/mtd.c erases blocks one by one and checks the status of each block (with mtd_block_isbad()) before calling mtd_erase(). Alternatively, we could add a 'bool skipbad' field to struct erase_info, but that means getting away from Linux implementation (which is already the case since uboot has an extra "int scrub" field).
Regards,
Boris
[1]https://elixir.bootlin.com/u-boot/v2018.09-rc1/source/drivers/mtd/nand/nand_...
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
drivers/mtd/nand/core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mtd/nand/core.c b/drivers/mtd/nand/core.c index 0b793695cc..888f765b90 100644 --- a/drivers/mtd/nand/core.c +++ b/drivers/mtd/nand/core.c @@ -162,7 +162,7 @@ int nanddev_mtd_erase(struct mtd_info *mtd, struct erase_info *einfo) nanddev_offs_to_pos(nand, einfo->addr + einfo->len - 1, &last); while (nanddev_pos_cmp(&pos, &last) <= 0) { ret = nanddev_erase(nand, &pos);
if (ret) {
if (ret && ret != -EIO) { einfo->fail_addr = nanddev_pos_to_offs(nand, &pos); return ret;
Hi Boris,
On 06.08.2018 22:06, Boris Brezillon wrote:
Hi Stefan,
On Mon, 6 Aug 2018 17:12:51 +0200 Stefan Roese sr@denx.de wrote:
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.
That's not what the raw NAND framework does [1], and I'm almost sure MTD users expect erase ops to stop when a bad block is found and "skip bad block" was not explicitly requested.
I see. AFAIR, the default behavior in U-Boot when erasing NAND chips is that bad blocks are skipped though. Thats why I came up with this idea.
I'd suggest moving to an approach where cmd/mtd.c erases blocks one by one and checks the status of each block (with mtd_block_isbad()) before calling mtd_erase(). Alternatively, we could add a 'bool skipbad' field to struct erase_info, but that means getting away from Linux implementation (which is already the case since uboot has an extra "int scrub" field).
Thanks for the suggestions.
I'll double-check all the new "mtd" commands in respect to handling bad blocks again later this week and will try to come up with different approach supporting erase, read and write in the same way.
Thanks, Stefan
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 --- cmd/mtd.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/cmd/mtd.c b/cmd/mtd.c index 221b12500f..38a89736cf 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 EINVAL; }
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 EINVAL; }
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 EINVAL; }
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 ENOMEM; }
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 -ret; }
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 EINVAL; }
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 EINVAL; }
erase_op.mtd = mtd;
On Mon, 6 Aug 2018 17:12:52 +0200 Stefan Roese sr@denx.de wrote:
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
cmd/mtd.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/cmd/mtd.c b/cmd/mtd.c index 221b12500f..38a89736cf 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 EINVAL;
How about using CMD_RET_FAILURE for all errors?
Hi Boris,
On 06.08.2018 22:38, Boris Brezillon wrote:
On Mon, 6 Aug 2018 17:12:52 +0200 Stefan Roese sr@denx.de wrote:
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
cmd/mtd.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/cmd/mtd.c b/cmd/mtd.c index 221b12500f..38a89736cf 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 EINVAL;How about using CMD_RET_FAILURE for all errors?
Good idea, thanks. Will change in v2.
Thanks, Stefan
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 (65536 page(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 --- cmd/mtd.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/cmd/mtd.c b/cmd/mtd.c index 38a89736cf..6d27698d1e 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 EINVAL; }
+ printf("Erasing 0x%08llx ... 0x%08llx (%d page(s))\n", + off, off + len - 1, mtd_len_to_pages(mtd, len)); + erase_op.mtd = mtd; erase_op.addr = off; erase_op.len = len;
On Mon, 6 Aug 2018 17:12:53 +0200 Stefan Roese sr@denx.de wrote:
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 (65536 page(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
cmd/mtd.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/cmd/mtd.c b/cmd/mtd.c index 38a89736cf..6d27698d1e 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 EINVAL; }
printf("Erasing 0x%08llx ... 0x%08llx (%d page(s))\n",off, off + len - 1, mtd_len_to_pages(mtd, len));
Just a detail, but we usually count things in eraseblocks (not pages) when erasing an MTD device (you can use mtd_div_by_eb(len, mtd) to do that).
- erase_op.mtd = mtd; erase_op.addr = off; erase_op.len = len;
Hi Boris,
On 06.08.2018 22:41, Boris Brezillon wrote:
On Mon, 6 Aug 2018 17:12:53 +0200 Stefan Roese sr@denx.de wrote:
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 (65536 page(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
cmd/mtd.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/cmd/mtd.c b/cmd/mtd.c index 38a89736cf..6d27698d1e 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 EINVAL; }
printf("Erasing 0x%08llx ... 0x%08llx (%d page(s))\n",off, off + len - 1, mtd_len_to_pages(mtd, len));Just a detail, but we usually count things in eraseblocks (not pages) when erasing an MTD device (you can use mtd_div_by_eb(len, mtd) to do that).
Will do for v2.
Thanks, Stefan
Hi Stefan,
On Mon, 6 Aug 2018 17:12:50 +0200 Stefan Roese sr@denx.de wrote:
Some SPI controller might now support full-duplex SPI transfers. This option can be enabled to use half-duplex operation mode for such SPI controllers.
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
drivers/spi/Kconfig | 8 ++++++++ drivers/spi/spi-mem.c | 31 ++++++++++++++++++++++++++++--- 2 files changed, 36 insertions(+), 3 deletions(-)
diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig index 9fbd26740d..5bd8289284 100644 --- a/drivers/spi/Kconfig +++ b/drivers/spi/Kconfig @@ -25,6 +25,14 @@ config SPI_MEM This extension is meant to simplify interaction with SPI memories by providing an high-level interface to send memory-like commands.
+config SPI_MEM_HALF_DUPLEX
- bool "Use half-duplex SPI transfer"
- depends on SPI_MEM
- help
Some SPI controller might not support full-duplex SPI transfers.This option can be enabled to use half-duplex operation mode forsuch SPI controllers.config ALTERA_SPI bool "Altera SPI driver" help diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c index 07ce799170..617fbbfe09 100644 --- a/drivers/spi/spi-mem.c +++ b/drivers/spi/spi-mem.c @@ -202,6 +202,10 @@ int spi_mem_exec_op(struct spi_slave *slave, const struct spi_mem_op *op) struct dm_spi_ops *ops = spi_get_ops(bus); unsigned int xfer_len, pos = 0; u8 *tx_buf, *rx_buf = NULL;
- unsigned int pos2;
- int tx_len;
- int rx_len;
- u32 flag; int ret; int i;
@@ -370,8 +374,29 @@ int spi_mem_exec_op(struct spi_slave *slave, const struct spi_mem_op *op) if (tx_data) memcpy(tx_buf + pos, op->data.buf.out, op->data.nbytes);
- ret = spi_xfer(slave, xfer_len * 8, tx_buf, rx_buf,
SPI_XFER_BEGIN | SPI_XFER_END);
- pos2 = pos;
- if (CONFIG_IS_ENABLED(SPI_MEM_HALF_DUPLEX)) {
if (rx_data) {tx_len = (sizeof(op->cmd.opcode) +op->addr.nbytes + op->dummy.nbytes);rx_len = op->data.nbytes;flag = SPI_XFER_BEGIN;pos2 = 0;} else {tx_len = xfer_len;rx_len = 0;flag = SPI_XFER_BEGIN | SPI_XFER_END;}ret = spi_xfer(slave, tx_len * 8, tx_buf, NULL, flag);if (rx_data) {ret = spi_xfer(slave, rx_len * 8, NULL,rx_buf, SPI_XFER_END);}
Why not doing that all the time and split things in 3 transfers instead of 2:
1/ the opcode + address + dummy cycles 2/ the tx data 3/ the rx data (optional)
This way you should have to allocate the rx/tx buf and you'd get rid of the memcpy.
Of course, that only works if all SPI controllers are capable of dealing with transfers that have only SPI_XFER_BEGIN or SPI_XFER_END set.
} else {
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++)
@@ -387,7 +412,7 @@ int spi_mem_exec_op(struct spi_slave *slave, const struct spi_mem_op *op) return ret;
if (rx_data)
memcpy(op->data.buf.in, rx_buf + pos, op->data.nbytes);
memcpy(op->data.buf.in, rx_buf + pos2, op->data.nbytes);kfree(tx_buf); kfree(rx_buf);
Regards,
Boris
Hi Boris,
On 06.08.2018 21:53, Boris Brezillon wrote:
Hi Stefan,
On Mon, 6 Aug 2018 17:12:50 +0200 Stefan Roese sr@denx.de wrote:
Some SPI controller might now support full-duplex SPI transfers. This option can be enabled to use half-duplex operation mode for such SPI controllers.
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
drivers/spi/Kconfig | 8 ++++++++ drivers/spi/spi-mem.c | 31 ++++++++++++++++++++++++++++--- 2 files changed, 36 insertions(+), 3 deletions(-)
diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig index 9fbd26740d..5bd8289284 100644 --- a/drivers/spi/Kconfig +++ b/drivers/spi/Kconfig @@ -25,6 +25,14 @@ config SPI_MEM This extension is meant to simplify interaction with SPI memories by providing an high-level interface to send memory-like commands.
+config SPI_MEM_HALF_DUPLEX
- bool "Use half-duplex SPI transfer"
- depends on SPI_MEM
- help
Some SPI controller might not support full-duplex SPI transfers.This option can be enabled to use half-duplex operation mode forsuch SPI controllers.- config ALTERA_SPI bool "Altera SPI driver" help
diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c index 07ce799170..617fbbfe09 100644 --- a/drivers/spi/spi-mem.c +++ b/drivers/spi/spi-mem.c @@ -202,6 +202,10 @@ int spi_mem_exec_op(struct spi_slave *slave, const struct spi_mem_op *op) struct dm_spi_ops *ops = spi_get_ops(bus); unsigned int xfer_len, pos = 0; u8 *tx_buf, *rx_buf = NULL;
- unsigned int pos2;
- int tx_len;
- int rx_len;
- u32 flag; int ret; int i;
@@ -370,8 +374,29 @@ int spi_mem_exec_op(struct spi_slave *slave, const struct spi_mem_op *op) if (tx_data) memcpy(tx_buf + pos, op->data.buf.out, op->data.nbytes);
- ret = spi_xfer(slave, xfer_len * 8, tx_buf, rx_buf,
SPI_XFER_BEGIN | SPI_XFER_END);
- pos2 = pos;
- if (CONFIG_IS_ENABLED(SPI_MEM_HALF_DUPLEX)) {
if (rx_data) {tx_len = (sizeof(op->cmd.opcode) +op->addr.nbytes + op->dummy.nbytes);rx_len = op->data.nbytes;flag = SPI_XFER_BEGIN;pos2 = 0;} else {tx_len = xfer_len;rx_len = 0;flag = SPI_XFER_BEGIN | SPI_XFER_END;}ret = spi_xfer(slave, tx_len * 8, tx_buf, NULL, flag);if (rx_data) {ret = spi_xfer(slave, rx_len * 8, NULL,rx_buf, SPI_XFER_END);}Why not doing that all the time and split things in 3 transfers instead of 2:
1/ the opcode + address + dummy cycles 2/ the tx data 3/ the rx data (optional)
This way you should have to allocate the rx/tx buf and you'd get rid of the memcpy.
I also thought about this but was not "brave" enough to drop the current single xfer. If you and Miquel think this is good idea, then I will gladly work on such an implementation. Getting rid of this compile time option would be good.
Of course, that only works if all SPI controllers are capable of dealing with transfers that have only SPI_XFER_BEGIN or SPI_XFER_END set.
I'm pretty sure that we can assume this is the case. Otherwise such a driver should be easy to fix to support this type of xfer.
Thanks, Stefan
On Tue, 7 Aug 2018 07:06:58 +0200 Stefan Roese sr@denx.de wrote:
Hi Boris,
On 06.08.2018 21:53, Boris Brezillon wrote:
Hi Stefan,
On Mon, 6 Aug 2018 17:12:50 +0200 Stefan Roese sr@denx.de wrote:
Some SPI controller might now support full-duplex SPI transfers. This option can be enabled to use half-duplex operation mode for such SPI controllers.
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
drivers/spi/Kconfig | 8 ++++++++ drivers/spi/spi-mem.c | 31 ++++++++++++++++++++++++++++--- 2 files changed, 36 insertions(+), 3 deletions(-)
diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig index 9fbd26740d..5bd8289284 100644 --- a/drivers/spi/Kconfig +++ b/drivers/spi/Kconfig @@ -25,6 +25,14 @@ config SPI_MEM This extension is meant to simplify interaction with SPI memories by providing an high-level interface to send memory-like commands.
+config SPI_MEM_HALF_DUPLEX
- bool "Use half-duplex SPI transfer"
- depends on SPI_MEM
- help
Some SPI controller might not support full-duplex SPI transfers.This option can be enabled to use half-duplex operation mode forsuch SPI controllers.- config ALTERA_SPI bool "Altera SPI driver" help
diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c index 07ce799170..617fbbfe09 100644 --- a/drivers/spi/spi-mem.c +++ b/drivers/spi/spi-mem.c @@ -202,6 +202,10 @@ int spi_mem_exec_op(struct spi_slave *slave, const struct spi_mem_op *op) struct dm_spi_ops *ops = spi_get_ops(bus); unsigned int xfer_len, pos = 0; u8 *tx_buf, *rx_buf = NULL;
- unsigned int pos2;
- int tx_len;
- int rx_len;
- u32 flag; int ret; int i;
@@ -370,8 +374,29 @@ int spi_mem_exec_op(struct spi_slave *slave, const struct spi_mem_op *op) if (tx_data) memcpy(tx_buf + pos, op->data.buf.out, op->data.nbytes);
- ret = spi_xfer(slave, xfer_len * 8, tx_buf, rx_buf,
SPI_XFER_BEGIN | SPI_XFER_END);
- pos2 = pos;
- if (CONFIG_IS_ENABLED(SPI_MEM_HALF_DUPLEX)) {
if (rx_data) {tx_len = (sizeof(op->cmd.opcode) +op->addr.nbytes + op->dummy.nbytes);rx_len = op->data.nbytes;flag = SPI_XFER_BEGIN;pos2 = 0;} else {tx_len = xfer_len;rx_len = 0;flag = SPI_XFER_BEGIN | SPI_XFER_END;}ret = spi_xfer(slave, tx_len * 8, tx_buf, NULL, flag);if (rx_data) {ret = spi_xfer(slave, rx_len * 8, NULL,rx_buf, SPI_XFER_END);}Why not doing that all the time and split things in 3 transfers instead of 2:
1/ the opcode + address + dummy cycles 2/ the tx data 3/ the rx data (optional)
Actually it's:
1/ the opcode + address + dummy cycles 2/ tx or rx data (optional)
but I guess you figured this out already.
participants (2)
-
Boris Brezillon -
Stefan Roese