2019-02-09 19:23:48

by Paul Cercueil

[permalink] [raw]
Subject: [PATCH v4 1/9] dt-bindings: mtd: ingenic: Add compatible strings for JZ4740 and JZ4725B

Add compatible strings to probe the jz4780-nand and jz4780-bch drivers
from devicetree on the JZ4725B and JZ4740 SoCs from Ingenic.

Signed-off-by: Paul Cercueil <[email protected]>
---

Changes:

v2: - Change 'ingenic,jz4725b-nand' compatible string to
'ingenic,jz4740-nand' to reflect driver change
- Add 'ingenic,jz4740-bch' compatible string
- Document 'ingenic,oob-layout' property

v3: - Removed 'ingenic,oob-layout' property
- Update compatible strings to what the driver supports

v4: No change

Documentation/devicetree/bindings/mtd/ingenic,jz4780-nand.txt | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/mtd/ingenic,jz4780-nand.txt b/Documentation/devicetree/bindings/mtd/ingenic,jz4780-nand.txt
index 29ea5853ca91..a5b940f18bf6 100644
--- a/Documentation/devicetree/bindings/mtd/ingenic,jz4780-nand.txt
+++ b/Documentation/devicetree/bindings/mtd/ingenic,jz4780-nand.txt
@@ -6,7 +6,10 @@ memory-controllers/ingenic,jz4780-nemc.txt), and thus NAND device nodes must
be children of the NEMC node.

Required NAND controller device properties:
-- compatible: Should be set to "ingenic,jz4780-nand".
+- compatible: Should be one of:
+ * ingenic,jz4740-nand
+ * ingenic,jz4725b-nand
+ * ingenic,jz4780-nand
- reg: For each bank with a NAND chip attached, should specify a bank number,
an offset of 0 and a size of 0x1000000 (i.e. the whole NEMC bank).

@@ -72,7 +75,10 @@ NAND devices. The following is a description of the device properties for a
BCH controller.

Required BCH properties:
-- compatible: Should be set to "ingenic,jz4780-bch".
+- compatible: Should be one of:
+ * ingenic,jz4740-ecc
+ * ingenic,jz4725b-bch
+ * ingenic,jz4780-bch
- reg: Should specify the BCH controller registers location and length.
- clocks: Clock for the BCH controller.

--
2.11.0



2019-02-09 19:23:56

by Paul Cercueil

[permalink] [raw]
Subject: [PATCH v4 2/9] dt-bindings: mtd: ingenic: Change 'BCH' to 'ECC' in documentation

The JZ4740 ECC hardware is not BCH but Reed-Solomon, so it makes more
sense to use the more generic ECC term.

Signed-off-by: Paul Cercueil <[email protected]>
---

Changes:

v3: New patch

v4: No change

.../devicetree/bindings/mtd/ingenic,jz4780-nand.txt | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/Documentation/devicetree/bindings/mtd/ingenic,jz4780-nand.txt b/Documentation/devicetree/bindings/mtd/ingenic,jz4780-nand.txt
index a5b940f18bf6..5a45cc54f46d 100644
--- a/Documentation/devicetree/bindings/mtd/ingenic,jz4780-nand.txt
+++ b/Documentation/devicetree/bindings/mtd/ingenic,jz4780-nand.txt
@@ -1,4 +1,4 @@
-* Ingenic JZ4780 NAND/BCH
+* Ingenic JZ4780 NAND/ECC

This file documents the device tree bindings for NAND flash devices on the
JZ4780. NAND devices are connected to the NEMC controller (described in
@@ -14,10 +14,10 @@ Required NAND controller device properties:
an offset of 0 and a size of 0x1000000 (i.e. the whole NEMC bank).

Optional NAND controller device properties:
-- ingenic,bch-controller: To make use of the hardware BCH controller, this
- property must contain a phandle for the BCH controller node. The required
+- ingenic,bch-controller: To make use of the hardware ECC controller, this
+ property must contain a phandle for the ECC controller node. The required
properties for this node are described below. If this is not specified,
- software BCH will be used instead.
+ software ECC will be used instead.

Optional children nodes:
- Individual NAND chips are children of the NAND controller node.
@@ -70,17 +70,17 @@ nemc: nemc@13410000 {
};
};

-The BCH controller is a separate SoC component used for error correction on
+The ECC controller is a separate SoC component used for error correction on
NAND devices. The following is a description of the device properties for a
-BCH controller.
+ECC controller.

-Required BCH properties:
+Required ECC properties:
- compatible: Should be one of:
* ingenic,jz4740-ecc
* ingenic,jz4725b-bch
* ingenic,jz4780-bch
-- reg: Should specify the BCH controller registers location and length.
-- clocks: Clock for the BCH controller.
+- reg: Should specify the ECC controller registers location and length.
+- clocks: Clock for the ECC controller.

Example:

--
2.11.0


2019-02-09 19:24:05

by Paul Cercueil

[permalink] [raw]
Subject: [PATCH v4 3/9] mtd: rawnand: Move drivers for Ingenic SoCs to subfolder

Before adding support for more SoCs and seeing the number of files for
these drivers grow, we move them to their own subfolder to keep it tidy.

Signed-off-by: Paul Cercueil <[email protected]>
---

Changes:

v2: New patch

v3: No change

v4: No change

drivers/mtd/nand/raw/Kconfig | 14 +-------------
drivers/mtd/nand/raw/Makefile | 3 +--
drivers/mtd/nand/raw/ingenic/Kconfig | 13 +++++++++++++
drivers/mtd/nand/raw/ingenic/Makefile | 2 ++
drivers/mtd/nand/raw/{ => ingenic}/jz4740_nand.c | 0
drivers/mtd/nand/raw/{ => ingenic}/jz4780_bch.c | 0
drivers/mtd/nand/raw/{ => ingenic}/jz4780_bch.h | 0
drivers/mtd/nand/raw/{ => ingenic}/jz4780_nand.c | 0
8 files changed, 17 insertions(+), 15 deletions(-)
create mode 100644 drivers/mtd/nand/raw/ingenic/Kconfig
create mode 100644 drivers/mtd/nand/raw/ingenic/Makefile
rename drivers/mtd/nand/raw/{ => ingenic}/jz4740_nand.c (100%)
rename drivers/mtd/nand/raw/{ => ingenic}/jz4780_bch.c (100%)
rename drivers/mtd/nand/raw/{ => ingenic}/jz4780_bch.h (100%)
rename drivers/mtd/nand/raw/{ => ingenic}/jz4780_nand.c (100%)

diff --git a/drivers/mtd/nand/raw/Kconfig b/drivers/mtd/nand/raw/Kconfig
index 1a55d3e3d4c5..d886be2fc174 100644
--- a/drivers/mtd/nand/raw/Kconfig
+++ b/drivers/mtd/nand/raw/Kconfig
@@ -470,19 +470,7 @@ config MTD_NAND_NUC900
This enables the driver for the NAND Flash on evaluation board based
on w90p910 / NUC9xx.

-config MTD_NAND_JZ4740
- tristate "Support for JZ4740 SoC NAND controller"
- depends on MACH_JZ4740 || COMPILE_TEST
- depends on HAS_IOMEM
- help
- Enables support for NAND Flash on JZ4740 SoC based boards.
-
-config MTD_NAND_JZ4780
- tristate "Support for NAND on JZ4780 SoC"
- depends on JZ4780_NEMC
- help
- Enables support for NAND Flash connected to the NEMC on JZ4780 SoC
- based boards, using the BCH controller for hardware error correction.
+source "drivers/mtd/nand/raw/ingenic/Kconfig"

config MTD_NAND_FSMC
tristate "Support for NAND on ST Micros FSMC"
diff --git a/drivers/mtd/nand/raw/Makefile b/drivers/mtd/nand/raw/Makefile
index 57159b349054..f419d373d090 100644
--- a/drivers/mtd/nand/raw/Makefile
+++ b/drivers/mtd/nand/raw/Makefile
@@ -45,8 +45,7 @@ obj-$(CONFIG_MTD_NAND_NUC900) += nuc900_nand.o
obj-$(CONFIG_MTD_NAND_MPC5121_NFC) += mpc5121_nfc.o
obj-$(CONFIG_MTD_NAND_VF610_NFC) += vf610_nfc.o
obj-$(CONFIG_MTD_NAND_RICOH) += r852.o
-obj-$(CONFIG_MTD_NAND_JZ4740) += jz4740_nand.o
-obj-$(CONFIG_MTD_NAND_JZ4780) += jz4780_nand.o jz4780_bch.o
+obj-y += ingenic/
obj-$(CONFIG_MTD_NAND_GPMI_NAND) += gpmi-nand/
obj-$(CONFIG_MTD_NAND_XWAY) += xway_nand.o
obj-$(CONFIG_MTD_NAND_BCM47XXNFLASH) += bcm47xxnflash/
diff --git a/drivers/mtd/nand/raw/ingenic/Kconfig b/drivers/mtd/nand/raw/ingenic/Kconfig
new file mode 100644
index 000000000000..67806c87b2c4
--- /dev/null
+++ b/drivers/mtd/nand/raw/ingenic/Kconfig
@@ -0,0 +1,13 @@
+config MTD_NAND_JZ4740
+ tristate "Support for JZ4740 SoC NAND controller"
+ depends on MACH_JZ4740 || COMPILE_TEST
+ depends on HAS_IOMEM
+ help
+ Enables support for NAND Flash on JZ4740 SoC based boards.
+
+config MTD_NAND_JZ4780
+ tristate "Support for NAND on JZ4780 SoC"
+ depends on JZ4780_NEMC
+ help
+ Enables support for NAND Flash connected to the NEMC on JZ4780 SoC
+ based boards, using the BCH controller for hardware error correction.
diff --git a/drivers/mtd/nand/raw/ingenic/Makefile b/drivers/mtd/nand/raw/ingenic/Makefile
new file mode 100644
index 000000000000..44c2ca053d24
--- /dev/null
+++ b/drivers/mtd/nand/raw/ingenic/Makefile
@@ -0,0 +1,2 @@
+obj-$(CONFIG_MTD_NAND_JZ4740) += jz4740_nand.o
+obj-$(CONFIG_MTD_NAND_JZ4780) += jz4780_nand.o jz4780_bch.o
diff --git a/drivers/mtd/nand/raw/jz4740_nand.c b/drivers/mtd/nand/raw/ingenic/jz4740_nand.c
similarity index 100%
rename from drivers/mtd/nand/raw/jz4740_nand.c
rename to drivers/mtd/nand/raw/ingenic/jz4740_nand.c
diff --git a/drivers/mtd/nand/raw/jz4780_bch.c b/drivers/mtd/nand/raw/ingenic/jz4780_bch.c
similarity index 100%
rename from drivers/mtd/nand/raw/jz4780_bch.c
rename to drivers/mtd/nand/raw/ingenic/jz4780_bch.c
diff --git a/drivers/mtd/nand/raw/jz4780_bch.h b/drivers/mtd/nand/raw/ingenic/jz4780_bch.h
similarity index 100%
rename from drivers/mtd/nand/raw/jz4780_bch.h
rename to drivers/mtd/nand/raw/ingenic/jz4780_bch.h
diff --git a/drivers/mtd/nand/raw/jz4780_nand.c b/drivers/mtd/nand/raw/ingenic/jz4780_nand.c
similarity index 100%
rename from drivers/mtd/nand/raw/jz4780_nand.c
rename to drivers/mtd/nand/raw/ingenic/jz4780_nand.c
--
2.11.0


2019-02-09 19:24:16

by Paul Cercueil

[permalink] [raw]
Subject: [PATCH v4 4/9] mtd: rawnand: ingenic: Use SPDX license notifiers

Use SPDX license notifiers instead of GPLv2 license text in the headers.

Signed-off-by: Paul Cercueil <[email protected]>
Reviewed-by: Boris Brezillon <[email protected]>
---

v2: No changes

v3: No changes

v4: No changes

drivers/mtd/nand/raw/ingenic/jz4780_bch.c | 5 +----
drivers/mtd/nand/raw/ingenic/jz4780_bch.h | 5 +----
drivers/mtd/nand/raw/ingenic/jz4780_nand.c | 5 +----
3 files changed, 3 insertions(+), 12 deletions(-)

diff --git a/drivers/mtd/nand/raw/ingenic/jz4780_bch.c b/drivers/mtd/nand/raw/ingenic/jz4780_bch.c
index 7201827809e9..7e4e5e627603 100644
--- a/drivers/mtd/nand/raw/ingenic/jz4780_bch.c
+++ b/drivers/mtd/nand/raw/ingenic/jz4780_bch.c
@@ -1,12 +1,9 @@
+// SPDX-License-Identifier: GPL-2.0
/*
* JZ4780 BCH controller
*
* Copyright (c) 2015 Imagination Technologies
* Author: Alex Smith <[email protected]>
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms of the GNU General Public License version 2 as published
- * by the Free Software Foundation.
*/

#include <linux/bitops.h>
diff --git a/drivers/mtd/nand/raw/ingenic/jz4780_bch.h b/drivers/mtd/nand/raw/ingenic/jz4780_bch.h
index bf4718088a3a..451e0c770160 100644
--- a/drivers/mtd/nand/raw/ingenic/jz4780_bch.h
+++ b/drivers/mtd/nand/raw/ingenic/jz4780_bch.h
@@ -1,12 +1,9 @@
+/* SPDX-License-Identifier: GPL-2.0 */
/*
* JZ4780 BCH controller
*
* Copyright (c) 2015 Imagination Technologies
* Author: Alex Smith <[email protected]>
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms of the GNU General Public License version 2 as published
- * by the Free Software Foundation.
*/

#ifndef __DRIVERS_MTD_NAND_JZ4780_BCH_H__
diff --git a/drivers/mtd/nand/raw/ingenic/jz4780_nand.c b/drivers/mtd/nand/raw/ingenic/jz4780_nand.c
index 22e58975f0d5..7f55358b860f 100644
--- a/drivers/mtd/nand/raw/ingenic/jz4780_nand.c
+++ b/drivers/mtd/nand/raw/ingenic/jz4780_nand.c
@@ -1,12 +1,9 @@
+// SPDX-License-Identifier: GPL-2.0
/*
* JZ4780 NAND driver
*
* Copyright (c) 2015 Imagination Technologies
* Author: Alex Smith <[email protected]>
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms of the GNU General Public License version 2 as published
- * by the Free Software Foundation.
*/

#include <linux/delay.h>
--
2.11.0


2019-02-09 19:24:23

by Paul Cercueil

[permalink] [raw]
Subject: [PATCH v4 5/9] mtd: rawnand: ingenic: Rename jz4780_nand driver to ingenic_nand

The jz4780_nand driver will be modified to handle all the Ingenic
JZ47xx SoCs that the upstream Linux kernel supports (JZ4740, JZ4725B,
JZ4770, JZ4780), so it makes sense to rename it.

Signed-off-by: Paul Cercueil <[email protected]>
---

v3: New patch

v4: No changes

drivers/mtd/nand/raw/ingenic/Makefile | 2 +-
.../raw/ingenic/{jz4780_nand.c => ingenic_nand.c} | 146 ++++++++++-----------
2 files changed, 74 insertions(+), 74 deletions(-)
rename drivers/mtd/nand/raw/ingenic/{jz4780_nand.c => ingenic_nand.c} (63%)

diff --git a/drivers/mtd/nand/raw/ingenic/Makefile b/drivers/mtd/nand/raw/ingenic/Makefile
index 44c2ca053d24..af2d5de21f60 100644
--- a/drivers/mtd/nand/raw/ingenic/Makefile
+++ b/drivers/mtd/nand/raw/ingenic/Makefile
@@ -1,2 +1,2 @@
obj-$(CONFIG_MTD_NAND_JZ4740) += jz4740_nand.o
-obj-$(CONFIG_MTD_NAND_JZ4780) += jz4780_nand.o jz4780_bch.o
+obj-$(CONFIG_MTD_NAND_JZ4780) += ingenic_nand.o jz4780_bch.o
diff --git a/drivers/mtd/nand/raw/ingenic/jz4780_nand.c b/drivers/mtd/nand/raw/ingenic/ingenic_nand.c
similarity index 63%
rename from drivers/mtd/nand/raw/ingenic/jz4780_nand.c
rename to drivers/mtd/nand/raw/ingenic/ingenic_nand.c
index 7f55358b860f..8c73f7c5be9a 100644
--- a/drivers/mtd/nand/raw/ingenic/jz4780_nand.c
+++ b/drivers/mtd/nand/raw/ingenic/ingenic_nand.c
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: GPL-2.0
/*
- * JZ4780 NAND driver
+ * Ingenic JZ47xx NAND driver
*
* Copyright (c) 2015 Imagination Technologies
* Author: Alex Smith <[email protected]>
@@ -24,7 +24,7 @@

#include "jz4780_bch.h"

-#define DRV_NAME "jz4780-nand"
+#define DRV_NAME "ingenic-nand"

#define OFFSET_DATA 0x00000000
#define OFFSET_CMD 0x00400000
@@ -33,22 +33,22 @@
/* Command delay when there is no R/B pin. */
#define RB_DELAY_US 100

