[PATCH v8 0/7] mtd: nand: gpmi: gpmi-nand DSM and bitflip support

[PATCH v8 0/7] mtd: nand: gpmi: gpmi-nand DSM and bitflip support

[Han Xu]

v1 ---> v2
change the erased page bitflip threshold to ecc strength in bitflip patch

v2 ---> v3
remove unnecessary function in mxs-dma
change the log message when legacy_set_geometry failed fix the comment message
for bitflip add comma for all field entries

v3 ---> v4
code style change for indentation

v4 ---> v5
split the DMA PM and i.MX7D support into two patches add more comments fix the
DMA clock count mismatch issue for i.MX7D

v5 ---> v6
add a sentinel entry for of_device_id to fix build error

v6 ---> v7
add CONFIG_PM_SLEEP for GPMI/DMA PM code

v7 ---> v8
remove empty line
simplify mxs_dma_init return check

Adrian Alonso (1):
  dmaengine: mxs: add i.MX7D APBH DMA support

Han Xu (4):
  mtd: nand: gpmi: may use minimum required ecc for 744 oobsize NAND
  mtd: nand: gpmi: add GPMI NAND support for i.MX7D
  mtd: nand: gpmi: correct bitflip for erased NAND page
  mtd: nand: gpmi: support NAND on i.MX6UL

Huang Shijie (2):
  mtd: nand: gpmi: add gpmi dsm supend/resume support
  dmaengine: mxs: APBH DMA supports deep sleep mode

 drivers/dma/mxs-dma.c                  |  69 +++++++++++++++--
 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 | 133 ++++++++++++++++++++++++++++-----
 drivers/mtd/nand/gpmi-nand/gpmi-nand.h |  14 +++-
 5 files changed, 214 insertions(+), 41 deletions(-)

-- 
1.9.1

[PATCH v8 1/7] mtd: nand: gpmi: add gpmi dsm supend/resume support

[Han Xu]

From: Huang Shijie <b32955@freescale.com>

i.MX6SX supports deep sleep mode(DSM) that may turn off GPMI/BCH power
during suspend, add gpmi nand suspend/resume function to release DMA
channel in suspend function and re-init GPMI/BCH controller during
resume function.

Although it is not necessary to restore GPMI/BCH registers value for
i.MX6QDL, the code doesn't distinguish different platforms to keep the
code simple.

Signed-off-by: Huang Shijie <b32955@freescale.com>
Signed-off-by: Han Xu <han.xu@freescale.com>
---
 drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 47 +++++++++++++++++++++++++++++++++-
 1 file changed, 46 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
index 5a9b696..ba5975f 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-2011 Freescale Semiconductor, Inc.
+ * Copyright (C) 2010-2015 Freescale Semiconductor, Inc.
  * Copyright (C) 2008 Embedded Alley Solutions, Inc.
  *
  * This program is free software; you can redistribute it and/or modify
@@ -2035,9 +2035,54 @@ static int gpmi_nand_remove(struct platform_device *pdev)
 	return 0;
 }
 
