linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/6] support gpmi on i.MX6UL/7D and HW bitflip on i.MX6QP/7D
@ 2016-06-10 17:01 Han Xu
  2016-06-10 17:01 ` [PATCH v2 1/6] mtd: nand: gpmi: add GPMI NAND support for i.MX7D Han Xu
                   ` (5 more replies)
  0 siblings, 6 replies; 13+ messages in thread
From: Han Xu @ 2016-06-10 17:01 UTC (permalink / raw)
  To: han.xu, boris.brezillon, richard, dwmw2, computersforpeace,
	robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak,
	devicetree
  Cc: linux-mtd, linux-kernel

add support for gpmi nand on i.MX6UL and i.MX7D, document the related
properties in DT and add the HW bitflip detection and correction for
i.MX6QP and i.MX7D.

v1 ---> v2:
fix several indent and open parenthesis issues
drop off the void cast
rebase to latest code and adopt to SW bitflip code
split bitflip and i.MX6QP code

Han Xu (6):
  mtd: nand: gpmi: add GPMI NAND support for i.MX7D
  mtd: nand: gpmi: document the clocks and clock-names in DT property
  mtd: nand: gpmi: add GPMI NAND support for i.MX6QP
  mtd: nand: gpmi: correct bitflip for erased NAND page
  mtd: nand: gpmi: support NAND on i.MX6UL
  mtd: nand: gpmi: document the new supported chip in DT

 .../devicetree/bindings/mtd/gpmi-nand.txt          | 12 +++-
 drivers/mtd/nand/gpmi-nand/bch-regs.h              | 24 +++++--
 drivers/mtd/nand/gpmi-nand/gpmi-lib.c              | 15 +++--
 drivers/mtd/nand/gpmi-nand/gpmi-nand.c             | 75 ++++++++++++++++++++--
 drivers/mtd/nand/gpmi-nand/gpmi-nand.h             | 15 ++++-
 5 files changed, 119 insertions(+), 22 deletions(-)

-- 
1.9.1

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

* [PATCH v2 1/6] mtd: nand: gpmi: add GPMI NAND support for i.MX7D
  2016-06-10 17:01 [PATCH v2 0/6] support gpmi on i.MX6UL/7D and HW bitflip on i.MX6QP/7D Han Xu
@ 2016-06-10 17:01 ` Han Xu
  2016-06-14 20:05   ` Boris Brezillon
  2016-06-10 17:01 ` [PATCH v2 2/6] mtd: nand: gpmi: document the clocks and clock-names in DT property Han Xu
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: Han Xu @ 2016-06-10 17:01 UTC (permalink / raw)
  To: han.xu, boris.brezillon, richard, dwmw2, computersforpeace,
	robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak,
	devicetree
  Cc: linux-mtd, linux-kernel

support GPMI NAND on i.MX7D

Signed-off-by: Han Xu <han.xu@nxp.com>
---
 drivers/mtd/nand/gpmi-nand/bch-regs.h  | 14 +++++++-------
 drivers/mtd/nand/gpmi-nand/gpmi-lib.c  | 10 ++++++----
 drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 30 ++++++++++++++++++++++++------
 drivers/mtd/nand/gpmi-nand/gpmi-nand.h |  7 +++++--
 4 files changed, 42 insertions(+), 19 deletions(-)

diff --git a/drivers/mtd/nand/gpmi-nand/bch-regs.h b/drivers/mtd/nand/gpmi-nand/bch-regs.h
index 05bb91f..228142c 100644
--- a/drivers/mtd/nand/gpmi-nand/bch-regs.h
+++ b/drivers/mtd/nand/gpmi-nand/bch-regs.h
@@ -1,7 +1,7 @@
 /*
  * Freescale GPMI NAND Flash Driver
  *
- * Copyright 2008-2011 Freescale Semiconductor, Inc.
+ * Copyright 2008-2016 Freescale Semiconductor, Inc.
  * Copyright 2008 Embedded Alley Solutions, Inc.
  *
  * This program is free software; you can redistribute it and/or modify
@@ -54,7 +54,7 @@
 #define MX6Q_BP_BCH_FLASH0LAYOUT0_ECC0		11
 #define MX6Q_BM_BCH_FLASH0LAYOUT0_ECC0	(0x1f << MX6Q_BP_BCH_FLASH0LAYOUT0_ECC0)
 #define BF_BCH_FLASH0LAYOUT0_ECC0(v, x)				\
-	(GPMI_IS_MX6(x)					\
+	((GPMI_IS_MX6(x) || GPMI_IS_MX7(x))				\
 		? (((v) << MX6Q_BP_BCH_FLASH0LAYOUT0_ECC0)	\
 			& MX6Q_BM_BCH_FLASH0LAYOUT0_ECC0)	\
 		: (((v) << BP_BCH_FLASH0LAYOUT0_ECC0)		\
@@ -65,7 +65,7 @@
 #define MX6Q_BM_BCH_FLASH0LAYOUT0_GF_13_14			\
 				(0x1 << MX6Q_BP_BCH_FLASH0LAYOUT0_GF_13_14)
 #define BF_BCH_FLASH0LAYOUT0_GF(v, x)				\
-	((GPMI_IS_MX6(x) && ((v) == 14))			\
+	(((GPMI_IS_MX6(x) || GPMI_IS_MX7(x)) && ((v) == 14))	\
 		? (((1) << MX6Q_BP_BCH_FLASH0LAYOUT0_GF_13_14)	\
 			& MX6Q_BM_BCH_FLASH0LAYOUT0_GF_13_14)	\
 		: 0						\
@@ -77,7 +77,7 @@
 #define MX6Q_BM_BCH_FLASH0LAYOUT0_DATA0_SIZE	\
 			(0x3ff << BP_BCH_FLASH0LAYOUT0_DATA0_SIZE)
 #define BF_BCH_FLASH0LAYOUT0_DATA0_SIZE(v, x)				\
-	(GPMI_IS_MX6(x)						\
+	((GPMI_IS_MX6(x) || GPMI_IS_MX7(x))	\
 		? (((v) >> 2) & MX6Q_BM_BCH_FLASH0LAYOUT0_DATA0_SIZE)	\
 		: ((v) & BM_BCH_FLASH0LAYOUT0_DATA0_SIZE)		\
 	)
@@ -96,7 +96,7 @@
 #define MX6Q_BP_BCH_FLASH0LAYOUT1_ECCN		11
 #define MX6Q_BM_BCH_FLASH0LAYOUT1_ECCN	(0x1f << MX6Q_BP_BCH_FLASH0LAYOUT1_ECCN)
 #define BF_BCH_FLASH0LAYOUT1_ECCN(v, x)				\
-	(GPMI_IS_MX6(x)					\
+	((GPMI_IS_MX6(x) || GPMI_IS_MX7(x))				\
 		? (((v) << MX6Q_BP_BCH_FLASH0LAYOUT1_ECCN)	\
 			& MX6Q_BM_BCH_FLASH0LAYOUT1_ECCN)	\
 		: (((v) << BP_BCH_FLASH0LAYOUT1_ECCN)		\
@@ -107,7 +107,7 @@
 #define MX6Q_BM_BCH_FLASH0LAYOUT1_GF_13_14			\
 				(0x1 << MX6Q_BP_BCH_FLASH0LAYOUT1_GF_13_14)
 #define BF_BCH_FLASH0LAYOUT1_GF(v, x)				\
-	((GPMI_IS_MX6(x) && ((v) == 14))			\
+	(((GPMI_IS_MX6(x) || GPMI_IS_MX7(x)) && ((v) == 14))	\
 		? (((1) << MX6Q_BP_BCH_FLASH0LAYOUT1_GF_13_14)	\
 			& MX6Q_BM_BCH_FLASH0LAYOUT1_GF_13_14)	\
 		: 0						\
@@ -119,7 +119,7 @@
 #define MX6Q_BM_BCH_FLASH0LAYOUT1_DATAN_SIZE	\
 			(0x3ff << BP_BCH_FLASH0LAYOUT1_DATAN_SIZE)
 #define BF_BCH_FLASH0LAYOUT1_DATAN_SIZE(v, x)				\
-	(GPMI_IS_MX6(x)						\
+	((GPMI_IS_MX6(x) || GPMI_IS_MX7(x))	\
 		? (((v) >> 2) & MX6Q_BM_BCH_FLASH0LAYOUT1_DATAN_SIZE)	\
 		: ((v) & BM_BCH_FLASH0LAYOUT1_DATAN_SIZE)		\
 	)
diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-lib.c b/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
index 0f68a99..358ff5d 100644
--- a/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
@@ -1,7 +1,7 @@
 /*
  * Freescale GPMI NAND Flash Driver
  *
- * Copyright (C) 2008-2011 Freescale Semiconductor, Inc.
+ * Copyright (C) 2008-2016 Freescale Semiconductor, Inc.
  * Copyright (C) 2008 Embedded Alley Solutions, Inc.
  *
  * This program is free software; you can redistribute it and/or modify
@@ -971,7 +971,8 @@ int gpmi_extra_init(struct gpmi_nand_data *this)
 	struct nand_chip *chip = &this->nand;
 
 	/* Enable the asynchronous EDO feature. */
-	if (GPMI_IS_MX6(this) && chip->onfi_version) {
+	if ((GPMI_IS_MX6(this) || GPMI_IS_MX7(this)) &&
+	    chip->onfi_version) {
 		int mode = onfi_get_async_timing_mode(chip);
 
 		/* We only support the timing mode 4 and mode 5. */
@@ -1093,12 +1094,13 @@ int gpmi_is_ready(struct gpmi_nand_data *this, unsigned chip)
 	if (GPMI_IS_MX23(this)) {
 		mask = MX23_BM_GPMI_DEBUG_READY0 << chip;
 		reg = readl(r->gpmi_regs + HW_GPMI_DEBUG);
-	} else if (GPMI_IS_MX28(this) || GPMI_IS_MX6(this)) {
+	} else if (GPMI_IS_MX28(this) || GPMI_IS_MX6(this) ||
+	           GPMI_IS_MX7(this)) {
 		/*
 		 * In the imx6, all the ready/busy pins are bound
 		 * together. So we only need to check chip 0.
 		 */
-		if (GPMI_IS_MX6(this))
+		if (GPMI_IS_MX6(this) || GPMI_IS_MX7(this))
 			chip = 0;
 
 		/* MX28 shares the same R/B register as MX6Q. */
diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
index 6e46156..a8936fd 100644
--- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
@@ -1,7 +1,7 @@
 /*
  * Freescale GPMI NAND Flash Driver
  *
- * Copyright (C) 2010-2015 Freescale Semiconductor, Inc.
+ * Copyright (C) 2010-2016 Freescale Semiconductor, Inc.
  * Copyright (C) 2008 Embedded Alley Solutions, Inc.
  *
  * This program is free software; you can redistribute it and/or modify
@@ -110,6 +110,12 @@ static const struct gpmi_devdata gpmi_devdata_imx6sx = {
 	.max_chain_delay = 12,
 };
 
+static const struct gpmi_devdata gpmi_devdata_imx7d = {
+	.type = IS_MX7D,
+	.bch_max_ecc_strength = 62,
+	.max_chain_delay = 12,
+};
+
 static irqreturn_t bch_irq(int irq, void *cookie)
 {
 	struct gpmi_nand_data *this = cookie;
@@ -601,6 +607,10 @@ static char *extra_clks_for_mx6q[GPMI_CLK_MAX] = {
 	"gpmi_apb", "gpmi_bch", "gpmi_bch_apb", "per1_bch",
 };
 
+static char *extra_clks_for_mx7d[GPMI_CLK_MAX] = {
+	"gpmi_bch_apb",
+};
+
 static int gpmi_get_clks(struct gpmi_nand_data *this)
 {
 	struct resources *r = &this->resources;
@@ -618,6 +628,8 @@ static int gpmi_get_clks(struct gpmi_nand_data *this)
 	/* Get extra clocks */
 	if (GPMI_IS_MX6(this))
 		extra_clks = extra_clks_for_mx6q;
+	if (GPMI_IS_MX7(this))
+		extra_clks = extra_clks_for_mx7d;
 	if (!extra_clks)
 		return 0;
 
@@ -634,7 +646,7 @@ static int gpmi_get_clks(struct gpmi_nand_data *this)
 		r->clock[i] = clk;
 	}
 
-	if (GPMI_IS_MX6(this))
+	if (GPMI_IS_MX6(this) || GPMI_IS_MX7(this))
 		/*
 		 * Set the default value for the gpmi clock.
 		 *
@@ -1965,8 +1977,8 @@ static int gpmi_init_last(struct gpmi_nand_data *this)
 	 *  (1) the chip is imx6, and
 	 *  (2) the size of the ECC parity is byte aligned.
 	 */
-	if (GPMI_IS_MX6(this) &&
-		((bch_geo->gf_len * bch_geo->ecc_strength) % 8) == 0) {
+	if ((GPMI_IS_MX6(this) || GPMI_IS_MX7(this)) &&
+	    ((bch_geo->gf_len * bch_geo->ecc_strength) % 8) == 0) {
 		ecc->read_subpage = gpmi_ecc_read_subpage;
 		chip->options |= NAND_SUBPAGE_READ;
 	}
@@ -1987,6 +1999,7 @@ static int gpmi_nand_init(struct gpmi_nand_data *this)
 	struct nand_chip *chip = &this->nand;
 	struct mtd_info  *mtd = nand_to_mtd(chip);
 	int ret;
+	int max_chips;
 
 	/* init current chip */
 	this->current_chip	= -1;
@@ -2021,7 +2034,8 @@ static int gpmi_nand_init(struct gpmi_nand_data *this)
 	if (ret)
 		goto err_out;
 
-	ret = nand_scan_ident(mtd, GPMI_IS_MX6(this) ? 2 : 1, NULL);
+	max_chips = (GPMI_IS_MX6(this) || GPMI_IS_MX7(this)) ? 2 : 1;
+	ret = nand_scan_ident(mtd, max_chips, NULL);
 	if (ret)
 		goto err_out;
 
@@ -2074,7 +2088,11 @@ static const struct of_device_id gpmi_nand_id_table[] = {
 	}, {
 		.compatible = "fsl,imx6sx-gpmi-nand",
 		.data = &gpmi_devdata_imx6sx,
-	}, {}
+	}, {
+		.compatible = "fsl,imx7d-gpmi-nand",
+		.data = &gpmi_devdata_imx7d,
+	}, { /* sentinel */ }
+
 };
 MODULE_DEVICE_TABLE(of, gpmi_nand_id_table);
 
diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.h b/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
index 4e49a1f..04a3584 100644
--- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
@@ -1,7 +1,7 @@
 /*
  * Freescale GPMI NAND Flash Driver
  *
- * Copyright (C) 2010-2011 Freescale Semiconductor, Inc.
+ * Copyright (C) 2010-2016 Freescale Semiconductor, Inc.
  * Copyright (C) 2008 Embedded Alley Solutions, Inc.
  *
  * This program is free software; you can redistribute it and/or modify
@@ -123,7 +123,8 @@ enum gpmi_type {
 	IS_MX23,
 	IS_MX28,
 	IS_MX6Q,
-	IS_MX6SX
+	IS_MX6SX,
+	IS_MX7D,
 };
 
 struct gpmi_devdata {
@@ -305,6 +306,8 @@ void gpmi_copy_bits(u8 *dst, size_t dst_bit_off,
 #define GPMI_IS_MX28(x)		((x)->devdata->type == IS_MX28)
 #define GPMI_IS_MX6Q(x)		((x)->devdata->type == IS_MX6Q)
 #define GPMI_IS_MX6SX(x)	((x)->devdata->type == IS_MX6SX)
+#define GPMI_IS_MX7D(x)		((x)->devdata->type == IS_MX7D)
 
 #define GPMI_IS_MX6(x)		(GPMI_IS_MX6Q(x) || GPMI_IS_MX6SX(x))
+#define GPMI_IS_MX7(x)		(GPMI_IS_MX7D(x))
 #endif
-- 
1.9.1

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

* [PATCH v2 2/6] mtd: nand: gpmi: document the clocks and clock-names in DT property
  2016-06-10 17:01 [PATCH v2 0/6] support gpmi on i.MX6UL/7D and HW bitflip on i.MX6QP/7D Han Xu
  2016-06-10 17:01 ` [PATCH v2 1/6] mtd: nand: gpmi: add GPMI NAND support for i.MX7D Han Xu
