linux-mtd.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] Merge m25p80 into spi-nor
@ 2019-04-09 16:26 Vignesh Raghavendra
  2019-04-09 16:26 ` [PATCH 1/2] mtd: spi-nor: Move m25p80 code in spi-nor.c Vignesh Raghavendra
  2019-04-09 16:26 ` [PATCH 2/2] mtd: spi-nor: Rework hwcaps selection for the spi-mem case Vignesh Raghavendra
  0 siblings, 2 replies; 6+ messages in thread
From: Vignesh Raghavendra @ 2019-04-09 16:26 UTC (permalink / raw)
  To: Boris Brezillon, Marek Vasut, Richard Weinberger, Tudor Ambarus
  Cc: Yogesh Narayan Gaur, Vignesh Raghavendra, linux-kernel,
	Frieder Schrempf, linux-mtd, Miquel Raynal

This is repost of patch 6 and 7 split from from Boris Brezillon's X-X-X
mode support series[1]

Background from cover letter for RFC[1]:
m25p80 is just a simple SPI NOR controller driver (a wrapper around the
SPI mem API). Not only it shouldn't be named after a specific SPI NOR
chip, but it also doesn't deserve a specific driver IMO, especially if
the end goal is to get rid of SPI NOR controller drivers found in
drivers/mtd/spi-nor/ and replace them by SPI mem drivers (which would
be placed in drivers/spi/). With this solution, we declare the SPI NOR
driver as a spi_mem_driver, just like the SPI NAND layer is declared as
a spi_mem driver (patch 1/2).
This solution also allows us to check at a fined-grain level (thanks to
the spi_mem_supports_op() function) which operations are supported and
which ones are not, while the original m25p80 logic was basing this
decision on the SPI_{RX,TX}_{DUAL,QUAD,OCTO} flags only (patch 2/2).

[1] https://patchwork.ozlabs.org/cover/982926/

Tested on TI' DRA7xx EVM with TI QSPI controller (a spi-mem driver) with
DMA (s25fl256) flash. I don't see any performance regression due to
bounce buffer copy introduced by this series

Boris Brezillon (2):
  mtd: spi-nor: Move m25p80 code in spi-nor.c
  mtd: spi-nor: Rework hwcaps selection for the spi-mem case

 drivers/mtd/devices/Kconfig   |  18 -
 drivers/mtd/devices/Makefile  |   1 -
 drivers/mtd/devices/m25p80.c  | 351 ----------------
 drivers/mtd/spi-nor/Kconfig   |   2 +
 drivers/mtd/spi-nor/spi-nor.c | 771 ++++++++++++++++++++++++++++++++--
 include/linux/mtd/spi-nor.h   |  22 +
 6 files changed, 759 insertions(+), 406 deletions(-)
 delete mode 100644 drivers/mtd/devices/m25p80.c

-- 
2.21.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* [PATCH 1/2] mtd: spi-nor: Move m25p80 code in spi-nor.c
  2019-04-09 16:26 [PATCH 0/2] Merge m25p80 into spi-nor Vignesh Raghavendra
