Re: [U-Boot] [PATCH 1/4] spi: spi-mem: Add optional half-duplex SPI transfer mode

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 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);
  

• }
  

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