@ 2016-06-10 17:01 ` Han Xu
  2016-06-14 12:46   ` Rob Herring
  2016-06-10 17:01 ` [PATCH v2 3/6] mtd: nand: gpmi: add GPMI NAND support for i.MX6QP Han Xu
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: Han Xu @ 2016-06-10 17:01 UTC (permalink / raw)
  To: han.xu, boris.brezillon, richard, dwmw2, computersforpeace,
	robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak,
	devicetree
  Cc: linux-mtd, linux-kernel

add the clocks and clock-names in DT property, gpmi-io clock is
mandatory for all platforms, but some platforms, such as i.MX6Q may
need more extra clocks for submodules. More details please refer to the
SoC reference manual.

Signed-off-by: Han Xu <han.xu@nxp.com>
---
 Documentation/devicetree/bindings/mtd/gpmi-nand.txt | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/Documentation/devicetree/bindings/mtd/gpmi-nand.txt b/Documentation/devicetree/bindings/mtd/gpmi-nand.txt
index d02acaf..c8d0e2f 100644
--- a/Documentation/devicetree/bindings/mtd/gpmi-nand.txt
+++ b/Documentation/devicetree/bindings/mtd/gpmi-nand.txt
@@ -13,6 +13,13 @@ Required properties:
     and GPMI DMA channel ID.
     Refer to dma.txt and fsl-mxs-dma.txt for details.
   - dma-names: Must be "rx-tx".
+  - clocks : The clocks needed by the gpmi controller. This field varies
+    depends on the SoC design, "gpmi-io" is mandatory but some platforms may
+    need several extra clocks, such as i.MX6Q, it requires "gpmi_apb,
+    gpmi_bch, gpmi_bch_apb and per1_bch" for all submodules. Please refer to
+    the HW design manual.
+  - clock-names : the name of the clocks, please refer to the HW design
+    manual.
 
 Optional properties:
   - nand-on-flash-bbt: boolean to enable on flash bbt option if not
@@ -51,6 +58,8 @@ gpmi-nand@8000c000 {
 	interrupt-names = "bch";
 	dmas = <&dma_apbh 4>;
 	dma-names = "rx-tx";
+	clocks = <&clks 50>,;
+	clock-names = "gpmi_io";
 
 	partition@0 {
 	...
-- 
1.9.1

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

* [PATCH v2 3/6] mtd: nand: gpmi: add GPMI NAND support for i.MX6QP
  2016-06-10 17:01 [PATCH v2 0/6] support gpmi on i.MX6UL/7D and HW bitflip on i.MX6QP/7D Han Xu
  2016-06-10 17:01 ` [PATCH v2 1/6] mtd: nand: gpmi: add GPMI NAND support for i.MX7D Han Xu
  2016-06-10 17:01 ` [PATCH v2 2/6] mtd: nand: gpmi: document the clocks and clock-names in DT property Han Xu
@ 2016-06-10 17:01 ` Han Xu
  2016-06-10 17:01 ` [PATCH v2 4/6] mtd: nand: gpmi: correct bitflip for erased NAND page Han Xu
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 13+ messages in thread
From: Han Xu @ 2016-06-10 17:01 UTC (permalink / raw)
  To: han.xu, boris.brezillon, richard, dwmw2, computersforpeace,
	robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak,
	devicetree
  Cc: linux-mtd, linux-kernel

support GPMI NAND on i.MX6QP

Signed-off-by: Han Xu <han.xu@nxp.com>
---
 drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 9 +++++++++
 drivers/mtd/nand/gpmi-nand/gpmi-nand.h | 5 ++++-
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
index a8936fd..aedaff3 100644
--- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
@@ -104,6 +104,12 @@ static const struct gpmi_devdata gpmi_devdata_imx6q = {
 	.max_chain_delay = 12,
 };
 
+static const struct gpmi_devdata gpmi_devdata_imx6qp = {
+	.type = IS_MX6QP,
+	.bch_max_ecc_strength = 40,
+	.max_chain_delay = 12,
+};
+
 static const struct gpmi_devdata gpmi_devdata_imx6sx = {
 	.type = IS_MX6SX,
 	.bch_max_ecc_strength = 62,
@@ -2086,6 +2092,9 @@ static const struct of_device_id gpmi_nand_id_table[] = {
 		.compatible = "fsl,imx6q-gpmi-nand",
 		.data = &gpmi_devdata_imx6q,
 	}, {
+		.compatible = "fsl,imx6qp-gpmi-nand",
+		.data = &gpmi_devdata_imx6qp,
+	}, {
 		.compatible = "fsl,imx6sx-gpmi-nand",
 		.data = &gpmi_devdata_imx6sx,
 	}, {
diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.h b/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
index 04a3584..1cee620 100644
--- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
@@ -123,6 +123,7 @@ enum gpmi_type {
 	IS_MX23,
 	IS_MX28,
 	IS_MX6Q,
+	IS_MX6QP,
 	IS_MX6SX,
 	IS_MX7D,
 };
@@ -305,9 +306,11 @@ void gpmi_copy_bits(u8 *dst, size_t dst_bit_off,
 #define GPMI_IS_MX23(x)		((x)->devdata->type == IS_MX23)
 #define GPMI_IS_MX28(x)		((x)->devdata->type == IS_MX28)
 #define GPMI_IS_MX6Q(x)		((x)->devdata->type == IS_MX6Q)
+#define GPMI_IS_MX6QP(x)	((x)->devdata->type == IS_MX6QP)
 #define GPMI_IS_MX6SX(x)	((x)->devdata->type == IS_MX6SX)
 #define GPMI_IS_MX7D(x)		((x)->devdata->type == IS_MX7D)
 
-#define GPMI_IS_MX6(x)		(GPMI_IS_MX6Q(x) || GPMI_IS_MX6SX(x))
+#define GPMI_IS_MX6(x)		(GPMI_IS_MX6Q(x) || GPMI_IS_MX6QP(x)	\
+				|| GPMI_IS_MX6SX(x))
 #define GPMI_IS_MX7(x)		(GPMI_IS_MX7D(x))
 #endif
-- 
1.9.1

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

* [PATCH v2 4/6] mtd: nand: gpmi: correct bitflip for erased NAND page
  2016-06-10 17:01 [PATCH v2 0/6] support gpmi on i.MX6UL/7D and HW bitflip on i.MX6QP/7D Han Xu
                   ` (2 preceding siblings ...)
  2016-06-10 17:01 ` [PATCH v2 3/6] mtd: nand: gpmi: add GPMI NAND support for i.MX6QP Han Xu
@ 2016-06-10 17:01 ` Han Xu
  2016-06-14 19:14   ` Boris Brezillon
  2016-06-10 17:01 ` [PATCH v2 5/6] mtd: nand: gpmi: support NAND on i.MX6UL Han Xu
  2016-06-10 17:01 ` [PATCH v2 6/6] mtd: nand: gpmi: document the new supported chip in DT Han Xu
  5 siblings, 1 reply; 13+ messages in thread
From: Han Xu @ 2016-06-10 17:01 UTC (permalink / raw)
  To: han.xu, boris.brezillon, richard, dwmw2, computersforpeace,
	robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak,
	devicetree
  Cc: linux-mtd, linux-kernel

i.MX6QP and i.MX7D BCH module integrated a new feature to detect the
bitflip number for erased NAND page. So for these two platform, set the
erase threshold to ecc_strength and if bitflip detected, GPMI driver
will correct the data to all 0xFF.

Signed-off-by: Han Xu <han.xu@nxp.com>
---
 drivers/mtd/nand/gpmi-nand/bch-regs.h  | 10 ++++++++++
 drivers/mtd/nand/gpmi-nand/gpmi-lib.c  |  5 +++++
 drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 27 ++++++++++++++++++++++++++-
 3 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/gpmi-nand/bch-regs.h b/drivers/mtd/nand/gpmi-nand/bch-regs.h
index 228142c..2c44b88 100644
--- a/drivers/mtd/nand/gpmi-nand/bch-regs.h
+++ b/drivers/mtd/nand/gpmi-nand/bch-regs.h
@@ -30,7 +30,13 @@
 #define BM_BCH_CTRL_COMPLETE_IRQ		(1 << 0)
 
 #define HW_BCH_STATUS0				0x00000010
+
 #define HW_BCH_MODE				0x00000020
+#define BP_BCH_MODE_ERASE_THRESHOLD		0
+#define BM_BCH_MODE_ERASE_THRESHOLD	(0xff << BP_BCH_MODE_ERASE_THRESHOLD)
+#define BF_BCH_MODE_ERASE_THRESHOLD(v)		\
+	(((v) << BP_BCH_MODE_ERASE_THRESHOLD) & BM_BCH_MODE_ERASE_THRESHOLD)
+
 #define HW_BCH_ENCODEPTR			0x00000030
 #define HW_BCH_DATAPTR				0x00000040
 #define HW_BCH_METAPTR				0x00000050
@@ -125,4 +131,8 @@
 	)
 
 #define HW_BCH_VERSION				0x00000160
+#define HW_BCH_DEBUG1				0x00000170
+#define BP_BCH_DEBUG1_ERASED_ZERO_COUNT	0
+#define BM_BCH_DEBUG1_ERASED_ZERO_COUNT		\
+		(0x1ff << BP_BCH_DEBUG1_ERASED_ZERO_COUNT)
 #endif
diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-lib.c b/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
index 358ff5d..0b5666a 100644
--- a/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
@@ -298,6 +298,11 @@ int bch_set_geometry(struct gpmi_nand_data *this)
 			| BF_BCH_FLASH0LAYOUT1_DATAN_SIZE(block_size, this),
 			r->bch_regs + HW_BCH_FLASH0LAYOUT1);
 
+	/* Set erase threshold to ecc_strength for mx6qp and mx7 */
+	if (GPMI_IS_MX6QP(this) || GPMI_IS_MX7(this))
+		writel(BF_BCH_MODE_ERASE_THRESHOLD(ecc_strength),
+			r->bch_regs + HW_BCH_MODE);
+
 	/* Set *all* chip selects to use layout 0. */
 	writel(0, r->bch_regs + HW_BCH_LAYOUTSELECT);
 
diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
index aedaff3..03bdb4d 100644
--- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
@@ -1043,6 +1043,7 @@ static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
 {
 	struct gpmi_nand_data *this = nand_get_controller_data(chip);
 	struct bch_geometry *nfc_geo = &this->bch_geometry;
+	void __iomem *bch_regs = this->resources.bch_regs;
 	void          *payload_virt;
 	dma_addr_t    payload_phys;
 	void          *auxiliary_virt;
@@ -1051,6 +1052,8 @@ static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
 	unsigned char *status;
 	unsigned int  max_bitflips = 0;
 	int           ret;
+	int bitflips = 0;
+	int bitflip_flag = 0;
 
 	dev_dbg(this->dev, "page number is : %d\n", page);
 	ret = read_page_prepare(this, buf, nfc_geo->payload_size,
@@ -1088,8 +1091,20 @@ static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
 			   payload_virt, payload_phys);
 
 	for (i = 0; i < nfc_geo->ecc_chunk_count; i++, status++) {
-		if ((*status == STATUS_GOOD) || (*status == STATUS_ERASED))
+		if (*status == STATUS_GOOD)
+			continue;
+		if (*status == STATUS_ERASED) {
+			if (GPMI_IS_MX6QP(this) || GPMI_IS_MX7(this)) {
+				bitflips = readl(bch_regs + HW_BCH_DEBUG1);
+				if (bitflips) {
+					bitflip_flag = 1;
+					max_bitflips = max_t(unsigned int,
+							     max_bitflips,
+							     bitflips);
+				}
+			}
 			continue;
+		}
 
 		if (*status == STATUS_UNCORRECTABLE) {
 			int eccbits = nfc_geo->ecc_strength * nfc_geo->gf_len;
@@ -1098,6 +1113,10 @@ static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
 			int eccbytes;
 			int flips;
 
+			/* shortcut for i.MX7 and i.MX6QP */
+			if (GPMI_IS_MX6QP(this) || GPMI_IS_MX7(this))
+				continue;
+
 			/* Read ECC bytes into our internal raw_buffer */
 			offset = nfc_geo->metadata_size * 8;
 			offset += ((8 * nfc_geo->ecc_chunk_size) + eccbits) * (i + 1);
@@ -1182,6 +1201,12 @@ static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
 		chip->oob_poi[0] = ((uint8_t *) auxiliary_virt)[0];
 	}
 
+	/* if bitflip occurred in erased page, change data to all 0xff */
+	if (bitflip_flag) {
+		memset(buf, ~0, nfc_geo->payload_size);
+		memset(chip->oob_poi, ~0, mtd->oobsize);
+	}
+
 	return max_bitflips;
 }
 
-- 
1.9.1

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

* [PATCH v2 5/6] mtd: nand: gpmi: support NAND on i.MX6UL
  2016-06-10 17:01 [PATCH v2 0/6] support gpmi on i.MX6UL/7D and HW bitflip on i.MX6QP/7D Han Xu
                   ` (3 preceding siblings ...)
  2016-06-10 17:01 ` [PATCH v2 4/6] mtd: nand: gpmi: correct bitflip for erased NAND page Han Xu
@ 2016-06-10 17:01 ` Han Xu
  2016-06-10 17:01 ` [PATCH v2 6/6] mtd: nand: gpmi: document the new supported chip in DT Han Xu
  5 siblings, 0 replies; 13+ messages in thread
From: Han Xu @ 2016-06-10 17:01 UTC (permalink / raw)
  To: han.xu, boris.brezillon, richard, dwmw2, computersforpeace,
	robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak,
	devicetree
  Cc: linux-mtd, linux-kernel

support GPMI NAND on i.MX6UL

Signed-off-by: Han Xu <han.xu@nxp.com>
---
 drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 9 +++++++++
 drivers/mtd/nand/gpmi-nand/gpmi-nand.h | 5 ++++-
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
index 03bdb4d..133483f 100644
--- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
@@ -122,6 +122,12 @@ static const struct gpmi_devdata gpmi_devdata_imx7d = {
 	.max_chain_delay = 12,
 };
 
+static const struct gpmi_devdata gpmi_devdata_imx6ul = {
+	.type = IS_MX6UL,
+	.bch_max_ecc_strength = 40,
+	.max_chain_delay = 12,
+};
+
 static irqreturn_t bch_irq(int irq, void *cookie)
 {
 	struct gpmi_nand_data *this = cookie;
@@ -2123,6 +2129,9 @@ static const struct of_device_id gpmi_nand_id_table[] = {
 		.compatible = "fsl,imx6sx-gpmi-nand",
 		.data = &gpmi_devdata_imx6sx,
 	}, {
+		.compatible = "fsl,imx6ul-gpmi-nand",
+		.data = &gpmi_devdata_imx6ul,
+	}, {
 		.compatible = "fsl,imx7d-gpmi-nand",
 		.data = &gpmi_devdata_imx7d,
 	}, { /* sentinel */ }
diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.h b/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
index 1cee620..8de122e 100644
--- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
@@ -126,6 +126,7 @@ enum gpmi_type {
 	IS_MX6QP,
 	IS_MX6SX,
 	IS_MX7D,
+	IS_MX6UL,
 };
 
 struct gpmi_devdata {
@@ -309,8 +310,10 @@ void gpmi_copy_bits(u8 *dst, size_t dst_bit_off,
 #define GPMI_IS_MX6QP(x)	((x)->devdata->type == IS_MX6QP)
 #define GPMI_IS_MX6SX(x)	((x)->devdata->type == IS_MX6SX)
 #define GPMI_IS_MX7D(x)		((x)->devdata->type == IS_MX7D)
+#define GPMI_IS_MX6UL(x)	((x)->devdata->type == IS_MX6UL)
 
 #define GPMI_IS_MX6(x)		(GPMI_IS_MX6Q(x) || GPMI_IS_MX6QP(x)	\
-				|| GPMI_IS_MX6SX(x))
+				|| GPMI_IS_MX6SX(x) || GPMI_IS_MX6UL(x))
+
 #define GPMI_IS_MX7(x)		(GPMI_IS_MX7D(x))
 #endif
-- 
1.9.1

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

* [PATCH v2 6/6] mtd: nand: gpmi: document the new supported chip in DT
  2016-06-10 17:01 [PATCH v2 0/6] support gpmi on i.MX6UL/7D and HW bitflip on i.MX6QP/7D Han Xu
                   ` (4 preceding siblings ...)
  2016-06-10 17:01 ` [PATCH v2 5/6] mtd: nand: gpmi: support NAND on i.MX6UL Han Xu
@ 2016-06-10 17:01 ` Han Xu
  2016-06-14 12:48   ` Rob Herring
  5 siblings, 1 reply; 13+ messages in thread
From: Han Xu @ 2016-06-10 17:01 UTC (permalink / raw)
  To: han.xu, boris.brezillon, richard, dwmw2, computersforpeace,
	robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak,
	devicetree
  Cc: linux-mtd, linux-kernel

listed all supported chips in DT.

Signed-off-by: Han Xu <han.xu@nxp.com>
---
 Documentation/devicetree/bindings/mtd/gpmi-nand.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/mtd/gpmi-nand.txt b/Documentation/devicetree/bindings/mtd/gpmi-nand.txt
index c8d0e2f..ed48f69 100644
--- a/Documentation/devicetree/bindings/mtd/gpmi-nand.txt
+++ b/Documentation/devicetree/bindings/mtd/gpmi-nand.txt
@@ -4,7 +4,8 @@ The GPMI nand controller provides an interface to control the
 NAND flash chips.
 
 Required properties:
-  - compatible : should be "fsl,<chip>-gpmi-nand"
+  - compatible : should be "fsl,<chip>-gpmi-nand", the chip should be imx23,
+    imx28, imx6q, imx6qp, imx6sx, imx6ul or imx7d.
   - reg : should contain registers location and length for gpmi and bch.
   - reg-names: Should contain the reg names "gpmi-nand" and "bch"
   - interrupts : BCH interrupt number.
-- 
1.9.1

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

* Re: [PATCH v2 2/6] mtd: nand: gpmi: document the clocks and clock-names in DT property
  2016-06-10 17:01 ` [PATCH v2 2/6] mtd: nand: gpmi: document the clocks and clock-names in DT property Han Xu