-struct jz4780_nand_cs {
+struct ingenic_nand_cs {
unsigned int bank;
void __iomem *base;
};

-struct jz4780_nand_controller {
+struct ingenic_nfc {
struct device *dev;
struct jz4780_bch *bch;
struct nand_controller controller;
unsigned int num_banks;
struct list_head chips;
int selected;
- struct jz4780_nand_cs cs[];
+ struct ingenic_nand_cs cs[];
};

-struct jz4780_nand_chip {
+struct ingenic_nand {
struct nand_chip chip;
struct list_head chip_list;

@@ -57,22 +57,21 @@ struct jz4780_nand_chip {
unsigned int reading: 1;
};

-static inline struct jz4780_nand_chip *to_jz4780_nand_chip(struct mtd_info *mtd)
+static inline struct ingenic_nand *to_ingenic_nand(struct mtd_info *mtd)
{
- return container_of(mtd_to_nand(mtd), struct jz4780_nand_chip, chip);
+ return container_of(mtd_to_nand(mtd), struct ingenic_nand, chip);
}

-static inline struct jz4780_nand_controller
-*to_jz4780_nand_controller(struct nand_controller *ctrl)
+static inline struct ingenic_nfc *to_ingenic_nfc(struct nand_controller *ctrl)
{
- return container_of(ctrl, struct jz4780_nand_controller, controller);
+ return container_of(ctrl, struct ingenic_nfc, controller);
}

-static void jz4780_nand_select_chip(struct nand_chip *chip, int chipnr)
+static void ingenic_nand_select_chip(struct nand_chip *chip, int chipnr)
{
- struct jz4780_nand_chip *nand = to_jz4780_nand_chip(nand_to_mtd(chip));
- struct jz4780_nand_controller *nfc = to_jz4780_nand_controller(nand->chip.controller);
- struct jz4780_nand_cs *cs;
+ struct ingenic_nand *nand = to_ingenic_nand(nand_to_mtd(chip));
+ struct ingenic_nfc *nfc = to_ingenic_nfc(nand->chip.controller);
+ struct ingenic_nand_cs *cs;

/* Ensure the currently selected chip is deasserted. */
if (chipnr == -1 && nfc->selected >= 0) {
@@ -83,12 +82,12 @@ static void jz4780_nand_select_chip(struct nand_chip *chip, int chipnr)
nfc->selected = chipnr;
}

-static void jz4780_nand_cmd_ctrl(struct nand_chip *chip, int cmd,
- unsigned int ctrl)
+static void ingenic_nand_cmd_ctrl(struct nand_chip *chip, int cmd,
+ unsigned int ctrl)
{
- struct jz4780_nand_chip *nand = to_jz4780_nand_chip(nand_to_mtd(chip));
- struct jz4780_nand_controller *nfc = to_jz4780_nand_controller(nand->chip.controller);
- struct jz4780_nand_cs *cs;
+ struct ingenic_nand *nand = to_ingenic_nand(nand_to_mtd(chip));
+ struct ingenic_nfc *nfc = to_ingenic_nfc(nand->chip.controller);
+ struct ingenic_nand_cs *cs;

if (WARN_ON(nfc->selected < 0))
return;
@@ -106,25 +105,25 @@ static void jz4780_nand_cmd_ctrl(struct nand_chip *chip, int cmd,
writeb(cmd, cs->base + OFFSET_CMD);
}

-static int jz4780_nand_dev_ready(struct nand_chip *chip)
+static int ingenic_nand_dev_ready(struct nand_chip *chip)
{
- struct jz4780_nand_chip *nand = to_jz4780_nand_chip(nand_to_mtd(chip));
+ struct ingenic_nand *nand = to_ingenic_nand(nand_to_mtd(chip));

return !gpiod_get_value_cansleep(nand->busy_gpio);
}

-static void jz4780_nand_ecc_hwctl(struct nand_chip *chip, int mode)
+static void ingenic_nand_ecc_hwctl(struct nand_chip *chip, int mode)
{
- struct jz4780_nand_chip *nand = to_jz4780_nand_chip(nand_to_mtd(chip));
+ struct ingenic_nand *nand = to_ingenic_nand(nand_to_mtd(chip));

nand->reading = (mode == NAND_ECC_READ);
}

-static int jz4780_nand_ecc_calculate(struct nand_chip *chip, const u8 *dat,
- u8 *ecc_code)
+static int ingenic_nand_ecc_calculate(struct nand_chip *chip, const u8 *dat,
+ u8 *ecc_code)
{
- struct jz4780_nand_chip *nand = to_jz4780_nand_chip(nand_to_mtd(chip));
- struct jz4780_nand_controller *nfc = to_jz4780_nand_controller(nand->chip.controller);
+ struct ingenic_nand *nand = to_ingenic_nand(nand_to_mtd(chip));
+ struct ingenic_nfc *nfc = to_ingenic_nfc(nand->chip.controller);
struct jz4780_bch_params params;

/*
@@ -141,11 +140,11 @@ static int jz4780_nand_ecc_calculate(struct nand_chip *chip, const u8 *dat,
return jz4780_bch_calculate(nfc->bch, &params, dat, ecc_code);
}

-static int jz4780_nand_ecc_correct(struct nand_chip *chip, u8 *dat,
- u8 *read_ecc, u8 *calc_ecc)
+static int ingenic_nand_ecc_correct(struct nand_chip *chip, u8 *dat,
+ u8 *read_ecc, u8 *calc_ecc)
{
- struct jz4780_nand_chip *nand = to_jz4780_nand_chip(nand_to_mtd(chip));
- struct jz4780_nand_controller *nfc = to_jz4780_nand_controller(nand->chip.controller);
+ struct ingenic_nand *nand = to_ingenic_nand(nand_to_mtd(chip));
+ struct ingenic_nfc *nfc = to_ingenic_nfc(nand->chip.controller);
struct jz4780_bch_params params;

params.size = nand->chip.ecc.size;
@@ -155,10 +154,10 @@ static int jz4780_nand_ecc_correct(struct nand_chip *chip, u8 *dat,
return jz4780_bch_correct(nfc->bch, &params, dat, read_ecc);
}

-static int jz4780_nand_attach_chip(struct nand_chip *chip)
+static int ingenic_nand_attach_chip(struct nand_chip *chip)
{
struct mtd_info *mtd = nand_to_mtd(chip);
- struct jz4780_nand_controller *nfc = to_jz4780_nand_controller(chip->controller);
+ struct ingenic_nfc *nfc = to_ingenic_nfc(chip->controller);
int eccbytes;

chip->ecc.bytes = fls((1 + 8) * chip->ecc.size) *
@@ -167,14 +166,13 @@ static int jz4780_nand_attach_chip(struct nand_chip *chip)
switch (chip->ecc.mode) {
case NAND_ECC_HW:
if (!nfc->bch) {
- dev_err(nfc->dev,
- "HW BCH selected, but BCH controller not found\n");
+ dev_err(nfc->dev, "HW BCH selected, but BCH controller not found\n");
return -ENODEV;
}

- chip->ecc.hwctl = jz4780_nand_ecc_hwctl;
- chip->ecc.calculate = jz4780_nand_ecc_calculate;
- chip->ecc.correct = jz4780_nand_ecc_correct;
+ chip->ecc.hwctl = ingenic_nand_ecc_hwctl;
+ chip->ecc.calculate = ingenic_nand_ecc_calculate;
+ chip->ecc.correct = ingenic_nand_ecc_correct;
/* fall through */
case NAND_ECC_SOFT:
dev_info(nfc->dev, "using %s (strength %d, size %d, bytes %d)\n",
@@ -209,18 +207,18 @@ static int jz4780_nand_attach_chip(struct nand_chip *chip)
return 0;
}

-static const struct nand_controller_ops jz4780_nand_controller_ops = {
- .attach_chip = jz4780_nand_attach_chip,
+static const struct nand_controller_ops ingenic_nand_controller_ops = {
+ .attach_chip = ingenic_nand_attach_chip,
};

-static int jz4780_nand_init_chip(struct platform_device *pdev,
- struct jz4780_nand_controller *nfc,
- struct device_node *np,
- unsigned int chipnr)
+static int ingenic_nand_init_chip(struct platform_device *pdev,
+ struct ingenic_nfc *nfc,
+ struct device_node *np,
+ unsigned int chipnr)
{
struct device *dev = &pdev->dev;
- struct jz4780_nand_chip *nand;
- struct jz4780_nand_cs *cs;
+ struct ingenic_nand *nand;
+ struct ingenic_nand_cs *cs;
struct resource *res;
struct nand_chip *chip;
struct mtd_info *mtd;
@@ -253,7 +251,7 @@ static int jz4780_nand_init_chip(struct platform_device *pdev,
dev_err(dev, "failed to request busy GPIO: %d\n", ret);
return ret;
} else if (nand->busy_gpio) {
- nand->chip.legacy.dev_ready = jz4780_nand_dev_ready;
+ nand->chip.legacy.dev_ready = ingenic_nand_dev_ready;
}

nand->wp_gpio = devm_gpiod_get_optional(dev, "wp", GPIOD_OUT_LOW);
@@ -276,13 +274,13 @@ static int jz4780_nand_init_chip(struct platform_device *pdev,
chip->legacy.IO_ADDR_W = cs->base + OFFSET_DATA;
chip->legacy.chip_delay = RB_DELAY_US;
chip->options = NAND_NO_SUBPAGE_WRITE;
- chip->legacy.select_chip = jz4780_nand_select_chip;
- chip->legacy.cmd_ctrl = jz4780_nand_cmd_ctrl;
+ chip->legacy.select_chip = ingenic_nand_select_chip;
+ chip->legacy.cmd_ctrl = ingenic_nand_cmd_ctrl;
chip->ecc.mode = NAND_ECC_HW;
chip->controller = &nfc->controller;
nand_set_flash_node(chip, np);

- chip->controller->ops = &jz4780_nand_controller_ops;
+ chip->controller->ops = &ingenic_nand_controller_ops;
ret = nand_scan(chip, 1);
if (ret)
return ret;
@@ -298,19 +296,20 @@ static int jz4780_nand_init_chip(struct platform_device *pdev,
return 0;
}

-static void jz4780_nand_cleanup_chips(struct jz4780_nand_controller *nfc)
+static void ingenic_nand_cleanup_chips(struct ingenic_nfc *nfc)
{
- struct jz4780_nand_chip *chip;
+ struct ingenic_nand *chip;

while (!list_empty(&nfc->chips)) {
- chip = list_first_entry(&nfc->chips, struct jz4780_nand_chip, chip_list);
+ chip = list_first_entry(&nfc->chips,
+ struct ingenic_nand, chip_list);
nand_release(&chip->chip);
list_del(&chip->chip_list);
}
}

-static int jz4780_nand_init_chips(struct jz4780_nand_controller *nfc,
- struct platform_device *pdev)
+static int ingenic_nand_init_chips(struct ingenic_nfc *nfc,
+ struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
struct device_node *np;
@@ -319,14 +318,15 @@ static int jz4780_nand_init_chips(struct jz4780_nand_controller *nfc,
int num_chips = of_get_child_count(dev->of_node);

if (num_chips > nfc->num_banks) {
- dev_err(dev, "found %d chips but only %d banks\n", num_chips, nfc->num_banks);
+ dev_err(dev, "found %d chips but only %d banks\n",
+ num_chips, nfc->num_banks);
return -EINVAL;
}

for_each_child_of_node(dev->of_node, np) {
- ret = jz4780_nand_init_chip(pdev, nfc, np, i);
+ ret = ingenic_nand_init_chip(pdev, nfc, np, i);
if (ret) {
- jz4780_nand_cleanup_chips(nfc);
+ ingenic_nand_cleanup_chips(nfc);
return ret;
}

@@ -336,11 +336,11 @@ static int jz4780_nand_init_chips(struct jz4780_nand_controller *nfc,
return 0;
}

-static int jz4780_nand_probe(struct platform_device *pdev)
+static int ingenic_nand_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
unsigned int num_banks;
- struct jz4780_nand_controller *nfc;
+ struct ingenic_nfc *nfc;
int ret;

num_banks = jz4780_nemc_num_banks(dev);
@@ -367,7 +367,7 @@ static int jz4780_nand_probe(struct platform_device *pdev)
nand_controller_init(&nfc->controller);
INIT_LIST_HEAD(&nfc->chips);

- ret = jz4780_nand_init_chips(nfc, pdev);
+ ret = ingenic_nand_init_chips(nfc, pdev);
if (ret) {
if (nfc->bch)
jz4780_bch_release(nfc->bch);
@@ -378,35 +378,35 @@ static int jz4780_nand_probe(struct platform_device *pdev)
return 0;
}

-static int jz4780_nand_remove(struct platform_device *pdev)
+static int ingenic_nand_remove(struct platform_device *pdev)
{
- struct jz4780_nand_controller *nfc = platform_get_drvdata(pdev);
+ struct ingenic_nfc *nfc = platform_get_drvdata(pdev);

if (nfc->bch)
jz4780_bch_release(nfc->bch);

- jz4780_nand_cleanup_chips(nfc);
+ ingenic_nand_cleanup_chips(nfc);

return 0;
}

-static const struct of_device_id jz4780_nand_dt_match[] = {
+static const struct of_device_id ingenic_nand_dt_match[] = {
{ .compatible = "ingenic,jz4780-nand" },
{},
};
-MODULE_DEVICE_TABLE(of, jz4780_nand_dt_match);
+MODULE_DEVICE_TABLE(of, ingenic_nand_dt_match);

-static struct platform_driver jz4780_nand_driver = {
- .probe = jz4780_nand_probe,
- .remove = jz4780_nand_remove,
+static struct platform_driver ingenic_nand_driver = {
+ .probe = ingenic_nand_probe,
+ .remove = ingenic_nand_remove,
.driver = {
.name = DRV_NAME,
- .of_match_table = of_match_ptr(jz4780_nand_dt_match),
+ .of_match_table = of_match_ptr(ingenic_nand_dt_match),
},
};
-module_platform_driver(jz4780_nand_driver);
+module_platform_driver(ingenic_nand_driver);

MODULE_AUTHOR("Alex Smith <[email protected]>");
MODULE_AUTHOR("Harvey Hunt <[email protected]>");
-MODULE_DESCRIPTION("Ingenic JZ4780 NAND driver");
+MODULE_DESCRIPTION("Ingenic JZ47xx NAND driver");
MODULE_LICENSE("GPL v2");
--
2.11.0


2019-02-09 19:25:00

by Paul Cercueil

[permalink] [raw]
Subject: [PATCH v4 7/9] mtd: rawnand: ingenic: Add support for the JZ4740

Add support for probing the ingenic-nand driver on the JZ4740 SoC from
Ingenic, and the jz4740-ecc driver to support the JZ4740-specific
ECC hardware.

Signed-off-by: Paul Cercueil <[email protected]>
---

Changes:

v2: New patch

v3: Also add support for the hardware ECC of the JZ4740 in this patch

v4: - Fix formatting issues
- Add MODULE_* macros

drivers/mtd/nand/raw/ingenic/Kconfig | 10 ++
drivers/mtd/nand/raw/ingenic/Makefile | 1 +
drivers/mtd/nand/raw/ingenic/ingenic_nand.c | 48 +++++--
drivers/mtd/nand/raw/ingenic/jz4740_ecc.c | 196 ++++++++++++++++++++++++++++
4 files changed, 244 insertions(+), 11 deletions(-)
create mode 100644 drivers/mtd/nand/raw/ingenic/jz4740_ecc.c

diff --git a/drivers/mtd/nand/raw/ingenic/Kconfig b/drivers/mtd/nand/raw/ingenic/Kconfig
index 4bf7d7af3b0a..cc663cc15119 100644
--- a/drivers/mtd/nand/raw/ingenic/Kconfig
+++ b/drivers/mtd/nand/raw/ingenic/Kconfig
@@ -17,6 +17,16 @@ if MTD_NAND_JZ4780
config MTD_NAND_INGENIC_ECC
tristate

+config MTD_NAND_JZ4740_ECC
+ tristate "Hardware BCH support for JZ4740 SoC"
+ select MTD_NAND_INGENIC_ECC
+ help
+ Enable this driver to support the Reed-Solomon error-correction
+ hardware present on the JZ4740 SoC from Ingenic.
+
+ This driver can also be built as a module. If so, the module
+ will be called jz4740-ecc.
+
config MTD_NAND_JZ4780_BCH
tristate "Hardware BCH support for JZ4780 SoC"
select MTD_NAND_INGENIC_ECC
diff --git a/drivers/mtd/nand/raw/ingenic/Makefile b/drivers/mtd/nand/raw/ingenic/Makefile
index f3c3c0f230b0..563b7effcf59 100644
--- a/drivers/mtd/nand/raw/ingenic/Makefile
+++ b/drivers/mtd/nand/raw/ingenic/Makefile
@@ -2,4 +2,5 @@ obj-$(CONFIG_MTD_NAND_JZ4740) += jz4740_nand.o
obj-$(CONFIG_MTD_NAND_JZ4780) += ingenic_nand.o

obj-$(CONFIG_MTD_NAND_INGENIC_ECC) += ingenic_ecc.o
+obj-$(CONFIG_MTD_NAND_JZ4740_ECC) += jz4740_ecc.o
obj-$(CONFIG_MTD_NAND_JZ4780_BCH) += jz4780_bch.o
diff --git a/drivers/mtd/nand/raw/ingenic/ingenic_nand.c b/drivers/mtd/nand/raw/ingenic/ingenic_nand.c
index 0f51fd15fe79..036938fdad51 100644
--- a/drivers/mtd/nand/raw/ingenic/ingenic_nand.c
+++ b/drivers/mtd/nand/raw/ingenic/ingenic_nand.c
@@ -13,6 +13,7 @@
#include <linux/module.h>
#include <linux/of.h>
#include <linux/of_address.h>
+#include <linux/of_device.h>
#include <linux/gpio/consumer.h>
#include <linux/platform_device.h>
#include <linux/slab.h>
@@ -26,13 +27,15 @@

#define DRV_NAME "ingenic-nand"

-#define OFFSET_DATA 0x00000000
-#define OFFSET_CMD 0x00400000
-#define OFFSET_ADDR 0x00800000
-
/* Command delay when there is no R/B pin. */
#define RB_DELAY_US 100

+struct jz_soc_info {
+ unsigned long data_offset;
+ unsigned long addr_offset;
+ unsigned long cmd_offset;
+};
+
struct ingenic_nand_cs {
unsigned int bank;
void __iomem *base;
@@ -41,6 +44,7 @@ struct ingenic_nand_cs {
struct ingenic_nfc {
struct device *dev;
struct ingenic_ecc *ecc;
+ const struct jz_soc_info *soc_info;
struct nand_controller controller;
unsigned int num_banks;
struct list_head chips;
@@ -100,9 +104,9 @@ static void ingenic_nand_cmd_ctrl(struct nand_chip *chip, int cmd,
return;

if (ctrl & NAND_ALE)
- writeb(cmd, cs->base + OFFSET_ADDR);
+ writeb(cmd, cs->base + nfc->soc_info->addr_offset);
else if (ctrl & NAND_CLE)
- writeb(cmd, cs->base + OFFSET_CMD);
+ writeb(cmd, cs->base + nfc->soc_info->cmd_offset);
}

static int ingenic_nand_dev_ready(struct nand_chip *chip)
@@ -160,8 +164,13 @@ static int ingenic_nand_attach_chip(struct nand_chip *chip)
struct ingenic_nfc *nfc = to_ingenic_nfc(chip->controller);
int eccbytes;

- chip->ecc.bytes = fls((1 + 8) * chip->ecc.size) *
- (chip->ecc.strength / 8);
+ if (chip->ecc.strength == 4) {
+ /* JZ4740 uses 9 bytes of ECC to correct maximum 4 errors */
+ chip->ecc.bytes = 9;
+ } else {
+ chip->ecc.bytes = fls((1 + 8) * chip->ecc.size) *
+ (chip->ecc.strength / 8);
+ }

switch (chip->ecc.mode) {
case NAND_ECC_HW:
@@ -270,8 +279,8 @@ static int ingenic_nand_init_chip(struct platform_device *pdev,
return -ENOMEM;
mtd->dev.parent = dev;

- chip->legacy.IO_ADDR_R = cs->base + OFFSET_DATA;
- chip->legacy.IO_ADDR_W = cs->base + OFFSET_DATA;
+ chip->legacy.IO_ADDR_R = cs->base + nfc->soc_info->data_offset;
+ chip->legacy.IO_ADDR_W = cs->base + nfc->soc_info->data_offset;
chip->legacy.chip_delay = RB_DELAY_US;
chip->options = NAND_NO_SUBPAGE_WRITE;
chip->legacy.select_chip = ingenic_nand_select_chip;
@@ -353,6 +362,10 @@ static int ingenic_nand_probe(struct platform_device *pdev)
if (!nfc)
return -ENOMEM;

+ nfc->soc_info = device_get_match_data(dev);
+ if (!nfc->soc_info)
+ return -EINVAL;
+
/*
* Check for ECC HW before we call nand_scan_ident, to prevent us from
* having to call it again if the ECC driver returns -EPROBE_DEFER.
@@ -390,8 +403,21 @@ static int ingenic_nand_remove(struct platform_device *pdev)
return 0;
}

+static const struct jz_soc_info jz4740_soc_info = {
+ .data_offset = 0x00000000,
+ .cmd_offset = 0x00008000,
+ .addr_offset = 0x00010000,
+};
+
+static const struct jz_soc_info jz4780_soc_info = {
+ .data_offset = 0x00000000,
+ .cmd_offset = 0x00400000,
+ .addr_offset = 0x00800000,
+};
+
static const struct of_device_id ingenic_nand_dt_match[] = {
- { .compatible = "ingenic,jz4780-nand" },
+ { .compatible = "ingenic,jz4740-nand", .data = &jz4740_soc_info },
+ { .compatible = "ingenic,jz4780-nand", .data = &jz4780_soc_info },
{},
};
MODULE_DEVICE_TABLE(of, ingenic_nand_dt_match);
diff --git a/drivers/mtd/nand/raw/ingenic/jz4740_ecc.c b/drivers/mtd/nand/raw/ingenic/jz4740_ecc.c
new file mode 100644
index 000000000000..83b42881720e
--- /dev/null
+++ b/drivers/mtd/nand/raw/ingenic/jz4740_ecc.c
@@ -0,0 +1,196 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * JZ4740 ECC controller driver
+ *
+ * Copyright (c) 2019 Paul Cercueil <[email protected]>
+ *
+ * based on jz4740-nand.c
+ */
+
+#include <linux/bitops.h>
+#include <linux/device.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+
+#include "ingenic_ecc.h"
+
+#define JZ_REG_NAND_ECC_CTRL 0x00
+#define JZ_REG_NAND_DATA 0x04
+#define JZ_REG_NAND_PAR0 0x08
+#define JZ_REG_NAND_PAR1 0x0C
+#define JZ_REG_NAND_PAR2 0x10
+#define JZ_REG_NAND_IRQ_STAT 0x14
+#define JZ_REG_NAND_IRQ_CTRL 0x18
+#define JZ_REG_NAND_ERR(x) (0x1C + ((x) << 2))
+
+#define JZ_NAND_ECC_CTRL_PAR_READY BIT(4)
+#define JZ_NAND_ECC_CTRL_ENCODING BIT(3)
+#define JZ_NAND_ECC_CTRL_RS BIT(2)
+#define JZ_NAND_ECC_CTRL_RESET BIT(1)
+#define JZ_NAND_ECC_CTRL_ENABLE BIT(0)
+
+#define JZ_NAND_STATUS_ERR_COUNT (BIT(31) | BIT(30) | BIT(29))
+#define JZ_NAND_STATUS_PAD_FINISH BIT(4)
+#define JZ_NAND_STATUS_DEC_FINISH BIT(3)
+#define JZ_NAND_STATUS_ENC_FINISH BIT(2)
+#define JZ_NAND_STATUS_UNCOR_ERROR BIT(1)
+#define JZ_NAND_STATUS_ERROR BIT(0)
+
+static const uint8_t empty_block_ecc[] = {
+ 0xcd, 0x9d, 0x90, 0x58, 0xf4, 0x8b, 0xff, 0xb7, 0x6f
+};
+
+static void jz4740_ecc_init(struct ingenic_ecc *ecc, bool encode)
+{
+ uint32_t reg;
+
+ /* Clear interrupt status */
+ writel(0, ecc->base + JZ_REG_NAND_IRQ_STAT);
+
+ /* Initialize and enable ECC hardware */
+ reg = readl(ecc->base + JZ_REG_NAND_ECC_CTRL);
+ reg |= JZ_NAND_ECC_CTRL_RESET;
+ reg |= JZ_NAND_ECC_CTRL_ENABLE;
+ reg |= JZ_NAND_ECC_CTRL_RS;
+ if (encode)
+ reg |= JZ_NAND_ECC_CTRL_ENCODING;
+ else
+ reg &= ~JZ_NAND_ECC_CTRL_ENCODING;
+
+ writel(reg, ecc->base + JZ_REG_NAND_ECC_CTRL);
+}
+
+static int jz4740_ecc_calculate(struct ingenic_ecc *ecc,
+ struct ingenic_ecc_params *params,
+ const u8 *buf, u8 *ecc_code)
+{
+ uint32_t reg, status;
+ unsigned int timeout = 1000;
+ int i;
+
+ jz4740_ecc_init(ecc, true);
+
+ do {
+ status = readl(ecc->base + JZ_REG_NAND_IRQ_STAT);
+ } while (!(status & JZ_NAND_STATUS_ENC_FINISH) && --timeout);
+
+ if (timeout == 0)
+ return -ETIMEDOUT;
+
+ reg = readl(ecc->base + JZ_REG_NAND_ECC_CTRL);
+ reg &= ~JZ_NAND_ECC_CTRL_ENABLE;
+ writel(reg, ecc->base + JZ_REG_NAND_ECC_CTRL);
+
+ for (i = 0; i < params->bytes; ++i)
+ ecc_code[i] = readb(ecc->base + JZ_REG_NAND_PAR0 + i);
+
+ /* If the written data is completely 0xff, we also want to write 0xff as
+ * ecc, otherwise we will get in trouble when doing subpage writes.
+ */
+ if (memcmp(ecc_code, empty_block_ecc, ARRAY_SIZE(empty_block_ecc)) == 0)
+ memset(ecc_code, 0xff, ARRAY_SIZE(empty_block_ecc));
+
+ return 0;
+}
+
+static void jz_nand_correct_data(uint8_t *buf, int index, int mask)
+{
+ int offset = index & 0x7;
+ uint16_t data;
+
+ index += (index >> 3);
+
+ data = buf[index];
+ data |= buf[index + 1] << 8;
+
+ mask ^= (data >> offset) & 0x1ff;
+ data &= ~(0x1ff << offset);
+ data |= (mask << offset);
+
+ buf[index] = data & 0xff;
+ buf[index + 1] = (data >> 8) & 0xff;
+}
+
+static int jz4740_ecc_correct(struct ingenic_ecc *ecc,
+ struct ingenic_ecc_params *params,
+ u8 *buf, u8 *ecc_code)
+{
+ int i, error_count, index;
+ uint32_t reg, status, error;
+ unsigned int timeout = 1000;
+
+ jz4740_ecc_init(ecc, false);
+
+ for (i = 0; i < params->bytes; ++i)
+ writeb(ecc_code[i], ecc->base + JZ_REG_NAND_PAR0 + i);
+
+ reg = readl(ecc->base + JZ_REG_NAND_ECC_CTRL);
+ reg |= JZ_NAND_ECC_CTRL_PAR_READY;
+ writel(reg, ecc->base + JZ_REG_NAND_ECC_CTRL);
+
+ do {
+ status = readl(ecc->base + JZ_REG_NAND_IRQ_STAT);
+ } while (!(status & JZ_NAND_STATUS_DEC_FINISH) && --timeout);
+
+ if (timeout == 0)
+ return -ETIMEDOUT;
+
+ reg = readl(ecc->base + JZ_REG_NAND_ECC_CTRL);
+ reg &= ~JZ_NAND_ECC_CTRL_ENABLE;
+ writel(reg, ecc->base + JZ_REG_NAND_ECC_CTRL);
+
+ if (status & JZ_NAND_STATUS_ERROR) {
+ if (status & JZ_NAND_STATUS_UNCOR_ERROR)
+ return -EBADMSG;
+
+ error_count = (status & JZ_NAND_STATUS_ERR_COUNT) >> 29;
+
+ for (i = 0; i < error_count; ++i) {
+ error = readl(ecc->base + JZ_REG_NAND_ERR(i));
+ index = ((error >> 16) & 0x1ff) - 1;
+ if (index >= 0 && index < params->size)
+ jz_nand_correct_data(buf, index, error & 0x1ff);
+ }
+
+ return error_count;
+ }
+
+ return 0;
+}
+
+static void jz4740_ecc_disable(struct ingenic_ecc *ecc)
+{
+ u32 reg;
+
+ writel(0, ecc->base + JZ_REG_NAND_IRQ_STAT);
+ reg = readl(ecc->base + JZ_REG_NAND_ECC_CTRL);
+ reg &= ~JZ_NAND_ECC_CTRL_ENABLE;
+ writel(reg, ecc->base + JZ_REG_NAND_ECC_CTRL);
+}
+
+static const struct ingenic_ecc_ops jz4740_ecc_ops = {
+ .disable = jz4740_ecc_disable,
+ .calculate = jz4740_ecc_calculate,
+ .correct = jz4740_ecc_correct,
+};
+
+static const struct of_device_id jz4740_ecc_dt_match[] = {
+ { .compatible = "ingenic,jz4740-ecc", .data = &jz4740_ecc_ops },
+ {},
+};
+MODULE_DEVICE_TABLE(of, jz4740_ecc_dt_match);
+
+static struct platform_driver jz4740_ecc_driver = {
+ .probe = ingenic_ecc_probe,
+ .driver = {
+ .name = "jz4740-ecc",
+ .of_match_table = jz4740_ecc_dt_match,
+ },
+};
+module_platform_driver(jz4740_ecc_driver);
+
+MODULE_AUTHOR("Paul Cercueil <[email protected]>");
+MODULE_DESCRIPTION("Ingenic JZ4740 ECC controller driver");
+MODULE_LICENSE("GPL v2");
--
2.11.0


2019-02-09 19:25:08

by Paul Cercueil

[permalink] [raw]
Subject: [PATCH v4 6/9] mtd: rawnand: ingenic: Separate top-level and SoC specific code

The ingenic-nand driver uses an API provided by the jz4780-bch driver.
This makes it difficult to support other SoCs in the jz4780-bch driver.
To work around this, we separate the API functions from the SoC-specific
code, so that these API functions are SoC-agnostic.

Signed-off-by: Paul Cercueil <[email protected]>
---

v2: Add an optional .probe() callback. It is used for instance to set
the clock rate in the JZ4780 backend.

v3: The common code is now inside the ingenic-ecc module. Each
SoC-specific ECC code is now in its own module, which leaves to the
user the choice of which (if any) ECC code should be supported.

v4: No change

drivers/mtd/nand/raw/ingenic/Kconfig | 17 +++
drivers/mtd/nand/raw/ingenic/Makefile | 5 +-
drivers/mtd/nand/raw/ingenic/ingenic_ecc.c | 157 +++++++++++++++++++++++++
drivers/mtd/nand/raw/ingenic/ingenic_ecc.h | 84 ++++++++++++++
drivers/mtd/nand/raw/ingenic/ingenic_nand.c | 38 +++----
drivers/mtd/nand/raw/ingenic/jz4780_bch.c | 170 +++++-----------------------
drivers/mtd/nand/raw/ingenic/jz4780_bch.h | 40 -------
7 files changed, 308 insertions(+), 203 deletions(-)
create mode 100644 drivers/mtd/nand/raw/ingenic/ingenic_ecc.c
create mode 100644 drivers/mtd/nand/raw/ingenic/ingenic_ecc.h
delete mode 100644 drivers/mtd/nand/raw/ingenic/jz4780_bch.h

diff --git a/drivers/mtd/nand/raw/ingenic/Kconfig b/drivers/mtd/nand/raw/ingenic/Kconfig
index 67806c87b2c4..4bf7d7af3b0a 100644
--- a/drivers/mtd/nand/raw/ingenic/Kconfig
+++ b/drivers/mtd/nand/raw/ingenic/Kconfig
@@ -11,3 +11,20 @@ config MTD_NAND_JZ4780
help
Enables support for NAND Flash connected to the NEMC on JZ4780 SoC
based boards, using the BCH controller for hardware error correction.
+
+if MTD_NAND_JZ4780
+
+config MTD_NAND_INGENIC_ECC
+ tristate
+
+config MTD_NAND_JZ4780_BCH
+ tristate "Hardware BCH support for JZ4780 SoC"
+ select MTD_NAND_INGENIC_ECC
+ help
+ Enable this driver to support the BCH error-correction hardware
+ present on the JZ4780 SoC from Ingenic.
+
+ This driver can also be built as a module. If so, the module
+ will be called jz4780-bch.
+
+endif # MTD_NAND_JZ4780
diff --git a/drivers/mtd/nand/raw/ingenic/Makefile b/drivers/mtd/nand/raw/ingenic/Makefile
index af2d5de21f60..f3c3c0f230b0 100644
--- a/drivers/mtd/nand/raw/ingenic/Makefile
+++ b/drivers/mtd/nand/raw/ingenic/Makefile
@@ -1,2 +1,5 @@
obj-$(CONFIG_MTD_NAND_JZ4740) += jz4740_nand.o
-obj-$(CONFIG_MTD_NAND_JZ4780) += ingenic_nand.o jz4780_bch.o
+obj-$(CONFIG_MTD_NAND_JZ4780) += ingenic_nand.o
+
+obj-$(CONFIG_MTD_NAND_INGENIC_ECC) += ingenic_ecc.o
+obj-$(CONFIG_MTD_NAND_JZ4780_BCH) += jz4780_bch.o
diff --git a/drivers/mtd/nand/raw/ingenic/ingenic_ecc.c b/drivers/mtd/nand/raw/ingenic/ingenic_ecc.c
new file mode 100644
index 000000000000..a5b2f31803ff
--- /dev/null
+++ b/drivers/mtd/nand/raw/ingenic/ingenic_ecc.c
@@ -0,0 +1,157 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * JZ47xx ECC common code
+ *
+ * Copyright (c) 2015 Imagination Technologies
+ * Author: Alex Smith <[email protected]>
+ */
+
+#include <linux/clk.h>
+#include <linux/init.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+
+#include "ingenic_ecc.h"
+
+/**
+ * ingenic_ecc_calculate() - calculate ECC for a data buffer
+ * @ecc: ECC device.
+ * @params: ECC parameters.
+ * @buf: input buffer with raw data.
+ * @ecc_code: output buffer with ECC.
+ *
+ * Return: 0 on success, -ETIMEDOUT if timed out while waiting for ECC
+ * controller.
+ */
+int ingenic_ecc_calculate(struct ingenic_ecc *ecc,
+ struct ingenic_ecc_params *params,
+ const u8 *buf, u8 *ecc_code)
+{
+ return ecc->ops->calculate(ecc, params, buf, ecc_code);
+}
+EXPORT_SYMBOL(ingenic_ecc_calculate);
+
+/**
+ * ingenic_ecc_correct() - detect and correct bit errors
+ * @ecc: ECC device.
+ * @params: ECC parameters.
+ * @buf: raw data read from the chip.
+ * @ecc_code: ECC read from the chip.
+ *
+ * Given the raw data and the ECC read from the NAND device, detects and
+ * corrects errors in the data.
+ *
+ * Return: the number of bit errors corrected, -EBADMSG if there are too many
+ * errors to correct or -ETIMEDOUT if we timed out waiting for the controller.
+ */
+int ingenic_ecc_correct(struct ingenic_ecc *ecc,
+ struct ingenic_ecc_params *params,
+ u8 *buf, u8 *ecc_code)
+{
+ return ecc->ops->correct(ecc, params, buf, ecc_code);
+}
+EXPORT_SYMBOL(ingenic_ecc_correct);
+
+/**
+ * ingenic_ecc_get() - get the ECC controller device
+ * @np: ECC device tree node.
+ *
+ * Gets the ECC controller device from the specified device tree node. The
+ * device must be released with ingenic_ecc_release() when it is no longer being
+ * used.
+ *
+ * Return: a pointer to ingenic_ecc, errors are encoded into the pointer.
+ * PTR_ERR(-EPROBE_DEFER) if the device hasn't been initialised yet.
+ */
+static struct ingenic_ecc *ingenic_ecc_get(struct device_node *np)
+{
+ struct platform_device *pdev;
+ struct ingenic_ecc *ecc;
+
+ pdev = of_find_device_by_node(np);
+ if (!pdev || !platform_get_drvdata(pdev))
+ return ERR_PTR(-EPROBE_DEFER);
+
+ get_device(&pdev->dev);
+
+ ecc = platform_get_drvdata(pdev);
+ clk_prepare_enable(ecc->clk);
+
+ return ecc;
+}
+
+/**
+ * of_ingenic_ecc_get() - get the ECC controller from a DT node
+ * @of_node: the node that contains a bch-controller property.
+ *
+ * Get the bch-controller property from the given device tree
+ * node and pass it to ingenic_ecc_get to do the work.
+ *
+ * Return: a pointer to ingenic_ecc, errors are encoded into the pointer.
+ * PTR_ERR(-EPROBE_DEFER) if the device hasn't been initialised yet.
+ */
+struct ingenic_ecc *of_ingenic_ecc_get(struct device_node *of_node)
+{
+ struct ingenic_ecc *ecc = NULL;
+ struct device_node *np;
+
+ np = of_parse_phandle(of_node, "ingenic,bch-controller", 0);
+
+ if (np) {
+ ecc = ingenic_ecc_get(np);
+ of_node_put(np);
+ }
+ return ecc;
+}
+EXPORT_SYMBOL(of_ingenic_ecc_get);
+
+/**
+ * ingenic_ecc_release() - release the ECC controller device
+ * @ecc: ECC device.
+ */
+void ingenic_ecc_release(struct ingenic_ecc *ecc)
+{
+ clk_disable_unprepare(ecc->clk);
+ put_device(ecc->dev);
+}
+EXPORT_SYMBOL(ingenic_ecc_release);
+
+int ingenic_ecc_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct ingenic_ecc *ecc;
+ struct resource *res;
+ int ret = 0;
+
+ ecc = devm_kzalloc(dev, sizeof(*ecc), GFP_KERNEL);
+ if (!ecc)
+ return -ENOMEM;
+
+ ecc->ops = device_get_match_data(dev);
+ if (!ecc->ops)
+ return -EINVAL;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ ecc->base = devm_ioremap_resource(dev, res);
+ if (IS_ERR(ecc->base))
+ return PTR_ERR(ecc->base);
+
+ ecc->ops->disable(ecc);
+
+ ecc->clk = devm_clk_get(dev, NULL);
+ if (IS_ERR(ecc->clk)) {
+ dev_err(dev, "failed to get clock: %ld\n", PTR_ERR(ecc->clk));
+ return PTR_ERR(ecc->clk);
+ }
+
+ mutex_init(&ecc->lock);
+
+ ecc->dev = dev;
+ platform_set_drvdata(pdev, ecc);
+
+ if (ecc->ops->probe)
+ ret = ecc->ops->probe(ecc);
+
+ return ret;
+}
+EXPORT_SYMBOL(ingenic_ecc_probe);
diff --git a/drivers/mtd/nand/raw/ingenic/ingenic_ecc.h b/drivers/mtd/nand/raw/ingenic/ingenic_ecc.h
new file mode 100644
index 000000000000..6dfb19f54c9b
--- /dev/null
+++ b/drivers/mtd/nand/raw/ingenic/ingenic_ecc.h
@@ -0,0 +1,84 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __DRIVERS_MTD_NAND_INGENIC_ECC_INTERNAL_H__
+#define __DRIVERS_MTD_NAND_INGENIC_ECC_INTERNAL_H__
+
+#include <linux/compiler_types.h>
+#include <linux/err.h>
+#include <linux/mutex.h>
+#include <linux/types.h>
+#include <uapi/asm-generic/errno-base.h>
+
+struct clk;
+struct device;
+struct ingenic_ecc;
+struct platform_device;
+
+/**
+ * struct ingenic_ecc_params - ECC parameters
+ * @size: data bytes per ECC step.
+ * @bytes: ECC bytes per step.
+ * @strength: number of correctable bits per ECC step.
+ */
+struct ingenic_ecc_params {
+ int size;
+ int bytes;
+ int strength;
+};
+
+#ifdef CONFIG_MTD_NAND_INGENIC_ECC
+int ingenic_ecc_calculate(struct ingenic_ecc *ecc,
+ struct ingenic_ecc_params *params,
+ const u8 *buf, u8 *ecc_code);
+int ingenic_ecc_correct(struct ingenic_ecc *ecc,
+ struct ingenic_ecc_params *params, u8 *buf,
+ u8 *ecc_code);
+
+void ingenic_ecc_release(struct ingenic_ecc *ecc);
+struct ingenic_ecc *of_ingenic_ecc_get(struct device_node *np);
+#else /* CONFIG_MTD_NAND_INGENIC_ECC */
+int ingenic_ecc_calculate(struct ingenic_ecc *ecc,
+ struct ingenic_ecc_params *params,
+ const u8 *buf, u8 *ecc_code)
+{
+ return -ENODEV;
+}
+
+int ingenic_ecc_correct(struct ingenic_ecc *ecc,
+ struct ingenic_ecc_params *params, u8 *buf,
+ u8 *ecc_code)
+{
+ return -ENODEV;
+}
+
+void ingenic_ecc_release(struct ingenic_ecc *ecc)
+{
+}
+
+struct ingenic_ecc *of_ingenic_ecc_get(struct device_node *np)
+{
+ return ERR_PTR(-ENODEV);
+}
+#endif /* CONFIG_MTD_NAND_INGENIC_ECC */
+
+struct ingenic_ecc_ops {
+ int (*probe)(struct ingenic_ecc *ecc);
+ void (*disable)(struct ingenic_ecc *ecc);
+ int (*calculate)(struct ingenic_ecc *ecc,
+ struct ingenic_ecc_params *params,
+ const u8 *buf, u8 *ecc_code);
+ int (*correct)(struct ingenic_ecc *ecc,
+ struct ingenic_ecc_params *params,
+ u8 *buf, u8 *ecc_code);
+};
+
+struct ingenic_ecc {
+ struct device *dev;
+ const struct ingenic_ecc_ops *ops;
+ void __iomem *base;
+ struct clk *clk;
+ struct mutex lock;
+};
+
+int ingenic_ecc_probe(struct platform_device *pdev);
+
+#endif /* __DRIVERS_MTD_NAND_INGENIC_ECC_INTERNAL_H__ */
diff --git a/drivers/mtd/nand/raw/ingenic/ingenic_nand.c b/drivers/mtd/nand/raw/ingenic/ingenic_nand.c
index 8c73f7c5be9a..0f51fd15fe79 100644
--- a/drivers/mtd/nand/raw/ingenic/ingenic_nand.c
+++ b/drivers/mtd/nand/raw/ingenic/ingenic_nand.c
@@ -22,7 +22,7 @@

#include <linux/jz4780-nemc.h>

-#include "jz4780_bch.h"
+#include "ingenic_ecc.h"

#define DRV_NAME "ingenic-nand"

@@ -40,7 +40,7 @@ struct ingenic_nand_cs {

struct ingenic_nfc {
struct device *dev;
- struct jz4780_bch *bch;
+ struct ingenic_ecc *ecc;
struct nand_controller controller;
unsigned int num_banks;
struct list_head chips;
@@ -124,10 +124,10 @@ static int ingenic_nand_ecc_calculate(struct nand_chip *chip, const u8 *dat,
{
struct ingenic_nand *nand = to_ingenic_nand(nand_to_mtd(chip));
struct ingenic_nfc *nfc = to_ingenic_nfc(nand->chip.controller);
- struct jz4780_bch_params params;
+ struct ingenic_ecc_params params;

/*
- * Don't need to generate the ECC when reading, BCH does it for us as
+ * Don't need to generate the ECC when reading, ECC does it for us as
* part of decoding/correction.
*/
if (nand->reading)
@@ -137,7 +137,7 @@ static int ingenic_nand_ecc_calculate(struct nand_chip *chip, const u8 *dat,
params.bytes = nand->chip.ecc.bytes;
params.strength = nand->chip.ecc.strength;

- return jz4780_bch_calculate(nfc->bch, &params, dat, ecc_code);
+ return ingenic_ecc_calculate(nfc->ecc, &params, dat, ecc_code);
}

static int ingenic_nand_ecc_correct(struct nand_chip *chip, u8 *dat,
@@ -145,13 +145,13 @@ static int ingenic_nand_ecc_correct(struct nand_chip *chip, u8 *dat,
{
struct ingenic_nand *nand = to_ingenic_nand(nand_to_mtd(chip));
struct ingenic_nfc *nfc = to_ingenic_nfc(nand->chip.controller);
- struct jz4780_bch_params params;
+ struct ingenic_ecc_params params;

params.size = nand->chip.ecc.size;
params.bytes = nand->chip.ecc.bytes;
params.strength = nand->chip.ecc.strength;

- return jz4780_bch_correct(nfc->bch, &params, dat, read_ecc);
+ return ingenic_ecc_correct(nfc->ecc, &params, dat, read_ecc);
}

static int ingenic_nand_attach_chip(struct nand_chip *chip)
@@ -165,8 +165,8 @@ static int ingenic_nand_attach_chip(struct nand_chip *chip)

switch (chip->ecc.mode) {
case NAND_ECC_HW:
- if (!nfc->bch) {
- dev_err(nfc->dev, "HW BCH selected, but BCH controller not found\n");
+ if (!nfc->ecc) {
+ dev_err(nfc->dev, "HW ECC selected, but ECC controller not found\n");
return -ENODEV;
}

@@ -176,7 +176,7 @@ static int ingenic_nand_attach_chip(struct nand_chip *chip)
/* fall through */
case NAND_ECC_SOFT:
dev_info(nfc->dev, "using %s (strength %d, size %d, bytes %d)\n",
- (nfc->bch) ? "hardware BCH" : "software ECC",
+ (nfc->ecc) ? "hardware ECC" : "software ECC",
chip->ecc.strength, chip->ecc.size, chip->ecc.bytes);
break;
case NAND_ECC_NONE:
@@ -354,12 +354,12 @@ static int ingenic_nand_probe(struct platform_device *pdev)
return -ENOMEM;

/*
- * Check for BCH HW before we call nand_scan_ident, to prevent us from
- * having to call it again if the BCH driver returns -EPROBE_DEFER.
+ * Check for ECC HW before we call nand_scan_ident, to prevent us from
+ * having to call it again if the ECC driver returns -EPROBE_DEFER.
*/
- nfc->bch = of_jz4780_bch_get(dev->of_node);
- if (IS_ERR(nfc->bch))
- return PTR_ERR(nfc->bch);
+ nfc->ecc = of_ingenic_ecc_get(dev->of_node);
+ if (IS_ERR(nfc->ecc))
+ return PTR_ERR(nfc->ecc);

nfc->dev = dev;
nfc->num_banks = num_banks;
@@ -369,8 +369,8 @@ static int ingenic_nand_probe(struct platform_device *pdev)

ret = ingenic_nand_init_chips(nfc, pdev);
if (ret) {
- if (nfc->bch)
- jz4780_bch_release(nfc->bch);
+ if (nfc->ecc)
+ ingenic_ecc_release(nfc->ecc);
return ret;
}

@@ -382,8 +382,8 @@ static int ingenic_nand_remove(struct platform_device *pdev)
{
struct ingenic_nfc *nfc = platform_get_drvdata(pdev);

- if (nfc->bch)
- jz4780_bch_release(nfc->bch);
+ if (nfc->ecc)
+ ingenic_ecc_release(nfc->ecc);

ingenic_nand_cleanup_chips(nfc);

diff --git a/drivers/mtd/nand/raw/ingenic/jz4780_bch.c b/drivers/mtd/nand/raw/ingenic/jz4780_bch.c
index 7e4e5e627603..1e86a1037d62 100644
--- a/drivers/mtd/nand/raw/ingenic/jz4780_bch.c
+++ b/drivers/mtd/nand/raw/ingenic/jz4780_bch.c
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: GPL-2.0
/*
- * JZ4780 BCH controller
+ * JZ4780 BCH controller driver
*
* Copyright (c) 2015 Imagination Technologies
* Author: Alex Smith <[email protected]>
@@ -8,18 +8,15 @@

#include <linux/bitops.h>
#include <linux/clk.h>
-#include <linux/delay.h>
-#include <linux/init.h>
+#include <linux/device.h>
+#include <linux/io.h>
#include <linux/iopoll.h>
#include <linux/module.h>
#include <linux/mutex.h>
-#include <linux/of.h>
#include <linux/of_platform.h>
#include <linux/platform_device.h>
-#include <linux/sched.h>
-#include <linux/slab.h>

-#include "jz4780_bch.h"
+#include "ingenic_ecc.h"

#define BCH_BHCR 0x0
#define BCH_BHCCR 0x8
@@ -62,15 +59,8 @@
/* Timeout for BCH calculation/correction. */
#define BCH_TIMEOUT_US 100000

-struct jz4780_bch {
- struct device *dev;
- void __iomem *base;
- struct clk *clk;
- struct mutex lock;
-};
-
-static void jz4780_bch_init(struct jz4780_bch *bch,
- struct jz4780_bch_params *params, bool encode)
+static void jz4780_bch_init(struct ingenic_ecc *bch,
+ struct ingenic_ecc_params *params, bool encode)
{
u32 reg;

@@ -90,13 +80,13 @@ static void jz4780_bch_init(struct jz4780_bch *bch,
writel(reg, bch->base + BCH_BHCR);
}

-static void jz4780_bch_disable(struct jz4780_bch *bch)
+static void jz4780_bch_disable(struct ingenic_ecc *bch)
{
writel(readl(bch->base + BCH_BHINT), bch->base + BCH_BHINT);
writel(BCH_BHCR_BCHE, bch->base + BCH_BHCCR);
}

-static void jz4780_bch_write_data(struct jz4780_bch *bch, const void *buf,
+static void jz4780_bch_write_data(struct ingenic_ecc *bch, const void *buf,
size_t size)
{
size_t size32 = size / sizeof(u32);
@@ -113,7 +103,7 @@ static void jz4780_bch_write_data(struct jz4780_bch *bch, const void *buf,
writeb(*src8++, bch->base + BCH_BHDR);
}

-static void jz4780_bch_read_parity(struct jz4780_bch *bch, void *buf,
+static void jz4780_bch_read_parity(struct ingenic_ecc *bch, void *buf,
size_t size)
{
size_t size32 = size / sizeof(u32);
@@ -143,7 +133,7 @@ static void jz4780_bch_read_parity(struct jz4780_bch *bch, void *buf,
}
}

-static bool jz4780_bch_wait_complete(struct jz4780_bch *bch, unsigned int irq,
+static bool jz4780_bch_wait_complete(struct ingenic_ecc *bch, unsigned int irq,
u32 *status)
{
u32 reg;
@@ -167,18 +157,9 @@ static bool jz4780_bch_wait_complete(struct jz4780_bch *bch, unsigned int irq,
return true;
}

-/**
- * jz4780_bch_calculate() - calculate ECC for a data buffer
- * @bch: BCH device.
- * @params: BCH parameters.
- * @buf: input buffer with raw data.
- * @ecc_code: output buffer with ECC.
- *
- * Return: 0 on success, -ETIMEDOUT if timed out while waiting for BCH
- * controller.
- */
-int jz4780_bch_calculate(struct jz4780_bch *bch, struct jz4780_bch_params *params,
- const u8 *buf, u8 *ecc_code)
+static int jz4780_calculate(struct ingenic_ecc *bch,
+ struct ingenic_ecc_params *params,
+ const u8 *buf, u8 *ecc_code)
{
int ret = 0;

@@ -197,23 +178,10 @@ int jz4780_bch_calculate(struct jz4780_bch *bch, struct jz4780_bch_params *param
mutex_unlock(&bch->lock);
return ret;
}
-EXPORT_SYMBOL(jz4780_bch_calculate);
-
-/**
- * jz4780_bch_correct() - detect and correct bit errors
- * @bch: BCH device.
- * @params: BCH parameters.
- * @buf: raw data read from the chip.
- * @ecc_code: ECC read from the chip.
- *
- * Given the raw data and the ECC read from the NAND device, detects and
- * corrects errors in the data.
- *
- * Return: the number of bit errors corrected, -EBADMSG if there are too many
- * errors to correct or -ETIMEDOUT if we timed out waiting for the controller.
- */
-int jz4780_bch_correct(struct jz4780_bch *bch, struct jz4780_bch_params *params,
- u8 *buf, u8 *ecc_code)
+
+static int jz4780_correct(struct ingenic_ecc *bch,
+ struct ingenic_ecc_params *params,
+ u8 *buf, u8 *ecc_code)
{
u32 reg, mask, index;
int i, ret, count;
@@ -259,113 +227,29 @@ int jz4780_bch_correct(struct jz4780_bch *bch, struct jz4780_bch_params *params,
mutex_unlock(&bch->lock);
return ret;
}
-EXPORT_SYMBOL(jz4780_bch_correct);
-
-/**
- * jz4780_bch_get() - get the BCH controller device
- * @np: BCH device tree node.
- *
- * Gets the BCH controller device from the specified device tree node. The
- * device must be released with jz4780_bch_release() when it is no longer being
- * used.
- *
- * Return: a pointer to jz4780_bch, errors are encoded into the pointer.
- * PTR_ERR(-EPROBE_DEFER) if the device hasn't been initialised yet.
- */
-static struct jz4780_bch *jz4780_bch_get(struct device_node *np)
-{
- struct platform_device *pdev;
- struct jz4780_bch *bch;
-
- pdev = of_find_device_by_node(np);
- if (!pdev || !platform_get_drvdata(pdev))
- return ERR_PTR(-EPROBE_DEFER);
-
- get_device(&pdev->dev);
-
- bch = platform_get_drvdata(pdev);
- clk_prepare_enable(bch->clk);
-
- return bch;
-}
-
-/**
- * of_jz4780_bch_get() - get the BCH controller from a DT node
- * @of_node: the node that contains a bch-controller property.
- *
- * Get the bch-controller property from the given device tree
- * node and pass it to jz4780_bch_get to do the work.
- *
- * Return: a pointer to jz4780_bch, errors are encoded into the pointer.
- * PTR_ERR(-EPROBE_DEFER) if the device hasn't been initialised yet.
- */
-struct jz4780_bch *of_jz4780_bch_get(struct device_node *of_node)
-{
- struct jz4780_bch *bch = NULL;
- struct device_node *np;
-
- np = of_parse_phandle(of_node, "ingenic,bch-controller", 0);
-
- if (np) {
- bch = jz4780_bch_get(np);
- of_node_put(np);
- }
- return bch;
-}
-EXPORT_SYMBOL(of_jz4780_bch_get);

-/**
- * jz4780_bch_release() - release the BCH controller device
- * @bch: BCH device.
- */
-void jz4780_bch_release(struct jz4780_bch *bch)
+static int jz4780_bch_probe(struct ingenic_ecc *bch)
{
- clk_disable_unprepare(bch->clk);
- put_device(bch->dev);
-}
-EXPORT_SYMBOL(jz4780_bch_release);
-
-static int jz4780_bch_probe(struct platform_device *pdev)
-{
- struct device *dev = &pdev->dev;
- struct jz4780_bch *bch;
- struct resource *res;
-
- bch = devm_kzalloc(dev, sizeof(*bch), GFP_KERNEL);
- if (!bch)
- return -ENOMEM;
-
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- bch->base = devm_ioremap_resource(dev, res);
- if (IS_ERR(bch->base))
- return PTR_ERR(bch->base);
-
- jz4780_bch_disable(bch);
-
- bch->clk = devm_clk_get(dev, NULL);
- if (IS_ERR(bch->clk)) {
- dev_err(dev, "failed to get clock: %ld\n", PTR_ERR(bch->clk));
- return PTR_ERR(bch->clk);
- }
-
clk_set_rate(bch->clk, BCH_CLK_RATE);

- mutex_init(&bch->lock);
-
- bch->dev = dev;
- platform_set_drvdata(pdev, bch);
-
return 0;
}

+static const struct ingenic_ecc_ops jz4780_bch_ops = {
+ .probe = jz4780_bch_probe,
+ .disable = jz4780_bch_disable,
+ .calculate = jz4780_calculate,
+ .correct = jz4780_correct,
+};
+
static const struct of_device_id jz4780_bch_dt_match[] = {
- { .compatible = "ingenic,jz4780-bch" },
+ { .compatible = "ingenic,jz4780-bch", .data = &jz4780_bch_ops },
{},
};
MODULE_DEVICE_TABLE(of, jz4780_bch_dt_match);

static struct platform_driver jz4780_bch_driver = {
- .probe = jz4780_bch_probe,
+ .probe = ingenic_ecc_probe,
.driver = {
.name = "jz4780-bch",
.of_match_table = of_match_ptr(jz4780_bch_dt_match),
diff --git a/drivers/mtd/nand/raw/ingenic/jz4780_bch.h b/drivers/mtd/nand/raw/ingenic/jz4780_bch.h
deleted file mode 100644
index 451e0c770160..000000000000
--- a/drivers/mtd/nand/raw/ingenic/jz4780_bch.h
+++ /dev/null
@@ -1,40 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-/*
- * JZ4780 BCH controller
- *
- * Copyright (c) 2015 Imagination Technologies
- * Author: Alex Smith <[email protected]>
- */
-
-#ifndef __DRIVERS_MTD_NAND_JZ4780_BCH_H__
-#define __DRIVERS_MTD_NAND_JZ4780_BCH_H__
-
-#include <linux/types.h>
-
-struct device;
-struct device_node;
-struct jz4780_bch;
-
-/**
- * struct jz4780_bch_params - BCH parameters
- * @size: data bytes per ECC step.
- * @bytes: ECC bytes per step.
- * @strength: number of correctable bits per ECC step.
- */
-struct jz4780_bch_params {
- int size;
- int bytes;
- int strength;
-};
-
-int jz4780_bch_calculate(struct jz4780_bch *bch,
- struct jz4780_bch_params *params,
- const u8 *buf, u8 *ecc_code);
-int jz4780_bch_correct(struct jz4780_bch *bch,
- struct jz4780_bch_params *params, u8 *buf,
- u8 *ecc_code);
-
-void jz4780_bch_release(struct jz4780_bch *bch);
-struct jz4780_bch *of_jz4780_bch_get(struct device_node *np);
-
-#endif /* __DRIVERS_MTD_NAND_JZ4780_BCH_H__ */
--
2.11.0


2019-02-09 19:25:10

by Paul Cercueil

[permalink] [raw]
Subject: [PATCH v4 9/9] mtd: rawnand: ingenic: Add ooblayout for the Qi Ben Nanonote

The Ben Nanonote from Qi Hardware expects a specific OOB layout on its
NAND.

Signed-off-by: Paul Cercueil <[email protected]>
---

Changes:

v2: New patch

v3: Use the qi,lb60 layout unconditionally if we detect that we're
running on that board.

v4: No change

drivers/mtd/nand/raw/ingenic/ingenic_nand.c | 41 ++++++++++++++++++++++++++++-
1 file changed, 40 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/raw/ingenic/ingenic_nand.c b/drivers/mtd/nand/raw/ingenic/ingenic_nand.c
index 4837bbce7099..3c4ad6d79764 100644
--- a/drivers/mtd/nand/raw/ingenic/ingenic_nand.c
+++ b/drivers/mtd/nand/raw/ingenic/ingenic_nand.c
@@ -72,6 +72,41 @@ static inline struct ingenic_nfc *to_ingenic_nfc(struct nand_controller *ctrl)
return container_of(ctrl, struct ingenic_nfc, controller);
}

+static int qi_lb60_ooblayout_ecc(struct mtd_info *mtd, int section,
+ struct mtd_oob_region *oobregion)
+{
+ struct nand_chip *chip = mtd_to_nand(mtd);
+ struct nand_ecc_ctrl *ecc = &chip->ecc;
+
+ if (section || !ecc->total)
+ return -ERANGE;
+
+ oobregion->length = ecc->total;
+ oobregion->offset = 12;
+
+ return 0;
+}
+
+static int qi_lb60_ooblayout_free(struct mtd_info *mtd, int section,
+ struct mtd_oob_region *oobregion)
+{
+ struct nand_chip *chip = mtd_to_nand(mtd);
+ struct nand_ecc_ctrl *ecc = &chip->ecc;
+
+ if (section)
+ return -ERANGE;
+
+ oobregion->length = mtd->oobsize - ecc->total - 12;
+ oobregion->offset = 12 + ecc->total;
+
+ return 0;
+}
+
+const struct mtd_ooblayout_ops qi_lb60_ooblayout_ops = {
+ .ecc = qi_lb60_ooblayout_ecc,
+ .free = qi_lb60_ooblayout_free,
+};
+
static int jz4725b_ooblayout_ecc(struct mtd_info *mtd, int section,
struct mtd_oob_region *oobregion)
{
@@ -247,7 +282,11 @@ static int ingenic_nand_attach_chip(struct nand_chip *chip)
return -EINVAL;
}

- mtd_set_ooblayout(mtd, nfc->soc_info->oob_layout);
+ /* For legacy reasons we use a different layout on the qi,lb60 board. */
+ if (of_machine_is_compatible("qi,lb60"))
+ mtd_set_ooblayout(mtd, &qi_lb60_ooblayout_ops);
+ else
+ mtd_set_ooblayout(mtd, nfc->soc_info->oob_layout);

return 0;
}
--
2.11.0


2019-02-09 19:26:16

by Paul Cercueil

[permalink] [raw]
Subject: [PATCH v4 8/9] mtd: rawnand: ingenic: Add support for the JZ4725B

The boot ROM of the JZ4725B SoC expects a specific OOB layout on the
NAND, so we use it unconditionally in the ingenic-nand driver.

Also add the jz4725b-bch driver to support the JZ4725B-specific BCH
hardware.

Signed-off-by: Paul Cercueil <[email protected]>
---

Changes:

v2: Instead of forcing the OOB layout, leave it to the board code or
devicetree to decide if the jz4725b-specific layout should be used
or not.

v3: - Revert the change in v2, as the previous behaviour was correct.
- Also add support for the hardware BCH of the JZ4725B in this
patch.

v4: - Add MODULE_* macros
- Add tweaks suggested by upstream feedback

drivers/mtd/nand/raw/ingenic/Kconfig | 10 +
drivers/mtd/nand/raw/ingenic/Makefile | 1 +
drivers/mtd/nand/raw/ingenic/ingenic_nand.c | 48 ++++-
drivers/mtd/nand/raw/ingenic/jz4725b_bch.c | 292 ++++++++++++++++++++++++++++
4 files changed, 350 insertions(+), 1 deletion(-)
create mode 100644 drivers/mtd/nand/raw/ingenic/jz4725b_bch.c

diff --git a/drivers/mtd/nand/raw/ingenic/Kconfig b/drivers/mtd/nand/raw/ingenic/Kconfig
index cc663cc15119..497b46b144f2 100644
--- a/drivers/mtd/nand/raw/ingenic/Kconfig
+++ b/drivers/mtd/nand/raw/ingenic/Kconfig
@@ -27,6 +27,16 @@ config MTD_NAND_JZ4740_ECC
This driver can also be built as a module. If so, the module
will be called jz4740-ecc.

+config MTD_NAND_JZ4725B_BCH
+ tristate "Hardware BCH support for JZ4725B SoC"
+ select MTD_NAND_INGENIC_ECC
+ help
+ Enable this driver to support the BCH error-correction hardware
+ present on the JZ4725B SoC from Ingenic.
+
+ This driver can also be built as a module. If so, the module
+ will be called jz4725b-bch.
+
config MTD_NAND_JZ4780_BCH
tristate "Hardware BCH support for JZ4780 SoC"
select MTD_NAND_INGENIC_ECC
diff --git a/drivers/mtd/nand/raw/ingenic/Makefile b/drivers/mtd/nand/raw/ingenic/Makefile
index 563b7effcf59..ab2c5f47e5b7 100644
--- a/drivers/mtd/nand/raw/ingenic/Makefile
+++ b/drivers/mtd/nand/raw/ingenic/Makefile
@@ -3,4 +3,5 @@ obj-$(CONFIG_MTD_NAND_JZ4780) += ingenic_nand.o

obj-$(CONFIG_MTD_NAND_INGENIC_ECC) += ingenic_ecc.o
obj-$(CONFIG_MTD_NAND_JZ4740_ECC) += jz4740_ecc.o
+obj-$(CONFIG_MTD_NAND_JZ4725B_BCH) += jz4725b_bch.o
obj-$(CONFIG_MTD_NAND_JZ4780_BCH) += jz4780_bch.o
diff --git a/drivers/mtd/nand/raw/ingenic/ingenic_nand.c b/drivers/mtd/nand/raw/ingenic/ingenic_nand.c
index 036938fdad51..4837bbce7099 100644
--- a/drivers/mtd/nand/raw/ingenic/ingenic_nand.c
+++ b/drivers/mtd/nand/raw/ingenic/ingenic_nand.c
@@ -34,6 +34,7 @@ struct jz_soc_info {
unsigned long data_offset;
unsigned long addr_offset;
unsigned long cmd_offset;
+ const struct mtd_ooblayout_ops *oob_layout;
};

struct ingenic_nand_cs {
@@ -71,6 +72,41 @@ static inline struct ingenic_nfc *to_ingenic_nfc(struct nand_controller *ctrl)
return container_of(ctrl, struct ingenic_nfc, controller);
}

+static int jz4725b_ooblayout_ecc(struct mtd_info *mtd, int section,
+ struct mtd_oob_region *oobregion)
+{
+ struct nand_chip *chip = mtd_to_nand(mtd);
+ struct nand_ecc_ctrl *ecc = &chip->ecc;
+
+ if (section || !ecc->total)
+ return -ERANGE;
+
+ oobregion->length = ecc->total;
+ oobregion->offset = 3;
+
+ return 0;
+}
+
+static int jz4725b_ooblayout_free(struct mtd_info *mtd, int section,
+ struct mtd_oob_region *oobregion)
+{
+ struct nand_chip *chip = mtd_to_nand(mtd);
+ struct nand_ecc_ctrl *ecc = &chip->ecc;
+
+ if (section)
+ return -ERANGE;
+
+ oobregion->length = mtd->oobsize - ecc->total - 3;
+ oobregion->offset = 3 + ecc->total;
+
+ return 0;
+}
+
+const struct mtd_ooblayout_ops jz4725b_ooblayout_ops = {
+ .ecc = jz4725b_ooblayout_ecc,
+ .free = jz4725b_ooblayout_free,
+};
+
static void ingenic_nand_select_chip(struct nand_chip *chip, int chipnr)
{
struct ingenic_nand *nand = to_ingenic_nand(nand_to_mtd(chip));
@@ -211,7 +247,7 @@ static int ingenic_nand_attach_chip(struct nand_chip *chip)
return -EINVAL;
}

- mtd_set_ooblayout(mtd, &nand_ooblayout_lp_ops);
+ mtd_set_ooblayout(mtd, nfc->soc_info->oob_layout);

return 0;
}
@@ -407,16 +443,26 @@ static const struct jz_soc_info jz4740_soc_info = {
.data_offset = 0x00000000,
.cmd_offset = 0x00008000,
.addr_offset = 0x00010000,
+ .oob_layout = &nand_ooblayout_lp_ops,
+};
+
+static const struct jz_soc_info jz4725b_soc_info = {
+ .data_offset = 0x00000000,
+ .cmd_offset = 0x00008000,
+ .addr_offset = 0x00010000,
+ .oob_layout = &jz4725b_ooblayout_ops,
};

static const struct jz_soc_info jz4780_soc_info = {
.data_offset = 0x00000000,
.cmd_offset = 0x00400000,
.addr_offset = 0x00800000,
+ .oob_layout = &nand_ooblayout_lp_ops,
};

static const struct of_device_id ingenic_nand_dt_match[] = {
{ .compatible = "ingenic,jz4740-nand", .data = &jz4740_soc_info },
+ { .compatible = "ingenic,jz4725b-nand", .data = &jz4725b_soc_info },
{ .compatible = "ingenic,jz4780-nand", .data = &jz4780_soc_info },
{},
};
diff --git a/drivers/mtd/nand/raw/ingenic/jz4725b_bch.c b/drivers/mtd/nand/raw/ingenic/jz4725b_bch.c
new file mode 100644
index 000000000000..f7094c34b5df
--- /dev/null
+++ b/drivers/mtd/nand/raw/ingenic/jz4725b_bch.c
@@ -0,0 +1,292 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * JZ4725B BCH controller driver
+ *
+ * Copyright (C) 2019 Paul Cercueil <[email protected]>
+ *
+ * Based on jz4780_bch.c
+ */
+
+#include <linux/bitops.h>
+#include <linux/device.h>
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+
+#include "ingenic_ecc.h"
+
+#define BCH_BHCR 0x0
+#define BCH_BHCSR 0x4
+#define BCH_BHCCR 0x8
+#define BCH_BHCNT 0xc
+#define BCH_BHDR 0x10
+#define BCH_BHPAR0 0x14
+#define BCH_BHERR0 0x28
+#define BCH_BHINT 0x24
+#define BCH_BHINTES 0x3c
+#define BCH_BHINTEC 0x40
+#define BCH_BHINTE 0x38
+
+#define BCH_BHCR_ENCE BIT(3)
+#define BCH_BHCR_BSEL BIT(2)
+#define BCH_BHCR_INIT BIT(1)
+#define BCH_BHCR_BCHE BIT(0)
+
+#define BCH_BHCNT_DEC_COUNT_SHIFT 16
+#define BCH_BHCNT_DEC_COUNT_MASK (0x3ff << BCH_BHCNT_DEC_COUNT_SHIFT)
+#define BCH_BHCNT_ENC_COUNT_SHIFT 0
+#define BCH_BHCNT_ENC_COUNT_MASK (0x3ff << BCH_BHCNT_ENC_COUNT_SHIFT)
+
+#define BCH_BHERR_INDEX0_SHIFT 0
+#define BCH_BHERR_INDEX0_MASK (0x1fff << BCH_BHERR_INDEX0_SHIFT)
+#define BCH_BHERR_INDEX1_SHIFT 16
+#define BCH_BHERR_INDEX1_MASK (0x1fff << BCH_BHERR_INDEX1_SHIFT)
+
+#define BCH_BHINT_ERRC_SHIFT 28
+#define BCH_BHINT_ERRC_MASK (0xf << BCH_BHINT_ERRC_SHIFT)
+#define BCH_BHINT_TERRC_SHIFT 16
+#define BCH_BHINT_TERRC_MASK (0x7f << BCH_BHINT_TERRC_SHIFT)
+#define BCH_BHINT_ALL_0 BIT(5)
+#define BCH_BHINT_ALL_F BIT(4)
+#define BCH_BHINT_DECF BIT(3)
+#define BCH_BHINT_ENCF BIT(2)
+#define BCH_BHINT_UNCOR BIT(1)
+#define BCH_BHINT_ERR BIT(0)
+
+/* Timeout for BCH calculation/correction. */
+#define BCH_TIMEOUT_US 100000
+
+static inline void jz4725b_bch_config_set(struct ingenic_ecc *bch, u32 cfg)
+{
+ writel(cfg, bch->base + BCH_BHCSR);
+}
+
+static inline void jz4725b_bch_config_clear(struct ingenic_ecc *bch, u32 cfg)
+{
+ writel(cfg, bch->base + BCH_BHCCR);
+}
+
+static int jz4725b_bch_init(struct ingenic_ecc *bch,
+ struct ingenic_ecc_params *params, bool calc_ecc)
+{
+ u32 reg, max_value;
+
+ /* Clear interrupt status. */
+ writel(readl(bch->base + BCH_BHINT), bch->base + BCH_BHINT);
+
+ /* Initialise and enable BCH. */
+ jz4725b_bch_config_clear(bch, 0x1f);
+ jz4725b_bch_config_set(bch, BCH_BHCR_BCHE);
+
+ if (params->strength == 8)
+ jz4725b_bch_config_set(bch, BCH_BHCR_BSEL);
+ else
+ jz4725b_bch_config_clear(bch, BCH_BHCR_BSEL);
+
+ if (calc_ecc) /* calculate ECC from data */
+ jz4725b_bch_config_set(bch, BCH_BHCR_ENCE);
+ else /* correct data from ECC */
+ jz4725b_bch_config_clear(bch, BCH_BHCR_ENCE);
+
+ jz4725b_bch_config_set(bch, BCH_BHCR_INIT);
+
+ max_value = BCH_BHCNT_ENC_COUNT_MASK >> BCH_BHCNT_ENC_COUNT_SHIFT;
+ if (params->size > max_value)
+ return -EINVAL;
+
+ max_value = BCH_BHCNT_DEC_COUNT_MASK >> BCH_BHCNT_DEC_COUNT_SHIFT;
+ if (params->size + params->bytes > max_value)
+ return -EINVAL;
+
+ /* Set up BCH count register. */
+ reg = params->size << BCH_BHCNT_ENC_COUNT_SHIFT;
+ reg |= (params->size + params->bytes) << BCH_BHCNT_DEC_COUNT_SHIFT;
+ writel(reg, bch->base + BCH_BHCNT);
+
+ return 0;
+}
+
+static void jz4725b_bch_disable(struct ingenic_ecc *bch)
+{
+ /* Clear interrupts */
+ writel(readl(bch->base + BCH_BHINT), bch->base + BCH_BHINT);
+
+ /* Disable the hardware */
+ jz4725b_bch_config_clear(bch, BCH_BHCR_BCHE);
+}
+
+static void jz4725b_bch_write_data(struct ingenic_ecc *bch, const u8 *buf,
+ size_t size)
+{
+ while (size--)
+ writeb(*buf++, bch->base + BCH_BHDR);
+}
+
+static void jz4725b_bch_read_parity(struct ingenic_ecc *bch, u8 *buf,
+ size_t size)
+{
+ size_t size32 = size / sizeof(u32);
+ size_t size8 = size % sizeof(u32);
+ u32 *dest32;
+ u8 *dest8;
+ u32 val, offset = 0;
+
+ dest32 = (u32 *)buf;
+ while (size32--) {
+ *dest32++ = readl_relaxed(bch->base + BCH_BHPAR0 + offset);
+ offset += sizeof(u32);
+ }
+
+ dest8 = (u8 *)dest32;
+ val = readl_relaxed(bch->base + BCH_BHPAR0 + offset);
+ switch (size8) {
+ case 3:
+ dest8[2] = (val >> 16) & 0xff;
+ case 2: /* fall-through */
+ dest8[1] = (val >> 8) & 0xff;
+ case 1: /* fall-through */
+ dest8[0] = val & 0xff;
+ break;
+ }
+}
+
+static int jz4725b_bch_wait_complete(struct ingenic_ecc *bch, unsigned int irq,
+ u32 *status)
+{
+ u32 reg;
+ int ret;
+
+ /*
+ * While we could use interrupts here and sleep until the operation
+ * completes, the controller works fairly quickly (usually a few
+ * microseconds) and so the overhead of sleeping until we get an
+ * interrupt quite noticeably decreases performance.
+ */
+ ret = readl_relaxed_poll_timeout(bch->base + BCH_BHINT, reg,
+ reg & irq, 0, BCH_TIMEOUT_US);
+ if (ret)
+ return ret;
+
+ if (status)
+ *status = reg;
+
+ writel(reg, bch->base + BCH_BHINT);
+
+ return 0;
+}
+
+static int jz4725b_calculate(struct ingenic_ecc *bch,
+ struct ingenic_ecc_params *params,
+ const u8 *buf, u8 *ecc_code)
+{
+ int ret;
+
+ mutex_lock(&bch->lock);
+ ret = jz4725b_bch_init(bch, params, true);
+ if (ret) {
+ dev_err(bch->dev, "Unable to init BCH with given parameters\n");
+ goto out_disable;
+ }
+
+ jz4725b_bch_write_data(bch, buf, params->size);
+
+ ret = jz4725b_bch_wait_complete(bch, BCH_BHINT_ENCF, NULL);
+ if (ret) {
+ dev_err(bch->dev, "timed out while calculating ECC\n");
+ goto out_disable;
+ }
+
+ jz4725b_bch_read_parity(bch, ecc_code, params->bytes);
+
+out_disable:
+ jz4725b_bch_disable(bch);
+ mutex_unlock(&bch->lock);
+
+ return ret;
+}
+
+static int jz4725b_correct(struct ingenic_ecc *bch,
+ struct ingenic_ecc_params *params,
+ u8 *buf, u8 *ecc_code)
+{
+ u32 reg, errors, bit;
+ unsigned int i;
+ int ret;
+
+ mutex_lock(&bch->lock);
+
+ ret = jz4725b_bch_init(bch, params, false);
+ if (ret) {
+ dev_err(bch->dev, "Unable to init BCH with given parameters\n");
+ goto out;
+ }
+
+ jz4725b_bch_write_data(bch, buf, params->size);
+ jz4725b_bch_write_data(bch, ecc_code, params->bytes);
+
+ ret = jz4725b_bch_wait_complete(bch, BCH_BHINT_DECF, &reg);
+ if (ret) {
+ dev_err(bch->dev, "timed out while correcting data\n");
+ goto out;
+ }
+
+ if (reg & (BCH_BHINT_ALL_F | BCH_BHINT_ALL_0)) {
+ /* Data and ECC is all 0xff or 0x00 - nothing to correct */
+ ret = 0;
+ goto out;
+ }
+
+ if (reg & BCH_BHINT_UNCOR) {
+ /* Uncorrectable ECC error */
+ ret = -EBADMSG;
+ goto out;
+ }
+
+ errors = (reg & BCH_BHINT_ERRC_MASK) >> BCH_BHINT_ERRC_SHIFT;
+
+ /* Correct any detected errors. */
+ for (i = 0; i < errors; i++) {
+ if (i & 1) {
+ bit = (reg & BCH_BHERR_INDEX1_MASK) >> BCH_BHERR_INDEX1_SHIFT;
+ } else {
+ reg = readl(bch->base + BCH_BHERR0 + (i * 4));
+ bit = (reg & BCH_BHERR_INDEX0_MASK) >> BCH_BHERR_INDEX0_SHIFT;
+ }
+
+ buf[(bit >> 3)] ^= BIT(bit & 0x7);
+ }
+
+out:
+ jz4725b_bch_disable(bch);
+ mutex_unlock(&bch->lock);
+
+ return ret;
+}
+
+static const struct ingenic_ecc_ops jz4725b_bch_ops = {
+ .disable = jz4725b_bch_disable,
+ .calculate = jz4725b_calculate,
+ .correct = jz4725b_correct,
+};
+
+static const struct of_device_id jz4725b_bch_dt_match[] = {
+ { .compatible = "ingenic,jz4725b-bch", .data = &jz4725b_bch_ops },
+ {},
+};
+MODULE_DEVICE_TABLE(of, jz4725b_bch_dt_match);
+
+static struct platform_driver jz4725b_bch_driver = {
+ .probe = ingenic_ecc_probe,
+ .driver = {
+ .name = "jz4725b-bch",
+ .of_match_table = jz4725b_bch_dt_match,
+ },
+};
+module_platform_driver(jz4725b_bch_driver);
+
+MODULE_AUTHOR("Paul Cercueil <[email protected]>");
+MODULE_DESCRIPTION("Ingenic JZ4725B BCH controller driver");
+MODULE_LICENSE("GPL v2");
--
2.11.0


2019-02-14 09:02:20

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v4 1/9] dt-bindings: mtd: ingenic: Add compatible strings for JZ4740 and JZ4725B

On Sat, 9 Feb 2019 16:22:57 -0300, Paul Cercueil wrote:
> Add compatible strings to probe the jz4780-nand and jz4780-bch drivers
> from devicetree on the JZ4725B and JZ4740 SoCs from Ingenic.
>
> Signed-off-by: Paul Cercueil <[email protected]>
> ---
>
> Changes:
>
> v2: - Change 'ingenic,jz4725b-nand' compatible string to
> 'ingenic,jz4740-nand' to reflect driver change
> - Add 'ingenic,jz4740-bch' compatible string
> - Document 'ingenic,oob-layout' property
>
> v3: - Removed 'ingenic,oob-layout' property
> - Update compatible strings to what the driver supports
>
> v4: No change
>
> Documentation/devicetree/bindings/mtd/ingenic,jz4780-nand.txt | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>

Reviewed-by: Rob Herring <[email protected]>

2019-02-14 09:03:16

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v4 2/9] dt-bindings: mtd: ingenic: Change 'BCH' to 'ECC' in documentation

On Sat, 9 Feb 2019 16:22:58 -0300, Paul Cercueil wrote:
> The JZ4740 ECC hardware is not BCH but Reed-Solomon, so it makes more
> sense to use the more generic ECC term.
>
> Signed-off-by: Paul Cercueil <[email protected]>
> ---
>
> Changes:
>
> v3: New patch
>
> v4: No change
>
> .../devicetree/bindings/mtd/ingenic,jz4780-nand.txt | 18 +++++++++---------
> 1 file changed, 9 insertions(+), 9 deletions(-)
>

Reviewed-by: Rob Herring <[email protected]>

2019-03-04 09:47:05

by Miquel Raynal

[permalink] [raw]
Subject: Re: [PATCH v4 1/9] dt-bindings: mtd: ingenic: Add compatible strings for JZ4740 and JZ4725B

Hi Paul,

Paul Cercueil <[email protected]> wrote on Sat, 9 Feb 2019 16:22:57
-0300:

> Add compatible strings to probe the jz4780-nand and jz4780-bch drivers
> from devicetree on the JZ4725B and JZ4740 SoCs from Ingenic.
>
> Signed-off-by: Paul Cercueil <[email protected]>
> ---
>
> Changes:
>
> v2: - Change 'ingenic,jz4725b-nand' compatible string to
> 'ingenic,jz4740-nand' to reflect driver change
> - Add 'ingenic,jz4740-bch' compatible string
> - Document 'ingenic,oob-layout' property
>
> v3: - Removed 'ingenic,oob-layout' property
> - Update compatible strings to what the driver supports
>
> v4: No change
>
> Documentation/devicetree/bindings/mtd/ingenic,jz4780-nand.txt | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/mtd/ingenic,jz4780-nand.txt b/Documentation/devicetree/bindings/mtd/ingenic,jz4780-nand.txt
> index 29ea5853ca91..a5b940f18bf6 100644
> --- a/Documentation/devicetree/bindings/mtd/ingenic,jz4780-nand.txt
> +++ b/Documentation/devicetree/bindings/mtd/ingenic,jz4780-nand.txt
> @@ -6,7 +6,10 @@ memory-controllers/ingenic,jz4780-nemc.txt), and thus NAND device nodes must
> be children of the NEMC node.
>
> Required NAND controller device properties:
> -- compatible: Should be set to "ingenic,jz4780-nand".
> +- compatible: Should be one of:
> + * ingenic,jz4740-nand
> + * ingenic,jz4725b-nand
> + * ingenic,jz4780-nand

Wouldn't "-nand-controller" suffix be better? Of course in the driver
you should still check for jz4780-nand.

> - reg: For each bank with a NAND chip attached, should specify a bank number,
> an offset of 0 and a size of 0x1000000 (i.e. the whole NEMC bank).
>
> @@ -72,7 +75,10 @@ NAND devices. The following is a description of the device properties for a
> BCH controller.
>
> Required BCH properties:
> -- compatible: Should be set to "ingenic,jz4780-bch".
> +- compatible: Should be one of:
> + * ingenic,jz4740-ecc
> + * ingenic,jz4725b-bch
> + * ingenic,jz4780-bch
> - reg: Should specify the BCH controller registers location and length.
> - clocks: Clock for the BCH controller.
>

Thanks,
Miquèl

2019-03-04 09:51:10

by Miquel Raynal

[permalink] [raw]
Subject: Re: [PATCH v4 2/9] dt-bindings: mtd: ingenic: Change 'BCH' to 'ECC' in documentation

Hi Paul,

Paul Cercueil <[email protected]> wrote on Sat, 9 Feb 2019 16:22:58
-0300:

> The JZ4740 ECC hardware is not BCH but Reed-Solomon, so it makes more
> sense to use the more generic ECC term.
>
> Signed-off-by: Paul Cercueil <[email protected]>
> ---
>
> Changes:
>
> v3: New patch
>
> v4: No change
>
> .../devicetree/bindings/mtd/ingenic,jz4780-nand.txt | 18 +++++++++---------
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/mtd/ingenic,jz4780-nand.txt b/Documentation/devicetree/bindings/mtd/ingenic,jz4780-nand.txt
> index a5b940f18bf6..5a45cc54f46d 100644
> --- a/Documentation/devicetree/bindings/mtd/ingenic,jz4780-nand.txt
> +++ b/Documentation/devicetree/bindings/mtd/ingenic,jz4780-nand.txt
> @@ -1,4 +1,4 @@
> -* Ingenic JZ4780 NAND/BCH
> +* Ingenic JZ4780 NAND/ECC
>
> This file documents the device tree bindings for NAND flash devices on the
> JZ4780. NAND devices are connected to the NEMC controller (described in
> @@ -14,10 +14,10 @@ Required NAND controller device properties:
> an offset of 0 and a size of 0x1000000 (i.e. the whole NEMC bank).
>
> Optional NAND controller device properties:
> -- ingenic,bch-controller: To make use of the hardware BCH controller, this
> - property must contain a phandle for the BCH controller node. The required
> +- ingenic,bch-controller: To make use of the hardware ECC controller, this
> + property must contain a phandle for the ECC controller node. The required

I think there is already a 'ecc-engine' property used by MTK and Atmel
NAND controllers to point to the ECC engine block. Please use this
property instead of the ingenic specific one.

> properties for this node are described below. If this is not specified,
> - software BCH will be used instead.
> + software ECC will be used instead.
>
> Optional children nodes:
> - Individual NAND chips are children of the NAND controller node.
> @@ -70,17 +70,17 @@ nemc: nemc@13410000 {
> };
> };
>
> -The BCH controller is a separate SoC component used for error correction on
> +The ECC controller is a separate SoC component used for error correction on
> NAND devices. The following is a description of the device properties for a
> -BCH controller.
> +ECC controller.
>
> -Required BCH properties:
> +Required ECC properties:
> - compatible: Should be one of:
> * ingenic,jz4740-ecc
> * ingenic,jz4725b-bch
> * ingenic,jz4780-bch
> -- reg: Should specify the BCH controller registers location and length.
> -- clocks: Clock for the BCH controller.
> +- reg: Should specify the ECC controller registers location and length.
> +- clocks: Clock for the ECC controller.
>
> Example:
>

Thanks,
Miquèl

2019-03-04 10:21:42

by Miquel Raynal

[permalink] [raw]
Subject: Re: [PATCH v4 6/9] mtd: rawnand: ingenic: Separate top-level and SoC specific code

Hi Paul,

Paul Cercueil <[email protected]> wrote on Sat, 9 Feb 2019 16:23:02
-0300:

> The ingenic-nand driver uses an API provided by the jz4780-bch driver.
> This makes it difficult to support other SoCs in the jz4780-bch driver.
> To work around this, we separate the API functions from the SoC-specific
> code, so that these API functions are SoC-agnostic.
>

I like the idea, actually I am working on this separation (see
[1]) and I would really appreciate that you try to implement the
interface when it will be available (v2 is coming this week, I think v3
will be the one to test when raw NAND devices will be properly
supported). I will add you in Cc: if you want to follow/review.

[1] http://lists.infradead.org/pipermail/linux-mtd/2019-February/087815.html

> Signed-off-by: Paul Cercueil <[email protected]>
> ---
>
> v2: Add an optional .probe() callback. It is used for instance to set
> the clock rate in the JZ4780 backend.
>
> v3: The common code is now inside the ingenic-ecc module. Each
> SoC-specific ECC code is now in its own module, which leaves to the
> user the choice of which (if any) ECC code should be supported.
>
> v4: No change
>
> drivers/mtd/nand/raw/ingenic/Kconfig | 17 +++
> drivers/mtd/nand/raw/ingenic/Makefile | 5 +-
> drivers/mtd/nand/raw/ingenic/ingenic_ecc.c | 157 +++++++++++++++++++++++++
> drivers/mtd/nand/raw/ingenic/ingenic_ecc.h | 84 ++++++++++++++
> drivers/mtd/nand/raw/ingenic/ingenic_nand.c | 38 +++----
> drivers/mtd/nand/raw/ingenic/jz4780_bch.c | 170 +++++-----------------------
> drivers/mtd/nand/raw/ingenic/jz4780_bch.h | 40 -------
> 7 files changed, 308 insertions(+), 203 deletions(-)
> create mode 100644 drivers/mtd/nand/raw/ingenic/ingenic_ecc.c
> create mode 100644 drivers/mtd/nand/raw/ingenic/ingenic_ecc.h
> delete mode 100644 drivers/mtd/nand/raw/ingenic/jz4780_bch.h
>

[...]

> diff --git a/drivers/mtd/nand/raw/ingenic/ingenic_nand.c b/drivers/mtd/nand/raw/ingenic/ingenic_nand.c
> index 8c73f7c5be9a..0f51fd15fe79 100644
> --- a/drivers/mtd/nand/raw/ingenic/ingenic_nand.c
> +++ b/drivers/mtd/nand/raw/ingenic/ingenic_nand.c
> @@ -22,7 +22,7 @@
>
> #include <linux/jz4780-nemc.h>
>
> -#include "jz4780_bch.h"
> +#include "ingenic_ecc.h"
>
> #define DRV_NAME "ingenic-nand"
>
> @@ -40,7 +40,7 @@ struct ingenic_nand_cs {
>
> struct ingenic_nfc {
> struct device *dev;
> - struct jz4780_bch *bch;
> + struct ingenic_ecc *ecc;
> struct nand_controller controller;
> unsigned int num_banks;
> struct list_head chips;
> @@ -124,10 +124,10 @@ static int ingenic_nand_ecc_calculate(struct nand_chip *chip, const u8 *dat,
> {
> struct ingenic_nand *nand = to_ingenic_nand(nand_to_mtd(chip));
> struct ingenic_nfc *nfc = to_ingenic_nfc(nand->chip.controller);
> - struct jz4780_bch_params params;
> + struct ingenic_ecc_params params;
>
> /*
> - * Don't need to generate the ECC when reading, BCH does it for us as
> + * Don't need to generate the ECC when reading, ECC does it for us as

"the ECC engine does it for us" would be more meaningful.

> * part of decoding/correction.
> */
> if (nand->reading)
> @@ -137,7 +137,7 @@ static int ingenic_nand_ecc_calculate(struct nand_chip *chip, const u8 *dat,
> params.bytes = nand->chip.ecc.bytes;
> params.strength = nand->chip.ecc.strength;
>
> - return jz4780_bch_calculate(nfc->bch, &params, dat, ecc_code);
> + return ingenic_ecc_calculate(nfc->ecc, &params, dat, ecc_code);
> }
>

Thanks,
Miquèl

2019-03-04 10:36:21

by Miquel Raynal

[permalink] [raw]
Subject: Re: [PATCH v4 7/9] mtd: rawnand: ingenic: Add support for the JZ4740

Hi Paul,

Paul Cercueil <[email protected]> wrote on Sat, 9 Feb 2019 16:23:03
-0300:

> Add support for probing the ingenic-nand driver on the JZ4740 SoC from
> Ingenic, and the jz4740-ecc driver to support the JZ4740-specific
> ECC hardware.
>
> Signed-off-by: Paul Cercueil <[email protected]>
> ---
>
> Changes:
>
> v2: New patch
>
> v3: Also add support for the hardware ECC of the JZ4740 in this patch
>
> v4: - Fix formatting issues
> - Add MODULE_* macros
>
> drivers/mtd/nand/raw/ingenic/Kconfig | 10 ++
> drivers/mtd/nand/raw/ingenic/Makefile | 1 +
> drivers/mtd/nand/raw/ingenic/ingenic_nand.c | 48 +++++--
> drivers/mtd/nand/raw/ingenic/jz4740_ecc.c | 196 ++++++++++++++++++++++++++++
> 4 files changed, 244 insertions(+), 11 deletions(-)
> create mode 100644 drivers/mtd/nand/raw/ingenic/jz4740_ecc.c
>

[...]

> switch (chip->ecc.mode) {
> case NAND_ECC_HW:
> @@ -270,8 +279,8 @@ static int ingenic_nand_init_chip(struct platform_device *pdev,
> return -ENOMEM;
> mtd->dev.parent = dev;
>
> - chip->legacy.IO_ADDR_R = cs->base + OFFSET_DATA;
> - chip->legacy.IO_ADDR_W = cs->base + OFFSET_DATA;
> + chip->legacy.IO_ADDR_R = cs->base + nfc->soc_info->data_offset;
> + chip->legacy.IO_ADDR_W = cs->base + nfc->soc_info->data_offset;
> chip->legacy.chip_delay = RB_DELAY_US;
> chip->options = NAND_NO_SUBPAGE_WRITE;
> chip->legacy.select_chip = ingenic_nand_select_chip;

I think Boris already asked for it, but it would be really great that
you update this driver to not use any legacy interface anymore.

> diff --git a/drivers/mtd/nand/raw/ingenic/jz4740_ecc.c b/drivers/mtd/nand/raw/ingenic/jz4740_ecc.c
> new file mode 100644
> index 000000000000..83b42881720e
> --- /dev/null
> +++ b/drivers/mtd/nand/raw/ingenic/jz4740_ecc.c
> @@ -0,0 +1,196 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * JZ4740 ECC controller driver
> + *
> + * Copyright (c) 2019 Paul Cercueil <[email protected]>
> + *
> + * based on jz4740-nand.c
> + */
> +
> +#include <linux/bitops.h>
> +#include <linux/device.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +
> +#include "ingenic_ecc.h"
> +
> +#define JZ_REG_NAND_ECC_CTRL 0x00
> +#define JZ_REG_NAND_DATA 0x04
> +#define JZ_REG_NAND_PAR0 0x08
> +#define JZ_REG_NAND_PAR1 0x0C
> +#define JZ_REG_NAND_PAR2 0x10
> +#define JZ_REG_NAND_IRQ_STAT 0x14
> +#define JZ_REG_NAND_IRQ_CTRL 0x18
> +#define JZ_REG_NAND_ERR(x) (0x1C + ((x) << 2))
> +
> +#define JZ_NAND_ECC_CTRL_PAR_READY BIT(4)
> +#define JZ_NAND_ECC_CTRL_ENCODING BIT(3)
> +#define JZ_NAND_ECC_CTRL_RS BIT(2)
> +#define JZ_NAND_ECC_CTRL_RESET BIT(1)
> +#define JZ_NAND_ECC_CTRL_ENABLE BIT(0)
> +
> +#define JZ_NAND_STATUS_ERR_COUNT (BIT(31) | BIT(30) | BIT(29))
> +#define JZ_NAND_STATUS_PAD_FINISH BIT(4)
> +#define JZ_NAND_STATUS_DEC_FINISH BIT(3)
> +#define JZ_NAND_STATUS_ENC_FINISH BIT(2)
> +#define JZ_NAND_STATUS_UNCOR_ERROR BIT(1)
> +#define JZ_NAND_STATUS_ERROR BIT(0)
> +
> +static const uint8_t empty_block_ecc[] = {
> + 0xcd, 0x9d, 0x90, 0x58, 0xf4, 0x8b, 0xff, 0xb7, 0x6f
> +};
> +
> +static void jz4740_ecc_init(struct ingenic_ecc *ecc, bool encode)
> +{
> + uint32_t reg;
> +
> + /* Clear interrupt status */
> + writel(0, ecc->base + JZ_REG_NAND_IRQ_STAT);
> +
> + /* Initialize and enable ECC hardware */
> + reg = readl(ecc->base + JZ_REG_NAND_ECC_CTRL);
> + reg |= JZ_NAND_ECC_CTRL_RESET;
> + reg |= JZ_NAND_ECC_CTRL_ENABLE;
> + reg |= JZ_NAND_ECC_CTRL_RS;
> + if (encode)
> + reg |= JZ_NAND_ECC_CTRL_ENCODING;
> + else
> + reg &= ~JZ_NAND_ECC_CTRL_ENCODING;
> +
> + writel(reg, ecc->base + JZ_REG_NAND_ECC_CTRL);
> +}
> +
> +static int jz4740_ecc_calculate(struct ingenic_ecc *ecc,
> + struct ingenic_ecc_params *params,
> + const u8 *buf, u8 *ecc_code)
> +{
> + uint32_t reg, status;
> + unsigned int timeout = 1000;
> + int i;
> +
> + jz4740_ecc_init(ecc, true);
> +
> + do {
> + status = readl(ecc->base + JZ_REG_NAND_IRQ_STAT);
> + } while (!(status & JZ_NAND_STATUS_ENC_FINISH) && --timeout);
> +
> + if (timeout == 0)
> + return -ETIMEDOUT;
> +
> + reg = readl(ecc->base + JZ_REG_NAND_ECC_CTRL);
> + reg &= ~JZ_NAND_ECC_CTRL_ENABLE;
> + writel(reg, ecc->base + JZ_REG_NAND_ECC_CTRL);
> +
> + for (i = 0; i < params->bytes; ++i)
> + ecc_code[i] = readb(ecc->base + JZ_REG_NAND_PAR0 + i);
> +
> + /* If the written data is completely 0xff, we also want to write 0xff as
> + * ecc, otherwise we will get in trouble when doing subpage writes.
> + */

Comment formatting

s/ecc/ECC/ in plain English

> + if (memcmp(ecc_code, empty_block_ecc, ARRAY_SIZE(empty_block_ecc)) == 0)
> + memset(ecc_code, 0xff, ARRAY_SIZE(empty_block_ecc));
> +
> + return 0;
> +}
> +


Thanks,
Miquèl

2019-03-04 10:37:23

by Miquel Raynal

[permalink] [raw]
Subject: Re: [PATCH v4 8/9] mtd: rawnand: ingenic: Add support for the JZ4725B

Hi Paul,

Paul Cercueil <[email protected]> wrote on Sat, 9 Feb 2019 16:23:04
-0300:

> The boot ROM of the JZ4725B SoC expects a specific OOB layout on the
> NAND, so we use it unconditionally in the ingenic-nand driver.
>
> Also add the jz4725b-bch driver to support the JZ4725B-specific BCH
> hardware.
>
> Signed-off-by: Paul Cercueil <[email protected]>
> ---
>
> Changes:
>
> v2: Instead of forcing the OOB layout, leave it to the board code or
> devicetree to decide if the jz4725b-specific layout should be used
> or not.
>
> v3: - Revert the change in v2, as the previous behaviour was correct.
> - Also add support for the hardware BCH of the JZ4725B in this
> patch.
>
> v4: - Add MODULE_* macros
> - Add tweaks suggested by upstream feedback
>
> drivers/mtd/nand/raw/ingenic/Kconfig | 10 +
> drivers/mtd/nand/raw/ingenic/Makefile | 1 +
> drivers/mtd/nand/raw/ingenic/ingenic_nand.c | 48 ++++-
> drivers/mtd/nand/raw/ingenic/jz4725b_bch.c | 292 ++++++++++++++++++++++++++++
> 4 files changed, 350 insertions(+), 1 deletion(-)
> create mode 100644 drivers/mtd/nand/raw/ingenic/jz4725b_bch.c
>

[...]

> +static int jz4725b_calculate(struct ingenic_ecc *bch,
> + struct ingenic_ecc_params *params,
> + const u8 *buf, u8 *ecc_code)
> +{
> + int ret;
> +
> + mutex_lock(&bch->lock);
> + ret = jz4725b_bch_init(bch, params, true);

I really don't like this bch_init name. A BCH initialization is what is
supposed to be done only once (probably at boot time), can you find a
better name or a better organization of the correct/calculate path?

> + if (ret) {
> + dev_err(bch->dev, "Unable to init BCH with given parameters\n");
> + goto out_disable;
> + }
> +
> + jz4725b_bch_write_data(bch, buf, params->size);
> +
> + ret = jz4725b_bch_wait_complete(bch, BCH_BHINT_ENCF, NULL);
> + if (ret) {
> + dev_err(bch->dev, "timed out while calculating ECC\n");
> + goto out_disable;
> + }
> +
> + jz4725b_bch_read_parity(bch, ecc_code, params->bytes);
> +
> +out_disable:
> + jz4725b_bch_disable(bch);
> + mutex_unlock(&bch->lock);
> +
> + return ret;
> +}
> +

[...]

Thanks,
Miquèl

2019-03-04 18:46:48

by Paul Cercueil

[permalink] [raw]
Subject: Re: [PATCH v4 7/9] mtd: rawnand: ingenic: Add support for the JZ4740


On Mon, Mar 4, 2019 at 11:34 AM, Miquel Raynal
<[email protected]> wrote:
> Hi Paul,
>
> Paul Cercueil <[email protected] <mailto:[email protected]>>
> wrote on Sat, 9 Feb 2019 16:23:03
> -0300:
>
>> Add support for probing the ingenic-nand driver on the JZ4740 SoC
>> from
>> Ingenic, and the jz4740-ecc driver to support the JZ4740-specific
>> ECC hardware.
>>
>> Signed-off-by: Paul Cercueil <[email protected]
>> <mailto:[email protected]>>
>> ---
>>
>> Changes:
>>
>> v2: New patch
>>
>> v3: Also add support for the hardware ECC of the JZ4740 in this
>> patch
>>
>> v4: - Fix formatting issues
>> - Add MODULE_* macros
>>
>> drivers/mtd/nand/raw/ingenic/Kconfig | 10 ++
>> drivers/mtd/nand/raw/ingenic/Makefile | 1 +
>> drivers/mtd/nand/raw/ingenic/ingenic_nand.c | 48 +++++--
>> drivers/mtd/nand/raw/ingenic/jz4740_ecc.c | 196
>> ++++++++++++++++++++++++++++
>> 4 files changed, 244 insertions(+), 11 deletions(-)
>> create mode 100644 drivers/mtd/nand/raw/ingenic/jz4740_ecc.c
>>
>
> [...]
>
>> switch (chip->ecc.mode) {
>> case NAND_ECC_HW:
>> @@ -270,8 +279,8 @@ static int ingenic_nand_init_chip(struct
>> platform_device *pdev,
>> return -ENOMEM;
>> mtd->dev.parent = dev;
>>
>> - chip->legacy.IO_ADDR_R = cs->base + OFFSET_DATA;
>> - chip->legacy.IO_ADDR_W = cs->base + OFFSET_DATA;
>> + chip->legacy.IO_ADDR_R = cs->base + nfc->soc_info->data_offset;
>> + chip->legacy.IO_ADDR_W = cs->base + nfc->soc_info->data_offset;
>> chip->legacy.chip_delay = RB_DELAY_US;
>> chip->options = NAND_NO_SUBPAGE_WRITE;
>> chip->legacy.select_chip = ingenic_nand_select_chip;
>
> I think Boris already asked for it, but it would be really great that
> you update this driver to not use any legacy interface anymore.

I thought I'd send a patch later. But I don't mind doing the update in
this patchset.

>> diff --git a/drivers/mtd/nand/raw/ingenic/jz4740_ecc.c
>> b/drivers/mtd/nand/raw/ingenic/jz4740_ecc.c
>> new file mode 100644
>> index 000000000000..83b42881720e
>> --- /dev/null
>> +++ b/drivers/mtd/nand/raw/ingenic/jz4740_ecc.c
>> @@ -0,0 +1,196 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * JZ4740 ECC controller driver
>> + *
>> + * Copyright (c) 2019 Paul Cercueil <[email protected]
>> <mailto:[email protected]>>
>> + *
>> + * based on jz4740-nand.c
>> + */
>> +
>> +#include <linux/bitops.h>
>> +#include <linux/device.h>
>> +#include <linux/io.h>
>> +#include <linux/module.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/platform_device.h>
>> +
>> +#include "ingenic_ecc.h"
>> +
>> +#define JZ_REG_NAND_ECC_CTRL 0x00
>> +#define JZ_REG_NAND_DATA 0x04
>> +#define JZ_REG_NAND_PAR0 0x08
>> +#define JZ_REG_NAND_PAR1 0x0C
>> +#define JZ_REG_NAND_PAR2 0x10
>> +#define JZ_REG_NAND_IRQ_STAT 0x14
>> +#define JZ_REG_NAND_IRQ_CTRL 0x18
>> +#define JZ_REG_NAND_ERR(x) (0x1C + ((x) << 2))
>> +
>> +#define JZ_NAND_ECC_CTRL_PAR_READY BIT(4)
>> +#define JZ_NAND_ECC_CTRL_ENCODING BIT(3)
>> +#define JZ_NAND_ECC_CTRL_RS BIT(2)
>> +#define JZ_NAND_ECC_CTRL_RESET BIT(1)
>> +#define JZ_NAND_ECC_CTRL_ENABLE BIT(0)
>> +
>> +#define JZ_NAND_STATUS_ERR_COUNT (BIT(31) | BIT(30) | BIT(29))
>> +#define JZ_NAND_STATUS_PAD_FINISH BIT(4)
>> +#define JZ_NAND_STATUS_DEC_FINISH BIT(3)
>> +#define JZ_NAND_STATUS_ENC_FINISH BIT(2)
>> +#define JZ_NAND_STATUS_UNCOR_ERROR BIT(1)
>> +#define JZ_NAND_STATUS_ERROR BIT(0)
>> +
>> +static const uint8_t empty_block_ecc[] = {
>> + 0xcd, 0x9d, 0x90, 0x58, 0xf4, 0x8b, 0xff, 0xb7, 0x6f
>> +};
>> +
>> +static void jz4740_ecc_init(struct ingenic_ecc *ecc, bool encode)
>> +{
>> + uint32_t reg;
>> +
>> + /* Clear interrupt status */
>> + writel(0, ecc->base + JZ_REG_NAND_IRQ_STAT);
>> +
>> + /* Initialize and enable ECC hardware */
>> + reg = readl(ecc->base + JZ_REG_NAND_ECC_CTRL);
>> + reg |= JZ_NAND_ECC_CTRL_RESET;
>> + reg |= JZ_NAND_ECC_CTRL_ENABLE;
>> + reg |= JZ_NAND_ECC_CTRL_RS;
>> + if (encode)
>> + reg |= JZ_NAND_ECC_CTRL_ENCODING;
>> + else
>> + reg &= ~JZ_NAND_ECC_CTRL_ENCODING;
>> +
>> + writel(reg, ecc->base + JZ_REG_NAND_ECC_CTRL);
>> +}
>> +
>> +static int jz4740_ecc_calculate(struct ingenic_ecc *ecc,
>> + struct ingenic_ecc_params *params,
>> + const u8 *buf, u8 *ecc_code)
>> +{
>> + uint32_t reg, status;
>> + unsigned int timeout = 1000;
>> + int i;
>> +
>> + jz4740_ecc_init(ecc, true);
>> +
>> + do {
>> + status = readl(ecc->base + JZ_REG_NAND_IRQ_STAT);
>> + } while (!(status & JZ_NAND_STATUS_ENC_FINISH) && --timeout);
>> +
>> + if (timeout == 0)
>> + return -ETIMEDOUT;
>> +
>> + reg = readl(ecc->base + JZ_REG_NAND_ECC_CTRL);
>> + reg &= ~JZ_NAND_ECC_CTRL_ENABLE;
>> + writel(reg, ecc->base + JZ_REG_NAND_ECC_CTRL);
>> +
>> + for (i = 0; i < params->bytes; ++i)
>> + ecc_code[i] = readb(ecc->base + JZ_REG_NAND_PAR0 + i);
>> +
>> + /* If the written data is completely 0xff, we also want to write
>> 0xff as
>> + * ecc, otherwise we will get in trouble when doing subpage
>> writes.
>> + */
>
> Comment formatting
>
> s/ecc/ECC/ in plain English
>
>> + if (memcmp(ecc_code, empty_block_ecc,
>> ARRAY_SIZE(empty_block_ecc)) == 0)
>> + memset(ecc_code, 0xff, ARRAY_SIZE(empty_block_ecc));
>> +
>> + return 0;
>> +}
>> +
>
>
> Thanks,
> Miqu?l



2019-03-04 18:47:10

by Paul Cercueil

[permalink] [raw]
Subject: Re: [PATCH v4 6/9] mtd: rawnand: ingenic: Separate top-level and SoC specific code


On Mon, Mar 4, 2019 at 11:20 AM, Miquel Raynal
<[email protected]> wrote:
> Hi Paul,
>
> Paul Cercueil <[email protected] <mailto:[email protected]>>
> wrote on Sat, 9 Feb 2019 16:23:02
> -0300:
>
>> The ingenic-nand driver uses an API provided by the jz4780-bch
>> driver.
>> This makes it difficult to support other SoCs in the jz4780-bch
>> driver.
>> To work around this, we separate the API functions from the
>> SoC-specific
>> code, so that these API functions are SoC-agnostic.
>>
>
> I like the idea, actually I am working on this separation (see
> [1]) and I would really appreciate that you try to implement the
> interface when it will be available (v2 is coming this week, I think
> v3
> will be the one to test when raw NAND devices will be properly
> supported). I will add you in Cc: if you want to follow/review.
>
> [1]
> <http://lists.infradead.org/pipermail/linux-mtd/2019-February/087815.html>

Do you think this will be ready for 5.2?

You can add me in Cc:.

>> Signed-off-by: Paul Cercueil <[email protected]
>> <mailto:[email protected]>>
>> ---
>>
>> v2: Add an optional .probe() callback. It is used for instance to
>> set
>> the clock rate in the JZ4780 backend.
>>
>> v3: The common code is now inside the ingenic-ecc module. Each
>> SoC-specific ECC code is now in its own module, which leaves to
>> the
>> user the choice of which (if any) ECC code should be supported.
>>
>> v4: No change
>>
>> drivers/mtd/nand/raw/ingenic/Kconfig | 17 +++
>> drivers/mtd/nand/raw/ingenic/Makefile | 5 +-
>> drivers/mtd/nand/raw/ingenic/ingenic_ecc.c | 157
>> +++++++++++++++++++++++++
>> drivers/mtd/nand/raw/ingenic/ingenic_ecc.h | 84 ++++++++++++++
>> drivers/mtd/nand/raw/ingenic/ingenic_nand.c | 38 +++----
>> drivers/mtd/nand/raw/ingenic/jz4780_bch.c | 170
>> +++++-----------------------
>> drivers/mtd/nand/raw/ingenic/jz4780_bch.h | 40 -------
>> 7 files changed, 308 insertions(+), 203 deletions(-)
>> create mode 100644 drivers/mtd/nand/raw/ingenic/ingenic_ecc.c
>> create mode 100644 drivers/mtd/nand/raw/ingenic/ingenic_ecc.h
>> delete mode 100644 drivers/mtd/nand/raw/ingenic/jz4780_bch.h
>>
>
> [...]
>
>> diff --git a/drivers/mtd/nand/raw/ingenic/ingenic_nand.c
>> b/drivers/mtd/nand/raw/ingenic/ingenic_nand.c
>> index 8c73f7c5be9a..0f51fd15fe79 100644
>> --- a/drivers/mtd/nand/raw/ingenic/ingenic_nand.c
>> +++ b/drivers/mtd/nand/raw/ingenic/ingenic_nand.c
>> @@ -22,7 +22,7 @@
>>
>> #include <linux/jz4780-nemc.h>
>>
>> -#include "jz4780_bch.h"
>> +#include "ingenic_ecc.h"
>>
>> #define DRV_NAME "ingenic-nand"
>>
>> @@ -40,7 +40,7 @@ struct ingenic_nand_cs {
>>
>> struct ingenic_nfc {
>> struct device *dev;
>> - struct jz4780_bch *bch;
>> + struct ingenic_ecc *ecc;
>> struct nand_controller controller;
>> unsigned int num_banks;
>> struct list_head chips;
>> @@ -124,10 +124,10 @@ static int ingenic_nand_ecc_calculate(struct
>> nand_chip *chip, const u8 *dat,
>> {
>> struct ingenic_nand *nand = to_ingenic_nand(nand_to_mtd(chip));
>> struct ingenic_nfc *nfc = to_ingenic_nfc(nand->chip.controller);
>> - struct jz4780_bch_params params;
>> + struct ingenic_ecc_params params;
>>
>> /*
>> - * Don't need to generate the ECC when reading, BCH does it for
>> us as
>> + * Don't need to generate the ECC when reading, ECC does it for
>> us as
>
> "the ECC engine does it for us" would be more meaningful.

OK.

>> * part of decoding/correction.
>> */
>> if (nand->reading)
>> @@ -137,7 +137,7 @@ static int ingenic_nand_ecc_calculate(struct
>> nand_chip *chip, const u8 *dat,
>> params.bytes = nand->chip.ecc.bytes;
>> params.strength = nand->chip.ecc.strength;
>>
>> - return jz4780_bch_calculate(nfc->bch, &params, dat, ecc_code);
>> + return ingenic_ecc_calculate(nfc->ecc, &params, dat, ecc_code);
>> }
>>
>
> Thanks,
> Miqu?l



2019-03-04 18:53:04

by Miquel Raynal

[permalink] [raw]
Subject: Re: [PATCH v4 1/9] dt-bindings: mtd: ingenic: Add compatible strings for JZ4740 and JZ4725B

Hi Paul,

> >> --- a/Documentation/devicetree/bindings/mtd/ingenic,jz4780-nand.txt
> >> +++ b/Documentation/devicetree/bindings/mtd/ingenic,jz4780-nand.txt
> >> @@ -6,7 +6,10 @@ memory-controllers/ingenic,jz4780-nemc.txt), and >> thus NAND device nodes must
> >> be children of the NEMC node.
> >> >> Required NAND controller device properties:
> >> -- compatible: Should be set to "ingenic,jz4780-nand".
> >> +- compatible: Should be one of:
> >> + * ingenic,jz4740-nand
> >> + * ingenic,jz4725b-nand
> >> + * ingenic,jz4780-nand
> >
> > Wouldn't "-nand-controller" suffix be better? Of course in the driver
> > you should still check for jz4780-nand.
>
> So I would be compatible with:
> * ingenic,jz4740-nand-controller
> * ingenic,jz4725b-nand-controller
> * ingenic,jz4780-nand
> ?

From a driver POV I would even prefer ingenic,jz4780-nand-controller. I
don't know what's best here. Maybe Boris or Rob can help?


Thanks,
Miquèl

2019-03-04 18:59:50

by Miquel Raynal

[permalink] [raw]
Subject: Re: [PATCH v4 2/9] dt-bindings: mtd: ingenic: Change 'BCH' to 'ECC' in documentation

Hi Paul,

Paul Cercueil <[email protected]> wrote on Mon, 04 Mar 2019 19:23:50
+0100:

> On Mon, Mar 4, 2019 at 10:50 AM, Miquel Raynal <[email protected]> wrote:
> > Hi Paul,
> >
> > Paul Cercueil <[email protected] <mailto:[email protected]>> > wrote on Sat, 9 Feb 2019 16:22:58
> > -0300:
> >
> >> The JZ4740 ECC hardware is not BCH but Reed-Solomon, so it makes >> more
> >> sense to use the more generic ECC term.
> >> >> Signed-off-by: Paul Cercueil <[email protected] >> <mailto:[email protected]>>
> >> ---
> >> >> Changes:
> >> >> v3: New patch
> >> >> v4: No change
> >> >> .../devicetree/bindings/mtd/ingenic,jz4780-nand.txt | 18 >> +++++++++---------
> >> 1 file changed, 9 insertions(+), 9 deletions(-)
> >> >> diff --git >> a/Documentation/devicetree/bindings/mtd/ingenic,jz4780-nand.txt >> b/Documentation/devicetree/bindings/mtd/ingenic,jz4780-nand.txt
> >> index a5b940f18bf6..5a45cc54f46d 100644
> >> --- a/Documentation/devicetree/bindings/mtd/ingenic,jz4780-nand.txt
> >> +++ b/Documentation/devicetree/bindings/mtd/ingenic,jz4780-nand.txt
> >> @@ -1,4 +1,4 @@
> >> -* Ingenic JZ4780 NAND/BCH
> >> +* Ingenic JZ4780 NAND/ECC
> >> >> This file documents the device tree bindings for NAND flash >> devices on the
> >> JZ4780. NAND devices are connected to the NEMC controller >> (described in
> >> @@ -14,10 +14,10 @@ Required NAND controller device properties:
> >> an offset of 0 and a size of 0x1000000 (i.e. the whole NEMC >> bank).
> >> >> Optional NAND controller device properties:
> >> -- ingenic,bch-controller: To make use of the hardware BCH >> controller, this
> >> - property must contain a phandle for the BCH controller node. The >> required
> >> +- ingenic,bch-controller: To make use of the hardware ECC >> controller, this
> >> + property must contain a phandle for the ECC controller node. The >> required
> >
> > I think there is already a 'ecc-engine' property used by MTK and Atmel
> > NAND controllers to point to the ECC engine block. Please use this
> > property instead of the ingenic specific one.
>
> ingenic,bch-controller is already in the devicetree ABI. I can't change it now...

Oh, right...

Well, same thing as before, maybe you can change it and keep the driver
backward compatible? I don't want new DT to use this property because
it is not generic and we must have a way from the core to find the ECC
engine handle when the controller does not embed one itself (thus, the
ecc-engine property which is around for quite some time already).

Thanks,
Miquèl

2019-03-04 19:06:52

by Miquel Raynal

[permalink] [raw]
Subject: Re: [PATCH v4 6/9] mtd: rawnand: ingenic: Separate top-level and SoC specific code

Hi Paul,

Paul Cercueil <[email protected]> wrote on Mon, 04 Mar 2019 19:26:08
+0100:

> On Mon, Mar 4, 2019 at 11:20 AM, Miquel Raynal <[email protected]> wrote:
> > Hi Paul,
> >
> > Paul Cercueil <[email protected] <mailto:[email protected]>> > wrote on Sat, 9 Feb 2019 16:23:02
> > -0300:
> >
> >> The ingenic-nand driver uses an API provided by the jz4780-bch >> driver.
> >> This makes it difficult to support other SoCs in the jz4780-bch >> driver.
> >> To work around this, we separate the API functions from the >> SoC-specific
> >> code, so that these API functions are SoC-agnostic.
> >> >
> > I like the idea, actually I am working on this separation (see
> > [1]) and I would really appreciate that you try to implement the
> > interface when it will be available (v2 is coming this week, I think > v3
> > will be the one to test when raw NAND devices will be properly
> > supported). I will add you in Cc: if you want to follow/review.
> >
> > [1] > <http://lists.infradead.org/pipermail/linux-mtd/2019-February/087815.html
> >
>
> Do you think this will be ready for 5.2?

Maybe, but I can't tell for sure. It will depend on how invasive the
raw NAND conversion is. I don't want to delay your work but maybe once
the interface will be ready to be implemented it would be a great
opportunity to do it with the Ingenic driver.

>
> You can add me in Cc:.

Thanks!
Miquèl

2019-03-04 19:08:30

by Miquel Raynal

[permalink] [raw]
Subject: Re: [PATCH v4 7/9] mtd: rawnand: ingenic: Add support for the JZ4740

Hi Paul,

Paul Cercueil <[email protected]> wrote on Mon, 04 Mar 2019 19:28:49
+0100:

> On Mon, Mar 4, 2019 at 11:34 AM, Miquel Raynal <[email protected]> wrote:
> > Hi Paul,
> >
> > Paul Cercueil <[email protected] <mailto:[email protected]>> > wrote on Sat, 9 Feb 2019 16:23:03
> > -0300:
> >
> >> Add support for probing the ingenic-nand driver on the JZ4740 SoC >> from
> >> Ingenic, and the jz4740-ecc driver to support the JZ4740-specific
> >> ECC hardware.
> >> >> Signed-off-by: Paul Cercueil <[email protected] >> <mailto:[email protected]>>
> >> ---
> >> >> Changes:
> >> >> v2: New patch
> >> >> v3: Also add support for the hardware ECC of the JZ4740 in this >> patch
> >> >> v4: - Fix formatting issues
> >> - Add MODULE_* macros
> >> >> drivers/mtd/nand/raw/ingenic/Kconfig | 10 ++
> >> drivers/mtd/nand/raw/ingenic/Makefile | 1 +
> >> drivers/mtd/nand/raw/ingenic/ingenic_nand.c | 48 +++++--
> >> drivers/mtd/nand/raw/ingenic/jz4740_ecc.c | 196 >> ++++++++++++++++++++++++++++
> >> 4 files changed, 244 insertions(+), 11 deletions(-)
> >> create mode 100644 drivers/mtd/nand/raw/ingenic/jz4740_ecc.c
> >> >
> > [...]
> >
> >> switch (chip->ecc.mode) {
> >> case NAND_ECC_HW:
> >> @@ -270,8 +279,8 @@ static int ingenic_nand_init_chip(struct >> platform_device *pdev,
> >> return -ENOMEM;
> >> mtd->dev.parent = dev;
> >> >> - chip->legacy.IO_ADDR_R = cs->base + OFFSET_DATA;
> >> - chip->legacy.IO_ADDR_W = cs->base + OFFSET_DATA;
> >> + chip->legacy.IO_ADDR_R = cs->base + nfc->soc_info->data_offset;
> >> + chip->legacy.IO_ADDR_W = cs->base + nfc->soc_info->data_offset;
> >> chip->legacy.chip_delay = RB_DELAY_US;
> >> chip->options = NAND_NO_SUBPAGE_WRITE;
> >> chip->legacy.select_chip = ingenic_nand_select_chip;
> >
> > I think Boris already asked for it, but it would be really great that
> > you update this driver to not use any legacy interface anymore.
>
> I thought I'd send a patch later. But I don't mind doing the update in
> this patchset.

Great! No it's okay, don't delay this patch set, doing it in another
thread is fine.


2019-03-04 19:11:45

by Miquel Raynal

[permalink] [raw]
Subject: Re: [PATCH v4 8/9] mtd: rawnand: ingenic: Add support for the JZ4725B

Hi Paul,

Paul Cercueil <[email protected]> wrote on Mon, 04 Mar 2019 19:30:04
+0100:

> On Mon, Mar 4, 2019 at 11:35 AM, Miquel Raynal <[email protected]> wrote:
> > Hi Paul,
> >
> > Paul Cercueil <[email protected] <mailto:[email protected]>> > wrote on Sat, 9 Feb 2019 16:23:04
> > -0300:
> >
> >> The boot ROM of the JZ4725B SoC expects a specific OOB layout on the
> >> NAND, so we use it unconditionally in the ingenic-nand driver.
> >> >> Also add the jz4725b-bch driver to support the JZ4725B-specific BCH
> >> hardware.
> >> >> Signed-off-by: Paul Cercueil <[email protected] >> <mailto:[email protected]>>
> >> ---
> >> >> Changes:
> >> >> v2: Instead of forcing the OOB layout, leave it to the board code or
> >> devicetree to decide if the jz4725b-specific layout should be >> used
> >> or not.
> >> >> v3: - Revert the change in v2, as the previous behaviour was >> correct.
> >> - Also add support for the hardware BCH of the JZ4725B in this
> >> patch.
> >> >> v4: - Add MODULE_* macros
> >> - Add tweaks suggested by upstream feedback
> >> >> drivers/mtd/nand/raw/ingenic/Kconfig | 10 +
> >> drivers/mtd/nand/raw/ingenic/Makefile | 1 +
> >> drivers/mtd/nand/raw/ingenic/ingenic_nand.c | 48 ++++-
> >> drivers/mtd/nand/raw/ingenic/jz4725b_bch.c | 292 >> ++++++++++++++++++++++++++++
> >> 4 files changed, 350 insertions(+), 1 deletion(-)
> >> create mode 100644 drivers/mtd/nand/raw/ingenic/jz4725b_bch.c
> >> >
> > [...]
> >
> >> +static int jz4725b_calculate(struct ingenic_ecc *bch,
> >> + struct ingenic_ecc_params *params,
> >> + const u8 *buf, u8 *ecc_code)
> >> +{
> >> + int ret;
> >> +
> >> + mutex_lock(&bch->lock);
> >> + ret = jz4725b_bch_init(bch, params, true);
> >
> > I really don't like this bch_init name. A BCH initialization is what > is
> > supposed to be done only once (probably at boot time), can you find a
> > better name or a better organization of the correct/calculate path?
>
> jz4725b_bch_setup() maybe?

Unless I am not understanding what this does, I don't get why you would
need to do this setup everytime you want to use the ECC engine. Are you
sure this is needed?


2019-03-04 19:23:23

by Paul Cercueil

[permalink] [raw]
Subject: Re: [PATCH v4 1/9] dt-bindings: mtd: ingenic: Add compatible strings for JZ4740 and JZ4725B

Hi Miquel,

On Mon, Mar 4, 2019 at 10:45 AM, Miquel Raynal
<[email protected]> wrote:
> Hi Paul,
>
> Paul Cercueil <[email protected] <mailto:[email protected]>>
> wrote on Sat, 9 Feb 2019 16:22:57
> -0300:
>
>> Add compatible strings to probe the jz4780-nand and jz4780-bch
>> drivers
>> from devicetree on the JZ4725B and JZ4740 SoCs from Ingenic.
>>
>> Signed-off-by: Paul Cercueil <[email protected]
>> <mailto:[email protected]>>
>> ---
>>
>> Changes:
>>
>> v2: - Change 'ingenic,jz4725b-nand' compatible string to
>> 'ingenic,jz4740-nand' to reflect driver change
>> - Add 'ingenic,jz4740-bch' compatible string
>> - Document 'ingenic,oob-layout' property
>>
>> v3: - Removed 'ingenic,oob-layout' property
>> - Update compatible strings to what the driver supports
>>
>> v4: No change
>>
>> Documentation/devicetree/bindings/mtd/ingenic,jz4780-nand.txt | 10
>> ++++++++--
>> 1 file changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git
>> a/Documentation/devicetree/bindings/mtd/ingenic,jz4780-nand.txt
>> b/Documentation/devicetree/bindings/mtd/ingenic,jz4780-nand.txt
>> index 29ea5853ca91..a5b940f18bf6 100644
>> --- a/Documentation/devicetree/bindings/mtd/ingenic,jz4780-nand.txt
>> +++ b/Documentation/devicetree/bindings/mtd/ingenic,jz4780-nand.txt
>> @@ -6,7 +6,10 @@ memory-controllers/ingenic,jz4780-nemc.txt), and
>> thus NAND device nodes must
>> be children of the NEMC node.
>>
>> Required NAND controller device properties:
>> -- compatible: Should be set to "ingenic,jz4780-nand".
>> +- compatible: Should be one of:
>> + * ingenic,jz4740-nand
>> + * ingenic,jz4725b-nand
>> + * ingenic,jz4780-nand
>
> Wouldn't "-nand-controller" suffix be better? Of course in the driver
> you should still check for jz4780-nand.

So I would be compatible with:
* ingenic,jz4740-nand-controller
* ingenic,jz4725b-nand-controller
* ingenic,jz4780-nand
?

>> - reg: For each bank with a NAND chip attached, should specify a
>> bank number,
>> an offset of 0 and a size of 0x1000000 (i.e. the whole NEMC
>> bank).
>>
>> @@ -72,7 +75,10 @@ NAND devices. The following is a description of
>> the device properties for a
>> BCH controller.
>>
>> Required BCH properties:
>> -- compatible: Should be set to "ingenic,jz4780-bch".
>> +- compatible: Should be one of:
>> + * ingenic,jz4740-ecc
>> + * ingenic,jz4725b-bch
>> + * ingenic,jz4780-bch
>> - reg: Should specify the BCH controller registers location and
>> length.
>> - clocks: Clock for the BCH controller.
>>
>
> Thanks,
> Miqu?l



2019-03-04 19:23:27

by Paul Cercueil

[permalink] [raw]
Subject: Re: [PATCH v4 2/9] dt-bindings: mtd: ingenic: Change 'BCH' to 'ECC' in documentation


On Mon, Mar 4, 2019 at 10:50 AM, Miquel Raynal
<[email protected]> wrote:
> Hi Paul,
>
> Paul Cercueil <[email protected] <mailto:[email protected]>>
> wrote on Sat, 9 Feb 2019 16:22:58
> -0300:
>
>> The JZ4740 ECC hardware is not BCH but Reed-Solomon, so it makes
>> more
>> sense to use the more generic ECC term.
>>
>> Signed-off-by: Paul Cercueil <[email protected]
>> <mailto:[email protected]>>
>> ---
>>
>> Changes:
>>
>> v3: New patch
>>
>> v4: No change
>>
>> .../devicetree/bindings/mtd/ingenic,jz4780-nand.txt | 18
>> +++++++++---------
>> 1 file changed, 9 insertions(+), 9 deletions(-)
>>
>> diff --git
>> a/Documentation/devicetree/bindings/mtd/ingenic,jz4780-nand.txt
>> b/Documentation/devicetree/bindings/mtd/ingenic,jz4780-nand.txt
>> index a5b940f18bf6..5a45cc54f46d 100644
>> --- a/Documentation/devicetree/bindings/mtd/ingenic,jz4780-nand.txt
>> +++ b/Documentation/devicetree/bindings/mtd/ingenic,jz4780-nand.txt
>> @@ -1,4 +1,4 @@
>> -* Ingenic JZ4780 NAND/BCH
>> +* Ingenic JZ4780 NAND/ECC
>>
>> This file documents the device tree bindings for NAND flash
>> devices on the
>> JZ4780. NAND devices are connected to the NEMC controller
>> (described in
>> @@ -14,10 +14,10 @@ Required NAND controller device properties:
>> an offset of 0 and a size of 0x1000000 (i.e. the whole NEMC
>> bank).
>>
>> Optional NAND controller device properties:
>> -- ingenic,bch-controller: To make use of the hardware BCH
>> controller, this
>> - property must contain a phandle for the BCH controller node. The
>> required
>> +- ingenic,bch-controller: To make use of the hardware ECC
>> controller, this
>> + property must contain a phandle for the ECC controller node. The
>> required
>
> I think there is already a 'ecc-engine' property used by MTK and Atmel
> NAND controllers to point to the ECC engine block. Please use this
> property instead of the ingenic specific one.

ingenic,bch-controller is already in the devicetree ABI. I can't change
it now...

>> properties for this node are described below. If this is not
>> specified,
>> - software BCH will be used instead.
>> + software ECC will be used instead.
>>
>> Optional children nodes:
>> - Individual NAND chips are children of the NAND controller node.
>> @@ -70,17 +70,17 @@ nemc: nemc@13410000 {
>> };
>> };
>>
>> -The BCH controller is a separate SoC component used for error
>> correction on
>> +The ECC controller is a separate SoC component used for error
>> correction on
>> NAND devices. The following is a description of the device
>> properties for a
>> -BCH controller.
>> +ECC controller.
>>
>> -Required BCH properties:
>> +Required ECC properties:
>> - compatible: Should be one of:
>> * ingenic,jz4740-ecc
>> * ingenic,jz4725b-bch
>> * ingenic,jz4780-bch
>> -- reg: Should specify the BCH controller registers location and
>> length.
>> -- clocks: Clock for the BCH controller.
>> +- reg: Should specify the ECC controller registers location and
>> length.
>> +- clocks: Clock for the ECC controller.
>>
>> Example:
>>
>
> Thanks,
> Miqu?l



2019-03-04 19:24:49

by Paul Cercueil

[permalink] [raw]
Subject: Re: [PATCH v4 8/9] mtd: rawnand: ingenic: Add support for the JZ4725B


On Mon, Mar 4, 2019 at 11:35 AM, Miquel Raynal
<[email protected]> wrote:
> Hi Paul,
>
> Paul Cercueil <[email protected] <mailto:[email protected]>>
> wrote on Sat, 9 Feb 2019 16:23:04
> -0300:
>
>> The boot ROM of the JZ4725B SoC expects a specific OOB layout on the
>> NAND, so we use it unconditionally in the ingenic-nand driver.
>>
>> Also add the jz4725b-bch driver to support the JZ4725B-specific BCH
>> hardware.
>>
>> Signed-off-by: Paul Cercueil <[email protected]
>> <mailto:[email protected]>>
>> ---
>>
>> Changes:
>>
>> v2: Instead of forcing the OOB layout, leave it to the board code or
>> devicetree to decide if the jz4725b-specific layout should be
>> used
>> or not.
>>
>> v3: - Revert the change in v2, as the previous behaviour was
>> correct.
>> - Also add support for the hardware BCH of the JZ4725B in this
>> patch.
>>
>> v4: - Add MODULE_* macros
>> - Add tweaks suggested by upstream feedback
>>
>> drivers/mtd/nand/raw/ingenic/Kconfig | 10 +
>> drivers/mtd/nand/raw/ingenic/Makefile | 1 +
>> drivers/mtd/nand/raw/ingenic/ingenic_nand.c | 48 ++++-
>> drivers/mtd/nand/raw/ingenic/jz4725b_bch.c | 292
>> ++++++++++++++++++++++++++++
>> 4 files changed, 350 insertions(+), 1 deletion(-)
>> create mode 100644 drivers/mtd/nand/raw/ingenic/jz4725b_bch.c
>>
>
> [...]
>
>> +static int jz4725b_calculate(struct ingenic_ecc *bch,
>> + struct ingenic_ecc_params *params,
>> + const u8 *buf, u8 *ecc_code)
>> +{
>> + int ret;
>> +
>> + mutex_lock(&bch->lock);
>> + ret = jz4725b_bch_init(bch, params, true);
>
> I really don't like this bch_init name. A BCH initialization is what
> is
> supposed to be done only once (probably at boot time), can you find a
> better name or a better organization of the correct/calculate path?

jz4725b_bch_setup() maybe?

>> + if (ret) {
>> + dev_err(bch->dev, "Unable to init BCH with given parameters\n");
>> + goto out_disable;
>> + }
>> +
>> + jz4725b_bch_write_data(bch, buf, params->size);
>> +
>> + ret = jz4725b_bch_wait_complete(bch, BCH_BHINT_ENCF, NULL);
>> + if (ret) {
>> + dev_err(bch->dev, "timed out while calculating ECC\n");
>> + goto out_disable;
>> + }
>> +
>> + jz4725b_bch_read_parity(bch, ecc_code, params->bytes);
>> +
>> +out_disable:
>> + jz4725b_bch_disable(bch);
>> + mutex_unlock(&bch->lock);
>> +
>> + return ret;
>> +}
>> +
>
> [...]
>
> Thanks,
> Miqu?l



2019-03-13 12:47:27

by Paul Cercueil

[permalink] [raw]
Subject: Re: [PATCH v4 8/9] mtd: rawnand: ingenic: Add support for the JZ4725B

Hi Miquel,

Le lun. 4 mars 2019 ? 16:09, Miquel Raynal <[email protected]>
a ?crit :
> Hi Paul,
>
> Paul Cercueil <[email protected]> wrote on Mon, 04 Mar 2019
> 19:30:04
> +0100:
>
>> On Mon, Mar 4, 2019 at 11:35 AM, Miquel Raynal
>> <[email protected]> wrote:
>> > Hi Paul,
>> >
>> > Paul Cercueil <[email protected]
>> <mailto:[email protected]>> > wrote on Sat, 9 Feb 2019 16:23:04
>> > -0300:
>> >
>> >> The boot ROM of the JZ4725B SoC expects a specific OOB layout
>> on the
>> >> NAND, so we use it unconditionally in the ingenic-nand driver.
>> >> >> Also add the jz4725b-bch driver to support the
>> JZ4725B-specific BCH
>> >> hardware.
>> >> >> Signed-off-by: Paul Cercueil <[email protected] >>
>> <mailto:[email protected]>>
>> >> ---
>> >> >> Changes:
>> >> >> v2: Instead of forcing the OOB layout, leave it to the board
>> code or
>> >> devicetree to decide if the jz4725b-specific layout should
>> be >> used
>> >> or not.
>> >> >> v3: - Revert the change in v2, as the previous behaviour was
>> >> correct.
>> >> - Also add support for the hardware BCH of the JZ4725B in
>> this
>> >> patch.
>> >> >> v4: - Add MODULE_* macros
>> >> - Add tweaks suggested by upstream feedback
>> >> >> drivers/mtd/nand/raw/ingenic/Kconfig | 10 +
>> >> drivers/mtd/nand/raw/ingenic/Makefile | 1 +
>> >> drivers/mtd/nand/raw/ingenic/ingenic_nand.c | 48 ++++-
>> >> drivers/mtd/nand/raw/ingenic/jz4725b_bch.c | 292 >>
>> ++++++++++++++++++++++++++++
>> >> 4 files changed, 350 insertions(+), 1 deletion(-)
>> >> create mode 100644 drivers/mtd/nand/raw/ingenic/jz4725b_bch.c
>> >> >
>> > [...]
>> >
>> >> +static int jz4725b_calculate(struct ingenic_ecc *bch,
>> >> + struct ingenic_ecc_params *params,
>> >> + const u8 *buf, u8 *ecc_code)
>> >> +{
>> >> + int ret;
>> >> +
>> >> + mutex_lock(&bch->lock);
>> >> + ret = jz4725b_bch_init(bch, params, true);
>> >
>> > I really don't like this bch_init name. A BCH initialization is
>> what > is
>> > supposed to be done only once (probably at boot time), can you
>> find a
>> > better name or a better organization of the correct/calculate
>> path?
>>
>> jz4725b_bch_setup() maybe?
>
> Unless I am not understanding what this does, I don't get why you
> would
> need to do this setup everytime you want to use the ECC engine. Are
> you
> sure this is needed?

It configures the hardware for a new ECC encoding or decoding sequence,
so
yes, it has to be done everytime.

-Paul


2019-03-13 12:56:36

by Paul Cercueil

[permalink] [raw]
Subject: Re: [PATCH v4 1/9] dt-bindings: mtd: ingenic: Add compatible strings for JZ4740 and JZ4725B

Hi,

Le lun. 4 mars 2019 ? 15:51, Miquel Raynal <[email protected]>
a ?crit :
> Hi Paul,
>
>> >> ---
>> a/Documentation/devicetree/bindings/mtd/ingenic,jz4780-nand.txt
>> >> +++
>> b/Documentation/devicetree/bindings/mtd/ingenic,jz4780-nand.txt
>> >> @@ -6,7 +6,10 @@ memory-controllers/ingenic,jz4780-nemc.txt),
>> and >> thus NAND device nodes must
>> >> be children of the NEMC node.
>> >> >> Required NAND controller device properties:
>> >> -- compatible: Should be set to "ingenic,jz4780-nand".
>> >> +- compatible: Should be one of:
>> >> + * ingenic,jz4740-nand
>> >> + * ingenic,jz4725b-nand
>> >> + * ingenic,jz4780-nand
>> >
>> > Wouldn't "-nand-controller" suffix be better? Of course in the
>> driver
>> > you should still check for jz4780-nand.
>>
>> So I would be compatible with:
>> * ingenic,jz4740-nand-controller
>> * ingenic,jz4725b-nand-controller
>> * ingenic,jz4780-nand
>> ?
>
> From a driver POV I would even prefer ingenic,jz4780-nand-controller.
> I
> don't know what's best here. Maybe Boris or Rob can help?

The "ingenic,jz4780-nand" compatible string is already out there and
used
in devicetree files, so I wouldn't change it just for the sake of it.

-Paul


2019-03-13 13:02:04

by Paul Cercueil

[permalink] [raw]
Subject: Re: [PATCH v4 2/9] dt-bindings: mtd: ingenic: Change 'BCH' to 'ECC' in documentation



Le lun. 4 mars 2019 ? 15:58, Miquel Raynal <[email protected]>
a ?crit :
> Hi Paul,
>
> Paul Cercueil <[email protected]> wrote on Mon, 04 Mar 2019
> 19:23:50
> +0100:
>
>> On Mon, Mar 4, 2019 at 10:50 AM, Miquel Raynal
>> <[email protected]> wrote:
>> > Hi Paul,
>> >
>> > Paul Cercueil <[email protected]
>> <mailto:[email protected]>> > wrote on Sat, 9 Feb 2019 16:22:58
>> > -0300:
>> >
>> >> The JZ4740 ECC hardware is not BCH but Reed-Solomon, so it
>> makes >> more
>> >> sense to use the more generic ECC term.
>> >> >> Signed-off-by: Paul Cercueil <[email protected] >>
>> <mailto:[email protected]>>
>> >> ---
>> >> >> Changes:
>> >> >> v3: New patch
>> >> >> v4: No change
>> >> >> .../devicetree/bindings/mtd/ingenic,jz4780-nand.txt | 18
>> >> +++++++++---------
>> >> 1 file changed, 9 insertions(+), 9 deletions(-)
>> >> >> diff --git >>
>> a/Documentation/devicetree/bindings/mtd/ingenic,jz4780-nand.txt >>
>> b/Documentation/devicetree/bindings/mtd/ingenic,jz4780-nand.txt
>> >> index a5b940f18bf6..5a45cc54f46d 100644
>> >> ---
>> a/Documentation/devicetree/bindings/mtd/ingenic,jz4780-nand.txt
>> >> +++
>> b/Documentation/devicetree/bindings/mtd/ingenic,jz4780-nand.txt
>> >> @@ -1,4 +1,4 @@
>> >> -* Ingenic JZ4780 NAND/BCH
>> >> +* Ingenic JZ4780 NAND/ECC
>> >> >> This file documents the device tree bindings for NAND flash
>> >> devices on the
>> >> JZ4780. NAND devices are connected to the NEMC controller >>
>> (described in
>> >> @@ -14,10 +14,10 @@ Required NAND controller device properties:
>> >> an offset of 0 and a size of 0x1000000 (i.e. the whole NEMC
>> >> bank).
>> >> >> Optional NAND controller device properties:
>> >> -- ingenic,bch-controller: To make use of the hardware BCH >>
>> controller, this
>> >> - property must contain a phandle for the BCH controller node.
>> The >> required
>> >> +- ingenic,bch-controller: To make use of the hardware ECC >>
>> controller, this
>> >> + property must contain a phandle for the ECC controller node.
>> The >> required
>> >
>> > I think there is already a 'ecc-engine' property used by MTK and
>> Atmel
>> > NAND controllers to point to the ECC engine block. Please use this
>> > property instead of the ingenic specific one.
>>
>> ingenic,bch-controller is already in the devicetree ABI. I can't
>> change it now...
>
> Oh, right...
>
> Well, same thing as before, maybe you can change it and keep the
> driver
> backward compatible? I don't want new DT to use this property because
> it is not generic and we must have a way from the core to find the ECC
> engine handle when the controller does not embed one itself (thus, the
> ecc-engine property which is around for quite some time already).

I can do that. Will the new ECC framework work with the "ecc-engine"
property?

-Paul


2019-03-13 13:11:18

by Boris Brezillon

[permalink] [raw]
Subject: Re: [PATCH v4 1/9] dt-bindings: mtd: ingenic: Add compatible strings for JZ4740 and JZ4725B

On Wed, 13 Mar 2019 09:55:34 -0300
Paul Cercueil <[email protected]> wrote:

> Hi,
>
> Le lun. 4 mars 2019 à 15:51, Miquel Raynal <[email protected]>
> a écrit :
> > Hi Paul,
> >
> >> >> ---
> >> a/Documentation/devicetree/bindings/mtd/ingenic,jz4780-nand.txt
> >> >> +++
> >> b/Documentation/devicetree/bindings/mtd/ingenic,jz4780-nand.txt
> >> >> @@ -6,7 +6,10 @@ memory-controllers/ingenic,jz4780-nemc.txt),
> >> and >> thus NAND device nodes must
> >> >> be children of the NEMC node.
> >> >> >> Required NAND controller device properties:
> >> >> -- compatible: Should be set to "ingenic,jz4780-nand".
> >> >> +- compatible: Should be one of:
> >> >> + * ingenic,jz4740-nand
> >> >> + * ingenic,jz4725b-nand
> >> >> + * ingenic,jz4780-nand
> >> >
> >> > Wouldn't "-nand-controller" suffix be better? Of course in the
> >> driver
> >> > you should still check for jz4780-nand.
> >>
> >> So I would be compatible with:
> >> * ingenic,jz4740-nand-controller
> >> * ingenic,jz4725b-nand-controller
> >> * ingenic,jz4780-nand
> >> ?
> >
> > From a driver POV I would even prefer ingenic,jz4780-nand-controller.
> > I
> > don't know what's best here. Maybe Boris or Rob can help?

Let's keep it consistent and have all compatibles follow the old
naming scheme (ingenic,<soc>-nand). But yes, for new drivers, I agree
that -nand-controller is better than just -nand.


2019-03-13 13:14:00

by Miquel Raynal

[permalink] [raw]
Subject: Re: [PATCH v4 2/9] dt-bindings: mtd: ingenic: Change 'BCH' to 'ECC' in documentation

Hi Paul,

Paul Cercueil <[email protected]> wrote on Wed, 13 Mar 2019 09:59:48
-0300:

> Le lun. 4 mars 2019 à 15:58, Miquel Raynal <[email protected]> a écrit :
> > Hi Paul,
> >
> > Paul Cercueil <[email protected]> wrote on Mon, 04 Mar 2019 > 19:23:50
> > +0100:
> >
> >> On Mon, Mar 4, 2019 at 10:50 AM, Miquel Raynal >> <[email protected]> wrote:
> >> > Hi Paul,
> >> >
> >> > Paul Cercueil <[email protected] >> <mailto:[email protected]>> > wrote on Sat, 9 Feb 2019 16:22:58
> >> > -0300:
> >> >
> >> >> The JZ4740 ECC hardware is not BCH but Reed-Solomon, so it >> makes >> more
> >> >> sense to use the more generic ECC term.
> >> >> >> Signed-off-by: Paul Cercueil <[email protected] >> >> <mailto:[email protected]>>
> >> >> ---
> >> >> >> Changes:
> >> >> >> v3: New patch
> >> >> >> v4: No change
> >> >> >> .../devicetree/bindings/mtd/ingenic,jz4780-nand.txt | 18 >> >> +++++++++---------
> >> >> 1 file changed, 9 insertions(+), 9 deletions(-)
> >> >> >> diff --git >> >> a/Documentation/devicetree/bindings/mtd/ingenic,jz4780-nand.txt >> >> b/Documentation/devicetree/bindings/mtd/ingenic,jz4780-nand.txt
> >> >> index a5b940f18bf6..5a45cc54f46d 100644
> >> >> --- >> a/Documentation/devicetree/bindings/mtd/ingenic,jz4780-nand.txt
> >> >> +++ >> b/Documentation/devicetree/bindings/mtd/ingenic,jz4780-nand.txt
> >> >> @@ -1,4 +1,4 @@
> >> >> -* Ingenic JZ4780 NAND/BCH
> >> >> +* Ingenic JZ4780 NAND/ECC
> >> >> >> This file documents the device tree bindings for NAND flash >> >> devices on the
> >> >> JZ4780. NAND devices are connected to the NEMC controller >> >> (described in
> >> >> @@ -14,10 +14,10 @@ Required NAND controller device properties:
> >> >> an offset of 0 and a size of 0x1000000 (i.e. the whole NEMC >> >> bank).
> >> >> >> Optional NAND controller device properties:
> >> >> -- ingenic,bch-controller: To make use of the hardware BCH >> >> controller, this
> >> >> - property must contain a phandle for the BCH controller node. >> The >> required
> >> >> +- ingenic,bch-controller: To make use of the hardware ECC >> >> controller, this
> >> >> + property must contain a phandle for the ECC controller node. >> The >> required
> >> >
> >> > I think there is already a 'ecc-engine' property used by MTK and >> Atmel
> >> > NAND controllers to point to the ECC engine block. Please use this
> >> > property instead of the ingenic specific one.
> >> >> ingenic,bch-controller is already in the devicetree ABI. I can't >> change it now...
> >
> > Oh, right...
> >
> > Well, same thing as before, maybe you can change it and keep the > driver
> > backward compatible? I don't want new DT to use this property because
> > it is not generic and we must have a way from the core to find the ECC
> > engine handle when the controller does not embed one itself (thus, the
> > ecc-engine property which is around for quite some time already).
>
> I can do that. Will the new ECC framework work with the "ecc-engine" property?

Yes.

Right now it does not, but when supporting raw NAND devices, it will
have to.


Thanks,
Miquèl