linux-mtd.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH 00/27] Introduce the generic ECC engine abstraction
@ 2019-02-21 10:01 Miquel Raynal
  2019-02-21 10:01 ` [RFC PATCH 01/27] mtd: nand: Move nand_device forward declaration to the top Miquel Raynal
                   ` (11 more replies)
  0 siblings, 12 replies; 36+ messages in thread
From: Miquel Raynal @ 2019-02-21 10:01 UTC (permalink / raw)
  To: Boris Brezillon, Richard Weinberger, David Woodhouse,
	Brian Norris, Marek Vasut, Tudor Ambarus
  Cc: Vignesh R, Tudor Ambarus, Julien Su, Schrempf Frieder, linux-mtd,
	Thomas Petazzoni, Miquel Raynal, Mason Yang, linux-arm-kernel

As of today, only raw NAND controllers used to feature an integrated
ECC engine and so controller drivers always embedded some code to
enable/disable the correction.

This statement is no longer correct as SPI-NAND devices might not
embed an on-die ECC engine and must make use of an external ECC
engine. We figured there are three possible situations for (generic)
NAND device: either the engine is 'on-die' (most of the SPI-NANDs, a
few raw NANDs), or the engine is part of the host controller (most raw
NANDs), or the engine may be external (SPI controllers might feature
an ECC engine, or there are still the possibility to use software
correction).


To solve this situation, this is a proposal on how to make things
work. We want to create an ECC engine object which has simple
callbacks:
* init/cleanup its context
* prepare an I/O operation
* finish an I/O operation
Details about what is going to happen in these callbacks is described
in drivers/mtd/nand/ecc/engine.c.

The logic in this series is:
1/ Use the generic NAND core for all NAND devices (raw and SPI).
2/ Create the ECC engine interface in drivers/mtd/nand/ecc/
3/ Move code in driver/mtd/nand/ecc.
4/ Make both software engines (Hamming and BCH) generic, move them in
   the ecc/ directory, clean them a bit and instantiate ECC
   engines. Write raw NAND helpers to use these two new engines.
5/ Isolate SPI-NAND on-die ECC engine in its own driver.
6/ Make use from the SPI-NAND layer of all the ECC engines listed
   above (on user request, people can now make use of soft BCH if they
   don't have an ECC-engine).

This work is still WIP, I expect a few 0-day regressions, maybe the
naming is not perfect but it gives an idea of what I would like to
introduce. The next steps are:
1/ Migrate the raw NAND core to make a proper use of these ECC
   engines.
2/ Deprecate in the raw NAND subsystem the interfaces used until now
   (I expect we should get rid of a lot of boilerplate).
3/ Introduce an external hardware ECC engine driver.


Thanks,
Miquèl


Miquel Raynal (27):
  mtd: nand: Move nand_device forward declaration to the top
  mtd: nand: Compile in the NAND core by default
  mtd: nand: Introduce the ECC engine abstraction
  mtd: Fix typo in mtd_ooblayout_set_databytes() description
  mtd: nand: Move standard OOB layouts to the NAND core
  mtd: nand: Move ECC specific functions to ecc/engine.c
  mtd: nand: ecc: Move BCH code into the ecc/ directory
  mtd: nand: ecc: Use SPDX license identifier for the software BCH code
  mtd: nand: ecc: Turn the software BCH implementation generic
  mtd: rawnand: Get rid of chip->ecc.priv
  mtd: nand: ecc: Move Hamming code into the ecc/ directory
  mtd: nand: ecc: Use SPDX license identifier for the software Hamming
    code
  mtd: nand: ecc: Clarify the software Hamming introductory line
  mtd: nand: ecc: Turn the software Hamming implementation generic
  mtd: nand: Remove useless include about software Hamming ECC
  mtd: nand: ecc: Let the software BCH ECC engine be a module
  mtd: nand: ecc: Let the software Hamming ECC engine be unselected
  mtd: nand: ecc: Create the software BCH engine instance
  mtd: nand: ecc: Create the software Hamming engine instance
  mtd: nand: Let software ECC engines be retrieved from the NAND core
  mtd: spinand: Fix typo in comment
  mtd: spinand: Let the SPI-NAND core flag a SPI-NAND chip
  mtd: spinand: Move the ECC helper functions into a separate file
  mtd: spinand: Instantiate a SPI-NAND on-die ECC engine
  mtd: nand: Add helpers to manage ECC engines and configurations
  mtd: spinand: Use the external ECC engine logic
  mtd: spinand: Propagate ECC information to the MTD structure

 arch/arm/mach-s3c24xx/common-smdk.c           |   1 -
 arch/arm/mach-s3c24xx/mach-anubis.c           |   1 -
 arch/arm/mach-s3c24xx/mach-at2440evb.c        |   1 -
 arch/arm/mach-s3c24xx/mach-bast.c             |   1 -
 arch/arm/mach-s3c24xx/mach-gta02.c            |   1 -
 arch/arm/mach-s3c24xx/mach-jive.c             |   1 -
 arch/arm/mach-s3c24xx/mach-mini2440.c         |   1 -
 arch/arm/mach-s3c24xx/mach-osiris.c           |   1 -
 arch/arm/mach-s3c24xx/mach-qt2410.c           |   1 -
 arch/arm/mach-s3c24xx/mach-rx3715.c           |   1 -
 arch/arm/mach-s3c24xx/mach-vstms.c            |   1 -
 drivers/mtd/mtdcore.c                         |   2 +-
 drivers/mtd/nand/Kconfig                      |  13 +-
 drivers/mtd/nand/Makefile                     |   1 +
 drivers/mtd/nand/core.c                       | 270 ++++++++-
 drivers/mtd/nand/ecc/Kconfig                  |  32 ++
 drivers/mtd/nand/ecc/Makefile                 |   5 +
 drivers/mtd/nand/ecc/engine.c                 | 305 ++++++++++
 drivers/mtd/nand/ecc/sw-bch-engine.c          | 421 ++++++++++++++
 .../nand_ecc.c => ecc/sw-hamming-engine.c}    | 343 ++++++++---
 drivers/mtd/nand/onenand/Kconfig              |   1 -
 drivers/mtd/nand/raw/Kconfig                  |  24 +-
 drivers/mtd/nand/raw/Makefile                 |   2 -
 drivers/mtd/nand/raw/atmel/nand-controller.c  |  12 +-
 drivers/mtd/nand/raw/cs553x_nand.c            |   3 +-
 drivers/mtd/nand/raw/denali.c                 |   3 +
 drivers/mtd/nand/raw/denali_pci.c             |   1 -
 drivers/mtd/nand/raw/fsl_elbc_nand.c          |   1 -
 drivers/mtd/nand/raw/fsl_ifc_nand.c           |   1 -
 drivers/mtd/nand/raw/fsl_upm.c                |   1 -
 drivers/mtd/nand/raw/fsmc_nand.c              |   3 +-
 drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c    |  12 +-
 drivers/mtd/nand/raw/lpc32xx_mlc.c            |   1 -
 drivers/mtd/nand/raw/lpc32xx_slc.c            |   3 +-
 drivers/mtd/nand/raw/marvell_nand.c           |   7 +-
 drivers/mtd/nand/raw/mtk_nand.c               |   4 +-
 drivers/mtd/nand/raw/nand_base.c              | 536 ++++++------------
 drivers/mtd/nand/raw/nand_bch.c               | 232 --------
 drivers/mtd/nand/raw/nand_esmt.c              |  11 +-
 drivers/mtd/nand/raw/nand_hynix.c             |  41 +-
 drivers/mtd/nand/raw/nand_jedec.c             |   4 +-
 drivers/mtd/nand/raw/nand_micron.c            |  14 +-
 drivers/mtd/nand/raw/nand_onfi.c              |   8 +-
 drivers/mtd/nand/raw/nand_samsung.c           |  19 +-
 drivers/mtd/nand/raw/nand_toshiba.c           |  13 +-
 drivers/mtd/nand/raw/nandsim.c                |   3 +-
 drivers/mtd/nand/raw/ndfc.c                   |   3 +-
 drivers/mtd/nand/raw/omap2.c                  |  32 +-
 drivers/mtd/nand/raw/pasemi_nand.c            |   1 -
 drivers/mtd/nand/raw/s3c2410.c                |   1 -
 drivers/mtd/nand/raw/sharpsl.c                |   3 +-
 drivers/mtd/nand/raw/sunxi_nand.c             |  37 +-
 drivers/mtd/nand/raw/tegra_nand.c             |  12 +-
 drivers/mtd/nand/raw/tmio_nand.c              |   7 +-
 drivers/mtd/nand/raw/txx9ndfmc.c              |   5 +-
 drivers/mtd/nand/spi/Kconfig                  |   1 -
 drivers/mtd/nand/spi/Makefile                 |   2 +-
 drivers/mtd/nand/spi/core.c                   | 174 ++----
 drivers/mtd/nand/spi/macronix.c               |   6 +-
 drivers/mtd/nand/spi/on-die-ecc-engine.c      | 152 +++++
 drivers/mtd/nand/spi/toshiba.c                |   6 +-
 drivers/mtd/sm_ftl.c                          |  29 +-
 drivers/mtd/tests/mtd_nandecctest.c           |  31 +-
 include/linux/mtd/mtd.h                       |   5 +
 include/linux/mtd/nand-spi-on-die-engine.h    |  35 ++
 include/linux/mtd/nand-sw-bch-engine.h        |  80 +++
 include/linux/mtd/nand-sw-hamming-engine.h    | 101 ++++
 include/linux/mtd/nand.h                      | 160 +++++-
 include/linux/mtd/nand_bch.h                  |  69 ---
 include/linux/mtd/nand_ecc.h                  |  42 --
 include/linux/mtd/rawnand.h                   |  40 +-
 include/linux/mtd/sharpsl.h                   |   1 -
 include/linux/mtd/spinand.h                   |   5 +-
 include/uapi/mtd/mtd-abi.h                    |   1 +
 74 files changed, 2242 insertions(+), 1162 deletions(-)
 create mode 100644 drivers/mtd/nand/ecc/Kconfig
 create mode 100644 drivers/mtd/nand/ecc/Makefile
 create mode 100644 drivers/mtd/nand/ecc/engine.c
 create mode 100644 drivers/mtd/nand/ecc/sw-bch-engine.c
 rename drivers/mtd/nand/{raw/nand_ecc.c => ecc/sw-hamming-engine.c} (61%)
 delete mode 100644 drivers/mtd/nand/raw/nand_bch.c
 create mode 100644 drivers/mtd/nand/spi/on-die-ecc-engine.c
 create mode 100644 include/linux/mtd/nand-spi-on-die-engine.h
 create mode 100644 include/linux/mtd/nand-sw-bch-engine.h
 create mode 100644 include/linux/mtd/nand-sw-hamming-engine.h
 delete mode 100644 include/linux/mtd/nand_bch.h
 delete mode 100644 include/linux/mtd/nand_ecc.h

-- 
2.19.1


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

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

* [RFC PATCH 01/27] mtd: nand: Move nand_device forward declaration to the top
  2019-02-21 10:01 [RFC PATCH 00/27] Introduce the generic ECC engine abstraction Miquel Raynal
@ 2019-02-21 10:01 ` Miquel Raynal
  2019-02-21 10:01 ` [RFC PATCH 02/27] mtd: nand: Compile in the NAND core by default Miquel Raynal
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 36+ messages in thread
From: Miquel Raynal @ 2019-02-21 10:01 UTC (permalink / raw)
  To: Boris Brezillon, Richard Weinberger, David Woodhouse,
	Brian Norris, Marek Vasut, Tudor Ambarus
  Cc: Vignesh R, Tudor Ambarus, Julien Su, Schrempf Frieder, linux-mtd,
	Thomas Petazzoni, Miquel Raynal, Mason Yang, linux-arm-kernel

This structure might be used earlier in this file, let's move the
forward declaration at the top.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 include/linux/mtd/nand.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index cebc38b6d6f5..30f0fb02abe2 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -12,6 +12,8 @@
 
 #include <linux/mtd/mtd.h>
 
+struct nand_device;
+
 /**
  * struct nand_memory_organization - Memory organization structure
  * @bits_per_cell: number of bits per NAND cell
@@ -133,8 +135,6 @@ struct nand_bbt {
 	unsigned long *cache;
 };
 
-struct nand_device;
-
 /**
  * struct nand_ops - NAND operations
  * @erase: erase a specific block. No need to check if the block is bad before
-- 
2.19.1


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

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

* [RFC PATCH 02/27] mtd: nand: Compile in the NAND core by default
  2019-02-21 10:01 [RFC PATCH 00/27] Introduce the generic ECC engine abstraction Miquel Raynal
  2019-02-21 10:01 ` [RFC PATCH 01/27] mtd: nand: Move nand_device forward declaration to the top Miquel Raynal
@ 2019-02-21 10:01 ` Miquel Raynal
  2019-02-21 10:55   ` Boris Brezillon
  2019-02-21 10:01 ` [RFC PATCH 03/27] mtd: nand: Introduce the ECC engine abstraction Miquel Raynal
                   ` (9 subsequent siblings)
  11 siblings, 1 reply; 36+ messages in thread
From: Miquel Raynal @ 2019-02-21 10:01 UTC (permalink / raw)
  To: Boris Brezillon, Richard Weinberger, David Woodhouse,
	Brian Norris, Marek Vasut, Tudor Ambarus
  Cc: Vignesh R, Tudor Ambarus, Julien Su, Schrempf Frieder, linux-mtd,
	Thomas Petazzoni, Miquel Raynal, Mason Yang, linux-arm-kernel

Force the NAND core be compiled-in when using any kind of NAND.

Also remove the redundant dependencies on MTD which is enforced by the
game of the if/endif blocs.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/Kconfig         | 12 ++++++++++--
 drivers/mtd/nand/onenand/Kconfig |  1 -
 drivers/mtd/nand/raw/Kconfig     |  1 -
 drivers/mtd/nand/spi/Kconfig     |  1 -
 4 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index 495751ed3fd7..e8d26a715922 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -1,6 +1,14 @@
-config MTD_NAND_CORE
-	tristate
+menuconfig MTD_NAND_CORE
+	tristate "NAND"
+	default y
+	help
+	  Embed the NAND core. This is the generic part between raw
+	  NAND, SPI NAND and Onenand.
+
+if MTD_NAND_CORE
 
 source "drivers/mtd/nand/onenand/Kconfig"
 source "drivers/mtd/nand/raw/Kconfig"
 source "drivers/mtd/nand/spi/Kconfig"
+
+endif
diff --git a/drivers/mtd/nand/onenand/Kconfig b/drivers/mtd/nand/onenand/Kconfig
index 9dc15748947b..c168f3b4b296 100644
--- a/drivers/mtd/nand/onenand/Kconfig
+++ b/drivers/mtd/nand/onenand/Kconfig
@@ -1,6 +1,5 @@
 menuconfig MTD_ONENAND
 	tristate "OneNAND Device Support"
-	depends on MTD
 	depends on HAS_IOMEM
 	help
 	  This enables support for accessing all type of OneNAND flash
diff --git a/drivers/mtd/nand/raw/Kconfig b/drivers/mtd/nand/raw/Kconfig
index 510a6b32820d..ebb8a3da9fa5 100644
--- a/drivers/mtd/nand/raw/Kconfig
+++ b/drivers/mtd/nand/raw/Kconfig
@@ -11,7 +11,6 @@ config MTD_NAND_ECC_SW_HAMMING_SMC
 
 menuconfig MTD_RAW_NAND
 	tristate "Raw/Parallel NAND Device Support"
-	depends on MTD
 	select MTD_NAND_ECC_SW_HAMMING
 	help
 	  This enables support for accessing all type of raw/parallel
diff --git a/drivers/mtd/nand/spi/Kconfig b/drivers/mtd/nand/spi/Kconfig
index 7c37d2929b68..7631050610ab 100644
--- a/drivers/mtd/nand/spi/Kconfig
+++ b/drivers/mtd/nand/spi/Kconfig
@@ -1,6 +1,5 @@
 menuconfig MTD_SPI_NAND
 	tristate "SPI NAND device Support"
-	select MTD_NAND_CORE
 	depends on SPI_MASTER
 	select SPI_MEM
 	help
-- 
2.19.1


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

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

* [RFC PATCH 03/27] mtd: nand: Introduce the ECC engine abstraction
  2019-02-21 10:01 [RFC PATCH 00/27] Introduce the generic ECC engine abstraction Miquel Raynal
  2019-02-21 10:01 ` [RFC PATCH 01/27] mtd: nand: Move nand_device forward declaration to the top Miquel Raynal
  2019-02-21 10:01 ` [RFC PATCH 02/27] mtd: nand: Compile in the NAND core by default Miquel Raynal
@ 2019-02-21 10:01 ` Miquel Raynal
  2019-02-21 11:16   ` Boris Brezillon
  2019-02-25 18:55   ` Boris Brezillon
  2019-02-21 10:01 ` [RFC PATCH 04/27] mtd: Fix typo in mtd_ooblayout_set_databytes() description Miquel Raynal
                   ` (8 subsequent siblings)
  11 siblings, 2 replies; 36+ messages in thread
From: Miquel Raynal @ 2019-02-21 10:01 UTC (permalink / raw)
  To: Boris Brezillon, Richard Weinberger, David Woodhouse,
	Brian Norris, Marek Vasut, Tudor Ambarus
  Cc: Vignesh R, Tudor Ambarus, Julien Su, Schrempf Frieder, linux-mtd,
	Thomas Petazzoni, Miquel Raynal, Mason Yang, linux-arm-kernel

Create a generic ECC engine object.

Later the ecc/engine.c file will receive more generic code coming from
the raw NAND specific part. This is a base to instantiate ECC engine
objects.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/Kconfig                     |   1 +
 drivers/mtd/nand/Makefile                    |   1 +
 drivers/mtd/nand/ecc/Kconfig                 |   3 +
 drivers/mtd/nand/ecc/Makefile                |   3 +
 drivers/mtd/nand/ecc/engine.c                | 134 +++++++++++++++++++
 drivers/mtd/nand/raw/atmel/nand-controller.c |   9 +-
 drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c   |  12 +-
 drivers/mtd/nand/raw/marvell_nand.c          |   7 +-
 drivers/mtd/nand/raw/mtk_nand.c              |   4 +-
 drivers/mtd/nand/raw/nand_base.c             |  17 +--
 drivers/mtd/nand/raw/nand_esmt.c             |  11 +-
 drivers/mtd/nand/raw/nand_hynix.c            |  41 +++---
 drivers/mtd/nand/raw/nand_jedec.c            |   4 +-
 drivers/mtd/nand/raw/nand_micron.c           |  14 +-
 drivers/mtd/nand/raw/nand_onfi.c             |   8 +-
 drivers/mtd/nand/raw/nand_samsung.c          |  19 +--
 drivers/mtd/nand/raw/nand_toshiba.c          |  11 +-
 drivers/mtd/nand/raw/sunxi_nand.c            |   5 +-
 drivers/mtd/nand/raw/tegra_nand.c            |   9 +-
 drivers/mtd/nand/spi/core.c                  |   8 +-
 drivers/mtd/nand/spi/macronix.c              |   6 +-
 drivers/mtd/nand/spi/toshiba.c               |   6 +-
 include/linux/mtd/nand.h                     | 112 ++++++++++++++--
 include/linux/mtd/spinand.h                  |   4 +-
 24 files changed, 349 insertions(+), 100 deletions(-)
 create mode 100644 drivers/mtd/nand/ecc/Kconfig
 create mode 100644 drivers/mtd/nand/ecc/Makefile
 create mode 100644 drivers/mtd/nand/ecc/engine.c

diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index e8d26a715922..bc0c26fcb190 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -10,5 +10,6 @@ if MTD_NAND_CORE
 source "drivers/mtd/nand/onenand/Kconfig"
 source "drivers/mtd/nand/raw/Kconfig"
 source "drivers/mtd/nand/spi/Kconfig"
+source "drivers/mtd/nand/ecc/Kconfig"
 
 endif
diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
index 7ecd80c0a66e..9772e781534d 100644
--- a/drivers/mtd/nand/Makefile
+++ b/drivers/mtd/nand/Makefile
@@ -6,3 +6,4 @@ obj-$(CONFIG_MTD_NAND_CORE) += nandcore.o
 obj-y	+= onenand/
 obj-y	+= raw/
 obj-y	+= spi/
+obj-y	+= ecc/
diff --git a/drivers/mtd/nand/ecc/Kconfig b/drivers/mtd/nand/ecc/Kconfig
new file mode 100644
index 000000000000..66c396dcbfbd
--- /dev/null
+++ b/drivers/mtd/nand/ecc/Kconfig
@@ -0,0 +1,3 @@
+menu "ECC engine support"
+
+endmenu
diff --git a/drivers/mtd/nand/ecc/Makefile b/drivers/mtd/nand/ecc/Makefile
new file mode 100644
index 000000000000..6a42577ba424
--- /dev/null
+++ b/drivers/mtd/nand/ecc/Makefile
@@ -0,0 +1,3 @@
+# SPDX-License-Identifier: GPL-2.0
+
+obj-$(CONFIG_MTD_NAND_CORE) += engine.o
diff --git a/drivers/mtd/nand/ecc/engine.c b/drivers/mtd/nand/ecc/engine.c
new file mode 100644
index 000000000000..e3d8bb092e2a
--- /dev/null
+++ b/drivers/mtd/nand/ecc/engine.c
@@ -0,0 +1,134 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Generic Error-Correcting Code (ECC) engine
+ *
+ * Copyright (C) 2019 Macronix
+ * Author:
+ *     Miquèl RAYNAL <miquel.raynal@bootlin.com>
+ *
+ *
+ * This file describes the abstraction of any NAND ECC engine. It has been
+ * designed to fit most cases, including parallel NANDs and SPI-NANDs.
+ *
+ * There are three main situations where instantiating this ECC engine makes
+ * sense:
+ *   - "external": The ECC engine is outside the NAND pipeline, typically this
+ *                 is a software ECC engine. One can also imagine a generic
+ *                 hardware ECC engine which would be an IP itself. Interacting
+ *                 with a SPI-NAND device without on-die ECC could be achieved
+ *                 thanks to the use of such external engine.
+ *   - "pipelined": The ECC engine is inside the NAND pipeline, ie. on the
+ *                  controller's side. This is the case of most of the raw NAND
+ *                  controllers. These controllers usually embed an hardware ECC
+ *                  engine which is managed thanks to the same register set as
+ *                  the controller's.
+ *   - "ondie": The ECC engine is inside the NAND pipeline, on the chip's side.
+ *              Some NAND chips can correct themselves the data. The on-die
+ *              correction can be enabled, disabled and the status of the
+ *              correction after a read may be retrieved with a NAND command
+ *              (may be vendor specific).
+ *
+ * Besides the initial setup and final cleanups, the interfaces are rather
+ * simple:
+ *   - "prepare": Prepare an I/O request, check the ECC engine is enabled or
+ *                disabled as requested before the I/O. In case of software
+ *                correction, this step may involve to derive the ECC bytes and
+ *                place them in the OOB area before a write.
+ *   - "finish": Finish an I/O request, check the status of the operation ie.
+ *               the data validity in case of a read (report to the upper layer
+ *               any bitflip/errors).
+ *
+ * Both prepare/finish callbacks are supposed to enclose I/O request and will
+ * behave differently depending on the desired correction:
+ *   - "raw": Correction disabled
+ *   - "ecc": Correction enabled
+ *
+ * The request direction is impacting the logic as well:
+ *   - "read": Load data from the NAND chip
+ *   - "write": Store data in the NAND chip
+ *
+ * Mixing all this combinations together gives the following behavior.
+ *
+ * ["external" ECC engine]
+ *   - external + prepare + raw + read: do nothing
+ *   - external + finish  + raw + read: do nothing
+ *   - external + prepare + raw + write: do nothing
+ *   - external + finish  + raw + write: do nothing
+ *   - external + prepare + ecc + read: do nothing
+ *   - external + finish  + ecc + read: calculate expected ECC bytes, extract
+ *                                      ECC bytes from OOB buffer, correct
+ *                                      and report any bitflip/error
+ *   - external + prepare + ecc + write: calculate ECC bytes and store them at
+ *                                       the right place in the OOB buffer based
+ *                                       on the OOB layout
+ *   - external + finish  + ecc + write: do nothing
+ *
+ * ["pipelined" ECC engine]
+ *   - pipelined + prepare + raw + read: disable the controller's ECC engine if
+ *                                       activated
+ *   - pipelined + finish  + raw + read: do nothing
+ *   - pipelined + prepare + raw + write: disable the controller's ECC engine if
+ *                                        activated
+ *   - pipelined + finish  + raw + write: do nothing
+ *   - pipelined + prepare + ecc + read: enable the controller's ECC engine if
+ *                                       deactivated
+ *   - pipelined + finish  + ecc + read: check the status, report any
+ *                                       error/bitflip
+ *   - pipelined + prepare + ecc + write: enable the controller's ECC engine if
+ *                                        deactivated
+ *   - pipelined + finish  + ecc + write: do nothing
+ *
+ * ["ondie" ECC engine]
+ *   - ondie + prepare + raw + read: send commands to disable the on-chip ECC
+ *                                   engine if activated
+ *   - ondie + finish  + raw + read: do nothing
+ *   - ondie + prepare + raw + write: send commands to disable the on-chip ECC
+ *                                    engine if activated
+ *   - ondie + finish  + raw + write: do nothing
+ *   - ondie + prepare + ecc + read: send commands to enable the on-chip ECC
+ *                                   engine if deactivated
+ *   - ondie + finish  + ecc + read: send commands to check the status, report
+ *                                   any error/bitflip
+ *   - ondie + prepare + ecc + write: send commands to enable the on-chip ECC
+ *                                    engine if deactivated
+ *   - ondie + finish  + ecc + write: do nothing
+ */
+
+#include <linux/module.h>
+#include <linux/mtd/nand.h>
+
+int nand_ecc_init_ctx(struct nand_device *nand)
+{
+	if (!nand->ecc.engine->ops->init_ctx)
+		return 0;
+
+	return nand->ecc.engine->ops->init_ctx(nand);
+}
+
+void nand_ecc_cleanup_ctx(struct nand_device *nand)
+{
+	if (nand->ecc.engine->ops->cleanup_ctx)
+		nand->ecc.engine->ops->cleanup_ctx(nand);
+}
+
+int nand_ecc_prepare_io_req(struct nand_device *nand,
+			    struct nand_page_io_req *req, void *oobbuf)
+{
+	if (!nand->ecc.engine->ops->prepare_io_req)
+		return 0;
+
+	return nand->ecc.engine->ops->prepare_io_req(nand, req, oobbuf);
+}
+
+int nand_ecc_finish_io_req(struct nand_device *nand,
+			   struct nand_page_io_req *req, void *oobbuf)
+{
+	if (!nand->ecc.engine->ops->finish_io_req)
+		return 0;
+
+	return nand->ecc.engine->ops->finish_io_req(nand, req, oobbuf);
+}
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Miquel Raynal <miquel.raynal@bootlin.com>");
+MODULE_DESCRIPTION("Generic ECC engine");
diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c
index 9867e9115399..3dacaa352a58 100644
--- a/drivers/mtd/nand/raw/atmel/nand-controller.c
+++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
@@ -1039,6 +1039,7 @@ static int atmel_hsmc_nand_pmecc_read_page_raw(struct nand_chip *chip,
 
 static int atmel_nand_pmecc_init(struct nand_chip *chip)
 {
+	struct nand_ecc_conf *requirements = &chip->base.ecc.requirements;
 	struct mtd_info *mtd = nand_to_mtd(chip);
 	struct atmel_nand *nand = to_atmel_nand(chip);
 	struct atmel_nand_controller *nc;
@@ -1068,15 +1069,15 @@ static int atmel_nand_pmecc_init(struct nand_chip *chip)
 		req.ecc.strength = ATMEL_PMECC_MAXIMIZE_ECC_STRENGTH;
 	else if (chip->ecc.strength)
 		req.ecc.strength = chip->ecc.strength;
-	else if (chip->base.eccreq.strength)
-		req.ecc.strength = chip->base.eccreq.strength;
+	else if (requirements->strength)
+		req.ecc.strength = requirements->strength;
 	else
 		req.ecc.strength = ATMEL_PMECC_MAXIMIZE_ECC_STRENGTH;
 
 	if (chip->ecc.size)
 		req.ecc.sectorsize = chip->ecc.size;
-	else if (chip->base.eccreq.step_size)
-		req.ecc.sectorsize = chip->base.eccreq.step_size;
+	else if (requirements->step_size)
+		req.ecc.sectorsize = requirements->step_size;
 	else
 		req.ecc.sectorsize = ATMEL_PMECC_SECTOR_SIZE_AUTO;
 
diff --git a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
index dbefb6bac5c9..1d0f556129de 100644
--- a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
+++ b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
@@ -204,8 +204,8 @@ static int set_geometry_by_ecc_info(struct gpmi_nand_data *this,
 	default:
 		dev_err(this->dev,
 			"unsupported nand chip. ecc bits : %d, ecc size : %d\n",
-			chip->base.eccreq.strength,
-			chip->base.eccreq.step_size);
+			chip->base.ecc.requirements.strength,
+			chip->base.ecc.requirements.step_size);
 		return -EINVAL;
 	}
 	geo->ecc_chunk_size = ecc_step;
@@ -418,13 +418,13 @@ int common_nfc_set_geometry(struct gpmi_nand_data *this)
 
 	if ((of_property_read_bool(this->dev->of_node, "fsl,use-minimum-ecc"))
 				|| legacy_set_geometry(this)) {
-		if (!(chip->base.eccreq.strength > 0 &&
-		      chip->base.eccreq.step_size > 0))
+		if (!(chip->base.ecc.requirements.strength > 0 &&
+		      chip->base.ecc.requirements.step_size > 0))
 			return -EINVAL;
 
 		return set_geometry_by_ecc_info(this,
-						chip->base.eccreq.strength,
-						chip->base.eccreq.step_size);
+						chip->base.ecc.requirements.strength,
+						chip->base.ecc.requirements.step_size);
 	}
 
 	return 0;