@ 2016-06-14 12:46   ` Rob Herring
  0 siblings, 0 replies; 13+ messages in thread
From: Rob Herring @ 2016-06-14 12:46 UTC (permalink / raw)
  To: Han Xu
  Cc: boris.brezillon, richard, dwmw2, computersforpeace, pawel.moll,
	mark.rutland, ijc+devicetree, galak, devicetree, linux-mtd,
	linux-kernel

On Fri, Jun 10, 2016 at 12:01:30PM -0500, Han Xu wrote:
> add the clocks and clock-names in DT property, gpmi-io clock is
> mandatory for all platforms, but some platforms, such as i.MX6Q may
> need more extra clocks for submodules. More details please refer to the
> SoC reference manual.
> 
> Signed-off-by: Han Xu <han.xu@nxp.com>
> ---
>  Documentation/devicetree/bindings/mtd/gpmi-nand.txt | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mtd/gpmi-nand.txt b/Documentation/devicetree/bindings/mtd/gpmi-nand.txt
> index d02acaf..c8d0e2f 100644
> --- a/Documentation/devicetree/bindings/mtd/gpmi-nand.txt
> +++ b/Documentation/devicetree/bindings/mtd/gpmi-nand.txt
> @@ -13,6 +13,13 @@ Required properties:
>      and GPMI DMA channel ID.
>      Refer to dma.txt and fsl-mxs-dma.txt for details.
>    - dma-names: Must be "rx-tx".
> +  - clocks : The clocks needed by the gpmi controller. This field varies
> +    depends on the SoC design, "gpmi-io" is mandatory but some platforms may
> +    need several extra clocks, such as i.MX6Q, it requires "gpmi_apb,
> +    gpmi_bch, gpmi_bch_apb and per1_bch" for all submodules. Please refer to
> +    the HW design manual.

You need to spell out exactly which clocks each chip/compatible string 
has.

I'm generally suspicious of varying number of clocks. My guess is all 
the versions have the additional clocks, but they are all just tied to 
the same parent. 

Rob

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

* Re: [PATCH v2 6/6] mtd: nand: gpmi: document the new supported chip in DT
  2016-06-10 17:01 ` [PATCH v2 6/6] mtd: nand: gpmi: document the new supported chip in DT Han Xu