+#ifdef CONFIG_PM_SLEEP
+static int gpmi_pm_suspend(struct device *dev)
+{
+	struct gpmi_nand_data *this = dev_get_drvdata(dev);
+
+	release_dma_channels(this);
+	return 0;
+}
+
+static int gpmi_pm_resume(struct device *dev)
+{
+	struct gpmi_nand_data *this = dev_get_drvdata(dev);
+	int ret;
+
+	ret = acquire_dma_channels(this);
+	if (ret < 0)
+		return ret;
+
+	/* re-init the GPMI registers */
+	this->flags &= ~GPMI_TIMING_INIT_OK;
+	ret = gpmi_init(this);
+	if (ret) {
+		dev_err(this->dev, "Error setting GPMI : %d\n", ret);
+		return ret;
+	}
+
+	/* re-init the BCH registers */
+	ret = bch_set_geometry(this);
+	if (ret) {
+		dev_err(this->dev, "Error setting BCH : %d\n", ret);
+		return ret;
+	}
+
+	/* re-init others */
+	gpmi_extra_init(this);
+
+	return 0;
+}
+#endif /* CONFIG_PM_SLEEP */
+
+static const struct dev_pm_ops gpmi_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(gpmi_pm_suspend, gpmi_pm_resume)
+};
+
 static struct platform_driver gpmi_nand_driver = {
 	.driver = {
 		.name = "gpmi-nand",
+		.pm = &gpmi_pm_ops,
 		.of_match_table = gpmi_nand_id_table,
 	},
 	.probe   = gpmi_nand_probe,
-- 
1.9.1

Re: [PATCH v8 1/7] mtd: nand: gpmi: add gpmi dsm supend/resume support

[Brian Norris]

On Wed, Dec 02, 2015 at 04:47:40PM -0600, Han Xu wrote:
> From: Huang Shijie <b32955@freescale.com>
> 
> i.MX6SX supports deep sleep mode(DSM) that may turn off GPMI/BCH power
> during suspend, add gpmi nand suspend/resume function to release DMA
> channel in suspend function and re-init GPMI/BCH controller during
> resume function.
> 
> Although it is not necessary to restore GPMI/BCH registers value for
> i.MX6QDL, the code doesn't distinguish different platforms to keep the
> code simple.
> 
> Signed-off-by: Huang Shijie <b32955@freescale.com>
> Signed-off-by: Han Xu <han.xu@freescale.com>

Pushed this patch to l2-mtd.git/next

[PATCH v8 2/7] dmaengine: mxs: APBH DMA supports deep sleep mode

[Han Xu]

From: Huang Shijie <b32955@freescale.com>

Deep Sleep Mode(dsm) turns off the power for APBH DMA module, DMA
need to be re-initialized when system resumed back.

Signed-off-by: Huang Shijie <b32955@freescale.com>
Signed-off-by: Han Xu <b45815@freescale.com>
---
 drivers/dma/mxs-dma.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/mxs-dma.c b/drivers/dma/mxs-dma.c
index 60de352..eba51fc 100644
--- a/drivers/dma/mxs-dma.c
+++ b/drivers/dma/mxs-dma.c
@@ -693,7 +693,7 @@ static enum dma_status mxs_dma_tx_status(struct dma_chan *chan,
 	return mxs_chan->status;
 }
 
-static int __init mxs_dma_init(struct mxs_dma_engine *mxs_dma)
+static int mxs_dma_init(struct mxs_dma_engine *mxs_dma)
 {
 	int ret;
 
@@ -835,6 +835,7 @@ static int __init mxs_dma_probe(struct platform_device *pdev)
 
 	mxs_dma->pdev = pdev;
 	mxs_dma->dma_device.dev = &pdev->dev;
+	dev_set_drvdata(&pdev->dev, mxs_dma);
 
 	/* mxs_dma gets 65535 bytes maximum sg size */
 	mxs_dma->dma_device.dev->dma_parms = &mxs_dma->dma_parms;
@@ -872,9 +873,23 @@ static int __init mxs_dma_probe(struct platform_device *pdev)
 	return 0;
 }
 
+#ifdef CONFIG_PM_SLEEP
+static int mxs_dma_pm_resume(struct device *dev)
+{
+	struct mxs_dma_engine *mxs_dma = dev_get_drvdata(dev);
+
+	return mxs_dma_init(mxs_dma);
+}
+#endif /* CONFIG_PM_SLEEP */
+
+static const struct dev_pm_ops mxs_dma_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(NULL, mxs_dma_pm_resume)
+};
+
 static struct platform_driver mxs_dma_driver = {
 	.driver		= {
 		.name	= "mxs-dma",
+		.pm = &mxs_dma_pm_ops,
 		.of_match_table = mxs_dma_dt_ids,
 	},
 	.id_table	= mxs_dma_ids,
-- 
1.9.1

Re: [PATCH v8 2/7] dmaengine: mxs: APBH DMA supports deep sleep mode

[Vinod Koul]

On Wed, Dec 02, 2015 at 04:47:41PM -0600, Han Xu wrote:
> From: Huang Shijie <b32955@freescale.com>
> 
> Deep Sleep Mode(dsm) turns off the power for APBH DMA module, DMA
> need to be re-initialized when system resumed back.

Acked-by: Vinod Koul <vinod.koul@intel.com>

-- 
~Vinod

[PATCH v8 3/7] dmaengine: mxs: add i.MX7D APBH DMA support

[Han Xu]

From: Adrian Alonso <aalonso@freescale.com>

supports APBH DMA on i.MX7D by add extra clock clk_io

Signed-off-by: Fugang Duan <B38611@freescale.com>
Signed-off-by: Adrian Alonso <aalonso@freescale.com>
Signed-off-by: Han Xu <b45815@freescale.com>
---
 drivers/dma/mxs-dma.c | 52 ++++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 45 insertions(+), 7 deletions(-)

diff --git a/drivers/dma/mxs-dma.c b/drivers/dma/mxs-dma.c
index eba51fc..3691b4a 100644
--- a/drivers/dma/mxs-dma.c
+++ b/drivers/dma/mxs-dma.c
@@ -1,5 +1,5 @@
 /*
- * Copyright 2011 Freescale Semiconductor, Inc. All Rights Reserved.
+ * Copyright 2011-2015 Freescale Semiconductor, Inc. All Rights Reserved.
  *
  * Refer to drivers/dma/imx-sdma.c
  *
@@ -28,7 +28,6 @@
 #include <linux/of_device.h>
 #include <linux/of_dma.h>
 #include <linux/list.h>
-
 #include <asm/irq.h>
 
 #include "dmaengine.h"
@@ -135,6 +134,7 @@ enum mxs_dma_devtype {
 enum mxs_dma_id {
 	IMX23_DMA,
 	IMX28_DMA,
+	IMX7D_DMA,
 };
 
 struct mxs_dma_engine {
@@ -142,6 +142,7 @@ struct mxs_dma_engine {
 	enum mxs_dma_devtype		type;
 	void __iomem			*base;
 	struct clk			*clk;
+	struct clk			*clk_io;
 	struct dma_device		dma_device;
 	struct device_dma_parameters	dma_parms;
 	struct mxs_dma_chan		mxs_chans[MXS_DMA_CHANNELS];
@@ -167,6 +168,9 @@ static struct mxs_dma_type mxs_dma_types[] = {
 	}, {
 		.id = IMX28_DMA,
 		.type = MXS_DMA_APBX,
+	}, {
+		.id = IMX7D_DMA,
+		.type = MXS_DMA_APBH,
 	}
 };
 
@@ -184,6 +188,9 @@ static const struct platform_device_id mxs_dma_ids[] = {
 		.name = "imx28-dma-apbx",
 		.driver_data = (kernel_ulong_t) &mxs_dma_types[3],
 	}, {
+		.name = "imx7d-dma-apbh",
+		.driver_data = (kernel_ulong_t) &mxs_dma_types[4],
+	}, {
 		/* end of list */
 	}
 };
@@ -193,6 +200,7 @@ static const struct of_device_id mxs_dma_dt_ids[] = {
 	{ .compatible = "fsl,imx23-dma-apbx", .data = &mxs_dma_ids[1], },
 	{ .compatible = "fsl,imx28-dma-apbh", .data = &mxs_dma_ids[2], },
 	{ .compatible = "fsl,imx28-dma-apbx", .data = &mxs_dma_ids[3], },
+	{ .compatible = "fsl,imx7d-dma-apbh", .data = &mxs_dma_ids[4], },
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, mxs_dma_dt_ids);
@@ -440,6 +448,13 @@ static int mxs_dma_alloc_chan_resources(struct dma_chan *chan)
 	if (ret)
 		goto err_clk;
 
