SPI-NAND support

Some MTD sublayers/drivers are implementing ->_read/write() and not ->_read/write_oob().

While for NAND devices both are usually valid, for NOR devices, using the _oob variant has no real meaning. But, as the MTD layer is supposed to hide as much as possible the flash complexity to the user, there is no reason to error out while it is just a matter of rewritting things internally.

Add a fallback on mtd->_read() (resp. mtd->_write()) when the user calls mtd_read_oob() (resp. mtd_write_oob()) while mtd->_read_oob() (resp. mtd->_write_oob) is not implemented. There is already a fallback on the _oob variant if the former is used.

Signed-off-by Miquel Raynal <[email protected]>

---
 drivers/mtd/mtdcore.c | 26 ++++++++++++++++++++------
 1 file changed, 20 insertions(+), 6 deletions(-)

drivers/mtd/mtdcore.c +0 -0

index ba170f212e..095c58cf8c 100644
@@ -1048,20 +1048,27 @@ int mtd_read_oob(struct mtd_info *mtd, loff_t from, struct mtd_oob_ops *ops)
 {
 	int ret_code;

drivers/mtd/mtdcore.c +9 -3

+		return -EOPNOTSUPP;
+
+	if (mtd->_read_oob)
+		ret_code = mtd->_read_oob(mtd, from, ops);
+	else
+		ret_code = mtd->_read(mtd, from, ops->len, &ops->retlen,
+				      ops->datbuf);
+
 	/*
 	 * In cases where ops->datbuf != NULL, mtd->_read_oob() has semantics
 	 * similar to mtd->_read(), returning a non-negative integer
 	 * representing max bitflips. In other cases, mtd->_read_oob() may
 	 * return -EUCLEAN. In all cases, perform similar logic to mtd_read().
 	 */
-	ret_code = mtd->_read_oob(mtd, from, ops);
 	if (unlikely(ret_code < 0))
 		return ret_code;
 	if (mtd->ecc_strength == 0)

@@ -1076,8 +1083,7 @@ int mtd_write_oob(struct mtd_info *mtd, loff_t to,
 	int ret;
 
 	ops->retlen = ops->oobretlen = 0;
-	if (!mtd->_write_oob)
-		return -EOPNOTSUPP;
+
 	if (!(mtd->flags & MTD_WRITEABLE))
 		return -EROFS;
 

drivers/mtd/mtdcore.c +7 -0

+		return -EOPNOTSUPP;
+
+	if (mtd->_write_oob)
+		return mtd->_write_oob(mtd, to, ops);
+	else
+		return mtd->_write(mtd, to, ops->len, &ops->retlen,
+				   ops->datbuf);
 }
 EXPORT_SYMBOL_GPL(mtd_write_oob);
 

Quoted 9 lines · On Wed, 11 Jul 2018 17:25:12 +0200 Miquel Raynal <[email protected]>…

Some MTD sublayers/drivers are implementing ->_read/write() and not ->_read/write_oob().

While for NAND devices both are usually valid, for NOR devices, using the _oob variant has no real meaning. But, as the MTD layer is supposed to hide as much as possible the flash complexity to the user, there is no reason to error out while it is just a matter of rewritting things internally.

Quoted 4 lines

Add a fallback on mtd->_read() (resp. mtd->_write()) when the user calls mtd_read_oob() (resp. mtd_write_oob()) while mtd->_read_oob() (resp. mtd->_write_oob) is not implemented. There is already a fallback on the _oob variant if the former is used.

Now I'm jealous. Can we have the same thing Linux :-).

Quoted 14 lines

Signed-off-by Miquel Raynal <[email protected]>

---
 drivers/mtd/mtdcore.c | 26 ++++++++++++++++++++------
 1 file changed, 20 insertions(+), 6 deletions(-)

drivers/mtd/mtdcore.c +0 -0

index ba170f212e..095c58cf8c 100644
@@ -1048,20 +1048,27 @@ int mtd_read_oob(struct mtd_info *mtd, loff_t from, struct mtd_oob_ops *ops)
 {
 	int ret_code;

drivers/mtd/mtdcore.c int mtd_read_oob(struct mtd_info *mtd, loff_t from, struct mtd_oob_ops *ops) · line 1057

Quoted 10 lines

drivers/mtd/mtdcore.c +2 -2

 	ops->retlen = ops->oobretlen = 0;
-	if (!mtd->_read_oob)
-		return -EOPNOTSUPP;
 
 	ret_code = mtd_check_oob_ops(mtd, from, ops);
 	if (ret_code)
 		return ret_code;
 
+	/* Check the validity of a potential fallback on mtd->_read */
+	if ((!mtd->_read_oob) && (!mtd->_read || ops->oobbuf))  

Parens around !mtd->_read_oob are not needed.

drivers/mtd/mtdcore.c int mtd_write_oob(struct mtd_info *mtd, loff_t to, · line 1095

Quoted 3 lines

drivers/mtd/mtdcore.c +2 -1

-	return mtd->_write_oob(mtd, to, ops);
+	/* Check the validity of a potential fallback on mtd->_write */
+	if ((!mtd->_write_oob) && (!mtd->_write || ops->oobbuf))  

Ditto.

NAND flavors, like serial and parallel, have a lot in common and would benefit to share code. Let's move raw (parallel) NAND specific code in a raw/ subdirectory, to ease the addition of a core file in nand/ and the introduction of a spi/ subdirectory specific to SPI NANDs.

Signed-off-by Miquel Raynal <[email protected]>

From: Boris Brezillon <[email protected]>

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 <[email protected]>

Signed-off-by Miquel Raynal <[email protected]>

---
  drivers/spi/Kconfig   |   7 +
  drivers/spi/Makefile  |   1 +
  drivers/spi/spi-mem.c | 500 ++++++++++++++++++++++++++++++++++++++++++++++++++
  include/spi-mem.h     | 258 ++++++++++++++++++++++++++
  include/spi.h         |  11 ++
  5 files changed, 777 insertions(+)

create mode 100644 drivers/spi/spi-mem.c
create mode 100644 include/spi-mem.h

drivers/spi/Kconfig +7 -0

index 235a8c7d73..0ee371b2d9 100644
@@ -15,6 +15,13 @@ config DM_SPI
 
  if DM_SPI
 
+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.
+
  config ALTERA_SPI
  	bool "Altera SPI driver"
  	help

drivers/spi/Makefile +1 -0

index 4b6000fd9a..982529a0e6 100644
@@ -10,6 +10,7 @@ ifdef CONFIG_DM_SPI
  obj-y += spi-uclass.o
  obj-$(CONFIG_SANDBOX) += spi-emul-uclass.o
  obj-$(CONFIG_SOFT_SPI) += soft_spi.o
+obj-$(CONFIG_SPI_MEM) += spi-mem.o
  else
  obj-y += spi.o
  obj-$(CONFIG_SOFT_SPI) += soft_spi_legacy.o

drivers/spi/spi-mem.c new file +373 -0

new file mode 100644
index 0000000000..1aabe56819
@@ -0,0 +1,500 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2018 Exceet Electronics GmbH
+ * Copyright (C) 2018 Bootlin
+ *
+ * Author: Boris Brezillon <[email protected]>
+ */
+
+#ifndef __UBOOT__
+#include <linux/dmaengine.h>
+#include <linux/pm_runtime.h>
+#include "internals.h"
+#else
+#include <spi.h>
+#include <spi-mem.h>
+#endif
+
+#ifndef __UBOOT__
+/**
+ * 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);
+#endif /* __UBOOT__ */
+
+static int spi_check_buswidth_req(struct spi_slave *slave, u8 buswidth, bool tx)
+{
+	u32 mode = slave->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;
+}
+
+bool spi_mem_default_supports_op(struct spi_slave *slave,
+				 const struct spi_mem_op *op)
+{
+	if (spi_check_buswidth_req(slave, op->cmd.buswidth, true))
+		return false;
+
+	if (op->addr.nbytes &&
+	    spi_check_buswidth_req(slave, op->addr.buswidth, true))
+		return false;
+
+	if (op->dummy.nbytes &&
+	    spi_check_buswidth_req(slave, op->dummy.buswidth, true))
+		return false;
+
+	if (op->data.nbytes &&
+	    spi_check_buswidth_req(slave, 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
+ * @slave: the SPI device
+ * @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_slave *slave,
+			 const struct spi_mem_op *op)
+{
+	struct udevice *bus = slave->dev->parent;
+	struct dm_spi_ops *ops = spi_get_ops(bus);
+
+	if (ops->mem_ops && ops->mem_ops->supports_op)
+		return ops->mem_ops->supports_op(slave, op);
+
+	return spi_mem_default_supports_op(slave, op);
+}
+EXPORT_SYMBOL_GPL(spi_mem_supports_op);
+
+/**
+ * spi_mem_exec_op() - Execute a memory operation
+ * @slave: the SPI device
+ * @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_slave *slave, const struct spi_mem_op *op)
+{
+	bool rx_data = op->data.nbytes && (op->data.dir == SPI_MEM_DATA_IN);
+	bool tx_data = op->data.nbytes && (op->data.dir == SPI_MEM_DATA_OUT);
+	struct udevice *bus = slave->dev->parent;
+	struct dm_spi_ops *ops = spi_get_ops(bus);
+	unsigned int xfer_len, pos = 0;
+	u8 *tx_buf, *rx_buf = NULL;
+	int ret;
+	int i;
+
+	if (!spi_mem_supports_op(slave, op))
+		return -ENOTSUPP;
+
+	if (ops->mem_ops) {
+#ifndef __UBOOT__
+		/*
+		 * 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);
+#endif
+		ret = ops->mem_ops->exec_op(slave, op);
+#ifndef __UBOOT__
+		mutex_unlock(&ctlr->io_mutex);
+		mutex_unlock(&ctlr->bus_lock_mutex);
+
+		if (ctlr->auto_runtime_pm)
+			pm_runtime_put(ctlr->dev.parent);
+#endif
+
+		/*
+		 * 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;
+	}
+
+#ifndef __UBOOT__
+	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(slave, &msg);
+
+	kfree(tmpbuf);
+
+	if (ret)
+		return ret;
+
+	if (msg.actual_length != totalxferlen)
+		return -EIO;
+#else
+
+	/* U-Boot does not support parallel SPI data lanes */
+	if ((op->cmd.buswidth != 1) ||
+	    (op->addr.nbytes && op->addr.buswidth != 1) ||
+	    (op->dummy.nbytes && op->dummy.buswidth != 1) ||
+	    (op->data.nbytes && op->data.buswidth != 1)) {
+		printf("Dual/Quad raw SPI transfers not supported\n");
+		return -ENOTSUPP;
+	}
+
+	xfer_len = sizeof(op->cmd.opcode) + op->addr.nbytes +
+		   op->dummy.nbytes + op->data.nbytes;
+
+	/*
+	 * Allocate a buffer to transmit the CMD, ADDR cycles with kmalloc() so
+	 * we're guaranteed that this buffer is DMA-able, as required by the
+	 * SPI layer.
+	 */
+	tx_buf = kzalloc(xfer_len, GFP_KERNEL);
+	if (!tx_buf)
+		return -ENOMEM;
+
+	if (rx_data) {
+		rx_buf = kzalloc(xfer_len, GFP_KERNEL);
+		if (!rx_buf)
+			return -ENOMEM;
+	}
+
+	ret = spi_claim_bus(slave);
+	if (ret < 0)
+		return ret;
+
+	tx_buf[pos++] = op->cmd.opcode;
+
+	if (op->addr.nbytes) {
+		for (i = 0; i < op->addr.nbytes; i++)
+			tx_buf[pos + i] = op->addr.val >>
+					 (8 * (op->addr.nbytes - i - 1));
+
+		pos += op->addr.nbytes;
+	}
+
+	if (op->dummy.nbytes) {
+		memset(tx_buf + pos, 0xff, op->dummy.nbytes);
+		pos += op->dummy.nbytes;
+	}
+
+	if (tx_data)
+		memcpy(tx_buf + pos, op->data.buf.out, op->data.nbytes);
+
+	ret = spi_xfer(slave, xfer_len * 8, tx_buf, rx_buf,

Quoted 8 lines

drivers/spi/spi-mem.c new file +8 -0

+		memset(tx_buf + pos, 0xff, op->dummy.nbytes);
+		pos += op->dummy.nbytes;
+	}
+
+	if (tx_data)
+		memcpy(tx_buf + pos, op->data.buf.out, op->data.nbytes);
+
+	ret = spi_xfer(slave, xfer_len * 8, tx_buf, rx_buf,

drivers/spi/spi-mem.c line 383

Quoted 10 lines

drivers/spi/spi-mem.c new file +10 -0

+		       SPI_XFER_BEGIN | SPI_XFER_END);
+	spi_release_bus(slave);
+
+	for (i = 0; i < pos; i++)
+		debug("%02x ", tx_buf[i]);
+	debug("| [%dB %s] ",
+	      tx_data || rx_data ? op->data.nbytes : 0,
+	      tx_data || rx_data ? (tx_data ? "out" : "in") : "-");
+	for (; i < xfer_len; i++)
+		debug("%02x ", rx_buf[i]);  

		debug("%02x ", tx_data ? tx_buf[i] : rx_buf[i]);

I won't have much more time today. Will get back to this in ~2 weeks.

cmd/mtd.c new file +137 -0

+	if (mtd->dev)
+		printf(" [device: %s] [parent: %s] [driver: %s]",
+		       mtd->dev->name, mtd->dev->parent->name,
+		       mtd->dev->driver->name);
+
+	printf("\n");
+}
 
 
+static int do_mtd(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+{
+	struct mtd_info *mtd;
+	const char *cmd;
+	char *mtdname;
+	int ret;
+
+	/* All MTD commands need at least two arguments */
+	if (argc < 2)
+		return CMD_RET_USAGE;
+
+	/* Parse the command name and its optional suffixes */
+	cmd = argv[1];
+
+	/* List the MTD devices if that is what the user wants */
+	if (strcmp(cmd, "list") == 0)
+		return do_mtd_list();
+
+	/*
+	 * The remaining commands require also at least a device ID.
+	 * Check the selected device is valid. Ensure it is probed.
+	 */
+	if (argc < 3)
+		return CMD_RET_USAGE;
+
+	mtdname = argv[2];
+	mtd = get_mtd_device_nm(mtdname);
+	if (IS_ERR_OR_NULL(mtd)) {
+		mtd_probe_devices_dm();
+		mtd = get_mtd_device_nm(mtdname);
+		if (IS_ERR_OR_NULL(mtd)) {
+			printf("MTD device %s not found, ret %ld\n",
+			       mtdname, PTR_ERR(mtd));
+			return 1;
+		}
+	}
+
+	argc -= 3;
+	argv += 3;
+
+	/* Do the parsing */
+	if (!strncmp(cmd, "read", 4) || !strncmp(cmd, "dump", 4) ||
+	    !strncmp(cmd, "write", 5)) {
+		struct mtd_oob_ops io_op = {};
+		bool dump, read, raw, woob;
+		uint user_addr = 0;
+		uint nb_pages;
+		u8 *buf;
+		u64 off, len;
+		u32 oob_len;
+
+		dump = !strncmp(cmd, "dump", 4);
+		read = dump || !strncmp(cmd, "read", 4);
+		raw = strstr(cmd, ".raw");
+		woob = strstr(cmd, ".oob");
+
+		if (!dump) {
+			if (!argc)
+				return CMD_RET_USAGE;
+
+			user_addr = simple_strtoul(argv[0], NULL, 16);
+			argc--;
+			argv++;
+		}
+
+		off = argc > 0 ? simple_strtoul(argv[0], NULL, 16) : 0;
+		len = argc > 1 ? simple_strtoul(argv[1], NULL, 16) : mtd->writesize;
+		nb_pages = mtd_len_to_pages(mtd, len);
+		oob_len = woob ? nb_pages * mtd->oobsize : 0;
+
+		if ((u32)off % mtd->writesize) {
+			printf("Offset not aligned with a page (0x%x)\n",
+			       mtd->writesize);
+			return -EINVAL;
+		}
+
+		if ((u32)len % mtd->writesize) {
+			printf("Size not a multiple of a page (0x%x)\n",
+			       mtd->writesize);
+			return -EINVAL;
+		}
+
+		if (dump)
+			buf = kmalloc(len + oob_len, GFP_KERNEL);
+		else
+			buf = map_sysmem(user_addr, 0);
+
+		if (!buf) {
+			printf("Could not map/allocate the user buffer\n");
+			return -ENOMEM;
+		}
+
+		printf("%s %lldB (%d page(s)) at offset 0x%08llx%s%s\n",
+		       read ? "Reading" : "Writing", len, nb_pages, off,
+		       raw ? " [raw]" : "", woob ? " [oob]" : "");
+
+		io_op.mode = raw ? MTD_OPS_RAW : MTD_OPS_AUTO_OOB;
+		io_op.len = len;
+		io_op.ooblen = oob_len;
+		io_op.datbuf = buf;
+		io_op.oobbuf = woob ? &buf[len] : NULL;
+
+		if (read)
+			ret = mtd_read_oob(mtd, off, &io_op);
+		else
+			ret = mtd_write_oob(mtd, off, &io_op);
+
+		if (ret) {
+			printf("%s on %s failed with error %d\n",
+			       read ? "Read" : "Write", mtd->name, ret);
+			return ret;
+		}
+
+		if (dump) {
+			uint page;
+
+			for (page = 0; page < nb_pages; page++) {
+				u64 data_off = page * mtd->writesize;
+				printf("\nDump %d data bytes from 0x%08llx:\n",
+				       mtd->writesize, data_off);
+				mtd_dump_buf(buf, mtd->writesize, data_off);
+
+				if (woob) {
+					u64 oob_off = page * mtd->oobsize;
+					printf("Dump %d OOB bytes from page at 0x%08llx:\n",
+					       mtd->oobsize, data_off);
+					mtd_dump_buf(&buf[len + oob_off],
+						     mtd->oobsize, 0);
+				}
+			}

cmd/mtd.c new file +40 -0

+			printf("Size not a multiple of a block (0x%x)\n",
+			       mtd->erasesize);
+			return -EINVAL;
+		}
+
+		erase_op.mtd = mtd;
+		erase_op.addr = off;
+		erase_op.len = len;
+		erase_op.scrub = scrub;
+
+		ret = mtd_erase(mtd, &erase_op);
+	} else {
+		return CMD_RET_USAGE;
+	}
+
+	return ret;
+}
+
+static char mtd_help_text[] =
+#ifdef CONFIG_SYS_LONGHELP
+	"- generic operations on memory technology devices\n\n"
+	"mtd list\n"
+	"mtd read[.raw][.oob]           <name> <addr> [<off> [<size>]]\n"
+	"mtd dump[.raw][.oob]           <name>        [<off> [<size>]]\n"
+	"mtd write[.raw][.oob]          <name> <addr> [<off> [<size>]]\n"
+	"mtd erase[.chip][.dontskipbad] <name>        [<off> [<size>]]\n"
+	"\n"
+	"With:\n"
+	"\t<name>: NAND partition/chip name\n"
+	"\t<addr>: user address from/to which data will be retrieved/stored\n"
+	"\t<off>: offset in <name> in bytes (default: start of the part)\n"
+	"\t\t* must be block-aligned for erase\n"
+	"\t\t* must be page-aligned otherwise\n"
+	"\t<size>: length of the operation in bytes\n"
+	"\t\t* must be a multiple of a block for erase (default: 1 block)\n"
+	"\t\t* must be a multiple of a page otherwise (default: 1 page)\n"
+#endif
+	"";
+
+U_BOOT_CMD(mtd, 10, 1, do_mtd, "MTD utils", mtd_help_text);

cmd/mtd.c line 40

Quoted 4 lines · On Wed, 11 Jul 2018 17:25:28 +0200 Miquel Raynal <[email protected]>…

cmd/mtd.c new file +4 -0

+
+static void mtd_show_device(struct mtd_info *mtd)
+{
+	printf("* %s", mtd->name);  

Printing the device type might be interesting? And maybe also the size, writesize, oobsize and erasesize?

Quoted 8 lines

cmd/mtd.c new file +7 -0

+	if (mtd->dev)
+		printf(" [device: %s] [parent: %s] [driver: %s]",
+		       mtd->dev->name, mtd->dev->parent->name,
+		       mtd->dev->driver->name);
+
+	printf("\n");
+}  

cmd/mtd.c line 229

Quoted 3 lines

cmd/mtd.c new file +3 -0

+	} else if (!strcmp(cmd, "erase")) {
+		bool scrub = strstr(cmd, ".dontskipbad");
+		bool full_erase = strstr(cmd, ".chip");  

Again, I think .chip extension is not needed. Just consider a full erase is requested when offset and length are not provided. Plus, chip is misleading since I guess mtd partitions are also exposed as mtd devices, and could thus be erased with the .chip extension as well.

cmd/mtd.c line 234

Quoted 6 lines

cmd/mtd.c new file +5 -0

+		struct erase_info erase_op = {};
+		u64 off, len;
+
+		off = argc > 0 ? simple_strtoul(argv[0], NULL, 16) : 0;
+		len = argc > 1 ? simple_strtoul(argv[1], NULL, 16) : mtd->erasesize;  

Hm. Are we sure we want the default to be 1 eraseblock? Shouldn't we consider that missing size means "erase up to the MTD device end". Same goes for the read/write accesses, but maybe not for dump where dumping a single page makes more sense.

cmd/mtd.c line 241

Quoted 8 lines

cmd/mtd.c new file +7 -0

+
+		if (full_erase) {
+			off = 0;
+			len = mtd->size;
+		}
+
+		if ((u32)off % mtd->erasesize) {  

Sounds dangerous. We have 8GB NANDs on sunxi platforms...

cmd/mtd.c line 247

Quoted 2 lines

cmd/mtd.c new file +2 -0

+
+		if ((u32)len % mtd->erasesize) {  

Same here. I guess there's a do_div() in uboot.

Quoted 9 lines

cmd/mtd.c line 241

Quoted 7 lines

cmd/mtd.c new file +7 -0

+
+		if (full_erase) {
+			off = 0;
+			len = mtd->size;
+		}
+
+		if ((u32)off % mtd->erasesize) {    

Sounds dangerous. We have 8GB NANDs on sunxi platforms...

[...]

Quoted 4 lines

cmd/mtd.c line 247

Quoted 2 lines

cmd/mtd.c new file +2 -0

+
+		if ((u32)len % mtd->erasesize) {    

Same here. I guess there's a do_div() in uboot.

In both cases I take the less significant bytes of a 64-bit value. The modulo operation is safe as long as mtd->erasesize is a 32-bit value too. I don't think there is any danger?

From: Boris Brezillon <[email protected]>

Add bindings for SPI NAND chips.

Signed-off-by Boris Brezillon <[email protected]>

Signed-off-by Miquel Raynal <[email protected]>

---
 doc/device-tree-bindings/mtd/spi-nand.txt | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)
 create mode 100644 doc/device-tree-bindings/mtd/spi-nand.txt

doc/device-tree-bindings/mtd/spi-nand.txt new file +17 -0

new file mode 100644
index 0000000000..d55f80196c
@@ -0,0 +1,27 @@
+SPI NAND flash
+
+Required properties:
+- compatible: should be "spi-nand"
+- reg: should encode the chip-select line used to access the NAND chip
+
+Optional properties
+- spi-max-frequency: maximum frequency of the SPI bus the chip can operate at.
+		     This should encode board limitations (i.e. max freq can't
+		     be achieved due to crosstalk on IO lines).
+		     When unspecified, the driver assumes the chip can run at
+		     the max frequency defined in the spec (information
+		     extracted chip detection time).
+- spi-tx-bus-width: The bus width (number of data wires) that is used for MOSI.
+		    Only encodes the board constraints (i.e. when not all IO
+		    signals are routed on the board). Device constraints are
+		    extracted when detecting the chip, and controller

Quoted 38 lines · On Wed, 11 Jul 2018 17:25:29 +0200 Miquel Raynal <[email protected]>…

From: Boris Brezillon <[email protected]>

Add bindings for SPI NAND chips.

Signed-off-by Boris Brezillon <[email protected]>

Signed-off-by Miquel Raynal <[email protected]>

---
 doc/device-tree-bindings/mtd/spi-nand.txt | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)
 create mode 100644 doc/device-tree-bindings/mtd/spi-nand.txt

doc/device-tree-bindings/mtd/spi-nand.txt new file +21 -0

new file mode 100644
index 0000000000..d55f80196c
@@ -0,0 +1,27 @@
+SPI NAND flash
+
+Required properties:
+- compatible: should be "spi-nand"
+- reg: should encode the chip-select line used to access the NAND chip
+
+Optional properties
+- spi-max-frequency: maximum frequency of the SPI bus the chip can operate at.
+		     This should encode board limitations (i.e. max freq can't
+		     be achieved due to crosstalk on IO lines).
+		     When unspecified, the driver assumes the chip can run at
+		     the max frequency defined in the spec (information
+		     extracted chip detection time).
+- spi-tx-bus-width: The bus width (number of data wires) that is used for MOSI.
+		    Only encodes the board constraints (i.e. when not all IO
+		    signals are routed on the board). Device constraints are
+		    extracted when detecting the chip, and controller
+		    constraints are exposed by the SPI mem controller. If this
+		    property is missing that means no constraint at the board
+		    level.
+- spi-rx-bus-width: The bus width (number of data wires) that is used for MISO.

doc/device-tree-bindings/mtd/spi-nand.txt line 20

Quoted 6 lines

doc/device-tree-bindings/mtd/spi-nand.txt new file +6 -0

+		    Only encodes the board constraints (i.e. when not all IO
+		    signals are routed on the board). Device constraints are
+		    extracted when detecting the chip, and controller
+		    constraints are exposed by the SPI mem controller. If this
+		    property is missing that means no constraint at the board
+		    level.  

This section has been dropped from the Linux doc.