@ 2019-04-09 16:26 ` Vignesh Raghavendra
  2019-04-15  8:13   ` Tudor.Ambarus
  2019-04-09 16:26 ` [PATCH 2/2] mtd: spi-nor: Rework hwcaps selection for the spi-mem case Vignesh Raghavendra
  1 sibling, 1 reply; 6+ messages in thread
From: Vignesh Raghavendra @ 2019-04-09 16:26 UTC (permalink / raw)
  To: Boris Brezillon, Marek Vasut, Richard Weinberger, Tudor Ambarus
  Cc: Yogesh Narayan Gaur, Vignesh Raghavendra, linux-kernel,
	Frieder Schrempf, linux-mtd, Miquel Raynal

From: Boris Brezillon <boris.brezillon@bootlin.com>

The m25p80 driver is actually a generic wrapper around the spi-mem
layer. Not only the driver name is misleading, but we'd expect such a
common logic to be directly available in the core. Another reason for
moving this code is that SPI NOR controller drivers should
progressively be replaced by SPI controller drivers implementing the
spi_mem_ops interface, and when the conversion is done, we should have
a single spi-nor driver directly interfacing with the spi-mem layer.

While moving the code we also fix a longstanding issue when
non-DMA-able buffers are passed by the MTD layer.

Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
[vigneshr@ti.com: use devm_kmalloc() for bounce buffer allocation]
Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com>
---
Changes from RFC:
* Use devm_kmalloc() for bounce buffer allocation as it supports cache
  line aligned buffers now
* Rebase onto latest next
* Fix build uses when spi-nor is built as a module

 drivers/mtd/devices/Kconfig   |  18 -
 drivers/mtd/devices/Makefile  |   1 -
 drivers/mtd/devices/m25p80.c  | 351 ------------------
 drivers/mtd/spi-nor/Kconfig   |   2 +
 drivers/mtd/spi-nor/spi-nor.c | 659 ++++++++++++++++++++++++++++++++--
 include/linux/mtd/spi-nor.h   |   8 +
 6 files changed, 643 insertions(+), 396 deletions(-)
 delete mode 100644 drivers/mtd/devices/m25p80.c

diff --git a/drivers/mtd/devices/Kconfig b/drivers/mtd/devices/Kconfig
index 7fcdaf6c279d..280ea90ea2d1 100644
--- a/drivers/mtd/devices/Kconfig
+++ b/drivers/mtd/devices/Kconfig
@@ -78,24 +78,6 @@ config MTD_DATAFLASH_OTP
 	  other key product data.  The second half is programmed with a
 	  unique-to-each-chip bit pattern at the factory.
 
-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,
-	  Spansion S25SL, SST 25VF, ST M25P, and Winbond W25X.  Other chips
-	  are supported as well.  See the driver source for the current list,
-	  or to add other chips.
-
-	  Note that the original DataFlash chips (AT45 series, not AT26DF),
-	  need an entirely different driver.
-
-	  Set up your spi devices with the right board-specific platform data,
-	  if you want to specify device partitioning or to use a device which
-	  doesn't support the JEDEC ID instruction.
-
 config MTD_MCHP23K256
 	tristate "Microchip 23K256 SRAM"
 	depends on SPI_MASTER
diff --git a/drivers/mtd/devices/Makefile b/drivers/mtd/devices/Makefile
index 94895eab3066..991c8d12c016 100644
--- a/drivers/mtd/devices/Makefile
+++ b/drivers/mtd/devices/Makefile
@@ -12,7 +12,6 @@ obj-$(CONFIG_MTD_MTDRAM)	+= mtdram.o
 obj-$(CONFIG_MTD_LART)		+= lart.o
 obj-$(CONFIG_MTD_BLOCK2MTD)	+= block2mtd.o
 obj-$(CONFIG_MTD_DATAFLASH)	+= mtd_dataflash.o
-obj-$(CONFIG_MTD_M25P80)	+= m25p80.o
 obj-$(CONFIG_MTD_MCHP23K256)	+= mchp23k256.o
 obj-$(CONFIG_MTD_SPEAR_SMI)	+= spear_smi.o
 obj-$(CONFIG_MTD_SST25L)	+= sst25l.o
diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
deleted file mode 100644
index 651bab6d4e31..000000000000
--- a/drivers/mtd/devices/m25p80.c
+++ /dev/null
@@ -1,351 +0,0 @@
-/*
- * MTD SPI driver for ST M25Pxx (and similar) serial flash chips
- *
- * Author: Mike Lavender, mike@steroidmicros.com
- *
- * Copyright (c) 2005, Intec Automation Inc.
- *
- * Some parts are based on lart.c by Abraham Van Der Merwe
- *
- * Cleaned up and generalized based on mtd_dataflash.c
- *
- * This code is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- *
- */
-
-#include <linux/err.h>
-#include <linux/errno.h>
-#include <linux/module.h>
-#include <linux/device.h>
-
-#include <linux/mtd/mtd.h>
-#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>
-
-struct m25p {
-	struct spi_mem		*spimem;
-	struct spi_nor		spi_nor;
-};
-
-static int m25p80_read_reg(struct spi_nor *nor, u8 code, u8 *val, int len)
-{
-	struct m25p *flash = nor->priv;
-	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, NULL, 1));
-	void *scratchbuf;
-	int ret;
-
-	scratchbuf = kmalloc(len, GFP_KERNEL);
-	if (!scratchbuf)
-		return -ENOMEM;
-
-	op.data.buf.in = scratchbuf;
-	ret = spi_mem_exec_op(flash->spimem, &op);
-	if (ret < 0)
-		dev_err(&flash->spimem->spi->dev, "error %d reading %x\n", ret,
-			code);
-	else
-		memcpy(val, scratchbuf, len);
-
-	kfree(scratchbuf);
-
-	return ret;
-}
-
-static int m25p80_write_reg(struct spi_nor *nor, u8 opcode, u8 *buf, int len)
-{
-	struct m25p *flash = nor->priv;
-	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, NULL, 1));
-	void *scratchbuf;
-	int ret;
-
-	scratchbuf = kmemdup(buf, len, GFP_KERNEL);
-	if (!scratchbuf)
-		return -ENOMEM;
-
-	op.data.buf.out = scratchbuf;
-	ret = spi_mem_exec_op(flash->spimem, &op);
-	kfree(scratchbuf);
-
-	return ret;
-}
-
-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_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_NO_DUMMY,
-				   SPI_MEM_OP_DATA_OUT(len, buf, 1));
-	int ret;
-
-	/* get transfer protocols. */
-	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.data.buswidth = spi_nor_get_protocol_data_nbits(nor->write_proto);
-
-	if (nor->program_opcode == SPINOR_OP_AAI_WP && nor->sst_write_second)
-		op.addr.nbytes = 0;
-
-	ret = spi_mem_adjust_op_size(flash->spimem, &op);
-	if (ret)
-		return ret;
-	op.data.nbytes = len < op.data.nbytes ? len : op.data.nbytes;
-
-	ret = spi_mem_exec_op(flash->spimem, &op);
-	if (ret)
-		return ret;
-
-	return op.data.nbytes;
-}
-
-/*
- * Read an address range from the nor chip.  The address range
- * may be any size provided it is within the physical boundaries.
- */
-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_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. */
-	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 */
-	op.dummy.nbytes = (nor->read_dummy * op.dummy.buswidth) / 8;
-
-	while (remaining) {
-		op.data.nbytes = remaining < UINT_MAX ? remaining : UINT_MAX;
-		ret = spi_mem_adjust_op_size(flash->spimem, &op);
-		if (ret)
-			return ret;
-
-		ret = spi_mem_exec_op(flash->spimem, &op);
-		if (ret)
-			return ret;
-
-		op.addr.val += op.data.nbytes;
-		remaining -= op.data.nbytes;
-		op.data.buf.in += op.data.nbytes;
-	}
-
-	return len;
-}
-
-/*
- * board specific setup should have ensured the SPI clock used here
- * 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_mem *spimem)
-{
-	struct spi_device *spi = spimem->spi;
-	struct flash_platform_data	*data;
-	struct m25p *flash;
-	struct spi_nor *nor;
-	struct spi_nor_hwcaps hwcaps = {
-		.mask = SNOR_HWCAPS_READ |
-			SNOR_HWCAPS_READ_FAST |
-			SNOR_HWCAPS_PP,
-	};
-	char *flash_name;
-	int ret;
-
-	data = dev_get_platdata(&spimem->spi->dev);
-
-	flash = devm_kzalloc(&spimem->spi->dev, sizeof(*flash), GFP_KERNEL);
-	if (!flash)
-		return -ENOMEM;
-
-	nor = &flash->spi_nor;
-
-	/* install the hooks */
-	nor->read = m25p80_read;
-	nor->write = m25p80_write;
-	nor->write_reg = m25p80_write_reg;
-	nor->read_reg = m25p80_read_reg;
-
-	nor->dev = &spimem->spi->dev;
-	spi_nor_set_flash_node(nor, spi->dev.of_node);
-	nor->priv = flash;
-
-	spi_mem_set_drvdata(spimem, flash);
-	flash->spimem = spimem;
-
-	if (spi->mode & SPI_RX_OCTAL) {
-		hwcaps.mask |= SNOR_HWCAPS_READ_1_1_8;
-
-		if (spi->mode & SPI_TX_OCTAL)
-			hwcaps.mask |= (SNOR_HWCAPS_READ_1_8_8 |
-					SNOR_HWCAPS_PP_1_1_8 |
-					SNOR_HWCAPS_PP_1_8_8);
-	} else if (spi->mode & SPI_RX_QUAD) {
-		hwcaps.mask |= SNOR_HWCAPS_READ_1_1_4;
-
-		if (spi->mode & SPI_TX_QUAD)
-			hwcaps.mask |= (SNOR_HWCAPS_READ_1_4_4 |
-					SNOR_HWCAPS_PP_1_1_4 |
-					SNOR_HWCAPS_PP_1_4_4);
-	} else if (spi->mode & SPI_RX_DUAL) {
-		hwcaps.mask |= SNOR_HWCAPS_READ_1_1_2;
-
-		if (spi->mode & SPI_TX_DUAL)
-			hwcaps.mask |= SNOR_HWCAPS_READ_1_2_2;
-	}
-
-	if (data && data->name)
-		nor->mtd.name = data->name;
-
-	if (!nor->mtd.name)
-		nor->mtd.name = spi_mem_get_name(spimem);
-
-	/* For some (historical?) reason many platforms provide two different
-	 * names in flash_platform_data: "name" and "type". Quite often name is
-	 * set to "m25p80" and then "type" provides a real chip name.
-	 * If that's the case, respect "type" and ignore a "name".
-	 */
-	if (data && data->type)
-		flash_name = data->type;
-	else if (!strcmp(spi->modalias, "spi-nor"))
-		flash_name = NULL; /* auto-detect */
-	else
-		flash_name = spi->modalias;
-
-	ret = spi_nor_scan(nor, flash_name, &hwcaps);
-	if (ret)
-		return ret;
-
-	return mtd_device_register(&nor->mtd, data ? data->parts : NULL,
-				   data ? data->nr_parts : 0);
-}
-
-
-static int m25p_remove(struct spi_mem *spimem)
-{
-	struct m25p	*flash = spi_mem_get_drvdata(spimem);
-
-	spi_nor_restore(&flash->spi_nor);
-
-	/* Clean up MTD stuff. */
-	return mtd_device_unregister(&flash->spi_nor.mtd);
-}
-
-static void m25p_shutdown(struct spi_mem *spimem)
-{
-	struct m25p *flash = spi_mem_get_drvdata(spimem);
-
-	spi_nor_restore(&flash->spi_nor);
-}
-/*
- * Do NOT add to this array without reading the following:
- *
- * Historically, many flash devices are bound to this driver by their name. But
- * since most of these flash are compatible to some extent, and their
- * differences can often be differentiated by the JEDEC read-ID command, we
- * encourage new users to add support to the spi-nor library, and simply bind
- * against a generic string here (e.g., "jedec,spi-nor").
- *
- * Many flash names are kept here in this list (as well as in spi-nor.c) to
- * keep them available as module aliases for existing platforms.
- */
-static const struct spi_device_id m25p_ids[] = {
-	/*
-	 * Allow non-DT platform devices to bind to the "spi-nor" modalias, and
-	 * hack around the fact that the SPI core does not provide uevent
-	 * matching for .of_match_table
-	 */
-	{"spi-nor"},
-
-	/*
-	 * Entries not used in DTs that should be safe to drop after replacing
-	 * them with "spi-nor" in platform data.
-	 */
-	{"s25sl064a"},	{"w25x16"},	{"m25p10"},	{"m25px64"},
-
-	/*
-	 * Entries that were used in DTs without "jedec,spi-nor" fallback and
-	 * should be kept for backward compatibility.
-	 */
-	{"at25df321a"},	{"at25df641"},	{"at26df081a"},
-	{"mx25l4005a"},	{"mx25l1606e"},	{"mx25l6405d"},	{"mx25l12805d"},
-	{"mx25l25635e"},{"mx66l51235l"},
-	{"n25q064"},	{"n25q128a11"},	{"n25q128a13"},	{"n25q512a"},
-	{"s25fl256s1"},	{"s25fl512s"},	{"s25sl12801"},	{"s25fl008k"},
-	{"s25fl064k"},
-	{"sst25vf040b"},{"sst25vf016b"},{"sst25vf032b"},{"sst25wf040"},
-	{"m25p40"},	{"m25p80"},	{"m25p16"},	{"m25p32"},
-	{"m25p64"},	{"m25p128"},
-	{"w25x80"},	{"w25x32"},	{"w25q32"},	{"w25q32dw"},
-	{"w25q80bl"},	{"w25q128"},	{"w25q256"},
-
-	/* Flashes that can't be detected using JEDEC */
-	{"m25p05-nonjedec"},	{"m25p10-nonjedec"},	{"m25p20-nonjedec"},
-	{"m25p40-nonjedec"},	{"m25p80-nonjedec"},	{"m25p16-nonjedec"},
-	{"m25p32-nonjedec"},	{"m25p64-nonjedec"},	{"m25p128-nonjedec"},
-
-	/* Everspin MRAMs (non-JEDEC) */
-	{ "mr25h128" }, /* 128 Kib, 40 MHz */
-	{ "mr25h256" }, /* 256 Kib, 40 MHz */
-	{ "mr25h10" },  /*   1 Mib, 40 MHz */
-	{ "mr25h40" },  /*   4 Mib, 40 MHz */
-
-	{ },
-};
-MODULE_DEVICE_TABLE(spi, m25p_ids);
-
-static const struct of_device_id m25p_of_table[] = {
-	/*
-	 * Generic compatibility for SPI NOR that can be identified by the
-	 * JEDEC READ ID opcode (0x9F). Use this, if possible.
-	 */
-	{ .compatible = "jedec,spi-nor" },
-	{}
-};
-MODULE_DEVICE_TABLE(of, m25p_of_table);
-
-static struct spi_mem_driver m25p80_driver = {
-	.spidrv = {
-		.driver = {
-			.name	= "m25p80",
-			.of_match_table = m25p_of_table,
-		},
-		.id_table	= m25p_ids,
-	},
-	.probe	= m25p_probe,
-	.remove	= m25p_remove,
-	.shutdown	= m25p_shutdown,
-
-	/* REVISIT: many of these chips have deep power-down modes, which
-	 * should clearly be entered on suspend() to minimize power use.
-	 * And also when they're otherwise idle...
-	 */
-};
-
-module_spi_mem_driver(m25p80_driver);
-
-MODULE_LICENSE("GPL");
-MODULE_AUTHOR("Mike Lavender");
-MODULE_DESCRIPTION("MTD SPI driver for ST M25Pxx flash chips");
diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
index dab986691267..0c1b7851621f 100644
--- a/drivers/mtd/spi-nor/Kconfig
+++ b/drivers/mtd/spi-nor/Kconfig
@@ -1,6 +1,8 @@
 menuconfig MTD_SPI_NOR
 	tristate "SPI-NOR device support"
 	depends on MTD
+	depends on MTD && SPI_MASTER
+	select SPI_MEM
 	help
 	  This is the framework for the SPI NOR which can be used by the SPI
 	  device drivers and the SPI-NOR device driver.
diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 73172d7f512b..03c8c346c9ae 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -19,6 +19,7 @@
 
 #include <linux/mtd/mtd.h>
 #include <linux/of_platform.h>
+#include <linux/sched/task_stack.h>
 #include <linux/spi/flash.h>
 #include <linux/mtd/spi-nor.h>
 
@@ -288,6 +289,174 @@ struct flash_info {
 
 #define JEDEC_MFR(info)	((info)->id[0])
 
+static int spi_nor_exec_op(struct spi_nor *nor, struct spi_mem_op *op,
+			   u64 *addr, void *buf, unsigned int len)
+{
+	int ret;
+	bool usebouncebuf;
+
+	if (!op || (len && !buf))
+		return -EINVAL;
+
+	if (op->addr.nbytes && addr)
+		op->addr.val = *addr;
+
+	op->data.nbytes = len;
+
+	if (object_is_on_stack(buf) || !virt_addr_valid(buf))
+		usebouncebuf = true;
+	if (len && usebouncebuf) {
+		if (len > nor->bouncebuf_size)
+			return -ENOTSUPP;
+
+		if (op->data.dir == SPI_MEM_DATA_IN) {
+			op->data.buf.in = nor->bouncebuf;
+		} else {
+			op->data.buf.out = nor->bouncebuf;
+			memcpy(nor->bouncebuf, buf, len);
+		}
+	} else {
+		op->data.buf.out = buf;
+	}
+
+	ret = spi_mem_exec_op(nor->spimem, op);
+	if (ret)
+		return ret;
+
+	if (usebouncebuf && len && op->data.dir == SPI_MEM_DATA_IN)
+		memcpy(buf, nor->bouncebuf, len);
+
+	return 0;
+}
+
+static int spi_nor_data_op(struct spi_nor *nor, struct spi_mem_op *op,
+			   void *buf, unsigned int len)
+{
+	return spi_nor_exec_op(nor, op, NULL, buf, len);
+}
+
+static int spi_nor_nodata_op(struct spi_nor *nor, struct spi_mem_op *op)
+{
+	return spi_nor_exec_op(nor, op, NULL, NULL, 0);
+}
+
+static ssize_t spi_nor_spimem_read_data(struct spi_nor *nor, loff_t ofs,
+					size_t len, u8 *buf)
+{
+	struct spi_mem_op op = SPI_MEM_OP(SPI_MEM_OP_CMD(nor->read_opcode, 1),
+					  SPI_MEM_OP_ADDR(nor->addr_width, ofs,
+							  1),
+					  SPI_MEM_OP_DUMMY(nor->read_dummy, 1),
+					  SPI_MEM_OP_DATA_IN(len, buf, 1));
+	bool usebouncebuf = false;
+	size_t remaining = len;
+	int ret;
+
+	if (object_is_on_stack(buf) || !virt_addr_valid(buf)) {
+		usebouncebuf = true;
+		op.data.buf.in = nor->bouncebuf;
+	}
+
+	/* get transfer protocols. */
+	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 */
+	op.dummy.nbytes = (nor->read_dummy * op.dummy.buswidth) / 8;
+
+	while (remaining) {
+		op.data.nbytes = remaining < UINT_MAX ? remaining : UINT_MAX;
+		if (!usebouncebuf)
+			op.data.buf.out = buf;
+		else if (op.data.nbytes > nor->bouncebuf_size)
+			op.data.nbytes = nor->bouncebuf_size;
+
+		ret = spi_mem_adjust_op_size(nor->spimem, &op);
+		if (ret)
+			return ret;
+
+		ret = spi_mem_exec_op(nor->spimem, &op);
+		if (ret)
+			return ret;
+
+		if (usebouncebuf)
+			memcpy(buf, nor->bouncebuf, op.data.nbytes);
+
+		op.addr.val += op.data.nbytes;
+		remaining -= op.data.nbytes;
+		buf += op.data.nbytes;
+	}
+
+	return len;
+}
+
+static ssize_t spi_nor_read_data(struct spi_nor *nor, loff_t ofs, size_t len,
+				 u8 *buf)
+{
+	if (nor->spimem)
+		return spi_nor_spimem_read_data(nor, ofs, len, buf);
+
+	return nor->read(nor, ofs, len, buf);
+}
+
+static ssize_t spi_nor_spimem_write_data(struct spi_nor *nor, loff_t ofs,
+					 size_t len, const u8 *buf)
+{
+	struct spi_mem_op op = SPI_MEM_OP(SPI_MEM_OP_CMD(nor->program_opcode,
+							 1),
+					  SPI_MEM_OP_ADDR(nor->addr_width, ofs,
+							  1),
+					  SPI_MEM_OP_NO_DUMMY,
+					  SPI_MEM_OP_DATA_OUT(len, NULL, 1));
+	bool usebouncebuf = false;
+	int ret;
+
+	if (object_is_on_stack(buf) || !virt_addr_valid(buf)) {
+		usebouncebuf = true;
+		op.data.buf.out = nor->bouncebuf;
+	}
+
+	/* get transfer protocols. */
+	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.data.buswidth = spi_nor_get_protocol_data_nbits(nor->write_proto);
+
+	if (nor->program_opcode == SPINOR_OP_AAI_WP && nor->sst_write_second)
+		op.addr.nbytes = 0;
+
+	op.data.nbytes = len < UINT_MAX ? len : UINT_MAX;
+
+	if (!usebouncebuf) {
+		op.data.buf.out = buf;
+	} else {
+		if (op.data.nbytes > nor->bouncebuf_size)
+			op.data.nbytes = nor->bouncebuf_size;
+
+		memcpy(nor->bouncebuf, buf, op.data.nbytes);
+	}
+
+	ret = spi_mem_adjust_op_size(nor->spimem, &op);
+	if (ret)
+		return ret;
+
+	ret = spi_mem_exec_op(nor->spimem, &op);
+	if (ret)
+		return ret;
+
+	return op.data.nbytes;
+}
+
+static ssize_t spi_nor_write_data(struct spi_nor *nor, loff_t ofs, size_t len,
+				  const u8 *buf)
+{
+	if (nor->spimem)
+		return spi_nor_spimem_write_data(nor, ofs, len, buf);
+
+	return nor->write(nor, ofs, len, buf);
+}
+
 /*
  * Read the status register, returning its value in the location
  * Return the status register value.
@@ -298,7 +467,18 @@ static int read_sr(struct spi_nor *nor)
 	int ret;
 	u8 val;
 
-	ret = nor->read_reg(nor, SPINOR_OP_RDSR, &val, 1);
+	if (nor->spimem) {
+		struct spi_mem_op op = SPI_MEM_OP(
+					SPI_MEM_OP_CMD(SPINOR_OP_RDSR, 1),
+					SPI_MEM_OP_NO_ADDR,
+					SPI_MEM_OP_NO_DUMMY,
+					SPI_MEM_OP_DATA_IN(0, NULL, 1));
+
+		ret = spi_nor_data_op(nor, &op, &val, 1);
+	} else {
+		ret = nor->read_reg(nor, SPINOR_OP_RDSR, &val, 1);
+	}
+
 	if (ret < 0) {
 		pr_err("error %d reading SR\n", (int) ret);
 		return ret;
@@ -317,7 +497,18 @@ static int read_fsr(struct spi_nor *nor)
 	int ret;
 	u8 val;
 
-	ret = nor->read_reg(nor, SPINOR_OP_RDFSR, &val, 1);
+	if (nor->spimem) {
+		struct spi_mem_op op = SPI_MEM_OP(
+					SPI_MEM_OP_CMD(SPINOR_OP_RDFSR, 1),
+					SPI_MEM_OP_NO_ADDR,
+					SPI_MEM_OP_NO_DUMMY,
+					SPI_MEM_OP_DATA_IN(0, NULL, 1));
+
+		ret = spi_nor_data_op(nor, &op, &val, 1);
+	} else {
+		ret = nor->read_reg(nor, SPINOR_OP_RDFSR, &val, 1);
+	}
+
 	if (ret < 0) {
 		pr_err("error %d reading FSR\n", ret);
 		return ret;
@@ -336,7 +527,18 @@ static int read_cr(struct spi_nor *nor)
 	int ret;
 	u8 val;
 
-	ret = nor->read_reg(nor, SPINOR_OP_RDCR, &val, 1);
+	if (nor->spimem) {
+		struct spi_mem_op op = SPI_MEM_OP(
+					SPI_MEM_OP_CMD(SPINOR_OP_RDCR, 1),
+					SPI_MEM_OP_NO_ADDR,
+					SPI_MEM_OP_NO_DUMMY,
+					SPI_MEM_OP_DATA_IN(0, NULL, 1));
+
+		ret = spi_nor_data_op(nor, &op, &val, 1);
+	} else {
+		ret = nor->read_reg(nor, SPINOR_OP_RDCR, &val, 1);
+	}
+
 	if (ret < 0) {
 		dev_err(nor->dev, "error %d reading CR\n", ret);
 		return ret;
@@ -351,6 +553,16 @@ static int read_cr(struct spi_nor *nor)
  */
 static int write_sr(struct spi_nor *nor, u8 val)
 {
+	if (nor->spimem) {
+		struct spi_mem_op op = SPI_MEM_OP(
+					SPI_MEM_OP_CMD(SPINOR_OP_WRSR, 1),
+					SPI_MEM_OP_NO_ADDR,
+					SPI_MEM_OP_NO_DUMMY,
+					SPI_MEM_OP_DATA_IN(1, NULL, 1));
+
+		return spi_nor_data_op(nor, &op, &val, 1);
+	}
+
 	nor->cmd_buf[0] = val;
 	return nor->write_reg(nor, SPINOR_OP_WRSR, nor->cmd_buf, 1);
 }
@@ -361,6 +573,16 @@ static int write_sr(struct spi_nor *nor, u8 val)
  */
 static int write_enable(struct spi_nor *nor)
 {
+	if (nor->spimem) {
+		struct spi_mem_op op = SPI_MEM_OP(
+					SPI_MEM_OP_CMD(SPINOR_OP_WREN, 1),
+					SPI_MEM_OP_NO_ADDR,
+					SPI_MEM_OP_NO_DUMMY,
+					SPI_MEM_OP_NO_DATA);
+
+		return spi_nor_nodata_op(nor, &op);
+	}
+
 	return nor->write_reg(nor, SPINOR_OP_WREN, NULL, 0);
 }
 
@@ -369,6 +591,16 @@ static int write_enable(struct spi_nor *nor)
  */
 static int write_disable(struct spi_nor *nor)
 {
+	if (nor->spimem) {
+		struct spi_mem_op op = SPI_MEM_OP(
+					SPI_MEM_OP_CMD(SPINOR_OP_WRDI, 1),
+					SPI_MEM_OP_NO_ADDR,
+					SPI_MEM_OP_NO_DUMMY,
+					SPI_MEM_OP_NO_DATA);
+
+		return spi_nor_nodata_op(nor, &op);
+	}
+
 	return nor->write_reg(nor, SPINOR_OP_WRDI, NULL, 0);
 }
 
@@ -468,12 +700,46 @@ static void spi_nor_set_4byte_opcodes(struct spi_nor *nor)
 	}
 }
 
+static int macronix_set_4byte(struct spi_nor *nor, bool enable)
+{
+	if (nor->spimem) {
+		struct spi_mem_op op = SPI_MEM_OP(
+					SPI_MEM_OP_CMD(enable ?
+						       SPINOR_OP_EN4B :
+						       SPINOR_OP_EX4B,
+						       1),
+					SPI_MEM_OP_NO_ADDR,
+					SPI_MEM_OP_NO_DUMMY,
+					SPI_MEM_OP_NO_DATA);
+
+		return spi_nor_nodata_op(nor, &op);
+	}
+
+	return nor->write_reg(nor, enable ? SPINOR_OP_EN4B : SPINOR_OP_EX4B,
+			      NULL, 0);
+}
+
+static int write_ear(struct spi_nor *nor, u8 ear)
+{
+	if (nor->spimem) {
+		struct spi_mem_op op = SPI_MEM_OP(
+					SPI_MEM_OP_CMD(SPINOR_OP_WREAR, 1),
+					SPI_MEM_OP_NO_ADDR,
+					SPI_MEM_OP_NO_DUMMY,
+					SPI_MEM_OP_DATA_OUT(0, NULL, 1));
+
+		return spi_nor_data_op(nor, &op, &ear, 1);
+	}
+
+	nor->cmd_buf[0] = ear;
+	return nor->write_reg(nor, SPINOR_OP_WREAR, nor->cmd_buf, 1);
+}
+
 /* Enable/disable 4-byte addressing mode. */
 static int set_4byte(struct spi_nor *nor, bool enable)
 {
 	int status;
 	bool need_wren = false;
-	u8 cmd;
 
 	switch (JEDEC_MFR(nor->info)) {
 	case SNOR_MFR_ST:
@@ -486,8 +752,7 @@ static int set_4byte(struct spi_nor *nor, bool enable)
 		if (need_wren)
 			write_enable(nor);
 
-		cmd = enable ? SPINOR_OP_EN4B : SPINOR_OP_EX4B;
-		status = nor->write_reg(nor, cmd, NULL, 0);
+		status = macronix_set_4byte(nor, enable);
 		if (need_wren)
 			write_disable(nor);
 
@@ -500,8 +765,7 @@ static int set_4byte(struct spi_nor *nor, bool enable)
 			 * We must clear the register to enable normal behavior.
 			 */
 			write_enable(nor);
-			nor->cmd_buf[0] = 0;
-			nor->write_reg(nor, SPINOR_OP_WREAR, nor->cmd_buf, 1);
+			write_ear(nor, 0);
 			write_disable(nor);
 		}
 
@@ -509,16 +773,42 @@ static int set_4byte(struct spi_nor *nor, bool enable)
 	default:
 		/* Spansion style */
 		nor->cmd_buf[0] = enable << 7;
+
+		if (nor->spimem) {
+			struct spi_mem_op op = SPI_MEM_OP(
+					SPI_MEM_OP_CMD(SPINOR_OP_BRWR, 1),
+					SPI_MEM_OP_NO_ADDR,
+					SPI_MEM_OP_NO_DUMMY,
+					SPI_MEM_OP_DATA_OUT(1, NULL, 1));
+
+			return spi_nor_data_op(nor, &op, nor->cmd_buf, 1);
+	}
+
 		return nor->write_reg(nor, SPINOR_OP_BRWR, nor->cmd_buf, 1);
 	}
 }
 
+static int xread_sr(struct spi_nor *nor, u8 *sr)
+{
+	if (nor->spimem) {
+		struct spi_mem_op op = SPI_MEM_OP(
+					SPI_MEM_OP_CMD(SPINOR_OP_XRDSR, 1),
+					SPI_MEM_OP_NO_ADDR,
+					SPI_MEM_OP_NO_DUMMY,
+					SPI_MEM_OP_DATA_IN(0, NULL, 1));
+
+		return spi_nor_data_op(nor, &op, sr, 1);
+	}
+
+	return nor->read_reg(nor, SPINOR_OP_XRDSR, sr, 1);
+}
+
 static int s3an_sr_ready(struct spi_nor *nor)
 {
 	int ret;
 	u8 val;
 
-	ret = nor->read_reg(nor, SPINOR_OP_XRDSR, &val, 1);
+	ret = xread_sr(nor, &val);
 	if (ret < 0) {
 		dev_err(nor->dev, "error %d reading XRDSR\n", (int) ret);
 		return ret;
@@ -527,6 +817,21 @@ static int s3an_sr_ready(struct spi_nor *nor)
 	return !!(val & XSR_RDY);
 }
 
+static int clear_sr(struct spi_nor *nor)
+{
+	if (nor->spimem) {
+		struct spi_mem_op op = SPI_MEM_OP(
+					SPI_MEM_OP_CMD(SPINOR_OP_CLSR, 1),
+					SPI_MEM_OP_NO_ADDR,
+					SPI_MEM_OP_NO_DUMMY,
+					SPI_MEM_OP_NO_DATA);
+
+		return spi_nor_nodata_op(nor, &op);
+	}
+
+	return nor->write_reg(nor, SPINOR_OP_CLSR, NULL, 0);
+}
+
 static int spi_nor_sr_ready(struct spi_nor *nor)
 {
 	int sr = read_sr(nor);
@@ -539,13 +844,28 @@ static int spi_nor_sr_ready(struct spi_nor *nor)
 		else
 			dev_err(nor->dev, "Programming Error occurred\n");
 
-		nor->write_reg(nor, SPINOR_OP_CLSR, NULL, 0);
+		clear_sr(nor);
 		return -EIO;
 	}
 
 	return !(sr & SR_WIP);
 }
 
+static int clear_fsr(struct spi_nor *nor)
+{
+	if (nor->spimem) {
+		struct spi_mem_op op = SPI_MEM_OP(
+					SPI_MEM_OP_CMD(SPINOR_OP_CLFSR, 1),
+					SPI_MEM_OP_NO_ADDR,
+					SPI_MEM_OP_NO_DUMMY,
+					SPI_MEM_OP_NO_DATA);
+
+		return spi_nor_nodata_op(nor, &op);
+	}
+
+	return nor->write_reg(nor, SPINOR_OP_CLFSR, NULL, 0);
+}
+
 static int spi_nor_fsr_ready(struct spi_nor *nor)
 {
 	int fsr = read_fsr(nor);
@@ -562,7 +882,7 @@ static int spi_nor_fsr_ready(struct spi_nor *nor)
 			dev_err(nor->dev,
 			"Attempted to modify a protected sector.\n");
 
-		nor->write_reg(nor, SPINOR_OP_CLFSR, NULL, 0);
+		clear_fsr(nor);
 		return -EIO;
 	}
 
@@ -630,6 +950,16 @@ static int erase_chip(struct spi_nor *nor)
 {
 	dev_dbg(nor->dev, " %lldKiB\n", (long long)(nor->mtd.size >> 10));
 
+	if (nor->spimem) {
+		struct spi_mem_op op = SPI_MEM_OP(
+					SPI_MEM_OP_CMD(SPINOR_OP_CHIP_ERASE, 1),
+					SPI_MEM_OP_NO_ADDR,
+					SPI_MEM_OP_NO_DUMMY,
+					SPI_MEM_OP_NO_DATA);
+
+		return spi_nor_nodata_op(nor, &op);
+	}
+
 	return nor->write_reg(nor, SPINOR_OP_CHIP_ERASE, NULL, 0);
 }
 
@@ -692,6 +1022,17 @@ static int spi_nor_erase_sector(struct spi_nor *nor, u32 addr)
 	if (nor->erase)
 		return nor->erase(nor, addr);
 
+	if (nor->spimem) {
+		struct spi_mem_op op = SPI_MEM_OP(
+					SPI_MEM_OP_CMD(nor->erase_opcode, 1),
+					SPI_MEM_OP_ADDR(nor->addr_width, addr,
+							1),
+					SPI_MEM_OP_NO_DUMMY,
+					SPI_MEM_OP_NO_DATA);
+
+		return spi_nor_nodata_op(nor, &op);
+	}
+
 	/*
 	 * Default implementation, if driver doesn't have a specialized HW
 	 * control
@@ -1406,7 +1747,18 @@ static int write_sr_cr(struct spi_nor *nor, u8 *sr_cr)
 
 	write_enable(nor);
 
-	ret = nor->write_reg(nor, SPINOR_OP_WRSR, sr_cr, 2);
+	if (nor->spimem) {
+		struct spi_mem_op op = SPI_MEM_OP(
+					SPI_MEM_OP_CMD(SPINOR_OP_WRSR, 1),
+					SPI_MEM_OP_NO_ADDR,
+					SPI_MEM_OP_NO_DUMMY,
+					SPI_MEM_OP_DATA_OUT(0, NULL, 1));
+
+		ret = spi_nor_data_op(nor, &op, sr_cr, 2);
+	} else {
+		ret = nor->write_reg(nor, SPINOR_OP_WRSR, sr_cr, 2);
+	}
+
 	if (ret < 0) {
 		dev_err(nor->dev,
 			"error while writing configuration register\n");
@@ -1585,6 +1937,36 @@ static int spansion_read_cr_quad_enable(struct spi_nor *nor)
 	return 0;
 }
 
+static int write_sr2(struct spi_nor *nor, u8 sr2)
+{
+	if (nor->spimem) {
+		struct spi_mem_op op = SPI_MEM_OP(
+					SPI_MEM_OP_CMD(SPINOR_OP_WRSR2, 1),
+					SPI_MEM_OP_NO_ADDR,
+					SPI_MEM_OP_NO_DUMMY,
+					SPI_MEM_OP_DATA_OUT(0, NULL, 1));
+
+		return spi_nor_data_op(nor, &op, &sr2, 1);
+	}
+
+	return nor->write_reg(nor, SPINOR_OP_WRSR2, &sr2, 1);
+}
+
+static int read_sr2(struct spi_nor *nor, u8 *sr2)
+{
+	if (nor->spimem) {
+		struct spi_mem_op op = SPI_MEM_OP(
+					SPI_MEM_OP_CMD(SPINOR_OP_RDSR2, 1),
+					SPI_MEM_OP_NO_ADDR,
+					SPI_MEM_OP_NO_DUMMY,
+					SPI_MEM_OP_DATA_IN(0, NULL, 1));
+
+		return spi_nor_data_op(nor, &op, sr2, 1);
+	}
+
+	return nor->read_reg(nor, SPINOR_OP_RDSR2, sr2, 1);
+}
+
 /**
  * sr2_bit7_quad_enable() - set QE bit in Status Register 2.
  * @nor:	pointer to a 'struct spi_nor'
@@ -1603,7 +1985,7 @@ static int sr2_bit7_quad_enable(struct spi_nor *nor)
 	int ret;
 
 	/* Check current Quad Enable bit value. */
-	ret = nor->read_reg(nor, SPINOR_OP_RDSR2, &sr2, 1);
+	ret = read_sr2(nor, &sr2);
 	if (ret)
 		return ret;
 	if (sr2 & SR2_QUAD_EN_BIT7)
@@ -1614,7 +1996,7 @@ static int sr2_bit7_quad_enable(struct spi_nor *nor)
 
 	write_enable(nor);
 
-	ret = nor->write_reg(nor, SPINOR_OP_WRSR2, &sr2, 1);
+	ret = write_sr2(nor, sr2);
 	if (ret < 0) {
 		dev_err(nor->dev, "error while writing status register 2\n");
 		return -EINVAL;
@@ -1626,8 +2008,7 @@ static int sr2_bit7_quad_enable(struct spi_nor *nor)
 		return ret;
 	}
 
-	/* Read back and check it. */
-	ret = nor->read_reg(nor, SPINOR_OP_RDSR2, &sr2, 1);
+	ret = read_sr2(nor, &sr2);
 	if (!(ret > 0 && (sr2 & SR2_QUAD_EN_BIT7))) {
 		dev_err(nor->dev, "SR2 Quad bit not set\n");
 		return -EINVAL;
@@ -2054,13 +2435,28 @@ static const struct flash_info spi_nor_ids[] = {
 	{ },
 };
 
+static int read_id(struct spi_nor *nor, u8 *id)
+{
+	if (nor->spimem) {
+		struct spi_mem_op op = SPI_MEM_OP(
+					SPI_MEM_OP_CMD(SPINOR_OP_RDID, 1),
+					SPI_MEM_OP_NO_ADDR,
+					SPI_MEM_OP_NO_DUMMY,
+					SPI_MEM_OP_DATA_IN(0, NULL, 1));
+
+		return spi_nor_data_op(nor, &op, id, SPI_NOR_MAX_ID_LEN);
+	}
+
+	return nor->read_reg(nor, SPINOR_OP_RDID, id, SPI_NOR_MAX_ID_LEN);
+}
+
 static const struct flash_info *spi_nor_read_id(struct spi_nor *nor)
 {
 	int			tmp;
 	u8			id[SPI_NOR_MAX_ID_LEN];
 	const struct flash_info	*info;
 
-	tmp = nor->read_reg(nor, SPINOR_OP_RDID, id, SPI_NOR_MAX_ID_LEN);
+	tmp = read_id(nor, id);
 	if (tmp < 0) {
 		dev_dbg(nor->dev, "error %d reading JEDEC ID\n", tmp);
 		return ERR_PTR(tmp);
@@ -2096,7 +2492,7 @@ static int spi_nor_read(struct mtd_info *mtd, loff_t from, size_t len,
 		if (nor->flags & SNOR_F_S3AN_ADDR_DEFAULT)
 			addr = spi_nor_s3an_addr_convert(nor, addr);
 
-		ret = nor->read(nor, addr, len, buf);
+		ret = spi_nor_read_data(nor, addr, len, buf);
 		if (ret == 0) {
 			/* We shouldn't see 0-length reads */
 			ret = -EIO;
@@ -2141,7 +2537,7 @@ static int sst_write(struct mtd_info *mtd, loff_t to, size_t len,
 		nor->program_opcode = SPINOR_OP_BP;
 
 		/* write one byte. */
-		ret = nor->write(nor, to, 1, buf);
+		ret = spi_nor_write_data(nor, to, 1, buf);
 		if (ret < 0)
 			goto sst_write_err;
 		WARN(ret != 1, "While writing 1 byte written %i bytes\n",
@@ -2157,7 +2553,7 @@ static int sst_write(struct mtd_info *mtd, loff_t to, size_t len,
 		nor->program_opcode = SPINOR_OP_AAI_WP;
 
 		/* write two bytes. */
-		ret = nor->write(nor, to, 2, buf + actual);
+		ret = spi_nor_write_data(nor, to, 2, buf + actual);
 		if (ret < 0)
 			goto sst_write_err;
 		WARN(ret != 2, "While writing 2 bytes written %i bytes\n",
@@ -2180,7 +2576,7 @@ static int sst_write(struct mtd_info *mtd, loff_t to, size_t len,
 		write_enable(nor);
 
 		nor->program_opcode = SPINOR_OP_BP;
-		ret = nor->write(nor, to, 1, buf + actual);
+		ret = spi_nor_write_data(nor, to, 1, buf + actual);
 		if (ret < 0)
 			goto sst_write_err;
 		WARN(ret != 1, "While writing 1 byte written %i bytes\n",
@@ -2242,7 +2638,7 @@ static int spi_nor_write(struct mtd_info *mtd, loff_t to, size_t len,
 			addr = spi_nor_s3an_addr_convert(nor, addr);
 
 		write_enable(nor);
-		ret = nor->write(nor, addr, page_remain, buf + i);
+		ret = spi_nor_write_data(nor, addr, page_remain, buf + i);
 		if (ret < 0)
 			goto write_err;
 		written = ret;
@@ -2261,8 +2657,10 @@ static int spi_nor_write(struct mtd_info *mtd, loff_t to, size_t len,
 
 static int spi_nor_check(struct spi_nor *nor)
 {
-	if (!nor->dev || !nor->read || !nor->write ||
-		!nor->read_reg || !nor->write_reg) {
+	if (!nor->dev ||
+	    (!nor->spimem &&
+	    (!nor->read || !nor->write || !nor->read_reg ||
+	      !nor->write_reg))) {
 		pr_err("spi-nor: please fill all the necessary fields!\n");
 		return -EINVAL;
 	}
@@ -2275,7 +2673,7 @@ static int s3an_nor_scan(struct spi_nor *nor)
 	int ret;
 	u8 val;
 
-	ret = nor->read_reg(nor, SPINOR_OP_XRDSR, &val, 1);
+	ret = xread_sr(nor, &val);
 	if (ret < 0) {
 		dev_err(nor->dev, "error %d reading XRDSR\n", (int) ret);
 		return ret;
@@ -2405,7 +2803,7 @@ static int spi_nor_read_raw(struct spi_nor *nor, u32 addr, size_t len, u8 *buf)
 	int ret;
 
 	while (len) {
-		ret = nor->read(nor, addr, len, buf);
+		ret = spi_nor_read_data(nor, addr, len, buf);
 		if (!ret || ret > len)
 			return -EIO;
 		if (ret < 0)
@@ -4188,6 +4586,215 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
 }
 EXPORT_SYMBOL_GPL(spi_nor_scan);
 
+static int spi_nor_probe(struct spi_mem *spimem)
+{
+	struct spi_device *spi = spimem->spi;
+	struct flash_platform_data *data;
+	struct spi_nor *nor;
+	struct spi_nor_hwcaps hwcaps = {
+		.mask = SNOR_HWCAPS_READ |
+			SNOR_HWCAPS_READ_FAST |
+			SNOR_HWCAPS_PP,
+	};
+	char *flash_name;
+	int ret;
+
+	data = dev_get_platdata(&spi->dev);
+
+	nor = devm_kzalloc(&spi->dev, sizeof(*nor), GFP_KERNEL);
+	if (!nor)
+		return -ENOMEM;
+
+	/*
+	 * We need the bounce buffer early to read/write registers when going
+	 * through the spi-mem layer (buffers have to be DMA-able).
+	 * We'll reallocate a new buffer if nor->page_size turns out to be
+	 * greater than PAGE_SIZE (which shouldn't happen before long since NOR
+	 * pages are usually less than 1KB) after spi_nor_scan() returns.
+	 */
+	nor->bouncebuf_size = PAGE_SIZE;
+	nor->bouncebuf = devm_kmalloc(&spi->dev, nor->bouncebuf_size,
+				      GFP_KERNEL);
+	if (!nor->bouncebuf)
+		return -ENOMEM;
+
+	nor->spimem = spimem;
+	nor->dev = &spi->dev;
+	spi_nor_set_flash_node(nor, spi->dev.of_node);
+
+	spi_mem_set_drvdata(spimem, nor);
+
+	if (spi->mode & SPI_RX_OCTAL) {
+		hwcaps.mask |= SNOR_HWCAPS_READ_1_1_8;
+
+		if (spi->mode & SPI_TX_OCTAL)
+			hwcaps.mask |= (SNOR_HWCAPS_READ_1_8_8 |
+					SNOR_HWCAPS_PP_1_1_8 |
+					SNOR_HWCAPS_PP_1_8_8);
+	} else if (spi->mode & SPI_RX_QUAD) {
+		hwcaps.mask |= SNOR_HWCAPS_READ_1_1_4;
+
+		if (spi->mode & SPI_TX_QUAD)
+			hwcaps.mask |= (SNOR_HWCAPS_READ_1_4_4 |
+					SNOR_HWCAPS_PP_1_1_4 |
+					SNOR_HWCAPS_PP_1_4_4);
+	} else if (spi->mode & SPI_RX_DUAL) {
+		hwcaps.mask |= SNOR_HWCAPS_READ_1_1_2;
+
+		if (spi->mode & SPI_TX_DUAL)
+			hwcaps.mask |= SNOR_HWCAPS_READ_1_2_2;
+	}
+
+	if (data && data->name)
+		nor->mtd.name = data->name;
+
+	if (!nor->mtd.name)
+		nor->mtd.name = spi_mem_get_name(spimem);
+
+	/*
+	 * For some (historical?) reason many platforms provide two different
+	 * names in flash_platform_data: "name" and "type". Quite often name is
+	 * set to "m25p80" and then "type" provides a real chip name.
+	 * If that's the case, respect "type" and ignore a "name".
+	 */
+	if (data && data->type)
+		flash_name = data->type;
+	else if (!strcmp(spi->modalias, "spi-nor"))
+		flash_name = NULL; /* auto-detect */
+	else
+		flash_name = spi->modalias;
+
+	ret = spi_nor_scan(nor, flash_name, &hwcaps);
+	if (ret)
+		return ret;
+
+	/*
+	 * None of the existing parts have > 512B pages, but let's play safe
+	 * and add this logic so that if anyone ever adds support for such
+	 * a NOR we don't end up with buffer overflows.
+	 */
+	if (nor->page_size > PAGE_SIZE) {
+		nor->bouncebuf_size = nor->page_size;
+		devm_kfree(nor->dev, nor->bouncebuf);
+		nor->bouncebuf = devm_kmalloc(nor->dev,
+					      nor->bouncebuf_size,
+					      GFP_KERNEL);
+		if (!nor->bouncebuf)
+			return -ENOMEM;
+	}
+
+	return mtd_device_register(&nor->mtd, data ? data->parts : NULL,
+				   data ? data->nr_parts : 0);
+}
+
+static int spi_nor_remove(struct spi_mem *spimem)
+{
+	struct spi_nor *nor = spi_mem_get_drvdata(spimem);
+	int ret;
+
+	spi_nor_restore(nor);
+
+	/* Clean up MTD stuff. */
+	ret = mtd_device_unregister(&nor->mtd);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static void spi_nor_shutdown(struct spi_mem *spimem)
+{
+	struct spi_nor *nor = spi_mem_get_drvdata(spimem);
+
+	spi_nor_restore(nor);
+}
+
+/*
+ * Do NOT add to this array without reading the following:
+ *
+ * Historically, many flash devices are bound to this driver by their name. But
+ * since most of these flash are compatible to some extent, and their
+ * differences can often be differentiated by the JEDEC read-ID command, we
+ * encourage new users to add support to the spi-nor library, and simply bind
+ * against a generic string here (e.g., "jedec,spi-nor").
+ *
+ * Many flash names are kept here in this list (as well as in spi-nor.c) to
+ * keep them available as module aliases for existing platforms.
+ */
+static const struct spi_device_id spi_nor_dev_ids[] = {
+	/*
+	 * Allow non-DT platform devices to bind to the "spi-nor" modalias, and
+	 * hack around the fact that the SPI core does not provide uevent
+	 * matching for .of_match_table
+	 */
+	{"spi-nor"},
+
+	/*
+	 * Entries not used in DTs that should be safe to drop after replacing
+	 * them with "spi-nor" in platform data.
+	 */
+	{"s25sl064a"},	{"w25x16"},	{"m25p10"},	{"m25px64"},
+
+	/*
+	 * Entries that were used in DTs without "jedec,spi-nor" fallback and
+	 * should be kept for backward compatibility.
+	 */
+	{"at25df321a"},	{"at25df641"},	{"at26df081a"},
+	{"mx25l4005a"},	{"mx25l1606e"},	{"mx25l6405d"},	{"mx25l12805d"},
+	{"mx25l25635e"},{"mx66l51235l"},
+	{"n25q064"},	{"n25q128a11"},	{"n25q128a13"},	{"n25q512a"},
+	{"s25fl256s1"},	{"s25fl512s"},	{"s25sl12801"},	{"s25fl008k"},
+	{"s25fl064k"},
+	{"sst25vf040b"},{"sst25vf016b"},{"sst25vf032b"},{"sst25wf040"},
+	{"m25p40"},	{"m25p80"},	{"m25p16"},	{"m25p32"},
+	{"m25p64"},	{"m25p128"},
+	{"w25x80"},	{"w25x32"},	{"w25q32"},	{"w25q32dw"},
+	{"w25q80bl"},	{"w25q128"},	{"w25q256"},
+
+	/* Flashes that can't be detected using JEDEC */
+	{"m25p05-nonjedec"},	{"m25p10-nonjedec"},	{"m25p20-nonjedec"},
+	{"m25p40-nonjedec"},	{"m25p80-nonjedec"},	{"m25p16-nonjedec"},
+	{"m25p32-nonjedec"},	{"m25p64-nonjedec"},	{"m25p128-nonjedec"},
+
+	/* Everspin MRAMs (non-JEDEC) */
+	{ "mr25h128" }, /* 128 Kib, 40 MHz */
+	{ "mr25h256" }, /* 256 Kib, 40 MHz */
+	{ "mr25h10" },  /*   1 Mib, 40 MHz */
+	{ "mr25h40" },  /*   4 Mib, 40 MHz */
+
+	{ },
+};
+MODULE_DEVICE_TABLE(spi, spi_nor_dev_ids);
+
+static const struct of_device_id spi_nor_of_table[] = {
+	/*
+	 * Generic compatibility for SPI NOR that can be identified by the
+	 * JEDEC READ ID opcode (0x9F). Use this, if possible.
+	 */
+	{ .compatible = "jedec,spi-nor" },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, spi_nor_of_table);
+
+/*
+ * REVISIT: many of these chips have deep power-down modes, which
+ * should clearly be entered on suspend() to minimize power use.
+ * And also when they're otherwise idle...
+ */
+static struct spi_mem_driver spi_nor_driver = {
+	.spidrv = {
+		.driver = {
+			.name = "spi-nor",
+			.of_match_table = spi_nor_of_table,
+		},
+		.id_table = spi_nor_dev_ids,
+	},
+	.probe = spi_nor_probe,
+	.remove = spi_nor_remove,
+	.shutdown = spi_nor_shutdown,
+};
+module_spi_mem_driver(spi_nor_driver);
+
 MODULE_LICENSE("GPL v2");
 MODULE_AUTHOR("Huang Shijie <shijie8@gmail.com>");
 MODULE_AUTHOR("Mike Lavender");
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index b3d360b0ee3d..ac16745f5ef8 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -9,6 +9,7 @@
 #include <linux/bitops.h>
 #include <linux/mtd/cfi.h>
 #include <linux/mtd/mtd.h>
+#include <linux/spi/spi-mem.h>
 
 /*
  * Manufacturer IDs
@@ -344,6 +345,10 @@ struct flash_info;
  * @mtd:		point to a mtd_info structure
  * @lock:		the lock for the read/write/erase/lock/unlock operations
  * @dev:		point to a spi device, or a spi nor controller device.
+ * @spimem:		point to the spi mem device
+ * @bouncebuf:		bounce buffer used when the buffer passed by the MTD
+ *                      layer is not DMA-able
+ * @bouncebuf_size:	size of the bounce buffe
  * @info:		spi-nor part JDEC MFR id and other info
  * @page_size:		the page size of the SPI NOR
  * @addr_width:		number of address bytes
@@ -380,6 +385,9 @@ struct spi_nor {
 	struct mtd_info		mtd;
 	struct mutex		lock;
 	struct device		*dev;
+	struct spi_mem		*spimem;
+	void			*bouncebuf;
+	unsigned int		bouncebuf_size;
 	const struct flash_info	*info;
 	u32			page_size;
 	u8			addr_width;
-- 
2.21.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* [PATCH 2/2] mtd: spi-nor: Rework hwcaps selection for the spi-mem case
  2019-04-09 16:26 [PATCH 0/2] Merge m25p80 into spi-nor Vignesh Raghavendra
  2019-04-09 16:26 ` [PATCH 1/2] mtd: spi-nor: Move m25p80 code in spi-nor.c Vignesh Raghavendra
