linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/18] External ECC engines & Macronix support
@ 2021-10-20 14:27 Miquel Raynal
  2021-10-20 14:27 ` [PATCH 01/18] dt-bindings: mtd: nand-controller: Fix the reg property description Miquel Raynal
                   ` (17 more replies)
  0 siblings, 18 replies; 35+ messages in thread
From: Miquel Raynal @ 2021-10-20 14:27 UTC (permalink / raw)
  To: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus,
	Mark Brown, Rob Herring
  Cc: linux-mtd, linux-spi, devicetree, linux-kernel, Julien Su,
	Jaime Liao, Thomas Petazzoni, Boris Brezillon, Xiangsheng Hou,
	Miquel Raynal

Hello all,

This series is now stable and brings support for external/modular ECC
engines, and let SPI controller using the ECC framework.

As a first example, Macronix ECC engine can be used as an
external engine (takes the data, proceeds to the calculations, writes
back the ECC bytes) or as a pipelined engine doing on-the-fly
calculations (which is very common in the raw NAND world).

In the device tree, the ECC engine should be described as a separated DT
node. Then:
* external case: the flash node should provide a nand-ecc-engine
  property pointing to the ECC engine node.
* pipelined case: the flash node should provide a nand-ecc-engine
  property pointing to the SPI controller, itself with another
  nand-ecc-engine property pointing at the ECC engine node.

This series comes with a bunch of improvements on the binding side as
well.

Cheers,
Miquèl

Changes since the RFC:
* Rebased on top of v5.15-rc1.
* Fixed the dirmap configuration.
* Added the various tags received.
* Fixed the bindings as reported by the robots.
* Fixed the return value of the helper counting bitflips.
* Included a fix from Jaime Liao in the external pattern logic.
* Added the yaml conversion of Macronix SPI controller description.
* Added the yaml conversion of the SPI-NAND description.
* Created a nand-chip.yaml file to share properties between SPI-NAND and
  raw NAND.

Mason Yang (1):
  mtd: spinand: macronix: Use random program load

Miquel Raynal (17):
  dt-bindings: mtd: nand-controller: Fix the reg property description
  dt-bindings: mtd: nand-controller: Fix a comment in the examples
  dt-bindings: mtd: nand-chip: Create a NAND chip description
  dt-bindings: mtd: spi-nand: Convert spi-nand description file to yaml
  dt-bindings: vendor-prefixes: Clarify Macronix prefix
  dt-bindings: spi: mxic: The interrupt property is not mandatory
  dt-bindings: spi: mxic: Convert to yaml
  dt-bindings: mtd: Describe Macronix NAND ECC engine
  dt-bindings: spi: mxic: Document the nand-ecc-engine property
  mtd: nand: ecc: Add infrastructure to support hardware engines
  mtd: nand: mxic-ecc: Add Macronix external ECC engine support
  mtd: nand: mxic-ecc: Support SPI pipelined mode
  spi: mxic: Fix the transmit path
  spi: mxic: Create a helper to configure the controller before an
    operation
  spi: mxic: Create a helper to ease the start of an operation
  spi: mxic: Add support for direct mapping
  spi: mxic: Add support for pipelined ECC operations

 .../bindings/mtd/mxicy,nand-ecc-engine.yaml   |  77 ++
 .../devicetree/bindings/mtd/nand-chip.yaml    |  71 ++
 .../bindings/mtd/nand-controller.yaml         |  57 +-
 .../devicetree/bindings/mtd/spi-nand.txt      |   5 -
 .../devicetree/bindings/mtd/spi-nand.yaml     |  27 +
 .../bindings/spi/mxicy,mx25f0a-spi.yaml       |  73 ++
 .../devicetree/bindings/spi/spi-mxic.txt      |  34 -
 .../devicetree/bindings/vendor-prefixes.yaml  |   3 +
 drivers/mtd/nand/Kconfig                      |   6 +
 drivers/mtd/nand/Makefile                     |   1 +
 drivers/mtd/nand/core.c                       |  10 +-
 drivers/mtd/nand/ecc-mxic.c                   | 799 ++++++++++++++++++
 drivers/mtd/nand/ecc.c                        |  89 ++
 drivers/mtd/nand/spi/macronix.c               |   2 +-
 drivers/spi/spi-mxic.c                        | 328 +++++--
 include/linux/mtd/nand-ecc-mxic.h             |  36 +
 include/linux/mtd/nand.h                      |  11 +
 17 files changed, 1483 insertions(+), 146 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mtd/mxicy,nand-ecc-engine.yaml
 create mode 100644 Documentation/devicetree/bindings/mtd/nand-chip.yaml
 delete mode 100644 Documentation/devicetree/bindings/mtd/spi-nand.txt
 create mode 100644 Documentation/devicetree/bindings/mtd/spi-nand.yaml
 create mode 100644 Documentation/devicetree/bindings/spi/mxicy,mx25f0a-spi.yaml
 delete mode 100644 Documentation/devicetree/bindings/spi/spi-mxic.txt
 create mode 100644 drivers/mtd/nand/ecc-mxic.c
 create mode 100644 include/linux/mtd/nand-ecc-mxic.h

-- 
2.27.0


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

* [PATCH 01/18] dt-bindings: mtd: nand-controller: Fix the reg property description
  2021-10-20 14:27 [PATCH 00/18] External ECC engines & Macronix support Miquel Raynal
@ 2021-10-20 14:27 ` Miquel Raynal
  2021-10-28 21:25   ` Rob Herring
  2021-10-20 14:27 ` [PATCH 02/18] dt-bindings: mtd: nand-controller: Fix a comment in the examples Miquel Raynal
                   ` (16 subsequent siblings)
  17 siblings, 1 reply; 35+ messages in thread
From: Miquel Raynal @ 2021-10-20 14:27 UTC (permalink / raw)
  To: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus,
	Mark Brown, Rob Herring
  Cc: linux-mtd, linux-spi, devicetree, linux-kernel, Julien Su,
	Jaime Liao, Thomas Petazzoni, Boris Brezillon, Xiangsheng Hou,
	Miquel Raynal

The reg property of a NAND device always references the chip-select(s).
The ready/busy lines are described in the nand-rb property. I believe
this was a harmless copy/paste error during the conversion to yaml.

Fixes: 212e49693592 ("dt-bindings: mtd: Add YAML schemas for the generic NAND options")
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 Documentation/devicetree/bindings/mtd/nand-controller.yaml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/mtd/nand-controller.yaml b/Documentation/devicetree/bindings/mtd/nand-controller.yaml
index bd217e6f5018..811f03978fc6 100644
--- a/Documentation/devicetree/bindings/mtd/nand-controller.yaml
+++ b/Documentation/devicetree/bindings/mtd/nand-controller.yaml
@@ -55,7 +55,7 @@ patternProperties:
     properties:
       reg:
         description:
-          Contains the native Ready/Busy IDs.
+          Contains the chip-select IDs.
 
       nand-ecc-engine:
         allOf:
-- 
2.27.0


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

* [PATCH 02/18] dt-bindings: mtd: nand-controller: Fix a comment in the examples
  2021-10-20 14:27 [PATCH 00/18] External ECC engines & Macronix support Miquel Raynal
  2021-10-20 14:27 ` [PATCH 01/18] dt-bindings: mtd: nand-controller: Fix the reg property description Miquel Raynal
@ 2021-10-20 14:27 ` Miquel Raynal
  2021-10-28 21:26   ` Rob Herring
  2021-10-20 14:27 ` [PATCH 03/18] dt-bindings: mtd: nand-chip: Create a NAND chip description Miquel Raynal
                   ` (15 subsequent siblings)
  17 siblings, 1 reply; 35+ messages in thread
From: Miquel Raynal @ 2021-10-20 14:27 UTC (permalink / raw)
  To: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus,
	Mark Brown, Rob Herring
  Cc: linux-mtd, linux-spi, devicetree, linux-kernel, Julien Su,
	Jaime Liao, Thomas Petazzoni, Boris Brezillon, Xiangsheng Hou,
	Miquel Raynal

The controller properties should be in the controller 'parent' node,
while properties in the children nodes are specific to the NAND
*chip*. This error was already present during the yaml conversion.

Fixes: 2d472aba15ff ("mtd: nand: document the NAND controller/NAND chip DT representation")
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 Documentation/devicetree/bindings/mtd/nand-controller.yaml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/mtd/nand-controller.yaml b/Documentation/devicetree/bindings/mtd/nand-controller.yaml
index 811f03978fc6..5cd144a9ec99 100644
--- a/Documentation/devicetree/bindings/mtd/nand-controller.yaml
+++ b/Documentation/devicetree/bindings/mtd/nand-controller.yaml
@@ -184,7 +184,7 @@ examples:
         nand-use-soft-ecc-engine;
         nand-ecc-algo = "bch";
 
-        /* controller specific properties */
+        /* NAND chip specific properties */
       };
 
       nand@1 {
-- 
2.27.0


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

* [PATCH 03/18] dt-bindings: mtd: nand-chip: Create a NAND chip description
  2021-10-20 14:27 [PATCH 00/18] External ECC engines & Macronix support Miquel Raynal
  2021-10-20 14:27 ` [PATCH 01/18] dt-bindings: mtd: nand-controller: Fix the reg property description Miquel Raynal
  2021-10-20 14:27 ` [PATCH 02/18] dt-bindings: mtd: nand-controller: Fix a comment in the examples Miquel Raynal
@ 2021-10-20 14:27 ` Miquel Raynal
  2021-10-20 21:14   ` Rob Herring
  2021-10-22 22:47   ` Rob Herring
  2021-10-20 14:27 ` [PATCH 04/18] dt-bindings: mtd: spi-nand: Convert spi-nand description file to yaml Miquel Raynal
                   ` (14 subsequent siblings)
  17 siblings, 2 replies; 35+ messages in thread
From: Miquel Raynal @ 2021-10-20 14:27 UTC (permalink / raw)
  To: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus,
	Mark Brown, Rob Herring
  Cc: linux-mtd, linux-spi, devicetree, linux-kernel, Julien Su,
	Jaime Liao, Thomas Petazzoni, Boris Brezillon, Xiangsheng Hou,
	Miquel Raynal

Move the NAND chip description out of the NAND controller file. Indeed,
a subsequent part of the properties supported by a raw NAND chip are
also supported by SPI-NAND chips. So let's create a generic NAND chip
description which will be pulled by nand-controller.yaml and later by
spi-nand.yaml as well.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 .../devicetree/bindings/mtd/nand-chip.yaml    | 71 +++++++++++++++++++
 .../bindings/mtd/nand-controller.yaml         | 53 ++------------
 2 files changed, 75 insertions(+), 49 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mtd/nand-chip.yaml

diff --git a/Documentation/devicetree/bindings/mtd/nand-chip.yaml b/Documentation/devicetree/bindings/mtd/nand-chip.yaml
new file mode 100644
index 000000000000..1f230a3ee27d
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/nand-chip.yaml
@@ -0,0 +1,71 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mtd/nand-chip.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NAND Chip and NAND Controller Generic Binding
+
+maintainers:
+  - Miquel Raynal <miquel.raynal@bootlin.com>
+
+description: |
+  This file covers the generic description of a NAND chip. It implies that the
+  bus interface should not be taken into account: both raw NAND devices and
+  SPI-NAND devices are concerned by this description.
+
+properties:
+  reg:
+    description:
+      Contains the chip-select IDs.
+
+  nand-ecc-engine:
+    allOf:
+      - $ref: /schemas/types.yaml#/definitions/phandle
+    description: |
+      A phandle on the hardware ECC engine if any. There are
+      basically three possibilities:
+      1/ The ECC engine is part of the NAND controller, in this
+      case the phandle should reference the parent node.
+      2/ The ECC engine is part of the NAND part (on-die), in this
+      case the phandle should reference the node itself.
+      3/ The ECC engine is external, in this case the phandle should
+      reference the specific ECC engine node.
+
+  nand-use-soft-ecc-engine:
+    type: boolean
+    description: Use a software ECC engine.
+
+  nand-no-ecc-engine:
+    type: boolean
+    description: Do not use any ECC correction.
+
+  nand-ecc-algo:
+    description:
+      Desired ECC algorithm.
+    $ref: /schemas/types.yaml#/definitions/string
+    enum: [hamming, bch, rs]
+
+  nand-ecc-strength:
+    description:
+      Maximum number of bits that can be corrected per ECC step.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 1
+
+  nand-ecc-step-size:
+    description:
+      Number of data bytes covered by a single ECC step.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 1
+
+  secure-regions:
+    $ref: /schemas/types.yaml#/definitions/uint64-matrix
+    description:
+      Regions in the NAND chip which are protected using a secure element
+      like Trustzone. This property contains the start address and size of
+      the secure regions present.
+
+required:
+  - reg
+
+additionalProperties: false
diff --git a/Documentation/devicetree/bindings/mtd/nand-controller.yaml b/Documentation/devicetree/bindings/mtd/nand-controller.yaml
index 5cd144a9ec99..44825dc95412 100644
--- a/Documentation/devicetree/bindings/mtd/nand-controller.yaml
+++ b/Documentation/devicetree/bindings/mtd/nand-controller.yaml
@@ -52,32 +52,15 @@ properties:
 patternProperties:
   "^nand@[a-f0-9]$":
     type: object
+
+    allOf:
+      - $ref: "nand-chip.yaml#"
+
     properties:
       reg:
         description:
           Contains the chip-select IDs.
 
-      nand-ecc-engine:
-        allOf:
-          - $ref: /schemas/types.yaml#/definitions/phandle
-        description: |
-          A phandle on the hardware ECC engine if any. There are
-          basically three possibilities:
-          1/ The ECC engine is part of the NAND controller, in this
-          case the phandle should reference the parent node.
-          2/ The ECC engine is part of the NAND part (on-die), in this
-          case the phandle should reference the node itself.
-          3/ The ECC engine is external, in this case the phandle should
-          reference the specific ECC engine node.
-
-      nand-use-soft-ecc-engine:
-        type: boolean
-        description: Use a software ECC engine.
-
-      nand-no-ecc-engine:
-        type: boolean
-        description: Do not use any ECC correction.
-
       nand-ecc-placement:
         allOf:
           - $ref: /schemas/types.yaml#/definitions/string
@@ -88,12 +71,6 @@ patternProperties:
           known to be stored in the OOB area, or "interleaved" if ECC
           bytes will be interleaved with regular data in the main area.
 
-      nand-ecc-algo:
-        description:
-          Desired ECC algorithm.
-        $ref: /schemas/types.yaml#/definitions/string
-        enum: [hamming, bch, rs]
-
       nand-bus-width:
         description:
           Bus width to the NAND chip
@@ -112,18 +89,6 @@ patternProperties:
           find Bad Block Markers (BBM). These markers will help to
           build a volatile BBT in RAM.
 
-      nand-ecc-strength:
-        description:
-          Maximum number of bits that can be corrected per ECC step.
-        $ref: /schemas/types.yaml#/definitions/uint32
-        minimum: 1
-
-      nand-ecc-step-size:
-        description:
-          Number of data bytes covered by a single ECC step.
-        $ref: /schemas/types.yaml#/definitions/uint32
-        minimum: 1
-
       nand-ecc-maximize:
         $ref: /schemas/types.yaml#/definitions/flag
         description:
@@ -154,13 +119,6 @@ patternProperties:
           Ready/Busy pins. Active state refers to the NAND ready state and
           should be set to GPIOD_ACTIVE_HIGH unless the signal is inverted.
 
-      secure-regions:
-        $ref: /schemas/types.yaml#/definitions/uint64-matrix
-        description:
-          Regions in the NAND chip which are protected using a secure element
-          like Trustzone. This property contains the start address and size of
-          the secure regions present.
-
     required:
       - reg
 
@@ -181,9 +139,6 @@ examples:
 
       nand@0 {
         reg = <0>; /* Native CS */
-        nand-use-soft-ecc-engine;
-        nand-ecc-algo = "bch";
-
         /* NAND chip specific properties */
       };
 
-- 
2.27.0


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

* [PATCH 04/18] dt-bindings: mtd: spi-nand: Convert spi-nand description file to yaml
  2021-10-20 14:27 [PATCH 00/18] External ECC engines & Macronix support Miquel Raynal
                   ` (2 preceding siblings ...)
  2021-10-20 14:27 ` [PATCH 03/18] dt-bindings: mtd: nand-chip: Create a NAND chip description Miquel Raynal
