All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v8 0/4] mtd: Add a SPI NAND driver
@ 2018-06-01 13:13 ` Boris Brezillon
  0 siblings, 0 replies; 24+ messages in thread
From: Boris Brezillon @ 2018-06-01 13:13 UTC (permalink / raw)
  To: David Woodhouse, Brian Norris, Boris Brezillon, Marek Vasut,
	Richard Weinberger, linux-mtd, Miquel Raynal, Mark Brown,
	linux-spi, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell,
	Kumar Gala, devicetree

Hello,

Not much has changed in this v8, I mainly fixed bugs/issues reported
by Miquel and Frider.
This is hopefully the last version, and if everything goes well I
should merge this stuff just after 4.18-rc1 is out.

Mark, Rob, just one thing I'm not sure about: in patch 2 I use
spi-{tx,rx}-bus-width to encode board limitations on the SPI bus, but
I'm not sure these properties were created to express that. Should I
create a new prop (spi-max-bus-width?) or is it fine to reuse those
spi-{tx,rx}-bus-width props?

Comments/reviews are welcome.

Thanks,

Boris

v8 changes:
- dropped patch 1 which has been applied
- fix various bugs in the core (see changelog in patch 1)
- add a commit message to patch 4

v7 changes:
- Use the spi-mem interface
- Add support for on-die ECC
- Add support for Winbond W25M02GV chip

v6 changes:
- includes generic NAND framework patches in series
- rebase on nand/next (commit 6076fd1e9d879521f7082a5e22185b71e480b777)
- remove on-die ECC support
- remove devm_free() since everything allocated by devm_kmalloc() will be
  automatically freed when device is released
- add comment header for structs in spinand.h
- remove spinand_register()/unregister(), call spinand_detect() in
  spinand_init() and only expose spinand_init()/cleanup()
- add nand_release_bbt() in bbt.c and use it in nand_cleanup() and
  spinand_cleanup()
- use BIT(n) instead (1 << n) in macro of spinand.h
- rename spinand_alloc() to devm_spinand_alloc()
- name lables in better way
- fix some typos
- add empty lines between code blocks

v5 changes:
- rebase patch on nand/next with Boris's generic NAND framework patches[3]
- replace pr_xxx() with dev_xxx()
- replace kzalloc()i/kfree() with devm_kzalloc()/devm_kfree()
- rename spinand_op_init() to spinand_init_op() for consistency
- remove command opcode in function comments
- use BIT(n) instead (1 << n) in macro
- remove manufactures.c and put spinand_manufacturers table in core.c
- change spinand_write_reg() u8 *buf argument to u8 value,
  since the length is always 1
- remove spinand_manufacture->detect() check, since it is always != NULL
- alloc spinand_ecc_engine struct in vendor.c when using on-die ECC
  (for hardware ECC, it should be in controllers/*.c)
- add comment header for struct spinand_op
- fix timeout bug in spinand_wait(), thanks for Arnaud's debug
- make spinand_manufacturers const
- add ecc_engine_ops pointer in struct micron_spinand_info
- make controller->cap assignment right with SPI_TX/RX_QUAD/DUAL flag

v4 changes:
- initialize struct mtd_oob_ops to 0 in bbt.c
- rename new added helper in nand.h to nand_check_xxxx()
- add struct mtd_oob_ops consistency check in nand_check_oob_ops()
- add dataleft in struct nand_page_iter instead of offs
- remove spinand_manufacturers->ops->detect() check since it is mandatory
- remove spinand_set_manufacturer_ops() and do the job in
  spinand_manufacturer_detect()
- move .priv out of struct spinand_controller
- add spinand_alloc/free/register/unregister() and make
  spinand_detect/init() static
- make BBT be configured by device tree
- chip->id.data stores raw ID directly
- refine device info print message after detect success
- add struct mtd_layout_ops pointer in struct micron_spinand_info
- remove micron_spinand_init() and do its job in micron_spinand_detect()
- fix BBT block cannot be erased bug

v3 changes:
- rebase patch on 4.11-rc1[2]
- change read ID method. read 4 bytes ID out then let ->detect() of each
  manufacutre driver to decode ID and detect the device.
- make SPI NAND id table private to each manufacutre driver
- fix coding style to make checkpatch.pl happy
- update the MAINTAINERS file for spi nand code
- add nand_size() helper in nand.h
- use nand_for_each_page() helper in spinand_do_read/write_ops()
- create helper to check boundaries in generic NAND code and use it
  in SPI NAND core
- rename spinand_base.c to core.c
- manufactures' drivers expose spinand_manufacturer struct instead of
  spinand_manufacturer_ops struct to keep Manufacture ID macro in
  manufactures' drivers and rename spinand_ids.c to manufacture.c
- rename spinand_micron.c to micron.c
- rename chips/ directory to controllers/
- rename generic_spi.c to generic-spi.c
- replace ->build_column_addr() and ->get_dummy() hooks with ->prepare_op() in
  spinand_manufacturer_ops struct
- rename __spinand_erase() to spinand_erase()
- rename spinand_erase() to spinand_erase_skip_bbt()
- rename spinand_scan_ident() to spinand_detect()
- rename spinand_scan_tail() to spinand_init()
- move non detect related code from spinand_detect() to spinand_init()
- remove spinand_fill_nandd, assign nand->ops in spinand_detect()
- merge v2 patch 3(bad block support) and patch 4(BBT support)
- drop getchip parameter, remove spinand_get/remove_device(), take the lock
  by caller directly
- fix function comment headers
- use nand_bbt_is_initialized() helper
- replace spinand_ecc_engine and spinand_controller object in spinand_device
  struct with pointer
- replace struct spinand_manufacturer_ops pointer in spinand_device struct
  with spinand_manufacturer struct

v2 changes:
- replace "spi_nand" with "spinand".
- rename spi nand related structs for better understanding.
- introduce spi nand controller, manufacturer and ecc_engine struct.
- add spi nand manufacturer initialization function refer to Boris's
  manuf-init branch.
- remove NAND_SKIP_BBTSCAN from series. Add it later when enabling HW ECC.
- reorganize series according to Boris's suggestion.

*** BLURB HERE ***

Boris Brezillon (1):
  dt-bindings: Add bindings for SPI NAND devices

Frieder Schrempf (1):
  mtd: spinand: Add initial support for Winbond W25M02GV

Peter Pan (2):
  mtd: nand: Add core infrastructure to support SPI NANDs
  mtd: spinand: Add initial support for Micron MT29F2G01ABAGD

 Documentation/devicetree/bindings/mtd/spi-nand.txt |   27 +
 drivers/mtd/nand/Kconfig                           |    1 +
 drivers/mtd/nand/Makefile                          |    1 +
 drivers/mtd/nand/spi/Kconfig                       |    7 +
 drivers/mtd/nand/spi/Makefile                      |    2 +
 drivers/mtd/nand/spi/core.c                        | 1129 ++++++++++++++++++++
 drivers/mtd/nand/spi/micron.c                      |  133 +++
 drivers/mtd/nand/spi/winbond.c                     |  141 +++
 include/linux/mtd/spinand.h                        |  419 ++++++++
 include/linux/spi/spi-mem.h                        |    4 +-
 10 files changed, 1863 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/mtd/spi-nand.txt
 create mode 100644 drivers/mtd/nand/spi/Kconfig
 create mode 100644 drivers/mtd/nand/spi/Makefile
 create mode 100644 drivers/mtd/nand/spi/core.c
 create mode 100644 drivers/mtd/nand/spi/micron.c
 create mode 100644 drivers/mtd/nand/spi/winbond.c
 create mode 100644 include/linux/mtd/spinand.h

-- 
2.14.1


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

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

* [PATCH v8 0/4] mtd: Add a SPI NAND driver
@ 2018-06-01 13:13 ` Boris Brezillon
  0 siblings, 0 replies; 24+ messages in thread
From: Boris Brezillon @ 2018-06-01 13:13 UTC (permalink / raw)
  To: David Woodhouse, Brian Norris, Boris Brezillon, Marek Vasut,
	Richard Weinberger, linux-mtd, Miquel Raynal, Mark Brown,
	linux-spi, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell,
	Kumar Gala, devicetree

Hello,

Not much has changed in this v8, I mainly fixed bugs/issues reported
by Miquel and Frider.
This is hopefully the last version, and if everything goes well I
should merge this stuff just after 4.18-rc1 is out.

Mark, Rob, just one thing I'm not sure about: in patch 2 I use
spi-{tx,rx}-bus-width to encode board limitations on the SPI bus, but
I'm not sure these properties were created to express that. Should I
create a new prop (spi-max-bus-width?) or is it fine to reuse those
spi-{tx,rx}-bus-width props?

Comments/reviews are welcome.

Thanks,

Boris

v8 changes:
- dropped patch 1 which has been applied
- fix various bugs in the core (see changelog in patch 1)
- add a commit message to patch 4

v7 changes:
- Use the spi-mem interface
- Add support for on-die ECC
- Add support for Winbond W25M02GV chip

v6 changes:
- includes generic NAND framework patches in series
- rebase on nand/next (commit 6076fd1e9d879521f7082a5e22185b71e480b777)
- remove on-die ECC support
- remove devm_free() since everything allocated by devm_kmalloc() will be
  automatically freed when device is released
- add comment header for structs in spinand.h
- remove spinand_register()/unregister(), call spinand_detect() in
  spinand_init() and only expose spinand_init()/cleanup()
- add nand_release_bbt() in bbt.c and use it in nand_cleanup() and
  spinand_cleanup()
- use BIT(n) instead (1 << n) in macro of spinand.h
- rename spinand_alloc() to devm_spinand_alloc()
- name lables in better way
- fix some typos
- add empty lines between code blocks

v5 changes:
- rebase patch on nand/next with Boris's generic NAND framework patches[3]
- replace pr_xxx() with dev_xxx()
- replace kzalloc()i/kfree() with devm_kzalloc()/devm_kfree()
- rename spinand_op_init() to spinand_init_op() for consistency
- remove command opcode in function comments
- use BIT(n) instead (1 << n) in macro
- remove manufactures.c and put spinand_manufacturers table in core.c
- change spinand_write_reg() u8 *buf argument to u8 value,
  since the length is always 1