@ 2019-04-09 16:26 ` Vignesh Raghavendra
  2019-04-15 11:59   ` Tudor.Ambarus
  1 sibling, 1 reply; 6+ messages in thread
From: Vignesh Raghavendra @ 2019-04-09 16:26 UTC (permalink / raw)
  To: Boris Brezillon, Marek Vasut, Richard Weinberger, Tudor Ambarus
  Cc: Yogesh Narayan Gaur, Vignesh Raghavendra, linux-kernel,
	Frieder Schrempf, linux-mtd, Miquel Raynal

From: Boris Brezillon <boris.brezillon@bootlin.com>

The spi-mem layer provides a spi_mem_supports_op() function to check
whether a specific operation is supported by the controller or not.
This is much more accurate than the hwcaps selection logic based on
SPI_{RX,TX}_ flags.

Rework the hwcaps selection logic to use spi_mem_supports_op() when
nor->spimem != NULL.

Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com>
---
Chagnes wrt RFC:
Fix checkpatch issues
Rebase onto latest

 drivers/mtd/spi-nor/spi-nor.c | 164 ++++++++++++++++++++++++++--------
 include/linux/mtd/spi-nor.h   |  14 +++
 2 files changed, 142 insertions(+), 36 deletions(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 03c8c346c9ae..d48ae3c9661d 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -2852,6 +2852,110 @@ static int spi_nor_read_sfdp(struct spi_nor *nor, u32 addr,
 	return ret;
 }
 
+static int spi_nor_spimem_check_readop(struct spi_nor *nor,
+				       const struct spi_nor_read_command *read)
+{
+	struct spi_mem_op op = SPI_MEM_OP(SPI_MEM_OP_CMD(read->opcode, 1),
+					  SPI_MEM_OP_ADDR(3, 0, 1),
+					  SPI_MEM_OP_DUMMY(0, 1),
+					  SPI_MEM_OP_DATA_IN(0, NULL, 1));
+
+	op.cmd.buswidth = spi_nor_get_protocol_inst_nbits(read->proto);
+	op.addr.buswidth = spi_nor_get_protocol_addr_nbits(read->proto);
+	op.data.buswidth = spi_nor_get_protocol_data_nbits(read->proto);
+	op.dummy.buswidth = op.addr.buswidth;
+	op.dummy.nbytes = (read->num_mode_clocks + read->num_wait_states) *
+			  op.dummy.buswidth / 8;
+
+	/*
+	 * First test with 3 address bytes. The opcode itself might already
+	 * be a 4B addressing opcode but we don't care, because SPI controller
+	 * implementation should not check the opcode, but just the sequence.
+	 */
+	if (!spi_mem_supports_op(nor->spimem, &op))
+		return -ENOTSUPP;
+
+	/* Now test with 4 address bytes. */
+	op.addr.nbytes = 4;
+	if (!spi_mem_supports_op(nor->spimem, &op))
+		return -ENOTSUPP;
+
+	return 0;
+}
+
+static int spi_nor_spimem_check_progop(struct spi_nor *nor,
+				       const struct spi_nor_pp_command *pp)
+{
+	struct spi_mem_op op = SPI_MEM_OP(SPI_MEM_OP_CMD(pp->opcode, 1),
+					  SPI_MEM_OP_ADDR(3, 0, 1),
+					  SPI_MEM_OP_NO_DUMMY,
+					  SPI_MEM_OP_DATA_OUT(0, NULL, 1));
+
+	op.cmd.buswidth = spi_nor_get_protocol_inst_nbits(pp->proto);
+	op.addr.buswidth = spi_nor_get_protocol_addr_nbits(pp->proto);
+	op.data.buswidth = spi_nor_get_protocol_data_nbits(pp->proto);
+
+	/*
+	 * First test with 3 address bytes. The opcode itself might already
+	 * be a 4B addressing opcode but we don't care, because SPI controller
+	 * implementation should not check the opcode, but just the sequence.
+	 */
+	if (!spi_mem_supports_op(nor->spimem, &op))
+		return -ENOTSUPP;
+
+	/* Now test with 4 address bytes. */
+	op.addr.nbytes = 4;
+	if (!spi_mem_supports_op(nor->spimem, &op))
+		return -ENOTSUPP;
+
+	return 0;
+}
+
+static void
+spi_nor_spimem_adjust_hwcaps(struct spi_nor *nor,
+			     const struct spi_nor_flash_parameter *params,
+			     u32 *hwcaps)
+{
+	unsigned int cap;
+
+	/* DTR modes are not supported yet, mask them all. */
+	*hwcaps &= ~SNOR_HWCAPS_DTR;
+
+	/* X-X-X modes are not supported yet, mask them all. */
+	*hwcaps &= ~SNOR_HWCAPS_X_X_X;
+
+	/* Start with read commands. */
+	for (cap = 0; cap < 32; cap++) {
+		int idx;
+
+		if (!(*hwcaps & BIT(cap)))
+			continue;
+
+		idx = spi_nor_hwcaps_read2cmd(BIT(cap));
+		if (idx < 0)
+			continue;
+
+		if (spi_nor_spimem_check_readop(nor, &params->reads[idx]))
+			*hwcaps &= ~BIT(cap);
+	}
+
+	/* Now check program commands. */
+	for (cap = 0; cap < 32; cap++) {
+		int idx;
+
+		if (!(*hwcaps & BIT(cap)))
+			continue;
+
+		idx = spi_nor_hwcaps_pp2cmd(BIT(cap));
+		if (idx < 0)
+			continue;
+
+		if (spi_nor_spimem_check_progop(nor,
+						&params->page_programs[idx]))
+			*hwcaps &= ~BIT(cap);
+	}
+}
+
 /**
  * spi_nor_read_sfdp_dma_unsafe() - read Serial Flash Discoverable Parameters.
  * @nor:	pointer to a 'struct spi_nor'
@@ -4259,16 +4363,25 @@ static int spi_nor_setup(struct spi_nor *nor,
 	 */
 	shared_mask = hwcaps->mask & params->hwcaps.mask;
 
-	/* SPI n-n-n protocols are not supported yet. */
-	ignored_mask = (SNOR_HWCAPS_READ_2_2_2 |
-			SNOR_HWCAPS_READ_4_4_4 |
-			SNOR_HWCAPS_READ_8_8_8 |
-			SNOR_HWCAPS_PP_4_4_4 |
-			SNOR_HWCAPS_PP_8_8_8);
-	if (shared_mask & ignored_mask) {
-		dev_dbg(nor->dev,
-			"SPI n-n-n protocols are not supported yet.\n");
-		shared_mask &= ~ignored_mask;
+	if (nor->spimem) {
+		/*
+		 * When called from spi_nor_probe(), all caps are set and we
+		 * need to discard some of them based on what the SPI
+		 * controller actually supports (using spi_mem_supports_op()).
+		 */
+		spi_nor_spimem_adjust_hwcaps(nor, params, &shared_mask);
+	} else {
+		/*
+		 * SPI n-n-n protocols are not supported when the SPI
+		 * controller directly implements the spi_nor interface.
+		 * Yet another reason to switch to spi-mem.
+		 */
+		ignored_mask = SNOR_HWCAPS_X_X_X;
+		if (shared_mask & ignored_mask) {
+			dev_dbg(nor->dev,
+				"SPI n-n-n protocols are not supported.\n");
+			shared_mask &= ~ignored_mask;
+		}
 	}
 
 	/* Select the (Fast) Read command. */
@@ -4591,11 +4704,11 @@ static int spi_nor_probe(struct spi_mem *spimem)
 	struct spi_device *spi = spimem->spi;
 	struct flash_platform_data *data;
 	struct spi_nor *nor;
-	struct spi_nor_hwcaps hwcaps = {
-		.mask = SNOR_HWCAPS_READ |
-			SNOR_HWCAPS_READ_FAST |
-			SNOR_HWCAPS_PP,
-	};
+	/*
+	 * Enable all caps by default. The core will mask them after
+	 * checking what's really supported using spi_mem_supports_op().
+	 */
+	struct spi_nor_hwcaps hwcaps = { .mask = SNOR_HWCAPS_ALL };
 	char *flash_name;
 	int ret;
 
@@ -4624,27 +4737,6 @@ static int spi_nor_probe(struct spi_mem *spimem)
 
 	spi_mem_set_drvdata(spimem, nor);
 
-	if (spi->mode & SPI_RX_OCTAL) {
-		hwcaps.mask |= SNOR_HWCAPS_READ_1_1_8;
-
-		if (spi->mode & SPI_TX_OCTAL)
-			hwcaps.mask |= (SNOR_HWCAPS_READ_1_8_8 |
-					SNOR_HWCAPS_PP_1_1_8 |
-					SNOR_HWCAPS_PP_1_8_8);
-	} else if (spi->mode & SPI_RX_QUAD) {
-		hwcaps.mask |= SNOR_HWCAPS_READ_1_1_4;
-
-		if (spi->mode & SPI_TX_QUAD)
-			hwcaps.mask |= (SNOR_HWCAPS_READ_1_4_4 |
-					SNOR_HWCAPS_PP_1_1_4 |
-					SNOR_HWCAPS_PP_1_4_4);
-	} else if (spi->mode & SPI_RX_DUAL) {
-		hwcaps.mask |= SNOR_HWCAPS_READ_1_1_2;
-
-		if (spi->mode & SPI_TX_DUAL)
-			hwcaps.mask |= SNOR_HWCAPS_READ_1_2_2;
-	}
-
 	if (data && data->name)
 		nor->mtd.name = data->name;
 
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index ac16745f5ef8..1d56c437f553 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -523,6 +523,20 @@ struct spi_nor_hwcaps {
 #define SNOR_HWCAPS_PP_1_8_8	BIT(21)
 #define SNOR_HWCAPS_PP_8_8_8	BIT(22)
 
+#define SNOR_HWCAPS_X_X_X	(SNOR_HWCAPS_READ_2_2_2 |	\
+				 SNOR_HWCAPS_READ_4_4_4 |	\
+				 SNOR_HWCAPS_READ_8_8_8 |	\
+				 SNOR_HWCAPS_PP_4_4_4 |		\
+				 SNOR_HWCAPS_PP_8_8_8)
+
+#define SNOR_HWCAPS_DTR		(SNOR_HWCAPS_READ_1_1_1_DTR |	\
+				 SNOR_HWCAPS_READ_1_2_2_DTR |	\
+				 SNOR_HWCAPS_READ_1_4_4_DTR |	\
+				 SNOR_HWCAPS_READ_1_8_8_DTR)
+
+#define SNOR_HWCAPS_ALL		(SNOR_HWCAPS_READ_MASK |	\
+				 SNOR_HWCAPS_PP_MASK)
+
 /**
  * spi_nor_scan() - scan the SPI NOR
  * @nor:	the spi_nor structure
-- 
2.21.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* Re: [PATCH 1/2] mtd: spi-nor: Move m25p80 code in spi-nor.c
  2019-04-09 16:26 ` [PATCH 1/2] mtd: spi-nor: Move m25p80 code in spi-nor.c Vignesh Raghavendra
@ 2019-04-15  8:13   ` Tudor.Ambarus
  2019-04-16 13:55     ` Vignesh Raghavendra
  0 siblings, 1 reply; 6+ messages in thread
From: Tudor.Ambarus @ 2019-04-15  8:13 UTC (permalink / raw)
  To: vigneshr, bbrezillon, marek.vasut, richard, miquel.raynal,
	andrew.smirnov
  Cc: frieder.schrempf, yogeshnarayan.gaur, linux-mtd, linux-kernel,
	miquel.raynal

Hi,

The general approach looks good, few comments below.

On 04/09/2019 07:26 PM, Vignesh Raghavendra wrote:
> External E-Mail
> 
> 
> From: Boris Brezillon <boris.brezillon@bootlin.com>
> 
> The m25p80 driver is actually a generic wrapper around the spi-mem
> layer. Not only the driver name is misleading, but we'd expect such a
> common logic to be directly available in the core. Another reason for
> moving this code is that SPI NOR controller drivers should
> progressively be replaced by SPI controller drivers implementing the
> spi_mem_ops interface, and when the conversion is done, we should have
> a single spi-nor driver directly interfacing with the spi-mem layer.
> 
> While moving the code we also fix a longstanding issue when
> non-DMA-able buffers are passed by the MTD layer.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
> [vigneshr@ti.com: use devm_kmalloc() for bounce buffer allocation]
> Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com>
> ---
> Changes from RFC:
> * Use devm_kmalloc() for bounce buffer allocation as it supports cache
>   line aligned buffers now

then spi-nand has to be updated too.

> * Rebase onto latest next
> * Fix build uses when spi-nor is built as a module
> 
>  drivers/mtd/devices/Kconfig   |  18 -
>  drivers/mtd/devices/Makefile  |   1 -
>  drivers/mtd/devices/m25p80.c  | 351 ------------------
>  drivers/mtd/spi-nor/Kconfig   |   2 +
>  drivers/mtd/spi-nor/spi-nor.c | 659 ++++++++++++++++++++++++++++++++--
>  include/linux/mtd/spi-nor.h   |   8 +
>  6 files changed, 643 insertions(+), 396 deletions(-)
>  delete mode 100644 drivers/mtd/devices/m25p80.c
> 
> diff --git a/drivers/mtd/devices/Kconfig b/drivers/mtd/devices/Kconfig
> index 7fcdaf6c279d..280ea90ea2d1 100644
> --- a/drivers/mtd/devices/Kconfig
> +++ b/drivers/mtd/devices/Kconfig
> @@ -78,24 +78,6 @@ config MTD_DATAFLASH_OTP
>  	  other key product data.  The second half is programmed with a
>  	  unique-to-each-chip bit pattern at the factory.
>  
> -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,
> -	  Spansion S25SL, SST 25VF, ST M25P, and Winbond W25X.  Other chips
> -	  are supported as well.  See the driver source for the current list,
> -	  or to add other chips.
> -
> -	  Note that the original DataFlash chips (AT45 series, not AT26DF),
> -	  need an entirely different driver.
> -
> -	  Set up your spi devices with the right board-specific platform data,
> -	  if you want to specify device partitioning or to use a device which
> -	  doesn't support the JEDEC ID instruction.
> -
>  config MTD_MCHP23K256
>  	tristate "Microchip 23K256 SRAM"
>  	depends on SPI_MASTER
> diff --git a/drivers/mtd/devices/Makefile b/drivers/mtd/devices/Makefile
> index 94895eab3066..991c8d12c016 100644
> --- a/drivers/mtd/devices/Makefile
> +++ b/drivers/mtd/devices/Makefile
> @@ -12,7 +12,6 @@ obj-$(CONFIG_MTD_MTDRAM)	+= mtdram.o
>  obj-$(CONFIG_MTD_LART)		+= lart.o
>  obj-$(CONFIG_MTD_BLOCK2MTD)	+= block2mtd.o
>  obj-$(CONFIG_MTD_DATAFLASH)	+= mtd_dataflash.o
> -obj-$(CONFIG_MTD_M25P80)	+= m25p80.o
>  obj-$(CONFIG_MTD_MCHP23K256)	+= mchp23k256.o
>  obj-$(CONFIG_MTD_SPEAR_SMI)	+= spear_smi.o
>  obj-$(CONFIG_MTD_SST25L)	+= sst25l.o
> diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
> deleted file mode 100644
> index 651bab6d4e31..000000000000
> --- a/drivers/mtd/devices/m25p80.c
> +++ /dev/null
> @@ -1,351 +0,0 @@
> -/*
> - * MTD SPI driver for ST M25Pxx (and similar) serial flash chips
> - *
> - * Author: Mike Lavender, mike@steroidmicros.com
> - *
> - * Copyright (c) 2005, Intec Automation Inc.
> - *
> - * Some parts are based on lart.c by Abraham Van Der Merwe
> - *
> - * Cleaned up and generalized based on mtd_dataflash.c
> - *
> - * This code is free software; you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License version 2 as
> - * published by the Free Software Foundation.
> - *
> - */
> -
> -#include <linux/err.h>
> -#include <linux/errno.h>
> -#include <linux/module.h>
> -#include <linux/device.h>
> -
> -#include <linux/mtd/mtd.h>
> -#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>
> -
> -struct m25p {
> -	struct spi_mem		*spimem;
> -	struct spi_nor		spi_nor;
> -};
> -
> -static int m25p80_read_reg(struct spi_nor *nor, u8 code, u8 *val, int len)
> -{
> -	struct m25p *flash = nor->priv;
> -	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, NULL, 1));
> -	void *scratchbuf;
> -	int ret;
> -
> -	scratchbuf = kmalloc(len, GFP_KERNEL);
> -	if (!scratchbuf)
> -		return -ENOMEM;
> -
> -	op.data.buf.in = scratchbuf;
> -	ret = spi_mem_exec_op(flash->spimem, &op);
> -	if (ret < 0)
> -		dev_err(&flash->spimem->spi->dev, "error %d reading %x\n", ret,
> -			code);
> -	else
> -		memcpy(val, scratchbuf, len);
> -
> -	kfree(scratchbuf);
> -
> -	return ret;
> -}
> -
> -static int m25p80_write_reg(struct spi_nor *nor, u8 opcode, u8 *buf, int len)
> -{
> -	struct m25p *flash = nor->priv;
> -	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, NULL, 1));
> -	void *scratchbuf;
> -	int ret;
> -
> -	scratchbuf = kmemdup(buf, len, GFP_KERNEL);
> -	if (!scratchbuf)
> -		return -ENOMEM;
> -
> -	op.data.buf.out = scratchbuf;
> -	ret = spi_mem_exec_op(flash->spimem, &op);
> -	kfree(scratchbuf);
> -
> -	return ret;
> -}
> -
> -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_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_NO_DUMMY,
> -				   SPI_MEM_OP_DATA_OUT(len, buf, 1));
> -	int ret;
> -
> -	/* get transfer protocols. */
> -	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.data.buswidth = spi_nor_get_protocol_data_nbits(nor->write_proto);
> -
> -	if (nor->program_opcode == SPINOR_OP_AAI_WP && nor->sst_write_second)
> -		op.addr.nbytes = 0;
> -
> -	ret = spi_mem_adjust_op_size(flash->spimem, &op);
> -	if (ret)
> -		return ret;
> -	op.data.nbytes = len < op.data.nbytes ? len : op.data.nbytes;
> -
> -	ret = spi_mem_exec_op(flash->spimem, &op);
> -	if (ret)
> -		return ret;
> -
> -	return op.data.nbytes;
> -}
> -
> -/*
> - * Read an address range from the nor chip.  The address range
> - * may be any size provided it is within the physical boundaries.
> - */
> -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_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. */
> -	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 */
> -	op.dummy.nbytes = (nor->read_dummy * op.dummy.buswidth) / 8;
> -
> -	while (remaining) {
> -		op.data.nbytes = remaining < UINT_MAX ? remaining : UINT_MAX;
> -		ret = spi_mem_adjust_op_size(flash->spimem, &op);
> -		if (ret)
> -			return ret;
> -
> -		ret = spi_mem_exec_op(flash->spimem, &op);
> -		if (ret)
> -			return ret;
> -
> -		op.addr.val += op.data.nbytes;
> -		remaining -= op.data.nbytes;
> -		op.data.buf.in += op.data.nbytes;
> -	}
> -
> -	return len;
> -}
> -
> -/*
> - * board specific setup should have ensured the SPI clock used here
> - * 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_mem *spimem)
> -{
> -	struct spi_device *spi = spimem->spi;
> -	struct flash_platform_data	*data;
> -	struct m25p *flash;
> -	struct spi_nor *nor;
> -	struct spi_nor_hwcaps hwcaps = {
> -		.mask = SNOR_HWCAPS_READ |
> -			SNOR_HWCAPS_READ_FAST |
> -			SNOR_HWCAPS_PP,
> -	};
> -	char *flash_name;
> -	int ret;
> -
> -	data = dev_get_platdata(&spimem->spi->dev);
> -
> -	flash = devm_kzalloc(&spimem->spi->dev, sizeof(*flash), GFP_KERNEL);
> -	if (!flash)
> -		return -ENOMEM;
> -
> -	nor = &flash->spi_nor;
> -
> -	/* install the hooks */
> -	nor->read = m25p80_read;
> -	nor->write = m25p80_write;
> -	nor->write_reg = m25p80_write_reg;
> -	nor->read_reg = m25p80_read_reg;
> -
> -	nor->dev = &spimem->spi->dev;
> -	spi_nor_set_flash_node(nor, spi->dev.of_node);
> -	nor->priv = flash;
> -
> -	spi_mem_set_drvdata(spimem, flash);
> -	flash->spimem = spimem;
> -
> -	if (spi->mode & SPI_RX_OCTAL) {
> -		hwcaps.mask |= SNOR_HWCAPS_READ_1_1_8;
> -
> -		if (spi->mode & SPI_TX_OCTAL)
> -			hwcaps.mask |= (SNOR_HWCAPS_READ_1_8_8 |
> -					SNOR_HWCAPS_PP_1_1_8 |
> -					SNOR_HWCAPS_PP_1_8_8);
> -	} else if (spi->mode & SPI_RX_QUAD) {
> -		hwcaps.mask |= SNOR_HWCAPS_READ_1_1_4;
> -
> -		if (spi->mode & SPI_TX_QUAD)
> -			hwcaps.mask |= (SNOR_HWCAPS_READ_1_4_4 |
> -					SNOR_HWCAPS_PP_1_1_4 |
> -					SNOR_HWCAPS_PP_1_4_4);
> -	} else if (spi->mode & SPI_RX_DUAL) {
> -		hwcaps.mask |= SNOR_HWCAPS_READ_1_1_2;
> -
> -		if (spi->mode & SPI_TX_DUAL)
> -			hwcaps.mask |= SNOR_HWCAPS_READ_1_2_2;
> -	}
> -
> -	if (data && data->name)
> -		nor->mtd.name = data->name;
> -
> -	if (!nor->mtd.name)
> -		nor->mtd.name = spi_mem_get_name(spimem);
> -
> -	/* For some (historical?) reason many platforms provide two different
> -	 * names in flash_platform_data: "name" and "type". Quite often name is
> -	 * set to "m25p80" and then "type" provides a real chip name.
> -	 * If that's the case, respect "type" and ignore a "name".
> -	 */
> -	if (data && data->type)
> -		flash_name = data->type;
> -	else if (!strcmp(spi->modalias, "spi-nor"))
> -		flash_name = NULL; /* auto-detect */
> -	else
> -		flash_name = spi->modalias;
> -
> -	ret = spi_nor_scan(nor, flash_name, &hwcaps);
> -	if (ret)
> -		return ret;
> -
> -	return mtd_device_register(&nor->mtd, data ? data->parts : NULL,
> -				   data ? data->nr_parts : 0);
> -}
> -
> -
> -static int m25p_remove(struct spi_mem *spimem)
> -{
> -	struct m25p	*flash = spi_mem_get_drvdata(spimem);
> -
> -	spi_nor_restore(&flash->spi_nor);
> -
> -	/* Clean up MTD stuff. */
> -	return mtd_device_unregister(&flash->spi_nor.mtd);
> -}
> -
> -static void m25p_shutdown(struct spi_mem *spimem)
> -{
> -	struct m25p *flash = spi_mem_get_drvdata(spimem);
> -
> -	spi_nor_restore(&flash->spi_nor);
> -}
> -/*
> - * Do NOT add to this array without reading the following:
> - *
> - * Historically, many flash devices are bound to this driver by their name. But
> - * since most of these flash are compatible to some extent, and their
> - * differences can often be differentiated by the JEDEC read-ID command, we
> - * encourage new users to add support to the spi-nor library, and simply bind
> - * against a generic string here (e.g., "jedec,spi-nor").
> - *
> - * Many flash names are kept here in this list (as well as in spi-nor.c) to
> - * keep them available as module aliases for existing platforms.
> - */
> -static const struct spi_device_id m25p_ids[] = {
> -	/*
> -	 * Allow non-DT platform devices to bind to the "spi-nor" modalias, and
> -	 * hack around the fact that the SPI core does not provide uevent
> -	 * matching for .of_match_table
> -	 */
> -	{"spi-nor"},
> -
> -	/*
> -	 * Entries not used in DTs that should be safe to drop after replacing
> -	 * them with "spi-nor" in platform data.
> -	 */
> -	{"s25sl064a"},	{"w25x16"},	{"m25p10"},	{"m25px64"},
> -
> -	/*
> -	 * Entries that were used in DTs without "jedec,spi-nor" fallback and
> -	 * should be kept for backward compatibility.
> -	 */
> -	{"at25df321a"},	{"at25df641"},	{"at26df081a"},
> -	{"mx25l4005a"},	{"mx25l1606e"},	{"mx25l6405d"},	{"mx25l12805d"},
> -	{"mx25l25635e"},{"mx66l51235l"},
> -	{"n25q064"},	{"n25q128a11"},	{"n25q128a13"},	{"n25q512a"},
> -	{"s25fl256s1"},	{"s25fl512s"},	{"s25sl12801"},	{"s25fl008k"},
> -	{"s25fl064k"},
> -	{"sst25vf040b"},{"sst25vf016b"},{"sst25vf032b"},{"sst25wf040"},
> -	{"m25p40"},	{"m25p80"},	{"m25p16"},	{"m25p32"},
> -	{"m25p64"},	{"m25p128"},
> -	{"w25x80"},	{"w25x32"},	{"w25q32"},	{"w25q32dw"},
> -	{"w25q80bl"},	{"w25q128"},	{"w25q256"},
> -
> -	/* Flashes that can't be detected using JEDEC */
> -	{"m25p05-nonjedec"},	{"m25p10-nonjedec"},	{"m25p20-nonjedec"},
> -	{"m25p40-nonjedec"},	{"m25p80-nonjedec"},	{"m25p16-nonjedec"},
> -	{"m25p32-nonjedec"},	{"m25p64-nonjedec"},	{"m25p128-nonjedec"},
> -
> -	/* Everspin MRAMs (non-JEDEC) */
> -	{ "mr25h128" }, /* 128 Kib, 40 MHz */
> -	{ "mr25h256" }, /* 256 Kib, 40 MHz */
> -	{ "mr25h10" },  /*   1 Mib, 40 MHz */
> -	{ "mr25h40" },  /*   4 Mib, 40 MHz */
> -
> -	{ },
> -};
> -MODULE_DEVICE_TABLE(spi, m25p_ids);
> -
> -static const struct of_device_id m25p_of_table[] = {
> -	/*
> -	 * Generic compatibility for SPI NOR that can be identified by the
> -	 * JEDEC READ ID opcode (0x9F). Use this, if possible.
> -	 */
> -	{ .compatible = "jedec,spi-nor" },
> -	{}
> -};
> -MODULE_DEVICE_TABLE(of, m25p_of_table);
> -
> -static struct spi_mem_driver m25p80_driver = {
> -	.spidrv = {
> -		.driver = {
> -			.name	= "m25p80",
> -			.of_match_table = m25p_of_table,
> -		},
> -		.id_table	= m25p_ids,
> -	},
> -	.probe	= m25p_probe,
> -	.remove	= m25p_remove,
> -	.shutdown	= m25p_shutdown,
> -
> -	/* REVISIT: many of these chips have deep power-down modes, which
> -	 * should clearly be entered on suspend() to minimize power use.
> -	 * And also when they're otherwise idle...
> -	 */
> -};
> -
> -module_spi_mem_driver(m25p80_driver);
> -
> -MODULE_LICENSE("GPL");
> -MODULE_AUTHOR("Mike Lavender");
> -MODULE_DESCRIPTION("MTD SPI driver for ST M25Pxx flash chips");
> diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
> index dab986691267..0c1b7851621f 100644
> --- a/drivers/mtd/spi-nor/Kconfig
> +++ b/drivers/mtd/spi-nor/Kconfig
> @@ -1,6 +1,8 @@
>  menuconfig MTD_SPI_NOR
>  	tristate "SPI-NOR device support"
>  	depends on MTD
> +	depends on MTD && SPI_MASTER
> +	select SPI_MEM
>  	help
>  	  This is the framework for the SPI NOR which can be used by the SPI
>  	  device drivers and the SPI-NOR device driver.
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 73172d7f512b..03c8c346c9ae 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -19,6 +19,7 @@
>  
>  #include <linux/mtd/mtd.h>
>  #include <linux/of_platform.h>
> +#include <linux/sched/task_stack.h>
>  #include <linux/spi/flash.h>
>  #include <linux/mtd/spi-nor.h>
>  
> @@ -288,6 +289,174 @@ struct flash_info {
>  
>  #define JEDEC_MFR(info)	((info)->id[0])
>  
> +static int spi_nor_exec_op(struct spi_nor *nor, struct spi_mem_op *op,
> +			   u64 *addr, void *buf, unsigned int len)

size_t for len?

> +{
> +	int ret;
> +	bool usebouncebuf;
> +
> +	if (!op || (len && !buf))
> +		return -EINVAL;
> +
> +	if (op->addr.nbytes && addr)
> +		op->addr.val = *addr;
> +
> +	op->data.nbytes = len;
> +
> +	if (object_is_on_stack(buf) || !virt_addr_valid(buf))
> +		usebouncebuf = true;
> +	if (len && usebouncebuf) {
> +		if (len > nor->bouncebuf_size)
> +			return -ENOTSUPP;
> +
> +		if (op->data.dir == SPI_MEM_DATA_IN) {
> +			op->data.buf.in = nor->bouncebuf;
> +		} else {
> +			op->data.buf.out = nor->bouncebuf;
> +			memcpy(nor->bouncebuf, buf, len);
> +		}
> +	} else {
> +		op->data.buf.out = buf;
> +	}
> +
> +	ret = spi_mem_exec_op(nor->spimem, op);
> +	if (ret)
> +		return ret;
> +
> +	if (usebouncebuf && len && op->data.dir == SPI_MEM_DATA_IN)
> +		memcpy(buf, nor->bouncebuf, len);
> +
> +	return 0;
> +}
> +
> +static int spi_nor_data_op(struct spi_nor *nor, struct spi_mem_op *op,
> +			   void *buf, unsigned int len)

size_t for len?

> +{
> +	return spi_nor_exec_op(nor, op, NULL, buf, len);
> +}
> +
> +static int spi_nor_nodata_op(struct spi_nor *nor, struct spi_mem_op *op)
> +{
> +	return spi_nor_exec_op(nor, op, NULL, NULL, 0);
> +}
> +
> +static ssize_t spi_nor_spimem_read_data(struct spi_nor *nor, loff_t ofs,
> +					size_t len, u8 *buf)
> +{
> +	struct spi_mem_op op = SPI_MEM_OP(SPI_MEM_OP_CMD(nor->read_opcode, 1),
> +					  SPI_MEM_OP_ADDR(nor->addr_width, ofs,
> +							  1),
> +					  SPI_MEM_OP_DUMMY(nor->read_dummy, 1),
> +					  SPI_MEM_OP_DATA_IN(len, buf, 1));
> +	bool usebouncebuf = false;
> +	size_t remaining = len;
> +	int ret;
> +
> +	if (object_is_on_stack(buf) || !virt_addr_valid(buf)) {
> +		usebouncebuf = true;
> +		op.data.buf.in = nor->bouncebuf;
> +	}
> +
> +	/* get transfer protocols. */
> +	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 */
> +	op.dummy.nbytes = (nor->read_dummy * op.dummy.buswidth) / 8;
> +
> +	while (remaining) {

Can't we get rid of this while? See Andrey's patch at
https://lkml.org/lkml/2019/4/1/32. Andrey's patches are not included in this
patch. We should consider integrating his work first, or squash his work in this
patch, or ask him to rework the series after this patch gets accepted. I'll let
you decide.

> +		op.data.nbytes = remaining < UINT_MAX ? remaining : UINT_MAX;
> +		if (!usebouncebuf)
> +			op.data.buf.out = buf;
> +		else if (op.data.nbytes > nor->bouncebuf_size)
> +			op.data.nbytes = nor->bouncebuf_size;
> +
> +		ret = spi_mem_adjust_op_size(nor->spimem, &op);
> +		if (ret)
> +			return ret;
> +
> +		ret = spi_mem_exec_op(nor->spimem, &op);
> +		if (ret)
> +			return ret;
> +
> +		if (usebouncebuf)
> +			memcpy(buf, nor->bouncebuf, op.data.nbytes);
> +
> +		op.addr.val += op.data.nbytes;
> +		remaining -= op.data.nbytes;
> +		buf += op.data.nbytes;
> +	}
> +
> +	return len;
> +}
> +
> +static ssize_t spi_nor_read_data(struct spi_nor *nor, loff_t ofs, size_t len,
> +				 u8 *buf)
> +{
> +	if (nor->spimem)
> +		return spi_nor_spimem_read_data(nor, ofs, len, buf);
> +
> +	return nor->read(nor, ofs, len, buf);
> +}
> +
> +static ssize_t spi_nor_spimem_write_data(struct spi_nor *nor, loff_t ofs,
> +					 size_t len, const u8 *buf)
> +{
> +	struct spi_mem_op op = SPI_MEM_OP(SPI_MEM_OP_CMD(nor->program_opcode,

how about moving SPI_MEM_OP() to the next line to avoid breaking lines below?
General comment.

> +							 1),
> +					  SPI_MEM_OP_ADDR(nor->addr_width, ofs,
> +							  1),
> +					  SPI_MEM_OP_NO_DUMMY,
> +					  SPI_MEM_OP_DATA_OUT(len, NULL, 1));
> +	bool usebouncebuf = false;
> +	int ret;
> +
> +	if (object_is_on_stack(buf) || !virt_addr_valid(buf)) {
> +		usebouncebuf = true;
> +		op.data.buf.out = nor->bouncebuf;
> +	}
> +
> +	/* get transfer protocols. */
> +	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.data.buswidth = spi_nor_get_protocol_data_nbits(nor->write_proto);
> +
> +	if (nor->program_opcode == SPINOR_OP_AAI_WP && nor->sst_write_second)
> +		op.addr.nbytes = 0;
> +
> +	op.data.nbytes = len < UINT_MAX ? len : UINT_MAX;
> +
> +	if (!usebouncebuf) {
> +		op.data.buf.out = buf;
> +	} else {
> +		if (op.data.nbytes > nor->bouncebuf_size)
> +			op.data.nbytes = nor->bouncebuf_size;
> +
> +		memcpy(nor->bouncebuf, buf, op.data.nbytes);
> +	}
> +
> +	ret = spi_mem_adjust_op_size(nor->spimem, &op);
> +	if (ret)
> +		return ret;
> +
> +	ret = spi_mem_exec_op(nor->spimem, &op);
> +	if (ret)
> +		return ret;
> +
> +	return op.data.nbytes;
> +}
> +
> +static ssize_t spi_nor_write_data(struct spi_nor *nor, loff_t ofs, size_t len,
> +				  const u8 *buf)
> +{
> +	if (nor->spimem)
> +		return spi_nor_spimem_write_data(nor, ofs, len, buf);
> +
> +	return nor->write(nor, ofs, len, buf);
> +}
> +
>  /*
>   * Read the status register, returning its value in the location
>   * Return the status register value.
> @@ -298,7 +467,18 @@ static int read_sr(struct spi_nor *nor)
>  	int ret;
>  	u8 val;
>  
> -	ret = nor->read_reg(nor, SPINOR_OP_RDSR, &val, 1);
> +	if (nor->spimem) {
> +		struct spi_mem_op op = SPI_MEM_OP(
> +					SPI_MEM_OP_CMD(SPINOR_OP_RDSR, 1),
> +					SPI_MEM_OP_NO_ADDR,
> +					SPI_MEM_OP_NO_DUMMY,
> +					SPI_MEM_OP_DATA_IN(0, NULL, 1));
> +
> +		ret = spi_nor_data_op(nor, &op, &val, 1);
> +	} else {
> +		ret = nor->read_reg(nor, SPINOR_OP_RDSR, &val, 1);
> +	}
> +
>  	if (ret < 0) {
>  		pr_err("error %d reading SR\n", (int) ret);
>  		return ret;
> @@ -317,7 +497,18 @@ static int read_fsr(struct spi_nor *nor)
>  	int ret;
>  	u8 val;
>  
> -	ret = nor->read_reg(nor, SPINOR_OP_RDFSR, &val, 1);
> +	if (nor->spimem) {
> +		struct spi_mem_op op = SPI_MEM_OP(
> +					SPI_MEM_OP_CMD(SPINOR_OP_RDFSR, 1),
> +					SPI_MEM_OP_NO_ADDR,
> +					SPI_MEM_OP_NO_DUMMY,
> +					SPI_MEM_OP_DATA_IN(0, NULL, 1));
> +
> +		ret = spi_nor_data_op(nor, &op, &val, 1);
> +	} else {
> +		ret = nor->read_reg(nor, SPINOR_OP_RDFSR, &val, 1);
> +	}
> +
>  	if (ret < 0) {
>  		pr_err("error %d reading FSR\n", ret);
>  		return ret;
> @@ -336,7 +527,18 @@ static int read_cr(struct spi_nor *nor)
>  	int ret;
>  	u8 val;
>  
> -	ret = nor->read_reg(nor, SPINOR_OP_RDCR, &val, 1);
> +	if (nor->spimem) {
> +		struct spi_mem_op op = SPI_MEM_OP(
> +					SPI_MEM_OP_CMD(SPINOR_OP_RDCR, 1),
> +					SPI_MEM_OP_NO_ADDR,
> +					SPI_MEM_OP_NO_DUMMY,
> +					SPI_MEM_OP_DATA_IN(0, NULL, 1));
> +
> +		ret = spi_nor_data_op(nor, &op, &val, 1);
> +	} else {
> +		ret = nor->read_reg(nor, SPINOR_OP_RDCR, &val, 1);
> +	}
> +
>  	if (ret < 0) {
>  		dev_err(nor->dev, "error %d reading CR\n", ret);
>  		return ret;
> @@ -351,6 +553,16 @@ static int read_cr(struct spi_nor *nor)
>   */
>  static int write_sr(struct spi_nor *nor, u8 val)
>  {
> +	if (nor->spimem) {
> +		struct spi_mem_op op = SPI_MEM_OP(
> +					SPI_MEM_OP_CMD(SPINOR_OP_WRSR, 1),
> +					SPI_MEM_OP_NO_ADDR,
> +					SPI_MEM_OP_NO_DUMMY,
> +					SPI_MEM_OP_DATA_IN(1, NULL, 1));
> +
> +		return spi_nor_data_op(nor, &op, &val, 1);
> +	}
> +
>  	nor->cmd_buf[0] = val;
>  	return nor->write_reg(nor, SPINOR_OP_WRSR, nor->cmd_buf, 1);
>  }
> @@ -361,6 +573,16 @@ static int write_sr(struct spi_nor *nor, u8 val)
>   */
>  static int write_enable(struct spi_nor *nor)
>  {
> +	if (nor->spimem) {
> +		struct spi_mem_op op = SPI_MEM_OP(
> +					SPI_MEM_OP_CMD(SPINOR_OP_WREN, 1),
> +					SPI_MEM_OP_NO_ADDR,
> +					SPI_MEM_OP_NO_DUMMY,
> +					SPI_MEM_OP_NO_DATA);
> +
> +		return spi_nor_nodata_op(nor, &op);
> +	}
> +
>  	return nor->write_reg(nor, SPINOR_OP_WREN, NULL, 0);
>  }
>  
> @@ -369,6 +591,16 @@ static int write_enable(struct spi_nor *nor)
>   */
>  static int write_disable(struct spi_nor *nor)
>  {
> +	if (nor->spimem) {
> +		struct spi_mem_op op = SPI_MEM_OP(
> +					SPI_MEM_OP_CMD(SPINOR_OP_WRDI, 1),
> +					SPI_MEM_OP_NO_ADDR,
> +					SPI_MEM_OP_NO_DUMMY,
> +					SPI_MEM_OP_NO_DATA);
> +
> +		return spi_nor_nodata_op(nor, &op);
> +	}
> +
>  	return nor->write_reg(nor, SPINOR_OP_WRDI, NULL, 0);
>  }
>  
> @@ -468,12 +700,46 @@ static void spi_nor_set_4byte_opcodes(struct spi_nor *nor)
>  	}
>  }
>  
> +static int macronix_set_4byte(struct spi_nor *nor, bool enable)
> +{
> +	if (nor->spimem) {
> +		struct spi_mem_op op = SPI_MEM_OP(
> +					SPI_MEM_OP_CMD(enable ?
> +						       SPINOR_OP_EN4B :
> +						       SPINOR_OP_EX4B,
> +						       1),
> +					SPI_MEM_OP_NO_ADDR,
> +					SPI_MEM_OP_NO_DUMMY,
> +					SPI_MEM_OP_NO_DATA);
> +
> +		return spi_nor_nodata_op(nor, &op);
> +	}
> +
> +	return nor->write_reg(nor, enable ? SPINOR_OP_EN4B : SPINOR_OP_EX4B,
> +			      NULL, 0);
> +}
> +
> +static int write_ear(struct spi_nor *nor, u8 ear)