+	/* enable the extra clk_io clock for i.MX7D */
+	if (mxs_dma->dev_id == IMX7D_DMA) {
+		ret = clk_prepare_enable(mxs_dma->clk_io);
+		if (ret)
+			goto err_clk_unprepare;
+	}
+
 	mxs_dma_reset_chan(chan);
 
 	dma_async_tx_descriptor_init(&mxs_chan->desc, chan);
@@ -450,6 +465,8 @@ static int mxs_dma_alloc_chan_resources(struct dma_chan *chan)
 
 	return 0;
 
+err_clk_unprepare:
+	clk_disable_unprepare(mxs_dma->clk);
 err_clk:
 	free_irq(mxs_chan->chan_irq, mxs_dma);
 err_irq:
@@ -471,6 +488,9 @@ static void mxs_dma_free_chan_resources(struct dma_chan *chan)
 	dma_free_coherent(mxs_dma->dma_device.dev, CCW_BLOCK_SIZE,
 			mxs_chan->ccw, mxs_chan->ccw_phys);
 
+	if (mxs_dma->dev_id == IMX7D_DMA)
+		clk_disable_unprepare(mxs_dma->clk_io);
+
 	clk_disable_unprepare(mxs_dma->clk);
 }
 
@@ -701,9 +721,15 @@ static int mxs_dma_init(struct mxs_dma_engine *mxs_dma)
 	if (ret)
 		return ret;
 
+	if (mxs_dma->dev_id == IMX7D_DMA) {
+		ret = clk_prepare_enable(mxs_dma->clk_io);
+		if (ret)
+			goto err_clk_bch;
+	}
+
 	ret = stmp_reset_block(mxs_dma->base);
 	if (ret)
-		goto err_out;
+		goto err_clk_io;
 
 	/* enable apbh burst */
 	if (dma_is_apbh(mxs_dma)) {
@@ -717,7 +743,10 @@ static int mxs_dma_init(struct mxs_dma_engine *mxs_dma)
 	writel(MXS_DMA_CHANNELS_MASK << MXS_DMA_CHANNELS,
 		mxs_dma->base + HW_APBHX_CTRL1 + STMP_OFFSET_REG_SET);
 
-err_out:
+err_clk_io:
+	if (mxs_dma->dev_id == IMX7D_DMA)
+		clk_disable_unprepare(mxs_dma->clk_io);
+err_clk_bch:
 	clk_disable_unprepare(mxs_dma->clk);
 	return ret;
 }
@@ -803,9 +832,18 @@ static int __init mxs_dma_probe(struct platform_device *pdev)
 	if (IS_ERR(mxs_dma->base))
 		return PTR_ERR(mxs_dma->base);
 
-	mxs_dma->clk = devm_clk_get(&pdev->dev, NULL);
-	if (IS_ERR(mxs_dma->clk))
-		return PTR_ERR(mxs_dma->clk);
+	if (mxs_dma->dev_id == IMX7D_DMA) {
+		mxs_dma->clk = devm_clk_get(&pdev->dev, "dma_apbh_bch");
+		if (IS_ERR(mxs_dma->clk))
+			return PTR_ERR(mxs_dma->clk);
+		mxs_dma->clk_io = devm_clk_get(&pdev->dev, "dma_apbh_io");
+		if (IS_ERR(mxs_dma->clk_io))
+			return PTR_ERR(mxs_dma->clk_io);
+	} else {
+		mxs_dma->clk = devm_clk_get(&pdev->dev, NULL);
+		if (IS_ERR(mxs_dma->clk))
+			return PTR_ERR(mxs_dma->clk);
+	}
 
 	dma_cap_set(DMA_SLAVE, mxs_dma->dma_device.cap_mask);
 	dma_cap_set(DMA_CYCLIC, mxs_dma->dma_device.cap_mask);
-- 
1.9.1

Re: [PATCH v8 3/7] dmaengine: mxs: add i.MX7D APBH DMA support

[Vinod Koul]

On Wed, Dec 02, 2015 at 04:47:42PM -0600, Han Xu wrote:
> From: Adrian Alonso <aalonso@freescale.com>
> 
> supports APBH DMA on i.MX7D by add extra clock clk_io

Acked-by: Vinod Koul <vinod.koul@intel.com>

-- 
~Vinod

[PATCH v8 4/7] mtd: nand: gpmi: may use minimum required ecc for 744 oobsize NAND

[Han Xu]

By default NAND driver will choose the highest ecc strength that oob
could contain, in this case, for some 8K+744 NAND flash, the ecc
strength will be up to 52bit, which beyonds the i.MX6QDL BCH capability
(40bit).

This patch allows the NAND driver try to use minimum required ecc
strength if it failed to use the highest ecc, even without explicitly
claiming "fsl,use-minimum-ecc" in dts.

Signed-off-by: Han Xu <b45815@freescale.com>
---
 drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 26 ++++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
index ba5975f..298c1d1 100644
--- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
@@ -136,7 +136,7 @@ static inline bool gpmi_check_ecc(struct gpmi_nand_data *this)
  *
  * We may have available oob space in this case.
  */