- remove spinand_manufacture->detect() check, since it is always != NULL
- alloc spinand_ecc_engine struct in vendor.c when using on-die ECC
  (for hardware ECC, it should be in controllers/*.c)
- add comment header for struct spinand_op
- fix timeout bug in spinand_wait(), thanks for Arnaud's debug
- make spinand_manufacturers const
- add ecc_engine_ops pointer in struct micron_spinand_info
- make controller->cap assignment right with SPI_TX/RX_QUAD/DUAL flag

v4 changes:
- initialize struct mtd_oob_ops to 0 in bbt.c
- rename new added helper in nand.h to nand_check_xxxx()
- add struct mtd_oob_ops consistency check in nand_check_oob_ops()
- add dataleft in struct nand_page_iter instead of offs
- remove spinand_manufacturers->ops->detect() check since it is mandatory
- remove spinand_set_manufacturer_ops() and do the job in
  spinand_manufacturer_detect()
- move .priv out of struct spinand_controller
- add spinand_alloc/free/register/unregister() and make
  spinand_detect/init() static
- make BBT be configured by device tree
- chip->id.data stores raw ID directly
- refine device info print message after detect success
- add struct mtd_layout_ops pointer in struct micron_spinand_info
- remove micron_spinand_init() and do its job in micron_spinand_detect()
- fix BBT block cannot be erased bug

v3 changes:
- rebase patch on 4.11-rc1[2]
- change read ID method. read 4 bytes ID out then let ->detect() of each
  manufacutre driver to decode ID and detect the device.
- make SPI NAND id table private to each manufacutre driver
- fix coding style to make checkpatch.pl happy
- update the MAINTAINERS file for spi nand code
- add nand_size() helper in nand.h
- use nand_for_each_page() helper in spinand_do_read/write_ops()
- create helper to check boundaries in generic NAND code and use it
  in SPI NAND core
- rename spinand_base.c to core.c
- manufactures' drivers expose spinand_manufacturer struct instead of
  spinand_manufacturer_ops struct to keep Manufacture ID macro in
  manufactures' drivers and rename spinand_ids.c to manufacture.c
- rename spinand_micron.c to micron.c
- rename chips/ directory to controllers/
- rename generic_spi.c to generic-spi.c
- replace ->build_column_addr() and ->get_dummy() hooks with ->prepare_op() in
  spinand_manufacturer_ops struct
- rename __spinand_erase() to spinand_erase()
- rename spinand_erase() to spinand_erase_skip_bbt()
- rename spinand_scan_ident() to spinand_detect()
- rename spinand_scan_tail() to spinand_init()
- move non detect related code from spinand_detect() to spinand_init()
- remove spinand_fill_nandd, assign nand->ops in spinand_detect()
- merge v2 patch 3(bad block support) and patch 4(BBT support)
- drop getchip parameter, remove spinand_get/remove_device(), take the lock
  by caller directly
- fix function comment headers
- use nand_bbt_is_initialized() helper
- replace spinand_ecc_engine and spinand_controller object in spinand_device
  struct with pointer
- replace struct spinand_manufacturer_ops pointer in spinand_device struct
  with spinand_manufacturer struct

v2 changes:
- replace "spi_nand" with "spinand".
- rename spi nand related structs for better understanding.
- introduce spi nand controller, manufacturer and ecc_engine struct.
- add spi nand manufacturer initialization function refer to Boris's
  manuf-init branch.
- remove NAND_SKIP_BBTSCAN from series. Add it later when enabling HW ECC.
- reorganize series according to Boris's suggestion.

*** BLURB HERE ***

Boris Brezillon (1):
  dt-bindings: Add bindings for SPI NAND devices

Frieder Schrempf (1):
  mtd: spinand: Add initial support for Winbond W25M02GV

Peter Pan (2):
  mtd: nand: Add core infrastructure to support SPI NANDs
  mtd: spinand: Add initial support for Micron MT29F2G01ABAGD

 Documentation/devicetree/bindings/mtd/spi-nand.txt |   27 +
 drivers/mtd/nand/Kconfig                           |    1 +
 drivers/mtd/nand/Makefile                          |    1 +
 drivers/mtd/nand/spi/Kconfig                       |    7 +
 drivers/mtd/nand/spi/Makefile                      |    2 +
 drivers/mtd/nand/spi/core.c                        | 1129 ++++++++++++++++++++
 drivers/mtd/nand/spi/micron.c                      |  133 +++
 drivers/mtd/nand/spi/winbond.c                     |  141 +++
 include/linux/mtd/spinand.h                        |  419 ++++++++
 include/linux/spi/spi-mem.h                        |    4 +-
 10 files changed, 1863 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/mtd/spi-nand.txt
 create mode 100644 drivers/mtd/nand/spi/Kconfig
 create mode 100644 drivers/mtd/nand/spi/Makefile
 create mode 100644 drivers/mtd/nand/spi/core.c
 create mode 100644 drivers/mtd/nand/spi/micron.c
 create mode 100644 drivers/mtd/nand/spi/winbond.c
 create mode 100644 include/linux/mtd/spinand.h

-- 
2.14.1

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

* [PATCH v8 1/4] mtd: nand: Add core infrastructure to support SPI NANDs
  2018-06-01 13:13 ` Boris Brezillon
@ 2018-06-01 13:13   ` Boris Brezillon
  -1 siblings, 0 replies; 24+ messages in thread
From: Boris Brezillon @ 2018-06-01 13:13 UTC (permalink / raw)
  To: David Woodhouse, Brian Norris, Boris Brezillon, Marek Vasut,
	Richard Weinberger, linux-mtd, Miquel Raynal, Mark Brown,
	linux-spi, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell,
	Kumar Gala, devicetree
  Cc: Peter Pan

From: Peter Pan <peterpandong@micron.com>

Add a SPI NAND framework based on the generic NAND framework and the
spi-mem infrastructure.

In its current state, this framework supports the following features:

- single/dual/quad IO modes
- on-die ECC

Signed-off-by: Peter Pan <peterpandong@micron.com>
Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
---
Changes in v8:
- Use spinand_upd_cfg() instead of spinand_get_cfg()+spinand_set_cfg()
  in spinand_init_quad_enable() and spinand_ecc_enable()
- Make sure spinand_init_quad_enable() worked
- Do not check return code of spinand_read_page() in spinand_isbad()
- Fix the spinand_init() error path
---
 drivers/mtd/nand/Kconfig      |    1 +
 drivers/mtd/nand/Makefile     |    1 +
 drivers/mtd/nand/spi/Kconfig  |    7 +
 drivers/mtd/nand/spi/Makefile |    2 +
 drivers/mtd/nand/spi/core.c   | 1111 +++++++++++++++++++++++++++++++++++++++++
 include/linux/mtd/spinand.h   |  415 +++++++++++++++
 include/linux/spi/spi-mem.h   |    4 +-
 7 files changed, 1540 insertions(+), 1 deletion(-)
 create mode 100644 drivers/mtd/nand/spi/Kconfig
 create mode 100644 drivers/mtd/nand/spi/Makefile
 create mode 100644 drivers/mtd/nand/spi/core.c
 create mode 100644 include/linux/mtd/spinand.h

diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index 88c7d3b4ff8b..9033215e62ea 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -4,3 +4,4 @@ config MTD_NAND_CORE
 source "drivers/mtd/nand/onenand/Kconfig"
 
 source "drivers/mtd/nand/raw/Kconfig"
+source "drivers/mtd/nand/spi/Kconfig"
diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
index 3f0cb87f1a57..7ecd80c0a66e 100644
--- a/drivers/mtd/nand/Makefile
+++ b/drivers/mtd/nand/Makefile
@@ -5,3 +5,4 @@ obj-$(CONFIG_MTD_NAND_CORE) += nandcore.o
 
 obj-y	+= onenand/
 obj-y	+= raw/
+obj-y	+= spi/
diff --git a/drivers/mtd/nand/spi/Kconfig b/drivers/mtd/nand/spi/Kconfig
new file mode 100644
index 000000000000..7c37d2929b68
--- /dev/null
+++ b/drivers/mtd/nand/spi/Kconfig
@@ -0,0 +1,7 @@
+menuconfig MTD_SPI_NAND
+	tristate "SPI NAND device Support"
+	select MTD_NAND_CORE
+	depends on SPI_MASTER
+	select SPI_MEM
+	help
+	  This is the framework for the SPI NAND device drivers.
diff --git a/drivers/mtd/nand/spi/Makefile b/drivers/mtd/nand/spi/Makefile
new file mode 100644
index 000000000000..feb79a1d1b46
--- /dev/null
+++ b/drivers/mtd/nand/spi/Makefile
@@ -0,0 +1,2 @@
+spinand-objs := core.o
+obj-$(CONFIG_MTD_SPI_NAND) += spinand.o
diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
new file mode 100644
index 000000000000..a99ea352eb85
--- /dev/null
+++ b/drivers/mtd/nand/spi/core.c
@@ -0,0 +1,1111 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2016-2017 Micron Technology, Inc.
+ *
+ * Authors:
+ *	Peter Pan <peterpandong@micron.com>
+ *	Boris Brezillon <boris.brezillon@bootlin.com>
+ */
+
+#define pr_fmt(fmt)	"spi-nand: " fmt
+
+#include <linux/device.h>
+#include <linux/jiffies.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mtd/spinand.h>
+#include <linux/of.h>
+#include <linux/slab.h>
+#include <linux/spi/spi.h>
+#include <linux/spi/spi-mem.h>
+
+static void spinand_cache_op_adjust_colum(struct spinand_device *spinand,
+					  const struct nand_page_io_req *req,
+					  u16 *column)
+{
+	struct nand_device *nand = spinand_to_nand(spinand);
+	unsigned int shift;
+
+	if (nand->memorg.planes_per_lun < 2)
+		return;
+
+	/* The plane number is passed in MSB just above the column address */
+	shift = fls(nand->memorg.pagesize);
+	*column |= req->pos.plane << shift;
+}
+
+static int spinand_read_reg_op(struct spinand_device *spinand, u8 reg, u8 *val)
+{
+	struct spi_mem_op op = SPINAND_GET_FEATURE_OP(reg,
+						      spinand->scratchbuf);
+	int ret;
+
+	ret = spi_mem_exec_op(spinand->spimem, &op);
+	if (ret)
+		return ret;
+
+	*val = *spinand->scratchbuf;
+	return 0;
+}
+
+static int spinand_write_reg_op(struct spinand_device *spinand, u8 reg, u8 val)
+{
+	struct spi_mem_op op = SPINAND_SET_FEATURE_OP(reg,
+						      spinand->scratchbuf);
+
+	*spinand->scratchbuf = val;
+	return spi_mem_exec_op(spinand->spimem, &op);
+}
+
+static int spinand_read_status(struct spinand_device *spinand, u8 *status)
+{
+	return spinand_read_reg_op(spinand, REG_STATUS, status);
+}
+
+static int spinand_get_cfg(struct spinand_device *spinand, u8 *cfg)
+{
+	struct nand_device *nand = spinand_to_nand(spinand);
+
+	if (WARN_ON(spinand->cur_target < 0 ||
+		    spinand->cur_target >= nand->memorg.ntargets))
+		return -EINVAL;
+
+	*cfg = spinand->cfg_cache[spinand->cur_target];
+	return 0;
+}
+
+static int spinand_set_cfg(struct spinand_device *spinand, u8 cfg)
+{
+	struct nand_device *nand = spinand_to_nand(spinand);
+	int ret;
+
+	if (WARN_ON(spinand->cur_target < 0 ||
+		    spinand->cur_target >= nand->memorg.ntargets))
+		return -EINVAL;
+
+	if (spinand->cfg_cache[spinand->cur_target] == cfg)
+		return 0;
+
+	ret = spinand_write_reg_op(spinand, REG_CFG, cfg);
+	if (ret)
+		return ret;
+
+	spinand->cfg_cache[spinand->cur_target] = cfg;
+	return 0;
+}
+
+/**
+ * spinand_upd_cfg() - Update the configuration register
+ * @spinand: the spinand device
+ * @mask: the mask encoding the bits to update in the config reg
+ * @val: the new value to apply
+ *
+ * Update the configuration register.
+ *
+ * Return: 0 on success, a negative error code otherwise.
+ */
+int spinand_upd_cfg(struct spinand_device *spinand, u8 mask, u8 val)
+{
+	int ret;
+	u8 cfg;
+
+	ret = spinand_get_cfg(spinand, &cfg);
+	if (ret)
+		return ret;
+
+	cfg &= ~mask;
+	cfg |= val;
+
+	return spinand_set_cfg(spinand, cfg);
+}
+
+/**
+ * spinand_select_target() - Select a specific NAND target/die
+ * @spinand: the spinand device
+ * @target: the target/die to select
+ *
+ * Select a new target/die. If chip only has one die, this function is a NOOP.
+ *
+ * Return: 0 on success, a negative error code otherwise.
+ */
+int spinand_select_target(struct spinand_device *spinand, unsigned int target)
+{
+	struct nand_device *nand = spinand_to_nand(spinand);
+	int ret;
+
+	if (WARN_ON(target >= nand->memorg.ntargets))
+		return -EINVAL;
+
+	if (spinand->cur_target == target)
+		return 0;
+
+	if (nand->memorg.ntargets == 1) {
+		spinand->cur_target = target;
+		return 0;
+	}
+
+	ret = spinand->select_target(spinand, target);
+	if (ret)
+		return ret;
+
+	spinand->cur_target = target;
+	return 0;
+}
+
+static int spinand_init_cfg_cache(struct spinand_device *spinand)
+{
+	struct nand_device *nand = spinand_to_nand(spinand);
+	struct device *dev = &spinand->spimem->spi->dev;
+	unsigned int target;
+	int ret;
+
+	spinand->cfg_cache = devm_kzalloc(dev,
+					  sizeof(*spinand->cfg_cache) *
+					  nand->memorg.ntargets,
+					  GFP_KERNEL);
+	if (!spinand->cfg_cache)
+		return -ENOMEM;
+
+	for (target = 0; target < nand->memorg.ntargets; target++) {
+		ret = spinand_select_target(spinand, target);
+		if (ret)
+			return ret;
+
+		/*
+		 * We use spinand_read_reg_op() instead of spinand_get_cfg()
+		 * here to bypass the config cache.
+		 */
+		ret = spinand_read_reg_op(spinand, REG_CFG,
+					  &spinand->cfg_cache[target]);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static int spinand_init_quad_enable(struct spinand_device *spinand)
+{
+	bool enable = false;
+
+	if (!(spinand->flags & SPINAND_HAS_QE_BIT))
+		return 0;
+
+	if (spinand->op_templates.read_cache->data.buswidth == 4 ||
+	    spinand->op_templates.write_cache->data.buswidth == 4 ||
+	    spinand->op_templates.update_cache->data.buswidth == 4)
+		enable = true;
+
+	return spinand_upd_cfg(spinand, CFG_QUAD_ENABLE,
+			       enable ? CFG_QUAD_ENABLE : 0);
+}
+
+static void spinand_ecc_enable(struct spinand_device *spinand,
+			       bool enable)
+{
+	WARN_ON(spinand_upd_cfg(spinand, CFG_ECC_ENABLE,
+				enable ? CFG_ECC_ENABLE : 0));
+}
+
+static int spinand_write_enable_op(struct spinand_device *spinand)
+{
+	struct spi_mem_op op = SPINAND_WR_EN_DIS_OP(true);
+
+	return spi_mem_exec_op(spinand->spimem, &op);
+}
+
+static int spinand_load_page_op(struct spinand_device *spinand,
+				const struct nand_page_io_req *req)
+{
+	struct nand_device *nand = spinand_to_nand(spinand);
+	unsigned int row = nanddev_pos_to_row(nand, &req->pos);
+	struct spi_mem_op op = SPINAND_PAGE_READ_OP(row);
+
+	return spi_mem_exec_op(spinand->spimem, &op);
+}
+
+static int spinand_read_from_cache_op(struct spinand_device *spinand,
+				      const struct nand_page_io_req *req)
+{
+	struct spi_mem_op op = *spinand->op_templates.read_cache;
+	struct nand_device *nand = spinand_to_nand(spinand);
+	struct mtd_info *mtd = nanddev_to_mtd(nand);
+	struct nand_page_io_req adjreq = *req;
+	unsigned int nbytes = 0;
+	void *buf = NULL;
+	u16 column = 0;
+	int ret;
+
+	if (req->datalen) {
+		adjreq.datalen = nanddev_page_size(nand);
+		adjreq.dataoffs = 0;
+		adjreq.databuf.in = spinand->databuf;
+		buf = spinand->databuf;
+		nbytes = adjreq.datalen;
+	}
+
+	if (req->ooblen) {
+		adjreq.ooblen = nanddev_per_page_oobsize(nand);
+		adjreq.ooboffs = 0;
+		adjreq.oobbuf.in = spinand->oobbuf;
+		nbytes += nanddev_per_page_oobsize(nand);
+		if (!buf) {
+			buf = spinand->oobbuf;
+			column = nanddev_page_size(nand);
+		}
+	}
+
+	spinand_cache_op_adjust_colum(spinand, &adjreq, &column);
+	op.addr.val = column;
+
+	/*
+	 * Some controllers are limited in term of max RX data size. In this
+	 * case, just repeat the READ_CACHE operation after updating the
+	 * column.
+	 */
+	while (nbytes) {
+		op.data.buf.in = buf;
+		op.data.nbytes = nbytes;
+		ret = spi_mem_adjust_op_size(spinand->spimem, &op);
+		if (ret)
+			return ret;
+
+		ret = spi_mem_exec_op(spinand->spimem, &op);
+		if (ret)
+			return ret;
+
+		buf += op.data.nbytes;
+		nbytes -= op.data.nbytes;
+		op.addr.val += op.data.nbytes;
+	}
+
+	if (req->datalen)
+		memcpy(req->databuf.in, spinand->databuf + req->dataoffs,
+		       req->datalen);
+
+	if (req->ooblen) {
+		if (req->mode == MTD_OPS_AUTO_OOB)
+			mtd_ooblayout_get_databytes(mtd, req->oobbuf.in,
+						    spinand->oobbuf,
+						    req->ooboffs,
+						    req->ooblen);
+		else
+			memcpy(req->oobbuf.in, spinand->oobbuf + req->ooboffs,
+			       req->ooblen);
+	}
+
+	return 0;
+}
+
+static int spinand_write_to_cache_op(struct spinand_device *spinand,
+				     const struct nand_page_io_req *req)
+{
+	struct spi_mem_op op = *spinand->op_templates.write_cache;
+	struct nand_device *nand = spinand_to_nand(spinand);
+	struct mtd_info *mtd = nanddev_to_mtd(nand);
+	struct nand_page_io_req adjreq = *req;
+	unsigned int nbytes = 0;
+	void *buf = NULL;
+	u16 column = 0;
+	int ret;
+
+	memset(spinand->databuf, 0xff,
+	       nanddev_page_size(nand) +
+	       nanddev_per_page_oobsize(nand));
+
+	if (req->datalen) {
+		memcpy(spinand->databuf + req->dataoffs, req->databuf.out,
+		       req->datalen);
+		adjreq.dataoffs = 0;
+		adjreq.datalen = nanddev_page_size(nand);
+		adjreq.databuf.out = spinand->databuf;
+		nbytes = adjreq.datalen;
+		buf = spinand->databuf;
+	}
+
+	if (req->ooblen) {
+		if (req->mode == MTD_OPS_AUTO_OOB)
+			mtd_ooblayout_set_databytes(mtd, req->oobbuf.out,
+						    spinand->oobbuf,
+						    req->ooboffs,
+						    req->ooblen);
+		else
+			memcpy(spinand->oobbuf + req->ooboffs, req->oobbuf.out,
+			       req->ooblen);
+
+		adjreq.ooblen = nanddev_per_page_oobsize(nand);
+		adjreq.ooboffs = 0;
+		nbytes += nanddev_per_page_oobsize(nand);
+		if (!buf) {
+			buf = spinand->oobbuf;
+			column = nanddev_page_size(nand);
+		}
+	}
+
+	spinand_cache_op_adjust_colum(spinand, &adjreq, &column);
+
+	op = *spinand->op_templates.write_cache;
+	op.addr.val = column;
+
+	/*
+	 * Some controllers are limited in term of max TX data size. In this
+	 * case, split the operation into one LOAD CACHE and one or more
+	 * LOAD RANDOM CACHE.
+	 */
+	while (nbytes) {
+		op.data.buf.out = buf;
+		op.data.nbytes = nbytes;
+
+		ret = spi_mem_adjust_op_size(spinand->spimem, &op);
+		if (ret)
+			return ret;
+
+		ret = spi_mem_exec_op(spinand->spimem, &op);
+		if (ret)
+			return ret;
+
+		buf += op.data.nbytes;
+		nbytes -= op.data.nbytes;
+		op.addr.val += op.data.nbytes;
+
+		/*
+		 * We need to use the RANDOM LOAD CACHE operation if there's
+		 * more than one iteration, because the LOAD operation resets
+		 * the cache to 0xff.
+		 */
+		if (nbytes) {
+			column = op.addr.val;
+			op = *spinand->op_templates.update_cache;
+			op.addr.val = column;
+		}
+	}
+
+	return 0;
+}
+
+static int spinand_program_op(struct spinand_device *spinand,
+			      const struct nand_page_io_req *req)
+{
+	struct nand_device *nand = spinand_to_nand(spinand);
+	unsigned int row = nanddev_pos_to_row(nand, &req->pos);
+	struct spi_mem_op op = SPINAND_PROG_EXEC_OP(row);
+
+	return spi_mem_exec_op(spinand->spimem, &op);
+}
+
+static int spinand_erase_op(struct spinand_device *spinand,
+			    const struct nand_pos *pos)
+{
+	struct nand_device *nand = spinand_to_nand(spinand);
+	unsigned int row = nanddev_pos_to_row(nand, pos);
+	struct spi_mem_op op = SPINAND_BLK_ERASE_OP(row);
+
+	return spi_mem_exec_op(spinand->spimem, &op);
+}
+
+static int spinand_wait(struct spinand_device *spinand, u8 *s)
+{
+	unsigned long timeo =  jiffies + msecs_to_jiffies(400);
+	u8 status;
+
+	do {
+		spinand_read_status(spinand, &status);
+		if (!(status & STATUS_BUSY))
+			goto out;
+	} while (time_before(jiffies, timeo));
+
+	/*
+	 * Extra read, just in case the STATUS_READY bit has changed
+	 * since our last check
+	 */
+	spinand_read_status(spinand, &status);
+
+out:
+	if (s)
+		*s = status;
+
+	return status & STATUS_BUSY ? -ETIMEDOUT : 0;
+}
+
+static int spinand_read_id_op(struct spinand_device *spinand, u8 *buf)
+{
+	struct spi_mem_op op = SPINAND_READID_OP(0, spinand->scratchbuf,
+						 SPINAND_MAX_ID_LEN);
+	int ret;
+
+	ret = spi_mem_exec_op(spinand->spimem, &op);
+	if (!ret)
+		memcpy(buf, spinand->scratchbuf, SPINAND_MAX_ID_LEN);
+
+	return 0;
+}
+
+static int spinand_reset_op(struct spinand_device *spinand)
+{
+	struct spi_mem_op op = SPINAND_RESET_OP;
+	int ret;
+
+	ret = spi_mem_exec_op(spinand->spimem, &op);
+	if (ret)
+		return ret;
+
+	return spinand_wait(spinand, NULL);
+}
+
+static int spinand_lock_block(struct spinand_device *spinand, u8 lock)
+{
+	return spinand_write_reg_op(spinand, REG_BLOCK_LOCK, lock);
+}
+
+static int spinand_check_ecc_status(struct spinand_device *spinand, u8 status)
+{
+	struct nand_device *nand = spinand_to_nand(spinand);
+
+	if (spinand->eccinfo.get_status)
+		return spinand->eccinfo.get_status(spinand, status);
+
+	switch (status & STATUS_ECC_MASK) {
+	case STATUS_ECC_NO_BITFLIPS:
+		return 0;
+
+	case STATUS_ECC_HAS_BITFLIPS:
+		/*
+		 * We have no way to know exactly how many bitflips have been
+		 * fixed, so let's return the maximum possible value so that
+		 * wear-leveling layers move the data immediately.
+		 */
+		return nand->eccreq.strength;
+
+	case STATUS_ECC_UNCOR_ERROR:
+		return -EBADMSG;
+
+	default:
+		break;
+	}
+
+	return -EINVAL;
+}
+
+static int spinand_read_page(struct spinand_device *spinand,
+			     const struct nand_page_io_req *req,
+			     bool ecc_enabled)
+{
+	struct nand_device *nand = spinand_to_nand(spinand);
+	struct device *dev = &spinand->spimem->spi->dev;
+	u8 status;
+	int ret;
+
+	spinand_load_page_op(spinand, req);
+
+	ret = spinand_wait(spinand, &status);
+	if (ret < 0) {
+		dev_err(dev, "failed to load page @%llx (err = %d)\n",
+			nanddev_pos_to_offs(nand, &req->pos), ret);
+		return ret;
+	}
+
+	spinand_read_from_cache_op(spinand, req);
+
+	if (!ecc_enabled)
+		return 0;
+
+	return spinand_check_ecc_status(spinand, status);
+}
+
+static int spinand_write_page(struct spinand_device *spinand,
+			      const struct nand_page_io_req *req)
+{
+	struct nand_device *nand = spinand_to_nand(spinand);
+	struct device *dev = &spinand->spimem->spi->dev;
+	u8 status;
+	int ret = 0;
+
+	spinand_write_enable_op(spinand);
+	spinand_write_to_cache_op(spinand, req);
+	spinand_program_op(spinand, req);
+
+	ret = spinand_wait(spinand, &status);
+	if (!ret && (status & STATUS_PROG_FAILED))
+		ret = -EIO;
+
+	if (ret < 0)
+		dev_err(dev, "failed to program page @%llx (err = %d)\n",
+			nanddev_pos_to_offs(nand, &req->pos), ret);
+
+	return ret;
+}
+
+static int spinand_mtd_read(struct mtd_info *mtd, loff_t from,
+			    struct mtd_oob_ops *ops)
+{
+	struct spinand_device *spinand = mtd_to_spinand(mtd);
+	struct nand_device *nand = mtd_to_nanddev(mtd);
+	unsigned int max_bitflips = 0;
+	struct nand_io_iter iter;
+	bool enable_ecc = false;
+	bool ecc_failed = false;
+	int ret = 0;
+
+	if (ops->mode != MTD_OPS_RAW && spinand->eccinfo.ooblayout)
+		enable_ecc = true;
+
+	mutex_lock(&spinand->lock);
+
+	nanddev_io_for_each_page(nand, from, ops, &iter) {
+		spinand_select_target(spinand, iter.req.pos.target);
+		spinand_ecc_enable(spinand, enable_ecc);
+		ret = spinand_read_page(spinand, &iter.req, enable_ecc);
+		if (ret < 0 && ret != -EBADMSG)
+			break;
+
+		if (ret == -EBADMSG) {
+			ecc_failed = true;
+			mtd->ecc_stats.failed++;
+			ret = 0;
+		} else {
+			mtd->ecc_stats.corrected += ret;
+			max_bitflips = max_t(unsigned int, max_bitflips, ret);
+		}
+
+		ops->retlen += iter.req.datalen;
+		ops->oobretlen += iter.req.ooblen;
+	}
+
+	mutex_unlock(&spinand->lock);
+
+	if (ecc_failed && !ret)
+		ret = -EBADMSG;
+
+	return ret ? ret : max_bitflips;
+}
+
+static int spinand_mtd_write(struct mtd_info *mtd, loff_t to,
+			     struct mtd_oob_ops *ops)
+{
+	struct spinand_device *spinand = mtd_to_spinand(mtd);
+	struct nand_device *nand = mtd_to_nanddev(mtd);
+	struct nand_io_iter iter;
+	bool enable_ecc = false;
+	int ret = 0;
+
+	if (ops->mode != MTD_OPS_RAW && mtd->ooblayout)
+		enable_ecc = true;
+
+	mutex_lock(&spinand->lock);
+
+	nanddev_io_for_each_page(nand, to, ops, &iter) {
+		spinand_select_target(spinand, iter.req.pos.target);
+		spinand_ecc_enable(spinand, enable_ecc);
+
+		ret = spinand_write_page(spinand, &iter.req);
+		if (ret)
+			break;
+
+		ops->retlen += iter.req.datalen;
+		ops->oobretlen += iter.req.ooblen;
+	}
+
+	mutex_unlock(&spinand->lock);
+
+	return ret;
+}
+
+static bool spinand_isbad(struct nand_device *nand, const struct nand_pos *pos)
+{
+	struct spinand_device *spinand = nand_to_spinand(nand);
+	struct nand_page_io_req req = {
+		.pos = *pos,
+		.ooblen = 2,
+		.ooboffs = 0,
+		.oobbuf.in = spinand->oobbuf,
+		.mode = MTD_OPS_RAW,
+	};
+
+	memset(spinand->oobbuf, 0, 2);
+	spinand_select_target(spinand, pos->target);
+	spinand_read_page(spinand, &req, false);
+	if (spinand->oobbuf[0] != 0xff || spinand->oobbuf[1] != 0xff)
+		return true;
+
+	return false;
+}
+
+static int spinand_mtd_block_isbad(struct mtd_info *mtd, loff_t offs)
+{
+	struct nand_device *nand = mtd_to_nanddev(mtd);
+	struct spinand_device *spinand = nand_to_spinand(nand);
+	struct nand_pos pos;
+	int ret;
+
+	nanddev_offs_to_pos(nand, offs, &pos);
+	mutex_lock(&spinand->lock);
+	ret = nanddev_isbad(nand, &pos);
+	mutex_unlock(&spinand->lock);
+
+	return ret;
+}
+
+static int spinand_markbad(struct nand_device *nand, const struct nand_pos *pos)
+{
+	struct spinand_device *spinand = nand_to_spinand(nand);
+	struct nand_page_io_req req = {
+		.pos = *pos,
+		.ooboffs = 0,
+		.ooblen = 2,
+		.oobbuf.out = spinand->oobbuf,
+	};
+
+	/* Erase block before marking it bad. */
+	spinand_select_target(spinand, pos->target);
+	spinand_write_enable_op(spinand);
+	spinand_erase_op(spinand, pos);
+
+	memset(spinand->oobbuf, 0, 2);
+	return spinand_write_page(spinand, &req);
+}
+
+static int spinand_mtd_block_markbad(struct mtd_info *mtd, loff_t offs)
+{
+	struct nand_device *nand = mtd_to_nanddev(mtd);
+	struct spinand_device *spinand = nand_to_spinand(nand);
+	struct nand_pos pos;
+	int ret;
+
+	nanddev_offs_to_pos(nand, offs, &pos);
+	mutex_lock(&spinand->lock);
+	ret = nanddev_markbad(nand, &pos);
+	mutex_unlock(&spinand->lock);
+
+	return ret;
+}
+
+static int spinand_erase(struct nand_device *nand, const struct nand_pos *pos)
+{
+	struct spinand_device *spinand = nand_to_spinand(nand);
+	struct device *dev = &spinand->spimem->spi->dev;
+	u8 status;
+	int ret;
+
+	spinand_select_target(spinand, pos->target);
+	spinand_write_enable_op(spinand);
+	spinand_erase_op(spinand, pos);
+
+	ret = spinand_wait(spinand, &status);
+
+	if (!ret && (status & STATUS_ERASE_FAILED))
+		ret = -EIO;
+
+	if (ret)
+		dev_err(dev, "failed to erase block %d (err = %d)\n",
+			pos->eraseblock, ret);
+
+	return ret;
+}
+
+static int spinand_mtd_erase(struct mtd_info *mtd,
+			     struct erase_info *einfo)
+{
+	struct spinand_device *spinand = mtd_to_spinand(mtd);
+	int ret;
+
+	mutex_lock(&spinand->lock);
+	ret = nanddev_mtd_erase(mtd, einfo);
+	mutex_unlock(&spinand->lock);
+
+	return ret;
+}
+
+static int spinand_mtd_block_isreserved(struct mtd_info *mtd, loff_t offs)
+{
+	struct spinand_device *spinand = mtd_to_spinand(mtd);
+	struct nand_device *nand = mtd_to_nanddev(mtd);
+	struct nand_pos pos;
+	int ret;
+
+	nanddev_offs_to_pos(nand, offs, &pos);
+	mutex_lock(&spinand->lock);
+	ret = nanddev_isreserved(nand, &pos);
+	mutex_unlock(&spinand->lock);
+
+	return ret;
+}
+
+const struct spi_mem_op *
+spinand_find_supported_op(struct spinand_device *spinand,
+			  const struct spi_mem_op *ops,
+			  unsigned int nops)
+{
+	unsigned int i;
+
+	for (i = 0; i < nops; i++) {
+		if (spi_mem_supports_op(spinand->spimem, &ops[i]))
+			return &ops[i];
+	}
+
+	return NULL;
+}
+
+static const struct nand_ops spinand_ops = {
+	.erase = spinand_erase,
+	.markbad = spinand_markbad,
+	.isbad = spinand_isbad,
+};
+
+static int spinand_manufacturer_detect(struct spinand_device *spinand)
+{
+	return -ENOTSUPP;
+}
+
+static int spinand_manufacturer_init(struct spinand_device *spinand)
+{
+	if (spinand->manufacturer->ops->init)
+		return spinand->manufacturer->ops->init(spinand);
+
+	return 0;
+}
+
+static void spinand_manufacturer_cleanup(struct spinand_device *spinand)
+{
+	/* Release manufacturer private data */
+	if (spinand->manufacturer->ops->cleanup)
+		return spinand->manufacturer->ops->cleanup(spinand);
+}
+
+static const struct spi_mem_op *
+spinand_select_op_variant(struct spinand_device *spinand,
+			  const struct spinand_op_variants *variants)
+{
+	struct nand_device *nand = spinand_to_nand(spinand);
+	unsigned int i;
+
+	for (i = 0; i < variants->nops; i++) {
+		struct spi_mem_op op = variants->ops[i];
+		unsigned int nbytes;
+		int ret;
+
+		nbytes = nanddev_per_page_oobsize(nand) +
+			 nanddev_page_size(nand);
+
+		while (nbytes) {
+			op.data.nbytes = nbytes;
+			ret = spi_mem_adjust_op_size(spinand->spimem, &op);
+			if (ret)
+				break;
+
+			if (!spi_mem_supports_op(spinand->spimem, &op))
+				break;
+
+			nbytes -= op.data.nbytes;
+		}
+
+		if (!nbytes)
+			return &variants->ops[i];
+	}
+
+	return NULL;
+}
+
+/**
+ * spinand_match_and_init() - Try to find a match between a device ID and an
+ *			      entry in a spinand_info table
+ * @spinand: SPI NAND object
+ * @table: SPI NAND device description table
+ * @table_size: size of the device description table
+ *
+ * Should be used by SPI NAND manufacturer drivers when they want to find a
+ * match between a device ID retrieved through the READ_ID command and an
+ * entry in the SPI NAND description table. If a match is found, the spinand
+ * object will be initialized with information provided by the matching
+ * spinand_info entry.
+ *
+ * Return: 0 on success, a negative error code otherwise.
+ */
+int spinand_match_and_init(struct spinand_device *spinand,
+			   const struct spinand_info *table,
+			   unsigned int table_size, u8 devid)
+{
+	struct nand_device *nand = spinand_to_nand(spinand);
+	unsigned int i;
+
+	for (i = 0; i < table_size; i++) {
+		const struct spinand_info *info = &table[i];
+		const struct spi_mem_op *op;
+
+		if (devid != info->devid)
+			continue;
+
+		nand->memorg = table[i].memorg;
+		nand->eccreq = table[i].eccreq;
+		spinand->eccinfo = table[i].eccinfo;
+		spinand->flags = table[i].flags;
+		spinand->select_target = table[i].select_target;
+
+		op = spinand_select_op_variant(spinand,
+					       info->op_variants.read_cache);
+		if (!op)
+			return -ENOTSUPP;
+
+		spinand->op_templates.read_cache = op;
+
+		op = spinand_select_op_variant(spinand,
+					       info->op_variants.write_cache);
+		if (!op)
+			return -ENOTSUPP;
+
+		spinand->op_templates.write_cache = op;
+
+		op = spinand_select_op_variant(spinand,
+					       info->op_variants.update_cache);
+		spinand->op_templates.update_cache = op;
+
+		return 0;
+	}
+
+	return -ENOTSUPP;
+}
+
+static int spinand_detect(struct spinand_device *spinand)
+{
+	struct device *dev = &spinand->spimem->spi->dev;
+	struct nand_device *nand = spinand_to_nand(spinand);
+	int ret;
+
+	ret = spinand_reset_op(spinand);
+	if (ret)
+		return ret;
+
+	ret = spinand_read_id_op(spinand, spinand->id.data);
+	if (ret)
+		return ret;
+
+	spinand->id.len = SPINAND_MAX_ID_LEN;
+
+	ret = spinand_manufacturer_detect(spinand);
+	if (ret) {
+		dev_err(dev, "unknown raw ID %*phN\n", SPINAND_MAX_ID_LEN,
+			spinand->id.data);
+		return ret;
+	}
+
+	if (nand->memorg.ntargets > 1 && !spinand->select_target) {
+		dev_err(dev,
+			"SPI NANDs with more than one die must implement ->select_target()\n");
+		return -EINVAL;
+	}
+
+	dev_info(&spinand->spimem->spi->dev,
+		 "%s SPI NAND was found.\n", spinand->manufacturer->name);
+	dev_info(&spinand->spimem->spi->dev,
+		 "%llu MiB, block size: %zu KiB, page size: %zu, OOB size: %u\n",
+		 nanddev_size(nand) >> 20, nanddev_eraseblock_size(nand) >> 10,
+		 nanddev_page_size(nand), nanddev_per_page_oobsize(nand));
+
+	return 0;
+}
+
+static int spinand_noecc_ooblayout_ecc(struct mtd_info *mtd, int section,
+				       struct mtd_oob_region *region)
+{
+	return -ERANGE;
+}
+
+static int spinand_noecc_ooblayout_free(struct mtd_info *mtd, int section,
+					struct mtd_oob_region *region)
+{
+	if (section)
+		return -ERANGE;
+
+	/* Reserve 2 bytes for the BBM. */
+	region->offset = 2;
+	region->length = 62;
+
+	return 0;
+}
+
+static const struct mtd_ooblayout_ops spinand_noecc_ooblayout = {
+	.ecc = spinand_noecc_ooblayout_ecc,
+	.free = spinand_noecc_ooblayout_free,
+};
+
+static int spinand_init(struct spinand_device *spinand)
+{
+	struct device *dev = &spinand->spimem->spi->dev;
+	struct mtd_info *mtd = spinand_to_mtd(spinand);
+	struct nand_device *nand = mtd_to_nanddev(mtd);
+	int ret, i;
+
+	/*
+	 * We need a scratch buffer because the spi_mem interface requires that
+	 * buf passed in spi_mem_op->data.buf be DMA-able.
+	 */
+	spinand->scratchbuf = kzalloc(SPINAND_MAX_ID_LEN, GFP_KERNEL);
+	if (!spinand->scratchbuf)
+		return -ENOMEM;
+
+	ret = spinand_detect(spinand);
+	if (ret)
+		goto err_free_bufs;
+
+	/*
+	 * Use kzalloc() instead of devm_kzalloc() here, because some drivers
+	 * may use this buffer for DMA access.
+	 * Memory allocated by devm_ does not guarantee DMA-safe alignment.
+	 */
+	spinand->databuf = kzalloc(nanddev_page_size(nand) +
+			       nanddev_per_page_oobsize(nand),
+			       GFP_KERNEL);
+	if (!spinand->databuf)
+		goto err_free_bufs;
+
+	spinand->oobbuf = spinand->databuf + nanddev_page_size(nand);
+
+	ret = spinand_init_cfg_cache(spinand);
+	if (ret)
+		goto err_free_bufs;
+
+	ret = spinand_init_quad_enable(spinand);
+	if (ret)
+		goto err_free_bufs;
+
+	ret = spinand_manufacturer_init(spinand);
+	if (ret) {
+		dev_err(dev,
+			"Failed to initialize the SPI NAND chip (err = %d)\n",
+			ret);
+		goto err_free_bufs;
+	}
+
+	/* After power up, all blocks are locked, so unlock them here. */
+	for (i = 0; i < nand->memorg.ntargets; i++) {
+		spinand_select_target(spinand, i);
+		spinand_lock_block(spinand, BL_ALL_UNLOCKED);
+	}
+
+	ret = nanddev_init(nand, &spinand_ops, THIS_MODULE);
+	if (ret)
+		goto err_manuf_cleanup;
+
+	/*
+	 * Right now, we don't support ECC, so let the whole oob
+	 * area is available for user.
+	 */
+	mtd->_read_oob = spinand_mtd_read;
+	mtd->_write_oob = spinand_mtd_write;
+	mtd->_block_isbad = spinand_mtd_block_isbad;
+	mtd->_block_markbad = spinand_mtd_block_markbad;
+	mtd->_block_isreserved = spinand_mtd_block_isreserved;
+	mtd->_erase = spinand_mtd_erase;
+
+	if (spinand->eccinfo.ooblayout)
+		mtd_set_ooblayout(mtd, spinand->eccinfo.ooblayout);
+	else
+		mtd_set_ooblayout(mtd, &spinand_noecc_ooblayout);
+
+	ret = mtd_ooblayout_count_freebytes(mtd);
+	if (ret < 0)
+		goto err_cleanup_nanddev;
+
+	return 0;
+
+err_cleanup_nanddev:
+	nanddev_cleanup(nand);
+
+err_manuf_cleanup:
+	spinand_manufacturer_cleanup(spinand);
+
+err_free_bufs:
+	kfree(spinand->databuf);
+	kfree(spinand->scratchbuf);
+	return ret;
+}
+
+static void spinand_cleanup(struct spinand_device *spinand)
+{
+	struct nand_device *nand = spinand_to_nand(spinand);
+
+	nanddev_cleanup(nand);
+	spinand_manufacturer_cleanup(spinand);
+	kfree(spinand->databuf);
+	kfree(spinand->scratchbuf);
+}
+
+static int spinand_probe(struct spi_mem *mem)
+{
+	struct spinand_device *spinand;
+	struct mtd_info *mtd;
+	int ret;
+
+	spinand = devm_kzalloc(&mem->spi->dev, sizeof(*spinand),
+			       GFP_KERNEL);
+	if (!spinand)
+		return -ENOMEM;
+
+	spinand->spimem = mem;
+	spi_mem_set_drvdata(mem, spinand);
+	spinand_set_of_node(spinand, mem->spi->dev.of_node);
+	mutex_init(&spinand->lock);
+	mtd = spinand_to_mtd(spinand);
+	mtd->dev.parent = &mem->spi->dev;
+
+	ret = spinand_init(spinand);
+	if (ret)
+		return ret;
+
+	ret = mtd_device_register(mtd, NULL, 0);
+	if (ret)
+		goto err_spinand_cleanup;
+
+	return 0;
+
+err_spinand_cleanup:
+	spinand_cleanup(spinand);
+
+	return ret;
+}
+
+static int spinand_remove(struct spi_mem *mem)
+{
+	struct spinand_device *spinand;
+	struct mtd_info *mtd;
+	int ret;
+
+	spinand = spi_mem_get_drvdata(mem);
+	mtd = spinand_to_mtd(spinand);
+
+	ret = mtd_device_unregister(mtd);
+	if (ret)
+		return ret;
+
+	spinand_cleanup(spinand);
+
+	return 0;
+}
+
+static const struct spi_device_id spinand_ids[] = {
+	{ .name = "spi-nand" },
+	{ /* sentinel */ },
+};
+
+#ifdef CONFIG_OF
+static const struct of_device_id spinand_of_ids[] = {
+	{ .compatible = "spi-nand" },
+	{ /* sentinel */ },
+};
+#endif
+
+static struct spi_mem_driver spinand_drv = {
+	.spidrv = {
+		.id_table = spinand_ids,
+		.driver = {
+			.name = "spi-nand",
+			.of_match_table = of_match_ptr(spinand_of_ids),
+		},
+	},
+	.probe = spinand_probe,
+	.remove = spinand_remove,
+};
+module_spi_mem_driver(spinand_drv);
+
+MODULE_DESCRIPTION("SPI NAND framework");
+MODULE_AUTHOR("Peter Pan<peterpandong@micron.com>");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/mtd/spinand.h b/include/linux/mtd/spinand.h
new file mode 100644
index 000000000000..17a4584c0017
--- /dev/null
+++ b/include/linux/mtd/spinand.h
@@ -0,0 +1,415 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2016-2017 Micron Technology, Inc.
+ *
+ *  Authors:
+ *	Peter Pan <peterpandong@micron.com>
+ */
+#ifndef __LINUX_MTD_SPINAND_H
+#define __LINUX_MTD_SPINAND_H
+
+#include <linux/mutex.h>
+#include <linux/bitops.h>
+#include <linux/device.h>
+#include <linux/mtd/mtd.h>
+#include <linux/mtd/nand.h>
+#include <linux/spi/spi.h>
+#include <linux/spi/spi-mem.h>
+
+/**
+ * Standard SPI NAND flash operations
+ */
+
+#define SPINAND_RESET_OP						\
+	SPI_MEM_OP(SPI_MEM_OP_CMD(0xff, 1),				\
+		   SPI_MEM_OP_NO_ADDR,					\
+		   SPI_MEM_OP_NO_DUMMY,					\
+		   SPI_MEM_OP_NO_DATA)
+
+#define SPINAND_WR_EN_DIS_OP(enable)					\
+	SPI_MEM_OP(SPI_MEM_OP_CMD((enable) ? 0x06 : 0x04, 1),		\
+		   SPI_MEM_OP_NO_ADDR,					\
+		   SPI_MEM_OP_NO_DUMMY,					\
+		   SPI_MEM_OP_NO_DATA)
+
+#define SPINAND_READID_OP(ndummy, buf, len)				\
+	SPI_MEM_OP(SPI_MEM_OP_CMD(0x9f, 1),				\
+		   SPI_MEM_OP_NO_ADDR,					\
+		   SPI_MEM_OP_DUMMY(ndummy, 1),				\
+		   SPI_MEM_OP_DATA_IN(len, buf, 1))
+
+#define SPINAND_SET_FEATURE_OP(reg, valptr)				\
+	SPI_MEM_OP(SPI_MEM_OP_CMD(0x1f, 1),				\
+		   SPI_MEM_OP_ADDR(1, reg, 1),				\
+		   SPI_MEM_OP_NO_DUMMY,					\
+		   SPI_MEM_OP_DATA_OUT(1, valptr, 1))
+
+#define SPINAND_GET_FEATURE_OP(reg, valptr)				\
+	SPI_MEM_OP(SPI_MEM_OP_CMD(0x0f, 1),				\
+		   SPI_MEM_OP_ADDR(1, reg, 1),				\
+		   SPI_MEM_OP_NO_DUMMY,					\
+		   SPI_MEM_OP_DATA_IN(1, valptr, 1))
+
+#define SPINAND_BLK_ERASE_OP(addr)					\
+	SPI_MEM_OP(SPI_MEM_OP_CMD(0xd8, 1),				\
+		   SPI_MEM_OP_ADDR(3, addr, 1),				\
+		   SPI_MEM_OP_NO_DUMMY,					\
+		   SPI_MEM_OP_NO_DATA)
+
+#define SPINAND_PAGE_READ_OP(addr)					\
+	SPI_MEM_OP(SPI_MEM_OP_CMD(0x13, 1),				\
+		   SPI_MEM_OP_ADDR(3, addr, 1),				\
+		   SPI_MEM_OP_NO_DUMMY,					\
+		   SPI_MEM_OP_NO_DATA)
+
+#define SPINAND_PAGE_READ_FROM_CACHE_OP(fast, addr, ndummy, buf, len)	\
+	SPI_MEM_OP(SPI_MEM_OP_CMD(fast ? 0x0b : 0x03, 1),		\
+		   SPI_MEM_OP_ADDR(2, addr, 1),				\
+		   SPI_MEM_OP_DUMMY(ndummy, 1),				\
+		   SPI_MEM_OP_DATA_IN(len, buf, 1))
+
+#define SPINAND_PAGE_READ_FROM_CACHE_X2_OP(addr, ndummy, buf, len)	\
+	SPI_MEM_OP(SPI_MEM_OP_CMD(0x3b, 1),				\
+		   SPI_MEM_OP_ADDR(2, addr, 1),				\
+		   SPI_MEM_OP_DUMMY(ndummy, 1),				\
+		   SPI_MEM_OP_DATA_IN(len, buf, 2))
+
+#define SPINAND_PAGE_READ_FROM_CACHE_X4_OP(addr, ndummy, buf, len)	\
+	SPI_MEM_OP(SPI_MEM_OP_CMD(0x6b, 1),				\
+		   SPI_MEM_OP_ADDR(2, addr, 1),				\
+		   SPI_MEM_OP_DUMMY(ndummy, 1),				\
+		   SPI_MEM_OP_DATA_IN(len, buf, 4))
+
+#define SPINAND_PAGE_READ_FROM_CACHE_DUALIO_OP(addr, ndummy, buf, len)	\
+	SPI_MEM_OP(SPI_MEM_OP_CMD(0xbb, 1),				\
+		   SPI_MEM_OP_ADDR(2, addr, 2),				\
+		   SPI_MEM_OP_DUMMY(ndummy, 2),				\
+		   SPI_MEM_OP_DATA_IN(len, buf, 2))
+
+#define SPINAND_PAGE_READ_FROM_CACHE_QUADIO_OP(addr, ndummy, buf, len)	\
+	SPI_MEM_OP(SPI_MEM_OP_CMD(0xeb, 1),				\
+		   SPI_MEM_OP_ADDR(2, addr, 4),				\
+		   SPI_MEM_OP_DUMMY(ndummy, 4),				\
+		   SPI_MEM_OP_DATA_IN(len, buf, 4))
+
+#define SPINAND_PROG_EXEC_OP(addr)					\
+	SPI_MEM_OP(SPI_MEM_OP_CMD(0x10, 1),				\
+		   SPI_MEM_OP_ADDR(3, addr, 1),				\
+		   SPI_MEM_OP_NO_DUMMY,					\
+		   SPI_MEM_OP_NO_DATA)
+
+#define SPINAND_PROG_LOAD(reset, addr, buf, len)			\
+	SPI_MEM_OP(SPI_MEM_OP_CMD(reset ? 0x02 : 0x84, 1),		\
+		   SPI_MEM_OP_ADDR(2, addr, 1),				\
+		   SPI_MEM_OP_NO_DUMMY,					\
+		   SPI_MEM_OP_DATA_OUT(len, buf, 1))
+
+#define SPINAND_PROG_LOAD_X4(reset, addr, buf, len)			\
+	SPI_MEM_OP(SPI_MEM_OP_CMD(reset ? 0x32 : 0x34, 1),		\
+		   SPI_MEM_OP_ADDR(2, addr, 1),				\
+		   SPI_MEM_OP_NO_DUMMY,					\
+		   SPI_MEM_OP_DATA_OUT(len, buf, 4))
+
+/**
+ * Standard SPI NAND flash commands
+ */
+#define SPINAND_CMD_PROG_LOAD_X4		0x32
+#define SPINAND_CMD_PROG_LOAD_RDM_DATA_X4	0x34
+
+/* feature register */
+#define REG_BLOCK_LOCK		0xa0
+#define BL_ALL_UNLOCKED		0x00
+
+/* configuration register */
+#define REG_CFG			0xb0
+#define CFG_ECC_ENABLE		BIT(4)
+#define CFG_QUAD_ENABLE		BIT(0)
+
+/* status register */
+#define REG_STATUS		0xc0
+#define STATUS_BUSY		BIT(0)
+#define STATUS_ERASE_FAILED	BIT(2)
+#define STATUS_PROG_FAILED	BIT(3)
+#define STATUS_ECC_MASK		GENMASK(5, 4)
+#define STATUS_ECC_NO_BITFLIPS	(0 << 4)
+#define STATUS_ECC_HAS_BITFLIPS	(1 << 4)
+#define STATUS_ECC_UNCOR_ERROR	(2 << 4)
+
+struct spinand_op;
+struct spinand_device;
+
+#define SPINAND_MAX_ID_LEN	4
+
+/**
+ * struct spinand_id - SPI NAND id structure
+ * @data: buffer containing the id bytes. Currently 4 bytes large, but can
+ *	  be extended if required
+ * @len: ID length
+ *
+ * struct_spinand_id->data contains all bytes returned after a READ_ID command,
+ * including dummy bytes if the chip does not emit ID bytes right after the
+ * READ_ID command. The responsibility to extract real ID bytes is left to
+ * struct_manufacurer_ops->detect().
+ */
+struct spinand_id {
+	u8 data[SPINAND_MAX_ID_LEN];
+	int len;
+};
+
+/**
+ * struct manufacurer_ops - SPI NAND manufacturer specific operations
+ * @detect: detect a SPI NAND device. Every time a SPI NAND device is probed
+ *	    the core calls the struct_manufacurer_ops->detect() hook of each
+ *	    registered manufacturer until one of them return 1. Note that
+ *	    the first thing to check in this hook is that the manufacturer ID
+ *	    in struct_spinand_device->id matches the manufacturer whose
+ *	    ->detect() hook has been called. Should return 1 if there's a
+ *	    match, 0 if the manufacturer ID does not match and a negative
+ *	    error code otherwise. When true is returned, the core assumes
+ *	    that properties of the NAND chip (spinand->base.memorg and
+ *	    spinand->base.eccreq) have been filled
+ * @init: initialize a SPI NAND device
+ * @cleanup: cleanup a SPI NAND device
+ *
+ * Each SPI NAND manufacturer driver should implement this interface so that
+ * NAND chips coming from this vendor can be detected and initialized properly.
+ */
+struct spinand_manufacturer_ops {
+	int (*detect)(struct spinand_device *spinand);
+	int (*init)(struct spinand_device *spinand);
+	void (*cleanup)(struct spinand_device *spinand);
+};
+
+/**
+ * struct spinand_manufacturer - SPI NAND manufacturer instance
+ * @id: manufacturer ID
+ * @name: manufacturer name
+ * @ops: manufacturer operations
+ */
+struct spinand_manufacturer {
+	u8 id;
+	char *name;
+	const struct spinand_manufacturer_ops *ops;
+};
+
+/**
+ * struct spinand_op_variants - SPI NAND operation variants
+ * @ops: the list of variants for a given operation
+ * @nops: the number of variants
+ *
+ * Some operations like read-from-cache/write-to-cache have several variants
+ * depending on the number of IO lines you use to transfer data or address
+ * cycles. This structure is a way to describe the different variants supported
+ * by a chip and let the core pick the best one based on the SPI mem controller
+ * capabilities.
+ */
+struct spinand_op_variants {
+	const struct spi_mem_op *ops;
+	unsigned int nops;
+};
+
+#define SPINAND_OP_VARIANTS(name, ...)					\
+	const struct spinand_op_variants name = {			\
+		.ops = (struct spi_mem_op[]) { __VA_ARGS__ },		\
+		.nops = sizeof((struct spi_mem_op[]){ __VA_ARGS__ }) /	\
+			sizeof(struct spi_mem_op),			\
+	}
+
+/**
+ * spinand_ecc_info - description of the on-die ECC implemented by a SPI NAND
+ *		      chip
+ * @get_status: get the ECC status. Should return a positive number encoding
+ *		the number of corrected bitflips if correction was possible or
+ *		-EBADMSG if there are uncorrectable errors. I can also return
+ *		other negative error codes if the error is not caused by
+ *		uncorrectable bitflips
+ * @ooblayout: the OOB layout used by the on-die ECC implementation
+ */
+struct spinand_ecc_info {
+	int (*get_status)(struct spinand_device *spinand, u8 status);
+	const struct mtd_ooblayout_ops *ooblayout;
+};
+
+#define SPINAND_HAS_QE_BIT		BIT(0)
+
+/**
+ * struct spinand_info - Structure used to describe SPI NAND chips
+ * @model: model name
+ * @devid: device ID
+ * @flags: OR-ing of the SPINAND_XXX flags
+ * @memorg: memory organization
+ * @eccreq: ECC requirements
+ * @eccinfo: on-die ECC info
+ * @op_variants: operations variants
+ * @op_variants.read_cache: variants of the read-cache operation
+ * @op_variants.write_cache: variants of the write-cache operation
+ * @op_variants.update_cache: variants of the update-cache operation
+ * @select_target: function used to select a target/die. Required only for
+ *		   multi-die chips
+ *
+ * Each SPI NAND manufacturer driver should have a spinand_info table
+ * describing all the chips supported by the driver.
+ */
+struct spinand_info {
+	const char *model;
+	u8 devid;
+	u32 flags;
+	struct nand_memory_organization memorg;
+	struct nand_ecc_req eccreq;
+	struct spinand_ecc_info eccinfo;
+	struct {
+		const struct spinand_op_variants *read_cache;
+		const struct spinand_op_variants *write_cache;
+		const struct spinand_op_variants *update_cache;
+	} op_variants;
+	int (*select_target)(struct spinand_device *spinand,
+			     unsigned int target);
+};
+
+#define SPINAND_INFO_OP_VARIANTS(__read, __write, __update)		\
+	{								\
+		.read_cache = __read,					\
+		.write_cache = __write,					\
+		.update_cache = __update,				\
+	}
+
+#define SPINAND_ECCINFO(__ooblayout, __get_status)			\
+	.eccinfo = {							\
+		.ooblayout = __ooblayout,				\
+		.get_status = __get_status,				\
+	}
+
+#define SPINAND_SELECT_TARGET(__func)					\
+	.select_target = __func,
+
+#define SPINAND_INFO(__model, __id, __memorg, __eccreq, __op_variants,	\
+		     __flags, ...)					\
+	{								\
+		.model = __model,					\
+		.devid = __id,						\
+		.memorg = __memorg,					\
+		.eccreq = __eccreq,					\
+		.op_variants = __op_variants,				\
+		.flags = __flags,					\
+		__VA_ARGS__						\
+	}
+
+/**
+ * struct spinand_device - SPI NAND device instance
+ * @base: NAND device instance
+ * @spimem: pointer to the SPI mem object
+ * @lock: lock used to serialize accesses to the NAND
+ * @id: NAND ID as returned by READ_ID
+ * @flags: NAND flags
+ * @op_templates: various SPI mem op templates
+ * @op_templates.read_cache: read cache op template
+ * @op_templates.write_cache: write cache op template
+ * @op_templates.update_cache: update cache op template
+ * @select_target: select a specific target/die. Usually called before sending
+ *		   a command addressing a page or an eraseblock embedded in
+ *		   this die. Only required if your chip exposes several dies
+ * @cur_target: currently selected target/die
+ * @eccinfo: on-die ECC information
+ * @cfg_cache: config register cache. One entry per die
+ * @databuf: bounce buffer for data
+ * @oobbuf: bounce buffer for OOB data
+ * @scratchbuf: buffer used for everything but page accesses. This is needed
+ *		because the spi-mem interface explicitly requests that buffers
+ *		passed in spi_mem_op be DMA-able, so we can't based the bufs on
+ *		the stack
+ * @manufacturer: SPI NAND manufacturer information
+ * @priv: manufacturer private data
+ */
+struct spinand_device {
+	struct nand_device base;
+	struct spi_mem *spimem;
+	struct mutex lock;
+	struct spinand_id id;
+	u32 flags;
+
+	struct {
+		const struct spi_mem_op *read_cache;
+		const struct spi_mem_op *write_cache;
+		const struct spi_mem_op *update_cache;
+	} op_templates;
+
+	int (*select_target)(struct spinand_device *spinand,
+			     unsigned int target);
+	unsigned int cur_target;
+
+	struct spinand_ecc_info eccinfo;
+
+	u8 *cfg_cache;
+	u8 *databuf;
+	u8 *oobbuf;
+	u8 *scratchbuf;
+	const struct spinand_manufacturer *manufacturer;
+	void *priv;
+};
+
+/**
+ * mtd_to_spinand() - Get the SPI NAND device attached to an MTD instance
+ * @mtd: MTD instance
+ *
+ * Return: the SPI NAND device attached to @mtd.
+ */
+static inline struct spinand_device *mtd_to_spinand(struct mtd_info *mtd)
+{
+	return container_of(mtd_to_nanddev(mtd), struct spinand_device, base);
+}
+
+/**
+ * spinand_to_mtd() - Get the MTD device embedded in a SPI NAND device
+ * @spinand: SPI NAND device
+ *
+ * Return: the MTD device embedded in @spinand.
+ */
+static inline struct mtd_info *spinand_to_mtd(struct spinand_device *spinand)
+{
+	return nanddev_to_mtd(&spinand->base);
+}
+
+/**
+ * nand_to_spinand() - Get the SPI NAND device embedding an NAND object
+ * @nand: NAND object
+ *
+ * Return: the SPI NAND device embedding @nand.
+ */
+static inline struct spinand_device *nand_to_spinand(struct nand_device *nand)
+{
+	return container_of(nand, struct spinand_device, base);
+}
+
+/**
+ * spinand_to_nand() - Get the NAND device embedded in a SPI NAND object
+ * @spinand: SPI NAND device
+ *
+ * Return: the NAND device embedded in @spinand.
+ */
+static inline struct nand_device *
+spinand_to_nand(struct spinand_device *spinand)
+{
+	return &spinand->base;
+}
+
+/**
+ * spinand_set_of_node - Attach a DT node to a SPI NAND device
+ * @spinand: SPI NAND device
+ * @np: DT node
+ *
+ * Attach a DT node to a SPI NAND device.
+ */
+static inline void spinand_set_of_node(struct spinand_device *spinand,
+				       struct device_node *np)
+{
+	nanddev_set_of_node(&spinand->base, np);
+}
+
+int spinand_match_and_init(struct spinand_device *dev,
+			   const struct spinand_info *table,
+			   unsigned int table_size, u8 devid);
+
+int spinand_upd_cfg(struct spinand_device *spinand, u8 mask, u8 val);
+int spinand_select_target(struct spinand_device *spinand, unsigned int target);
+
+#endif /* __LINUX_MTD_SPINAND_H */
diff --git a/include/linux/spi/spi-mem.h b/include/linux/spi/spi-mem.h
index bb4bd15ae1f6..4fa34a227a0f 100644
--- a/include/linux/spi/spi-mem.h
+++ b/include/linux/spi/spi-mem.h
@@ -3,7 +3,9 @@
  * Copyright (C) 2018 Exceet Electronics GmbH
  * Copyright (C) 2018 Bootlin
  *
- * Author: Boris Brezillon <boris.brezillon@bootlin.com>
+ * Author:
+ *	Peter Pan <peterpandong@micron.com>
+ *	Boris Brezillon <boris.brezillon@bootlin.com>
  */
 
 #ifndef __LINUX_SPI_MEM_H
-- 
2.14.1


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

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

* [PATCH v8 1/4] mtd: nand: Add core infrastructure to support SPI NANDs
@ 2018-06-01 13:13   ` Boris Brezillon
  0 siblings, 0 replies; 24+ messages in thread
From: Boris Brezillon @ 2018-06-01 13:13 UTC (permalink / raw)
  To: David Woodhouse, Brian Norris, Boris Brezillon, Marek Vasut,
	Richard Weinberger, linux-mtd, Miquel Raynal, Mark Brown,
	linux-spi, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell,
	Kumar Gala, devicetree
  Cc: Peter Pan

From: Peter Pan <peterpandong@micron.com>

Add a SPI NAND framework based on the generic NAND framework and the
spi-mem infrastructure.

In its current state, this framework supports the following features:

- single/dual/quad IO modes
- on-die ECC

Signed-off-by: Peter Pan <peterpandong@micron.com>
Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
---
Changes in v8:
- Use spinand_upd_cfg() instead of spinand_get_cfg()+spinand_set_cfg()
  in spinand_init_quad_enable() and spinand_ecc_enable()
- Make sure spinand_init_quad_enable() worked
- Do not check return code of spinand_read_page() in spinand_isbad()
- Fix the spinand_init() error path
---
 drivers/mtd/nand/Kconfig      |    1 +
 drivers/mtd/nand/Makefile     |    1 +
 drivers/mtd/nand/spi/Kconfig  |    7 +
 drivers/mtd/nand/spi/Makefile |    2 +
 drivers/mtd/nand/spi/core.c   | 1111 +++++++++++++++++++++++++++++++++++++++++
 include/linux/mtd/spinand.h   |  415 +++++++++++++++
 include/linux/spi/spi-mem.h   |    4 +-
 7 files changed, 1540 insertions(+), 1 deletion(-)
 create mode 100644 drivers/mtd/nand/spi/Kconfig
 create mode 100644 drivers/mtd/nand/spi/Makefile
 create mode 100644 drivers/mtd/nand/spi/core.c
 create mode 100644 include/linux/mtd/spinand.h

diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index 88c7d3b4ff8b..9033215e62ea 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -4,3 +4,4 @@ config MTD_NAND_CORE
 source "drivers/mtd/nand/onenand/Kconfig"
 
 source "drivers/mtd/nand/raw/Kconfig"
+source "drivers/mtd/nand/spi/Kconfig"
diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
index 3f0cb87f1a57..7ecd80c0a66e 100644
--- a/drivers/mtd/nand/Makefile
+++ b/drivers/mtd/nand/Makefile
@@ -5,3 +5,4 @@ obj-$(CONFIG_MTD_NAND_CORE) += nandcore.o
 
 obj-y	+= onenand/
 obj-y	+= raw/
+obj-y	+= spi/
diff --git a/drivers/mtd/nand/spi/Kconfig b/drivers/mtd/nand/spi/Kconfig
new file mode 100644
index 000000000000..7c37d2929b68
--- /dev/null
+++ b/drivers/mtd/nand/spi/Kconfig
@@ -0,0 +1,7 @@
+menuconfig MTD_SPI_NAND
+	tristate "SPI NAND device Support"
+	select MTD_NAND_CORE
+	depends on SPI_MASTER
+	select SPI_MEM
+	help
+	  This is the framework for the SPI NAND device drivers.
diff --git a/drivers/mtd/nand/spi/Makefile b/drivers/mtd/nand/spi/Makefile
new file mode 100644
index 000000000000..feb79a1d1b46
--- /dev/null
+++ b/drivers/mtd/nand/spi/Makefile
@@ -0,0 +1,2 @@
+spinand-objs := core.o
+obj-$(CONFIG_MTD_SPI_NAND) += spinand.o
diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
new file mode 100644
index 000000000000..a99ea352eb85
--- /dev/null
+++ b/drivers/mtd/nand/spi/core.c
@@ -0,0 +1,1111 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2016-2017 Micron Technology, Inc.
+ *
+ * Authors:
+ *	Peter Pan <peterpandong@micron.com>
+ *	Boris Brezillon <boris.brezillon@bootlin.com>
+ */
+
+#define pr_fmt(fmt)	"spi-nand: " fmt
+
+#include <linux/device.h>
+#include <linux/jiffies.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mtd/spinand.h>
+#include <linux/of.h>
+#include <linux/slab.h>
+#include <linux/spi/spi.h>
+#include <linux/spi/spi-mem.h>
+
+static void spinand_cache_op_adjust_colum(struct spinand_device *spinand,
+					  const struct nand_page_io_req *req,
+					  u16 *column)
+{
+	struct nand_device *nand = spinand_to_nand(spinand);
+	unsigned int shift;
+
+	if (nand->memorg.planes_per_lun < 2)
+		return;
+
+	/* The plane number is passed in MSB just above the column address */
+	shift = fls(nand->memorg.pagesize);
+	*column |= req->pos.plane << shift;
+}
+
+static int spinand_read_reg_op(struct spinand_device *spinand, u8 reg, u8 *val)
+{
+	struct spi_mem_op op = SPINAND_GET_FEATURE_OP(reg,
+						      spinand->scratchbuf);
+	int ret;
+
+	ret = spi_mem_exec_op(spinand->spimem, &op);
+	if (ret)
+		return ret;
+
+	*val = *spinand->scratchbuf;
+	return 0;
+}
+
+static int spinand_write_reg_op(struct spinand_device *spinand, u8 reg, u8 val)
+{
+	struct spi_mem_op op = SPINAND_SET_FEATURE_OP(reg,
+						      spinand->scratchbuf);
+
+	*spinand->scratchbuf = val;
+	return spi_mem_exec_op(spinand->spimem, &op);
+}
+
+static int spinand_read_status(struct spinand_device *spinand, u8 *status)
+{
+	return spinand_read_reg_op(spinand, REG_STATUS, status);
+}
+
+static int spinand_get_cfg(struct spinand_device *spinand, u8 *cfg)
+{
+	struct nand_device *nand = spinand_to_nand(spinand);
+
+	if (WARN_ON(spinand->cur_target < 0 ||
+		    spinand->cur_target >= nand->memorg.ntargets))
+		return -EINVAL;
+
+	*cfg = spinand->cfg_cache[spinand->cur_target];
+	return 0;
+}
+
+static int spinand_set_cfg(struct spinand_device *spinand, u8 cfg)
+{
+	struct nand_device *nand = spinand_to_nand(spinand);
+	int ret;
+
+	if (WARN_ON(spinand->cur_target < 0 ||
+		    spinand->cur_target >= nand->memorg.ntargets))
+		return -EINVAL;
+
+	if (spinand->cfg_cache[spinand->cur_target] == cfg)
+		return 0;
+
+	ret = spinand_write_reg_op(spinand, REG_CFG, cfg);
+	if (ret)
+		return ret;
+
+	spinand->cfg_cache[spinand->cur_target] = cfg;
+	return 0;
+}
+
+/**
+ * spinand_upd_cfg() - Update the configuration register
+ * @spinand: the spinand device
+ * @mask: the mask encoding the bits to update in the config reg
+ * @val: the new value to apply
+ *
+ * Update the configuration register.
+ *
+ * Return: 0 on success, a negative error code otherwise.
+ */
+int spinand_upd_cfg(struct spinand_device *spinand, u8 mask, u8 val)
+{
+	int ret;
+	u8 cfg;
+
+	ret = spinand_get_cfg(spinand, &cfg);
+	if (ret)
+		return ret;
+
+	cfg &= ~mask;
+	cfg |= val;
+
+	return spinand_set_cfg(spinand, cfg);
+}
+
+/**
+ * spinand_select_target() - Select a specific NAND target/die
+ * @spinand: the spinand device
+ * @target: the target/die to select
+ *
+ * Select a new target/die. If chip only has one die, this function is a NOOP.
+ *
+ * Return: 0 on success, a negative error code otherwise.
+ */
+int spinand_select_target(struct spinand_device *spinand, unsigned int target)
+{
+	struct nand_device *nand = spinand_to_nand(spinand);
+	int ret;
+
+	if (WARN_ON(target >= nand->memorg.ntargets))
+		return -EINVAL;
+
+	if (spinand->cur_target == target)
+		return 0;
+
+	if (nand->memorg.ntargets == 1) {
+		spinand->cur_target = target;
+		return 0;
+	}
+
+	ret = spinand->select_target(spinand, target);
+	if (ret)
+		return ret;
+
+	spinand->cur_target = target;
+	return 0;
+}
+
+static int spinand_init_cfg_cache(struct spinand_device *spinand)
+{
+	struct nand_device *nand = spinand_to_nand(spinand);
+	struct device *dev = &spinand->spimem->spi->dev;
+	unsigned int target;
+	int ret;
+
+	spinand->cfg_cache = devm_kzalloc(dev,
+					  sizeof(*spinand->cfg_cache) *
+					  nand->memorg.ntargets,
+					  GFP_KERNEL);
+	if (!spinand->cfg_cache)
+		return -ENOMEM;
+
+	for (target = 0; target < nand->memorg.ntargets; target++) {
+		ret = spinand_select_target(spinand, target);
+		if (ret)
+			return ret;
+
+		/*
+		 * We use spinand_read_reg_op() instead of spinand_get_cfg()
+		 * here to bypass the config cache.
+		 */
+		ret = spinand_read_reg_op(spinand, REG_CFG,
+					  &spinand->cfg_cache[target]);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static int spinand_init_quad_enable(struct spinand_device *spinand)
+{
+	bool enable = false;
+
+	if (!(spinand->flags & SPINAND_HAS_QE_BIT))
+		return 0;
+
+	if (spinand->op_templates.read_cache->data.buswidth == 4 ||
+	    spinand->op_templates.write_cache->data.buswidth == 4 ||
+	    spinand->op_templates.update_cache->data.buswidth == 4)
+		enable = true;
+
+	return spinand_upd_cfg(spinand, CFG_QUAD_ENABLE,
+			       enable ? CFG_QUAD_ENABLE : 0);
+}
+
+static void spinand_ecc_enable(struct spinand_device *spinand,
+			       bool enable)
+{
+	WARN_ON(spinand_upd_cfg(spinand, CFG_ECC_ENABLE,
+				enable ? CFG_ECC_ENABLE : 0));
+}
+
+static int spinand_write_enable_op(struct spinand_device *spinand)
+{
+	struct spi_mem_op op = SPINAND_WR_EN_DIS_OP(true);
+
+	return spi_mem_exec_op(spinand->spimem, &op);
+}
+
+static int spinand_load_page_op(struct spinand_device *spinand,
+				const struct nand_page_io_req *req)
+{
+	struct nand_device *nand = spinand_to_nand(spinand);
+	unsigned int row = nanddev_pos_to_row(nand, &req->pos);
+	struct spi_mem_op op = SPINAND_PAGE_READ_OP(row);
+
+	return spi_mem_exec_op(spinand->spimem, &op);
+}
+
+static int spinand_read_from_cache_op(struct spinand_device *spinand,
+				      const struct nand_page_io_req *req)
+{
+	struct spi_mem_op op = *spinand->op_templates.read_cache;
+	struct nand_device *nand = spinand_to_nand(spinand);
+	struct mtd_info *mtd = nanddev_to_mtd(nand);
+	struct nand_page_io_req adjreq = *req;
+	unsigned int nbytes = 0;
+	void *buf = NULL;
+	u16 column = 0;
+	int ret;
+
+	if (req->datalen) {
+		adjreq.datalen = nanddev_page_size(nand);
+		adjreq.dataoffs = 0;
+		adjreq.databuf.in = spinand->databuf;
+		buf = spinand->databuf;
+		nbytes = adjreq.datalen;
+	}
+
+	if (req->ooblen) {
+		adjreq.ooblen = nanddev_per_page_oobsize(nand);
+		adjreq.ooboffs = 0;
+		adjreq.oobbuf.in = spinand->oobbuf;
+		nbytes += nanddev_per_page_oobsize(nand);
+		if (!buf) {
+			buf = spinand->oobbuf;
+			column = nanddev_page_size(nand);
+		}
+	}
+
+	spinand_cache_op_adjust_colum(spinand, &adjreq, &column);
+	op.addr.val = column;
+
+	/*
+	 * Some controllers are limited in term of max RX data size. In this
+	 * case, just repeat the READ_CACHE operation after updating the
+	 * column.
+	 */
+	while (nbytes) {
+		op.data.buf.in = buf;
+		op.data.nbytes = nbytes;
+		ret = spi_mem_adjust_op_size(spinand->spimem, &op);
+		if (ret)
+			return ret;
+
+		ret = spi_mem_exec_op(spinand->spimem, &op);
+		if (ret)
+			return ret;
+
+		buf += op.data.nbytes;
+		nbytes -= op.data.nbytes;
+		op.addr.val += op.data.nbytes;
+	}
+
+	if (req->datalen)
+		memcpy(req->databuf.in, spinand->databuf + req->dataoffs,
+		       req->datalen);
+
+	if (req->ooblen) {
+		if (req->mode == MTD_OPS_AUTO_OOB)
+			mtd_ooblayout_get_databytes(mtd, req->oobbuf.in,
+						    spinand->oobbuf,
+						    req->ooboffs,
+						    req->ooblen);
+		else
+			memcpy(req->oobbuf.in, spinand->oobbuf + req->ooboffs,
+			       req->ooblen);
+	}
+
+	return 0;
+}
+
+static int spinand_write_to_cache_op(struct spinand_device *spinand,
+				     const struct nand_page_io_req *req)
+{
+	struct spi_mem_op op = *spinand->op_templates.write_cache;
+	struct nand_device *nand = spinand_to_nand(spinand);
+	struct mtd_info *mtd = nanddev_to_mtd(nand);
+	struct nand_page_io_req adjreq = *req;
+	unsigned int nbytes = 0;
+	void *buf = NULL;
+	u16 column = 0;
+	int ret;
+
+	memset(spinand->databuf, 0xff,
+	       nanddev_page_size(nand) +
+	       nanddev_per_page_oobsize(nand));
+
+	if (req->datalen) {
+		memcpy(spinand->databuf + req->dataoffs, req->databuf.out,
+		       req->datalen);
+		adjreq.dataoffs = 0;
+		adjreq.datalen = nanddev_page_size(nand);
+		adjreq.databuf.out = spinand->databuf;
+		nbytes = adjreq.datalen;
+		buf = spinand->databuf;
+	}
+
+	if (req->ooblen) {
+		if (req->mode == MTD_OPS_AUTO_OOB)
+			mtd_ooblayout_set_databytes(mtd, req->oobbuf.out,
+						    spinand->oobbuf,
+						    req->ooboffs,
+						    req->ooblen);
+		else
+			memcpy(spinand->oobbuf + req->ooboffs, req->oobbuf.out,
+			       req->ooblen);
+
+		adjreq.ooblen = nanddev_per_page_oobsize(nand);
+		adjreq.ooboffs = 0;
+		nbytes += nanddev_per_page_oobsize(nand);
+		if (!buf) {
+			buf = spinand->oobbuf;
+			column = nanddev_page_size(nand);
+		}
+	}
+
+	spinand_cache_op_adjust_colum(spinand, &adjreq, &column);
+
+	op = *spinand->op_templates.write_cache;
+	op.addr.val = column;
+
+	/*
+	 * Some controllers are limited in term of max TX data size. In this
+	 * case, split the operation into one LOAD CACHE and one or more
+	 * LOAD RANDOM CACHE.
+	 */
+	while (nbytes) {
+		op.data.buf.out = buf;
+		op.data.nbytes = nbytes;
+
+		ret = spi_mem_adjust_op_size(spinand->spimem, &op);
+		if (ret)
+			return ret;
+
+		ret = spi_mem_exec_op(spinand->spimem, &op);
+		if (ret)
+			return ret;
+
+		buf += op.data.nbytes;
+		nbytes -= op.data.nbytes;
+		op.addr.val += op.data.nbytes;
+
+		/*
+		 * We need to use the RANDOM LOAD CACHE operation if there's
+		 * more than one iteration, because the LOAD operation resets
+		 * the cache to 0xff.
+		 */
+		if (nbytes) {
+			column = op.addr.val;
+			op = *spinand->op_templates.update_cache;
+			op.addr.val = column;
+		}
+	}
+
+	return 0;
+}
+
+static int spinand_program_op(struct spinand_device *spinand,
+			      const struct nand_page_io_req *req)
+{
+	struct nand_device *nand = spinand_to_nand(spinand);
+	unsigned int row = nanddev_pos_to_row(nand, &req->pos);
+	struct spi_mem_op op = SPINAND_PROG_EXEC_OP(row);
+
+	return spi_mem_exec_op(spinand->spimem, &op);
+}
+
+static int spinand_erase_op(struct spinand_device *spinand,
+			    const struct nand_pos *pos)
+{
+	struct nand_device *nand = spinand_to_nand(spinand);
+	unsigned int row = nanddev_pos_to_row(nand, pos);
+	struct spi_mem_op op = SPINAND_BLK_ERASE_OP(row);
+
+	return spi_mem_exec_op(spinand->spimem, &op);
+}
+
+static int spinand_wait(struct spinand_device *spinand, u8 *s)
+{
+	unsigned long timeo =  jiffies + msecs_to_jiffies(400);
+	u8 status;
+
+	do {
+		spinand_read_status(spinand, &status);
+		if (!(status & STATUS_BUSY))
+			goto out;
+	} while (time_before(jiffies, timeo));
+
+	/*
+	 * Extra read, just in case the STATUS_READY bit has changed
+	 * since our last check
+	 */
+	spinand_read_status(spinand, &status);
+
+out:
+	if (s)
+		*s = status;
+
+	return status & STATUS_BUSY ? -ETIMEDOUT : 0;
+}
+
+static int spinand_read_id_op(struct spinand_device *spinand, u8 *buf)
+{
+	struct spi_mem_op op = SPINAND_READID_OP(0, spinand->scratchbuf,
+						 SPINAND_MAX_ID_LEN);
+	int ret;
+
+	ret = spi_mem_exec_op(spinand->spimem, &op);
+	if (!ret)
+		memcpy(buf, spinand->scratchbuf, SPINAND_MAX_ID_LEN);
+
+	return 0;
+}
+
+static int spinand_reset_op(struct spinand_device *spinand)
+{
+	struct spi_mem_op op = SPINAND_RESET_OP;
+	int ret;
+
+	ret = spi_mem_exec_op(spinand->spimem, &op);
+	if (ret)
+		return ret;
+
+	return spinand_wait(spinand, NULL);
+}
+
+static int spinand_lock_block(struct spinand_device *spinand, u8 lock)
+{
+	return spinand_write_reg_op(spinand, REG_BLOCK_LOCK, lock);
+}
+
+static int spinand_check_ecc_status(struct spinand_device *spinand, u8 status)
+{
+	struct nand_device *nand = spinand_to_nand(spinand);
+
+	if (spinand->eccinfo.get_status)
+		return spinand->eccinfo.get_status(spinand, status);
+
+	switch (status & STATUS_ECC_MASK) {
+	case STATUS_ECC_NO_BITFLIPS:
+		return 0;
+
+	case STATUS_ECC_HAS_BITFLIPS:
+		/*
+		 * We have no way to know exactly how many bitflips have been
+		 * fixed, so let's return the maximum possible value so that
+		 * wear-leveling layers move the data immediately.
+		 */
+		return nand->eccreq.strength;
+
+	case STATUS_ECC_UNCOR_ERROR:
+		return -EBADMSG;
+
+	default:
+		break;
+	}
+
+	return -EINVAL;
+}
+
+static int spinand_read_page(struct spinand_device *spinand,
+			     const struct nand_page_io_req *req,
+			     bool ecc_enabled)
+{
+	struct nand_device *nand = spinand_to_nand(spinand);
+	struct device *dev = &spinand->spimem->spi->dev;
+	u8 status;
+	int ret;
+
+	spinand_load_page_op(spinand, req);
+
+	ret = spinand_wait(spinand, &status);
+	if (ret < 0) {
+		dev_err(dev, "failed to load page @%llx (err = %d)\n",
+			nanddev_pos_to_offs(nand, &req->pos), ret);
+		return ret;
+	}
+
+	spinand_read_from_cache_op(spinand, req);
+
+	if (!ecc_enabled)
+		return 0;
+
+	return spinand_check_ecc_status(spinand, status);
+}
+
+static int spinand_write_page(struct spinand_device *spinand,
+			      const struct nand_page_io_req *req)
+{
+	struct nand_device *nand = spinand_to_nand(spinand);
+	struct device *dev = &spinand->spimem->spi->dev;
+	u8 status;
+	int ret = 0;
+
+	spinand_write_enable_op(spinand);
+	spinand_write_to_cache_op(spinand, req);
+	spinand_program_op(spinand, req);
+
+	ret = spinand_wait(spinand, &status);
+	if (!ret && (status & STATUS_PROG_FAILED))
+		ret = -EIO;
+
+	if (ret < 0)
+		dev_err(dev, "failed to program page @%llx (err = %d)\n",
+			nanddev_pos_to_offs(nand, &req->pos), ret);
+
+	return ret;
+}
+
+static int spinand_mtd_read(struct mtd_info *mtd, loff_t from,
+			    struct mtd_oob_ops *ops)
+{
+	struct spinand_device *spinand = mtd_to_spinand(mtd);
+	struct nand_device *nand = mtd_to_nanddev(mtd);
+	unsigned int max_bitflips = 0;
+	struct nand_io_iter iter;
+	bool enable_ecc = false;
+	bool ecc_failed = false;
+	int ret = 0;
+
+	if (ops->mode != MTD_OPS_RAW && spinand->eccinfo.ooblayout)
+		enable_ecc = true;
+
+	mutex_lock(&spinand->lock);
+
+	nanddev_io_for_each_page(nand, from, ops, &iter) {
+		spinand_select_target(spinand, iter.req.pos.target);
+		spinand_ecc_enable(spinand, enable_ecc);
+		ret = spinand_read_page(spinand, &iter.req, enable_ecc);
+		if (ret < 0 && ret != -EBADMSG)
+			break;
+
+		if (ret == -EBADMSG) {
+			ecc_failed = true;
+			mtd->ecc_stats.failed++;
+			ret = 0;
+		} else {
+			mtd->ecc_stats.corrected += ret;
+			max_bitflips = max_t(unsigned int, max_bitflips, ret);
+		}
+
+		ops->retlen += iter.req.datalen;
+		ops->oobretlen += iter.req.ooblen;
+	}
+
+	mutex_unlock(&spinand->lock);
+
+	if (ecc_failed && !ret)
+		ret = -EBADMSG;
+
+	return ret ? ret : max_bitflips;
+}
+
+static int spinand_mtd_write(struct mtd_info *mtd, loff_t to,
+			     struct mtd_oob_ops *ops)
+{
+	struct spinand_device *spinand = mtd_to_spinand(mtd);
+	struct nand_device *nand = mtd_to_nanddev(mtd);
+	struct nand_io_iter iter;
+	bool enable_ecc = false;
+	int ret = 0;
+
+	if (ops->mode != MTD_OPS_RAW && mtd->ooblayout)
+		enable_ecc = true;
+
+	mutex_lock(&spinand->lock);
+
+	nanddev_io_for_each_page(nand, to, ops, &iter) {
+		spinand_select_target(spinand, iter.req.pos.target);
+		spinand_ecc_enable(spinand, enable_ecc);
+
+		ret = spinand_write_page(spinand, &iter.req);
+		if (ret)
+			break;
+
+		ops->retlen += iter.req.datalen;
+		ops->oobretlen += iter.req.ooblen;
+	}
+
+	mutex_unlock(&spinand->lock);
+
+	return ret;
+}
+
+static bool spinand_isbad(struct nand_device *nand, const struct nand_pos *pos)
+{
+	struct spinand_device *spinand = nand_to_spinand(nand);
+	struct nand_page_io_req req = {
+		.pos = *pos,
+		.ooblen = 2,
+		.ooboffs = 0,
+		.oobbuf.in = spinand->oobbuf,
+		.mode = MTD_OPS_RAW,
+	};
+
+	memset(spinand->oobbuf, 0, 2);
+	spinand_select_target(spinand, pos->target);
+	spinand_read_page(spinand, &req, false);
+	if (spinand->oobbuf[0] != 0xff || spinand->oobbuf[1] != 0xff)
+		return true;
+
+	return false;
+}
+
+static int spinand_mtd_block_isbad(struct mtd_info *mtd, loff_t offs)
+{
+	struct nand_device *nand = mtd_to_nanddev(mtd);
+	struct spinand_device *spinand = nand_to_spinand(nand);
+	struct nand_pos pos;
+	int ret;
+
+	nanddev_offs_to_pos(nand, offs, &pos);
+	mutex_lock(&spinand->lock);
+	ret = nanddev_isbad(nand, &pos);
+	mutex_unlock(&spinand->lock);
+
+	return ret;
+}
+
+static int spinand_markbad(struct nand_device *nand, const struct nand_pos *pos)
+{
+	struct spinand_device *spinand = nand_to_spinand(nand);
+	struct nand_page_io_req req = {
+		.pos = *pos,
+		.ooboffs = 0,
+		.ooblen = 2,
+		.oobbuf.out = spinand->oobbuf,
+	};
+
+	/* Erase block before marking it bad. */
+	spinand_select_target(spinand, pos->target);
+	spinand_write_enable_op(spinand);
+	spinand_erase_op(spinand, pos);
+
+	memset(spinand->oobbuf, 0, 2);
+	return spinand_write_page(spinand, &req);
+}
+
+static int spinand_mtd_block_markbad(struct mtd_info *mtd, loff_t offs)
+{
+	struct nand_device *nand = mtd_to_nanddev(mtd);
+	struct spinand_device *spinand = nand_to_spinand(nand);
+	struct nand_pos pos;
+	int ret;
+
+	nanddev_offs_to_pos(nand, offs, &pos);
+	mutex_lock(&spinand->lock);
+	ret = nanddev_markbad(nand, &pos);
+	mutex_unlock(&spinand->lock);
+
+	return ret;
+}
+
+static int spinand_erase(struct nand_device *nand, const struct nand_pos *pos)
+{
+	struct spinand_device *spinand = nand_to_spinand(nand);
+	struct device *dev = &spinand->spimem->spi->dev;
+	u8 status;
+	int ret;
+
+	spinand_select_target(spinand, pos->target);
+	spinand_write_enable_op(spinand);
+	spinand_erase_op(spinand, pos);
+
+	ret = spinand_wait(spinand, &status);
+
+	if (!ret && (status & STATUS_ERASE_FAILED))
+		ret = -EIO;
+
+	if (ret)
+		dev_err(dev, "failed to erase block %d (err = %d)\n",
+			pos->eraseblock, ret);
+
+	return ret;
+}
+
+static int spinand_mtd_erase(struct mtd_info *mtd,
+			     struct erase_info *einfo)
+{
+	struct spinand_device *spinand = mtd_to_spinand(mtd);
+	int ret;
+
+	mutex_lock(&spinand->lock);
+	ret = nanddev_mtd_erase(mtd, einfo);
+	mutex_unlock(&spinand->lock);
+
+	return ret;
+}
+
+static int spinand_mtd_block_isreserved(struct mtd_info *mtd, loff_t offs)
+{
+	struct spinand_device *spinand = mtd_to_spinand(mtd);
+	struct nand_device *nand = mtd_to_nanddev(mtd);
+	struct nand_pos pos;
+	int ret;
+
+	nanddev_offs_to_pos(nand, offs, &pos);
+	mutex_lock(&spinand->lock);
+	ret = nanddev_isreserved(nand, &pos);
+	mutex_unlock(&spinand->lock);
+
+	return ret;
+}
+
+const struct spi_mem_op *
+spinand_find_supported_op(struct spinand_device *spinand,
+			  const struct spi_mem_op *ops,
+			  unsigned int nops)
+{
+	unsigned int i;
+
+	for (i = 0; i < nops; i++) {
+		if (spi_mem_supports_op(spinand->spimem, &ops[i]))
+			return &ops[i];
+	}
+
+	return NULL;
+}
+
+static const struct nand_ops spinand_ops = {
+	.erase = spinand_erase,
+	.markbad = spinand_markbad,
+	.isbad = spinand_isbad,
+};
+
+static int spinand_manufacturer_detect(struct spinand_device *spinand)
+{
+	return -ENOTSUPP;
+}
+
+static int spinand_manufacturer_init(struct spinand_device *spinand)
+{
+	if (spinand->manufacturer->ops->init)
+		return spinand->manufacturer->ops->init(spinand);
+
+	return 0;
+}
+
+static void spinand_manufacturer_cleanup(struct spinand_device *spinand)
+{
+	/* Release manufacturer private data */
+	if (spinand->manufacturer->ops->cleanup)
+		return spinand->manufacturer->ops->cleanup(spinand);
+}
+
+static const struct spi_mem_op *
+spinand_select_op_variant(struct spinand_device *spinand,
+			  const struct spinand_op_variants *variants)
+{
+	struct nand_device *nand = spinand_to_nand(spinand);
+	unsigned int i;
+
+	for (i = 0; i < variants->nops; i++) {
+		struct spi_mem_op op = variants->ops[i];
+		unsigned int nbytes;
+		int ret;
+
+		nbytes = nanddev_per_page_oobsize(nand) +
+			 nanddev_page_size(nand);
+
+		while (nbytes) {
+			op.data.nbytes = nbytes;
+			ret = spi_mem_adjust_op_size(spinand->spimem, &op);
+			if (ret)
+				break;
+
+			if (!spi_mem_supports_op(spinand->spimem, &op))
+				break;
+
+			nbytes -= op.data.nbytes;
+		}
+
+		if (!nbytes)
+			return &variants->ops[i];
+	}
+
+	return NULL;
+}
+
+/**
+ * spinand_match_and_init() - Try to find a match between a device ID and an
+ *			      entry in a spinand_info table
+ * @spinand: SPI NAND object
+ * @table: SPI NAND device description table
+ * @table_size: size of the device description table
+ *
+ * Should be used by SPI NAND manufacturer drivers when they want to find a
+ * match between a device ID retrieved through the READ_ID command and an
+ * entry in the SPI NAND description table. If a match is found, the spinand
+ * object will be initialized with information provided by the matching
+ * spinand_info entry.
+ *
+ * Return: 0 on success, a negative error code otherwise.
+ */
+int spinand_match_and_init(struct spinand_device *spinand,
+			   const struct spinand_info *table,
+			   unsigned int table_size, u8 devid)
+{
+	struct nand_device *nand = spinand_to_nand(spinand);
+	unsigned int i;
+
+	for (i = 0; i < table_size; i++) {
+		const struct spinand_info *info = &table[i];
+		const struct spi_mem_op *op;
+
+		if (devid != info->devid)
+			continue;
+
+		nand->memorg = table[i].memorg;
+		nand->eccreq = table[i].eccreq;
+		spinand->eccinfo = table[i].eccinfo;
+		spinand->flags = table[i].flags;
+		spinand->select_target = table[i].select_target;
+
+		op = spinand_select_op_variant(spinand,
+					       info->op_variants.read_cache);
+		if (!op)
+			return -ENOTSUPP;
+
+		spinand->op_templates.read_cache = op;
+
+		op = spinand_select_op_variant(spinand,
+					       info->op_variants.write_cache);
+		if (!op)
+			return -ENOTSUPP;
+
+		spinand->op_templates.write_cache = op;
+
+		op = spinand_select_op_variant(spinand,
+					       info->op_variants.update_cache);
+		spinand->op_templates.update_cache = op;
+
+		return 0;
+	}
+
+	return -ENOTSUPP;
+}
+
+static int spinand_detect(struct spinand_device *spinand)
+{
+	struct device *dev = &spinand->spimem->spi->dev;
+	struct nand_device *nand = spinand_to_nand(spinand);
+	int ret;
+
+	ret = spinand_reset_op(spinand);
+	if (ret)
+		return ret;
+
+	ret = spinand_read_id_op(spinand, spinand->id.data);
+	if (ret)
+		return ret;
+
+	spinand->id.len = SPINAND_MAX_ID_LEN;
+
+	ret = spinand_manufacturer_detect(spinand);
+	if (ret) {
+		dev_err(dev, "unknown raw ID %*phN\n", SPINAND_MAX_ID_LEN,
+			spinand->id.data);
+		return ret;
+	}
+
+	if (nand->memorg.ntargets > 1 && !spinand->select_target) {
+		dev_err(dev,
+			"SPI NANDs with more than one die must implement ->select_target()\n");
+		return -EINVAL;
+	}
+
+	dev_info(&spinand->spimem->spi->dev,
+		 "%s SPI NAND was found.\n", spinand->manufacturer->name);
+	dev_info(&spinand->spimem->spi->dev,
+		 "%llu MiB, block size: %zu KiB, page size: %zu, OOB size: %u\n",
+		 nanddev_size(nand) >> 20, nanddev_eraseblock_size(nand) >> 10,
+		 nanddev_page_size(nand), nanddev_per_page_oobsize(nand));
+
+	return 0;
+}
+
+static int spinand_noecc_ooblayout_ecc(struct mtd_info *mtd, int section,
+				       struct mtd_oob_region *region)
+{
+	return -ERANGE;
+}
+
+static int spinand_noecc_ooblayout_free(struct mtd_info *mtd, int section,
+					struct mtd_oob_region *region)
+{
+	if (section)
+		return -ERANGE;
+
+	/* Reserve 2 bytes for the BBM. */
+	region->offset = 2;
+	region->length = 62;
+
+	return 0;
+}
+
+static const struct mtd_ooblayout_ops spinand_noecc_ooblayout = {
+	.ecc = spinand_noecc_ooblayout_ecc,
+	.free = spinand_noecc_ooblayout_free,
+};
+
+static int spinand_init(struct spinand_device *spinand)
+{
+	struct device *dev = &spinand->spimem->spi->dev;
+	struct mtd_info *mtd = spinand_to_mtd(spinand);
+	struct nand_device *nand = mtd_to_nanddev(mtd);
+	int ret, i;
+
+	/*
+	 * We need a scratch buffer because the spi_mem interface requires that
+	 * buf passed in spi_mem_op->data.buf be DMA-able.
+	 */
+	spinand->scratchbuf = kzalloc(SPINAND_MAX_ID_LEN, GFP_KERNEL);
+	if (!spinand->scratchbuf)
+		return -ENOMEM;
+
+	ret = spinand_detect(spinand);
+	if (ret)
+		goto err_free_bufs;
+
+	/*
+	 * Use kzalloc() instead of devm_kzalloc() here, because some drivers
+	 * may use this buffer for DMA access.
+	 * Memory allocated by devm_ does not guarantee DMA-safe alignment.
+	 */
+	spinand->databuf = kzalloc(nanddev_page_size(nand) +
+			       nanddev_per_page_oobsize(nand),
+			       GFP_KERNEL);
+	if (!spinand->databuf)
+		goto err_free_bufs;
+
+	spinand->oobbuf = spinand->databuf + nanddev_page_size(nand);
+
+	ret = spinand_init_cfg_cache(spinand);
+	if (ret)
+		goto err_free_bufs;
+
+	ret = spinand_init_quad_enable(spinand);
+	if (ret)
+		goto err_free_bufs;
+
+	ret = spinand_manufacturer_init(spinand);
+	if (ret) {
+		dev_err(dev,
+			"Failed to initialize the SPI NAND chip (err = %d)\n",
+			ret);
+		goto err_free_bufs;
+	}
+
+	/* After power up, all blocks are locked, so unlock them here. */
+	for (i = 0; i < nand->memorg.ntargets; i++) {
+		spinand_select_target(spinand, i);
+		spinand_lock_block(spinand, BL_ALL_UNLOCKED);
+	}
+
+	ret = nanddev_init(nand, &spinand_ops, THIS_MODULE);
+	if (ret)
+		goto err_manuf_cleanup;
+
+	/*
+	 * Right now, we don't support ECC, so let the whole oob
+	 * area is available for user.
+	 */
+	mtd->_read_oob = spinand_mtd_read;
+	mtd->_write_oob = spinand_mtd_write;
+	mtd->_block_isbad = spinand_mtd_block_isbad;
+	mtd->_block_markbad = spinand_mtd_block_markbad;
+	mtd->_block_isreserved = spinand_mtd_block_isreserved;
+	mtd->_erase = spinand_mtd_erase;
+
+	if (spinand->eccinfo.ooblayout)
+		mtd_set_ooblayout(mtd, spinand->eccinfo.ooblayout);
+	else
+		mtd_set_ooblayout(mtd, &spinand_noecc_ooblayout);
+
+	ret = mtd_ooblayout_count_freebytes(mtd);
+	if (ret < 0)
+		goto err_cleanup_nanddev;
+
+	return 0;
+
+err_cleanup_nanddev:
+	nanddev_cleanup(nand);
+
+err_manuf_cleanup:
+	spinand_manufacturer_cleanup(spinand);
+
+err_free_bufs:
+	kfree(spinand->databuf);
+	kfree(spinand->scratchbuf);
+	return ret;
+}
+
+static void spinand_cleanup(struct spinand_device *spinand)
+{
+	struct nand_device *nand = spinand_to_nand(spinand);
+
+	nanddev_cleanup(nand);
+	spinand_manufacturer_cleanup(spinand);
+	kfree(spinand->databuf);
+	kfree(spinand->scratchbuf);
+}
+
+static int spinand_probe(struct spi_mem *mem)
+{
+	struct spinand_device *spinand;
+	struct mtd_info *mtd;
+	int ret;
+
+	spinand = devm_kzalloc(&mem->spi->dev, sizeof(*spinand),
+			       GFP_KERNEL);
+	if (!spinand)
+		return -ENOMEM;
+
+	spinand->spimem = mem;
+	spi_mem_set_drvdata(mem, spinand);
+	spinand_set_of_node(spinand, mem->spi->dev.of_node);
+	mutex_init(&spinand->lock);
+	mtd = spinand_to_mtd(spinand);
+	mtd->dev.parent = &mem->spi->dev;
+
+	ret = spinand_init(spinand);
+	if (ret)
+		return ret;
+
+	ret = mtd_device_register(mtd, NULL, 0);
+	if (ret)
+		goto err_spinand_cleanup;
+
+	return 0;
+
+err_spinand_cleanup:
+	spinand_cleanup(spinand);
+
+	return ret;
+}
+
+static int spinand_remove(struct spi_mem *mem)
+{
+	struct spinand_device *spinand;
+	struct mtd_info *mtd;
+	int ret;
+
+	spinand = spi_mem_get_drvdata(mem);
+	mtd = spinand_to_mtd(spinand);
+
+	ret = mtd_device_unregister(mtd);
+	if (ret)
+		return ret;
+
+	spinand_cleanup(spinand);
+
+	return 0;
+}
+
+static const struct spi_device_id spinand_ids[] = {
+	{ .name = "spi-nand" },
+	{ /* sentinel */ },
+};
+
+#ifdef CONFIG_OF
+static const struct of_device_id spinand_of_ids[] = {
+	{ .compatible = "spi-nand" },
+	{ /* sentinel */ },
+};
+#endif
+
+static struct spi_mem_driver spinand_drv = {
+	.spidrv = {
+		.id_table = spinand_ids,
+		.driver = {
+			.name = "spi-nand",
+			.of_match_table = of_match_ptr(spinand_of_ids),
+		},
+	},
+	.probe = spinand_probe,
+	.remove = spinand_remove,
+};
+module_spi_mem_driver(spinand_drv);
+
+MODULE_DESCRIPTION("SPI NAND framework");
+MODULE_AUTHOR("Peter Pan<peterpandong@micron.com>");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/mtd/spinand.h b/include/linux/mtd/spinand.h
new file mode 100644
index 000000000000..17a4584c0017
--- /dev/null
+++ b/include/linux/mtd/spinand.h
@@ -0,0 +1,415 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2016-2017 Micron Technology, Inc.
+ *
+ *  Authors:
+ *	Peter Pan <peterpandong@micron.com>
+ */
+#ifndef __LINUX_MTD_SPINAND_H
+#define __LINUX_MTD_SPINAND_H
+
+#include <linux/mutex.h>
+#include <linux/bitops.h>
+#include <linux/device.h>
+#include <linux/mtd/mtd.h>
+#include <linux/mtd/nand.h>
+#include <linux/spi/spi.h>
+#include <linux/spi/spi-mem.h>
+
+/**
+ * Standard SPI NAND flash operations
+ */
+
+#define SPINAND_RESET_OP						\
+	SPI_MEM_OP(SPI_MEM_OP_CMD(0xff, 1),				\
+		   SPI_MEM_OP_NO_ADDR,					\
+		   SPI_MEM_OP_NO_DUMMY,					\
+		   SPI_MEM_OP_NO_DATA)
+
+#define SPINAND_WR_EN_DIS_OP(enable)					\
+	SPI_MEM_OP(SPI_MEM_OP_CMD((enable) ? 0x06 : 0x04, 1),		\
+		   SPI_MEM_OP_NO_ADDR,					\
+		   SPI_MEM_OP_NO_DUMMY,					\
+		   SPI_MEM_OP_NO_DATA)
+
+#define SPINAND_READID_OP(ndummy, buf, len)				\
+	SPI_MEM_OP(SPI_MEM_OP_CMD(0x9f, 1),				\
+		   SPI_MEM_OP_NO_ADDR,					\
+		   SPI_MEM_OP_DUMMY(ndummy, 1),				\
+		   SPI_MEM_OP_DATA_IN(len, buf, 1))
+
+#define SPINAND_SET_FEATURE_OP(reg, valptr)				\
+	SPI_MEM_OP(SPI_MEM_OP_CMD(0x1f, 1),				\
+		   SPI_MEM_OP_ADDR(1, reg, 1),				\
+		   SPI_MEM_OP_NO_DUMMY,					\
+		   SPI_MEM_OP_DATA_OUT(1, valptr, 1))
+
+#define SPINAND_GET_FEATURE_OP(reg, valptr)				\
+	SPI_MEM_OP(SPI_MEM_OP_CMD(0x0f, 1),				\
+		   SPI_MEM_OP_ADDR(1, reg, 1),				\
+		   SPI_MEM_OP_NO_DUMMY,					\
+		   SPI_MEM_OP_DATA_IN(1, valptr, 1))
+
+#define SPINAND_BLK_ERASE_OP(addr)					\
+	SPI_MEM_OP(SPI_MEM_OP_CMD(0xd8, 1),				\
+		   SPI_MEM_OP_ADDR(3, addr, 1),				\
+		   SPI_MEM_OP_NO_DUMMY,					\
+		   SPI_MEM_OP_NO_DATA)
+
+#define SPINAND_PAGE_READ_OP(addr)					\
+	SPI_MEM_OP(SPI_MEM_OP_CMD(0x13, 1),				\
+		   SPI_MEM_OP_ADDR(3, addr, 1),				\
+		   SPI_MEM_OP_NO_DUMMY,					\
+		   SPI_MEM_OP_NO_DATA)
+
+#define SPINAND_PAGE_READ_FROM_CACHE_OP(fast, addr, ndummy, buf, len)	\
+	SPI_MEM_OP(SPI_MEM_OP_CMD(fast ? 0x0b : 0x03, 1),		\
+		   SPI_MEM_OP_ADDR(2, addr, 1),				\
+		   SPI_MEM_OP_DUMMY(ndummy, 1),				\
+		   SPI_MEM_OP_DATA_IN(len, buf, 1))
+
+#define SPINAND_PAGE_READ_FROM_CACHE_X2_OP(addr, ndummy, buf, len)	\
+	SPI_MEM_OP(SPI_MEM_OP_CMD(0x3b, 1),				\
+		   SPI_MEM_OP_ADDR(2, addr, 1),				\
+		   SPI_MEM_OP_DUMMY(ndummy, 1),				\
+		   SPI_MEM_OP_DATA_IN(len, buf, 2))
+
+#define SPINAND_PAGE_READ_FROM_CACHE_X4_OP(addr, ndummy, buf, len)	\
+	SPI_MEM_OP(SPI_MEM_OP_CMD(0x6b, 1),				\
+		   SPI_MEM_OP_ADDR(2, addr, 1),				\
+		   SPI_MEM_OP_DUMMY(ndummy, 1),				\
+		   SPI_MEM_OP_DATA_IN(len, buf, 4))
+
+#define SPINAND_PAGE_READ_FROM_CACHE_DUALIO_OP(addr, ndummy, buf, len)	\
+	SPI_MEM_OP(SPI_MEM_OP_CMD(0xbb, 1),				\
+		   SPI_MEM_OP_ADDR(2, addr, 2),				\
+		   SPI_MEM_OP_DUMMY(ndummy, 2),				\
+		   SPI_MEM_OP_DATA_IN(len, buf, 2))
+
+#define SPINAND_PAGE_READ_FROM_CACHE_QUADIO_OP(addr, ndummy, buf, len)	\
+	SPI_MEM_OP(SPI_MEM_OP_CMD(0xeb, 1),				\
+		   SPI_MEM_OP_ADDR(2, addr, 4),				\
+		   SPI_MEM_OP_DUMMY(ndummy, 4),				\
+		   SPI_MEM_OP_DATA_IN(len, buf, 4))
+
+#define SPINAND_PROG_EXEC_OP(addr)					\
+	SPI_MEM_OP(SPI_MEM_OP_CMD(0x10, 1),				\
+		   SPI_MEM_OP_ADDR(3, addr, 1),				\
+		   SPI_MEM_OP_NO_DUMMY,					\
+		   SPI_MEM_OP_NO_DATA)
+
+#define SPINAND_PROG_LOAD(reset, addr, buf, len)			\
+	SPI_MEM_OP(SPI_MEM_OP_CMD(reset ? 0x02 : 0x84, 1),		\
+		   SPI_MEM_OP_ADDR(2, addr, 1),				\
+		   SPI_MEM_OP_NO_DUMMY,					\
+		   SPI_MEM_OP_DATA_OUT(len, buf, 1))
+
+#define SPINAND_PROG_LOAD_X4(reset, addr, buf, len)			\
+	SPI_MEM_OP(SPI_MEM_OP_CMD(reset ? 0x32 : 0x34, 1),		\
+		   SPI_MEM_OP_ADDR(2, addr, 1),				\
+		   SPI_MEM_OP_NO_DUMMY,					\
+		   SPI_MEM_OP_DATA_OUT(len, buf, 4))
+
+/**
+ * Standard SPI NAND flash commands
+ */
+#define SPINAND_CMD_PROG_LOAD_X4		0x32
+#define SPINAND_CMD_PROG_LOAD_RDM_DATA_X4	0x34
+
+/* feature register */
+#define REG_BLOCK_LOCK		0xa0
+#define BL_ALL_UNLOCKED		0x00
+
+/* configuration register */
+#define REG_CFG			0xb0
+#define CFG_ECC_ENABLE		BIT(4)
+#define CFG_QUAD_ENABLE		BIT(0)
+
+/* status register */
+#define REG_STATUS		0xc0
+#define STATUS_BUSY		BIT(0)
+#define STATUS_ERASE_FAILED	BIT(2)
+#define STATUS_PROG_FAILED	BIT(3)
+#define STATUS_ECC_MASK		GENMASK(5, 4)
+#define STATUS_ECC_NO_BITFLIPS	(0 << 4)
+#define STATUS_ECC_HAS_BITFLIPS	(1 << 4)
+#define STATUS_ECC_UNCOR_ERROR	(2 << 4)
+
+struct spinand_op;
+struct spinand_device;
+
+#define SPINAND_MAX_ID_LEN	4
+
+/**
+ * struct spinand_id - SPI NAND id structure
+ * @data: buffer containing the id bytes. Currently 4 bytes large, but can
+ *	  be extended if required
+ * @len: ID length
+ *
+ * struct_spinand_id->data contains all bytes returned after a READ_ID command,
+ * including dummy bytes if the chip does not emit ID bytes right after the
+ * READ_ID command. The responsibility to extract real ID bytes is left to
+ * struct_manufacurer_ops->detect().
+ */
+struct spinand_id {
+	u8 data[SPINAND_MAX_ID_LEN];
+	int len;
+};
+
+/**
+ * struct manufacurer_ops - SPI NAND manufacturer specific operations
+ * @detect: detect a SPI NAND device. Every time a SPI NAND device is probed
+ *	    the core calls the struct_manufacurer_ops->detect() hook of each
+ *	    registered manufacturer until one of them return 1. Note that
+ *	    the first thing to check in this hook is that the manufacturer ID
+ *	    in struct_spinand_device->id matches the manufacturer whose
+ *	    ->detect() hook has been called. Should return 1 if there's a
+ *	    match, 0 if the manufacturer ID does not match and a negative
+ *	    error code otherwise. When true is returned, the core assumes
+ *	    that properties of the NAND chip (spinand->base.memorg and
+ *	    spinand->base.eccreq) have been filled
+ * @init: initialize a SPI NAND device
+ * @cleanup: cleanup a SPI NAND device
+ *
+ * Each SPI NAND manufacturer driver should implement this interface so that
+ * NAND chips coming from this vendor can be detected and initialized properly.
+ */
+struct spinand_manufacturer_ops {
+	int (*detect)(struct spinand_device *spinand);
+	int (*init)(struct spinand_device *spinand);
+	void (*cleanup)(struct spinand_device *spinand);
+};
+
+/**
+ * struct spinand_manufacturer - SPI NAND manufacturer instance
+ * @id: manufacturer ID
+ * @name: manufacturer name
+ * @ops: manufacturer operations
+ */
+struct spinand_manufacturer {
+	u8 id;
+	char *name;
+	const struct spinand_manufacturer_ops *ops;
+};
+
+/**
+ * struct spinand_op_variants - SPI NAND operation variants
+ * @ops: the list of variants for a given operation
+ * @nops: the number of variants
+ *
+ * Some operations like read-from-cache/write-to-cache have several variants
+ * depending on the number of IO lines you use to transfer data or address
+ * cycles. This structure is a way to describe the different variants supported
+ * by a chip and let the core pick the best one based on the SPI mem controller
+ * capabilities.
+ */
+struct spinand_op_variants {
+	const struct spi_mem_op *ops;
+	unsigned int nops;
+};
+
+#define SPINAND_OP_VARIANTS(name, ...)					\
+	const struct spinand_op_variants name = {			\
+		.ops = (struct spi_mem_op[]) { __VA_ARGS__ },		\
+		.nops = sizeof((struct spi_mem_op[]){ __VA_ARGS__ }) /	\
+			sizeof(struct spi_mem_op),			\
+	}
+
+/**
+ * spinand_ecc_info - description of the on-die ECC implemented by a SPI NAND
+ *		      chip
+ * @get_status: get the ECC status. Should return a positive number encoding
+ *		the number of corrected bitflips if correction was possible or
+ *		-EBADMSG if there are uncorrectable errors. I can also return
+ *		other negative error codes if the error is not caused by
+ *		uncorrectable bitflips
+ * @ooblayout: the OOB layout used by the on-die ECC implementation
+ */
+struct spinand_ecc_info {
+	int (*get_status)(struct spinand_device *spinand, u8 status);
+	const struct mtd_ooblayout_ops *ooblayout;
+};
+
+#define SPINAND_HAS_QE_BIT		BIT(0)
+
+/**
+ * struct spinand_info - Structure used to describe SPI NAND chips
+ * @model: model name
+ * @devid: device ID
+ * @flags: OR-ing of the SPINAND_XXX flags
+ * @memorg: memory organization
+ * @eccreq: ECC requirements
+ * @eccinfo: on-die ECC info
+ * @op_variants: operations variants
+ * @op_variants.read_cache: variants of the read-cache operation
+ * @op_variants.write_cache: variants of the write-cache operation
+ * @op_variants.update_cache: variants of the update-cache operation
+ * @select_target: function used to select a target/die. Required only for
+ *		   multi-die chips
+ *
+ * Each SPI NAND manufacturer driver should have a spinand_info table
+ * describing all the chips supported by the driver.
+ */
+struct spinand_info {
+	const char *model;
+	u8 devid;
+	u32 flags;
+	struct nand_memory_organization memorg;
+	struct nand_ecc_req eccreq;
+	struct spinand_ecc_info eccinfo;
+	struct {
+		const struct spinand_op_variants *read_cache;
+		const struct spinand_op_variants *write_cache;
+		const struct spinand_op_variants *update_cache;
+	} op_variants;
+	int (*select_target)(struct spinand_device *spinand,
+			     unsigned int target);
+};
+
+#define SPINAND_INFO_OP_VARIANTS(__read, __write, __update)		\
+	{								\
+		.read_cache = __read,					\
+		.write_cache = __write,					\
+		.update_cache = __update,				\
+	}
+
+#define SPINAND_ECCINFO(__ooblayout, __get_status)			\
+	.eccinfo = {							\
+		.ooblayout = __ooblayout,				\
+		.get_status = __get_status,				\
+	}
+
+#define SPINAND_SELECT_TARGET(__func)					\
+	.select_target = __func,
+
+#define SPINAND_INFO(__model, __id, __memorg, __eccreq, __op_variants,	\
+		     __flags, ...)					\
+	{								\
+		.model = __model,					\
+		.devid = __id,						\
+		.memorg = __memorg,					\
+		.eccreq = __eccreq,					\
+		.op_variants = __op_variants,				\
+		.flags = __flags,					\
+		__VA_ARGS__						\
+	}
+
+/**
+ * struct spinand_device - SPI NAND device instance
+ * @base: NAND device instance
+ * @spimem: pointer to the SPI mem object
+ * @lock: lock used to serialize accesses to the NAND
+ * @id: NAND ID as returned by READ_ID
+ * @flags: NAND flags
+ * @op_templates: various SPI mem op templates
+ * @op_templates.read_cache: read cache op template
+ * @op_templates.write_cache: write cache op template
+ * @op_templates.update_cache: update cache op template
+ * @select_target: select a specific target/die. Usually called before sending
+ *		   a command addressing a page or an eraseblock embedded in
+ *		   this die. Only required if your chip exposes several dies
+ * @cur_target: currently selected target/die
+ * @eccinfo: on-die ECC information
+ * @cfg_cache: config register cache. One entry per die
+ * @databuf: bounce buffer for data
+ * @oobbuf: bounce buffer for OOB data
+ * @scratchbuf: buffer used for everything but page accesses. This is needed
+ *		because the spi-mem interface explicitly requests that buffers
+ *		passed in spi_mem_op be DMA-able, so we can't based the bufs on
+ *		the stack
+ * @manufacturer: SPI NAND manufacturer information
+ * @priv: manufacturer private data
+ */
+struct spinand_device {
+	struct nand_device base;
+	struct spi_mem *spimem;
+	struct mutex lock;
+	struct spinand_id id;
+	u32 flags;
+
+	struct {
+		const struct spi_mem_op *read_cache;
+		const struct spi_mem_op *write_cache;
+		const struct spi_mem_op *update_cache;
+	} op_templates;
+
+	int (*select_target)(struct spinand_device *spinand,
+			     unsigned int target);
+	unsigned int cur_target;
+
+	struct spinand_ecc_info eccinfo;
+
+	u8 *cfg_cache;
+	u8 *databuf;
+	u8 *oobbuf;
+	u8 *scratchbuf;
+	const struct spinand_manufacturer *manufacturer;
+	void *priv;
+};
+
+/**
+ * mtd_to_spinand() - Get the SPI NAND device attached to an MTD instance
+ * @mtd: MTD instance
+ *
+ * Return: the SPI NAND device attached to @mtd.
+ */
+static inline struct spinand_device *mtd_to_spinand(struct mtd_info *mtd)
+{
+	return container_of(mtd_to_nanddev(mtd), struct spinand_device, base);
+}
+
+/**
+ * spinand_to_mtd() - Get the MTD device embedded in a SPI NAND device
+ * @spinand: SPI NAND device
+ *
+ * Return: the MTD device embedded in @spinand.
+ */
+static inline struct mtd_info *spinand_to_mtd(struct spinand_device *spinand)
+{
+	return nanddev_to_mtd(&spinand->base);
+}
+
+/**
+ * nand_to_spinand() - Get the SPI NAND device embedding an NAND object
+ * @nand: NAND object
+ *
+ * Return: the SPI NAND device embedding @nand.
+ */
+static inline struct spinand_device *nand_to_spinand(struct nand_device *nand)
+{
+	return container_of(nand, struct spinand_device, base);
+}
+
+/**
+ * spinand_to_nand() - Get the NAND device embedded in a SPI NAND object
+ * @spinand: SPI NAND device
+ *
+ * Return: the NAND device embedded in @spinand.
+ */
+static inline struct nand_device *
+spinand_to_nand(struct spinand_device *spinand)
+{
+	return &spinand->base;
+}
+
+/**
+ * spinand_set_of_node - Attach a DT node to a SPI NAND device
+ * @spinand: SPI NAND device
+ * @np: DT node
+ *
+ * Attach a DT node to a SPI NAND device.
+ */
+static inline void spinand_set_of_node(struct spinand_device *spinand,
+				       struct device_node *np)
+{
+	nanddev_set_of_node(&spinand->base, np);
+}
+
+int spinand_match_and_init(struct spinand_device *dev,
+			   const struct spinand_info *table,
+			   unsigned int table_size, u8 devid);
+
+int spinand_upd_cfg(struct spinand_device *spinand, u8 mask, u8 val);
+int spinand_select_target(struct spinand_device *spinand, unsigned int target);
+
+#endif /* __LINUX_MTD_SPINAND_H */
diff --git a/include/linux/spi/spi-mem.h b/include/linux/spi/spi-mem.h
index bb4bd15ae1f6..4fa34a227a0f 100644
--- a/include/linux/spi/spi-mem.h
+++ b/include/linux/spi/spi-mem.h
@@ -3,7 +3,9 @@
  * Copyright (C) 2018 Exceet Electronics GmbH
  * Copyright (C) 2018 Bootlin
  *
- * Author: Boris Brezillon <boris.brezillon@bootlin.com>
+ * Author:
+ *	Peter Pan <peterpandong@micron.com>
+ *	Boris Brezillon <boris.brezillon@bootlin.com>
  */
 
 #ifndef __LINUX_SPI_MEM_H
-- 
2.14.1

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

* [PATCH v8 2/4] dt-bindings: Add bindings for SPI NAND devices
  2018-06-01 13:13 ` Boris Brezillon
@ 2018-06-01 13:13   ` Boris Brezillon
  -1 siblings, 0 replies; 24+ messages in thread
From: Boris Brezillon @ 2018-06-01 13:13 UTC (permalink / raw)
  To: David Woodhouse, Brian Norris, Boris Brezillon, Marek Vasut,
	Richard Weinberger, linux-mtd, Miquel Raynal, Mark Brown,
	linux-spi, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell,
	Kumar Gala, devicetree

Add bindings for SPI NAND chips.

Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
---
Changes in v8:
- Fixed a typo in the commit message
---
 Documentation/devicetree/bindings/mtd/spi-nand.txt | 27 ++++++++++++++++++++++
 1 file changed, 27 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mtd/spi-nand.txt

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


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

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

* [PATCH v8 2/4] dt-bindings: Add bindings for SPI NAND devices
@ 2018-06-01 13:13   ` Boris Brezillon
  0 siblings, 0 replies; 24+ messages in thread
From: Boris Brezillon @ 2018-06-01 13:13 UTC (permalink / raw)
  To: David Woodhouse, Brian Norris, Boris Brezillon, Marek Vasut,
	Richard Weinberger, linux-mtd, Miquel Raynal, Mark Brown,
	linux-spi, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell,
	Kumar Gala, devicetree

Add bindings for SPI NAND chips.

Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
---
Changes in v8:
- Fixed a typo in the commit message
---
 Documentation/devicetree/bindings/mtd/spi-nand.txt | 27 ++++++++++++++++++++++
 1 file changed, 27 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mtd/spi-nand.txt

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

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

* [PATCH v8 3/4] mtd: spinand: Add initial support for Micron MT29F2G01ABAGD
  2018-06-01 13:13 ` Boris Brezillon
@ 2018-06-01 13:13   ` Boris Brezillon
  -1 siblings, 0 replies; 24+ messages in thread
From: Boris Brezillon @ 2018-06-01 13:13 UTC (permalink / raw)
  To: David Woodhouse, Brian Norris, Boris Brezillon, Marek Vasut,
	Richard Weinberger, linux-mtd, Miquel Raynal, Mark Brown,
	linux-spi, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell,
	Kumar Gala, devicetree
  Cc: Peter Pan

From: Peter Pan <peterpandong@micron.com>

Add a basic driver for Micron SPI NANDs. Only one device is supported
right now, but the driver will be extended to support more devices
afterwards.

Signed-off-by: Peter Pan <peterpandong@micron.com>
Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
---
Changes in v8:
- Changed the subject prefix
---
 drivers/mtd/nand/spi/Makefile |   2 +-
 drivers/mtd/nand/spi/core.c   |  17 ++++++
 drivers/mtd/nand/spi/micron.c | 133 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/mtd/spinand.h   |   3 +
 4 files changed, 154 insertions(+), 1 deletion(-)
 create mode 100644 drivers/mtd/nand/spi/micron.c

diff --git a/drivers/mtd/nand/spi/Makefile b/drivers/mtd/nand/spi/Makefile
index feb79a1d1b46..79a1f1e79221 100644
--- a/drivers/mtd/nand/spi/Makefile
+++ b/drivers/mtd/nand/spi/Makefile
@@ -1,2 +1,2 @@
-spinand-objs := core.o
+spinand-objs := core.o micron.o
 obj-$(CONFIG_MTD_SPI_NAND) += spinand.o
diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
index a99ea352eb85..2194a7f9b550 100644
--- a/drivers/mtd/nand/spi/core.c
+++ b/drivers/mtd/nand/spi/core.c
@@ -751,8 +751,25 @@ static const struct nand_ops spinand_ops = {
 	.isbad = spinand_isbad,
 };
 
+static const struct spinand_manufacturer *spinand_manufacturers[] = {
+	&micron_spinand_manufacturer,
+};
+
 static int spinand_manufacturer_detect(struct spinand_device *spinand)
 {
+	unsigned int i;
+	int ret;
+
+	for (i = 0; i < ARRAY_SIZE(spinand_manufacturers); i++) {
+		ret = spinand_manufacturers[i]->ops->detect(spinand);
+		if (ret > 0) {
+			spinand->manufacturer = spinand_manufacturers[i];
+			return 0;
+		} else if (ret < 0) {
+			return ret;
+		}
+	}
+
 	return -ENOTSUPP;
 }
 
diff --git a/drivers/mtd/nand/spi/micron.c b/drivers/mtd/nand/spi/micron.c
new file mode 100644
index 000000000000..9c4381d6847b
--- /dev/null
+++ b/drivers/mtd/nand/spi/micron.c
@@ -0,0 +1,133 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2016-2017 Micron Technology, Inc.
+ *
+ * Authors:
+ *	Peter Pan <peterpandong@micron.com>
+ */
+
+#include <linux/device.h>
+#include <linux/kernel.h>
+#include <linux/mtd/spinand.h>
+
+#define SPINAND_MFR_MICRON		0x2c
+
+#define MICRON_STATUS_ECC_MASK		GENMASK(7, 4)
+#define MICRON_STATUS_ECC_NO_BITFLIPS	(0 << 4)
+#define MICRON_STATUS_ECC_1TO3_BITFLIPS	(1 << 4)
+#define MICRON_STATUS_ECC_4TO6_BITFLIPS	(3 << 4)
+#define MICRON_STATUS_ECC_7TO8_BITFLIPS	(5 << 4)
+
+static SPINAND_OP_VARIANTS(read_cache_variants,
+		SPINAND_PAGE_READ_FROM_CACHE_QUADIO_OP(0, 2, NULL, 0),
+		SPINAND_PAGE_READ_FROM_CACHE_X4_OP(0, 1, NULL, 0),
+		SPINAND_PAGE_READ_FROM_CACHE_DUALIO_OP(0, 1, NULL, 0),
+		SPINAND_PAGE_READ_FROM_CACHE_X2_OP(0, 1, NULL, 0),
+		SPINAND_PAGE_READ_FROM_CACHE_OP(true, 0, 1, NULL, 0),
+		SPINAND_PAGE_READ_FROM_CACHE_OP(false, 0, 1, NULL, 0));
+
+static SPINAND_OP_VARIANTS(write_cache_variants,
+		SPINAND_PROG_LOAD_X4(true, 0, NULL, 0),
+		SPINAND_PROG_LOAD(true, 0, NULL, 0));
+
+static SPINAND_OP_VARIANTS(update_cache_variants,
+		SPINAND_PROG_LOAD_X4(false, 0, NULL, 0),
+		SPINAND_PROG_LOAD(false, 0, NULL, 0));
+
+static int mt29f2g01abagd_ooblayout_ecc(struct mtd_info *mtd, int section,
+					struct mtd_oob_region *region)
+{
+	if (section)
+		return -ERANGE;
+
+	region->offset = 64;
+	region->length = 64;
+
+	return 0;
+}
+
+static int mt29f2g01abagd_ooblayout_free(struct mtd_info *mtd, int section,
+					 struct mtd_oob_region *region)
+{
+	if (section)
+		return -ERANGE;
+
+	/* Reserve 2 bytes for the BBM. */
+	region->offset = 2;
+	region->length = 62;
+
+	return 0;
+}
+
+static const struct mtd_ooblayout_ops mt29f2g01abagd_ooblayout = {
+	.ecc = mt29f2g01abagd_ooblayout_ecc,
+	.free = mt29f2g01abagd_ooblayout_free,
+};
+
+static int mt29f2g01abagd_ecc_get_status(struct spinand_device *spinand,
+					 u8 status)
+{
+	switch (status & MICRON_STATUS_ECC_MASK) {
+	case STATUS_ECC_NO_BITFLIPS:
+		return 0;
+
+	case STATUS_ECC_UNCOR_ERROR:
+		return -EBADMSG;
+
+	case MICRON_STATUS_ECC_1TO3_BITFLIPS:
+		return 3;
+
+	case MICRON_STATUS_ECC_4TO6_BITFLIPS:
+		return 6;
+
+	case MICRON_STATUS_ECC_7TO8_BITFLIPS:
+		return 8;
+
+	default:
+		break;
+	}
+
+	return -EINVAL;
+}
+
+static const struct spinand_info micron_spinand_table[] = {
+	SPINAND_INFO("MT29F2G01ABAGD", 0x24,
+		     NAND_MEMORG(1, 2048, 128, 64, 2048, 2, 1, 1),
+		     NAND_ECCREQ(8, 512),
+		     SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
+					      &write_cache_variants,
+					      &update_cache_variants),
+		     0,
+		     SPINAND_ECCINFO(&mt29f2g01abagd_ooblayout,
+				     mt29f2g01abagd_ecc_get_status)),
+};
+
+static int micron_spinand_detect(struct spinand_device *spinand)
+{
+	u8 *id = spinand->id.data;
+	int ret;
+
+	/*
+	 * Micron SPI NAND read ID need a dummy byte,
+	 * so the first byte in raw_id is dummy.
+	 */
+	if (id[1] != SPINAND_MFR_MICRON)
+		return 0;
+
+	ret = spinand_match_and_init(spinand, micron_spinand_table,
+				     ARRAY_SIZE(micron_spinand_table), id[2]);
+	if (ret)
+		return ret;
+
+	return 1;
+}
+
+static const struct spinand_manufacturer_ops micron_spinand_manuf_ops = {
+	.detect = micron_spinand_detect,
+};
+
+const struct spinand_manufacturer micron_spinand_manufacturer = {
+	.id = SPINAND_MFR_MICRON,
+	.name = "Micron",
+	.ops = &micron_spinand_manuf_ops,
+};
diff --git a/include/linux/mtd/spinand.h b/include/linux/mtd/spinand.h
index 17a4584c0017..11a18c1993e8 100644
--- a/include/linux/mtd/spinand.h
+++ b/include/linux/mtd/spinand.h
@@ -192,6 +192,9 @@ struct spinand_manufacturer {
 	const struct spinand_manufacturer_ops *ops;
 };
 
+/* SPI NAND manufacturers */
+extern const struct spinand_manufacturer micron_spinand_manufacturer;
+
 /**
  * struct spinand_op_variants - SPI NAND operation variants
  * @ops: the list of variants for a given operation
-- 
2.14.1


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

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

* [PATCH v8 3/4] mtd: spinand: Add initial support for Micron MT29F2G01ABAGD
@ 2018-06-01 13:13   ` Boris Brezillon
  0 siblings, 0 replies; 24+ messages in thread
From: Boris Brezillon @ 2018-06-01 13:13 UTC (permalink / raw)
  To: David Woodhouse, Brian Norris, Boris Brezillon, Marek Vasut,
	Richard Weinberger, linux-mtd, Miquel Raynal, Mark Brown,
	linux-spi, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell,
	Kumar Gala, devicetree
  Cc: Peter Pan

From: Peter Pan <peterpandong@micron.com>

Add a basic driver for Micron SPI NANDs. Only one device is supported
right now, but the driver will be extended to support more devices
afterwards.

Signed-off-by: Peter Pan <peterpandong@micron.com>
Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
---
Changes in v8:
- Changed the subject prefix
---
 drivers/mtd/nand/spi/Makefile |   2 +-
 drivers/mtd/nand/spi/core.c   |  17 ++++++
 drivers/mtd/nand/spi/micron.c | 133 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/mtd/spinand.h   |   3 +
 4 files changed, 154 insertions(+), 1 deletion(-)
 create mode 100644 drivers/mtd/nand/spi/micron.c

diff --git a/drivers/mtd/nand/spi/Makefile b/drivers/mtd/nand/spi/Makefile
index feb79a1d1b46..79a1f1e79221 100644
--- a/drivers/mtd/nand/spi/Makefile
+++ b/drivers/mtd/nand/spi/Makefile
@@ -1,2 +1,2 @@
-spinand-objs := core.o
+spinand-objs := core.o micron.o
 obj-$(CONFIG_MTD_SPI_NAND) += spinand.o
diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
index a99ea352eb85..2194a7f9b550 100644
--- a/drivers/mtd/nand/spi/core.c
+++ b/drivers/mtd/nand/spi/core.c
@@ -751,8 +751,25 @@ static const struct nand_ops spinand_ops = {
 	.isbad = spinand_isbad,
 };
 
+static const struct spinand_manufacturer *spinand_manufacturers[] = {
+	&micron_spinand_manufacturer,
+};
+
 static int spinand_manufacturer_detect(struct spinand_device *spinand)
 {
+	unsigned int i;
+	int ret;
+
+	for (i = 0; i < ARRAY_SIZE(spinand_manufacturers); i++) {
+		ret = spinand_manufacturers[i]->ops->detect(spinand);
+		if (ret > 0) {
+			spinand->manufacturer = spinand_manufacturers[i];
+			return 0;
+		} else if (ret < 0) {
+			return ret;
+		}
+	}
+
 	return -ENOTSUPP;
 }
 
diff --git a/drivers/mtd/nand/spi/micron.c b/drivers/mtd/nand/spi/micron.c
new file mode 100644
index 000000000000..9c4381d6847b
--- /dev/null
+++ b/drivers/mtd/nand/spi/micron.c
@@ -0,0 +1,133 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2016-2017 Micron Technology, Inc.
+ *
+ * Authors:
+ *	Peter Pan <peterpandong@micron.com>
+ */
+
+#include <linux/device.h>
+#include <linux/kernel.h>
+#include <linux/mtd/spinand.h>
+
+#define SPINAND_MFR_MICRON		0x2c
+
+#define MICRON_STATUS_ECC_MASK		GENMASK(7, 4)
+#define MICRON_STATUS_ECC_NO_BITFLIPS	(0 << 4)
+#define MICRON_STATUS_ECC_1TO3_BITFLIPS	(1 << 4)
+#define MICRON_STATUS_ECC_4TO6_BITFLIPS	(3 << 4)
+#define MICRON_STATUS_ECC_7TO8_BITFLIPS	(5 << 4)
+
+static SPINAND_OP_VARIANTS(read_cache_variants,
+		SPINAND_PAGE_READ_FROM_CACHE_QUADIO_OP(0, 2, NULL, 0),
+		SPINAND_PAGE_READ_FROM_CACHE_X4_OP(0, 1, NULL, 0),
+		SPINAND_PAGE_READ_FROM_CACHE_DUALIO_OP(0, 1, NULL, 0),
+		SPINAND_PAGE_READ_FROM_CACHE_X2_OP(0, 1, NULL, 0),
+		SPINAND_PAGE_READ_FROM_CACHE_OP(true, 0, 1, NULL, 0),
+		SPINAND_PAGE_READ_FROM_CACHE_OP(false, 0, 1, NULL, 0));
+
+static SPINAND_OP_VARIANTS(write_cache_variants,
+		SPINAND_PROG_LOAD_X4(true, 0, NULL, 0),
+		SPINAND_PROG_LOAD(true, 0, NULL, 0));
+
+static SPINAND_OP_VARIANTS(update_cache_variants,
+		SPINAND_PROG_LOAD_X4(false, 0, NULL, 0),
+		SPINAND_PROG_LOAD(false, 0, NULL, 0));
+
+static int mt29f2g01abagd_ooblayout_ecc(struct mtd_info *mtd, int section,
+					struct mtd_oob_region *region)
+{
+	if (section)
+		return -ERANGE;
+
+	region->offset = 64;
+	region->length = 64;
+
+	return 0;
+}
+
+static int mt29f2g01abagd_ooblayout_free(struct mtd_info *mtd, int section,
+					 struct mtd_oob_region *region)
+{
+	if (section)
+		return -ERANGE;
+
+	/* Reserve 2 bytes for the BBM. */
+	region->offset = 2;
+	region->length = 62;
+
+	return 0;
+}
+
+static const struct mtd_ooblayout_ops mt29f2g01abagd_ooblayout = {
+	.ecc = mt29f2g01abagd_ooblayout_ecc,
+	.free = mt29f2g01abagd_ooblayout_free,
+};
+
+static int mt29f2g01abagd_ecc_get_status(struct spinand_device *spinand,
+					 u8 status)
+{
+	switch (status & MICRON_STATUS_ECC_MASK) {
+	case STATUS_ECC_NO_BITFLIPS:
+		return 0;
+
+	case STATUS_ECC_UNCOR_ERROR:
+		return -EBADMSG;
+
+	case MICRON_STATUS_ECC_1TO3_BITFLIPS:
+		return 3;
+
+	case MICRON_STATUS_ECC_4TO6_BITFLIPS:
+		return 6;
+
+	case MICRON_STATUS_ECC_7TO8_BITFLIPS:
+		return 8;
+
+	default:
+		break;
+	}
+
+	return -EINVAL;
+}
+
+static const struct spinand_info micron_spinand_table[] = {
+	SPINAND_INFO("MT29F2G01ABAGD", 0x24,
+		     NAND_MEMORG(1, 2048, 128, 64, 2048, 2, 1, 1),
+		     NAND_ECCREQ(8, 512),
+		     SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
+					      &write_cache_variants,
+					      &update_cache_variants),
+		     0,
+		     SPINAND_ECCINFO(&mt29f2g01abagd_ooblayout,
+				     mt29f2g01abagd_ecc_get_status)),
+};
+
+static int micron_spinand_detect(struct spinand_device *spinand)
+{
+	u8 *id = spinand->id.data;
+	int ret;
+
+	/*
+	 * Micron SPI NAND read ID need a dummy byte,
+	 * so the first byte in raw_id is dummy.
+	 */
+	if (id[1] != SPINAND_MFR_MICRON)
+		return 0;
+
+	ret = spinand_match_and_init(spinand, micron_spinand_table,
+				     ARRAY_SIZE(micron_spinand_table), id[2]);
+	if (ret)
+		return ret;
+
+	return 1;
+}
+
+static const struct spinand_manufacturer_ops micron_spinand_manuf_ops = {
+	.detect = micron_spinand_detect,
+};
+
+const struct spinand_manufacturer micron_spinand_manufacturer = {
+	.id = SPINAND_MFR_MICRON,
+	.name = "Micron",
+	.ops = &micron_spinand_manuf_ops,
+};
diff --git a/include/linux/mtd/spinand.h b/include/linux/mtd/spinand.h
index 17a4584c0017..11a18c1993e8 100644
--- a/include/linux/mtd/spinand.h
+++ b/include/linux/mtd/spinand.h
@@ -192,6 +192,9 @@ struct spinand_manufacturer {
 	const struct spinand_manufacturer_ops *ops;
 };
 