spi_nor_write_ear?

> +{
> +	if (nor->spimem) {
> +		struct spi_mem_op op = SPI_MEM_OP(
> +					SPI_MEM_OP_CMD(SPINOR_OP_WREAR, 1),
> +					SPI_MEM_OP_NO_ADDR,
> +					SPI_MEM_OP_NO_DUMMY,
> +					SPI_MEM_OP_DATA_OUT(0, NULL, 1));
> +
> +		return spi_nor_data_op(nor, &op, &ear, 1);
> +	}
> +
> +	nor->cmd_buf[0] = ear;

how about dropping this init and
	return nor->write_reg(nor, SPINOR_OP_WREAR, &ear, 1);

> +	return nor->write_reg(nor, SPINOR_OP_WREAR, nor->cmd_buf, 1);
> +}
> +
>  /* Enable/disable 4-byte addressing mode. */
>  static int set_4byte(struct spi_nor *nor, bool enable)
>  {
>  	int status;
>  	bool need_wren = false;
> -	u8 cmd;
>  
>  	switch (JEDEC_MFR(nor->info)) {
>  	case SNOR_MFR_ST:
> @@ -486,8 +752,7 @@ static int set_4byte(struct spi_nor *nor, bool enable)
>  		if (need_wren)
>  			write_enable(nor);
>  
> -		cmd = enable ? SPINOR_OP_EN4B : SPINOR_OP_EX4B;
> -		status = nor->write_reg(nor, cmd, NULL, 0);
> +		status = macronix_set_4byte(nor, enable);
>  		if (need_wren)
>  			write_disable(nor);
>  
> @@ -500,8 +765,7 @@ static int set_4byte(struct spi_nor *nor, bool enable)
>  			 * We must clear the register to enable normal behavior.
>  			 */
>  			write_enable(nor);
> -			nor->cmd_buf[0] = 0;
> -			nor->write_reg(nor, SPINOR_OP_WREAR, nor->cmd_buf, 1);
> +			write_ear(nor, 0);
>  			write_disable(nor);
>  		}
>  
> @@ -509,16 +773,42 @@ static int set_4byte(struct spi_nor *nor, bool enable)
>  	default:
>  		/* Spansion style */

how about introducing a spansion_set_4byte() and return it directly?

>  		nor->cmd_buf[0] = enable << 7;
> +
> +		if (nor->spimem) {
> +			struct spi_mem_op op = SPI_MEM_OP(
> +					SPI_MEM_OP_CMD(SPINOR_OP_BRWR, 1),
> +					SPI_MEM_OP_NO_ADDR,
> +					SPI_MEM_OP_NO_DUMMY,
> +					SPI_MEM_OP_DATA_OUT(1, NULL, 1));
> +
> +			return spi_nor_data_op(nor, &op, nor->cmd_buf, 1);
> +	}
> +
>  		return nor->write_reg(nor, SPINOR_OP_BRWR, nor->cmd_buf, 1);
>  	}
>  }
>  
> +static int xread_sr(struct spi_nor *nor, u8 *sr)