-static bool set_geometry_by_ecc_info(struct gpmi_nand_data *this)
+static int set_geometry_by_ecc_info(struct gpmi_nand_data *this)
 {
 	struct bch_geometry *geo = &this->bch_geometry;
 	struct mtd_info *mtd = &this->mtd;
@@ -145,7 +145,7 @@ static bool set_geometry_by_ecc_info(struct gpmi_nand_data *this)
 	unsigned int block_mark_bit_offset;
 
 	if (!(chip->ecc_strength_ds > 0 && chip->ecc_step_ds > 0))
-		return false;
+		return -EINVAL;
 
 	switch (chip->ecc_step_ds) {
 	case SZ_512:
@@ -158,19 +158,19 @@ static bool set_geometry_by_ecc_info(struct gpmi_nand_data *this)
 		dev_err(this->dev,
 			"unsupported nand chip. ecc bits : %d, ecc size : %d\n",
 			chip->ecc_strength_ds, chip->ecc_step_ds);
-		return false;
+		return -EINVAL;
 	}
 	geo->ecc_chunk_size = chip->ecc_step_ds;
 	geo->ecc_strength = round_up(chip->ecc_strength_ds, 2);
 	if (!gpmi_check_ecc(this))
-		return false;
+		return -EINVAL;
 
 	/* Keep the C >= O */
 	if (geo->ecc_chunk_size < mtd->oobsize) {
 		dev_err(this->dev,
 			"unsupported nand chip. ecc size: %d, oob size : %d\n",
 			chip->ecc_step_ds, mtd->oobsize);
-		return false;
+		return -EINVAL;
 	}
 
 	/* The default value, see comment in the legacy_set_geometry(). */
@@ -242,7 +242,7 @@ static bool set_geometry_by_ecc_info(struct gpmi_nand_data *this)
 				+ ALIGN(geo->ecc_chunk_count, 4);
 
 	if (!this->swap_block_mark)
-		return true;
+		return 0;
 
 	/* For bit swap. */
 	block_mark_bit_offset = mtd->writesize * 8 -
@@ -251,7 +251,7 @@ static bool set_geometry_by_ecc_info(struct gpmi_nand_data *this)
 
 	geo->block_mark_byte_offset = block_mark_bit_offset / 8;
 	geo->block_mark_bit_offset  = block_mark_bit_offset % 8;
-	return true;
+	return 0;
 }
 
 static int legacy_set_geometry(struct gpmi_nand_data *this)
