[PATCH v2 0/9] Ingenic JZ4780 NAND patchset v2

[PATCH v2 0/9] Ingenic JZ4780 NAND patchset v2

[Paul Cercueil]

Hi,

As requested by Boris, I added a patch to move all the Ingenic NAND
drivers to their own directory.

In this V2 I added support for the JZ4740 SoC. The combo of the
jz4780-nemc, jz4780-nand and jz4740-bch now obsolete the old and dusty
jz4740-nand driver.

To support the only upstream JZ4740-based board we have, the Ben
Nanonote, I added an option to specify the OOB layout of that device
from a device property string.

Finally, I dropped the last two patches that moved the platform NAND
code to devicetree; I will upstream them as part of a different
patchset.

Cheers,
-Paul

[PATCH v2 1/9] mtd: rawnand: Move drivers for Ingenic SoCs to subfolder

[Paul Cercueil]

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 <paul@crapouillou.net>
---

Changes:

v2: New patch

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

[PATCH v2 2/9] dt-bindings: mtd: ingenic: Add compatible strings for the JZ4740

[Paul Cercueil]

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 <paul@crapouillou.net>
---

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

 .../devicetree/bindings/mtd/ingenic,jz4780-nand.txt | 13 +++++++++++--
 1 file changed, 11 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..294b56a2098a 100644
--- a/Documentation/devicetree/bindings/mtd/ingenic,jz4780-nand.txt
+++ b/Documentation/devicetree/bindings/mtd/ingenic,jz4780-nand.txt
@@ -6,7 +6,9 @@ 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,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).
 
@@ -15,6 +17,10 @@ Optional NAND controller device properties:
   property must contain a phandle for the BCH controller node. The required
   properties for this node are described below. If this is not specified,
   software BCH will be used instead.
+- ingenic,oob-layout: Specify the OOB layout to replace the default one.
+  Valid values are:
+  * ingenic,jz4725b
+  * qi,lb60
 
 Optional children nodes:
 - Individual NAND chips are children of the NAND controller node.
@@ -72,7 +78,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-bch
+  * ingenic,jz4725b-bch
+  * ingenic,jz4780-bch
 - reg: Should specify the BCH controller registers location and length.
 - clocks: Clock for the BCH controller.
 
-- 
2.20.1

Re: [PATCH v2 2/9] dt-bindings: mtd: ingenic: Add compatible strings for the JZ4740

[Rob Herring]

On Sat,  2 Feb 2019 20:19:19 -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 <paul@crapouillou.net>
> ---
> 
> 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
> 
>  .../devicetree/bindings/mtd/ingenic,jz4780-nand.txt | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 

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

[PATCH v2 3/9] mtd: rawnand: jz4780: Use SPDX license notifiers

[Paul Cercueil]

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

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
Reviewed-by: Boris Brezillon <bbrezillon@kernel.org>
---

Changes:

v2: 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 <alex.smith@imgtec.com>
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms of the GNU General Public License version 2 as published
- * by the Free Software Foundation.
  */
 
 #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 <alex.smith@imgtec.com>
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms of the GNU General Public License version 2 as published
- * by the Free Software Foundation.
  */
 
 #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 <alex.smith@imgtec.com>
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms of the GNU General Public License version 2 as published
- * by the Free Software Foundation.
  */
 
 #include <linux/delay.h>
-- 
2.20.1

[PATCH v2 4/9] mtd: rawnand: jz4780: Add support for the JZ4740

[Paul Cercueil]

Add support for probing the jz4780-nand driver on the JZ4740 SoC from
Ingenic.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---

Changes:

v2: - Add support for the JZ4740 and not the JZ4725B: they behave the
      same, and JZ4740 is fully upstream while JZ4725B is not. The
      JZ4725B devicetree will then simply use the "ingenic,jz4740-nand"
      compatible string.
    - Fix the number of bytes for the ECC when the ECC strength is 4.
      This is needed for the JZ4740, which uses Reed-Solomon instead of
      BCH.

 drivers/mtd/nand/raw/ingenic/jz4780_nand.c | 48 +++++++++++++++++-----
 1 file changed, 37 insertions(+), 11 deletions(-)

diff --git a/drivers/mtd/nand/raw/ingenic/jz4780_nand.c b/drivers/mtd/nand/raw/ingenic/jz4780_nand.c
index 7f55358b860f..c0855fef7735 100644
--- a/drivers/mtd/nand/raw/ingenic/jz4780_nand.c
+++ b/drivers/mtd/nand/raw/ingenic/jz4780_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	"jz4780-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 jz4780_nand_cs {
 	unsigned int bank;
 	void __iomem *base;