diff --git a/drivers/mtd/nand/raw/marvell_nand.c b/drivers/mtd/nand/raw/marvell_nand.c
index d21e808bb075..4d0d3c34ac12 100644
--- a/drivers/mtd/nand/raw/marvell_nand.c
+++ b/drivers/mtd/nand/raw/marvell_nand.c
@@ -2244,13 +2244,14 @@ static int marvell_nand_ecc_init(struct mtd_info *mtd,
 				 struct nand_ecc_ctrl *ecc)
 {
 	struct nand_chip *chip = mtd_to_nand(mtd);
+	struct nand_ecc_conf *requirements = &chip->base.ecc.requirements;
 	struct marvell_nfc *nfc = to_marvell_nfc(chip->controller);
 	int ret;
 
 	if (ecc->mode != NAND_ECC_NONE && (!ecc->size || !ecc->strength)) {
-		if (chip->base.eccreq.step_size && chip->base.eccreq.strength) {
-			ecc->size = chip->base.eccreq.step_size;
-			ecc->strength = chip->base.eccreq.strength;
+		if (requirements->step_size && requirements->strength) {
+			ecc->size = requirements->step_size;
+			ecc->strength = requirements->strength;
 		} else {
 			dev_info(nfc->dev,
 				 "No minimum ECC strength, using 1b/512B\n");
diff --git a/drivers/mtd/nand/raw/mtk_nand.c b/drivers/mtd/nand/raw/mtk_nand.c
index bfb89aca4155..b66eb96b7d49 100644
--- a/drivers/mtd/nand/raw/mtk_nand.c
+++ b/drivers/mtd/nand/raw/mtk_nand.c
@@ -1197,8 +1197,8 @@ static int mtk_nfc_ecc_init(struct device *dev, struct mtd_info *mtd)
 	/* if optional dt settings not present */
 	if (!nand->ecc.size || !nand->ecc.strength) {
 		/* use datasheet requirements */
-		nand->ecc.strength = nand->base.eccreq.strength;
-		nand->ecc.size = nand->base.eccreq.step_size;
+		nand->ecc.strength = nand->base.ecc.requirements.strength;
+		nand->ecc.size = nand->base.ecc.requirements.step_size;
 
 		/*
 		 * align eccstrength and eccsize
diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index 13efa206c7e6..82ac620bbdac 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -4592,8 +4592,8 @@ static bool find_full_id_nand(struct nand_chip *chip,
 					   memorg->pagesize *
 					   memorg->pages_per_eraseblock);
 		chip->options |= type->options;
-		chip->base.eccreq.strength = NAND_ECC_STRENGTH(type);
-		chip->base.eccreq.step_size = NAND_ECC_STEP(type);
+		chip->base.ecc.requirements.strength = NAND_ECC_STRENGTH(type);
+		chip->base.ecc.requirements.step_size = NAND_ECC_STEP(type);
 		chip->onfi_timing_mode_default =
 					type->onfi_timing_mode_default;
 
@@ -5266,8 +5266,8 @@ nand_match_ecc_req(struct nand_chip *chip,
 {
 	struct mtd_info *mtd = nand_to_mtd(chip);
 	const struct nand_ecc_step_info *stepinfo;
-	int req_step = chip->base.eccreq.step_size;
-	int req_strength = chip->base.eccreq.strength;
+	int req_step = chip->base.ecc.requirements.step_size;
+	int req_strength = chip->base.ecc.requirements.strength;
 	int req_corr, step_size, strength, nsteps, ecc_bytes, ecc_bytes_total;
 	int best_step, best_strength, best_ecc_bytes;
 	int best_ecc_bytes_total = INT_MAX;
@@ -5458,9 +5458,10 @@ static bool nand_ecc_strength_good(struct nand_chip *chip)
 {
 	struct mtd_info *mtd = nand_to_mtd(chip);
 	struct nand_ecc_ctrl *ecc = &chip->ecc;
+	struct nand_ecc_conf *requirements = &chip->base.ecc.requirements;
 	int corr, ds_corr;
 
-	if (ecc->size == 0 || chip->base.eccreq.step_size == 0)
+	if (ecc->size == 0 || requirements->step_size == 0)
 		/* Not enough information */
 		return true;
 
@@ -5469,10 +5470,10 @@ static bool nand_ecc_strength_good(struct nand_chip *chip)
 	 * the correction density.
 	 */
 	corr = (mtd->writesize * ecc->strength) / ecc->size;
-	ds_corr = (mtd->writesize * chip->base.eccreq.strength) /
-		  chip->base.eccreq.step_size;
+	ds_corr = (mtd->writesize * requirements->strength) /
+		  requirements->step_size;
 
-	return corr >= ds_corr && ecc->strength >= chip->base.eccreq.strength;
+	return corr >= ds_corr && ecc->strength >= requirements->strength;
 }
 
 static int rawnand_erase(struct nand_device *nand, const struct nand_pos *pos)
diff --git a/drivers/mtd/nand/raw/nand_esmt.c b/drivers/mtd/nand/raw/nand_esmt.c
index 3de5e89482f5..c3fc85f77ff8 100644
--- a/drivers/mtd/nand/raw/nand_esmt.c
+++ b/drivers/mtd/nand/raw/nand_esmt.c
@@ -10,24 +10,25 @@
 
 static void esmt_nand_decode_id(struct nand_chip *chip)
 {
+	struct nand_ecc_conf *requirements = &chip->base.ecc.requirements;
 	nand_decode_ext_id(chip);
 
 	/* Extract ECC requirements from 5th id byte. */
 	if (chip->id.len >= 5 && nand_is_slc(chip)) {
-		chip->base.eccreq.step_size = 512;
+		requirements->step_size = 512;
 		switch (chip->id.data[4] & 0x3) {
 		case 0x0:
-			chip->base.eccreq.strength = 4;
+			requirements->strength = 4;
 			break;
 		case 0x1:
-			chip->base.eccreq.strength = 2;
+			requirements->strength = 2;
 			break;
 		case 0x2:
-			chip->base.eccreq.strength = 1;
+			requirements->strength = 1;
 			break;
 		default:
 			WARN(1, "Could not get ECC info");
-			chip->base.eccreq.step_size = 0;
+			requirements->step_size = 0;
 			break;
 		}
 	}
diff --git a/drivers/mtd/nand/raw/nand_hynix.c b/drivers/mtd/nand/raw/nand_hynix.c
index 821d221b83eb..3c05991e3445 100644
--- a/drivers/mtd/nand/raw/nand_hynix.c
+++ b/drivers/mtd/nand/raw/nand_hynix.c
@@ -504,34 +504,35 @@ static void hynix_nand_extract_oobsize(struct nand_chip *chip,
 static void hynix_nand_extract_ecc_requirements(struct nand_chip *chip,
 						bool valid_jedecid)
 {
+	struct nand_ecc_conf *requirements = &chip->base.ecc.requirements;
 	u8 ecc_level = (chip->id.data[4] >> 4) & 0x7;
 
 	if (valid_jedecid) {
 		/* Reference: H27UCG8T2E datasheet */
-		chip->base.eccreq.step_size = 1024;
+		requirements->step_size = 1024;
 
 		switch (ecc_level) {
 		case 0:
-			chip->base.eccreq.step_size = 0;
-			chip->base.eccreq.strength = 0;
+			requirements->step_size = 0;
+			requirements->strength = 0;
 			break;
 		case 1:
-			chip->base.eccreq.strength = 4;
+			requirements->strength = 4;
 			break;
 		case 2:
-			chip->base.eccreq.strength = 24;
+			requirements->strength = 24;
 			break;
 		case 3:
-			chip->base.eccreq.strength = 32;
+			requirements->strength = 32;
 			break;
 		case 4:
-			chip->base.eccreq.strength = 40;
+			requirements->strength = 40;
 			break;
 		case 5:
-			chip->base.eccreq.strength = 50;
+			requirements->strength = 50;
 			break;
 		case 6:
-			chip->base.eccreq.strength = 60;
+			requirements->strength = 60;
 			break;
 		default:
 			/*
@@ -552,14 +553,14 @@ static void hynix_nand_extract_ecc_requirements(struct nand_chip *chip,
 		if (nand_tech < 3) {
 			/* > 26nm, reference: H27UBG8T2A datasheet */
 			if (ecc_level < 5) {
-				chip->base.eccreq.step_size = 512;
-				chip->base.eccreq.strength = 1 << ecc_level;
+				requirements->step_size = 512;
+				requirements->strength = 1 << ecc_level;
 			} else if (ecc_level < 7) {
 				if (ecc_level == 5)
-					chip->base.eccreq.step_size = 2048;
+					requirements->step_size = 2048;
 				else
-					chip->base.eccreq.step_size = 1024;
-				chip->base.eccreq.strength = 24;
+					requirements->step_size = 1024;
+				requirements->strength = 24;
 			} else {
 				/*
 				 * We should never reach this case, but if that
@@ -572,14 +573,14 @@ static void hynix_nand_extract_ecc_requirements(struct nand_chip *chip,
 		} else {
 			/* <= 26nm, reference: H27UBG8T2B datasheet */
 			if (!ecc_level) {
-				chip->base.eccreq.step_size = 0;
-				chip->base.eccreq.strength = 0;
+				requirements->step_size = 0;
+				requirements->strength = 0;
 			} else if (ecc_level < 5) {
-				chip->base.eccreq.step_size = 512;
-				chip->base.eccreq.strength = 1 << (ecc_level - 1);
+				requirements->step_size = 512;
+				requirements->strength = 1 << (ecc_level - 1);
 			} else {
-				chip->base.eccreq.step_size = 1024;
-				chip->base.eccreq.strength = 24 +
+				requirements->step_size = 1024;
+				requirements->strength = 24 +
 							(8 * (ecc_level - 5));
 			}
 		}
diff --git a/drivers/mtd/nand/raw/nand_jedec.c b/drivers/mtd/nand/raw/nand_jedec.c
index 9b540e76f84f..46afa25abc70 100644
--- a/drivers/mtd/nand/raw/nand_jedec.c
+++ b/drivers/mtd/nand/raw/nand_jedec.c
@@ -110,8 +110,8 @@ int nand_jedec_detect(struct nand_chip *chip)
 	ecc = &p->ecc_info[0];
 
 	if (ecc->codeword_size >= 9) {
-		chip->base.eccreq.strength = ecc->ecc_bits;
-		chip->base.eccreq.step_size = 1 << ecc->codeword_size;
+		chip->base.ecc.requirements.strength = ecc->ecc_bits;
+		chip->base.ecc.requirements.step_size = 1 << ecc->codeword_size;
 	} else {
 		pr_warn("Invalid codeword size\n");
 	}
diff --git a/drivers/mtd/nand/raw/nand_micron.c b/drivers/mtd/nand/raw/nand_micron.c
index 7a2cef02eacd..dd2358a01f54 100644
--- a/drivers/mtd/nand/raw/nand_micron.c
+++ b/drivers/mtd/nand/raw/nand_micron.c
@@ -379,6 +379,7 @@ enum {
  */
 static int micron_supports_on_die_ecc(struct nand_chip *chip)
 {
+	struct nand_ecc_conf *requirements = &chip->base.ecc.requirements;
 	u8 id[5];
 	int ret;
 
@@ -391,7 +392,7 @@ static int micron_supports_on_die_ecc(struct nand_chip *chip)
 	/*
 	 * We only support on-die ECC of 4/512 or 8/512
 	 */
-	if  (chip->base.eccreq.strength != 4 && chip->base.eccreq.strength != 8)
+	if  (requirements->strength != 4 && requirements->strength != 8)
 		return MICRON_ON_DIE_UNSUPPORTED;
 
 	/* 0x2 means on-die ECC is available. */
@@ -424,7 +425,7 @@ static int micron_supports_on_die_ecc(struct nand_chip *chip)
 	/*
 	 * We only support on-die ECC of 4/512 or 8/512
 	 */
-	if  (chip->base.eccreq.strength != 4 && chip->base.eccreq.strength != 8)
+	if  (requirements->strength != 4 && requirements->strength != 8)
 		return MICRON_ON_DIE_UNSUPPORTED;
 
 	return MICRON_ON_DIE_SUPPORTED;
@@ -432,6 +433,7 @@ static int micron_supports_on_die_ecc(struct nand_chip *chip)
 
 static int micron_nand_init(struct nand_chip *chip)
 {
+	struct nand_ecc_conf *requirements = &chip->base.ecc.requirements;
 	struct mtd_info *mtd = nand_to_mtd(chip);
 	struct micron_nand *micron;
 	int ondie;
@@ -479,7 +481,7 @@ static int micron_nand_init(struct nand_chip *chip)
 		 * That's not needed for 8-bit ECC, because the status expose
 		 * a better approximation of the number of bitflips in a page.
 		 */
-		if (chip->base.eccreq.strength == 4) {
+		if (requirements->strength == 4) {
 			micron->ecc.rawbuf = kmalloc(mtd->writesize +
 						     mtd->oobsize,
 						     GFP_KERNEL);
@@ -489,16 +491,16 @@ static int micron_nand_init(struct nand_chip *chip)
 			}
 		}
 
-		if (chip->base.eccreq.strength == 4)
+		if (requirements->strength == 4)
 			mtd_set_ooblayout(mtd,
 					  &micron_nand_on_die_4_ooblayout_ops);
 		else
 			mtd_set_ooblayout(mtd,
 					  &micron_nand_on_die_8_ooblayout_ops);
 
-		chip->ecc.bytes = chip->base.eccreq.strength * 2;
+		chip->ecc.bytes = requirements->strength * 2;
 		chip->ecc.size = 512;
-		chip->ecc.strength = chip->base.eccreq.strength;
+		chip->ecc.strength = requirements->strength;
 		chip->ecc.algo = NAND_ECC_BCH;
 		chip->ecc.read_page = micron_nand_read_page_on_die_ecc;
 		chip->ecc.write_page = micron_nand_write_page_on_die_ecc;
diff --git a/drivers/mtd/nand/raw/nand_onfi.c b/drivers/mtd/nand/raw/nand_onfi.c
index 0b879bd0a68c..9d21b47ebef1 100644
--- a/drivers/mtd/nand/raw/nand_onfi.c
+++ b/drivers/mtd/nand/raw/nand_onfi.c
@@ -94,8 +94,8 @@ static int nand_flash_detect_ext_param_page(struct nand_chip *chip,
 		goto ext_out;
 	}
 
-	chip->base.eccreq.strength = ecc->ecc_bits;
-	chip->base.eccreq.step_size = 1 << ecc->codeword_size;
+	chip->base.ecc.requirements.strength = ecc->ecc_bits;
+	chip->base.ecc.requirements.step_size = 1 << ecc->codeword_size;
 	ret = 0;
 
 ext_out:
@@ -252,8 +252,8 @@ int nand_onfi_detect(struct nand_chip *chip)
 		chip->options |= NAND_BUSWIDTH_16;
 
 	if (p->ecc_bits != 0xff) {
-		chip->base.eccreq.strength = p->ecc_bits;
-		chip->base.eccreq.step_size = 512;
+		chip->base.ecc.requirements.strength = p->ecc_bits;
+		chip->base.ecc.requirements.step_size = 512;
 	} else if (onfi_version >= 21 &&
 		(le16_to_cpu(p->features) & ONFI_FEATURE_EXT_PARAM_PAGE)) {
 
diff --git a/drivers/mtd/nand/raw/nand_samsung.c b/drivers/mtd/nand/raw/nand_samsung.c
index f7d7041b6213..4874ba33db15 100644
--- a/drivers/mtd/nand/raw/nand_samsung.c
+++ b/drivers/mtd/nand/raw/nand_samsung.c
@@ -19,6 +19,7 @@
 
 static void samsung_nand_decode_id(struct nand_chip *chip)
 {
+	struct nand_ecc_conf *requirements = &chip->base.ecc.requirements;
 	struct mtd_info *mtd = nand_to_mtd(chip);
 	struct nand_memory_organization *memorg;
 
@@ -80,23 +81,23 @@ static void samsung_nand_decode_id(struct nand_chip *chip)
 		/* Extract ECC requirements from 5th id byte*/
 		extid = (chip->id.data[4] >> 4) & 0x07;
 		if (extid < 5) {
-			chip->base.eccreq.step_size = 512;
-			chip->base.eccreq.strength = 1 << extid;
+			requirements->step_size = 512;
+			requirements->strength = 1 << extid;
 		} else {
-			chip->base.eccreq.step_size = 1024;
+			requirements->step_size = 1024;
 			switch (extid) {
 			case 5:
-				chip->base.eccreq.strength = 24;
+				requirements->strength = 24;
 				break;
 			case 6:
-				chip->base.eccreq.strength = 40;
+				requirements->strength = 40;
 				break;
 			case 7:
-				chip->base.eccreq.strength = 60;
+				requirements->strength = 60;
 				break;
 			default:
 				WARN(1, "Could not decode ECC info");
-				chip->base.eccreq.step_size = 0;
+				requirements->step_size = 0;
 			}
 		}
 	} else {
@@ -106,8 +107,8 @@ static void samsung_nand_decode_id(struct nand_chip *chip)
 			switch (chip->id.data[1]) {
 			/* K9F4G08U0D-S[I|C]B0(T00) */
 			case 0xDC:
-				chip->base.eccreq.step_size = 512;
-				chip->base.eccreq.strength = 1;
+				requirements->step_size = 512;
+				requirements->strength = 1;
 				break;
 
 			/* K9F1G08U0E 21nm chips do not support subpage write */
diff --git a/drivers/mtd/nand/raw/nand_toshiba.c b/drivers/mtd/nand/raw/nand_toshiba.c
index 13f9632f1cb4..f00d958d724f 100644
--- a/drivers/mtd/nand/raw/nand_toshiba.c
+++ b/drivers/mtd/nand/raw/nand_toshiba.c
@@ -100,6 +100,7 @@ static void toshiba_nand_benand_init(struct nand_chip *chip)
 
 static void toshiba_nand_decode_id(struct nand_chip *chip)
 {
+	struct nand_ecc_conf *requirements = &chip->base.ecc.requirements;
 	struct mtd_info *mtd = nand_to_mtd(chip);
 	struct nand_memory_organization *memorg;
 
@@ -130,20 +131,20 @@ static void toshiba_nand_decode_id(struct nand_chip *chip)
 	 *  - 24nm: 8 bit ECC for each 512Byte is required.
 	 */
 	if (chip->id.len >= 6 && nand_is_slc(chip)) {
-		chip->base.eccreq.step_size = 512;
+		requirements->step_size = 512;
 		switch (chip->id.data[5] & 0x7) {
 		case 0x4:
-			chip->base.eccreq.strength = 1;
+			requirements->strength = 1;
 			break;
 		case 0x5:
-			chip->base.eccreq.strength = 4;
+			requirements->strength = 4;
 			break;
 		case 0x6:
-			chip->base.eccreq.strength = 8;
+			requirements->strength = 8;
 			break;
 		default:
 			WARN(1, "Could not get ECC info");
-			chip->base.eccreq.step_size = 0;
+			requirements->step_size = 0;
 			break;
 		}
 	}
diff --git a/drivers/mtd/nand/raw/sunxi_nand.c b/drivers/mtd/nand/raw/sunxi_nand.c
index 69599854fdd6..43b22564da10 100644
--- a/drivers/mtd/nand/raw/sunxi_nand.c
+++ b/drivers/mtd/nand/raw/sunxi_nand.c
@@ -1807,6 +1807,7 @@ static void sunxi_nand_ecc_cleanup(struct nand_ecc_ctrl *ecc)
 
 static int sunxi_nand_attach_chip(struct nand_chip *nand)
 {
+	struct nand_ecc_conf *requirements = &nand->base.ecc.requirements;
 	struct mtd_info *mtd = nand_to_mtd(nand);
 	struct nand_ecc_ctrl *ecc = &nand->ecc;
 	struct device_node *np = nand_get_flash_node(nand);
@@ -1821,8 +1822,8 @@ static int sunxi_nand_attach_chip(struct nand_chip *nand)
 	nand->options |= NAND_SUBPAGE_READ;
 
 	if (!ecc->size) {
-		ecc->size = nand->base.eccreq.step_size;
-		ecc->strength = nand->base.eccreq.strength;
+		ecc->size = requirements->step_size;
+		ecc->strength = requirements->strength;
 	}
 
 	if (!ecc->size || !ecc->strength)
diff --git a/drivers/mtd/nand/raw/tegra_nand.c b/drivers/mtd/nand/raw/tegra_nand.c
index 3cc9a4c41443..31d547d96795 100644
--- a/drivers/mtd/nand/raw/tegra_nand.c
+++ b/drivers/mtd/nand/raw/tegra_nand.c
@@ -853,7 +853,7 @@ static int tegra_nand_get_strength(struct nand_chip *chip, const int *strength,
 		} else {
 			strength_sel = strength[i];
 
-			if (strength_sel < chip->base.eccreq.strength)
+			if (strength_sel < chip->base.ecc.requirements.strength)
 				continue;
 		}
 
@@ -906,6 +906,7 @@ static int tegra_nand_select_strength(struct nand_chip *chip, int oobsize)
 static int tegra_nand_attach_chip(struct nand_chip *chip)
 {
 	struct tegra_nand_controller *ctrl = to_tegra_ctrl(chip->controller);
+	struct nand_ecc_conf *requirements = &chip->base.ecc.requirements;
 	struct tegra_nand_chip *nand = to_tegra_chip(chip);
 	struct mtd_info *mtd = nand_to_mtd(chip);
 	int bits_per_step;
@@ -917,9 +918,9 @@ static int tegra_nand_attach_chip(struct nand_chip *chip)
 	chip->ecc.mode = NAND_ECC_HW;
 	chip->ecc.size = 512;
 	chip->ecc.steps = mtd->writesize / chip->ecc.size;
-	if (chip->base.eccreq.step_size != 512) {
+	if (requirements->step_size != 512) {
 		dev_err(ctrl->dev, "Unsupported step size %d\n",
-			chip->base.eccreq.step_size);
+			requirements->step_size);
 		return -EINVAL;
 	}
 
@@ -950,7 +951,7 @@ static int tegra_nand_attach_chip(struct nand_chip *chip)
 		if (ret < 0) {
 			dev_err(ctrl->dev,
 				"No valid strength found, minimum %d\n",
-				chip->base.eccreq.strength);
+				requirements->strength);
 			return ret;
 		}
 
diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
index ed5e340dff51..ab41b9434d87 100644
--- a/drivers/mtd/nand/spi/core.c
+++ b/drivers/mtd/nand/spi/core.c
@@ -480,7 +480,7 @@ static int spinand_check_ecc_status(struct spinand_device *spinand, u8 status)
 		 * fixed, so let's return the maximum possible value so that
 		 * wear-leveling layers move the data immediately.
 		 */
-		return nand->eccreq.strength;
+		return nand->ecc.ctx.conf.strength;
 
 	case STATUS_ECC_UNCOR_ERROR:
 		return -EBADMSG;
@@ -558,7 +558,7 @@ static int spinand_mtd_read(struct mtd_info *mtd, loff_t from,
 
 	mutex_lock(&spinand->lock);
 
-	nanddev_io_for_each_page(nand, from, ops, &iter) {
+	nanddev_io_for_each_page(nand, NAND_PAGE_READ, from, ops, &iter) {
 		ret = spinand_select_target(spinand, iter.req.pos.target);
 		if (ret)
 			break;
@@ -606,7 +606,7 @@ static int spinand_mtd_write(struct mtd_info *mtd, loff_t to,
 
 	mutex_lock(&spinand->lock);
 
-	nanddev_io_for_each_page(nand, to, ops, &iter) {
+	nanddev_io_for_each_page(nand, NAND_PAGE_WRITE, to, ops, &iter) {
 		ret = spinand_select_target(spinand, iter.req.pos.target);
 		if (ret)
 			break;
@@ -869,7 +869,7 @@ int spinand_match_and_init(struct spinand_device *spinand,
 			continue;
 
 		nand->memorg = table[i].memorg;
-		nand->eccreq = table[i].eccreq;
+		nand->ecc.requirements = table[i].eccreq;
 		spinand->eccinfo = table[i].eccinfo;
 		spinand->flags = table[i].flags;
 		spinand->select_target = table[i].select_target;
diff --git a/drivers/mtd/nand/spi/macronix.c b/drivers/mtd/nand/spi/macronix.c
index c6300d9d63f9..ef71312966a2 100644
--- a/drivers/mtd/nand/spi/macronix.c
+++ b/drivers/mtd/nand/spi/macronix.c
@@ -78,10 +78,10 @@ static int mx35lf1ge4ab_ecc_get_status(struct spinand_device *spinand,
 		 * data around if it's not necessary.
 		 */
 		if (mx35lf1ge4ab_get_eccsr(spinand, &eccsr))
-			return nand->eccreq.strength;
+			return nand->ecc.ctx.conf.strength;
 
-		if (WARN_ON(eccsr > nand->eccreq.strength || !eccsr))
-			return nand->eccreq.strength;
+		if (WARN_ON(eccsr > nand->ecc.ctx.conf.strength || !eccsr))
+			return nand->ecc.ctx.conf.strength;
 
 		return eccsr;
 
diff --git a/drivers/mtd/nand/spi/toshiba.c b/drivers/mtd/nand/spi/toshiba.c
index 00ddab08e6c6..4ac336d3a304 100644
--- a/drivers/mtd/nand/spi/toshiba.c
+++ b/drivers/mtd/nand/spi/toshiba.c
@@ -77,12 +77,12 @@ static int tc58cvg2s0h_ecc_get_status(struct spinand_device *spinand,
 		 * data around if it's not necessary.
 		 */
 		if (spi_mem_exec_op(spinand->spimem, &op))
-			return nand->eccreq.strength;
+			return nand->ecc.ctx.conf.strength;
 
 		mbf >>= 4;
 
-		if (WARN_ON(mbf > nand->eccreq.strength || !mbf))
-			return nand->eccreq.strength;
+		if (WARN_ON(mbf > nand->ecc.ctx.conf.strength || !mbf))
+			return nand->ecc.ctx.conf.strength;
 
 		return mbf;
 
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index 30f0fb02abe2..9e3b018d0b83 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -82,8 +82,14 @@ struct nand_pos {
 	unsigned int page;
 };
 
+enum nand_page_io_req_type {
+	NAND_PAGE_READ = 0,
+	NAND_PAGE_WRITE,
+};
+
 /**
  * struct nand_page_io_req - NAND I/O request object
+ * @type: the type of page I/O: read or write
  * @pos: the position this I/O request is targeting
  * @dataoffs: the offset within the page
  * @datalen: number of data bytes to read from/write to this page
@@ -99,6 +105,7 @@ struct nand_pos {
  * specific commands/operations.
  */
 struct nand_page_io_req {
+	enum nand_page_io_req_type type;
 	struct nand_pos pos;
 	unsigned int dataoffs;
 	unsigned int datalen;
@@ -116,13 +123,35 @@ struct nand_page_io_req {
 };
 
 /**
- * struct nand_ecc_req - NAND ECC requirements
+ * struct nand_ecc_conf - NAND ECC configuration
+ * @strength: ECC strength
+ * @step_size: Number of bytes per step
+ * @total: Total number of bytes used for storing ECC codes, this is used by
+ *         generic OOB layouts
+ */
+struct nand_ecc_conf {
+	unsigned int strength;
+	unsigned int step_size;
+	unsigned int total;
+};
+
+/**
+ * struct nand_ecc_user_conf - User desired ECC configuration
+ * @mode: ECC mode
+ * @algo: ECC algorithm
  * @strength: ECC strength
  * @step_size: ECC step/block size
+ * @maximize: ECC parameters must be maximized depending on the device
+ *            capabilities
+ * @flags: User flags
  */
-struct nand_ecc_req {
+struct nand_ecc_user_conf {
+	int mode;
+	unsigned int algo;
 	unsigned int strength;
 	unsigned int step_size;
+	unsigned int maximize;
+	unsigned int flags;
 };
 
 #define NAND_ECCREQ(str, stp) { .strength = (str), .step_size = (stp) }
@@ -157,11 +186,76 @@ struct nand_ops {
 	bool (*isbad)(struct nand_device *nand, const struct nand_pos *pos);
 };
 
+/**
+ * struct nand_ecc_context - Context for the ECC engine
+ *
+ * @conf: basic ECC engine parameters
+ * @priv: ECC engine driver private data
+ */
+struct nand_ecc_context {
+	struct nand_ecc_conf conf;
+	void *priv;
+};
+
+/**
+ * struct nand_ecc_engine_ops - Generic ECC engine operations
+ *
+ * @init_ctx: given a desired user configuration for the pointed NAND device,
+ *            requests the ECC engine driver to setup a configuration with
+ *            values it supports.
+ * @cleanup_ctx: clean the context initialized by @init_ctx.
+ * @prepare_io_req: is called before reading/writing a page to prepare the I/O
+ *                  request to be performed with ECC correction.
+ * @finish_io_req: is called after reading/writing a page to terminate the I/O
+ *                 request and ensure proper ECC correction.
+ */
+struct nand_ecc_engine_ops {
+	int (*init_ctx)(struct nand_device *nand);
+	void (*cleanup_ctx)(struct nand_device *nand);
+	int (*prepare_io_req)(struct nand_device *nand,
+			      struct nand_page_io_req *req,
+			      void *oobbuf);
+	int (*finish_io_req)(struct nand_device *nand,
+			     struct nand_page_io_req *req,
+			     void *oobbuf);
+};
+
+/**
+ * struct nand_ecc_engine - Generic ECC engine abstraction for NAND devices
+ *
+ * @ops: ECC engine operations
+ */
+struct nand_ecc_engine {
+	struct nand_ecc_engine_ops *ops;
+};
+
+int nand_ecc_init_ctx(struct nand_device *nand);
+void nand_ecc_cleanup_ctx(struct nand_device *nand);
+int nand_ecc_prepare_io_req(struct nand_device *nand,
+			    struct nand_page_io_req *req, void *oobbuf);
+int nand_ecc_finish_io_req(struct nand_device *nand,
+			   struct nand_page_io_req *req, void *oobbuf);
+
+/**
+ * struct nand_ecc - High-level ECC object
+ *
+ * @requirements: ECC requirements from the NAND chip perspective
+ * @user_conf: user desires in terms of ECC parameters
+ * @ctx: ECC context for the ECC engine, derived from the device @requirements
+ *       and @user_conf
+ * @engine: ECC engine
+ */
+struct nand_ecc {
+	struct nand_ecc_conf requirements;
+	struct nand_ecc_user_conf user_conf;
+	struct nand_ecc_context ctx;
+	struct nand_ecc_engine *engine;
+};
+
 /**
  * struct nand_device - NAND device
  * @mtd: MTD instance attached to the NAND device
  * @memorg: memory layout
- * @eccreq: ECC requirements
  * @rowconv: position to row address converter
  * @bbt: bad block table info
  * @ops: NAND operations attached to the NAND device
@@ -169,8 +263,8 @@ struct nand_ops {
  * Generic NAND object. Specialized NAND layers (raw NAND, SPI NAND, OneNAND)
  * should declare their own NAND object embedding a nand_device struct (that's
  * how inheritance is done).
- * struct_nand_device->memorg and struct_nand_device->eccreq should be filled
- * at device detection time to reflect the NAND device
+ * struct_nand_device->memorg and struct_nand_device->ecc.ctx.conf should
+ * be filled at device detection time to reflect the NAND device
  * capabilities/requirements. Once this is done nanddev_init() can be called.
  * It will take care of converting NAND information into MTD ones, which means
  * the specialized NAND layers should never manually tweak
@@ -179,7 +273,7 @@ struct nand_ops {
 struct nand_device {
 	struct mtd_info mtd;
 	struct nand_memory_organization memorg;
-	struct nand_ecc_req eccreq;
+	struct nand_ecc ecc;
 	struct nand_row_converter rowconv;
 	struct nand_bbt bbt;
 	const struct nand_ops *ops;
@@ -624,11 +718,13 @@ static inline void nanddev_pos_next_page(struct nand_device *nand,
  * layer.
  */
 static inline void nanddev_io_iter_init(struct nand_device *nand,
+					enum nand_page_io_req_type reqtype,
 					loff_t offs, struct mtd_oob_ops *req,
 					struct nand_io_iter *iter)
 {
 	struct mtd_info *mtd = nanddev_to_mtd(nand);
 
+	iter->req.type = reqtype;
 	iter->req.mode = req->mode;
 	iter->req.dataoffs = nanddev_offs_to_pos(nand, offs, &iter->req.pos);
 	iter->req.ooboffs = req->ooboffs;
@@ -698,8 +794,8 @@ static inline bool nanddev_io_iter_end(struct nand_device *nand,
  *
  * Should be used for iterate over pages that are contained in an MTD request.
  */
-#define nanddev_io_for_each_page(nand, start, req, iter)		\
-	for (nanddev_io_iter_init(nand, start, req, iter);		\
+#define nanddev_io_for_each_page(nand, type, start, req, iter)		\
+	for (nanddev_io_iter_init(nand, type, start, req, iter);	\
 	     !nanddev_io_iter_end(nand, iter);				\
 	     nanddev_io_iter_next_page(nand, iter))
 
diff --git a/include/linux/mtd/spinand.h b/include/linux/mtd/spinand.h
index b92e2aa955b6..75a28dc79a9c 100644
--- a/include/linux/mtd/spinand.h
+++ b/include/linux/mtd/spinand.h
@@ -168,7 +168,7 @@ struct spinand_id {
  *	    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
+ *	    spinand->base.ecc.ctx.conf) have been filled
  * @init: initialize a SPI NAND device
  * @cleanup: cleanup a SPI NAND device
  *
@@ -263,7 +263,7 @@ struct spinand_info {
 	u8 devid;
 	u32 flags;
 	struct nand_memory_organization memorg;
-	struct nand_ecc_req eccreq;
+	struct nand_ecc_conf eccreq;
 	struct spinand_ecc_info eccinfo;
 	struct {
 		const struct spinand_op_variants *read_cache;
-- 
2.19.1


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

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

* [RFC PATCH 04/27] mtd: Fix typo in mtd_ooblayout_set_databytes() description
  2019-02-21 10:01 [RFC PATCH 00/27] Introduce the generic ECC engine abstraction Miquel Raynal
                   ` (2 preceding siblings ...)
  2019-02-21 10:01 ` [RFC PATCH 03/27] mtd: nand: Introduce the ECC engine abstraction Miquel Raynal
@ 2019-02-21 10:01 ` Miquel Raynal
  2019-02-21 10:01 ` [RFC PATCH 05/27] mtd: nand: Move standard OOB layouts to the NAND core Miquel Raynal
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 36+ messages in thread
From: Miquel Raynal @ 2019-02-21 10:01 UTC (permalink / raw)
  To: Boris Brezillon, Richard Weinberger, David Woodhouse,
	Brian Norris, Marek Vasut, Tudor Ambarus
  Cc: Vignesh R, Tudor Ambarus, Julien Su, Schrempf Frieder, linux-mtd,
	Thomas Petazzoni, Miquel Raynal, Mason Yang, linux-arm-kernel

Fix a probable copy/paste error: the function works like
mtd_ooblayout_set_bytes(), not the *_get_bytes() alternate.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/mtdcore.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index 21e3cdc04036..6ed48018163c 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -1592,7 +1592,7 @@ EXPORT_SYMBOL_GPL(mtd_ooblayout_get_databytes);
  * @start: first ECC byte to set
  * @nbytes: number of ECC bytes to set
  *
- * Works like mtd_ooblayout_get_bytes(), except it acts on free bytes.
+ * Works like mtd_ooblayout_set_bytes(), except it acts on free bytes.
  *
  * Returns zero on success, a negative error code otherwise.
  */
-- 
2.19.1


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

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

* [RFC PATCH 05/27] mtd: nand: Move standard OOB layouts to the NAND core
  2019-02-21 10:01 [RFC PATCH 00/27] Introduce the generic ECC engine abstraction Miquel Raynal
                   ` (3 preceding siblings ...)
  2019-02-21 10:01 ` [RFC PATCH 04/27] mtd: Fix typo in mtd_ooblayout_set_databytes() description Miquel Raynal
@ 2019-02-21 10:01 ` Miquel Raynal
  2019-02-21 11:19   ` Boris Brezillon
  2019-02-21 10:01 ` [RFC PATCH 06/27] mtd: nand: Move ECC specific functions to ecc/engine.c Miquel Raynal
                   ` (6 subsequent siblings)
  11 siblings, 1 reply; 36+ messages in thread
From: Miquel Raynal @ 2019-02-21 10:01 UTC (permalink / raw)
  To: Boris Brezillon, Richard Weinberger, David Woodhouse,
	Brian Norris, Marek Vasut, Tudor Ambarus
  Cc: Vignesh R, Tudor Ambarus, Julien Su, Schrempf Frieder, linux-mtd,
	Thomas Petazzoni, Miquel Raynal, Mason Yang, linux-arm-kernel

These OOB layouts are generic to all NAND chips, the should not be
restricted to be used only by raw NAND controller drivers. They might
later be used by generic ECC engines and SPI-NAND devices as well.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/core.c             | 161 ++++++++++++++++++++++++++++
 drivers/mtd/nand/raw/nand_base.c    | 161 +---------------------------
 drivers/mtd/nand/raw/nand_toshiba.c |   2 +
 include/linux/mtd/nand.h            |   4 +
 include/linux/mtd/rawnand.h         |   3 -
 5 files changed, 168 insertions(+), 163 deletions(-)

diff --git a/drivers/mtd/nand/core.c b/drivers/mtd/nand/core.c
index 0a2be5e6d669..a76d206d233a 100644
--- a/drivers/mtd/nand/core.c
+++ b/drivers/mtd/nand/core.c
@@ -270,6 +270,167 @@ void nanddev_cleanup(struct nand_device *nand)
 }
 EXPORT_SYMBOL_GPL(nanddev_cleanup);
 
+/* Define default oob placement schemes for large and small page devices */
+static int nand_ooblayout_ecc_sp(struct mtd_info *mtd, int section,
+				 struct mtd_oob_region *oobregion)
+{
+	struct nand_device *nand = mtd_to_nanddev(mtd);
+	unsigned int total_ecc_bytes = nand->ecc.ctx.conf.total;
+
+	if (section > 1)
+		return -ERANGE;
+
+	if (!section) {
+		oobregion->offset = 0;
+		if (mtd->oobsize == 16)
+			oobregion->length = 4;
+		else
+			oobregion->length = 3;
+	} else {
+		if (mtd->oobsize == 8)
+			return -ERANGE;
+
+		oobregion->offset = 6;
+		oobregion->length = total_ecc_bytes - 4;
+	}
+
+	return 0;
+}
+
+static int nand_ooblayout_free_sp(struct mtd_info *mtd, int section,
+				  struct mtd_oob_region *oobregion)
+{
+	if (section > 1)
+		return -ERANGE;
+
+	if (mtd->oobsize == 16) {
+		if (section)
+			return -ERANGE;
+
+		oobregion->length = 8;
+		oobregion->offset = 8;
+	} else {
+		oobregion->length = 2;
+		if (!section)
+			oobregion->offset = 3;
+		else
+			oobregion->offset = 6;
+	}
+
+	return 0;
+}
+
+const struct mtd_ooblayout_ops nand_ooblayout_sp_ops = {
+	.ecc = nand_ooblayout_ecc_sp,
+	.free = nand_ooblayout_free_sp,
+};
+EXPORT_SYMBOL_GPL(nand_ooblayout_sp_ops);
+
+static int nand_ooblayout_ecc_lp(struct mtd_info *mtd, int section,
+				 struct mtd_oob_region *oobregion)
+{
+	struct nand_device *nand = mtd_to_nanddev(mtd);
+	unsigned int total_ecc_bytes = nand->ecc.ctx.conf.total;
+
+	if (section || !total_ecc_bytes)
+		return -ERANGE;
+
+	oobregion->length = total_ecc_bytes;
+	oobregion->offset = mtd->oobsize - oobregion->length;
+
+	return 0;
+}
+
+static int nand_ooblayout_free_lp(struct mtd_info *mtd, int section,
+				  struct mtd_oob_region *oobregion)
+{
+	struct nand_device *nand = mtd_to_nanddev(mtd);
+	unsigned int total_ecc_bytes = nand->ecc.ctx.conf.total;
+
+	if (section)
+		return -ERANGE;
+
+	oobregion->length = mtd->oobsize - total_ecc_bytes - 2;
+	oobregion->offset = 2;
+
+	return 0;
+}
+
+const struct mtd_ooblayout_ops nand_ooblayout_lp_ops = {
+	.ecc = nand_ooblayout_ecc_lp,
+	.free = nand_ooblayout_free_lp,
+};
+EXPORT_SYMBOL_GPL(nand_ooblayout_lp_ops);
+
+/*
+ * Support the old "large page" layout used for 1-bit Hamming ECC where ECC
+ * are placed at a fixed offset.
+ */
+static int nand_ooblayout_ecc_lp_hamming(struct mtd_info *mtd, int section,
+					 struct mtd_oob_region *oobregion)
+{
+	struct nand_device *nand = mtd_to_nanddev(mtd);
+	unsigned int total_ecc_bytes = nand->ecc.ctx.conf.total;
+
+	if (section)
+		return -ERANGE;
+
+	switch (mtd->oobsize) {
+	case 64:
+		oobregion->offset = 40;
+		break;
+	case 128:
+		oobregion->offset = 80;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	oobregion->length = total_ecc_bytes;
+	if (oobregion->offset + oobregion->length > mtd->oobsize)
+		return -ERANGE;
+
+	return 0;
+}
+
+static int nand_ooblayout_free_lp_hamming(struct mtd_info *mtd, int section,
+					  struct mtd_oob_region *oobregion)
+{
+	struct nand_device *nand = mtd_to_nanddev(mtd);
+	unsigned int total_ecc_bytes = nand->ecc.ctx.conf.total;
+	int ecc_offset = 0;
+
+	if (section < 0 || section > 1)
+		return -ERANGE;
+
+	switch (mtd->oobsize) {
+	case 64:
+		ecc_offset = 40;
+		break;
+	case 128:
+		ecc_offset = 80;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	if (section == 0) {
+		oobregion->offset = 2;
+		oobregion->length = ecc_offset - 2;
+	} else {
+		oobregion->offset = ecc_offset + total_ecc_bytes;
+		oobregion->length = mtd->oobsize - oobregion->offset;
+	}
+
+	return 0;
+}
+
+const struct mtd_ooblayout_ops nand_ooblayout_lp_hamming_ops = {
+	.ecc = nand_ooblayout_ecc_lp_hamming,
+	.free = nand_ooblayout_free_lp_hamming,
+};
+EXPORT_SYMBOL_GPL(nand_ooblayout_lp_hamming_ops);
+
 MODULE_DESCRIPTION("Generic NAND framework");
 MODULE_AUTHOR("Boris Brezillon <boris.brezillon@free-electrons.com>");
 MODULE_LICENSE("GPL v2");
diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index 82ac620bbdac..0851ac63e70d 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -38,6 +38,7 @@
 #include <linux/mm.h>
 #include <linux/types.h>
 #include <linux/mtd/mtd.h>
+#include <linux/mtd/nand.h>
 #include <linux/mtd/nand_ecc.h>
 #include <linux/mtd/nand_bch.h>
 #include <linux/interrupt.h>
@@ -49,166 +50,6 @@
 
 #include "internals.h"
 
-/* Define default oob placement schemes for large and small page devices */
-static int nand_ooblayout_ecc_sp(struct mtd_info *mtd, int section,
-				 struct mtd_oob_region *oobregion)
-{
-	struct nand_chip *chip = mtd_to_nand(mtd);
-	struct nand_ecc_ctrl *ecc = &chip->ecc;
-
-	if (section > 1)
-		return -ERANGE;
-
-	if (!section) {
-		oobregion->offset = 0;
-		if (mtd->oobsize == 16)
-			oobregion->length = 4;
-		else
-			oobregion->length = 3;
-	} else {
-		if (mtd->oobsize == 8)
-			return -ERANGE;
-
-		oobregion->offset = 6;
-		oobregion->length = ecc->total - 4;
-	}
-
-	return 0;
-}
-
-static int nand_ooblayout_free_sp(struct mtd_info *mtd, int section,
-				  struct mtd_oob_region *oobregion)
-{
-	if (section > 1)
-		return -ERANGE;
-
-	if (mtd->oobsize == 16) {
-		if (section)
-			return -ERANGE;
-
-		oobregion->length = 8;
-		oobregion->offset = 8;
-	} else {
-		oobregion->length = 2;
-		if (!section)
-			oobregion->offset = 3;
-		else
-			oobregion->offset = 6;
-	}
-
-	return 0;
-}
-
-const struct mtd_ooblayout_ops nand_ooblayout_sp_ops = {
-	.ecc = nand_ooblayout_ecc_sp,
-	.free = nand_ooblayout_free_sp,
-};
-EXPORT_SYMBOL_GPL(nand_ooblayout_sp_ops);
-
-static int nand_ooblayout_ecc_lp(struct mtd_info *mtd, int section,
-				 struct mtd_oob_region *oobregion)
-{
-	struct nand_chip *chip = mtd_to_nand(mtd);
-	struct nand_ecc_ctrl *ecc = &chip->ecc;
-
-	if (section || !ecc->total)
-		return -ERANGE;
-
-	oobregion->length = ecc->total;
-	oobregion->offset = mtd->oobsize - oobregion->length;
-
-	return 0;
-}
-
-static int nand_ooblayout_free_lp(struct mtd_info *mtd, int section,
-				  struct mtd_oob_region *oobregion)
-{
-	struct nand_chip *chip = mtd_to_nand(mtd);
-	struct nand_ecc_ctrl *ecc = &chip->ecc;
-
-	if (section)
-		return -ERANGE;
-
-	oobregion->length = mtd->oobsize - ecc->total - 2;
-	oobregion->offset = 2;
-
-	return 0;
-}
-
-const struct mtd_ooblayout_ops nand_ooblayout_lp_ops = {
-	.ecc = nand_ooblayout_ecc_lp,
-	.free = nand_ooblayout_free_lp,
-};
-EXPORT_SYMBOL_GPL(nand_ooblayout_lp_ops);
-
-/*
- * Support the old "large page" layout used for 1-bit Hamming ECC where ECC
- * are placed at a fixed offset.
- */
-static int nand_ooblayout_ecc_lp_hamming(struct mtd_info *mtd, int section,
-					 struct mtd_oob_region *oobregion)
-{
-	struct nand_chip *chip = mtd_to_nand(mtd);
-	struct nand_ecc_ctrl *ecc = &chip->ecc;
-
-	if (section)
-		return -ERANGE;
-
-	switch (mtd->oobsize) {
-	case 64:
-		oobregion->offset = 40;
-		break;
-	case 128:
-		oobregion->offset = 80;
-		break;
-	default:
-		return -EINVAL;
-	}
-
-	oobregion->length = ecc->total;
-	if (oobregion->offset + oobregion->length > mtd->oobsize)
-		return -ERANGE;
-
-	return 0;
-}
-
-static int nand_ooblayout_free_lp_hamming(struct mtd_info *mtd, int section,
-					  struct mtd_oob_region *oobregion)
-{
-	struct nand_chip *chip = mtd_to_nand(mtd);
-	struct nand_ecc_ctrl *ecc = &chip->ecc;
-	int ecc_offset = 0;
-
-	if (section < 0 || section > 1)
-		return -ERANGE;
-
-	switch (mtd->oobsize) {
-	case 64:
-		ecc_offset = 40;
-		break;
-	case 128:
-		ecc_offset = 80;
-		break;
-	default:
-		return -EINVAL;
-	}
-
-	if (section == 0) {
-		oobregion->offset = 2;
-		oobregion->length = ecc_offset - 2;
-	} else {
-		oobregion->offset = ecc_offset + ecc->total;
-		oobregion->length = mtd->oobsize - oobregion->offset;
-	}
-
-	return 0;
-}
-
-static const struct mtd_ooblayout_ops nand_ooblayout_lp_hamming_ops = {
-	.ecc = nand_ooblayout_ecc_lp_hamming,
-	.free = nand_ooblayout_free_lp_hamming,
-};
-
 static int check_offs_len(struct nand_chip *chip, loff_t ofs, uint64_t len)
 {
 	int ret = 0;
diff --git a/drivers/mtd/nand/raw/nand_toshiba.c b/drivers/mtd/nand/raw/nand_toshiba.c
index f00d958d724f..566cc1a11679 100644
--- a/drivers/mtd/nand/raw/nand_toshiba.c
+++ b/drivers/mtd/nand/raw/nand_toshiba.c
@@ -15,6 +15,8 @@
  * GNU General Public License for more details.
  */
 
+#include <linux/mtd/nand.h>
+
 #include "internals.h"
 
 /* Bit for detecting BENAND */
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index 9e3b018d0b83..da214aaa0b8a 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -122,6 +122,10 @@ struct nand_page_io_req {
 	int mode;
 };
 
+extern const struct mtd_ooblayout_ops nand_ooblayout_sp_ops;
+extern const struct mtd_ooblayout_ops nand_ooblayout_lp_ops;
+extern const struct mtd_ooblayout_ops nand_ooblayout_lp_hamming_ops;
+
 /**
  * struct nand_ecc_conf - NAND ECC configuration
  * @strength: ECC strength
diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
index 14748183508b..0059f5952187 100644
--- a/include/linux/mtd/rawnand.h
+++ b/include/linux/mtd/rawnand.h
@@ -1098,9 +1098,6 @@ struct nand_chip {
 	} manufacturer;
 };
 
-extern const struct mtd_ooblayout_ops nand_ooblayout_sp_ops;
-extern const struct mtd_ooblayout_ops nand_ooblayout_lp_ops;
-
 static inline struct nand_chip *mtd_to_nand(struct mtd_info *mtd)
 {
 	return container_of(mtd, struct nand_chip, base.mtd);
-- 
2.19.1


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

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

* [RFC PATCH 06/27] mtd: nand: Move ECC specific functions to ecc/engine.c
  2019-02-21 10:01 [RFC PATCH 00/27] Introduce the generic ECC engine abstraction Miquel Raynal
                   ` (4 preceding siblings ...)
  2019-02-21 10:01 ` [RFC PATCH 05/27] mtd: nand: Move standard OOB layouts to the NAND core Miquel Raynal
@ 2019-02-21 10:01 ` Miquel Raynal
  2019-02-21 10:01 ` [RFC PATCH 07/27] mtd: nand: ecc: Move BCH code into the ecc/ directory Miquel Raynal
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 36+ messages in thread
From: Miquel Raynal @ 2019-02-21 10:01 UTC (permalink / raw)
  To: Boris Brezillon, Richard Weinberger, David Woodhouse,
	Brian Norris, Marek Vasut, Tudor Ambarus
  Cc: Vignesh R, Tudor Ambarus, Julien Su, Schrempf Frieder, linux-mtd,
	Thomas Petazzoni, Miquel Raynal, Mason Yang, linux-arm-kernel

Move generic code that will benefit to be shared out of the raw NAND
subsystem.

While at it, remove the maximize option and turn it into a user
desire.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/ecc/engine.c                | 157 +++++++++++++++++
 drivers/mtd/nand/raw/atmel/nand-controller.c |   3 +-
 drivers/mtd/nand/raw/denali.c                |   3 +
 drivers/mtd/nand/raw/denali_pci.c            |   1 -
 drivers/mtd/nand/raw/nand_base.c             | 168 +++----------------
 drivers/mtd/nand/raw/sunxi_nand.c            |   3 +-
 drivers/mtd/nand/raw/tegra_nand.c            |   3 +-
 include/linux/mtd/nand.h                     |  32 ++++
 include/linux/mtd/rawnand.h                  |  21 +--
 9 files changed, 220 insertions(+), 171 deletions(-)

diff --git a/drivers/mtd/nand/ecc/engine.c b/drivers/mtd/nand/ecc/engine.c
index e3d8bb092e2a..7dd3f9772698 100644
--- a/drivers/mtd/nand/ecc/engine.c
+++ b/drivers/mtd/nand/ecc/engine.c
@@ -129,6 +129,163 @@ int nand_ecc_finish_io_req(struct nand_device *nand,
 	return nand->ecc.engine->ops->finish_io_req(nand, req, oobbuf);
 }
 
+static const char * const nand_ecc_modes[] = {
+	[NAND_ECC_NONE]		= "none",
+	[NAND_ECC_SOFT]		= "soft",
+	[NAND_ECC_HW]		= "hw",
+	[NAND_ECC_HW_SYNDROME]	= "hw_syndrome",
+	[NAND_ECC_HW_OOB_FIRST]	= "hw_oob_first",
+	[NAND_ECC_ON_DIE]	= "on-die",
+};
+
+static int of_get_nand_ecc_mode(struct device_node *np)
+{
+	const char *pm;
+	int err, i;
+
+	err = of_property_read_string(np, "nand-ecc-mode", &pm);
+	if (err < 0)
+		return err;
+
+	for (i = 0; i < ARRAY_SIZE(nand_ecc_modes); i++)
+		if (!strcasecmp(pm, nand_ecc_modes[i]))
+			return i;
+
+	/*
+	 * For backward compatibility we support few obsoleted values that don't
+	 * have their mappings into nand_ecc_modes_t anymore (they were merged
+	 * with other enums).
+	 */
+	if (!strcasecmp(pm, "soft_bch"))
+		return NAND_ECC_SOFT;
+
+	return -ENODEV;
+}
+
+static const char * const nand_ecc_algos[] = {
+	[NAND_ECC_HAMMING]	= "hamming",
+	[NAND_ECC_BCH]		= "bch",
+	[NAND_ECC_RS]		= "rs",
+};
+
+static int of_get_nand_ecc_algo(struct device_node *np)
+{
+	const char *pm;
+	int err, i;
+
+	err = of_property_read_string(np, "nand-ecc-algo", &pm);
+	if (!err) {
+		for (i = NAND_ECC_HAMMING; i < ARRAY_SIZE(nand_ecc_algos); i++)
+			if (!strcasecmp(pm, nand_ecc_algos[i]))
+				return i;
+		return -ENODEV;
+	}
+
+	/*
+	 * For backward compatibility we also read "nand-ecc-mode" checking
+	 * for some obsoleted values that were specifying ECC algorithm.
+	 */
+	err = of_property_read_string(np, "nand-ecc-mode", &pm);
+	if (err < 0)
+		return err;
+
+	if (!strcasecmp(pm, "soft"))
+		return NAND_ECC_HAMMING;
+	else if (!strcasecmp(pm, "soft_bch"))
+		return NAND_ECC_BCH;
+
+	return -ENODEV;
+}
+
+static int of_get_nand_ecc_step_size(struct device_node *np)
+{
+	int ret;
+	u32 val;
+
+	ret = of_property_read_u32(np, "nand-ecc-step-size", &val);
+	return ret ? ret : val;
+}
+
+static int of_get_nand_ecc_strength(struct device_node *np)
+{
+	int ret;
+	u32 val;
+
+	ret = of_property_read_u32(np, "nand-ecc-strength", &val);
+	return ret ? ret : val;
+}
+
+static inline bool of_get_nand_ecc_maximize(struct device_node *np)
+{
+	return of_property_read_bool(np, "nand-ecc-maximize");
+}
+
+void nand_ecc_read_user_conf(struct nand_device *nand)
+{
+	struct device_node *dn = nanddev_get_flash_node(nand);
+	int algo, strength, size;
+
+	nand->ecc.user_conf.mode = of_get_nand_ecc_mode(dn);
+
+	algo = of_get_nand_ecc_algo(dn);
+	if (algo >= 0)
+		nand->ecc.user_conf.algo = algo;
+	else
+		nand->ecc.user_conf.algo = NAND_ECC_UNKNOWN;
+
+	strength = of_get_nand_ecc_strength(dn);
+	if (strength >= 0)
+		nand->ecc.user_conf.strength = strength;
+	else
+		nand->ecc.user_conf.strength = 0;
+
+	size = of_get_nand_ecc_step_size(dn);
+	if (size >= 0)
+		nand->ecc.user_conf.step_size = size;
+	else
+		nand->ecc.user_conf.step_size = 0;
+
+	nand->ecc.user_conf.maximize = of_get_nand_ecc_maximize(dn);
+}
+
+/**
+ * nand_ecc_correction_is_enough - Check if the chip configuration meets the
+ *                                 datasheet requirements.
+ *
+ * @nand: Device to check
+ *
+ * If our configuration corrects A bits per B bytes and the minimum
+ * required correction level is X bits per Y bytes, then we must ensure
+ * both of the following are true:
+ *
+ * (1) A / B >= X / Y
+ * (2) A >= X
+ *
+ * Requirement (1) ensures we can correct for the required bitflip density.
+ * Requirement (2) ensures we can correct even when all bitflips are clumped
+ * in the same sector.
+ */
+bool nand_ecc_correction_is_enough(struct nand_device *nand)
+{
+	struct nand_ecc_conf *reqs = &nand->ecc.requirements;
+	struct nand_ecc_conf *conf = &nand->ecc.ctx.conf;
+	struct mtd_info *mtd = nanddev_to_mtd(nand);
+	int corr, ds_corr;
+
+	if (conf->step_size == 0 || reqs->step_size == 0)
+		/* Not enough information */
+		return true;
+
+	/*
+	 * We get the number of corrected bits per page to compare
+	 * the correction density.
+	 */
+	corr = (mtd->writesize * conf->strength) / conf->step_size;
+	ds_corr = (mtd->writesize * reqs->strength) / reqs->step_size;
+
+	return corr >= ds_corr && conf->strength >= reqs->strength;
+}
+
 MODULE_LICENSE("GPL v2");
 MODULE_AUTHOR("Miquel Raynal <miquel.raynal@bootlin.com>");
 MODULE_DESCRIPTION("Generic ECC engine");
diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c
index 3dacaa352a58..b9589ae94352 100644
--- a/drivers/mtd/nand/raw/atmel/nand-controller.c
+++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
@@ -1041,6 +1041,7 @@ static int atmel_nand_pmecc_init(struct nand_chip *chip)
 {
 	struct nand_ecc_conf *requirements = &chip->base.ecc.requirements;
 	struct mtd_info *mtd = nand_to_mtd(chip);
+	struct nand_device *nanddev = mtd_to_nanddev(mtd);
 	struct atmel_nand *nand = to_atmel_nand(chip);
 	struct atmel_nand_controller *nc;
 	struct atmel_pmecc_user_req req;
@@ -1065,7 +1066,7 @@ static int atmel_nand_pmecc_init(struct nand_chip *chip)
 			chip->ecc.size = val;
 	}
 
-	if (chip->ecc.options & NAND_ECC_MAXIMIZE)
+	if (nanddev->ecc.user_conf.maximize)
 		req.ecc.strength = ATMEL_PMECC_MAXIMIZE_ECC_STRENGTH;
 	else if (chip->ecc.strength)
 		req.ecc.strength = chip->ecc.strength;
diff --git a/drivers/mtd/nand/raw/denali.c b/drivers/mtd/nand/raw/denali.c
index 3ec0242b0aa2..d5c5dc83ba4f 100644
--- a/drivers/mtd/nand/raw/denali.c
+++ b/drivers/mtd/nand/raw/denali.c
@@ -1284,6 +1284,7 @@ int denali_init(struct denali_nand_info *denali)
 {
 	struct nand_chip *chip = &denali->nand;
 	struct mtd_info *mtd = nand_to_mtd(chip);
+	struct nand_device *nanddev = mtd_to_nanddev(mtd);
 	u32 features = ioread32(denali->reg + FEATURES);
 	int ret;
 
@@ -1329,6 +1330,8 @@ int denali_init(struct denali_nand_info *denali)
 	if (denali->clk_rate && denali->clk_x_rate)
 		chip->options |= NAND_KEEP_TIMINGS;
 
+	nanddev->ecc.user_conf.maximize = true;
+
 	chip->legacy.dummy_controller.ops = &denali_controller_ops;
 	ret = nand_scan(chip, denali->max_banks);
 	if (ret)
diff --git a/drivers/mtd/nand/raw/denali_pci.c b/drivers/mtd/nand/raw/denali_pci.c
index 48e9ac54ad53..bf3cb7c0480f 100644
--- a/drivers/mtd/nand/raw/denali_pci.c
+++ b/drivers/mtd/nand/raw/denali_pci.c
@@ -64,7 +64,6 @@ static int denali_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	denali->dev = &dev->dev;
 	denali->irq = dev->irq;
 	denali->ecc_caps = &denali_pci_ecc_caps;
-	denali->nand.ecc.options |= NAND_ECC_MAXIMIZE;
 	denali->clk_rate = 50000000;		/* 50 MHz */
 	denali->clk_x_rate = 200000000;		/* 200 MHz */
 
diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index 0851ac63e70d..39cd2b163dc9 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -4698,92 +4698,6 @@ static int nand_detect(struct nand_chip *chip, struct nand_flash_dev *type)
 	return ret;
 }
 
-static const char * const nand_ecc_modes[] = {
-	[NAND_ECC_NONE]		= "none",
-	[NAND_ECC_SOFT]		= "soft",
-	[NAND_ECC_HW]		= "hw",
-	[NAND_ECC_HW_SYNDROME]	= "hw_syndrome",
-	[NAND_ECC_HW_OOB_FIRST]	= "hw_oob_first",
-	[NAND_ECC_ON_DIE]	= "on-die",
-};
-
-static int of_get_nand_ecc_mode(struct device_node *np)
-{
-	const char *pm;
-	int err, i;
-
-	err = of_property_read_string(np, "nand-ecc-mode", &pm);
-	if (err < 0)
-		return err;
-
-	for (i = 0; i < ARRAY_SIZE(nand_ecc_modes); i++)
-		if (!strcasecmp(pm, nand_ecc_modes[i]))
-			return i;
-
-	/*
-	 * For backward compatibility we support few obsoleted values that don't
-	 * have their mappings into nand_ecc_modes_t anymore (they were merged
-	 * with other enums).
-	 */
-	if (!strcasecmp(pm, "soft_bch"))
-		return NAND_ECC_SOFT;
-
-	return -ENODEV;
-}
-
-static const char * const nand_ecc_algos[] = {
-	[NAND_ECC_HAMMING]	= "hamming",
-	[NAND_ECC_BCH]		= "bch",
-	[NAND_ECC_RS]		= "rs",
-};
-
-static int of_get_nand_ecc_algo(struct device_node *np)
-{
-	const char *pm;
-	int err, i;
-
-	err = of_property_read_string(np, "nand-ecc-algo", &pm);
-	if (!err) {
-		for (i = NAND_ECC_HAMMING; i < ARRAY_SIZE(nand_ecc_algos); i++)
-			if (!strcasecmp(pm, nand_ecc_algos[i]))
-				return i;
-		return -ENODEV;
-	}
-
-	/*
-	 * For backward compatibility we also read "nand-ecc-mode" checking
-	 * for some obsoleted values that were specifying ECC algorithm.
-	 */
-	err = of_property_read_string(np, "nand-ecc-mode", &pm);
-	if (err < 0)
-		return err;
-
-	if (!strcasecmp(pm, "soft"))
-		return NAND_ECC_HAMMING;
-	else if (!strcasecmp(pm, "soft_bch"))
-		return NAND_ECC_BCH;
-
-	return -ENODEV;
-}
-
-static int of_get_nand_ecc_step_size(struct device_node *np)
-{
-	int ret;
-	u32 val;
-
-	ret = of_property_read_u32(np, "nand-ecc-step-size", &val);
-	return ret ? ret : val;
-}
-
-static int of_get_nand_ecc_strength(struct device_node *np)
-{
-	int ret;
-	u32 val;
-
-	ret = of_property_read_u32(np, "nand-ecc-strength", &val);
-	return ret ? ret : val;
-}
-
 static int of_get_nand_bus_width(struct device_node *np)
 {
 	u32 val;
@@ -4805,10 +4719,10 @@ static bool of_get_nand_on_flash_bbt(struct device_node *np)
 	return of_property_read_bool(np, "nand-on-flash-bbt");
 }
 
-static int nand_dt_init(struct nand_chip *chip)
+static int rawnand_dt_init(struct nand_chip *chip)
 {
 	struct device_node *dn = nand_get_flash_node(chip);
-	int ecc_mode, ecc_algo, ecc_strength, ecc_step;
+	struct nand_device *nand = mtd_to_nanddev(nand_to_mtd(chip));
 
 	if (!dn)
 		return 0;
@@ -4822,25 +4736,17 @@ static int nand_dt_init(struct nand_chip *chip)
 	if (of_get_nand_on_flash_bbt(dn))
 		chip->bbt_options |= NAND_BBT_USE_FLASH;
 
-	ecc_mode = of_get_nand_ecc_mode(dn);
-	ecc_algo = of_get_nand_ecc_algo(dn);
-	ecc_strength = of_get_nand_ecc_strength(dn);
-	ecc_step = of_get_nand_ecc_step_size(dn);
+	nand_ecc_read_user_conf(nand);
 
-	if (ecc_mode >= 0)
-		chip->ecc.mode = ecc_mode;
+	/* Raw NAND default mode is hardware */
+	if (nand->ecc.user_conf.mode < 0)
+		nand->ecc.user_conf.mode = NAND_ECC_HW;
 
-	if (ecc_algo >= 0)
-		chip->ecc.algo = ecc_algo;
-
-	if (ecc_strength >= 0)
-		chip->ecc.strength = ecc_strength;
-
-	if (ecc_step > 0)
-		chip->ecc.size = ecc_step;
-
-	if (of_property_read_bool(dn, "nand-ecc-maximize"))
-		chip->ecc.options |= NAND_ECC_MAXIMIZE;
+	/* Legacy parameters */
+	chip->ecc.mode = nand->ecc.user_conf.mode;
+	chip->ecc.algo = nand->ecc.user_conf.algo;
+	chip->ecc.strength = nand->ecc.user_conf.strength;
+	chip->ecc.size = nand->ecc.user_conf.step_size;
 
 	return 0;
 }
@@ -4876,7 +4782,7 @@ static int nand_scan_ident(struct nand_chip *chip, unsigned int maxchips,
 	/* Enforce the right timings for reset/detection */
 	onfi_fill_data_interface(chip, NAND_SDR_IFACE, 0);
 
-	ret = nand_dt_init(chip);
+	ret = rawnand_dt_init(chip);
 	if (ret)
 		return ret;
 
@@ -4940,6 +4846,7 @@ static void nand_scan_ident_cleanup(struct nand_chip *chip)
 static int nand_set_ecc_soft_ops(struct nand_chip *chip)
 {
 	struct mtd_info *mtd = nand_to_mtd(chip);
+	struct nand_device *nanddev = mtd_to_nanddev(mtd);
 	struct nand_ecc_ctrl *ecc = &chip->ecc;
 
 	if (WARN_ON(ecc->mode != NAND_ECC_SOFT))
@@ -5011,7 +4918,7 @@ static int nand_set_ecc_soft_ops(struct nand_chip *chip)
 		 * used.
 		 */
 		if (mtd->ooblayout == &nand_ooblayout_lp_ops &&
-		    ecc->options & NAND_ECC_MAXIMIZE) {
+		    nanddev->ecc.user_conf.maximize) {
 			int steps, bytes;
 
 			/* Always prefer 1k blocks over 512bytes ones */
@@ -5249,11 +5156,12 @@ nand_maximize_ecc(struct nand_chip *chip,
  * @caps: ECC engine caps info structure
  * @oobavail: OOB size that the ECC engine can use
  *
- * Choose the ECC configuration according to following logic
+ * Choose the ECC configuration according to following logic.
  *
  * 1. If both ECC step size and ECC strength are already set (usually by DT)
  *    then check if it is supported by this controller.
- * 2. If NAND_ECC_MAXIMIZE is set, then select maximum ECC strength.
+ * 2. If the user provided the nand-ecc-maximize property, then select maximum
+ *    ECC strength.
  * 3. Otherwise, try to match the ECC step size and ECC strength closest
  *    to the chip's requirement. If available OOB size can't fit the chip
  *    requirement then fallback to the maximum ECC step size and ECC strength.
@@ -5264,6 +5172,7 @@ int nand_ecc_choose_conf(struct nand_chip *chip,
 			 const struct nand_ecc_caps *caps, int oobavail)
 {
 	struct mtd_info *mtd = nand_to_mtd(chip);
+	struct nand_device *nanddev = mtd_to_nanddev(mtd);
 
 	if (WARN_ON(oobavail < 0 || oobavail > mtd->oobsize))
 		return -EINVAL;
@@ -5271,7 +5180,7 @@ int nand_ecc_choose_conf(struct nand_chip *chip,
 	if (chip->ecc.size && chip->ecc.strength)
 		return nand_check_ecc_caps(chip, caps, oobavail);
 
-	if (chip->ecc.options & NAND_ECC_MAXIMIZE)
+	if (nanddev->ecc.user_conf.maximize)
 		return nand_maximize_ecc(chip, caps, oobavail);
 
 	if (!nand_match_ecc_req(chip, caps, oobavail))
@@ -5281,42 +5190,6 @@ int nand_ecc_choose_conf(struct nand_chip *chip,
 }
 EXPORT_SYMBOL_GPL(nand_ecc_choose_conf);
 
-/*
- * Check if the chip configuration meet the datasheet requirements.
-
- * If our configuration corrects A bits per B bytes and the minimum
- * required correction level is X bits per Y bytes, then we must ensure
- * both of the following are true:
- *
- * (1) A / B >= X / Y
- * (2) A >= X
- *
- * Requirement (1) ensures we can correct for the required bitflip density.
- * Requirement (2) ensures we can correct even when all bitflips are clumped
- * in the same sector.
- */
-static bool nand_ecc_strength_good(struct nand_chip *chip)
-{
-	struct mtd_info *mtd = nand_to_mtd(chip);
-	struct nand_ecc_ctrl *ecc = &chip->ecc;
-	struct nand_ecc_conf *requirements = &chip->base.ecc.requirements;
-	int corr, ds_corr;
-
-	if (ecc->size == 0 || requirements->step_size == 0)
-		/* Not enough information */
-		return true;
-
-	/*
-	 * We get the number of corrected bits per page to compare
-	 * the correction density.
-	 */
-	corr = (mtd->writesize * ecc->strength) / ecc->size;
-	ds_corr = (mtd->writesize * requirements->strength) /
-		  requirements->step_size;
-
-	return corr >= ds_corr && ecc->strength >= requirements->strength;
-}
-
 static int rawnand_erase(struct nand_device *nand, const struct nand_pos *pos)
 {
 	struct nand_chip *chip = container_of(nand, struct nand_chip,
@@ -5372,6 +5245,7 @@ static const struct nand_ops rawnand_ops = {
 static int nand_scan_tail(struct nand_chip *chip)
 {
 	struct mtd_info *mtd = nand_to_mtd(chip);
+	struct nand_device *nanddev = mtd_to_nanddev(mtd);
 	struct nand_ecc_ctrl *ecc = &chip->ecc;
 	int ret, i;
 
@@ -5593,7 +5467,7 @@ static int nand_scan_tail(struct nand_chip *chip)
 	mtd->oobavail = ret;
 
 	/* ECC sanity check: warn if it's too weak */
-	if (!nand_ecc_strength_good(chip))
+	if (!nand_ecc_correction_is_enough(nanddev))
 		pr_warn("WARNING: %s: the ECC used on your system is too weak compared to the one required by the NAND chip\n",
 			mtd->name);
 
diff --git a/drivers/mtd/nand/raw/sunxi_nand.c b/drivers/mtd/nand/raw/sunxi_nand.c
index 43b22564da10..b56f89c992f3 100644
--- a/drivers/mtd/nand/raw/sunxi_nand.c
+++ b/drivers/mtd/nand/raw/sunxi_nand.c
@@ -1681,6 +1681,7 @@ static int sunxi_nand_hw_ecc_ctrl_init(struct mtd_info *mtd,
 				       struct device_node *np)
 {
 	static const u8 strengths[] = { 16, 24, 28, 32, 40, 48, 56, 60, 64 };
+	struct nand_device *nanddev = mtd_to_nanddev(mtd);
 	struct nand_chip *nand = mtd_to_nand(mtd);
 	struct sunxi_nand_chip *sunxi_nand = to_sunxi_nand(nand);
 	struct sunxi_nfc *nfc = to_sunxi_nfc(sunxi_nand->nand.controller);
@@ -1689,7 +1690,7 @@ static int sunxi_nand_hw_ecc_ctrl_init(struct mtd_info *mtd,
 	int ret;
 	int i;
 
-	if (ecc->options & NAND_ECC_MAXIMIZE) {
+	if (nanddev->ecc.user_conf.maximize) {
 		int bytes;
 
 		ecc->size = 1024;
diff --git a/drivers/mtd/nand/raw/tegra_nand.c b/drivers/mtd/nand/raw/tegra_nand.c
index 31d547d96795..862a17610b83 100644
--- a/drivers/mtd/nand/raw/tegra_nand.c
+++ b/drivers/mtd/nand/raw/tegra_nand.c
@@ -838,7 +838,8 @@ static int tegra_nand_get_strength(struct nand_chip *chip, const int *strength,
 				   int strength_len, int bits_per_step,
 				   int oobsize)
 {
-	bool maximize = chip->ecc.options & NAND_ECC_MAXIMIZE;
+	struct nand_device *nanddev = mtd_to_nanddev(nand_to_mtd(chip));
+	bool maximize = nanddev->ecc.user_conf.maximize;
 	int i;
 
 	/*
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index da214aaa0b8a..4482eb2bbfd4 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -126,6 +126,25 @@ extern const struct mtd_ooblayout_ops nand_ooblayout_sp_ops;
 extern const struct mtd_ooblayout_ops nand_ooblayout_lp_ops;
 extern const struct mtd_ooblayout_ops nand_ooblayout_lp_hamming_ops;
 
+/*
+ * Constants for ECC_MODES
+ */
+typedef enum {
+	NAND_ECC_NONE,
+	NAND_ECC_SOFT,
+	NAND_ECC_HW,
+	NAND_ECC_HW_SYNDROME,
+	NAND_ECC_HW_OOB_FIRST,
+	NAND_ECC_ON_DIE,
+} nand_ecc_modes_t;
+
+enum nand_ecc_algo {
+	NAND_ECC_UNKNOWN,
+	NAND_ECC_HAMMING,
+	NAND_ECC_BCH,
+	NAND_ECC_RS,
+};
+
 /**
  * struct nand_ecc_conf - NAND ECC configuration
  * @strength: ECC strength
@@ -233,12 +252,14 @@ struct nand_ecc_engine {
 	struct nand_ecc_engine_ops *ops;
 };
 
+void nand_ecc_read_user_conf(struct nand_device *nand);
 int nand_ecc_init_ctx(struct nand_device *nand);
 void nand_ecc_cleanup_ctx(struct nand_device *nand);
 int nand_ecc_prepare_io_req(struct nand_device *nand,
 			    struct nand_page_io_req *req, void *oobbuf);
 int nand_ecc_finish_io_req(struct nand_device *nand,
 			   struct nand_page_io_req *req, void *oobbuf);
+bool nand_ecc_correction_is_enough(struct nand_device *nand);
 
 /**
  * struct nand_ecc - High-level ECC object
@@ -323,6 +344,17 @@ static inline struct mtd_info *nanddev_to_mtd(struct nand_device *nand)
 	return &nand->mtd;
 }
 
+/**
+ * nanddev_get_flash_node() - Get the device node attached to a NAND device
+ * @nand: NAND device
+ *
+ * Return: the device node linked to @nand.
+ */
+static inline struct device_node *nanddev_get_flash_node(struct nand_device *nand)
+{
+	return mtd_get_of_node(nanddev_to_mtd(nand));
+}
+
 /*
  * nanddev_bits_per_cell() - Get the number of bits per cell
  * @nand: NAND device
diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
index 0059f5952187..41d26dc010f2 100644
--- a/include/linux/mtd/rawnand.h
+++ b/include/linux/mtd/rawnand.h
@@ -19,6 +19,7 @@
 #include <linux/wait.h>
 #include <linux/spinlock.h>
 #include <linux/mtd/mtd.h>
+#include <linux/mtd/nand.h>
 #include <linux/mtd/flashchip.h>
 #include <linux/mtd/bbm.h>
 #include <linux/mtd/jedec.h>
@@ -84,25 +85,6 @@ struct nand_chip;
 
 #define NAND_DATA_IFACE_CHECK_ONLY	-1
 
-/*
- * Constants for ECC_MODES
- */
-typedef enum {
-	NAND_ECC_NONE,
-	NAND_ECC_SOFT,
-	NAND_ECC_HW,
-	NAND_ECC_HW_SYNDROME,
-	NAND_ECC_HW_OOB_FIRST,
-	NAND_ECC_ON_DIE,
-} nand_ecc_modes_t;
-
-enum nand_ecc_algo {
-	NAND_ECC_UNKNOWN,
-	NAND_ECC_HAMMING,
-	NAND_ECC_BCH,
-	NAND_ECC_RS,
-};
-
 /*
  * Constants for Hardware ECC
  */
@@ -120,7 +102,6 @@ enum nand_ecc_algo {
  * pages and you want to rely on the default implementation.
  */
 #define NAND_ECC_GENERIC_ERASED_CHECK	BIT(0)
-#define NAND_ECC_MAXIMIZE		BIT(1)
 
 /*
  * When using software implementation of Hamming, we can specify which byte
-- 
2.19.1


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

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

* [RFC PATCH 07/27] mtd: nand: ecc: Move BCH code into the ecc/ directory
  2019-02-21 10:01 [RFC PATCH 00/27] Introduce the generic ECC engine abstraction Miquel Raynal
                   ` (5 preceding siblings ...)
  2019-02-21 10:01 ` [RFC PATCH 06/27] mtd: nand: Move ECC specific functions to ecc/engine.c Miquel Raynal
@ 2019-02-21 10:01 ` Miquel Raynal
  2019-02-21 10:01 ` [RFC PATCH 08/27] mtd: nand: ecc: Use SPDX license identifier for the software BCH code Miquel Raynal
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 36+ messages in thread
From: Miquel Raynal @ 2019-02-21 10:01 UTC (permalink / raw)
  To: Boris Brezillon, Richard Weinberger, David Woodhouse,
	Brian Norris, Marek Vasut, Tudor Ambarus
  Cc: Vignesh R, Tudor Ambarus, Julien Su, Schrempf Frieder, linux-mtd,
	Thomas Petazzoni, Miquel Raynal, Mason Yang, linux-arm-kernel

Move generic ECC code in the ecc/ sub-directory for later re-use by
the SPI NAND layer.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/ecc/Kconfig                           | 10 ++++++++++
 drivers/mtd/nand/ecc/Makefile                          |  1 +
 .../mtd/nand/{raw/nand_bch.c => ecc/sw-bch-engine.c}   |  2 +-
 drivers/mtd/nand/raw/Kconfig                           | 10 ----------
 drivers/mtd/nand/raw/Makefile                          |  1 -
 drivers/mtd/nand/raw/nand_base.c                       |  2 +-
 drivers/mtd/nand/raw/nandsim.c                         |  2 +-
 drivers/mtd/nand/raw/omap2.c                           |  2 +-
 include/linux/mtd/{nand_bch.h => nand-sw-bch-engine.h} |  6 +++---
 9 files changed, 18 insertions(+), 18 deletions(-)
 rename drivers/mtd/nand/{raw/nand_bch.c => ecc/sw-bch-engine.c} (99%)
 rename include/linux/mtd/{nand_bch.h => nand-sw-bch-engine.h} (93%)

diff --git a/drivers/mtd/nand/ecc/Kconfig b/drivers/mtd/nand/ecc/Kconfig
index 66c396dcbfbd..a05f1d474446 100644
--- a/drivers/mtd/nand/ecc/Kconfig
+++ b/drivers/mtd/nand/ecc/Kconfig
@@ -1,3 +1,13 @@
 menu "ECC engine support"
 
+config MTD_NAND_ECC_SW_BCH
+	bool "Software BCH ECC engine"
+	select BCH
+	default n
+	help
+	  This enables support for software BCH error correction. Binary BCH
+	  codes are more powerful and cpu intensive than traditional Hamming
+	  ECC codes. They are used with NAND devices requiring more than 1 bit
+	  of error correction.
+
 endmenu
diff --git a/drivers/mtd/nand/ecc/Makefile b/drivers/mtd/nand/ecc/Makefile
index 6a42577ba424..fdbfc2bc38d7 100644
--- a/drivers/mtd/nand/ecc/Makefile
+++ b/drivers/mtd/nand/ecc/Makefile
@@ -1,3 +1,4 @@
 # SPDX-License-Identifier: GPL-2.0
 
 obj-$(CONFIG_MTD_NAND_CORE) += engine.o
+obj-$(CONFIG_MTD_NAND_ECC_SW_BCH)	+= sw-bch-engine.o
diff --git a/drivers/mtd/nand/raw/nand_bch.c b/drivers/mtd/nand/ecc/sw-bch-engine.c
similarity index 99%
rename from drivers/mtd/nand/raw/nand_bch.c
rename to drivers/mtd/nand/ecc/sw-bch-engine.c
index 574c0ca16160..37dc0f0061d3 100644
--- a/drivers/mtd/nand/raw/nand_bch.c
+++ b/drivers/mtd/nand/ecc/sw-bch-engine.c
@@ -26,7 +26,7 @@
 #include <linux/bitops.h>
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/rawnand.h>
-#include <linux/mtd/nand_bch.h>
+#include <linux/mtd/nand-sw-bch-engine.h>
 #include <linux/bch.h>
 
 /**
diff --git a/drivers/mtd/nand/raw/Kconfig b/drivers/mtd/nand/raw/Kconfig
index ebb8a3da9fa5..478bc3f3c042 100644
--- a/drivers/mtd/nand/raw/Kconfig
+++ b/drivers/mtd/nand/raw/Kconfig
@@ -19,16 +19,6 @@ menuconfig MTD_RAW_NAND
 
 if MTD_RAW_NAND
 
-config MTD_NAND_ECC_SW_BCH
-	bool "Support software BCH ECC"
-	select BCH
-	default n
-	help
-	  This enables support for software BCH error correction. Binary BCH
-	  codes are more powerful and cpu intensive than traditional Hamming
-	  ECC codes. They are used with NAND devices requiring more than 1 bit
-	  of error correction.
-
 comment "Raw/parallel NAND flash controllers"
 
 config MTD_NAND_DENALI
diff --git a/drivers/mtd/nand/raw/Makefile b/drivers/mtd/nand/raw/Makefile
index ffe95b157131..18ae42077b05 100644
--- a/drivers/mtd/nand/raw/Makefile
+++ b/drivers/mtd/nand/raw/Makefile
@@ -2,7 +2,6 @@
 
 obj-$(CONFIG_MTD_RAW_NAND)		+= nand.o
 obj-$(CONFIG_MTD_NAND_ECC_SW_HAMMING)	+= nand_ecc.o
-obj-$(CONFIG_MTD_NAND_ECC_SW_BCH)	+= nand_bch.o
 obj-$(CONFIG_MTD_SM_COMMON) 		+= sm_common.o
 
 obj-$(CONFIG_MTD_NAND_CAFE)		+= cafe_nand.o
diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index 39cd2b163dc9..8a9cbe3aa7c1 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -40,7 +40,7 @@
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/nand.h>
 #include <linux/mtd/nand_ecc.h>
-#include <linux/mtd/nand_bch.h>
+#include <linux/mtd/nand-sw-bch-engine.h>
 #include <linux/interrupt.h>
 #include <linux/bitops.h>
 #include <linux/io.h>
diff --git a/drivers/mtd/nand/raw/nandsim.c b/drivers/mtd/nand/raw/nandsim.c
index 670abe8d59ca..797a6e4999cc 100644
--- a/drivers/mtd/nand/raw/nandsim.c
+++ b/drivers/mtd/nand/raw/nandsim.c
@@ -36,7 +36,7 @@
 #include <linux/string.h>
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/rawnand.h>
-#include <linux/mtd/nand_bch.h>
+#include <linux/mtd/nand-sw-bch-engine.h>
 #include <linux/mtd/partitions.h>
 #include <linux/delay.h>
 #include <linux/list.h>
diff --git a/drivers/mtd/nand/raw/omap2.c b/drivers/mtd/nand/raw/omap2.c
index eaf14813a3ab..38550148a53b 100644
--- a/drivers/mtd/nand/raw/omap2.c
+++ b/drivers/mtd/nand/raw/omap2.c
@@ -26,7 +26,7 @@
 #include <linux/of.h>
 #include <linux/of_device.h>
 
-#include <linux/mtd/nand_bch.h>
+#include <linux/mtd/nand-sw-bch-engine.h>
 #include <linux/platform_data/elm.h>
 
 #include <linux/omap-gpmc.h>
diff --git a/include/linux/mtd/nand_bch.h b/include/linux/mtd/nand-sw-bch-engine.h
similarity index 93%
rename from include/linux/mtd/nand_bch.h
rename to include/linux/mtd/nand-sw-bch-engine.h
index 06ce2b655c13..a682a15fa165 100644
--- a/include/linux/mtd/nand_bch.h
+++ b/include/linux/mtd/nand-sw-bch-engine.h
@@ -8,8 +8,8 @@
  * This file is the header for the NAND BCH ECC implementation.
  */
 
-#ifndef __MTD_NAND_BCH_H__
-#define __MTD_NAND_BCH_H__
+#ifndef __MTD_NAND_ECC_SW_BCH_H__
+#define __MTD_NAND_ECC_SW_BCH_H__
 
 struct mtd_info;
 struct nand_chip;
@@ -66,4 +66,4 @@ static inline void nand_bch_free(struct nand_bch_control *nbc) {}
 
 #endif /* CONFIG_MTD_NAND_ECC_SW_BCH */
 
-#endif /* __MTD_NAND_BCH_H__ */
+#endif /* __MTD_NAND_ECC_SW_BCH_H__ */
-- 
2.19.1


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

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

* [RFC PATCH 08/27] mtd: nand: ecc: Use SPDX license identifier for the software BCH code
  2019-02-21 10:01 [RFC PATCH 00/27] Introduce the generic ECC engine abstraction Miquel Raynal
                   ` (6 preceding siblings ...)
  2019-02-21 10:01 ` [RFC PATCH 07/27] mtd: nand: ecc: Move BCH code into the ecc/ directory Miquel Raynal
@ 2019-02-21 10:01 ` Miquel Raynal
  2019-02-21 11:25   ` Boris Brezillon
  2019-02-21 10:01 ` [RFC PATCH 09/27] mtd: nand: ecc: Turn the software BCH implementation generic Miquel Raynal
                   ` (3 subsequent siblings)
  11 siblings, 1 reply; 36+ messages in thread
From: Miquel Raynal @ 2019-02-21 10:01 UTC (permalink / raw)
  To: Boris Brezillon, Richard Weinberger, David Woodhouse,
	Brian Norris, Marek Vasut, Tudor Ambarus
  Cc: Vignesh R, Tudor Ambarus, Julien Su, Schrempf Frieder, linux-mtd,
	Thomas Petazzoni, Miquel Raynal, Mason Yang, linux-arm-kernel

Remove the legacy license text, also update the MODULE_LICENSE macro
in accordance with the text at the top of the file (GPL v2 or
higher).

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/ecc/sw-bch-engine.c   | 17 ++---------------
 include/linux/mtd/nand-sw-bch-engine.h |  5 +----
 2 files changed, 3 insertions(+), 19 deletions(-)

diff --git a/drivers/mtd/nand/ecc/sw-bch-engine.c b/drivers/mtd/nand/ecc/sw-bch-engine.c
index 37dc0f0061d3..871c4dd9f71d 100644
--- a/drivers/mtd/nand/ecc/sw-bch-engine.c
+++ b/drivers/mtd/nand/ecc/sw-bch-engine.c
@@ -1,22 +1,9 @@
+// SPDX-License-Identifier: GPL-2.0+
 /*
  * This file provides ECC correction for more than 1 bit per block of data,
  * using binary BCH codes. It relies on the generic BCH library lib/bch.c.
  *
  * Copyright © 2011 Ivan Djelic <ivan.djelic@parrot.com>
- *
- * This file is free software; you can redistribute it and/or modify it
- * under the terms of the GNU General Public License as published by the
- * Free Software Foundation; either version 2 or (at your option) any
- * later version.
- *
- * This file is distributed in the hope that it will be useful, but WITHOUT
- * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
- * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
- * for more details.
- *
- * You should have received a copy of the GNU General Public License along
- * with this file; if not, write to the Free Software Foundation, Inc.,
- * 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
  */
 
 #include <linux/types.h>
@@ -227,6 +214,6 @@ void nand_bch_free(struct nand_bch_control *nbc)
 }
 EXPORT_SYMBOL(nand_bch_free);
 
-MODULE_LICENSE("GPL");
+MODULE_LICENSE("GPL v2");
 MODULE_AUTHOR("Ivan Djelic <ivan.djelic@parrot.com>");
 MODULE_DESCRIPTION("NAND software BCH ECC support");
diff --git a/include/linux/mtd/nand-sw-bch-engine.h b/include/linux/mtd/nand-sw-bch-engine.h
index a682a15fa165..12107aa7c371 100644
--- a/include/linux/mtd/nand-sw-bch-engine.h
+++ b/include/linux/mtd/nand-sw-bch-engine.h
@@ -1,10 +1,7 @@
+/* SPDX-License-Identifier: GPL-2.0 */
 /*
  * Copyright © 2011 Ivan Djelic <ivan.djelic@parrot.com>
  *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- *
  * This file is the header for the NAND BCH ECC implementation.
  */
 
-- 
2.19.1


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

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

* [RFC PATCH 09/27] mtd: nand: ecc: Turn the software BCH implementation generic
  2019-02-21 10:01 [RFC PATCH 00/27] Introduce the generic ECC engine abstraction Miquel Raynal
                   ` (7 preceding siblings ...)
  2019-02-21 10:01 ` [RFC PATCH 08/27] mtd: nand: ecc: Use SPDX license identifier for the software BCH code Miquel Raynal
@ 2019-02-21 10:01 ` Miquel Raynal
  2019-02-21 12:26   ` Boris Brezillon
  2019-02-21 10:01 ` [RFC PATCH 10/27] mtd: rawnand: Get rid of chip->ecc.priv Miquel Raynal
                   ` (2 subsequent siblings)
  11 siblings, 1 reply; 36+ messages in thread
From: Miquel Raynal @ 2019-02-21 10:01 UTC (permalink / raw)
  To: Boris Brezillon, Richard Weinberger, David Woodhouse,
	Brian Norris, Marek Vasut, Tudor Ambarus
  Cc: Vignesh R, Tudor Ambarus, Julien Su, Schrempf Frieder, linux-mtd,
	Thomas Petazzoni, Miquel Raynal, Mason Yang, linux-arm-kernel

Add helpers in the raw NAND core to call the generic functions that
will be re-used by the SPI-NAND layer.

While at it, do some cleanup in the file and its header.

The only reason why rawnand_bch helpers are exported is that the OMAP2
driver uses them. This is something that should be fixed and these
helpers turned static.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/ecc/sw-bch-engine.c   | 192 +++++++++++--------------
 drivers/mtd/nand/raw/nand_base.c       |  87 +++++++++--
 drivers/mtd/nand/raw/nandsim.c         |   3 +-
 drivers/mtd/nand/raw/omap2.c           |  32 ++---
 include/linux/mtd/nand-sw-bch-engine.h |  77 +++++-----
 include/linux/mtd/rawnand.h            |   5 +
 6 files changed, 223 insertions(+), 173 deletions(-)

diff --git a/drivers/mtd/nand/ecc/sw-bch-engine.c b/drivers/mtd/nand/ecc/sw-bch-engine.c
index 871c4dd9f71d..fd982e520989 100644
--- a/drivers/mtd/nand/ecc/sw-bch-engine.c
+++ b/drivers/mtd/nand/ecc/sw-bch-engine.c
@@ -11,133 +11,132 @@
 #include <linux/module.h>
 #include <linux/slab.h>
 #include <linux/bitops.h>
-#include <linux/mtd/mtd.h>
-#include <linux/mtd/rawnand.h>
+#include <linux/mtd/nand.h>
 #include <linux/mtd/nand-sw-bch-engine.h>
-#include <linux/bch.h>
 
 /**
- * struct nand_bch_control - private NAND BCH control structure
- * @bch:       BCH control structure
- * @errloc:    error location array
- * @eccmask:   XOR ecc mask, allows erased pages to be decoded as valid
+ * ecc_sw_bch_calculate - Calculate the ECC corresponding to a data block
+ *
+ * @nand: NAND device
+ * @buf: Input buffer with raw data
+ * @code: Output buffer with ECC
  */
-struct nand_bch_control {
-	struct bch_control   *bch;
-	unsigned int         *errloc;
-	unsigned char        *eccmask;
-};
-
-/**
- * nand_bch_calculate_ecc - [NAND Interface] Calculate ECC for data block
- * @chip:	NAND chip object
- * @buf:	input buffer with raw data
- * @code:	output buffer with ECC
- */
-int nand_bch_calculate_ecc(struct nand_chip *chip, const unsigned char *buf,
-			   unsigned char *code)
+int ecc_sw_bch_calculate(struct nand_device *nand, const unsigned char *buf,
+			 unsigned char *code)
 {
-	struct nand_bch_control *nbc = chip->ecc.priv;
+	struct ecc_sw_bch_conf *engine_conf = nand->ecc.ctx.priv;
 	unsigned int i;
 
-	memset(code, 0, chip->ecc.bytes);
-	encode_bch(nbc->bch, buf, chip->ecc.size, code);
+	memset(code, 0, engine_conf->code_size);
+	encode_bch(engine_conf->bch, buf, nand->ecc.ctx.conf.step_size, code);
 
 	/* apply mask so that an erased page is a valid codeword */
-	for (i = 0; i < chip->ecc.bytes; i++)
-		code[i] ^= nbc->eccmask[i];
+	for (i = 0; i < engine_conf->code_size; i++)
+		code[i] ^= engine_conf->eccmask[i];
 
 	return 0;
 }
-EXPORT_SYMBOL(nand_bch_calculate_ecc);
+EXPORT_SYMBOL(ecc_sw_bch_calculate);
 
 /**
- * nand_bch_correct_data - [NAND Interface] Detect and correct bit error(s)
- * @chip:	NAND chip object
- * @buf:	raw data read from the chip
- * @read_ecc:	ECC from the chip
- * @calc_ecc:	the ECC calculated from raw data
+ * ecc_sw_bch_correct - Detect, correct and report bit error(s)
  *
- * Detect and correct bit errors for a data byte block
+ * @nand: NAND device
+ * @buf: Raw data read from the chip
+ * @read_ecc: ECC bytes from the chip
+ * @calc_ecc: ECC calculated from the raw data
+ *
+ * Detect and correct bit errors for a data block.
  */
-int nand_bch_correct_data(struct nand_chip *chip, unsigned char *buf,
-			  unsigned char *read_ecc, unsigned char *calc_ecc)
+int ecc_sw_bch_correct(struct nand_device *nand, unsigned char *buf,
+		       unsigned char *read_ecc, unsigned char *calc_ecc)
 {
-	struct nand_bch_control *nbc = chip->ecc.priv;
-	unsigned int *errloc = nbc->errloc;
+	struct ecc_sw_bch_conf *engine_conf = nand->ecc.ctx.priv;
+	unsigned int step_size = nand->ecc.ctx.conf.step_size;
+	unsigned int *errloc = engine_conf->errloc;
 	int i, count;
 
-	count = decode_bch(nbc->bch, NULL, chip->ecc.size, read_ecc, calc_ecc,
-			   NULL, errloc);
+	count = decode_bch(engine_conf->bch, NULL, step_size, read_ecc,
+			   calc_ecc, NULL, errloc);
 	if (count > 0) {
 		for (i = 0; i < count; i++) {
-			if (errloc[i] < (chip->ecc.size*8))
-				/* error is located in data, correct it */
+			if (errloc[i] < (step_size * 8))
+				/* The error is in the data: correct it */
 				buf[errloc[i] >> 3] ^= (1 << (errloc[i] & 7));
-			/* else error in ecc, no action needed */
 
+			/* Otherwise the error is in the ECC: nothing to do */
 			pr_debug("%s: corrected bitflip %u\n", __func__,
-					errloc[i]);
+				 errloc[i]);
 		}
 	} else if (count < 0) {
-		pr_err("ecc unrecoverable error\n");
+		pr_err("ECC unrecoverable error\n");
 		count = -EBADMSG;
 	}
+
 	return count;
 }
-EXPORT_SYMBOL(nand_bch_correct_data);
+EXPORT_SYMBOL(ecc_sw_bch_correct);
 
 /**
- * nand_bch_init - [NAND Interface] Initialize NAND BCH error correction
- * @mtd:	MTD block structure
+ * ecc_sw_bch_cleanup - Cleanup software BCH ECC resources
+ * @nand: NAND device
+ */
+void ecc_sw_bch_cleanup(struct nand_device *nand)
+{
+	struct ecc_sw_bch_conf *engine_conf = nand->ecc.ctx.priv;
+
+	free_bch(engine_conf->bch);
+	kfree(engine_conf->errloc);
+	kfree(engine_conf->eccmask);
+}
+EXPORT_SYMBOL(ecc_sw_bch_cleanup);
+
+/**
+ * ecc_sw_bch_init - Initialize software BCH ECC engine
+ * @nand: NAND device
  *
- * Returns:
- *  a pointer to a new NAND BCH control structure, or NULL upon failure
+ * Returns: a pointer to a new NAND BCH control structure, or NULL upon failure
  *
- * Initialize NAND BCH error correction. Parameters @eccsize and @eccbytes
- * are used to compute BCH parameters m (Galois field order) and t (error
- * correction capability). @eccbytes should be equal to the number of bytes
- * required to store m*t bits, where m is such that 2^m-1 > @eccsize*8.
+ * Initialize NAND BCH error correction. @nand.ecc parameters 'step_size' and
+ * 'bytes' are used to compute BCH parameters m (Galois field order) and t
+ * (error correction capability). 'bytes' should be equal to the number of bytes
+ * required to store m*t bits, where m is such that 2^m-1 > step_size*8.
  *
  * Example: to configure 4 bit correction per 512 bytes, you should pass
- * @eccsize = 512  (thus, m=13 is the smallest integer such that 2^m-1 > 512*8)
- * @eccbytes = 7   (7 bytes are required to store m*t = 13*4 = 52 bits)
+ * step_size = 512 (thus, m=13 is the smallest integer such that 2^m-1 > 512*8)
+ * bytes = 7 (7 bytes are required to store m*t = 13*4 = 52 bits)
  */
-struct nand_bch_control *nand_bch_init(struct mtd_info *mtd)
+int ecc_sw_bch_init(struct nand_device *nand)
 {
-	struct nand_chip *nand = mtd_to_nand(mtd);
+	struct mtd_info *mtd = nanddev_to_mtd(nand);
 	unsigned int m, t, eccsteps, i;
-	struct nand_bch_control *nbc = NULL;
+	struct ecc_sw_bch_conf *engine_conf = nand->ecc.ctx.priv;
 	unsigned char *erased_page;
-	unsigned int eccsize = nand->ecc.size;
-	unsigned int eccbytes = nand->ecc.bytes;
-	unsigned int eccstrength = nand->ecc.strength;
+	unsigned int eccsize = nand->ecc.ctx.conf.step_size;
+	unsigned int eccbytes = engine_conf->code_size;
+	unsigned int eccstrength = nand->ecc.ctx.conf.strength;
 
 	if (!eccbytes && eccstrength) {
 		eccbytes = DIV_ROUND_UP(eccstrength * fls(8 * eccsize), 8);
-		nand->ecc.bytes = eccbytes;
+		engine_conf->code_size = eccbytes;
 	}
 
 	if (!eccsize || !eccbytes) {
 		pr_warn("ecc parameters not supplied\n");
-		goto fail;
+		return -EINVAL;
 	}
 
 	m = fls(1+8*eccsize);
 	t = (eccbytes*8)/m;
 
-	nbc = kzalloc(sizeof(*nbc), GFP_KERNEL);
-	if (!nbc)
-		goto fail;
-
-	nbc->bch = init_bch(m, t, 0);
-	if (!nbc->bch)
-		goto fail;
+	engine_conf->bch = init_bch(m, t, 0);
+	if (!engine_conf->bch)
+		return -EINVAL;
 
 	/* verify that eccbytes has the expected value */
-	if (nbc->bch->ecc_bytes != eccbytes) {
+	if (engine_conf->bch->ecc_bytes != eccbytes) {
 		pr_warn("invalid eccbytes %u, should be %u\n",
-			eccbytes, nbc->bch->ecc_bytes);
+			eccbytes, engine_conf->bch->ecc_bytes);
 		goto fail;
 	}
 
@@ -155,24 +154,15 @@ struct nand_bch_control *nand_bch_init(struct mtd_info *mtd)
 		goto fail;
 	}
 
-	/*
-	 * ecc->steps and ecc->total might be used by mtd->ooblayout->ecc(),
-	 * which is called by mtd_ooblayout_count_eccbytes().
-	 * Make sure they are properly initialized before calling
-	 * mtd_ooblayout_count_eccbytes().
-	 * FIXME: we should probably rework the sequencing in nand_scan_tail()
-	 * to avoid setting those fields twice.
-	 */
-	nand->ecc.steps = eccsteps;
-	nand->ecc.total = eccsteps * eccbytes;
 	if (mtd_ooblayout_count_eccbytes(mtd) != (eccsteps*eccbytes)) {
 		pr_warn("invalid ecc layout\n");
 		goto fail;
 	}
 
-	nbc->eccmask = kmalloc(eccbytes, GFP_KERNEL);
-	nbc->errloc = kmalloc_array(t, sizeof(*nbc->errloc), GFP_KERNEL);
-	if (!nbc->eccmask || !nbc->errloc)
+	engine_conf->eccmask = kmalloc(eccbytes, GFP_KERNEL);
+	engine_conf->errloc = kmalloc_array(t, sizeof(*engine_conf->errloc),
+					    GFP_KERNEL);
+	if (!engine_conf->eccmask || !engine_conf->errloc)
 		goto fail;
 	/*
 	 * compute and store the inverted ecc of an erased ecc block
@@ -182,37 +172,25 @@ struct nand_bch_control *nand_bch_init(struct mtd_info *mtd)
 		goto fail;
 
 	memset(erased_page, 0xff, eccsize);
-	memset(nbc->eccmask, 0, eccbytes);
-	encode_bch(nbc->bch, erased_page, eccsize, nbc->eccmask);
+	memset(engine_conf->eccmask, 0, eccbytes);
+	encode_bch(engine_conf->bch, erased_page, eccsize,
+		   engine_conf->eccmask);
 	kfree(erased_page);
 
 	for (i = 0; i < eccbytes; i++)
-		nbc->eccmask[i] ^= 0xff;
+		engine_conf->eccmask[i] ^= 0xff;
 
 	if (!eccstrength)
-		nand->ecc.strength = (eccbytes * 8) / fls(8 * eccsize);
+		nand->ecc.ctx.conf.strength = (eccbytes * 8) / fls(8 * eccsize);
+
+	return 0;
 
-	return nbc;
 fail:
-	nand_bch_free(nbc);
-	return NULL;
-}
-EXPORT_SYMBOL(nand_bch_init);
+	ecc_sw_bch_cleanup(nand);
 
-/**
- * nand_bch_free - [NAND Interface] Release NAND BCH ECC resources
- * @nbc:	NAND BCH control structure
- */
-void nand_bch_free(struct nand_bch_control *nbc)
-{
-	if (nbc) {
-		free_bch(nbc->bch);
-		kfree(nbc->errloc);
-		kfree(nbc->eccmask);
-		kfree(nbc);
-	}
+	return -EINVAL;
 }
-EXPORT_SYMBOL(nand_bch_free);
+EXPORT_SYMBOL(ecc_sw_bch_init);
 
 MODULE_LICENSE("GPL v2");
 MODULE_AUTHOR("Ivan Djelic <ivan.djelic@parrot.com>");
diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index 8a9cbe3aa7c1..64a6e9b840a4 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -4843,11 +4843,73 @@ static void nand_scan_ident_cleanup(struct nand_chip *chip)
 	kfree(chip->parameters.onfi);
 }
 
+int rawnand_sw_bch_init(struct nand_chip *chip)
+{
+	struct nand_device *base = &chip->base;
+	struct ecc_sw_bch_conf *engine_conf;
+	int ret;
+
+	base->ecc.user_conf.mode = NAND_ECC_SOFT;
+	base->ecc.user_conf.algo = NAND_ECC_BCH;
+	base->ecc.user_conf.step_size = chip->ecc.size;
+	base->ecc.user_conf.strength = chip->ecc.strength;
+
+	engine_conf = kzalloc(sizeof(*engine_conf), GFP_KERNEL);
+	if (!engine_conf)
+		return -ENOMEM;
+
+	engine_conf->code_size = chip->ecc.bytes;
+
+	base->ecc.ctx.priv = engine_conf;
+
+	ret = ecc_sw_bch_init(base);
+	if (ret)
+		kfree(base->ecc.ctx.priv);
+
+	chip->ecc.size = base->ecc.ctx.conf.step_size;
+	chip->ecc.strength = base->ecc.ctx.conf.strength;
+	chip->ecc.total = base->ecc.ctx.conf.total;
+	chip->ecc.steps = engine_conf->nsteps;
+	chip->ecc.bytes = engine_conf->code_size;
+
+	return ret;
+}
+EXPORT_SYMBOL(rawnand_sw_bch_init);
+
+static int rawnand_sw_bch_calculate(struct nand_chip *chip,
+				    const unsigned char *buf,
+				    unsigned char *code)
+{
+	struct nand_device *base = &chip->base;
+
+	return ecc_sw_bch_calculate(base, buf, code);
+}
+
+int rawnand_sw_bch_correct(struct nand_chip *chip, unsigned char *buf,
+			   unsigned char *read_ecc, unsigned char *calc_ecc)
+{
+	struct nand_device *base = &chip->base;
+
+	return ecc_sw_bch_correct(base, buf, read_ecc, calc_ecc);
+}
+EXPORT_SYMBOL(rawnand_sw_bch_correct);
+
+void rawnand_sw_bch_cleanup(struct nand_chip *chip)
+{
+	struct nand_device *base = &chip->base;
+
+	ecc_sw_bch_cleanup(base);
+
+	kfree(base->ecc.ctx.priv);
+}
+EXPORT_SYMBOL(rawnand_sw_bch_cleanup);
+
 static int nand_set_ecc_soft_ops(struct nand_chip *chip)
 {
 	struct mtd_info *mtd = nand_to_mtd(chip);
 	struct nand_device *nanddev = mtd_to_nanddev(mtd);
 	struct nand_ecc_ctrl *ecc = &chip->ecc;
+	int ret;
 
 	if (WARN_ON(ecc->mode != NAND_ECC_SOFT))
 		return -EINVAL;
@@ -4873,12 +4935,12 @@ static int nand_set_ecc_soft_ops(struct nand_chip *chip)
 
 		return 0;
 	case NAND_ECC_BCH:
-		if (!mtd_nand_has_bch()) {
+		if (!IS_ENABLED(CONFIG_MTD_NAND_ECC_SW_BCH)) {
 			WARN(1, "CONFIG_MTD_NAND_ECC_SW_BCH not enabled\n");
 			return -EINVAL;
 		}
-		ecc->calculate = nand_bch_calculate_ecc;
-		ecc->correct = nand_bch_correct_data;
+		ecc->calculate = rawnand_sw_bch_calculate;
+		ecc->correct = rawnand_sw_bch_correct;
 		ecc->read_page = nand_read_page_swecc;
 		ecc->read_subpage = nand_read_subpage;
 		ecc->write_page = nand_write_page_swecc;
@@ -4930,13 +4992,14 @@ static int nand_set_ecc_soft_ops(struct nand_chip *chip)
 			ecc->strength = bytes * 8 / fls(8 * ecc->size);
 		}
 
-		/* See nand_bch_init() for details. */
+		/* See ecc_sw_bch_init() for details. */
 		ecc->bytes = 0;
-		ecc->priv = nand_bch_init(mtd);
-		if (!ecc->priv) {
+		ret = rawnand_sw_bch_init(chip);
+		if (ret) {
 			WARN(1, "BCH ECC initialization failed!\n");
-			return -EINVAL;
+			return ret;
 		}
+
 		return 0;
 	default:
 		WARN(1, "Unsupported ECC algorithm!\n");
@@ -5443,13 +5506,17 @@ static int nand_scan_tail(struct nand_chip *chip)
 	 * Set the number of read / write steps for one page depending on ECC
 	 * mode.
 	 */
-	ecc->steps = mtd->writesize / ecc->size;
+	if (!ecc->steps)
+		ecc->steps = mtd->writesize / ecc->size;
 	if (ecc->steps * ecc->size != mtd->writesize) {
 		WARN(1, "Invalid ECC parameters\n");
 		ret = -EINVAL;
 		goto err_nand_manuf_cleanup;
 	}
-	ecc->total = ecc->steps * ecc->bytes;
+
+	if (!ecc->total)
+		ecc->total = ecc->steps * ecc->bytes;
+
 	if (ecc->total > mtd->oobsize) {
 		WARN(1, "Total number of ECC bytes exceeded oobsize\n");
 		ret = -EINVAL;
@@ -5638,7 +5705,7 @@ void nand_cleanup(struct nand_chip *chip)
 {
 	if (chip->ecc.mode == NAND_ECC_SOFT &&
 	    chip->ecc.algo == NAND_ECC_BCH)
-		nand_bch_free((struct nand_bch_control *)chip->ecc.priv);
+		rawnand_sw_bch_cleanup(chip);
 
 	/* Free bad block table memory */
 	kfree(chip->bbt);
diff --git a/drivers/mtd/nand/raw/nandsim.c b/drivers/mtd/nand/raw/nandsim.c
index 797a6e4999cc..a60b972dd211 100644
--- a/drivers/mtd/nand/raw/nandsim.c
+++ b/drivers/mtd/nand/raw/nandsim.c
@@ -36,7 +36,6 @@
 #include <linux/string.h>
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/rawnand.h>
-#include <linux/mtd/nand-sw-bch-engine.h>
 #include <linux/mtd/partitions.h>
 #include <linux/delay.h>
 #include <linux/list.h>
@@ -2175,7 +2174,7 @@ static int ns_attach_chip(struct nand_chip *chip)
 	if (!bch)
 		return 0;
 
-	if (!mtd_nand_has_bch()) {
+	if (!IS_ENABLED(CONFIG_MTD_NAND_ECC_SW_BCH)) {
 		NS_ERR("BCH ECC support is disabled\n");
 		return -EINVAL;
 	}
diff --git a/drivers/mtd/nand/raw/omap2.c b/drivers/mtd/nand/raw/omap2.c
index 38550148a53b..5e526499824e 100644
--- a/drivers/mtd/nand/raw/omap2.c
+++ b/drivers/mtd/nand/raw/omap2.c
@@ -26,7 +26,6 @@
 #include <linux/of.h>
 #include <linux/of_device.h>
 
-#include <linux/mtd/nand-sw-bch-engine.h>
 #include <linux/platform_data/elm.h>
 
 #include <linux/omap-gpmc.h>
@@ -2051,16 +2050,16 @@ static int omap_nand_attach_chip(struct nand_chip *chip)
 		chip->ecc.bytes		= 7;
 		chip->ecc.strength	= 4;
 		chip->ecc.hwctl		= omap_enable_hwecc_bch;
-		chip->ecc.correct	= nand_bch_correct_data;
+		chip->ecc.correct	= rawnand_sw_bch_correct;
 		chip->ecc.calculate	= omap_calculate_ecc_bch_sw;
 		mtd_set_ooblayout(mtd, &omap_sw_ooblayout_ops);
 		/* Reserve one byte for the OMAP marker */
 		oobbytes_per_step	= chip->ecc.bytes + 1;
 		/* Software BCH library is used for locating errors */
-		chip->ecc.priv		= nand_bch_init(mtd);
-		if (!chip->ecc.priv) {
+		err = rawnand_sw_bch_init(chip);
+		if (err) {
 			dev_err(dev, "Unable to use BCH library\n");
-			return -EINVAL;
+			return err;
 		}
 		break;
 
@@ -2093,16 +2092,16 @@ static int omap_nand_attach_chip(struct nand_chip *chip)
 		chip->ecc.bytes		= 13;
 		chip->ecc.strength	= 8;
 		chip->ecc.hwctl		= omap_enable_hwecc_bch;
-		chip->ecc.correct	= nand_bch_correct_data;
+		chip->ecc.correct	= rawnand_sw_bch_correct;
 		chip->ecc.calculate	= omap_calculate_ecc_bch_sw;
 		mtd_set_ooblayout(mtd, &omap_sw_ooblayout_ops);
 		/* Reserve one byte for the OMAP marker */
 		oobbytes_per_step	= chip->ecc.bytes + 1;
 		/* Software BCH library is used for locating errors */
-		chip->ecc.priv		= nand_bch_init(mtd);
-		if (!chip->ecc.priv) {
+		err = rawnand_sw_bch_init(chip);
+		if (err) {
 			dev_err(dev, "unable to use BCH library\n");
-			return -EINVAL;
+			return err;
 		}
 		break;
 
@@ -2208,7 +2207,6 @@ static int omap_nand_probe(struct platform_device *pdev)
 	nand_chip		= &info->nand;
 	mtd			= nand_to_mtd(nand_chip);
 	mtd->dev.parent		= &pdev->dev;
-	nand_chip->ecc.priv	= NULL;
 	nand_set_flash_node(nand_chip, dev->of_node);
 
 	if (!mtd->name) {
@@ -2278,10 +2276,9 @@ static int omap_nand_probe(struct platform_device *pdev)
 return_error:
 	if (!IS_ERR_OR_NULL(info->dma))
 		dma_release_channel(info->dma);
-	if (nand_chip->ecc.priv) {
-		nand_bch_free(nand_chip->ecc.priv);
-		nand_chip->ecc.priv = NULL;
-	}
+
+	rawnand_sw_bch_cleanup(nand_chip);
+
 	return err;
 }
 
@@ -2290,10 +2287,9 @@ static int omap_nand_remove(struct platform_device *pdev)
 	struct mtd_info *mtd = platform_get_drvdata(pdev);
 	struct nand_chip *nand_chip = mtd_to_nand(mtd);
 	struct omap_nand_info *info = mtd_to_omap(mtd);
-	if (nand_chip->ecc.priv) {
-		nand_bch_free(nand_chip->ecc.priv);
-		nand_chip->ecc.priv = NULL;
-	}
+
+	rawnand_sw_bch_cleanup(nand_chip);
+
 	if (info->dma)
 		dma_release_channel(info->dma);
 	nand_release(nand_chip);
diff --git a/include/linux/mtd/nand-sw-bch-engine.h b/include/linux/mtd/nand-sw-bch-engine.h
index 12107aa7c371..d8abfc9fc288 100644
--- a/include/linux/mtd/nand-sw-bch-engine.h
+++ b/include/linux/mtd/nand-sw-bch-engine.h
@@ -8,58 +8,63 @@
 #ifndef __MTD_NAND_ECC_SW_BCH_H__
 #define __MTD_NAND_ECC_SW_BCH_H__
 
-struct mtd_info;
-struct nand_chip;
-struct nand_bch_control;
+#include <linux/mtd/nand.h>
+#include <linux/bch.h>
+
+/**
+ * struct ecc_sw_bch_conf - private software BCH ECC engine structure
+ * @reqooblen: Save the actual user OOB length requested before overwriting it
+ * @code_size: Number of bytes needed to store a code (one code per step)
+ * @nsteps: Number of steps
+ * @calc_buf: Buffer to use when calculating ECC bytes
+ * @code_buf: Buffer to use when reading (raw) ECC bytes from the chip
+ * @bch: BCH control structure
+ * @errloc: error location array
+ * @eccmask: XOR ecc mask, allows erased pages to be decoded as valid
+ */
+struct ecc_sw_bch_conf {
+	unsigned int reqooblen;
+	unsigned int code_size;
+	unsigned int nsteps;
+	u8 *calc_buf;
+	u8 *code_buf;
+	struct bch_control *bch;
+	unsigned int *errloc;
+	unsigned char *eccmask;
+};
 
 #if defined(CONFIG_MTD_NAND_ECC_SW_BCH)
 
-static inline int mtd_nand_has_bch(void) { return 1; }
-
-/*
- * Calculate BCH ecc code
- */
-int nand_bch_calculate_ecc(struct nand_chip *chip, const u_char *dat,
-			   u_char *ecc_code);
-
-/*
- * Detect and correct bit errors
- */
-int nand_bch_correct_data(struct nand_chip *chip, u_char *dat,
-			  u_char *read_ecc, u_char *calc_ecc);
-/*
- * Initialize BCH encoder/decoder
- */
-struct nand_bch_control *nand_bch_init(struct mtd_info *mtd);
-/*
- * Release BCH encoder/decoder resources
- */
-void nand_bch_free(struct nand_bch_control *nbc);
+int ecc_sw_bch_calculate(struct nand_device *nand, const unsigned char *buf,
+			 unsigned char *code);
+int ecc_sw_bch_correct(struct nand_device *nand, unsigned char *buf,
+		       unsigned char *read_ecc, unsigned char *calc_ecc);
+int ecc_sw_bch_init(struct nand_device *nand);
+void ecc_sw_bch_cleanup(struct nand_device *nand);
 
 #else /* !CONFIG_MTD_NAND_ECC_SW_BCH */
 
-static inline int mtd_nand_has_bch(void) { return 0; }
-
-static inline int
-nand_bch_calculate_ecc(struct nand_chip *chip, const u_char *dat,
-		       u_char *ecc_code)
+static inline int ecc_sw_bch_calculate(struct nand_device *nand,
+				       const unsigned char *buf,
+				       unsigned char *code)
 {
-	return -1;
+	return -ENOTSUPP;
 }
 
-static inline int
-nand_bch_correct_data(struct nand_chip *chip, unsigned char *buf,
-		      unsigned char *read_ecc, unsigned char *calc_ecc)
+static inline int ecc_sw_bch_correct(struct nand_device *nand,
+				     unsigned char *buf,
+				     unsigned char *read_ecc,
+				     unsigned char *calc_ecc)
 {
 	return -ENOTSUPP;
 }
 
-static inline struct nand_bch_control *nand_bch_init(struct mtd_info *mtd)
+static inline int ecc_sw_bch_init(struct nand_device *nand)
 {
-	return NULL;
+	return -EINVAL;
 }
 
-static inline void nand_bch_free(struct nand_bch_control *nbc) {}
+static inline void ecc_sw_bch_cleanup(struct nand_device *nand) {}
 
 #endif /* CONFIG_MTD_NAND_ECC_SW_BCH */
 
diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
index 41d26dc010f2..d4718dcef753 100644
--- a/include/linux/mtd/rawnand.h
+++ b/include/linux/mtd/rawnand.h
@@ -1232,6 +1232,11 @@ static inline int nand_opcode_8bits(unsigned int command)
 	return 0;
 }
 
+int rawnand_sw_bch_init(struct nand_chip *chip);
+int rawnand_sw_bch_correct(struct nand_chip *chip, unsigned char *buf,
+			   unsigned char *read_ecc, unsigned char *calc_ecc);
+void rawnand_sw_bch_cleanup(struct nand_chip *chip);
+
 int nand_check_erased_ecc_chunk(void *data, int datalen,
 				void *ecc, int ecclen,
 				void *extraoob, int extraooblen,
-- 
2.19.1


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

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

* [RFC PATCH 10/27] mtd: rawnand: Get rid of chip->ecc.priv
  2019-02-21 10:01 [RFC PATCH 00/27] Introduce the generic ECC engine abstraction Miquel Raynal
                   ` (8 preceding siblings ...)
  2019-02-21 10:01 ` [RFC PATCH 09/27] mtd: nand: ecc: Turn the software BCH implementation generic Miquel Raynal
@ 2019-02-21 10:01 ` Miquel Raynal
  2019-02-21 13:01   ` Boris Brezillon
  2019-02-21 10:02 ` [RFC PATCH 11/27] mtd: nand: ecc: Move Hamming code into the ecc/ directory Miquel Raynal
  2019-02-21 10:02 ` [RFC PATCH 12/27] mtd: nand: ecc: Use SPDX license identifier for the software Hamming code Miquel Raynal
  11 siblings, 1 reply; 36+ messages in thread
From: Miquel Raynal @ 2019-02-21 10:01 UTC (permalink / raw)
  To: Boris Brezillon, Richard Weinberger, David Woodhouse,
	Brian Norris, Marek Vasut, Tudor Ambarus
  Cc: Vignesh R, Tudor Ambarus, Julien Su, Schrempf Frieder, linux-mtd,
	Thomas Petazzoni, Miquel Raynal, Mason Yang, linux-arm-kernel

nand_ecc_ctrl embeds a private pointer which had only a meaning in the
sunxi driver. This structure will soon be deprecated, but as this
field is actually not needed, let's just drop it.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/sunxi_nand.c | 29 ++++++++++++++---------------
 include/linux/mtd/rawnand.h       |  2 --
 2 files changed, 14 insertions(+), 17 deletions(-)

diff --git a/drivers/mtd/nand/raw/sunxi_nand.c b/drivers/mtd/nand/raw/sunxi_nand.c
index b56f89c992f3..d05f71f33b5c 100644
--- a/drivers/mtd/nand/raw/sunxi_nand.c
+++ b/drivers/mtd/nand/raw/sunxi_nand.c
@@ -230,6 +230,7 @@ static inline struct sunxi_nand_chip *to_sunxi_nand(struct nand_chip *nand)
  *			this NAND controller
  * @complete:		a completion object used to wait for NAND
  *			controller events
+ * @ecc:		ECC controller structure
  */
 struct sunxi_nfc {
 	struct nand_controller controller;
@@ -243,6 +244,7 @@ struct sunxi_nfc {
 	struct list_head chips;
 	struct completion complete;
 	struct dma_chan *dmac;
+	struct sunxi_nand_hw_ecc *ecc;
 };
 
 static inline struct sunxi_nfc *to_sunxi_nfc(struct nand_controller *ctrl)
@@ -774,14 +776,13 @@ static void sunxi_nfc_hw_ecc_enable(struct mtd_info *mtd)
 {
 	struct nand_chip *nand = mtd_to_nand(mtd);
 	struct sunxi_nfc *nfc = to_sunxi_nfc(nand->controller);
-	struct sunxi_nand_hw_ecc *data = nand->ecc.priv;
 	u32 ecc_ctl;
 
 	ecc_ctl = readl(nfc->regs + NFC_REG_ECC_CTL);
 	ecc_ctl &= ~(NFC_ECC_MODE_MSK | NFC_ECC_PIPELINE |
 		     NFC_ECC_BLOCK_SIZE_MSK);
-	ecc_ctl |= NFC_ECC_EN | NFC_ECC_MODE(data->mode) | NFC_ECC_EXCEPTION |
-		   NFC_ECC_PIPELINE;
+	ecc_ctl |= NFC_ECC_EN | NFC_ECC_MODE(nfc->ecc->mode) |
+		   NFC_ECC_EXCEPTION | NFC_ECC_PIPELINE;
 
 	if (nand->ecc.size == 512)
 		ecc_ctl |= NFC_ECC_BLOCK_512;
@@ -1671,9 +1672,9 @@ static const struct mtd_ooblayout_ops sunxi_nand_ooblayout_ops = {
 	.free = sunxi_nand_ooblayout_free,
 };
 
-static void sunxi_nand_hw_ecc_ctrl_cleanup(struct nand_ecc_ctrl *ecc)
+static void sunxi_nand_hw_ecc_ctrl_cleanup(struct sunxi_nfc *nfc)
 {
-	kfree(ecc->priv);
+	kfree(nfc->ecc);
 }
 
 static int sunxi_nand_hw_ecc_ctrl_init(struct mtd_info *mtd,
@@ -1685,7 +1686,6 @@ static int sunxi_nand_hw_ecc_ctrl_init(struct mtd_info *mtd,
 	struct nand_chip *nand = mtd_to_nand(mtd);
 	struct sunxi_nand_chip *sunxi_nand = to_sunxi_nand(nand);
 	struct sunxi_nfc *nfc = to_sunxi_nfc(sunxi_nand->nand.controller);
-	struct sunxi_nand_hw_ecc *data;
 	int nsectors;
 	int ret;
 	int i;
@@ -1722,8 +1722,8 @@ static int sunxi_nand_hw_ecc_ctrl_init(struct mtd_info *mtd,
 	if (ecc->size != 512 && ecc->size != 1024)
 		return -EINVAL;
 
-	data = kzalloc(sizeof(*data), GFP_KERNEL);
-	if (!data)
+	nfc->ecc = kzalloc(sizeof(*nfc->ecc), GFP_KERNEL);
+	if (!nfc->ecc)
 		return -ENOMEM;
 
 	/* Prefer 1k ECC chunk over 512 ones */
@@ -1750,7 +1750,7 @@ static int sunxi_nand_hw_ecc_ctrl_init(struct mtd_info *mtd,
 		goto err;
 	}
 
-	data->mode = i;
+	nfc->ecc->mode = i;
 
 	/* HW ECC always request ECC bytes for 1024 bytes blocks */
 	ecc->bytes = DIV_ROUND_UP(ecc->strength * fls(8 * 1024), 8);
@@ -1768,7 +1768,6 @@ static int sunxi_nand_hw_ecc_ctrl_init(struct mtd_info *mtd,
 	ecc->read_oob = sunxi_nfc_hw_ecc_read_oob;
 	ecc->write_oob = sunxi_nfc_hw_ecc_write_oob;
 	mtd_set_ooblayout(mtd, &sunxi_nand_ooblayout_ops);
-	ecc->priv = data;
 
 	if (nfc->dmac) {
 		ecc->read_page = sunxi_nfc_hw_ecc_read_page_dma;
@@ -1789,16 +1788,16 @@ static int sunxi_nand_hw_ecc_ctrl_init(struct mtd_info *mtd,
 	return 0;
 
 err:
-	kfree(data);
+	kfree(nfc->ecc);
 
 	return ret;
 }
 
-static void sunxi_nand_ecc_cleanup(struct nand_ecc_ctrl *ecc)
+static void sunxi_nand_ecc_cleanup(struct sunxi_nfc *nfc)
 {
-	switch (ecc->mode) {
+	switch (nfc->ecc->mode) {
 	case NAND_ECC_HW:
-		sunxi_nand_hw_ecc_ctrl_cleanup(ecc);
+		sunxi_nand_hw_ecc_ctrl_cleanup(nfc);
 		break;
 	case NAND_ECC_NONE:
 	default:
@@ -1980,7 +1979,7 @@ static void sunxi_nand_chips_cleanup(struct sunxi_nfc *nfc)
 		chip = list_first_entry(&nfc->chips, struct sunxi_nand_chip,
 					node);
 		nand_release(&chip->nand);
-		sunxi_nand_ecc_cleanup(&chip->nand.ecc);
+		sunxi_nand_ecc_cleanup(nfc);
 		list_del(&chip->node);
 	}
 }
diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
index d4718dcef753..afe75e707e2c 100644
--- a/include/linux/mtd/rawnand.h
+++ b/include/linux/mtd/rawnand.h
@@ -279,7 +279,6 @@ static const struct nand_ecc_caps __name = {			\
  * @prepad:	padding information for syndrome based ECC generators
  * @postpad:	padding information for syndrome based ECC generators
  * @options:	ECC specific options (see NAND_ECC_XXX flags defined above)
- * @priv:	pointer to private ECC control data
  * @calc_buf:	buffer for calculated ECC, size is oobsize.
  * @code_buf:	buffer for ECC read from flash, size is oobsize.
  * @hwctl:	function to control hardware ECC generator. Must only
@@ -331,7 +330,6 @@ struct nand_ecc_ctrl {
 	int prepad;
 	int postpad;
 	unsigned int options;
-	void *priv;
 	u8 *calc_buf;
 	u8 *code_buf;
 	void (*hwctl)(struct nand_chip *chip, int mode);
-- 
2.19.1


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

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

* [RFC PATCH 11/27] mtd: nand: ecc: Move Hamming code into the ecc/ directory
  2019-02-21 10:01 [RFC PATCH 00/27] Introduce the generic ECC engine abstraction Miquel Raynal
                   ` (9 preceding siblings ...)
  2019-02-21 10:01 ` [RFC PATCH 10/27] mtd: rawnand: Get rid of chip->ecc.priv Miquel Raynal
@ 2019-02-21 10:02 ` Miquel Raynal
  2019-02-21 10:02 ` [RFC PATCH 12/27] mtd: nand: ecc: Use SPDX license identifier for the software Hamming code Miquel Raynal
  11 siblings, 0 replies; 36+ messages in thread
From: Miquel Raynal @ 2019-02-21 10:02 UTC (permalink / raw)
  To: Boris Brezillon, Richard Weinberger, David Woodhouse,
	Brian Norris, Marek Vasut, Tudor Ambarus
  Cc: Vignesh R, Tudor Ambarus, Julien Su, Schrempf Frieder, linux-mtd,
	Thomas Petazzoni, Miquel Raynal, Mason Yang, linux-arm-kernel

Move generic ECC code in the ecc/ sub-directory for later re-use by
the SPI NAND layer.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 arch/arm/mach-s3c24xx/common-smdk.c                   |  2 +-
 arch/arm/mach-s3c24xx/mach-anubis.c                   |  2 +-
 arch/arm/mach-s3c24xx/mach-at2440evb.c                |  2 +-
 arch/arm/mach-s3c24xx/mach-bast.c                     |  2 +-
 arch/arm/mach-s3c24xx/mach-gta02.c                    |  2 +-
 arch/arm/mach-s3c24xx/mach-jive.c                     |  2 +-
 arch/arm/mach-s3c24xx/mach-mini2440.c                 |  2 +-
 arch/arm/mach-s3c24xx/mach-osiris.c                   |  2 +-
 arch/arm/mach-s3c24xx/mach-qt2410.c                   |  2 +-
 arch/arm/mach-s3c24xx/mach-rx3715.c                   |  2 +-
 arch/arm/mach-s3c24xx/mach-vstms.c                    |  2 +-
 drivers/mtd/nand/ecc/Kconfig                          | 11 +++++++++++
 drivers/mtd/nand/ecc/Makefile                         |  1 +
 .../nand/{raw/nand_ecc.c => ecc/sw-hamming-engine.c}  |  2 +-
 drivers/mtd/nand/raw/Kconfig                          | 11 -----------
 drivers/mtd/nand/raw/Makefile                         |  1 -
 drivers/mtd/nand/raw/cs553x_nand.c                    |  2 +-
 drivers/mtd/nand/raw/fsl_elbc_nand.c                  |  2 +-
 drivers/mtd/nand/raw/fsl_ifc_nand.c                   |  2 +-
 drivers/mtd/nand/raw/fsl_upm.c                        |  2 +-
 drivers/mtd/nand/raw/fsmc_nand.c                      |  2 +-
 drivers/mtd/nand/raw/lpc32xx_mlc.c                    |  2 +-
 drivers/mtd/nand/raw/lpc32xx_slc.c                    |  2 +-
 drivers/mtd/nand/raw/nand_base.c                      |  2 +-
 drivers/mtd/nand/raw/ndfc.c                           |  2 +-
 drivers/mtd/nand/raw/pasemi_nand.c                    |  2 +-
 drivers/mtd/nand/raw/s3c2410.c                        |  2 +-
 drivers/mtd/nand/raw/sharpsl.c                        |  2 +-
 drivers/mtd/nand/raw/tmio_nand.c                      |  2 +-
 drivers/mtd/nand/raw/txx9ndfmc.c                      |  2 +-
 drivers/mtd/sm_ftl.c                                  |  2 +-
 drivers/mtd/tests/mtd_nandecctest.c                   |  2 +-
 .../mtd/{nand_ecc.h => nand-sw-hamming-engine.h}      |  6 +++---
 include/linux/mtd/sharpsl.h                           |  2 +-
 34 files changed, 44 insertions(+), 44 deletions(-)
 rename drivers/mtd/nand/{raw/nand_ecc.c => ecc/sw-hamming-engine.c} (99%)
 rename include/linux/mtd/{nand_ecc.h => nand-sw-hamming-engine.h} (90%)

diff --git a/arch/arm/mach-s3c24xx/common-smdk.c b/arch/arm/mach-s3c24xx/common-smdk.c
index 58e30cad386c..8fe2e62ef730 100644
--- a/arch/arm/mach-s3c24xx/common-smdk.c
+++ b/arch/arm/mach-s3c24xx/common-smdk.c
@@ -19,7 +19,7 @@
 
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/rawnand.h>
-#include <linux/mtd/nand_ecc.h>
+#include <linux/mtd/nand-sw-hamming-engine.h>
 #include <linux/mtd/partitions.h>
 #include <linux/io.h>
 
diff --git a/arch/arm/mach-s3c24xx/mach-anubis.c b/arch/arm/mach-s3c24xx/mach-anubis.c
index 072966dcad78..8f1c505185b4 100644
--- a/arch/arm/mach-s3c24xx/mach-anubis.c
+++ b/arch/arm/mach-s3c24xx/mach-anubis.c
@@ -36,7 +36,7 @@
 
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/rawnand.h>
-#include <linux/mtd/nand_ecc.h>
+#include <linux/mtd/nand-sw-hamming-engine.h>
 #include <linux/mtd/partitions.h>
 
 #include <net/ax88796.h>
diff --git a/arch/arm/mach-s3c24xx/mach-at2440evb.c b/arch/arm/mach-s3c24xx/mach-at2440evb.c
index 58c5ef3cf1d7..94057a8b8778 100644
--- a/arch/arm/mach-s3c24xx/mach-at2440evb.c
+++ b/arch/arm/mach-s3c24xx/mach-at2440evb.c
@@ -37,7 +37,7 @@
 
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/rawnand.h>
-#include <linux/mtd/nand_ecc.h>
+#include <linux/mtd/nand-sw-hamming-engine.h>
 #include <linux/mtd/partitions.h>
 
 #include <plat/devs.h>
diff --git a/arch/arm/mach-s3c24xx/mach-bast.c b/arch/arm/mach-s3c24xx/mach-bast.c
index a7c3955ae8f6..08d88540becf 100644
--- a/arch/arm/mach-s3c24xx/mach-bast.c
+++ b/arch/arm/mach-s3c24xx/mach-bast.c
@@ -24,7 +24,7 @@
 
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/rawnand.h>
-#include <linux/mtd/nand_ecc.h>
+#include <linux/mtd/nand-sw-hamming-engine.h>
 #include <linux/mtd/partitions.h>
 
 #include <linux/platform_data/asoc-s3c24xx_simtec.h>
diff --git a/arch/arm/mach-s3c24xx/mach-gta02.c b/arch/arm/mach-s3c24xx/mach-gta02.c
index 594901f3b8e5..5882b370cbcf 100644
--- a/arch/arm/mach-s3c24xx/mach-gta02.c
+++ b/arch/arm/mach-s3c24xx/mach-gta02.c
@@ -36,7 +36,7 @@
 
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/rawnand.h>
-#include <linux/mtd/nand_ecc.h>
+#include <linux/mtd/nand-sw-hamming-engine.h>
 #include <linux/mtd/partitions.h>
 #include <linux/mtd/physmap.h>
 
diff --git a/arch/arm/mach-s3c24xx/mach-jive.c b/arch/arm/mach-s3c24xx/mach-jive.c
index 885e8f12e4b9..658be0b24fcb 100644
--- a/arch/arm/mach-s3c24xx/mach-jive.c
+++ b/arch/arm/mach-s3c24xx/mach-jive.c
@@ -40,7 +40,7 @@
 
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/rawnand.h>
-#include <linux/mtd/nand_ecc.h>
+#include <linux/mtd/nand-sw-hamming-engine.h>
 #include <linux/mtd/partitions.h>
 
 #include <plat/gpio-cfg.h>
diff --git a/arch/arm/mach-s3c24xx/mach-mini2440.c b/arch/arm/mach-s3c24xx/mach-mini2440.c
index 9035f868fb34..abbdc44dfb8f 100644
--- a/arch/arm/mach-s3c24xx/mach-mini2440.c
+++ b/arch/arm/mach-s3c24xx/mach-mini2440.c
@@ -46,7 +46,7 @@
 
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/rawnand.h>
-#include <linux/mtd/nand_ecc.h>
+#include <linux/mtd/nand-sw-hamming-engine.h>
 #include <linux/mtd/partitions.h>
 
 #include <plat/gpio-cfg.h>
diff --git a/arch/arm/mach-s3c24xx/mach-osiris.c b/arch/arm/mach-s3c24xx/mach-osiris.c
index ee3630cb236a..c00ec4d851b7 100644
--- a/arch/arm/mach-s3c24xx/mach-osiris.c
+++ b/arch/arm/mach-s3c24xx/mach-osiris.c
@@ -33,7 +33,7 @@
 
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/rawnand.h>
-#include <linux/mtd/nand_ecc.h>
+#include <linux/mtd/nand-sw-hamming-engine.h>
 #include <linux/mtd/partitions.h>
 
 #include <plat/cpu.h>
diff --git a/arch/arm/mach-s3c24xx/mach-qt2410.c b/arch/arm/mach-s3c24xx/mach-qt2410.c
index 5d48e5b6e738..9d6e92b51e60 100644
--- a/arch/arm/mach-s3c24xx/mach-qt2410.c
+++ b/arch/arm/mach-s3c24xx/mach-qt2410.c
@@ -21,7 +21,7 @@
 #include <linux/io.h>
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/rawnand.h>
-#include <linux/mtd/nand_ecc.h>
+#include <linux/mtd/nand-sw-hamming-engine.h>
 #include <linux/mtd/partitions.h>
 
 #include <asm/mach/arch.h>
diff --git a/arch/arm/mach-s3c24xx/mach-rx3715.c b/arch/arm/mach-s3c24xx/mach-rx3715.c
index 529c6faf862f..f9668f4634dc 100644
--- a/arch/arm/mach-s3c24xx/mach-rx3715.c
+++ b/arch/arm/mach-s3c24xx/mach-rx3715.c
@@ -22,7 +22,7 @@
 #include <linux/io.h>
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/rawnand.h>
-#include <linux/mtd/nand_ecc.h>
+#include <linux/mtd/nand-sw-hamming-engine.h>
 #include <linux/mtd/partitions.h>
 
 #include <asm/mach/arch.h>
diff --git a/arch/arm/mach-s3c24xx/mach-vstms.c b/arch/arm/mach-s3c24xx/mach-vstms.c
index d76b28b65e65..94f6d5ce0246 100644
--- a/arch/arm/mach-s3c24xx/mach-vstms.c
+++ b/arch/arm/mach-s3c24xx/mach-vstms.c
@@ -16,7 +16,7 @@
 #include <linux/io.h>
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/rawnand.h>
-#include <linux/mtd/nand_ecc.h>
+#include <linux/mtd/nand-sw-hamming-engine.h>
 #include <linux/mtd/partitions.h>
 #include <linux/memblock.h>
 
diff --git a/drivers/mtd/nand/ecc/Kconfig b/drivers/mtd/nand/ecc/Kconfig
index a05f1d474446..6ce9007e1d9b 100644
--- a/drivers/mtd/nand/ecc/Kconfig
+++ b/drivers/mtd/nand/ecc/Kconfig
@@ -1,5 +1,16 @@
 menu "ECC engine support"
 
+config MTD_NAND_ECC_SW_HAMMING
+	tristate
+
+config MTD_NAND_ECC_SW_HAMMING_SMC
+	bool "NAND ECC Smart Media byte order"
+	depends on MTD_NAND_ECC_SW_HAMMING
+	default n
+	help
+	  Software ECC according to the Smart Media Specification.
+	  The original Linux implementation had byte 0 and 1 swapped.
+
 config MTD_NAND_ECC_SW_BCH
 	bool "Software BCH ECC engine"
 	select BCH
diff --git a/drivers/mtd/nand/ecc/Makefile b/drivers/mtd/nand/ecc/Makefile
index fdbfc2bc38d7..2215d7aa8d8d 100644
--- a/drivers/mtd/nand/ecc/Makefile
+++ b/drivers/mtd/nand/ecc/Makefile
@@ -1,4 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0
 
 obj-$(CONFIG_MTD_NAND_CORE) += engine.o
+obj-$(CONFIG_MTD_NAND_ECC_SW_HAMMING)	+= sw-hamming-engine.o
 obj-$(CONFIG_MTD_NAND_ECC_SW_BCH)	+= sw-bch-engine.o
diff --git a/drivers/mtd/nand/raw/nand_ecc.c b/drivers/mtd/nand/ecc/sw-hamming-engine.c
similarity index 99%
rename from drivers/mtd/nand/raw/nand_ecc.c
rename to drivers/mtd/nand/ecc/sw-hamming-engine.c
index 4f4347533058..9abef611456f 100644
--- a/drivers/mtd/nand/raw/nand_ecc.c
+++ b/drivers/mtd/nand/ecc/sw-hamming-engine.c
@@ -33,7 +33,7 @@
 #include <linux/module.h>
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/rawnand.h>
-#include <linux/mtd/nand_ecc.h>
+#include <linux/mtd/nand-sw-hamming-engine.h>
 #include <asm/byteorder.h>
 
 /*
diff --git a/drivers/mtd/nand/raw/Kconfig b/drivers/mtd/nand/raw/Kconfig
index 478bc3f3c042..995fa78bdedb 100644
--- a/drivers/mtd/nand/raw/Kconfig
+++ b/drivers/mtd/nand/raw/Kconfig
@@ -1,14 +1,3 @@
-config MTD_NAND_ECC_SW_HAMMING
-	tristate
-
-config MTD_NAND_ECC_SW_HAMMING_SMC
-	bool "NAND ECC Smart Media byte order"
-	depends on MTD_NAND_ECC_SW_HAMMING
-	default n
-	help
-	  Software ECC according to the Smart Media Specification.
-	  The original Linux implementation had byte 0 and 1 swapped.
-
 menuconfig MTD_RAW_NAND
 	tristate "Raw/Parallel NAND Device Support"
 	select MTD_NAND_ECC_SW_HAMMING
diff --git a/drivers/mtd/nand/raw/Makefile b/drivers/mtd/nand/raw/Makefile
index 18ae42077b05..a5ce9cec8dd3 100644
--- a/drivers/mtd/nand/raw/Makefile
+++ b/drivers/mtd/nand/raw/Makefile
@@ -1,7 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0
 
 obj-$(CONFIG_MTD_RAW_NAND)		+= nand.o
-obj-$(CONFIG_MTD_NAND_ECC_SW_HAMMING)	+= nand_ecc.o
 obj-$(CONFIG_MTD_SM_COMMON) 		+= sm_common.o
 
 obj-$(CONFIG_MTD_NAND_CAFE)		+= cafe_nand.o
diff --git a/drivers/mtd/nand/raw/cs553x_nand.c b/drivers/mtd/nand/raw/cs553x_nand.c
index c6f578aff5d9..19596ba72ac6 100644
--- a/drivers/mtd/nand/raw/cs553x_nand.c
+++ b/drivers/mtd/nand/raw/cs553x_nand.c
@@ -23,7 +23,7 @@
 #include <linux/delay.h>
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/rawnand.h>
-#include <linux/mtd/nand_ecc.h>
+#include <linux/mtd/nand-sw-hamming-engine.h>
 #include <linux/mtd/partitions.h>
 
 #include <asm/msr.h>
diff --git a/drivers/mtd/nand/raw/fsl_elbc_nand.c b/drivers/mtd/nand/raw/fsl_elbc_nand.c
index 293a5b71833a..3aa88346d7cc 100644
--- a/drivers/mtd/nand/raw/fsl_elbc_nand.c
+++ b/drivers/mtd/nand/raw/fsl_elbc_nand.c
@@ -35,7 +35,7 @@
 
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/rawnand.h>
-#include <linux/mtd/nand_ecc.h>
+#include <linux/mtd/nand-sw-hamming-engine.h>
 #include <linux/mtd/partitions.h>
 
 #include <asm/io.h>
diff --git a/drivers/mtd/nand/raw/fsl_ifc_nand.c b/drivers/mtd/nand/raw/fsl_ifc_nand.c
index 04a3dcd675bf..3815487bcddd 100644
--- a/drivers/mtd/nand/raw/fsl_ifc_nand.c
+++ b/drivers/mtd/nand/raw/fsl_ifc_nand.c
@@ -28,7 +28,7 @@
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/rawnand.h>
 #include <linux/mtd/partitions.h>
-#include <linux/mtd/nand_ecc.h>
+#include <linux/mtd/nand-sw-hamming-engine.h>
 #include <linux/fsl_ifc.h>
 #include <linux/iopoll.h>
 
diff --git a/drivers/mtd/nand/raw/fsl_upm.c b/drivers/mtd/nand/raw/fsl_upm.c
index 5ccc28ec0985..c60bd00d9f2b 100644
--- a/drivers/mtd/nand/raw/fsl_upm.c
+++ b/drivers/mtd/nand/raw/fsl_upm.c
@@ -15,7 +15,7 @@
 #include <linux/module.h>
 #include <linux/delay.h>
 #include <linux/mtd/rawnand.h>
-#include <linux/mtd/nand_ecc.h>
+#include <linux/mtd/nand-sw-hamming-engine.h>
 #include <linux/mtd/partitions.h>
 #include <linux/mtd/mtd.h>
 #include <linux/of_address.h>
diff --git a/drivers/mtd/nand/raw/fsmc_nand.c b/drivers/mtd/nand/raw/fsmc_nand.c
index 325b4414dccc..9d11007b1ee5 100644
--- a/drivers/mtd/nand/raw/fsmc_nand.c
+++ b/drivers/mtd/nand/raw/fsmc_nand.c
@@ -26,7 +26,7 @@
 #include <linux/types.h>
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/rawnand.h>
-#include <linux/mtd/nand_ecc.h>
+#include <linux/mtd/nand-sw-hamming-engine.h>
 #include <linux/platform_device.h>
 #include <linux/of.h>
 #include <linux/mtd/partitions.h>
diff --git a/drivers/mtd/nand/raw/lpc32xx_mlc.c b/drivers/mtd/nand/raw/lpc32xx_mlc.c
index 086964f8d424..b821dc083542 100644
--- a/drivers/mtd/nand/raw/lpc32xx_mlc.c
+++ b/drivers/mtd/nand/raw/lpc32xx_mlc.c
@@ -41,7 +41,7 @@
 #include <linux/mm.h>
 #include <linux/dma-mapping.h>
 #include <linux/dmaengine.h>
-#include <linux/mtd/nand_ecc.h>
+#include <linux/mtd/nand-sw-hamming-engine.h>
 
 #define DRV_NAME "lpc32xx_mlc"
 
diff --git a/drivers/mtd/nand/raw/lpc32xx_slc.c b/drivers/mtd/nand/raw/lpc32xx_slc.c
index a2c5fdc875bd..447c23d4f3b4 100644
--- a/drivers/mtd/nand/raw/lpc32xx_slc.c
+++ b/drivers/mtd/nand/raw/lpc32xx_slc.c
@@ -32,7 +32,7 @@
 #include <linux/mm.h>
 #include <linux/dma-mapping.h>
 #include <linux/dmaengine.h>
-#include <linux/mtd/nand_ecc.h>
+#include <linux/mtd/nand-sw-hamming-engine.h>
 #include <linux/gpio.h>
 #include <linux/of.h>
 #include <linux/of_gpio.h>
diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index 64a6e9b840a4..395a9bde5e15 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -39,7 +39,7 @@
 #include <linux/types.h>
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/nand.h>
-#include <linux/mtd/nand_ecc.h>
+#include <linux/mtd/nand-sw-hamming-engine.h>
 #include <linux/mtd/nand-sw-bch-engine.h>
 #include <linux/interrupt.h>
 #include <linux/bitops.h>
diff --git a/drivers/mtd/nand/raw/ndfc.c b/drivers/mtd/nand/raw/ndfc.c
index 9857e0e5acd4..d552b727b821 100644
--- a/drivers/mtd/nand/raw/ndfc.c
+++ b/drivers/mtd/nand/raw/ndfc.c
@@ -23,7 +23,7 @@
  */
 #include <linux/module.h>
 #include <linux/mtd/rawnand.h>
-#include <linux/mtd/nand_ecc.h>
+#include <linux/mtd/nand-sw-hamming-engine.h>
 #include <linux/mtd/partitions.h>
 #include <linux/mtd/ndfc.h>
 #include <linux/slab.h>
diff --git a/drivers/mtd/nand/raw/pasemi_nand.c b/drivers/mtd/nand/raw/pasemi_nand.c
index 643cd22af009..3944dc653609 100644
--- a/drivers/mtd/nand/raw/pasemi_nand.c
+++ b/drivers/mtd/nand/raw/pasemi_nand.c
@@ -26,7 +26,7 @@
 #include <linux/module.h>
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/rawnand.h>
-#include <linux/mtd/nand_ecc.h>
+#include <linux/mtd/nand-sw-hamming-engine.h>
 #include <linux/of_address.h>
 #include <linux/of_irq.h>
 #include <linux/of_platform.h>
diff --git a/drivers/mtd/nand/raw/s3c2410.c b/drivers/mtd/nand/raw/s3c2410.c
index adc7a196e383..8d79813e83d5 100644
--- a/drivers/mtd/nand/raw/s3c2410.c
+++ b/drivers/mtd/nand/raw/s3c2410.c
@@ -43,7 +43,7 @@
 
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/rawnand.h>
-#include <linux/mtd/nand_ecc.h>
+#include <linux/mtd/nand-sw-hamming-engine.h>
 #include <linux/mtd/partitions.h>
 
 #include <linux/platform_data/mtd-nand-s3c2410.h>
diff --git a/drivers/mtd/nand/raw/sharpsl.c b/drivers/mtd/nand/raw/sharpsl.c
index c82f26c8b58c..32918a7ed6c4 100644
--- a/drivers/mtd/nand/raw/sharpsl.c
+++ b/drivers/mtd/nand/raw/sharpsl.c
@@ -16,7 +16,7 @@
 #include <linux/delay.h>
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/rawnand.h>
-#include <linux/mtd/nand_ecc.h>
+#include <linux/mtd/nand-sw-hamming-engine.h>
 #include <linux/mtd/partitions.h>
 #include <linux/mtd/sharpsl.h>
 #include <linux/interrupt.h>
diff --git a/drivers/mtd/nand/raw/tmio_nand.c b/drivers/mtd/nand/raw/tmio_nand.c
index f3b59e649b7d..63f5100cf70b 100644
--- a/drivers/mtd/nand/raw/tmio_nand.c
+++ b/drivers/mtd/nand/raw/tmio_nand.c
@@ -35,7 +35,7 @@
 #include <linux/ioport.h>
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/rawnand.h>
-#include <linux/mtd/nand_ecc.h>
+#include <linux/mtd/nand-sw-hamming-engine.h>
 #include <linux/mtd/partitions.h>
 #include <linux/slab.h>
 
diff --git a/drivers/mtd/nand/raw/txx9ndfmc.c b/drivers/mtd/nand/raw/txx9ndfmc.c
index ddf0420c0997..f87f598b7688 100644
--- a/drivers/mtd/nand/raw/txx9ndfmc.c
+++ b/drivers/mtd/nand/raw/txx9ndfmc.c
@@ -17,7 +17,7 @@
 #include <linux/delay.h>
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/rawnand.h>
-#include <linux/mtd/nand_ecc.h>
+#include <linux/mtd/nand-sw-hamming-engine.h>
 #include <linux/mtd/partitions.h>
 #include <linux/io.h>
 #include <linux/platform_data/txx9/ndfmc.h>
diff --git a/drivers/mtd/sm_ftl.c b/drivers/mtd/sm_ftl.c
index e0955a98a0f4..57a7d98d6a84 100644
--- a/drivers/mtd/sm_ftl.c
+++ b/drivers/mtd/sm_ftl.c
@@ -16,7 +16,7 @@
 #include <linux/sysfs.h>
 #include <linux/bitops.h>
 #include <linux/slab.h>
-#include <linux/mtd/nand_ecc.h>
+#include <linux/mtd/nand-sw-hamming-engine.h>
 #include "nand/raw/sm_common.h"
 #include "sm_ftl.h"
 
diff --git a/drivers/mtd/tests/mtd_nandecctest.c b/drivers/mtd/tests/mtd_nandecctest.c
index 73b06304c975..95a94643f955 100644
--- a/drivers/mtd/tests/mtd_nandecctest.c
+++ b/drivers/mtd/tests/mtd_nandecctest.c
@@ -7,7 +7,7 @@
 #include <linux/string.h>
 #include <linux/bitops.h>
 #include <linux/slab.h>
-#include <linux/mtd/nand_ecc.h>
+#include <linux/mtd/nand-sw-hamming-engine.h>
 
 #include "mtd_test.h"
 
diff --git a/include/linux/mtd/nand_ecc.h b/include/linux/mtd/nand-sw-hamming-engine.h
similarity index 90%
rename from include/linux/mtd/nand_ecc.h
rename to include/linux/mtd/nand-sw-hamming-engine.h
index 0b3bb156c344..c0f015b1a077 100644
--- a/include/linux/mtd/nand_ecc.h
+++ b/include/linux/mtd/nand-sw-hamming-engine.h
@@ -10,8 +10,8 @@
  * This file is the header for the ECC algorithm.
  */
 
-#ifndef __MTD_NAND_ECC_H__
-#define __MTD_NAND_ECC_H__
+#ifndef __MTD_NAND_ECC_SW_HAMMING_H__
+#define __MTD_NAND_ECC_SW_HAMMING_H__
 
 struct nand_chip;
 
@@ -39,4 +39,4 @@ int __nand_correct_data(u_char *dat, u_char *read_ecc, u_char *calc_ecc,
 int nand_correct_data(struct nand_chip *chip, u_char *dat, u_char *read_ecc,
 		      u_char *calc_ecc);
 
-#endif /* __MTD_NAND_ECC_H__ */
+#endif /* __MTD_NAND_ECC_SW_HAMMING_H__ */
diff --git a/include/linux/mtd/sharpsl.h b/include/linux/mtd/sharpsl.h
index e1845fc4afbd..77528c2e0a80 100644
--- a/include/linux/mtd/sharpsl.h
+++ b/include/linux/mtd/sharpsl.h
@@ -9,7 +9,7 @@
  */
 
 #include <linux/mtd/rawnand.h>
-#include <linux/mtd/nand_ecc.h>
+#include <linux/mtd/nand-sw-hamming-engine.h>
 #include <linux/mtd/partitions.h>
 
 struct sharpsl_nand_platform_data {
-- 
2.19.1


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

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

* [RFC PATCH 12/27] mtd: nand: ecc: Use SPDX license identifier for the software Hamming code
  2019-02-21 10:01 [RFC PATCH 00/27] Introduce the generic ECC engine abstraction Miquel Raynal
                   ` (10 preceding siblings ...)
  2019-02-21 10:02 ` [RFC PATCH 11/27] mtd: nand: ecc: Move Hamming code into the ecc/ directory Miquel Raynal
@ 2019-02-21 10:02 ` Miquel Raynal
  11 siblings, 0 replies; 36+ messages in thread
From: Miquel Raynal @ 2019-02-21 10:02 UTC (permalink / raw)
  To: Boris Brezillon, Richard Weinberger, David Woodhouse,
	Brian Norris, Marek Vasut, Tudor Ambarus
  Cc: Vignesh R, Tudor Ambarus, Julien Su, Schrempf Frieder, linux-mtd,
	Thomas Petazzoni, Miquel Raynal, Mason Yang, linux-arm-kernel

Remove the legacy license text, also update the MODULE_LICENSE macro
in accordance with the text at the top of the file (GPL v2 or
higher).

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/ecc/sw-hamming-engine.c   | 18 ++----------------
 include/linux/mtd/nand-sw-hamming-engine.h |  5 +----
 2 files changed, 3 insertions(+), 20 deletions(-)

diff --git a/drivers/mtd/nand/ecc/sw-hamming-engine.c b/drivers/mtd/nand/ecc/sw-hamming-engine.c
index 9abef611456f..6ba0205f1b4f 100644
--- a/drivers/mtd/nand/ecc/sw-hamming-engine.c
+++ b/drivers/mtd/nand/ecc/sw-hamming-engine.c
@@ -1,3 +1,4 @@
+// SPDX-License-Identifier: GPL-2.0+
 /*
  * This file contains an ECC algorithm that detects and corrects 1 bit
  * errors in a 256 byte block of data.
@@ -11,21 +12,6 @@
  *
  * Information on how this algorithm works and how it was developed
  * can be found in Documentation/mtd/nand_ecc.txt
- *
- * This file is free software; you can redistribute it and/or modify it
- * under the terms of the GNU General Public License as published by the
- * Free Software Foundation; either version 2 or (at your option) any
- * later version.
- *
- * This file is distributed in the hope that it will be useful, but WITHOUT
- * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
- * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
- * for more details.
- *
- * You should have received a copy of the GNU General Public License along
- * with this file; if not, write to the Free Software Foundation, Inc.,
- * 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
- *
  */
 
 #include <linux/types.h>
@@ -493,6 +479,6 @@ int nand_correct_data(struct nand_chip *chip, unsigned char *buf,
 }
 EXPORT_SYMBOL(nand_correct_data);
 
-MODULE_LICENSE("GPL");
+MODULE_LICENSE("GPL v2");
 MODULE_AUTHOR("Frans Meulenbroeks <fransmeulenbroeks@gmail.com>");
 MODULE_DESCRIPTION("Generic NAND ECC support");
diff --git a/include/linux/mtd/nand-sw-hamming-engine.h b/include/linux/mtd/nand-sw-hamming-engine.h
index c0f015b1a077..1bd7493c682b 100644
--- a/include/linux/mtd/nand-sw-hamming-engine.h
+++ b/include/linux/mtd/nand-sw-hamming-engine.h
@@ -1,12 +1,9 @@
+/* SPDX-License-Identifier: GPL-2.0 */
 /*
  *  Copyright (C) 2000-2010 Steven J. Hill <sjhill@realitydiluted.com>
  *			    David Woodhouse <dwmw2@infradead.org>
  *			    Thomas Gleixner <tglx@linutronix.de>
  *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- *
  * This file is the header for the ECC algorithm.
  */
 
-- 
2.19.1


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

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

* Re: [RFC PATCH 02/27] mtd: nand: Compile in the NAND core by default
  2019-02-21 10:01 ` [RFC PATCH 02/27] mtd: nand: Compile in the NAND core by default Miquel Raynal
@ 2019-02-21 10:55   ` Boris Brezillon
  2019-02-21 11:06     ` Miquel Raynal
  0 siblings, 1 reply; 36+ messages in thread
From: Boris Brezillon @ 2019-02-21 10:55 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Mason Yang, Vignesh R, Tudor Ambarus, Julien Su,
	Richard Weinberger, Schrempf Frieder, Marek Vasut, linux-mtd,
	Thomas Petazzoni, Brian Norris, David Woodhouse,
	linux-arm-kernel

On Thu, 21 Feb 2019 11:01:51 +0100
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> Force the NAND core be compiled-in when using any kind of NAND.

Why?

> 
> Also remove the redundant dependencies on MTD which is enforced by the
> game of the if/endif blocs.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/mtd/nand/Kconfig         | 12 ++++++++++--
>  drivers/mtd/nand/onenand/Kconfig |  1 -
>  drivers/mtd/nand/raw/Kconfig     |  1 -
>  drivers/mtd/nand/spi/Kconfig     |  1 -
>  4 files changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
> index 495751ed3fd7..e8d26a715922 100644
> --- a/drivers/mtd/nand/Kconfig
> +++ b/drivers/mtd/nand/Kconfig
> @@ -1,6 +1,14 @@
> -config MTD_NAND_CORE
> -	tristate
> +menuconfig MTD_NAND_CORE
> +	tristate "NAND"

Definitely not something we want to expose in menuconfig.

> +	default y

I don't see what's the problem with letting SPINAND, RAWNAND and
ONENAND select this options if they actually need it.

> +	help
> +	  Embed the NAND core. This is the generic part between raw
> +	  NAND, SPI NAND and Onenand.
> +
> +if MTD_NAND_CORE
>  
>  source "drivers/mtd/nand/onenand/Kconfig"
>  source "drivers/mtd/nand/raw/Kconfig"
>  source "drivers/mtd/nand/spi/Kconfig"
> +
> +endif
> diff --git a/drivers/mtd/nand/onenand/Kconfig b/drivers/mtd/nand/onenand/Kconfig
> index 9dc15748947b..c168f3b4b296 100644
> --- a/drivers/mtd/nand/onenand/Kconfig
> +++ b/drivers/mtd/nand/onenand/Kconfig
> @@ -1,6 +1,5 @@
>  menuconfig MTD_ONENAND
>  	tristate "OneNAND Device Support"
> -	depends on MTD
>  	depends on HAS_IOMEM
>  	help
>  	  This enables support for accessing all type of OneNAND flash
> diff --git a/drivers/mtd/nand/raw/Kconfig b/drivers/mtd/nand/raw/Kconfig
> index 510a6b32820d..ebb8a3da9fa5 100644
> --- a/drivers/mtd/nand/raw/Kconfig
> +++ b/drivers/mtd/nand/raw/Kconfig
> @@ -11,7 +11,6 @@ config MTD_NAND_ECC_SW_HAMMING_SMC
>  
>  menuconfig MTD_RAW_NAND
>  	tristate "Raw/Parallel NAND Device Support"
> -	depends on MTD
>  	select MTD_NAND_ECC_SW_HAMMING
>  	help
>  	  This enables support for accessing all type of raw/parallel
> diff --git a/drivers/mtd/nand/spi/Kconfig b/drivers/mtd/nand/spi/Kconfig
> index 7c37d2929b68..7631050610ab 100644
> --- a/drivers/mtd/nand/spi/Kconfig
> +++ b/drivers/mtd/nand/spi/Kconfig
> @@ -1,6 +1,5 @@
>  menuconfig MTD_SPI_NAND
>  	tristate "SPI NAND device Support"
> -	select MTD_NAND_CORE
>  	depends on SPI_MASTER
>  	select SPI_MEM
>  	help


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

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

* Re: [RFC PATCH 02/27] mtd: nand: Compile in the NAND core by default
  2019-02-21 10:55   ` Boris Brezillon
@ 2019-02-21 11:06     ` Miquel Raynal
  2019-02-21 11:14       ` Boris Brezillon
  0 siblings, 1 reply; 36+ messages in thread
From: Miquel Raynal @ 2019-02-21 11:06 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: Mason Yang, Vignesh R, Tudor Ambarus, Julien Su,
	Richard Weinberger, Schrempf Frieder, Marek Vasut, linux-mtd,
	Thomas Petazzoni, Brian Norris, David Woodhouse,
	linux-arm-kernel

Hi Boris,

Boris Brezillon <bbrezillon@kernel.org> wrote on Thu, 21 Feb 2019
11:55:54 +0100:

> On Thu, 21 Feb 2019 11:01:51 +0100
> Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> 
> > Force the NAND core be compiled-in when using any kind of NAND.  
> 
> Why?

Because all the logic that comes later in the series relies on this
change. I need the NAND core to be compiled-in, as well the ECC engine
core, as well as everything that is shared between raw NAND and
SPI-NAND which I move in the NAND core.

> 
> > 
> > Also remove the redundant dependencies on MTD which is enforced by the
> > game of the if/endif blocs.
> > 
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
> >  drivers/mtd/nand/Kconfig         | 12 ++++++++++--
> >  drivers/mtd/nand/onenand/Kconfig |  1 -
> >  drivers/mtd/nand/raw/Kconfig     |  1 -
> >  drivers/mtd/nand/spi/Kconfig     |  1 -
> >  4 files changed, 10 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
> > index 495751ed3fd7..e8d26a715922 100644
> > --- a/drivers/mtd/nand/Kconfig
> > +++ b/drivers/mtd/nand/Kconfig
> > @@ -1,6 +1,14 @@
> > -config MTD_NAND_CORE
> > -	tristate
> > +menuconfig MTD_NAND_CORE
> > +	tristate "NAND"  
> 
> Definitely not something we want to expose in menuconfig.

I considered the NAND core as essential when using raw NAND and
SPI-NAND. Hence, turning it into a menuconfig option make the whole
NAND subsystem available (or not) and will appear in a different
"menuconfig page", which really enhances the readability.

> 
> > +	default y  
> 
> I don't see what's the problem with letting SPINAND, RAWNAND and
> ONENAND select this options if they actually need it.

See the reason above, hence having it "default y" is needed to do not
break existing defconfigs.


Thanks,
Miquèl

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

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

* Re: [RFC PATCH 02/27] mtd: nand: Compile in the NAND core by default
  2019-02-21 11:06     ` Miquel Raynal
@ 2019-02-21 11:14       ` Boris Brezillon
  2019-02-21 11:46         ` Miquel Raynal
  0 siblings, 1 reply; 36+ messages in thread
From: Boris Brezillon @ 2019-02-21 11:14 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Mason Yang, Vignesh R, Tudor Ambarus, Julien Su,
	Richard Weinberger, Schrempf Frieder, Marek Vasut, linux-mtd,
	Thomas Petazzoni, Brian Norris, David Woodhouse,
	linux-arm-kernel

On Thu, 21 Feb 2019 12:06:10 +0100
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> Hi Boris,
> 
> Boris Brezillon <bbrezillon@kernel.org> wrote on Thu, 21 Feb 2019
> 11:55:54 +0100:
> 
> > On Thu, 21 Feb 2019 11:01:51 +0100
> > Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >   
> > > Force the NAND core be compiled-in when using any kind of NAND.    
> > 
> > Why?  
> 
> Because all the logic that comes later in the series relies on this
> change. I need the NAND core to be compiled-in,

Hm, not exactly compiled-in as it can still be selected as a module.

> as well the ECC engine
> core, as well as everything that is shared between raw NAND and
> SPI-NAND which I move in the NAND core.

Yes, the core is required for SPI NAND and soon will be for RAW NAND,
but I still don't see the problem with the "select NAND_CORE" approach,
sorry.

> 
> >   
> > > 
> > > Also remove the redundant dependencies on MTD which is enforced by the
> > > game of the if/endif blocs.
> > > 
> > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > > ---
> > >  drivers/mtd/nand/Kconfig         | 12 ++++++++++--
> > >  drivers/mtd/nand/onenand/Kconfig |  1 -
> > >  drivers/mtd/nand/raw/Kconfig     |  1 -
> > >  drivers/mtd/nand/spi/Kconfig     |  1 -
> > >  4 files changed, 10 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
> > > index 495751ed3fd7..e8d26a715922 100644
> > > --- a/drivers/mtd/nand/Kconfig
> > > +++ b/drivers/mtd/nand/Kconfig
> > > @@ -1,6 +1,14 @@
> > > -config MTD_NAND_CORE
> > > -	tristate
> > > +menuconfig MTD_NAND_CORE
> > > +	tristate "NAND"    
> > 
> > Definitely not something we want to expose in menuconfig.  
> 
> I considered the NAND core as essential when using raw NAND and
> SPI-NAND.

It is needed for SPI NAND, and will be for raw NAND for sure, but I
still see no reasons for turning it into a user-visible option.

> Hence, turning it into a menuconfig option make the whole
> NAND subsystem available (or not) and will appear in a different
> "menuconfig page", which really enhances the readability.

That's a different thing, and I have no problem with adding an extra
level in the Kconfig hierarchy.

> 
> >   
> > > +	default y    
> > 
> > I don't see what's the problem with letting SPINAND, RAWNAND and
> > ONENAND select this options if they actually need it.  
> 
> See the reason above, hence having it "default y" is needed to do not
> break existing defconfigs.

Not if you make RAW_NAND select NAND_CORE.

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

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

* Re: [RFC PATCH 03/27] mtd: nand: Introduce the ECC engine abstraction
  2019-02-21 10:01 ` [RFC PATCH 03/27] mtd: nand: Introduce the ECC engine abstraction Miquel Raynal
@ 2019-02-21 11:16   ` Boris Brezillon
  2019-02-27  9:26     ` Miquel Raynal
  2019-02-25 18:55   ` Boris Brezillon
  1 sibling, 1 reply; 36+ messages in thread
From: Boris Brezillon @ 2019-02-21 11:16 UTC (permalink / raw)
  To: Miquel Raynal, linux-arm-kernel, Mason Yang
  Cc: Vignesh R, Tudor Ambarus, Julien Su, Richard Weinberger,
	Schrempf Frieder, Marek Vasut, linux-mtd, Thomas Petazzoni,
	Brian Norris, David Woodhouse

On Thu, 21 Feb 2019 11:01:52 +0100
Miquel Raynal <miquel.raynal@bootlin.com> wrote:


> diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
> index 30f0fb02abe2..9e3b018d0b83 100644
> --- a/include/linux/mtd/nand.h
> +++ b/include/linux/mtd/nand.h
> @@ -82,8 +82,14 @@ struct nand_pos {
>  	unsigned int page;
>  };
>  
> +enum nand_page_io_req_type {
> +	NAND_PAGE_READ = 0,
> +	NAND_PAGE_WRITE,
> +};
> +
>  /**
>   * struct nand_page_io_req - NAND I/O request object
> + * @type: the type of page I/O: read or write
>   * @pos: the position this I/O request is targeting
>   * @dataoffs: the offset within the page
>   * @datalen: number of data bytes to read from/write to this page
> @@ -99,6 +105,7 @@ struct nand_pos {
>   * specific commands/operations.
>   */
>  struct nand_page_io_req {
> +	enum nand_page_io_req_type type;

Can you add the reqtype enum and type field (+ patch the iterator
helpers) in a separate patch?

>  	struct nand_pos pos;
>  	unsigned int dataoffs;
>  	unsigned int datalen;
> @@ -116,13 +123,35 @@ struct nand_page_io_req {
>  };
>  
>  /**
> - * struct nand_ecc_req - NAND ECC requirements
> + * struct nand_ecc_conf - NAND ECC configuration
> + * @strength: ECC strength
> + * @step_size: Number of bytes per step
> + * @total: Total number of bytes used for storing ECC codes, this is used by
> + *         generic OOB layouts
> + */
> +struct nand_ecc_conf {

Please do the s/nand_ecc_req/nand_ecc_conf/ in a separate patch.

> +	unsigned int strength;
> +	unsigned int step_size;
> +	unsigned int total;

Do we really need to add this total field here? Looks like something
that should be kept private to the ECC engine implementation.

> +};
> +
> +/**
> + * struct nand_ecc_user_conf - User desired ECC configuration
> + * @mode: ECC mode
> + * @algo: ECC algorithm
>   * @strength: ECC strength
>   * @step_size: ECC step/block size
> + * @maximize: ECC parameters must be maximized depending on the device
> + *            capabilities
> + * @flags: User flags
>   */
> -struct nand_ecc_req {
> +struct nand_ecc_user_conf {
> +	int mode;

We should definitely name that one differently ('provider' maybe).

> +	unsigned int algo;
>  	unsigned int strength;
>  	unsigned int step_size;
> +	unsigned int maximize;
> +	unsigned int flags;

maximize could be a flag.

>  };
>  
>  #define NAND_ECCREQ(str, stp) { .strength = (str), .step_size = (stp) }
> @@ -157,11 +186,76 @@ struct nand_ops {
>  	bool (*isbad)(struct nand_device *nand, const struct nand_pos *pos);
>  };


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

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

* Re: [RFC PATCH 05/27] mtd: nand: Move standard OOB layouts to the NAND core
  2019-02-21 10:01 ` [RFC PATCH 05/27] mtd: nand: Move standard OOB layouts to the NAND core Miquel Raynal
@ 2019-02-21 11:19   ` Boris Brezillon
  2019-02-21 11:47     ` Miquel Raynal
  0 siblings, 1 reply; 36+ messages in thread
From: Boris Brezillon @ 2019-02-21 11:19 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Mason Yang, Vignesh R, Tudor Ambarus, Julien Su,
	Richard Weinberger, Schrempf Frieder, Marek Vasut, linux-mtd,
	Thomas Petazzoni, Brian Norris, David Woodhouse,
	linux-arm-kernel

On Thu, 21 Feb 2019 11:01:54 +0100
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> These OOB layouts are generic to all NAND chips, the should not be
> restricted to be used only by raw NAND controller drivers. They might
> later be used by generic ECC engines and SPI-NAND devices as well.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/mtd/nand/core.c             | 161 ++++++++++++++++++++++++++++

Can we place those definitions in ecc.c instead of core.c

>  drivers/mtd/nand/raw/nand_base.c    | 161 +---------------------------
>  drivers/mtd/nand/raw/nand_toshiba.c |   2 +
>  include/linux/mtd/nand.h            |   4 +
>  include/linux/mtd/rawnand.h         |   3 -
>  5 files changed, 168 insertions(+), 163 deletions(-)

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

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

* Re: [RFC PATCH 08/27] mtd: nand: ecc: Use SPDX license identifier for the software BCH code
  2019-02-21 10:01 ` [RFC PATCH 08/27] mtd: nand: ecc: Use SPDX license identifier for the software BCH code Miquel Raynal
@ 2019-02-21 11:25   ` Boris Brezillon
  2019-02-21 11:48     ` Miquel Raynal
  0 siblings, 1 reply; 36+ messages in thread
From: Boris Brezillon @ 2019-02-21 11:25 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Mason Yang, Vignesh R, Tudor Ambarus, Julien Su,
	Richard Weinberger, Schrempf Frieder, Marek Vasut, linux-mtd,
	Thomas Petazzoni, Brian Norris, David Woodhouse,
	linux-arm-kernel

On Thu, 21 Feb 2019 11:01:57 +0100
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> Remove the legacy license text, also update the MODULE_LICENSE macro
> in accordance with the text at the top of the file (GPL v2 or
> higher).
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/mtd/nand/ecc/sw-bch-engine.c   | 17 ++---------------
>  include/linux/mtd/nand-sw-bch-engine.h |  5 +----
>  2 files changed, 3 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/mtd/nand/ecc/sw-bch-engine.c b/drivers/mtd/nand/ecc/sw-bch-engine.c
> index 37dc0f0061d3..871c4dd9f71d 100644
> --- a/drivers/mtd/nand/ecc/sw-bch-engine.c
> +++ b/drivers/mtd/nand/ecc/sw-bch-engine.c
> @@ -1,22 +1,9 @@
> +// SPDX-License-Identifier: GPL-2.0+
>  /*
>   * This file provides ECC correction for more than 1 bit per block of data,
>   * using binary BCH codes. It relies on the generic BCH library lib/bch.c.
>   *
>   * Copyright © 2011 Ivan Djelic <ivan.djelic@parrot.com>
> - *
> - * This file is free software; you can redistribute it and/or modify it
> - * under the terms of the GNU General Public License as published by the
> - * Free Software Foundation; either version 2 or (at your option) any
> - * later version.
> - *
> - * This file is distributed in the hope that it will be useful, but WITHOUT
> - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> - * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
> - * for more details.
> - *
> - * You should have received a copy of the GNU General Public License along
> - * with this file; if not, write to the Free Software Foundation, Inc.,
> - * 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
>   */
>  
>  #include <linux/types.h>
> @@ -227,6 +214,6 @@ void nand_bch_free(struct nand_bch_control *nbc)
>  }
>  EXPORT_SYMBOL(nand_bch_free);
>  
> -MODULE_LICENSE("GPL");
> +MODULE_LICENSE("GPL v2");

GPL was correct here, see [1]

[1]https://elixir.bootlin.com/linux/v5.0-rc7/source/include/linux/module.h#L175

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

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

* Re: [RFC PATCH 02/27] mtd: nand: Compile in the NAND core by default
  2019-02-21 11:14       ` Boris Brezillon
@ 2019-02-21 11:46         ` Miquel Raynal
  2019-02-21 12:08           ` Boris Brezillon
  0 siblings, 1 reply; 36+ messages in thread
From: Miquel Raynal @ 2019-02-21 11:46 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: Mason Yang, Vignesh R, Tudor Ambarus, Julien Su,
	Richard Weinberger, Schrempf Frieder, Marek Vasut, linux-mtd,
	Thomas Petazzoni, Brian Norris, David Woodhouse,
	linux-arm-kernel

Hi Boris,

Boris Brezillon <bbrezillon@kernel.org> wrote on Thu, 21 Feb 2019
12:14:37 +0100:

> On Thu, 21 Feb 2019 12:06:10 +0100
> Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> 
> > Hi Boris,
> > 
> > Boris Brezillon <bbrezillon@kernel.org> wrote on Thu, 21 Feb 2019
> > 11:55:54 +0100:
> >   
> > > On Thu, 21 Feb 2019 11:01:51 +0100
> > > Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > >     
> > > > Force the NAND core be compiled-in when using any kind of NAND.      
> > > 
> > > Why?    
> > 
> > Because all the logic that comes later in the series relies on this
> > change. I need the NAND core to be compiled-in,  
> 
> Hm, not exactly compiled-in as it can still be selected as a module.

Yes, do you think it is a problem?

> 
> > as well the ECC engine
> > core, as well as everything that is shared between raw NAND and
> > SPI-NAND which I move in the NAND core.  
> 
> Yes, the core is required for SPI NAND and soon will be for RAW NAND,
> but I still don't see the problem with the "select NAND_CORE" approach,
> sorry.
> 
> >   
> > >     
> > > > 
> > > > Also remove the redundant dependencies on MTD which is enforced by the
> > > > game of the if/endif blocs.
> > > > 
> > > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > > > ---
> > > >  drivers/mtd/nand/Kconfig         | 12 ++++++++++--
> > > >  drivers/mtd/nand/onenand/Kconfig |  1 -
> > > >  drivers/mtd/nand/raw/Kconfig     |  1 -
> > > >  drivers/mtd/nand/spi/Kconfig     |  1 -
> > > >  4 files changed, 10 insertions(+), 5 deletions(-)
> > > > 
> > > > diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
> > > > index 495751ed3fd7..e8d26a715922 100644
> > > > --- a/drivers/mtd/nand/Kconfig
> > > > +++ b/drivers/mtd/nand/Kconfig
> > > > @@ -1,6 +1,14 @@
> > > > -config MTD_NAND_CORE
> > > > -	tristate
> > > > +menuconfig MTD_NAND_CORE
> > > > +	tristate "NAND"      
> > > 
> > > Definitely not something we want to expose in menuconfig.    
> > 
> > I considered the NAND core as essential when using raw NAND and
> > SPI-NAND.  
> 
> It is needed for SPI NAND, and will be for raw NAND for sure, but I
> still see no reasons for turning it into a user-visible option.
> 
> > Hence, turning it into a menuconfig option make the whole
> > NAND subsystem available (or not) and will appear in a different
> > "menuconfig page", which really enhances the readability.  
> 
> That's a different thing, and I have no problem with adding an extra
> level in the Kconfig hierarchy.

Then why not using this symbol? I don't get why you are opposed.

So you would like a config NAND_CORE (invisible) and a visible
menuconfig NAND? And all entries in the NAND menu selecting NAND_CORE?

> 
> >   
> > >     
> > > > +	default y      
> > > 
> > > I don't see what's the problem with letting SPINAND, RAWNAND and
> > > ONENAND select this options if they actually need it.    
> > 
> > See the reason above, hence having it "default y" is needed to do not
> > break existing defconfigs.  
> 
> Not if you make RAW_NAND select NAND_CORE.


Thanks,
Miquèl

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

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

* Re: [RFC PATCH 05/27] mtd: nand: Move standard OOB layouts to the NAND core
  2019-02-21 11:19   ` Boris Brezillon
@ 2019-02-21 11:47     ` Miquel Raynal
  2019-02-21 12:10       ` Boris Brezillon
  0 siblings, 1 reply; 36+ messages in thread
From: Miquel Raynal @ 2019-02-21 11:47 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: Mason Yang, Vignesh R, Tudor Ambarus, Julien Su,
	Richard Weinberger, Schrempf Frieder, Marek Vasut, linux-mtd,
	Thomas Petazzoni, Brian Norris, David Woodhouse,
	linux-arm-kernel

Hi Boris,

Boris Brezillon <bbrezillon@kernel.org> wrote on Thu, 21 Feb 2019
12:19:00 +0100:

> On Thu, 21 Feb 2019 11:01:54 +0100
> Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> 
> > These OOB layouts are generic to all NAND chips, the should not be
> > restricted to be used only by raw NAND controller drivers. They might
> > later be used by generic ECC engines and SPI-NAND devices as well.
> > 
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
> >  drivers/mtd/nand/core.c             | 161 ++++++++++++++++++++++++++++  
> 
> Can we place those definitions in ecc.c instead of core.c

Right after (but I can move this patch in the series easily) I create a
drivers/mtd/nand/ecc/ directory with a engine.c file. If I understand
your suggestion correctly, you would prefer to have the OOB layouts in
this file.

> 
> >  drivers/mtd/nand/raw/nand_base.c    | 161 +---------------------------
> >  drivers/mtd/nand/raw/nand_toshiba.c |   2 +
> >  include/linux/mtd/nand.h            |   4 +
> >  include/linux/mtd/rawnand.h         |   3 -
> >  5 files changed, 168 insertions(+), 163 deletions(-)  

Thanks,
Miquèl

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

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

* Re: [RFC PATCH 08/27] mtd: nand: ecc: Use SPDX license identifier for the software BCH code
  2019-02-21 11:25   ` Boris Brezillon
@ 2019-02-21 11:48     ` Miquel Raynal
  0 siblings, 0 replies; 36+ messages in thread
From: Miquel Raynal @ 2019-02-21 11:48 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: Mason Yang, Vignesh R, Tudor Ambarus, Julien Su,
	Richard Weinberger, Schrempf Frieder, Marek Vasut, linux-mtd,
	Thomas Petazzoni, Brian Norris, David Woodhouse,
	linux-arm-kernel

Hi Boris,

Boris Brezillon <bbrezillon@kernel.org> wrote on Thu, 21 Feb 2019
12:25:38 +0100:

> On Thu, 21 Feb 2019 11:01:57 +0100
> Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> 
> > Remove the legacy license text, also update the MODULE_LICENSE macro
> > in accordance with the text at the top of the file (GPL v2 or
> > higher).
> > 
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
> >  drivers/mtd/nand/ecc/sw-bch-engine.c   | 17 ++---------------
> >  include/linux/mtd/nand-sw-bch-engine.h |  5 +----
> >  2 files changed, 3 insertions(+), 19 deletions(-)
> > 
> > diff --git a/drivers/mtd/nand/ecc/sw-bch-engine.c b/drivers/mtd/nand/ecc/sw-bch-engine.c
> > index 37dc0f0061d3..871c4dd9f71d 100644
> > --- a/drivers/mtd/nand/ecc/sw-bch-engine.c
> > +++ b/drivers/mtd/nand/ecc/sw-bch-engine.c
> > @@ -1,22 +1,9 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> >  /*
> >   * This file provides ECC correction for more than 1 bit per block of data,
> >   * using binary BCH codes. It relies on the generic BCH library lib/bch.c.
> >   *
> >   * Copyright © 2011 Ivan Djelic <ivan.djelic@parrot.com>
> > - *
> > - * This file is free software; you can redistribute it and/or modify it
> > - * under the terms of the GNU General Public License as published by the
> > - * Free Software Foundation; either version 2 or (at your option) any
> > - * later version.
> > - *
> > - * This file is distributed in the hope that it will be useful, but WITHOUT
> > - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> > - * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
> > - * for more details.
> > - *
> > - * You should have received a copy of the GNU General Public License along
> > - * with this file; if not, write to the Free Software Foundation, Inc.,
> > - * 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
> >   */
> >  
> >  #include <linux/types.h>
> > @@ -227,6 +214,6 @@ void nand_bch_free(struct nand_bch_control *nbc)
> >  }
> >  EXPORT_SYMBOL(nand_bch_free);
> >  
> > -MODULE_LICENSE("GPL");
> > +MODULE_LICENSE("GPL v2");  
> 
> GPL was correct here, see [1]
> 
> [1]https://elixir.bootlin.com/linux/v5.0-rc7/source/include/linux/module.h#L175

Oh right, then it's the same for the similar patch over the Hamming
file. I will correct both, thanks for pointing it.

Thanks,
Miquèl

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

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

* Re: [RFC PATCH 02/27] mtd: nand: Compile in the NAND core by default
  2019-02-21 11:46         ` Miquel Raynal
@ 2019-02-21 12:08           ` Boris Brezillon
  2019-02-21 12:52             ` Miquel Raynal
  0 siblings, 1 reply; 36+ messages in thread
From: Boris Brezillon @ 2019-02-21 12:08 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Mason Yang, Vignesh R, Tudor Ambarus, Julien Su,
	Richard Weinberger, Schrempf Frieder, Marek Vasut, linux-mtd,
	Thomas Petazzoni, Brian Norris, David Woodhouse,
	linux-arm-kernel

On Thu, 21 Feb 2019 12:46:28 +0100
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> Hi Boris,
> 
> Boris Brezillon <bbrezillon@kernel.org> wrote on Thu, 21 Feb 2019
> 12:14:37 +0100:
> 
> > On Thu, 21 Feb 2019 12:06:10 +0100
> > Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >   
> > > Hi Boris,
> > > 
> > > Boris Brezillon <bbrezillon@kernel.org> wrote on Thu, 21 Feb 2019
> > > 11:55:54 +0100:
> > >     
> > > > On Thu, 21 Feb 2019 11:01:51 +0100
> > > > Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > > >       
> > > > > Force the NAND core be compiled-in when using any kind of NAND.        
> > > > 
> > > > Why?      
> > > 
> > > Because all the logic that comes later in the series relies on this
> > > change. I need the NAND core to be compiled-in,    
> > 
> > Hm, not exactly compiled-in as it can still be selected as a module.  
> 
> Yes, do you think it is a problem?

Yes, it is, at least for the SPI NAND case (and soon the RAW NAND case
too) since you remove the select but don't add a depends on. If you
select NAND_CORE as a module and SPI_NAND compiled-in => BOOM!

> 
> >   
> > > as well the ECC engine
> > > core, as well as everything that is shared between raw NAND and
> > > SPI-NAND which I move in the NAND core.    
> > 
> > Yes, the core is required for SPI NAND and soon will be for RAW NAND,
> > but I still don't see the problem with the "select NAND_CORE" approach,
> > sorry.
> >   
> > >     
> > > >       
> > > > > 
> > > > > Also remove the redundant dependencies on MTD which is enforced by the
> > > > > game of the if/endif blocs.
> > > > > 
> > > > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > > > > ---
> > > > >  drivers/mtd/nand/Kconfig         | 12 ++++++++++--
> > > > >  drivers/mtd/nand/onenand/Kconfig |  1 -
> > > > >  drivers/mtd/nand/raw/Kconfig     |  1 -
> > > > >  drivers/mtd/nand/spi/Kconfig     |  1 -
> > > > >  4 files changed, 10 insertions(+), 5 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
> > > > > index 495751ed3fd7..e8d26a715922 100644
> > > > > --- a/drivers/mtd/nand/Kconfig
> > > > > +++ b/drivers/mtd/nand/Kconfig
> > > > > @@ -1,6 +1,14 @@
> > > > > -config MTD_NAND_CORE
> > > > > -	tristate
> > > > > +menuconfig MTD_NAND_CORE
> > > > > +	tristate "NAND"        
> > > > 
> > > > Definitely not something we want to expose in menuconfig.      
> > > 
> > > I considered the NAND core as essential when using raw NAND and
> > > SPI-NAND.    
> > 
> > It is needed for SPI NAND, and will be for raw NAND for sure, but I
> > still see no reasons for turning it into a user-visible option.
> >   
> > > Hence, turning it into a menuconfig option make the whole
> > > NAND subsystem available (or not) and will appear in a different
> > > "menuconfig page", which really enhances the readability.    
> > 
> > That's a different thing, and I have no problem with adding an extra
> > level in the Kconfig hierarchy.  
> 
> Then why not using this symbol? I don't get why you are opposed.

Because, not only you're forcing onenand users to pull the nand core
code in, which is not needed until the conversion to the generic NAND
layer is done, but you're actually forcing that for all existing
defconfigs because of your "default y" approach. Also, I don't see
what's the benefit of letting users know about this intermediate
framework when all they care about is supporting a specific class of
devices.

> 
> So you would like a config NAND_CORE (invisible) and a visible
> menuconfig NAND? And all entries in the NAND menu selecting NAND_CORE?

Not all entries, just SPI NAND, and soon, raw NAND. And yes, I'd prefer
that.

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

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

* Re: [RFC PATCH 05/27] mtd: nand: Move standard OOB layouts to the NAND core
  2019-02-21 11:47     ` Miquel Raynal
@ 2019-02-21 12:10       ` Boris Brezillon
  0 siblings, 0 replies; 36+ messages in thread
From: Boris Brezillon @ 2019-02-21 12:10 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Mason Yang, Vignesh R, Tudor Ambarus, Julien Su,
	Richard Weinberger, Schrempf Frieder, Marek Vasut, linux-mtd,
	Thomas Petazzoni, Brian Norris, David Woodhouse,
	linux-arm-kernel

On Thu, 21 Feb 2019 12:47:42 +0100
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> Hi Boris,
> 
> Boris Brezillon <bbrezillon@kernel.org> wrote on Thu, 21 Feb 2019
> 12:19:00 +0100:
> 
> > On Thu, 21 Feb 2019 11:01:54 +0100
> > Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >   
> > > These OOB layouts are generic to all NAND chips, the should not be
> > > restricted to be used only by raw NAND controller drivers. They might
> > > later be used by generic ECC engines and SPI-NAND devices as well.
> > > 
> > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > > ---
> > >  drivers/mtd/nand/core.c             | 161 ++++++++++++++++++++++++++++    
> > 
> > Can we place those definitions in ecc.c instead of core.c  
> 
> Right after (but I can move this patch in the series easily) I create a
> drivers/mtd/nand/ecc/ directory with a engine.c file. If I understand
> your suggestion correctly, you would prefer to have the OOB layouts in
> this file.

Yep.

> 
> >   
> > >  drivers/mtd/nand/raw/nand_base.c    | 161 +---------------------------
> > >  drivers/mtd/nand/raw/nand_toshiba.c |   2 +
> > >  include/linux/mtd/nand.h            |   4 +
> > >  include/linux/mtd/rawnand.h         |   3 -
> > >  5 files changed, 168 insertions(+), 163 deletions(-)    
> 
> Thanks,
> Miquèl


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

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

* Re: [RFC PATCH 09/27] mtd: nand: ecc: Turn the software BCH implementation generic
  2019-02-21 10:01 ` [RFC PATCH 09/27] mtd: nand: ecc: Turn the software BCH implementation generic Miquel Raynal
@ 2019-02-21 12:26   ` Boris Brezillon
  2019-02-21 12:53     ` Miquel Raynal
  0 siblings, 1 reply; 36+ messages in thread
From: Boris Brezillon @ 2019-02-21 12:26 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Mason Yang, Vignesh R, Tudor Ambarus, Julien Su,
	Richard Weinberger, Schrempf Frieder, Marek Vasut, linux-mtd,
	Thomas Petazzoni, Brian Norris, David Woodhouse,
	linux-arm-kernel

On Thu, 21 Feb 2019 11:01:58 +0100
Miquel Raynal <miquel.raynal@bootlin.com> wrote:


>  /**
> - * nand_bch_correct_data - [NAND Interface] Detect and correct bit error(s)
> - * @chip:	NAND chip object
> - * @buf:	raw data read from the chip
> - * @read_ecc:	ECC from the chip
> - * @calc_ecc:	the ECC calculated from raw data
> + * ecc_sw_bch_correct - Detect, correct and report bit error(s)
>   *
> - * Detect and correct bit errors for a data byte block
> + * @nand: NAND device
> + * @buf: Raw data read from the chip
> + * @read_ecc: ECC bytes from the chip
> + * @calc_ecc: ECC calculated from the raw data
> + *
> + * Detect and correct bit errors for a data block.
>   */
> -int nand_bch_correct_data(struct nand_chip *chip, unsigned char *buf,
> -			  unsigned char *read_ecc, unsigned char *calc_ecc)
> +int ecc_sw_bch_correct(struct nand_device *nand, unsigned char *buf,
> +		       unsigned char *read_ecc, unsigned char *calc_ecc)

I'd keep the nand_ prefix, as ECC is employed in other places in the
kernel and here the API is clearly tied to the nand_device object.

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

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

* Re: [RFC PATCH 02/27] mtd: nand: Compile in the NAND core by default
  2019-02-21 12:08           ` Boris Brezillon
@ 2019-02-21 12:52             ` Miquel Raynal
  0 siblings, 0 replies; 36+ messages in thread
From: Miquel Raynal @ 2019-02-21 12:52 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: Mason Yang, Vignesh R, Tudor Ambarus, Julien Su,
	Richard Weinberger, Schrempf Frieder, Marek Vasut, linux-mtd,
	Thomas Petazzoni, Brian Norris, David Woodhouse,
	linux-arm-kernel

Hi Boris,

Boris Brezillon <bbrezillon@kernel.org> wrote on Thu, 21 Feb 2019
13:08:15 +0100:

> On Thu, 21 Feb 2019 12:46:28 +0100
> Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> 
> > Hi Boris,
> > 
> > Boris Brezillon <bbrezillon@kernel.org> wrote on Thu, 21 Feb 2019
> > 12:14:37 +0100:
> >   
> > > On Thu, 21 Feb 2019 12:06:10 +0100
> > > Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > >     
> > > > Hi Boris,
> > > > 
> > > > Boris Brezillon <bbrezillon@kernel.org> wrote on Thu, 21 Feb 2019
> > > > 11:55:54 +0100:
> > > >       
> > > > > On Thu, 21 Feb 2019 11:01:51 +0100
> > > > > Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > > > >         
> > > > > > Force the NAND core be compiled-in when using any kind of NAND.          
> > > > > 
> > > > > Why?        
> > > > 
> > > > Because all the logic that comes later in the series relies on this
> > > > change. I need the NAND core to be compiled-in,      
> > > 
> > > Hm, not exactly compiled-in as it can still be selected as a module.    
> > 
> > Yes, do you think it is a problem?  
> 
> Yes, it is, at least for the SPI NAND case (and soon the RAW NAND case
> too) since you remove the select but don't add a depends on. If you
> select NAND_CORE as a module and SPI_NAND compiled-in => BOOM!

I am not entirely sure but I think SPI NAND cannot be compiled-in if it
is "under" a menu that has been selected as a module. But anyway, it's
just a matter of seeing things, and the onenand argument is valid to
me. I will change it to the below proposal you accepted.

> 
> >   
> > >     
> > > > as well the ECC engine
> > > > core, as well as everything that is shared between raw NAND and
> > > > SPI-NAND which I move in the NAND core.      
> > > 
> > > Yes, the core is required for SPI NAND and soon will be for RAW NAND,
> > > but I still don't see the problem with the "select NAND_CORE" approach,
> > > sorry.
> > >     
> > > >       
> > > > >         
> > > > > > 
> > > > > > Also remove the redundant dependencies on MTD which is enforced by the
> > > > > > game of the if/endif blocs.
> > > > > > 
> > > > > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > > > > > ---
> > > > > >  drivers/mtd/nand/Kconfig         | 12 ++++++++++--
> > > > > >  drivers/mtd/nand/onenand/Kconfig |  1 -
> > > > > >  drivers/mtd/nand/raw/Kconfig     |  1 -
> > > > > >  drivers/mtd/nand/spi/Kconfig     |  1 -
> > > > > >  4 files changed, 10 insertions(+), 5 deletions(-)
> > > > > > 
> > > > > > diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
> > > > > > index 495751ed3fd7..e8d26a715922 100644
> > > > > > --- a/drivers/mtd/nand/Kconfig
> > > > > > +++ b/drivers/mtd/nand/Kconfig
> > > > > > @@ -1,6 +1,14 @@
> > > > > > -config MTD_NAND_CORE
> > > > > > -	tristate
> > > > > > +menuconfig MTD_NAND_CORE
> > > > > > +	tristate "NAND"          
> > > > > 
> > > > > Definitely not something we want to expose in menuconfig.        
> > > > 
> > > > I considered the NAND core as essential when using raw NAND and
> > > > SPI-NAND.      
> > > 
> > > It is needed for SPI NAND, and will be for raw NAND for sure, but I
> > > still see no reasons for turning it into a user-visible option.
> > >     
> > > > Hence, turning it into a menuconfig option make the whole
> > > > NAND subsystem available (or not) and will appear in a different
> > > > "menuconfig page", which really enhances the readability.      
> > > 
> > > That's a different thing, and I have no problem with adding an extra
> > > level in the Kconfig hierarchy.    
> > 
> > Then why not using this symbol? I don't get why you are opposed.  
> 
> Because, not only you're forcing onenand users to pull the nand core
> code in, which is not needed until the conversion to the generic NAND
> layer is done, but you're actually forcing that for all existing
> defconfigs because of your "default y" approach. Also, I don't see
> what's the benefit of letting users know about this intermediate
> framework when all they care about is supporting a specific class of
> devices.
> 
> > 
> > So you would like a config NAND_CORE (invisible) and a visible
> > menuconfig NAND? And all entries in the NAND menu selecting NAND_CORE?  
> 
> Not all entries, just SPI NAND, and soon, raw NAND. And yes, I'd prefer
> that.


Thanks,
Miquèl

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

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

* Re: [RFC PATCH 09/27] mtd: nand: ecc: Turn the software BCH implementation generic
  2019-02-21 12:26   ` Boris Brezillon
@ 2019-02-21 12:53     ` Miquel Raynal
  0 siblings, 0 replies; 36+ messages in thread
From: Miquel Raynal @ 2019-02-21 12:53 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: Mason Yang, Vignesh R, Tudor Ambarus, Julien Su,
	Richard Weinberger, Schrempf Frieder, Marek Vasut, linux-mtd,
	Thomas Petazzoni, Brian Norris, David Woodhouse,
	linux-arm-kernel

Hi Boris,

Boris Brezillon <bbrezillon@kernel.org> wrote on Thu, 21 Feb 2019
13:26:52 +0100:

> On Thu, 21 Feb 2019 11:01:58 +0100
> Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> 
> 
> >  /**
> > - * nand_bch_correct_data - [NAND Interface] Detect and correct bit error(s)
> > - * @chip:	NAND chip object
> > - * @buf:	raw data read from the chip
> > - * @read_ecc:	ECC from the chip
> > - * @calc_ecc:	the ECC calculated from raw data
> > + * ecc_sw_bch_correct - Detect, correct and report bit error(s)
> >   *
> > - * Detect and correct bit errors for a data byte block
> > + * @nand: NAND device
> > + * @buf: Raw data read from the chip
> > + * @read_ecc: ECC bytes from the chip
> > + * @calc_ecc: ECC calculated from the raw data
> > + *
> > + * Detect and correct bit errors for a data block.
> >   */
> > -int nand_bch_correct_data(struct nand_chip *chip, unsigned char *buf,
> > -			  unsigned char *read_ecc, unsigned char *calc_ecc)
> > +int ecc_sw_bch_correct(struct nand_device *nand, unsigned char *buf,
> > +		       unsigned char *read_ecc, unsigned char *calc_ecc)  
> 
> I'd keep the nand_ prefix, as ECC is employed in other places in the
> kernel and here the API is clearly tied to the nand_device object.

I had a looong internal conflict about it. It's longer with nand_ but
you are right it is tied to the NAND core, so I'll do the change.


Thanks,
Miquèl

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

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

* Re: [RFC PATCH 10/27] mtd: rawnand: Get rid of chip->ecc.priv
  2019-02-21 10:01 ` [RFC PATCH 10/27] mtd: rawnand: Get rid of chip->ecc.priv Miquel Raynal
@ 2019-02-21 13:01   ` Boris Brezillon
  0 siblings, 0 replies; 36+ messages in thread
From: Boris Brezillon @ 2019-02-21 13:01 UTC (permalink / raw)
  To: Miquel Raynal, Mason Yang
  Cc: Vignesh R, Tudor Ambarus, Julien Su, Richard Weinberger,
	Schrempf Frieder, Marek Vasut, linux-mtd, Thomas Petazzoni,
	Brian Norris, David Woodhouse, linux-arm-kernel

On Thu, 21 Feb 2019 11:01:59 +0100
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> nand_ecc_ctrl embeds a private pointer which had only a meaning in the
> sunxi driver. This structure will soon be deprecated, but as this
> field is actually not needed, let's just drop it.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/mtd/nand/raw/sunxi_nand.c | 29 ++++++++++++++---------------
>  include/linux/mtd/rawnand.h       |  2 --
>  2 files changed, 14 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/sunxi_nand.c b/drivers/mtd/nand/raw/sunxi_nand.c
> index b56f89c992f3..d05f71f33b5c 100644
> --- a/drivers/mtd/nand/raw/sunxi_nand.c
> +++ b/drivers/mtd/nand/raw/sunxi_nand.c
> @@ -230,6 +230,7 @@ static inline struct sunxi_nand_chip *to_sunxi_nand(struct nand_chip *nand)
>   *			this NAND controller
>   * @complete:		a completion object used to wait for NAND
>   *			controller events
> + * @ecc:		ECC controller structure
>   */
>  struct sunxi_nfc {
>  	struct nand_controller controller;
> @@ -243,6 +244,7 @@ struct sunxi_nfc {
>  	struct list_head chips;
>  	struct completion complete;
>  	struct dma_chan *dmac;
> +	struct sunxi_nand_hw_ecc *ecc;

It's a per-chip object, not a per-controller one, should be moved to
sunxi_nand_chip.

>  };
>  

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

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

* Re: [RFC PATCH 03/27] mtd: nand: Introduce the ECC engine abstraction
  2019-02-21 10:01 ` [RFC PATCH 03/27] mtd: nand: Introduce the ECC engine abstraction Miquel Raynal
  2019-02-21 11:16   ` Boris Brezillon
@ 2019-02-25 18:55   ` Boris Brezillon
  2019-02-27 13:56     ` Miquel Raynal
  1 sibling, 1 reply; 36+ messages in thread
From: Boris Brezillon @ 2019-02-25 18:55 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Mason Yang, Vignesh R, Tudor Ambarus, Julien Su,
	Richard Weinberger, Schrempf Frieder, Marek Vasut, linux-mtd,
	Thomas Petazzoni, Brian Norris, David Woodhouse,
	linux-arm-kernel

On Thu, 21 Feb 2019 11:01:52 +0100
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> +
> +/**
> + * struct nand_ecc_engine_ops - Generic ECC engine operations
> + *
> + * @init_ctx: given a desired user configuration for the pointed NAND device,
> + *            requests the ECC engine driver to setup a configuration with
> + *            values it supports.
> + * @cleanup_ctx: clean the context initialized by @init_ctx.
> + * @prepare_io_req: is called before reading/writing a page to prepare the I/O
> + *                  request to be performed with ECC correction.
> + * @finish_io_req: is called after reading/writing a page to terminate the I/O
> + *                 request and ensure proper ECC correction.
> + */
> +struct nand_ecc_engine_ops {

We might want to add a

	void (*put_engine)(struct nand_ecc_engine *engine);

here if we want the nanddev cleanup path to be generic.
This hook would be implemented by drivers where the ECC engine object is
refcounted (typically the case for HW ECC engines shared by the raw NAND
controller and the SPI controller).

Alternatively, you can just add one nand_put_xxx_ecc_engine() func per
engine class (SW, ondie and HW).

> +	int (*init_ctx)(struct nand_device *nand);
> +	void (*cleanup_ctx)(struct nand_device *nand);
> +	int (*prepare_io_req)(struct nand_device *nand,
> +			      struct nand_page_io_req *req,
> +			      void *oobbuf);
> +	int (*finish_io_req)(struct nand_device *nand,
> +			     struct nand_page_io_req *req,
> +			     void *oobbuf);
> +};

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

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

* Re: [RFC PATCH 03/27] mtd: nand: Introduce the ECC engine abstraction
  2019-02-21 11:16   ` Boris Brezillon
@ 2019-02-27  9:26     ` Miquel Raynal
  2019-02-27  9:47       ` Boris Brezillon
  0 siblings, 1 reply; 36+ messages in thread
From: Miquel Raynal @ 2019-02-27  9:26 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: Mason Yang, Vignesh R, Tudor Ambarus, Julien Su,
	Richard Weinberger, Schrempf Frieder, Marek Vasut, linux-mtd,
	Thomas Petazzoni, Brian Norris, David Woodhouse,
	linux-arm-kernel

Hi Boris,

Boris Brezillon <bbrezillon@kernel.org> wrote on Thu, 21 Feb 2019
12:16:25 +0100:

> On Thu, 21 Feb 2019 11:01:52 +0100
> Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> 
> 
> > diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
> > index 30f0fb02abe2..9e3b018d0b83 100644
> > --- a/include/linux/mtd/nand.h
> > +++ b/include/linux/mtd/nand.h
> > @@ -82,8 +82,14 @@ struct nand_pos {
> >  	unsigned int page;
> >  };
> >  
> > +enum nand_page_io_req_type {
> > +	NAND_PAGE_READ = 0,
> > +	NAND_PAGE_WRITE,
> > +};
> > +
> >  /**
> >   * struct nand_page_io_req - NAND I/O request object
> > + * @type: the type of page I/O: read or write
> >   * @pos: the position this I/O request is targeting
> >   * @dataoffs: the offset within the page
> >   * @datalen: number of data bytes to read from/write to this page
> > @@ -99,6 +105,7 @@ struct nand_pos {
> >   * specific commands/operations.
> >   */
> >  struct nand_page_io_req {
> > +	enum nand_page_io_req_type type;  
> 
> Can you add the reqtype enum and type field (+ patch the iterator
> helpers) in a separate patch?

Sure.

> 
> >  	struct nand_pos pos;
> >  	unsigned int dataoffs;
> >  	unsigned int datalen;
> > @@ -116,13 +123,35 @@ struct nand_page_io_req {
> >  };
> >  
> >  /**
> > - * struct nand_ecc_req - NAND ECC requirements
> > + * struct nand_ecc_conf - NAND ECC configuration
> > + * @strength: ECC strength
> > + * @step_size: Number of bytes per step
> > + * @total: Total number of bytes used for storing ECC codes, this is used by
> > + *         generic OOB layouts
> > + */
> > +struct nand_ecc_conf {  
> 
> Please do the s/nand_ecc_req/nand_ecc_conf/ in a separate patch.

Ok.

> 
> > +	unsigned int strength;
> > +	unsigned int step_size;
> > +	unsigned int total;  
> 
> Do we really need to add this total field here? Looks like something
> that should be kept private to the ECC engine implementation.

It was initially private, but I realized it was needed by generic parts
(including for instance the generic OOB layouts) so it could not be
made private.

I just moved the 'total' entry out of the struct nand_ecc_conf and
moved it in the struct nand_ecc_ctx. It is still public but not in the
very generic "conf" structure anymore.

> 
> > +};
> > +
> > +/**
> > + * struct nand_ecc_user_conf - User desired ECC configuration
> > + * @mode: ECC mode
> > + * @algo: ECC algorithm
> >   * @strength: ECC strength
> >   * @step_size: ECC step/block size
> > + * @maximize: ECC parameters must be maximized depending on the device
> > + *            capabilities
> > + * @flags: User flags
> >   */
> > -struct nand_ecc_req {
> > +struct nand_ecc_user_conf {
> > +	int mode;  
> 
> We should definitely name that one differently ('provider' maybe).

Changed to provider.

> 
> > +	unsigned int algo;
> >  	unsigned int strength;
> >  	unsigned int step_size;
> > +	unsigned int maximize;
> > +	unsigned int flags;  
> 
> maximize could be a flag.

It is now.

> 
> >  };
> >  
> >  #define NAND_ECCREQ(str, stp) { .strength = (str), .step_size = (stp) }
> > @@ -157,11 +186,76 @@ struct nand_ops {
> >  	bool (*isbad)(struct nand_device *nand, const struct nand_pos *pos);
> >  };  
> 


Thanks,
Miquèl

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

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

* Re: [RFC PATCH 03/27] mtd: nand: Introduce the ECC engine abstraction
  2019-02-27  9:26     ` Miquel Raynal
@ 2019-02-27  9:47       ` Boris Brezillon
  0 siblings, 0 replies; 36+ messages in thread
From: Boris Brezillon @ 2019-02-27  9:47 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Mason Yang, Vignesh R, Tudor Ambarus, Julien Su,
	Richard Weinberger, Schrempf Frieder, Marek Vasut, linux-mtd,
	Thomas Petazzoni, Brian Norris, David Woodhouse,
	linux-arm-kernel

On Wed, 27 Feb 2019 10:26:42 +0100
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> >   
> > > +	unsigned int strength;
> > > +	unsigned int step_size;
> > > +	unsigned int total;    
> > 
> > Do we really need to add this total field here? Looks like something
> > that should be kept private to the ECC engine implementation.  
> 
> It was initially private, but I realized it was needed by generic parts
> (including for instance the generic OOB layouts) so it could not be
> made private.
> 
> I just moved the 'total' entry out of the struct nand_ecc_conf and
> moved it in the struct nand_ecc_ctx. It is still public but not in the
> very generic "conf" structure anymore.

Ack.

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

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

* Re: [RFC PATCH 03/27] mtd: nand: Introduce the ECC engine abstraction
  2019-02-25 18:55   ` Boris Brezillon
@ 2019-02-27 13:56     ` Miquel Raynal
  2019-02-27 14:06       ` Boris Brezillon
  0 siblings, 1 reply; 36+ messages in thread
From: Miquel Raynal @ 2019-02-27 13:56 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: Mason Yang, Vignesh R, Tudor Ambarus, Julien Su,
	Richard Weinberger, Schrempf Frieder, Marek Vasut, linux-mtd,
	Thomas Petazzoni, Brian Norris, David Woodhouse,
	linux-arm-kernel

Hi Boris,

Boris Brezillon <bbrezillon@kernel.org> wrote on Mon, 25 Feb 2019
19:55:43 +0100:

> On Thu, 21 Feb 2019 11:01:52 +0100
> Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> 
> > +
> > +/**
> > + * struct nand_ecc_engine_ops - Generic ECC engine operations
> > + *
> > + * @init_ctx: given a desired user configuration for the pointed NAND device,
> > + *            requests the ECC engine driver to setup a configuration with
> > + *            values it supports.
> > + * @cleanup_ctx: clean the context initialized by @init_ctx.
> > + * @prepare_io_req: is called before reading/writing a page to prepare the I/O
> > + *                  request to be performed with ECC correction.
> > + * @finish_io_req: is called after reading/writing a page to terminate the I/O
> > + *                 request and ensure proper ECC correction.
> > + */
> > +struct nand_ecc_engine_ops {  
> 
> We might want to add a
> 
> 	void (*put_engine)(struct nand_ecc_engine *engine);
> 
> here if we want the nanddev cleanup path to be generic.
> This hook would be implemented by drivers where the ECC engine object is
> refcounted (typically the case for HW ECC engines shared by the raw NAND
> controller and the SPI controller).
> 
> Alternatively, you can just add one nand_put_xxx_ecc_engine() func per
> engine class (SW, ondie and HW).

Can't this be handled in the init/cleanup_ctx() path directly?

Furthermore if this is just a hook to do reference counting.


Thanks,
Miquèl

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

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

* Re: [RFC PATCH 03/27] mtd: nand: Introduce the ECC engine abstraction
  2019-02-27 13:56     ` Miquel Raynal
@ 2019-02-27 14:06       ` Boris Brezillon
  2019-02-27 14:19         ` Miquel Raynal
  0 siblings, 1 reply; 36+ messages in thread
From: Boris Brezillon @ 2019-02-27 14:06 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Mason Yang, Vignesh R, Tudor Ambarus, Julien Su,
	Richard Weinberger, Boris Brezillon, Schrempf Frieder,
	Marek Vasut, linux-mtd, Thomas Petazzoni, Brian Norris,
	David Woodhouse, linux-arm-kernel

On Wed, 27 Feb 2019 14:56:07 +0100
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> Hi Boris,
> 
> Boris Brezillon <bbrezillon@kernel.org> wrote on Mon, 25 Feb 2019
> 19:55:43 +0100:
> 
> > On Thu, 21 Feb 2019 11:01:52 +0100
> > Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >   
> > > +
> > > +/**
> > > + * struct nand_ecc_engine_ops - Generic ECC engine operations
> > > + *
> > > + * @init_ctx: given a desired user configuration for the pointed NAND device,
> > > + *            requests the ECC engine driver to setup a configuration with
> > > + *            values it supports.
> > > + * @cleanup_ctx: clean the context initialized by @init_ctx.
> > > + * @prepare_io_req: is called before reading/writing a page to prepare the I/O
> > > + *                  request to be performed with ECC correction.
> > > + * @finish_io_req: is called after reading/writing a page to terminate the I/O
> > > + *                 request and ensure proper ECC correction.
> > > + */
> > > +struct nand_ecc_engine_ops {    
> > 
> > We might want to add a
> > 
> > 	void (*put_engine)(struct nand_ecc_engine *engine);
> > 
> > here if we want the nanddev cleanup path to be generic.
> > This hook would be implemented by drivers where the ECC engine object is
> > refcounted (typically the case for HW ECC engines shared by the raw NAND
> > controller and the SPI controller).
> > 
> > Alternatively, you can just add one nand_put_xxx_ecc_engine() func per
> > engine class (SW, ondie and HW).  
> 
> Can't this be handled in the init/cleanup_ctx() path directly?

You really have to get the reference before init_ctx() otherwise the
engine might disappear between your get() and init() call, and, to keep
things symmetric, I think it's best to handle the put() outside the
cleanup_ctx() path.

> 
> Furthermore if this is just a hook to do reference counting.

Well, what this put() does depends on the class of engine. For SW and
on-die ECC it can be a NOOP (that's true only if you keep the approach
where you have a single instance shared by everyone for SW-based ECC
engines).
For HW-controller-side ECC engines, you'll have to call device_get() on
the parent device in your nand_get_hw_ecc_engine() function while you
hold the lock protecting the ECC engine list. And device_put() will be
called in nand_put_hw_ecc_engine().

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

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

* Re: [RFC PATCH 03/27] mtd: nand: Introduce the ECC engine abstraction
  2019-02-27 14:06       ` Boris Brezillon
@ 2019-02-27 14:19         ` Miquel Raynal
  2019-02-27 14:28           ` Boris Brezillon
  0 siblings, 1 reply; 36+ messages in thread
From: Miquel Raynal @ 2019-02-27 14:19 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: Mason Yang, Vignesh R, Tudor Ambarus, Julien Su,
	Richard Weinberger, Boris Brezillon, Schrempf Frieder,
	Marek Vasut, linux-mtd, Thomas Petazzoni, Brian Norris,
	David Woodhouse, linux-arm-kernel

Hi Boris,

Boris Brezillon <boris.brezillon@collabora.com> wrote on Wed, 27 Feb
2019 15:06:33 +0100:

> On Wed, 27 Feb 2019 14:56:07 +0100
> Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> 
> > Hi Boris,
> > 
> > Boris Brezillon <bbrezillon@kernel.org> wrote on Mon, 25 Feb 2019
> > 19:55:43 +0100:
> >   
> > > On Thu, 21 Feb 2019 11:01:52 +0100
> > > Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > >     
> > > > +
> > > > +/**
> > > > + * struct nand_ecc_engine_ops - Generic ECC engine operations
> > > > + *
> > > > + * @init_ctx: given a desired user configuration for the pointed NAND device,
> > > > + *            requests the ECC engine driver to setup a configuration with
> > > > + *            values it supports.
> > > > + * @cleanup_ctx: clean the context initialized by @init_ctx.
> > > > + * @prepare_io_req: is called before reading/writing a page to prepare the I/O
> > > > + *                  request to be performed with ECC correction.
> > > > + * @finish_io_req: is called after reading/writing a page to terminate the I/O
> > > > + *                 request and ensure proper ECC correction.
> > > > + */
> > > > +struct nand_ecc_engine_ops {      
> > > 
> > > We might want to add a
> > > 
> > > 	void (*put_engine)(struct nand_ecc_engine *engine);
> > > 
> > > here if we want the nanddev cleanup path to be generic.
> > > This hook would be implemented by drivers where the ECC engine object is
> > > refcounted (typically the case for HW ECC engines shared by the raw NAND
> > > controller and the SPI controller).
> > > 
> > > Alternatively, you can just add one nand_put_xxx_ecc_engine() func per
> > > engine class (SW, ondie and HW).    
> > 
> > Can't this be handled in the init/cleanup_ctx() path directly?  
> 
> You really have to get the reference before init_ctx() otherwise the
> engine might disappear between your get() and init() call, and, to keep
> things symmetric, I think it's best to handle the put() outside the
> cleanup_ctx() path.
> 
> > 
> > Furthermore if this is just a hook to do reference counting.  
> 
> Well, what this put() does depends on the class of engine. For SW and
> on-die ECC it can be a NOOP (that's true only if you keep the approach
> where you have a single instance shared by everyone for SW-based ECC
> engines).
> For HW-controller-side ECC engines, you'll have to call device_get() on
> the parent device in your nand_get_hw_ecc_engine() function while you
> hold the lock protecting the ECC engine list. And device_put() will be
> called in nand_put_hw_ecc_engine().


I see.

Then I prefer keeping the logic in the core, not in the engine driver
and propose a

        void nand_ecc_put_engine(struct nand_ecc_engine *engine)

which will do nothing for on-die/sw engines and drop the reference for
hw engines. I will also rename the "find_ecc_engine" to "get_engine" so
that the call to the "put" helper has more meaning.


Thanks,
Miquèl

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

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

* Re: [RFC PATCH 03/27] mtd: nand: Introduce the ECC engine abstraction
  2019-02-27 14:19         ` Miquel Raynal
@ 2019-02-27 14:28           ` Boris Brezillon
  2019-02-27 14:34             ` Miquel Raynal
  0 siblings, 1 reply; 36+ messages in thread
From: Boris Brezillon @ 2019-02-27 14:28 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Mason Yang, Vignesh R, Tudor Ambarus, Julien Su,
	Richard Weinberger, Boris Brezillon, Schrempf Frieder,
	Marek Vasut, linux-mtd, Thomas Petazzoni, Brian Norris,
	David Woodhouse, linux-arm-kernel

On Wed, 27 Feb 2019 15:19:57 +0100
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> Hi Boris,
> 
> Boris Brezillon <boris.brezillon@collabora.com> wrote on Wed, 27 Feb
> 2019 15:06:33 +0100:
> 
> > On Wed, 27 Feb 2019 14:56:07 +0100
> > Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >   
> > > Hi Boris,
> > > 
> > > Boris Brezillon <bbrezillon@kernel.org> wrote on Mon, 25 Feb 2019
> > > 19:55:43 +0100:
> > >     
> > > > On Thu, 21 Feb 2019 11:01:52 +0100
> > > > Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > > >       
> > > > > +
> > > > > +/**
> > > > > + * struct nand_ecc_engine_ops - Generic ECC engine operations
> > > > > + *
> > > > > + * @init_ctx: given a desired user configuration for the pointed NAND device,
> > > > > + *            requests the ECC engine driver to setup a configuration with
> > > > > + *            values it supports.
> > > > > + * @cleanup_ctx: clean the context initialized by @init_ctx.
> > > > > + * @prepare_io_req: is called before reading/writing a page to prepare the I/O
> > > > > + *                  request to be performed with ECC correction.
> > > > > + * @finish_io_req: is called after reading/writing a page to terminate the I/O
> > > > > + *                 request and ensure proper ECC correction.
> > > > > + */
> > > > > +struct nand_ecc_engine_ops {        
> > > > 
> > > > We might want to add a
> > > > 
> > > > 	void (*put_engine)(struct nand_ecc_engine *engine);
> > > > 
> > > > here if we want the nanddev cleanup path to be generic.
> > > > This hook would be implemented by drivers where the ECC engine object is
> > > > refcounted (typically the case for HW ECC engines shared by the raw NAND
> > > > controller and the SPI controller).
> > > > 
> > > > Alternatively, you can just add one nand_put_xxx_ecc_engine() func per
> > > > engine class (SW, ondie and HW).      
> > > 
> > > Can't this be handled in the init/cleanup_ctx() path directly?    
> > 
> > You really have to get the reference before init_ctx() otherwise the
> > engine might disappear between your get() and init() call, and, to keep
> > things symmetric, I think it's best to handle the put() outside the
> > cleanup_ctx() path.
> >   
> > > 
> > > Furthermore if this is just a hook to do reference counting.    
> > 
> > Well, what this put() does depends on the class of engine. For SW and
> > on-die ECC it can be a NOOP (that's true only if you keep the approach
> > where you have a single instance shared by everyone for SW-based ECC
> > engines).
> > For HW-controller-side ECC engines, you'll have to call device_get() on
> > the parent device in your nand_get_hw_ecc_engine() function while you
> > hold the lock protecting the ECC engine list. And device_put() will be
> > called in nand_put_hw_ecc_engine().  
> 
> 
> I see.
> 
> Then I prefer keeping the logic in the core, not in the engine driver
> and propose a
> 
>         void nand_ecc_put_engine(struct nand_ecc_engine *engine)
> 
> which will do nothing for on-die/sw engines and drop the reference for
> hw engines. I will also rename the "find_ecc_engine" to "get_engine" so
> that the call to the "put" helper has more meaning.

Ack for most of it. One thing I'd like to clarify: it's probably better
to have a separate function called nand_ecc_put_hw_engine() which you'll
call from nand_ecc_put_engine() when you're dealing with an
HW ECC engine rather than calling put_device() directly from
nand_ecc_put_engine(). This way you keep the code for HW ECC engine
well isolated.

Same goes for the nand_ecc_get_engine() path, just delegate to
nand_ecc_get_hw_engine() when ->provider == HW_ECC.

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

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

* Re: [RFC PATCH 03/27] mtd: nand: Introduce the ECC engine abstraction
  2019-02-27 14:28           ` Boris Brezillon
@ 2019-02-27 14:34             ` Miquel Raynal
  0 siblings, 0 replies; 36+ messages in thread
From: Miquel Raynal @ 2019-02-27 14:34 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: Mason Yang, Vignesh R, Tudor Ambarus, Julien Su,
	Richard Weinberger, Boris Brezillon, Schrempf Frieder,
	Marek Vasut, linux-mtd, Thomas Petazzoni, Brian Norris,
	David Woodhouse, linux-arm-kernel


> > > > Furthermore if this is just a hook to do reference counting.      
> > > 
> > > Well, what this put() does depends on the class of engine. For SW and
> > > on-die ECC it can be a NOOP (that's true only if you keep the approach
> > > where you have a single instance shared by everyone for SW-based ECC
> > > engines).
> > > For HW-controller-side ECC engines, you'll have to call device_get() on
> > > the parent device in your nand_get_hw_ecc_engine() function while you
> > > hold the lock protecting the ECC engine list. And device_put() will be
> > > called in nand_put_hw_ecc_engine().    
> > 
> > 
> > I see.
> > 
> > Then I prefer keeping the logic in the core, not in the engine driver
> > and propose a
> > 
> >         void nand_ecc_put_engine(struct nand_ecc_engine *engine)
> > 
> > which will do nothing for on-die/sw engines and drop the reference for
> > hw engines. I will also rename the "find_ecc_engine" to "get_engine" so
> > that the call to the "put" helper has more meaning.  
> 
> Ack for most of it. One thing I'd like to clarify: it's probably better
> to have a separate function called nand_ecc_put_hw_engine() which you'll
> call from nand_ecc_put_engine() when you're dealing with an
> HW ECC engine rather than calling put_device() directly from
> nand_ecc_put_engine(). This way you keep the code for HW ECC engine
> well isolated.
> 
> Same goes for the nand_ecc_get_engine() path, just delegate to
> nand_ecc_get_hw_engine() when ->provider == HW_ECC.

Ack.

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

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

end of thread, other threads:[~2019-02-27 14:34 UTC | newest]

Thread overview: 36+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-21 10:01 [RFC PATCH 00/27] Introduce the generic ECC engine abstraction Miquel Raynal
2019-02-21 10:01 ` [RFC PATCH 01/27] mtd: nand: Move nand_device forward declaration to the top Miquel Raynal
2019-02-21 10:01 ` [RFC PATCH 02/27] mtd: nand: Compile in the NAND core by default Miquel Raynal
2019-02-21 10:55   ` Boris Brezillon
2019-02-21 11:06     ` Miquel Raynal
2019-02-21 11:14       ` Boris Brezillon
2019-02-21 11:46         ` Miquel Raynal
2019-02-21 12:08           ` Boris Brezillon
2019-02-21 12:52             ` Miquel Raynal
2019-02-21 10:01 ` [RFC PATCH 03/27] mtd: nand: Introduce the ECC engine abstraction Miquel Raynal
2019-02-21 11:16   ` Boris Brezillon
2019-02-27  9:26     ` Miquel Raynal
2019-02-27  9:47       ` Boris Brezillon
2019-02-25 18:55   ` Boris Brezillon
2019-02-27 13:56     ` Miquel Raynal
2019-02-27 14:06       ` Boris Brezillon
2019-02-27 14:19         ` Miquel Raynal
2019-02-27 14:28           ` Boris Brezillon
2019-02-27 14:34             ` Miquel Raynal
2019-02-21 10:01 ` [RFC PATCH 04/27] mtd: Fix typo in mtd_ooblayout_set_databytes() description Miquel Raynal
2019-02-21 10:01 ` [RFC PATCH 05/27] mtd: nand: Move standard OOB layouts to the NAND core Miquel Raynal
2019-02-21 11:19   ` Boris Brezillon
2019-02-21 11:47     ` Miquel Raynal
2019-02-21 12:10       ` Boris Brezillon
2019-02-21 10:01 ` [RFC PATCH 06/27] mtd: nand: Move ECC specific functions to ecc/engine.c Miquel Raynal
2019-02-21 10:01 ` [RFC PATCH 07/27] mtd: nand: ecc: Move BCH code into the ecc/ directory Miquel Raynal
2019-02-21 10:01 ` [RFC PATCH 08/27] mtd: nand: ecc: Use SPDX license identifier for the software BCH code Miquel Raynal
2019-02-21 11:25   ` Boris Brezillon
2019-02-21 11:48     ` Miquel Raynal
2019-02-21 10:01 ` [RFC PATCH 09/27] mtd: nand: ecc: Turn the software BCH implementation generic Miquel Raynal
2019-02-21 12:26   ` Boris Brezillon
2019-02-21 12:53     ` Miquel Raynal
2019-02-21 10:01 ` [RFC PATCH 10/27] mtd: rawnand: Get rid of chip->ecc.priv Miquel Raynal
2019-02-21 13:01   ` Boris Brezillon
2019-02-21 10:02 ` [RFC PATCH 11/27] mtd: nand: ecc: Move Hamming code into the ecc/ directory Miquel Raynal
2019-02-21 10:02 ` [RFC PATCH 12/27] mtd: nand: ecc: Use SPDX license identifier for the software Hamming code Miquel Raynal

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