@@ -285,7 +285,8 @@ static int legacy_set_geometry(struct gpmi_nand_data *this)
 	geo->ecc_strength = get_ecc_strength(this);
 	if (!gpmi_check_ecc(this)) {
 		dev_err(this->dev,
-			"required ecc strength of the NAND chip: %d is not supported by the GPMI controller (%d)\n",
+			"ecc strength: %d cannot be supported by the controller (%d)\n"
+			"try to use minimum ecc strength that NAND chip required\n",
 			geo->ecc_strength,
 			this->devdata->bch_max_ecc_strength);
 		return -EINVAL;
@@ -366,10 +367,11 @@ static int legacy_set_geometry(struct gpmi_nand_data *this)
 
 int common_nfc_set_geometry(struct gpmi_nand_data *this)
 {
-	if (of_property_read_bool(this->dev->of_node, "fsl,use-minimum-ecc")
-		&& set_geometry_by_ecc_info(this))
-		return 0;
-	return legacy_set_geometry(this);
+	if ((of_property_read_bool(this->dev->of_node, "fsl,use-minimum-ecc"))
+				|| legacy_set_geometry(this))
+		return set_geometry_by_ecc_info(this);
+
+	return 0;
 }
 
 struct dma_chan *get_dma_chan(struct gpmi_nand_data *this)
-- 
1.9.1

Re: [PATCH v8 4/7] mtd: nand: gpmi: may use minimum required ecc for 744 oobsize NAND

[Huang Shijie]

On Wed, Dec 02, 2015 at 04:47:43PM -0600, Han Xu wrote:
> By default NAND driver will choose the highest ecc strength that oob
> could contain, in this case, for some 8K+744 NAND flash, the ecc
> strength will be up to 52bit, which beyonds the i.MX6QDL BCH capability
> (40bit).
>
> This patch allows the NAND driver try to use minimum required ecc
> strength if it failed to use the highest ecc, even without explicitly
> claiming "fsl,use-minimum-ecc" in dts.
>
> Signed-off-by: Han Xu <b45815@freescale.com>
> ---
>  drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 26 ++++++++++++++------------
>  1 file changed, 14 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> index ba5975f..298c1d1 100644
> --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> @@ -136,7 +136,7 @@ static inline bool gpmi_check_ecc(struct gpmi_nand_data *this)
>   *
>   * We may have available oob space in this case.
>   */
> -static bool set_geometry_by_ecc_info(struct gpmi_nand_data *this)
> +static int set_geometry_by_ecc_info(struct gpmi_nand_data *this)
>  {
>       struct bch_geometry *geo = &this->bch_geometry;
>       struct mtd_info *mtd = &this->mtd;
> @@ -145,7 +145,7 @@ static bool set_geometry_by_ecc_info(struct gpmi_nand_data *this)
>       unsigned int block_mark_bit_offset;
>
>       if (!(chip->ecc_strength_ds > 0 && chip->ecc_step_ds > 0))
> -             return false;
> +             return -EINVAL;
>
>       switch (chip->ecc_step_ds) {
>       case SZ_512:
> @@ -158,19 +158,19 @@ static bool set_geometry_by_ecc_info(struct gpmi_nand_data *this)
>               dev_err(this->dev,
>                       "unsupported nand chip. ecc bits : %d, ecc size : %d\n",
>                       chip->ecc_strength_ds, chip->ecc_step_ds);
> -             return false;
> +             return -EINVAL;
>       }
>       geo->ecc_chunk_size = chip->ecc_step_ds;
>       geo->ecc_strength = round_up(chip->ecc_strength_ds, 2);
>       if (!gpmi_check_ecc(this))
> -             return false;
> +             return -EINVAL;
>
>       /* Keep the C >= O */
>       if (geo->ecc_chunk_size < mtd->oobsize) {
>               dev_err(this->dev,
>                       "unsupported nand chip. ecc size: %d, oob size : %d\n",
>                       chip->ecc_step_ds, mtd->oobsize);
> -             return false;
> +             return -EINVAL;
>       }
>
>       /* The default value, see comment in the legacy_set_geometry(). */
> @@ -242,7 +242,7 @@ static bool set_geometry_by_ecc_info(struct gpmi_nand_data *this)
>                               + ALIGN(geo->ecc_chunk_count, 4);
>
>       if (!this->swap_block_mark)
> -             return true;
> +             return 0;
>
>       /* For bit swap. */
>       block_mark_bit_offset = mtd->writesize * 8 -
> @@ -251,7 +251,7 @@ static bool set_geometry_by_ecc_info(struct gpmi_nand_data *this)
>
>       geo->block_mark_byte_offset = block_mark_bit_offset / 8;
>       geo->block_mark_bit_offset  = block_mark_bit_offset % 8;
> -     return true;
> +     return 0;
>  }
>
>  static int legacy_set_geometry(struct gpmi_nand_data *this)
> @@ -285,7 +285,8 @@ static int legacy_set_geometry(struct gpmi_nand_data *this)
>       geo->ecc_strength = get_ecc_strength(this);
>       if (!gpmi_check_ecc(this)) {
>               dev_err(this->dev,
> -                     "required ecc strength of the NAND chip: %d is not supported by the GPMI controller (%d)\n",
> +                     "ecc strength: %d cannot be supported by the controller (%d)\n"
> +                     "try to use minimum ecc strength that NAND chip required\n",
>                       geo->ecc_strength,
>                       this->devdata->bch_max_ecc_strength);
>               return -EINVAL;
> @@ -366,10 +367,11 @@ static int legacy_set_geometry(struct gpmi_nand_data *this)
>
>  int common_nfc_set_geometry(struct gpmi_nand_data *this)
>  {
> -     if (of_property_read_bool(this->dev->of_node, "fsl,use-minimum-ecc")
> -             && set_geometry_by_ecc_info(this))
> -             return 0;
> -     return legacy_set_geometry(this);
> +     if ((of_property_read_bool(this->dev->of_node, "fsl,use-minimum-ecc"))
> +                             || legacy_set_geometry(this))
> +             return set_geometry_by_ecc_info(this);
> +
> +     return 0;
>  }
>
>  struct dma_chan *get_dma_chan(struct gpmi_nand_data *this)
> --
> 1.9.1
>
Acked-by: Huang Shijie <shijie.huang@arm.com>
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

Re: [PATCH v8 4/7] mtd: nand: gpmi: may use minimum required ecc for 744 oobsize NAND

[Brian Norris]

On Wed, Dec 02, 2015 at 04:47:43PM -0600, Han Xu wrote:
> By default NAND driver will choose the highest ecc strength that oob
> could contain, in this case, for some 8K+744 NAND flash, the ecc
> strength will be up to 52bit, which beyonds the i.MX6QDL BCH capability
> (40bit).
> 
> This patch allows the NAND driver try to use minimum required ecc
> strength if it failed to use the highest ecc, even without explicitly
> claiming "fsl,use-minimum-ecc" in dts.
> 
> Signed-off-by: Han Xu <b45815@freescale.com>

Pushed this one to l2-mtd.git/next too.

Would it help to implement support for the "nand-ecc-step-size" and
"nand-ecc-strength" properties sometime? That would be more
maintainable, as it's more specific. What if you need a little stronger
than the minimum ECC? You also are relying on not changing the default
behavior of the driver, for the "legacy" ECC calculation still. That
ties your hands a bit.

Brian

[PATCH v8 5/7] mtd: nand: gpmi: add GPMI NAND support for i.MX7D

[Han Xu]

support GPMI NAND on i.MX7D

Signed-off-by: Han Xu <b45815@freescale.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 | 27 ++++++++++++++++++++++-----
 drivers/mtd/nand/gpmi-nand/gpmi-nand.h |  7 +++++--
 4 files changed, 40 insertions(+), 18 deletions(-)

diff --git a/drivers/mtd/nand/gpmi-nand/bch-regs.h b/drivers/mtd/nand/gpmi-nand/bch-regs.h
index 05bb91f..53e58bc 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-2015 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 43fa16b..1f26a79 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-2015 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 298c1d1..9f67f0f 100644
--- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
@@ -77,6 +77,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;
@@ -575,6 +581,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;
@@ -592,6 +602,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;
 
@@ -608,7 +620,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.
 		 *
@@ -1869,8 +1881,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;
 	}
@@ -1936,7 +1948,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);
+	ret = nand_scan_ident(mtd, GPMI_IS_MX6(this) ||
+			GPMI_IS_MX7(this) ? 2 : 1, NULL);
 	if (ret)
 		goto err_out;
 