@ 2021-10-20 14:27 ` Miquel Raynal
  2021-10-20 21:14   ` Rob Herring
  2021-10-28 21:28   ` Rob Herring
  2021-10-20 14:27 ` [PATCH 05/18] dt-bindings: vendor-prefixes: Clarify Macronix prefix Miquel Raynal
                   ` (13 subsequent siblings)
  17 siblings, 2 replies; 35+ messages in thread
From: Miquel Raynal @ 2021-10-20 14:27 UTC (permalink / raw)
  To: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus,
	Mark Brown, Rob Herring
  Cc: linux-mtd, linux-spi, devicetree, linux-kernel, Julien Su,
	Jaime Liao, Thomas Petazzoni, Boris Brezillon, Xiangsheng Hou,
	Miquel Raynal

Let's get rid of spi-nand.txt by converting it to yaml schema. While at
converting this file, let's actually pull all the generic properties
from nand-chip.yaml which might apply to a SPI-NAND chip.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 .../devicetree/bindings/mtd/spi-nand.txt      |  5 ----
 .../devicetree/bindings/mtd/spi-nand.yaml     | 27 +++++++++++++++++++
 2 files changed, 27 insertions(+), 5 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/mtd/spi-nand.txt
 create mode 100644 Documentation/devicetree/bindings/mtd/spi-nand.yaml

diff --git a/Documentation/devicetree/bindings/mtd/spi-nand.txt b/Documentation/devicetree/bindings/mtd/spi-nand.txt
deleted file mode 100644
index 8b51f3b6d55c..000000000000
--- a/Documentation/devicetree/bindings/mtd/spi-nand.txt
+++ /dev/null
@@ -1,5 +0,0 @@
-SPI NAND flash
-
-Required properties:
-- compatible: should be "spi-nand"
-- reg: should encode the chip-select line used to access the NAND chip
diff --git a/Documentation/devicetree/bindings/mtd/spi-nand.yaml b/Documentation/devicetree/bindings/mtd/spi-nand.yaml
new file mode 100644
index 000000000000..19ac57dcffc3
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/spi-nand.yaml
@@ -0,0 +1,27 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mtd/spi-nand.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: SPI-NAND flash device tree bindings
+
+maintainers:
+  - Miquel Raynal <miquel.raynal@bootlin.com>
+
+allOf:
+  - $ref: "nand-chip.yaml#"
+
+properties:
+  compatible:
+    const: spi-nand
+
+  reg:
+    maxItems: 1
+    description: Encode the chip-select line on the SPI bus
+
+required:
+  - compatible
+  - reg
+
+unevaluatedProperties: false
-- 
2.27.0


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

* [PATCH 05/18] dt-bindings: vendor-prefixes: Clarify Macronix prefix
  2021-10-20 14:27 [PATCH 00/18] External ECC engines & Macronix support Miquel Raynal
                   ` (3 preceding siblings ...)
  2021-10-20 14:27 ` [PATCH 04/18] dt-bindings: mtd: spi-nand: Convert spi-nand description file to yaml Miquel Raynal
@ 2021-10-20 14:27 ` Miquel Raynal
  2021-10-28 21:31   ` Rob Herring
  2021-10-20 14:27 ` [PATCH 06/18] dt-bindings: spi: mxic: The interrupt property is not mandatory Miquel Raynal
                   ` (12 subsequent siblings)
  17 siblings, 1 reply; 35+ messages in thread
From: Miquel Raynal @ 2021-10-20 14:27 UTC (permalink / raw)
  To: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus,
	Mark Brown, Rob Herring
  Cc: linux-mtd, linux-spi, devicetree, linux-kernel, Julien Su,
	Jaime Liao, Thomas Petazzoni, Boris Brezillon, Xiangsheng Hou,
	Miquel Raynal

When looking at compatible prefixes, Macronix is sometimes referred as
"mxicy":
- mxicy,mx25r1635f
- mxicy,mx25u6435f
- mxicy,mx25v8035f
- mxicy,mx25f0a-spi
and sometimes as "mxic":
- mxic,multi-itfc-v009-nand-controller
- mxic,enable-randomizer-otp

The oldest prefix that is also the one preferred by Macronix engineers
is "mxicy", so document the other one and mark it deprecated.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 Documentation/devicetree/bindings/vendor-prefixes.yaml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index a867f7102c35..93d65dc3746c 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -774,6 +774,9 @@ patternProperties:
     description: Mundo Reader S.L.
   "^murata,.*":
     description: Murata Manufacturing Co., Ltd.
+  "^mxic,.*":
+    description: Macronix International Co., Ltd.
+    deprecated: true
   "^mxicy,.*":
     description: Macronix International Co., Ltd.
   "^myir,.*":
-- 
2.27.0


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

* [PATCH 06/18] dt-bindings: spi: mxic: The interrupt property is not mandatory
  2021-10-20 14:27 [PATCH 00/18] External ECC engines & Macronix support Miquel Raynal
                   ` (4 preceding siblings ...)
  2021-10-20 14:27 ` [PATCH 05/18] dt-bindings: vendor-prefixes: Clarify Macronix prefix Miquel Raynal
@ 2021-10-20 14:27 ` Miquel Raynal
  2021-10-28 21:32   ` Rob Herring
  2021-10-20 14:27 ` [PATCH 07/18] dt-bindings: spi: mxic: Convert to yaml Miquel Raynal
                   ` (11 subsequent siblings)
  17 siblings, 1 reply; 35+ messages in thread
From: Miquel Raynal @ 2021-10-20 14:27 UTC (permalink / raw)
  To: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus,
	Mark Brown, Rob Herring
  Cc: linux-mtd, linux-spi, devicetree, linux-kernel, Julien Su,
	Jaime Liao, Thomas Petazzoni, Boris Brezillon, Xiangsheng Hou,
	Miquel Raynal

The interrupt property is not mandatory at all, this property should not
be part of the required properties list, so move it into the optional
properties list.

Fixes: 326e5c8d4a87 ("dt-binding: spi: Document Macronix controller bindings")
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 Documentation/devicetree/bindings/spi/spi-mxic.txt | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/spi/spi-mxic.txt b/Documentation/devicetree/bindings/spi/spi-mxic.txt
index 529f2dab2648..7bcbb229b78b 100644
--- a/Documentation/devicetree/bindings/spi/spi-mxic.txt
+++ b/Documentation/devicetree/bindings/spi/spi-mxic.txt
@@ -8,11 +8,13 @@ Required properties:
 - reg: should contain 2 entries, one for the registers and one for the direct
        mapping area
 - reg-names: should contain "regs" and "dirmap"
-- interrupts: interrupt line connected to the SPI controller
 - clock-names: should contain "ps_clk", "send_clk" and "send_dly_clk"
 - clocks: should contain 3 entries for the "ps_clk", "send_clk" and
 	  "send_dly_clk" clocks
 
+Optional properties:
+- interrupts: interrupt line connected to the SPI controller
+
 Example:
 
 	spi@43c30000 {
-- 
2.27.0


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

* [PATCH 07/18] dt-bindings: spi: mxic: Convert to yaml
  2021-10-20 14:27 [PATCH 00/18] External ECC engines & Macronix support Miquel Raynal
                   ` (5 preceding siblings ...)
  2021-10-20 14:27 ` [PATCH 06/18] dt-bindings: spi: mxic: The interrupt property is not mandatory Miquel Raynal
@ 2021-10-20 14:27 ` Miquel Raynal
  2021-10-28 21:34   ` Rob Herring
  2021-10-20 14:27 ` [PATCH 08/18] dt-bindings: mtd: Describe Macronix NAND ECC engine Miquel Raynal
                   ` (10 subsequent siblings)
  17 siblings, 1 reply; 35+ messages in thread
From: Miquel Raynal @ 2021-10-20 14:27 UTC (permalink / raw)
  To: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus,
	Mark Brown, Rob Herring
  Cc: linux-mtd, linux-spi, devicetree, linux-kernel, Julien Su,
	Jaime Liao, Thomas Petazzoni, Boris Brezillon, Xiangsheng Hou,
	Miquel Raynal

Straightforward conversion from regular text to yaml schema of the
Macronix SPI controller DT bindings.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 .../bindings/spi/mxicy,mx25f0a-spi.yaml       | 67 +++++++++++++++++++
 .../devicetree/bindings/spi/spi-mxic.txt      | 36 ----------
 2 files changed, 67 insertions(+), 36 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/spi/mxicy,mx25f0a-spi.yaml
 delete mode 100644 Documentation/devicetree/bindings/spi/spi-mxic.txt

diff --git a/Documentation/devicetree/bindings/spi/mxicy,mx25f0a-spi.yaml b/Documentation/devicetree/bindings/spi/mxicy,mx25f0a-spi.yaml
new file mode 100644
index 000000000000..4036c14fc533
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/mxicy,mx25f0a-spi.yaml
@@ -0,0 +1,67 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/spi/mxicy,mx25f0a-spi.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Macronix SPI controller device tree bindings
+
+maintainers:
+  - Miquel Raynal <miquel.raynal@bootlin.com>
+
+allOf:
+  - $ref: "spi-controller.yaml#"
+
+properties:
+  compatible:
+    const: mxicy,mx25f0a-spi
+
+  reg:
+    minItems: 2
+    maxItems: 2
+
+  reg-names:
+    items:
+      - const: regs
+      - const: dirmap
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    minItems: 3
+    maxItems: 3
+
+  clock-names:
+    items:
+      - const: send_clk
+      - const: send_dly_clk
+      - const: ps_clk
+
+  "#address-cells":
+    const: 1
+  "#size-cells":
+    const: 0
+
+required:
+  - compatible
+  - reg
+  - reg-names
+  - clocks
+  - clock-names
+  - "#address-cells"
+  - "#size-cells"
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    spi@43c30000 {
+      compatible = "mxicy,mx25f0a-spi";
+      reg = <0x43c30000 0x10000>, <0xa0000000 0x20000000>;
+      reg-names = "regs", "dirmap";
+      clocks = <&clkwizard 0>, <&clkwizard 1>, <&clkc 18>;
+      clock-names = "send_clk", "send_dly_clk", "ps_clk";
+      #address-cells = <1>;
+      #size-cells = <0>;
+    };
diff --git a/Documentation/devicetree/bindings/spi/spi-mxic.txt b/Documentation/devicetree/bindings/spi/spi-mxic.txt
deleted file mode 100644
index 7bcbb229b78b..000000000000
--- a/Documentation/devicetree/bindings/spi/spi-mxic.txt
+++ /dev/null
@@ -1,36 +0,0 @@
-Macronix SPI controller Device Tree Bindings
---------------------------------------------
-
-Required properties:
-- compatible: should be "mxicy,mx25f0a-spi"
-- #address-cells: should be 1
-- #size-cells: should be 0
-- reg: should contain 2 entries, one for the registers and one for the direct
-       mapping area
-- reg-names: should contain "regs" and "dirmap"
-- clock-names: should contain "ps_clk", "send_clk" and "send_dly_clk"
-- clocks: should contain 3 entries for the "ps_clk", "send_clk" and
-	  "send_dly_clk" clocks
-
-Optional properties:
-- interrupts: interrupt line connected to the SPI controller
-
-Example:
-
-	spi@43c30000 {
-		compatible = "mxicy,mx25f0a-spi";
-		reg = <0x43c30000 0x10000>, <0xa0000000 0x20000000>;
-		reg-names = "regs", "dirmap";
-		clocks = <&clkwizard 0>, <&clkwizard 1>, <&clkc 18>;
-		clock-names = "send_clk", "send_dly_clk", "ps_clk";
-		#address-cells = <1>;
-		#size-cells = <0>;
-
-		flash@0 {
-			compatible = "jedec,spi-nor";
-			reg = <0>;
-			spi-max-frequency = <25000000>;
-			spi-tx-bus-width = <4>;
-			spi-rx-bus-width = <4>;
-		};
-	};
-- 
2.27.0


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

* [PATCH 08/18] dt-bindings: mtd: Describe Macronix NAND ECC engine
  2021-10-20 14:27 [PATCH 00/18] External ECC engines & Macronix support Miquel Raynal
                   ` (6 preceding siblings ...)
  2021-10-20 14:27 ` [PATCH 07/18] dt-bindings: spi: mxic: Convert to yaml Miquel Raynal
@ 2021-10-20 14:27 ` Miquel Raynal
  2021-10-28 21:35   ` Rob Herring
  2021-10-20 14:28 ` [PATCH 09/18] dt-bindings: spi: mxic: Document the nand-ecc-engine property Miquel Raynal
                   ` (9 subsequent siblings)
  17 siblings, 1 reply; 35+ messages in thread
From: Miquel Raynal @ 2021-10-20 14:27 UTC (permalink / raw)
  To: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus,
	Mark Brown, Rob Herring
  Cc: linux-mtd, linux-spi, devicetree, linux-kernel, Julien Su,
	Jaime Liao, Thomas Petazzoni, Boris Brezillon, Xiangsheng Hou,
	Miquel Raynal

Describe Macronix NAND ECC engine. This engine may be used as an
external engine or can be pipelined with either a raw NAND controller or
a SPI controller. Both hardware designs with a SPI controller are shown
in the examples.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 .../bindings/mtd/mxicy,nand-ecc-engine.yaml   | 77 +++++++++++++++++++
 1 file changed, 77 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mtd/mxicy,nand-ecc-engine.yaml

diff --git a/Documentation/devicetree/bindings/mtd/mxicy,nand-ecc-engine.yaml b/Documentation/devicetree/bindings/mtd/mxicy,nand-ecc-engine.yaml
new file mode 100644
index 000000000000..804479999ccb
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/mxicy,nand-ecc-engine.yaml
@@ -0,0 +1,77 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mtd/mxicy,nand-ecc-engine.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Macronix NAND ECC engine device tree bindings
+
+maintainers:
+  - Miquel Raynal <miquel.raynal@bootlin.com>
+
+properties:
+  compatible:
+    const: mxicy,nand-ecc-engine-rev3
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    /* External configuration */
+    spi_controller0: spi@43c30000 {
+        compatible = "mxicy,mx25f0a-spi";
+        reg = <0x43c30000 0x10000>, <0xa0000000 0x4000000>;
+        reg-names = "regs", "dirmap";
+        clocks = <&clkwizard 0>, <&clkwizard 1>, <&clkc 15>;
+        clock-names = "send_clk", "send_dly_clk", "ps_clk";
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        flash@0 {
+            compatible = "spi-nand";
+            reg = <0>;
+            nand-ecc-engine = <&ecc_engine0>;
+        };
+    };
+
+    ecc_engine0: ecc@43c40000 {
+        compatible = "mxicy,nand-ecc-engine-rev3";
+        reg = <0x43c40000 0x10000>;
+    };
+
+  - |
+    /* Pipelined configuration */
+    spi_controller1: spi@43c30000 {
+        compatible = "mxicy,mx25f0a-spi";
+        reg = <0x43c30000 0x10000>, <0xa0000000 0x4000000>;
+        reg-names = "regs", "dirmap";
+        clocks = <&clkwizard 0>, <&clkwizard 1>, <&clkc 15>;
+        clock-names = "send_clk", "send_dly_clk", "ps_clk";
+        #address-cells = <1>;
+        #size-cells = <0>;
+        nand-ecc-engine = <&ecc_engine1>;
+
+        flash@0 {
+            compatible = "spi-nand";
+            reg = <0>;
+            nand-ecc-engine = <&spi_controller1>;
+        };
+    };
+
+    ecc_engine1: ecc@43c40000 {
+        compatible = "mxicy,nand-ecc-engine-rev3";
+        reg = <0x43c40000 0x10000>;
+    };
-- 
2.27.0


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

* [PATCH 09/18] dt-bindings: spi: mxic: Document the nand-ecc-engine property
  2021-10-20 14:27 [PATCH 00/18] External ECC engines & Macronix support Miquel Raynal
                   ` (7 preceding siblings ...)
  2021-10-20 14:27 ` [PATCH 08/18] dt-bindings: mtd: Describe Macronix NAND ECC engine Miquel Raynal
@ 2021-10-20 14:28 ` Miquel Raynal
  2021-10-28 21:38   ` Rob Herring
  2021-10-20 14:28 ` [PATCH 10/18] mtd: spinand: macronix: Use random program load Miquel Raynal
                   ` (8 subsequent siblings)
  17 siblings, 1 reply; 35+ messages in thread
From: Miquel Raynal @ 2021-10-20 14:28 UTC (permalink / raw)
  To: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus,
	Mark Brown, Rob Herring
  Cc: linux-mtd, linux-spi, devicetree, linux-kernel, Julien Su,
	Jaime Liao, Thomas Petazzoni, Boris Brezillon, Xiangsheng Hou,
	Miquel Raynal

This SPI controller supports interacting with an external ECC
engine. The nand-ecc-engine property already exist in the NAND world but
also applies to SPI controller nodes which have external correction
capabilities like Macronix's.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 .../devicetree/bindings/spi/mxicy,mx25f0a-spi.yaml          | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/spi/mxicy,mx25f0a-spi.yaml b/Documentation/devicetree/bindings/spi/mxicy,mx25f0a-spi.yaml
index 4036c14fc533..01618a77627d 100644
--- a/Documentation/devicetree/bindings/spi/mxicy,mx25f0a-spi.yaml
+++ b/Documentation/devicetree/bindings/spi/mxicy,mx25f0a-spi.yaml
@@ -43,6 +43,12 @@ properties:
   "#size-cells":
     const: 0
 
+  nand-ecc-engine:
+    allOf:
+      - $ref: /schemas/types.yaml#/definitions/phandle
+    description: NAND ECC engine used by the SPI controller in order to perform
+      on-the-fly correction when using a SPI-NAND memory.
+
 required:
   - compatible
   - reg
-- 
2.27.0


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

* [PATCH 10/18] mtd: spinand: macronix: Use random program load
  2021-10-20 14:27 [PATCH 00/18] External ECC engines & Macronix support Miquel Raynal
                   ` (8 preceding siblings ...)
  2021-10-20 14:28 ` [PATCH 09/18] dt-bindings: spi: mxic: Document the nand-ecc-engine property Miquel Raynal
@ 2021-10-20 14:28 ` Miquel Raynal
  2021-10-20 14:28 ` [PATCH 11/18] mtd: nand: ecc: Add infrastructure to support hardware engines Miquel Raynal
                   ` (7 subsequent siblings)
  17 siblings, 0 replies; 35+ messages in thread
From: Miquel Raynal @ 2021-10-20 14:28 UTC (permalink / raw)
  To: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus,
	Mark Brown, Rob Herring
  Cc: linux-mtd, linux-spi, devicetree, linux-kernel, Julien Su,
	Jaime Liao, Thomas Petazzoni, Boris Brezillon, Xiangsheng Hou,
	Mason Yang, Miquel Raynal

From: Mason Yang <masonccyang@mxic.com.tw>

Macronix SPI-NAND chips might benefit from an external ECC
engine. Such an engine might need to access random columns, thus needing
to use random commands (0x84 instead of 0x02).

