[PATCH v4 0/7] spi: Extend the framework to generically support memory devices

[PATCH v4 0/7] spi: Extend the framework to generically support memory devices

[Boris Brezillon]

Hello,

Shrinking a bit the explanation on why the spi-mem abstraction is
needed (a detailed explanation is available here [2]). In addition to
what as been said in my initial explanation I'll add that making it part
of the SPI framework instead of as an extra independent layer is
justified by the fact that some controllers support both SPI memory
operations and regular SPI transfers, and it's cleaner to have both
features exposed through a single driver.

For those who want to have the full picture, here is a branch [1]
containing the SPI NAND framework based on top of this spi-mem layer.

Thanks,

Boris

[1]https://github.com/bbrezillon/linux/tree/spi-mem
[2]https://www.spinics.net/lists/linux-spi/msg12058.html

Boris Brezillon (7):
  spi: Extend the core to ease integration of SPI memory controllers
  spi: Make support for regular transfers optional when ->mem_ops !=
    NULL
  spi: bcm-qspi: Implement the spi_mem interface
  spi: bcm53xx: Implement the spi_mem interface
  spi: ti-qspi: Implement the spi_mem interface
  mtd: spi-nor: Use the spi_mem_xx() API
  spi: Get rid of the spi_flash_read() API

 drivers/mtd/devices/Kconfig  |   1 +
 drivers/mtd/devices/m25p80.c | 236 +++++++++----------------
 drivers/spi/Kconfig          |   7 +
 drivers/spi/Makefile         |   1 +
 drivers/spi/spi-bcm-qspi.c   | 162 ++++++++---------
 drivers/spi/spi-bcm53xx.c    |  37 ++--
 drivers/spi/spi-mem.c        | 410 +++++++++++++++++++++++++++++++++++++++++++
 drivers/spi/spi-ti-qspi.c    |  85 +++++----
 drivers/spi/spi.c            |  90 +++-------
 include/linux/spi/spi-mem.h  | 249 ++++++++++++++++++++++++++
 include/linux/spi/spi.h      |  60 +------
 11 files changed, 941 insertions(+), 397 deletions(-)
 create mode 100644 drivers/spi/spi-mem.c
 create mode 100644 include/linux/spi/spi-mem.h

-- 
2.14.1

[PATCH v4 1/7] spi: Extend the core to ease integration of SPI memory controllers

[Boris Brezillon]

Some controllers are exposing high-level interfaces to access various
kind of SPI memories. Unfortunately they do not fit in the current
spi_controller model and usually have drivers placed in
drivers/mtd/spi-nor which are only supporting SPI NORs and not SPI
memories in general.

This is an attempt at defining a SPI memory interface which works for
all kinds of SPI memories (NORs, NANDs, SRAMs).

Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
---
Changes in v4:
- none

Changes in v3:
- Fix check in spi_mem_supports_op()
- Include internals.h

Changes in v2:
- Move all changes not directly related to spi_mem out of this commit
- Do not abuse ternary operation
- Reword a few comments and fix typos
- Do not make the default buswidth check in spi_mem_support_ops() if
  the controller implements this hook
- Make the addr value an u64 instead of an array of byte
- Move all spi-mem code/defs to separate header/source files and add a
  new Kconfig option to enable this interface
---
 drivers/spi/Kconfig         |   7 +
 drivers/spi/Makefile        |   1 +
 drivers/spi/spi-mem.c       | 410 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/spi/spi-mem.h | 249 +++++++++++++++++++++++++++
 include/linux/spi/spi.h     |   7 +
 5 files changed, 674 insertions(+)
 create mode 100644 drivers/spi/spi-mem.c
 create mode 100644 include/linux/spi/spi-mem.h

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 2d4146ce2f1b..59936ab80a0d 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -47,6 +47,13 @@ config SPI_MASTER
 
 if SPI_MASTER
 
+config SPI_MEM
+	bool "SPI memory extension"
+	help
+	  Enable this option if you want to enable the SPI memory extension.
+	  This extension is meant to simplify interaction with SPI memories
+	  by providing an high-level interface to send memory-like commands.
+
 comment "SPI Master Controller Drivers"
 
 config SPI_ALTERA
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index b935f10eb961..79d6f1c7b05b 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -8,6 +8,7 @@ ccflags-$(CONFIG_SPI_DEBUG) := -DDEBUG
 # small core, mostly translating board-specific
 # config declarations into driver model code
 obj-$(CONFIG_SPI_MASTER)		+= spi.o
+obj-$(CONFIG_SPI_MEM)			+= spi-mem.o
 obj-$(CONFIG_SPI_SPIDEV)		+= spidev.o
 obj-$(CONFIG_SPI_LOOPBACK_TEST)		+= spi-loopback-test.o
 
diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c
new file mode 100644
index 000000000000..990770dfa5cf
--- /dev/null
+++ b/drivers/spi/spi-mem.c
@@ -0,0 +1,410 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2018 Exceet Electronics GmbH
+ * Copyright (C) 2018 Bootlin
+ *
+ * Author: Boris Brezillon <boris.brezillon@bootlin.com>
+ */
+#include <linux/dmaengine.h>
+#include <linux/pm_runtime.h>
+#include <linux/spi/spi.h>
+#include <linux/spi/spi-mem.h>
+
+#include "internals.h"
+
+/**
+ * spi_controller_dma_map_mem_op_data() - DMA-map the buffer attached to a
+ *					  memory operation
+ * @ctlr: the SPI controller requesting this dma_map()
+ * @op: the memory operation containing the buffer to map
+ * @sgt: a pointer to a non-initialized sg_table that will be filled by this
+ *	 function
+ *
+ * Some controllers might want to do DMA on the data buffer embedded in @op.
+ * This helper prepares everything for you and provides a ready-to-use
+ * sg_table. This function is not intended to be called from spi drivers.
+ * Only SPI controller drivers should use it.
+ * Note that the caller must ensure the memory region pointed by
+ * op->data.buf.{in,out} is DMA-able before calling this function.
+ *
+ * Return: 0 in case of success, a negative error code otherwise.
+ */
+int spi_controller_dma_map_mem_op_data(struct spi_controller *ctlr,
+				       const struct spi_mem_op *op,
+				       struct sg_table *sgt)
+{
+	struct device *dmadev;
+
+	if (!op->data.nbytes)
+		return -EINVAL;
+
+	if (op->data.dir == SPI_MEM_DATA_OUT && ctlr->dma_tx)
+		dmadev = ctlr->dma_tx->device->dev;
+	else if (op->data.dir == SPI_MEM_DATA_IN && ctlr->dma_rx)
+		dmadev = ctlr->dma_rx->device->dev;
+	else
+		dmadev = ctlr->dev.parent;
+
+	if (!dmadev)
+		return -EINVAL;
+
+	return spi_map_buf(ctlr, dmadev, sgt, op->data.buf.in, op->data.nbytes,
+			   op->data.dir == SPI_MEM_DATA_IN ?
+			   DMA_FROM_DEVICE : DMA_TO_DEVICE);
+}
+EXPORT_SYMBOL_GPL(spi_controller_dma_map_mem_op_data);
+
+/**
+ * spi_controller_dma_unmap_mem_op_data() - DMA-unmap the buffer attached to a
+ *					    memory operation
+ * @ctlr: the SPI controller requesting this dma_unmap()
+ * @op: the memory operation containing the buffer to unmap
+ * @sgt: a pointer to an sg_table previously initialized by
+ *	 spi_controller_dma_map_mem_op_data()
+ *
+ * Some controllers might want to do DMA on the data buffer embedded in @op.
+ * This helper prepares things so that the CPU can access the
+ * op->data.buf.{in,out} buffer again.
+ *
+ * This function is not intended to be called from SPI drivers. Only SPI
+ * controller drivers should use it.
+ *
+ * This function should be called after the DMA operation has finished and is
+ * only valid if the previous spi_controller_dma_map_mem_op_data() call
+ * returned 0.
+ *
+ * Return: 0 in case of success, a negative error code otherwise.
+ */
+void spi_controller_dma_unmap_mem_op_data(struct spi_controller *ctlr,
+					  const struct spi_mem_op *op,
+					  struct sg_table *sgt)
+{
+	struct device *dmadev;
+
+	if (!op->data.nbytes)
+		return;
+
+	if (op->data.dir == SPI_MEM_DATA_OUT && ctlr->dma_tx)
+		dmadev = ctlr->dma_tx->device->dev;
+	else if (op->data.dir == SPI_MEM_DATA_IN && ctlr->dma_rx)
+		dmadev = ctlr->dma_rx->device->dev;
+	else
+		dmadev = ctlr->dev.parent;
+
+	spi_unmap_buf(ctlr, dmadev, sgt,
+		      op->data.dir == SPI_MEM_DATA_IN ?
+		      DMA_FROM_DEVICE : DMA_TO_DEVICE);
+}
+EXPORT_SYMBOL_GPL(spi_controller_dma_unmap_mem_op_data);
+
+static int spi_check_buswidth_req(struct spi_mem *mem, u8 buswidth, bool tx)
+{
+	u32 mode = mem->spi->mode;
+
+	switch (buswidth) {
+	case 1:
+		return 0;
+
+	case 2:
+		if ((tx && (mode & (SPI_TX_DUAL | SPI_TX_QUAD))) ||
+		    (!tx && (mode & (SPI_RX_DUAL | SPI_RX_QUAD))))
+			return 0;
+
+		break;
+
+	case 4:
+		if ((tx && (mode & SPI_TX_QUAD)) ||
+		    (!tx && (mode & SPI_RX_QUAD)))
+			return 0;
+
+		break;
+
+	default:
+		break;
+	}
+
+	return -ENOTSUPP;
+}
+
+static bool spi_mem_default_supports_op(struct spi_mem *mem,
+					const struct spi_mem_op *op)
+{
+	if (spi_check_buswidth_req(mem, op->cmd.buswidth, true))
+		return false;
+
+	if (op->addr.nbytes &&
+	    spi_check_buswidth_req(mem, op->addr.buswidth, true))
+		return false;
+
+	if (op->dummy.nbytes &&
+	    spi_check_buswidth_req(mem, op->dummy.buswidth, true))
+		return false;
+
+	if (op->data.nbytes &&
+	    spi_check_buswidth_req(mem, op->data.buswidth,
+				   op->data.dir == SPI_MEM_DATA_OUT))
+		return false;
+
+	return true;
+}
+EXPORT_SYMBOL_GPL(spi_mem_default_supports_op);
+
+/**
+ * spi_mem_supports_op() - Check if a memory device and the controller it is
+ *			   connected to support a specific memory operation
+ * @mem: the SPI memory
+ * @op: the memory operation to check
+ *
+ * Some controllers are only supporting Single or Dual IOs, others might only
+ * support specific opcodes, or it can even be that the controller and device
+ * both support Quad IOs but the hardware prevents you from using it because
+ * only 2 IO lines are connected.
+ *
+ * This function checks whether a specific operation is supported.
+ *
+ * Return: true if @op is supported, false otherwise.
+ */
+bool spi_mem_supports_op(struct spi_mem *mem, const struct spi_mem_op *op)
+{
+	struct spi_controller *ctlr = mem->spi->controller;
+
+	if (ctlr->mem_ops && ctlr->mem_ops->supports_op)
+		return ctlr->mem_ops->supports_op(mem, op);
+
+	return spi_mem_default_supports_op(mem, op);
+}
+EXPORT_SYMBOL_GPL(spi_mem_supports_op);
+
+/**
+ * spi_mem_exec_op() - Execute a memory operation
+ * @mem: the SPI memory
+ * @op: the memory operation to execute
+ *
+ * Executes a memory operation.
+ *
+ * This function first checks that @op is supported and then tries to execute
+ * it.
+ *
+ * Return: 0 in case of success, a negative error code otherwise.
+ */
+int spi_mem_exec_op(struct spi_mem *mem, const struct spi_mem_op *op)
+{
+	unsigned int tmpbufsize, xferpos = 0, totalxferlen = 0;
+	struct spi_controller *ctlr = mem->spi->controller;
+	struct spi_transfer xfers[4] = { };
+	struct spi_message msg;
+	u8 *tmpbuf;
+	int ret;
+
+	if (!spi_mem_supports_op(mem, op))
+		return -ENOTSUPP;
+
+	if (ctlr->mem_ops) {
+		/*
+		 * Flush the message queue before executing our SPI memory
+		 * operation to prevent preemption of regular SPI transfers.
+		 */
+		spi_flush_queue(ctlr);
+
+		if (ctlr->auto_runtime_pm) {
+			ret = pm_runtime_get_sync(ctlr->dev.parent);
+			if (ret < 0) {
+				dev_err(&ctlr->dev,
+					"Failed to power device: %d\n",
+					ret);
+				return ret;
+			}
+		}
+
+		mutex_lock(&ctlr->bus_lock_mutex);
+		mutex_lock(&ctlr->io_mutex);
+		ret = ctlr->mem_ops->exec_op(mem, op);
+		mutex_unlock(&ctlr->io_mutex);
+		mutex_unlock(&ctlr->bus_lock_mutex);
+
+		if (ctlr->auto_runtime_pm)
+			pm_runtime_put(ctlr->dev.parent);
+
+		/*
+		 * Some controllers only optimize specific paths (typically the
+		 * read path) and expect the core to use the regular SPI
+		 * interface in other cases.
+		 */
+		if (!ret || ret != -ENOTSUPP)
+			return ret;
+	}
+
+	tmpbufsize = sizeof(op->cmd.opcode) + op->addr.nbytes +
+		     op->dummy.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.
+	 */
+	tmpbuf = kzalloc(tmpbufsize, GFP_KERNEL | GFP_DMA);
+	if (!tmpbuf)
+		return -ENOMEM;
+
+	spi_message_init(&msg);
+
+	tmpbuf[0] = op->cmd.opcode;
+	xfers[xferpos].tx_buf = tmpbuf;
+	xfers[xferpos].len = sizeof(op->cmd.opcode);
+	xfers[xferpos].tx_nbits = op->cmd.buswidth;
+	spi_message_add_tail(&xfers[xferpos], &msg);
+	xferpos++;
+	totalxferlen++;
+
+	if (op->addr.nbytes) {
+		int i;
+
+		for (i = 0; i < op->addr.nbytes; i++)
+			tmpbuf[i + 1] = op->addr.val >>
+					(8 * (op->addr.nbytes - i - 1));
+
+		xfers[xferpos].tx_buf = tmpbuf + 1;
+		xfers[xferpos].len = op->addr.nbytes;
+		xfers[xferpos].tx_nbits = op->addr.buswidth;
+		spi_message_add_tail(&xfers[xferpos], &msg);
+		xferpos++;
+		totalxferlen += op->addr.nbytes;
+	}
+
+	if (op->dummy.nbytes) {
+		memset(tmpbuf + op->addr.nbytes + 1, 0xff, op->dummy.nbytes);
+		xfers[xferpos].tx_buf = tmpbuf + op->addr.nbytes + 1;
+		xfers[xferpos].len = op->dummy.nbytes;
+		xfers[xferpos].tx_nbits = op->dummy.buswidth;
+		spi_message_add_tail(&xfers[xferpos], &msg);
+		xferpos++;
+		totalxferlen += op->dummy.nbytes;
+	}
+
+	if (op->data.nbytes) {
+		if (op->data.dir == SPI_MEM_DATA_IN) {
+			xfers[xferpos].rx_buf = op->data.buf.in;
+			xfers[xferpos].rx_nbits = op->data.buswidth;
+		} else {
+			xfers[xferpos].tx_buf = op->data.buf.out;
+			xfers[xferpos].tx_nbits = op->data.buswidth;
+		}
+
+		xfers[xferpos].len = op->data.nbytes;
+		spi_message_add_tail(&xfers[xferpos], &msg);
+		xferpos++;
+		totalxferlen += op->data.nbytes;
+	}
+
+	ret = spi_sync(mem->spi, &msg);
+
+	kfree(tmpbuf);
+
+	if (ret)
+		return ret;
+
+	if (msg.actual_length != totalxferlen)
+		return -EIO;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(spi_mem_exec_op);
+
+/**
+ * spi_mem_adjust_op_size() - Adjust the data size of a SPI mem operation to
+ *			      match controller limitations
+ * @mem: the SPI memory
+ * @op: the operation to adjust
+ *
+ * Some controllers have FIFO limitations and must split a data transfer
+ * operation into multiple ones, others require a specific alignment for
+ * optimized accesses. This function allows SPI mem drivers to split a single
+ * operation into multiple sub-operations when required.
+ *
+ * Return: a negative error code if the controller can't properly adjust @op,
+ *	   0 otherwise. Note that @op->data.nbytes will be updated if @op
+ *	   can't be handled in a single step.
+ */
+int spi_mem_adjust_op_size(struct spi_mem *mem, struct spi_mem_op *op)
+{
+	struct spi_controller *ctlr = mem->spi->controller;
+
+	if (ctlr->mem_ops && ctlr->mem_ops->adjust_op_size)
+		return ctlr->mem_ops->adjust_op_size(mem, op);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(spi_mem_adjust_op_size);
+
+static inline struct spi_mem_driver *to_spi_mem_drv(struct device_driver *drv)
+{
+	return container_of(drv, struct spi_mem_driver, spidrv.driver);
+}
+
+static int spi_mem_probe(struct spi_device *spi)
+{
+	struct spi_mem_driver *memdrv = to_spi_mem_drv(spi->dev.driver);
+	struct spi_mem *mem;
+
+	mem = devm_kzalloc(&spi->dev, sizeof(*mem), GFP_KERNEL);
+	if (!mem)
+		return -ENOMEM;
+
+	mem->spi = spi;
+	spi_set_drvdata(spi, mem);
+
+	return memdrv->probe(mem);
+}
+
+static int spi_mem_remove(struct spi_device *spi)
+{
+	struct spi_mem_driver *memdrv = to_spi_mem_drv(spi->dev.driver);
+	struct spi_mem *mem = spi_get_drvdata(spi);
+
+	if (memdrv->remove)
+		return memdrv->remove(mem);
+
+	return 0;
+}
+
+static void spi_mem_shutdown(struct spi_device *spi)
+{
+	struct spi_mem_driver *memdrv = to_spi_mem_drv(spi->dev.driver);
+	struct spi_mem *mem = spi_get_drvdata(spi);
+
+	if (memdrv->shutdown)
+		memdrv->shutdown(mem);
+}
+
+/**
+ * spi_mem_driver_register_with_owner() - Register a SPI memory driver
+ * @memdrv: the SPI memory driver to register
+ * @owner: the owner of this driver
+ *
+ * Registers a SPI memory driver.
+ *
+ * Return: 0 in case of success, a negative error core otherwise.
+ */
+
+int spi_mem_driver_register_with_owner(struct spi_mem_driver *memdrv,
+				       struct module *owner)
+{
+	memdrv->spidrv.probe = spi_mem_probe;
+	memdrv->spidrv.remove = spi_mem_remove;
+	memdrv->spidrv.shutdown = spi_mem_shutdown;
+
+	return __spi_register_driver(owner, &memdrv->spidrv);
+}
+EXPORT_SYMBOL_GPL(spi_mem_driver_register_with_owner);
+
+/**
+ * spi_mem_driver_unregister_with_owner() - Unregister a SPI memory driver
+ * @memdrv: the SPI memory driver to unregister
+ *
+ * Unregisters a SPI memory driver.
+ */
+void spi_mem_driver_unregister(struct spi_mem_driver *memdrv)
+{
+	spi_unregister_driver(&memdrv->spidrv);
+}
+EXPORT_SYMBOL_GPL(spi_mem_driver_unregister);
diff --git a/include/linux/spi/spi-mem.h b/include/linux/spi/spi-mem.h
new file mode 100644
index 000000000000..bb4bd15ae1f6
--- /dev/null
+++ b/include/linux/spi/spi-mem.h
@@ -0,0 +1,249 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright (C) 2018 Exceet Electronics GmbH
+ * Copyright (C) 2018 Bootlin
+ *
+ * Author: Boris Brezillon <boris.brezillon@bootlin.com>
+ */
+
+#ifndef __LINUX_SPI_MEM_H
+#define __LINUX_SPI_MEM_H
+
+#include <linux/spi/spi.h>
+
+#define SPI_MEM_OP_CMD(__opcode, __buswidth)			\
+	{							\
+		.buswidth = __buswidth,				\
+		.opcode = __opcode,				\
+	}
+
+#define SPI_MEM_OP_ADDR(__nbytes, __val, __buswidth)		\
+	{							\
+		.nbytes = __nbytes,				\
+		.val = __val,					\
+		.buswidth = __buswidth,				\
+	}
+
+#define SPI_MEM_OP_NO_ADDR	{ }
+
+#define SPI_MEM_OP_DUMMY(__nbytes, __buswidth)			\
+	{							\
+		.nbytes = __nbytes,				\
+		.buswidth = __buswidth,				\
+	}
+
+#define SPI_MEM_OP_NO_DUMMY	{ }
+
+#define SPI_MEM_OP_DATA_IN(__nbytes, __buf, __buswidth)		\
+	{							\
+		.dir = SPI_MEM_DATA_IN,				\
+		.nbytes = __nbytes,				\
+		.buf.in = __buf,				\
+		.buswidth = __buswidth,				\
+	}
+
+#define SPI_MEM_OP_DATA_OUT(__nbytes, __buf, __buswidth)	\
+	{							\
+		.dir = SPI_MEM_DATA_OUT,			\
+		.nbytes = __nbytes,				\
+		.buf.out = __buf,				\
+		.buswidth = __buswidth,				\
+	}
+
+#define SPI_MEM_OP_NO_DATA	{ }
+
+/**
+ * enum spi_mem_data_dir - describes the direction of a SPI memory data
+ *			   transfer from the controller perspective
+ * @SPI_MEM_DATA_IN: data coming from the SPI memory
+ * @SPI_MEM_DATA_OUT: data sent the SPI memory
+ */
+enum spi_mem_data_dir {
+	SPI_MEM_DATA_IN,
+	SPI_MEM_DATA_OUT,
+};
+
+/**
+ * struct spi_mem_op - describes a SPI memory operation
+ * @cmd.buswidth: number of IO lines used to transmit the command
+ * @cmd.opcode: operation opcode
+ * @addr.nbytes: number of address bytes to send. Can be zero if the operation
+ *		 does not need to send an address
+ * @addr.buswidth: number of IO lines used to transmit the address cycles
+ * @addr.val: address value. This value is always sent MSB first on the bus.
+ *	      Note that only @addr.nbytes are taken into account in this
+ *	      address value, so users should make sure the value fits in the
+ *	      assigned number of bytes.
+ * @dummy.nbytes: number of dummy bytes to send after an opcode or address. Can
+ *		  be zero if the operation does not require dummy bytes
+ * @dummy.buswidth: number of IO lanes used to transmit the dummy bytes
+ * @data.buswidth: number of IO lanes used to send/receive the data
+ * @data.dir: direction of the transfer
+ * @data.buf.in: input buffer
+ * @data.buf.out: output buffer
+ */
+struct spi_mem_op {
+	struct {
+		u8 buswidth;
+		u8 opcode;
+	} cmd;
+
+	struct {
+		u8 nbytes;
+		u8 buswidth;
+		u64 val;
+	} addr;
+
+	struct {
+		u8 nbytes;
+		u8 buswidth;
+	} dummy;
+
+	struct {
+		u8 buswidth;
+		enum spi_mem_data_dir dir;
+		unsigned int nbytes;
+		/* buf.{in,out} must be DMA-able. */
+		union {
+			void *in;
+			const void *out;
+		} buf;
+	} data;
+};
+
+#define SPI_MEM_OP(__cmd, __addr, __dummy, __data)		\
+	{							\
+		.cmd = __cmd,					\
+		.addr = __addr,					\
+		.dummy = __dummy,				\
+		.data = __data,					\
+	}
+
+/**
+ * struct spi_mem - describes a SPI memory device
+ * @spi: the underlying SPI device
+ * @drvpriv: spi_mem_drviver private data
+ *
+ * Extra information that describe the SPI memory device and may be needed by
+ * the controller to properly handle this device should be placed here.
+ *
+ * One example would be the device size since some controller expose their SPI
+ * mem devices through a io-mapped region.
+ */
+struct spi_mem {
+	struct spi_device *spi;
+	void *drvpriv;
+};
+
+/**
+ * struct spi_mem_set_drvdata() - attach driver private data to a SPI mem
+ *				  device
+ * @mem: memory device
+ * @data: data to attach to the memory device
+ */
+static inline void spi_mem_set_drvdata(struct spi_mem *mem, void *data)
+{
+	mem->drvpriv = data;
+}
+
+/**
+ * struct spi_mem_get_drvdata() - get driver private data attached to a SPI mem
+ *				  device
+ * @mem: memory device
+ *
+ * Return: the data attached to the mem device.
+ */
+static inline void *spi_mem_get_drvdata(struct spi_mem *mem)
+{
+	return mem->drvpriv;
+}
+
+/**
+ * struct spi_controller_mem_ops - SPI memory operations
+ * @adjust_op_size: shrink the data xfer of an operation to match controller's
+ *		    limitations (can be alignment of max RX/TX size
+ *		    limitations)
+ * @supports_op: check if an operation is supported by the controller
+ * @exec_op: execute a SPI memory operation
+ *
+ * This interface should be implemented by SPI controllers providing an
+ * high-level interface to execute SPI memory operation, which is usually the
+ * case for QSPI controllers.
+ */
+struct spi_controller_mem_ops {
+	int (*adjust_op_size)(struct spi_mem *mem, struct spi_mem_op *op);
+	bool (*supports_op)(struct spi_mem *mem,
+			    const struct spi_mem_op *op);
+	int (*exec_op)(struct spi_mem *mem,
+		       const struct spi_mem_op *op);
+};
+
+/**
+ * struct spi_mem_driver - SPI memory driver
+ * @spidrv: inherit from a SPI driver
+ * @probe: probe a SPI memory. Usually where detection/initialization takes
+ *	   place
+ * @remove: remove a SPI memory
+ * @shutdown: take appropriate action when the system is shutdown
+ *
+ * This is just a thin wrapper around a spi_driver. The core takes care of
+ * allocating the spi_mem object and forwarding the probe/remove/shutdown
+ * request to the spi_mem_driver. The reason we use this wrapper is because
+ * we might have to stuff more information into the spi_mem struct to let
+ * SPI controllers know more about the SPI memory they interact with, and
+ * having this intermediate layer allows us to do that without adding more
+ * useless fields to the spi_device object.
+ */
+struct spi_mem_driver {
+	struct spi_driver spidrv;
+	int (*probe)(struct spi_mem *mem);
+	int (*remove)(struct spi_mem *mem);
+	void (*shutdown)(struct spi_mem *mem);
+};
+
+#if IS_ENABLED(CONFIG_SPI_MEM)
+int spi_controller_dma_map_mem_op_data(struct spi_controller *ctlr,
+				       const struct spi_mem_op *op,
+				       struct sg_table *sg);
+
+void spi_controller_dma_unmap_mem_op_data(struct spi_controller *ctlr,
+					  const struct spi_mem_op *op,
+					  struct sg_table *sg);
+#else
+static inline int
+spi_controller_dma_map_mem_op_data(struct spi_controller *ctlr,
+				   const struct spi_mem_op *op,
+				   struct sg_table *sg)
+{
+	return -ENOTSUPP;
+}
+
+static inline void
+spi_controller_dma_unmap_mem_op_data(struct spi_controller *ctlr,
+				     const struct spi_mem_op *op,
+				     struct sg_table *sg)
+{
+}
+#endif /* CONFIG_SPI_MEM */
+
+int spi_mem_adjust_op_size(struct spi_mem *mem, struct spi_mem_op *op);
+
+bool spi_mem_supports_op(struct spi_mem *mem,
+			 const struct spi_mem_op *op);
+
+int spi_mem_exec_op(struct spi_mem *mem,
+		    const struct spi_mem_op *op);
+
+int spi_mem_driver_register_with_owner(struct spi_mem_driver *drv,
+				       struct module *owner);
+
+void spi_mem_driver_unregister(struct spi_mem_driver *drv);
+
+#define spi_mem_driver_register(__drv)                                  \
+	spi_mem_driver_register_with_owner(__drv, THIS_MODULE)
+
+#define module_spi_mem_driver(__drv)                                    \
+	module_driver(__drv, spi_mem_driver_register,                   \
+		      spi_mem_driver_unregister)
+
+#endif /* __LINUX_SPI_MEM_H */
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index bc6bb325d1bf..a7e0bbed738c 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -27,6 +27,7 @@ struct property_entry;
 struct spi_controller;
 struct spi_transfer;
 struct spi_flash_read_message;
+struct spi_controller_mem_ops;
 
 /*
  * INTERFACES between SPI master-side drivers and SPI slave protocol handlers,
@@ -376,6 +377,9 @@ static inline void spi_unregister_driver(struct spi_driver *sdrv)
  *                    transfer_one callback.
  * @handle_err: the subsystem calls the driver to handle an error that occurs
  *		in the generic implementation of transfer_one_message().
+ * @mem_ops: optimized/dedicated operations for interactions with SPI memory.
+ *	     This field is optional and should only be implemented if the
+ *	     controller has native support for memory like operations.
  * @unprepare_message: undo any work done by prepare_message().
  * @slave_abort: abort the ongoing transfer request on an SPI slave controller
  * @spi_flash_read: to support spi-controller hardwares that provide
@@ -564,6 +568,9 @@ struct spi_controller {
 	void (*handle_err)(struct spi_controller *ctlr,
 			   struct spi_message *message);
 
+	/* Optimized handlers for SPI memory-like operations. */
+	const struct spi_controller_mem_ops *mem_ops;
+
 	/* gpio chip select */
 	int			*cs_gpios;
 
-- 
2.14.1

Re: [PATCH v4 1/7] spi: Extend the core to ease integration of SPI memory controllers

[Frieder Schrempf]

On 26.04.2018 18:18, Boris Brezillon wrote:
> Some controllers are exposing high-level interfaces to access various
> kind of SPI memories. Unfortunately they do not fit in the current
> spi_controller model and usually have drivers placed in
> drivers/mtd/spi-nor which are only supporting SPI NORs and not SPI
> memories in general.
> 
> This is an attempt at defining a SPI memory interface which works for
> all kinds of SPI memories (NORs, NANDs, SRAMs).
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>

Reviewed-by: Frieder Schrempf <frieder.schrempf@exceet.de>
Tested-by: Frieder Schrempf <frieder.schrempf@exceet.de>

Regards,

Frieder

> ---
> Changes in v4:
> - none
> 
> Changes in v3:
> - Fix check in spi_mem_supports_op()
> - Include internals.h
> 
> Changes in v2:
> - Move all changes not directly related to spi_mem out of this commit
> - Do not abuse ternary operation
> - Reword a few comments and fix typos
> - Do not make the default buswidth check in spi_mem_support_ops() if
>    the controller implements this hook
> - Make the addr value an u64 instead of an array of byte
> - Move all spi-mem code/defs to separate header/source files and add a
>    new Kconfig option to enable this interface
> ---
>   drivers/spi/Kconfig         |   7 +
>   drivers/spi/Makefile        |   1 +
>   drivers/spi/spi-mem.c       | 410 ++++++++++++++++++++++++++++++++++++++++++++
>   include/linux/spi/spi-mem.h | 249 +++++++++++++++++++++++++++
>   include/linux/spi/spi.h     |   7 +
>   5 files changed, 674 insertions(+)
>   create mode 100644 drivers/spi/spi-mem.c
>   create mode 100644 include/linux/spi/spi-mem.h
> 
> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
> index 2d4146ce2f1b..59936ab80a0d 100644
> --- a/drivers/spi/Kconfig
> +++ b/drivers/spi/Kconfig
> @@ -47,6 +47,13 @@ config SPI_MASTER
>   
>   if SPI_MASTER
>   
> +config SPI_MEM
> +	bool "SPI memory extension"
> +	help
> +	  Enable this option if you want to enable the SPI memory extension.
> +	  This extension is meant to simplify interaction with SPI memories
> +	  by providing an high-level interface to send memory-like commands.
> +
>   comment "SPI Master Controller Drivers"
>   
>   config SPI_ALTERA
> diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
> index b935f10eb961..79d6f1c7b05b 100644
> --- a/drivers/spi/Makefile
> +++ b/drivers/spi/Makefile
> @@ -8,6 +8,7 @@ ccflags-$(CONFIG_SPI_DEBUG) := -DDEBUG
>   # small core, mostly translating board-specific
>   # config declarations into driver model code
>   obj-$(CONFIG_SPI_MASTER)		+= spi.o
> +obj-$(CONFIG_SPI_MEM)			+= spi-mem.o
>   obj-$(CONFIG_SPI_SPIDEV)		+= spidev.o
>   obj-$(CONFIG_SPI_LOOPBACK_TEST)		+= spi-loopback-test.o
>   
> diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c
> new file mode 100644
> index 000000000000..990770dfa5cf
> --- /dev/null
> +++ b/drivers/spi/spi-mem.c
> @@ -0,0 +1,410 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2018 Exceet Electronics GmbH
> + * Copyright (C) 2018 Bootlin
> + *
> + * Author: Boris Brezillon <boris.brezillon@bootlin.com>
> + */
> +#include <linux/dmaengine.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/spi/spi.h>
> +#include <linux/spi/spi-mem.h>
> +
> +#include "internals.h"
> +
> +/**
> + * spi_controller_dma_map_mem_op_data() - DMA-map the buffer attached to a
> + *					  memory operation
> + * @ctlr: the SPI controller requesting this dma_map()
> + * @op: the memory operation containing the buffer to map
> + * @sgt: a pointer to a non-initialized sg_table that will be filled by this
> + *	 function
> + *
> + * Some controllers might want to do DMA on the data buffer embedded in @op.
> + * This helper prepares everything for you and provides a ready-to-use
> + * sg_table. This function is not intended to be called from spi drivers.
> + * Only SPI controller drivers should use it.
> + * Note that the caller must ensure the memory region pointed by
> + * op->data.buf.{in,out} is DMA-able before calling this function.
> + *
> + * Return: 0 in case of success, a negative error code otherwise.
> + */
> +int spi_controller_dma_map_mem_op_data(struct spi_controller *ctlr,
> +				       const struct spi_mem_op *op,
> +				       struct sg_table *sgt)
> +{
> +	struct device *dmadev;
> +
> +	if (!op->data.nbytes)
> +		return -EINVAL;
> +
> +	if (op->data.dir == SPI_MEM_DATA_OUT && ctlr->dma_tx)
> +		dmadev = ctlr->dma_tx->device->dev;
> +	else if (op->data.dir == SPI_MEM_DATA_IN && ctlr->dma_rx)
> +		dmadev = ctlr->dma_rx->device->dev;
> +	else
> +		dmadev = ctlr->dev.parent;
> +
> +	if (!dmadev)
> +		return -EINVAL;
> +
> +	return spi_map_buf(ctlr, dmadev, sgt, op->data.buf.in, op->data.nbytes,
> +			   op->data.dir == SPI_MEM_DATA_IN ?
> +			   DMA_FROM_DEVICE : DMA_TO_DEVICE);
> +}
> +EXPORT_SYMBOL_GPL(spi_controller_dma_map_mem_op_data);
> +
> +/**
> + * spi_controller_dma_unmap_mem_op_data() - DMA-unmap the buffer attached to a
> + *					    memory operation
> + * @ctlr: the SPI controller requesting this dma_unmap()
> + * @op: the memory operation containing the buffer to unmap
> + * @sgt: a pointer to an sg_table previously initialized by
> + *	 spi_controller_dma_map_mem_op_data()
> + *
> + * Some controllers might want to do DMA on the data buffer embedded in @op.
> + * This helper prepares things so that the CPU can access the
> + * op->data.buf.{in,out} buffer again.
> + *
> + * This function is not intended to be called from SPI drivers. Only SPI
> + * controller drivers should use it.
> + *
> + * This function should be called after the DMA operation has finished and is
> + * only valid if the previous spi_controller_dma_map_mem_op_data() call
> + * returned 0.
> + *
> + * Return: 0 in case of success, a negative error code otherwise.
> + */
> +void spi_controller_dma_unmap_mem_op_data(struct spi_controller *ctlr,
> +					  const struct spi_mem_op *op,
> +					  struct sg_table *sgt)
> +{
> +	struct device *dmadev;
> +
> +	if (!op->data.nbytes)
> +		return;
> +
> +	if (op->data.dir == SPI_MEM_DATA_OUT && ctlr->dma_tx)
> +		dmadev = ctlr->dma_tx->device->dev;
> +	else if (op->data.dir == SPI_MEM_DATA_IN && ctlr->dma_rx)
> +		dmadev = ctlr->dma_rx->device->dev;
> +	else
> +		dmadev = ctlr->dev.parent;
> +
> +	spi_unmap_buf(ctlr, dmadev, sgt,
> +		      op->data.dir == SPI_MEM_DATA_IN ?
> +		      DMA_FROM_DEVICE : DMA_TO_DEVICE);
> +}
> +EXPORT_SYMBOL_GPL(spi_controller_dma_unmap_mem_op_data);
> +
> +static int spi_check_buswidth_req(struct spi_mem *mem, u8 buswidth, bool tx)
> +{
> +	u32 mode = mem->spi->mode;
> +
> +	switch (buswidth) {
> +	case 1:
> +		return 0;
> +
> +	case 2:
> +		if ((tx && (mode & (SPI_TX_DUAL | SPI_TX_QUAD))) ||
> +		    (!tx && (mode & (SPI_RX_DUAL | SPI_RX_QUAD))))
> +			return 0;
> +
> +		break;
> +
> +	case 4:
> +		if ((tx && (mode & SPI_TX_QUAD)) ||
> +		    (!tx && (mode & SPI_RX_QUAD)))
> +			return 0;
> +
> +		break;
> +
> +	default:
> +		break;
> +	}
> +
> +	return -ENOTSUPP;
> +}
> +
> +static bool spi_mem_default_supports_op(struct spi_mem *mem,
> +					const struct spi_mem_op *op)
> +{
> +	if (spi_check_buswidth_req(mem, op->cmd.buswidth, true))
> +		return false;
> +
> +	if (op->addr.nbytes &&
> +	    spi_check_buswidth_req(mem, op->addr.buswidth, true))
> +		return false;
> +
> +	if (op->dummy.nbytes &&
> +	    spi_check_buswidth_req(mem, op->dummy.buswidth, true))
> +		return false;
> +
> +	if (op->data.nbytes &&
> +	    spi_check_buswidth_req(mem, op->data.buswidth,
> +				   op->data.dir == SPI_MEM_DATA_OUT))
> +		return false;
> +
> +	return true;
> +}
> +EXPORT_SYMBOL_GPL(spi_mem_default_supports_op);
> +
> +/**
> + * spi_mem_supports_op() - Check if a memory device and the controller it is
> + *			   connected to support a specific memory operation
> + * @mem: the SPI memory
> + * @op: the memory operation to check
> + *
> + * Some controllers are only supporting Single or Dual IOs, others might only
> + * support specific opcodes, or it can even be that the controller and device
> + * both support Quad IOs but the hardware prevents you from using it because
> + * only 2 IO lines are connected.
> + *
> + * This function checks whether a specific operation is supported.
> + *
> + * Return: true if @op is supported, false otherwise.
> + */
> +bool spi_mem_supports_op(struct spi_mem *mem, const struct spi_mem_op *op)
> +{
> +	struct spi_controller *ctlr = mem->spi->controller;
> +
> +	if (ctlr->mem_ops && ctlr->mem_ops->supports_op)
> +		return ctlr->mem_ops->supports_op(mem, op);
> +
> +	return spi_mem_default_supports_op(mem, op);
> +}
> +EXPORT_SYMBOL_GPL(spi_mem_supports_op);
> +
> +/**
> + * spi_mem_exec_op() - Execute a memory operation
> + * @mem: the SPI memory
> + * @op: the memory operation to execute
> + *
> + * Executes a memory operation.
> + *
> + * This function first checks that @op is supported and then tries to execute
> + * it.
> + *
> + * Return: 0 in case of success, a negative error code otherwise.
> + */
> +int spi_mem_exec_op(struct spi_mem *mem, const struct spi_mem_op *op)
> +{
> +	unsigned int tmpbufsize, xferpos = 0, totalxferlen = 0;
> +	struct spi_controller *ctlr = mem->spi->controller;
> +	struct spi_transfer xfers[4] = { };
> +	struct spi_message msg;
> +	u8 *tmpbuf;
> +	int ret;
> +
> +	if (!spi_mem_supports_op(mem, op))
> +		return -ENOTSUPP;
> +
> +	if (ctlr->mem_ops) {
> +		/*
> +		 * Flush the message queue before executing our SPI memory
> +		 * operation to prevent preemption of regular SPI transfers.
> +		 */
> +		spi_flush_queue(ctlr);
> +
> +		if (ctlr->auto_runtime_pm) {
> +			ret = pm_runtime_get_sync(ctlr->dev.parent);
> +			if (ret < 0) {
> +				dev_err(&ctlr->dev,
> +					"Failed to power device: %d\n",
> +					ret);
> +				return ret;
> +			}
> +		}
> +
> +		mutex_lock(&ctlr->bus_lock_mutex);
> +		mutex_lock(&ctlr->io_mutex);
> +		ret = ctlr->mem_ops->exec_op(mem, op);
> +		mutex_unlock(&ctlr->io_mutex);
> +		mutex_unlock(&ctlr->bus_lock_mutex);
> +
> +		if (ctlr->auto_runtime_pm)
> +			pm_runtime_put(ctlr->dev.parent);
> +
> +		/*
> +		 * Some controllers only optimize specific paths (typically the
> +		 * read path) and expect the core to use the regular SPI
> +		 * interface in other cases.
> +		 */
> +		if (!ret || ret != -ENOTSUPP)
> +			return ret;
> +	}
> +
> +	tmpbufsize = sizeof(op->cmd.opcode) + op->addr.nbytes +
> +		     op->dummy.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.
> +	 */
> +	tmpbuf = kzalloc(tmpbufsize, GFP_KERNEL | GFP_DMA);
> +	if (!tmpbuf)
> +		return -ENOMEM;
> +
> +	spi_message_init(&msg);
> +
> +	tmpbuf[0] = op->cmd.opcode;
> +	xfers[xferpos].tx_buf = tmpbuf;
> +	xfers[xferpos].len = sizeof(op->cmd.opcode);
> +	xfers[xferpos].tx_nbits = op->cmd.buswidth;
> +	spi_message_add_tail(&xfers[xferpos], &msg);
> +	xferpos++;
> +	totalxferlen++;
> +
> +	if (op->addr.nbytes) {
> +		int i;
> +
> +		for (i = 0; i < op->addr.nbytes; i++)
> +			tmpbuf[i + 1] = op->addr.val >>
> +					(8 * (op->addr.nbytes - i - 1));
> +
> +		xfers[xferpos].tx_buf = tmpbuf + 1;
> +		xfers[xferpos].len = op->addr.nbytes;
> +		xfers[xferpos].tx_nbits = op->addr.buswidth;
> +		spi_message_add_tail(&xfers[xferpos], &msg);
> +		xferpos++;
> +		totalxferlen += op->addr.nbytes;
> +	}
> +
> +	if (op->dummy.nbytes) {
> +		memset(tmpbuf + op->addr.nbytes + 1, 0xff, op->dummy.nbytes);
> +		xfers[xferpos].tx_buf = tmpbuf + op->addr.nbytes + 1;
> +		xfers[xferpos].len = op->dummy.nbytes;
> +		xfers[xferpos].tx_nbits = op->dummy.buswidth;
> +		spi_message_add_tail(&xfers[xferpos], &msg);
> +		xferpos++;
> +		totalxferlen += op->dummy.nbytes;
> +	}
> +
> +	if (op->data.nbytes) {
> +		if (op->data.dir == SPI_MEM_DATA_IN) {
> +			xfers[xferpos].rx_buf = op->data.buf.in;
> +			xfers[xferpos].rx_nbits = op->data.buswidth;
> +		} else {
> +			xfers[xferpos].tx_buf = op->data.buf.out;
> +			xfers[xferpos].tx_nbits = op->data.buswidth;
> +		}
> +
> +		xfers[xferpos].len = op->data.nbytes;
> +		spi_message_add_tail(&xfers[xferpos], &msg);
> +		xferpos++;
> +		totalxferlen += op->data.nbytes;
> +	}
> +
> +	ret = spi_sync(mem->spi, &msg);
> +
> +	kfree(tmpbuf);
> +
> +	if (ret)
> +		return ret;
> +
> +	if (msg.actual_length != totalxferlen)
> +		return -EIO;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(spi_mem_exec_op);
> +
> +/**
> + * spi_mem_adjust_op_size() - Adjust the data size of a SPI mem operation to
> + *			      match controller limitations
> + * @mem: the SPI memory
> + * @op: the operation to adjust
> + *
> + * Some controllers have FIFO limitations and must split a data transfer
> + * operation into multiple ones, others require a specific alignment for
> + * optimized accesses. This function allows SPI mem drivers to split a single
> + * operation into multiple sub-operations when required.
> + *
> + * Return: a negative error code if the controller can't properly adjust @op,
> + *	   0 otherwise. Note that @op->data.nbytes will be updated if @op
> + *	   can't be handled in a single step.
> + */
> +int spi_mem_adjust_op_size(struct spi_mem *mem, struct spi_mem_op *op)
> +{
> +	struct spi_controller *ctlr = mem->spi->controller;
> +
> +	if (ctlr->mem_ops && ctlr->mem_ops->adjust_op_size)
> +		return ctlr->mem_ops->adjust_op_size(mem, op);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(spi_mem_adjust_op_size);
> +
> +static inline struct spi_mem_driver *to_spi_mem_drv(struct device_driver *drv)
> +{
> +	return container_of(drv, struct spi_mem_driver, spidrv.driver);
> +}
> +
> +static int spi_mem_probe(struct spi_device *spi)
> +{
> +	struct spi_mem_driver *memdrv = to_spi_mem_drv(spi->dev.driver);
> +	struct spi_mem *mem;
> +
> +	mem = devm_kzalloc(&spi->dev, sizeof(*mem), GFP_KERNEL);
> +	if (!mem)
> +		return -ENOMEM;
> +
> +	mem->spi = spi;
> +	spi_set_drvdata(spi, mem);
> +
> +	return memdrv->probe(mem);
> +}
> +
> +static int spi_mem_remove(struct spi_device *spi)
> +{
> +	struct spi_mem_driver *memdrv = to_spi_mem_drv(spi->dev.driver);
> +	struct spi_mem *mem = spi_get_drvdata(spi);
> +
> +	if (memdrv->remove)
> +		return memdrv->remove(mem);
> +
> +	return 0;
> +}
> +
> +static void spi_mem_shutdown(struct spi_device *spi)
> +{
> +	struct spi_mem_driver *memdrv = to_spi_mem_drv(spi->dev.driver);
> +	struct spi_mem *mem = spi_get_drvdata(spi);
> +
> +	if (memdrv->shutdown)
> +		memdrv->shutdown(mem);
> +}
> +
> +/**
> + * spi_mem_driver_register_with_owner() - Register a SPI memory driver
> + * @memdrv: the SPI memory driver to register
> + * @owner: the owner of this driver
> + *
> + * Registers a SPI memory driver.
> + *
> + * Return: 0 in case of success, a negative error core otherwise.
> + */
> +
> +int spi_mem_driver_register_with_owner(struct spi_mem_driver *memdrv,
> +				       struct module *owner)
> +{
> +	memdrv->spidrv.probe = spi_mem_probe;
> +	memdrv->spidrv.remove = spi_mem_remove;
> +	memdrv->spidrv.shutdown = spi_mem_shutdown;
> +
> +	return __spi_register_driver(owner, &memdrv->spidrv);
> +}
> +EXPORT_SYMBOL_GPL(spi_mem_driver_register_with_owner);
> +
> +/**
> + * spi_mem_driver_unregister_with_owner() - Unregister a SPI memory driver
> + * @memdrv: the SPI memory driver to unregister
> + *
> + * Unregisters a SPI memory driver.
> + */
> +void spi_mem_driver_unregister(struct spi_mem_driver *memdrv)
> +{
> +	spi_unregister_driver(&memdrv->spidrv);
> +}
> +EXPORT_SYMBOL_GPL(spi_mem_driver_unregister);
> diff --git a/include/linux/spi/spi-mem.h b/include/linux/spi/spi-mem.h
> new file mode 100644
> index 000000000000..bb4bd15ae1f6
> --- /dev/null
> +++ b/include/linux/spi/spi-mem.h
> @@ -0,0 +1,249 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Copyright (C) 2018 Exceet Electronics GmbH
> + * Copyright (C) 2018 Bootlin
> + *
> + * Author: Boris Brezillon <boris.brezillon@bootlin.com>
> + */
> +
> +#ifndef __LINUX_SPI_MEM_H
> +#define __LINUX_SPI_MEM_H
> +
> +#include <linux/spi/spi.h>
> +
> +#define SPI_MEM_OP_CMD(__opcode, __buswidth)			\
> +	{							\
> +		.buswidth = __buswidth,				\
> +		.opcode = __opcode,				\
> +	}
> +
> +#define SPI_MEM_OP_ADDR(__nbytes, __val, __buswidth)		\
> +	{							\
> +		.nbytes = __nbytes,				\
> +		.val = __val,					\
> +		.buswidth = __buswidth,				\
> +	}
> +
> +#define SPI_MEM_OP_NO_ADDR	{ }
> +
> +#define SPI_MEM_OP_DUMMY(__nbytes, __buswidth)			\
> +	{							\
> +		.nbytes = __nbytes,				\
> +		.buswidth = __buswidth,				\
> +	}
> +
> +#define SPI_MEM_OP_NO_DUMMY	{ }
> +
> +#define SPI_MEM_OP_DATA_IN(__nbytes, __buf, __buswidth)		\
> +	{							\
> +		.dir = SPI_MEM_DATA_IN,				\
> +		.nbytes = __nbytes,				\
> +		.buf.in = __buf,				\
> +		.buswidth = __buswidth,				\
> +	}
> +
> +#define SPI_MEM_OP_DATA_OUT(__nbytes, __buf, __buswidth)	\
> +	{							\
> +		.dir = SPI_MEM_DATA_OUT,			\
> +		.nbytes = __nbytes,				\
> +		.buf.out = __buf,				\
> +		.buswidth = __buswidth,				\
> +	}
> +
> +#define SPI_MEM_OP_NO_DATA	{ }
> +
> +/**
> + * enum spi_mem_data_dir - describes the direction of a SPI memory data
> + *			   transfer from the controller perspective
> + * @SPI_MEM_DATA_IN: data coming from the SPI memory
> + * @SPI_MEM_DATA_OUT: data sent the SPI memory
> + */
> +enum spi_mem_data_dir {
> +	SPI_MEM_DATA_IN,
> +	SPI_MEM_DATA_OUT,
> +};
> +
> +/**
> + * struct spi_mem_op - describes a SPI memory operation
> + * @cmd.buswidth: number of IO lines used to transmit the command
> + * @cmd.opcode: operation opcode
> + * @addr.nbytes: number of address bytes to send. Can be zero if the operation
> + *		 does not need to send an address
> + * @addr.buswidth: number of IO lines used to transmit the address cycles
> + * @addr.val: address value. This value is always sent MSB first on the bus.
> + *	      Note that only @addr.nbytes are taken into account in this
> + *	      address value, so users should make sure the value fits in the
> + *	      assigned number of bytes.
> + * @dummy.nbytes: number of dummy bytes to send after an opcode or address. Can
> + *		  be zero if the operation does not require dummy bytes
> + * @dummy.buswidth: number of IO lanes used to transmit the dummy bytes
> + * @data.buswidth: number of IO lanes used to send/receive the data
> + * @data.dir: direction of the transfer
> + * @data.buf.in: input buffer
> + * @data.buf.out: output buffer
> + */
> +struct spi_mem_op {
> +	struct {
> +		u8 buswidth;
> +		u8 opcode;
> +	} cmd;
> +
> +	struct {
> +		u8 nbytes;
> +		u8 buswidth;
> +		u64 val;
> +	} addr;
> +
> +	struct {
> +		u8 nbytes;
> +		u8 buswidth;
> +	} dummy;
> +
> +	struct {
> +		u8 buswidth;
> +		enum spi_mem_data_dir dir;
> +		unsigned int nbytes;
> +		/* buf.{in,out} must be DMA-able. */
> +		union {
> +			void *in;
> +			const void *out;
> +		} buf;
> +	} data;
> +};
> +
> +#define SPI_MEM_OP(__cmd, __addr, __dummy, __data)		\
> +	{							\
> +		.cmd = __cmd,					\
> +		.addr = __addr,					\
> +		.dummy = __dummy,				\
> +		.data = __data,					\
> +	}
> +
> +/**
> + * struct spi_mem - describes a SPI memory device
> + * @spi: the underlying SPI device
> + * @drvpriv: spi_mem_drviver private data
> + *
> + * Extra information that describe the SPI memory device and may be needed by
> + * the controller to properly handle this device should be placed here.
> + *
> + * One example would be the device size since some controller expose their SPI
> + * mem devices through a io-mapped region.
> + */
> +struct spi_mem {
> +	struct spi_device *spi;
> +	void *drvpriv;
> +};
> +
> +/**
> + * struct spi_mem_set_drvdata() - attach driver private data to a SPI mem
> + *				  device
> + * @mem: memory device
> + * @data: data to attach to the memory device
> + */
> +static inline void spi_mem_set_drvdata(struct spi_mem *mem, void *data)
> +{
> +	mem->drvpriv = data;
> +}
> +
> +/**
> + * struct spi_mem_get_drvdata() - get driver private data attached to a SPI mem
> + *				  device
> + * @mem: memory device
> + *
> + * Return: the data attached to the mem device.
> + */
> +static inline void *spi_mem_get_drvdata(struct spi_mem *mem)
> +{
> +	return mem->drvpriv;
> +}
> +
> +/**
> + * struct spi_controller_mem_ops - SPI memory operations
> + * @adjust_op_size: shrink the data xfer of an operation to match controller's
> + *		    limitations (can be alignment of max RX/TX size
> + *		    limitations)
> + * @supports_op: check if an operation is supported by the controller
> + * @exec_op: execute a SPI memory operation
> + *
> + * This interface should be implemented by SPI controllers providing an
> + * high-level interface to execute SPI memory operation, which is usually the
> + * case for QSPI controllers.
> + */
> +struct spi_controller_mem_ops {
> +	int (*adjust_op_size)(struct spi_mem *mem, struct spi_mem_op *op);
> +	bool (*supports_op)(struct spi_mem *mem,
> +			    const struct spi_mem_op *op);
> +	int (*exec_op)(struct spi_mem *mem,
> +		       const struct spi_mem_op *op);
> +};
> +
> +/**
> + * struct spi_mem_driver - SPI memory driver
> + * @spidrv: inherit from a SPI driver
> + * @probe: probe a SPI memory. Usually where detection/initialization takes
> + *	   place
> + * @remove: remove a SPI memory
> + * @shutdown: take appropriate action when the system is shutdown
> + *
> + * This is just a thin wrapper around a spi_driver. The core takes care of
> + * allocating the spi_mem object and forwarding the probe/remove/shutdown
> + * request to the spi_mem_driver. The reason we use this wrapper is because
> + * we might have to stuff more information into the spi_mem struct to let
> + * SPI controllers know more about the SPI memory they interact with, and
> + * having this intermediate layer allows us to do that without adding more
> + * useless fields to the spi_device object.
> + */
> +struct spi_mem_driver {
> +	struct spi_driver spidrv;
> +	int (*probe)(struct spi_mem *mem);
> +	int (*remove)(struct spi_mem *mem);
> +	void (*shutdown)(struct spi_mem *mem);
> +};
> +
> +#if IS_ENABLED(CONFIG_SPI_MEM)
> +int spi_controller_dma_map_mem_op_data(struct spi_controller *ctlr,
> +				       const struct spi_mem_op *op,
> +				       struct sg_table *sg);
> +
> +void spi_controller_dma_unmap_mem_op_data(struct spi_controller *ctlr,
> +					  const struct spi_mem_op *op,
> +					  struct sg_table *sg);
> +#else
> +static inline int
> +spi_controller_dma_map_mem_op_data(struct spi_controller *ctlr,
> +				   const struct spi_mem_op *op,
> +				   struct sg_table *sg)
> +{
> +	return -ENOTSUPP;
> +}
> +
> +static inline void
> +spi_controller_dma_unmap_mem_op_data(struct spi_controller *ctlr,
> +				     const struct spi_mem_op *op,
> +				     struct sg_table *sg)
> +{
> +}
> +#endif /* CONFIG_SPI_MEM */
> +
> +int spi_mem_adjust_op_size(struct spi_mem *mem, struct spi_mem_op *op);
> +
> +bool spi_mem_supports_op(struct spi_mem *mem,
> +			 const struct spi_mem_op *op);
> +
> +int spi_mem_exec_op(struct spi_mem *mem,
> +		    const struct spi_mem_op *op);
> +
> +int spi_mem_driver_register_with_owner(struct spi_mem_driver *drv,
> +				       struct module *owner);
> +
> +void spi_mem_driver_unregister(struct spi_mem_driver *drv);
> +
> +#define spi_mem_driver_register(__drv)                                  \
> +	spi_mem_driver_register_with_owner(__drv, THIS_MODULE)
> +
> +#define module_spi_mem_driver(__drv)                                    \
> +	module_driver(__drv, spi_mem_driver_register,                   \
> +		      spi_mem_driver_unregister)
> +
> +#endif /* __LINUX_SPI_MEM_H */
> diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
> index bc6bb325d1bf..a7e0bbed738c 100644
> --- a/include/linux/spi/spi.h
> +++ b/include/linux/spi/spi.h
> @@ -27,6 +27,7 @@ struct property_entry;
>   struct spi_controller;
>   struct spi_transfer;
>   struct spi_flash_read_message;
> +struct spi_controller_mem_ops;
>   
>   /*
>    * INTERFACES between SPI master-side drivers and SPI slave protocol handlers,
> @@ -376,6 +377,9 @@ static inline void spi_unregister_driver(struct spi_driver *sdrv)
>    *                    transfer_one callback.
>    * @handle_err: the subsystem calls the driver to handle an error that occurs
>    *		in the generic implementation of transfer_one_message().
> + * @mem_ops: optimized/dedicated operations for interactions with SPI memory.
> + *	     This field is optional and should only be implemented if the
> + *	     controller has native support for memory like operations.
>    * @unprepare_message: undo any work done by prepare_message().
>    * @slave_abort: abort the ongoing transfer request on an SPI slave controller
>    * @spi_flash_read: to support spi-controller hardwares that provide
> @@ -564,6 +568,9 @@ struct spi_controller {
>   	void (*handle_err)(struct spi_controller *ctlr,
>   			   struct spi_message *message);
>   
> +	/* Optimized handlers for SPI memory-like operations. */
> +	const struct spi_controller_mem_ops *mem_ops;
> +
>   	/* gpio chip select */
>   	int			*cs_gpios;
>   
> 

[PATCH v4 2/7] spi: Make support for regular transfers optional when ->mem_ops != NULL

[Boris Brezillon]

Some SPI/QuadSPI controllers only expose a high-level SPI memory
interface, thus preventing any regular SPI transfers from being done.

In that case, SPI controller drivers can leave all ->transfer_xxx()
hooks empty and only implement the spi_mem_ops interface.

Adjust the core to allow such situations:
- extend spi_controller_check_ops() to accept situations where all
  ->transfer_xxx() pointers are NULL only if ->mem_ops != NULL
- make sure we do not initialize the SPI message queue if
  ctlr->transfer_one and ctlr->transfer_one_message are missing
- return -ENOTSUPP if someone tries to do a regular SPI transfer on
  a controller that does not support it

Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
---
Changes in v4:
- none

Changes in v3:
- none

Changes in v2:
- patch added in v2
---
 drivers/spi/spi.c | 33 ++++++++++++++++++++++++++-------
 1 file changed, 26 insertions(+), 7 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 9ab65fb2738e..c85b0cf7b4a9 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -28,6 +28,7 @@
 #include <linux/slab.h>
 #include <linux/mod_devicetable.h>
 #include <linux/spi/spi.h>
+#include <linux/spi/spi-mem.h>
 #include <linux/of_gpio.h>
 #include <linux/pm_runtime.h>
 #include <linux/pm_domain.h>
@@ -2071,12 +2072,19 @@ static int of_spi_register_master(struct spi_controller *ctlr)
 static int spi_controller_check_ops(struct spi_controller *ctlr)
 {
 	/*
-	 * The controller must at least implement one of the ->transfer()
-	 * hooks.
+	 * The controller may implement only the high-level SPI-memory like
+	 * operations if it does not support regular SPI transfers, and this is
+	 * valid use case.
+	 * If ->mem_ops is NULL, we request that at least one of the
+	 * ->transfer_xxx() method be implemented.
 	 */
-	if (!ctlr->transfer && !ctlr->transfer_one &&
-	    !ctlr->transfer_one_message)
+	if (ctlr->mem_ops) {
+		if (!ctlr->mem_ops->exec_op)
+			return -EINVAL;
+	} else if (!ctlr->transfer && !ctlr->transfer_one &&
+		   !ctlr->transfer_one_message) {
 		return -EINVAL;
+	}
 
 	return 0;
 }
@@ -2187,10 +2195,14 @@ int spi_register_controller(struct spi_controller *ctlr)
 			spi_controller_is_slave(ctlr) ? "slave" : "master",
 			dev_name(&ctlr->dev));
 
-	/* If we're using a queued driver, start the queue */
-	if (ctlr->transfer)
+	/*
+	 * If we're using a queued driver, start the queue. Note that we don't
+	 * need the queueing logic if the driver is only supporting high-level
+	 * memory operations.
+	 */
+	if (ctlr->transfer) {
 		dev_info(dev, "controller is unqueued, this is deprecated\n");
-	else {
+	} else if (ctlr->transfer_one || ctlr->transfer_one_message) {
 		status = spi_controller_initialize_queue(ctlr);
 		if (status) {
 			device_del(&ctlr->dev);
@@ -2920,6 +2932,13 @@ static int __spi_async(struct spi_device *spi, struct spi_message *message)
 {
 	struct spi_controller *ctlr = spi->controller;
 
+	/*
+	 * Some controllers do not support doing regular SPI transfers. Return
+	 * ENOTSUPP when this is the case.
+	 */
+	if (!ctlr->transfer)
+		return -ENOTSUPP;
+
 	message->spi = spi;
 
 	SPI_STATISTICS_INCREMENT_FIELD(&ctlr->statistics, spi_async);
-- 
2.14.1

Re: [PATCH v4 2/7] spi: Make support for regular transfers optional when ->mem_ops != NULL

[Frieder Schrempf]

On 26.04.2018 18:18, Boris Brezillon wrote:
> Some SPI/QuadSPI controllers only expose a high-level SPI memory
> interface, thus preventing any regular SPI transfers from being done.
> 
> In that case, SPI controller drivers can leave all ->transfer_xxx()
> hooks empty and only implement the spi_mem_ops interface.
> 
> Adjust the core to allow such situations:
> - extend spi_controller_check_ops() to accept situations where all
>    ->transfer_xxx() pointers are NULL only if ->mem_ops != NULL
> - make sure we do not initialize the SPI message queue if
>    ctlr->transfer_one and ctlr->transfer_one_message are missing
> - return -ENOTSUPP if someone tries to do a regular SPI transfer on
>    a controller that does not support it
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>

Reviewed-by: Frieder Schrempf <frieder.schrempf@exceet.de>
Tested-by: Frieder Schrempf <frieder.schrempf@exceet.de>

Regards,

Frieder

> ---
> Changes in v4:
> - none
> 
> Changes in v3:
> - none
> 
> Changes in v2:
> - patch added in v2
> ---
>   drivers/spi/spi.c | 33 ++++++++++++++++++++++++++-------
>   1 file changed, 26 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> index 9ab65fb2738e..c85b0cf7b4a9 100644
> --- a/drivers/spi/spi.c
> +++ b/drivers/spi/spi.c
> @@ -28,6 +28,7 @@
>   #include <linux/slab.h>
>   #include <linux/mod_devicetable.h>
>   #include <linux/spi/spi.h>
> +#include <linux/spi/spi-mem.h>
>   #include <linux/of_gpio.h>
>   #include <linux/pm_runtime.h>
>   #include <linux/pm_domain.h>
> @@ -2071,12 +2072,19 @@ static int of_spi_register_master(struct spi_controller *ctlr)
>   static int spi_controller_check_ops(struct spi_controller *ctlr)
>   {
>   	/*
> -	 * The controller must at least implement one of the ->transfer()
> -	 * hooks.
> +	 * The controller may implement only the high-level SPI-memory like
> +	 * operations if it does not support regular SPI transfers, and this is
> +	 * valid use case.
> +	 * If ->mem_ops is NULL, we request that at least one of the
> +	 * ->transfer_xxx() method be implemented.
>   	 */
> -	if (!ctlr->transfer && !ctlr->transfer_one &&
> -	    !ctlr->transfer_one_message)
> +	if (ctlr->mem_ops) {
> +		if (!ctlr->mem_ops->exec_op)
> +			return -EINVAL;
> +	} else if (!ctlr->transfer && !ctlr->transfer_one &&
> +		   !ctlr->transfer_one_message) {
>   		return -EINVAL;
> +	}
>   
>   	return 0;
>   }
> @@ -2187,10 +2195,14 @@ int spi_register_controller(struct spi_controller *ctlr)
>   			spi_controller_is_slave(ctlr) ? "slave" : "master",
>   			dev_name(&ctlr->dev));
>   
> -	/* If we're using a queued driver, start the queue */
> -	if (ctlr->transfer)
> +	/*
> +	 * If we're using a queued driver, start the queue. Note that we don't
> +	 * need the queueing logic if the driver is only supporting high-level
> +	 * memory operations.
> +	 */
> +	if (ctlr->transfer) {
>   		dev_info(dev, "controller is unqueued, this is deprecated\n");
> -	else {
> +	} else if (ctlr->transfer_one || ctlr->transfer_one_message) {
>   		status = spi_controller_initialize_queue(ctlr);
>   		if (status) {
>   			device_del(&ctlr->dev);
> @@ -2920,6 +2932,13 @@ static int __spi_async(struct spi_device *spi, struct spi_message *message)
>   {
>   	struct spi_controller *ctlr = spi->controller;
>   
> +	/*
> +	 * Some controllers do not support doing regular SPI transfers. Return
> +	 * ENOTSUPP when this is the case.
> +	 */
> +	if (!ctlr->transfer)
> +		return -ENOTSUPP;
> +
>   	message->spi = spi;
>   
>   	SPI_STATISTICS_INCREMENT_FIELD(&ctlr->statistics, spi_async);
> 

[PATCH v4 3/7] spi: bcm-qspi: Implement the spi_mem interface

[Boris Brezillon]

The spi_mem interface is meant to replace the ->spi_flash_read() one.
Implement the ->exec_op() method to ease removal of the old interface.

Not that ->spi_flash_read() is now implemented as a wrapper around the
new bcm_qspi_exec_mem_op() function so that we can easily get rid of
it when ->spi_flash_read() is removed.

Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
Reviewed-by: Kamal Dasu <kdasu.kdev@gmail.com>
Tested-by: Kamal Dasu <kdasu.kdev@gmail.com>
---
Changes in v4:
- Add Kamal's R-b/T-b tags

Changes in v3:
- none

Changes in v2:
- include spi-mem.h
- treat op->addr.val differently since it's now an u64
---
 drivers/spi/spi-bcm-qspi.c | 190 ++++++++++++++++++++++++++-------------------
 1 file changed, 111 insertions(+), 79 deletions(-)

diff --git a/drivers/spi/spi-bcm-qspi.c b/drivers/spi/spi-bcm-qspi.c
index 1596d35498c5..9f94268a68b5 100644
--- a/drivers/spi/spi-bcm-qspi.c
+++ b/drivers/spi/spi-bcm-qspi.c
@@ -30,6 +30,7 @@
 #include <linux/platform_device.h>
 #include <linux/slab.h>
 #include <linux/spi/spi.h>
+#include <linux/spi/spi-mem.h>
 #include <linux/sysfs.h>
 #include <linux/types.h>
 #include "spi-bcm-qspi.h"
@@ -215,10 +216,10 @@ struct bcm_qspi {
 	int bspi_maj_rev;
 	int bspi_min_rev;
 	int bspi_enabled;
-	struct spi_flash_read_message *bspi_rf_msg;
-	u32 bspi_rf_msg_idx;
-	u32 bspi_rf_msg_len;
-	u32 bspi_rf_msg_status;
+	const struct spi_mem_op *bspi_rf_op;
+	u32 bspi_rf_op_idx;
+	u32 bspi_rf_op_len;
+	u32 bspi_rf_op_status;
 	struct bcm_xfer_mode xfer_mode;
 	u32 s3_strap_override_ctrl;
 	bool bspi_mode;
@@ -313,26 +314,26 @@ static inline void bcm_qspi_bspi_lr_clear(struct bcm_qspi *qspi)
 
 static void bcm_qspi_bspi_lr_data_read(struct bcm_qspi *qspi)
 {
-	u32 *buf = (u32 *)qspi->bspi_rf_msg->buf;
+	u32 *buf = (u32 *)qspi->bspi_rf_op->data.buf.in;
 	u32 data = 0;
 
-	dev_dbg(&qspi->pdev->dev, "xfer %p rx %p rxlen %d\n", qspi->bspi_rf_msg,
-		qspi->bspi_rf_msg->buf, qspi->bspi_rf_msg_len);
+	dev_dbg(&qspi->pdev->dev, "xfer %p rx %p rxlen %d\n", qspi->bspi_rf_op,
+		qspi->bspi_rf_op->data.buf.in, qspi->bspi_rf_op_len);
 	while (!bcm_qspi_bspi_lr_is_fifo_empty(qspi)) {
 		data = bcm_qspi_bspi_lr_read_fifo(qspi);
-		if (likely(qspi->bspi_rf_msg_len >= 4) &&
+		if (likely(qspi->bspi_rf_op_len >= 4) &&
 		    IS_ALIGNED((uintptr_t)buf, 4)) {
-			buf[qspi->bspi_rf_msg_idx++] = data;
-			qspi->bspi_rf_msg_len -= 4;
+			buf[qspi->bspi_rf_op_idx++] = data;
+			qspi->bspi_rf_op_len -= 4;
 		} else {
 			/* Read out remaining bytes, make sure*/
-			u8 *cbuf = (u8 *)&buf[qspi->bspi_rf_msg_idx];
+			u8 *cbuf = (u8 *)&buf[qspi->bspi_rf_op_idx];
 
 			data = cpu_to_le32(data);
-			while (qspi->bspi_rf_msg_len) {
+			while (qspi->bspi_rf_op_len) {
 				*cbuf++ = (u8)data;
 				data >>= 8;
-				qspi->bspi_rf_msg_len--;
+				qspi->bspi_rf_op_len--;
 			}
 		}
 	}
@@ -349,14 +350,12 @@ static void bcm_qspi_bspi_set_xfer_params(struct bcm_qspi *qspi, u8 cmd_byte,
 }
 
 static int bcm_qspi_bspi_set_flex_mode(struct bcm_qspi *qspi,
-				       struct spi_flash_read_message *msg,
-				       int hp)
+				       const struct spi_mem_op *op, int hp)
 {
 	int bpc = 0, bpp = 0;
-	u8 command = msg->read_opcode;
-	int width  = msg->data_nbits ? msg->data_nbits : SPI_NBITS_SINGLE;
-	int addrlen = msg->addr_width;
-	int addr_nbits = msg->addr_nbits ? msg->addr_nbits : SPI_NBITS_SINGLE;
+	u8 command = op->cmd.opcode;
+	int width  = op->cmd.buswidth ? op->cmd.buswidth : SPI_NBITS_SINGLE;
+	int addrlen = op->addr.nbytes * 8;
 	int flex_mode = 1;
 
 	dev_dbg(&qspi->pdev->dev, "set flex mode w %x addrlen %x hp %d\n",
@@ -365,7 +364,7 @@ static int bcm_qspi_bspi_set_flex_mode(struct bcm_qspi *qspi,
 	if (addrlen == BSPI_ADDRLEN_4BYTES)
 		bpp = BSPI_BPP_ADDR_SELECT_MASK;
 
-	bpp |= msg->dummy_bytes * (8/addr_nbits);
+	bpp |= (op->dummy.nbytes * 8) / op->dummy.buswidth;
 
 	switch (width) {
 	case SPI_NBITS_SINGLE:
@@ -397,11 +396,10 @@ static int bcm_qspi_bspi_set_flex_mode(struct bcm_qspi *qspi,
 }
 
 static int bcm_qspi_bspi_set_override(struct bcm_qspi *qspi,
-				      struct spi_flash_read_message *msg,
-				      int hp)
+				      const struct spi_mem_op *op, int hp)
 {
-	int width = msg->data_nbits ? msg->data_nbits : SPI_NBITS_SINGLE;
-	int addrlen = msg->addr_width;
+	int width = op->data.buswidth ? op->data.buswidth : SPI_NBITS_SINGLE;
+	int addrlen = op->addr.nbytes;
 	u32 data = bcm_qspi_read(qspi, BSPI, BSPI_STRAP_OVERRIDE_CTRL);
 
 	dev_dbg(&qspi->pdev->dev, "set override mode w %x addrlen %x hp %d\n",
@@ -437,17 +435,17 @@ static int bcm_qspi_bspi_set_override(struct bcm_qspi *qspi,
 	/* set the override mode */
 	data |=	BSPI_STRAP_OVERRIDE_CTRL_OVERRIDE;
 	bcm_qspi_write(qspi, BSPI, BSPI_STRAP_OVERRIDE_CTRL, data);
-	bcm_qspi_bspi_set_xfer_params(qspi, msg->read_opcode, 0, 0, 0);
+	bcm_qspi_bspi_set_xfer_params(qspi, op->cmd.opcode, 0, 0, 0);
 
 	return 0;
 }
 
 static int bcm_qspi_bspi_set_mode(struct bcm_qspi *qspi,
-				  struct spi_flash_read_message *msg, int hp)
+				  const struct spi_mem_op *op, int hp)
 {
 	int error = 0;
-	int width = msg->data_nbits ? msg->data_nbits : SPI_NBITS_SINGLE;
-	int addrlen = msg->addr_width;
+	int width = op->data.buswidth ? op->data.buswidth : SPI_NBITS_SINGLE;
+	int addrlen = op->addr.nbytes;
 
 	/* default mode */
 	qspi->xfer_mode.flex_mode = true;
@@ -460,12 +458,12 @@ static int bcm_qspi_bspi_set_mode(struct bcm_qspi *qspi,
 		if (val & mask || qspi->s3_strap_override_ctrl & mask) {
 			qspi->xfer_mode.flex_mode = false;
 			bcm_qspi_write(qspi, BSPI, BSPI_FLEX_MODE_ENABLE, 0);
-			error = bcm_qspi_bspi_set_override(qspi, msg, hp);
+			error = bcm_qspi_bspi_set_override(qspi, op, hp);
 		}
 	}
 
 	if (qspi->xfer_mode.flex_mode)
-		error = bcm_qspi_bspi_set_flex_mode(qspi, msg, hp);
+		error = bcm_qspi_bspi_set_flex_mode(qspi, op, hp);
 
 	if (error) {
 		dev_warn(&qspi->pdev->dev,
@@ -794,19 +792,20 @@ static int write_to_hw(struct bcm_qspi *qspi, struct spi_device *spi)
 	return slot;
 }
 
-static int bcm_qspi_bspi_flash_read(struct spi_device *spi,
-				    struct spi_flash_read_message *msg)
+static int bcm_qspi_bspi_exec_mem_op(struct spi_device *spi,
+				     const struct spi_mem_op *op)
 {
 	struct bcm_qspi *qspi = spi_master_get_devdata(spi->master);
-	u32 addr = 0, len, rdlen, len_words;
+	u32 addr = 0, len, rdlen, len_words, from = 0;
 	int ret = 0;
 	unsigned long timeo = msecs_to_jiffies(100);
 	struct bcm_qspi_soc_intc *soc_intc = qspi->soc_intc;
 
 	if (bcm_qspi_bspi_ver_three(qspi))
-		if (msg->addr_width == BSPI_ADDRLEN_4BYTES)
+		if (op->addr.nbytes == BSPI_ADDRLEN_4BYTES)
 			return -EIO;
 
+	from = op->addr.val;
 	bcm_qspi_chip_select(qspi, spi->chip_select);
 	bcm_qspi_write(qspi, MSPI, MSPI_WRITE_LOCK, 0);
 
@@ -815,15 +814,15 @@ static int bcm_qspi_bspi_flash_read(struct spi_device *spi,
 	 * the upper address byte to bspi
 	 */
 	if (bcm_qspi_bspi_ver_three(qspi) == false) {
-		addr = msg->from & 0xff000000;
+		addr = from & 0xff000000;
 		bcm_qspi_write(qspi, BSPI,
 			       BSPI_BSPI_FLASH_UPPER_ADDR_BYTE, addr);
 	}
 
 	if (!qspi->xfer_mode.flex_mode)
-		addr = msg->from;
+		addr = from;
 	else
-		addr = msg->from & 0x00ffffff;
+		addr = from & 0x00ffffff;
 
 	if (bcm_qspi_bspi_ver_three(qspi) == true)
 		addr = (addr + 0xc00000) & 0xffffff;
@@ -832,8 +831,8 @@ static int bcm_qspi_bspi_flash_read(struct spi_device *spi,
 	 * read into the entire buffer by breaking the reads
 	 * into RAF buffer read lengths
 	 */
-	len = msg->len;
-	qspi->bspi_rf_msg_idx = 0;
+	len = op->data.nbytes;
+	qspi->bspi_rf_op_idx = 0;
 
 	do {
 		if (len > BSPI_READ_LENGTH)
@@ -844,9 +843,9 @@ static int bcm_qspi_bspi_flash_read(struct spi_device *spi,
 		reinit_completion(&qspi->bspi_done);
 		bcm_qspi_enable_bspi(qspi);
 		len_words = (rdlen + 3) >> 2;
-		qspi->bspi_rf_msg = msg;
-		qspi->bspi_rf_msg_status = 0;
-		qspi->bspi_rf_msg_len = rdlen;
+		qspi->bspi_rf_op = op;
+		qspi->bspi_rf_op_status = 0;
+		qspi->bspi_rf_op_len = rdlen;
 		dev_dbg(&qspi->pdev->dev,
 			"bspi xfr addr 0x%x len 0x%x", addr, rdlen);
 		bcm_qspi_write(qspi, BSPI, BSPI_RAF_START_ADDR, addr);
@@ -871,7 +870,6 @@ static int bcm_qspi_bspi_flash_read(struct spi_device *spi,
 		}
 
 		/* set msg return length */
-		msg->retlen += rdlen;
 		addr += rdlen;
 		len -= rdlen;
 	} while (len);
@@ -906,61 +904,62 @@ static int bcm_qspi_transfer_one(struct spi_master *master,
 	return 0;
 }
 
-static int bcm_qspi_mspi_flash_read(struct spi_device *spi,
-				    struct spi_flash_read_message *msg)
+static int bcm_qspi_mspi_exec_mem_op(struct spi_device *spi,
+				     const struct spi_mem_op *op)
 {
-	struct bcm_qspi *qspi = spi_master_get_devdata(spi->master);
+	struct spi_master *master = spi->master;
+	struct bcm_qspi *qspi = spi_master_get_devdata(master);
 	struct spi_transfer t[2];
-	u8 cmd[6];
-	int ret;
+	u8 cmd[6] = { };
+	int ret, i;
 
 	memset(cmd, 0, sizeof(cmd));
 	memset(t, 0, sizeof(t));
 
 	/* tx */
 	/* opcode is in cmd[0] */
-	cmd[0] = msg->read_opcode;
-	cmd[1] = msg->from >> (msg->addr_width * 8 -  8);
-	cmd[2] = msg->from >> (msg->addr_width * 8 - 16);
-	cmd[3] = msg->from >> (msg->addr_width * 8 - 24);
-	cmd[4] = msg->from >> (msg->addr_width * 8 - 32);
+	cmd[0] = op->cmd.opcode;
+	for (i = 0; i < op->addr.nbytes; i++)
+		cmd[1 + i] = op->addr.val >> (8 * (op->addr.nbytes - i - 1));
+
 	t[0].tx_buf = cmd;
-	t[0].len = msg->addr_width + msg->dummy_bytes + 1;
+	t[0].len = op->addr.nbytes + op->dummy.nbytes + 1;
 	t[0].bits_per_word = spi->bits_per_word;
-	t[0].tx_nbits = msg->opcode_nbits;
+	t[0].tx_nbits = op->cmd.buswidth;
 	/* lets mspi know that this is not last transfer */
 	qspi->trans_pos.mspi_last_trans = false;
-	ret = bcm_qspi_transfer_one(spi->master, spi, &t[0]);
+	ret = bcm_qspi_transfer_one(master, spi, &t[0]);
 
 	/* rx */
 	qspi->trans_pos.mspi_last_trans = true;
 	if (!ret) {
 		/* rx */
-		t[1].rx_buf = msg->buf;
-		t[1].len = msg->len;
-		t[1].rx_nbits =  msg->data_nbits;
+		t[1].rx_buf = op->data.buf.in;
+		t[1].len = op->data.nbytes;
+		t[1].rx_nbits =  op->data.buswidth;
 		t[1].bits_per_word = spi->bits_per_word;
-		ret = bcm_qspi_transfer_one(spi->master, spi, &t[1]);
+		ret = bcm_qspi_transfer_one(master, spi, &t[1]);
 	}
 
-	if (!ret)
-		msg->retlen = msg->len;
-
 	return ret;
 }
 
-static int bcm_qspi_flash_read(struct spi_device *spi,
-			       struct spi_flash_read_message *msg)
+static int bcm_qspi_exec_mem_op(struct spi_device *spi,
+				const struct spi_mem_op *op)
 {
 	struct bcm_qspi *qspi = spi_master_get_devdata(spi->master);
 	int ret = 0;
 	bool mspi_read = false;
-	u32 addr, len;
+	u32 addr = 0, len;
 	u_char *buf;
 
-	buf = msg->buf;
-	addr = msg->from;
-	len = msg->len;
+	if (!op->data.nbytes || !op->addr.nbytes || op->addr.nbytes > 4 ||
+	    op->data.dir != SPI_MEM_DATA_IN)
+		return -ENOTSUPP;
+
+	buf = op->data.buf.in;
+	addr = op->addr.val;
+	len = op->data.nbytes;
 
 	if (bcm_qspi_bspi_ver_three(qspi) == true) {
 		/*
@@ -982,12 +981,40 @@ static int bcm_qspi_flash_read(struct spi_device *spi,
 		mspi_read = true;
 
 	if (mspi_read)
-		return bcm_qspi_mspi_flash_read(spi, msg);
+		return bcm_qspi_mspi_exec_mem_op(spi, op);
 
-	ret = bcm_qspi_bspi_set_mode(qspi, msg, -1);
+	ret = bcm_qspi_bspi_set_mode(qspi, op, -1);
 
 	if (!ret)
-		ret = bcm_qspi_bspi_flash_read(spi, msg);
+		ret = bcm_qspi_bspi_exec_mem_op(spi, op);
+
+	return ret;
+}
+
+static int bcm_qspi_exec_mem_op_wrapper(struct spi_mem *mem,
+					const struct spi_mem_op *op)
+{
+	return bcm_qspi_exec_mem_op(mem->spi, op);
+}
+
+static int bcm_qspi_flash_read_wrapper(struct spi_device *spi,
+				       struct spi_flash_read_message *msg)
+{
+	int ret;
+	struct spi_mem_op op = SPI_MEM_OP(SPI_MEM_OP_CMD(msg->read_opcode, 1),
+					  SPI_MEM_OP_ADDR(msg->addr_width,
+							  msg->from,
+							  msg->addr_nbits),
+					  SPI_MEM_OP_DUMMY(msg->dummy_bytes,
+							   msg->addr_nbits),
+					  SPI_MEM_OP_DATA_IN(msg->len,
+							     msg->buf,
+							     msg->data_nbits));
+
+	msg->retlen = 0;
+	ret = bcm_qspi_exec_mem_op(spi, &op);
+	if (!ret)
+		msg->retlen = msg->len;
 
 	return ret;
 }
@@ -1026,10 +1053,10 @@ static irqreturn_t bcm_qspi_bspi_lr_l2_isr(int irq, void *dev_id)
 	struct bcm_qspi_soc_intc *soc_intc = qspi->soc_intc;
 	u32 status = qspi_dev_id->irqp->mask;
 
-	if (qspi->bspi_enabled && qspi->bspi_rf_msg) {
+	if (qspi->bspi_enabled && qspi->bspi_rf_op) {
 		bcm_qspi_bspi_lr_data_read(qspi);
-		if (qspi->bspi_rf_msg_len == 0) {
-			qspi->bspi_rf_msg = NULL;
+		if (qspi->bspi_rf_op_len == 0) {
+			qspi->bspi_rf_op = NULL;
 			if (qspi->soc_intc) {
 				/* disable soc BSPI interrupt */
 				soc_intc->bcm_qspi_int_set(soc_intc, BSPI_DONE,
@@ -1038,7 +1065,7 @@ static irqreturn_t bcm_qspi_bspi_lr_l2_isr(int irq, void *dev_id)
 				status = INTR_BSPI_LR_SESSION_DONE_MASK;
 			}
 
-			if (qspi->bspi_rf_msg_status)
+			if (qspi->bspi_rf_op_status)
 				bcm_qspi_bspi_lr_clear(qspi);
 			else
 				bcm_qspi_bspi_flush_prefetch_buffers(qspi);
@@ -1050,7 +1077,7 @@ static irqreturn_t bcm_qspi_bspi_lr_l2_isr(int irq, void *dev_id)
 	}
 
 	status &= INTR_BSPI_LR_SESSION_DONE_MASK;
-	if (qspi->bspi_enabled && status && qspi->bspi_rf_msg_len == 0)
+	if (qspi->bspi_enabled && status && qspi->bspi_rf_op_len == 0)
 		complete(&qspi->bspi_done);
 
 	return IRQ_HANDLED;
@@ -1063,7 +1090,7 @@ static irqreturn_t bcm_qspi_bspi_lr_err_l2_isr(int irq, void *dev_id)
 	struct bcm_qspi_soc_intc *soc_intc = qspi->soc_intc;
 
 	dev_err(&qspi->pdev->dev, "BSPI INT error\n");
-	qspi->bspi_rf_msg_status = -EIO;
+	qspi->bspi_rf_op_status = -EIO;
 	if (qspi->soc_intc)
 		/* clear soc interrupt */
 		soc_intc->bcm_qspi_int_ack(soc_intc, BSPI_ERR);
@@ -1186,6 +1213,10 @@ static void bcm_qspi_hw_uninit(struct bcm_qspi *qspi)
 
 }
 
+static const struct spi_controller_mem_ops bcm_qspi_mem_ops = {
+	.exec_op = bcm_qspi_exec_mem_op_wrapper,
+};
+
 static const struct of_device_id bcm_qspi_of_match[] = {
 	{ .compatible = "brcm,spi-bcm-qspi" },
 	{},
@@ -1228,7 +1259,8 @@ int bcm_qspi_probe(struct platform_device *pdev,
 	master->mode_bits = SPI_CPHA | SPI_CPOL | SPI_RX_DUAL | SPI_RX_QUAD;
 	master->setup = bcm_qspi_setup;
 	master->transfer_one = bcm_qspi_transfer_one;
-	master->spi_flash_read = bcm_qspi_flash_read;
+	master->spi_flash_read = bcm_qspi_flash_read_wrapper;
+	master->mem_ops = &bcm_qspi_mem_ops;
 	master->cleanup = bcm_qspi_cleanup;
 	master->dev.of_node = dev->of_node;
 	master->num_chipselect = NUM_CHIPSELECT;
-- 
2.14.1

[PATCH v4 4/7] spi: bcm53xx: Implement the spi_mem interface

[Boris Brezillon]

The spi_mem interface is meant to replace the spi_flash_read() one.
Implement the ->exec_op() method so that we can smoothly get rid of the
old interface.

Note that the current ->flash_read() implementation looks a bit fragile
since it does not take the ->read_opcode passed by the spi-nor layer
into account, which means if might not work with all kind of NORs.

Anyway, I left the logic unchanged and added a few extra checks to make
sure we're receiving something that looks like a NOR read operation.

Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
---
Changes in v4:
- none

Changes in v3:
- include spi-mem.h from spi-bcm53xx.c instead of spi-bcm2835.c

Changes in v2:
- include spi-mem.h
- treat op->addr.val differently since it's now an u64
---
 drivers/spi/spi-bcm53xx.c | 37 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi-bcm53xx.c b/drivers/spi/spi-bcm53xx.c
index d02ceb7a29d1..5044e4e4a263 100644
--- a/drivers/spi/spi-bcm53xx.c
+++ b/drivers/spi/spi-bcm53xx.c
@@ -14,6 +14,7 @@
 #include <linux/delay.h>
 #include <linux/bcma/bcma.h>
 #include <linux/spi/spi.h>
+#include <linux/spi/spi-mem.h>
 
 #include "spi-bcm53xx.h"
 
@@ -257,6 +258,38 @@ static int bcm53xxspi_transfer_one(struct spi_master *master,
 	return 0;
 }
 
+static int bcm53xxspi_exec_mem_op(struct spi_mem *mem,
+				  const struct spi_mem_op *op)
+{
+	struct bcm53xxspi *b53spi = spi_master_get_devdata(mem->spi->master);
+	u32 from;
+
+	/*
+	 * FIXME: There's nothing in this driver programming the opcode and
+	 * buswidth to be used when a read is done on the mmio window, but it
+	 * seems to be used to access a SPI NOR device, so restrict access
+	 * access to SPINOR_OP_READ commands.
+	 */
+	if (!op->data.nbytes || op->data.dir != SPI_MEM_DATA_IN ||
+	    op->addr.nbytes != 3 || op->cmd.opcode != 0x3)
+		return -ENOTSUPP;
+
+	/* Return -ENOTSUPP so that the core can fall back to normal reads. */
+	from = op->addr.val;
+	if (from + op->data.nbytes > BCM53XXSPI_FLASH_WINDOW)
+		return -ENOTSUPP;
+
+	bcm53xxspi_enable_bspi(b53spi);
+	memcpy_fromio(op->data.buf.in, b53spi->mmio_base + from,
+		      op->data.nbytes);
+
+	return 0;
+}
+
+static const struct spi_controller_mem_ops bcm53xxspi_mem_ops = {
+	.exec_op = bcm53xxspi_exec_mem_op,
+};
+
 static int bcm53xxspi_flash_read(struct spi_device *spi,
 				 struct spi_flash_read_message *msg)
 {
@@ -311,8 +344,10 @@ static int bcm53xxspi_bcma_probe(struct bcma_device *core)
 
 	master->dev.of_node = dev->of_node;
 	master->transfer_one = bcm53xxspi_transfer_one;
-	if (b53spi->mmio_base)
+	if (b53spi->mmio_base) {
+		master->mem_ops = &bcm53xxspi_mem_ops;
 		master->spi_flash_read = bcm53xxspi_flash_read;
+	}
 
 	bcma_set_drvdata(core, b53spi);
 
-- 
2.14.1

[PATCH v4 5/7] spi: ti-qspi: Implement the spi_mem interface

[Boris Brezillon]

The spi_mem interface is meant to replace the spi_flash_read() one.
Implement the ->exec_op() method so that we can smoothly get rid of the
old interface.

Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
---
Changes in v4:
- fix ->mmap_size assignment

Changes in v3:
- none

Changes in v2:
- include spi-mem.h
- treat op->addr.val differently since it's now an u64
- Fix 'buf is DMA-able' check
- Extract max mmap size from resource_size()
---
 drivers/spi/spi-ti-qspi.c | 84 +++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 71 insertions(+), 13 deletions(-)

diff --git a/drivers/spi/spi-ti-qspi.c b/drivers/spi/spi-ti-qspi.c
index c24d9b45a27c..1b12c2ff8be3 100644
--- a/drivers/spi/spi-ti-qspi.c
+++ b/drivers/spi/spi-ti-qspi.c
@@ -36,6 +36,7 @@
 #include <linux/sizes.h>
 
 #include <linux/spi/spi.h>
+#include <linux/spi/spi-mem.h>
 
 struct ti_qspi_regs {
 	u32 clkctrl;
@@ -50,6 +51,7 @@ struct ti_qspi {
 	struct spi_master	*master;
 	void __iomem            *base;
 	void __iomem            *mmap_base;
+	size_t			mmap_size;
 	struct regmap		*ctrl_base;
 	unsigned int		ctrl_reg;
 	struct clk		*fclk;
@@ -434,12 +436,10 @@ static int ti_qspi_dma_xfer(struct ti_qspi *qspi, dma_addr_t dma_dst,
 	return 0;
 }
 
-static int ti_qspi_dma_bounce_buffer(struct ti_qspi *qspi,
-				     struct spi_flash_read_message *msg)
+static int ti_qspi_dma_bounce_buffer(struct ti_qspi *qspi, loff_t offs,
+				     void *to, size_t readsize)
 {
-	size_t readsize = msg->len;
-	void *to = msg->buf;
-	dma_addr_t dma_src = qspi->mmap_phys_base + msg->from;
+	dma_addr_t dma_src = qspi->mmap_phys_base + offs;
 	int ret = 0;
 
 	/*
@@ -507,13 +507,14 @@ static void ti_qspi_disable_memory_map(struct spi_device *spi)
 	qspi->mmap_enabled = false;
 }
 
-static void ti_qspi_setup_mmap_read(struct spi_device *spi,
-				    struct spi_flash_read_message *msg)
+static void ti_qspi_setup_mmap_read(struct spi_device *spi, u8 opcode,
+				    u8 data_nbits, u8 addr_width,
+				    u8 dummy_bytes)
 {
 	struct ti_qspi  *qspi = spi_master_get_devdata(spi->master);
-	u32 memval = msg->read_opcode;
+	u32 memval = opcode;
 
-	switch (msg->data_nbits) {
+	switch (data_nbits) {
 	case SPI_NBITS_QUAD:
 		memval |= QSPI_SETUP_RD_QUAD;
 		break;
@@ -524,8 +525,8 @@ static void ti_qspi_setup_mmap_read(struct spi_device *spi,
 		memval |= QSPI_SETUP_RD_NORMAL;
 		break;
 	}
-	memval |= ((msg->addr_width - 1) << QSPI_SETUP_ADDR_SHIFT |
-		   msg->dummy_bytes << QSPI_SETUP_DUMMY_SHIFT);
+	memval |= ((addr_width - 1) << QSPI_SETUP_ADDR_SHIFT |
+		   dummy_bytes << QSPI_SETUP_DUMMY_SHIFT);
 	ti_qspi_write(qspi, memval,
 		      QSPI_SPI_SETUP_REG(spi->chip_select));
 }
@@ -546,13 +547,15 @@ static int ti_qspi_spi_flash_read(struct spi_device *spi,
 
 	if (!qspi->mmap_enabled)
 		ti_qspi_enable_memory_map(spi);
-	ti_qspi_setup_mmap_read(spi, msg);
+	ti_qspi_setup_mmap_read(spi, msg->read_opcode, msg->data_nbits,
+				msg->addr_width, msg->dummy_bytes);
 
 	if (qspi->rx_chan) {
 		if (msg->cur_msg_mapped)
 			ret = ti_qspi_dma_xfer_sg(qspi, msg->rx_sg, msg->from);
 		else
-			ret = ti_qspi_dma_bounce_buffer(qspi, msg);
+			ret = ti_qspi_dma_bounce_buffer(qspi, msg->from,
+							msg->buf, msg->len);
 		if (ret)
 			goto err_unlock;
 	} else {
@@ -566,6 +569,58 @@ static int ti_qspi_spi_flash_read(struct spi_device *spi,
 	return ret;
 }
 
+static int ti_qspi_exec_mem_op(struct spi_mem *mem,
+			       const struct spi_mem_op *op)
+{
+	struct ti_qspi *qspi = spi_master_get_devdata(mem->spi->master);
+	u32 from = 0;
+	int ret = 0;
+
+	/* Only optimize read path. */
+	if (!op->data.nbytes || op->data.dir != SPI_MEM_DATA_IN ||
+	    !op->addr.nbytes || op->addr.nbytes > 4)
+		return -ENOTSUPP;
+
+	/* Address exceeds MMIO window size, fall back to regular mode. */
+	from = op->addr.val;
+	if (from + op->data.nbytes > qspi->mmap_size)
+		return -ENOTSUPP;
+
+	mutex_lock(&qspi->list_lock);
+
+	if (!qspi->mmap_enabled)
+		ti_qspi_enable_memory_map(mem->spi);
+	ti_qspi_setup_mmap_read(mem->spi, op->cmd.opcode, op->data.buswidth,
+				op->addr.nbytes, op->dummy.nbytes);
+
+	if (qspi->rx_chan) {
+		struct sg_table sgt;
+
+		if (virt_addr_valid(op->data.buf.in) &&
+		    !spi_controller_dma_map_mem_op_data(mem->spi->master, op,
+							&sgt)) {
+			ret = ti_qspi_dma_xfer_sg(qspi, sgt, from);
+			spi_controller_dma_unmap_mem_op_data(mem->spi->master,
+							     op, &sgt);
+		} else {
+			ret = ti_qspi_dma_bounce_buffer(qspi, from,
+							op->data.buf.in,
+							op->data.nbytes);
+		}
+	} else {
+		memcpy_fromio(op->data.buf.in, qspi->mmap_base + from,
+			      op->data.nbytes);
+	}
+
+	mutex_unlock(&qspi->list_lock);
+
+	return ret;
+}
+
+static const struct spi_controller_mem_ops ti_qspi_mem_ops = {
+	.exec_op = ti_qspi_exec_mem_op,
+};
+
 static int ti_qspi_start_transfer_one(struct spi_master *master,
 		struct spi_message *m)
 {
@@ -673,6 +728,7 @@ static int ti_qspi_probe(struct platform_device *pdev)
 	master->bits_per_word_mask = SPI_BPW_MASK(32) | SPI_BPW_MASK(16) |
 				     SPI_BPW_MASK(8);
 	master->spi_flash_read = ti_qspi_spi_flash_read;
+	master->mem_ops = &ti_qspi_mem_ops;
 
 	if (!of_property_read_u32(np, "num-cs", &num_cs))
 		master->num_chipselect = num_cs;
@@ -701,6 +757,7 @@ static int ti_qspi_probe(struct platform_device *pdev)
 				"memory mapped resource not required\n");
 		}
 	}
+	qspi->mmap_size = resource_size(res_mmap);
 
 	irq = platform_get_irq(pdev, 0);
 	if (irq < 0) {
@@ -785,6 +842,7 @@ static int ti_qspi_probe(struct platform_device *pdev)
 				 PTR_ERR(qspi->mmap_base));
 			qspi->mmap_base = NULL;
 			master->spi_flash_read = NULL;
+			master->mem_ops = NULL;
 		}
 	}
 	qspi->mmap_enabled = false;
-- 
2.14.1

Re: [PATCH v4 5/7] spi: ti-qspi: Implement the spi_mem interface

[Vignesh R]

On Thursday 26 April 2018 09:48 PM, Boris Brezillon wrote:
> The spi_mem interface is meant to replace the spi_flash_read() one.
> Implement the ->exec_op() method so that we can smoothly get rid of the
> old interface.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
> ---
> Changes in v4:
> - fix ->mmap_size assignment
> 
> Changes in v3:
> - none
> 
> Changes in v2:
> - include spi-mem.h
> - treat op->addr.val differently since it's now an u64
> - Fix 'buf is DMA-able' check
> - Extract max mmap size from resource_size()
> ---
>  drivers/spi/spi-ti-qspi.c | 84 +++++++++++++++++++++++++++++++++++++++--------
>  1 file changed, 71 insertions(+), 13 deletions(-)
> 


Reviewed-by: Vignesh R <vigneshr@ti.com>
Tested-by: Vignesh R <vigneshr@ti.com>


Regards
Vignesh

> diff --git a/drivers/spi/spi-ti-qspi.c b/drivers/spi/spi-ti-qspi.c
> index c24d9b45a27c..1b12c2ff8be3 100644
> --- a/drivers/spi/spi-ti-qspi.c
> +++ b/drivers/spi/spi-ti-qspi.c
> @@ -36,6 +36,7 @@
>  #include <linux/sizes.h>
>  
>  #include <linux/spi/spi.h>
> +#include <linux/spi/spi-mem.h>
>  
>  struct ti_qspi_regs {
>  	u32 clkctrl;
> @@ -50,6 +51,7 @@ struct ti_qspi {
>  	struct spi_master	*master;
>  	void __iomem            *base;
>  	void __iomem            *mmap_base;
> +	size_t			mmap_size;
>  	struct regmap		*ctrl_base;
>  	unsigned int		ctrl_reg;
>  	struct clk		*fclk;
> @@ -434,12 +436,10 @@ static int ti_qspi_dma_xfer(struct ti_qspi *qspi, dma_addr_t dma_dst,
>  	return 0;
>  }
>  
> -static int ti_qspi_dma_bounce_buffer(struct ti_qspi *qspi,
> -				     struct spi_flash_read_message *msg)
> +static int ti_qspi_dma_bounce_buffer(struct ti_qspi *qspi, loff_t offs,
> +				     void *to, size_t readsize)
>  {
> -	size_t readsize = msg->len;
> -	void *to = msg->buf;
> -	dma_addr_t dma_src = qspi->mmap_phys_base + msg->from;
> +	dma_addr_t dma_src = qspi->mmap_phys_base + offs;
>  	int ret = 0;
>  
>  	/*
> @@ -507,13 +507,14 @@ static void ti_qspi_disable_memory_map(struct spi_device *spi)
>  	qspi->mmap_enabled = false;
>  }
>  
> -static void ti_qspi_setup_mmap_read(struct spi_device *spi,
> -				    struct spi_flash_read_message *msg)
> +static void ti_qspi_setup_mmap_read(struct spi_device *spi, u8 opcode,
> +				    u8 data_nbits, u8 addr_width,
> +				    u8 dummy_bytes)
>  {
>  	struct ti_qspi  *qspi = spi_master_get_devdata(spi->master);
> -	u32 memval = msg->read_opcode;
> +	u32 memval = opcode;
>  
> -	switch (msg->data_nbits) {
> +	switch (data_nbits) {
>  	case SPI_NBITS_QUAD:
>  		memval |= QSPI_SETUP_RD_QUAD;
>  		break;
> @@ -524,8 +525,8 @@ static void ti_qspi_setup_mmap_read(struct spi_device *spi,
>  		memval |= QSPI_SETUP_RD_NORMAL;
>  		break;
>  	}
> -	memval |= ((msg->addr_width - 1) << QSPI_SETUP_ADDR_SHIFT |
> -		   msg->dummy_bytes << QSPI_SETUP_DUMMY_SHIFT);
> +	memval |= ((addr_width - 1) << QSPI_SETUP_ADDR_SHIFT |
> +		   dummy_bytes << QSPI_SETUP_DUMMY_SHIFT);
>  	ti_qspi_write(qspi, memval,
>  		      QSPI_SPI_SETUP_REG(spi->chip_select));
>  }
> @@ -546,13 +547,15 @@ static int ti_qspi_spi_flash_read(struct spi_device *spi,
>  
>  	if (!qspi->mmap_enabled)
>  		ti_qspi_enable_memory_map(spi);
> -	ti_qspi_setup_mmap_read(spi, msg);
> +	ti_qspi_setup_mmap_read(spi, msg->read_opcode, msg->data_nbits,
> +				msg->addr_width, msg->dummy_bytes);
>  
>  	if (qspi->rx_chan) {
>  		if (msg->cur_msg_mapped)
>  			ret = ti_qspi_dma_xfer_sg(qspi, msg->rx_sg, msg->from);
>  		else
> -			ret = ti_qspi_dma_bounce_buffer(qspi, msg);
> +			ret = ti_qspi_dma_bounce_buffer(qspi, msg->from,
> +							msg->buf, msg->len);
>  		if (ret)
>  			goto err_unlock;
>  	} else {
> @@ -566,6 +569,58 @@ static int ti_qspi_spi_flash_read(struct spi_device *spi,
>  	return ret;
>  }
>  
> +static int ti_qspi_exec_mem_op(struct spi_mem *mem,
> +			       const struct spi_mem_op *op)
> +{
> +	struct ti_qspi *qspi = spi_master_get_devdata(mem->spi->master);
> +	u32 from = 0;
> +	int ret = 0;
> +
> +	/* Only optimize read path. */
> +	if (!op->data.nbytes || op->data.dir != SPI_MEM_DATA_IN ||
> +	    !op->addr.nbytes || op->addr.nbytes > 4)
> +		return -ENOTSUPP;
> +
> +	/* Address exceeds MMIO window size, fall back to regular mode. */
> +	from = op->addr.val;
> +	if (from + op->data.nbytes > qspi->mmap_size)
> +		return -ENOTSUPP;
> +
> +	mutex_lock(&qspi->list_lock);
> +
> +	if (!qspi->mmap_enabled)
> +		ti_qspi_enable_memory_map(mem->spi);
> +	ti_qspi_setup_mmap_read(mem->spi, op->cmd.opcode, op->data.buswidth,
> +				op->addr.nbytes, op->dummy.nbytes);
> +
> +	if (qspi->rx_chan) {
> +		struct sg_table sgt;
> +
> +		if (virt_addr_valid(op->data.buf.in) &&
> +		    !spi_controller_dma_map_mem_op_data(mem->spi->master, op,
> +							&sgt)) {
> +			ret = ti_qspi_dma_xfer_sg(qspi, sgt, from);
> +			spi_controller_dma_unmap_mem_op_data(mem->spi->master,
> +							     op, &sgt);
> +		} else {
> +			ret = ti_qspi_dma_bounce_buffer(qspi, from,
> +							op->data.buf.in,
> +							op->data.nbytes);
> +		}
> +	} else {
> +		memcpy_fromio(op->data.buf.in, qspi->mmap_base + from,
> +			      op->data.nbytes);
> +	}
> +
> +	mutex_unlock(&qspi->list_lock);
> +
> +	return ret;
> +}
> +
> +static const struct spi_controller_mem_ops ti_qspi_mem_ops = {
> +	.exec_op = ti_qspi_exec_mem_op,
> +};
> +
>  static int ti_qspi_start_transfer_one(struct spi_master *master,
>  		struct spi_message *m)
>  {
> @@ -673,6 +728,7 @@ static int ti_qspi_probe(struct platform_device *pdev)
>  	master->bits_per_word_mask = SPI_BPW_MASK(32) | SPI_BPW_MASK(16) |
>  				     SPI_BPW_MASK(8);
>  	master->spi_flash_read = ti_qspi_spi_flash_read;
> +	master->mem_ops = &ti_qspi_mem_ops;
>  
>  	if (!of_property_read_u32(np, "num-cs", &num_cs))
>  		master->num_chipselect = num_cs;
> @@ -701,6 +757,7 @@ static int ti_qspi_probe(struct platform_device *pdev)
>  				"memory mapped resource not required\n");
>  		}
>  	}
> +	qspi->mmap_size = resource_size(res_mmap);
>  
>  	irq = platform_get_irq(pdev, 0);
>  	if (irq < 0) {
> @@ -785,6 +842,7 @@ static int ti_qspi_probe(struct platform_device *pdev)
>  				 PTR_ERR(qspi->mmap_base));
>  			qspi->mmap_base = NULL;
>  			master->spi_flash_read = NULL;
> +			master->mem_ops = NULL;
>  		}
>  	}
>  	qspi->mmap_enabled = false;
> 

Applied "spi: ti-qspi: Implement the spi_mem interface" to the spi tree

[Mark Brown]

The patch

   spi: ti-qspi: Implement the spi_mem interface

has been applied to the spi tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From b95cb394ab591d1d6dcc7281b415fde0a3de2ae1 Mon Sep 17 00:00:00 2001
From: Boris Brezillon <boris.brezillon@bootlin.com>
Date: Thu, 26 Apr 2018 18:18:18 +0200
Subject: [PATCH] spi: ti-qspi: Implement the spi_mem interface

The spi_mem interface is meant to replace the spi_flash_read() one.
Implement the ->exec_op() method so that we can smoothly get rid of the
old interface.

Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
Reviewed-by: Vignesh R <vigneshr@ti.com>
Tested-by: Vignesh R <vigneshr@ti.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 drivers/spi/spi-ti-qspi.c | 84 +++++++++++++++++++++++++++++++++------
 1 file changed, 71 insertions(+), 13 deletions(-)

diff --git a/drivers/spi/spi-ti-qspi.c b/drivers/spi/spi-ti-qspi.c
index c24d9b45a27c..1b12c2ff8be3 100644
--- a/drivers/spi/spi-ti-qspi.c
+++ b/drivers/spi/spi-ti-qspi.c
@@ -36,6 +36,7 @@
 #include <linux/sizes.h>
 
 #include <linux/spi/spi.h>
+#include <linux/spi/spi-mem.h>
 
 struct ti_qspi_regs {
 	u32 clkctrl;
@@ -50,6 +51,7 @@ struct ti_qspi {
 	struct spi_master	*master;
 	void __iomem            *base;
 	void __iomem            *mmap_base;
+	size_t			mmap_size;
 	struct regmap		*ctrl_base;
 	unsigned int		ctrl_reg;
 	struct clk		*fclk;
@@ -434,12 +436,10 @@ static int ti_qspi_dma_xfer(struct ti_qspi *qspi, dma_addr_t dma_dst,
 	return 0;
 }
 
-static int ti_qspi_dma_bounce_buffer(struct ti_qspi *qspi,
-				     struct spi_flash_read_message *msg)
+static int ti_qspi_dma_bounce_buffer(struct ti_qspi *qspi, loff_t offs,
+				     void *to, size_t readsize)
 {
-	size_t readsize = msg->len;
-	void *to = msg->buf;
-	dma_addr_t dma_src = qspi->mmap_phys_base + msg->from;
+	dma_addr_t dma_src = qspi->mmap_phys_base + offs;
 	int ret = 0;
 
 	/*
@@ -507,13 +507,14 @@ static void ti_qspi_disable_memory_map(struct spi_device *spi)
 	qspi->mmap_enabled = false;
 }
 
-static void ti_qspi_setup_mmap_read(struct spi_device *spi,
-				    struct spi_flash_read_message *msg)
+static void ti_qspi_setup_mmap_read(struct spi_device *spi, u8 opcode,
+				    u8 data_nbits, u8 addr_width,
+				    u8 dummy_bytes)
 {
 	struct ti_qspi  *qspi = spi_master_get_devdata(spi->master);
-	u32 memval = msg->read_opcode;
+	u32 memval = opcode;
 
-	switch (msg->data_nbits) {
+	switch (data_nbits) {
 	case SPI_NBITS_QUAD:
 		memval |= QSPI_SETUP_RD_QUAD;
 		break;
@@ -524,8 +525,8 @@ static void ti_qspi_setup_mmap_read(struct spi_device *spi,
 		memval |= QSPI_SETUP_RD_NORMAL;
 		break;
 	}
-	memval |= ((msg->addr_width - 1) << QSPI_SETUP_ADDR_SHIFT |
-		   msg->dummy_bytes << QSPI_SETUP_DUMMY_SHIFT);
+	memval |= ((addr_width - 1) << QSPI_SETUP_ADDR_SHIFT |
+		   dummy_bytes << QSPI_SETUP_DUMMY_SHIFT);
 	ti_qspi_write(qspi, memval,
 		      QSPI_SPI_SETUP_REG(spi->chip_select));
 }
@@ -546,13 +547,15 @@ static int ti_qspi_spi_flash_read(struct spi_device *spi,
 
 	if (!qspi->mmap_enabled)
 		ti_qspi_enable_memory_map(spi);
-	ti_qspi_setup_mmap_read(spi, msg);
+	ti_qspi_setup_mmap_read(spi, msg->read_opcode, msg->data_nbits,
+				msg->addr_width, msg->dummy_bytes);
 
 	if (qspi->rx_chan) {
 		if (msg->cur_msg_mapped)
 			ret = ti_qspi_dma_xfer_sg(qspi, msg->rx_sg, msg->from);
 		else
-			ret = ti_qspi_dma_bounce_buffer(qspi, msg);
+			ret = ti_qspi_dma_bounce_buffer(qspi, msg->from,
+							msg->buf, msg->len);
 		if (ret)
 			goto err_unlock;
 	} else {
@@ -566,6 +569,58 @@ static int ti_qspi_spi_flash_read(struct spi_device *spi,
 	return ret;
 }
 
+static int ti_qspi_exec_mem_op(struct spi_mem *mem,
+			       const struct spi_mem_op *op)
+{
+	struct ti_qspi *qspi = spi_master_get_devdata(mem->spi->master);
+	u32 from = 0;
+	int ret = 0;
+
+	/* Only optimize read path. */
+	if (!op->data.nbytes || op->data.dir != SPI_MEM_DATA_IN ||
+	    !op->addr.nbytes || op->addr.nbytes > 4)
+		return -ENOTSUPP;
+
+	/* Address exceeds MMIO window size, fall back to regular mode. */
+	from = op->addr.val;
+	if (from + op->data.nbytes > qspi->mmap_size)
+		return -ENOTSUPP;
+
+	mutex_lock(&qspi->list_lock);
+
+	if (!qspi->mmap_enabled)
+		ti_qspi_enable_memory_map(mem->spi);
+	ti_qspi_setup_mmap_read(mem->spi, op->cmd.opcode, op->data.buswidth,
+				op->addr.nbytes, op->dummy.nbytes);
+
+	if (qspi->rx_chan) {
+		struct sg_table sgt;
+
+		if (virt_addr_valid(op->data.buf.in) &&
+		    !spi_controller_dma_map_mem_op_data(mem->spi->master, op,
+							&sgt)) {
+			ret = ti_qspi_dma_xfer_sg(qspi, sgt, from);
+			spi_controller_dma_unmap_mem_op_data(mem->spi->master,
+							     op, &sgt);
+		} else {
+			ret = ti_qspi_dma_bounce_buffer(qspi, from,
+							op->data.buf.in,
+							op->data.nbytes);
+		}
+	} else {
+		memcpy_fromio(op->data.buf.in, qspi->mmap_base + from,
+			      op->data.nbytes);
+	}
+
+	mutex_unlock(&qspi->list_lock);
+
+	return ret;
+}
+
+static const struct spi_controller_mem_ops ti_qspi_mem_ops = {
+	.exec_op = ti_qspi_exec_mem_op,
+};
+
 static int ti_qspi_start_transfer_one(struct spi_master *master,
 		struct spi_message *m)
 {
@@ -673,6 +728,7 @@ static int ti_qspi_probe(struct platform_device *pdev)
 	master->bits_per_word_mask = SPI_BPW_MASK(32) | SPI_BPW_MASK(16) |
 				     SPI_BPW_MASK(8);
 	master->spi_flash_read = ti_qspi_spi_flash_read;
+	master->mem_ops = &ti_qspi_mem_ops;
 
 	if (!of_property_read_u32(np, "num-cs", &num_cs))
 		master->num_chipselect = num_cs;
@@ -701,6 +757,7 @@ static int ti_qspi_probe(struct platform_device *pdev)
 				"memory mapped resource not required\n");
 		}
 	}
+	qspi->mmap_size = resource_size(res_mmap);
 
 	irq = platform_get_irq(pdev, 0);
 	if (irq < 0) {
@@ -785,6 +842,7 @@ static int ti_qspi_probe(struct platform_device *pdev)
 				 PTR_ERR(qspi->mmap_base));
 			qspi->mmap_base = NULL;
 			master->spi_flash_read = NULL;
+			master->mem_ops = NULL;
 		}
 	}
 	qspi->mmap_enabled = false;
-- 
2.17.0

[PATCH v4 6/7] mtd: spi-nor: Use the spi_mem_xx() API

[Boris Brezillon]

The spi_mem_xxx() API has been introduced to replace the
spi_flash_read() one. Make use of it so we can get rid of
spi_flash_read().

Note that using spi_mem_xx() also simplifies the code because this API
takes care of using the regular spi_sync() interface when the optimized
->mem_ops interface is not implemented by the controller.

Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
---
Changes in v4:
- none

Changes in v3:
- none

Changes in v2:
- include spi-mem.h
- treat op->addr.val differently since it's now an u64
- Replace SPI_MEM_OP_DATA_OUT() by SPI_MEM_OP_DATA_IN() in m25p80_read()
---
 drivers/mtd/devices/Kconfig  |   1 +
 drivers/mtd/devices/m25p80.c | 236 +++++++++++++++----------------------------
 2 files changed, 80 insertions(+), 157 deletions(-)

diff --git a/drivers/mtd/devices/Kconfig b/drivers/mtd/devices/Kconfig
index 6def5445e03e..57b02c4b3f63 100644
--- a/drivers/mtd/devices/Kconfig
+++ b/drivers/mtd/devices/Kconfig
@@ -81,6 +81,7 @@ config MTD_DATAFLASH_OTP
 config MTD_M25P80
 	tristate "Support most SPI Flash chips (AT26DF, M25P, W25X, ...)"
 	depends on SPI_MASTER && MTD_SPI_NOR
+	select SPI_MEM
 	help
 	  This enables access to most modern SPI flash chips, used for
 	  program and data storage.   Series supported include Atmel AT26DF,
diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
index a4e18f6aaa33..3dc022d3b53e 100644
--- a/drivers/mtd/devices/m25p80.c
+++ b/drivers/mtd/devices/m25p80.c
@@ -24,12 +24,13 @@
 #include <linux/mtd/partitions.h>
 
 #include <linux/spi/spi.h>
+#include <linux/spi/spi-mem.h>
 #include <linux/spi/flash.h>
 #include <linux/mtd/spi-nor.h>
 
 #define	MAX_CMD_SIZE		6
 struct m25p {
-	struct spi_device	*spi;
+	struct spi_mem		*spimem;
 	struct spi_nor		spi_nor;
 	u8			command[MAX_CMD_SIZE];
 };
@@ -37,97 +38,68 @@ struct m25p {
 static int m25p80_read_reg(struct spi_nor *nor, u8 code, u8 *val, int len)
 {
 	struct m25p *flash = nor->priv;
-	struct spi_device *spi = flash->spi;
+	struct spi_mem_op op = SPI_MEM_OP(SPI_MEM_OP_CMD(code, 1),
+					  SPI_MEM_OP_NO_ADDR,
+					  SPI_MEM_OP_NO_DUMMY,
+					  SPI_MEM_OP_DATA_IN(len, val, 1));
 	int ret;
 
-	ret = spi_write_then_read(spi, &code, 1, val, len);
+	ret = spi_mem_exec_op(flash->spimem, &op);
 	if (ret < 0)
-		dev_err(&spi->dev, "error %d reading %x\n", ret, code);
+		dev_err(&flash->spimem->spi->dev, "error %d reading %x\n", ret,
+			code);
 
 	return ret;
 }
 
-static void m25p_addr2cmd(struct spi_nor *nor, unsigned int addr, u8 *cmd)
-{
-	/* opcode is in cmd[0] */
-	cmd[1] = addr >> (nor->addr_width * 8 -  8);
-	cmd[2] = addr >> (nor->addr_width * 8 - 16);
-	cmd[3] = addr >> (nor->addr_width * 8 - 24);
-	cmd[4] = addr >> (nor->addr_width * 8 - 32);
-}
-
-static int m25p_cmdsz(struct spi_nor *nor)
-{
-	return 1 + nor->addr_width;
-}
-
 static int m25p80_write_reg(struct spi_nor *nor, u8 opcode, u8 *buf, int len)
 {
 	struct m25p *flash = nor->priv;
-	struct spi_device *spi = flash->spi;
-
-	flash->command[0] = opcode;
-	if (buf)
-		memcpy(&flash->command[1], buf, len);
+	struct spi_mem_op op = SPI_MEM_OP(SPI_MEM_OP_CMD(opcode, 1),
+					  SPI_MEM_OP_NO_ADDR,
+					  SPI_MEM_OP_NO_DUMMY,
+					  SPI_MEM_OP_DATA_OUT(len, buf, 1));
 
-	return spi_write(spi, flash->command, len + 1);
+	return spi_mem_exec_op(flash->spimem, &op);
 }
 
 static ssize_t m25p80_write(struct spi_nor *nor, loff_t to, size_t len,
 			    const u_char *buf)
 {
 	struct m25p *flash = nor->priv;
-	struct spi_device *spi = flash->spi;
-	unsigned int inst_nbits, addr_nbits, data_nbits, data_idx;
-	struct spi_transfer t[3] = {};
-	struct spi_message m;
-	int cmd_sz = m25p_cmdsz(nor);
-	ssize_t ret;
+	struct spi_mem_op op =
+			SPI_MEM_OP(SPI_MEM_OP_CMD(nor->program_opcode, 1),
+				   SPI_MEM_OP_ADDR(nor->addr_width, to, 1),
+				   SPI_MEM_OP_DUMMY(0, 1),
+				   SPI_MEM_OP_DATA_OUT(len, buf, 1));
+	size_t remaining = len;
+	int ret;
 
 	/* get transfer protocols. */
-	inst_nbits = spi_nor_get_protocol_inst_nbits(nor->write_proto);
-	addr_nbits = spi_nor_get_protocol_addr_nbits(nor->write_proto);
-	data_nbits = spi_nor_get_protocol_data_nbits(nor->write_proto);
-
-	spi_message_init(&m);
+	op.cmd.buswidth = spi_nor_get_protocol_inst_nbits(nor->write_proto);
+	op.addr.buswidth = spi_nor_get_protocol_addr_nbits(nor->write_proto);
+	op.dummy.buswidth = op.addr.buswidth;
+	op.data.buswidth = spi_nor_get_protocol_data_nbits(nor->write_proto);
 
 	if (nor->program_opcode == SPINOR_OP_AAI_WP && nor->sst_write_second)
-		cmd_sz = 1;
-
-	flash->command[0] = nor->program_opcode;
-	m25p_addr2cmd(nor, to, flash->command);
+		op.addr.nbytes = 0;
 
-	t[0].tx_buf = flash->command;
-	t[0].tx_nbits = inst_nbits;
-	t[0].len = cmd_sz;
-	spi_message_add_tail(&t[0], &m);
-
-	/* split the op code and address bytes into two transfers if needed. */
-	data_idx = 1;
-	if (addr_nbits != inst_nbits) {
-		t[0].len = 1;
+	while (remaining) {
+		op.data.nbytes = remaining < UINT_MAX ? remaining : UINT_MAX;
+		ret = spi_mem_adjust_op_size(flash->spimem, &op);
+		if (ret)
+			return ret;
 
-		t[1].tx_buf = &flash->command[1];
-		t[1].tx_nbits = addr_nbits;
-		t[1].len = cmd_sz - 1;
-		spi_message_add_tail(&t[1], &m);
+		ret = spi_mem_exec_op(flash->spimem, &op);
+		if (ret)
+			return ret;
 
-		data_idx = 2;
+		op.addr.val += op.data.nbytes;
+		remaining -= op.data.nbytes;
+		op.data.buf.out += op.data.nbytes;
 	}
 
-	t[data_idx].tx_buf = buf;
-	t[data_idx].tx_nbits = data_nbits;
-	t[data_idx].len = len;
-	spi_message_add_tail(&t[data_idx], &m);
-
-	ret = spi_sync(spi, &m);
-	if (ret)
-		return ret;
-
-	ret = m.actual_length - cmd_sz;
-	if (ret < 0)
-		return -EIO;
-	return ret;
+	return len;
 }
 
 /*
@@ -138,92 +110,39 @@ static ssize_t m25p80_read(struct spi_nor *nor, loff_t from, size_t len,
 			   u_char *buf)
 {
 	struct m25p *flash = nor->priv;
-	struct spi_device *spi = flash->spi;
-	unsigned int inst_nbits, addr_nbits, data_nbits, data_idx;
-	struct spi_transfer t[3];
-	struct spi_message m;
-	unsigned int dummy = nor->read_dummy;
-	ssize_t ret;
-	int cmd_sz;
+	struct spi_mem_op op =
+			SPI_MEM_OP(SPI_MEM_OP_CMD(nor->read_opcode, 1),
+				   SPI_MEM_OP_ADDR(nor->addr_width, from, 1),
+				   SPI_MEM_OP_DUMMY(nor->read_dummy, 1),
+				   SPI_MEM_OP_DATA_IN(len, buf, 1));
+	size_t remaining = len;
+	int ret;
 
 	/* get transfer protocols. */
-	inst_nbits = spi_nor_get_protocol_inst_nbits(nor->read_proto);
-	addr_nbits = spi_nor_get_protocol_addr_nbits(nor->read_proto);
-	data_nbits = spi_nor_get_protocol_data_nbits(nor->read_proto);
+	op.cmd.buswidth = spi_nor_get_protocol_inst_nbits(nor->read_proto);
+	op.addr.buswidth = spi_nor_get_protocol_addr_nbits(nor->read_proto);
+	op.dummy.buswidth = op.addr.buswidth;
+	op.data.buswidth = spi_nor_get_protocol_data_nbits(nor->read_proto);
 
 	/* convert the dummy cycles to the number of bytes */
-	dummy = (dummy * addr_nbits) / 8;
-
-	if (spi_flash_read_supported(spi)) {
-		struct spi_flash_read_message msg;
-
-		memset(&msg, 0, sizeof(msg));
+	op.dummy.nbytes = (nor->read_dummy * op.dummy.buswidth) / 8;
 
-		msg.buf = buf;
-		msg.from = from;
-		msg.len = len;
-		msg.read_opcode = nor->read_opcode;
-		msg.addr_width = nor->addr_width;
-		msg.dummy_bytes = dummy;
-		msg.opcode_nbits = inst_nbits;
-		msg.addr_nbits = addr_nbits;
-		msg.data_nbits = data_nbits;
-
-		ret = spi_flash_read(spi, &msg);
-		if (ret < 0)
+	while (remaining) {
+		op.data.nbytes = remaining < UINT_MAX ? remaining : UINT_MAX;
+		ret = spi_mem_adjust_op_size(flash->spimem, &op);
+		if (ret)
 			return ret;
-		return msg.retlen;
-	}
 
-	spi_message_init(&m);
-	memset(t, 0, (sizeof t));
-
-	flash->command[0] = nor->read_opcode;
-	m25p_addr2cmd(nor, from, flash->command);
-
-	t[0].tx_buf = flash->command;
-	t[0].tx_nbits = inst_nbits;
-	t[0].len = m25p_cmdsz(nor) + dummy;
-	spi_message_add_tail(&t[0], &m);
-
-	/*
-	 * Set all dummy/mode cycle bits to avoid sending some manufacturer
-	 * specific pattern, which might make the memory enter its Continuous
-	 * Read mode by mistake.
-	 * Based on the different mode cycle bit patterns listed and described
-	 * in the JESD216B specification, the 0xff value works for all memories
-	 * and all manufacturers.
-	 */
-	cmd_sz = t[0].len;
-	memset(flash->command + cmd_sz - dummy, 0xff, dummy);
-
-	/* split the op code and address bytes into two transfers if needed. */
-	data_idx = 1;
-	if (addr_nbits != inst_nbits) {
-		t[0].len = 1;
-
-		t[1].tx_buf = &flash->command[1];
-		t[1].tx_nbits = addr_nbits;
-		t[1].len = cmd_sz - 1;
-		spi_message_add_tail(&t[1], &m);
+		ret = spi_mem_exec_op(flash->spimem, &op);
+		if (ret)
+			return ret;
 
-		data_idx = 2;
+		op.addr.val += op.data.nbytes;
+		remaining -= op.data.nbytes;
+		op.data.buf.in += op.data.nbytes;
 	}
 
-	t[data_idx].rx_buf = buf;
-	t[data_idx].rx_nbits = data_nbits;
-	t[data_idx].len = min3(len, spi_max_transfer_size(spi),
-			       spi_max_message_size(spi) - cmd_sz);
-	spi_message_add_tail(&t[data_idx], &m);
-
-	ret = spi_sync(spi, &m);
-	if (ret)
-		return ret;
-
-	ret = m.actual_length - cmd_sz;
-	if (ret < 0)
-		return -EIO;
-	return ret;
+	return len;
 }
 
 /*
@@ -231,8 +150,9 @@ static ssize_t m25p80_read(struct spi_nor *nor, loff_t from, size_t len,
  * matches what the READ command supports, at least until this driver
  * understands FAST_READ (for clocks over 25 MHz).
  */
-static int m25p_probe(struct spi_device *spi)
+static int m25p_probe(struct spi_mem *spimem)
 {
+	struct spi_device *spi = spimem->spi;
 	struct flash_platform_data	*data;
 	struct m25p *flash;
 	struct spi_nor *nor;
@@ -244,9 +164,9 @@ static int m25p_probe(struct spi_device *spi)
 	char *flash_name;
 	int ret;
 
-	data = dev_get_platdata(&spi->dev);
+	data = dev_get_platdata(&spimem->spi->dev);
 
-	flash = devm_kzalloc(&spi->dev, sizeof(*flash), GFP_KERNEL);
+	flash = devm_kzalloc(&spimem->spi->dev, sizeof(*flash), GFP_KERNEL);
 	if (!flash)
 		return -ENOMEM;
 
@@ -258,12 +178,12 @@ static int m25p_probe(struct spi_device *spi)
 	nor->write_reg = m25p80_write_reg;
 	nor->read_reg = m25p80_read_reg;
 
-	nor->dev = &spi->dev;
+	nor->dev = &spimem->spi->dev;
 	spi_nor_set_flash_node(nor, spi->dev.of_node);
 	nor->priv = flash;
 
 	spi_set_drvdata(spi, flash);
-	flash->spi = spi;
+	flash->spimem = spimem;
 
 	if (spi->mode & SPI_RX_QUAD) {
 		hwcaps.mask |= SNOR_HWCAPS_READ_1_1_4;
@@ -303,9 +223,9 @@ static int m25p_probe(struct spi_device *spi)
 }
 
 
-static int m25p_remove(struct spi_device *spi)
+static int m25p_remove(struct spi_mem *spimem)
 {
-	struct m25p	*flash = spi_get_drvdata(spi);
+	struct m25p	*flash = spi_mem_get_drvdata(spimem);
 
 	spi_nor_restore(&flash->spi_nor);
 
@@ -313,9 +233,9 @@ static int m25p_remove(struct spi_device *spi)
 	return mtd_device_unregister(&flash->spi_nor.mtd);
 }
 
-static void m25p_shutdown(struct spi_device *spi)
+static void m25p_shutdown(struct spi_mem *spimem)
 {
-	struct m25p *flash = spi_get_drvdata(spi);
+	struct m25p *flash = spi_mem_get_drvdata(spimem);
 
 	spi_nor_restore(&flash->spi_nor);
 }
@@ -386,12 +306,14 @@ static const struct of_device_id m25p_of_table[] = {
 };
 MODULE_DEVICE_TABLE(of, m25p_of_table);
 
-static struct spi_driver m25p80_driver = {
-	.driver = {
-		.name	= "m25p80",
-		.of_match_table = m25p_of_table,
+static struct spi_mem_driver m25p80_driver = {
+	.spidrv = {
+		.driver = {
+			.name	= "m25p80",
+			.of_match_table = m25p_of_table,
+		},
+		.id_table	= m25p_ids,
 	},
-	.id_table	= m25p_ids,
 	.probe	= m25p_probe,
 	.remove	= m25p_remove,
 	.shutdown	= m25p_shutdown,
@@ -402,7 +324,7 @@ static struct spi_driver m25p80_driver = {
 	 */
 };
 
-module_spi_driver(m25p80_driver);
+module_spi_mem_driver(m25p80_driver);
 
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Mike Lavender");
-- 
2.14.1

Re: [PATCH v4 6/7] mtd: spi-nor: Use the spi_mem_xx() API

[Frieder Schrempf]

On 26.04.2018 18:18, Boris Brezillon wrote:
> The spi_mem_xxx() API has been introduced to replace the
> spi_flash_read() one. Make use of it so we can get rid of
> spi_flash_read().
> 
> Note that using spi_mem_xx() also simplifies the code because this API
> takes care of using the regular spi_sync() interface when the optimized
> ->mem_ops interface is not implemented by the controller.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>

Reviewed-by: Frieder Schrempf <frieder.schrempf@exceet.de>
Tested-by: Frieder Schrempf <frieder.schrempf@exceet.de>

Regards,

Frieder

> ---
> Changes in v4:
> - none
> 
> Changes in v3:
> - none
> 
> Changes in v2:
> - include spi-mem.h
> - treat op->addr.val differently since it's now an u64
> - Replace SPI_MEM_OP_DATA_OUT() by SPI_MEM_OP_DATA_IN() in m25p80_read()
> ---
>   drivers/mtd/devices/Kconfig  |   1 +
>   drivers/mtd/devices/m25p80.c | 236 +++++++++++++++----------------------------
>   2 files changed, 80 insertions(+), 157 deletions(-)
> 
> diff --git a/drivers/mtd/devices/Kconfig b/drivers/mtd/devices/Kconfig
> index 6def5445e03e..57b02c4b3f63 100644
> --- a/drivers/mtd/devices/Kconfig
> +++ b/drivers/mtd/devices/Kconfig
> @@ -81,6 +81,7 @@ config MTD_DATAFLASH_OTP
>   config MTD_M25P80
>   	tristate "Support most SPI Flash chips (AT26DF, M25P, W25X, ...)"
>   	depends on SPI_MASTER && MTD_SPI_NOR
> +	select SPI_MEM
>   	help
>   	  This enables access to most modern SPI flash chips, used for
>   	  program and data storage.   Series supported include Atmel AT26DF,
> diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
> index a4e18f6aaa33..3dc022d3b53e 100644
> --- a/drivers/mtd/devices/m25p80.c
> +++ b/drivers/mtd/devices/m25p80.c
> @@ -24,12 +24,13 @@
>   #include <linux/mtd/partitions.h>
>   
>   #include <linux/spi/spi.h>
> +#include <linux/spi/spi-mem.h>
>   #include <linux/spi/flash.h>
>   #include <linux/mtd/spi-nor.h>
>   
>   #define	MAX_CMD_SIZE		6
>   struct m25p {
> -	struct spi_device	*spi;
> +	struct spi_mem		*spimem;
>   	struct spi_nor		spi_nor;
>   	u8			command[MAX_CMD_SIZE];
>   };
> @@ -37,97 +38,68 @@ struct m25p {
>   static int m25p80_read_reg(struct spi_nor *nor, u8 code, u8 *val, int len)
>   {
>   	struct m25p *flash = nor->priv;
> -	struct spi_device *spi = flash->spi;
> +	struct spi_mem_op op = SPI_MEM_OP(SPI_MEM_OP_CMD(code, 1),
> +					  SPI_MEM_OP_NO_ADDR,
> +					  SPI_MEM_OP_NO_DUMMY,
> +					  SPI_MEM_OP_DATA_IN(len, val, 1));
>   	int ret;
>   
> -	ret = spi_write_then_read(spi, &code, 1, val, len);
> +	ret = spi_mem_exec_op(flash->spimem, &op);
>   	if (ret < 0)
> -		dev_err(&spi->dev, "error %d reading %x\n", ret, code);
> +		dev_err(&flash->spimem->spi->dev, "error %d reading %x\n", ret,
> +			code);
>   
>   	return ret;
>   }
>   
> -static void m25p_addr2cmd(struct spi_nor *nor, unsigned int addr, u8 *cmd)
> -{
> -	/* opcode is in cmd[0] */
> -	cmd[1] = addr >> (nor->addr_width * 8 -  8);
> -	cmd[2] = addr >> (nor->addr_width * 8 - 16);
> -	cmd[3] = addr >> (nor->addr_width * 8 - 24);
> -	cmd[4] = addr >> (nor->addr_width * 8 - 32);
> -}
> -
> -static int m25p_cmdsz(struct spi_nor *nor)
> -{
> -	return 1 + nor->addr_width;
> -}
> -
>   static int m25p80_write_reg(struct spi_nor *nor, u8 opcode, u8 *buf, int len)
>   {
>   	struct m25p *flash = nor->priv;
> -	struct spi_device *spi = flash->spi;
> -
> -	flash->command[0] = opcode;
> -	if (buf)
> -		memcpy(&flash->command[1], buf, len);
> +	struct spi_mem_op op = SPI_MEM_OP(SPI_MEM_OP_CMD(opcode, 1),
> +					  SPI_MEM_OP_NO_ADDR,
> +					  SPI_MEM_OP_NO_DUMMY,
> +					  SPI_MEM_OP_DATA_OUT(len, buf, 1));
>   
> -	return spi_write(spi, flash->command, len + 1);
> +	return spi_mem_exec_op(flash->spimem, &op);
>   }
>   
>   static ssize_t m25p80_write(struct spi_nor *nor, loff_t to, size_t len,
>   			    const u_char *buf)
>   {
>   	struct m25p *flash = nor->priv;
> -	struct spi_device *spi = flash->spi;
> -	unsigned int inst_nbits, addr_nbits, data_nbits, data_idx;
> -	struct spi_transfer t[3] = {};
> -	struct spi_message m;
> -	int cmd_sz = m25p_cmdsz(nor);
> -	ssize_t ret;
> +	struct spi_mem_op op =
> +			SPI_MEM_OP(SPI_MEM_OP_CMD(nor->program_opcode, 1),
> +				   SPI_MEM_OP_ADDR(nor->addr_width, to, 1),
> +				   SPI_MEM_OP_DUMMY(0, 1),
> +				   SPI_MEM_OP_DATA_OUT(len, buf, 1));
> +	size_t remaining = len;
> +	int ret;
>   
>   	/* get transfer protocols. */
> -	inst_nbits = spi_nor_get_protocol_inst_nbits(nor->write_proto);
> -	addr_nbits = spi_nor_get_protocol_addr_nbits(nor->write_proto);
> -	data_nbits = spi_nor_get_protocol_data_nbits(nor->write_proto);
> -
> -	spi_message_init(&m);
> +	op.cmd.buswidth = spi_nor_get_protocol_inst_nbits(nor->write_proto);
> +	op.addr.buswidth = spi_nor_get_protocol_addr_nbits(nor->write_proto);
> +	op.dummy.buswidth = op.addr.buswidth;
> +	op.data.buswidth = spi_nor_get_protocol_data_nbits(nor->write_proto);
>   
>   	if (nor->program_opcode == SPINOR_OP_AAI_WP && nor->sst_write_second)
> -		cmd_sz = 1;
> -
> -	flash->command[0] = nor->program_opcode;
> -	m25p_addr2cmd(nor, to, flash->command);
> +		op.addr.nbytes = 0;
>   
> -	t[0].tx_buf = flash->command;
> -	t[0].tx_nbits = inst_nbits;
> -	t[0].len = cmd_sz;
> -	spi_message_add_tail(&t[0], &m);
> -
> -	/* split the op code and address bytes into two transfers if needed. */
> -	data_idx = 1;
> -	if (addr_nbits != inst_nbits) {
> -		t[0].len = 1;
> +	while (remaining) {
> +		op.data.nbytes = remaining < UINT_MAX ? remaining : UINT_MAX;
> +		ret = spi_mem_adjust_op_size(flash->spimem, &op);
> +		if (ret)
> +			return ret;
>   
> -		t[1].tx_buf = &flash->command[1];
> -		t[1].tx_nbits = addr_nbits;
> -		t[1].len = cmd_sz - 1;
> -		spi_message_add_tail(&t[1], &m);
> +		ret = spi_mem_exec_op(flash->spimem, &op);
> +		if (ret)
> +			return ret;
>   
> -		data_idx = 2;
> +		op.addr.val += op.data.nbytes;
> +		remaining -= op.data.nbytes;
> +		op.data.buf.out += op.data.nbytes;
>   	}
>   
> -	t[data_idx].tx_buf = buf;
> -	t[data_idx].tx_nbits = data_nbits;
> -	t[data_idx].len = len;
> -	spi_message_add_tail(&t[data_idx], &m);
> -
> -	ret = spi_sync(spi, &m);
> -	if (ret)
> -		return ret;
> -
> -	ret = m.actual_length - cmd_sz;
> -	if (ret < 0)
> -		return -EIO;
> -	return ret;
> +	return len;
>   }
>   
>   /*
> @@ -138,92 +110,39 @@ static ssize_t m25p80_read(struct spi_nor *nor, loff_t from, size_t len,
>   			   u_char *buf)
>   {
>   	struct m25p *flash = nor->priv;
> -	struct spi_device *spi = flash->spi;
> -	unsigned int inst_nbits, addr_nbits, data_nbits, data_idx;
> -	struct spi_transfer t[3];
> -	struct spi_message m;
> -	unsigned int dummy = nor->read_dummy;
> -	ssize_t ret;
> -	int cmd_sz;
> +	struct spi_mem_op op =
> +			SPI_MEM_OP(SPI_MEM_OP_CMD(nor->read_opcode, 1),
> +				   SPI_MEM_OP_ADDR(nor->addr_width, from, 1),
> +				   SPI_MEM_OP_DUMMY(nor->read_dummy, 1),
> +				   SPI_MEM_OP_DATA_IN(len, buf, 1));
> +	size_t remaining = len;
> +	int ret;
>   
>   	/* get transfer protocols. */
> -	inst_nbits = spi_nor_get_protocol_inst_nbits(nor->read_proto);
> -	addr_nbits = spi_nor_get_protocol_addr_nbits(nor->read_proto);
> -	data_nbits = spi_nor_get_protocol_data_nbits(nor->read_proto);
> +	op.cmd.buswidth = spi_nor_get_protocol_inst_nbits(nor->read_proto);
> +	op.addr.buswidth = spi_nor_get_protocol_addr_nbits(nor->read_proto);
> +	op.dummy.buswidth = op.addr.buswidth;
> +	op.data.buswidth = spi_nor_get_protocol_data_nbits(nor->read_proto);
>   
>   	/* convert the dummy cycles to the number of bytes */
> -	dummy = (dummy * addr_nbits) / 8;
> -
> -	if (spi_flash_read_supported(spi)) {
> -		struct spi_flash_read_message msg;
> -
> -		memset(&msg, 0, sizeof(msg));
> +	op.dummy.nbytes = (nor->read_dummy * op.dummy.buswidth) / 8;
>   
> -		msg.buf = buf;
> -		msg.from = from;
> -		msg.len = len;
> -		msg.read_opcode = nor->read_opcode;
> -		msg.addr_width = nor->addr_width;
> -		msg.dummy_bytes = dummy;
> -		msg.opcode_nbits = inst_nbits;
> -		msg.addr_nbits = addr_nbits;
> -		msg.data_nbits = data_nbits;
> -
> -		ret = spi_flash_read(spi, &msg);
> -		if (ret < 0)
> +	while (remaining) {
> +		op.data.nbytes = remaining < UINT_MAX ? remaining : UINT_MAX;
> +		ret = spi_mem_adjust_op_size(flash->spimem, &op);
> +		if (ret)
>   			return ret;
> -		return msg.retlen;
> -	}
>   
> -	spi_message_init(&m);
> -	memset(t, 0, (sizeof t));
> -
> -	flash->command[0] = nor->read_opcode;
> -	m25p_addr2cmd(nor, from, flash->command);
> -
> -	t[0].tx_buf = flash->command;
> -	t[0].tx_nbits = inst_nbits;
> -	t[0].len = m25p_cmdsz(nor) + dummy;
> -	spi_message_add_tail(&t[0], &m);
> -
> -	/*
> -	 * Set all dummy/mode cycle bits to avoid sending some manufacturer
> -	 * specific pattern, which might make the memory enter its Continuous
> -	 * Read mode by mistake.
> -	 * Based on the different mode cycle bit patterns listed and described
> -	 * in the JESD216B specification, the 0xff value works for all memories
> -	 * and all manufacturers.
> -	 */
> -	cmd_sz = t[0].len;
> -	memset(flash->command + cmd_sz - dummy, 0xff, dummy);
> -
> -	/* split the op code and address bytes into two transfers if needed. */
> -	data_idx = 1;
> -	if (addr_nbits != inst_nbits) {
> -		t[0].len = 1;
> -
> -		t[1].tx_buf = &flash->command[1];
> -		t[1].tx_nbits = addr_nbits;
> -		t[1].len = cmd_sz - 1;
> -		spi_message_add_tail(&t[1], &m);
> +		ret = spi_mem_exec_op(flash->spimem, &op);
> +		if (ret)
> +			return ret;
>   
> -		data_idx = 2;
> +		op.addr.val += op.data.nbytes;
> +		remaining -= op.data.nbytes;
> +		op.data.buf.in += op.data.nbytes;
>   	}
>   
> -	t[data_idx].rx_buf = buf;
> -	t[data_idx].rx_nbits = data_nbits;
> -	t[data_idx].len = min3(len, spi_max_transfer_size(spi),
> -			       spi_max_message_size(spi) - cmd_sz);
> -	spi_message_add_tail(&t[data_idx], &m);
> -
> -	ret = spi_sync(spi, &m);
> -	if (ret)
> -		return ret;
> -
> -	ret = m.actual_length - cmd_sz;
> -	if (ret < 0)
> -		return -EIO;
> -	return ret;
> +	return len;
>   }
>   
>   /*
> @@ -231,8 +150,9 @@ static ssize_t m25p80_read(struct spi_nor *nor, loff_t from, size_t len,
>    * matches what the READ command supports, at least until this driver
>    * understands FAST_READ (for clocks over 25 MHz).
>    */
> -static int m25p_probe(struct spi_device *spi)
> +static int m25p_probe(struct spi_mem *spimem)
>   {
> +	struct spi_device *spi = spimem->spi;
>   	struct flash_platform_data	*data;
>   	struct m25p *flash;
>   	struct spi_nor *nor;
> @@ -244,9 +164,9 @@ static int m25p_probe(struct spi_device *spi)
>   	char *flash_name;
>   	int ret;
>   
> -	data = dev_get_platdata(&spi->dev);
> +	data = dev_get_platdata(&spimem->spi->dev);
>   
> -	flash = devm_kzalloc(&spi->dev, sizeof(*flash), GFP_KERNEL);
> +	flash = devm_kzalloc(&spimem->spi->dev, sizeof(*flash), GFP_KERNEL);
>   	if (!flash)
>   		return -ENOMEM;
>   
> @@ -258,12 +178,12 @@ static int m25p_probe(struct spi_device *spi)
>   	nor->write_reg = m25p80_write_reg;
>   	nor->read_reg = m25p80_read_reg;
>   
> -	nor->dev = &spi->dev;
> +	nor->dev = &spimem->spi->dev;
>   	spi_nor_set_flash_node(nor, spi->dev.of_node);
>   	nor->priv = flash;
>   
>   	spi_set_drvdata(spi, flash);
> -	flash->spi = spi;
> +	flash->spimem = spimem;
>   
>   	if (spi->mode & SPI_RX_QUAD) {
>   		hwcaps.mask |= SNOR_HWCAPS_READ_1_1_4;
> @@ -303,9 +223,9 @@ static int m25p_probe(struct spi_device *spi)
>   }
>   
>   
> -static int m25p_remove(struct spi_device *spi)
> +static int m25p_remove(struct spi_mem *spimem)
>   {
> -	struct m25p	*flash = spi_get_drvdata(spi);
> +	struct m25p	*flash = spi_mem_get_drvdata(spimem);
>   
>   	spi_nor_restore(&flash->spi_nor);
>   
> @@ -313,9 +233,9 @@ static int m25p_remove(struct spi_device *spi)
>   	return mtd_device_unregister(&flash->spi_nor.mtd);
>   }
>   
> -static void m25p_shutdown(struct spi_device *spi)
> +static void m25p_shutdown(struct spi_mem *spimem)
>   {
> -	struct m25p *flash = spi_get_drvdata(spi);
> +	struct m25p *flash = spi_mem_get_drvdata(spimem);
>   
>   	spi_nor_restore(&flash->spi_nor);
>   }
> @@ -386,12 +306,14 @@ static const struct of_device_id m25p_of_table[] = {
>   };
>   MODULE_DEVICE_TABLE(of, m25p_of_table);
>   
> -static struct spi_driver m25p80_driver = {
> -	.driver = {
> -		.name	= "m25p80",
> -		.of_match_table = m25p_of_table,
> +static struct spi_mem_driver m25p80_driver = {
> +	.spidrv = {
> +		.driver = {
> +			.name	= "m25p80",
> +			.of_match_table = m25p_of_table,
> +		},
> +		.id_table	= m25p_ids,
>   	},
> -	.id_table	= m25p_ids,
>   	.probe	= m25p_probe,
>   	.remove	= m25p_remove,
>   	.shutdown	= m25p_shutdown,
> @@ -402,7 +324,7 @@ static struct spi_driver m25p80_driver = {
>   	 */
>   };
>   
> -module_spi_driver(m25p80_driver);
> +module_spi_mem_driver(m25p80_driver);
>   
>   MODULE_LICENSE("GPL");
>   MODULE_AUTHOR("Mike Lavender");
> 

Re: [PATCH v4 6/7] mtd: spi-nor: Use the spi_mem_xx() API

[Cyrille Pitchen]

Hi all,

Le 26/04/2018 à 18:18, Boris Brezillon a écrit :
> The spi_mem_xxx() API has been introduced to replace the
> spi_flash_read() one. Make use of it so we can get rid of
> spi_flash_read().
> 
> Note that using spi_mem_xx() also simplifies the code because this API
> takes care of using the regular spi_sync() interface when the optimized
> ->mem_ops interface is not implemented by the controller.Hi a
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
> ---
> Changes in v4:
> - none
> 
> Changes in v3:
> - none
> 
> Changes in v2:
> - include spi-mem.h
> - treat op->addr.val differently since it's now an u64
> - Replace SPI_MEM_OP_DATA_OUT() by SPI_MEM_OP_DATA_IN() in m25p80_read()
> ---
>  drivers/mtd/devices/Kconfig  |   1 +
>  drivers/mtd/devices/m25p80.c | 236 +++++++++++++++----------------------------
>  2 files changed, 80 insertions(+), 157 deletions(-)
> 
> diff --git a/drivers/mtd/devices/Kconfig b/drivers/mtd/devices/Kconfig
> index 6def5445e03e..57b02c4b3f63 100644
> --- a/drivers/mtd/devices/Kconfig
> +++ b/drivers/mtd/devices/Kconfig
> @@ -81,6 +81,7 @@ config MTD_DATAFLASH_OTP
>  config MTD_M25P80
>  	tristate "Support most SPI Flash chips (AT26DF, M25P, W25X, ...)"
>  	depends on SPI_MASTER && MTD_SPI_NOR
> +	select SPI_MEM
>  	help
>  	  This enables access to most modern SPI flash chips, used for
>  	  program and data storage.   Series supported include Atmel AT26DF,
> diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
> index a4e18f6aaa33..3dc022d3b53e 100644
> --- a/drivers/mtd/devices/m25p80.c
> +++ b/drivers/mtd/devices/m25p80.c
> @@ -24,12 +24,13 @@
>  #include <linux/mtd/partitions.h>
>  
>  #include <linux/spi/spi.h>
> +#include <linux/spi/spi-mem.h>
>  #include <linux/spi/flash.h>
>  #include <linux/mtd/spi-nor.h>
>  
>  #define	MAX_CMD_SIZE		6
>  struct m25p {
> -	struct spi_device	*spi;
> +	struct spi_mem		*spimem;
>  	struct spi_nor		spi_nor;
>  	u8			command[MAX_CMD_SIZE];
>  };
> @@ -37,97 +38,68 @@ struct m25p {
>  static int m25p80_read_reg(struct spi_nor *nor, u8 code, u8 *val, int len)
>  {
>  	struct m25p *flash = nor->priv;
> -	struct spi_device *spi = flash->spi;
> +	struct spi_mem_op op = SPI_MEM_OP(SPI_MEM_OP_CMD(code, 1),
> +					  SPI_MEM_OP_NO_ADDR,
> +					  SPI_MEM_OP_NO_DUMMY,
> +					  SPI_MEM_OP_DATA_IN(len, val, 1));
>  	int ret;
>  
> -	ret = spi_write_then_read(spi, &code, 1, val, len);
> +	ret = spi_mem_exec_op(flash->spimem, &op);
>  	if (ret < 0)
> -		dev_err(&spi->dev, "error %d reading %x\n", ret, code);
> +		dev_err(&flash->spimem->spi->dev, "error %d reading %x\n", ret,
> +			code);
>  
>  	return ret;
>  }
>  
> -static void m25p_addr2cmd(struct spi_nor *nor, unsigned int addr, u8 *cmd)
> -{
> -	/* opcode is in cmd[0] */
> -	cmd[1] = addr >> (nor->addr_width * 8 -  8);
> -	cmd[2] = addr >> (nor->addr_width * 8 - 16);
> -	cmd[3] = addr >> (nor->addr_width * 8 - 24);
> -	cmd[4] = addr >> (nor->addr_width * 8 - 32);
> -}
> -
> -static int m25p_cmdsz(struct spi_nor *nor)
> -{
> -	return 1 + nor->addr_width;
> -}
> -
>  static int m25p80_write_reg(struct spi_nor *nor, u8 opcode, u8 *buf, int len)
>  {
>  	struct m25p *flash = nor->priv;
> -	struct spi_device *spi = flash->spi;
> -
> -	flash->command[0] = opcode;
> -	if (buf)
> -		memcpy(&flash->command[1], buf, len);
> +	struct spi_mem_op op = SPI_MEM_OP(SPI_MEM_OP_CMD(opcode, 1),
> +					  SPI_MEM_OP_NO_ADDR,
> +					  SPI_MEM_OP_NO_DUMMY,

I didn't test nor compiled to verify but I have some doubt:
here the 'op' variable is allocated on the stack and both definitions
of SPI_MEM_OP_NO_ADDR and SPI_MEM_OP_NO_DUMMY macros are { }.

It might be some rule of the C language that I ignore but are we guaranteed
that the .nbytes member of both structures is actually initialized to 0 ?

I would say no: I think since the structure is allocated on the stack all
members not being explicitly initialized are left uninitialized by the
compiler when it generates the assembler code. However, I'm not 100% sure of
that.

Anyway, what about explicitly set the .nbytes member to 0 in the definition
of SPI_MEM_OP_NO* helper macros ?

> +					  SPI_MEM_OP_DATA_OUT(len, buf, 1));
>  
> -	return spi_write(spi, flash->command, len + 1);
> +	return spi_mem_exec_op(flash->spimem, &op);
>  }
>  
>  static ssize_t m25p80_write(struct spi_nor *nor, loff_t to, size_t len,
>  			    const u_char *buf)
>  {
>  	struct m25p *flash = nor->priv;
> -	struct spi_device *spi = flash->spi;
> -	unsigned int inst_nbits, addr_nbits, data_nbits, data_idx;
> -	struct spi_transfer t[3] = {};
> -	struct spi_message m;
> -	int cmd_sz = m25p_cmdsz(nor);
> -	ssize_t ret;
> +	struct spi_mem_op op =
> +			SPI_MEM_OP(SPI_MEM_OP_CMD(nor->program_opcode, 1),
> +				   SPI_MEM_OP_ADDR(nor->addr_width, to, 1),
> +				   SPI_MEM_OP_DUMMY(0, 1),

Why do you use SPI_MEM_OP_DUMMY(0, 1) here instead of SPI_MEM_OP_NO_DUMMY() ?

Best regards,

Cyrille

> +				   SPI_MEM_OP_DATA_OUT(len, buf, 1));
> +	size_t remaining = len;
> +	int ret;
>  
>  	/* get transfer protocols. */
> -	inst_nbits = spi_nor_get_protocol_inst_nbits(nor->write_proto);
> -	addr_nbits = spi_nor_get_protocol_addr_nbits(nor->write_proto);
> -	data_nbits = spi_nor_get_protocol_data_nbits(nor->write_proto);
> -
> -	spi_message_init(&m);
> +	op.cmd.buswidth = spi_nor_get_protocol_inst_nbits(nor->write_proto);
> +	op.addr.buswidth = spi_nor_get_protocol_addr_nbits(nor->write_proto);
> +	op.dummy.buswidth = op.addr.buswidth;
> +	op.data.buswidth = spi_nor_get_protocol_data_nbits(nor->write_proto);
>  
>  	if (nor->program_opcode == SPINOR_OP_AAI_WP && nor->sst_write_second)
> -		cmd_sz = 1;
> -
> -	flash->command[0] = nor->program_opcode;
> -	m25p_addr2cmd(nor, to, flash->command);
> +		op.addr.nbytes = 0;
>  
> -	t[0].tx_buf = flash->command;
> -	t[0].tx_nbits = inst_nbits;
> -	t[0].len = cmd_sz;
> -	spi_message_add_tail(&t[0], &m);
> -
> -	/* split the op code and address bytes into two transfers if needed. */
> -	data_idx = 1;
> -	if (addr_nbits != inst_nbits) {
> -		t[0].len = 1;
> +	while (remaining) {
> +		op.data.nbytes = remaining < UINT_MAX ? remaining : UINT_MAX;
> +		ret = spi_mem_adjust_op_size(flash->spimem, &op);
> +		if (ret)
> +			return ret;
>  
> -		t[1].tx_buf = &flash->command[1];
> -		t[1].tx_nbits = addr_nbits;
> -		t[1].len = cmd_sz - 1;
> -		spi_message_add_tail(&t[1], &m);
> +		ret = spi_mem_exec_op(flash->spimem, &op);
> +		if (ret)
> +			return ret;
>  
> -		data_idx = 2;
> +		op.addr.val += op.data.nbytes;
> +		remaining -= op.data.nbytes;
> +		op.data.buf.out += op.data.nbytes;
>  	}
>  
> -	t[data_idx].tx_buf = buf;
> -	t[data_idx].tx_nbits = data_nbits;
> -	t[data_idx].len = len;
> -	spi_message_add_tail(&t[data_idx], &m);
> -
> -	ret = spi_sync(spi, &m);
> -	if (ret)
> -		return ret;
> -
> -	ret = m.actual_length - cmd_sz;
> -	if (ret < 0)
> -		return -EIO;
> -	return ret;
> +	return len;
>  }
>  
>  /*
> @@ -138,92 +110,39 @@ static ssize_t m25p80_read(struct spi_nor *nor, loff_t from, size_t len,
>  			   u_char *buf)
>  {
>  	struct m25p *flash = nor->priv;
> -	struct spi_device *spi = flash->spi;
> -	unsigned int inst_nbits, addr_nbits, data_nbits, data_idx;
> -	struct spi_transfer t[3];
> -	struct spi_message m;
> -	unsigned int dummy = nor->read_dummy;
> -	ssize_t ret;
> -	int cmd_sz;
> +	struct spi_mem_op op =
> +			SPI_MEM_OP(SPI_MEM_OP_CMD(nor->read_opcode, 1),
> +				   SPI_MEM_OP_ADDR(nor->addr_width, from, 1),
> +				   SPI_MEM_OP_DUMMY(nor->read_dummy, 1),
> +				   SPI_MEM_OP_DATA_IN(len, buf, 1));
> +	size_t remaining = len;
> +	int ret;
>  
>  	/* get transfer protocols. */
> -	inst_nbits = spi_nor_get_protocol_inst_nbits(nor->read_proto);
> -	addr_nbits = spi_nor_get_protocol_addr_nbits(nor->read_proto);
> -	data_nbits = spi_nor_get_protocol_data_nbits(nor->read_proto);
> +	op.cmd.buswidth = spi_nor_get_protocol_inst_nbits(nor->read_proto);
> +	op.addr.buswidth = spi_nor_get_protocol_addr_nbits(nor->read_proto);
> +	op.dummy.buswidth = op.addr.buswidth;
> +	op.data.buswidth = spi_nor_get_protocol_data_nbits(nor->read_proto);
>  
>  	/* convert the dummy cycles to the number of bytes */
> -	dummy = (dummy * addr_nbits) / 8;
> -
> -	if (spi_flash_read_supported(spi)) {
> -		struct spi_flash_read_message msg;
> -
> -		memset(&msg, 0, sizeof(msg));
> +	op.dummy.nbytes = (nor->read_dummy * op.dummy.buswidth) / 8;
>  
> -		msg.buf = buf;
> -		msg.from = from;
> -		msg.len = len;
> -		msg.read_opcode = nor->read_opcode;
> -		msg.addr_width = nor->addr_width;
> -		msg.dummy_bytes = dummy;
> -		msg.opcode_nbits = inst_nbits;
> -		msg.addr_nbits = addr_nbits;
> -		msg.data_nbits = data_nbits;
> -
> -		ret = spi_flash_read(spi, &msg);
> -		if (ret < 0)
> +	while (remaining) {
> +		op.data.nbytes = remaining < UINT_MAX ? remaining : UINT_MAX;
> +		ret = spi_mem_adjust_op_size(flash->spimem, &op);
> +		if (ret)
>  			return ret;
> -		return msg.retlen;
> -	}
>  
> -	spi_message_init(&m);
> -	memset(t, 0, (sizeof t));
> -
> -	flash->command[0] = nor->read_opcode;
> -	m25p_addr2cmd(nor, from, flash->command);
> -
> -	t[0].tx_buf = flash->command;
> -	t[0].tx_nbits = inst_nbits;
> -	t[0].len = m25p_cmdsz(nor) + dummy;
> -	spi_message_add_tail(&t[0], &m);
> -
> -	/*
> -	 * Set all dummy/mode cycle bits to avoid sending some manufacturer
> -	 * specific pattern, which might make the memory enter its Continuous
> -	 * Read mode by mistake.
> -	 * Based on the different mode cycle bit patterns listed and described
> -	 * in the JESD216B specification, the 0xff value works for all memories
> -	 * and all manufacturers.
> -	 */
> -	cmd_sz = t[0].len;
> -	memset(flash->command + cmd_sz - dummy, 0xff, dummy);
> -
> -	/* split the op code and address bytes into two transfers if needed. */
> -	data_idx = 1;
> -	if (addr_nbits != inst_nbits) {
> -		t[0].len = 1;
> -
> -		t[1].tx_buf = &flash->command[1];
> -		t[1].tx_nbits = addr_nbits;
> -		t[1].len = cmd_sz - 1;
> -		spi_message_add_tail(&t[1], &m);
> +		ret = spi_mem_exec_op(flash->spimem, &op);
> +		if (ret)
> +			return ret;
>  
> -		data_idx = 2;
> +		op.addr.val += op.data.nbytes;
> +		remaining -= op.data.nbytes;
> +		op.data.buf.in += op.data.nbytes;
>  	}
>  
> -	t[data_idx].rx_buf = buf;
> -	t[data_idx].rx_nbits = data_nbits;
> -	t[data_idx].len = min3(len, spi_max_transfer_size(spi),
> -			       spi_max_message_size(spi) - cmd_sz);
> -	spi_message_add_tail(&t[data_idx], &m);
> -
> -	ret = spi_sync(spi, &m);
> -	if (ret)
> -		return ret;
> -
> -	ret = m.actual_length - cmd_sz;
> -	if (ret < 0)
> -		return -EIO;
> -	return ret;
> +	return len;
>  }
>  
>  /*
> @@ -231,8 +150,9 @@ static ssize_t m25p80_read(struct spi_nor *nor, loff_t from, size_t len,
>   * matches what the READ command supports, at least until this driver
>   * understands FAST_READ (for clocks over 25 MHz).
>   */
> -static int m25p_probe(struct spi_device *spi)
> +static int m25p_probe(struct spi_mem *spimem)
>  {
> +	struct spi_device *spi = spimem->spi;
>  	struct flash_platform_data	*data;
>  	struct m25p *flash;
>  	struct spi_nor *nor;
> @@ -244,9 +164,9 @@ static int m25p_probe(struct spi_device *spi)
>  	char *flash_name;
>  	int ret;
>  
> -	data = dev_get_platdata(&spi->dev);
> +	data = dev_get_platdata(&spimem->spi->dev);
>  
> -	flash = devm_kzalloc(&spi->dev, sizeof(*flash), GFP_KERNEL);
> +	flash = devm_kzalloc(&spimem->spi->dev, sizeof(*flash), GFP_KERNEL);
>  	if (!flash)
>  		return -ENOMEM;
>  
> @@ -258,12 +178,12 @@ static int m25p_probe(struct spi_device *spi)
>  	nor->write_reg = m25p80_write_reg;
>  	nor->read_reg = m25p80_read_reg;
>  
> -	nor->dev = &spi->dev;
> +	nor->dev = &spimem->spi->dev;
>  	spi_nor_set_flash_node(nor, spi->dev.of_node);
>  	nor->priv = flash;
>  
>  	spi_set_drvdata(spi, flash);
> -	flash->spi = spi;
> +	flash->spimem = spimem;
>  
>  	if (spi->mode & SPI_RX_QUAD) {
>  		hwcaps.mask |= SNOR_HWCAPS_READ_1_1_4;
> @@ -303,9 +223,9 @@ static int m25p_probe(struct spi_device *spi)
>  }
>  
>  
> -static int m25p_remove(struct spi_device *spi)
> +static int m25p_remove(struct spi_mem *spimem)
>  {
> -	struct m25p	*flash = spi_get_drvdata(spi);
> +	struct m25p	*flash = spi_mem_get_drvdata(spimem);
>  
>  	spi_nor_restore(&flash->spi_nor);
>  
> @@ -313,9 +233,9 @@ static int m25p_remove(struct spi_device *spi)
>  	return mtd_device_unregister(&flash->spi_nor.mtd);
>  }
>  
> -static void m25p_shutdown(struct spi_device *spi)
> +static void m25p_shutdown(struct spi_mem *spimem)
>  {
> -	struct m25p *flash = spi_get_drvdata(spi);
> +	struct m25p *flash = spi_mem_get_drvdata(spimem);
>  
>  	spi_nor_restore(&flash->spi_nor);
>  }
> @@ -386,12 +306,14 @@ static const struct of_device_id m25p_of_table[] = {
>  };
>  MODULE_DEVICE_TABLE(of, m25p_of_table);
>  
> -static struct spi_driver m25p80_driver = {
> -	.driver = {
> -		.name	= "m25p80",
> -		.of_match_table = m25p_of_table,
> +static struct spi_mem_driver m25p80_driver = {
> +	.spidrv = {
> +		.driver = {
> +			.name	= "m25p80",
> +			.of_match_table = m25p_of_table,
> +		},
> +		.id_table	= m25p_ids,
>  	},
> -	.id_table	= m25p_ids,
>  	.probe	= m25p_probe,
>  	.remove	= m25p_remove,
>  	.shutdown	= m25p_shutdown,
> @@ -402,7 +324,7 @@ static struct spi_driver m25p80_driver = {
>  	 */
>  };
>  
> -module_spi_driver(m25p80_driver);
> +module_spi_mem_driver(m25p80_driver);
>  
>  MODULE_LICENSE("GPL");
>  MODULE_AUTHOR("Mike Lavender");
> 

Re: [PATCH v4 6/7] mtd: spi-nor: Use the spi_mem_xx() API

[Cyrille Pitchen]

Le 23/05/2018 à 17:44, Cyrille Pitchen a écrit :
> Hi all,
> 
> Le 26/04/2018 à 18:18, Boris Brezillon a écrit :
>> The spi_mem_xxx() API has been introduced to replace the
>> spi_flash_read() one. Make use of it so we can get rid of
>> spi_flash_read().
>>
>> Note that using spi_mem_xx() also simplifies the code because this API
>> takes care of using the regular spi_sync() interface when the optimized
>> ->mem_ops interface is not implemented by the controller.Hi a
>>
>> Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
>> ---
>> Changes in v4:
>> - none
>>
>> Changes in v3:
>> - none
>>
>> Changes in v2:
>> - include spi-mem.h
>> - treat op->addr.val differently since it's now an u64
>> - Replace SPI_MEM_OP_DATA_OUT() by SPI_MEM_OP_DATA_IN() in m25p80_read()
>> ---
>>  drivers/mtd/devices/Kconfig  |   1 +
>>  drivers/mtd/devices/m25p80.c | 236 +++++++++++++++----------------------------
>>  2 files changed, 80 insertions(+), 157 deletions(-)
>>
>> diff --git a/drivers/mtd/devices/Kconfig b/drivers/mtd/devices/Kconfig
>> index 6def5445e03e..57b02c4b3f63 100644
>> --- a/drivers/mtd/devices/Kconfig
>> +++ b/drivers/mtd/devices/Kconfig
>> @@ -81,6 +81,7 @@ config MTD_DATAFLASH_OTP
>>  config MTD_M25P80
>>  	tristate "Support most SPI Flash chips (AT26DF, M25P, W25X, ...)"
>>  	depends on SPI_MASTER && MTD_SPI_NOR
>> +	select SPI_MEM
>>  	help
>>  	  This enables access to most modern SPI flash chips, used for
>>  	  program and data storage.   Series supported include Atmel AT26DF,
>> diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
>> index a4e18f6aaa33..3dc022d3b53e 100644
>> --- a/drivers/mtd/devices/m25p80.c
>> +++ b/drivers/mtd/devices/m25p80.c
>> @@ -24,12 +24,13 @@
>>  #include <linux/mtd/partitions.h>
>>  
>>  #include <linux/spi/spi.h>
>> +#include <linux/spi/spi-mem.h>
>>  #include <linux/spi/flash.h>
>>  #include <linux/mtd/spi-nor.h>
>>  
>>  #define	MAX_CMD_SIZE		6
>>  struct m25p {
>> -	struct spi_device	*spi;
>> +	struct spi_mem		*spimem;
>>  	struct spi_nor		spi_nor;
>>  	u8			command[MAX_CMD_SIZE];
>>  };
>> @@ -37,97 +38,68 @@ struct m25p {
>>  static int m25p80_read_reg(struct spi_nor *nor, u8 code, u8 *val, int len)
>>  {
>>  	struct m25p *flash = nor->priv;
>> -	struct spi_device *spi = flash->spi;
>> +	struct spi_mem_op op = SPI_MEM_OP(SPI_MEM_OP_CMD(code, 1),
>> +					  SPI_MEM_OP_NO_ADDR,
>> +					  SPI_MEM_OP_NO_DUMMY,
>> +					  SPI_MEM_OP_DATA_IN(len, val, 1));
>>  	int ret;
>>  
>> -	ret = spi_write_then_read(spi, &code, 1, val, len);
>> +	ret = spi_mem_exec_op(flash->spimem, &op);
>>  	if (ret < 0)
>> -		dev_err(&spi->dev, "error %d reading %x\n", ret, code);
>> +		dev_err(&flash->spimem->spi->dev, "error %d reading %x\n", ret,
>> +			code);
>>  
>>  	return ret;
>>  }
>>  
>> -static void m25p_addr2cmd(struct spi_nor *nor, unsigned int addr, u8 *cmd)
>> -{
>> -	/* opcode is in cmd[0] */
>> -	cmd[1] = addr >> (nor->addr_width * 8 -  8);
>> -	cmd[2] = addr >> (nor->addr_width * 8 - 16);
>> -	cmd[3] = addr >> (nor->addr_width * 8 - 24);
>> -	cmd[4] = addr >> (nor->addr_width * 8 - 32);
>> -}
>> -
>> -static int m25p_cmdsz(struct spi_nor *nor)
>> -{
>> -	return 1 + nor->addr_width;
>> -}
>> -
>>  static int m25p80_write_reg(struct spi_nor *nor, u8 opcode, u8 *buf, int len)
>>  {
>>  	struct m25p *flash = nor->priv;
>> -	struct spi_device *spi = flash->spi;
>> -
>> -	flash->command[0] = opcode;
>> -	if (buf)
>> -		memcpy(&flash->command[1], buf, len);
>> +	struct spi_mem_op op = SPI_MEM_OP(SPI_MEM_OP_CMD(opcode, 1),
>> +					  SPI_MEM_OP_NO_ADDR,
>> +					  SPI_MEM_OP_NO_DUMMY,
> 
> I didn't test nor compiled to verify but I have some doubt:
> here the 'op' variable is allocated on the stack and both definitions
> of SPI_MEM_OP_NO_ADDR and SPI_MEM_OP_NO_DUMMY macros are { }.
> 
> It might be some rule of the C language that I ignore but are we guaranteed
> that the .nbytes member of both structures is actually initialized to 0 ?
> 
> I would say no: I think since the structure is allocated on the stack all
> members not being explicitly initialized are left uninitialized by the
> compiler when it generates the assembler code. However, I'm not 100% sure of
> that.
>

Actually, I'm reading that with C99 extensions, "omitted field members are
implicitly initialized the same as objects that have static storage duration."
https://gcc.gnu.org/onlinedocs/gcc/Designated-Inits.html

sorry for the noise!
 
> Anyway, what about explicitly set the .nbytes member to 0 in the definition
> of SPI_MEM_OP_NO* helper macros ?
> 
>> +					  SPI_MEM_OP_DATA_OUT(len, buf, 1));
>>  
>> -	return spi_write(spi, flash->command, len + 1);
>> +	return spi_mem_exec_op(flash->spimem, &op);
>>  }
>>  
>>  static ssize_t m25p80_write(struct spi_nor *nor, loff_t to, size_t len,
>>  			    const u_char *buf)
>>  {
>>  	struct m25p *flash = nor->priv;
>> -	struct spi_device *spi = flash->spi;
>> -	unsigned int inst_nbits, addr_nbits, data_nbits, data_idx;
>> -	struct spi_transfer t[3] = {};
>> -	struct spi_message m;
>> -	int cmd_sz = m25p_cmdsz(nor);
>> -	ssize_t ret;
>> +	struct spi_mem_op op =
>> +			SPI_MEM_OP(SPI_MEM_OP_CMD(nor->program_opcode, 1),
>> +				   SPI_MEM_OP_ADDR(nor->addr_width, to, 1),
>> +				   SPI_MEM_OP_DUMMY(0, 1),
> 
> Why do you use SPI_MEM_OP_DUMMY(0, 1) here instead of SPI_MEM_OP_NO_DUMMY() ?
> 
> Best regards,
> 
> Cyrille
> 
>> +				   SPI_MEM_OP_DATA_OUT(len, buf, 1));
>> +	size_t remaining = len;
>> +	int ret;
>>  
>>  	/* get transfer protocols. */
>> -	inst_nbits = spi_nor_get_protocol_inst_nbits(nor->write_proto);
>> -	addr_nbits = spi_nor_get_protocol_addr_nbits(nor->write_proto);
>> -	data_nbits = spi_nor_get_protocol_data_nbits(nor->write_proto);
>> -
>> -	spi_message_init(&m);
>> +	op.cmd.buswidth = spi_nor_get_protocol_inst_nbits(nor->write_proto);
>> +	op.addr.buswidth = spi_nor_get_protocol_addr_nbits(nor->write_proto);
>> +	op.dummy.buswidth = op.addr.buswidth;
>> +	op.data.buswidth = spi_nor_get_protocol_data_nbits(nor->write_proto);
>>  
>>  	if (nor->program_opcode == SPINOR_OP_AAI_WP && nor->sst_write_second)
>> -		cmd_sz = 1;
>> -
>> -	flash->command[0] = nor->program_opcode;
>> -	m25p_addr2cmd(nor, to, flash->command);
>> +		op.addr.nbytes = 0;
>>  
>> -	t[0].tx_buf = flash->command;
>> -	t[0].tx_nbits = inst_nbits;
>> -	t[0].len = cmd_sz;
>> -	spi_message_add_tail(&t[0], &m);
>> -
>> -	/* split the op code and address bytes into two transfers if needed. */
>> -	data_idx = 1;
>> -	if (addr_nbits != inst_nbits) {
>> -		t[0].len = 1;
>> +	while (remaining) {
>> +		op.data.nbytes = remaining < UINT_MAX ? remaining : UINT_MAX;
>> +		ret = spi_mem_adjust_op_size(flash->spimem, &op);
>> +		if (ret)
>> +			return ret;
>>  
>> -		t[1].tx_buf = &flash->command[1];
>> -		t[1].tx_nbits = addr_nbits;
>> -		t[1].len = cmd_sz - 1;
>> -		spi_message_add_tail(&t[1], &m);
>> +		ret = spi_mem_exec_op(flash->spimem, &op);
>> +		if (ret)
>> +			return ret;
>>  
>> -		data_idx = 2;
>> +		op.addr.val += op.data.nbytes;
>> +		remaining -= op.data.nbytes;
>> +		op.data.buf.out += op.data.nbytes;
>>  	}
>>  
>> -	t[data_idx].tx_buf = buf;
>> -	t[data_idx].tx_nbits = data_nbits;
>> -	t[data_idx].len = len;
>> -	spi_message_add_tail(&t[data_idx], &m);
>> -
>> -	ret = spi_sync(spi, &m);
>> -	if (ret)
>> -		return ret;
>> -
>> -	ret = m.actual_length - cmd_sz;
>> -	if (ret < 0)
>> -		return -EIO;
>> -	return ret;
>> +	return len;
>>  }
>>  
>>  /*
>> @@ -138,92 +110,39 @@ static ssize_t m25p80_read(struct spi_nor *nor, loff_t from, size_t len,
>>  			   u_char *buf)
>>  {
>>  	struct m25p *flash = nor->priv;
>> -	struct spi_device *spi = flash->spi;
>> -	unsigned int inst_nbits, addr_nbits, data_nbits, data_idx;
>> -	struct spi_transfer t[3];
>> -	struct spi_message m;
>> -	unsigned int dummy = nor->read_dummy;
>> -	ssize_t ret;
>> -	int cmd_sz;
>> +	struct spi_mem_op op =
>> +			SPI_MEM_OP(SPI_MEM_OP_CMD(nor->read_opcode, 1),
>> +				   SPI_MEM_OP_ADDR(nor->addr_width, from, 1),
>> +				   SPI_MEM_OP_DUMMY(nor->read_dummy, 1),
>> +				   SPI_MEM_OP_DATA_IN(len, buf, 1));
>> +	size_t remaining = len;
>> +	int ret;
>>  
>>  	/* get transfer protocols. */
>> -	inst_nbits = spi_nor_get_protocol_inst_nbits(nor->read_proto);
>> -	addr_nbits = spi_nor_get_protocol_addr_nbits(nor->read_proto);
>> -	data_nbits = spi_nor_get_protocol_data_nbits(nor->read_proto);
>> +	op.cmd.buswidth = spi_nor_get_protocol_inst_nbits(nor->read_proto);
>> +	op.addr.buswidth = spi_nor_get_protocol_addr_nbits(nor->read_proto);
>> +	op.dummy.buswidth = op.addr.buswidth;
>> +	op.data.buswidth = spi_nor_get_protocol_data_nbits(nor->read_proto);
>>  
>>  	/* convert the dummy cycles to the number of bytes */
>> -	dummy = (dummy * addr_nbits) / 8;
>> -
>> -	if (spi_flash_read_supported(spi)) {
>> -		struct spi_flash_read_message msg;
>> -
>> -		memset(&msg, 0, sizeof(msg));
>> +	op.dummy.nbytes = (nor->read_dummy * op.dummy.buswidth) / 8;
>>  
>> -		msg.buf = buf;
>> -		msg.from = from;
>> -		msg.len = len;
>> -		msg.read_opcode = nor->read_opcode;
>> -		msg.addr_width = nor->addr_width;
>> -		msg.dummy_bytes = dummy;
>> -		msg.opcode_nbits = inst_nbits;
>> -		msg.addr_nbits = addr_nbits;
>> -		msg.data_nbits = data_nbits;
>> -
>> -		ret = spi_flash_read(spi, &msg);
>> -		if (ret < 0)
>> +	while (remaining) {
>> +		op.data.nbytes = remaining < UINT_MAX ? remaining : UINT_MAX;
>> +		ret = spi_mem_adjust_op_size(flash->spimem, &op);
>> +		if (ret)
>>  			return ret;
>> -		return msg.retlen;
>> -	}
>>  
>> -	spi_message_init(&m);
>> -	memset(t, 0, (sizeof t));
>> -
>> -	flash->command[0] = nor->read_opcode;
>> -	m25p_addr2cmd(nor, from, flash->command);
>> -
>> -	t[0].tx_buf = flash->command;
>> -	t[0].tx_nbits = inst_nbits;
>> -	t[0].len = m25p_cmdsz(nor) + dummy;
>> -	spi_message_add_tail(&t[0], &m);
>> -
>> -	/*
>> -	 * Set all dummy/mode cycle bits to avoid sending some manufacturer
>> -	 * specific pattern, which might make the memory enter its Continuous
>> -	 * Read mode by mistake.
>> -	 * Based on the different mode cycle bit patterns listed and described
>> -	 * in the JESD216B specification, the 0xff value works for all memories
>> -	 * and all manufacturers.
>> -	 */
>> -	cmd_sz = t[0].len;
>> -	memset(flash->command + cmd_sz - dummy, 0xff, dummy);
>> -
>> -	/* split the op code and address bytes into two transfers if needed. */
>> -	data_idx = 1;
>> -	if (addr_nbits != inst_nbits) {
>> -		t[0].len = 1;
>> -
>> -		t[1].tx_buf = &flash->command[1];
>> -		t[1].tx_nbits = addr_nbits;
>> -		t[1].len = cmd_sz - 1;
>> -		spi_message_add_tail(&t[1], &m);
>> +		ret = spi_mem_exec_op(flash->spimem, &op);
>> +		if (ret)
>> +			return ret;
>>  
>> -		data_idx = 2;
>> +		op.addr.val += op.data.nbytes;
>> +		remaining -= op.data.nbytes;
>> +		op.data.buf.in += op.data.nbytes;
>>  	}
>>  
>> -	t[data_idx].rx_buf = buf;
>> -	t[data_idx].rx_nbits = data_nbits;
>> -	t[data_idx].len = min3(len, spi_max_transfer_size(spi),
>> -			       spi_max_message_size(spi) - cmd_sz);
>> -	spi_message_add_tail(&t[data_idx], &m);
>> -
>> -	ret = spi_sync(spi, &m);
>> -	if (ret)
>> -		return ret;
>> -
>> -	ret = m.actual_length - cmd_sz;
>> -	if (ret < 0)
>> -		return -EIO;
>> -	return ret;
>> +	return len;
>>  }
>>  
>>  /*
>> @@ -231,8 +150,9 @@ static ssize_t m25p80_read(struct spi_nor *nor, loff_t from, size_t len,
>>   * matches what the READ command supports, at least until this driver
>>   * understands FAST_READ (for clocks over 25 MHz).
>>   */
>> -static int m25p_probe(struct spi_device *spi)
>> +static int m25p_probe(struct spi_mem *spimem)
>>  {
>> +	struct spi_device *spi = spimem->spi;
>>  	struct flash_platform_data	*data;
>>  	struct m25p *flash;
>>  	struct spi_nor *nor;
>> @@ -244,9 +164,9 @@ static int m25p_probe(struct spi_device *spi)
>>  	char *flash_name;
>>  	int ret;
>>  
>> -	data = dev_get_platdata(&spi->dev);
>> +	data = dev_get_platdata(&spimem->spi->dev);
>>  
>> -	flash = devm_kzalloc(&spi->dev, sizeof(*flash), GFP_KERNEL);
>> +	flash = devm_kzalloc(&spimem->spi->dev, sizeof(*flash), GFP_KERNEL);
>>  	if (!flash)
>>  		return -ENOMEM;
>>  
>> @@ -258,12 +178,12 @@ static int m25p_probe(struct spi_device *spi)
>>  	nor->write_reg = m25p80_write_reg;
>>  	nor->read_reg = m25p80_read_reg;
>>  
>> -	nor->dev = &spi->dev;
>> +	nor->dev = &spimem->spi->dev;
>>  	spi_nor_set_flash_node(nor, spi->dev.of_node);
>>  	nor->priv = flash;
>>  
>>  	spi_set_drvdata(spi, flash);
>> -	flash->spi = spi;
>> +	flash->spimem = spimem;
>>  
>>  	if (spi->mode & SPI_RX_QUAD) {
>>  		hwcaps.mask |= SNOR_HWCAPS_READ_1_1_4;
>> @@ -303,9 +223,9 @@ static int m25p_probe(struct spi_device *spi)
>>  }
>>  
>>  
>> -static int m25p_remove(struct spi_device *spi)
>> +static int m25p_remove(struct spi_mem *spimem)
>>  {
>> -	struct m25p	*flash = spi_get_drvdata(spi);
>> +	struct m25p	*flash = spi_mem_get_drvdata(spimem);
>>  
>>  	spi_nor_restore(&flash->spi_nor);
>>  
>> @@ -313,9 +233,9 @@ static int m25p_remove(struct spi_device *spi)
>>  	return mtd_device_unregister(&flash->spi_nor.mtd);
>>  }
>>  
>> -static void m25p_shutdown(struct spi_device *spi)
>> +static void m25p_shutdown(struct spi_mem *spimem)
>>  {
>> -	struct m25p *flash = spi_get_drvdata(spi);
>> +	struct m25p *flash = spi_mem_get_drvdata(spimem);
>>  
>>  	spi_nor_restore(&flash->spi_nor);
>>  }
>> @@ -386,12 +306,14 @@ static const struct of_device_id m25p_of_table[] = {
>>  };
>>  MODULE_DEVICE_TABLE(of, m25p_of_table);
>>  
>> -static struct spi_driver m25p80_driver = {
>> -	.driver = {
>> -		.name	= "m25p80",
>> -		.of_match_table = m25p_of_table,
>> +static struct spi_mem_driver m25p80_driver = {
>> +	.spidrv = {
>> +		.driver = {
>> +			.name	= "m25p80",
>> +			.of_match_table = m25p_of_table,
>> +		},
>> +		.id_table	= m25p_ids,
>>  	},
>> -	.id_table	= m25p_ids,
>>  	.probe	= m25p_probe,
>>  	.remove	= m25p_remove,
>>  	.shutdown	= m25p_shutdown,
>> @@ -402,7 +324,7 @@ static struct spi_driver m25p80_driver = {
>>  	 */
>>  };
>>  
>> -module_spi_driver(m25p80_driver);
>> +module_spi_mem_driver(m25p80_driver);
>>  
>>  MODULE_LICENSE("GPL");
>>  MODULE_AUTHOR("Mike Lavender");
>>
> 
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
> 

Re: [PATCH v4 6/7] mtd: spi-nor: Use the spi_mem_xx() API

[Geert Uytterhoeven]

Hi Cyrille,

On Wed, May 23, 2018 at 5:56 PM, Cyrille Pitchen
<cyrille.pitchen@microchip.com> wrote:
> Le 23/05/2018 à 17:44, Cyrille Pitchen a écrit :
>> Le 26/04/2018 à 18:18, Boris Brezillon a écrit :
>>>  static int m25p80_write_reg(struct spi_nor *nor, u8 opcode, u8 *buf, int len)
>>>  {
>>>      struct m25p *flash = nor->priv;
>>> -    struct spi_device *spi = flash->spi;
>>> -
>>> -    flash->command[0] = opcode;
>>> -    if (buf)
>>> -            memcpy(&flash->command[1], buf, len);
>>> +    struct spi_mem_op op = SPI_MEM_OP(SPI_MEM_OP_CMD(opcode, 1),
>>> +                                      SPI_MEM_OP_NO_ADDR,
>>> +                                      SPI_MEM_OP_NO_DUMMY,
>>
>> I didn't test nor compiled to verify but I have some doubt:
>> here the 'op' variable is allocated on the stack and both definitions
>> of SPI_MEM_OP_NO_ADDR and SPI_MEM_OP_NO_DUMMY macros are { }.
>>
>> It might be some rule of the C language that I ignore but are we guaranteed
>> that the .nbytes member of both structures is actually initialized to 0 ?
>>
>> I would say no: I think since the structure is allocated on the stack all
>> members not being explicitly initialized are left uninitialized by the
>> compiler when it generates the assembler code. However, I'm not 100% sure of
>> that.
>
> Actually, I'm reading that with C99 extensions, "omitted field members are
> implicitly initialized the same as objects that have static storage duration."
> https://gcc.gnu.org/onlinedocs/gcc/Designated-Inits.html

JFTR, omitted field members are.
But IIRC, implicit gaps due to alignment rules aren't, which mostly matters
for leaking random data to userspace.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v4 6/7] mtd: spi-nor: Use the spi_mem_xx() API

[Boris Brezillon]

Hi Cyrille,

On Wed, 23 May 2018 17:44:32 +0200
Cyrille Pitchen <cyrille.pitchen@microchip.com> wrote:

> > +					  SPI_MEM_OP_DATA_OUT(len, buf, 1));
> >  
> > -	return spi_write(spi, flash->command, len + 1);
> > +	return spi_mem_exec_op(flash->spimem, &op);
> >  }
> >  
> >  static ssize_t m25p80_write(struct spi_nor *nor, loff_t to, size_t len,
> >  			    const u_char *buf)
> >  {
> >  	struct m25p *flash = nor->priv;
> > -	struct spi_device *spi = flash->spi;
> > -	unsigned int inst_nbits, addr_nbits, data_nbits, data_idx;
> > -	struct spi_transfer t[3] = {};
> > -	struct spi_message m;
> > -	int cmd_sz = m25p_cmdsz(nor);
> > -	ssize_t ret;
> > +	struct spi_mem_op op =
> > +			SPI_MEM_OP(SPI_MEM_OP_CMD(nor->program_opcode, 1),
> > +				   SPI_MEM_OP_ADDR(nor->addr_width, to, 1),
> > +				   SPI_MEM_OP_DUMMY(0, 1),  
> 
> Why do you use SPI_MEM_OP_DUMMY(0, 1) here instead of SPI_MEM_OP_NO_DUMMY() ?

I guess I just copied the definition from m25p80_read() and modified
the definition without noticing this one could be replaced by
SPI_MEM_OP_NO_DUMMY. Not a big deal though, since SPI_MEM_OP_DUMMY(0,
1) still means "no dummy cycles".

Do you plan to send a patch to change that, or should I?

Regards,

Boris

[PATCH v4 7/7] spi: Get rid of the spi_flash_read() API

[Boris Brezillon]

This API has been replaced by the spi_mem_xx() one, its only user
(spi-nor) has been converted to spi_mem_xx() and all SPI controller
drivers that were implementing the ->spi_flash_xxx() hooks are also
implementing the spi_mem ones. So we can safely get rid of this API.

Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
---
Changes in v4:
- none

Changes in v3:
- none

Changes in v2:
- none
---
 drivers/spi/spi-bcm-qspi.c | 34 +++------------------------
 drivers/spi/spi-bcm53xx.c  | 20 +---------------
 drivers/spi/spi-ti-qspi.c  | 41 ---------------------------------
 drivers/spi/spi.c          | 57 ----------------------------------------------
 include/linux/spi/spi.h    | 53 ------------------------------------------
 5 files changed, 4 insertions(+), 201 deletions(-)

diff --git a/drivers/spi/spi-bcm-qspi.c b/drivers/spi/spi-bcm-qspi.c
index 9f94268a68b5..57ceec6c6301 100644
--- a/drivers/spi/spi-bcm-qspi.c
+++ b/drivers/spi/spi-bcm-qspi.c
@@ -944,9 +944,10 @@ static int bcm_qspi_mspi_exec_mem_op(struct spi_device *spi,
 	return ret;
 }
 
-static int bcm_qspi_exec_mem_op(struct spi_device *spi,
+static int bcm_qspi_exec_mem_op(struct spi_mem *mem,
 				const struct spi_mem_op *op)
 {
+	struct spi_device *spi = mem->spi;
 	struct bcm_qspi *qspi = spi_master_get_devdata(spi->master);
 	int ret = 0;
 	bool mspi_read = false;
@@ -991,34 +992,6 @@ static int bcm_qspi_exec_mem_op(struct spi_device *spi,
 	return ret;
 }
 
-static int bcm_qspi_exec_mem_op_wrapper(struct spi_mem *mem,
-					const struct spi_mem_op *op)
-{
-	return bcm_qspi_exec_mem_op(mem->spi, op);
-}
-
-static int bcm_qspi_flash_read_wrapper(struct spi_device *spi,
-				       struct spi_flash_read_message *msg)
-{
-	int ret;
-	struct spi_mem_op op = SPI_MEM_OP(SPI_MEM_OP_CMD(msg->read_opcode, 1),
-					  SPI_MEM_OP_ADDR(msg->addr_width,
-							  msg->from,
-							  msg->addr_nbits),
-					  SPI_MEM_OP_DUMMY(msg->dummy_bytes,
-							   msg->addr_nbits),
-					  SPI_MEM_OP_DATA_IN(msg->len,
-							     msg->buf,
-							     msg->data_nbits));
-
-	msg->retlen = 0;
-	ret = bcm_qspi_exec_mem_op(spi, &op);
-	if (!ret)
-		msg->retlen = msg->len;
-
-	return ret;
-}
-
 static void bcm_qspi_cleanup(struct spi_device *spi)
 {
 	struct bcm_qspi_parms *xp = spi_get_ctldata(spi);
@@ -1214,7 +1187,7 @@ static void bcm_qspi_hw_uninit(struct bcm_qspi *qspi)
 }
 
 static const struct spi_controller_mem_ops bcm_qspi_mem_ops = {
-	.exec_op = bcm_qspi_exec_mem_op_wrapper,
+	.exec_op = bcm_qspi_exec_mem_op,
 };
 
 static const struct of_device_id bcm_qspi_of_match[] = {
@@ -1259,7 +1232,6 @@ int bcm_qspi_probe(struct platform_device *pdev,
 	master->mode_bits = SPI_CPHA | SPI_CPOL | SPI_RX_DUAL | SPI_RX_QUAD;
 	master->setup = bcm_qspi_setup;
 	master->transfer_one = bcm_qspi_transfer_one;
-	master->spi_flash_read = bcm_qspi_flash_read_wrapper;
 	master->mem_ops = &bcm_qspi_mem_ops;
 	master->cleanup = bcm_qspi_cleanup;
 	master->dev.of_node = dev->of_node;
diff --git a/drivers/spi/spi-bcm53xx.c b/drivers/spi/spi-bcm53xx.c
index 5044e4e4a263..890b58e8645f 100644
--- a/drivers/spi/spi-bcm53xx.c
+++ b/drivers/spi/spi-bcm53xx.c
@@ -290,22 +290,6 @@ static const struct spi_controller_mem_ops bcm53xxspi_mem_ops = {
 	.exec_op = bcm53xxspi_exec_mem_op,
 };
 
-static int bcm53xxspi_flash_read(struct spi_device *spi,
-				 struct spi_flash_read_message *msg)
-{
-	struct bcm53xxspi *b53spi = spi_master_get_devdata(spi->master);
-	int ret = 0;
-
-	if (msg->from + msg->len > BCM53XXSPI_FLASH_WINDOW)
-		return -EINVAL;
-
-	bcm53xxspi_enable_bspi(b53spi);
-	memcpy_fromio(msg->buf, b53spi->mmio_base + msg->from, msg->len);
-	msg->retlen = msg->len;
-
-	return ret;
-}
-
 /**************************************************
  * BCMA
  **************************************************/
@@ -344,10 +328,8 @@ static int bcm53xxspi_bcma_probe(struct bcma_device *core)
 
 	master->dev.of_node = dev->of_node;
 	master->transfer_one = bcm53xxspi_transfer_one;
-	if (b53spi->mmio_base) {
+	if (b53spi->mmio_base)
 		master->mem_ops = &bcm53xxspi_mem_ops;
-		master->spi_flash_read = bcm53xxspi_flash_read;
-	}
 
 	bcma_set_drvdata(core, b53spi);
 
diff --git a/drivers/spi/spi-ti-qspi.c b/drivers/spi/spi-ti-qspi.c
index 1b12c2ff8be3..c54b760e00ed 100644
--- a/drivers/spi/spi-ti-qspi.c
+++ b/drivers/spi/spi-ti-qspi.c
@@ -531,44 +531,6 @@ static void ti_qspi_setup_mmap_read(struct spi_device *spi, u8 opcode,
 		      QSPI_SPI_SETUP_REG(spi->chip_select));
 }
 
-static bool ti_qspi_spi_flash_can_dma(struct spi_device *spi,
-				      struct spi_flash_read_message *msg)
-{
-	return virt_addr_valid(msg->buf);
-}
-
-static int ti_qspi_spi_flash_read(struct spi_device *spi,
-				  struct spi_flash_read_message *msg)
-{
-	struct ti_qspi *qspi = spi_master_get_devdata(spi->master);
-	int ret = 0;
-
-	mutex_lock(&qspi->list_lock);
-
-	if (!qspi->mmap_enabled)
-		ti_qspi_enable_memory_map(spi);
-	ti_qspi_setup_mmap_read(spi, msg->read_opcode, msg->data_nbits,
-				msg->addr_width, msg->dummy_bytes);
-
-	if (qspi->rx_chan) {
-		if (msg->cur_msg_mapped)
-			ret = ti_qspi_dma_xfer_sg(qspi, msg->rx_sg, msg->from);
-		else
-			ret = ti_qspi_dma_bounce_buffer(qspi, msg->from,
-							msg->buf, msg->len);
-		if (ret)
-			goto err_unlock;
-	} else {
-		memcpy_fromio(msg->buf, qspi->mmap_base + msg->from, msg->len);
-	}
-	msg->retlen = msg->len;
-
-err_unlock:
-	mutex_unlock(&qspi->list_lock);
-
-	return ret;
-}
-
 static int ti_qspi_exec_mem_op(struct spi_mem *mem,
 			       const struct spi_mem_op *op)
 {
@@ -727,7 +689,6 @@ static int ti_qspi_probe(struct platform_device *pdev)
 	master->dev.of_node = pdev->dev.of_node;
 	master->bits_per_word_mask = SPI_BPW_MASK(32) | SPI_BPW_MASK(16) |
 				     SPI_BPW_MASK(8);
-	master->spi_flash_read = ti_qspi_spi_flash_read;
 	master->mem_ops = &ti_qspi_mem_ops;
 
 	if (!of_property_read_u32(np, "num-cs", &num_cs))
@@ -827,7 +788,6 @@ static int ti_qspi_probe(struct platform_device *pdev)
 		dma_release_channel(qspi->rx_chan);
 		goto no_dma;
 	}
-	master->spi_flash_can_dma = ti_qspi_spi_flash_can_dma;
 	master->dma_rx = qspi->rx_chan;
 	init_completion(&qspi->transfer_complete);
 	if (res_mmap)
@@ -841,7 +801,6 @@ static int ti_qspi_probe(struct platform_device *pdev)
 				 "mmap failed with error %ld using PIO mode\n",
 				 PTR_ERR(qspi->mmap_base));
 			qspi->mmap_base = NULL;
-			master->spi_flash_read = NULL;
 			master->mem_ops = NULL;
 		}
 	}
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index c85b0cf7b4a9..8ee1ba13eb23 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -3055,63 +3055,6 @@ int spi_async_locked(struct spi_device *spi, struct spi_message *message)
 }
 EXPORT_SYMBOL_GPL(spi_async_locked);
 
-
-int spi_flash_read(struct spi_device *spi,
-		   struct spi_flash_read_message *msg)
-
-{
-	struct spi_controller *master = spi->controller;
-	struct device *rx_dev = NULL;
-	int ret;
-
-	if ((msg->opcode_nbits == SPI_NBITS_DUAL ||
-	     msg->addr_nbits == SPI_NBITS_DUAL) &&
-	    !(spi->mode & (SPI_TX_DUAL | SPI_TX_QUAD)))
-		return -EINVAL;
-	if ((msg->opcode_nbits == SPI_NBITS_QUAD ||
-	     msg->addr_nbits == SPI_NBITS_QUAD) &&
-	    !(spi->mode & SPI_TX_QUAD))
-		return -EINVAL;
-	if (msg->data_nbits == SPI_NBITS_DUAL &&
-	    !(spi->mode & (SPI_RX_DUAL | SPI_RX_QUAD)))
-		return -EINVAL;
-	if (msg->data_nbits == SPI_NBITS_QUAD &&
-	    !(spi->mode &  SPI_RX_QUAD))
-		return -EINVAL;
-
-	if (master->auto_runtime_pm) {
-		ret = pm_runtime_get_sync(master->dev.parent);
-		if (ret < 0) {
-			dev_err(&master->dev, "Failed to power device: %d\n",
-				ret);
-			return ret;
-		}
-	}
-
-	mutex_lock(&master->bus_lock_mutex);
-	mutex_lock(&master->io_mutex);
-	if (master->dma_rx && master->spi_flash_can_dma(spi, msg)) {
-		rx_dev = master->dma_rx->device->dev;
-		ret = spi_map_buf(master, rx_dev, &msg->rx_sg,
-				  msg->buf, msg->len,
-				  DMA_FROM_DEVICE);
-		if (!ret)
-			msg->cur_msg_mapped = true;
-	}
-	ret = master->spi_flash_read(spi, msg);
-	if (msg->cur_msg_mapped)
-		spi_unmap_buf(master, rx_dev, &msg->rx_sg,
-			      DMA_FROM_DEVICE);
-	mutex_unlock(&master->io_mutex);
-	mutex_unlock(&master->bus_lock_mutex);
-
-	if (master->auto_runtime_pm)
-		pm_runtime_put(master->dev.parent);
-
-	return ret;
-}
-EXPORT_SYMBOL_GPL(spi_flash_read);
-
 /*-------------------------------------------------------------------------*/
 
 /* Utility methods for SPI protocol drivers, layered on
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index a7e0bbed738c..a64235e05321 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -26,7 +26,6 @@ struct dma_chan;
 struct property_entry;
 struct spi_controller;
 struct spi_transfer;
-struct spi_flash_read_message;
 struct spi_controller_mem_ops;
 
 /*
@@ -382,11 +381,6 @@ static inline void spi_unregister_driver(struct spi_driver *sdrv)
  *	     controller has native support for memory like operations.
  * @unprepare_message: undo any work done by prepare_message().
  * @slave_abort: abort the ongoing transfer request on an SPI slave controller
- * @spi_flash_read: to support spi-controller hardwares that provide
- *                  accelerated interface to read from flash devices.
- * @spi_flash_can_dma: analogous to can_dma() interface, but for
- *		       controllers implementing spi_flash_read.
- * @flash_read_supported: spi device supports flash read
  * @cs_gpios: Array of GPIOs to use as chip select lines; one per CS
  *	number. Any individual value may be -ENOENT for CS lines that
  *	are not GPIOs (driven by the SPI controller itself).
@@ -552,11 +546,6 @@ struct spi_controller {
 	int (*unprepare_message)(struct spi_controller *ctlr,
 				 struct spi_message *message);
 	int (*slave_abort)(struct spi_controller *ctlr);
-	int (*spi_flash_read)(struct  spi_device *spi,
-			      struct spi_flash_read_message *msg);
-	bool (*spi_flash_can_dma)(struct spi_device *spi,
-				  struct spi_flash_read_message *msg);
-	bool (*flash_read_supported)(struct spi_device *spi);
 
 	/*
 	 * These hooks are for drivers that use a generic implementation
@@ -1190,48 +1179,6 @@ static inline ssize_t spi_w8r16be(struct spi_device *spi, u8 cmd)
 	return be16_to_cpu(result);
 }
 
-/**
- * struct spi_flash_read_message - flash specific information for
- * spi-masters that provide accelerated flash read interfaces
- * @buf: buffer to read data
- * @from: offset within the flash from where data is to be read
- * @len: length of data to be read
- * @retlen: actual length of data read
- * @read_opcode: read_opcode to be used to communicate with flash
- * @addr_width: number of address bytes
- * @dummy_bytes: number of dummy bytes
- * @opcode_nbits: number of lines to send opcode
- * @addr_nbits: number of lines to send address
- * @data_nbits: number of lines for data
- * @rx_sg: Scatterlist for receive data read from flash
- * @cur_msg_mapped: message has been mapped for DMA
- */
-struct spi_flash_read_message {
-	void *buf;
-	loff_t from;
-	size_t len;
-	size_t retlen;
-	u8 read_opcode;
-	u8 addr_width;
-	u8 dummy_bytes;
-	u8 opcode_nbits;
-	u8 addr_nbits;
-	u8 data_nbits;
-	struct sg_table rx_sg;
-	bool cur_msg_mapped;
-};
-
-/* SPI core interface for flash read support */
-static inline bool spi_flash_read_supported(struct spi_device *spi)
-{
-	return spi->controller->spi_flash_read &&
-	       (!spi->controller->flash_read_supported ||
-	       spi->controller->flash_read_supported(spi));
-}
-
-int spi_flash_read(struct spi_device *spi,
-		   struct spi_flash_read_message *msg);
-
 /*---------------------------------------------------------------------------*/
 
 /*
-- 
2.14.1

Re: [PATCH v4 7/7] spi: Get rid of the spi_flash_read() API

[Frieder Schrempf]

On 26.04.2018 18:18, Boris Brezillon wrote:
> This API has been replaced by the spi_mem_xx() one, its only user
> (spi-nor) has been converted to spi_mem_xx() and all SPI controller
> drivers that were implementing the ->spi_flash_xxx() hooks are also
> implementing the spi_mem ones. So we can safely get rid of this API.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>

Reviewed-by: Frieder Schrempf <frieder.schrempf@exceet.de>
Tested-by: Frieder Schrempf <frieder.schrempf@exceet.de>

Regards,

Frieder

> ---
> Changes in v4:
> - none
> 
> Changes in v3:
> - none
> 
> Changes in v2:
> - none
> ---
>   drivers/spi/spi-bcm-qspi.c | 34 +++------------------------
>   drivers/spi/spi-bcm53xx.c  | 20 +---------------
>   drivers/spi/spi-ti-qspi.c  | 41 ---------------------------------
>   drivers/spi/spi.c          | 57 ----------------------------------------------
>   include/linux/spi/spi.h    | 53 ------------------------------------------
>   5 files changed, 4 insertions(+), 201 deletions(-)
> 
> diff --git a/drivers/spi/spi-bcm-qspi.c b/drivers/spi/spi-bcm-qspi.c
> index 9f94268a68b5..57ceec6c6301 100644
> --- a/drivers/spi/spi-bcm-qspi.c
> +++ b/drivers/spi/spi-bcm-qspi.c
> @@ -944,9 +944,10 @@ static int bcm_qspi_mspi_exec_mem_op(struct spi_device *spi,
>   	return ret;
>   }
>   
> -static int bcm_qspi_exec_mem_op(struct spi_device *spi,
> +static int bcm_qspi_exec_mem_op(struct spi_mem *mem,
>   				const struct spi_mem_op *op)
>   {
> +	struct spi_device *spi = mem->spi;
>   	struct bcm_qspi *qspi = spi_master_get_devdata(spi->master);
>   	int ret = 0;
>   	bool mspi_read = false;
> @@ -991,34 +992,6 @@ static int bcm_qspi_exec_mem_op(struct spi_device *spi,
>   	return ret;
>   }
>   
> -static int bcm_qspi_exec_mem_op_wrapper(struct spi_mem *mem,
> -					const struct spi_mem_op *op)
> -{
> -	return bcm_qspi_exec_mem_op(mem->spi, op);
> -}
> -
> -static int bcm_qspi_flash_read_wrapper(struct spi_device *spi,
> -				       struct spi_flash_read_message *msg)
> -{
> -	int ret;
> -	struct spi_mem_op op = SPI_MEM_OP(SPI_MEM_OP_CMD(msg->read_opcode, 1),
> -					  SPI_MEM_OP_ADDR(msg->addr_width,
> -							  msg->from,
> -							  msg->addr_nbits),
> -					  SPI_MEM_OP_DUMMY(msg->dummy_bytes,
> -							   msg->addr_nbits),
> -					  SPI_MEM_OP_DATA_IN(msg->len,
> -							     msg->buf,
> -							     msg->data_nbits));
> -
> -	msg->retlen = 0;
> -	ret = bcm_qspi_exec_mem_op(spi, &op);
> -	if (!ret)
> -		msg->retlen = msg->len;
> -
> -	return ret;
> -}
> -
>   static void bcm_qspi_cleanup(struct spi_device *spi)
>   {
>   	struct bcm_qspi_parms *xp = spi_get_ctldata(spi);
> @@ -1214,7 +1187,7 @@ static void bcm_qspi_hw_uninit(struct bcm_qspi *qspi)
>   }
>   
>   static const struct spi_controller_mem_ops bcm_qspi_mem_ops = {
> -	.exec_op = bcm_qspi_exec_mem_op_wrapper,
> +	.exec_op = bcm_qspi_exec_mem_op,
>   };
>   
>   static const struct of_device_id bcm_qspi_of_match[] = {
> @@ -1259,7 +1232,6 @@ int bcm_qspi_probe(struct platform_device *pdev,
>   	master->mode_bits = SPI_CPHA | SPI_CPOL | SPI_RX_DUAL | SPI_RX_QUAD;
>   	master->setup = bcm_qspi_setup;
>   	master->transfer_one = bcm_qspi_transfer_one;
> -	master->spi_flash_read = bcm_qspi_flash_read_wrapper;
>   	master->mem_ops = &bcm_qspi_mem_ops;
>   	master->cleanup = bcm_qspi_cleanup;
>   	master->dev.of_node = dev->of_node;
> diff --git a/drivers/spi/spi-bcm53xx.c b/drivers/spi/spi-bcm53xx.c
> index 5044e4e4a263..890b58e8645f 100644
> --- a/drivers/spi/spi-bcm53xx.c
> +++ b/drivers/spi/spi-bcm53xx.c
> @@ -290,22 +290,6 @@ static const struct spi_controller_mem_ops bcm53xxspi_mem_ops = {
>   	.exec_op = bcm53xxspi_exec_mem_op,
>   };
>   
> -static int bcm53xxspi_flash_read(struct spi_device *spi,
> -				 struct spi_flash_read_message *msg)
> -{
> -	struct bcm53xxspi *b53spi = spi_master_get_devdata(spi->master);
> -	int ret = 0;
> -
> -	if (msg->from + msg->len > BCM53XXSPI_FLASH_WINDOW)
> -		return -EINVAL;
> -
> -	bcm53xxspi_enable_bspi(b53spi);
> -	memcpy_fromio(msg->buf, b53spi->mmio_base + msg->from, msg->len);
> -	msg->retlen = msg->len;
> -
> -	return ret;
> -}
> -
>   /**************************************************
>    * BCMA
>    **************************************************/
> @@ -344,10 +328,8 @@ static int bcm53xxspi_bcma_probe(struct bcma_device *core)
>   
>   	master->dev.of_node = dev->of_node;
>   	master->transfer_one = bcm53xxspi_transfer_one;
> -	if (b53spi->mmio_base) {
> +	if (b53spi->mmio_base)
>   		master->mem_ops = &bcm53xxspi_mem_ops;
> -		master->spi_flash_read = bcm53xxspi_flash_read;
> -	}
>   
>   	bcma_set_drvdata(core, b53spi);
>   
> diff --git a/drivers/spi/spi-ti-qspi.c b/drivers/spi/spi-ti-qspi.c
> index 1b12c2ff8be3..c54b760e00ed 100644
> --- a/drivers/spi/spi-ti-qspi.c
> +++ b/drivers/spi/spi-ti-qspi.c
> @@ -531,44 +531,6 @@ static void ti_qspi_setup_mmap_read(struct spi_device *spi, u8 opcode,
>   		      QSPI_SPI_SETUP_REG(spi->chip_select));
>   }
>   
> -static bool ti_qspi_spi_flash_can_dma(struct spi_device *spi,
> -				      struct spi_flash_read_message *msg)
> -{
> -	return virt_addr_valid(msg->buf);
> -}
> -
> -static int ti_qspi_spi_flash_read(struct spi_device *spi,
> -				  struct spi_flash_read_message *msg)
> -{
> -	struct ti_qspi *qspi = spi_master_get_devdata(spi->master);
> -	int ret = 0;
> -
> -	mutex_lock(&qspi->list_lock);
> -
> -	if (!qspi->mmap_enabled)
> -		ti_qspi_enable_memory_map(spi);
> -	ti_qspi_setup_mmap_read(spi, msg->read_opcode, msg->data_nbits,
> -				msg->addr_width, msg->dummy_bytes);
> -
> -	if (qspi->rx_chan) {
> -		if (msg->cur_msg_mapped)
> -			ret = ti_qspi_dma_xfer_sg(qspi, msg->rx_sg, msg->from);
> -		else
> -			ret = ti_qspi_dma_bounce_buffer(qspi, msg->from,
> -							msg->buf, msg->len);
> -		if (ret)
> -			goto err_unlock;
> -	} else {
> -		memcpy_fromio(msg->buf, qspi->mmap_base + msg->from, msg->len);
> -	}
> -	msg->retlen = msg->len;
> -
> -err_unlock:
> -	mutex_unlock(&qspi->list_lock);
> -
> -	return ret;
> -}
> -
>   static int ti_qspi_exec_mem_op(struct spi_mem *mem,
>   			       const struct spi_mem_op *op)
>   {
> @@ -727,7 +689,6 @@ static int ti_qspi_probe(struct platform_device *pdev)
>   	master->dev.of_node = pdev->dev.of_node;
>   	master->bits_per_word_mask = SPI_BPW_MASK(32) | SPI_BPW_MASK(16) |
>   				     SPI_BPW_MASK(8);
> -	master->spi_flash_read = ti_qspi_spi_flash_read;
>   	master->mem_ops = &ti_qspi_mem_ops;
>   
>   	if (!of_property_read_u32(np, "num-cs", &num_cs))
> @@ -827,7 +788,6 @@ static int ti_qspi_probe(struct platform_device *pdev)
>   		dma_release_channel(qspi->rx_chan);
>   		goto no_dma;
>   	}
> -	master->spi_flash_can_dma = ti_qspi_spi_flash_can_dma;
>   	master->dma_rx = qspi->rx_chan;
>   	init_completion(&qspi->transfer_complete);
>   	if (res_mmap)
> @@ -841,7 +801,6 @@ static int ti_qspi_probe(struct platform_device *pdev)
>   				 "mmap failed with error %ld using PIO mode\n",
>   				 PTR_ERR(qspi->mmap_base));
>   			qspi->mmap_base = NULL;
> -			master->spi_flash_read = NULL;
>   			master->mem_ops = NULL;
>   		}
>   	}
> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> index c85b0cf7b4a9..8ee1ba13eb23 100644
> --- a/drivers/spi/spi.c
> +++ b/drivers/spi/spi.c
> @@ -3055,63 +3055,6 @@ int spi_async_locked(struct spi_device *spi, struct spi_message *message)
>   }
>   EXPORT_SYMBOL_GPL(spi_async_locked);
>   
> -
> -int spi_flash_read(struct spi_device *spi,
> -		   struct spi_flash_read_message *msg)
> -
> -{
> -	struct spi_controller *master = spi->controller;
> -	struct device *rx_dev = NULL;
> -	int ret;
> -
> -	if ((msg->opcode_nbits == SPI_NBITS_DUAL ||
> -	     msg->addr_nbits == SPI_NBITS_DUAL) &&
> -	    !(spi->mode & (SPI_TX_DUAL | SPI_TX_QUAD)))
> -		return -EINVAL;
> -	if ((msg->opcode_nbits == SPI_NBITS_QUAD ||
> -	     msg->addr_nbits == SPI_NBITS_QUAD) &&
> -	    !(spi->mode & SPI_TX_QUAD))
> -		return -EINVAL;
> -	if (msg->data_nbits == SPI_NBITS_DUAL &&
> -	    !(spi->mode & (SPI_RX_DUAL | SPI_RX_QUAD)))
> -		return -EINVAL;
> -	if (msg->data_nbits == SPI_NBITS_QUAD &&
> -	    !(spi->mode &  SPI_RX_QUAD))
> -		return -EINVAL;
> -
> -	if (master->auto_runtime_pm) {
> -		ret = pm_runtime_get_sync(master->dev.parent);
> -		if (ret < 0) {
> -			dev_err(&master->dev, "Failed to power device: %d\n",
> -				ret);
> -			return ret;
> -		}
> -	}
> -
> -	mutex_lock(&master->bus_lock_mutex);
> -	mutex_lock(&master->io_mutex);
> -	if (master->dma_rx && master->spi_flash_can_dma(spi, msg)) {
> -		rx_dev = master->dma_rx->device->dev;
> -		ret = spi_map_buf(master, rx_dev, &msg->rx_sg,
> -				  msg->buf, msg->len,
> -				  DMA_FROM_DEVICE);
> -		if (!ret)
> -			msg->cur_msg_mapped = true;
> -	}
> -	ret = master->spi_flash_read(spi, msg);
> -	if (msg->cur_msg_mapped)
> -		spi_unmap_buf(master, rx_dev, &msg->rx_sg,
> -			      DMA_FROM_DEVICE);
> -	mutex_unlock(&master->io_mutex);
> -	mutex_unlock(&master->bus_lock_mutex);
> -
> -	if (master->auto_runtime_pm)
> -		pm_runtime_put(master->dev.parent);
> -
> -	return ret;
> -}
> -EXPORT_SYMBOL_GPL(spi_flash_read);
> -
>   /*-------------------------------------------------------------------------*/
>   
>   /* Utility methods for SPI protocol drivers, layered on
> diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
> index a7e0bbed738c..a64235e05321 100644
> --- a/include/linux/spi/spi.h
> +++ b/include/linux/spi/spi.h
> @@ -26,7 +26,6 @@ struct dma_chan;
>   struct property_entry;
>   struct spi_controller;
>   struct spi_transfer;
> -struct spi_flash_read_message;
>   struct spi_controller_mem_ops;
>   
>   /*
> @@ -382,11 +381,6 @@ static inline void spi_unregister_driver(struct spi_driver *sdrv)
>    *	     controller has native support for memory like operations.
>    * @unprepare_message: undo any work done by prepare_message().
>    * @slave_abort: abort the ongoing transfer request on an SPI slave controller
> - * @spi_flash_read: to support spi-controller hardwares that provide
> - *                  accelerated interface to read from flash devices.
> - * @spi_flash_can_dma: analogous to can_dma() interface, but for
> - *		       controllers implementing spi_flash_read.
> - * @flash_read_supported: spi device supports flash read
>    * @cs_gpios: Array of GPIOs to use as chip select lines; one per CS
>    *	number. Any individual value may be -ENOENT for CS lines that
>    *	are not GPIOs (driven by the SPI controller itself).
> @@ -552,11 +546,6 @@ struct spi_controller {
>   	int (*unprepare_message)(struct spi_controller *ctlr,
>   				 struct spi_message *message);
>   	int (*slave_abort)(struct spi_controller *ctlr);
> -	int (*spi_flash_read)(struct  spi_device *spi,
> -			      struct spi_flash_read_message *msg);
> -	bool (*spi_flash_can_dma)(struct spi_device *spi,
> -				  struct spi_flash_read_message *msg);
> -	bool (*flash_read_supported)(struct spi_device *spi);
>   
>   	/*
>   	 * These hooks are for drivers that use a generic implementation
> @@ -1190,48 +1179,6 @@ static inline ssize_t spi_w8r16be(struct spi_device *spi, u8 cmd)
>   	return be16_to_cpu(result);
>   }
>   
> -/**
> - * struct spi_flash_read_message - flash specific information for
> - * spi-masters that provide accelerated flash read interfaces
> - * @buf: buffer to read data
> - * @from: offset within the flash from where data is to be read
> - * @len: length of data to be read
> - * @retlen: actual length of data read
> - * @read_opcode: read_opcode to be used to communicate with flash
> - * @addr_width: number of address bytes
> - * @dummy_bytes: number of dummy bytes
> - * @opcode_nbits: number of lines to send opcode
> - * @addr_nbits: number of lines to send address
> - * @data_nbits: number of lines for data
> - * @rx_sg: Scatterlist for receive data read from flash
> - * @cur_msg_mapped: message has been mapped for DMA
> - */
> -struct spi_flash_read_message {
> -	void *buf;
> -	loff_t from;
> -	size_t len;
> -	size_t retlen;
> -	u8 read_opcode;
> -	u8 addr_width;
> -	u8 dummy_bytes;
> -	u8 opcode_nbits;
> -	u8 addr_nbits;
> -	u8 data_nbits;
> -	struct sg_table rx_sg;
> -	bool cur_msg_mapped;
> -};
> -
> -/* SPI core interface for flash read support */
> -static inline bool spi_flash_read_supported(struct spi_device *spi)
> -{
> -	return spi->controller->spi_flash_read &&
> -	       (!spi->controller->flash_read_supported ||
> -	       spi->controller->flash_read_supported(spi));
> -}
> -
> -int spi_flash_read(struct spi_device *spi,
> -		   struct spi_flash_read_message *msg);
> -
>   /*---------------------------------------------------------------------------*/
>   
>   /*
> 

Re: [PATCH v4 0/7] spi: Extend the framework to generically support memory devices

[Frieder Schrempf]

Hi Boris,

On 26.04.2018 18:18, Boris Brezillon wrote:
> Hello,
> 
> Shrinking a bit the explanation on why the spi-mem abstraction is
> needed (a detailed explanation is available here [2]). In addition to
> what as been said in my initial explanation I'll add that making it part
> of the SPI framework instead of as an extra independent layer is
> justified by the fact that some controllers support both SPI memory
> operations and regular SPI transfers, and it's cleaner to have both
> features exposed through a single driver.
> 
> For those who want to have the full picture, here is a branch [1]
> containing the SPI NAND framework based on top of this spi-mem layer.
> 
> Thanks,
> 
> Boris
> 
> [1]https://smex12-5-en-ctp.trendmicro.com:443/wis/clicktime/v1/query?url=https%3a%2f%2fgithub.com%2fbbrezillon%2flinux%2ftree%2fspi%2dmem&umid=78d7b933-335d-48ad-a7f9-47b716b20fc3&auth=541e45255b6517100d80c2b1b80b6933b203c492-068a43ed6b9776ab1e94d8bfe0b7982ce0a4715d
> [2]https://www.spinics.net/lists/linux-spi/msg12058.html
> 
> Boris Brezillon (7):
>    spi: Extend the core to ease integration of SPI memory controllers
>    spi: Make support for regular transfers optional when ->mem_ops !=
>      NULL
>    spi: bcm-qspi: Implement the spi_mem interface
>    spi: bcm53xx: Implement the spi_mem interface
>    spi: ti-qspi: Implement the spi_mem interface
>    mtd: spi-nor: Use the spi_mem_xx() API
>    spi: Get rid of the spi_flash_read() API

I have been using these patches quite a bit in different setups:

* with your spi_mem port of fsl-quadspi and SPI NAND/NOR
* with a spi_mem port of stm32-quadspi and SPI NOR
* with generic SPI and SPI NOR

So to make this official, I will add R-b/T-b tags in separate replies to 
the individual patches.

Regards,

Frieder

> 
>   drivers/mtd/devices/Kconfig  |   1 +
>   drivers/mtd/devices/m25p80.c | 236 +++++++++----------------
>   drivers/spi/Kconfig          |   7 +
>   drivers/spi/Makefile         |   1 +
>   drivers/spi/spi-bcm-qspi.c   | 162 ++++++++---------
>   drivers/spi/spi-bcm53xx.c    |  37 ++--
>   drivers/spi/spi-mem.c        | 410 +++++++++++++++++++++++++++++++++++++++++++
>   drivers/spi/spi-ti-qspi.c    |  85 +++++----
>   drivers/spi/spi.c            |  90 +++-------
>   include/linux/spi/spi-mem.h  | 249 ++++++++++++++++++++++++++
>   include/linux/spi/spi.h      |  60 +------
>   11 files changed, 941 insertions(+), 397 deletions(-)
>   create mode 100644 drivers/spi/spi-mem.c
>   create mode 100644 include/linux/spi/spi-mem.h
>