@@ -1979,7 +1992,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 = (void *)&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 544062f..58b3d69 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-2015 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 {
@@ -306,6 +307,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

Re: [PATCH v8 5/7] mtd: nand: gpmi: add GPMI NAND support for i.MX7D

[Huang Shijie]

On Wed, Dec 02, 2015 at 04:47:44PM -0600, Han Xu wrote:
> support GPMI NAND on i.MX7D
>
> Signed-off-by: Han Xu <b45815@freescale.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 | 27 ++++++++++++++++++++++-----
>  drivers/mtd/nand/gpmi-nand/gpmi-nand.h |  7 +++++--
>  4 files changed, 40 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/mtd/nand/gpmi-nand/bch-regs.h b/drivers/mtd/nand/gpmi-nand/bch-regs.h
> index 05bb91f..53e58bc 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-2015 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 43fa16b..1f26a79 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-2015 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 298c1d1..9f67f0f 100644
> --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> @@ -77,6 +77,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;
> @@ -575,6 +581,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;
> @@ -592,6 +602,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;
>
> @@ -608,7 +620,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.
>                *
> @@ -1869,8 +1881,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;
>       }
> @@ -1936,7 +1948,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);
> +     ret = nand_scan_ident(mtd, GPMI_IS_MX6(this) ||
> +                     GPMI_IS_MX7(this) ? 2 : 1, NULL);
>       if (ret)
>               goto err_out;
>
> @@ -1979,7 +1992,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 = (void *)&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 544062f..58b3d69 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-2015 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 {
> @@ -306,6 +307,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
>
Acked-by: Huang Shijie <shijie.huang@arm.com>
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

Re: [PATCH v8 5/7] mtd: nand: gpmi: add GPMI NAND support for i.MX7D

[Brian Norris]

On Wed, Dec 02, 2015 at 04:47:44PM -0600, Han Xu wrote:
> support GPMI NAND on i.MX7D
> 
> Signed-off-by: Han Xu <b45815@freescale.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 | 27 ++++++++++++++++++++++-----
>  drivers/mtd/nand/gpmi-nand/gpmi-nand.h |  7 +++++--
>  4 files changed, 40 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/mtd/nand/gpmi-nand/bch-regs.h b/drivers/mtd/nand/gpmi-nand/bch-regs.h
> index 05bb91f..53e58bc 100644
> --- a/drivers/mtd/nand/gpmi-nand/bch-regs.h
> +++ b/drivers/mtd/nand/gpmi-nand/bch-regs.h

...

> @@ -575,6 +581,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;
> @@ -592,6 +602,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;
>  

...

It looks like these clock names were never documented. Shouldn't this be
part of Documentation/devicetree/bindings/mtd/gpmi-nand.txt?

Otherwise, the patch looks good to me.

Brian

[PATCH v8 6/7] mtd: nand: gpmi: correct bitflip for erased NAND page

[Han Xu]

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 <b45815@freescale.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 | 24 +++++++++++++++++++++++-
 drivers/mtd/nand/gpmi-nand/gpmi-nand.h |  5 ++++-
 4 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/nand/gpmi-nand/bch-regs.h b/drivers/mtd/nand/gpmi-nand/bch-regs.h
index 53e58bc..a84d72b 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 1f26a79..0548d84 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 9f67f0f..9dea56e 100644
--- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
@@ -71,6 +71,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,
@@ -1010,6 +1016,7 @@ static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
 {
 	struct gpmi_nand_data *this = chip->priv;
 	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;
@@ -1018,6 +1025,7 @@ 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 flag = 0;
 
 	dev_dbg(this->dev, "page number is : %d\n", page);
 	ret = read_page_prepare(this, buf, nfc_geo->payload_size,
@@ -1050,9 +1058,16 @@ static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
 	status = auxiliary_virt + nfc_geo->auxiliary_status_offset;
 
 	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))
+				if (readl(bch_regs + HW_BCH_DEBUG1))
+					flag = 1;
+			continue;
+		}
+
 		if (*status == STATUS_UNCORRECTABLE) {
 			mtd->ecc_stats.failed++;
 			continue;
@@ -1081,6 +1096,10 @@ static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
 			nfc_geo->payload_size,
 			payload_virt, payload_phys);
 
+	/* if bitflip occurred in erased page, change data to all 0xff */
+	if (flag)
+		memset(buf, 0xff, nfc_geo->payload_size);
+
 	return max_bitflips;
 }
 
@@ -1990,6 +2009,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 = (void *)&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 58b3d69..149a442 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,
 };
@@ -306,9 +307,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

Re: [PATCH v8 6/7] mtd: nand: gpmi: correct bitflip for erased NAND page

[Huang Shijie]

On Wed, Dec 02, 2015 at 04:47:45PM -0600, Han Xu 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 <b45815@freescale.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 | 24 +++++++++++++++++++++++-
>  drivers/mtd/nand/gpmi-nand/gpmi-nand.h |  5 ++++-
>  4 files changed, 42 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mtd/nand/gpmi-nand/bch-regs.h b/drivers/mtd/nand/gpmi-nand/bch-regs.h
> index 53e58bc..a84d72b 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 1f26a79..0548d84 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 9f67f0f..9dea56e 100644
> --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> @@ -71,6 +71,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,
> @@ -1010,6 +1016,7 @@ static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
>  {
>       struct gpmi_nand_data *this = chip->priv;
>       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;
> @@ -1018,6 +1025,7 @@ 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 flag = 0;
>
>       dev_dbg(this->dev, "page number is : %d\n", page);
>       ret = read_page_prepare(this, buf, nfc_geo->payload_size,
> @@ -1050,9 +1058,16 @@ static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
>       status = auxiliary_virt + nfc_geo->auxiliary_status_offset;
>
>       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))
> +                             if (readl(bch_regs + HW_BCH_DEBUG1))
> +                                     flag = 1;
> +                     continue;
> +             }
> +
>               if (*status == STATUS_UNCORRECTABLE) {
>                       mtd->ecc_stats.failed++;
>                       continue;
> @@ -1081,6 +1096,10 @@ static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
>                       nfc_geo->payload_size,
>                       payload_virt, payload_phys);
>
> +     /* if bitflip occurred in erased page, change data to all 0xff */
> +     if (flag)
> +             memset(buf, 0xff, nfc_geo->payload_size);
> +
>       return max_bitflips;
>  }
>
> @@ -1990,6 +2009,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 = (void *)&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 58b3d69..149a442 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,
>  };
> @@ -306,9 +307,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
>
Very good feature.

Acked-by: Huang Shijie <shijie.huang@arm.com>
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

Re: [PATCH v8 6/7] mtd: nand: gpmi: correct bitflip for erased NAND page

[Brian Norris]

+ Markus Pargmann

On Wed, Dec 02, 2015 at 04:47:45PM -0600, Han Xu 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.

Looks like Marcus is also trying to add SW correction for the same
issue:

http://patchwork.ozlabs.org/patch/559557/

How do those 2 interact (if at all)?

Brian