@@ -40,6 +43,7 @@ struct jz4780_nand_cs {
 
 struct jz4780_nand_controller {
 	struct device *dev;
+	const struct jz_soc_info *soc_info;
 	struct jz4780_bch *bch;
 	struct nand_controller controller;
 	unsigned int num_banks;
@@ -101,9 +105,9 @@ static void jz4780_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 jz4780_nand_dev_ready(struct nand_chip *chip)
@@ -161,8 +165,13 @@ static int jz4780_nand_attach_chip(struct nand_chip *chip)
 	struct jz4780_nand_controller *nfc = to_jz4780_nand_controller(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:
@@ -272,8 +281,8 @@ static int jz4780_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 = jz4780_nand_select_chip;
@@ -353,6 +362,10 @@ static int jz4780_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 BCH HW before we call nand_scan_ident, to prevent us from
 	 * having to call it again if the BCH driver returns -EPROBE_DEFER.
@@ -390,8 +403,21 @@ static int jz4780_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 jz4780_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, jz4780_nand_dt_match);
-- 
2.20.1

Re: [PATCH v2 4/9] mtd: rawnand: jz4780: Add support for the JZ4740

[Boris Brezillon]

On Sat,  2 Feb 2019 20:19:21 -0300
Paul Cercueil <paul@crapouillou.net> wrote:

> Add support for probing the jz4780-nand driver on the JZ4740 SoC from
> Ingenic.
> 
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---
> 
> Changes:
> 
> v2: - Add support for the JZ4740 and not the JZ4725B: they behave the
>       same, and JZ4740 is fully upstream while JZ4725B is not. The
>       JZ4725B devicetree will then simply use the "ingenic,jz4740-nand"
>       compatible string.
>     - Fix the number of bytes for the ECC when the ECC strength is 4.
>       This is needed for the JZ4740, which uses Reed-Solomon instead of
>       BCH.
> 
>  drivers/mtd/nand/raw/ingenic/jz4780_nand.c | 48 +++++++++++++++++-----

If we're going to make the driver compatible with jz4740 and jz4725b
maybe we should rename the source files jz47xx_{nand,bch}.{c,h}.

>  1 file changed, 37 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/ingenic/jz4780_nand.c b/drivers/mtd/nand/raw/ingenic/jz4780_nand.c
> index 7f55358b860f..c0855fef7735 100644
> --- a/drivers/mtd/nand/raw/ingenic/jz4780_nand.c
> +++ b/drivers/mtd/nand/raw/ingenic/jz4780_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	"jz4780-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 jz4780_nand_cs {
>  	unsigned int bank;
>  	void __iomem *base;
> @@ -40,6 +43,7 @@ struct jz4780_nand_cs {
>  
>  struct jz4780_nand_controller {
>  	struct device *dev;
> +	const struct jz_soc_info *soc_info;
>  	struct jz4780_bch *bch;
>  	struct nand_controller controller;
>  	unsigned int num_banks;
> @@ -101,9 +105,9 @@ static void jz4780_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 jz4780_nand_dev_ready(struct nand_chip *chip)
> @@ -161,8 +165,13 @@ static int jz4780_nand_attach_chip(struct nand_chip *chip)
>  	struct jz4780_nand_controller *nfc = to_jz4780_nand_controller(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:
> @@ -272,8 +281,8 @@ static int jz4780_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 = jz4780_nand_select_chip;
> @@ -353,6 +362,10 @@ static int jz4780_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 BCH HW before we call nand_scan_ident, to prevent us from
>  	 * having to call it again if the BCH driver returns -EPROBE_DEFER.
> @@ -390,8 +403,21 @@ static int jz4780_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 jz4780_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, jz4780_nand_dt_match);

Re: [PATCH v2 4/9] mtd: rawnand: jz4780: Add support for the JZ4740

[Paul Cercueil]

Le dim. 3 févr. 2019 à 4:31, Boris Brezillon <bbrezillon@kernel.org> 
a écrit :
> On Sat,  2 Feb 2019 20:19:21 -0300
> Paul Cercueil <paul@crapouillou.net> wrote:
> 
>>  Add support for probing the jz4780-nand driver on the JZ4740 SoC 
>> from
>>  Ingenic.
>> 
>>  Signed-off-by: Paul Cercueil <paul@crapouillou.net>
>>  ---
>> 
>>  Changes:
>> 
>>  v2: - Add support for the JZ4740 and not the JZ4725B: they behave 
>> the
>>        same, and JZ4740 is fully upstream while JZ4725B is not. The
>>        JZ4725B devicetree will then simply use the 
>> "ingenic,jz4740-nand"
>>        compatible string.
>>      - Fix the number of bytes for the ECC when the ECC strength is 
>> 4.
>>        This is needed for the JZ4740, which uses Reed-Solomon 
>> instead of
>>        BCH.
>> 
>>   drivers/mtd/nand/raw/ingenic/jz4780_nand.c | 48 
>> +++++++++++++++++-----
> 
> If we're going to make the driver compatible with jz4740 and jz4725b
> maybe we should rename the source files jz47xx_{nand,bch}.{c,h}.

I don't know about that. Adding support for new hardware isn't a good 
reason to
rename the driver, or so I've been told around here, as you then make 
it harder
to review the git history of the driver.

>>   1 file changed, 37 insertions(+), 11 deletions(-)
>> 
>>  diff --git a/drivers/mtd/nand/raw/ingenic/jz4780_nand.c 
>> b/drivers/mtd/nand/raw/ingenic/jz4780_nand.c
>>  index 7f55358b860f..c0855fef7735 100644
>>  --- a/drivers/mtd/nand/raw/ingenic/jz4780_nand.c
>>  +++ b/drivers/mtd/nand/raw/ingenic/jz4780_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	"jz4780-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 jz4780_nand_cs {
>>   	unsigned int bank;
>>   	void __iomem *base;
>>  @@ -40,6 +43,7 @@ struct jz4780_nand_cs {
>> 
>>   struct jz4780_nand_controller {
>>   	struct device *dev;
>>  +	const struct jz_soc_info *soc_info;
>>   	struct jz4780_bch *bch;
>>   	struct nand_controller controller;
>>   	unsigned int num_banks;
>>  @@ -101,9 +105,9 @@ static void jz4780_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 jz4780_nand_dev_ready(struct nand_chip *chip)
>>  @@ -161,8 +165,13 @@ static int jz4780_nand_attach_chip(struct 
>> nand_chip *chip)
>>   	struct jz4780_nand_controller *nfc = 
>> to_jz4780_nand_controller(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:
>>  @@ -272,8 +281,8 @@ static int jz4780_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 = jz4780_nand_select_chip;
>>  @@ -353,6 +362,10 @@ static int jz4780_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 BCH HW before we call nand_scan_ident, to prevent us 
>> from
>>   	 * having to call it again if the BCH driver returns 
>> -EPROBE_DEFER.
>>  @@ -390,8 +403,21 @@ static int jz4780_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 jz4780_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, jz4780_nand_dt_match);
> 

Re: [PATCH v2 4/9] mtd: rawnand: jz4780: Add support for the JZ4740

[Boris Brezillon]

On Sun, 03 Feb 2019 10:56:53 -0300
Paul Cercueil <paul@crapouillou.net> wrote:

> Le dim. 3 févr. 2019 à 4:31, Boris Brezillon <bbrezillon@kernel.org> 
> a écrit :
> > On Sat,  2 Feb 2019 20:19:21 -0300
> > Paul Cercueil <paul@crapouillou.net> wrote:
> >   
> >>  Add support for probing the jz4780-nand driver on the JZ4740 SoC 
> >> from
> >>  Ingenic.
> >> 
> >>  Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> >>  ---
> >> 
> >>  Changes:
> >> 
> >>  v2: - Add support for the JZ4740 and not the JZ4725B: they behave 
> >> the
> >>        same, and JZ4740 is fully upstream while JZ4725B is not. The
> >>        JZ4725B devicetree will then simply use the 
> >> "ingenic,jz4740-nand"
> >>        compatible string.
> >>      - Fix the number of bytes for the ECC when the ECC strength is 
> >> 4.
> >>        This is needed for the JZ4740, which uses Reed-Solomon 
> >> instead of
> >>        BCH.
> >> 
> >>   drivers/mtd/nand/raw/ingenic/jz4780_nand.c | 48 
> >> +++++++++++++++++-----  
> > 
> > If we're going to make the driver compatible with jz4740 and jz4725b
> > maybe we should rename the source files jz47xx_{nand,bch}.{c,h}.  
> 
> I don't know about that. Adding support for new hardware isn't a good 
> reason to
> rename the driver, or so I've been told around here, as you then make 
> it harder
> to review the git history of the driver.

You already move files to a sub-directory so that doesn't make a huge
difference, history will be hard to follow because of this move anyway.

Re: [PATCH v2 4/9] mtd: rawnand: jz4780: Add support for the JZ4740

[Paul Cercueil]

Le dim. 3 févr. 2019 à 11:08, Boris Brezillon <bbrezillon@kernel.org> 
a écrit :
> On Sun, 03 Feb 2019 10:56:53 -0300
> Paul Cercueil <paul@crapouillou.net> wrote:
> 
>>  Le dim. 3 févr. 2019 à 4:31, Boris Brezillon 
>> <bbrezillon@kernel.org>
>>  a écrit :
>>  > On Sat,  2 Feb 2019 20:19:21 -0300
>>  > Paul Cercueil <paul@crapouillou.net> wrote:
>>  >
>>  >>  Add support for probing the jz4780-nand driver on the JZ4740 SoC
>>  >> from
>>  >>  Ingenic.
>>  >>
>>  >>  Signed-off-by: Paul Cercueil <paul@crapouillou.net>
>>  >>  ---
>>  >>
>>  >>  Changes:
>>  >>
>>  >>  v2: - Add support for the JZ4740 and not the JZ4725B: they 
>> behave
>>  >> the
>>  >>        same, and JZ4740 is fully upstream while JZ4725B is not. 
>> The
>>  >>        JZ4725B devicetree will then simply use the
>>  >> "ingenic,jz4740-nand"
>>  >>        compatible string.
>>  >>      - Fix the number of bytes for the ECC when the ECC strength 
>> is
>>  >> 4.
>>  >>        This is needed for the JZ4740, which uses Reed-Solomon
>>  >> instead of
>>  >>        BCH.
>>  >>
>>  >>   drivers/mtd/nand/raw/ingenic/jz4780_nand.c | 48
>>  >> +++++++++++++++++-----
>>  >
>>  > If we're going to make the driver compatible with jz4740 and 
>> jz4725b
>>  > maybe we should rename the source files jz47xx_{nand,bch}.{c,h}.
>> 
>>  I don't know about that. Adding support for new hardware isn't a 
>> good
>>  reason to
>>  rename the driver, or so I've been told around here, as you then 
>> make
>>  it harder
>>  to review the git history of the driver.
> 
> You already move files to a sub-directory so that doesn't make a huge
> difference, history will be hard to follow because of this move 
> anyway.

Yes, but if I merge the *_bch.c files together, and eventually drop
jz4740-nand.c, does it still make sense to move to a sub-directory?

Re: [PATCH v2 4/9] mtd: rawnand: jz4780: Add support for the JZ4740

[Boris Brezillon]

On Sun, 03 Feb 2019 11:10:50 -0300
Paul Cercueil <paul@crapouillou.net> wrote:

> Le dim. 3 févr. 2019 à 11:08, Boris Brezillon <bbrezillon@kernel.org> 
> a écrit :
> > On Sun, 03 Feb 2019 10:56:53 -0300
> > Paul Cercueil <paul@crapouillou.net> wrote:
> >   
> >>  Le dim. 3 févr. 2019 à 4:31, Boris Brezillon 
> >> <bbrezillon@kernel.org>
> >>  a écrit :  
> >>  > On Sat,  2 Feb 2019 20:19:21 -0300
> >>  > Paul Cercueil <paul@crapouillou.net> wrote:
> >>  >  
> >>  >>  Add support for probing the jz4780-nand driver on the JZ4740 SoC
> >>  >> from
> >>  >>  Ingenic.
> >>  >>
> >>  >>  Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> >>  >>  ---
> >>  >>
> >>  >>  Changes:
> >>  >>
> >>  >>  v2: - Add support for the JZ4740 and not the JZ4725B: they   
> >> behave  
> >>  >> the
> >>  >>        same, and JZ4740 is fully upstream while JZ4725B is not.   
> >> The  
> >>  >>        JZ4725B devicetree will then simply use the
> >>  >> "ingenic,jz4740-nand"
> >>  >>        compatible string.
> >>  >>      - Fix the number of bytes for the ECC when the ECC strength   
> >> is  
> >>  >> 4.
> >>  >>        This is needed for the JZ4740, which uses Reed-Solomon
> >>  >> instead of
> >>  >>        BCH.
> >>  >>
> >>  >>   drivers/mtd/nand/raw/ingenic/jz4780_nand.c | 48
> >>  >> +++++++++++++++++-----  
> >>  >
> >>  > If we're going to make the driver compatible with jz4740 and   
> >> jz4725b  
> >>  > maybe we should rename the source files jz47xx_{nand,bch}.{c,h}.  
> >> 
> >>  I don't know about that. Adding support for new hardware isn't a 
> >> good
> >>  reason to
> >>  rename the driver, or so I've been told around here, as you then 
> >> make
> >>  it harder
> >>  to review the git history of the driver.  
> > 
> > You already move files to a sub-directory so that doesn't make a huge
> > difference, history will be hard to follow because of this move 
> > anyway.  
> 
> Yes, but if I merge the *_bch.c files together, and eventually drop
> jz4740-nand.c, does it still make sense to move to a sub-directory?
> 

Still prefer to make things explicit over preserving file names to
make git log <path> history linear. And anyway, how hard is it to figure
out that the last commit in git log drivers/mtd/nand/raw/<driver>.c is
moving the file (there might even be an option for that)?

[PATCH v2 5/9] mtd: rawnand: jz4780: Add ooblayout for the JZ4725B

[Paul Cercueil]

The boot ROM of the JZ4725B SoC expects a specific OOB layout on the
NAND.

Add an optional "ingenic,oob-layout" device property. When set to
"ingenic,jz4725b", this specific OOB layout is used.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---

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.

 drivers/mtd/nand/raw/ingenic/jz4780_nand.c | 51 +++++++++++++++++++++-
 1 file changed, 50 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/raw/ingenic/jz4780_nand.c b/drivers/mtd/nand/raw/ingenic/jz4780_nand.c
index c0855fef7735..baebb9a5c7c8 100644
--- a/drivers/mtd/nand/raw/ingenic/jz4780_nand.c
+++ b/drivers/mtd/nand/raw/ingenic/jz4780_nand.c
@@ -44,6 +44,7 @@ struct jz4780_nand_cs {
 struct jz4780_nand_controller {
 	struct device *dev;
 	const struct jz_soc_info *soc_info;
+	const struct mtd_ooblayout_ops *oob_layout;
 	struct jz4780_bch *bch;
 	struct nand_controller controller;
 	unsigned int num_banks;
@@ -213,7 +214,7 @@ static int jz4780_nand_attach_chip(struct nand_chip *chip)
 		return -EINVAL;
 	}
 
-	mtd_set_ooblayout(mtd, &nand_ooblayout_lp_ops);
+	mtd_set_ooblayout(mtd, nfc->oob_layout);
 
 	return 0;
 }
@@ -345,11 +346,47 @@ static int jz4780_nand_init_chips(struct jz4780_nand_controller *nfc,
 	return 0;
 }
 
+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 int jz4780_nand_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	unsigned int num_banks;
 	struct jz4780_nand_controller *nfc;
+	const char *layout;
 	int ret;
 
 	num_banks = jz4780_nemc_num_banks(dev);
@@ -366,6 +403,18 @@ static int jz4780_nand_probe(struct platform_device *pdev)
 	if (!nfc->soc_info)
 		return -EINVAL;
 
+	nfc->oob_layout = &nand_ooblayout_lp_ops;
+
+	ret = device_property_read_string(dev, "ingenic,oob-layout", &layout);
+	if (!ret) {
+		if (!strcmp(layout, "ingenic,jz4725b")) {
+			nfc->oob_layout  = &jz4725b_ooblayout_ops;
+		} else {
+			dev_err(dev, "Unrecognized OOB layout %s\n", layout);
+			return -EINVAL;
+		}
+	}
+
 	/*
 	 * 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.
-- 
2.20.1

Re: [PATCH v2 5/9] mtd: rawnand: jz4780: Add ooblayout for the JZ4725B

[Boris Brezillon]

On Sat,  2 Feb 2019 20:19:22 -0300
Paul Cercueil <paul@crapouillou.net> wrote:

> The boot ROM of the JZ4725B SoC expects a specific OOB layout on the
> NAND.
> 
> Add an optional "ingenic,oob-layout" device property. When set to
> "ingenic,jz4725b", this specific OOB layout is used.

It's a SoC-specific layout, please add a compatible for ingenic,jz4725b
and use it to determine the layout to use.

> 
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---
> 
> 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.
> 
>  drivers/mtd/nand/raw/ingenic/jz4780_nand.c | 51 +++++++++++++++++++++-
>  1 file changed, 50 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/nand/raw/ingenic/jz4780_nand.c b/drivers/mtd/nand/raw/ingenic/jz4780_nand.c
> index c0855fef7735..baebb9a5c7c8 100644
> --- a/drivers/mtd/nand/raw/ingenic/jz4780_nand.c
> +++ b/drivers/mtd/nand/raw/ingenic/jz4780_nand.c
> @@ -44,6 +44,7 @@ struct jz4780_nand_cs {
>  struct jz4780_nand_controller {
>  	struct device *dev;
>  	const struct jz_soc_info *soc_info;
> +	const struct mtd_ooblayout_ops *oob_layout;
>  	struct jz4780_bch *bch;
>  	struct nand_controller controller;
>  	unsigned int num_banks;
> @@ -213,7 +214,7 @@ static int jz4780_nand_attach_chip(struct nand_chip *chip)
>  		return -EINVAL;
>  	}
>  
> -	mtd_set_ooblayout(mtd, &nand_ooblayout_lp_ops);
> +	mtd_set_ooblayout(mtd, nfc->oob_layout);
>  
>  	return 0;
>  }
> @@ -345,11 +346,47 @@ static int jz4780_nand_init_chips(struct jz4780_nand_controller *nfc,
>  	return 0;
>  }
>  
> +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 int jz4780_nand_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
>  	unsigned int num_banks;
>  	struct jz4780_nand_controller *nfc;
> +	const char *layout;
>  	int ret;
>  
>  	num_banks = jz4780_nemc_num_banks(dev);
> @@ -366,6 +403,18 @@ static int jz4780_nand_probe(struct platform_device *pdev)
>  	if (!nfc->soc_info)
>  		return -EINVAL;
>  
> +	nfc->oob_layout = &nand_ooblayout_lp_ops;
> +
> +	ret = device_property_read_string(dev, "ingenic,oob-layout", &layout);
> +	if (!ret) {
> +		if (!strcmp(layout, "ingenic,jz4725b")) {
> +			nfc->oob_layout  = &jz4725b_ooblayout_ops;
> +		} else {
> +			dev_err(dev, "Unrecognized OOB layout %s\n", layout);
> +			return -EINVAL;
> +		}
> +	}
> +
>  	/*
>  	 * 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.

[PATCH v2 6/9] mtd: rawnand: jz4780: Add ooblayout for the Qi Ben Nanonote

[Paul Cercueil]

The Ben Nanonote from Qi Hardware expects a specific OOB layout on its
NAND. If the "ingenic,oob-layout" device property is set to "qi,lb60",
this specific OOB layout is used.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---

Changes:

v2: New patch

 drivers/mtd/nand/raw/ingenic/jz4780_nand.c | 37 ++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/drivers/mtd/nand/raw/ingenic/jz4780_nand.c b/drivers/mtd/nand/raw/ingenic/jz4780_nand.c
index baebb9a5c7c8..4b304eceae8d 100644
--- a/drivers/mtd/nand/raw/ingenic/jz4780_nand.c
+++ b/drivers/mtd/nand/raw/ingenic/jz4780_nand.c
@@ -346,6 +346,41 @@ static int jz4780_nand_init_chips(struct jz4780_nand_controller *nfc,
 	return 0;
 }
 
+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)
 {
@@ -409,6 +444,8 @@ static int jz4780_nand_probe(struct platform_device *pdev)
 	if (!ret) {
 		if (!strcmp(layout, "ingenic,jz4725b")) {
 			nfc->oob_layout  = &jz4725b_ooblayout_ops;
+		} else if (!strcmp(layout, "qi,lb60")) {
+			nfc->oob_layout = &qi_lb60_ooblayout_ops;
 		} else {
 			dev_err(dev, "Unrecognized OOB layout %s\n", layout);
 			return -EINVAL;
-- 
2.20.1

Re: [PATCH v2 6/9] mtd: rawnand: jz4780: Add ooblayout for the Qi Ben Nanonote

[Boris Brezillon]

On Sat,  2 Feb 2019 20:19:23 -0300
Paul Cercueil <paul@crapouillou.net> wrote:

> The Ben Nanonote from Qi Hardware expects a specific OOB layout on its
> NAND. If the "ingenic,oob-layout" device property is set to "qi,lb60",
> this specific OOB layout is used.

I'm really not a big fan of this ingenic,oob-layout property, it
encourages people to use new custom layouts which is just a pain to
maintain. I understand that we don't have the choice for this board as
it's already upstream, but maybe we can avoid adding this prop and
check the root compat (which should contain the board name). 

> 
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---
> 
> Changes:
> 
> v2: New patch
> 
>  drivers/mtd/nand/raw/ingenic/jz4780_nand.c | 37 ++++++++++++++++++++++
>  1 file changed, 37 insertions(+)
> 
> diff --git a/drivers/mtd/nand/raw/ingenic/jz4780_nand.c b/drivers/mtd/nand/raw/ingenic/jz4780_nand.c
> index baebb9a5c7c8..4b304eceae8d 100644
> --- a/drivers/mtd/nand/raw/ingenic/jz4780_nand.c
> +++ b/drivers/mtd/nand/raw/ingenic/jz4780_nand.c
> @@ -346,6 +346,41 @@ static int jz4780_nand_init_chips(struct jz4780_nand_controller *nfc,
>  	return 0;
>  }
>  
> +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)
>  {
> @@ -409,6 +444,8 @@ static int jz4780_nand_probe(struct platform_device *pdev)
>  	if (!ret) {
>  		if (!strcmp(layout, "ingenic,jz4725b")) {
>  			nfc->oob_layout  = &jz4725b_ooblayout_ops;
> +		} else if (!strcmp(layout, "qi,lb60")) {
> +			nfc->oob_layout = &qi_lb60_ooblayout_ops;
>  		} else {
>  			dev_err(dev, "Unrecognized OOB layout %s\n", layout);
>  			return -EINVAL;

[PATCH v2 7/9] mtd: rawnand: jz4780-bch: Separate top-level and SoC specific code

[Paul Cercueil]

The jz4780-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 <paul@crapouillou.net>
---

Changes:

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

 drivers/mtd/nand/raw/ingenic/Makefile         |   3 +-
 drivers/mtd/nand/raw/ingenic/jz4780_bch.c     | 170 ++---------------
 .../mtd/nand/raw/ingenic/jz4780_bch_common.c  | 177 ++++++++++++++++++
 .../nand/raw/ingenic/jz4780_bch_internal.h    |  35 ++++
 4 files changed, 230 insertions(+), 155 deletions(-)
 create mode 100644 drivers/mtd/nand/raw/ingenic/jz4780_bch_common.c
 create mode 100644 drivers/mtd/nand/raw/ingenic/jz4780_bch_internal.h

diff --git a/drivers/mtd/nand/raw/ingenic/Makefile b/drivers/mtd/nand/raw/ingenic/Makefile
index 44c2ca053d24..c308062a6620 100644
--- a/drivers/mtd/nand/raw/ingenic/Makefile
+++ b/drivers/mtd/nand/raw/ingenic/Makefile
@@ -1,2 +1,3 @@
 obj-$(CONFIG_MTD_NAND_JZ4740) += jz4740_nand.o
-obj-$(CONFIG_MTD_NAND_JZ4780) += jz4780_nand.o jz4780_bch.o
+obj-$(CONFIG_MTD_NAND_JZ4780) += jz4780_nand.o jz4780_bch_common.o \
+				 jz4780_bch.o
diff --git a/drivers/mtd/nand/raw/ingenic/jz4780_bch.c b/drivers/mtd/nand/raw/ingenic/jz4780_bch.c
index 7e4e5e627603..1b4486c38c6b 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 backend code for the jz4780-bch driver
  *
  * Copyright (c) 2015 Imagination Technologies
  * Author: Alex Smith <alex.smith@imgtec.com>
@@ -8,18 +8,13 @@
 
 #include <linux/bitops.h>
 #include <linux/clk.h>
-#include <linux/delay.h>
-#include <linux/init.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 <linux/device.h>
 
 #include "jz4780_bch.h"
+#include "jz4780_bch_internal.h"
 
 #define BCH_BHCR			0x0
 #define BCH_BHCCR			0x8
@@ -62,13 +57,6 @@
 /* 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)
 {
@@ -167,18 +155,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 jz4780_bch *bch,
+			    struct jz4780_bch_params *params,
+			    const u8 *buf, u8 *ecc_code)
 {
 	int ret = 0;
 
@@ -197,23 +176,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 jz4780_bch *bch,
+			  struct jz4780_bch_params *params,
+			  u8 *buf, u8 *ecc_code)
 {
 	u32 reg, mask, index;
 	int i, ret, count;
@@ -259,121 +225,17 @@ 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 jz4780_bch *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 of_device_id jz4780_bch_dt_match[] = {
-	{ .compatible = "ingenic,jz4780-bch" },
-	{},
+const struct jz4780_bch_ops jz4780_bch_jz4780_ops = {
+	.probe = jz4780_bch_probe,
+	.disable = jz4780_bch_disable,
+	.calculate = jz4780_calculate,
+	.correct = jz4780_correct,
 };
-MODULE_DEVICE_TABLE(of, jz4780_bch_dt_match);
-
-static struct platform_driver jz4780_bch_driver = {
-	.probe		= jz4780_bch_probe,
-	.driver	= {
-		.name	= "jz4780-bch",
-		.of_match_table = of_match_ptr(jz4780_bch_dt_match),
-	},
-};
-module_platform_driver(jz4780_bch_driver);
-
-MODULE_AUTHOR("Alex Smith <alex@alex-smith.me.uk>");
-MODULE_AUTHOR("Harvey Hunt <harveyhuntnexus@gmail.com>");
-MODULE_DESCRIPTION("Ingenic JZ4780 BCH error correction driver");
-MODULE_LICENSE("GPL v2");
diff --git a/drivers/mtd/nand/raw/ingenic/jz4780_bch_common.c b/drivers/mtd/nand/raw/ingenic/jz4780_bch_common.c
new file mode 100644
index 000000000000..1c1fc418ce8e
--- /dev/null
+++ b/drivers/mtd/nand/raw/ingenic/jz4780_bch_common.c
@@ -0,0 +1,177 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * JZ4780 BCH controller
+ *
+ * Copyright (c) 2015 Imagination Technologies
+ * Author: Alex Smith <alex.smith@imgtec.com>
+ */
+
+#include <linux/clk.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+
+#include "jz4780_bch_internal.h"
+#include "jz4780_bch.h"
+
+/**
+ * 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)
+{
+	return bch->ops->calculate(bch, params, buf, ecc_code);
+}
+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)
+{
+	return bch->ops->correct(bch, params, buf, ecc_code);
+}
+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)
+{
+	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;
+	int ret = 0;
+
+	bch = devm_kzalloc(dev, sizeof(*bch), GFP_KERNEL);
+	if (!bch)
+		return -ENOMEM;
+
+	bch->ops = device_get_match_data(dev);
+	if (!bch->ops)
+		return -EINVAL;
+
+	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);
+
+	bch->ops->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);
+	}
+
+	mutex_init(&bch->lock);
+
+	bch->dev = dev;
+	platform_set_drvdata(pdev, bch);
+
+	if (bch->ops->probe)
+		ret = bch->ops->probe(bch);
+
+	return ret;
+}
+
+static const struct of_device_id jz4780_bch_dt_match[] = {
+	{ .compatible = "ingenic,jz4780-bch", .data = &jz4780_bch_jz4780_ops },
+	{},
+};
+MODULE_DEVICE_TABLE(of, jz4780_bch_dt_match);
+
+static struct platform_driver jz4780_bch_driver = {
+	.probe		= jz4780_bch_probe,
+	.driver	= {
+		.name	= "jz4780-bch",
+		.of_match_table = of_match_ptr(jz4780_bch_dt_match),
+	},
+};
+module_platform_driver(jz4780_bch_driver);
+
+MODULE_AUTHOR("Alex Smith <alex@alex-smith.me.uk>");
+MODULE_AUTHOR("Harvey Hunt <harveyhuntnexus@gmail.com>");
+MODULE_DESCRIPTION("Ingenic JZ4780 BCH error correction driver");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/mtd/nand/raw/ingenic/jz4780_bch_internal.h b/drivers/mtd/nand/raw/ingenic/jz4780_bch_internal.h
new file mode 100644
index 000000000000..f109b3c8d039
--- /dev/null
+++ b/drivers/mtd/nand/raw/ingenic/jz4780_bch_internal.h
@@ -0,0 +1,35 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __DRIVERS_MTD_NAND_JZ4780_BCH_INTERNAL_H__
+#define __DRIVERS_MTD_NAND_JZ4780_BCH_INTERNAL_H__
+
+#include <linux/compiler_types.h>
+#include <linux/mutex.h>
+#include <linux/types.h>
+
+struct jz4780_bch_params;
+struct jz4780_bch;
+struct device;
+struct clk;
+
+struct jz4780_bch_ops {
+	int (*probe)(struct jz4780_bch *bch);
+	void (*disable)(struct jz4780_bch *bch);
+	int (*calculate)(struct jz4780_bch *bch,
+			 struct jz4780_bch_params *params,
+			 const u8 *buf, u8 *ecc_code);
+	int (*correct)(struct jz4780_bch *bch,
+			struct jz4780_bch_params *params,
+			u8 *buf, u8 *ecc_code);
+};
+
+struct jz4780_bch {
+	struct device *dev;
+	const struct jz4780_bch_ops *ops;
+	void __iomem *base;
+	struct clk *clk;
+	struct mutex lock;
+};
+
+extern const struct jz4780_bch_ops jz4780_bch_jz4780_ops;
+
+#endif /* __DRIVERS_MTD_NAND_JZ4780_BCH_INTERNAL_H__ */
-- 
2.20.1

[PATCH v2 8/9] mtd: rawnand: jz4780-bch: Add support for the JZ4725B

[Paul Cercueil]

Add the backend code for the jz4780-bch driver to support the JZ4725B
SoC from Ingenic.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---

Changes:

v2: No changes

 drivers/mtd/nand/raw/ingenic/Makefile         |   2 +-
 drivers/mtd/nand/raw/ingenic/jz4725b_bch.c    | 234 ++++++++++++++++++
 .../mtd/nand/raw/ingenic/jz4780_bch_common.c  |   1 +
 .../nand/raw/ingenic/jz4780_bch_internal.h    |   1 +
 4 files changed, 237 insertions(+), 1 deletion(-)
 create mode 100644 drivers/mtd/nand/raw/ingenic/jz4725b_bch.c

diff --git a/drivers/mtd/nand/raw/ingenic/Makefile b/drivers/mtd/nand/raw/ingenic/Makefile
index c308062a6620..f38b467490cf 100644
--- a/drivers/mtd/nand/raw/ingenic/Makefile
+++ b/drivers/mtd/nand/raw/ingenic/Makefile
@@ -1,3 +1,3 @@
 obj-$(CONFIG_MTD_NAND_JZ4740) += jz4740_nand.o
 obj-$(CONFIG_MTD_NAND_JZ4780) += jz4780_nand.o jz4780_bch_common.o \
-				 jz4780_bch.o
+				 jz4780_bch.o jz4725b_bch.o
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..54f9c5796e83
--- /dev/null
+++ b/drivers/mtd/nand/raw/ingenic/jz4725b_bch.c
@@ -0,0 +1,234 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * JZ4780 backend code for the jz4780-bch driver
+ *
+ * Copyright (C) 2018 Paul Cercueil <paul@crapouillou.net>
+ *
+ * Based on jz4780_bch.c
+ */
+
+#include <linux/bitops.h>
+#include <linux/iopoll.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/device.h>
+
+#include "jz4780_bch.h"
+#include "jz4780_bch_internal.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_BSEL_SHIFT		2
+#define BCH_BHCR_BSEL_MASK		(0x1 << BCH_BHCR_BSEL_SHIFT)
+#define BCH_BHCR_ENCE			BIT(3)
+#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 void jz4725b_bch_init(struct jz4780_bch *bch,
+			     struct jz4780_bch_params *params, bool encode)
+{
+	u32 reg;
+
+	/* Clear interrupt status. */
+	writel(readl(bch->base + BCH_BHINT), bch->base + BCH_BHINT);
+
+	/* Initialise and enable BCH. */
+	writel(0x1f, bch->base + BCH_BHCCR);
+	writel(BCH_BHCR_BCHE, bch->base + BCH_BHCSR);
+
+	if (params->strength == 8)
+		writel(BCH_BHCR_BSEL_MASK, bch->base + BCH_BHCSR);
+	else
+		writel(BCH_BHCR_BSEL_MASK, bch->base + BCH_BHCCR);
+
+	if (encode)
+		writel(BCH_BHCR_ENCE, bch->base + BCH_BHCSR);
+	else
+		writel(BCH_BHCR_ENCE, bch->base + BCH_BHCCR);
+
+	writel(BCH_BHCR_INIT, bch->base + BCH_BHCSR);
+
+	/* 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);
+}
+
+static void jz4725b_bch_disable(struct jz4780_bch *bch)
+{
+	writel(readl(bch->base + BCH_BHINT), bch->base + BCH_BHINT);
+	writel(BCH_BHCR_BCHE, bch->base + BCH_BHCCR);
+}
+
+static void jz4725b_bch_write_data(struct jz4780_bch *bch, const u8 *buf,
+				   size_t size)
+{
+	while (size--)
+		writeb(*buf++, bch->base + BCH_BHDR);
+}
+
+static void jz4725b_bch_read_parity(struct jz4780_bch *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(bch->base + BCH_BHPAR0 + offset);
+		offset += sizeof(u32);
+	}
+
+	dest8 = (u8 *)dest32;
+	val = readl(bch->base + BCH_BHPAR0 + offset);
+	switch (size8) {
+	case 3:
+		dest8[2] = (val >> 16) & 0xff;
+	case 2:
+		dest8[1] = (val >> 8) & 0xff;
+	case 1:
+		dest8[0] = val & 0xff;
+		break;
+	}
+}
+
+static bool jz4725b_bch_wait_complete(struct jz4780_bch *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_poll_timeout(bch->base + BCH_BHINT, reg,
+				 (reg & irq) == irq, 0, BCH_TIMEOUT_US);
+	if (ret)
+		return false;
+
+	if (status)
+		*status = reg;
+
+	writel(reg, bch->base + BCH_BHINT);
+	return true;
+}
+
+static int jz4725b_calculate(struct jz4780_bch *bch,
+			     struct jz4780_bch_params *params,
+			     const u8 *buf, u8 *ecc_code)
+{
+	int ret = 0;
+
+	mutex_lock(&bch->lock);
+	jz4725b_bch_init(bch, params, true);
+	jz4725b_bch_write_data(bch, buf, params->size);
+
+	if (jz4725b_bch_wait_complete(bch, BCH_BHINT_ENCF, NULL)) {
+		jz4725b_bch_read_parity(bch, ecc_code, params->bytes);
+	} else {
+		dev_err(bch->dev, "timed out while calculating ECC\n");
+		ret = -ETIMEDOUT;
+	}
+
+	jz4725b_bch_disable(bch);
+	mutex_unlock(&bch->lock);
+	return ret;
+}
+
+static int jz4725b_correct(struct jz4780_bch *bch,
+			   struct jz4780_bch_params *params,
+			   u8 *buf, u8 *ecc_code)
+{
+	u32 reg, errors, bit;
+	unsigned int i;
+	int ret = 0;
+
+	mutex_lock(&bch->lock);
+
+	jz4725b_bch_init(bch, params, false);
+	jz4725b_bch_write_data(bch, buf, params->size);
+	jz4725b_bch_write_data(bch, ecc_code, params->bytes);
+
+	if (!jz4725b_bch_wait_complete(bch, BCH_BHINT_DECF, &reg)) {
+		dev_err(bch->dev, "timed out while correcting data\n");
+		ret = -ETIMEDOUT;
+		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;
+}
+
+const struct jz4780_bch_ops jz4780_bch_jz4725b_ops = {
+	.disable = jz4725b_bch_disable,
+	.calculate = jz4725b_calculate,
+	.correct = jz4725b_correct,
+};
diff --git a/drivers/mtd/nand/raw/ingenic/jz4780_bch_common.c b/drivers/mtd/nand/raw/ingenic/jz4780_bch_common.c
index 1c1fc418ce8e..f505816193a8 100644
--- a/drivers/mtd/nand/raw/ingenic/jz4780_bch_common.c
+++ b/drivers/mtd/nand/raw/ingenic/jz4780_bch_common.c
@@ -157,6 +157,7 @@ static int jz4780_bch_probe(struct platform_device *pdev)
 }
 
 static const struct of_device_id jz4780_bch_dt_match[] = {
+	{ .compatible = "ingenic,jz4725b-bch", .data = &jz4780_bch_jz4725b_ops},
 	{ .compatible = "ingenic,jz4780-bch", .data = &jz4780_bch_jz4780_ops },
 	{},
 };
diff --git a/drivers/mtd/nand/raw/ingenic/jz4780_bch_internal.h b/drivers/mtd/nand/raw/ingenic/jz4780_bch_internal.h
index f109b3c8d039..462aded811b1 100644
--- a/drivers/mtd/nand/raw/ingenic/jz4780_bch_internal.h
+++ b/drivers/mtd/nand/raw/ingenic/jz4780_bch_internal.h
@@ -30,6 +30,7 @@ struct jz4780_bch {
 	struct mutex lock;
 };
 
+extern const struct jz4780_bch_ops jz4780_bch_jz4725b_ops;
 extern const struct jz4780_bch_ops jz4780_bch_jz4780_ops;
 
 #endif /* __DRIVERS_MTD_NAND_JZ4780_BCH_INTERNAL_H__ */
-- 
2.20.1

[PATCH v2 9/9] mtd: rawnand: jz4780-bch: Add support for the JZ4740

[Paul Cercueil]

Add the backend code for the jz4780-bch driver to support the JZ4740
SoC from Ingenic.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---

Changes:

v2: New patch

 drivers/mtd/nand/raw/ingenic/Makefile         |   2 +-
 drivers/mtd/nand/raw/ingenic/jz4740_bch.c     | 173 ++++++++++++++++++
 .../mtd/nand/raw/ingenic/jz4780_bch_common.c  |   1 +
 .../nand/raw/ingenic/jz4780_bch_internal.h    |   1 +
 4 files changed, 176 insertions(+), 1 deletion(-)
 create mode 100644 drivers/mtd/nand/raw/ingenic/jz4740_bch.c

diff --git a/drivers/mtd/nand/raw/ingenic/Makefile b/drivers/mtd/nand/raw/ingenic/Makefile
index f38b467490cf..d16c96113a93 100644
--- a/drivers/mtd/nand/raw/ingenic/Makefile
+++ b/drivers/mtd/nand/raw/ingenic/Makefile
@@ -1,3 +1,3 @@
 obj-$(CONFIG_MTD_NAND_JZ4740) += jz4740_nand.o
 obj-$(CONFIG_MTD_NAND_JZ4780) += jz4780_nand.o jz4780_bch_common.o \
-				 jz4780_bch.o jz4725b_bch.o
+				 jz4780_bch.o jz4725b_bch.o jz4740_bch.o
diff --git a/drivers/mtd/nand/raw/ingenic/jz4740_bch.c b/drivers/mtd/nand/raw/ingenic/jz4740_bch.c
new file mode 100644
index 000000000000..61ea109cee9d
--- /dev/null
+++ b/drivers/mtd/nand/raw/ingenic/jz4740_bch.c
@@ -0,0 +1,173 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * JZ4740 backend code for the jz4780-bch driver
+ * based on jz4740-nand.c
+ *
+ * Copyright (c) 2019 Paul Cercueil <paul@crapouillou.net>
+ */
+
+#include <linux/bitops.h>
+#include <linux/device.h>
+
+#include "jz4780_bch.h"
+#include "jz4780_bch_internal.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_bch_init(struct jz4780_bch *bch, bool encode)
+{
+	uint32_t reg;
+
+	/* Clear interrupt status */
+	writel(0, bch->base + JZ_REG_NAND_IRQ_STAT);
+
+	/* Initialize and enable BCH */
+	reg = readl(bch->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, bch->base + JZ_REG_NAND_ECC_CTRL);
+}
+
+static int jz4740_bch_calculate(struct jz4780_bch *bch,
+				struct jz4780_bch_params *params,
+				const u8 *buf, u8 *ecc_code)
+{
+	uint32_t reg, status;
+	unsigned int timeout = 1000;
+	int i;
+
+	jz4740_bch_init(bch, true);
+
+	do {
+		status = readl(bch->base + JZ_REG_NAND_IRQ_STAT);
+	} while (!(status & JZ_NAND_STATUS_ENC_FINISH) && --timeout);
+
+	if (timeout == 0)
+		return -ETIMEDOUT;
+
+	reg = readl(bch->base + JZ_REG_NAND_ECC_CTRL);
+	reg &= ~JZ_NAND_ECC_CTRL_ENABLE;
+	writel(reg, bch->base + JZ_REG_NAND_ECC_CTRL);
+
+	for (i = 0; i < params->bytes; ++i)
+		ecc_code[i] = readb(bch->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_bch_correct(struct jz4780_bch *bch,
+			      struct jz4780_bch_params *params,
+			      u8 *buf, u8 *ecc_code)
+{
+	int i, error_count, index;
+	uint32_t reg, status, error;
+	unsigned int timeout = 1000;
+
+	jz4740_bch_init(bch, false);
+
+	for (i = 0; i < params->bytes; ++i)
+		writeb(ecc_code[i], bch->base + JZ_REG_NAND_PAR0 + i);
+
+	reg = readl(bch->base + JZ_REG_NAND_ECC_CTRL);
+	reg |= JZ_NAND_ECC_CTRL_PAR_READY;
+	writel(reg, bch->base + JZ_REG_NAND_ECC_CTRL);
+
+	do {
+		status = readl(bch->base + JZ_REG_NAND_IRQ_STAT);
+	} while (!(status & JZ_NAND_STATUS_DEC_FINISH) && --timeout);
+
+	if (timeout == 0)
+		return -ETIMEDOUT;
+
+	reg = readl(bch->base + JZ_REG_NAND_ECC_CTRL);
+	reg &= ~JZ_NAND_ECC_CTRL_ENABLE;
+	writel(reg, bch->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(bch->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_bch_disable(struct jz4780_bch *bch)
+{
+	u32 reg;
+
+	writel(0, bch->base + JZ_REG_NAND_IRQ_STAT);
+	reg = readl(bch->base + JZ_REG_NAND_ECC_CTRL);
+	reg &= ~JZ_NAND_ECC_CTRL_ENABLE;
+	writel(reg, bch->base + JZ_REG_NAND_ECC_CTRL);
+}
+
+const struct jz4780_bch_ops jz4780_bch_jz4740_ops = {
+	.disable = jz4740_bch_disable,
+	.calculate = jz4740_bch_calculate,
+	.correct = jz4740_bch_correct,
+};
diff --git a/drivers/mtd/nand/raw/ingenic/jz4780_bch_common.c b/drivers/mtd/nand/raw/ingenic/jz4780_bch_common.c
index f505816193a8..c2326286abb2 100644
--- a/drivers/mtd/nand/raw/ingenic/jz4780_bch_common.c
+++ b/drivers/mtd/nand/raw/ingenic/jz4780_bch_common.c
@@ -157,6 +157,7 @@ static int jz4780_bch_probe(struct platform_device *pdev)
 }
 
 static const struct of_device_id jz4780_bch_dt_match[] = {
+	{ .compatible = "ingenic,jz4740-bch", .data = &jz4780_bch_jz4740_ops},
 	{ .compatible = "ingenic,jz4725b-bch", .data = &jz4780_bch_jz4725b_ops},
 	{ .compatible = "ingenic,jz4780-bch", .data = &jz4780_bch_jz4780_ops },
 	{},
diff --git a/drivers/mtd/nand/raw/ingenic/jz4780_bch_internal.h b/drivers/mtd/nand/raw/ingenic/jz4780_bch_internal.h
index 462aded811b1..7909a49c57db 100644
--- a/drivers/mtd/nand/raw/ingenic/jz4780_bch_internal.h
+++ b/drivers/mtd/nand/raw/ingenic/jz4780_bch_internal.h
@@ -30,6 +30,7 @@ struct jz4780_bch {
 	struct mutex lock;
 };
 
+extern const struct jz4780_bch_ops jz4780_bch_jz4740_ops;
 extern const struct jz4780_bch_ops jz4780_bch_jz4725b_ops;
 extern const struct jz4780_bch_ops jz4780_bch_jz4780_ops;
 
-- 
2.20.1

Re: [PATCH v2 9/9] mtd: rawnand: jz4780-bch: Add support for the JZ4740

[Boris Brezillon]

On Sat,  2 Feb 2019 20:19:26 -0300
Paul Cercueil <paul@crapouillou.net> wrote:

> Add the backend code for the jz4780-bch driver to support the JZ4740
> SoC from Ingenic.
> 
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---
> 
> Changes:
> 
> v2: New patch
> 
>  drivers/mtd/nand/raw/ingenic/Makefile         |   2 +-
>  drivers/mtd/nand/raw/ingenic/jz4740_bch.c     | 173 ++++++++++++++++++
>  .../mtd/nand/raw/ingenic/jz4780_bch_common.c  |   1 +
>  .../nand/raw/ingenic/jz4780_bch_internal.h    |   1 +
>  4 files changed, 176 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/mtd/nand/raw/ingenic/jz4740_bch.c
> 
> diff --git a/drivers/mtd/nand/raw/ingenic/Makefile b/drivers/mtd/nand/raw/ingenic/Makefile
> index f38b467490cf..d16c96113a93 100644
> --- a/drivers/mtd/nand/raw/ingenic/Makefile
> +++ b/drivers/mtd/nand/raw/ingenic/Makefile
> @@ -1,3 +1,3 @@
>  obj-$(CONFIG_MTD_NAND_JZ4740) += jz4740_nand.o
>  obj-$(CONFIG_MTD_NAND_JZ4780) += jz4780_nand.o jz4780_bch_common.o \
> -				 jz4780_bch.o jz4725b_bch.o
> +				 jz4780_bch.o jz4725b_bch.o jz4740_bch.o

I still don't see the point of the jz4780_bch_common/jz47xxx_bch
separation. You seem to always embed all objects anyway, so you can
just put the code for both engines in the same source file and decide
which one to use based on the compat (which you already do anyway). 

> diff --git a/drivers/mtd/nand/raw/ingenic/jz4740_bch.c b/drivers/mtd/nand/raw/ingenic/jz4740_bch.c
> new file mode 100644
> index 000000000000..61ea109cee9d
> --- /dev/null
> +++ b/drivers/mtd/nand/raw/ingenic/jz4740_bch.c
> @@ -0,0 +1,173 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * JZ4740 backend code for the jz4780-bch driver
> + * based on jz4740-nand.c
> + *
> + * Copyright (c) 2019 Paul Cercueil <paul@crapouillou.net>
> + */
> +
> +#include <linux/bitops.h>
> +#include <linux/device.h>
> +
> +#include "jz4780_bch.h"
> +#include "jz4780_bch_internal.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_bch_init(struct jz4780_bch *bch, bool encode)
> +{
> +	uint32_t reg;
> +
> +	/* Clear interrupt status */
> +	writel(0, bch->base + JZ_REG_NAND_IRQ_STAT);
> +
> +	/* Initialize and enable BCH */
> +	reg = readl(bch->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, bch->base + JZ_REG_NAND_ECC_CTRL);
> +}
> +
> +static int jz4740_bch_calculate(struct jz4780_bch *bch,
> +				struct jz4780_bch_params *params,
> +				const u8 *buf, u8 *ecc_code)
> +{
> +	uint32_t reg, status;
> +	unsigned int timeout = 1000;
> +	int i;
> +
> +	jz4740_bch_init(bch, true);
> +
> +	do {
> +		status = readl(bch->base + JZ_REG_NAND_IRQ_STAT);
> +	} while (!(status & JZ_NAND_STATUS_ENC_FINISH) && --timeout);
> +
> +	if (timeout == 0)
> +		return -ETIMEDOUT;
> +
> +	reg = readl(bch->base + JZ_REG_NAND_ECC_CTRL);
> +	reg &= ~JZ_NAND_ECC_CTRL_ENABLE;
> +	writel(reg, bch->base + JZ_REG_NAND_ECC_CTRL);
> +
> +	for (i = 0; i < params->bytes; ++i)
> +		ecc_code[i] = readb(bch->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_bch_correct(struct jz4780_bch *bch,
> +			      struct jz4780_bch_params *params,
> +			      u8 *buf, u8 *ecc_code)
> +{
> +	int i, error_count, index;
> +	uint32_t reg, status, error;
> +	unsigned int timeout = 1000;
> +
> +	jz4740_bch_init(bch, false);
> +
> +	for (i = 0; i < params->bytes; ++i)
> +		writeb(ecc_code[i], bch->base + JZ_REG_NAND_PAR0 + i);
> +
> +	reg = readl(bch->base + JZ_REG_NAND_ECC_CTRL);
> +	reg |= JZ_NAND_ECC_CTRL_PAR_READY;
> +	writel(reg, bch->base + JZ_REG_NAND_ECC_CTRL);
> +
> +	do {
> +		status = readl(bch->base + JZ_REG_NAND_IRQ_STAT);
> +	} while (!(status & JZ_NAND_STATUS_DEC_FINISH) && --timeout);
> +
> +	if (timeout == 0)
> +		return -ETIMEDOUT;
> +
> +	reg = readl(bch->base + JZ_REG_NAND_ECC_CTRL);
> +	reg &= ~JZ_NAND_ECC_CTRL_ENABLE;
> +	writel(reg, bch->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(bch->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_bch_disable(struct jz4780_bch *bch)
> +{
> +	u32 reg;
> +
> +	writel(0, bch->base + JZ_REG_NAND_IRQ_STAT);
> +	reg = readl(bch->base + JZ_REG_NAND_ECC_CTRL);
> +	reg &= ~JZ_NAND_ECC_CTRL_ENABLE;
> +	writel(reg, bch->base + JZ_REG_NAND_ECC_CTRL);
> +}
> +
> +const struct jz4780_bch_ops jz4780_bch_jz4740_ops = {
> +	.disable = jz4740_bch_disable,
> +	.calculate = jz4740_bch_calculate,
> +	.correct = jz4740_bch_correct,
> +};
> diff --git a/drivers/mtd/nand/raw/ingenic/jz4780_bch_common.c b/drivers/mtd/nand/raw/ingenic/jz4780_bch_common.c
> index f505816193a8..c2326286abb2 100644
> --- a/drivers/mtd/nand/raw/ingenic/jz4780_bch_common.c
> +++ b/drivers/mtd/nand/raw/ingenic/jz4780_bch_common.c
> @@ -157,6 +157,7 @@ static int jz4780_bch_probe(struct platform_device *pdev)
>  }
>  
>  static const struct of_device_id jz4780_bch_dt_match[] = {
> +	{ .compatible = "ingenic,jz4740-bch", .data = &jz4780_bch_jz4740_ops},
>  	{ .compatible = "ingenic,jz4725b-bch", .data = &jz4780_bch_jz4725b_ops},
>  	{ .compatible = "ingenic,jz4780-bch", .data = &jz4780_bch_jz4780_ops },
>  	{},
> diff --git a/drivers/mtd/nand/raw/ingenic/jz4780_bch_internal.h b/drivers/mtd/nand/raw/ingenic/jz4780_bch_internal.h
> index 462aded811b1..7909a49c57db 100644
> --- a/drivers/mtd/nand/raw/ingenic/jz4780_bch_internal.h
> +++ b/drivers/mtd/nand/raw/ingenic/jz4780_bch_internal.h
> @@ -30,6 +30,7 @@ struct jz4780_bch {
>  	struct mutex lock;
>  };
>  
> +extern const struct jz4780_bch_ops jz4780_bch_jz4740_ops;
>  extern const struct jz4780_bch_ops jz4780_bch_jz4725b_ops;
>  extern const struct jz4780_bch_ops jz4780_bch_jz4780_ops;
>  

Re: [PATCH v2 9/9] mtd: rawnand: jz4780-bch: Add support for the JZ4740

[Paul Cercueil]

Le dim. 3 févr. 2019 à 4:35, Boris Brezillon <bbrezillon@kernel.org> 
a écrit :
> On Sat,  2 Feb 2019 20:19:26 -0300
> Paul Cercueil <paul@crapouillou.net> wrote:
> 
>>  Add the backend code for the jz4780-bch driver to support the JZ4740
>>  SoC from Ingenic.
>> 
>>  Signed-off-by: Paul Cercueil <paul@crapouillou.net>
>>  ---
>> 
>>  Changes:
>> 
>>  v2: New patch
>> 
>>   drivers/mtd/nand/raw/ingenic/Makefile         |   2 +-
>>   drivers/mtd/nand/raw/ingenic/jz4740_bch.c     | 173 
>> ++++++++++++++++++
>>   .../mtd/nand/raw/ingenic/jz4780_bch_common.c  |   1 +
>>   .../nand/raw/ingenic/jz4780_bch_internal.h    |   1 +
>>   4 files changed, 176 insertions(+), 1 deletion(-)
>>   create mode 100644 drivers/mtd/nand/raw/ingenic/jz4740_bch.c
>> 
>>  diff --git a/drivers/mtd/nand/raw/ingenic/Makefile 
>> b/drivers/mtd/nand/raw/ingenic/Makefile
>>  index f38b467490cf..d16c96113a93 100644
>>  --- a/drivers/mtd/nand/raw/ingenic/Makefile
>>  +++ b/drivers/mtd/nand/raw/ingenic/Makefile
>>  @@ -1,3 +1,3 @@
>>   obj-$(CONFIG_MTD_NAND_JZ4740) += jz4740_nand.o
>>   obj-$(CONFIG_MTD_NAND_JZ4780) += jz4780_nand.o jz4780_bch_common.o 
>> \
>>  -				 jz4780_bch.o jz4725b_bch.o
>>  +				 jz4780_bch.o jz4725b_bch.o jz4740_bch.o
> 
> I still don't see the point of the jz4780_bch_common/jz47xxx_bch
> separation. You seem to always embed all objects anyway, so you can
> just put the code for both engines in the same source file and decide
> which one to use based on the compat (which you already do anyway).

Each SoC has a different set of registers for the BCH hardware. I can 
try to
cram everything into one file, but it won't be that much cleaner.

>>  diff --git a/drivers/mtd/nand/raw/ingenic/jz4740_bch.c 
>> b/drivers/mtd/nand/raw/ingenic/jz4740_bch.c
>>  new file mode 100644
>>  index 000000000000..61ea109cee9d
>>  --- /dev/null
>>  +++ b/drivers/mtd/nand/raw/ingenic/jz4740_bch.c
>>  @@ -0,0 +1,173 @@
>>  +// SPDX-License-Identifier: GPL-2.0
>>  +/*
>>  + * JZ4740 backend code for the jz4780-bch driver
>>  + * based on jz4740-nand.c
>>  + *
>>  + * Copyright (c) 2019 Paul Cercueil <paul@crapouillou.net>
>>  + */
>>  +
>>  +#include <linux/bitops.h>
>>  +#include <linux/device.h>
>>  +
>>  +#include "jz4780_bch.h"
>>  +#include "jz4780_bch_internal.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_bch_init(struct jz4780_bch *bch, bool encode)
>>  +{
>>  +	uint32_t reg;
>>  +
>>  +	/* Clear interrupt status */
>>  +	writel(0, bch->base + JZ_REG_NAND_IRQ_STAT);
>>  +
>>  +	/* Initialize and enable BCH */
>>  +	reg = readl(bch->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, bch->base + JZ_REG_NAND_ECC_CTRL);
>>  +}
>>  +
>>  +static int jz4740_bch_calculate(struct jz4780_bch *bch,
>>  +				struct jz4780_bch_params *params,
>>  +				const u8 *buf, u8 *ecc_code)
>>  +{
>>  +	uint32_t reg, status;
>>  +	unsigned int timeout = 1000;
>>  +	int i;
>>  +
>>  +	jz4740_bch_init(bch, true);
>>  +
>>  +	do {
>>  +		status = readl(bch->base + JZ_REG_NAND_IRQ_STAT);
>>  +	} while (!(status & JZ_NAND_STATUS_ENC_FINISH) && --timeout);
>>  +
>>  +	if (timeout == 0)
>>  +		return -ETIMEDOUT;
>>  +
>>  +	reg = readl(bch->base + JZ_REG_NAND_ECC_CTRL);
>>  +	reg &= ~JZ_NAND_ECC_CTRL_ENABLE;
>>  +	writel(reg, bch->base + JZ_REG_NAND_ECC_CTRL);
>>  +
>>  +	for (i = 0; i < params->bytes; ++i)
>>  +		ecc_code[i] = readb(bch->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_bch_correct(struct jz4780_bch *bch,
>>  +			      struct jz4780_bch_params *params,
>>  +			      u8 *buf, u8 *ecc_code)
>>  +{
>>  +	int i, error_count, index;
>>  +	uint32_t reg, status, error;
>>  +	unsigned int timeout = 1000;
>>  +
>>  +	jz4740_bch_init(bch, false);
>>  +
>>  +	for (i = 0; i < params->bytes; ++i)
>>  +		writeb(ecc_code[i], bch->base + JZ_REG_NAND_PAR0 + i);
>>  +
>>  +	reg = readl(bch->base + JZ_REG_NAND_ECC_CTRL);
>>  +	reg |= JZ_NAND_ECC_CTRL_PAR_READY;
>>  +	writel(reg, bch->base + JZ_REG_NAND_ECC_CTRL);
>>  +
>>  +	do {
>>  +		status = readl(bch->base + JZ_REG_NAND_IRQ_STAT);
>>  +	} while (!(status & JZ_NAND_STATUS_DEC_FINISH) && --timeout);
>>  +
>>  +	if (timeout == 0)
>>  +		return -ETIMEDOUT;
>>  +
>>  +	reg = readl(bch->base + JZ_REG_NAND_ECC_CTRL);
>>  +	reg &= ~JZ_NAND_ECC_CTRL_ENABLE;
>>  +	writel(reg, bch->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(bch->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_bch_disable(struct jz4780_bch *bch)
>>  +{
>>  +	u32 reg;
>>  +
>>  +	writel(0, bch->base + JZ_REG_NAND_IRQ_STAT);
>>  +	reg = readl(bch->base + JZ_REG_NAND_ECC_CTRL);
>>  +	reg &= ~JZ_NAND_ECC_CTRL_ENABLE;
>>  +	writel(reg, bch->base + JZ_REG_NAND_ECC_CTRL);
>>  +}
>>  +
>>  +const struct jz4780_bch_ops jz4780_bch_jz4740_ops = {
>>  +	.disable = jz4740_bch_disable,
>>  +	.calculate = jz4740_bch_calculate,
>>  +	.correct = jz4740_bch_correct,
>>  +};
>>  diff --git a/drivers/mtd/nand/raw/ingenic/jz4780_bch_common.c 
>> b/drivers/mtd/nand/raw/ingenic/jz4780_bch_common.c
>>  index f505816193a8..c2326286abb2 100644
>>  --- a/drivers/mtd/nand/raw/ingenic/jz4780_bch_common.c
>>  +++ b/drivers/mtd/nand/raw/ingenic/jz4780_bch_common.c
>>  @@ -157,6 +157,7 @@ static int jz4780_bch_probe(struct 
>> platform_device *pdev)
>>   }
>> 
>>   static const struct of_device_id jz4780_bch_dt_match[] = {
>>  +	{ .compatible = "ingenic,jz4740-bch", .data = 
>> &jz4780_bch_jz4740_ops},
>>   	{ .compatible = "ingenic,jz4725b-bch", .data = 
>> &jz4780_bch_jz4725b_ops},
>>   	{ .compatible = "ingenic,jz4780-bch", .data = 
>> &jz4780_bch_jz4780_ops },
>>   	{},
>>  diff --git a/drivers/mtd/nand/raw/ingenic/jz4780_bch_internal.h 
>> b/drivers/mtd/nand/raw/ingenic/jz4780_bch_internal.h
>>  index 462aded811b1..7909a49c57db 100644
>>  --- a/drivers/mtd/nand/raw/ingenic/jz4780_bch_internal.h
>>  +++ b/drivers/mtd/nand/raw/ingenic/jz4780_bch_internal.h
>>  @@ -30,6 +30,7 @@ struct jz4780_bch {
>>   	struct mutex lock;
>>   };
>> 
>>  +extern const struct jz4780_bch_ops jz4780_bch_jz4740_ops;
>>   extern const struct jz4780_bch_ops jz4780_bch_jz4725b_ops;
>>   extern const struct jz4780_bch_ops jz4780_bch_jz4780_ops;
>> 
> 

Re: [PATCH v2 9/9] mtd: rawnand: jz4780-bch: Add support for the JZ4740

[Boris Brezillon]

On Sun, 03 Feb 2019 10:58:13 -0300
Paul Cercueil <paul@crapouillou.net> wrote:

> Le dim. 3 févr. 2019 à 4:35, Boris Brezillon <bbrezillon@kernel.org> 
> a écrit :
> > On Sat,  2 Feb 2019 20:19:26 -0300
> > Paul Cercueil <paul@crapouillou.net> wrote:
> >   
> >>  Add the backend code for the jz4780-bch driver to support the JZ4740
> >>  SoC from Ingenic.
> >> 
> >>  Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> >>  ---
> >> 
> >>  Changes:
> >> 
> >>  v2: New patch
> >> 
> >>   drivers/mtd/nand/raw/ingenic/Makefile         |   2 +-
> >>   drivers/mtd/nand/raw/ingenic/jz4740_bch.c     | 173 
> >> ++++++++++++++++++
> >>   .../mtd/nand/raw/ingenic/jz4780_bch_common.c  |   1 +
> >>   .../nand/raw/ingenic/jz4780_bch_internal.h    |   1 +
> >>   4 files changed, 176 insertions(+), 1 deletion(-)
> >>   create mode 100644 drivers/mtd/nand/raw/ingenic/jz4740_bch.c
> >> 
> >>  diff --git a/drivers/mtd/nand/raw/ingenic/Makefile 
> >> b/drivers/mtd/nand/raw/ingenic/Makefile
> >>  index f38b467490cf..d16c96113a93 100644
> >>  --- a/drivers/mtd/nand/raw/ingenic/Makefile
> >>  +++ b/drivers/mtd/nand/raw/ingenic/Makefile
> >>  @@ -1,3 +1,3 @@
> >>   obj-$(CONFIG_MTD_NAND_JZ4740) += jz4740_nand.o
> >>   obj-$(CONFIG_MTD_NAND_JZ4780) += jz4780_nand.o jz4780_bch_common.o 
> >> \
> >>  -				 jz4780_bch.o jz4725b_bch.o
> >>  +				 jz4780_bch.o jz4725b_bch.o jz4740_bch.o  
> > 
> > I still don't see the point of the jz4780_bch_common/jz47xxx_bch
> > separation. You seem to always embed all objects anyway, so you can
> > just put the code for both engines in the same source file and decide
> > which one to use based on the compat (which you already do anyway).  
> 
> Each SoC has a different set of registers for the BCH hardware. I can 
> try to
> cram everything into one file, but it won't be that much cleaner.

Then maybe they deserve separate drivers/modules.

BTW, didn't you say that one IP uses Reed-Salomon instead of BCH. I'd
suggest prefixing structs and functions with jz47xx_ecc instead of
jz47xx_bch and naming the common part jz47xx_ecc.c to reflect that.

Re: [PATCH v2 9/9] mtd: rawnand: jz4780-bch: Add support for the JZ4740

[Paul Cercueil]

Le dim. 3 févr. 2019 à 11:16, Boris Brezillon <bbrezillon@kernel.org> 
a écrit :
> On Sun, 03 Feb 2019 10:58:13 -0300
> Paul Cercueil <paul@crapouillou.net> wrote:
> 
>>  Le dim. 3 févr. 2019 à 4:35, Boris Brezillon 
>> <bbrezillon@kernel.org>
>>  a écrit :
>>  > On Sat,  2 Feb 2019 20:19:26 -0300
>>  > Paul Cercueil <paul@crapouillou.net> wrote:
>>  >
>>  >>  Add the backend code for the jz4780-bch driver to support the 
>> JZ4740
>>  >>  SoC from Ingenic.
>>  >>
>>  >>  Signed-off-by: Paul Cercueil <paul@crapouillou.net>
>>  >>  ---
>>  >>
>>  >>  Changes:
>>  >>
>>  >>  v2: New patch
>>  >>
>>  >>   drivers/mtd/nand/raw/ingenic/Makefile         |   2 +-
>>  >>   drivers/mtd/nand/raw/ingenic/jz4740_bch.c     | 173
>>  >> ++++++++++++++++++
>>  >>   .../mtd/nand/raw/ingenic/jz4780_bch_common.c  |   1 +
>>  >>   .../nand/raw/ingenic/jz4780_bch_internal.h    |   1 +
>>  >>   4 files changed, 176 insertions(+), 1 deletion(-)
>>  >>   create mode 100644 drivers/mtd/nand/raw/ingenic/jz4740_bch.c
>>  >>
>>  >>  diff --git a/drivers/mtd/nand/raw/ingenic/Makefile
>>  >> b/drivers/mtd/nand/raw/ingenic/Makefile
>>  >>  index f38b467490cf..d16c96113a93 100644
>>  >>  --- a/drivers/mtd/nand/raw/ingenic/Makefile
>>  >>  +++ b/drivers/mtd/nand/raw/ingenic/Makefile
>>  >>  @@ -1,3 +1,3 @@
>>  >>   obj-$(CONFIG_MTD_NAND_JZ4740) += jz4740_nand.o
>>  >>   obj-$(CONFIG_MTD_NAND_JZ4780) += jz4780_nand.o 
>> jz4780_bch_common.o
>>  >> \
>>  >>  -				 jz4780_bch.o jz4725b_bch.o
>>  >>  +				 jz4780_bch.o jz4725b_bch.o jz4740_bch.o
>>  >
>>  > I still don't see the point of the jz4780_bch_common/jz47xxx_bch
>>  > separation. You seem to always embed all objects anyway, so you 
>> can
>>  > just put the code for both engines in the same source file and 
>> decide
>>  > which one to use based on the compat (which you already do 
>> anyway).
>> 
>>  Each SoC has a different set of registers for the BCH hardware. I 
>> can
>>  try to
>>  cram everything into one file, but it won't be that much cleaner.
> 
> Then maybe they deserve separate drivers/modules.
> 
> BTW, didn't you say that one IP uses Reed-Salomon instead of BCH. I'd
> suggest prefixing structs and functions with jz47xx_ecc instead of
> jz47xx_bch and naming the common part jz47xx_ecc.c to reflect that.

Would it be a good idea to make a generic ECC API that the jz47xx_nand
driver could use? Then the three jz47xx BCH codepaths could be separate
drivers that register with the generic ECC core.

Re: [PATCH v2 9/9] mtd: rawnand: jz4780-bch: Add support for the JZ4740

[Boris Brezillon]

On Sun, 03 Feb 2019 11:56:32 -0300
Paul Cercueil <paul@crapouillou.net> wrote:

> Le dim. 3 févr. 2019 à 11:16, Boris Brezillon <bbrezillon@kernel.org> 
> a écrit :
> > On Sun, 03 Feb 2019 10:58:13 -0300
> > Paul Cercueil <paul@crapouillou.net> wrote:
> >   
> >>  Le dim. 3 févr. 2019 à 4:35, Boris Brezillon 
> >> <bbrezillon@kernel.org>
> >>  a écrit :  
> >>  > On Sat,  2 Feb 2019 20:19:26 -0300
> >>  > Paul Cercueil <paul@crapouillou.net> wrote:
> >>  >  
> >>  >>  Add the backend code for the jz4780-bch driver to support the   
> >> JZ4740  
> >>  >>  SoC from Ingenic.
> >>  >>
> >>  >>  Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> >>  >>  ---
> >>  >>
> >>  >>  Changes:
> >>  >>
> >>  >>  v2: New patch
> >>  >>
> >>  >>   drivers/mtd/nand/raw/ingenic/Makefile         |   2 +-
> >>  >>   drivers/mtd/nand/raw/ingenic/jz4740_bch.c     | 173
> >>  >> ++++++++++++++++++
> >>  >>   .../mtd/nand/raw/ingenic/jz4780_bch_common.c  |   1 +
> >>  >>   .../nand/raw/ingenic/jz4780_bch_internal.h    |   1 +
> >>  >>   4 files changed, 176 insertions(+), 1 deletion(-)
> >>  >>   create mode 100644 drivers/mtd/nand/raw/ingenic/jz4740_bch.c
> >>  >>
> >>  >>  diff --git a/drivers/mtd/nand/raw/ingenic/Makefile
> >>  >> b/drivers/mtd/nand/raw/ingenic/Makefile
> >>  >>  index f38b467490cf..d16c96113a93 100644
> >>  >>  --- a/drivers/mtd/nand/raw/ingenic/Makefile
> >>  >>  +++ b/drivers/mtd/nand/raw/ingenic/Makefile
> >>  >>  @@ -1,3 +1,3 @@
> >>  >>   obj-$(CONFIG_MTD_NAND_JZ4740) += jz4740_nand.o
> >>  >>   obj-$(CONFIG_MTD_NAND_JZ4780) += jz4780_nand.o   
> >> jz4780_bch_common.o  
> >>  >> \
> >>  >>  -				 jz4780_bch.o jz4725b_bch.o
> >>  >>  +				 jz4780_bch.o jz4725b_bch.o jz4740_bch.o  
> >>  >
> >>  > I still don't see the point of the jz4780_bch_common/jz47xxx_bch
> >>  > separation. You seem to always embed all objects anyway, so you   
> >> can  
> >>  > just put the code for both engines in the same source file and   
> >> decide  
> >>  > which one to use based on the compat (which you already do   
> >> anyway).
> >> 
> >>  Each SoC has a different set of registers for the BCH hardware. I 
> >> can
> >>  try to
> >>  cram everything into one file, but it won't be that much cleaner.  
> > 
> > Then maybe they deserve separate drivers/modules.
> > 
> > BTW, didn't you say that one IP uses Reed-Salomon instead of BCH. I'd
> > suggest prefixing structs and functions with jz47xx_ecc instead of
> > jz47xx_bch and naming the common part jz47xx_ecc.c to reflect that.  
> 
> Would it be a good idea to make a generic ECC API that the jz47xx_nand
> driver could use? Then the three jz47xx BCH codepaths could be separate
> drivers that register with the generic ECC core.

Definitely. Actually, Miquel is already working on that, but I don't
think it's a good idea to wait for this new framework to be finished to
get your changes merged. So I'd recommend having a jz specific ECC API
(pretty much the one you have in the common file expect it would be
prefixed with _ecc instead of _bch) and convert it to the generic
approach afterwards. Given the size of the common.c file, you can even
put everything in jz47xx_ecc.h as inline funcs to avoid having yet
another module.

Re: [PATCH v2 9/9] mtd: rawnand: jz4780-bch: Add support for the JZ4740

[Paul Cercueil]

Le dim. 3 févr. 2019 à 12:07, Boris Brezillon <bbrezillon@kernel.org> 
a écrit :
> On Sun, 03 Feb 2019 11:56:32 -0300
> Paul Cercueil <paul@crapouillou.net> wrote:
> 
>>  Le dim. 3 févr. 2019 à 11:16, Boris Brezillon 
>> <bbrezillon@kernel.org>
>>  a écrit :
>>  > On Sun, 03 Feb 2019 10:58:13 -0300
>>  > Paul Cercueil <paul@crapouillou.net> wrote:
>>  >
>>  >>  Le dim. 3 févr. 2019 à 4:35, Boris Brezillon
>>  >> <bbrezillon@kernel.org>
>>  >>  a écrit :
>>  >>  > On Sat,  2 Feb 2019 20:19:26 -0300
>>  >>  > Paul Cercueil <paul@crapouillou.net> wrote:
>>  >>  >
>>  >>  >>  Add the backend code for the jz4780-bch driver to support 
>> the
>>  >> JZ4740
>>  >>  >>  SoC from Ingenic.
>>  >>  >>
>>  >>  >>  Signed-off-by: Paul Cercueil <paul@crapouillou.net>
>>  >>  >>  ---
>>  >>  >>
>>  >>  >>  Changes:
>>  >>  >>
>>  >>  >>  v2: New patch
>>  >>  >>
>>  >>  >>   drivers/mtd/nand/raw/ingenic/Makefile         |   2 +-
>>  >>  >>   drivers/mtd/nand/raw/ingenic/jz4740_bch.c     | 173
>>  >>  >> ++++++++++++++++++
>>  >>  >>   .../mtd/nand/raw/ingenic/jz4780_bch_common.c  |   1 +
>>  >>  >>   .../nand/raw/ingenic/jz4780_bch_internal.h    |   1 +
>>  >>  >>   4 files changed, 176 insertions(+), 1 deletion(-)
>>  >>  >>   create mode 100644 
>> drivers/mtd/nand/raw/ingenic/jz4740_bch.c
>>  >>  >>
>>  >>  >>  diff --git a/drivers/mtd/nand/raw/ingenic/Makefile
>>  >>  >> b/drivers/mtd/nand/raw/ingenic/Makefile
>>  >>  >>  index f38b467490cf..d16c96113a93 100644
>>  >>  >>  --- a/drivers/mtd/nand/raw/ingenic/Makefile
>>  >>  >>  +++ b/drivers/mtd/nand/raw/ingenic/Makefile
>>  >>  >>  @@ -1,3 +1,3 @@
>>  >>  >>   obj-$(CONFIG_MTD_NAND_JZ4740) += jz4740_nand.o
>>  >>  >>   obj-$(CONFIG_MTD_NAND_JZ4780) += jz4780_nand.o
>>  >> jz4780_bch_common.o
>>  >>  >> \
>>  >>  >>  -				 jz4780_bch.o jz4725b_bch.o
>>  >>  >>  +				 jz4780_bch.o jz4725b_bch.o jz4740_bch.o
>>  >>  >
>>  >>  > I still don't see the point of the 
>> jz4780_bch_common/jz47xxx_bch
>>  >>  > separation. You seem to always embed all objects anyway, so 
>> you
>>  >> can
>>  >>  > just put the code for both engines in the same source file and
>>  >> decide
>>  >>  > which one to use based on the compat (which you already do
>>  >> anyway).
>>  >>
>>  >>  Each SoC has a different set of registers for the BCH hardware. 
>> I
>>  >> can
>>  >>  try to
>>  >>  cram everything into one file, but it won't be that much 
>> cleaner.
>>  >
>>  > Then maybe they deserve separate drivers/modules.
>>  >
>>  > BTW, didn't you say that one IP uses Reed-Salomon instead of BCH. 
>> I'd
>>  > suggest prefixing structs and functions with jz47xx_ecc instead of
>>  > jz47xx_bch and naming the common part jz47xx_ecc.c to reflect 
>> that.
>> 
>>  Would it be a good idea to make a generic ECC API that the 
>> jz47xx_nand
>>  driver could use? Then the three jz47xx BCH codepaths could be 
>> separate
>>  drivers that register with the generic ECC core.
> 
> Definitely. Actually, Miquel is already working on that, but I don't
> think it's a good idea to wait for this new framework to be finished 
> to
> get your changes merged. So I'd recommend having a jz specific ECC API
> (pretty much the one you have in the common file expect it would be
> prefixed with _ecc instead of _bch) and convert it to the generic
> approach afterwards. Given the size of the common.c file, you can even
> put everything in jz47xx_ecc.h as inline funcs to avoid having yet
> another module.

Ok, perfect, I'll do that.
Thank you for your input!

Re: [PATCH v2 9/9] mtd: rawnand: jz4780-bch: Add support for the JZ4740

[kbuild test robot]

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

Hi Paul,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on mtd/nand/next]
[also build test ERROR on v5.0-rc4 next-20190204]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Paul-Cercueil/Ingenic-JZ4780-NAND-patchset-v2/20190204-163709
base:   git://git.infradead.org/linux-mtd.git nand/next
config: sparc64-allmodconfig (attached as .config)
compiler: sparc64-linux-gnu-gcc (Debian 8.2.0-11) 8.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=8.2.0 make.cross ARCH=sparc64 

All errors (new ones prefixed by >>):

   drivers/mtd/nand/raw/ingenic/jz4740_bch.c: In function 'jz4740_bch_init':
   drivers/mtd/nand/raw/ingenic/jz4740_bch.c:46:2: error: implicit declaration of function 'writel' [-Werror=implicit-function-declaration]
     writel(0, bch->base + JZ_REG_NAND_IRQ_STAT);
     ^~~~~~
>> drivers/mtd/nand/raw/ingenic/jz4740_bch.c:49:8: error: implicit declaration of function 'readl'; did you mean 'd_real'? [-Werror=implicit-function-declaration]
     reg = readl(bch->base + JZ_REG_NAND_ECC_CTRL);
           ^~~~~
           d_real
   drivers/mtd/nand/raw/ingenic/jz4740_bch.c: In function 'jz4740_bch_calculate':
   drivers/mtd/nand/raw/ingenic/jz4740_bch.c:83:17: error: implicit declaration of function 'readb' [-Werror=implicit-function-declaration]
      ecc_code[i] = readb(bch->base + JZ_REG_NAND_PAR0 + i);
                    ^~~~~
   drivers/mtd/nand/raw/ingenic/jz4740_bch.c: In function 'jz4740_bch_correct':
   drivers/mtd/nand/raw/ingenic/jz4740_bch.c:123:3: error: implicit declaration of function 'writeb'; did you mean 'up_write'? [-Werror=implicit-function-declaration]
      writeb(ecc_code[i], bch->base + JZ_REG_NAND_PAR0 + i);
      ^~~~~~
      up_write
   cc1: some warnings being treated as errors

vim +49 drivers/mtd/nand/raw/ingenic/jz4740_bch.c

    40	
    41	static void jz4740_bch_init(struct jz4780_bch *bch, bool encode)
    42	{
    43		uint32_t reg;
    44	
    45		/* Clear interrupt status */
  > 46		writel(0, bch->base + JZ_REG_NAND_IRQ_STAT);
    47	
    48		/* Initialize and enable BCH */
  > 49		reg = readl(bch->base + JZ_REG_NAND_ECC_CTRL);
    50		reg |= JZ_NAND_ECC_CTRL_RESET;
    51		reg |= JZ_NAND_ECC_CTRL_ENABLE;
    52		reg |= JZ_NAND_ECC_CTRL_RS;
    53		if (encode)
    54			reg |= JZ_NAND_ECC_CTRL_ENCODING;
    55		else
    56			reg &= ~JZ_NAND_ECC_CTRL_ENCODING;
    57	
    58		writel(reg, bch->base + JZ_REG_NAND_ECC_CTRL);
    59	}
    60	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

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

Re: [PATCH v2 9/9] mtd: rawnand: jz4780-bch: Add support for the JZ4740

[kbuild test robot]

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

Hi Paul,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on mtd/nand/next]
[also build test ERROR on v5.0-rc4]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Paul-Cercueil/Ingenic-JZ4780-NAND-patchset-v2/20190204-163709
base:   git://git.infradead.org/linux-mtd.git nand/next
config: sh-allmodconfig (attached as .config)
compiler: sh4-linux-gnu-gcc (Debian 8.2.0-11) 8.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=8.2.0 make.cross ARCH=sh 

All errors (new ones prefixed by >>):

   drivers/mtd/nand/raw/ingenic/jz4740_bch.c: In function 'jz4740_bch_init':
>> drivers/mtd/nand/raw/ingenic/jz4740_bch.c:46:2: error: implicit declaration of function 'writel' [-Werror=implicit-function-declaration]
     writel(0, bch->base + JZ_REG_NAND_IRQ_STAT);
     ^~~~~~
>> drivers/mtd/nand/raw/ingenic/jz4740_bch.c:49:8: error: implicit declaration of function 'readl' [-Werror=implicit-function-declaration]
     reg = readl(bch->base + JZ_REG_NAND_ECC_CTRL);
           ^~~~~
   drivers/mtd/nand/raw/ingenic/jz4740_bch.c: In function 'jz4740_bch_calculate':
>> drivers/mtd/nand/raw/ingenic/jz4740_bch.c:83:17: error: implicit declaration of function 'readb' [-Werror=implicit-function-declaration]
      ecc_code[i] = readb(bch->base + JZ_REG_NAND_PAR0 + i);
                    ^~~~~
   drivers/mtd/nand/raw/ingenic/jz4740_bch.c: In function 'jz4740_bch_correct':
>> drivers/mtd/nand/raw/ingenic/jz4740_bch.c:123:3: error: implicit declaration of function 'writeb'; did you mean 'up_write'? [-Werror=implicit-function-declaration]
      writeb(ecc_code[i], bch->base + JZ_REG_NAND_PAR0 + i);
      ^~~~~~
      up_write
   cc1: some warnings being treated as errors

vim +/writel +46 drivers/mtd/nand/raw/ingenic/jz4740_bch.c

    40	
    41	static void jz4740_bch_init(struct jz4780_bch *bch, bool encode)
    42	{
    43		uint32_t reg;
    44	
    45		/* Clear interrupt status */
  > 46		writel(0, bch->base + JZ_REG_NAND_IRQ_STAT);
    47	
    48		/* Initialize and enable BCH */
  > 49		reg = readl(bch->base + JZ_REG_NAND_ECC_CTRL);
    50		reg |= JZ_NAND_ECC_CTRL_RESET;
    51		reg |= JZ_NAND_ECC_CTRL_ENABLE;
    52		reg |= JZ_NAND_ECC_CTRL_RS;
    53		if (encode)
    54			reg |= JZ_NAND_ECC_CTRL_ENCODING;
    55		else
    56			reg &= ~JZ_NAND_ECC_CTRL_ENCODING;
    57	
    58		writel(reg, bch->base + JZ_REG_NAND_ECC_CTRL);
    59	}
    60	
    61	static int jz4740_bch_calculate(struct jz4780_bch *bch,
    62					struct jz4780_bch_params *params,
    63					const u8 *buf, u8 *ecc_code)
    64	{
    65		uint32_t reg, status;
    66		unsigned int timeout = 1000;
    67		int i;
    68	
    69		jz4740_bch_init(bch, true);
    70	
    71		do {
    72			status = readl(bch->base + JZ_REG_NAND_IRQ_STAT);
    73		} while (!(status & JZ_NAND_STATUS_ENC_FINISH) && --timeout);
    74	
    75		if (timeout == 0)
    76			return -ETIMEDOUT;
    77	
    78		reg = readl(bch->base + JZ_REG_NAND_ECC_CTRL);
    79		reg &= ~JZ_NAND_ECC_CTRL_ENABLE;
    80		writel(reg, bch->base + JZ_REG_NAND_ECC_CTRL);
    81	
    82		for (i = 0; i < params->bytes; ++i)
  > 83			ecc_code[i] = readb(bch->base + JZ_REG_NAND_PAR0 + i);
    84	
    85		/* If the written data is completely 0xff, we also want to write 0xff as
    86		 * ecc, otherwise we will get in trouble when doing subpage writes.
    87		 */
    88		if (memcmp(ecc_code, empty_block_ecc, ARRAY_SIZE(empty_block_ecc)) == 0)
    89			memset(ecc_code, 0xff, ARRAY_SIZE(empty_block_ecc));
    90	
    91		return 0;
    92	}
    93	
    94	static void jz_nand_correct_data(uint8_t *buf, int index, int mask)
    95	{
    96		int offset = index & 0x7;
    97		uint16_t data;
    98	
    99		index += (index >> 3);
   100	
   101		data = buf[index];
   102		data |= buf[index + 1] << 8;
   103	
   104		mask ^= (data >> offset) & 0x1ff;
   105		data &= ~(0x1ff << offset);
   106		data |= (mask << offset);
   107	
   108		buf[index] = data & 0xff;
   109		buf[index + 1] = (data >> 8) & 0xff;
   110	}
   111	
   112	static int jz4740_bch_correct(struct jz4780_bch *bch,
   113				      struct jz4780_bch_params *params,
   114				      u8 *buf, u8 *ecc_code)
   115	{
   116		int i, error_count, index;
   117		uint32_t reg, status, error;
   118		unsigned int timeout = 1000;
   119	
   120		jz4740_bch_init(bch, false);
   121	
   122		for (i = 0; i < params->bytes; ++i)
 > 123			writeb(ecc_code[i], bch->base + JZ_REG_NAND_PAR0 + i);
   124	
   125		reg = readl(bch->base + JZ_REG_NAND_ECC_CTRL);
   126		reg |= JZ_NAND_ECC_CTRL_PAR_READY;
   127		writel(reg, bch->base + JZ_REG_NAND_ECC_CTRL);
   128	
   129		do {
   130			status = readl(bch->base + JZ_REG_NAND_IRQ_STAT);
   131		} while (!(status & JZ_NAND_STATUS_DEC_FINISH) && --timeout);
   132	
   133		if (timeout == 0)
   134			return -ETIMEDOUT;
   135	
   136		reg = readl(bch->base + JZ_REG_NAND_ECC_CTRL);
   137		reg &= ~JZ_NAND_ECC_CTRL_ENABLE;
   138		writel(reg, bch->base + JZ_REG_NAND_ECC_CTRL);
   139	
   140		if (status & JZ_NAND_STATUS_ERROR) {
   141			if (status & JZ_NAND_STATUS_UNCOR_ERROR)
   142				return -EBADMSG;
   143	
   144			error_count = (status & JZ_NAND_STATUS_ERR_COUNT) >> 29;
   145	
   146			for (i = 0; i < error_count; ++i) {
   147				error = readl(bch->base + JZ_REG_NAND_ERR(i));
   148				index = ((error >> 16) & 0x1ff) - 1;
   149				if (index >= 0 && index < params->size)
   150					jz_nand_correct_data(buf, index, error & 0x1ff);
   151			}
   152	
   153			return error_count;
   154		}
   155	
   156		return 0;
   157	}
   158	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

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

Re: [PATCH v2 0/9] Ingenic JZ4780 NAND patchset v2

[Boris Brezillon]

On Sat,  2 Feb 2019 20:19:17 -0300
Paul Cercueil <paul@crapouillou.net> wrote:

> Hi,
> 
> As requested by Boris, I added a patch to move all the Ingenic NAND
> drivers to their own directory.
> 
> In this V2 I added support for the JZ4740 SoC. The combo of the
> jz4780-nemc, jz4780-nand and jz4740-bch now obsolete the old and dusty
> jz4740-nand driver.

Any plan to get the old driver removed? I guess you want to migrate the
boards to DT first.

> 
> To support the only upstream JZ4740-based board we have, the Ben
> Nanonote, I added an option to specify the OOB layout of that device
> from a device property string.
> 
> Finally, I dropped the last two patches that moved the platform NAND
> code to devicetree; I will upstream them as part of a different
> patchset.
> 
> Cheers,
> -Paul
> 

Re: [PATCH v2 0/9] Ingenic JZ4780 NAND patchset v2

[Paul Cercueil]

Hi,

Le dim. 3 févr. 2019 à 4:20, Boris Brezillon <bbrezillon@kernel.org> 
a écrit :
> On Sat,  2 Feb 2019 20:19:17 -0300
> Paul Cercueil <paul@crapouillou.net> wrote:
> 
>>  Hi,
>> 
>>  As requested by Boris, I added a patch to move all the Ingenic NAND
>>  drivers to their own directory.
>> 
>>  In this V2 I added support for the JZ4740 SoC. The combo of the
>>  jz4780-nemc, jz4780-nand and jz4740-bch now obsolete the old and 
>> dusty
>>  jz4740-nand driver.
> 
> Any plan to get the old driver removed? I guess you want to migrate 
> the
> boards to DT first.

Yes, that's the plan - migrate the qi_lb60 board first then drop the old
driver.

>> 
>>  To support the only upstream JZ4740-based board we have, the Ben
>>  Nanonote, I added an option to specify the OOB layout of that device
>>  from a device property string.
>> 
>>  Finally, I dropped the last two patches that moved the platform NAND
>>  code to devicetree; I will upstream them as part of a different
>>  patchset.
>> 
>>  Cheers,
>>  -Paul
>> 
> 

Re: [PATCH v2 0/9] Ingenic JZ4780 NAND patchset v2

[Boris Brezillon]

On Sun, 03 Feb 2019 10:01:36 -0300
Paul Cercueil <paul@crapouillou.net> wrote:

> Hi,
> 
> Le dim. 3 févr. 2019 à 4:20, Boris Brezillon <bbrezillon@kernel.org> 
> a écrit :
> > On Sat,  2 Feb 2019 20:19:17 -0300
> > Paul Cercueil <paul@crapouillou.net> wrote:
> >   
> >>  Hi,
> >> 
> >>  As requested by Boris, I added a patch to move all the Ingenic NAND
> >>  drivers to their own directory.
> >> 
> >>  In this V2 I added support for the JZ4740 SoC. The combo of the
> >>  jz4780-nemc, jz4780-nand and jz4740-bch now obsolete the old and 
> >> dusty
> >>  jz4740-nand driver.  
> > 
> > Any plan to get the old driver removed? I guess you want to migrate 
> > the
> > boards to DT first.  
> 
> Yes, that's the plan - migrate the qi_lb60 board first then drop the old
> driver.

Good, I hate having 2 drivers for the same IP live in parallel for too
long.