spi_nor_xread_sr?

> +{
> +	if (nor->spimem) {
> +		struct spi_mem_op op = SPI_MEM_OP(
> +					SPI_MEM_OP_CMD(SPINOR_OP_XRDSR, 1),
> +					SPI_MEM_OP_NO_ADDR,
> +					SPI_MEM_OP_NO_DUMMY,
> +					SPI_MEM_OP_DATA_IN(0, NULL, 1));
> +
> +		return spi_nor_data_op(nor, &op, sr, 1);
> +	}
> +
> +	return nor->read_reg(nor, SPINOR_OP_XRDSR, sr, 1);
> +}
> +
>  static int s3an_sr_ready(struct spi_nor *nor)
>  {
>  	int ret;
>  	u8 val;
>  
> -	ret = nor->read_reg(nor, SPINOR_OP_XRDSR, &val, 1);
> +	ret = xread_sr(nor, &val);
>  	if (ret < 0) {
>  		dev_err(nor->dev, "error %d reading XRDSR\n", (int) ret);
>  		return ret;
> @@ -527,6 +817,21 @@ static int s3an_sr_ready(struct spi_nor *nor)
>  	return !!(val & XSR_RDY);
>  }
>  
> +static int clear_sr(struct spi_nor *nor)

spi_nor_clear_sr?

> +{
> +	if (nor->spimem) {
> +		struct spi_mem_op op = SPI_MEM_OP(
> +					SPI_MEM_OP_CMD(SPINOR_OP_CLSR, 1),
> +					SPI_MEM_OP_NO_ADDR,
> +					SPI_MEM_OP_NO_DUMMY,
> +					SPI_MEM_OP_NO_DATA);
> +
> +		return spi_nor_nodata_op(nor, &op);
> +	}
> +
> +	return nor->write_reg(nor, SPINOR_OP_CLSR, NULL, 0);
> +}
> +
>  static int spi_nor_sr_ready(struct spi_nor *nor)
>  {
>  	int sr = read_sr(nor);
> @@ -539,13 +844,28 @@ static int spi_nor_sr_ready(struct spi_nor *nor)
>  		else
>  			dev_err(nor->dev, "Programming Error occurred\n");
>  
> -		nor->write_reg(nor, SPINOR_OP_CLSR, NULL, 0);
> +		clear_sr(nor);
>  		return -EIO;
>  	}
>  
>  	return !(sr & SR_WIP);
>  }
>  
> +static int clear_fsr(struct spi_nor *nor)

spi_nor_clear_fsr?