> Signed-off-by: Han Xu <b45815@freescale.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 | 24 +++++++++++++++++++++++-
>  drivers/mtd/nand/gpmi-nand/gpmi-nand.h |  5 ++++-
>  4 files changed, 42 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mtd/nand/gpmi-nand/bch-regs.h b/drivers/mtd/nand/gpmi-nand/bch-regs.h
> index 53e58bc..a84d72b 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 1f26a79..0548d84 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 9f67f0f..9dea56e 100644
> --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> @@ -71,6 +71,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,
> @@ -1010,6 +1016,7 @@ static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
>  {
>  	struct gpmi_nand_data *this = chip->priv;
>  	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;
> @@ -1018,6 +1025,7 @@ 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 flag = 0;
>  
>  	dev_dbg(this->dev, "page number is : %d\n", page);
>  	ret = read_page_prepare(this, buf, nfc_geo->payload_size,
> @@ -1050,9 +1058,16 @@ static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
>  	status = auxiliary_virt + nfc_geo->auxiliary_status_offset;
>  
>  	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))
> +				if (readl(bch_regs + HW_BCH_DEBUG1))
> +					flag = 1;
> +			continue;
> +		}
> +
>  		if (*status == STATUS_UNCORRECTABLE) {
>  			mtd->ecc_stats.failed++;
>  			continue;
> @@ -1081,6 +1096,10 @@ static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
>  			nfc_geo->payload_size,
>  			payload_virt, payload_phys);
>  
> +	/* if bitflip occurred in erased page, change data to all 0xff */
> +	if (flag)
> +		memset(buf, 0xff, nfc_geo->payload_size);
> +
>  	return max_bitflips;
>  }
>  
> @@ -1990,6 +2009,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 = (void *)&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 58b3d69..149a442 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,
>  };
> @@ -306,9 +307,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
> 

Re: [PATCH v8 6/7] mtd: nand: gpmi: correct bitflip for erased NAND page

[Markus Pargmann]

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

Hi,

On Saturday, January 23, 2016 03:01:47 PM Brian Norris wrote:
> + Markus Pargmann
> 
> On Wed, Dec 02, 2015 at 04:47:45PM -0600, Han Xu 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.
> 
> Looks like Marcus is also trying to add SW correction for the same
> issue:
> 
> http://patchwork.ozlabs.org/patch/559557/
> 
> How do those 2 interact (if at all)?

Thanks for CCing.

> 
> Brian
> 
> > Signed-off-by: Han Xu <b45815@freescale.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 | 24 +++++++++++++++++++++++-
> >  drivers/mtd/nand/gpmi-nand/gpmi-nand.h |  5 ++++-
> >  4 files changed, 42 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/mtd/nand/gpmi-nand/bch-regs.h b/drivers/mtd/nand/gpmi-nand/bch-regs.h
> > index 53e58bc..a84d72b 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 1f26a79..0548d84 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);

This is probably different on imx6qp and imx7?!
On the other imx6 processors, setting this will instruct the BCH engine
to allow 'ecc_strength' number of bitflips in an erased area without a
reported ECC error. Unfortunately it does not fix the bitflips.

So what happens on imx6q,dl,s in this case:
1) BCH detects bitflips in the erased page, but ignores them due to the
ERASE_THRESHOLD setting up to 'ecc_strength' bitflips.
2) The gpmi driver does not notice any errors as BCH did not set that
there is any ECC error in the data. So the driver assumes correct data
and does not fix the existing bitflips in the erased page.
3) UBI does not see any ECC errors and does not check the page.
4) UBIFS does check the content of erased pages at some points and
breaks at this point. The filesystem can't be mounted anymore.

This is why I added my patch a software check in case of an
uncorrectable ECC error. The ERASE_THRESHOLD has to be 0 for this to
work.

As this patch only affects imx6qp and imx7, I assume that there is a
better mechanism now.

Best Regards,

Markus

> > +
> >  	/* 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 9f67f0f..9dea56e 100644
> > --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> > +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> > @@ -71,6 +71,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,
> > @@ -1010,6 +1016,7 @@ static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
> >  {
> >  	struct gpmi_nand_data *this = chip->priv;
> >  	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;
> > @@ -1018,6 +1025,7 @@ 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 flag = 0;
> >  
> >  	dev_dbg(this->dev, "page number is : %d\n", page);
> >  	ret = read_page_prepare(this, buf, nfc_geo->payload_size,
> > @@ -1050,9 +1058,16 @@ static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
> >  	status = auxiliary_virt + nfc_geo->auxiliary_status_offset;
> >  
> >  	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))
> > +				if (readl(bch_regs + HW_BCH_DEBUG1))
> > +					flag = 1;
> > +			continue;
> > +		}
> > +
> >  		if (*status == STATUS_UNCORRECTABLE) {
> >  			mtd->ecc_stats.failed++;
> >  			continue;
> > @@ -1081,6 +1096,10 @@ static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
> >  			nfc_geo->payload_size,
> >  			payload_virt, payload_phys);
> >  
> > +	/* if bitflip occurred in erased page, change data to all 0xff */
> > +	if (flag)
> > +		memset(buf, 0xff, nfc_geo->payload_size);
> > +
> >  	return max_bitflips;
> >  }
> >  
> > @@ -1990,6 +2009,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 = (void *)&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 58b3d69..149a442 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,
> >  };
> > @@ -306,9 +307,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
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v8 6/7] mtd: nand: gpmi: correct bitflip for erased NAND page

[Han Xu]