@ 2016-06-14 12:48   ` Rob Herring
  0 siblings, 0 replies; 13+ messages in thread
From: Rob Herring @ 2016-06-14 12:48 UTC (permalink / raw)
  To: Han Xu
  Cc: boris.brezillon, richard, dwmw2, computersforpeace, pawel.moll,
	mark.rutland, ijc+devicetree, galak, devicetree, linux-mtd,
	linux-kernel

On Fri, Jun 10, 2016 at 12:01:34PM -0500, Han Xu wrote:
> listed all supported chips in DT.
> 
> Signed-off-by: Han Xu <han.xu@nxp.com>
> ---
>  Documentation/devicetree/bindings/mtd/gpmi-nand.txt | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

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

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

* Re: [PATCH v2 4/6] mtd: nand: gpmi: correct bitflip for erased NAND page
  2016-06-10 17:01 ` [PATCH v2 4/6] mtd: nand: gpmi: correct bitflip for erased NAND page Han Xu
@ 2016-06-14 19:14   ` Boris Brezillon
  2016-06-21 15:53     ` Han Xu
  0 siblings, 1 reply; 13+ messages in thread
From: Boris Brezillon @ 2016-06-14 19:14 UTC (permalink / raw)
  To: Han Xu
  Cc: richard, dwmw2, computersforpeace, robh+dt, pawel.moll,
	mark.rutland, ijc+devicetree, galak, devicetree, linux-mtd,
	linux-kernel

On Fri, 10 Jun 2016 12:01:32 -0500
Han Xu <han.xu@nxp.com> wrote:

> i.MX6QP and i.MX7D BCH module integrated a new feature to detect the
> bitflip number for erased NAND page. So for these two platform, set the
> erase threshold to ecc_strength and if bitflip detected, GPMI driver
> will correct the data to all 0xFF.
> 
> Signed-off-by: Han Xu <han.xu@nxp.com>
> ---
>  drivers/mtd/nand/gpmi-nand/bch-regs.h  | 10 ++++++++++
>  drivers/mtd/nand/gpmi-nand/gpmi-lib.c  |  5 +++++
>  drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 27 ++++++++++++++++++++++++++-
>  3 files changed, 41 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/nand/gpmi-nand/bch-regs.h b/drivers/mtd/nand/gpmi-nand/bch-regs.h
> index 228142c..2c44b88 100644
> --- a/drivers/mtd/nand/gpmi-nand/bch-regs.h
> +++ b/drivers/mtd/nand/gpmi-nand/bch-regs.h
> @@ -30,7 +30,13 @@
>  #define BM_BCH_CTRL_COMPLETE_IRQ		(1 << 0)
>  
>  #define HW_BCH_STATUS0				0x00000010
> +
>  #define HW_BCH_MODE				0x00000020
> +#define BP_BCH_MODE_ERASE_THRESHOLD		0
> +#define BM_BCH_MODE_ERASE_THRESHOLD	(0xff << BP_BCH_MODE_ERASE_THRESHOLD)
> +#define BF_BCH_MODE_ERASE_THRESHOLD(v)		\
> +	(((v) << BP_BCH_MODE_ERASE_THRESHOLD) & BM_BCH_MODE_ERASE_THRESHOLD)
> +
>  #define HW_BCH_ENCODEPTR			0x00000030
>  #define HW_BCH_DATAPTR				0x00000040
>  #define HW_BCH_METAPTR				0x00000050
> @@ -125,4 +131,8 @@
>  	)
>  
>  #define HW_BCH_VERSION				0x00000160
> +#define HW_BCH_DEBUG1				0x00000170
> +#define BP_BCH_DEBUG1_ERASED_ZERO_COUNT	0
> +#define BM_BCH_DEBUG1_ERASED_ZERO_COUNT		\
> +		(0x1ff << BP_BCH_DEBUG1_ERASED_ZERO_COUNT)
>  #endif
> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-lib.c b/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
> index 358ff5d..0b5666a 100644
> --- a/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
> @@ -298,6 +298,11 @@ int bch_set_geometry(struct gpmi_nand_data *this)
>  			| BF_BCH_FLASH0LAYOUT1_DATAN_SIZE(block_size, this),
>  			r->bch_regs + HW_BCH_FLASH0LAYOUT1);
>  
> +	/* Set erase threshold to ecc_strength for mx6qp and mx7 */
> +	if (GPMI_IS_MX6QP(this) || GPMI_IS_MX7(this))
> +		writel(BF_BCH_MODE_ERASE_THRESHOLD(ecc_strength),
> +			r->bch_regs + HW_BCH_MODE);
> +
>  	/* Set *all* chip selects to use layout 0. */
>  	writel(0, r->bch_regs + HW_BCH_LAYOUTSELECT);
>  
> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> index aedaff3..03bdb4d 100644
> --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> @@ -1043,6 +1043,7 @@ static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
>  {
>  	struct gpmi_nand_data *this = nand_get_controller_data(chip);
>  	struct bch_geometry *nfc_geo = &this->bch_geometry;
> +	void __iomem *bch_regs = this->resources.bch_regs;
>  	void          *payload_virt;
>  	dma_addr_t    payload_phys;
>  	void          *auxiliary_virt;
> @@ -1051,6 +1052,8 @@ static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
>  	unsigned char *status;
>  	unsigned int  max_bitflips = 0;
>  	int           ret;
> +	int bitflips = 0;
> +	int bitflip_flag = 0;

Hm, I would choose another name, but I'm not even sure this is really
needed (see below).

>  
>  	dev_dbg(this->dev, "page number is : %d\n", page);
>  	ret = read_page_prepare(this, buf, nfc_geo->payload_size,
> @@ -1088,8 +1091,20 @@ static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
>  			   payload_virt, payload_phys);
>  
>  	for (i = 0; i < nfc_geo->ecc_chunk_count; i++, status++) {
> -		if ((*status == STATUS_GOOD) || (*status == STATUS_ERASED))
> +		if (*status == STATUS_GOOD)
> +			continue;
> +		if (*status == STATUS_ERASED) {
> +			if (GPMI_IS_MX6QP(this) || GPMI_IS_MX7(this)) {
> +				bitflips = readl(bch_regs + HW_BCH_DEBUG1);

You could use ret here or define a flips variable locally to this code
(see what's done for the software correction).

BTW, why not memset-ing the data chunk here (as done for the SW
correction part). This way you wouldn't need the extra bitflips_flag
variable.

> +				if (bitflips) {
> +					bitflip_flag = 1;
> +					max_bitflips = max_t(unsigned int,
> +							     max_bitflips,
> +							     bitflips);
> +				}
> +			}
>  			continue;
> +		}
>  
>  		if (*status == STATUS_UNCORRECTABLE) {
>  			int eccbits = nfc_geo->ecc_strength * nfc_geo->gf_len;
> @@ -1098,6 +1113,10 @@ static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
>  			int eccbytes;
>  			int flips;
>  
> +			/* shortcut for i.MX7 and i.MX6QP */
> +			if (GPMI_IS_MX6QP(this) || GPMI_IS_MX7(this))

You should increment mtd->ecc_stats.failed in this case.

> +				continue;
> +
>  			/* Read ECC bytes into our internal raw_buffer */
>  			offset = nfc_geo->metadata_size * 8;
>  			offset += ((8 * nfc_geo->ecc_chunk_size) + eccbits) * (i + 1);
> @@ -1182,6 +1201,12 @@ static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
>  		chip->oob_poi[0] = ((uint8_t *) auxiliary_virt)[0];
>  	}
>  
> +	/* if bitflip occurred in erased page, change data to all 0xff */
> +	if (bitflip_flag) {
> +		memset(buf, ~0, nfc_geo->payload_size);
> +		memset(chip->oob_poi, ~0, mtd->oobsize);
> +	}
> +
>  	return max_bitflips;
>  }
>  



-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

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

* Re: [PATCH v2 1/6] mtd: nand: gpmi: add GPMI NAND support for i.MX7D
  2016-06-10 17:01 ` [PATCH v2 1/6] mtd: nand: gpmi: add GPMI NAND support for i.MX7D Han Xu
@ 2016-06-14 20:05   ` Boris Brezillon
  0 siblings, 0 replies; 13+ messages in thread
From: Boris Brezillon @ 2016-06-14 20:05 UTC (permalink / raw)
  To: Han Xu
  Cc: richard, dwmw2, computersforpeace, robh+dt, pawel.moll,
	mark.rutland, ijc+devicetree, galak, devicetree, linux-mtd,
	linux-kernel

On Fri, 10 Jun 2016 12:01:29 -0500
Han Xu <han.xu@nxp.com> wrote:

> support GPMI NAND on i.MX7D

I see that a lot of conditional path are testing NAND_IS_MXn(X).
That would probably a good thing to move all these different code
blocks into their own functions, and declare function pointers in
struct gpmi_devdata to avoid doing these tests.

The same goes for all the macros that are testing the controller
revision.

That's not something I'm asking you to modify in this series, but a
future possible improvement.

> 
> Signed-off-by: Han Xu <han.xu@nxp.com>
> ---
>  drivers/mtd/nand/gpmi-nand/bch-regs.h  | 14 +++++++-------
>  drivers/mtd/nand/gpmi-nand/gpmi-lib.c  | 10 ++++++----
>  drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 30 ++++++++++++++++++++++++------
>  drivers/mtd/nand/gpmi-nand/gpmi-nand.h |  7 +++++--
>  4 files changed, 42 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/mtd/nand/gpmi-nand/bch-regs.h b/drivers/mtd/nand/gpmi-nand/bch-regs.h
> index 05bb91f..228142c 100644
> --- a/drivers/mtd/nand/gpmi-nand/bch-regs.h
> +++ b/drivers/mtd/nand/gpmi-nand/bch-regs.h
> @@ -1,7 +1,7 @@
>  /*
>   * Freescale GPMI NAND Flash Driver
>   *
> - * Copyright 2008-2011 Freescale Semiconductor, Inc.
> + * Copyright 2008-2016 Freescale Semiconductor, Inc.
>   * Copyright 2008 Embedded Alley Solutions, Inc.
>   *
>   * This program is free software; you can redistribute it and/or modify
> @@ -54,7 +54,7 @@
>  #define MX6Q_BP_BCH_FLASH0LAYOUT0_ECC0		11
>  #define MX6Q_BM_BCH_FLASH0LAYOUT0_ECC0	(0x1f << MX6Q_BP_BCH_FLASH0LAYOUT0_ECC0)
>  #define BF_BCH_FLASH0LAYOUT0_ECC0(v, x)				\
> -	(GPMI_IS_MX6(x)					\
> +	((GPMI_IS_MX6(x) || GPMI_IS_MX7(x))				\
>  		? (((v) << MX6Q_BP_BCH_FLASH0LAYOUT0_ECC0)	\
>  			& MX6Q_BM_BCH_FLASH0LAYOUT0_ECC0)	\
>  		: (((v) << BP_BCH_FLASH0LAYOUT0_ECC0)		\
> @@ -65,7 +65,7 @@
>  #define MX6Q_BM_BCH_FLASH0LAYOUT0_GF_13_14			\
>  				(0x1 << MX6Q_BP_BCH_FLASH0LAYOUT0_GF_13_14)
>  #define BF_BCH_FLASH0LAYOUT0_GF(v, x)				\
> -	((GPMI_IS_MX6(x) && ((v) == 14))			\
> +	(((GPMI_IS_MX6(x) || GPMI_IS_MX7(x)) && ((v) == 14))	\
>  		? (((1) << MX6Q_BP_BCH_FLASH0LAYOUT0_GF_13_14)	\
>  			& MX6Q_BM_BCH_FLASH0LAYOUT0_GF_13_14)	\
>  		: 0						\
> @@ -77,7 +77,7 @@
>  #define MX6Q_BM_BCH_FLASH0LAYOUT0_DATA0_SIZE	\
>  			(0x3ff << BP_BCH_FLASH0LAYOUT0_DATA0_SIZE)
>  #define BF_BCH_FLASH0LAYOUT0_DATA0_SIZE(v, x)				\
> -	(GPMI_IS_MX6(x)						\
> +	((GPMI_IS_MX6(x) || GPMI_IS_MX7(x))	\
>  		? (((v) >> 2) & MX6Q_BM_BCH_FLASH0LAYOUT0_DATA0_SIZE)	\
>  		: ((v) & BM_BCH_FLASH0LAYOUT0_DATA0_SIZE)		\
>  	)
> @@ -96,7 +96,7 @@
>  #define MX6Q_BP_BCH_FLASH0LAYOUT1_ECCN		11
>  #define MX6Q_BM_BCH_FLASH0LAYOUT1_ECCN	(0x1f << MX6Q_BP_BCH_FLASH0LAYOUT1_ECCN)
>  #define BF_BCH_FLASH0LAYOUT1_ECCN(v, x)				\
> -	(GPMI_IS_MX6(x)					\
> +	((GPMI_IS_MX6(x) || GPMI_IS_MX7(x))				\
>  		? (((v) << MX6Q_BP_BCH_FLASH0LAYOUT1_ECCN)	\
>  			& MX6Q_BM_BCH_FLASH0LAYOUT1_ECCN)	\
>  		: (((v) << BP_BCH_FLASH0LAYOUT1_ECCN)		\
> @@ -107,7 +107,7 @@
>  #define MX6Q_BM_BCH_FLASH0LAYOUT1_GF_13_14			\
>  				(0x1 << MX6Q_BP_BCH_FLASH0LAYOUT1_GF_13_14)
>  #define BF_BCH_FLASH0LAYOUT1_GF(v, x)				\
> -	((GPMI_IS_MX6(x) && ((v) == 14))			\
> +	(((GPMI_IS_MX6(x) || GPMI_IS_MX7(x)) && ((v) == 14))	\
>  		? (((1) << MX6Q_BP_BCH_FLASH0LAYOUT1_GF_13_14)	\
>  			& MX6Q_BM_BCH_FLASH0LAYOUT1_GF_13_14)	\
>  		: 0						\
> @@ -119,7 +119,7 @@
>  #define MX6Q_BM_BCH_FLASH0LAYOUT1_DATAN_SIZE	\
>  			(0x3ff << BP_BCH_FLASH0LAYOUT1_DATAN_SIZE)
>  #define BF_BCH_FLASH0LAYOUT1_DATAN_SIZE(v, x)				\
> -	(GPMI_IS_MX6(x)						\
> +	((GPMI_IS_MX6(x) || GPMI_IS_MX7(x))	\
>  		? (((v) >> 2) & MX6Q_BM_BCH_FLASH0LAYOUT1_DATAN_SIZE)	\
>  		: ((v) & BM_BCH_FLASH0LAYOUT1_DATAN_SIZE)		\
>  	)
> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-lib.c b/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
> index 0f68a99..358ff5d 100644
> --- a/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
> @@ -1,7 +1,7 @@
>  /*
>   * Freescale GPMI NAND Flash Driver
>   *
> - * Copyright (C) 2008-2011 Freescale Semiconductor, Inc.
> + * Copyright (C) 2008-2016 Freescale Semiconductor, Inc.
>   * Copyright (C) 2008 Embedded Alley Solutions, Inc.
>   *
>   * This program is free software; you can redistribute it and/or modify
> @@ -971,7 +971,8 @@ int gpmi_extra_init(struct gpmi_nand_data *this)
>  	struct nand_chip *chip = &this->nand;
>  
>  	/* Enable the asynchronous EDO feature. */
> -	if (GPMI_IS_MX6(this) && chip->onfi_version) {
> +	if ((GPMI_IS_MX6(this) || GPMI_IS_MX7(this)) &&
> +	    chip->onfi_version) {
>  		int mode = onfi_get_async_timing_mode(chip);
>  
>  		/* We only support the timing mode 4 and mode 5. */
> @@ -1093,12 +1094,13 @@ int gpmi_is_ready(struct gpmi_nand_data *this, unsigned chip)
>  	if (GPMI_IS_MX23(this)) {
>  		mask = MX23_BM_GPMI_DEBUG_READY0 << chip;
>  		reg = readl(r->gpmi_regs + HW_GPMI_DEBUG);
> -	} else if (GPMI_IS_MX28(this) || GPMI_IS_MX6(this)) {
> +	} else if (GPMI_IS_MX28(this) || GPMI_IS_MX6(this) ||
> +	           GPMI_IS_MX7(this)) {
>  		/*
>  		 * In the imx6, all the ready/busy pins are bound
>  		 * together. So we only need to check chip 0.
>  		 */
> -		if (GPMI_IS_MX6(this))
> +		if (GPMI_IS_MX6(this) || GPMI_IS_MX7(this))
>  			chip = 0;
>  
>  		/* MX28 shares the same R/B register as MX6Q. */
> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> index 6e46156..a8936fd 100644
> --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> @@ -1,7 +1,7 @@
>  /*
>   * Freescale GPMI NAND Flash Driver
>   *
> - * Copyright (C) 2010-2015 Freescale Semiconductor, Inc.
> + * Copyright (C) 2010-2016 Freescale Semiconductor, Inc.
>   * Copyright (C) 2008 Embedded Alley Solutions, Inc.
>   *
>   * This program is free software; you can redistribute it and/or modify
> @@ -110,6 +110,12 @@ static const struct gpmi_devdata gpmi_devdata_imx6sx = {
>  	.max_chain_delay = 12,
>  };
>  
> +static const struct gpmi_devdata gpmi_devdata_imx7d = {
> +	.type = IS_MX7D,
> +	.bch_max_ecc_strength = 62,
> +	.max_chain_delay = 12,
> +};
> +
>  static irqreturn_t bch_irq(int irq, void *cookie)
>  {
>  	struct gpmi_nand_data *this = cookie;
> @@ -601,6 +607,10 @@ static char *extra_clks_for_mx6q[GPMI_CLK_MAX] = {
>  	"gpmi_apb", "gpmi_bch", "gpmi_bch_apb", "per1_bch",
>  };
>  
> +static char *extra_clks_for_mx7d[GPMI_CLK_MAX] = {
> +	"gpmi_bch_apb",
> +};
> +
>  static int gpmi_get_clks(struct gpmi_nand_data *this)
>  {
>  	struct resources *r = &this->resources;
> @@ -618,6 +628,8 @@ static int gpmi_get_clks(struct gpmi_nand_data *this)
>  	/* Get extra clocks */
>  	if (GPMI_IS_MX6(this))
>  		extra_clks = extra_clks_for_mx6q;
> +	if (GPMI_IS_MX7(this))
> +		extra_clks = extra_clks_for_mx7d;
>  	if (!extra_clks)
>  		return 0;
>  
> @@ -634,7 +646,7 @@ static int gpmi_get_clks(struct gpmi_nand_data *this)
>  		r->clock[i] = clk;
>  	}
>  
> -	if (GPMI_IS_MX6(this))
> +	if (GPMI_IS_MX6(this) || GPMI_IS_MX7(this))
>  		/*
>  		 * Set the default value for the gpmi clock.
>  		 *
> @@ -1965,8 +1977,8 @@ static int gpmi_init_last(struct gpmi_nand_data *this)
>  	 *  (1) the chip is imx6, and
>  	 *  (2) the size of the ECC parity is byte aligned.
>  	 */
> -	if (GPMI_IS_MX6(this) &&
> -		((bch_geo->gf_len * bch_geo->ecc_strength) % 8) == 0) {
> +	if ((GPMI_IS_MX6(this) || GPMI_IS_MX7(this)) &&
> +	    ((bch_geo->gf_len * bch_geo->ecc_strength) % 8) == 0) {
>  		ecc->read_subpage = gpmi_ecc_read_subpage;
>  		chip->options |= NAND_SUBPAGE_READ;
>  	}
> @@ -1987,6 +1999,7 @@ static int gpmi_nand_init(struct gpmi_nand_data *this)
>  	struct nand_chip *chip = &this->nand;
>  	struct mtd_info  *mtd = nand_to_mtd(chip);
>  	int ret;
> +	int max_chips;
>  
>  	/* init current chip */
>  	this->current_chip	= -1;
> @@ -2021,7 +2034,8 @@ static int gpmi_nand_init(struct gpmi_nand_data *this)
>  	if (ret)
>  		goto err_out;
>  
> -	ret = nand_scan_ident(mtd, GPMI_IS_MX6(this) ? 2 : 1, NULL);
> +	max_chips = (GPMI_IS_MX6(this) || GPMI_IS_MX7(this)) ? 2 : 1;
> +	ret = nand_scan_ident(mtd, max_chips, NULL);
>  	if (ret)
>  		goto err_out;
>  
> @@ -2074,7 +2088,11 @@ static const struct of_device_id gpmi_nand_id_table[] = {
>  	}, {
>  		.compatible = "fsl,imx6sx-gpmi-nand",
>  		.data = &gpmi_devdata_imx6sx,
> -	}, {}
> +	}, {
> +		.compatible = "fsl,imx7d-gpmi-nand",
> +		.data = &gpmi_devdata_imx7d,
> +	}, { /* sentinel */ }
> +
>  };
>  MODULE_DEVICE_TABLE(of, gpmi_nand_id_table);
>  
> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.h b/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
> index 4e49a1f..04a3584 100644
> --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
> @@ -1,7 +1,7 @@
>  /*
>   * Freescale GPMI NAND Flash Driver
>   *
> - * Copyright (C) 2010-2011 Freescale Semiconductor, Inc.
> + * Copyright (C) 2010-2016 Freescale Semiconductor, Inc.
>   * Copyright (C) 2008 Embedded Alley Solutions, Inc.
>   *
>   * This program is free software; you can redistribute it and/or modify
> @@ -123,7 +123,8 @@ enum gpmi_type {
>  	IS_MX23,
>  	IS_MX28,
>  	IS_MX6Q,
> -	IS_MX6SX
> +	IS_MX6SX,
> +	IS_MX7D,
>  };
>  
>  struct gpmi_devdata {
> @@ -305,6 +306,8 @@ void gpmi_copy_bits(u8 *dst, size_t dst_bit_off,
>  #define GPMI_IS_MX28(x)		((x)->devdata->type == IS_MX28)
>  #define GPMI_IS_MX6Q(x)		((x)->devdata->type == IS_MX6Q)
>  #define GPMI_IS_MX6SX(x)	((x)->devdata->type == IS_MX6SX)
> +#define GPMI_IS_MX7D(x)		((x)->devdata->type == IS_MX7D)
>  
>  #define GPMI_IS_MX6(x)		(GPMI_IS_MX6Q(x) || GPMI_IS_MX6SX(x))
> +#define GPMI_IS_MX7(x)		(GPMI_IS_MX7D(x))
>  #endif



-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

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

* Re: [PATCH v2 4/6] mtd: nand: gpmi: correct bitflip for erased NAND page
  2016-06-14 19:14   ` Boris Brezillon
@ 2016-06-21 15:53     ` Han Xu
  2016-06-21 16:18       ` Boris Brezillon
  0 siblings, 1 reply; 13+ messages in thread
From: Han Xu @ 2016-06-21 15:53 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: Han Xu, mark.rutland, devicetree, pawel.moll, ijc+devicetree,
	Richard Weinberger, linux-kernel, robh+dt, linux-mtd, galak,
	Brian Norris, David Woodhouse

On Tue, Jun 14, 2016 at 2:14 PM, Boris Brezillon
<boris.brezillon@free-electrons.com> wrote:
> On Fri, 10 Jun 2016 12:01:32 -0500
> Han Xu <han.xu@nxp.com> wrote:
>
>> i.MX6QP and i.MX7D BCH module integrated a new feature to detect the
>> bitflip number for erased NAND page. So for these two platform, set the
>> erase threshold to ecc_strength and if bitflip detected, GPMI driver
>> will correct the data to all 0xFF.
>>
>> Signed-off-by: Han Xu <han.xu@nxp.com>
>> ---
>>  drivers/mtd/nand/gpmi-nand/bch-regs.h  | 10 ++++++++++
>>  drivers/mtd/nand/gpmi-nand/gpmi-lib.c  |  5 +++++
>>  drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 27 ++++++++++++++++++++++++++-
>>  3 files changed, 41 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mtd/nand/gpmi-nand/bch-regs.h b/drivers/mtd/nand/gpmi-nand/bch-regs.h
>> index 228142c..2c44b88 100644
>> --- a/drivers/mtd/nand/gpmi-nand/bch-regs.h
>> +++ b/drivers/mtd/nand/gpmi-nand/bch-regs.h
>> @@ -30,7 +30,13 @@
>>  #define BM_BCH_CTRL_COMPLETE_IRQ             (1 << 0)
>>
>>  #define HW_BCH_STATUS0                               0x00000010
>> +
>>  #define HW_BCH_MODE                          0x00000020
>> +#define BP_BCH_MODE_ERASE_THRESHOLD          0
>> +#define BM_BCH_MODE_ERASE_THRESHOLD  (0xff << BP_BCH_MODE_ERASE_THRESHOLD)
>> +#define BF_BCH_MODE_ERASE_THRESHOLD(v)               \
>> +     (((v) << BP_BCH_MODE_ERASE_THRESHOLD) & BM_BCH_MODE_ERASE_THRESHOLD)
>> +
>>  #define HW_BCH_ENCODEPTR                     0x00000030
>>  #define HW_BCH_DATAPTR                               0x00000040
>>  #define HW_BCH_METAPTR                               0x00000050
>> @@ -125,4 +131,8 @@
>>       )
>>
>>  #define HW_BCH_VERSION                               0x00000160
>> +#define HW_BCH_DEBUG1                                0x00000170
>> +#define BP_BCH_DEBUG1_ERASED_ZERO_COUNT      0
>> +#define BM_BCH_DEBUG1_ERASED_ZERO_COUNT              \
>> +             (0x1ff << BP_BCH_DEBUG1_ERASED_ZERO_COUNT)
>>  #endif
>> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-lib.c b/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
>> index 358ff5d..0b5666a 100644
>> --- a/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
>> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
>> @@ -298,6 +298,11 @@ int bch_set_geometry(struct gpmi_nand_data *this)
>>                       | BF_BCH_FLASH0LAYOUT1_DATAN_SIZE(block_size, this),
>>                       r->bch_regs + HW_BCH_FLASH0LAYOUT1);
>>
>> +     /* Set erase threshold to ecc_strength for mx6qp and mx7 */
>> +     if (GPMI_IS_MX6QP(this) || GPMI_IS_MX7(this))
>> +             writel(BF_BCH_MODE_ERASE_THRESHOLD(ecc_strength),
>> +                     r->bch_regs + HW_BCH_MODE);
>> +
>>       /* Set *all* chip selects to use layout 0. */
>>       writel(0, r->bch_regs + HW_BCH_LAYOUTSELECT);
>>
>> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
>> index aedaff3..03bdb4d 100644
>> --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
>> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
>> @@ -1043,6 +1043,7 @@ static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
>>  {
>>       struct gpmi_nand_data *this = nand_get_controller_data(chip);
>>       struct bch_geometry *nfc_geo = &this->bch_geometry;
>> +     void __iomem *bch_regs = this->resources.bch_regs;
>>       void          *payload_virt;
>>       dma_addr_t    payload_phys;
>>       void          *auxiliary_virt;
>> @@ -1051,6 +1052,8 @@ static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
>>       unsigned char *status;
>>       unsigned int  max_bitflips = 0;
>>       int           ret;
>> +     int bitflips = 0;
>> +     int bitflip_flag = 0;
>
> Hm, I would choose another name, but I'm not even sure this is really
> needed (see below).
>
>>
>>       dev_dbg(this->dev, "page number is : %d\n", page);
>>       ret = read_page_prepare(this, buf, nfc_geo->payload_size,
>> @@ -1088,8 +1091,20 @@ static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
>>                          payload_virt, payload_phys);
>>
>>       for (i = 0; i < nfc_geo->ecc_chunk_count; i++, status++) {
>> -             if ((*status == STATUS_GOOD) || (*status == STATUS_ERASED))
>> +             if (*status == STATUS_GOOD)
>> +                     continue;
>> +             if (*status == STATUS_ERASED) {
>> +                     if (GPMI_IS_MX6QP(this) || GPMI_IS_MX7(this)) {
>> +                             bitflips = readl(bch_regs + HW_BCH_DEBUG1);
>
> You could use ret here or define a flips variable locally to this code
> (see what's done for the software correction).
>
> BTW, why not memset-ing the data chunk here (as done for the SW
> correction part). This way you wouldn't need the extra bitflips_flag
> variable.
>
>> +                             if (bitflips) {
>> +                                     bitflip_flag = 1;
>> +                                     max_bitflips = max_t(unsigned int,
>> +                                                          max_bitflips,
>> +                                                          bitflips);
>> +                             }
>> +                     }
>>                       continue;
>> +             }
>>
>>               if (*status == STATUS_UNCORRECTABLE) {
>>                       int eccbits = nfc_geo->ecc_strength * nfc_geo->gf_len;
>> @@ -1098,6 +1113,10 @@ static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
>>                       int eccbytes;
>>                       int flips;
>>
>> +                     /* shortcut for i.MX7 and i.MX6QP */
>> +                     if (GPMI_IS_MX6QP(this) || GPMI_IS_MX7(this))
>
> You should increment mtd->ecc_stats.failed in this case.

Do you mean update the mtd->ecc_status.corrected when flips happened?
Nothing failed in this case.

>
>> +                             continue;
>> +
>>                       /* Read ECC bytes into our internal raw_buffer */
>>                       offset = nfc_geo->metadata_size * 8;
>>                       offset += ((8 * nfc_geo->ecc_chunk_size) + eccbits) * (i + 1);
>> @@ -1182,6 +1201,12 @@ static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
>>               chip->oob_poi[0] = ((uint8_t *) auxiliary_virt)[0];
>>       }
>>
>> +     /* if bitflip occurred in erased page, change data to all 0xff */
>> +     if (bitflip_flag) {
>> +             memset(buf, ~0, nfc_geo->payload_size);
>> +             memset(chip->oob_poi, ~0, mtd->oobsize);
>> +     }
>> +
>>       return max_bitflips;
>>  }
>>
>
>
>
> --
> Boris Brezillon, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/



-- 
Sincerely,

Han XU

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

* Re: [PATCH v2 4/6] mtd: nand: gpmi: correct bitflip for erased NAND page
  2016-06-21 15:53     ` Han Xu
@ 2016-06-21 16:18       ` Boris Brezillon
  0 siblings, 0 replies; 13+ messages in thread
From: Boris Brezillon @ 2016-06-21 16:18 UTC (permalink / raw)
  To: Han Xu
  Cc: Han Xu, mark.rutland, devicetree, pawel.moll, ijc+devicetree,
	Richard Weinberger, linux-kernel, robh+dt, linux-mtd, galak,
	Brian Norris, David Woodhouse

On Tue, 21 Jun 2016 10:53:48 -0500
Han Xu <xhnjupt@gmail.com> wrote:

> On Tue, Jun 14, 2016 at 2:14 PM, Boris Brezillon
> <boris.brezillon@free-electrons.com> wrote:
> > On Fri, 10 Jun 2016 12:01:32 -0500
> > Han Xu <han.xu@nxp.com> wrote:
> >  
> >> i.MX6QP and i.MX7D BCH module integrated a new feature to detect the
> >> bitflip number for erased NAND page. So for these two platform, set the
> >> erase threshold to ecc_strength and if bitflip detected, GPMI driver
> >> will correct the data to all 0xFF.
> >>
> >> Signed-off-by: Han Xu <han.xu@nxp.com>
> >> ---
> >>  drivers/mtd/nand/gpmi-nand/bch-regs.h  | 10 ++++++++++
> >>  drivers/mtd/nand/gpmi-nand/gpmi-lib.c  |  5 +++++
> >>  drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 27 ++++++++++++++++++++++++++-
> >>  3 files changed, 41 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/mtd/nand/gpmi-nand/bch-regs.h b/drivers/mtd/nand/gpmi-nand/bch-regs.h
> >> index 228142c..2c44b88 100644
> >> --- a/drivers/mtd/nand/gpmi-nand/bch-regs.h
> >> +++ b/drivers/mtd/nand/gpmi-nand/bch-regs.h
> >> @@ -30,7 +30,13 @@
> >>  #define BM_BCH_CTRL_COMPLETE_IRQ             (1 << 0)
> >>
> >>  #define HW_BCH_STATUS0                               0x00000010
> >> +
> >>  #define HW_BCH_MODE                          0x00000020
> >> +#define BP_BCH_MODE_ERASE_THRESHOLD          0
> >> +#define BM_BCH_MODE_ERASE_THRESHOLD  (0xff << BP_BCH_MODE_ERASE_THRESHOLD)
> >> +#define BF_BCH_MODE_ERASE_THRESHOLD(v)               \
> >> +     (((v) << BP_BCH_MODE_ERASE_THRESHOLD) & BM_BCH_MODE_ERASE_THRESHOLD)
> >> +
> >>  #define HW_BCH_ENCODEPTR                     0x00000030
> >>  #define HW_BCH_DATAPTR                               0x00000040
> >>  #define HW_BCH_METAPTR                               0x00000050
> >> @@ -125,4 +131,8 @@
> >>       )
> >>
> >>  #define HW_BCH_VERSION                               0x00000160
> >> +#define HW_BCH_DEBUG1                                0x00000170
> >> +#define BP_BCH_DEBUG1_ERASED_ZERO_COUNT      0
> >> +#define BM_BCH_DEBUG1_ERASED_ZERO_COUNT              \
> >> +             (0x1ff << BP_BCH_DEBUG1_ERASED_ZERO_COUNT)
> >>  #endif
> >> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-lib.c b/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
> >> index 358ff5d..0b5666a 100644
> >> --- a/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
> >> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
> >> @@ -298,6 +298,11 @@ int bch_set_geometry(struct gpmi_nand_data *this)
> >>                       | BF_BCH_FLASH0LAYOUT1_DATAN_SIZE(block_size, this),
> >>                       r->bch_regs + HW_BCH_FLASH0LAYOUT1);
> >>
> >> +     /* Set erase threshold to ecc_strength for mx6qp and mx7 */
> >> +     if (GPMI_IS_MX6QP(this) || GPMI_IS_MX7(this))
> >> +             writel(BF_BCH_MODE_ERASE_THRESHOLD(ecc_strength),
> >> +                     r->bch_regs + HW_BCH_MODE);
> >> +
> >>       /* Set *all* chip selects to use layout 0. */
> >>       writel(0, r->bch_regs + HW_BCH_LAYOUTSELECT);
> >>
> >> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> >> index aedaff3..03bdb4d 100644
> >> --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> >> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> >> @@ -1043,6 +1043,7 @@ static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
> >>  {
> >>       struct gpmi_nand_data *this = nand_get_controller_data(chip);
> >>       struct bch_geometry *nfc_geo = &this->bch_geometry;
> >> +     void __iomem *bch_regs = this->resources.bch_regs;
> >>       void          *payload_virt;
> >>       dma_addr_t    payload_phys;
> >>       void          *auxiliary_virt;
> >> @@ -1051,6 +1052,8 @@ static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
> >>       unsigned char *status;
> >>       unsigned int  max_bitflips = 0;
> >>       int           ret;
> >> +     int bitflips = 0;
> >> +     int bitflip_flag = 0;  
> >
> > Hm, I would choose another name, but I'm not even sure this is really
> > needed (see below).
> >  
> >>
> >>       dev_dbg(this->dev, "page number is : %d\n", page);
> >>       ret = read_page_prepare(this, buf, nfc_geo->payload_size,
> >> @@ -1088,8 +1091,20 @@ static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
> >>                          payload_virt, payload_phys);
> >>
> >>       for (i = 0; i < nfc_geo->ecc_chunk_count; i++, status++) {
> >> -             if ((*status == STATUS_GOOD) || (*status == STATUS_ERASED))
> >> +             if (*status == STATUS_GOOD)
> >> +                     continue;
> >> +             if (*status == STATUS_ERASED) {
> >> +                     if (GPMI_IS_MX6QP(this) || GPMI_IS_MX7(this)) {
> >> +                             bitflips = readl(bch_regs + HW_BCH_DEBUG1);  
> >
> > You could use ret here or define a flips variable locally to this code
> > (see what's done for the software correction).
> >
> > BTW, why not memset-ing the data chunk here (as done for the SW
> > correction part). This way you wouldn't need the extra bitflips_flag
> > variable.
> >  
> >> +                             if (bitflips) {
> >> +                                     bitflip_flag = 1;
> >> +                                     max_bitflips = max_t(unsigned int,
> >> +                                                          max_bitflips,
> >> +                                                          bitflips);
> >> +                             }
> >> +                     }
> >>                       continue;
> >> +             }
> >>
> >>               if (*status == STATUS_UNCORRECTABLE) {
> >>                       int eccbits = nfc_geo->ecc_strength * nfc_geo->gf_len;
> >> @@ -1098,6 +1113,10 @@ static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
> >>                       int eccbytes;
> >>                       int flips;
> >>
> >> +                     /* shortcut for i.MX7 and i.MX6QP */
> >> +                     if (GPMI_IS_MX6QP(this) || GPMI_IS_MX7(this))  
> >
> > You should increment mtd->ecc_stats.failed in this case.  
> 
> Do you mean update the mtd->ecc_status.corrected when flips happened?
> Nothing failed in this case.

No, I really meant mtd->ecc_stats.failed.
*status == STATUS_UNCORRECTABLE on MX7 and MX6QP really means we have
an uncorrectable, but you're no longer incrementing ecc_stats->failed
because of this 'continue' statement.

So it should be something like:

			if (GPMI_IS_MX6QP(this) || GPMI_IS_MX7(this)) {
				mtd->ecc_status.failed++;
				continue;
			}

> 
> >  
> >> +                             continue;
> >> +
> >>                       /* Read ECC bytes into our internal raw_buffer */
> >>                       offset = nfc_geo->metadata_size * 8;
> >>                       offset += ((8 * nfc_geo->ecc_chunk_size) + eccbits) * (i + 1);
> >> @@ -1182,6 +1201,12 @@ static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
> >>               chip->oob_poi[0] = ((uint8_t *) auxiliary_virt)[0];
> >>       }
> >>
> >> +     /* if bitflip occurred in erased page, change data to all 0xff */
> >> +     if (bitflip_flag) {
> >> +             memset(buf, ~0, nfc_geo->payload_size);
> >> +             memset(chip->oob_poi, ~0, mtd->oobsize);
> >> +     }
> >> +
> >>       return max_bitflips;
> >>  }
> >>  
> >
> >
> >
> > --
> > Boris Brezillon, Free Electrons
> > Embedded Linux and Kernel engineering
> > http://free-electrons.com
> >
> > ______________________________________________________
> > Linux MTD discussion mailing list
> > http://lists.infradead.org/mailman/listinfo/linux-mtd/  
> 
> 
> 

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

end of thread, other threads:[~2016-06-21 16:24 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-10 17:01 [PATCH v2 0/6] support gpmi on i.MX6UL/7D and HW bitflip on i.MX6QP/7D Han Xu
2016-06-10 17:01 ` [PATCH v2 1/6] mtd: nand: gpmi: add GPMI NAND support for i.MX7D Han Xu
2016-06-14 20:05   ` Boris Brezillon
2016-06-10 17:01 ` [PATCH v2 2/6] mtd: nand: gpmi: document the clocks and clock-names in DT property Han Xu
2016-06-14 12:46   ` Rob Herring
2016-06-10 17:01 ` [PATCH v2 3/6] mtd: nand: gpmi: add GPMI NAND support for i.MX6QP Han Xu
2016-06-10 17:01 ` [PATCH v2 4/6] mtd: nand: gpmi: correct bitflip for erased NAND page Han Xu
2016-06-14 19:14   ` Boris Brezillon
2016-06-21 15:53     ` Han Xu
2016-06-21 16:18       ` Boris Brezillon
2016-06-10 17:01 ` [PATCH v2 5/6] mtd: nand: gpmi: support NAND on i.MX6UL Han Xu
2016-06-10 17:01 ` [PATCH v2 6/6] mtd: nand: gpmi: document the new supported chip in DT Han Xu
2016-06-14 12:48   ` Rob Herring

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