> +{
> +	if (nor->spimem) {
> +		struct spi_mem_op op = SPI_MEM_OP(
> +					SPI_MEM_OP_CMD(SPINOR_OP_CLFSR, 1),
> +					SPI_MEM_OP_NO_ADDR,
> +					SPI_MEM_OP_NO_DUMMY,
> +					SPI_MEM_OP_NO_DATA);
> +
> +		return spi_nor_nodata_op(nor, &op);
> +	}
> +
> +	return nor->write_reg(nor, SPINOR_OP_CLFSR, NULL, 0);
> +}
> +
>  static int spi_nor_fsr_ready(struct spi_nor *nor)
>  {
>  	int fsr = read_fsr(nor);
> @@ -562,7 +882,7 @@ static int spi_nor_fsr_ready(struct spi_nor *nor)
>  			dev_err(nor->dev,
>  			"Attempted to modify a protected sector.\n");
>  
> -		nor->write_reg(nor, SPINOR_OP_CLFSR, NULL, 0);
> +		clear_fsr(nor);
>  		return -EIO;
>  	}
>  
> @@ -630,6 +950,16 @@ static int erase_chip(struct spi_nor *nor)
>  {
>  	dev_dbg(nor->dev, " %lldKiB\n", (long long)(nor->mtd.size >> 10));
>  
> +	if (nor->spimem) {
> +		struct spi_mem_op op = SPI_MEM_OP(
> +					SPI_MEM_OP_CMD(SPINOR_OP_CHIP_ERASE, 1),
> +					SPI_MEM_OP_NO_ADDR,
> +					SPI_MEM_OP_NO_DUMMY,
> +					SPI_MEM_OP_NO_DATA);
> +
> +		return spi_nor_nodata_op(nor, &op);
> +	}
> +
>  	return nor->write_reg(nor, SPINOR_OP_CHIP_ERASE, NULL, 0);
>  }
>  
> @@ -692,6 +1022,17 @@ static int spi_nor_erase_sector(struct spi_nor *nor, u32 addr)
>  	if (nor->erase)
>  		return nor->erase(nor, addr);
>  
> +	if (nor->spimem) {
> +		struct spi_mem_op op = SPI_MEM_OP(
> +					SPI_MEM_OP_CMD(nor->erase_opcode, 1),
> +					SPI_MEM_OP_ADDR(nor->addr_width, addr,
> +							1),
> +					SPI_MEM_OP_NO_DUMMY,
> +					SPI_MEM_OP_NO_DATA);
> +
> +		return spi_nor_nodata_op(nor, &op);
> +	}
> +
>  	/*
>  	 * Default implementation, if driver doesn't have a specialized HW
>  	 * control
> @@ -1406,7 +1747,18 @@ static int write_sr_cr(struct spi_nor *nor, u8 *sr_cr)
>  
>  	write_enable(nor);
>  
> -	ret = nor->write_reg(nor, SPINOR_OP_WRSR, sr_cr, 2);
> +	if (nor->spimem) {
> +		struct spi_mem_op op = SPI_MEM_OP(
> +					SPI_MEM_OP_CMD(SPINOR_OP_WRSR, 1),
> +					SPI_MEM_OP_NO_ADDR,
> +					SPI_MEM_OP_NO_DUMMY,
> +					SPI_MEM_OP_DATA_OUT(0, NULL, 1));
> +
> +		ret = spi_nor_data_op(nor, &op, sr_cr, 2);
> +	} else {
> +		ret = nor->write_reg(nor, SPINOR_OP_WRSR, sr_cr, 2);
> +	}
> +
>  	if (ret < 0) {
>  		dev_err(nor->dev,
>  			"error while writing configuration register\n");
> @@ -1585,6 +1937,36 @@ static int spansion_read_cr_quad_enable(struct spi_nor *nor)
>  	return 0;
>  }
>  
> +static int write_sr2(struct spi_nor *nor, u8 sr2)

spi_nor_write_sr2?

> +{
> +	if (nor->spimem) {
> +		struct spi_mem_op op = SPI_MEM_OP(
> +					SPI_MEM_OP_CMD(SPINOR_OP_WRSR2, 1),
> +					SPI_MEM_OP_NO_ADDR,
> +					SPI_MEM_OP_NO_DUMMY,
> +					SPI_MEM_OP_DATA_OUT(0, NULL, 1));
> +
> +		return spi_nor_data_op(nor, &op, &sr2, 1);
> +	}
> +
> +	return nor->write_reg(nor, SPINOR_OP_WRSR2, &sr2, 1);
> +}
> +
> +static int read_sr2(struct spi_nor *nor, u8 *sr2)

spi_nor_read_sr2?

> +{
> +	if (nor->spimem) {
> +		struct spi_mem_op op = SPI_MEM_OP(
> +					SPI_MEM_OP_CMD(SPINOR_OP_RDSR2, 1),
> +					SPI_MEM_OP_NO_ADDR,
> +					SPI_MEM_OP_NO_DUMMY,
> +					SPI_MEM_OP_DATA_IN(0, NULL, 1));
> +
> +		return spi_nor_data_op(nor, &op, sr2, 1);
> +	}
> +
> +	return nor->read_reg(nor, SPINOR_OP_RDSR2, sr2, 1);
> +}
> +
>  /**
>   * sr2_bit7_quad_enable() - set QE bit in Status Register 2.
>   * @nor:	pointer to a 'struct spi_nor'
> @@ -1603,7 +1985,7 @@ static int sr2_bit7_quad_enable(struct spi_nor *nor)
>  	int ret;
>  
>  	/* Check current Quad Enable bit value. */
> -	ret = nor->read_reg(nor, SPINOR_OP_RDSR2, &sr2, 1);
> +	ret = read_sr2(nor, &sr2);
>  	if (ret)
>  		return ret;
>  	if (sr2 & SR2_QUAD_EN_BIT7)
> @@ -1614,7 +1996,7 @@ static int sr2_bit7_quad_enable(struct spi_nor *nor)
>  
>  	write_enable(nor);
>  
> -	ret = nor->write_reg(nor, SPINOR_OP_WRSR2, &sr2, 1);
> +	ret = write_sr2(nor, sr2);
>  	if (ret < 0) {
>  		dev_err(nor->dev, "error while writing status register 2\n");
>  		return -EINVAL;
> @@ -1626,8 +2008,7 @@ static int sr2_bit7_quad_enable(struct spi_nor *nor)
>  		return ret;
>  	}
>  
> -	/* Read back and check it. */
> -	ret = nor->read_reg(nor, SPINOR_OP_RDSR2, &sr2, 1);
> +	ret = read_sr2(nor, &sr2);
>  	if (!(ret > 0 && (sr2 & SR2_QUAD_EN_BIT7))) {
>  		dev_err(nor->dev, "SR2 Quad bit not set\n");
>  		return -EINVAL;
> @@ -2054,13 +2435,28 @@ static const struct flash_info spi_nor_ids[] = {
>  	{ },
>  };
>  
> +static int read_id(struct spi_nor *nor, u8 *id)

this function is called just from spi_nor_read_id(). How about incorporating
this code into spi_nor_read_id() so that we use functions that are prepended
with "spi_nor" to not pollute the namespace?
> +{
> +	if (nor->spimem) {
> +		struct spi_mem_op op = SPI_MEM_OP(
> +					SPI_MEM_OP_CMD(SPINOR_OP_RDID, 1),
> +					SPI_MEM_OP_NO_ADDR,
> +					SPI_MEM_OP_NO_DUMMY,
> +					SPI_MEM_OP_DATA_IN(0, NULL, 1));
> +
> +		return spi_nor_data_op(nor, &op, id, SPI_NOR_MAX_ID_LEN);
> +	}
> +
> +	return nor->read_reg(nor, SPINOR_OP_RDID, id, SPI_NOR_MAX_ID_LEN);
> +}
> +
>  static const struct flash_info *spi_nor_read_id(struct spi_nor *nor)
>  {
>  	int			tmp;
>  	u8			id[SPI_NOR_MAX_ID_LEN];
>  	const struct flash_info	*info;
>  
> -	tmp = nor->read_reg(nor, SPINOR_OP_RDID, id, SPI_NOR_MAX_ID_LEN);
> +	tmp = read_id(nor, id);
>  	if (tmp < 0) {
>  		dev_dbg(nor->dev, "error %d reading JEDEC ID\n", tmp);
>  		return ERR_PTR(tmp);
> @@ -2096,7 +2492,7 @@ static int spi_nor_read(struct mtd_info *mtd, loff_t from, size_t len,
>  		if (nor->flags & SNOR_F_S3AN_ADDR_DEFAULT)
>  			addr = spi_nor_s3an_addr_convert(nor, addr);
>  
> -		ret = nor->read(nor, addr, len, buf);
> +		ret = spi_nor_read_data(nor, addr, len, buf);
>  		if (ret == 0) {
>  			/* We shouldn't see 0-length reads */
>  			ret = -EIO;
> @@ -2141,7 +2537,7 @@ static int sst_write(struct mtd_info *mtd, loff_t to, size_t len,
>  		nor->program_opcode = SPINOR_OP_BP;
>  
>  		/* write one byte. */
> -		ret = nor->write(nor, to, 1, buf);
> +		ret = spi_nor_write_data(nor, to, 1, buf);
>  		if (ret < 0)
>  			goto sst_write_err;
>  		WARN(ret != 1, "While writing 1 byte written %i bytes\n",
> @@ -2157,7 +2553,7 @@ static int sst_write(struct mtd_info *mtd, loff_t to, size_t len,
>  		nor->program_opcode = SPINOR_OP_AAI_WP;
>  
>  		/* write two bytes. */
> -		ret = nor->write(nor, to, 2, buf + actual);
> +		ret = spi_nor_write_data(nor, to, 2, buf + actual);
>  		if (ret < 0)
>  			goto sst_write_err;
>  		WARN(ret != 2, "While writing 2 bytes written %i bytes\n",
> @@ -2180,7 +2576,7 @@ static int sst_write(struct mtd_info *mtd, loff_t to, size_t len,
>  		write_enable(nor);
>  
>  		nor->program_opcode = SPINOR_OP_BP;
> -		ret = nor->write(nor, to, 1, buf + actual);
> +		ret = spi_nor_write_data(nor, to, 1, buf + actual);
>  		if (ret < 0)
>  			goto sst_write_err;
>  		WARN(ret != 1, "While writing 1 byte written %i bytes\n",
> @@ -2242,7 +2638,7 @@ static int spi_nor_write(struct mtd_info *mtd, loff_t to, size_t len,
>  			addr = spi_nor_s3an_addr_convert(nor, addr);
>  
>  		write_enable(nor);
> -		ret = nor->write(nor, addr, page_remain, buf + i);
> +		ret = spi_nor_write_data(nor, addr, page_remain, buf + i);
>  		if (ret < 0)
>  			goto write_err;
>  		written = ret;
> @@ -2261,8 +2657,10 @@ static int spi_nor_write(struct mtd_info *mtd, loff_t to, size_t len,
>  
>  static int spi_nor_check(struct spi_nor *nor)
>  {
> -	if (!nor->dev || !nor->read || !nor->write ||
> -		!nor->read_reg || !nor->write_reg) {
> +	if (!nor->dev ||
> +	    (!nor->spimem &&
> +	    (!nor->read || !nor->write || !nor->read_reg ||
> +	      !nor->write_reg))) {
>  		pr_err("spi-nor: please fill all the necessary fields!\n");
>  		return -EINVAL;
>  	}
> @@ -2275,7 +2673,7 @@ static int s3an_nor_scan(struct spi_nor *nor)
>  	int ret;
>  	u8 val;
>  
> -	ret = nor->read_reg(nor, SPINOR_OP_XRDSR, &val, 1);
> +	ret = xread_sr(nor, &val);
>  	if (ret < 0) {
>  		dev_err(nor->dev, "error %d reading XRDSR\n", (int) ret);
>  		return ret;
> @@ -2405,7 +2803,7 @@ static int spi_nor_read_raw(struct spi_nor *nor, u32 addr, size_t len, u8 *buf)
>  	int ret;
>  
>  	while (len) {
> -		ret = nor->read(nor, addr, len, buf);
> +		ret = spi_nor_read_data(nor, addr, len, buf);
>  		if (!ret || ret > len)
>  			return -EIO;
>  		if (ret < 0)
> @@ -4188,6 +4586,215 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
>  }
>  EXPORT_SYMBOL_GPL(spi_nor_scan);
>  
> +static int spi_nor_probe(struct spi_mem *spimem)
> +{
> +	struct spi_device *spi = spimem->spi;
> +	struct flash_platform_data *data;
> +	struct spi_nor *nor;
> +	struct spi_nor_hwcaps hwcaps = {
> +		.mask = SNOR_HWCAPS_READ |
> +			SNOR_HWCAPS_READ_FAST |
> +			SNOR_HWCAPS_PP,
> +	};
> +	char *flash_name;
> +	int ret;
> +
> +	data = dev_get_platdata(&spi->dev);

you can do the initialization at declaration

> +
> +	nor = devm_kzalloc(&spi->dev, sizeof(*nor), GFP_KERNEL);
> +	if (!nor)
> +		return -ENOMEM;
> +
> +	/*
> +	 * We need the bounce buffer early to read/write registers when going
> +	 * through the spi-mem layer (buffers have to be DMA-able).
> +	 * We'll reallocate a new buffer if nor->page_size turns out to be
> +	 * greater than PAGE_SIZE (which shouldn't happen before long since NOR
> +	 * pages are usually less than 1KB) after spi_nor_scan() returns.
> +	 */
> +	nor->bouncebuf_size = PAGE_SIZE;
> +	nor->bouncebuf = devm_kmalloc(&spi->dev, nor->bouncebuf_size,
> +				      GFP_KERNEL);
> +	if (!nor->bouncebuf)
> +		return -ENOMEM;
> +
> +	nor->spimem = spimem;
> +	nor->dev = &spi->dev;
> +	spi_nor_set_flash_node(nor, spi->dev.of_node);
> +
> +	spi_mem_set_drvdata(spimem, nor);
> +
> +	if (spi->mode & SPI_RX_OCTAL) {
> +		hwcaps.mask |= SNOR_HWCAPS_READ_1_1_8;
> +
> +		if (spi->mode & SPI_TX_OCTAL)
> +			hwcaps.mask |= (SNOR_HWCAPS_READ_1_8_8 |
> +					SNOR_HWCAPS_PP_1_1_8 |
> +					SNOR_HWCAPS_PP_1_8_8);
> +	} else if (spi->mode & SPI_RX_QUAD) {
> +		hwcaps.mask |= SNOR_HWCAPS_READ_1_1_4;
> +
> +		if (spi->mode & SPI_TX_QUAD)
> +			hwcaps.mask |= (SNOR_HWCAPS_READ_1_4_4 |
> +					SNOR_HWCAPS_PP_1_1_4 |
> +					SNOR_HWCAPS_PP_1_4_4);
> +	} else if (spi->mode & SPI_RX_DUAL) {
> +		hwcaps.mask |= SNOR_HWCAPS_READ_1_1_2;
> +
> +		if (spi->mode & SPI_TX_DUAL)
> +			hwcaps.mask |= SNOR_HWCAPS_READ_1_2_2;
> +	}
> +
> +	if (data && data->name)
> +		nor->mtd.name = data->name;
> +
> +	if (!nor->mtd.name)
> +		nor->mtd.name = spi_mem_get_name(spimem);
> +
> +	/*
> +	 * For some (historical?) reason many platforms provide two different
> +	 * names in flash_platform_data: "name" and "type". Quite often name is
> +	 * set to "m25p80" and then "type" provides a real chip name.
> +	 * If that's the case, respect "type" and ignore a "name".
> +	 */
> +	if (data && data->type)
> +		flash_name = data->type;
> +	else if (!strcmp(spi->modalias, "spi-nor"))
> +		flash_name = NULL; /* auto-detect */
> +	else
> +		flash_name = spi->modalias;
> +
> +	ret = spi_nor_scan(nor, flash_name, &hwcaps);
> +	if (ret)
> +		return ret;
> +
> +	/*
> +	 * None of the existing parts have > 512B pages, but let's play safe
> +	 * and add this logic so that if anyone ever adds support for such
> +	 * a NOR we don't end up with buffer overflows.
> +	 */
> +	if (nor->page_size > PAGE_SIZE) {
> +		nor->bouncebuf_size = nor->page_size;
> +		devm_kfree(nor->dev, nor->bouncebuf);
> +		nor->bouncebuf = devm_kmalloc(nor->dev,
> +					      nor->bouncebuf_size,
> +					      GFP_KERNEL);
> +		if (!nor->bouncebuf)
> +			return -ENOMEM;
> +	}
> +
> +	return mtd_device_register(&nor->mtd, data ? data->parts : NULL,
> +				   data ? data->nr_parts : 0);
> +}
> +
> +static int spi_nor_remove(struct spi_mem *spimem)
> +{
> +	struct spi_nor *nor = spi_mem_get_drvdata(spimem);
> +	int ret;
> +
> +	spi_nor_restore(nor);
> +
> +	/* Clean up MTD stuff. */
> +	ret = mtd_device_unregister(&nor->mtd);
> +	if (ret)
> +		return ret;> +
> +	return 0;

return mtd_device_unregister(&nor->mtd); directly

> +}
> +
> +static void spi_nor_shutdown(struct spi_mem *spimem)
> +{
> +	struct spi_nor *nor = spi_mem_get_drvdata(spimem);
> +
> +	spi_nor_restore(nor);
> +}
> +
> +/*
> + * Do NOT add to this array without reading the following:
> + *
> + * Historically, many flash devices are bound to this driver by their name. But
> + * since most of these flash are compatible to some extent, and their
> + * differences can often be differentiated by the JEDEC read-ID command, we
> + * encourage new users to add support to the spi-nor library, and simply bind
> + * against a generic string here (e.g., "jedec,spi-nor").
> + *
> + * Many flash names are kept here in this list (as well as in spi-nor.c) to
> + * keep them available as module aliases for existing platforms.
> + */
> +static const struct spi_device_id spi_nor_dev_ids[] = {
> +	/*
> +	 * Allow non-DT platform devices to bind to the "spi-nor" modalias, and
> +	 * hack around the fact that the SPI core does not provide uevent
> +	 * matching for .of_match_table
> +	 */
> +	{"spi-nor"},
> +
> +	/*
> +	 * Entries not used in DTs that should be safe to drop after replacing
> +	 * them with "spi-nor" in platform data.
> +	 */
> +	{"s25sl064a"},	{"w25x16"},	{"m25p10"},	{"m25px64"},
> +
> +	/*
> +	 * Entries that were used in DTs without "jedec,spi-nor" fallback and
> +	 * should be kept for backward compatibility.
> +	 */
> +	{"at25df321a"},	{"at25df641"},	{"at26df081a"},
> +	{"mx25l4005a"},	{"mx25l1606e"},	{"mx25l6405d"},	{"mx25l12805d"},
> +	{"mx25l25635e"},{"mx66l51235l"},
> +	{"n25q064"},	{"n25q128a11"},	{"n25q128a13"},	{"n25q512a"},
> +	{"s25fl256s1"},	{"s25fl512s"},	{"s25sl12801"},	{"s25fl008k"},
> +	{"s25fl064k"},
> +	{"sst25vf040b"},{"sst25vf016b"},{"sst25vf032b"},{"sst25wf040"},
> +	{"m25p40"},	{"m25p80"},	{"m25p16"},	{"m25p32"},
> +	{"m25p64"},	{"m25p128"},
> +	{"w25x80"},	{"w25x32"},	{"w25q32"},	{"w25q32dw"},
> +	{"w25q80bl"},	{"w25q128"},	{"w25q256"},
> +
> +	/* Flashes that can't be detected using JEDEC */
> +	{"m25p05-nonjedec"},	{"m25p10-nonjedec"},	{"m25p20-nonjedec"},
> +	{"m25p40-nonjedec"},	{"m25p80-nonjedec"},	{"m25p16-nonjedec"},
> +	{"m25p32-nonjedec"},	{"m25p64-nonjedec"},	{"m25p128-nonjedec"},
> +
> +	/* Everspin MRAMs (non-JEDEC) */
> +	{ "mr25h128" }, /* 128 Kib, 40 MHz */
> +	{ "mr25h256" }, /* 256 Kib, 40 MHz */
> +	{ "mr25h10" },  /*   1 Mib, 40 MHz */
> +	{ "mr25h40" },  /*   4 Mib, 40 MHz */
> +
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(spi, spi_nor_dev_ids);
> +
> +static const struct of_device_id spi_nor_of_table[] = {
> +	/*
> +	 * Generic compatibility for SPI NOR that can be identified by the
> +	 * JEDEC READ ID opcode (0x9F). Use this, if possible.
> +	 */
> +	{ .compatible = "jedec,spi-nor" },
> +	{ /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, spi_nor_of_table);
> +
> +/*
> + * REVISIT: many of these chips have deep power-down modes, which
> + * should clearly be entered on suspend() to minimize power use.
> + * And also when they're otherwise idle...
> + */
> +static struct spi_mem_driver spi_nor_driver = {
> +	.spidrv = {
> +		.driver = {
> +			.name = "spi-nor",
> +			.of_match_table = spi_nor_of_table,
> +		},
> +		.id_table = spi_nor_dev_ids,
> +	},
> +	.probe = spi_nor_probe,
> +	.remove = spi_nor_remove,
> +	.shutdown = spi_nor_shutdown,
> +};
> +module_spi_mem_driver(spi_nor_driver);
> +
>  MODULE_LICENSE("GPL v2");
>  MODULE_AUTHOR("Huang Shijie <shijie8@gmail.com>");
>  MODULE_AUTHOR("Mike Lavender");
> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
> index b3d360b0ee3d..ac16745f5ef8 100644
> --- a/include/linux/mtd/spi-nor.h
> +++ b/include/linux/mtd/spi-nor.h
> @@ -9,6 +9,7 @@
>  #include <linux/bitops.h>
>  #include <linux/mtd/cfi.h>
>  #include <linux/mtd/mtd.h>
> +#include <linux/spi/spi-mem.h>
>  
>  /*
>   * Manufacturer IDs
> @@ -344,6 +345,10 @@ struct flash_info;
>   * @mtd:		point to a mtd_info structure
>   * @lock:		the lock for the read/write/erase/lock/unlock operations
>   * @dev:		point to a spi device, or a spi nor controller device.
> + * @spimem:		point to the spi mem device
> + * @bouncebuf:		bounce buffer used when the buffer passed by the MTD
> + *                      layer is not DMA-able
> + * @bouncebuf_size:	size of the bounce buffe
                                           ^buffer

>   * @info:		spi-nor part JDEC MFR id and other info
>   * @page_size:		the page size of the SPI NOR
>   * @addr_width:		number of address bytes
> @@ -380,6 +385,9 @@ struct spi_nor {
>  	struct mtd_info		mtd;
>  	struct mutex		lock;
>  	struct device		*dev;
> +	struct spi_mem		*spimem;
> +	void			*bouncebuf;
> +	unsigned int		bouncebuf_size;

size_t?

Please run checkpatch --strict over the patch, there are few things to fix.

Do you think it is worth documenting the new functions?

Cheers,
ta
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* Re: [PATCH 2/2] mtd: spi-nor: Rework hwcaps selection for the spi-mem case
  2019-04-09 16:26 ` [PATCH 2/2] mtd: spi-nor: Rework hwcaps selection for the spi-mem case Vignesh Raghavendra
@ 2019-04-15 11:59   ` Tudor.Ambarus
  0 siblings, 0 replies; 6+ messages in thread
From: Tudor.Ambarus @ 2019-04-15 11:59 UTC (permalink / raw)
  To: vigneshr, bbrezillon, marek.vasut, richard
  Cc: frieder.schrempf, yogeshnarayan.gaur, linux-mtd, linux-kernel,
	miquel.raynal

Hi,

On 04/09/2019 07:26 PM, Vignesh Raghavendra wrote:
> External E-Mail
> 
> 
> From: Boris Brezillon <boris.brezillon@bootlin.com>
> 
> The spi-mem layer provides a spi_mem_supports_op() function to check
> whether a specific operation is supported by the controller or not.
> This is much more accurate than the hwcaps selection logic based on
> SPI_{RX,TX}_ flags.
> 
> Rework the hwcaps selection logic to use spi_mem_supports_op() when
> nor->spimem != NULL.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
> Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com>
> ---
> Chagnes wrt RFC:
> Fix checkpatch issues
> Rebase onto latest
> 
>  drivers/mtd/spi-nor/spi-nor.c | 164 ++++++++++++++++++++++++++--------
>  include/linux/mtd/spi-nor.h   |  14 +++
>  2 files changed, 142 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 03c8c346c9ae..d48ae3c9661d 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -2852,6 +2852,110 @@ static int spi_nor_read_sfdp(struct spi_nor *nor, u32 addr,
>  	return ret;
>  }
>  
> +static int spi_nor_spimem_check_readop(struct spi_nor *nor,
> +				       const struct spi_nor_read_command *read)
> +{
> +	struct spi_mem_op op = SPI_MEM_OP(SPI_MEM_OP_CMD(read->opcode, 1),
> +					  SPI_MEM_OP_ADDR(3, 0, 1),
> +					  SPI_MEM_OP_DUMMY(0, 1),
> +					  SPI_MEM_OP_DATA_IN(0, NULL, 1));
> +
> +	op.cmd.buswidth = spi_nor_get_protocol_inst_nbits(read->proto);
> +	op.addr.buswidth = spi_nor_get_protocol_addr_nbits(read->proto);
> +	op.data.buswidth = spi_nor_get_protocol_data_nbits(read->proto);
> +	op.dummy.buswidth = op.addr.buswidth;
> +	op.dummy.nbytes = (read->num_mode_clocks + read->num_wait_states) *
> +			  op.dummy.buswidth / 8;
> +
> +	/*
> +	 * First test with 3 address bytes. The opcode itself might already
> +	 * be a 4B addressing opcode but we don't care, because SPI controller
> +	 * implementation should not check the opcode, but just the sequence.
> +	 */
> +	if (!spi_mem_supports_op(nor->spimem, &op))
> +		return -ENOTSUPP;
> +
> +	/* Now test with 4 address bytes. */

shouldn't this be verified only when the previous check failed?

> +	op.addr.nbytes = 4;
> +	if (!spi_mem_supports_op(nor->spimem, &op))
> +		return -ENOTSUPP;
> +
> +	return 0;
> +}
> +
> +static int spi_nor_spimem_check_progop(struct spi_nor *nor,

s/progop/pp?

> +				       const struct spi_nor_pp_command *pp)
> +{
> +	struct spi_mem_op op = SPI_MEM_OP(SPI_MEM_OP_CMD(pp->opcode, 1),
> +					  SPI_MEM_OP_ADDR(3, 0, 1),
> +					  SPI_MEM_OP_NO_DUMMY,
> +					  SPI_MEM_OP_DATA_OUT(0, NULL, 1));
> +
> +	op.cmd.buswidth = spi_nor_get_protocol_inst_nbits(pp->proto);
> +	op.addr.buswidth = spi_nor_get_protocol_addr_nbits(pp->proto);
> +	op.data.buswidth = spi_nor_get_protocol_data_nbits(pp->proto);
> +
maybe we can introduce a spi_nor_spimem_check_op() for the block from below to
reduce code duplication

> +	/*
> +	 * First test with 3 address bytes. The opcode itself might already
> +	 * be a 4B addressing opcode but we don't care, because SPI controller
> +	 * implementation should not check the opcode, but just the sequence.
> +	 */
> +	if (!spi_mem_supports_op(nor->spimem, &op))
> +		return -ENOTSUPP;
> +
> +	/* Now test with 4 address bytes. */
> +	op.addr.nbytes = 4;
> +	if (!spi_mem_supports_op(nor->spimem, &op))
> +		return -ENOTSUPP;
> +
> +	return 0;
> +}
> +
> +static void
> +spi_nor_spimem_adjust_hwcaps(struct spi_nor *nor,
> +			     const struct spi_nor_flash_parameter *params,
> +			     u32 *hwcaps)
> +{
> +	unsigned int cap;
> +
> +	/* DTR modes are not supported yet, mask them all. */
> +	*hwcaps &= ~SNOR_HWCAPS_DTR;
> +
> +	/* X-X-X modes are not supported yet, mask them all. */
> +	*hwcaps &= ~SNOR_HWCAPS_X_X_X;
> +
> +	/* Start with read commands. */
> +	for (cap = 0; cap < 32; cap++) {

maybe a macro for 32?

> +		int idx;
> +
> +		if (!(*hwcaps & BIT(cap)))
> +			continue;
> +
> +		idx = spi_nor_hwcaps_read2cmd(BIT(cap));
> +		if (idx < 0)
> +			continue;
> +
> +		if (spi_nor_spimem_check_readop(nor, &params->reads[idx]))
> +			*hwcaps &= ~BIT(cap);
> +	}
> +
> +	/* Now check program commands. */
> +	for (cap = 0; cap < 32; cap++) {

can't we use a single loop for both read & pp?

> +		int idx;
> +
> +		if (!(*hwcaps & BIT(cap)))
> +			continue;
> +
> +		idx = spi_nor_hwcaps_pp2cmd(BIT(cap));
> +		if (idx < 0)
> +			continue;
> +
> +		if (spi_nor_spimem_check_progop(nor,
> +						&params->page_programs[idx]))
> +			*hwcaps &= ~BIT(cap);
> +	}
> +}
> +
>  /**
>   * spi_nor_read_sfdp_dma_unsafe() - read Serial Flash Discoverable Parameters.
>   * @nor:	pointer to a 'struct spi_nor'
> @@ -4259,16 +4363,25 @@ static int spi_nor_setup(struct spi_nor *nor,
>  	 */
>  	shared_mask = hwcaps->mask & params->hwcaps.mask;
>  
> -	/* SPI n-n-n protocols are not supported yet. */
> -	ignored_mask = (SNOR_HWCAPS_READ_2_2_2 |
> -			SNOR_HWCAPS_READ_4_4_4 |
> -			SNOR_HWCAPS_READ_8_8_8 |
> -			SNOR_HWCAPS_PP_4_4_4 |
> -			SNOR_HWCAPS_PP_8_8_8);
> -	if (shared_mask & ignored_mask) {
> -		dev_dbg(nor->dev,
> -			"SPI n-n-n protocols are not supported yet.\n");
> -		shared_mask &= ~ignored_mask;
> +	if (nor->spimem) {
> +		/*
> +		 * When called from spi_nor_probe(), all caps are set and we
> +		 * need to discard some of them based on what the SPI
> +		 * controller actually supports (using spi_mem_supports_op()).
> +		 */
> +		spi_nor_spimem_adjust_hwcaps(nor, params, &shared_mask);
> +	} else {
> +		/*
> +		 * SPI n-n-n protocols are not supported when the SPI
> +		 * controller directly implements the spi_nor interface.
> +		 * Yet another reason to switch to spi-mem.
> +		 */
> +		ignored_mask = SNOR_HWCAPS_X_X_X;
> +		if (shared_mask & ignored_mask) {
> +			dev_dbg(nor->dev,
> +				"SPI n-n-n protocols are not supported.\n");
> +			shared_mask &= ~ignored_mask;
> +		}
>  	}
>  
>  	/* Select the (Fast) Read command. */
> @@ -4591,11 +4704,11 @@ static int spi_nor_probe(struct spi_mem *spimem)
>  	struct spi_device *spi = spimem->spi;
>  	struct flash_platform_data *data;
>  	struct spi_nor *nor;
> -	struct spi_nor_hwcaps hwcaps = {
> -		.mask = SNOR_HWCAPS_READ |
> -			SNOR_HWCAPS_READ_FAST |
> -			SNOR_HWCAPS_PP,
> -	};
> +	/*
> +	 * Enable all caps by default. The core will mask them after
> +	 * checking what's really supported using spi_mem_supports_op().
> +	 */
> +	struct spi_nor_hwcaps hwcaps = { .mask = SNOR_HWCAPS_ALL };

const struct spi_nor_hwcaps hwcaps = { .mask = SNOR_HWCAPS_ALL };

Cheers,
ta

>  	char *flash_name;
>  	int ret;
>  
> @@ -4624,27 +4737,6 @@ static int spi_nor_probe(struct spi_mem *spimem)
>  
>  	spi_mem_set_drvdata(spimem, nor);
>  
> -	if (spi->mode & SPI_RX_OCTAL) {
> -		hwcaps.mask |= SNOR_HWCAPS_READ_1_1_8;
> -
> -		if (spi->mode & SPI_TX_OCTAL)
> -			hwcaps.mask |= (SNOR_HWCAPS_READ_1_8_8 |
> -					SNOR_HWCAPS_PP_1_1_8 |
> -					SNOR_HWCAPS_PP_1_8_8);
> -	} else if (spi->mode & SPI_RX_QUAD) {
> -		hwcaps.mask |= SNOR_HWCAPS_READ_1_1_4;
> -
> -		if (spi->mode & SPI_TX_QUAD)
> -			hwcaps.mask |= (SNOR_HWCAPS_READ_1_4_4 |
> -					SNOR_HWCAPS_PP_1_1_4 |
> -					SNOR_HWCAPS_PP_1_4_4);
> -	} else if (spi->mode & SPI_RX_DUAL) {
> -		hwcaps.mask |= SNOR_HWCAPS_READ_1_1_2;
> -
> -		if (spi->mode & SPI_TX_DUAL)
> -			hwcaps.mask |= SNOR_HWCAPS_READ_1_2_2;
> -	}
> -
>  	if (data && data->name)
>  		nor->mtd.name = data->name;
>  
> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
> index ac16745f5ef8..1d56c437f553 100644
> --- a/include/linux/mtd/spi-nor.h
> +++ b/include/linux/mtd/spi-nor.h
> @@ -523,6 +523,20 @@ struct spi_nor_hwcaps {
>  #define SNOR_HWCAPS_PP_1_8_8	BIT(21)
>  #define SNOR_HWCAPS_PP_8_8_8	BIT(22)
>  
> +#define SNOR_HWCAPS_X_X_X	(SNOR_HWCAPS_READ_2_2_2 |	\
> +				 SNOR_HWCAPS_READ_4_4_4 |	\
> +				 SNOR_HWCAPS_READ_8_8_8 |	\
> +				 SNOR_HWCAPS_PP_4_4_4 |		\
> +				 SNOR_HWCAPS_PP_8_8_8)
> +
> +#define SNOR_HWCAPS_DTR		(SNOR_HWCAPS_READ_1_1_1_DTR |	\
> +				 SNOR_HWCAPS_READ_1_2_2_DTR |	\
> +				 SNOR_HWCAPS_READ_1_4_4_DTR |	\
> +				 SNOR_HWCAPS_READ_1_8_8_DTR)
> +
> +#define SNOR_HWCAPS_ALL		(SNOR_HWCAPS_READ_MASK |	\
> +				 SNOR_HWCAPS_PP_MASK)
> +
>  /**
>   * spi_nor_scan() - scan the SPI NOR
>   * @nor:	the spi_nor structure
> 
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* Re: [PATCH 1/2] mtd: spi-nor: Move m25p80 code in spi-nor.c
  2019-04-15  8:13   ` Tudor.Ambarus
@ 2019-04-16 13:55     ` Vignesh Raghavendra
  0 siblings, 0 replies; 6+ messages in thread
From: Vignesh Raghavendra @ 2019-04-16 13:55 UTC (permalink / raw)
  To: Tudor.Ambarus, bbrezillon, marek.vasut, richard, miquel.raynal,
	andrew.smirnov
  Cc: frieder.schrempf, yogeshnarayan.gaur, linux-mtd, linux-kernel

Hi Tudor,

On 15/04/19 1:43 PM, Tudor.Ambarus@microchip.com wrote:
> Hi,
> 
> The general approach looks good, few comments below.
> 
> On 04/09/2019 07:26 PM, Vignesh Raghavendra wrote:
>> External E-Mail
>>
>>
>> From: Boris Brezillon <boris.brezillon@bootlin.com>
>>
>> The m25p80 driver is actually a generic wrapper around the spi-mem
>> layer. Not only the driver name is misleading, but we'd expect such a
>> common logic to be directly available in the core. Another reason for
>> moving this code is that SPI NOR controller drivers should
>> progressively be replaced by SPI controller drivers implementing the
>> spi_mem_ops interface, and when the conversion is done, we should have
>> a single spi-nor driver directly interfacing with the spi-mem layer.
>>
>> While moving the code we also fix a longstanding issue when
>> non-DMA-able buffers are passed by the MTD layer.
>>
>> Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
>> [vigneshr@ti.com: use devm_kmalloc() for bounce buffer allocation]
>> Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com>
>> ---
>> Changes from RFC:
>> * Use devm_kmalloc() for bounce buffer allocation as it supports cache
>>   line aligned buffers now
> 
> then spi-nand has to be updated too.
> 

Yes, but as a separate patch I presume.

[...]

>> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
>> index 73172d7f512b..03c8c346c9ae 100644
>> --- a/drivers/mtd/spi-nor/spi-nor.c
>> +++ b/drivers/mtd/spi-nor/spi-nor.c
>> @@ -19,6 +19,7 @@
>>  
>>  #include <linux/mtd/mtd.h>
>>  #include <linux/of_platform.h>
>> +#include <linux/sched/task_stack.h>
>>  #include <linux/spi/flash.h>
>>  #include <linux/mtd/spi-nor.h>
>>  
>> @@ -288,6 +289,174 @@ struct flash_info {
>>  
>>  #define JEDEC_MFR(info)	((info)->id[0])
>>  
>> +static int spi_nor_exec_op(struct spi_nor *nor, struct spi_mem_op *op,
>> +			   u64 *addr, void *buf, unsigned int len)
> 
> size_t for len?
> 

Right will fix throughout.
I am also thinking of updating op.data.nbytes to size_t type in
spi-mem.h (and also spi_mem_dirmap_info->length) as part of a separate
patch.

>> +{
>> +	int ret;
>> +	bool usebouncebuf;
>> +
>> +	if (!op || (len && !buf))
>> +		return -EINVAL;
>> +
>> +	if (op->addr.nbytes && addr)
>> +		op->addr.val = *addr;
>> +
>> +	op->data.nbytes = len;
>> +
>> +	if (object_is_on_stack(buf) || !virt_addr_valid(buf))
>> +		usebouncebuf = true;
>> +	if (len && usebouncebuf) {
>> +		if (len > nor->bouncebuf_size)
>> +			return -ENOTSUPP;
>> +
>> +		if (op->data.dir == SPI_MEM_DATA_IN) {
>> +			op->data.buf.in = nor->bouncebuf;
>> +		} else {
>> +			op->data.buf.out = nor->bouncebuf;
>> +			memcpy(nor->bouncebuf, buf, len);
>> +		}
>> +	} else {
>> +		op->data.buf.out = buf;
>> +	}
>> +
>> +	ret = spi_mem_exec_op(nor->spimem, op);
>> +	if (ret)
>> +		return ret;
>> +
>> +	if (usebouncebuf && len && op->data.dir == SPI_MEM_DATA_IN)
>> +		memcpy(buf, nor->bouncebuf, len);
>> +
>> +	return 0;
>> +}
>> +
>> +static int spi_nor_data_op(struct spi_nor *nor, struct spi_mem_op *op,
>> +			   void *buf, unsigned int len)
> 
> size_t for len?
> 

Ok

>> +{
>> +	return spi_nor_exec_op(nor, op, NULL, buf, len);
>> +}
>> +
>> +static int spi_nor_nodata_op(struct spi_nor *nor, struct spi_mem_op *op)
>> +{
>> +	return spi_nor_exec_op(nor, op, NULL, NULL, 0);
>> +}
>> +
>> +static ssize_t spi_nor_spimem_read_data(struct spi_nor *nor, loff_t ofs,
>> +					size_t len, u8 *buf)
>> +{
>> +	struct spi_mem_op op = SPI_MEM_OP(SPI_MEM_OP_CMD(nor->read_opcode, 1),
>> +					  SPI_MEM_OP_ADDR(nor->addr_width, ofs,
>> +							  1),
>> +					  SPI_MEM_OP_DUMMY(nor->read_dummy, 1),
>> +					  SPI_MEM_OP_DATA_IN(len, buf, 1));
>> +	bool usebouncebuf = false;
>> +	size_t remaining = len;
>> +	int ret;
>> +
>> +	if (object_is_on_stack(buf) || !virt_addr_valid(buf)) {
>> +		usebouncebuf = true;
>> +		op.data.buf.in = nor->bouncebuf;
>> +	}
>> +
>> +	/* get transfer protocols. */
>> +	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 */
>> +	op.dummy.nbytes = (nor->read_dummy * op.dummy.buswidth) / 8;
>> +
>> +	while (remaining) {
> 
> Can't we get rid of this while? See Andrey's patch at
> https://lkml.org/lkml/2019/4/1/32. Andrey's patches are not included in this
> patch. We should consider integrating his work first, or squash his work in this
> patch, or ask him to rework the series after this patch gets accepted. I'll let
> you decide.
> 

I think its better to squash that series into this patch. Will take care
of that

>> +		op.data.nbytes = remaining < UINT_MAX ? remaining : UINT_MAX;
>> +		if (!usebouncebuf)
>> +			op.data.buf.out = buf;
>> +		else if (op.data.nbytes > nor->bouncebuf_size)
>> +			op.data.nbytes = nor->bouncebuf_size;
>> +
>> +		ret = spi_mem_adjust_op_size(nor->spimem, &op);
>> +		if (ret)
>> +			return ret;
>> +
>> +		ret = spi_mem_exec_op(nor->spimem, &op);
>> +		if (ret)
>> +			return ret;
>> +
>> +		if (usebouncebuf)
>> +			memcpy(buf, nor->bouncebuf, op.data.nbytes);
>> +
>> +		op.addr.val += op.data.nbytes;
>> +		remaining -= op.data.nbytes;
>> +		buf += op.data.nbytes;
>> +	}
>> +
>> +	return len;
>> +}
>> +
>> +static ssize_t spi_nor_read_data(struct spi_nor *nor, loff_t ofs, size_t len,
>> +				 u8 *buf)
>> +{
>> +	if (nor->spimem)
>> +		return spi_nor_spimem_read_data(nor, ofs, len, buf);
>> +
>> +	return nor->read(nor, ofs, len, buf);
>> +}
>> +
>> +static ssize_t spi_nor_spimem_write_data(struct spi_nor *nor, loff_t ofs,
>> +					 size_t len, const u8 *buf)
>> +{
>> +	struct spi_mem_op op = SPI_MEM_OP(SPI_MEM_OP_CMD(nor->program_opcode,
> 
> how about moving SPI_MEM_OP() to the next line to avoid breaking lines below?
> General comment.
> 

Agreed, will fix.

>> +							 1),
>> +					  SPI_MEM_OP_ADDR(nor->addr_width, ofs,
>> +							  1),
>> +					  SPI_MEM_OP_NO_DUMMY,
>> +					  SPI_MEM_OP_DATA_OUT(len, NULL, 1));
>> +	bool usebouncebuf = false;
>> +	int ret;
>> +
>> +	if (object_is_on_stack(buf) || !virt_addr_valid(buf)) {
>> +		usebouncebuf = true;
>> +		op.data.buf.out = nor->bouncebuf;
>> +	}
>> +
>> +	/* get transfer protocols. */
>> +	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.data.buswidth = spi_nor_get_protocol_data_nbits(nor->write_proto);
>> +
>> +	if (nor->program_opcode == SPINOR_OP_AAI_WP && nor->sst_write_second)
>> +		op.addr.nbytes = 0;
>> +
>> +	op.data.nbytes = len < UINT_MAX ? len : UINT_MAX;
>> +
>> +	if (!usebouncebuf) {
>> +		op.data.buf.out = buf;
>> +	} else {
>> +		if (op.data.nbytes > nor->bouncebuf_size)
>> +			op.data.nbytes = nor->bouncebuf_size;
>> +
>> +		memcpy(nor->bouncebuf, buf, op.data.nbytes);
>> +	}
>> +
>> +	ret = spi_mem_adjust_op_size(nor->spimem, &op);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = spi_mem_exec_op(nor->spimem, &op);
>> +	if (ret)
>> +		return ret;
>> +
>> +	return op.data.nbytes;
>> +}
>> +
[...]
>>  /* Enable/disable 4-byte addressing mode. */
>>  static int set_4byte(struct spi_nor *nor, bool enable)
>>  {
>>  	int status;
>>  	bool need_wren = false;
>> -	u8 cmd;
>>  
>>  	switch (JEDEC_MFR(nor->info)) {
>>  	case SNOR_MFR_ST:
>> @@ -486,8 +752,7 @@ static int set_4byte(struct spi_nor *nor, bool enable)
>>  		if (need_wren)
>>  			write_enable(nor);
>>  
>> -		cmd = enable ? SPINOR_OP_EN4B : SPINOR_OP_EX4B;
>> -		status = nor->write_reg(nor, cmd, NULL, 0);
>> +		status = macronix_set_4byte(nor, enable);
>>  		if (need_wren)
>>  			write_disable(nor);
>>  
>> @@ -500,8 +765,7 @@ static int set_4byte(struct spi_nor *nor, bool enable)
>>  			 * We must clear the register to enable normal behavior.
>>  			 */
>>  			write_enable(nor);
>> -			nor->cmd_buf[0] = 0;
>> -			nor->write_reg(nor, SPINOR_OP_WREAR, nor->cmd_buf, 1);
>> +			write_ear(nor, 0);
>>  			write_disable(nor);
>>  		}
>>  
>> @@ -509,16 +773,42 @@ static int set_4byte(struct spi_nor *nor, bool enable)
>>  	default:
>>  		/* Spansion style */
> 
> how about introducing a spansion_set_4byte() and return it directly?

Will do.

> 
>>  		nor->cmd_buf[0] = enable << 7;
>> +
>> +		if (nor->spimem) {
>> +			struct spi_mem_op op = SPI_MEM_OP(
>> +					SPI_MEM_OP_CMD(SPINOR_OP_BRWR, 1),
>> +					SPI_MEM_OP_NO_ADDR,
>> +					SPI_MEM_OP_NO_DUMMY,
>> +					SPI_MEM_OP_DATA_OUT(1, NULL, 1));
>> +
>> +			return spi_nor_data_op(nor, &op, nor->cmd_buf, 1);
>> +	}
>> +
>>  		return nor->write_reg(nor, SPINOR_OP_BRWR, nor->cmd_buf, 1);
>>  	}
>>  }
>>  
>> +static int xread_sr(struct spi_nor *nor, u8 *sr)
> 
> spi_nor_xread_sr?
> 

Ok

>> +{
>> +	if (nor->spimem) {
>> +		struct spi_mem_op op = SPI_MEM_OP(
>> +					SPI_MEM_OP_CMD(SPINOR_OP_XRDSR, 1),
>> +					SPI_MEM_OP_NO_ADDR,
>> +					SPI_MEM_OP_NO_DUMMY,
>> +					SPI_MEM_OP_DATA_IN(0, NULL, 1));
>> +
>> +		return spi_nor_data_op(nor, &op, sr, 1);
>> +	}
>> +
>> +	return nor->read_reg(nor, SPINOR_OP_XRDSR, sr, 1);
>> +}
>> +
>>  static int s3an_sr_ready(struct spi_nor *nor)
>>  {
>>  	int ret;
>>  	u8 val;
>>  
>> -	ret = nor->read_reg(nor, SPINOR_OP_XRDSR, &val, 1);
>> +	ret = xread_sr(nor, &val);
>>  	if (ret < 0) {
>>  		dev_err(nor->dev, "error %d reading XRDSR\n", (int) ret);
>>  		return ret;
>> @@ -527,6 +817,21 @@ static int s3an_sr_ready(struct spi_nor *nor)
>>  	return !!(val & XSR_RDY);
>>  }
>>  
>> +static int clear_sr(struct spi_nor *nor)
> 
> spi_nor_clear_sr?
> 

Ok

>> +{
>> +	if (nor->spimem) {
>> +		struct spi_mem_op op = SPI_MEM_OP(
>> +					SPI_MEM_OP_CMD(SPINOR_OP_CLSR, 1),
>> +					SPI_MEM_OP_NO_ADDR,
>> +					SPI_MEM_OP_NO_DUMMY,
>> +					SPI_MEM_OP_NO_DATA);
>> +
>> +		return spi_nor_nodata_op(nor, &op);
>> +	}
>> +
>> +	return nor->write_reg(nor, SPINOR_OP_CLSR, NULL, 0);
>> +}
>> +
>>  static int spi_nor_sr_ready(struct spi_nor *nor)
>>  {
>>  	int sr = read_sr(nor);
>> @@ -539,13 +844,28 @@ static int spi_nor_sr_ready(struct spi_nor *nor)
>>  		else
>>  			dev_err(nor->dev, "Programming Error occurred\n");
>>  
>> -		nor->write_reg(nor, SPINOR_OP_CLSR, NULL, 0);
>> +		clear_sr(nor);
>>  		return -EIO;
>>  	}
>>  
>>  	return !(sr & SR_WIP);
>>  }
>>  
>> +static int clear_fsr(struct spi_nor *nor)
> 
> spi_nor_clear_fsr?
> 

Ok

>> +{
>> +	if (nor->spimem) {
>> +		struct spi_mem_op op = SPI_MEM_OP(
>> +					SPI_MEM_OP_CMD(SPINOR_OP_CLFSR, 1),
>> +					SPI_MEM_OP_NO_ADDR,
>> +					SPI_MEM_OP_NO_DUMMY,
>> +					SPI_MEM_OP_NO_DATA);
>> +
>> +		return spi_nor_nodata_op(nor, &op);
>> +	}
>> +
>> +	return nor->write_reg(nor, SPINOR_OP_CLFSR, NULL, 0);
>> +}
>> +
>>  static int spi_nor_fsr_ready(struct spi_nor *nor)
>>  {
>>  	int fsr = read_fsr(nor);
>> @@ -562,7 +882,7 @@ static int spi_nor_fsr_ready(struct spi_nor *nor)
>>  			dev_err(nor->dev,
>>  			"Attempted to modify a protected sector.\n");
>>  
>> -		nor->write_reg(nor, SPINOR_OP_CLFSR, NULL, 0);
>> +		clear_fsr(nor);
>>  		return -EIO;
>>  	}
>>  
>> @@ -630,6 +950,16 @@ static int erase_chip(struct spi_nor *nor)
>>  {
>>  	dev_dbg(nor->dev, " %lldKiB\n", (long long)(nor->mtd.size >> 10));
>>  
>> +	if (nor->spimem) {
>> +		struct spi_mem_op op = SPI_MEM_OP(
>> +					SPI_MEM_OP_CMD(SPINOR_OP_CHIP_ERASE, 1),
>> +					SPI_MEM_OP_NO_ADDR,
>> +					SPI_MEM_OP_NO_DUMMY,
>> +					SPI_MEM_OP_NO_DATA);
>> +
>> +		return spi_nor_nodata_op(nor, &op);
>> +	}
>> +
>>  	return nor->write_reg(nor, SPINOR_OP_CHIP_ERASE, NULL, 0);
>>  }
>>  
>> @@ -692,6 +1022,17 @@ static int spi_nor_erase_sector(struct spi_nor *nor, u32 addr)
>>  	if (nor->erase)
>>  		return nor->erase(nor, addr);
>>  
>> +	if (nor->spimem) {
>> +		struct spi_mem_op op = SPI_MEM_OP(
>> +					SPI_MEM_OP_CMD(nor->erase_opcode, 1),
>> +					SPI_MEM_OP_ADDR(nor->addr_width, addr,
>> +							1),
>> +					SPI_MEM_OP_NO_DUMMY,
>> +					SPI_MEM_OP_NO_DATA);
>> +
>> +		return spi_nor_nodata_op(nor, &op);
>> +	}
>> +
>>  	/*
>>  	 * Default implementation, if driver doesn't have a specialized HW
>>  	 * control
>> @@ -1406,7 +1747,18 @@ static int write_sr_cr(struct spi_nor *nor, u8 *sr_cr)
>>  
>>  	write_enable(nor);
>>  
>> -	ret = nor->write_reg(nor, SPINOR_OP_WRSR, sr_cr, 2);
>> +	if (nor->spimem) {
>> +		struct spi_mem_op op = SPI_MEM_OP(
>> +					SPI_MEM_OP_CMD(SPINOR_OP_WRSR, 1),
>> +					SPI_MEM_OP_NO_ADDR,
>> +					SPI_MEM_OP_NO_DUMMY,
>> +					SPI_MEM_OP_DATA_OUT(0, NULL, 1));
>> +
>> +		ret = spi_nor_data_op(nor, &op, sr_cr, 2);
>> +	} else {
>> +		ret = nor->write_reg(nor, SPINOR_OP_WRSR, sr_cr, 2);
>> +	}
>> +
>>  	if (ret < 0) {
>>  		dev_err(nor->dev,
>>  			"error while writing configuration register\n");
>> @@ -1585,6 +1937,36 @@ static int spansion_read_cr_quad_enable(struct spi_nor *nor)
>>  	return 0;
>>  }
>>  
>> +static int write_sr2(struct spi_nor *nor, u8 sr2)
> 
> spi_nor_write_sr2?
> 

Ok

>> +{
>> +	if (nor->spimem) {
>> +		struct spi_mem_op op = SPI_MEM_OP(
>> +					SPI_MEM_OP_CMD(SPINOR_OP_WRSR2, 1),
>> +					SPI_MEM_OP_NO_ADDR,
>> +					SPI_MEM_OP_NO_DUMMY,
>> +					SPI_MEM_OP_DATA_OUT(0, NULL, 1));
>> +
>> +		return spi_nor_data_op(nor, &op, &sr2, 1);
>> +	}
>> +
>> +	return nor->write_reg(nor, SPINOR_OP_WRSR2, &sr2, 1);
>> +}
>> +
>> +static int read_sr2(struct spi_nor *nor, u8 *sr2)
> 
> spi_nor_read_sr2?
> 

Ok

>> +{
>> +	if (nor->spimem) {
>> +		struct spi_mem_op op = SPI_MEM_OP(
>> +					SPI_MEM_OP_CMD(SPINOR_OP_RDSR2, 1),
>> +					SPI_MEM_OP_NO_ADDR,
>> +					SPI_MEM_OP_NO_DUMMY,
>> +					SPI_MEM_OP_DATA_IN(0, NULL, 1));
>> +
>> +		return spi_nor_data_op(nor, &op, sr2, 1);
>> +	}
>> +
>> +	return nor->read_reg(nor, SPINOR_OP_RDSR2, sr2, 1);
>> +}
>> +
>>  /**
>>   * sr2_bit7_quad_enable() - set QE bit in Status Register 2.
>>   * @nor:	pointer to a 'struct spi_nor'
>> @@ -1603,7 +1985,7 @@ static int sr2_bit7_quad_enable(struct spi_nor *nor)
>>  	int ret;
>>  
>>  	/* Check current Quad Enable bit value. */
>> -	ret = nor->read_reg(nor, SPINOR_OP_RDSR2, &sr2, 1);
>> +	ret = read_sr2(nor, &sr2);
>>  	if (ret)
>>  		return ret;
>>  	if (sr2 & SR2_QUAD_EN_BIT7)
>> @@ -1614,7 +1996,7 @@ static int sr2_bit7_quad_enable(struct spi_nor *nor)
>>  
>>  	write_enable(nor);
>>  
>> -	ret = nor->write_reg(nor, SPINOR_OP_WRSR2, &sr2, 1);
>> +	ret = write_sr2(nor, sr2);
>>  	if (ret < 0) {
>>  		dev_err(nor->dev, "error while writing status register 2\n");
>>  		return -EINVAL;
>> @@ -1626,8 +2008,7 @@ static int sr2_bit7_quad_enable(struct spi_nor *nor)
>>  		return ret;
>>  	}
>>  
>> -	/* Read back and check it. */
>> -	ret = nor->read_reg(nor, SPINOR_OP_RDSR2, &sr2, 1);
>> +	ret = read_sr2(nor, &sr2);
>>  	if (!(ret > 0 && (sr2 & SR2_QUAD_EN_BIT7))) {
>>  		dev_err(nor->dev, "SR2 Quad bit not set\n");
>>  		return -EINVAL;
>> @@ -2054,13 +2435,28 @@ static const struct flash_info spi_nor_ids[] = {
>>  	{ },
>>  };
>>  
>> +static int read_id(struct spi_nor *nor, u8 *id)
> 
> this function is called just from spi_nor_read_id(). How about incorporating
> this code into spi_nor_read_id() so that we use functions that are prepended
> with "spi_nor" to not pollute the namespace?

I agree, but inlining this code would break 80 char limit due to extra
indentation required for nor->read_reg() call below.

So how about renaming this function to spi_nor_read_id() and current
spi_nor_read_id() function to spi_nor_get_flash_info()?

>> +{
>> +	if (nor->spimem) {
>> +		struct spi_mem_op op = SPI_MEM_OP(
>> +					SPI_MEM_OP_CMD(SPINOR_OP_RDID, 1),
>> +					SPI_MEM_OP_NO_ADDR,
>> +					SPI_MEM_OP_NO_DUMMY,
>> +					SPI_MEM_OP_DATA_IN(0, NULL, 1));
>> +
>> +		return spi_nor_data_op(nor, &op, id, SPI_NOR_MAX_ID_LEN);
>> +	}
>> +
>> +	return nor->read_reg(nor, SPINOR_OP_RDID, id, SPI_NOR_MAX_ID_LEN);
>> +}
>> +
>>  static const struct flash_info *spi_nor_read_id(struct spi_nor *nor)
>>  {
>>  	int			tmp;
>>  	u8			id[SPI_NOR_MAX_ID_LEN];
>>  	const struct flash_info	*info;
>>  
>> -	tmp = nor->read_reg(nor, SPINOR_OP_RDID, id, SPI_NOR_MAX_ID_LEN);
>> +	tmp = read_id(nor, id);
>>  	if (tmp < 0) {
>>  		dev_dbg(nor->dev, "error %d reading JEDEC ID\n", tmp);
>>  		return ERR_PTR(tmp);
>> @@ -2096,7 +2492,7 @@ static int spi_nor_read(struct mtd_info *mtd, loff_t from, size_t len,
>>  		if (nor->flags & SNOR_F_S3AN_ADDR_DEFAULT)
>>  			addr = spi_nor_s3an_addr_convert(nor, addr);
>>  
>> -		ret = nor->read(nor, addr, len, buf);
>> +		ret = spi_nor_read_data(nor, addr, len, buf);
>>  		if (ret == 0) {
>>  			/* We shouldn't see 0-length reads */
>>  			ret = -EIO;
>> @@ -2141,7 +2537,7 @@ static int sst_write(struct mtd_info *mtd, loff_t to, size_t len,
>>  		nor->program_opcode = SPINOR_OP_BP;
>>  
>>  		/* write one byte. */
>> -		ret = nor->write(nor, to, 1, buf);
>> +		ret = spi_nor_write_data(nor, to, 1, buf);
>>  		if (ret < 0)
>>  			goto sst_write_err;
>>  		WARN(ret != 1, "While writing 1 byte written %i bytes\n",
>> @@ -2157,7 +2553,7 @@ static int sst_write(struct mtd_info *mtd, loff_t to, size_t len,
>>  		nor->program_opcode = SPINOR_OP_AAI_WP;
>>  
>>  		/* write two bytes. */
>> -		ret = nor->write(nor, to, 2, buf + actual);
>> +		ret = spi_nor_write_data(nor, to, 2, buf + actual);
>>  		if (ret < 0)
>>  			goto sst_write_err;
>>  		WARN(ret != 2, "While writing 2 bytes written %i bytes\n",
>> @@ -2180,7 +2576,7 @@ static int sst_write(struct mtd_info *mtd, loff_t to, size_t len,
>>  		write_enable(nor);
>>  
>>  		nor->program_opcode = SPINOR_OP_BP;
>> -		ret = nor->write(nor, to, 1, buf + actual);
>> +		ret = spi_nor_write_data(nor, to, 1, buf + actual);
>>  		if (ret < 0)
>>  			goto sst_write_err;
>>  		WARN(ret != 1, "While writing 1 byte written %i bytes\n",
>> @@ -2242,7 +2638,7 @@ static int spi_nor_write(struct mtd_info *mtd, loff_t to, size_t len,
>>  			addr = spi_nor_s3an_addr_convert(nor, addr);
>>  
>>  		write_enable(nor);
>> -		ret = nor->write(nor, addr, page_remain, buf + i);
>> +		ret = spi_nor_write_data(nor, addr, page_remain, buf + i);
>>  		if (ret < 0)
>>  			goto write_err;
>>  		written = ret;
>> @@ -2261,8 +2657,10 @@ static int spi_nor_write(struct mtd_info *mtd, loff_t to, size_t len,
>>  
>>  static int spi_nor_check(struct spi_nor *nor)
>>  {
>> -	if (!nor->dev || !nor->read || !nor->write ||
>> -		!nor->read_reg || !nor->write_reg) {
>> +	if (!nor->dev ||
>> +	    (!nor->spimem &&
>> +	    (!nor->read || !nor->write || !nor->read_reg ||
>> +	      !nor->write_reg))) {
>>  		pr_err("spi-nor: please fill all the necessary fields!\n");
>>  		return -EINVAL;
>>  	}
>> @@ -2275,7 +2673,7 @@ static int s3an_nor_scan(struct spi_nor *nor)
>>  	int ret;
>>  	u8 val;
>>  
>> -	ret = nor->read_reg(nor, SPINOR_OP_XRDSR, &val, 1);
>> +	ret = xread_sr(nor, &val);
>>  	if (ret < 0) {
>>  		dev_err(nor->dev, "error %d reading XRDSR\n", (int) ret);
>>  		return ret;
>> @@ -2405,7 +2803,7 @@ static int spi_nor_read_raw(struct spi_nor *nor, u32 addr, size_t len, u8 *buf)
>>  	int ret;
>>  
>>  	while (len) {
>> -		ret = nor->read(nor, addr, len, buf);
>> +		ret = spi_nor_read_data(nor, addr, len, buf);
>>  		if (!ret || ret > len)
>>  			return -EIO;
>>  		if (ret < 0)
>> @@ -4188,6 +4586,215 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
>>  }
>>  EXPORT_SYMBOL_GPL(spi_nor_scan);
>>  
>> +static int spi_nor_probe(struct spi_mem *spimem)
>> +{
>> +	struct spi_device *spi = spimem->spi;
>> +	struct flash_platform_data *data;
>> +	struct spi_nor *nor;
>> +	struct spi_nor_hwcaps hwcaps = {
>> +		.mask = SNOR_HWCAPS_READ |
>> +			SNOR_HWCAPS_READ_FAST |
>> +			SNOR_HWCAPS_PP,
>> +	};
>> +	char *flash_name;
>> +	int ret;
>> +
>> +	data = dev_get_platdata(&spi->dev);
> 
> you can do the initialization at declaration
> 
will update..

>> +
>> +	nor = devm_kzalloc(&spi->dev, sizeof(*nor), GFP_KERNEL);
>> +	if (!nor)
>> +		return -ENOMEM;
>> +
>> +	/*
>> +	 * We need the bounce buffer early to read/write registers when going
>> +	 * through the spi-mem layer (buffers have to be DMA-able).
>> +	 * We'll reallocate a new buffer if nor->page_size turns out to be
>> +	 * greater than PAGE_SIZE (which shouldn't happen before long since NOR
>> +	 * pages are usually less than 1KB) after spi_nor_scan() returns.
>> +	 */
>> +	nor->bouncebuf_size = PAGE_SIZE;
>> +	nor->bouncebuf = devm_kmalloc(&spi->dev, nor->bouncebuf_size,
>> +				      GFP_KERNEL);
>> +	if (!nor->bouncebuf)
>> +		return -ENOMEM;
>> +
>> +	nor->spimem = spimem;
>> +	nor->dev = &spi->dev;
>> +	spi_nor_set_flash_node(nor, spi->dev.of_node);
>> +
>> +	spi_mem_set_drvdata(spimem, nor);
>> +
>> +	if (spi->mode & SPI_RX_OCTAL) {
>> +		hwcaps.mask |= SNOR_HWCAPS_READ_1_1_8;
>> +
>> +		if (spi->mode & SPI_TX_OCTAL)
>> +			hwcaps.mask |= (SNOR_HWCAPS_READ_1_8_8 |
>> +					SNOR_HWCAPS_PP_1_1_8 |
>> +					SNOR_HWCAPS_PP_1_8_8);
>> +	} else if (spi->mode & SPI_RX_QUAD) {
>> +		hwcaps.mask |= SNOR_HWCAPS_READ_1_1_4;
>> +
>> +		if (spi->mode & SPI_TX_QUAD)
>> +			hwcaps.mask |= (SNOR_HWCAPS_READ_1_4_4 |
>> +					SNOR_HWCAPS_PP_1_1_4 |
>> +					SNOR_HWCAPS_PP_1_4_4);
>> +	} else if (spi->mode & SPI_RX_DUAL) {
>> +		hwcaps.mask |= SNOR_HWCAPS_READ_1_1_2;
>> +
>> +		if (spi->mode & SPI_TX_DUAL)
>> +			hwcaps.mask |= SNOR_HWCAPS_READ_1_2_2;
>> +	}
>> +
>> +	if (data && data->name)
>> +		nor->mtd.name = data->name;
>> +
>> +	if (!nor->mtd.name)
>> +		nor->mtd.name = spi_mem_get_name(spimem);
>> +
>> +	/*
>> +	 * For some (historical?) reason many platforms provide two different
>> +	 * names in flash_platform_data: "name" and "type". Quite often name is
>> +	 * set to "m25p80" and then "type" provides a real chip name.
>> +	 * If that's the case, respect "type" and ignore a "name".
>> +	 */
>> +	if (data && data->type)
>> +		flash_name = data->type;
>> +	else if (!strcmp(spi->modalias, "spi-nor"))
>> +		flash_name = NULL; /* auto-detect */
>> +	else
>> +		flash_name = spi->modalias;
>> +
>> +	ret = spi_nor_scan(nor, flash_name, &hwcaps);
>> +	if (ret)
>> +		return ret;
>> +
>> +	/*
>> +	 * None of the existing parts have > 512B pages, but let's play safe
>> +	 * and add this logic so that if anyone ever adds support for such
>> +	 * a NOR we don't end up with buffer overflows.
>> +	 */
>> +	if (nor->page_size > PAGE_SIZE) {
>> +		nor->bouncebuf_size = nor->page_size;
>> +		devm_kfree(nor->dev, nor->bouncebuf);
>> +		nor->bouncebuf = devm_kmalloc(nor->dev,
>> +					      nor->bouncebuf_size,
>> +					      GFP_KERNEL);
>> +		if (!nor->bouncebuf)
>> +			return -ENOMEM;
>> +	}
>> +
>> +	return mtd_device_register(&nor->mtd, data ? data->parts : NULL,
>> +				   data ? data->nr_parts : 0);
>> +}
>> +
>> +static int spi_nor_remove(struct spi_mem *spimem)
>> +{
>> +	struct spi_nor *nor = spi_mem_get_drvdata(spimem);
>> +	int ret;
>> +
>> +	spi_nor_restore(nor);
>> +
>> +	/* Clean up MTD stuff. */
>> +	ret = mtd_device_unregister(&nor->mtd);
>> +	if (ret)
>> +		return ret;> +
>> +	return 0;
> 
> return mtd_device_unregister(&nor->mtd); directly

Ok

> 
>> +}
>> +
>> +static void spi_nor_shutdown(struct spi_mem *spimem)
>> +{
>> +	struct spi_nor *nor = spi_mem_get_drvdata(spimem);
>> +
>> +	spi_nor_restore(nor);
>> +}
>> +
>> +/*
>> + * Do NOT add to this array without reading the following:
>> + *
>> + * Historically, many flash devices are bound to this driver by their name. But
>> + * since most of these flash are compatible to some extent, and their
>> + * differences can often be differentiated by the JEDEC read-ID command, we
>> + * encourage new users to add support to the spi-nor library, and simply bind
>> + * against a generic string here (e.g., "jedec,spi-nor").
>> + *
>> + * Many flash names are kept here in this list (as well as in spi-nor.c) to
>> + * keep them available as module aliases for existing platforms.
>> + */
>> +static const struct spi_device_id spi_nor_dev_ids[] = {
>> +	/*
>> +	 * Allow non-DT platform devices to bind to the "spi-nor" modalias, and
>> +	 * hack around the fact that the SPI core does not provide uevent
>> +	 * matching for .of_match_table
>> +	 */
>> +	{"spi-nor"},
>> +
>> +	/*
>> +	 * Entries not used in DTs that should be safe to drop after replacing
>> +	 * them with "spi-nor" in platform data.
>> +	 */
>> +	{"s25sl064a"},	{"w25x16"},	{"m25p10"},	{"m25px64"},
>> +
>> +	/*
>> +	 * Entries that were used in DTs without "jedec,spi-nor" fallback and
>> +	 * should be kept for backward compatibility.
>> +	 */
>> +	{"at25df321a"},	{"at25df641"},	{"at26df081a"},
>> +	{"mx25l4005a"},	{"mx25l1606e"},	{"mx25l6405d"},	{"mx25l12805d"},
>> +	{"mx25l25635e"},{"mx66l51235l"},
>> +	{"n25q064"},	{"n25q128a11"},	{"n25q128a13"},	{"n25q512a"},
>> +	{"s25fl256s1"},	{"s25fl512s"},	{"s25sl12801"},	{"s25fl008k"},
>> +	{"s25fl064k"},
>> +	{"sst25vf040b"},{"sst25vf016b"},{"sst25vf032b"},{"sst25wf040"},
>> +	{"m25p40"},	{"m25p80"},	{"m25p16"},	{"m25p32"},
>> +	{"m25p64"},	{"m25p128"},
>> +	{"w25x80"},	{"w25x32"},	{"w25q32"},	{"w25q32dw"},
>> +	{"w25q80bl"},	{"w25q128"},	{"w25q256"},
>> +
>> +	/* Flashes that can't be detected using JEDEC */
>> +	{"m25p05-nonjedec"},	{"m25p10-nonjedec"},	{"m25p20-nonjedec"},
>> +	{"m25p40-nonjedec"},	{"m25p80-nonjedec"},	{"m25p16-nonjedec"},
>> +	{"m25p32-nonjedec"},	{"m25p64-nonjedec"},	{"m25p128-nonjedec"},
>> +
>> +	/* Everspin MRAMs (non-JEDEC) */
>> +	{ "mr25h128" }, /* 128 Kib, 40 MHz */
>> +	{ "mr25h256" }, /* 256 Kib, 40 MHz */
>> +	{ "mr25h10" },  /*   1 Mib, 40 MHz */
>> +	{ "mr25h40" },  /*   4 Mib, 40 MHz */
>> +
>> +	{ },
>> +};
>> +MODULE_DEVICE_TABLE(spi, spi_nor_dev_ids);
>> +
>> +static const struct of_device_id spi_nor_of_table[] = {
>> +	/*
>> +	 * Generic compatibility for SPI NOR that can be identified by the
>> +	 * JEDEC READ ID opcode (0x9F). Use this, if possible.
>> +	 */
>> +	{ .compatible = "jedec,spi-nor" },
>> +	{ /* sentinel */ },
>> +};
>> +MODULE_DEVICE_TABLE(of, spi_nor_of_table);
>> +
>> +/*
>> + * REVISIT: many of these chips have deep power-down modes, which
>> + * should clearly be entered on suspend() to minimize power use.
>> + * And also when they're otherwise idle...
>> + */
>> +static struct spi_mem_driver spi_nor_driver = {
>> +	.spidrv = {
>> +		.driver = {
>> +			.name = "spi-nor",
>> +			.of_match_table = spi_nor_of_table,
>> +		},
>> +		.id_table = spi_nor_dev_ids,
>> +	},
>> +	.probe = spi_nor_probe,
>> +	.remove = spi_nor_remove,
>> +	.shutdown = spi_nor_shutdown,
>> +};
>> +module_spi_mem_driver(spi_nor_driver);
>> +
>>  MODULE_LICENSE("GPL v2");
>>  MODULE_AUTHOR("Huang Shijie <shijie8@gmail.com>");
>>  MODULE_AUTHOR("Mike Lavender");
>> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
>> index b3d360b0ee3d..ac16745f5ef8 100644
>> --- a/include/linux/mtd/spi-nor.h
>> +++ b/include/linux/mtd/spi-nor.h
>> @@ -9,6 +9,7 @@
>>  #include <linux/bitops.h>
>>  #include <linux/mtd/cfi.h>
>>  #include <linux/mtd/mtd.h>
>> +#include <linux/spi/spi-mem.h>
>>  
>>  /*
>>   * Manufacturer IDs
>> @@ -344,6 +345,10 @@ struct flash_info;
>>   * @mtd:		point to a mtd_info structure
>>   * @lock:		the lock for the read/write/erase/lock/unlock operations
>>   * @dev:		point to a spi device, or a spi nor controller device.
>> + * @spimem:		point to the spi mem device
>> + * @bouncebuf:		bounce buffer used when the buffer passed by the MTD
>> + *                      layer is not DMA-able
>> + * @bouncebuf_size:	size of the bounce buffe
>                                            ^buffer
> 
>>   * @info:		spi-nor part JDEC MFR id and other info
>>   * @page_size:		the page size of the SPI NOR
>>   * @addr_width:		number of address bytes
>> @@ -380,6 +385,9 @@ struct spi_nor {
>>  	struct mtd_info		mtd;
>>  	struct mutex		lock;
>>  	struct device		*dev;
>> +	struct spi_mem		*spimem;
>> +	void			*bouncebuf;
>> +	unsigned int		bouncebuf_size;
> 
> size_t?
> 
> Please run checkpatch --strict over the patch, there are few things to fix.
> 

I will address all except for the ones reported for spi_nor_dev_ids[] as
they existed before so that alignment looks good.

> Do you think it is worth documenting the new functions?
> 

Will add at places where functions are not trivial.

Thanks for the review!


-- 
Regards
Vignesh

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

end of thread, other threads:[~2019-04-16 13:54 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-09 16:26 [PATCH 0/2] Merge m25p80 into spi-nor Vignesh Raghavendra
2019-04-09 16:26 ` [PATCH 1/2] mtd: spi-nor: Move m25p80 code in spi-nor.c Vignesh Raghavendra
2019-04-15  8:13   ` Tudor.Ambarus
2019-04-16 13:55     ` Vignesh Raghavendra
2019-04-09 16:26 ` [PATCH 2/2] mtd: spi-nor: Rework hwcaps selection for the spi-mem case Vignesh Raghavendra
2019-04-15 11:59   ` Tudor.Ambarus

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).