On Tue, Feb 2, 2016 at 7:28 AM, Markus Pargmann <mpa@pengutronix.de> wrote:
> Hi,
>
> On Saturday, January 23, 2016 03:01:47 PM Brian Norris wrote:
>> + Markus Pargmann
>>
>> On Wed, Dec 02, 2015 at 04:47:45PM -0600, Han Xu 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.
>>
>> Looks like Marcus is also trying to add SW correction for the same
>> issue:
>>
>> http://patchwork.ozlabs.org/patch/559557/
>>
>> How do those 2 interact (if at all)?
>
> Thanks for CCing.
>
>>
>> Brian
>>
>> > Signed-off-by: Han Xu <b45815@freescale.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 | 24 +++++++++++++++++++++++-
>> >  drivers/mtd/nand/gpmi-nand/gpmi-nand.h |  5 ++++-
>> >  4 files changed, 42 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/drivers/mtd/nand/gpmi-nand/bch-regs.h b/drivers/mtd/nand/gpmi-nand/bch-regs.h
>> > index 53e58bc..a84d72b 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 1f26a79..0548d84 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);
>
> This is probably different on imx6qp and imx7?!
> On the other imx6 processors, setting this will instruct the BCH engine
> to allow 'ecc_strength' number of bitflips in an erased area without a
> reported ECC error. Unfortunately it does not fix the bitflips.
>
> So what happens on imx6q,dl,s in this case:
> 1) BCH detects bitflips in the erased page, but ignores them due to the
> ERASE_THRESHOLD setting up to 'ecc_strength' bitflips.
> 2) The gpmi driver does not notice any errors as BCH did not set that
> there is any ECC error in the data. So the driver assumes correct data
> and does not fix the existing bitflips in the erased page.
> 3) UBI does not see any ECC errors and does not check the page.
> 4) UBIFS does check the content of erased pages at some points and
> breaks at this point. The filesystem can't be mounted anymore.
>
> This is why I added my patch a software check in case of an
> uncorrectable ECC error. The ERASE_THRESHOLD has to be 0 for this to
> work.
>
> As this patch only affects imx6qp and imx7, I assume that there is a
> better mechanism now.

IC team add this new feature in i.mx6qp and i.mx7 for bitflip, which
provide a easy way to fix the issue, while we still need some SW check
for other i.mx6 platforms. So these patches don't conflict with
others.

Han Xu

>
> Best Regards,
>
> Markus
>
>> > +
>> >     /* 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 9f67f0f..9dea56e 100644
>> > --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
>> > +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
>> > @@ -71,6 +71,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,
>> > @@ -1010,6 +1016,7 @@ static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
>> >  {
>> >     struct gpmi_nand_data *this = chip->priv;
>> >     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;
>> > @@ -1018,6 +1025,7 @@ 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 flag = 0;
>> >
>> >     dev_dbg(this->dev, "page number is : %d\n", page);
>> >     ret = read_page_prepare(this, buf, nfc_geo->payload_size,
>> > @@ -1050,9 +1058,16 @@ static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
>> >     status = auxiliary_virt + nfc_geo->auxiliary_status_offset;
>> >
>> >     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))
>> > +                           if (readl(bch_regs + HW_BCH_DEBUG1))
>> > +                                   flag = 1;
>> > +                   continue;
>> > +           }
>> > +
>> >             if (*status == STATUS_UNCORRECTABLE) {
>> >                     mtd->ecc_stats.failed++;
>> >                     continue;
>> > @@ -1081,6 +1096,10 @@ static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
>> >                     nfc_geo->payload_size,
>> >                     payload_virt, payload_phys);
>> >
>> > +   /* if bitflip occurred in erased page, change data to all 0xff */
>> > +   if (flag)
>> > +           memset(buf, 0xff, nfc_geo->payload_size);
>> > +
>> >     return max_bitflips;
>> >  }
>> >
>> > @@ -1990,6 +2009,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 = (void *)&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 58b3d69..149a442 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,
>> >  };
>> > @@ -306,9 +307,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
>>
>
> --
> Pengutronix e.K.                           |                             |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
>

[PATCH v8 7/7] mtd: nand: gpmi: support NAND on i.MX6UL

[Han Xu]

support GPMI NAND on i.MX6UL

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

diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
index 9dea56e..41d9012 100644
--- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
@@ -89,6 +89,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;
@@ -2015,6 +2021,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 = (void *)&gpmi_devdata_imx6ul,
+	}, {
 		.compatible = "fsl,imx7d-gpmi-nand",
 		.data = (void *)&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 149a442..331f98e 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 {
@@ -310,8 +311,9 @@ 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

Re: [PATCH v8 7/7] mtd: nand: gpmi: support NAND on i.MX6UL

[Huang Shijie]

On Wed, Dec 02, 2015 at 04:47:46PM -0600, Han Xu wrote:
> support GPMI NAND on i.MX6UL
>
> Signed-off-by: Han Xu <han.xu@freescale.com>
> ---
>  drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 9 +++++++++
>  drivers/mtd/nand/gpmi-nand/gpmi-nand.h | 4 +++-
>  2 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> index 9dea56e..41d9012 100644
> --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> @@ -89,6 +89,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;
> @@ -2015,6 +2021,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 = (void *)&gpmi_devdata_imx6ul,
> +     }, {
>               .compatible = "fsl,imx7d-gpmi-nand",
>               .data = (void *)&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 149a442..331f98e 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 {
> @@ -310,8 +311,9 @@ 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
>
Acked-by: Huang Shijie <shijie.huang@arm.com>
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

Re: [PATCH v8 7/7] mtd: nand: gpmi: support NAND on i.MX6UL

[Brian Norris]

On Wed, Dec 02, 2015 at 04:47:46PM -0600, Han Xu wrote:
> support GPMI NAND on i.MX6UL
> 
> Signed-off-by: Han Xu <han.xu@freescale.com>
> ---
>  drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 9 +++++++++
>  drivers/mtd/nand/gpmi-nand/gpmi-nand.h | 4 +++-
>  2 files changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> index 9dea56e..41d9012 100644
> --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> @@ -89,6 +89,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;
> @@ -2015,6 +2021,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 = (void *)&gpmi_devdata_imx6ul,
> +	}, {
>  		.compatible = "fsl,imx7d-gpmi-nand",
>  		.data = (void *)&gpmi_devdata_imx7d,
>  	}, { /* sentinel */ }

DT binding documentation?

[...]

Brian