[U-Boot] [PATCH 1/4 v2] spi: spi-mem: Use 2 SPI messages instead of a single full-duplex one

* [U-Boot] [PATCH 1/4 v2] spi: spi-mem: Use 2 SPI messages instead of a single full-duplex one
@ 2018-08-07 12:16 Stefan Roese
  2018-08-07 12:16 ` [U-Boot] [PATCH 2/4 v2] cmd: mtd: Don't abort erase operation when a bad block is detected Stefan Roese
                   ` (3 more replies)
  0 siblings, 4 replies; 17+ messages in thread
From: Stefan Roese @ 2018-08-07 12:16 UTC (permalink / raw)
  To: u-boot

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 transmitted.

With this change, no buffers need to be allocated anymore. 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>
---
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 | 74 +++++++++++++++++++++++--------------------
 1 file changed, 39 insertions(+), 35 deletions(-)

diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c
index 07ce799170..84e33aa979 100644
--- a/drivers/spi/spi-mem.c
+++ b/drivers/spi/spi-mem.c
@@ -15,6 +15,8 @@
 #include <spi-mem.h>
 #endif
 
+#define OP_BUFFER_SIZE_MAX	16	/* Max size for cmd + addr + dummy */
+
 #ifndef __UBOOT__
 /**
  * spi_controller_dma_map_mem_op_data() - DMA-map the buffer attached to a
@@ -200,8 +202,12 @@ int spi_mem_exec_op(struct spi_slave *slave, const struct spi_mem_op *op)
 	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;
+	u8 *rx_buf;
+	u8 op_buf[OP_BUFFER_SIZE_MAX];
+	int op_len;
+	u32 flag;
 	int ret;
 	int i;
 
@@ -330,67 +336,65 @@ 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;
+	tx_buf = op->data.buf.out;
+	rx_buf = op->data.buf.in;
 
-	/*
-	 * 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;
+	op_len = sizeof(op->cmd.opcode) + op->addr.nbytes + op->dummy.nbytes;
+	if (op_len > OP_BUFFER_SIZE_MAX) {
+		printf("Length for cmd+addr+dummy too big (%d)\n", op_len);
+		return -EIO;
 	}
+	memset(op_buf, 0x00, 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_data && !rx_data)
+		flag |= SPI_XFER_END;
+	ret = spi_xfer(slave, op_len * 8, op_buf, NULL, flag);
+
+	if (tx_data) {
+		/* 2nd transfer a: tx data path */
+		ret = spi_xfer(slave, op->data.nbytes * 8, tx_buf, NULL,
+			       SPI_XFER_END);
 	}
 
-	if (tx_data)
-		memcpy(tx_buf + pos, op->data.buf.out, op->data.nbytes);
+	if (rx_data) {
+		/* 2nd transfer b: rx data path */
+		ret = spi_xfer(slave, op->data.nbytes * 8, NULL, rx_buf,
+			       SPI_XFER_END);
+	}
 
-	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++)
+	for (i = 0; i < op->data.nbytes; i++)
 		debug("%02x ", tx_data ? tx_buf[i] : rx_buf[i]);
 	debug("[ret %d]\n", ret);
 
 	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;
-- 
2.18.0

^ permalink raw reply related	[flat|nested] 17+ messages in thread