Signed-off-by: Mason Yang <masonccyang@mxic.com.tw>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/spi/macronix.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/spi/macronix.c b/drivers/mtd/nand/spi/macronix.c
index 3f31f1381a62..dce835132a1e 100644
--- a/drivers/mtd/nand/spi/macronix.c
+++ b/drivers/mtd/nand/spi/macronix.c
@@ -20,7 +20,7 @@ static SPINAND_OP_VARIANTS(read_cache_variants,
 
 static SPINAND_OP_VARIANTS(write_cache_variants,
 		SPINAND_PROG_LOAD_X4(true, 0, NULL, 0),
-		SPINAND_PROG_LOAD(true, 0, NULL, 0));
+		SPINAND_PROG_LOAD(false, 0, NULL, 0));
 
 static SPINAND_OP_VARIANTS(update_cache_variants,
 		SPINAND_PROG_LOAD_X4(false, 0, NULL, 0),
-- 
2.27.0


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

* [PATCH 11/18] mtd: nand: ecc: Add infrastructure to support hardware engines
  2021-10-20 14:27 [PATCH 00/18] External ECC engines & Macronix support Miquel Raynal
                   ` (9 preceding siblings ...)
  2021-10-20 14:28 ` [PATCH 10/18] mtd: spinand: macronix: Use random program load Miquel Raynal
@ 2021-10-20 14:28 ` Miquel Raynal
  2021-10-20 14:28 ` [PATCH 12/18] mtd: nand: mxic-ecc: Add Macronix external ECC engine support Miquel Raynal
                   ` (6 subsequent siblings)
  17 siblings, 0 replies; 35+ messages in thread
From: Miquel Raynal @ 2021-10-20 14:28 UTC (permalink / raw)
  To: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus,
	Mark Brown, Rob Herring
  Cc: linux-mtd, linux-spi, devicetree, linux-kernel, Julien Su,
	Jaime Liao, Thomas Petazzoni, Boris Brezillon, Xiangsheng Hou,
	Miquel Raynal

Add the necessary helpers to register/unregister hardware ECC engines
that will be called from ECC engine drivers.

Also add helpers to get the right engine from the user
perspective. Keep a reference of the in use ECC engine in order to
prevent modules to be unloaded. Put the reference when the engine gets
retired.

A static list of hardware (only) ECC engines is setup to keep track of
the registered engines.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/core.c  | 10 +++--
 drivers/mtd/nand/ecc.c   | 89 ++++++++++++++++++++++++++++++++++++++++
 include/linux/mtd/nand.h | 11 +++++
 3 files changed, 107 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/nand/core.c b/drivers/mtd/nand/core.c
index df9646685c91..5628bd410f7e 100644
--- a/drivers/mtd/nand/core.c
+++ b/drivers/mtd/nand/core.c
@@ -232,7 +232,9 @@ static int nanddev_get_ecc_engine(struct nand_device *nand)
 		nand->ecc.engine = nand_ecc_get_on_die_hw_engine(nand);
 		break;
 	case NAND_ECC_ENGINE_TYPE_ON_HOST:
-		pr_err("On-host hardware ECC engines not supported yet\n");
+		nand->ecc.engine = nand_ecc_get_on_host_hw_engine(nand);
+		if (PTR_ERR(nand->ecc.engine) == -EPROBE_DEFER)
+			return -EPROBE_DEFER;
 		break;
 	default:
 		pr_err("Missing ECC engine type\n");
@@ -252,7 +254,7 @@ static int nanddev_put_ecc_engine(struct nand_device *nand)
 {
 	switch (nand->ecc.ctx.conf.engine_type) {
 	case NAND_ECC_ENGINE_TYPE_ON_HOST:
-		pr_err("On-host hardware ECC engines not supported yet\n");
+		nand_ecc_put_on_host_hw_engine(nand);
 		break;
 	case NAND_ECC_ENGINE_TYPE_NONE:
 	case NAND_ECC_ENGINE_TYPE_SOFT:
@@ -297,7 +299,9 @@ int nanddev_ecc_engine_init(struct nand_device *nand)
 	/* Look for the ECC engine to use */
 	ret = nanddev_get_ecc_engine(nand);
 	if (ret) {
-		pr_err("No ECC engine found\n");
+		if (ret != -EPROBE_DEFER)
+			pr_err("No ECC engine found\n");
+
 		return ret;
 	}
 
diff --git a/drivers/mtd/nand/ecc.c b/drivers/mtd/nand/ecc.c
index 6c43dfda01d4..3aa56c990d11 100644
--- a/drivers/mtd/nand/ecc.c
+++ b/drivers/mtd/nand/ecc.c
@@ -96,6 +96,12 @@
 #include <linux/module.h>
 #include <linux/mtd/nand.h>
 #include <linux/slab.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_platform.h>
+
+static LIST_HEAD(on_host_hw_engines);
+static DEFINE_MUTEX(on_host_hw_engines_mutex);
 
 /**
  * nand_ecc_init_ctx - Init the ECC engine context
@@ -611,6 +617,89 @@ struct nand_ecc_engine *nand_ecc_get_on_die_hw_engine(struct nand_device *nand)
 }
 EXPORT_SYMBOL(nand_ecc_get_on_die_hw_engine);
 
+int nand_ecc_register_on_host_hw_engine(struct nand_ecc_engine *engine)
+{
+	struct nand_ecc_engine *item;
+
+	if (!engine)
+		return -ENOTSUPP;
+
+	/* Prevent multiple registrations of one engine */
+	list_for_each_entry(item, &on_host_hw_engines, node)
+		if (item == engine)
+			return 0;
+
+	mutex_lock(&on_host_hw_engines_mutex);
+	list_add_tail(&engine->node, &on_host_hw_engines);
+	mutex_unlock(&on_host_hw_engines_mutex);
+
+	return 0;
+}
+EXPORT_SYMBOL(nand_ecc_register_on_host_hw_engine);
+
+int nand_ecc_unregister_on_host_hw_engine(struct nand_ecc_engine *engine)
+{
+	if (!engine)
+		return -ENOTSUPP;
+
+	mutex_lock(&on_host_hw_engines_mutex);
+	list_del(&engine->node);
+	mutex_unlock(&on_host_hw_engines_mutex);
+
+	return 0;
+}
+EXPORT_SYMBOL(nand_ecc_unregister_on_host_hw_engine);
+
+struct nand_ecc_engine *nand_ecc_match_on_host_hw_engine(struct device *dev)
+{
+	struct nand_ecc_engine *item;
+
+	list_for_each_entry(item, &on_host_hw_engines, node)
+		if (item->dev == dev)
+			return item;
+
+	return NULL;
+}
+EXPORT_SYMBOL(nand_ecc_match_on_host_hw_engine);
+
+struct nand_ecc_engine *nand_ecc_get_on_host_hw_engine(struct nand_device *nand)
+{
+	struct nand_ecc_engine *engine = NULL;
+	struct device *dev = &nand->mtd.dev;
+	struct platform_device *pdev;
+	struct device_node *np;
+
+	if (list_empty(&on_host_hw_engines))
+		return NULL;
+
+	/* Check for an explicit nand-ecc-engine property */
+	np = of_parse_phandle(dev->of_node, "nand-ecc-engine", 0);
+	if (np) {
+		pdev = of_find_device_by_node(np);
+		if (!pdev)
+			return ERR_PTR(-EPROBE_DEFER);
+
+		engine = nand_ecc_match_on_host_hw_engine(&pdev->dev);
+		platform_device_put(pdev);
+		of_node_put(np);
+
+		if (!engine)
+			return ERR_PTR(-EPROBE_DEFER);
+	}
+
+	if (engine)
+		get_device(engine->dev);
+
+	return engine;
+}
+EXPORT_SYMBOL(nand_ecc_get_on_host_hw_engine);
+
+void nand_ecc_put_on_host_hw_engine(struct nand_device *nand)
+{
+	put_device(nand->ecc.engine->dev);
+}
+EXPORT_SYMBOL(nand_ecc_put_on_host_hw_engine);
+
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Miquel Raynal <miquel.raynal@bootlin.com>");
 MODULE_DESCRIPTION("Generic ECC engine");
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index 32fc7edf65b3..5ffd3e359515 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -265,10 +265,16 @@ struct nand_ecc_engine_ops {
 
 /**
  * struct nand_ecc_engine - ECC engine abstraction for NAND devices
+ * @dev: Host device
+ * @node: Private field for registration time
  * @ops: ECC engine operations
+ * @priv: Private data
  */
 struct nand_ecc_engine {
+	struct device *dev;
+	struct list_head node;
 	struct nand_ecc_engine_ops *ops;
+	void *priv;
 };
 
 void of_get_nand_ecc_user_config(struct nand_device *nand);
@@ -279,8 +285,13 @@ int nand_ecc_prepare_io_req(struct nand_device *nand,
 int nand_ecc_finish_io_req(struct nand_device *nand,
 			   struct nand_page_io_req *req);
 bool nand_ecc_is_strong_enough(struct nand_device *nand);
+int nand_ecc_register_on_host_hw_engine(struct nand_ecc_engine *engine);
+int nand_ecc_unregister_on_host_hw_engine(struct nand_ecc_engine *engine);
 struct nand_ecc_engine *nand_ecc_get_sw_engine(struct nand_device *nand);
 struct nand_ecc_engine *nand_ecc_get_on_die_hw_engine(struct nand_device *nand);
+struct nand_ecc_engine *nand_ecc_get_on_host_hw_engine(struct nand_device *nand);
+struct nand_ecc_engine *nand_ecc_match_on_host_hw_engine(struct device *dev);
+void nand_ecc_put_on_host_hw_engine(struct nand_device *nand);
 
 #if IS_ENABLED(CONFIG_MTD_NAND_ECC_SW_HAMMING)
 struct nand_ecc_engine *nand_ecc_sw_hamming_get_engine(void);
-- 
2.27.0


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

* [PATCH 12/18] mtd: nand: mxic-ecc: Add Macronix external ECC engine support
  2021-10-20 14:27 [PATCH 00/18] External ECC engines & Macronix support Miquel Raynal
                   ` (10 preceding siblings ...)
  2021-10-20 14:28 ` [PATCH 11/18] mtd: nand: ecc: Add infrastructure to support hardware engines Miquel Raynal
@ 2021-10-20 14:28 ` Miquel Raynal
  2021-10-20 14:28 ` [PATCH 13/18] mtd: nand: mxic-ecc: Support SPI pipelined mode Miquel Raynal
                   ` (5 subsequent siblings)
  17 siblings, 0 replies; 35+ messages in thread
From: Miquel Raynal @ 2021-10-20 14:28 UTC (permalink / raw)
  To: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus,
	Mark Brown, Rob Herring
  Cc: linux-mtd, linux-spi, devicetree, linux-kernel, Julien Su,
	Jaime Liao, Thomas Petazzoni, Boris Brezillon, Xiangsheng Hou,
	Miquel Raynal

Some SPI-NAND chips do not support on-die ECC. For these chips,
correction must apply on the SPI controller end. In order to avoid
doing all the calculations by software, Macronix provides a specific
engine that can offload the intensive work.

Add Macronix ECC engine support, this engine can work in conjunction
with a SPI controller and a raw NAND controller, it can be pipelined
or external and supports linear and syndrome layouts.

Right now the simplest configuration is supported: SPI controller
external and linear ECC engine.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/Kconfig    |   6 +
 drivers/mtd/nand/Makefile   |   1 +
 drivers/mtd/nand/ecc-mxic.c | 657 ++++++++++++++++++++++++++++++++++++
 3 files changed, 664 insertions(+)
 create mode 100644 drivers/mtd/nand/ecc-mxic.c

diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index b40455234cbd..8431292ff49d 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -46,6 +46,12 @@ config MTD_NAND_ECC_SW_BCH
 	  ECC codes. They are used with NAND devices requiring more than 1 bit
 	  of error correction.
 
+config MTD_NAND_ECC_MXIC
+	bool "Macronix external hardware ECC engine"
+	select MTD_NAND_ECC
+	help
+	  This enables support for the hardware ECC engine from Macronix.
+
 endmenu
 
 endmenu
diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
index 1c0b46960eb1..a4e6b7ae0614 100644
--- a/drivers/mtd/nand/Makefile
+++ b/drivers/mtd/nand/Makefile
@@ -10,3 +10,4 @@ obj-y	+= spi/
 nandcore-$(CONFIG_MTD_NAND_ECC) += ecc.o
 nandcore-$(CONFIG_MTD_NAND_ECC_SW_HAMMING) += ecc-sw-hamming.o
 nandcore-$(CONFIG_MTD_NAND_ECC_SW_BCH) += ecc-sw-bch.o
+nandcore-$(CONFIG_MTD_NAND_ECC_MXIC) += ecc-mxic.o
diff --git a/drivers/mtd/nand/ecc-mxic.c b/drivers/mtd/nand/ecc-mxic.c
new file mode 100644
index 000000000000..d9b251ab9201
--- /dev/null
+++ b/drivers/mtd/nand/ecc-mxic.c
@@ -0,0 +1,657 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Support for Macronix external hardware ECC engine for NAND devices, also
+ * called DPE for Data Processing Engine.
+ *
+ * Copyright © 2019 Macronix
+ * Author: Miquel Raynal <miquel.raynal@bootlin.com>
+ */
+
+#include <linux/dma-mapping.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mtd/mtd.h>
+#include <linux/mtd/nand.h>
+#include <linux/of_device.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+
+/* DPE Configuration */
+#define DP_CONFIG 0x00
+#define   ECC_EN BIT(0)
+#define   ECC_TYP(idx) (((idx) << 3) & GENMASK(6, 3))
+/* DPE Interrupt Status */
+#define INTRPT_STS 0x04
+#define   TRANS_CMPLT BIT(0)
+#define   SDMA_MAIN BIT(1)
+#define   SDMA_SPARE BIT(2)
+#define   ECC_ERR BIT(3)
+#define   TO_SPARE BIT(4)
+#define   TO_MAIN BIT(5)
+/* DPE Interrupt Status Enable */
+#define INTRPT_STS_EN 0x08
+/* DPE Interrupt Signal Enable */
+#define INTRPT_SIG_EN 0x0C
+/* Host Controller Configuration */
+#define HC_CONFIG 0x10
+#define   MEM2MEM BIT(4) /* TRANS_TYP_IO in the spec */
+#define   ECC_PACKED 0 /* LAYOUT_TYP_INTEGRATED in the spec */
+#define   ECC_INTERLEAVED BIT(2) /* LAYOUT_TYP_DISTRIBUTED in the spec */
+#define   BURST_TYP_FIXED 0
+#define   BURST_TYP_INCREASING BIT(0)
+/* Host Controller Slave Address */
+#define HC_SLV_ADDR 0x14
+/* ECC Chunk Size */
+#define CHUNK_SIZE 0x20
+/* Main Data Size */
+#define MAIN_SIZE 0x24
+/* Spare Data Size */
+#define SPARE_SIZE 0x28
+#define   META_SZ(reg) ((reg) & GENMASK(7, 0))
+#define   PARITY_SZ(reg) (((reg) & GENMASK(15, 8)) >> 8)
+#define   RSV_SZ(reg) (((reg) & GENMASK(23, 16)) >> 16)
+#define   SPARE_SZ(reg) ((reg) >> 24)
+/* ECC Chunk Count */
+#define CHUNK_CNT 0x30
+/* SDMA Control */
+#define SDMA_CTRL 0x40
+#define   WRITE_NAND 0
+#define   READ_NAND BIT(1)
+#define   CONT_NAND BIT(29)
+#define   CONT_SYSM BIT(30) /* Continue System Memory? */
+#define   SDMA_STRT BIT(31)
+/* SDMA Address of Main Data */
+#define SDMA_MAIN_ADDR 0x44
+/* SDMA Address of Spare Data */
+#define SDMA_SPARE_ADDR 0x48
+/* DPE Version Number */
+#define DP_VER 0xD0
+#define   DP_VER_OFFSET 16
+
+/* Status bytes between each chunk of spare data */
+#define STAT_BYTES 4
+#define   NO_ERR 0x00
+#define   MAX_CORR_ERR 0x28
+#define   UNCORR_ERR 0xFE
+#define   ERASED_CHUNK 0xFF
+
+struct mxic_ecc_engine {
+	struct device *dev;
+	void __iomem *regs;
+
+	/* ECC machinery */
+	unsigned int data_step_sz;
+	unsigned int oob_step_sz;
+	unsigned int parity_sz;
+	unsigned int meta_sz;
+	u8 *status;
+	int steps;
+
+	/* Completion boilerplate */
+	int irq;
+	struct completion complete;
+
+	/* DMA boilerplate */
+	struct nand_ecc_req_tweak_ctx req_ctx;
+	u8 *oobwithstat;
+	struct scatterlist sg[2];
+	struct nand_page_io_req *req;
+};
+
+static int mxic_ecc_ooblayout_ecc(struct mtd_info *mtd, int section,
+				  struct mtd_oob_region *oobregion)
+{
+	struct nand_device *nand = mtd_to_nanddev(mtd);
+	struct mxic_ecc_engine *eng = nand->ecc.ctx.priv;
+
+	if (section < 0 || section >= eng->steps)
+		return -ERANGE;
+
+	oobregion->offset = (section * eng->oob_step_sz) + eng->meta_sz;
+	oobregion->length = eng->parity_sz;
+
+	return 0;
+}
+
+static int mxic_ecc_ooblayout_free(struct mtd_info *mtd, int section,
+				   struct mtd_oob_region *oobregion)
+{
+	struct nand_device *nand = mtd_to_nanddev(mtd);
+	struct mxic_ecc_engine *eng = nand->ecc.ctx.priv;
+
+	if (section < 0 || section >= eng->steps)
+		return -ERANGE;
+
+	if (!section) {
+		oobregion->offset = 2;
+		oobregion->length = eng->meta_sz - 2;
+	} else {
+		oobregion->offset = section * eng->oob_step_sz;
+		oobregion->length = eng->meta_sz;
+	}
+
+	return 0;
+}
+
+static const struct mtd_ooblayout_ops mxic_ecc_ooblayout_ops = {
+	.ecc = mxic_ecc_ooblayout_ecc,
+	.free = mxic_ecc_ooblayout_free,
+};
+
+static void mxic_ecc_disable_engine(struct mxic_ecc_engine *eng)
+{
+	u32 reg;
+
+	reg = readl(eng->regs + DP_CONFIG);
+	reg &= ~ECC_EN;
+	writel(reg, eng->regs + DP_CONFIG);
+}
+
+static void mxic_ecc_enable_engine(struct mxic_ecc_engine *eng)
+{
+	u32 reg;
+
+	reg = readl(eng->regs + DP_CONFIG);
+	reg |= ECC_EN;
+	writel(reg, eng->regs + DP_CONFIG);
+}
+
+static void mxic_ecc_disable_int(struct mxic_ecc_engine *eng)
+{
+	writel(0, eng->regs + INTRPT_SIG_EN);
+}
+
+static void mxic_ecc_enable_int(struct mxic_ecc_engine *eng)
+{
+	writel(TRANS_CMPLT, eng->regs + INTRPT_SIG_EN);
+}
+
+static irqreturn_t mxic_ecc_isr(int irq, void *dev_id)
+{
+	struct mxic_ecc_engine *eng = dev_id;
+	u32 sts;
+
+	sts = readl(eng->regs + INTRPT_STS);
+	if (!sts)
+		return IRQ_NONE;
+
+	if (sts & TRANS_CMPLT)
+		complete(&eng->complete);
+
+	writel(sts, eng->regs + INTRPT_STS);
+
+	return IRQ_HANDLED;
+}
+
+static int mxic_ecc_init_ctx(struct nand_device *nand, struct device *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct nand_ecc_props *conf = &nand->ecc.ctx.conf;
+	struct nand_ecc_props *reqs = &nand->ecc.requirements;
+	struct nand_ecc_props *user = &nand->ecc.user_conf;
+	struct mtd_info *mtd = nanddev_to_mtd(nand);
+	struct mxic_ecc_engine *eng;
+	int step_size = 0, strength = 0, desired_correction = 0, steps, idx;
+	int possible_strength[] = {4, 8, 40, 48};
+	int spare_size[] = {32, 32, 96, 96};
+	u32 spare_reg;
+	int ret;
+
+	eng = devm_kzalloc(dev, sizeof(*eng), GFP_KERNEL);
+	if (!eng)
+		return -ENOMEM;
+
+	nand->ecc.ctx.priv = eng;
+	nand->ecc.engine->priv = eng;
+
+	eng->dev = dev;
+
+	/*
+	 * Both memory regions for the ECC engine itself and the AXI slave
+	 * address are mandatory.
+	 */
+	eng->regs = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(eng->regs)) {
+		dev_err(dev, "Missing memory region\n");
+		return PTR_ERR(eng->regs);
+	}
+
+	mxic_ecc_disable_engine(eng);
+	mxic_ecc_disable_int(eng);
+
+	/* IRQ is optional yet much more efficient */
+	eng->irq = platform_get_irq_byname(pdev, "ecc-engine");
+	if (eng->irq > 0) {
+		ret = devm_request_irq(dev, eng->irq, mxic_ecc_isr, 0,
+				       "mxic-ecc", eng);
+		if (ret)
+			return ret;
+	} else {
+		dev_info(dev, "No ECC engine IRQ (%d), using polling\n",
+			 eng->irq);
+		eng->irq = 0;
+	}
+
+	/* Only large page NAND chips may use BCH */
+	if (mtd->oobsize < 64) {
+		pr_err("BCH cannot be used with small page NAND chips\n");
+		return -EINVAL;
+	}
+
+	mtd_set_ooblayout(mtd, &mxic_ecc_ooblayout_ops);
+
+	/* Enable all status bits */
+	writel(TRANS_CMPLT | SDMA_MAIN | SDMA_SPARE | ECC_ERR |
+	       TO_SPARE | TO_MAIN, eng->regs + INTRPT_STS_EN);
+
+	/* Configure the correction depending on the NAND device topology */
+	if (user->step_size && user->strength) {
+		step_size = user->step_size;
+		strength = user->strength;
+	} else if (reqs->step_size && reqs->strength) {
+		step_size = reqs->step_size;
+		strength = reqs->strength;
+	}
+
+	if (step_size && strength) {
+		steps = mtd->writesize / step_size;
+		desired_correction = steps * strength;
+	}
+
+	/* Step size is fixed to 1kiB, strength may vary (4 possible values) */
+	conf->step_size = SZ_1K;
+	steps = mtd->writesize / conf->step_size;
+
+	eng->status = devm_kzalloc(dev, steps * sizeof(u8), GFP_KERNEL);
+	if (!eng->status)
+		return -ENOMEM;
+
+	if (desired_correction) {
+		strength = desired_correction / steps;
+
+		for (idx = 0; idx < ARRAY_SIZE(possible_strength); idx++)
+			if (possible_strength[idx] >= strength)
+				break;
+
+		idx = min_t(unsigned int, idx,
+			    ARRAY_SIZE(possible_strength) - 1);
+	} else {
+		/* Missing data, maximize the correction */
+		idx = ARRAY_SIZE(possible_strength) - 1;
+	}
+
+	/* Tune the selected strength until it fits in the OOB area */
+	for (; idx >= 0; idx--) {
+		if (spare_size[idx] * steps <= mtd->oobsize)
+			break;
+	}
+
+	/* This engine cannot be used with this NAND device */
+	if (idx < 0)
+		return -EINVAL;
+
+	/* Configure the engine for the desired strength */
+	writel(ECC_TYP(idx), eng->regs + DP_CONFIG);
+	conf->strength = possible_strength[idx];
+	spare_reg = readl(eng->regs + SPARE_SIZE);
+
+	eng->steps = steps;
+	eng->data_step_sz = mtd->writesize / steps;
+	eng->oob_step_sz = mtd->oobsize / steps;
+	eng->parity_sz = PARITY_SZ(spare_reg);
+	eng->meta_sz = META_SZ(spare_reg);
+
+	/* Ensure buffers will contain enough bytes to store the STAT_BYTES */
+	eng->req_ctx.oob_buffer_size = nanddev_per_page_oobsize(nand) +
+				       (eng->steps * STAT_BYTES);
+	ret = nand_ecc_init_req_tweaking(&eng->req_ctx, nand);
+	if (ret)
+		return ret;
+
+	eng->oobwithstat = kmalloc(mtd->oobsize + (eng->steps * STAT_BYTES),
+				   GFP_KERNEL);
+	if (!eng->oobwithstat) {
+		ret = -ENOMEM;
+		goto cleanup_req_tweak;
+	}
+
+	sg_init_table(eng->sg, 2);
+
+	/* Configuration dump and sanity checks */
+	dev_err(dev, "DPE version number: %d\n",
+		readl(eng->regs + DP_VER) >> DP_VER_OFFSET);
+	dev_err(dev, "Chunk size: %d\n", readl(eng->regs + CHUNK_SIZE));
+	dev_err(dev, "Main size: %d\n", readl(eng->regs + MAIN_SIZE));
+	dev_err(dev, "Spare size: %d\n", SPARE_SZ(spare_reg));
+	dev_err(dev, "Rsv size: %ld\n", RSV_SZ(spare_reg));
+	dev_err(dev, "Parity size: %d\n", eng->parity_sz);
+	dev_err(dev, "Meta size: %d\n", eng->meta_sz);
+
+	if (eng->meta_sz + eng->parity_sz + RSV_SZ(spare_reg) != SPARE_SZ(spare_reg)) {
+		dev_err(dev, "Wrong OOB configuration: %d + %d + %ld != %d\n",
+			eng->meta_sz, eng->parity_sz, RSV_SZ(spare_reg),
+			SPARE_SZ(spare_reg));
+		ret = -EINVAL;
+		goto free_oobwithstat;
+	}
+
+	if (eng->oob_step_sz != SPARE_SZ(spare_reg)) {
+		dev_err(dev, "Wrong OOB configuration: %d != %d\n",
+			eng->oob_step_sz, SPARE_SZ(spare_reg));
+		ret = -EINVAL;
+		goto free_oobwithstat;
+	}
+
+	return 0;
+
+free_oobwithstat:
+	kfree(eng->oobwithstat);
+cleanup_req_tweak:
+	nand_ecc_cleanup_req_tweaking(&eng->req_ctx);
+
+	return ret;
+}
+
+static int mxic_ecc_init_ctx_external(struct nand_device *nand)
+{
+	struct device *dev = nand->ecc.engine->dev;
+	struct mxic_ecc_engine *eng;
+	int ret;
+
+	dev_info(dev, "Macronix ECC engine in external mode\n");
+
+	ret = mxic_ecc_init_ctx(nand, dev);
+	if (ret)
+		return ret;
+
+	eng = nand->ecc.ctx.priv;
+
+	/* Trigger each step manually */
+	writel(1, eng->regs + CHUNK_CNT);
+	writel(BURST_TYP_INCREASING | ECC_PACKED | MEM2MEM,
+	       eng->regs + HC_CONFIG);
+
+	return 0;
+}
+
+static void mxic_ecc_cleanup_ctx(struct nand_device *nand)
+{
+	struct mxic_ecc_engine *eng = nand->ecc.ctx.priv;
+
+	if (eng) {
+		nand_ecc_cleanup_req_tweaking(&eng->req_ctx);
+		kfree(eng->oobwithstat);
+	}
+}
+
+static int mxic_ecc_data_xfer_wait_for_completion(struct mxic_ecc_engine *eng)
+{
+	u32 val;
+	int ret;
+
+	if (eng->irq) {
+		init_completion(&eng->complete);
+		mxic_ecc_enable_int(eng);
+		ret = wait_for_completion_timeout(&eng->complete,
+						  msecs_to_jiffies(1000));
+		mxic_ecc_disable_int(eng);
+	} else {
+		ret = readl_poll_timeout(eng->regs + INTRPT_STS, val,
+					 val & TRANS_CMPLT, 10, USEC_PER_SEC);
+		writel(val, eng->regs + INTRPT_STS);
+	}
+
+	if (ret) {
+		dev_err(eng->dev, "Timeout on data xfer completion (sts 0x%08x)\n", val);
+		return -ETIMEDOUT;
+	}
+
+	return 0;
+}
+
+static int mxic_ecc_process_data(struct mxic_ecc_engine *eng)
+{
+	/* Retrieve the direction */
+	unsigned int dir = (eng->req->type == NAND_PAGE_READ) ?
+			   READ_NAND : WRITE_NAND;
+
+	/* Trigger processing */
+	writel(SDMA_STRT | dir, eng->regs + SDMA_CTRL);
+
+	/* Wait for completion */
+	return mxic_ecc_data_xfer_wait_for_completion(eng);
+}
+
+static void mxic_ecc_extract_status_bytes(struct mxic_ecc_engine *eng, u8 *buf)
+{
+	int next_stat_pos;
+	int step;
+
+	/* Extract the ECC status */
+	for (step = 0; step < eng->steps; step++) {
+		next_stat_pos = eng->oob_step_sz +
+				((STAT_BYTES + eng->oob_step_sz) * step);
+
+		eng->status[step] = buf[next_stat_pos];
+	}
+}
+
+static void mxic_ecc_reconstruct_oobbuf(struct mxic_ecc_engine *eng,
+					u8 *dst, const u8 *src)
+{
+	int step;
+
+	/* Reconstruct the OOB buffer linearly (without the ECC status bytes) */
+	for (step = 0; step < eng->steps; step++)
+		memcpy(dst + (step * eng->oob_step_sz),
+		       src + (step * (eng->oob_step_sz + STAT_BYTES)),
+		       eng->oob_step_sz);
+}
+
+static void mxic_ecc_add_room_in_oobbuf(struct mxic_ecc_engine *eng,
+					u8 *dst, const u8 *src)
+{
+	int step;
+
+	/* Add some space in the OOB buffer for the status bytes */
+	for (step = 0; step < eng->steps; step++)
+		memcpy(dst + (step * (eng->oob_step_sz + STAT_BYTES)),
+		       src + (step * eng->oob_step_sz),
+		       eng->oob_step_sz);
+}
+
+static int mxic_ecc_count_biterrs(struct mxic_ecc_engine *eng, struct mtd_info *mtd)
+{
+	struct device *dev = eng->dev;
+	unsigned int max_bf = 0;
+	bool failure = false;
+	int step;
+
+	for (step = 0; step < eng->steps; step++) {
+		u8 stat = eng->status[step];
+
+		if (stat == NO_ERR) {
+			dev_dbg(dev, "ECC step %d: no error\n", step);
+		} else if (stat == ERASED_CHUNK) {
+			dev_dbg(dev, "ECC step %d: erased\n", step);
+		} else if (stat == UNCORR_ERR || stat > MAX_CORR_ERR) {
+			dev_dbg(dev, "ECC step %d: uncorrectable\n", step);
+			mtd->ecc_stats.failed++;
+			failure = true;
+		} else {
+			dev_dbg(dev, "ECC step %d: %d bits corrected\n",
+				step, stat);
+			max_bf = max_t(unsigned int, max_bf, stat);
+			mtd->ecc_stats.corrected += stat;
+		}
+	}
+
+	return failure ? -EBADMSG : max_bf;
+}
+
+/* External ECC engine helpers */
+static int mxic_ecc_prepare_io_req_external(struct nand_device *nand,
+					    struct nand_page_io_req *req)
+{
+	struct mxic_ecc_engine *eng = nand->ecc.ctx.priv;
+	struct mtd_info *mtd = nanddev_to_mtd(nand);
+	int offset, nents, step, ret;
+
+	if (req->mode == MTD_OPS_RAW)
+		return 0;
+
+	nand_ecc_tweak_req(&eng->req_ctx, req);
+	eng->req = req;
+
+	if (req->type == NAND_PAGE_READ)
+		return 0;
+
+	mxic_ecc_add_room_in_oobbuf(eng, eng->oobwithstat, eng->req->oobbuf.out);
+
+	sg_set_buf(&eng->sg[0], req->databuf.out, req->datalen);
+	sg_set_buf(&eng->sg[1], eng->oobwithstat,
+		   req->ooblen + (eng->steps * STAT_BYTES));
+
+	nents = dma_map_sg(eng->dev, eng->sg, 2, DMA_BIDIRECTIONAL);
+	if (!nents)
+		return -EINVAL;
+
+	mxic_ecc_enable_engine(eng);
+
+	for (step = 0; step < eng->steps; step++) {
+		writel(sg_dma_address(&eng->sg[0]) + (step * eng->data_step_sz),
+		       eng->regs + SDMA_MAIN_ADDR);
+		writel(sg_dma_address(&eng->sg[1]) + (step * (eng->oob_step_sz + STAT_BYTES)),
+		       eng->regs + SDMA_SPARE_ADDR);
+		ret = mxic_ecc_process_data(eng);
+		if (ret)
+			break;
+	}
+
+	mxic_ecc_disable_engine(eng);
+
+	dma_unmap_sg(eng->dev, eng->sg, 2, DMA_BIDIRECTIONAL);
+
+	/* Retrieve the calculated ECC bytes */
+	for (step = 0; step < eng->steps; step++) {
+		offset = eng->meta_sz + (step * eng->oob_step_sz);
+		mtd_ooblayout_get_eccbytes(mtd,
+					   (u8 *)eng->req->oobbuf.out + offset,
+					   eng->oobwithstat + (step * STAT_BYTES),
+					   step * eng->parity_sz,
+					   eng->parity_sz);
+	}
+
+	return ret;
+}
+
+static int mxic_ecc_finish_io_req_external(struct nand_device *nand,
+					   struct nand_page_io_req *req)
+{
+	struct mxic_ecc_engine *eng = nand->ecc.ctx.priv;
+	struct mtd_info *mtd = nanddev_to_mtd(nand);
+	int nents, step, ret;
+
+	if (req->mode == MTD_OPS_RAW)
+		return 0;
+
+	if (req->type == NAND_PAGE_WRITE) {
+		nand_ecc_restore_req(&eng->req_ctx, req);
+		return 0;
+	}
+
+	/* Copy the OOB buffer and add room for the ECC engine status bytes */
+	mxic_ecc_add_room_in_oobbuf(eng, eng->oobwithstat, eng->req->oobbuf.in);
+
+	sg_set_buf(&eng->sg[0], req->databuf.in, req->datalen);
+	sg_set_buf(&eng->sg[1], eng->oobwithstat,
+		   req->ooblen + (eng->steps * STAT_BYTES));
+	nents = dma_map_sg(eng->dev, eng->sg, 2, DMA_BIDIRECTIONAL);
+	if (!nents)
+		return -EINVAL;
+
+	mxic_ecc_enable_engine(eng);
+
+	for (step = 0; step < eng->steps; step++) {
+		writel(sg_dma_address(&eng->sg[0]) + (step * eng->data_step_sz),
+		       eng->regs + SDMA_MAIN_ADDR);
+		writel(sg_dma_address(&eng->sg[1]) + (step * (eng->oob_step_sz + STAT_BYTES)),
+		       eng->regs + SDMA_SPARE_ADDR);
+		ret = mxic_ecc_process_data(eng);
+		if (ret)
+			break;
+	}
+
+	mxic_ecc_disable_engine(eng);
+
+	dma_unmap_sg(eng->dev, eng->sg, 2, DMA_BIDIRECTIONAL);
+
+	/* Extract the status bytes and reconstruct the buffer */
+	mxic_ecc_extract_status_bytes(eng, eng->oobwithstat);
+	mxic_ecc_reconstruct_oobbuf(eng, eng->req->oobbuf.in, eng->oobwithstat);
+
+	nand_ecc_restore_req(&eng->req_ctx, req);
+
+	return mxic_ecc_count_biterrs(eng, mtd);
+}
+
+static struct nand_ecc_engine_ops mxic_ecc_engine_external_ops = {
+	.init_ctx = mxic_ecc_init_ctx_external,
+	.cleanup_ctx = mxic_ecc_cleanup_ctx,
+	.prepare_io_req = mxic_ecc_prepare_io_req_external,
+	.finish_io_req = mxic_ecc_finish_io_req_external,
+};
+
+static int mxic_ecc_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct nand_ecc_engine *ecceng;
+
+	ecceng = devm_kzalloc(dev, sizeof(*ecceng), GFP_KERNEL);
+	if (!ecceng)
+		return -ENOMEM;
+
+	ecceng->dev = dev;
+	ecceng->ops = &mxic_ecc_engine_external_ops;
+	nand_ecc_register_on_host_hw_engine(ecceng);
+
+	return 0;
+}
+
+static int mxic_ecc_remove(struct platform_device *pdev)
+{
+	struct nand_ecc_engine *ecceng;
+
+	ecceng = nand_ecc_match_on_host_hw_engine(&pdev->dev);
+	if (ecceng)
+		nand_ecc_unregister_on_host_hw_engine(ecceng);
+
+	return 0;
+}
+
+static const struct of_device_id mxic_ecc_of_ids[] = {
+	{
+		.compatible = "mxicy,nand-ecc-engine-rev3",
+	},
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, mxic_ecc_of_ids);
+
+static struct platform_driver mxic_ecc_driver = {
+	.driver	= {
+		.name = "mxic-nand-ecc-engine",
+		.of_match_table = mxic_ecc_of_ids,
+	},
+	.probe = mxic_ecc_probe,
+	.remove	= mxic_ecc_remove,
+};
+module_platform_driver(mxic_ecc_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Miquel Raynal <miquel.raynal@bootlin.com>");
+MODULE_DESCRIPTION("Macronix NAND hardware ECC controller");
-- 
2.27.0


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

* [PATCH 13/18] mtd: nand: mxic-ecc: Support SPI pipelined mode
  2021-10-20 14:27 [PATCH 00/18] External ECC engines & Macronix support Miquel Raynal
                   ` (11 preceding siblings ...)
  2021-10-20 14:28 ` [PATCH 12/18] mtd: nand: mxic-ecc: Add Macronix external ECC engine support Miquel Raynal
@ 2021-10-20 14:28 ` Miquel Raynal
  2021-10-21  5:22   ` kernel test robot
  2021-10-20 14:28 ` [PATCH 14/18] spi: mxic: Fix the transmit path Miquel Raynal
                   ` (4 subsequent siblings)
  17 siblings, 1 reply; 35+ messages in thread
From: Miquel Raynal @ 2021-10-20 14:28 UTC (permalink / raw)
  To: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus,
	Mark Brown, Rob Herring
  Cc: linux-mtd, linux-spi, devicetree, linux-kernel, Julien Su,
	Jaime Liao, Thomas Petazzoni, Boris Brezillon, Xiangsheng Hou,
	Miquel Raynal

Introduce the support for another possible configuration: the ECC
engine may work as DMA master (pipelined) and move itself the data
to/from the NAND chip into the buffer, applying the necessary
corrections/computations on the fly.

This driver offers an ECC engine implementation that must be
instatiated from a SPI controller driver.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/ecc-mxic.c       | 148 +++++++++++++++++++++++++++++-
 include/linux/mtd/nand-ecc-mxic.h |  36 ++++++++
 2 files changed, 181 insertions(+), 3 deletions(-)
 create mode 100644 include/linux/mtd/nand-ecc-mxic.h

diff --git a/drivers/mtd/nand/ecc-mxic.c b/drivers/mtd/nand/ecc-mxic.c
index d9b251ab9201..555ba7f19027 100644
--- a/drivers/mtd/nand/ecc-mxic.c
+++ b/drivers/mtd/nand/ecc-mxic.c
@@ -39,7 +39,9 @@
 #define INTRPT_SIG_EN 0x0C
 /* Host Controller Configuration */
 #define HC_CONFIG 0x10
+#define   DEV2MEM 0 /* TRANS_TYP_DMA in the spec */
 #define   MEM2MEM BIT(4) /* TRANS_TYP_IO in the spec */
+#define   MAPPING BIT(5) /* TRANS_TYP_MAPPING in the spec */
 #define   ECC_PACKED 0 /* LAYOUT_TYP_INTEGRATED in the spec */
 #define   ECC_INTERLEAVED BIT(2) /* LAYOUT_TYP_DISTRIBUTED in the spec */
 #define   BURST_TYP_FIXED 0
@@ -101,6 +103,7 @@ struct mxic_ecc_engine {
 	u8 *oobwithstat;
 	struct scatterlist sg[2];
 	struct nand_page_io_req *req;
+	unsigned int pageoffs;
 };
 
 static int mxic_ecc_ooblayout_ecc(struct mtd_info *mtd, int section,
@@ -188,6 +191,31 @@ static irqreturn_t mxic_ecc_isr(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
+static struct device *mxic_ecc_get_engine_dev(struct device *dev)
+{
+	struct platform_device *eccpdev;
+	struct device_node *np;
+
+	/*
+	 * If the device node contains this property, it means the device does
+	 * not represent the actual ECC engine.
+	 */
+	np = of_parse_phandle(dev->of_node, "nand-ecc-engine", 0);
+	if (!np)
+		return dev;
+
+	eccpdev = of_find_device_by_node(np);
+	if (!eccpdev) {
+		of_node_put(np);
+		return NULL;
+	}
+
+	platform_device_put(eccpdev);
+	of_node_put(np);
+
+	return &eccpdev->dev;
+}
+
 static int mxic_ecc_init_ctx(struct nand_device *nand, struct device *dev)
 {
 	struct platform_device *pdev = to_platform_device(dev);
@@ -379,6 +407,41 @@ static int mxic_ecc_init_ctx_external(struct nand_device *nand)
 	return 0;
 }
 
+static int mxic_ecc_init_ctx_pipelined(struct nand_device *nand)
+{
+	struct mxic_ecc_engine *eng;
+	struct device *dev;
+	int ret;
+
+	/*
+	 * In the case of a pipelined engine, the device registering the ECC
+	 * engine is not the actual ECC engine device but the host controller.
+	 */
+	dev = mxic_ecc_get_engine_dev(nand->ecc.engine->dev);
+	if (!dev)
+		return -EINVAL;
+
+	dev_info(dev, "Macronix ECC engine in pipelined/mapping mode\n");
+
+	ret = mxic_ecc_init_ctx(nand, dev);
+	if (ret)
+		return ret;
+
+	eng = nand->ecc.ctx.priv;
+
+	/* All steps should be handled in one go directly by the internal DMA */
+	writel(eng->steps, eng->regs + CHUNK_CNT);
+
+	/*
+	 * Interleaved ECC scheme cannot be used otherwise factory bad block
+	 * markers would be lost. A packed layout is mandatory.
+	 */
+	writel(BURST_TYP_INCREASING | ECC_PACKED | MAPPING,
+	       eng->regs + HC_CONFIG);
+
+	return 0;
+}
+
 static void mxic_ecc_cleanup_ctx(struct nand_device *nand)
 {
 	struct mxic_ecc_engine *eng = nand->ecc.ctx.priv;
@@ -414,18 +477,22 @@ static int mxic_ecc_data_xfer_wait_for_completion(struct mxic_ecc_engine *eng)
 	return 0;
 }
 
-static int mxic_ecc_process_data(struct mxic_ecc_engine *eng)
+int mxic_ecc_process_data(struct mxic_ecc_engine *eng, dma_addr_t dirmap)
 {
 	/* Retrieve the direction */
 	unsigned int dir = (eng->req->type == NAND_PAGE_READ) ?
 			   READ_NAND : WRITE_NAND;
 
+	if (dirmap)
+		writel(dirmap, eng->regs + HC_SLV_ADDR);
+
 	/* Trigger processing */
 	writel(SDMA_STRT | dir, eng->regs + SDMA_CTRL);
 
 	/* Wait for completion */
 	return mxic_ecc_data_xfer_wait_for_completion(eng);
 }
+EXPORT_SYMBOL_GPL(mxic_ecc_process_data);
 
 static void mxic_ecc_extract_status_bytes(struct mxic_ecc_engine *eng, u8 *buf)
 {
@@ -528,7 +595,7 @@ static int mxic_ecc_prepare_io_req_external(struct nand_device *nand,
 		       eng->regs + SDMA_MAIN_ADDR);
 		writel(sg_dma_address(&eng->sg[1]) + (step * (eng->oob_step_sz + STAT_BYTES)),
 		       eng->regs + SDMA_SPARE_ADDR);
-		ret = mxic_ecc_process_data(eng);
+		ret = mxic_ecc_process_data(eng, 0);
 		if (ret)
 			break;
 	}
@@ -582,7 +649,7 @@ static int mxic_ecc_finish_io_req_external(struct nand_device *nand,
 		       eng->regs + SDMA_MAIN_ADDR);
 		writel(sg_dma_address(&eng->sg[1]) + (step * (eng->oob_step_sz + STAT_BYTES)),
 		       eng->regs + SDMA_SPARE_ADDR);
-		ret = mxic_ecc_process_data(eng);
+		ret = mxic_ecc_process_data(eng, 0);
 		if (ret)
 			break;
 	}
@@ -600,6 +667,64 @@ static int mxic_ecc_finish_io_req_external(struct nand_device *nand,
 	return mxic_ecc_count_biterrs(eng, mtd);
 }
 
+/* Pipelined ECC engine helpers */
+static int mxic_ecc_prepare_io_req_pipelined(struct nand_device *nand,
+					     struct nand_page_io_req *req)
+{
+	struct mxic_ecc_engine *eng = nand->ecc.ctx.priv;
+	int nents;
+
+	if (req->mode == MTD_OPS_RAW)
+		return 0;
+
+	nand_ecc_tweak_req(&eng->req_ctx, req);
+	eng->req = req;
+
+	/* Copy the OOB buffer and add room for the ECC engine status bytes */
+	mxic_ecc_add_room_in_oobbuf(eng, eng->oobwithstat, eng->req->oobbuf.in);
+
+	sg_set_buf(&eng->sg[0], req->databuf.in, req->datalen);
+	sg_set_buf(&eng->sg[1], eng->oobwithstat,
+		   req->ooblen + (eng->steps * STAT_BYTES));
+
+	nents = dma_map_sg(eng->dev, eng->sg, 2, DMA_BIDIRECTIONAL);
+	if (!nents)
+		return -EINVAL;
+
+	writel(sg_dma_address(&eng->sg[0]), eng->regs + SDMA_MAIN_ADDR);
+	writel(sg_dma_address(&eng->sg[1]), eng->regs + SDMA_SPARE_ADDR);
+
+	mxic_ecc_enable_engine(eng);
+
+	return 0;
+}
+
+static int mxic_ecc_finish_io_req_pipelined(struct nand_device *nand,
+					    struct nand_page_io_req *req)
+{
+	struct mxic_ecc_engine *eng = nand->ecc.ctx.priv;
+	struct mtd_info *mtd = nanddev_to_mtd(nand);
+	int ret = 0;
+
+	if (req->mode == MTD_OPS_RAW)
+		return 0;
+
+	mxic_ecc_disable_engine(eng);
+
+	dma_unmap_sg(eng->dev, eng->sg, 2, DMA_BIDIRECTIONAL);
+
+	if (req->type == NAND_PAGE_READ) {
+		mxic_ecc_extract_status_bytes(eng, eng->oobwithstat);
+		mxic_ecc_reconstruct_oobbuf(eng, eng->req->oobbuf.in,
+					    eng->oobwithstat);
+		ret = mxic_ecc_count_biterrs(eng, mtd);
+	}
+
+	nand_ecc_restore_req(&eng->req_ctx, req);
+
+	return ret;
+}
+
 static struct nand_ecc_engine_ops mxic_ecc_engine_external_ops = {
 	.init_ctx = mxic_ecc_init_ctx_external,
 	.cleanup_ctx = mxic_ecc_cleanup_ctx,
@@ -607,6 +732,23 @@ static struct nand_ecc_engine_ops mxic_ecc_engine_external_ops = {
 	.finish_io_req = mxic_ecc_finish_io_req_external,
 };
 
+static struct nand_ecc_engine_ops mxic_ecc_engine_pipelined_ops = {
+	.init_ctx = mxic_ecc_init_ctx_pipelined,
+	.cleanup_ctx = mxic_ecc_cleanup_ctx,
+	.prepare_io_req = mxic_ecc_prepare_io_req_pipelined,
+	.finish_io_req = mxic_ecc_finish_io_req_pipelined,
+};
+
+struct nand_ecc_engine_ops *mxic_ecc_get_pipelined_ops(void)
+{
+	return &mxic_ecc_engine_pipelined_ops;
+}
+EXPORT_SYMBOL_GPL(mxic_ecc_get_pipelined_ops);
+
+/*
+ * Only the external ECC engine is exported as the pipelined is SoC specific, so
+ * it is registered directly by the drivers that wrap it.
+ */
 static int mxic_ecc_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
diff --git a/include/linux/mtd/nand-ecc-mxic.h b/include/linux/mtd/nand-ecc-mxic.h
new file mode 100644
index 000000000000..d48b7fe05644
--- /dev/null
+++ b/include/linux/mtd/nand-ecc-mxic.h
@@ -0,0 +1,36 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright © 2019 Macronix
+ * Author: Miquèl Raynal <miquel.raynal@bootlin.com>
+ *
+ * Header for the Macronix external ECC engine.
+ */
+
+#ifndef __MTD_NAND_ECC_MXIC_H__
+#define __MTD_NAND_ECC_MXIC_H__
+
+#include <linux/device.h>
+
+struct mxic_ecc_engine;
+
+#if IS_ENABLED(CONFIG_MTD_NAND_ECC_MXIC)
+
+struct nand_ecc_engine_ops *mxic_ecc_get_pipelined_ops(void);
+int mxic_ecc_process_data(struct mxic_ecc_engine *eng, dma_addr_t dirmap);
+
+#else /* !CONFIG_MTD_NAND_ECC_MXIC */
+
+struct nand_ecc_engine_ops *mxic_ecc_get_pipelined_ops(void)
+{
+	return NULL;
+}
+
+static inline
+int mxic_ecc_process_data(struct mxic_ecc_engine *eng, dma_addr_t dirmap)
+{
+	return -ENOTSUPP;
+}
+
+#endif /* CONFIG_MTD_NAND_ECC_MXIC */
+
+#endif /* __MTD_NAND_ECC_MXIC_H__ */
-- 
2.27.0


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

* [PATCH 14/18] spi: mxic: Fix the transmit path
  2021-10-20 14:27 [PATCH 00/18] External ECC engines & Macronix support Miquel Raynal
                   ` (12 preceding siblings ...)
  2021-10-20 14:28 ` [PATCH 13/18] mtd: nand: mxic-ecc: Support SPI pipelined mode Miquel Raynal
@ 2021-10-20 14:28 ` Miquel Raynal
  2021-10-20 14:28 ` [PATCH 15/18] spi: mxic: Create a helper to configure the controller before an operation Miquel Raynal
                   ` (3 subsequent siblings)
  17 siblings, 0 replies; 35+ messages in thread
From: Miquel Raynal @ 2021-10-20 14:28 UTC (permalink / raw)
  To: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus,
	Mark Brown, Rob Herring
  Cc: linux-mtd, linux-spi, devicetree, linux-kernel, Julien Su,
	Jaime Liao, Thomas Petazzoni, Boris Brezillon, Xiangsheng Hou,
	Miquel Raynal, stable, Mason Yang, Zhengxun Li

By working with external hardware ECC engines, we figured out that
Under certain circumstances, it is needed for the SPI controller to
check INT_TX_EMPTY and INT_RX_NOT_EMPTY in both receive and transmit
path (not only in the receive path). The delay penalty being
negligible, move this code in the common path.

Fixes: b942d80b0a39 ("spi: Add MXIC controller driver")
Cc: stable@vger.kernel.org
Suggested-by: Mason Yang <masonccyang@mxic.com.tw>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Reviewed-by: Zhengxun Li <zhengxunli@mxic.com.tw>
---
 drivers/spi/spi-mxic.c | 28 ++++++++++++----------------
 1 file changed, 12 insertions(+), 16 deletions(-)

diff --git a/drivers/spi/spi-mxic.c b/drivers/spi/spi-mxic.c
index 45889947afed..03fce4493aa7 100644
--- a/drivers/spi/spi-mxic.c
+++ b/drivers/spi/spi-mxic.c
@@ -304,25 +304,21 @@ static int mxic_spi_data_xfer(struct mxic_spi *mxic, const void *txbuf,
 
 		writel(data, mxic->regs + TXD(nbytes % 4));
 
+		ret = readl_poll_timeout(mxic->regs + INT_STS, sts,
+					 sts & INT_TX_EMPTY, 0, USEC_PER_SEC);
+		if (ret)
+			return ret;
+
+		ret = readl_poll_timeout(mxic->regs + INT_STS, sts,
+					 sts & INT_RX_NOT_EMPTY, 0,
+					 USEC_PER_SEC);
+		if (ret)
+			return ret;
+
+		data = readl(mxic->regs + RXD);
 		if (rxbuf) {
-			ret = readl_poll_timeout(mxic->regs + INT_STS, sts,
-						 sts & INT_TX_EMPTY, 0,
-						 USEC_PER_SEC);
-			if (ret)
-				return ret;
-
-			ret = readl_poll_timeout(mxic->regs + INT_STS, sts,
-						 sts & INT_RX_NOT_EMPTY, 0,
-						 USEC_PER_SEC);
-			if (ret)
-				return ret;
-
-			data = readl(mxic->regs + RXD);
 			data >>= (8 * (4 - nbytes));
 			memcpy(rxbuf + pos, &data, nbytes);
-			WARN_ON(readl(mxic->regs + INT_STS) & INT_RX_NOT_EMPTY);
-		} else {
-			readl(mxic->regs + RXD);
 		}
 		WARN_ON(readl(mxic->regs + INT_STS) & INT_RX_NOT_EMPTY);
 
-- 
2.27.0


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

* [PATCH 15/18] spi: mxic: Create a helper to configure the controller before an operation
  2021-10-20 14:27 [PATCH 00/18] External ECC engines & Macronix support Miquel Raynal
                   ` (13 preceding siblings ...)
  2021-10-20 14:28 ` [PATCH 14/18] spi: mxic: Fix the transmit path Miquel Raynal
@ 2021-10-20 14:28 ` Miquel Raynal
  2021-10-20 14:28 ` [PATCH 16/18] spi: mxic: Create a helper to ease the start of " Miquel Raynal
                   ` (2 subsequent siblings)
  17 siblings, 0 replies; 35+ messages in thread
From: Miquel Raynal @ 2021-10-20 14:28 UTC (permalink / raw)
  To: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus,
	Mark Brown, Rob Herring
  Cc: linux-mtd, linux-spi, devicetree, linux-kernel, Julien Su,
	Jaime Liao, Thomas Petazzoni, Boris Brezillon, Xiangsheng Hou,
	Miquel Raynal

Create the mxic_spi_set_hc_cfg() helper to configure the HC_CFG
register. This helper will soon be used by the dirmap implementation and
having this code factorized out earlier will clarify this addition.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/spi/spi-mxic.c | 31 +++++++++++++++++++------------
 1 file changed, 19 insertions(+), 12 deletions(-)

diff --git a/drivers/spi/spi-mxic.c b/drivers/spi/spi-mxic.c
index 03fce4493aa7..068543c40ce7 100644
--- a/drivers/spi/spi-mxic.c
+++ b/drivers/spi/spi-mxic.c
@@ -280,6 +280,22 @@ static void mxic_spi_hw_init(struct mxic_spi *mxic)
 	       mxic->regs + HC_CFG);
 }
 
+static u32 mxic_spi_prep_hc_cfg(struct spi_device *spi, u32 flags)
+{
+	int nio = 1;
+
+	if (spi->mode & (SPI_TX_OCTAL | SPI_RX_OCTAL))
+		nio = 8;
+	else if (spi->mode & (SPI_TX_QUAD | SPI_RX_QUAD))
+		nio = 4;
+	else if (spi->mode & (SPI_TX_DUAL | SPI_RX_DUAL))
+		nio = 2;
+
+	return flags | HC_CFG_NIO(nio) |
+	       HC_CFG_TYPE(spi->chip_select, HC_CFG_TYPE_SPI_NOR) |
+	       HC_CFG_SLV_ACT(spi->chip_select) | HC_CFG_IDLE_SIO_LVL(1);
+}
+
 static int mxic_spi_data_xfer(struct mxic_spi *mxic, const void *txbuf,
 			      void *rxbuf, unsigned int len)
 {
@@ -357,7 +373,7 @@ static int mxic_spi_mem_exec_op(struct spi_mem *mem,
 				const struct spi_mem_op *op)
 {
 	struct mxic_spi *mxic = spi_master_get_devdata(mem->spi->master);
-	int nio = 1, i, ret;
+	int i, ret;
 	u32 ss_ctrl;
 	u8 addr[8], cmd[2];
 
@@ -365,18 +381,9 @@ static int mxic_spi_mem_exec_op(struct spi_mem *mem,
 	if (ret)
 		return ret;
 
-	if (mem->spi->mode & (SPI_TX_OCTAL | SPI_RX_OCTAL))
-		nio = 8;
-	else if (mem->spi->mode & (SPI_TX_QUAD | SPI_RX_QUAD))
-		nio = 4;
-	else if (mem->spi->mode & (SPI_TX_DUAL | SPI_RX_DUAL))
-		nio = 2;
-
-	writel(HC_CFG_NIO(nio) |
-	       HC_CFG_TYPE(mem->spi->chip_select, HC_CFG_TYPE_SPI_NOR) |
-	       HC_CFG_SLV_ACT(mem->spi->chip_select) | HC_CFG_IDLE_SIO_LVL(1) |
-	       HC_CFG_MAN_CS_EN,
+	writel(mxic_spi_prep_hc_cfg(mem->spi, HC_CFG_MAN_CS_EN),
 	       mxic->regs + HC_CFG);
+
 	writel(HC_EN_BIT, mxic->regs + HC_EN);
 
 	ss_ctrl = OP_CMD_BYTES(op->cmd.nbytes) |
-- 
2.27.0


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

* [PATCH 16/18] spi: mxic: Create a helper to ease the start of an operation
  2021-10-20 14:27 [PATCH 00/18] External ECC engines & Macronix support Miquel Raynal
                   ` (14 preceding siblings ...)
  2021-10-20 14:28 ` [PATCH 15/18] spi: mxic: Create a helper to configure the controller before an operation Miquel Raynal
@ 2021-10-20 14:28 ` Miquel Raynal
  2021-10-20 14:28 ` [PATCH 17/18] spi: mxic: Add support for direct mapping Miquel Raynal
  2021-10-20 14:28 ` [PATCH 18/18] spi: mxic: Add support for pipelined ECC operations Miquel Raynal
  17 siblings, 0 replies; 35+ messages in thread
From: Miquel Raynal @ 2021-10-20 14:28 UTC (permalink / raw)
  To: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus,
	Mark Brown, Rob Herring
  Cc: linux-mtd, linux-spi, devicetree, linux-kernel, Julien Su,
	Jaime Liao, Thomas Petazzoni, Boris Brezillon, Xiangsheng Hou,
	Miquel Raynal

Create the mxic_spi_mem_prep_op_cfg() helper to provide the content to
write to the register controlling the next IO command. This helper will
soon be used by the dirmap implementation and having this code
factorized out earlier will clarify this addition.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/spi/spi-mxic.c | 53 +++++++++++++++++++++++-------------------
 1 file changed, 29 insertions(+), 24 deletions(-)

diff --git a/drivers/spi/spi-mxic.c b/drivers/spi/spi-mxic.c
index 068543c40ce7..3c4e64cbe812 100644
--- a/drivers/spi/spi-mxic.c
+++ b/drivers/spi/spi-mxic.c
@@ -296,6 +296,33 @@ static u32 mxic_spi_prep_hc_cfg(struct spi_device *spi, u32 flags)
 	       HC_CFG_SLV_ACT(spi->chip_select) | HC_CFG_IDLE_SIO_LVL(1);
 }
 
+static u32 mxic_spi_mem_prep_op_cfg(const struct spi_mem_op *op)
+{
+	u32 cfg = OP_CMD_BYTES(op->cmd.nbytes) |
+		  OP_CMD_BUSW(fls(op->cmd.buswidth) - 1) |
+		  (op->cmd.dtr ? OP_CMD_DDR : 0);
+
+	if (op->addr.nbytes)
+		cfg |= OP_ADDR_BYTES(op->addr.nbytes) |
+		       OP_ADDR_BUSW(fls(op->addr.buswidth) - 1) |
+		       (op->addr.dtr ? OP_ADDR_DDR : 0);
+
+	if (op->dummy.nbytes)
+		cfg |= OP_DUMMY_CYC(op->dummy.nbytes);
+
+	if (op->data.nbytes) {
+		cfg |= OP_DATA_BUSW(fls(op->data.buswidth) - 1) |
+		       (op->data.dtr ? OP_DATA_DDR : 0);
+		if (op->data.dir == SPI_MEM_DATA_IN) {
+			cfg |= OP_READ;
+			if (op->data.dtr)
+				cfg |= OP_DQS_EN;
+		}
+	}
+
+	return cfg;
+}
+
 static int mxic_spi_data_xfer(struct mxic_spi *mxic, const void *txbuf,
 			      void *rxbuf, unsigned int len)
 {
@@ -374,7 +401,6 @@ static int mxic_spi_mem_exec_op(struct spi_mem *mem,
 {
 	struct mxic_spi *mxic = spi_master_get_devdata(mem->spi->master);
 	int i, ret;
-	u32 ss_ctrl;
 	u8 addr[8], cmd[2];
 
 	ret = mxic_spi_set_freq(mxic, mem->spi->max_speed_hz);
@@ -386,29 +412,8 @@ static int mxic_spi_mem_exec_op(struct spi_mem *mem,
 
 	writel(HC_EN_BIT, mxic->regs + HC_EN);
 
-	ss_ctrl = OP_CMD_BYTES(op->cmd.nbytes) |
-		  OP_CMD_BUSW(fls(op->cmd.buswidth) - 1) |
-		  (op->cmd.dtr ? OP_CMD_DDR : 0);
-
-	if (op->addr.nbytes)
-		ss_ctrl |= OP_ADDR_BYTES(op->addr.nbytes) |
-			   OP_ADDR_BUSW(fls(op->addr.buswidth) - 1) |
-			   (op->addr.dtr ? OP_ADDR_DDR : 0);
-
-	if (op->dummy.nbytes)
-		ss_ctrl |= OP_DUMMY_CYC(op->dummy.nbytes);
-
-	if (op->data.nbytes) {
-		ss_ctrl |= OP_DATA_BUSW(fls(op->data.buswidth) - 1) |
-			   (op->data.dtr ? OP_DATA_DDR : 0);
-		if (op->data.dir == SPI_MEM_DATA_IN) {
-			ss_ctrl |= OP_READ;
-			if (op->data.dtr)
-				ss_ctrl |= OP_DQS_EN;
-		}
-	}
-
-	writel(ss_ctrl, mxic->regs + SS_CTRL(mem->spi->chip_select));
+	writel(mxic_spi_mem_prep_op_cfg(op),
+	       mxic->regs + SS_CTRL(mem->spi->chip_select));
 
 	writel(readl(mxic->regs + HC_CFG) | HC_CFG_MAN_CS_ASSERT,
 	       mxic->regs + HC_CFG);
-- 
2.27.0


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

* [PATCH 17/18] spi: mxic: Add support for direct mapping
  2021-10-20 14:27 [PATCH 00/18] External ECC engines & Macronix support Miquel Raynal
                   ` (15 preceding siblings ...)
  2021-10-20 14:28 ` [PATCH 16/18] spi: mxic: Create a helper to ease the start of " Miquel Raynal
@ 2021-10-20 14:28 ` Miquel Raynal
  2021-10-20 14:28 ` [PATCH 18/18] spi: mxic: Add support for pipelined ECC operations Miquel Raynal
  17 siblings, 0 replies; 35+ messages in thread
From: Miquel Raynal @ 2021-10-20 14:28 UTC (permalink / raw)
  To: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus,
	Mark Brown, Rob Herring
  Cc: linux-mtd, linux-spi, devicetree, linux-kernel, Julien Su,
	Jaime Liao, Thomas Petazzoni, Boris Brezillon, Xiangsheng Hou,
	Miquel Raynal, Zhengxun Li

Implement the ->dirmap_create() and ->dirmap_read/write() hooks to
provide a fast path for read and write accesses.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Tested-by: Zhengxun Li <zhengxunli@mxic.com.tw>
Reviewed-by: Zhengxun Li <zhengxunli@mxic.com.tw>
---
 drivers/spi/spi-mxic.c | 112 +++++++++++++++++++++++++++++++++++++++--
 1 file changed, 109 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/spi-mxic.c b/drivers/spi/spi-mxic.c
index 3c4e64cbe812..ae697173eaee 100644
--- a/drivers/spi/spi-mxic.c
+++ b/drivers/spi/spi-mxic.c
@@ -172,6 +172,11 @@ struct mxic_spi {
 	struct clk *send_dly_clk;
 	void __iomem *regs;
 	u32 cur_speed_hz;
+	struct {
+		void __iomem *map;
+		dma_addr_t dma;
+		size_t size;
+	} linear;
 };
 
 static int mxic_spi_clk_enable(struct mxic_spi *mxic)
@@ -296,7 +301,8 @@ static u32 mxic_spi_prep_hc_cfg(struct spi_device *spi, u32 flags)
 	       HC_CFG_SLV_ACT(spi->chip_select) | HC_CFG_IDLE_SIO_LVL(1);
 }
 
-static u32 mxic_spi_mem_prep_op_cfg(const struct spi_mem_op *op)
+static u32 mxic_spi_mem_prep_op_cfg(const struct spi_mem_op *op,
+				    unsigned int data_len)
 {
 	u32 cfg = OP_CMD_BYTES(op->cmd.nbytes) |
 		  OP_CMD_BUSW(fls(op->cmd.buswidth) - 1) |
@@ -310,7 +316,8 @@ static u32 mxic_spi_mem_prep_op_cfg(const struct spi_mem_op *op)
 	if (op->dummy.nbytes)
 		cfg |= OP_DUMMY_CYC(op->dummy.nbytes);
 
-	if (op->data.nbytes) {
+	/* Direct mapping data.nbytes field is not populated */
+	if (data_len) {
 		cfg |= OP_DATA_BUSW(fls(op->data.buswidth) - 1) |
 		       (op->data.dtr ? OP_DATA_DDR : 0);
 		if (op->data.dir == SPI_MEM_DATA_IN) {
@@ -371,6 +378,77 @@ static int mxic_spi_data_xfer(struct mxic_spi *mxic, const void *txbuf,
 	return 0;
 }
 
+static ssize_t mxic_spi_mem_dirmap_read(struct spi_mem_dirmap_desc *desc,
+					u64 offs, size_t len, void *buf)
+{
+	struct mxic_spi *mxic = spi_master_get_devdata(desc->mem->spi->master);
+	int ret;
+	u32 sts;
+
+	if (WARN_ON(offs + desc->info.offset + len > U32_MAX))
+		return -EINVAL;
+
+	writel(mxic_spi_prep_hc_cfg(desc->mem->spi, 0), mxic->regs + HC_CFG);
+
+	writel(mxic_spi_mem_prep_op_cfg(&desc->info.op_tmpl, len),
+	       mxic->regs + LRD_CFG);
+	writel(desc->info.offset + offs, mxic->regs + LRD_ADDR);
+	len = min_t(size_t, len, mxic->linear.size);
+	writel(len, mxic->regs + LRD_RANGE);
+	writel(LMODE_CMD0(desc->info.op_tmpl.cmd.opcode) |
+	       LMODE_SLV_ACT(desc->mem->spi->chip_select) |
+	       LMODE_EN,
+	       mxic->regs + LRD_CTRL);
+
+	memcpy_fromio(buf, mxic->linear.map, len);
+
+	writel(INT_LRD_DIS, mxic->regs + INT_STS);
+	writel(0, mxic->regs + LRD_CTRL);
+
+	ret = readl_poll_timeout(mxic->regs + INT_STS, sts,
+				 sts & INT_LRD_DIS, 0, USEC_PER_SEC);
+	if (ret)
+		return ret;
+
+	return len;
+}
+
+static ssize_t mxic_spi_mem_dirmap_write(struct spi_mem_dirmap_desc *desc,
+					 u64 offs, size_t len,
+					 const void *buf)
+{
+	struct mxic_spi *mxic = spi_master_get_devdata(desc->mem->spi->master);
+	u32 sts;
+	int ret;
+
+	if (WARN_ON(offs + desc->info.offset + len > U32_MAX))
+		return -EINVAL;
+
+	writel(mxic_spi_prep_hc_cfg(desc->mem->spi, 0), mxic->regs + HC_CFG);
+
+	writel(mxic_spi_mem_prep_op_cfg(&desc->info.op_tmpl, len),
+	       mxic->regs + LWR_CFG);
+	writel(desc->info.offset + offs, mxic->regs + LWR_ADDR);
+	len = min_t(size_t, len, mxic->linear.size);
+	writel(len, mxic->regs + LWR_RANGE);
+	writel(LMODE_CMD0(desc->info.op_tmpl.cmd.opcode) |
+	       LMODE_SLV_ACT(desc->mem->spi->chip_select) |
+	       LMODE_EN,
+	       mxic->regs + LWR_CTRL);
+
+	memcpy_toio(mxic->linear.map, buf, len);
+
+	writel(INT_LWR_DIS, mxic->regs + INT_STS);
+	writel(0, mxic->regs + LWR_CTRL);
+
+	ret = readl_poll_timeout(mxic->regs + INT_STS, sts,
+				 sts & INT_LWR_DIS, 0, USEC_PER_SEC);
+	if (ret)
+		return ret;
+
+	return len;
+}
+
 static bool mxic_spi_mem_supports_op(struct spi_mem *mem,
 				     const struct spi_mem_op *op)
 {
@@ -396,6 +474,22 @@ static bool mxic_spi_mem_supports_op(struct spi_mem *mem,
 		return spi_mem_dtr_supports_op(mem, op);
 }
 
+static int mxic_spi_mem_dirmap_create(struct spi_mem_dirmap_desc *desc)
+{
+	struct mxic_spi *mxic = spi_master_get_devdata(desc->mem->spi->master);
+
+	if (!mxic->linear.map)
+		return -ENOTSUPP;
+
+	if (desc->info.offset + desc->info.length > U32_MAX)
+		return -ENOTSUPP;
+
+	if (!mxic_spi_mem_supports_op(desc->mem, &desc->info.op_tmpl))
+		return -ENOTSUPP;
+
+	return 0;
+}
+
 static int mxic_spi_mem_exec_op(struct spi_mem *mem,
 				const struct spi_mem_op *op)
 {
@@ -412,7 +506,7 @@ static int mxic_spi_mem_exec_op(struct spi_mem *mem,
 
 	writel(HC_EN_BIT, mxic->regs + HC_EN);
 
-	writel(mxic_spi_mem_prep_op_cfg(op),
+	writel(mxic_spi_mem_prep_op_cfg(op, op->data.nbytes),
 	       mxic->regs + SS_CTRL(mem->spi->chip_select));
 
 	writel(readl(mxic->regs + HC_CFG) | HC_CFG_MAN_CS_ASSERT,
@@ -454,6 +548,9 @@ static int mxic_spi_mem_exec_op(struct spi_mem *mem,
 static const struct spi_controller_mem_ops mxic_spi_mem_ops = {
 	.supports_op = mxic_spi_mem_supports_op,
 	.exec_op = mxic_spi_mem_exec_op,
+	.dirmap_create = mxic_spi_mem_dirmap_create,
+	.dirmap_read = mxic_spi_mem_dirmap_read,
+	.dirmap_write = mxic_spi_mem_dirmap_write,
 };
 
 static void mxic_spi_set_cs(struct spi_device *spi, bool lvl)
@@ -583,6 +680,15 @@ static int mxic_spi_probe(struct platform_device *pdev)
 	if (IS_ERR(mxic->regs))
 		return PTR_ERR(mxic->regs);
 
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dirmap");
+	mxic->linear.map = devm_ioremap_resource(&pdev->dev, res);
+	if (!IS_ERR(mxic->linear.map)) {
+		mxic->linear.dma = res->start;
+		mxic->linear.size = resource_size(res);
+	} else {
+		mxic->linear.map = NULL;
+	}
+
 	pm_runtime_enable(&pdev->dev);
 	master->auto_runtime_pm = true;
 
-- 
2.27.0


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

* [PATCH 18/18] spi: mxic: Add support for pipelined ECC operations
  2021-10-20 14:27 [PATCH 00/18] External ECC engines & Macronix support Miquel Raynal
                   ` (16 preceding siblings ...)
  2021-10-20 14:28 ` [PATCH 17/18] spi: mxic: Add support for direct mapping Miquel Raynal
@ 2021-10-20 14:28 ` Miquel Raynal
  2021-10-20 19:39   ` kernel test robot
  17 siblings, 1 reply; 35+ messages in thread
From: Miquel Raynal @ 2021-10-20 14:28 UTC (permalink / raw)
  To: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus,
	Mark Brown, Rob Herring
  Cc: linux-mtd, linux-spi, devicetree, linux-kernel, Julien Su,
	Jaime Liao, Thomas Petazzoni, Boris Brezillon, Xiangsheng Hou,
	Miquel Raynal

Some SPI-NAND chips do not have a proper on-die ECC engine providing
error correction/detection. This is particularly an issue on embedded
devices with limited resources because all the computations must
happen in software, unless an external hardware engine is provided.

These external engines are new and can be of two categories: external
or pipelined. Macronix is providing both, the former being already
supported. The second, however, is very SoC implementation dependent
and must be instantiated by the SPI host controller directly.

An entire subsystem has been contributed to support these engines which
makes the insertion into another subsystem such as SPI quite
straightforward without the need for a lot of specific functions.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/spi/spi-mxic.c | 114 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 112 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi-mxic.c b/drivers/spi/spi-mxic.c
index ae697173eaee..21cdf0beed2c 100644
--- a/drivers/spi/spi-mxic.c
+++ b/drivers/spi/spi-mxic.c
@@ -12,6 +12,8 @@
 #include <linux/io.h>
 #include <linux/iopoll.h>
 #include <linux/module.h>
+#include <linux/mtd/nand.h>
+#include <linux/mtd/nand-ecc-mxic.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
 #include <linux/spi/spi.h>
@@ -167,6 +169,7 @@
 #define HW_TEST(x)		(0xe0 + ((x) * 4))
 
 struct mxic_spi {
+	struct device *dev;
 	struct clk *ps_clk;
 	struct clk *send_clk;
 	struct clk *send_dly_clk;
@@ -177,6 +180,10 @@ struct mxic_spi {
 		dma_addr_t dma;
 		size_t size;
 	} linear;
+	struct {
+		struct nand_ecc_engine *engine;
+		bool enabled;
+	} ecc;
 };
 
 static int mxic_spi_clk_enable(struct mxic_spi *mxic)
@@ -378,6 +385,13 @@ static int mxic_spi_data_xfer(struct mxic_spi *mxic, const void *txbuf,
 	return 0;
 }
 
+static struct mxic_ecc_engine *mxic_spi_to_ecc_engine(struct mxic_spi *mxic)
+{
+	struct nand_ecc_engine *ecc_engine = mxic->ecc.engine;
+
+	return ecc_engine->priv;
+}
+
 static ssize_t mxic_spi_mem_dirmap_read(struct spi_mem_dirmap_desc *desc,
 					u64 offs, size_t len, void *buf)
 {
@@ -400,7 +414,14 @@ static ssize_t mxic_spi_mem_dirmap_read(struct spi_mem_dirmap_desc *desc,
 	       LMODE_EN,
 	       mxic->regs + LRD_CTRL);
 
-	memcpy_fromio(buf, mxic->linear.map, len);
+	if (mxic->ecc.enabled) {
+		ret = mxic_ecc_process_data(mxic_spi_to_ecc_engine(mxic),
+					    mxic->linear.dma + offs);
+		if (ret)
+			return ret;
+	} else {
+		memcpy_fromio(buf, mxic->linear.map, len);
+	}
 
 	writel(INT_LRD_DIS, mxic->regs + INT_STS);
 	writel(0, mxic->regs + LRD_CTRL);
@@ -436,7 +457,14 @@ static ssize_t mxic_spi_mem_dirmap_write(struct spi_mem_dirmap_desc *desc,
 	       LMODE_EN,
 	       mxic->regs + LWR_CTRL);
 
-	memcpy_toio(mxic->linear.map, buf, len);
+	if (mxic->ecc.enabled) {
+		ret = mxic_ecc_process_data(mxic_spi_to_ecc_engine(mxic),
+					    mxic->linear.dma + offs);
+		if (ret)
+			return ret;
+	} else {
+		memcpy_toio(mxic->linear.map, buf, len);
+	}
 
 	writel(INT_LWR_DIS, mxic->regs + INT_STS);
 	writel(0, mxic->regs + LWR_CTRL);
@@ -615,6 +643,80 @@ static int mxic_spi_transfer_one(struct spi_master *master,
 	return 0;
 }
 
+/* ECC wrapper */
+static int mxic_spi_mem_ecc_init_ctx(struct nand_device *nand)
+{
+	struct nand_ecc_engine_ops *ops = mxic_ecc_get_pipelined_ops();
+
+	return ops->init_ctx(nand);
+}
+
+static void mxic_spi_mem_ecc_cleanup_ctx(struct nand_device *nand)
+{
+	struct nand_ecc_engine_ops *ops = mxic_ecc_get_pipelined_ops();
+
+	ops->cleanup_ctx(nand);
+}
+
+static struct mxic_spi *mxic_nand_to_spi(struct nand_device *nand)
+{
+	struct device *dev = nand->ecc.engine->dev;
+	struct spi_master *master = dev_get_drvdata(dev);
+	struct mxic_spi *mxic = spi_master_get_devdata(master);
+
+	return mxic;
+}
+
+static int mxic_spi_mem_ecc_prepare_io_req(struct nand_device *nand,
+					   struct nand_page_io_req *req)
+{
+	struct nand_ecc_engine_ops *ops = mxic_ecc_get_pipelined_ops();
+	struct mxic_spi *mxic = mxic_nand_to_spi(nand);
+
+	mxic->ecc.enabled = (req->mode != MTD_OPS_RAW);
+
+	return ops->prepare_io_req(nand, req);
+}
+
+static int mxic_spi_mem_ecc_finish_io_req(struct nand_device *nand,
+					  struct nand_page_io_req *req)
+{
+	struct nand_ecc_engine_ops *ops = mxic_ecc_get_pipelined_ops();
+	struct mxic_spi *mxic = mxic_nand_to_spi(nand);
+
+	mxic->ecc.enabled = false;
+
+	return ops->finish_io_req(nand, req);
+}
+
+static struct nand_ecc_engine_ops mxic_spi_mem_ecc_engine_pipelined_ops = {
+	.init_ctx = mxic_spi_mem_ecc_init_ctx,
+	.cleanup_ctx = mxic_spi_mem_ecc_cleanup_ctx,
+	.prepare_io_req = mxic_spi_mem_ecc_prepare_io_req,
+	.finish_io_req = mxic_spi_mem_ecc_finish_io_req,
+};
+
+static int mxic_spi_mem_ecc_probe(struct platform_device *pdev,
+				  struct mxic_spi *mxic)
+{
+	struct nand_ecc_engine *ecceng;
+
+	if (!mxic_ecc_get_pipelined_ops())
+		return -EOPNOTSUPP;
+
+	ecceng = devm_kzalloc(&pdev->dev, sizeof(*ecceng), GFP_KERNEL);
+	if (!ecceng)
+		return -ENOMEM;
+
+	ecceng->dev = &pdev->dev;
+	ecceng->ops = &mxic_spi_mem_ecc_engine_pipelined_ops;
+
+	nand_ecc_register_on_host_hw_engine(ecceng);
+	mxic->ecc.engine = ecceng;
+
+	return 0;
+}
+
 static int __maybe_unused mxic_spi_runtime_suspend(struct device *dev)
 {
 	struct spi_master *master = dev_get_drvdata(dev);
@@ -660,6 +762,7 @@ static int mxic_spi_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, master);
 
 	mxic = spi_master_get_devdata(master);
+	mxic->dev = &pdev->dev;
 
 	master->dev.of_node = pdev->dev.of_node;
 
@@ -705,6 +808,10 @@ static int mxic_spi_probe(struct platform_device *pdev)
 
 	mxic_spi_hw_init(mxic);
 
+	ret = mxic_spi_mem_ecc_probe(pdev, mxic);
+	if (ret)
+		dev_warn(&pdev->dev, "SPI-mem ECC engine not available\n");
+
 	ret = spi_register_master(master);
 	if (ret) {
 		dev_err(&pdev->dev, "spi_register_master failed\n");
@@ -717,8 +824,11 @@ static int mxic_spi_probe(struct platform_device *pdev)
 static int mxic_spi_remove(struct platform_device *pdev)
 {
 	struct spi_master *master = platform_get_drvdata(pdev);
+	struct mxic_spi *mxic = spi_master_get_devdata(master);
+	struct nand_ecc_engine *ecc_engine = mxic->ecc.engine;
 
 	pm_runtime_disable(&pdev->dev);
+	nand_ecc_unregister_on_host_hw_engine(ecc_engine);
 	spi_unregister_master(master);
 
 	return 0;
-- 
2.27.0


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

* Re: [PATCH 18/18] spi: mxic: Add support for pipelined ECC operations
  2021-10-20 14:28 ` [PATCH 18/18] spi: mxic: Add support for pipelined ECC operations Miquel Raynal
@ 2021-10-20 19:39   ` kernel test robot
  0 siblings, 0 replies; 35+ messages in thread
From: kernel test robot @ 2021-10-20 19:39 UTC (permalink / raw)
  To: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
	Tudor Ambarus, Mark Brown, Rob Herring
  Cc: llvm, kbuild-all, linux-mtd, linux-spi, devicetree, linux-kernel,
	Julien Su

[-- Attachment #1: Type: text/plain, Size: 2746 bytes --]

Hi Miquel,

I love your patch! Perhaps something to improve:

[auto build test WARNING on broonie-spi/for-next]
[also build test WARNING on mtd/nand/next mtd/mtd/next mtd/mtd/fixes v5.15-rc6 next-20211020]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Miquel-Raynal/External-ECC-engines-Macronix-support/20211020-223036
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
config: riscv-randconfig-r004-20211019 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 9660563950aaed54020bfdf0be07e7096a9553e4)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install riscv cross compiling tool for clang build
        # apt-get install binutils-riscv64-linux-gnu
        # https://github.com/0day-ci/linux/commit/9b602c348c84bbe0698c891b14e29cf25ef74b6a
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Miquel-Raynal/External-ECC-engines-Macronix-support/20211020-223036
        git checkout 9b602c348c84bbe0698c891b14e29cf25ef74b6a
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=riscv 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   In file included from drivers/spi/spi-mxic.c:16:
>> include/linux/mtd/nand-ecc-mxic.h:23:29: warning: no previous prototype for function 'mxic_ecc_get_pipelined_ops' [-Wmissing-prototypes]
   struct nand_ecc_engine_ops *mxic_ecc_get_pipelined_ops(void)
                               ^
   include/linux/mtd/nand-ecc-mxic.h:23:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   struct nand_ecc_engine_ops *mxic_ecc_get_pipelined_ops(void)
   ^
   static 
   1 warning generated.


vim +/mxic_ecc_get_pipelined_ops +23 include/linux/mtd/nand-ecc-mxic.h

ccf53b4df2cab6 Miquel Raynal 2021-10-20  22  
ccf53b4df2cab6 Miquel Raynal 2021-10-20 @23  struct nand_ecc_engine_ops *mxic_ecc_get_pipelined_ops(void)
ccf53b4df2cab6 Miquel Raynal 2021-10-20  24  {
ccf53b4df2cab6 Miquel Raynal 2021-10-20  25  	return NULL;
ccf53b4df2cab6 Miquel Raynal 2021-10-20  26  }
ccf53b4df2cab6 Miquel Raynal 2021-10-20  27  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 45283 bytes --]

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

* Re: [PATCH 03/18] dt-bindings: mtd: nand-chip: Create a NAND chip description
  2021-10-20 14:27 ` [PATCH 03/18] dt-bindings: mtd: nand-chip: Create a NAND chip description Miquel Raynal
@ 2021-10-20 21:14   ` Rob Herring
  2021-10-22 22:47   ` Rob Herring
  1 sibling, 0 replies; 35+ messages in thread
From: Rob Herring @ 2021-10-20 21:14 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: linux-mtd, linux-spi, Richard Weinberger, Julien Su,
	Thomas Petazzoni, Boris Brezillon, Mark Brown, Xiangsheng Hou,
	devicetree, Jaime Liao, Tudor Ambarus, Vignesh Raghavendra,
	Rob Herring, linux-kernel

On Wed, 20 Oct 2021 16:27:54 +0200, Miquel Raynal wrote:
> Move the NAND chip description out of the NAND controller file. Indeed,
> a subsequent part of the properties supported by a raw NAND chip are
> also supported by SPI-NAND chips. So let's create a generic NAND chip
> description which will be pulled by nand-controller.yaml and later by
> spi-nand.yaml as well.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  .../devicetree/bindings/mtd/nand-chip.yaml    | 71 +++++++++++++++++++
>  .../bindings/mtd/nand-controller.yaml         | 53 ++------------
>  2 files changed, 75 insertions(+), 49 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/mtd/nand-chip.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mtd/brcm,brcmnand.example.dt.yaml: nand-controller@f0442800: nand@1: '#address-cells', '#size-cells', 'compatible', 'nand-on-flash-bbt' do not match any of the regexes: 'pinctrl-[0-9]+'
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mtd/brcm,brcmnand.example.dt.yaml: nand-controller@f0442800: nand@1: '#address-cells', '#size-cells', 'compatible', 'nand-on-flash-bbt' do not match any of the regexes: 'pinctrl-[0-9]+'
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mtd/nand-controller.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mtd/brcm,brcmnand.example.dt.yaml: nand-controller@10000200: nand@0: '#address-cells', '#size-cells', 'compatible', 'nand-on-flash-bbt' do not match any of the regexes: 'pinctrl-[0-9]+'
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mtd/brcm,brcmnand.example.dt.yaml: nand-controller@10000200: nand@0: '#address-cells', '#size-cells', 'compatible', 'nand-on-flash-bbt' do not match any of the regexes: 'pinctrl-[0-9]+'
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mtd/nand-controller.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mtd/ingenic,nand.example.dt.yaml: nand-controller@1: nand@1: 'nand-ecc-mode', 'nand-on-flash-bbt', 'partitions' do not match any of the regexes: 'pinctrl-[0-9]+'
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mtd/ingenic,nand.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mtd/ingenic,nand.example.dt.yaml: nand-controller@1: nand@1: 'nand-ecc-mode', 'nand-on-flash-bbt', 'partitions' do not match any of the regexes: 'pinctrl-[0-9]+'
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mtd/nand-controller.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mtd/rockchip,nand-controller.example.dt.yaml: nand-controller@ff4b0000: nand@0: 'label', 'nand-bus-width', 'nand-ecc-mode', 'nand-is-boot-medium', 'rockchip,boot-blks', 'rockchip,boot-ecc-strength' do not match any of the regexes: 'pinctrl-[0-9]+'
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mtd/rockchip,nand-controller.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mtd/rockchip,nand-controller.example.dt.yaml: nand-controller@ff4b0000: nand@0: 'label', 'nand-bus-width', 'nand-ecc-mode', 'nand-is-boot-medium', 'rockchip,boot-blks', 'rockchip,boot-ecc-strength' do not match any of the regexes: 'pinctrl-[0-9]+'
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mtd/nand-controller.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mtd/st,stm32-fmc2-nand.example.dt.yaml: nand-controller@58002000: nand@0: '#address-cells', '#size-cells', 'nand-on-flash-bbt' do not match any of the regexes: 'pinctrl-[0-9]+'
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mtd/st,stm32-fmc2-nand.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mtd/st,stm32-fmc2-nand.example.dt.yaml: nand-controller@58002000: nand@0: '#address-cells', '#size-cells', 'nand-on-flash-bbt' do not match any of the regexes: 'pinctrl-[0-9]+'
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mtd/nand-controller.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mtd/intel,lgm-nand.example.dt.yaml: nand-controller@e0f00000: nand@0: 'nand-ecc-mode' does not match any of the regexes: 'pinctrl-[0-9]+'
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mtd/nand-controller.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mtd/intel,lgm-nand.example.dt.yaml: nand-controller@e0f00000: nand@0: 'nand-ecc-mode' does not match any of the regexes: 'pinctrl-[0-9]+'
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mtd/intel,lgm-nand.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mtd/qcom,nandc.example.dt.yaml: nand-controller@1ac00000: nand@0: 'nand-bus-width', 'partitions' do not match any of the regexes: 'pinctrl-[0-9]+'
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mtd/nand-controller.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mtd/qcom,nandc.example.dt.yaml: nand-controller@1ac00000: nand@0: 'nand-bus-width', 'partitions' do not match any of the regexes: 'pinctrl-[0-9]+'
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mtd/qcom,nandc.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mtd/qcom,nandc.example.dt.yaml: nand-controller@79b0000: nand@0: 'nand-bus-width', 'partitions' do not match any of the regexes: 'pinctrl-[0-9]+'
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mtd/nand-controller.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mtd/qcom,nandc.example.dt.yaml: nand-controller@79b0000: nand@0: 'nand-bus-width', 'partitions' do not match any of the regexes: 'pinctrl-[0-9]+'
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mtd/qcom,nandc.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/memory-controllers/st,stm32-fmc2-ebi.example.dt.yaml: nand-controller@4,0: nand@0: '#address-cells', '#size-cells', 'nand-on-flash-bbt' do not match any of the regexes: 'pinctrl-[0-9]+'
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mtd/st,stm32-fmc2-nand.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/memory-controllers/st,stm32-fmc2-ebi.example.dt.yaml: nand-controller@4,0: nand@0: '#address-cells', '#size-cells', 'nand-on-flash-bbt' do not match any of the regexes: 'pinctrl-[0-9]+'
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mtd/nand-controller.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1543921

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.


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

* Re: [PATCH 04/18] dt-bindings: mtd: spi-nand: Convert spi-nand description file to yaml
  2021-10-20 14:27 ` [PATCH 04/18] dt-bindings: mtd: spi-nand: Convert spi-nand description file to yaml Miquel Raynal
@ 2021-10-20 21:14   ` Rob Herring
  2021-10-21 14:09     ` Miquel Raynal
  2021-10-28 21:28   ` Rob Herring
  1 sibling, 1 reply; 35+ messages in thread
From: Rob Herring @ 2021-10-20 21:14 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Tudor Ambarus, linux-mtd, Julien Su, linux-spi, Jaime Liao,
	Thomas Petazzoni, Richard Weinberger, Vignesh Raghavendra,
	Boris Brezillon, Mark Brown, devicetree, Xiangsheng Hou,
	Rob Herring, linux-kernel

On Wed, 20 Oct 2021 16:27:55 +0200, Miquel Raynal wrote:
> Let's get rid of spi-nand.txt by converting it to yaml schema. While at
> converting this file, let's actually pull all the generic properties
> from nand-chip.yaml which might apply to a SPI-NAND chip.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  .../devicetree/bindings/mtd/spi-nand.txt      |  5 ----
>  .../devicetree/bindings/mtd/spi-nand.yaml     | 27 +++++++++++++++++++
>  2 files changed, 27 insertions(+), 5 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/mtd/spi-nand.txt
>  create mode 100644 Documentation/devicetree/bindings/mtd/spi-nand.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Unknown file referenced: [Errno 2] No such file or directory: '/usr/local/lib/python3.8/dist-packages/dtschema/schemas/mtd/nand-chip.yaml'
xargs: dt-doc-validate: exited with status 255; aborting
make[1]: *** Deleting file 'Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.example.dt.yaml'
Unknown file referenced: [Errno 2] No such file or directory: '/usr/local/lib/python3.8/dist-packages/dtschema/schemas/mtd/nand-chip.yaml'
make[1]: *** [scripts/Makefile.lib:385: Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.example.dt.yaml] Error 255
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1441: dt_binding_check] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1543922

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.


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

* Re: [PATCH 13/18] mtd: nand: mxic-ecc: Support SPI pipelined mode
  2021-10-20 14:28 ` [PATCH 13/18] mtd: nand: mxic-ecc: Support SPI pipelined mode Miquel Raynal
@ 2021-10-21  5:22   ` kernel test robot
  0 siblings, 0 replies; 35+ messages in thread
From: kernel test robot @ 2021-10-21  5:22 UTC (permalink / raw)
  To: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
	Tudor Ambarus, Mark Brown, Rob Herring
  Cc: llvm, kbuild-all, linux-mtd, linux-spi, devicetree, linux-kernel,
	Julien Su

[-- Attachment #1: Type: text/plain, Size: 3238 bytes --]

Hi Miquel,

I love your patch! Perhaps something to improve:

[auto build test WARNING on broonie-spi/for-next]
[also build test WARNING on mtd/nand/next mtd/mtd/next mtd/mtd/fixes v5.15-rc6 next-20211020]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Miquel-Raynal/External-ECC-engines-Macronix-support/20211020-223036
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
config: hexagon-randconfig-r014-20211021 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 3cea2505fd8d99a9ba0cb625aecfe28a47c4e3f8)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/ccf53b4df2cab6209b1c56f22e1a1955f59b8ea0
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Miquel-Raynal/External-ECC-engines-Macronix-support/20211020-223036
        git checkout ccf53b4df2cab6209b1c56f22e1a1955f59b8ea0
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=hexagon 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/mtd/nand/ecc-mxic.c:480:5: warning: no previous prototype for function 'mxic_ecc_process_data' [-Wmissing-prototypes]
   int mxic_ecc_process_data(struct mxic_ecc_engine *eng, dma_addr_t dirmap)
       ^
   drivers/mtd/nand/ecc-mxic.c:480:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   int mxic_ecc_process_data(struct mxic_ecc_engine *eng, dma_addr_t dirmap)
   ^
   static 
>> drivers/mtd/nand/ecc-mxic.c:742:29: warning: no previous prototype for function 'mxic_ecc_get_pipelined_ops' [-Wmissing-prototypes]
   struct nand_ecc_engine_ops *mxic_ecc_get_pipelined_ops(void)
                               ^
   drivers/mtd/nand/ecc-mxic.c:742:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   struct nand_ecc_engine_ops *mxic_ecc_get_pipelined_ops(void)
   ^
   static 
   2 warnings generated.


vim +/mxic_ecc_process_data +480 drivers/mtd/nand/ecc-mxic.c

   479	
 > 480	int mxic_ecc_process_data(struct mxic_ecc_engine *eng, dma_addr_t dirmap)
   481	{
   482		/* Retrieve the direction */
   483		unsigned int dir = (eng->req->type == NAND_PAGE_READ) ?
   484				   READ_NAND : WRITE_NAND;
   485	
   486		if (dirmap)
   487			writel(dirmap, eng->regs + HC_SLV_ADDR);
   488	
   489		/* Trigger processing */
   490		writel(SDMA_STRT | dir, eng->regs + SDMA_CTRL);
   491	
   492		/* Wait for completion */
   493		return mxic_ecc_data_xfer_wait_for_completion(eng);
   494	}
   495	EXPORT_SYMBOL_GPL(mxic_ecc_process_data);
   496	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 29024 bytes --]

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

* Re: [PATCH 04/18] dt-bindings: mtd: spi-nand: Convert spi-nand description file to yaml
  2021-10-20 21:14   ` Rob Herring
@ 2021-10-21 14:09     ` Miquel Raynal
  2021-10-22 22:36       ` Rob Herring
  0 siblings, 1 reply; 35+ messages in thread
From: Miquel Raynal @ 2021-10-21 14:09 UTC (permalink / raw)
  To: Rob Herring
  Cc: Tudor Ambarus, linux-mtd, Julien Su, linux-spi, Jaime Liao,
	Thomas Petazzoni, Richard Weinberger, Vignesh Raghavendra,
	Boris Brezillon, Mark Brown, devicetree, Xiangsheng Hou,
	Rob Herring, linux-kernel

Hi Rob,

robh@kernel.org wrote on Wed, 20 Oct 2021 16:14:47 -0500:

> On Wed, 20 Oct 2021 16:27:55 +0200, Miquel Raynal wrote:
> > Let's get rid of spi-nand.txt by converting it to yaml schema. While at
> > converting this file, let's actually pull all the generic properties
> > from nand-chip.yaml which might apply to a SPI-NAND chip.
> > 
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
> >  .../devicetree/bindings/mtd/spi-nand.txt      |  5 ----
> >  .../devicetree/bindings/mtd/spi-nand.yaml     | 27 +++++++++++++++++++
> >  2 files changed, 27 insertions(+), 5 deletions(-)
> >  delete mode 100644 Documentation/devicetree/bindings/mtd/spi-nand.txt
> >  create mode 100644 Documentation/devicetree/bindings/mtd/spi-nand.yaml
> >   
> 
> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
> 
> yamllint warnings/errors:
> 
> dtschema/dtc warnings/errors:
> Unknown file referenced: [Errno 2] No such file or directory: '/usr/local/lib/python3.8/dist-packages/dtschema/schemas/mtd/nand-chip.yaml'
> xargs: dt-doc-validate: exited with status 255; aborting
> make[1]: *** Deleting file 'Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.example.dt.yaml'
> Unknown file referenced: [Errno 2] No such file or directory: '/usr/local/lib/python3.8/dist-packages/dtschema/schemas/mtd/nand-chip.yaml'
> make[1]: *** [scripts/Makefile.lib:385: Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.example.dt.yaml] Error 255
> make[1]: *** Waiting for unfinished jobs....
> make: *** [Makefile:1441: dt_binding_check] Error 2

I am not able to reproduce this error and in general I don't understand
it. There is no relationship between this change and
snps,dw-apb-ssi.yaml. Also the fact that nand-chip-yaml do not exist,
it was just created in the patch before so I wonder how much I should
trust this error.

Also, maybe I am not using the tools properly, but it is very hard to
send correct bindings at the first try. Running make dt_binding_check
takes ages, any change in one yaml file will recheck the entire data
base and filtering out on a single yaml file is generally too
restrictive and still prints unrelated errors of syntax on other files.
I don't know how much of this is actually expected and/or if someone is
working on it.

Thanks,
Miquèl

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

* Re: [PATCH 04/18] dt-bindings: mtd: spi-nand: Convert spi-nand description file to yaml
  2021-10-21 14:09     ` Miquel Raynal
@ 2021-10-22 22:36       ` Rob Herring
  2021-11-24 13:54         ` Miquel Raynal
  0 siblings, 1 reply; 35+ messages in thread
From: Rob Herring @ 2021-10-22 22:36 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Tudor Ambarus, linux-mtd, Julien Su, linux-spi, Jaime Liao,
	Thomas Petazzoni, Richard Weinberger, Vignesh Raghavendra,
	Boris Brezillon, Mark Brown, devicetree, Xiangsheng Hou,
	linux-kernel

On Thu, Oct 21, 2021 at 04:09:32PM +0200, Miquel Raynal wrote:
> Hi Rob,
> 
> robh@kernel.org wrote on Wed, 20 Oct 2021 16:14:47 -0500:
> 
> > On Wed, 20 Oct 2021 16:27:55 +0200, Miquel Raynal wrote:
> > > Let's get rid of spi-nand.txt by converting it to yaml schema. While at
> > > converting this file, let's actually pull all the generic properties
> > > from nand-chip.yaml which might apply to a SPI-NAND chip.
> > > 
> > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > > ---
> > >  .../devicetree/bindings/mtd/spi-nand.txt      |  5 ----
> > >  .../devicetree/bindings/mtd/spi-nand.yaml     | 27 +++++++++++++++++++
> > >  2 files changed, 27 insertions(+), 5 deletions(-)
> > >  delete mode 100644 Documentation/devicetree/bindings/mtd/spi-nand.txt
> > >  create mode 100644 Documentation/devicetree/bindings/mtd/spi-nand.yaml
> > >   
> > 
> > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> > on your patch (DT_CHECKER_FLAGS is new in v5.13):
> > 
> > yamllint warnings/errors:
> > 
> > dtschema/dtc warnings/errors:
> > Unknown file referenced: [Errno 2] No such file or directory: '/usr/local/lib/python3.8/dist-packages/dtschema/schemas/mtd/nand-chip.yaml'
> > xargs: dt-doc-validate: exited with status 255; aborting
> > make[1]: *** Deleting file 'Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.example.dt.yaml'
> > Unknown file referenced: [Errno 2] No such file or directory: '/usr/local/lib/python3.8/dist-packages/dtschema/schemas/mtd/nand-chip.yaml'
> > make[1]: *** [scripts/Makefile.lib:385: Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.example.dt.yaml] Error 255
> > make[1]: *** Waiting for unfinished jobs....
> > make: *** [Makefile:1441: dt_binding_check] Error 2
> 
> I am not able to reproduce this error and in general I don't understand
> it. There is no relationship between this change and
> snps,dw-apb-ssi.yaml. Also the fact that nand-chip-yaml do not exist,
> it was just created in the patch before so I wonder how much I should
> trust this error.

I think you can ignore this. The prior patch should have been applied, 
but looks like it wasn't. My script's patch applying logic is not what 
I'd call robust.

> Also, maybe I am not using the tools properly, but it is very hard to
> send correct bindings at the first try. Running make dt_binding_check
> takes ages, any change in one yaml file will recheck the entire data
> base and filtering out on a single yaml file is generally too
> restrictive and still prints unrelated errors of syntax on other files.

Do you set 'DT_SCHEMA_FILES'? That will check just the schema you set 
it to. You still need to not set it at the end because any schema could 
apply to any example, so we have to check everything.

Also using DT_SCHEMA_FILES should be a bit faster with what's queued for 
5.16.

> I don't know how much of this is actually expected and/or if someone is
> working on it.

Due to python startup times being slow, it turns out to generally be 
faster to not have make track changes and do things incrementally. 
That's why all the schema are checked at once (though sharded with 
xargs). So I'm not really sure there's much we can do. I've certainly 
investigated it.

Rob

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

* Re: [PATCH 03/18] dt-bindings: mtd: nand-chip: Create a NAND chip description
  2021-10-20 14:27 ` [PATCH 03/18] dt-bindings: mtd: nand-chip: Create a NAND chip description Miquel Raynal
  2021-10-20 21:14   ` Rob Herring
@ 2021-10-22 22:47   ` Rob Herring
  1 sibling, 0 replies; 35+ messages in thread
From: Rob Herring @ 2021-10-22 22:47 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus,
	Mark Brown, linux-mtd, linux-spi, devicetree, linux-kernel,
	Julien Su, Jaime Liao, Thomas Petazzoni, Boris Brezillon,
	Xiangsheng Hou

On Wed, Oct 20, 2021 at 04:27:54PM +0200, Miquel Raynal wrote:
> Move the NAND chip description out of the NAND controller file. Indeed,
> a subsequent part of the properties supported by a raw NAND chip are
> also supported by SPI-NAND chips. So let's create a generic NAND chip
> description which will be pulled by nand-controller.yaml and later by
> spi-nand.yaml as well.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  .../devicetree/bindings/mtd/nand-chip.yaml    | 71 +++++++++++++++++++
>  .../bindings/mtd/nand-controller.yaml         | 53 ++------------
>  2 files changed, 75 insertions(+), 49 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/mtd/nand-chip.yaml
> 
> diff --git a/Documentation/devicetree/bindings/mtd/nand-chip.yaml b/Documentation/devicetree/bindings/mtd/nand-chip.yaml
> new file mode 100644
> index 000000000000..1f230a3ee27d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/nand-chip.yaml
> @@ -0,0 +1,71 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mtd/nand-chip.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: NAND Chip and NAND Controller Generic Binding
> +
> +maintainers:
> +  - Miquel Raynal <miquel.raynal@bootlin.com>
> +
> +description: |
> +  This file covers the generic description of a NAND chip. It implies that the
> +  bus interface should not be taken into account: both raw NAND devices and
> +  SPI-NAND devices are concerned by this description.
> +
> +properties:
> +  reg:
> +    description:
> +      Contains the chip-select IDs.
> +
> +  nand-ecc-engine:
> +    allOf:
> +      - $ref: /schemas/types.yaml#/definitions/phandle
> +    description: |
> +      A phandle on the hardware ECC engine if any. There are
> +      basically three possibilities:
> +      1/ The ECC engine is part of the NAND controller, in this
> +      case the phandle should reference the parent node.
> +      2/ The ECC engine is part of the NAND part (on-die), in this
> +      case the phandle should reference the node itself.
> +      3/ The ECC engine is external, in this case the phandle should
> +      reference the specific ECC engine node.
> +
> +  nand-use-soft-ecc-engine:
> +    type: boolean
> +    description: Use a software ECC engine.
> +
> +  nand-no-ecc-engine:
> +    type: boolean
> +    description: Do not use any ECC correction.
> +
> +  nand-ecc-algo:
> +    description:
> +      Desired ECC algorithm.
> +    $ref: /schemas/types.yaml#/definitions/string
> +    enum: [hamming, bch, rs]
> +
> +  nand-ecc-strength:
> +    description:
> +      Maximum number of bits that can be corrected per ECC step.
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 1
> +
> +  nand-ecc-step-size:
> +    description:
> +      Number of data bytes covered by a single ECC step.
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 1
> +
> +  secure-regions:
> +    $ref: /schemas/types.yaml#/definitions/uint64-matrix
> +    description:
> +      Regions in the NAND chip which are protected using a secure element
> +      like Trustzone. This property contains the start address and size of
> +      the secure regions present.
> +
> +required:
> +  - reg
> +
> +additionalProperties: false

This is the source of the errors reported as this wasn't set before. If 
we're allowing custom properties (not defined here) within nand chip 
nodes, then each schema with custom properties has to reference 
nand-chip.yaml, set 'unevaluatedProperties: false', and then define 
their custom properties. And then this needs to be true. 

Rob

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

* Re: [PATCH 01/18] dt-bindings: mtd: nand-controller: Fix the reg property description
  2021-10-20 14:27 ` [PATCH 01/18] dt-bindings: mtd: nand-controller: Fix the reg property description Miquel Raynal
@ 2021-10-28 21:25   ` Rob Herring
  0 siblings, 0 replies; 35+ messages in thread
From: Rob Herring @ 2021-10-28 21:25 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: linux-mtd, devicetree, Thomas Petazzoni, Boris Brezillon,
	Xiangsheng Hou, Jaime Liao, Richard Weinberger, linux-kernel,
	linux-spi, Vignesh Raghavendra, Mark Brown, Rob Herring,
	Tudor Ambarus, Julien Su

On Wed, 20 Oct 2021 16:27:52 +0200, Miquel Raynal wrote:
> The reg property of a NAND device always references the chip-select(s).
> The ready/busy lines are described in the nand-rb property. I believe
> this was a harmless copy/paste error during the conversion to yaml.
> 
> Fixes: 212e49693592 ("dt-bindings: mtd: Add YAML schemas for the generic NAND options")
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  Documentation/devicetree/bindings/mtd/nand-controller.yaml | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 

Acked-by: Rob Herring <robh@kernel.org>

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

* Re: [PATCH 02/18] dt-bindings: mtd: nand-controller: Fix a comment in the examples
  2021-10-20 14:27 ` [PATCH 02/18] dt-bindings: mtd: nand-controller: Fix a comment in the examples Miquel Raynal
@ 2021-10-28 21:26   ` Rob Herring
  0 siblings, 0 replies; 35+ messages in thread
From: Rob Herring @ 2021-10-28 21:26 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: linux-mtd, Thomas Petazzoni, Boris Brezillon, linux-kernel,
	linux-spi, devicetree, Julien Su, Xiangsheng Hou,
	Vignesh Raghavendra, Jaime Liao, Rob Herring, Richard Weinberger,
	Tudor Ambarus, Mark Brown

On Wed, 20 Oct 2021 16:27:53 +0200, Miquel Raynal wrote:
> The controller properties should be in the controller 'parent' node,
> while properties in the children nodes are specific to the NAND
> *chip*. This error was already present during the yaml conversion.
> 
> Fixes: 2d472aba15ff ("mtd: nand: document the NAND controller/NAND chip DT representation")
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  Documentation/devicetree/bindings/mtd/nand-controller.yaml | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 

Acked-by: Rob Herring <robh@kernel.org>

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

* Re: [PATCH 04/18] dt-bindings: mtd: spi-nand: Convert spi-nand description file to yaml
  2021-10-20 14:27 ` [PATCH 04/18] dt-bindings: mtd: spi-nand: Convert spi-nand description file to yaml Miquel Raynal
  2021-10-20 21:14   ` Rob Herring
@ 2021-10-28 21:28   ` Rob Herring
  1 sibling, 0 replies; 35+ messages in thread
From: Rob Herring @ 2021-10-28 21:28 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus,
	Mark Brown, linux-mtd, linux-spi, devicetree, linux-kernel,
	Julien Su, Jaime Liao, Thomas Petazzoni, Boris Brezillon,
	Xiangsheng Hou

On Wed, Oct 20, 2021 at 04:27:55PM +0200, Miquel Raynal wrote:
> Let's get rid of spi-nand.txt by converting it to yaml schema. While at
> converting this file, let's actually pull all the generic properties
> from nand-chip.yaml which might apply to a SPI-NAND chip.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  .../devicetree/bindings/mtd/spi-nand.txt      |  5 ----
>  .../devicetree/bindings/mtd/spi-nand.yaml     | 27 +++++++++++++++++++
>  2 files changed, 27 insertions(+), 5 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/mtd/spi-nand.txt
>  create mode 100644 Documentation/devicetree/bindings/mtd/spi-nand.yaml

Reviewed-by: Rob Herring <robh@kernel.org>

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

* Re: [PATCH 05/18] dt-bindings: vendor-prefixes: Clarify Macronix prefix
  2021-10-20 14:27 ` [PATCH 05/18] dt-bindings: vendor-prefixes: Clarify Macronix prefix Miquel Raynal
@ 2021-10-28 21:31   ` Rob Herring
  0 siblings, 0 replies; 35+ messages in thread
From: Rob Herring @ 2021-10-28 21:31 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: linux-mtd, Jaime Liao, Vignesh Raghavendra, Boris Brezillon,
	Xiangsheng Hou, Thomas Petazzoni, linux-kernel, Mark Brown,
	linux-spi, devicetree, Richard Weinberger, Julien Su,
	Tudor Ambarus, Rob Herring

On Wed, 20 Oct 2021 16:27:56 +0200, Miquel Raynal wrote:
> When looking at compatible prefixes, Macronix is sometimes referred as
> "mxicy":
> - mxicy,mx25r1635f
> - mxicy,mx25u6435f
> - mxicy,mx25v8035f
> - mxicy,mx25f0a-spi
> and sometimes as "mxic":
> - mxic,multi-itfc-v009-nand-controller
> - mxic,enable-randomizer-otp
> 
> The oldest prefix that is also the one preferred by Macronix engineers
> is "mxicy", so document the other one and mark it deprecated.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  Documentation/devicetree/bindings/vendor-prefixes.yaml | 3 +++
>  1 file changed, 3 insertions(+)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

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

* Re: [PATCH 06/18] dt-bindings: spi: mxic: The interrupt property is not mandatory
  2021-10-20 14:27 ` [PATCH 06/18] dt-bindings: spi: mxic: The interrupt property is not mandatory Miquel Raynal
@ 2021-10-28 21:32   ` Rob Herring
  0 siblings, 0 replies; 35+ messages in thread
From: Rob Herring @ 2021-10-28 21:32 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Mark Brown, Boris Brezillon, Rob Herring, Richard Weinberger,
	linux-mtd, linux-spi, devicetree, Tudor Ambarus, linux-kernel,
	Xiangsheng Hou, Thomas Petazzoni, Julien Su, Vignesh Raghavendra,
	Jaime Liao

On Wed, 20 Oct 2021 16:27:57 +0200, Miquel Raynal wrote:
> The interrupt property is not mandatory at all, this property should not
> be part of the required properties list, so move it into the optional
> properties list.
> 
> Fixes: 326e5c8d4a87 ("dt-binding: spi: Document Macronix controller bindings")
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  Documentation/devicetree/bindings/spi/spi-mxic.txt | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

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

* Re: [PATCH 07/18] dt-bindings: spi: mxic: Convert to yaml
  2021-10-20 14:27 ` [PATCH 07/18] dt-bindings: spi: mxic: Convert to yaml Miquel Raynal
@ 2021-10-28 21:34   ` Rob Herring
  0 siblings, 0 replies; 35+ messages in thread
From: Rob Herring @ 2021-10-28 21:34 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus,
	Mark Brown, linux-mtd, linux-spi, devicetree, linux-kernel,
	Julien Su, Jaime Liao, Thomas Petazzoni, Boris Brezillon,
	Xiangsheng Hou

On Wed, Oct 20, 2021 at 04:27:58PM +0200, Miquel Raynal wrote:
> Straightforward conversion from regular text to yaml schema of the
> Macronix SPI controller DT bindings.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  .../bindings/spi/mxicy,mx25f0a-spi.yaml       | 67 +++++++++++++++++++
>  .../devicetree/bindings/spi/spi-mxic.txt      | 36 ----------
>  2 files changed, 67 insertions(+), 36 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/spi/mxicy,mx25f0a-spi.yaml
>  delete mode 100644 Documentation/devicetree/bindings/spi/spi-mxic.txt
> 
> diff --git a/Documentation/devicetree/bindings/spi/mxicy,mx25f0a-spi.yaml b/Documentation/devicetree/bindings/spi/mxicy,mx25f0a-spi.yaml
> new file mode 100644
> index 000000000000..4036c14fc533
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/spi/mxicy,mx25f0a-spi.yaml
> @@ -0,0 +1,67 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/spi/mxicy,mx25f0a-spi.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Macronix SPI controller device tree bindings
> +
> +maintainers:
> +  - Miquel Raynal <miquel.raynal@bootlin.com>
> +
> +allOf:
> +  - $ref: "spi-controller.yaml#"
> +
> +properties:
> +  compatible:
> +    const: mxicy,mx25f0a-spi
> +
> +  reg:
> +    minItems: 2
> +    maxItems: 2
> +
> +  reg-names:
> +    items:
> +      - const: regs
> +      - const: dirmap
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    minItems: 3
> +    maxItems: 3
> +
> +  clock-names:
> +    items:
> +      - const: send_clk
> +      - const: send_dly_clk
> +      - const: ps_clk
> +

> +  "#address-cells":
> +    const: 1
> +  "#size-cells":
> +    const: 0

Can drop these as spi-controller.yaml already defines them.

> +
> +required:
> +  - compatible
> +  - reg
> +  - reg-names
> +  - clocks
> +  - clock-names

> +  - "#address-cells"
> +  - "#size-cells"

And makes them required.

With that,

Reviewed-by: Rob Herring <robh@kernel.org>

> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    spi@43c30000 {
> +      compatible = "mxicy,mx25f0a-spi";
> +      reg = <0x43c30000 0x10000>, <0xa0000000 0x20000000>;
> +      reg-names = "regs", "dirmap";
> +      clocks = <&clkwizard 0>, <&clkwizard 1>, <&clkc 18>;
> +      clock-names = "send_clk", "send_dly_clk", "ps_clk";
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +    };

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

* Re: [PATCH 08/18] dt-bindings: mtd: Describe Macronix NAND ECC engine
  2021-10-20 14:27 ` [PATCH 08/18] dt-bindings: mtd: Describe Macronix NAND ECC engine Miquel Raynal
@ 2021-10-28 21:35   ` Rob Herring
  0 siblings, 0 replies; 35+ messages in thread
From: Rob Herring @ 2021-10-28 21:35 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Xiangsheng Hou, Richard Weinberger, Mark Brown,
	Vignesh Raghavendra, Boris Brezillon, linux-mtd, linux-kernel,
	Tudor Ambarus, linux-spi, devicetree, Julien Su,
	Thomas Petazzoni, Jaime Liao, Rob Herring

On Wed, 20 Oct 2021 16:27:59 +0200, Miquel Raynal wrote:
> Describe Macronix NAND ECC engine. This engine may be used as an
> external engine or can be pipelined with either a raw NAND controller or
> a SPI controller. Both hardware designs with a SPI controller are shown
> in the examples.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  .../bindings/mtd/mxicy,nand-ecc-engine.yaml   | 77 +++++++++++++++++++
>  1 file changed, 77 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mtd/mxicy,nand-ecc-engine.yaml
> 

Reviewed-by: Rob Herring <robh@kernel.org>

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

* Re: [PATCH 09/18] dt-bindings: spi: mxic: Document the nand-ecc-engine property
  2021-10-20 14:28 ` [PATCH 09/18] dt-bindings: spi: mxic: Document the nand-ecc-engine property Miquel Raynal
@ 2021-10-28 21:38   ` Rob Herring
  0 siblings, 0 replies; 35+ messages in thread
From: Rob Herring @ 2021-10-28 21:38 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus,
	Mark Brown, linux-mtd, linux-spi, devicetree, linux-kernel,
	Julien Su, Jaime Liao, Thomas Petazzoni, Boris Brezillon,
	Xiangsheng Hou

On Wed, Oct 20, 2021 at 04:28:00PM +0200, Miquel Raynal wrote:
> This SPI controller supports interacting with an external ECC
> engine. The nand-ecc-engine property already exist in the NAND world but
> also applies to SPI controller nodes which have external correction
> capabilities like Macronix's.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  .../devicetree/bindings/spi/mxicy,mx25f0a-spi.yaml          | 6 ++++++
>  1 file changed, 6 insertions(+)

This should come before patch 8. The example would fail if 
'unevaluatedProperties' did anything.

Reviewed-by: Rob Herring <robh@kernel.org>

> 
> diff --git a/Documentation/devicetree/bindings/spi/mxicy,mx25f0a-spi.yaml b/Documentation/devicetree/bindings/spi/mxicy,mx25f0a-spi.yaml
> index 4036c14fc533..01618a77627d 100644
> --- a/Documentation/devicetree/bindings/spi/mxicy,mx25f0a-spi.yaml
> +++ b/Documentation/devicetree/bindings/spi/mxicy,mx25f0a-spi.yaml
> @@ -43,6 +43,12 @@ properties:
>    "#size-cells":
>      const: 0
>  
> +  nand-ecc-engine:
> +    allOf:
> +      - $ref: /schemas/types.yaml#/definitions/phandle
> +    description: NAND ECC engine used by the SPI controller in order to perform
> +      on-the-fly correction when using a SPI-NAND memory.
> +
>  required:
>    - compatible
>    - reg
> -- 
> 2.27.0
> 
> 

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

* Re: [PATCH 04/18] dt-bindings: mtd: spi-nand: Convert spi-nand description file to yaml
  2021-10-22 22:36       ` Rob Herring
@ 2021-11-24 13:54         ` Miquel Raynal
  0 siblings, 0 replies; 35+ messages in thread
From: Miquel Raynal @ 2021-11-24 13:54 UTC (permalink / raw)
  To: Rob Herring
  Cc: Tudor Ambarus, linux-mtd, Julien Su, linux-spi, Jaime Liao,
	Thomas Petazzoni, Richard Weinberger, Vignesh Raghavendra,
	Boris Brezillon, Mark Brown, devicetree, Xiangsheng Hou,
	linux-kernel

Hi Rob,

robh@kernel.org wrote on Fri, 22 Oct 2021 17:36:18 -0500:

> On Thu, Oct 21, 2021 at 04:09:32PM +0200, Miquel Raynal wrote:
> > Hi Rob,
> > 
> > robh@kernel.org wrote on Wed, 20 Oct 2021 16:14:47 -0500:
> >   
> > > On Wed, 20 Oct 2021 16:27:55 +0200, Miquel Raynal wrote:  
> > > > Let's get rid of spi-nand.txt by converting it to yaml schema. While at
> > > > converting this file, let's actually pull all the generic properties
> > > > from nand-chip.yaml which might apply to a SPI-NAND chip.
> > > > 
> > > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > > > ---
> > > >  .../devicetree/bindings/mtd/spi-nand.txt      |  5 ----
> > > >  .../devicetree/bindings/mtd/spi-nand.yaml     | 27 +++++++++++++++++++
> > > >  2 files changed, 27 insertions(+), 5 deletions(-)
> > > >  delete mode 100644 Documentation/devicetree/bindings/mtd/spi-nand.txt
> > > >  create mode 100644 Documentation/devicetree/bindings/mtd/spi-nand.yaml
> > > >     
> > > 
> > > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> > > on your patch (DT_CHECKER_FLAGS is new in v5.13):
> > > 
> > > yamllint warnings/errors:
> > > 
> > > dtschema/dtc warnings/errors:
> > > Unknown file referenced: [Errno 2] No such file or directory: '/usr/local/lib/python3.8/dist-packages/dtschema/schemas/mtd/nand-chip.yaml'
> > > xargs: dt-doc-validate: exited with status 255; aborting
> > > make[1]: *** Deleting file 'Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.example.dt.yaml'
> > > Unknown file referenced: [Errno 2] No such file or directory: '/usr/local/lib/python3.8/dist-packages/dtschema/schemas/mtd/nand-chip.yaml'
> > > make[1]: *** [scripts/Makefile.lib:385: Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.example.dt.yaml] Error 255
> > > make[1]: *** Waiting for unfinished jobs....
> > > make: *** [Makefile:1441: dt_binding_check] Error 2  
> > 
> > I am not able to reproduce this error and in general I don't understand
> > it. There is no relationship between this change and
> > snps,dw-apb-ssi.yaml. Also the fact that nand-chip-yaml do not exist,
> > it was just created in the patch before so I wonder how much I should
> > trust this error.  
> 
> I think you can ignore this. The prior patch should have been applied, 
> but looks like it wasn't. My script's patch applying logic is not what 
> I'd call robust.

Ok, I understand what's happening behind the scenes, no problem with
this.

> 
> > Also, maybe I am not using the tools properly, but it is very hard to
> > send correct bindings at the first try. Running make dt_binding_check
> > takes ages, any change in one yaml file will recheck the entire data
> > base and filtering out on a single yaml file is generally too
> > restrictive and still prints unrelated errors of syntax on other files.  
> 
> Do you set 'DT_SCHEMA_FILES'? That will check just the schema you set 
> it to. You still need to not set it at the end because any schema could 
> apply to any example, so we have to check everything.

Actually I think this is what could be enhanced: when I use
DT_SCHEMA_FILES the output is always polluted with errors with nothing
(at least from my user point of view) related to the files I am
working on. See [1] for an example of output that I found hard to parse
for errors related to my changes: I am looking for issues in
nand-chip.yaml and (50 seconds later) I get warnings for:
- ilitek,ili9341.yaml
- snps,dwcmshc-sdhci.yaml
- ti,sn65dsi83.yaml
- nxp,dwmac-imx.yaml
- fsl,imx6q-pcie.yaml

Do you think the reporting can be improved?

> Also using DT_SCHEMA_FILES should be a bit faster with what's queued for 
> 5.16.

Great!

> 
> > I don't know how much of this is actually expected and/or if someone is
> > working on it.  
> 
> Due to python startup times being slow, it turns out to generally be 
> faster to not have make track changes and do things incrementally. 
> That's why all the schema are checked at once (though sharded with 
> xargs). So I'm not really sure there's much we can do. I've certainly 
> investigated it.

I understand it's not that easy and takes a lot of time, thanks anyway
for all your work on this topic.

Thanks,
Miquèl

[1]
$ time make dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/mtd/nand-chip.yaml
  LINT    Documentation/devicetree/bindings
./Documentation/devicetree/bindings/display/panel/ilitek,ili9341.yaml:25:9: [warning] wrong indentation: expected 10 but found 8 (indentation)
  CHKDT   Documentation/devicetree/bindings/processed-schema-examples.json
/home/mraynal/macronix/linux/Documentation/devicetree/bindings/pci/: properties:interrupt-names: 'oneOf' conditional failed, one must be fixed:
	[{'const': 'msi'}] is too short
	False schema does not allow 1
	hint: "minItems" is only needed if less than the "items" list length
	from schema $id: http://devicetree.org/meta-schemas/items.yaml#
/home/mraynal/macronix/linux/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml: properties:reg: 'oneOf' conditional failed, one must be fixed:
	[{'description': 'Offset and length of the register set for the device'}] is too short
	False schema does not allow 1
	hint: "minItems" is only needed if less than the "items" list length
	from schema $id: http://devicetree.org/meta-schemas/items.yaml#
/home/mraynal/macronix/linux/Documentation/devicetree/bindings/display/bridge/: properties:ports:properties:port@0:properties:endpoint:properties:data-lanes: {'required': ['maxItems']} is not allowed for {'description': 'array of physical DSI data lane indexes.', 'minItems': 1, 'maxItems': 4, 'items': [{'const': 1}, {'const': 2}, {'const': 3}, {'const': 4}]}
	hint: "maxItems" is not needed with an "items" list
	from schema $id: http://devicetree.org/meta-schemas/items.yaml#
/home/mraynal/macronix/linux/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml: properties:ports:properties:port@1:properties:endpoint:properties:data-lanes: {'required': ['maxItems']} is not allowed for {'description': 'array of physical DSI data lane indexes.', 'minItems': 1, 'maxItems': 4, 'items': [{'const': 1}, {'const': 2}, {'const': 3}, {'const': 4}]}
	hint: "maxItems" is not needed with an "items" list
	from schema $id: http://devicetree.org/meta-schemas/items.yaml#
/home/mraynal/macronix/linux/Documentation/devicetree/bindings/net/nxp,dwmac-imx.yaml: properties:clocks: {'required': ['maxItems']} is not allowed for {'minItems': 3, 'maxItems': 5, 'items': [{'description': 'MAC host clock'}, {'description': 'MAC apb clock'}, {'description': 'MAC timer clock'}, {'description': 'MAC RGMII TX clock'}, {'description': 'EQOS MEM clock'}]}
	hint: "maxItems" is not needed with an "items" list
	from schema $id: http://devicetree.org/meta-schemas/items.yaml#
  SCHEMA  Documentation/devicetree/bindings/processed-schema-examples.json
/home/mraynal/macronix/linux/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml: ignoring, error in schema: properties: interrupt-names
warning: no schema found in file: ./Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
/home/mraynal/macronix/linux/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml: ignoring, error in schema: properties: reg
warning: no schema found in file: ./Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
/home/mraynal/macronix/linux/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml: ignoring, error in schema: properties: ports: properties: port@0: properties: endpoint: properties: data-lanes
warning: no schema found in file: ./Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
/home/mraynal/macronix/linux/Documentation/devicetree/bindings/net/nxp,dwmac-imx.yaml: ignoring, error in schema: properties: clocks
warning: no schema found in file: ./Documentation/devicetree/bindings/net/nxp,dwmac-imx.yaml
  DTEX    Documentation/devicetree/bindings/mtd/nand-chip.example.dts
  DTC     Documentation/devicetree/bindings/mtd/nand-chip.example.dt.yaml
  CHECK   Documentation/devicetree/bindings/mtd/nand-chip.example.dt.yaml

real	0m38,958s
user	0m51,604s
sys	0m0,632s

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

end of thread, other threads:[~2021-11-24 13:57 UTC | newest]

Thread overview: 35+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-20 14:27 [PATCH 00/18] External ECC engines & Macronix support Miquel Raynal
2021-10-20 14:27 ` [PATCH 01/18] dt-bindings: mtd: nand-controller: Fix the reg property description Miquel Raynal
2021-10-28 21:25   ` Rob Herring
2021-10-20 14:27 ` [PATCH 02/18] dt-bindings: mtd: nand-controller: Fix a comment in the examples Miquel Raynal
2021-10-28 21:26   ` Rob Herring
2021-10-20 14:27 ` [PATCH 03/18] dt-bindings: mtd: nand-chip: Create a NAND chip description Miquel Raynal
2021-10-20 21:14   ` Rob Herring
2021-10-22 22:47   ` Rob Herring
2021-10-20 14:27 ` [PATCH 04/18] dt-bindings: mtd: spi-nand: Convert spi-nand description file to yaml Miquel Raynal
2021-10-20 21:14   ` Rob Herring
2021-10-21 14:09     ` Miquel Raynal
2021-10-22 22:36       ` Rob Herring
2021-11-24 13:54         ` Miquel Raynal
2021-10-28 21:28   ` Rob Herring
2021-10-20 14:27 ` [PATCH 05/18] dt-bindings: vendor-prefixes: Clarify Macronix prefix Miquel Raynal
2021-10-28 21:31   ` Rob Herring
2021-10-20 14:27 ` [PATCH 06/18] dt-bindings: spi: mxic: The interrupt property is not mandatory Miquel Raynal
2021-10-28 21:32   ` Rob Herring
2021-10-20 14:27 ` [PATCH 07/18] dt-bindings: spi: mxic: Convert to yaml Miquel Raynal
2021-10-28 21:34   ` Rob Herring
2021-10-20 14:27 ` [PATCH 08/18] dt-bindings: mtd: Describe Macronix NAND ECC engine Miquel Raynal
2021-10-28 21:35   ` Rob Herring
2021-10-20 14:28 ` [PATCH 09/18] dt-bindings: spi: mxic: Document the nand-ecc-engine property Miquel Raynal
2021-10-28 21:38   ` Rob Herring
2021-10-20 14:28 ` [PATCH 10/18] mtd: spinand: macronix: Use random program load Miquel Raynal
2021-10-20 14:28 ` [PATCH 11/18] mtd: nand: ecc: Add infrastructure to support hardware engines Miquel Raynal
2021-10-20 14:28 ` [PATCH 12/18] mtd: nand: mxic-ecc: Add Macronix external ECC engine support Miquel Raynal
2021-10-20 14:28 ` [PATCH 13/18] mtd: nand: mxic-ecc: Support SPI pipelined mode Miquel Raynal
2021-10-21  5:22   ` kernel test robot
2021-10-20 14:28 ` [PATCH 14/18] spi: mxic: Fix the transmit path Miquel Raynal
2021-10-20 14:28 ` [PATCH 15/18] spi: mxic: Create a helper to configure the controller before an operation Miquel Raynal
2021-10-20 14:28 ` [PATCH 16/18] spi: mxic: Create a helper to ease the start of " Miquel Raynal
2021-10-20 14:28 ` [PATCH 17/18] spi: mxic: Add support for direct mapping Miquel Raynal
2021-10-20 14:28 ` [PATCH 18/18] spi: mxic: Add support for pipelined ECC operations Miquel Raynal
2021-10-20 19:39   ` kernel test robot

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).