+/* SPI NAND manufacturers */
+extern const struct spinand_manufacturer micron_spinand_manufacturer;
+
 /**
  * struct spinand_op_variants - SPI NAND operation variants
  * @ops: the list of variants for a given operation
-- 
2.14.1

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

* [PATCH v8 4/4] mtd: spinand: Add initial support for Winbond W25M02GV
  2018-06-01 13:13 ` Boris Brezillon
@ 2018-06-01 13:14   ` Boris Brezillon
  -1 siblings, 0 replies; 24+ messages in thread
From: Boris Brezillon @ 2018-06-01 13:14 UTC (permalink / raw)
  To: David Woodhouse, Brian Norris, Boris Brezillon, Marek Vasut,
	Richard Weinberger, linux-mtd, Miquel Raynal, Mark Brown,
	linux-spi, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell,
	Kumar Gala, devicetree
  Cc: Frieder Schrempf

From: Frieder Schrempf <frieder.schrempf@exceet.de>

Add support for the W25M02GV chip.

Signed-off-by: Frieder Schrempf <frieder.schrempf@exceet.de>
Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
---
Changes in v8:
- Add a commit message
- Changed the subject prefix
---
 drivers/mtd/nand/spi/Makefile  |   2 +-
 drivers/mtd/nand/spi/core.c    |   1 +
 drivers/mtd/nand/spi/winbond.c | 141 +++++++++++++++++++++++++++++++++++++++++
 include/linux/mtd/spinand.h    |   1 +
 4 files changed, 144 insertions(+), 1 deletion(-)
 create mode 100644 drivers/mtd/nand/spi/winbond.c

diff --git a/drivers/mtd/nand/spi/Makefile b/drivers/mtd/nand/spi/Makefile
index 79a1f1e79221..983a94f7f205 100644
--- a/drivers/mtd/nand/spi/Makefile
+++ b/drivers/mtd/nand/spi/Makefile
@@ -1,2 +1,2 @@
-spinand-objs := core.o micron.o
+spinand-objs := core.o micron.o winbond.o
 obj-$(CONFIG_MTD_SPI_NAND) += spinand.o
diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
index 2194a7f9b550..83fd039fe4d8 100644
--- a/drivers/mtd/nand/spi/core.c
+++ b/drivers/mtd/nand/spi/core.c
@@ -753,6 +753,7 @@ static const struct nand_ops spinand_ops = {
 
 static const struct spinand_manufacturer *spinand_manufacturers[] = {
 	&micron_spinand_manufacturer,
+	&winbond_spinand_manufacturer,
 };
 
 static int spinand_manufacturer_detect(struct spinand_device *spinand)
diff --git a/drivers/mtd/nand/spi/winbond.c b/drivers/mtd/nand/spi/winbond.c
new file mode 100644
index 000000000000..67baa1b32c00
--- /dev/null
+++ b/drivers/mtd/nand/spi/winbond.c
@@ -0,0 +1,141 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2017 exceet electronics GmbH
+ *
+ * Authors:
+ *	Frieder Schrempf <frieder.schrempf@exceet.de>
+ *	Boris Brezillon <boris.brezillon@bootlin.com>
+ */
+
+#include <linux/device.h>
+#include <linux/kernel.h>
+#include <linux/mtd/spinand.h>
+
+#define SPINAND_MFR_WINBOND		0xEF
+
+#define WINBOND_CFG_BUF_READ		BIT(3)
+
+static SPINAND_OP_VARIANTS(read_cache_variants,
+		SPINAND_PAGE_READ_FROM_CACHE_QUADIO_OP(0, 2, NULL, 0),
+		SPINAND_PAGE_READ_FROM_CACHE_X4_OP(0, 1, NULL, 0),
+		SPINAND_PAGE_READ_FROM_CACHE_DUALIO_OP(0, 1, NULL, 0),
+		SPINAND_PAGE_READ_FROM_CACHE_X2_OP(0, 1, NULL, 0),
+		SPINAND_PAGE_READ_FROM_CACHE_OP(true, 0, 1, NULL, 0),
+		SPINAND_PAGE_READ_FROM_CACHE_OP(false, 0, 1, NULL, 0));
+
+static SPINAND_OP_VARIANTS(write_cache_variants,
+		SPINAND_PROG_LOAD_X4(true, 0, NULL, 0),
+		SPINAND_PROG_LOAD(true, 0, NULL, 0));
+
+static SPINAND_OP_VARIANTS(update_cache_variants,
+		SPINAND_PROG_LOAD_X4(false, 0, NULL, 0),
+		SPINAND_PROG_LOAD(false, 0, NULL, 0));
+
+static int w25m02gv_ooblayout_ecc(struct mtd_info *mtd, int section,
+				  struct mtd_oob_region *region)
+{
+	if (section > 3)
+		return -ERANGE;
+
+	region->offset = (16 * section) + 8;
+	region->length = 8;
+
+	return 0;
+}
+
+static int w25m02gv_ooblayout_free(struct mtd_info *mtd, int section,
+				   struct mtd_oob_region *region)
+{
+	if (section > 3)
+		return -ERANGE;
+
+	region->offset = (16 * section) + 2;
+	region->length = 6;
+
+	return 0;
+}
+
+static const struct mtd_ooblayout_ops w25m02gv_ooblayout = {
+	.ecc = w25m02gv_ooblayout_ecc,
+	.free = w25m02gv_ooblayout_free,
+};
+
+static int w25m02gv_select_target(struct spinand_device *spinand,
+				  unsigned int target)
+{
+	struct spi_mem_op op = SPI_MEM_OP(SPI_MEM_OP_CMD(0xc2, 1),
+					  SPI_MEM_OP_NO_ADDR,
+					  SPI_MEM_OP_NO_DUMMY,
+					  SPI_MEM_OP_DATA_OUT(1,
+							spinand->scratchbuf,
+							1));
+
+	*spinand->scratchbuf = target;
+	return spi_mem_exec_op(spinand->spimem, &op);
+}
+
+static const struct spinand_info winbond_spinand_table[] = {
+	SPINAND_INFO("W25M02GV", 0xAB,
+		     NAND_MEMORG(1, 2048, 64, 64, 1024, 1, 1, 2),
+		     NAND_ECCREQ(1, 512),
+		     SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
+					      &write_cache_variants,
+					      &update_cache_variants),
+		     0,
+		     SPINAND_ECCINFO(&w25m02gv_ooblayout, NULL),
+		     SPINAND_SELECT_TARGET(w25m02gv_select_target)),
+};
+
+/**
+ * winbond_spinand_detect - initialize device related part in spinand_device
+ * struct if it is a Winbond device.
+ * @spinand: SPI NAND device structure
+ */
+static int winbond_spinand_detect(struct spinand_device *spinand)
+{
+	u8 *id = spinand->id.data;
+	int ret;
+
+	/*
+	 * Winbond SPI NAND read ID need a dummy byte,
+	 * so the first byte in raw_id is dummy.
+	 */
+	if (id[1] != SPINAND_MFR_WINBOND)
+		return 0;
+
+	ret = spinand_match_and_init(spinand, winbond_spinand_table,
+				     ARRAY_SIZE(winbond_spinand_table), id[2]);
+	if (ret)
+		return ret;
+
+	return 1;
+}
+
+static int winbond_spinand_init(struct spinand_device *spinand)
+{
+	struct nand_device *nand = spinand_to_nand(spinand);
+	unsigned int i;
+
+	/*
+	 * Make sure all dies are in buffer read mode and not continuous read
+	 * mode.
+	 */
+	for (i = 0; i < nand->memorg.ntargets; i++) {
+		spinand_select_target(spinand, i);
+		spinand_upd_cfg(spinand, WINBOND_CFG_BUF_READ,
+				WINBOND_CFG_BUF_READ);
+	}
+
+	return 0;
+}
+
+static const struct spinand_manufacturer_ops winbond_spinand_manuf_ops = {
+	.detect = winbond_spinand_detect,
+	.init = winbond_spinand_init,
+};
+
+const struct spinand_manufacturer winbond_spinand_manufacturer = {
+	.id = SPINAND_MFR_WINBOND,
+	.name = "Winbond",
+	.ops = &winbond_spinand_manuf_ops,
+};
diff --git a/include/linux/mtd/spinand.h b/include/linux/mtd/spinand.h
index 11a18c1993e8..31d993628a0d 100644
--- a/include/linux/mtd/spinand.h
+++ b/include/linux/mtd/spinand.h
@@ -194,6 +194,7 @@ struct spinand_manufacturer {
 
 /* SPI NAND manufacturers */
 extern const struct spinand_manufacturer micron_spinand_manufacturer;
+extern const struct spinand_manufacturer winbond_spinand_manufacturer;
 
 /**
  * struct spinand_op_variants - SPI NAND operation variants
-- 
2.14.1


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

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

* [PATCH v8 4/4] mtd: spinand: Add initial support for Winbond W25M02GV
@ 2018-06-01 13:14   ` Boris Brezillon
  0 siblings, 0 replies; 24+ messages in thread
From: Boris Brezillon @ 2018-06-01 13:14 UTC (permalink / raw)
  To: David Woodhouse, Brian Norris, Boris Brezillon, Marek Vasut,
	Richard Weinberger, linux-mtd, Miquel Raynal, Mark Brown,
	linux-spi, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell,
	Kumar Gala, devicetree
  Cc: Frieder Schrempf

From: Frieder Schrempf <frieder.schrempf@exceet.de>

Add support for the W25M02GV chip.

Signed-off-by: Frieder Schrempf <frieder.schrempf@exceet.de>
Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
---
Changes in v8:
- Add a commit message
- Changed the subject prefix
---
 drivers/mtd/nand/spi/Makefile  |   2 +-
 drivers/mtd/nand/spi/core.c    |   1 +
 drivers/mtd/nand/spi/winbond.c | 141 +++++++++++++++++++++++++++++++++++++++++
 include/linux/mtd/spinand.h    |   1 +
 4 files changed, 144 insertions(+), 1 deletion(-)
 create mode 100644 drivers/mtd/nand/spi/winbond.c

diff --git a/drivers/mtd/nand/spi/Makefile b/drivers/mtd/nand/spi/Makefile
index 79a1f1e79221..983a94f7f205 100644
--- a/drivers/mtd/nand/spi/Makefile
+++ b/drivers/mtd/nand/spi/Makefile
@@ -1,2 +1,2 @@
-spinand-objs := core.o micron.o
+spinand-objs := core.o micron.o winbond.o
 obj-$(CONFIG_MTD_SPI_NAND) += spinand.o
diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
index 2194a7f9b550..83fd039fe4d8 100644
--- a/drivers/mtd/nand/spi/core.c
+++ b/drivers/mtd/nand/spi/core.c
@@ -753,6 +753,7 @@ static const struct nand_ops spinand_ops = {
 
 static const struct spinand_manufacturer *spinand_manufacturers[] = {
 	&micron_spinand_manufacturer,
+	&winbond_spinand_manufacturer,
 };
 
 static int spinand_manufacturer_detect(struct spinand_device *spinand)
diff --git a/drivers/mtd/nand/spi/winbond.c b/drivers/mtd/nand/spi/winbond.c
new file mode 100644
index 000000000000..67baa1b32c00
--- /dev/null
+++ b/drivers/mtd/nand/spi/winbond.c
@@ -0,0 +1,141 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2017 exceet electronics GmbH
+ *
+ * Authors:
+ *	Frieder Schrempf <frieder.schrempf@exceet.de>
+ *	Boris Brezillon <boris.brezillon@bootlin.com>
+ */
+
+#include <linux/device.h>
+#include <linux/kernel.h>
+#include <linux/mtd/spinand.h>
+
+#define SPINAND_MFR_WINBOND		0xEF
+
+#define WINBOND_CFG_BUF_READ		BIT(3)
+
+static SPINAND_OP_VARIANTS(read_cache_variants,
+		SPINAND_PAGE_READ_FROM_CACHE_QUADIO_OP(0, 2, NULL, 0),
+		SPINAND_PAGE_READ_FROM_CACHE_X4_OP(0, 1, NULL, 0),
+		SPINAND_PAGE_READ_FROM_CACHE_DUALIO_OP(0, 1, NULL, 0),
+		SPINAND_PAGE_READ_FROM_CACHE_X2_OP(0, 1, NULL, 0),
+		SPINAND_PAGE_READ_FROM_CACHE_OP(true, 0, 1, NULL, 0),
+		SPINAND_PAGE_READ_FROM_CACHE_OP(false, 0, 1, NULL, 0));
+
+static SPINAND_OP_VARIANTS(write_cache_variants,
+		SPINAND_PROG_LOAD_X4(true, 0, NULL, 0),
+		SPINAND_PROG_LOAD(true, 0, NULL, 0));
+
+static SPINAND_OP_VARIANTS(update_cache_variants,
+		SPINAND_PROG_LOAD_X4(false, 0, NULL, 0),
+		SPINAND_PROG_LOAD(false, 0, NULL, 0));
+
+static int w25m02gv_ooblayout_ecc(struct mtd_info *mtd, int section,
+				  struct mtd_oob_region *region)
+{
+	if (section > 3)
+		return -ERANGE;
+
+	region->offset = (16 * section) + 8;
+	region->length = 8;
+
+	return 0;
+}
+
+static int w25m02gv_ooblayout_free(struct mtd_info *mtd, int section,
+				   struct mtd_oob_region *region)
+{
+	if (section > 3)
+		return -ERANGE;
+
+	region->offset = (16 * section) + 2;
+	region->length = 6;
+
+	return 0;
+}
+
+static const struct mtd_ooblayout_ops w25m02gv_ooblayout = {
+	.ecc = w25m02gv_ooblayout_ecc,
+	.free = w25m02gv_ooblayout_free,
+};
+
+static int w25m02gv_select_target(struct spinand_device *spinand,
+				  unsigned int target)
+{
+	struct spi_mem_op op = SPI_MEM_OP(SPI_MEM_OP_CMD(0xc2, 1),
+					  SPI_MEM_OP_NO_ADDR,
+					  SPI_MEM_OP_NO_DUMMY,
+					  SPI_MEM_OP_DATA_OUT(1,
+							spinand->scratchbuf,
+							1));
+
+	*spinand->scratchbuf = target;
+	return spi_mem_exec_op(spinand->spimem, &op);
+}
+
+static const struct spinand_info winbond_spinand_table[] = {
+	SPINAND_INFO("W25M02GV", 0xAB,
+		     NAND_MEMORG(1, 2048, 64, 64, 1024, 1, 1, 2),
+		     NAND_ECCREQ(1, 512),
+		     SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
+					      &write_cache_variants,
+					      &update_cache_variants),
+		     0,
+		     SPINAND_ECCINFO(&w25m02gv_ooblayout, NULL),
+		     SPINAND_SELECT_TARGET(w25m02gv_select_target)),
+};
+
+/**
+ * winbond_spinand_detect - initialize device related part in spinand_device
+ * struct if it is a Winbond device.
+ * @spinand: SPI NAND device structure
+ */
+static int winbond_spinand_detect(struct spinand_device *spinand)
+{
+	u8 *id = spinand->id.data;
+	int ret;
+
+	/*
+	 * Winbond SPI NAND read ID need a dummy byte,
+	 * so the first byte in raw_id is dummy.
+	 */
+	if (id[1] != SPINAND_MFR_WINBOND)
+		return 0;
+
+	ret = spinand_match_and_init(spinand, winbond_spinand_table,
+				     ARRAY_SIZE(winbond_spinand_table), id[2]);
+	if (ret)
+		return ret;
+
+	return 1;
+}
+
+static int winbond_spinand_init(struct spinand_device *spinand)
+{
+	struct nand_device *nand = spinand_to_nand(spinand);
+	unsigned int i;
+
+	/*
+	 * Make sure all dies are in buffer read mode and not continuous read
+	 * mode.
+	 */
+	for (i = 0; i < nand->memorg.ntargets; i++) {
+		spinand_select_target(spinand, i);
+		spinand_upd_cfg(spinand, WINBOND_CFG_BUF_READ,
+				WINBOND_CFG_BUF_READ);
+	}
+
+	return 0;
+}
+
+static const struct spinand_manufacturer_ops winbond_spinand_manuf_ops = {
+	.detect = winbond_spinand_detect,
+	.init = winbond_spinand_init,
+};
+
+const struct spinand_manufacturer winbond_spinand_manufacturer = {
+	.id = SPINAND_MFR_WINBOND,
+	.name = "Winbond",
+	.ops = &winbond_spinand_manuf_ops,
+};
diff --git a/include/linux/mtd/spinand.h b/include/linux/mtd/spinand.h
index 11a18c1993e8..31d993628a0d 100644
--- a/include/linux/mtd/spinand.h
+++ b/include/linux/mtd/spinand.h
@@ -194,6 +194,7 @@ struct spinand_manufacturer {
 
 /* SPI NAND manufacturers */
 extern const struct spinand_manufacturer micron_spinand_manufacturer;
+extern const struct spinand_manufacturer winbond_spinand_manufacturer;
 
 /**
  * struct spinand_op_variants - SPI NAND operation variants
-- 
2.14.1

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

* Re: [PATCH v8 2/4] dt-bindings: Add bindings for SPI NAND devices
  2018-06-01 13:13   ` Boris Brezillon
@ 2018-06-01 14:42     ` Geert Uytterhoeven
  -1 siblings, 0 replies; 24+ messages in thread
From: Geert Uytterhoeven @ 2018-06-01 14:42 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: Mark Rutland,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Pawel Moll, Ian Campbell, Richard Weinberger, Kumar Gala,
	Rob Herring, linux-spi, Marek Vasut, Mark Brown, MTD Maling List,
	Miquel Raynal, Brian Norris, David Woodhouse

Hi Boris,

I became interested after reading the cover letter...

On Fri, Jun 1, 2018 at 3:13 PM, Boris Brezillon
<boris.brezillon@bootlin.com> wrote:
> Add bindings for SPI NAND chips.
>
> Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>

Thanks for your patch!

> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/spi-nand.txt
> @@ -0,0 +1,27 @@
> +SPI NAND flash
> +
> +Required properties:
> +- compatible: should be "spi-nand"
> +- reg: should encode the chip-select line used to access the NAND chip
> +
> +Optional properties
> +- spi-max-frequency: maximum frequency of the SPI bus the chip can operate at.
> +                    This should encode board limitations (i.e. max freq can't
> +                    be achieved due to crosstalk on IO lines).
> +                    When unspecified, the driver assumes the chip can run at
> +                    the max frequency defined in the spec (information
> +                    extracted chip detection time).

This is a standard property according to
Documentation/devicetree/bindings/spi/spi-bus.txt. Can't you just refer
to that file, or just omit it, as it applies to all SPI slaves anyway?

> +- spi-tx-bus-width: The bus width (number of data wires) that is used for MOSI.
> +                   Only encodes the board constraints (i.e. when not all IO
> +                   signals are routed on the board). Device constraints are
> +                   extracted when detecting the chip, and controller
> +                   constraints are exposed by the SPI mem controller. If this
> +                   property is missing that means no constraint at the board
> +                   level.
> +- spi-rx-bus-width: The bus width (number of data wires) that is used for MISO.
> +                   Only encodes the board constraints (i.e. when not all IO
> +                   signals are routed on the board). Device constraints are
> +                   extracted when detecting the chip, and controller
> +                   constraints are exposed by the SPI mem controller. If this
> +                   property is missing that means no constraint at the board
> +                   level.

This does not match Documentation/devicetree/bindings/spi/spi-bus.txt,
which says the default is 1.

As these properties are handled by the SPI core in of_spi_parse_dt, why
would you want to deviate?

Commenting to the question in the cover letter: what would be the
purpose of spi-max-bus-width?

Thanks!

Gr{oetje,eeting}s,

                        Geert

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

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

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

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

* Re: [PATCH v8 2/4] dt-bindings: Add bindings for SPI NAND devices
@ 2018-06-01 14:42     ` Geert Uytterhoeven
  0 siblings, 0 replies; 24+ messages in thread
From: Geert Uytterhoeven @ 2018-06-01 14:42 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: David Woodhouse, Brian Norris, Marek Vasut, Richard Weinberger,
	MTD Maling List, Miquel Raynal, Mark Brown, linux-spi,
	Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

Hi Boris,

I became interested after reading the cover letter...

On Fri, Jun 1, 2018 at 3:13 PM, Boris Brezillon
<boris.brezillon@bootlin.com> wrote:
> Add bindings for SPI NAND chips.
>
> Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>

Thanks for your patch!

> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/spi-nand.txt
> @@ -0,0 +1,27 @@
> +SPI NAND flash
> +
> +Required properties:
> +- compatible: should be "spi-nand"
> +- reg: should encode the chip-select line used to access the NAND chip
> +
> +Optional properties
> +- spi-max-frequency: maximum frequency of the SPI bus the chip can operate at.
> +                    This should encode board limitations (i.e. max freq can't
> +                    be achieved due to crosstalk on IO lines).
> +                    When unspecified, the driver assumes the chip can run at
> +                    the max frequency defined in the spec (information
> +                    extracted chip detection time).

This is a standard property according to
Documentation/devicetree/bindings/spi/spi-bus.txt. Can't you just refer
to that file, or just omit it, as it applies to all SPI slaves anyway?

> +- spi-tx-bus-width: The bus width (number of data wires) that is used for MOSI.
> +                   Only encodes the board constraints (i.e. when not all IO
> +                   signals are routed on the board). Device constraints are
> +                   extracted when detecting the chip, and controller
> +                   constraints are exposed by the SPI mem controller. If this
> +                   property is missing that means no constraint at the board
> +                   level.
> +- spi-rx-bus-width: The bus width (number of data wires) that is used for MISO.
> +                   Only encodes the board constraints (i.e. when not all IO
> +                   signals are routed on the board). Device constraints are
> +                   extracted when detecting the chip, and controller
> +                   constraints are exposed by the SPI mem controller. If this
> +                   property is missing that means no constraint at the board
> +                   level.

This does not match Documentation/devicetree/bindings/spi/spi-bus.txt,
which says the default is 1.

As these properties are handled by the SPI core in of_spi_parse_dt, why
would you want to deviate?

Commenting to the question in the cover letter: what would be the
purpose of spi-max-bus-width?

Thanks!

Gr{oetje,eeting}s,

                        Geert

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

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

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

* Re: [PATCH v8 2/4] dt-bindings: Add bindings for SPI NAND devices
  2018-06-01 14:42     ` Geert Uytterhoeven
@ 2018-06-01 17:09       ` Boris Brezillon
  -1 siblings, 0 replies; 24+ messages in thread
From: Boris Brezillon @ 2018-06-01 17:09 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Mark Rutland,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Pawel Moll, Ian Campbell, Richard Weinberger, Kumar Gala,
	Rob Herring, linux-spi, Marek Vasut, Mark Brown, MTD Maling List,
	Miquel Raynal, Brian Norris, David Woodhouse

Hi Geert,

On Fri, 1 Jun 2018 16:42:26 +0200
Geert Uytterhoeven <geert@linux-m68k.org> wrote:

> Hi Boris,
> 
> I became interested after reading the cover letter...
> 
> On Fri, Jun 1, 2018 at 3:13 PM, Boris Brezillon
> <boris.brezillon@bootlin.com> wrote:
> > Add bindings for SPI NAND chips.
> >
> > Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
> 
> Thanks for your patch!

And thanks for reviewing it ;-).

> 
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mtd/spi-nand.txt
> > @@ -0,0 +1,27 @@
> > +SPI NAND flash
> > +
> > +Required properties:
> > +- compatible: should be "spi-nand"
> > +- reg: should encode the chip-select line used to access the NAND chip
> > +
> > +Optional properties
> > +- spi-max-frequency: maximum frequency of the SPI bus the chip can operate at.
> > +                    This should encode board limitations (i.e. max freq can't
> > +                    be achieved due to crosstalk on IO lines).
> > +                    When unspecified, the driver assumes the chip can run at
> > +                    the max frequency defined in the spec (information
> > +                    extracted chip detection time).
> 
> This is a standard property according to
> Documentation/devicetree/bindings/spi/spi-bus.txt. Can't you just refer
> to that file, or just omit it, as it applies to all SPI slaves anyway?

The thing is, the maximum frequency supported by a SPI NAND is directly
encoded in the NAND device ID and can be auto-detected. Why should we
define spi-max-frequency in the DT when we can automatically detect
this information? The only reason one might want to override
spi-max-frequency is when the board design impose such restrictions,
hence the precision I give here. 

> 
> > +- spi-tx-bus-width: The bus width (number of data wires) that is used for MOSI.
> > +                   Only encodes the board constraints (i.e. when not all IO
> > +                   signals are routed on the board). Device constraints are
> > +                   extracted when detecting the chip, and controller
> > +                   constraints are exposed by the SPI mem controller. If this
> > +                   property is missing that means no constraint at the board
> > +                   level.
> > +- spi-rx-bus-width: The bus width (number of data wires) that is used for MISO.
> > +                   Only encodes the board constraints (i.e. when not all IO
> > +                   signals are routed on the board). Device constraints are
> > +                   extracted when detecting the chip, and controller
> > +                   constraints are exposed by the SPI mem controller. If this
> > +                   property is missing that means no constraint at the board
> > +                   level.
> 
> This does not match Documentation/devicetree/bindings/spi/spi-bus.txt,
> which says the default is 1.

Yes, I know.

> 
> As these properties are handled by the SPI core in of_spi_parse_dt, why
> would you want to deviate?

Because, again, this information can be extracted from the NAND ID, and
the only reason we might want to override the information extracted
from the NAND ID is when the board design adds extra restrictions
(like, only 2 SIO lines wired on the 4 available).

> 
> Commenting to the question in the cover letter: what would be the
> purpose of spi-max-bus-width?

Defining how many IO lines are wired on the board design. If this
property is missing we would assume all IO lines are wired and the
restrictions would be negotiated between the controller and the device
without requiring explicit description in the DT.

Regards,

Boris 

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

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

* Re: [PATCH v8 2/4] dt-bindings: Add bindings for SPI NAND devices
@ 2018-06-01 17:09       ` Boris Brezillon
  0 siblings, 0 replies; 24+ messages in thread
From: Boris Brezillon @ 2018-06-01 17:09 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: David Woodhouse, Brian Norris, Marek Vasut, Richard Weinberger,
	MTD Maling List, Miquel Raynal, Mark Brown, linux-spi,
	Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

Hi Geert,

On Fri, 1 Jun 2018 16:42:26 +0200
Geert Uytterhoeven <geert@linux-m68k.org> wrote:

> Hi Boris,
> 
> I became interested after reading the cover letter...
> 
> On Fri, Jun 1, 2018 at 3:13 PM, Boris Brezillon
> <boris.brezillon@bootlin.com> wrote:
> > Add bindings for SPI NAND chips.
> >
> > Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
> 
> Thanks for your patch!

And thanks for reviewing it ;-).

> 
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mtd/spi-nand.txt
> > @@ -0,0 +1,27 @@
> > +SPI NAND flash
> > +
> > +Required properties:
> > +- compatible: should be "spi-nand"
> > +- reg: should encode the chip-select line used to access the NAND chip
> > +
> > +Optional properties
> > +- spi-max-frequency: maximum frequency of the SPI bus the chip can operate at.
> > +                    This should encode board limitations (i.e. max freq can't
> > +                    be achieved due to crosstalk on IO lines).
> > +                    When unspecified, the driver assumes the chip can run at
> > +                    the max frequency defined in the spec (information
> > +                    extracted chip detection time).
> 
> This is a standard property according to
> Documentation/devicetree/bindings/spi/spi-bus.txt. Can't you just refer
> to that file, or just omit it, as it applies to all SPI slaves anyway?

The thing is, the maximum frequency supported by a SPI NAND is directly
encoded in the NAND device ID and can be auto-detected. Why should we
define spi-max-frequency in the DT when we can automatically detect
this information? The only reason one might want to override
spi-max-frequency is when the board design impose such restrictions,
hence the precision I give here. 

> 
> > +- spi-tx-bus-width: The bus width (number of data wires) that is used for MOSI.
> > +                   Only encodes the board constraints (i.e. when not all IO
> > +                   signals are routed on the board). Device constraints are
> > +                   extracted when detecting the chip, and controller
> > +                   constraints are exposed by the SPI mem controller. If this
> > +                   property is missing that means no constraint at the board
> > +                   level.
> > +- spi-rx-bus-width: The bus width (number of data wires) that is used for MISO.
> > +                   Only encodes the board constraints (i.e. when not all IO
> > +                   signals are routed on the board). Device constraints are
> > +                   extracted when detecting the chip, and controller
> > +                   constraints are exposed by the SPI mem controller. If this
> > +                   property is missing that means no constraint at the board
> > +                   level.
> 
> This does not match Documentation/devicetree/bindings/spi/spi-bus.txt,
> which says the default is 1.

Yes, I know.

> 
> As these properties are handled by the SPI core in of_spi_parse_dt, why
> would you want to deviate?

Because, again, this information can be extracted from the NAND ID, and
the only reason we might want to override the information extracted
from the NAND ID is when the board design adds extra restrictions
(like, only 2 SIO lines wired on the 4 available).

> 
> Commenting to the question in the cover letter: what would be the
> purpose of spi-max-bus-width?

Defining how many IO lines are wired on the board design. If this
property is missing we would assume all IO lines are wired and the
restrictions would be negotiated between the controller and the device
without requiring explicit description in the DT.

Regards,

Boris 

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

* Re: [PATCH v8 2/4] dt-bindings: Add bindings for SPI NAND devices
  2018-06-01 17:09       ` Boris Brezillon
@ 2018-06-01 17:40         ` Geert Uytterhoeven
  -1 siblings, 0 replies; 24+ messages in thread
From: Geert Uytterhoeven @ 2018-06-01 17:40 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: Mark Rutland,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Pawel Moll, Ian Campbell, Richard Weinberger, Kumar Gala,
	Rob Herring, linux-spi, Marek Vasut, Mark Brown, MTD Maling List,
	Miquel Raynal, Brian Norris, David Woodhouse

Hi Boris,

On Fri, Jun 1, 2018 at 7:09 PM, Boris Brezillon
<boris.brezillon@bootlin.com> wrote:
> On Fri, 1 Jun 2018 16:42:26 +0200
> Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>> On Fri, Jun 1, 2018 at 3:13 PM, Boris Brezillon
>> <boris.brezillon@bootlin.com> wrote:
>> > Add bindings for SPI NAND chips.

>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings/mtd/spi-nand.txt
>> > @@ -0,0 +1,27 @@
>> > +SPI NAND flash
>> > +
>> > +Required properties:
>> > +- compatible: should be "spi-nand"
>> > +- reg: should encode the chip-select line used to access the NAND chip
>> > +
>> > +Optional properties
>> > +- spi-max-frequency: maximum frequency of the SPI bus the chip can operate at.
>> > +                    This should encode board limitations (i.e. max freq can't
>> > +                    be achieved due to crosstalk on IO lines).
>> > +                    When unspecified, the driver assumes the chip can run at
>> > +                    the max frequency defined in the spec (information
>> > +                    extracted chip detection time).
>>
>> This is a standard property according to
>> Documentation/devicetree/bindings/spi/spi-bus.txt. Can't you just refer
>> to that file, or just omit it, as it applies to all SPI slaves anyway?
>
> The thing is, the maximum frequency supported by a SPI NAND is directly
> encoded in the NAND device ID and can be auto-detected. Why should we
> define spi-max-frequency in the DT when we can automatically detect
> this information? The only reason one might want to override

Because that's how we dealt with this traditionally. Or at least I thought so.
But include/linux/spi/spi.h says:

 * @max_speed_hz: Maximum clock rate to be used with this chip
 *      (on this board); may be changed by the device's driver.
 *      The spi_transfer.speed_hz can override this for each transfer.

and many drivers seem to do so.

> spi-max-frequency is when the board design impose such restrictions,
> hence the precision I give here.

Which is exactly the meaning of the standard property, isn't it?

So I think it can just be omitted here anyway.  If it's present, the SPI
core will make sure it's taken into account.

>> > +- spi-tx-bus-width: The bus width (number of data wires) that is used for MOSI.
>> > +                   Only encodes the board constraints (i.e. when not all IO
>> > +                   signals are routed on the board). Device constraints are
>> > +                   extracted when detecting the chip, and controller
>> > +                   constraints are exposed by the SPI mem controller. If this
>> > +                   property is missing that means no constraint at the board
>> > +                   level.
>> > +- spi-rx-bus-width: The bus width (number of data wires) that is used for MISO.
>> > +                   Only encodes the board constraints (i.e. when not all IO
>> > +                   signals are routed on the board). Device constraints are
>> > +                   extracted when detecting the chip, and controller
>> > +                   constraints are exposed by the SPI mem controller. If this
>> > +                   property is missing that means no constraint at the board
>> > +                   level.
>>
>> This does not match Documentation/devicetree/bindings/spi/spi-bus.txt,
>> which says the default is 1.
>
> Yes, I know.
>
>> As these properties are handled by the SPI core in of_spi_parse_dt, why
>> would you want to deviate?
>
> Because, again, this information can be extracted from the NAND ID, and
> the only reason we might want to override the information extracted
> from the NAND ID is when the board design adds extra restrictions
> (like, only 2 SIO lines wired on the 4 available).

In struct spi_device, this is specified using the SPI_[RT]_X_{DUAL,QUAD}
bits in the mode field. Documentation says:

 * @mode: The spi mode defines how data is clocked out and in.
 *      This may be changed by the device's driver.
 *      The "active low" default for chipselect mode can be overridden
 *      (by specifying SPI_CS_HIGH) as can the "MSB first" default for
 *      each word in a transfer (by specifying SPI_LSB_FIRST).

So this may also be specified by the device driver, but it seems no driver
does so for the dual/quad bits, except for drivers/mtd/spi-nor/nxp-spifi.c
(for rx only).

But here we do have the generic SPI DT bindings saying the default is 1.
So personally, I wouldn't go for the option of the least surprise, and
don't deviate from the generic bindings.

>> Commenting to the question in the cover letter: what would be the
>> purpose of spi-max-bus-width?
>
> Defining how many IO lines are wired on the board design. If this
> property is missing we would assume all IO lines are wired and the
> restrictions would be negotiated between the controller and the device
> without requiring explicit description in the DT.

Which is exactly the meaning of the standard property, except for the
default of all vs. 1.

I'll have to defer to Mark (broonie) for his opinion about deviating from
the way this is handled traditionally, and assuming different defaults...

Gr{oetje,eeting}s,

                        Geert

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

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

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

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

* Re: [PATCH v8 2/4] dt-bindings: Add bindings for SPI NAND devices
@ 2018-06-01 17:40         ` Geert Uytterhoeven
  0 siblings, 0 replies; 24+ messages in thread
From: Geert Uytterhoeven @ 2018-06-01 17:40 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: David Woodhouse, Brian Norris, Marek Vasut, Richard Weinberger,
	MTD Maling List, Miquel Raynal, Mark Brown, linux-spi,
	Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

Hi Boris,

On Fri, Jun 1, 2018 at 7:09 PM, Boris Brezillon
<boris.brezillon@bootlin.com> wrote:
> On Fri, 1 Jun 2018 16:42:26 +0200
> Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>> On Fri, Jun 1, 2018 at 3:13 PM, Boris Brezillon
>> <boris.brezillon@bootlin.com> wrote:
>> > Add bindings for SPI NAND chips.

>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings/mtd/spi-nand.txt
>> > @@ -0,0 +1,27 @@
>> > +SPI NAND flash
>> > +
>> > +Required properties:
>> > +- compatible: should be "spi-nand"
>> > +- reg: should encode the chip-select line used to access the NAND chip
>> > +
>> > +Optional properties
>> > +- spi-max-frequency: maximum frequency of the SPI bus the chip can operate at.
>> > +                    This should encode board limitations (i.e. max freq can't
>> > +                    be achieved due to crosstalk on IO lines).
>> > +                    When unspecified, the driver assumes the chip can run at
>> > +                    the max frequency defined in the spec (information
>> > +                    extracted chip detection time).
>>
>> This is a standard property according to
>> Documentation/devicetree/bindings/spi/spi-bus.txt. Can't you just refer
>> to that file, or just omit it, as it applies to all SPI slaves anyway?
>
> The thing is, the maximum frequency supported by a SPI NAND is directly
> encoded in the NAND device ID and can be auto-detected. Why should we
> define spi-max-frequency in the DT when we can automatically detect
> this information? The only reason one might want to override

Because that's how we dealt with this traditionally. Or at least I thought so.
But include/linux/spi/spi.h says:

 * @max_speed_hz: Maximum clock rate to be used with this chip
 *      (on this board); may be changed by the device's driver.
 *      The spi_transfer.speed_hz can override this for each transfer.

and many drivers seem to do so.

> spi-max-frequency is when the board design impose such restrictions,
> hence the precision I give here.

Which is exactly the meaning of the standard property, isn't it?

So I think it can just be omitted here anyway.  If it's present, the SPI
core will make sure it's taken into account.

>> > +- spi-tx-bus-width: The bus width (number of data wires) that is used for MOSI.
>> > +                   Only encodes the board constraints (i.e. when not all IO
>> > +                   signals are routed on the board). Device constraints are
>> > +                   extracted when detecting the chip, and controller
>> > +                   constraints are exposed by the SPI mem controller. If this
>> > +                   property is missing that means no constraint at the board
>> > +                   level.
>> > +- spi-rx-bus-width: The bus width (number of data wires) that is used for MISO.
>> > +                   Only encodes the board constraints (i.e. when not all IO
>> > +                   signals are routed on the board). Device constraints are
>> > +                   extracted when detecting the chip, and controller
>> > +                   constraints are exposed by the SPI mem controller. If this
>> > +                   property is missing that means no constraint at the board
>> > +                   level.
>>
>> This does not match Documentation/devicetree/bindings/spi/spi-bus.txt,
>> which says the default is 1.
>
> Yes, I know.
>
>> As these properties are handled by the SPI core in of_spi_parse_dt, why
>> would you want to deviate?
>
> Because, again, this information can be extracted from the NAND ID, and
> the only reason we might want to override the information extracted
> from the NAND ID is when the board design adds extra restrictions
> (like, only 2 SIO lines wired on the 4 available).

In struct spi_device, this is specified using the SPI_[RT]_X_{DUAL,QUAD}
bits in the mode field. Documentation says:

 * @mode: The spi mode defines how data is clocked out and in.
 *      This may be changed by the device's driver.
 *      The "active low" default for chipselect mode can be overridden
 *      (by specifying SPI_CS_HIGH) as can the "MSB first" default for
 *      each word in a transfer (by specifying SPI_LSB_FIRST).

So this may also be specified by the device driver, but it seems no driver
does so for the dual/quad bits, except for drivers/mtd/spi-nor/nxp-spifi.c
(for rx only).

But here we do have the generic SPI DT bindings saying the default is 1.
So personally, I wouldn't go for the option of the least surprise, and
don't deviate from the generic bindings.

>> Commenting to the question in the cover letter: what would be the
>> purpose of spi-max-bus-width?
>
> Defining how many IO lines are wired on the board design. If this
> property is missing we would assume all IO lines are wired and the
> restrictions would be negotiated between the controller and the device
> without requiring explicit description in the DT.

Which is exactly the meaning of the standard property, except for the
default of all vs. 1.

I'll have to defer to Mark (broonie) for his opinion about deviating from
the way this is handled traditionally, and assuming different defaults...

Gr{oetje,eeting}s,

                        Geert

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

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

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

* Re: [PATCH v8 2/4] dt-bindings: Add bindings for SPI NAND devices
  2018-06-01 17:09       ` Boris Brezillon
@ 2018-06-01 19:18         ` Rob Herring
  -1 siblings, 0 replies; 24+ messages in thread
From: Rob Herring @ 2018-06-01 19:18 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: Mark Rutland,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Pawel Moll, Ian Campbell, Richard Weinberger, Kumar Gala,
	linux-spi, Marek Vasut, Mark Brown, Geert Uytterhoeven,
	Miquel Raynal, MTD Maling List, Brian Norris, David Woodhouse

On Fri, Jun 1, 2018 at 12:09 PM, Boris Brezillon
<boris.brezillon@bootlin.com> wrote:
> Hi Geert,
>
> On Fri, 1 Jun 2018 16:42:26 +0200
> Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
>> Hi Boris,
>>
>> I became interested after reading the cover letter...
>>
>> On Fri, Jun 1, 2018 at 3:13 PM, Boris Brezillon
>> <boris.brezillon@bootlin.com> wrote:
>> > Add bindings for SPI NAND chips.
>> >
>> > Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
>>
>> Thanks for your patch!
>
> And thanks for reviewing it ;-).
>
>>
>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings/mtd/spi-nand.txt
>> > @@ -0,0 +1,27 @@
>> > +SPI NAND flash
>> > +
>> > +Required properties:
>> > +- compatible: should be "spi-nand"

Seems awfully generic if you expect this alone. No chips with quirks
yet? Is there a standard for detection like jedec?

>> > +- reg: should encode the chip-select line used to access the NAND chip

"see spi.txt" should be enough.

>> > +
>> > +Optional properties
>> > +- spi-max-frequency: maximum frequency of the SPI bus the chip can operate at.
>> > +                    This should encode board limitations (i.e. max freq can't
>> > +                    be achieved due to crosstalk on IO lines).
>> > +                    When unspecified, the driver assumes the chip can run at
>> > +                    the max frequency defined in the spec (information
>> > +                    extracted chip detection time).
>>
>> This is a standard property according to
>> Documentation/devicetree/bindings/spi/spi-bus.txt. Can't you just refer
>> to that file, or just omit it, as it applies to all SPI slaves anyway?
>
> The thing is, the maximum frequency supported by a SPI NAND is directly
> encoded in the NAND device ID and can be auto-detected. Why should we
> define spi-max-frequency in the DT when we can automatically detect
> this information? The only reason one might want to override
> spi-max-frequency is when the board design impose such restrictions,
> hence the precision I give here.

This should always be the case. The operating frequency should be
min(host max, device max) unless the board has restrictions and needs
to set spi-max-frequency (and we really should have used
'bus-frequency' here). No doubt though, that is not what has been
done.

>> > +- spi-tx-bus-width: The bus width (number of data wires) that is used for MOSI.
>> > +                   Only encodes the board constraints (i.e. when not all IO
>> > +                   signals are routed on the board). Device constraints are
>> > +                   extracted when detecting the chip, and controller
>> > +                   constraints are exposed by the SPI mem controller. If this
>> > +                   property is missing that means no constraint at the board
>> > +                   level.
>> > +- spi-rx-bus-width: The bus width (number of data wires) that is used for MISO.
>> > +                   Only encodes the board constraints (i.e. when not all IO
>> > +                   signals are routed on the board). Device constraints are
>> > +                   extracted when detecting the chip, and controller
>> > +                   constraints are exposed by the SPI mem controller. If this
>> > +                   property is missing that means no constraint at the board
>> > +                   level.
>>
>> This does not match Documentation/devicetree/bindings/spi/spi-bus.txt,
>> which says the default is 1.
>
> Yes, I know.

Like frequency, this should have been similar. I imagine for the
common case, the number of host and device lines are 1 so it doesn't
really apply as this was added later on. Perhaps we can reword the
common definition to work for both?

Rob

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

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

* Re: [PATCH v8 2/4] dt-bindings: Add bindings for SPI NAND devices
@ 2018-06-01 19:18         ` Rob Herring
  0 siblings, 0 replies; 24+ messages in thread
From: Rob Herring @ 2018-06-01 19:18 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: Geert Uytterhoeven, David Woodhouse, Brian Norris, Marek Vasut,
	Richard Weinberger, MTD Maling List, Miquel Raynal, Mark Brown,
	linux-spi, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

On Fri, Jun 1, 2018 at 12:09 PM, Boris Brezillon
<boris.brezillon@bootlin.com> wrote:
> Hi Geert,
>
> On Fri, 1 Jun 2018 16:42:26 +0200
> Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
>> Hi Boris,
>>
>> I became interested after reading the cover letter...
>>
>> On Fri, Jun 1, 2018 at 3:13 PM, Boris Brezillon
>> <boris.brezillon@bootlin.com> wrote:
>> > Add bindings for SPI NAND chips.
>> >
>> > Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
>>
>> Thanks for your patch!
>
> And thanks for reviewing it ;-).
>
>>
>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings/mtd/spi-nand.txt
>> > @@ -0,0 +1,27 @@
>> > +SPI NAND flash
>> > +
>> > +Required properties:
>> > +- compatible: should be "spi-nand"

Seems awfully generic if you expect this alone. No chips with quirks
yet? Is there a standard for detection like jedec?

>> > +- reg: should encode the chip-select line used to access the NAND chip

"see spi.txt" should be enough.

>> > +
>> > +Optional properties
>> > +- spi-max-frequency: maximum frequency of the SPI bus the chip can operate at.
>> > +                    This should encode board limitations (i.e. max freq can't
>> > +                    be achieved due to crosstalk on IO lines).
>> > +                    When unspecified, the driver assumes the chip can run at
>> > +                    the max frequency defined in the spec (information
>> > +                    extracted chip detection time).
>>
>> This is a standard property according to
>> Documentation/devicetree/bindings/spi/spi-bus.txt. Can't you just refer
>> to that file, or just omit it, as it applies to all SPI slaves anyway?
>
> The thing is, the maximum frequency supported by a SPI NAND is directly
> encoded in the NAND device ID and can be auto-detected. Why should we
> define spi-max-frequency in the DT when we can automatically detect
> this information? The only reason one might want to override
> spi-max-frequency is when the board design impose such restrictions,
> hence the precision I give here.

This should always be the case. The operating frequency should be
min(host max, device max) unless the board has restrictions and needs
to set spi-max-frequency (and we really should have used
'bus-frequency' here). No doubt though, that is not what has been
done.

>> > +- spi-tx-bus-width: The bus width (number of data wires) that is used for MOSI.
>> > +                   Only encodes the board constraints (i.e. when not all IO
>> > +                   signals are routed on the board). Device constraints are
>> > +                   extracted when detecting the chip, and controller
>> > +                   constraints are exposed by the SPI mem controller. If this
>> > +                   property is missing that means no constraint at the board
>> > +                   level.
>> > +- spi-rx-bus-width: The bus width (number of data wires) that is used for MISO.
>> > +                   Only encodes the board constraints (i.e. when not all IO
>> > +                   signals are routed on the board). Device constraints are
>> > +                   extracted when detecting the chip, and controller
>> > +                   constraints are exposed by the SPI mem controller. If this
>> > +                   property is missing that means no constraint at the board
>> > +                   level.
>>
>> This does not match Documentation/devicetree/bindings/spi/spi-bus.txt,
>> which says the default is 1.
>
> Yes, I know.

Like frequency, this should have been similar. I imagine for the
common case, the number of host and device lines are 1 so it doesn't
really apply as this was added later on. Perhaps we can reword the
common definition to work for both?

Rob

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

* Re: [PATCH v8 2/4] dt-bindings: Add bindings for SPI NAND devices
  2018-06-01 19:18         ` Rob Herring
@ 2018-06-01 20:32           ` Boris Brezillon
  -1 siblings, 0 replies; 24+ messages in thread
From: Boris Brezillon @ 2018-06-01 20:32 UTC (permalink / raw)
  To: Rob Herring
  Cc: Mark Rutland,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Pawel Moll, Ian Campbell, Richard Weinberger, Kumar Gala,
	linux-spi, Marek Vasut, Mark Brown, Geert Uytterhoeven,
	Miquel Raynal, MTD Maling List, Brian Norris, David Woodhouse

Hi Rob,

On Fri, 1 Jun 2018 14:18:35 -0500
Rob Herring <robh+dt@kernel.org> wrote:

> On Fri, Jun 1, 2018 at 12:09 PM, Boris Brezillon
> <boris.brezillon@bootlin.com> wrote:
> > Hi Geert,
> >
> > On Fri, 1 Jun 2018 16:42:26 +0200
> > Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> >
> >> Hi Boris,
> >>
> >> I became interested after reading the cover letter...
> >>
> >> On Fri, Jun 1, 2018 at 3:13 PM, Boris Brezillon
> >> <boris.brezillon@bootlin.com> wrote:
> >> > Add bindings for SPI NAND chips.
> >> >
> >> > Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
> >>
> >> Thanks for your patch!
> >
> > And thanks for reviewing it ;-).
> >
> >>
> >> > --- /dev/null
> >> > +++ b/Documentation/devicetree/bindings/mtd/spi-nand.txt
> >> > @@ -0,0 +1,27 @@
> >> > +SPI NAND flash
> >> > +
> >> > +Required properties:
> >> > +- compatible: should be "spi-nand"
> 
> Seems awfully generic if you expect this alone. No chips with quirks
> yet?

Believe it or not, but it seems that NAND vendors agreed on a common
instruction set, made the READID instruction mandatory, and the 2 bytes
returned by this operation seem to be unique and allow us to reliably
extract information about he SPI NAND chip. I'm not saying this will
keep going like that (I'm pretty sure they'll screw up the READID
detection and start re-using IDs for different NANDs at some point),
but so far it seems to work.

> Is there a standard for detection like jedec?

Not that I know of. Some NANDs embed an ONFi table, which is usually
used on parallel/raw NANDs, but this is not mandatory.

> 
> >> > +- reg: should encode the chip-select line used to access the NAND chip
> 
> "see spi.txt" should be enough.

Okay.

> 
> >> > +
> >> > +Optional properties
> >> > +- spi-max-frequency: maximum frequency of the SPI bus the chip can operate at.
> >> > +                    This should encode board limitations (i.e. max freq can't
> >> > +                    be achieved due to crosstalk on IO lines).
> >> > +                    When unspecified, the driver assumes the chip can run at
> >> > +                    the max frequency defined in the spec (information
> >> > +                    extracted chip detection time).
> >>
> >> This is a standard property according to
> >> Documentation/devicetree/bindings/spi/spi-bus.txt. Can't you just refer
> >> to that file, or just omit it, as it applies to all SPI slaves anyway?
> >
> > The thing is, the maximum frequency supported by a SPI NAND is directly
> > encoded in the NAND device ID and can be auto-detected. Why should we
> > define spi-max-frequency in the DT when we can automatically detect
> > this information? The only reason one might want to override
> > spi-max-frequency is when the board design impose such restrictions,
> > hence the precision I give here.
> 
> This should always be the case. The operating frequency should be
> min(host max, device max) unless the board has restrictions and needs
> to set spi-max-frequency (and we really should have used
> 'bus-frequency' here).

That was my feeling too.

> No doubt though, that is not what has been
> done.

Definitely not, actually, I'm not even sure that this sort if
negotiation is supported by the framework. Right now, the SPI NAND
driver does not try to set max device freq, but that's something I was
planning to work on at some point, and since bindings are supposed to
be set in stone I thought I would clarify the meaning of
spi-max-frequency for the SPI NAND use case.

> 
> >> > +- spi-tx-bus-width: The bus width (number of data wires) that is used for MOSI.
> >> > +                   Only encodes the board constraints (i.e. when not all IO
> >> > +                   signals are routed on the board). Device constraints are
> >> > +                   extracted when detecting the chip, and controller
> >> > +                   constraints are exposed by the SPI mem controller. If this
> >> > +                   property is missing that means no constraint at the board
> >> > +                   level.
> >> > +- spi-rx-bus-width: The bus width (number of data wires) that is used for MISO.
> >> > +                   Only encodes the board constraints (i.e. when not all IO
> >> > +                   signals are routed on the board). Device constraints are
> >> > +                   extracted when detecting the chip, and controller
> >> > +                   constraints are exposed by the SPI mem controller. If this
> >> > +                   property is missing that means no constraint at the board
> >> > +                   level.
> >>
> >> This does not match Documentation/devicetree/bindings/spi/spi-bus.txt,
> >> which says the default is 1.
> >
> > Yes, I know.
> 
> Like frequency, this should have been similar. I imagine for the
> common case, the number of host and device lines are 1 so it doesn't
> really apply as this was added later on. Perhaps we can reword the
> common definition to work for both?

Do you have a suggestion? Also, I fear that globally changing the
meaning of these props will make most implementations not compliant
with this new definition.

Regards,

Boris

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

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

* Re: [PATCH v8 2/4] dt-bindings: Add bindings for SPI NAND devices
@ 2018-06-01 20:32           ` Boris Brezillon
  0 siblings, 0 replies; 24+ messages in thread
From: Boris Brezillon @ 2018-06-01 20:32 UTC (permalink / raw)
  To: Rob Herring
  Cc: Geert Uytterhoeven, David Woodhouse, Brian Norris, Marek Vasut,
	Richard Weinberger, MTD Maling List, Miquel Raynal, Mark Brown,
	linux-spi, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

Hi Rob,

On Fri, 1 Jun 2018 14:18:35 -0500
Rob Herring <robh+dt@kernel.org> wrote:

> On Fri, Jun 1, 2018 at 12:09 PM, Boris Brezillon
> <boris.brezillon@bootlin.com> wrote:
> > Hi Geert,
> >
> > On Fri, 1 Jun 2018 16:42:26 +0200
> > Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> >
> >> Hi Boris,
> >>
> >> I became interested after reading the cover letter...
> >>
> >> On Fri, Jun 1, 2018 at 3:13 PM, Boris Brezillon
> >> <boris.brezillon@bootlin.com> wrote:
> >> > Add bindings for SPI NAND chips.
> >> >
> >> > Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
> >>
> >> Thanks for your patch!
> >
> > And thanks for reviewing it ;-).
> >
> >>
> >> > --- /dev/null
> >> > +++ b/Documentation/devicetree/bindings/mtd/spi-nand.txt
> >> > @@ -0,0 +1,27 @@
> >> > +SPI NAND flash
> >> > +
> >> > +Required properties:
> >> > +- compatible: should be "spi-nand"
> 
> Seems awfully generic if you expect this alone. No chips with quirks
> yet?

Believe it or not, but it seems that NAND vendors agreed on a common
instruction set, made the READID instruction mandatory, and the 2 bytes
returned by this operation seem to be unique and allow us to reliably
extract information about he SPI NAND chip. I'm not saying this will
keep going like that (I'm pretty sure they'll screw up the READID
detection and start re-using IDs for different NANDs at some point),
but so far it seems to work.

> Is there a standard for detection like jedec?

Not that I know of. Some NANDs embed an ONFi table, which is usually
used on parallel/raw NANDs, but this is not mandatory.

> 
> >> > +- reg: should encode the chip-select line used to access the NAND chip
> 
> "see spi.txt" should be enough.

Okay.

> 
> >> > +
> >> > +Optional properties
> >> > +- spi-max-frequency: maximum frequency of the SPI bus the chip can operate at.
> >> > +                    This should encode board limitations (i.e. max freq can't
> >> > +                    be achieved due to crosstalk on IO lines).
> >> > +                    When unspecified, the driver assumes the chip can run at
> >> > +                    the max frequency defined in the spec (information
> >> > +                    extracted chip detection time).
> >>
> >> This is a standard property according to
> >> Documentation/devicetree/bindings/spi/spi-bus.txt. Can't you just refer
> >> to that file, or just omit it, as it applies to all SPI slaves anyway?
> >
> > The thing is, the maximum frequency supported by a SPI NAND is directly
> > encoded in the NAND device ID and can be auto-detected. Why should we
> > define spi-max-frequency in the DT when we can automatically detect
> > this information? The only reason one might want to override
> > spi-max-frequency is when the board design impose such restrictions,
> > hence the precision I give here.
> 
> This should always be the case. The operating frequency should be
> min(host max, device max) unless the board has restrictions and needs
> to set spi-max-frequency (and we really should have used
> 'bus-frequency' here).

That was my feeling too.

> No doubt though, that is not what has been
> done.

Definitely not, actually, I'm not even sure that this sort if
negotiation is supported by the framework. Right now, the SPI NAND
driver does not try to set max device freq, but that's something I was
planning to work on at some point, and since bindings are supposed to
be set in stone I thought I would clarify the meaning of
spi-max-frequency for the SPI NAND use case.

> 
> >> > +- spi-tx-bus-width: The bus width (number of data wires) that is used for MOSI.
> >> > +                   Only encodes the board constraints (i.e. when not all IO
> >> > +                   signals are routed on the board). Device constraints are
> >> > +                   extracted when detecting the chip, and controller
> >> > +                   constraints are exposed by the SPI mem controller. If this
> >> > +                   property is missing that means no constraint at the board
> >> > +                   level.
> >> > +- spi-rx-bus-width: The bus width (number of data wires) that is used for MISO.
> >> > +                   Only encodes the board constraints (i.e. when not all IO
> >> > +                   signals are routed on the board). Device constraints are
> >> > +                   extracted when detecting the chip, and controller
> >> > +                   constraints are exposed by the SPI mem controller. If this
> >> > +                   property is missing that means no constraint at the board
> >> > +                   level.
> >>
> >> This does not match Documentation/devicetree/bindings/spi/spi-bus.txt,
> >> which says the default is 1.
> >
> > Yes, I know.
> 
> Like frequency, this should have been similar. I imagine for the
> common case, the number of host and device lines are 1 so it doesn't
> really apply as this was added later on. Perhaps we can reword the
> common definition to work for both?

Do you have a suggestion? Also, I fear that globally changing the
meaning of these props will make most implementations not compliant
with this new definition.

Regards,

Boris

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

* Re: [PATCH v8 2/4] dt-bindings: Add bindings for SPI NAND devices
  2018-06-01 17:40         ` Geert Uytterhoeven
@ 2018-06-01 21:07           ` Boris Brezillon
  -1 siblings, 0 replies; 24+ messages in thread
From: Boris Brezillon @ 2018-06-01 21:07 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Mark Rutland,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Pawel Moll, Ian Campbell, Richard Weinberger, Kumar Gala,
	Rob Herring, linux-spi, Marek Vasut, Mark Brown, MTD Maling List,
	Miquel Raynal, Brian Norris, David Woodhouse

On Fri, 1 Jun 2018 19:40:00 +0200
Geert Uytterhoeven <geert@linux-m68k.org> wrote:

> Hi Boris,
> 
> On Fri, Jun 1, 2018 at 7:09 PM, Boris Brezillon
> <boris.brezillon@bootlin.com> wrote:
> > On Fri, 1 Jun 2018 16:42:26 +0200
> > Geert Uytterhoeven <geert@linux-m68k.org> wrote:  
> >> On Fri, Jun 1, 2018 at 3:13 PM, Boris Brezillon
> >> <boris.brezillon@bootlin.com> wrote:  
> >> > Add bindings for SPI NAND chips.  
> 
> >> > --- /dev/null
> >> > +++ b/Documentation/devicetree/bindings/mtd/spi-nand.txt
> >> > @@ -0,0 +1,27 @@
> >> > +SPI NAND flash
> >> > +
> >> > +Required properties:
> >> > +- compatible: should be "spi-nand"
> >> > +- reg: should encode the chip-select line used to access the NAND chip
> >> > +
> >> > +Optional properties
> >> > +- spi-max-frequency: maximum frequency of the SPI bus the chip can operate at.
> >> > +                    This should encode board limitations (i.e. max freq can't
> >> > +                    be achieved due to crosstalk on IO lines).
> >> > +                    When unspecified, the driver assumes the chip can run at
> >> > +                    the max frequency defined in the spec (information
> >> > +                    extracted chip detection time).  
> >>
> >> This is a standard property according to
> >> Documentation/devicetree/bindings/spi/spi-bus.txt. Can't you just refer
> >> to that file, or just omit it, as it applies to all SPI slaves anyway?  
> >
> > The thing is, the maximum frequency supported by a SPI NAND is directly
> > encoded in the NAND device ID and can be auto-detected. Why should we
> > define spi-max-frequency in the DT when we can automatically detect
> > this information? The only reason one might want to override  
> 
> Because that's how we dealt with this traditionally. Or at least I thought so.
> But include/linux/spi/spi.h says:
> 
>  * @max_speed_hz: Maximum clock rate to be used with this chip
>  *      (on this board); may be changed by the device's driver.
>  *      The spi_transfer.speed_hz can override this for each transfer.
> 
> and many drivers seem to do so.

And that's wrong, isn't it? I mean, if you have a constraint at the
board level, why should we allow the SPI device driver to override this
value? We should have:

	max(controller_max, device_max, board_max)

and not (this is my understanding of how SPI max-freq selection works)

	if (board_max)
		max = board_max;
	else
		max = controller_max;

	if (device_max)
		max = device_max;

> 
> > spi-max-frequency is when the board design impose such restrictions,
> > hence the precision I give here.  
> 
> Which is exactly the meaning of the standard property, isn't it?

Maybe it is how people expect the max-freq selection to work, but in
practice it doesn't seem to work like that (see spi_setup() and how the
value passed by the device driver overrides everything without even
checking if the controller and board constraints impose a lower freq).

> 
> So I think it can just be omitted here anyway.  If it's present, the SPI
> core will make sure it's taken into account.

Yes, I should probably just refer to spi-bus.txt here.

> 
> >> > +- spi-tx-bus-width: The bus width (number of data wires) that is used for MOSI.
> >> > +                   Only encodes the board constraints (i.e. when not all IO
> >> > +                   signals are routed on the board). Device constraints are
> >> > +                   extracted when detecting the chip, and controller
> >> > +                   constraints are exposed by the SPI mem controller. If this
> >> > +                   property is missing that means no constraint at the board
> >> > +                   level.
> >> > +- spi-rx-bus-width: The bus width (number of data wires) that is used for MISO.
> >> > +                   Only encodes the board constraints (i.e. when not all IO
> >> > +                   signals are routed on the board). Device constraints are
> >> > +                   extracted when detecting the chip, and controller
> >> > +                   constraints are exposed by the SPI mem controller. If this
> >> > +                   property is missing that means no constraint at the board
> >> > +                   level.  
> >>
> >> This does not match Documentation/devicetree/bindings/spi/spi-bus.txt,
> >> which says the default is 1.  
> >
> > Yes, I know.
> >  
> >> As these properties are handled by the SPI core in of_spi_parse_dt, why
> >> would you want to deviate?  
> >
> > Because, again, this information can be extracted from the NAND ID, and
> > the only reason we might want to override the information extracted
> > from the NAND ID is when the board design adds extra restrictions
> > (like, only 2 SIO lines wired on the 4 available).  
> 
> In struct spi_device, this is specified using the SPI_[RT]_X_{DUAL,QUAD}
> bits in the mode field. Documentation says:
> 
>  * @mode: The spi mode defines how data is clocked out and in.
>  *      This may be changed by the device's driver.
>  *      The "active low" default for chipselect mode can be overridden
>  *      (by specifying SPI_CS_HIGH) as can the "MSB first" default for
>  *      each word in a transfer (by specifying SPI_LSB_FIRST).
> 
> So this may also be specified by the device driver, but it seems no driver
> does so for the dual/quad bits, except for drivers/mtd/spi-nor/nxp-spifi.c
> (for rx only).

Same problem as for spi-max-frequency, the driver can override the value
that was specified in the DT, and the core does not seem to check
if the board or controller constraint were stronger.

> 
> But here we do have the generic SPI DT bindings saying the default is 1.
> So personally, I wouldn't go for the option of the least surprise, and
> don't deviate from the generic bindings.

I do have a problem with this approach: we are in the process of
converting existing spi-nor controller drivers to the SPI mem model in
order to allow the same controller to indifferently control a SPI NAND
or a SPI NOR. Such drivers had their own bindings which most of the
time was close to the bindings described in spi-bus.txt with a few
exceptions. One of these exception is that drivers tend to not
explicitly describe the bus width but still use the max buswidth
supported by the controller (QuadSPI) to interface with NOR chips.

When we convert those drivers we have 2 choices:
1/ make existing setup work in a degraded mode (1-1-1) until they update
   their DT to explicitly specify the spi-{rx,tx}-bus-width
2/ consider that when spi-{rx,tx}-bus-width is missing the device should
   work in the fastest possible mode matching the device+controller
   constraints

I went for option #2, but maybe #1 is fine. Note that spi-nand is not a
problem here because this is a new binding, but I thought it would be
better to clarify the meaning of these props before people start
(ab)using them.

> 
> >> Commenting to the question in the cover letter: what would be the
> >> purpose of spi-max-bus-width?  
> >
> > Defining how many IO lines are wired on the board design. If this
> > property is missing we would assume all IO lines are wired and the
> > restrictions would be negotiated between the controller and the device
> > without requiring explicit description in the DT.  
> 
> Which is exactly the meaning of the standard property, except for the
> default of all vs. 1.

And that's a huge difference. Say you have a board design and several
equivalence for the SPI NAND chip, one is supporting mode 1-1-4 (AKA
QuadSPI) and the other one is supporting only mode 1-1-1 (Single SPI).
With your solution we'd have to have 2 DTs or force all models to
operate in 1-1-1 mode. 

> 
> I'll have to defer to Mark (broonie) for his opinion about deviating from
> the way this is handled traditionally, and assuming different defaults...

I'm not saying that we should blindly change the meaning of those
existing props, I just wanted to start a discussion to see how the
problem I'm mentioning here should be handled (addition of new props is
something I'm perfectly fine with).

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

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

* Re: [PATCH v8 2/4] dt-bindings: Add bindings for SPI NAND devices
@ 2018-06-01 21:07           ` Boris Brezillon
  0 siblings, 0 replies; 24+ messages in thread
From: Boris Brezillon @ 2018-06-01 21:07 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: David Woodhouse, Brian Norris, Marek Vasut, Richard Weinberger,
	MTD Maling List, Miquel Raynal, Mark Brown, linux-spi,
	Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

On Fri, 1 Jun 2018 19:40:00 +0200
Geert Uytterhoeven <geert@linux-m68k.org> wrote:

> Hi Boris,
> 
> On Fri, Jun 1, 2018 at 7:09 PM, Boris Brezillon
> <boris.brezillon@bootlin.com> wrote:
> > On Fri, 1 Jun 2018 16:42:26 +0200
> > Geert Uytterhoeven <geert@linux-m68k.org> wrote:  
> >> On Fri, Jun 1, 2018 at 3:13 PM, Boris Brezillon
> >> <boris.brezillon@bootlin.com> wrote:  
> >> > Add bindings for SPI NAND chips.  
> 
> >> > --- /dev/null
> >> > +++ b/Documentation/devicetree/bindings/mtd/spi-nand.txt
> >> > @@ -0,0 +1,27 @@
> >> > +SPI NAND flash
> >> > +
> >> > +Required properties:
> >> > +- compatible: should be "spi-nand"
> >> > +- reg: should encode the chip-select line used to access the NAND chip
> >> > +
> >> > +Optional properties
> >> > +- spi-max-frequency: maximum frequency of the SPI bus the chip can operate at.
> >> > +                    This should encode board limitations (i.e. max freq can't
> >> > +                    be achieved due to crosstalk on IO lines).
> >> > +                    When unspecified, the driver assumes the chip can run at
> >> > +                    the max frequency defined in the spec (information
> >> > +                    extracted chip detection time).  
> >>
> >> This is a standard property according to
> >> Documentation/devicetree/bindings/spi/spi-bus.txt. Can't you just refer
> >> to that file, or just omit it, as it applies to all SPI slaves anyway?  
> >
> > The thing is, the maximum frequency supported by a SPI NAND is directly
> > encoded in the NAND device ID and can be auto-detected. Why should we
> > define spi-max-frequency in the DT when we can automatically detect
> > this information? The only reason one might want to override  
> 
> Because that's how we dealt with this traditionally. Or at least I thought so.
> But include/linux/spi/spi.h says:
> 
>  * @max_speed_hz: Maximum clock rate to be used with this chip
>  *      (on this board); may be changed by the device's driver.
>  *      The spi_transfer.speed_hz can override this for each transfer.
> 
> and many drivers seem to do so.

And that's wrong, isn't it? I mean, if you have a constraint at the
board level, why should we allow the SPI device driver to override this
value? We should have:

	max(controller_max, device_max, board_max)

and not (this is my understanding of how SPI max-freq selection works)

	if (board_max)
		max = board_max;
	else
		max = controller_max;

	if (device_max)
		max = device_max;

> 
> > spi-max-frequency is when the board design impose such restrictions,
> > hence the precision I give here.  
> 
> Which is exactly the meaning of the standard property, isn't it?

Maybe it is how people expect the max-freq selection to work, but in
practice it doesn't seem to work like that (see spi_setup() and how the
value passed by the device driver overrides everything without even
checking if the controller and board constraints impose a lower freq).

> 
> So I think it can just be omitted here anyway.  If it's present, the SPI
> core will make sure it's taken into account.

Yes, I should probably just refer to spi-bus.txt here.

> 
> >> > +- spi-tx-bus-width: The bus width (number of data wires) that is used for MOSI.
> >> > +                   Only encodes the board constraints (i.e. when not all IO
> >> > +                   signals are routed on the board). Device constraints are
> >> > +                   extracted when detecting the chip, and controller
> >> > +                   constraints are exposed by the SPI mem controller. If this
> >> > +                   property is missing that means no constraint at the board
> >> > +                   level.
> >> > +- spi-rx-bus-width: The bus width (number of data wires) that is used for MISO.
> >> > +                   Only encodes the board constraints (i.e. when not all IO
> >> > +                   signals are routed on the board). Device constraints are
> >> > +                   extracted when detecting the chip, and controller
> >> > +                   constraints are exposed by the SPI mem controller. If this
> >> > +                   property is missing that means no constraint at the board
> >> > +                   level.  
> >>
> >> This does not match Documentation/devicetree/bindings/spi/spi-bus.txt,
> >> which says the default is 1.  
> >
> > Yes, I know.
> >  
> >> As these properties are handled by the SPI core in of_spi_parse_dt, why
> >> would you want to deviate?  
> >
> > Because, again, this information can be extracted from the NAND ID, and
> > the only reason we might want to override the information extracted
> > from the NAND ID is when the board design adds extra restrictions
> > (like, only 2 SIO lines wired on the 4 available).  
> 
> In struct spi_device, this is specified using the SPI_[RT]_X_{DUAL,QUAD}
> bits in the mode field. Documentation says:
> 
>  * @mode: The spi mode defines how data is clocked out and in.
>  *      This may be changed by the device's driver.
>  *      The "active low" default for chipselect mode can be overridden
>  *      (by specifying SPI_CS_HIGH) as can the "MSB first" default for
>  *      each word in a transfer (by specifying SPI_LSB_FIRST).
> 
> So this may also be specified by the device driver, but it seems no driver
> does so for the dual/quad bits, except for drivers/mtd/spi-nor/nxp-spifi.c
> (for rx only).

Same problem as for spi-max-frequency, the driver can override the value
that was specified in the DT, and the core does not seem to check
if the board or controller constraint were stronger.

> 
> But here we do have the generic SPI DT bindings saying the default is 1.
> So personally, I wouldn't go for the option of the least surprise, and
> don't deviate from the generic bindings.

I do have a problem with this approach: we are in the process of
converting existing spi-nor controller drivers to the SPI mem model in
order to allow the same controller to indifferently control a SPI NAND
or a SPI NOR. Such drivers had their own bindings which most of the
time was close to the bindings described in spi-bus.txt with a few
exceptions. One of these exception is that drivers tend to not
explicitly describe the bus width but still use the max buswidth
supported by the controller (QuadSPI) to interface with NOR chips.

When we convert those drivers we have 2 choices:
1/ make existing setup work in a degraded mode (1-1-1) until they update
   their DT to explicitly specify the spi-{rx,tx}-bus-width
2/ consider that when spi-{rx,tx}-bus-width is missing the device should
   work in the fastest possible mode matching the device+controller
   constraints

I went for option #2, but maybe #1 is fine. Note that spi-nand is not a
problem here because this is a new binding, but I thought it would be
better to clarify the meaning of these props before people start
(ab)using them.

> 
> >> Commenting to the question in the cover letter: what would be the
> >> purpose of spi-max-bus-width?  
> >
> > Defining how many IO lines are wired on the board design. If this
> > property is missing we would assume all IO lines are wired and the
> > restrictions would be negotiated between the controller and the device
> > without requiring explicit description in the DT.  
> 
> Which is exactly the meaning of the standard property, except for the
> default of all vs. 1.

And that's a huge difference. Say you have a board design and several
equivalence for the SPI NAND chip, one is supporting mode 1-1-4 (AKA
QuadSPI) and the other one is supporting only mode 1-1-1 (Single SPI).
With your solution we'd have to have 2 DTs or force all models to
operate in 1-1-1 mode. 

> 
> I'll have to defer to Mark (broonie) for his opinion about deviating from
> the way this is handled traditionally, and assuming different defaults...

I'm not saying that we should blindly change the meaning of those
existing props, I just wanted to start a discussion to see how the
problem I'm mentioning here should be handled (addition of new props is
something I'm perfectly fine with).

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

* Re: [PATCH v8 2/4] dt-bindings: Add bindings for SPI NAND devices
  2018-06-01 17:40         ` Geert Uytterhoeven
@ 2018-06-04 10:04           ` Boris Brezillon
  -1 siblings, 0 replies; 24+ messages in thread
From: Boris Brezillon @ 2018-06-04 10:04 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Mark Rutland,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Pawel Moll, Ian Campbell, Richard Weinberger, Kumar Gala,
	Rob Herring, linux-spi, Marek Vasut, Mark Brown, MTD Maling List,
	Miquel Raynal, Brian Norris, David Woodhouse

On Fri, 1 Jun 2018 19:40:00 +0200
Geert Uytterhoeven <geert@linux-m68k.org> wrote:

> Hi Boris,
> 
> On Fri, Jun 1, 2018 at 7:09 PM, Boris Brezillon
> <boris.brezillon@bootlin.com> wrote:
> > On Fri, 1 Jun 2018 16:42:26 +0200
> > Geert Uytterhoeven <geert@linux-m68k.org> wrote:  
> >> On Fri, Jun 1, 2018 at 3:13 PM, Boris Brezillon
> >> <boris.brezillon@bootlin.com> wrote:  
> >> > Add bindings for SPI NAND chips.  
> 
> >> > --- /dev/null
> >> > +++ b/Documentation/devicetree/bindings/mtd/spi-nand.txt
> >> > @@ -0,0 +1,27 @@
> >> > +SPI NAND flash
> >> > +
> >> > +Required properties:
> >> > +- compatible: should be "spi-nand"
> >> > +- reg: should encode the chip-select line used to access the NAND chip
> >> > +
> >> > +Optional properties
> >> > +- spi-max-frequency: maximum frequency of the SPI bus the chip can operate at.
> >> > +                    This should encode board limitations (i.e. max freq can't
> >> > +                    be achieved due to crosstalk on IO lines).
> >> > +                    When unspecified, the driver assumes the chip can run at
> >> > +                    the max frequency defined in the spec (information
> >> > +                    extracted chip detection time).  
> >>
> >> This is a standard property according to
> >> Documentation/devicetree/bindings/spi/spi-bus.txt. Can't you just refer
> >> to that file, or just omit it, as it applies to all SPI slaves anyway?  
> >
> > The thing is, the maximum frequency supported by a SPI NAND is directly
> > encoded in the NAND device ID and can be auto-detected. Why should we
> > define spi-max-frequency in the DT when we can automatically detect
> > this information? The only reason one might want to override  
> 
> Because that's how we dealt with this traditionally. Or at least I thought so.
> But include/linux/spi/spi.h says:
> 
>  * @max_speed_hz: Maximum clock rate to be used with this chip
>  *      (on this board); may be changed by the device's driver.
>  *      The spi_transfer.speed_hz can override this for each transfer.
> 
> and many drivers seem to do so.
> 
> > spi-max-frequency is when the board design impose such restrictions,
> > hence the precision I give here.  
> 
> Which is exactly the meaning of the standard property, isn't it?
> 
> So I think it can just be omitted here anyway.  If it's present, the SPI
> core will make sure it's taken into account.
> 
> >> > +- spi-tx-bus-width: The bus width (number of data wires) that is used for MOSI.
> >> > +                   Only encodes the board constraints (i.e. when not all IO
> >> > +                   signals are routed on the board). Device constraints are
> >> > +                   extracted when detecting the chip, and controller
> >> > +                   constraints are exposed by the SPI mem controller. If this
> >> > +                   property is missing that means no constraint at the board
> >> > +                   level.
> >> > +- spi-rx-bus-width: The bus width (number of data wires) that is used for MISO.
> >> > +                   Only encodes the board constraints (i.e. when not all IO
> >> > +                   signals are routed on the board). Device constraints are
> >> > +                   extracted when detecting the chip, and controller
> >> > +                   constraints are exposed by the SPI mem controller. If this
> >> > +                   property is missing that means no constraint at the board
> >> > +                   level.  
> >>
> >> This does not match Documentation/devicetree/bindings/spi/spi-bus.txt,
> >> which says the default is 1.  
> >
> > Yes, I know.
> >  
> >> As these properties are handled by the SPI core in of_spi_parse_dt, why
> >> would you want to deviate?  
> >
> > Because, again, this information can be extracted from the NAND ID, and
> > the only reason we might want to override the information extracted
> > from the NAND ID is when the board design adds extra restrictions
> > (like, only 2 SIO lines wired on the 4 available).  
> 
> In struct spi_device, this is specified using the SPI_[RT]_X_{DUAL,QUAD}
> bits in the mode field. Documentation says:
> 
>  * @mode: The spi mode defines how data is clocked out and in.
>  *      This may be changed by the device's driver.
>  *      The "active low" default for chipselect mode can be overridden
>  *      (by specifying SPI_CS_HIGH) as can the "MSB first" default for
>  *      each word in a transfer (by specifying SPI_LSB_FIRST).
> 
> So this may also be specified by the device driver, but it seems no driver
> does so for the dual/quad bits, except for drivers/mtd/spi-nor/nxp-spifi.c
> (for rx only).
> 
> But here we do have the generic SPI DT bindings saying the default is 1.
> So personally, I wouldn't go for the option of the least surprise, and
> don't deviate from the generic bindings.
> 
> >> Commenting to the question in the cover letter: what would be the
> >> purpose of spi-max-bus-width?  
> >
> > Defining how many IO lines are wired on the board design. If this
> > property is missing we would assume all IO lines are wired and the
> > restrictions would be negotiated between the controller and the device
> > without requiring explicit description in the DT.  
> 
> Which is exactly the meaning of the standard property, except for the
> default of all vs. 1.
> 
> I'll have to defer to Mark (broonie) for his opinion about deviating from
> the way this is handled traditionally, and assuming different defaults...

I think I'll drop the extra description on spi-max-frequency and
spi-{rx,tx}-bus-width props for now. Those are anyway not taken into
account by the SPI NAND driver yet.

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

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

* Re: [PATCH v8 2/4] dt-bindings: Add bindings for SPI NAND devices
@ 2018-06-04 10:04           ` Boris Brezillon
  0 siblings, 0 replies; 24+ messages in thread
From: Boris Brezillon @ 2018-06-04 10:04 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: David Woodhouse, Brian Norris, Marek Vasut, Richard Weinberger,
	MTD Maling List, Miquel Raynal, Mark Brown, linux-spi,
	Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

On Fri, 1 Jun 2018 19:40:00 +0200
Geert Uytterhoeven <geert@linux-m68k.org> wrote:

> Hi Boris,
> 
> On Fri, Jun 1, 2018 at 7:09 PM, Boris Brezillon
> <boris.brezillon@bootlin.com> wrote:
> > On Fri, 1 Jun 2018 16:42:26 +0200
> > Geert Uytterhoeven <geert@linux-m68k.org> wrote:  
> >> On Fri, Jun 1, 2018 at 3:13 PM, Boris Brezillon
> >> <boris.brezillon@bootlin.com> wrote:  
> >> > Add bindings for SPI NAND chips.  
> 
> >> > --- /dev/null
> >> > +++ b/Documentation/devicetree/bindings/mtd/spi-nand.txt
> >> > @@ -0,0 +1,27 @@
> >> > +SPI NAND flash
> >> > +
> >> > +Required properties:
> >> > +- compatible: should be "spi-nand"
> >> > +- reg: should encode the chip-select line used to access the NAND chip
> >> > +
> >> > +Optional properties
> >> > +- spi-max-frequency: maximum frequency of the SPI bus the chip can operate at.
> >> > +                    This should encode board limitations (i.e. max freq can't
> >> > +                    be achieved due to crosstalk on IO lines).
> >> > +                    When unspecified, the driver assumes the chip can run at
> >> > +                    the max frequency defined in the spec (information
> >> > +                    extracted chip detection time).  
> >>
> >> This is a standard property according to
> >> Documentation/devicetree/bindings/spi/spi-bus.txt. Can't you just refer
> >> to that file, or just omit it, as it applies to all SPI slaves anyway?  
> >
> > The thing is, the maximum frequency supported by a SPI NAND is directly
> > encoded in the NAND device ID and can be auto-detected. Why should we
> > define spi-max-frequency in the DT when we can automatically detect
> > this information? The only reason one might want to override  
> 
> Because that's how we dealt with this traditionally. Or at least I thought so.
> But include/linux/spi/spi.h says:
> 
>  * @max_speed_hz: Maximum clock rate to be used with this chip
>  *      (on this board); may be changed by the device's driver.
>  *      The spi_transfer.speed_hz can override this for each transfer.
> 
> and many drivers seem to do so.
> 
> > spi-max-frequency is when the board design impose such restrictions,
> > hence the precision I give here.  
> 
> Which is exactly the meaning of the standard property, isn't it?
> 
> So I think it can just be omitted here anyway.  If it's present, the SPI
> core will make sure it's taken into account.
> 
> >> > +- spi-tx-bus-width: The bus width (number of data wires) that is used for MOSI.
> >> > +                   Only encodes the board constraints (i.e. when not all IO
> >> > +                   signals are routed on the board). Device constraints are
> >> > +                   extracted when detecting the chip, and controller
> >> > +                   constraints are exposed by the SPI mem controller. If this
> >> > +                   property is missing that means no constraint at the board
> >> > +                   level.
> >> > +- spi-rx-bus-width: The bus width (number of data wires) that is used for MISO.
> >> > +                   Only encodes the board constraints (i.e. when not all IO
> >> > +                   signals are routed on the board). Device constraints are
> >> > +                   extracted when detecting the chip, and controller
> >> > +                   constraints are exposed by the SPI mem controller. If this
> >> > +                   property is missing that means no constraint at the board
> >> > +                   level.  
> >>
> >> This does not match Documentation/devicetree/bindings/spi/spi-bus.txt,
> >> which says the default is 1.  
> >
> > Yes, I know.
> >  
> >> As these properties are handled by the SPI core in of_spi_parse_dt, why
> >> would you want to deviate?  
> >
> > Because, again, this information can be extracted from the NAND ID, and
> > the only reason we might want to override the information extracted
> > from the NAND ID is when the board design adds extra restrictions
> > (like, only 2 SIO lines wired on the 4 available).  
> 
> In struct spi_device, this is specified using the SPI_[RT]_X_{DUAL,QUAD}
> bits in the mode field. Documentation says:
> 
>  * @mode: The spi mode defines how data is clocked out and in.
>  *      This may be changed by the device's driver.
>  *      The "active low" default for chipselect mode can be overridden
>  *      (by specifying SPI_CS_HIGH) as can the "MSB first" default for
>  *      each word in a transfer (by specifying SPI_LSB_FIRST).
> 
> So this may also be specified by the device driver, but it seems no driver
> does so for the dual/quad bits, except for drivers/mtd/spi-nor/nxp-spifi.c
> (for rx only).
> 
> But here we do have the generic SPI DT bindings saying the default is 1.
> So personally, I wouldn't go for the option of the least surprise, and
> don't deviate from the generic bindings.
> 
> >> Commenting to the question in the cover letter: what would be the
> >> purpose of spi-max-bus-width?  
> >
> > Defining how many IO lines are wired on the board design. If this
> > property is missing we would assume all IO lines are wired and the
> > restrictions would be negotiated between the controller and the device
> > without requiring explicit description in the DT.  
> 
> Which is exactly the meaning of the standard property, except for the
> default of all vs. 1.
> 
> I'll have to defer to Mark (broonie) for his opinion about deviating from
> the way this is handled traditionally, and assuming different defaults...

I think I'll drop the extra description on spi-max-frequency and
spi-{rx,tx}-bus-width props for now. Those are anyway not taken into
account by the SPI NAND driver yet.

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

end of thread, other threads:[~2018-06-04 10:05 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-01 13:13 [PATCH v8 0/4] mtd: Add a SPI NAND driver Boris Brezillon
2018-06-01 13:13 ` Boris Brezillon
2018-06-01 13:13 ` [PATCH v8 1/4] mtd: nand: Add core infrastructure to support SPI NANDs Boris Brezillon
2018-06-01 13:13   ` Boris Brezillon
2018-06-01 13:13 ` [PATCH v8 2/4] dt-bindings: Add bindings for SPI NAND devices Boris Brezillon
2018-06-01 13:13   ` Boris Brezillon
2018-06-01 14:42   ` Geert Uytterhoeven
2018-06-01 14:42     ` Geert Uytterhoeven
2018-06-01 17:09     ` Boris Brezillon
2018-06-01 17:09       ` Boris Brezillon
2018-06-01 17:40       ` Geert Uytterhoeven
2018-06-01 17:40         ` Geert Uytterhoeven
2018-06-01 21:07         ` Boris Brezillon
2018-06-01 21:07           ` Boris Brezillon
2018-06-04 10:04         ` Boris Brezillon
2018-06-04 10:04           ` Boris Brezillon
2018-06-01 19:18       ` Rob Herring
2018-06-01 19:18         ` Rob Herring
2018-06-01 20:32         ` Boris Brezillon
2018-06-01 20:32           ` Boris Brezillon
2018-06-01 13:13 ` [PATCH v8 3/4] mtd: spinand: Add initial support for Micron MT29F2G01ABAGD Boris Brezillon
2018-06-01 13:13   ` Boris Brezillon
2018-06-01 13:14 ` [PATCH v8 4/4] mtd: spinand: Add initial support for Winbond W25M02GV Boris Brezillon
2018-06-01 13:14   ` Boris Brezillon

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.