[PATCH 0/6] Misc Marvell NAND controller driver changes

[PATCH 0/6] Misc Marvell NAND controller driver changes

[Miquel Raynal]

Here are a few cleanups/fixes for the Marvell NAND controller
driver. While working on another driver, I figured some
mistakes/unclear names that I wanted to change.

Cheers,
Miquèl

Miquel Raynal (6):
  mtd: rawnand: marvell: Fix the condition on a return code
  mtd: rawnand: marvell: Use devm_platform_ioremap_res()
  mtd: rawnand: marvell: Use nand_cleanup() when the device is not yet
    registered
  mtd: rawnand: marvell: Fix probe error path
  mtd: rawnand: marvell: Rename a function to clarify
  mtd: rawnand: marvell: Rename the ->correct() function

 drivers/mtd/nand/raw/marvell_nand.c | 54 ++++++++++++++---------------
 1 file changed, 26 insertions(+), 28 deletions(-)

-- 
2.20.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 1/6] mtd: rawnand: marvell: Fix the condition on a return code

[Miquel Raynal]

In a previous fix, I changed the condition on which the timeout of an
IRQ is reached from:

    if (!ret)

into:

    if (ret && !pending)

While having a non-zero return code is usual in the Linux kernel, here
ret comes from a wait_for_completion_timeout() which returns 0 when
the waiting period is too long.

Hence, the revised condition should be:

    if (!ret && !pending)

The faulty patch did not produce any error because of the !pending
condition so this change is finally purely cosmetic and does not
change the actual driver behavior.

Fixes: cafb56dd741e ("mtd: rawnand: marvell: prevent timeouts on a loaded machine")
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/marvell_nand.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/raw/marvell_nand.c b/drivers/mtd/nand/raw/marvell_nand.c
index 179f0ca585f8..f2bf88336326 100644
--- a/drivers/mtd/nand/raw/marvell_nand.c
+++ b/drivers/mtd/nand/raw/marvell_nand.c
@@ -707,7 +707,7 @@ static int marvell_nfc_wait_op(struct nand_chip *chip, unsigned int timeout_ms)
 	 * In case the interrupt was not served in the required time frame,
 	 * check if the ISR was not served or if something went actually wrong.
 	 */
-	if (ret && !pending) {
+	if (!ret && !pending) {
 		dev_err(nfc->dev, "Timeout waiting for RB signal\n");
 		return -ETIMEDOUT;
 	}
-- 
2.20.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 1/6] mtd: rawnand: marvell: Fix the condition on a return code

[Boris Brezillon]

On Tue, 21 Apr 2020 18:48:52 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> In a previous fix, I changed the condition on which the timeout of an
> IRQ is reached from:
> 
>     if (!ret)
> 
> into:
> 
>     if (ret && !pending)
> 
> While having a non-zero return code is usual in the Linux kernel, here
> ret comes from a wait_for_completion_timeout() which returns 0 when
> the waiting period is too long.
> 
> Hence, the revised condition should be:
> 
>     if (!ret && !pending)
> 
> The faulty patch did not produce any error because of the !pending
> condition so this change is finally purely cosmetic and does not
> change the actual driver behavior.
> 
> Fixes: cafb56dd741e ("mtd: rawnand: marvell: prevent timeouts on a loaded machine")
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>

Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>

> ---
>  drivers/mtd/nand/raw/marvell_nand.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/nand/raw/marvell_nand.c b/drivers/mtd/nand/raw/marvell_nand.c
> index 179f0ca585f8..f2bf88336326 100644
> --- a/drivers/mtd/nand/raw/marvell_nand.c
> +++ b/drivers/mtd/nand/raw/marvell_nand.c
> @@ -707,7 +707,7 @@ static int marvell_nfc_wait_op(struct nand_chip *chip, unsigned int timeout_ms)
>  	 * In case the interrupt was not served in the required time frame,
>  	 * check if the ISR was not served or if something went actually wrong.
>  	 */
> -	if (ret && !pending) {
> +	if (!ret && !pending) {
>  		dev_err(nfc->dev, "Timeout waiting for RB signal\n");
>  		return -ETIMEDOUT;
>  	}


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 2/6] mtd: rawnand: marvell: Use devm_platform_ioremap_res()

[Miquel Raynal]

Switch from the old platform_get_resource()/devm_ioremap_resource()
couple to the newer devm_platform_ioremap_resource() helper.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/marvell_nand.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/mtd/nand/raw/marvell_nand.c b/drivers/mtd/nand/raw/marvell_nand.c
index f2bf88336326..7906bd3fc8cb 100644
--- a/drivers/mtd/nand/raw/marvell_nand.c
+++ b/drivers/mtd/nand/raw/marvell_nand.c
@@ -2854,7 +2854,6 @@ static int marvell_nfc_init(struct marvell_nfc *nfc)
 static int marvell_nfc_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
-	struct resource *r;
 	struct marvell_nfc *nfc;
 	int ret;
 	int irq;
@@ -2869,8 +2868,7 @@ static int marvell_nfc_probe(struct platform_device *pdev)
 	nfc->controller.ops = &marvell_nand_controller_ops;
 	INIT_LIST_HEAD(&nfc->chips);
 
-	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	nfc->regs = devm_ioremap_resource(dev, r);
+	nfc->regs = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(nfc->regs))
 		return PTR_ERR(nfc->regs);
 
-- 
2.20.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 2/6] mtd: rawnand: marvell: Use devm_platform_ioremap_res()

[Boris Brezillon]

On Tue, 21 Apr 2020 18:48:53 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> Switch from the old platform_get_resource()/devm_ioremap_resource()
> couple to the newer devm_platform_ioremap_resource() helper.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>

Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>

> ---
>  drivers/mtd/nand/raw/marvell_nand.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/marvell_nand.c b/drivers/mtd/nand/raw/marvell_nand.c
> index f2bf88336326..7906bd3fc8cb 100644
> --- a/drivers/mtd/nand/raw/marvell_nand.c
> +++ b/drivers/mtd/nand/raw/marvell_nand.c
> @@ -2854,7 +2854,6 @@ static int marvell_nfc_init(struct marvell_nfc *nfc)
>  static int marvell_nfc_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> -	struct resource *r;
>  	struct marvell_nfc *nfc;
>  	int ret;
>  	int irq;
> @@ -2869,8 +2868,7 @@ static int marvell_nfc_probe(struct platform_device *pdev)
>  	nfc->controller.ops = &marvell_nand_controller_ops;
>  	INIT_LIST_HEAD(&nfc->chips);
>  
> -	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -	nfc->regs = devm_ioremap_resource(dev, r);
> +	nfc->regs = devm_platform_ioremap_resource(pdev, 0);
>  	if (IS_ERR(nfc->regs))
>  		return PTR_ERR(nfc->regs);
>  


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 3/6] mtd: rawnand: marvell: Use nand_cleanup() when the device is not yet registered

[Miquel Raynal]

Do not call nand_release() while the MTD device has not been
registered, use nand_cleanup() instead.

Fixes: 02f26ecf8c77 ("mtd: nand: add reworked Marvell NAND controller driver")
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/marvell_nand.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/raw/marvell_nand.c b/drivers/mtd/nand/raw/marvell_nand.c
index 7906bd3fc8cb..350949b34eee 100644
--- a/drivers/mtd/nand/raw/marvell_nand.c
+++ b/drivers/mtd/nand/raw/marvell_nand.c
@@ -2664,7 +2664,7 @@ static int marvell_nand_chip_init(struct device *dev, struct marvell_nfc *nfc,
 		ret = mtd_device_register(mtd, NULL, 0);
 	if (ret) {
 		dev_err(dev, "failed to register mtd device: %d\n", ret);
-		nand_release(chip);
+		nand_cleanup(chip);
 		return ret;
 	}
 
-- 
2.20.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 3/6] mtd: rawnand: marvell: Use nand_cleanup() when the device is not yet registered

[Boris Brezillon]

On Tue, 21 Apr 2020 18:48:54 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> Do not call nand_release() while the MTD device has not been
> registered, use nand_cleanup() instead.
> 
> Fixes: 02f26ecf8c77 ("mtd: nand: add reworked Marvell NAND controller driver")
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>

Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>

> ---
>  drivers/mtd/nand/raw/marvell_nand.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/nand/raw/marvell_nand.c b/drivers/mtd/nand/raw/marvell_nand.c
> index 7906bd3fc8cb..350949b34eee 100644
> --- a/drivers/mtd/nand/raw/marvell_nand.c
> +++ b/drivers/mtd/nand/raw/marvell_nand.c
> @@ -2664,7 +2664,7 @@ static int marvell_nand_chip_init(struct device *dev, struct marvell_nfc *nfc,
>  		ret = mtd_device_register(mtd, NULL, 0);
>  	if (ret) {
>  		dev_err(dev, "failed to register mtd device: %d\n", ret);
> -		nand_release(chip);
> +		nand_cleanup(chip);
>  		return ret;
>  	}
>  


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 4/6] mtd: rawnand: marvell: Fix probe error path

[Miquel Raynal]

Ensure all chips are deregistered and cleaned in case of error during
the probe.

Fixes: 02f26ecf8c77 ("mtd: nand: add reworked Marvell NAND controller driver")
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/marvell_nand.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/mtd/nand/raw/marvell_nand.c b/drivers/mtd/nand/raw/marvell_nand.c
index 350949b34eee..cb1e1a295002 100644
--- a/drivers/mtd/nand/raw/marvell_nand.c
+++ b/drivers/mtd/nand/raw/marvell_nand.c
@@ -2673,6 +2673,16 @@ static int marvell_nand_chip_init(struct device *dev, struct marvell_nfc *nfc,
 	return 0;
 }
 
+static void marvell_nand_chips_cleanup(struct marvell_nfc *nfc)
+{
+	struct marvell_nand_chip *entry, *temp;
+
+	list_for_each_entry_safe(entry, temp, &nfc->chips, node) {
+		nand_release(&entry->chip);
+		list_del(&entry->node);
+	}
+}
+
 static int marvell_nand_chips_init(struct device *dev, struct marvell_nfc *nfc)
 {
 	struct device_node *np = dev->of_node;
@@ -2707,6 +2717,7 @@ static int marvell_nand_chips_init(struct device *dev, struct marvell_nfc *nfc)
 		ret = marvell_nand_chip_init(dev, nfc, nand_np);
 		if (ret) {
 			of_node_put(nand_np);
+			marvell_nand_chips_cleanup(nfc);
 			return ret;
 		}
 	}
@@ -2714,16 +2725,6 @@ static int marvell_nand_chips_init(struct device *dev, struct marvell_nfc *nfc)
 	return 0;
 }
 
-static void marvell_nand_chips_cleanup(struct marvell_nfc *nfc)
-{
-	struct marvell_nand_chip *entry, *temp;
-
-	list_for_each_entry_safe(entry, temp, &nfc->chips, node) {
-		nand_release(&entry->chip);
-		list_del(&entry->node);
-	}
-}
-
 static int marvell_nfc_init_dma(struct marvell_nfc *nfc)
 {
 	struct platform_device *pdev = container_of(nfc->dev,
-- 
2.20.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 4/6] mtd: rawnand: marvell: Fix probe error path

[Boris Brezillon]

On Tue, 21 Apr 2020 18:48:55 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> Ensure all chips are deregistered and cleaned in case of error during
> the probe.
> 
> Fixes: 02f26ecf8c77 ("mtd: nand: add reworked Marvell NAND controller driver")
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/mtd/nand/raw/marvell_nand.c | 21 +++++++++++----------
>  1 file changed, 11 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/marvell_nand.c b/drivers/mtd/nand/raw/marvell_nand.c
> index 350949b34eee..cb1e1a295002 100644
> --- a/drivers/mtd/nand/raw/marvell_nand.c
> +++ b/drivers/mtd/nand/raw/marvell_nand.c
> @@ -2673,6 +2673,16 @@ static int marvell_nand_chip_init(struct device *dev, struct marvell_nfc *nfc,
>  	return 0;
>  }
>  
> +static void marvell_nand_chips_cleanup(struct marvell_nfc *nfc)
> +{
> +	struct marvell_nand_chip *entry, *temp;
> +
> +	list_for_each_entry_safe(entry, temp, &nfc->chips, node) {
> +		nand_release(&entry->chip);
> +		list_del(&entry->node);
> +	}
> +}
> +
>  static int marvell_nand_chips_init(struct device *dev, struct marvell_nfc *nfc)
>  {
>  	struct device_node *np = dev->of_node;
> @@ -2707,6 +2717,7 @@ static int marvell_nand_chips_init(struct device *dev, struct marvell_nfc *nfc)
>  		ret = marvell_nand_chip_init(dev, nfc, nand_np);
>  		if (ret) {
>  			of_node_put(nand_np);
> +			marvell_nand_chips_cleanup(nfc);
>  			return ret;

I'd rather add an err_chips_cleanup label for that one so you don't
have to duplicate it if there are other error paths where this cleanup
is needed. 

>  		}
>  	}
> @@ -2714,16 +2725,6 @@ static int marvell_nand_chips_init(struct device *dev, struct marvell_nfc *nfc)
>  	return 0;
>  }
>  
> -static void marvell_nand_chips_cleanup(struct marvell_nfc *nfc)
> -{
> -	struct marvell_nand_chip *entry, *temp;
> -
> -	list_for_each_entry_safe(entry, temp, &nfc->chips, node) {
> -		nand_release(&entry->chip);
> -		list_del(&entry->node);
> -	}
> -}
> -
>  static int marvell_nfc_init_dma(struct marvell_nfc *nfc)
>  {
>  	struct platform_device *pdev = container_of(nfc->dev,


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 5/6] mtd: rawnand: marvell: Rename a function to clarify

[Miquel Raynal]

Cosmetic change to clarify the purpose of the function.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/marvell_nand.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/nand/raw/marvell_nand.c b/drivers/mtd/nand/raw/marvell_nand.c
index cb1e1a295002..2db143a97626 100644
--- a/drivers/mtd/nand/raw/marvell_nand.c
+++ b/drivers/mtd/nand/raw/marvell_nand.c
@@ -2166,8 +2166,8 @@ static const struct mtd_ooblayout_ops marvell_nand_ooblayout_ops = {
 	.free = marvell_nand_ooblayout_free,
 };
 
-static int marvell_nand_hw_ecc_ctrl_init(struct mtd_info *mtd,
-					 struct nand_ecc_ctrl *ecc)
+static int marvell_nand_hw_ecc_controller_init(struct mtd_info *mtd,
+					       struct nand_ecc_ctrl *ecc)
 {
 	struct nand_chip *chip = mtd_to_nand(mtd);
 	struct marvell_nfc *nfc = to_marvell_nfc(chip->controller);
@@ -2261,7 +2261,7 @@ static int marvell_nand_ecc_init(struct mtd_info *mtd,
 
 	switch (ecc->mode) {
 	case NAND_ECC_HW:
-		ret = marvell_nand_hw_ecc_ctrl_init(mtd, ecc);
+		ret = marvell_nand_hw_ecc_controller_init(mtd, ecc);
 		if (ret)
 			return ret;
 		break;
-- 
2.20.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 5/6] mtd: rawnand: marvell: Rename a function to clarify

[Boris Brezillon]

On Tue, 21 Apr 2020 18:48:56 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> Cosmetic change to clarify the purpose of the function.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>

Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>

> ---
>  drivers/mtd/nand/raw/marvell_nand.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/marvell_nand.c b/drivers/mtd/nand/raw/marvell_nand.c
> index cb1e1a295002..2db143a97626 100644
> --- a/drivers/mtd/nand/raw/marvell_nand.c
> +++ b/drivers/mtd/nand/raw/marvell_nand.c
> @@ -2166,8 +2166,8 @@ static const struct mtd_ooblayout_ops marvell_nand_ooblayout_ops = {
>  	.free = marvell_nand_ooblayout_free,
>  };
>  
> -static int marvell_nand_hw_ecc_ctrl_init(struct mtd_info *mtd,
> -					 struct nand_ecc_ctrl *ecc)
> +static int marvell_nand_hw_ecc_controller_init(struct mtd_info *mtd,
> +					       struct nand_ecc_ctrl *ecc)
>  {
>  	struct nand_chip *chip = mtd_to_nand(mtd);
>  	struct marvell_nfc *nfc = to_marvell_nfc(chip->controller);
> @@ -2261,7 +2261,7 @@ static int marvell_nand_ecc_init(struct mtd_info *mtd,
>  
>  	switch (ecc->mode) {
>  	case NAND_ECC_HW:
> -		ret = marvell_nand_hw_ecc_ctrl_init(mtd, ecc);
> +		ret = marvell_nand_hw_ecc_controller_init(mtd, ecc);
>  		if (ret)
>  			return ret;
>  		break;


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 6/6] mtd: rawnand: marvell: Rename the ->correct() function

[Miquel Raynal]

There is no correction involved at this point, it is just a matter of
reading registers and checking whether bitflips have occurred or
not. Rename the function to clarify it.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/marvell_nand.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/mtd/nand/raw/marvell_nand.c b/drivers/mtd/nand/raw/marvell_nand.c
index 2db143a97626..3e448b89aaad 100644
--- a/drivers/mtd/nand/raw/marvell_nand.c
+++ b/drivers/mtd/nand/raw/marvell_nand.c
@@ -932,14 +932,14 @@ static void marvell_nfc_check_empty_chunk(struct nand_chip *chip,
 }
 
 /*
- * Check a chunk is correct or not according to hardware ECC engine.
+ * Check if a chunk is correct or not according to the hardware ECC engine.
  * mtd->ecc_stats.corrected is updated, as well as max_bitflips, however
  * mtd->ecc_stats.failure is not, the function will instead return a non-zero
  * value indicating that a check on the emptyness of the subpage must be
- * performed before declaring the subpage corrupted.
+ * performed before actually declaring the subpage as "corrupted".
  */
-static int marvell_nfc_hw_ecc_correct(struct nand_chip *chip,
-				      unsigned int *max_bitflips)
+static int marvell_nfc_hw_ecc_check_bitflips(struct nand_chip *chip,
+					     unsigned int *max_bitflips)
 {
 	struct mtd_info *mtd = nand_to_mtd(chip);
 	struct marvell_nfc *nfc = to_marvell_nfc(chip->controller);
@@ -1053,7 +1053,7 @@ static int marvell_nfc_hw_ecc_hmg_read_page(struct nand_chip *chip, u8 *buf,
 	marvell_nfc_enable_hw_ecc(chip);
 	marvell_nfc_hw_ecc_hmg_do_read_page(chip, buf, chip->oob_poi, false,
 					    page);
-	ret = marvell_nfc_hw_ecc_correct(chip, &max_bitflips);
+	ret = marvell_nfc_hw_ecc_check_bitflips(chip, &max_bitflips);
 	marvell_nfc_disable_hw_ecc(chip);
 
 	if (!ret)
@@ -1336,7 +1336,7 @@ static int marvell_nfc_hw_ecc_bch_read_page(struct nand_chip *chip,
 		/* Read the chunk and detect number of bitflips */
 		marvell_nfc_hw_ecc_bch_read_chunk(chip, chunk, data, data_len,
 						  spare, spare_len, page);
-		ret = marvell_nfc_hw_ecc_correct(chip, &max_bitflips);
+		ret = marvell_nfc_hw_ecc_check_bitflips(chip, &max_bitflips);
 		if (ret)
 			failure_mask |= BIT(chunk);
 
@@ -1358,10 +1358,9 @@ static int marvell_nfc_hw_ecc_bch_read_page(struct nand_chip *chip,
 	 */
 
 	/*
-	 * In case there is any subpage read error reported by ->correct(), we
-	 * usually re-read only ECC bytes in raw mode and check if the whole
-	 * page is empty. In this case, it is normal that the ECC check failed
-	 * and we just ignore the error.
+	 * In case there is any subpage read error, we usually re-read only ECC
+	 * bytes in raw mode and check if the whole page is empty. In this case,
+	 * it is normal that the ECC check failed and we just ignore the error.
 	 *
 	 * However, it has been empirically observed that for some layouts (e.g
 	 * 2k page, 8b strength per 512B chunk), the controller tries to correct
-- 
2.20.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 6/6] mtd: rawnand: marvell: Rename the ->correct() function

[Boris Brezillon]

On Tue, 21 Apr 2020 18:48:57 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> There is no correction involved at this point, it is just a matter of
> reading registers and checking whether bitflips have occurred or
> not. Rename the function to clarify it.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>

Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>

> ---
>  drivers/mtd/nand/raw/marvell_nand.c | 19 +++++++++----------
>  1 file changed, 9 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/marvell_nand.c b/drivers/mtd/nand/raw/marvell_nand.c
> index 2db143a97626..3e448b89aaad 100644
> --- a/drivers/mtd/nand/raw/marvell_nand.c
> +++ b/drivers/mtd/nand/raw/marvell_nand.c
> @@ -932,14 +932,14 @@ static void marvell_nfc_check_empty_chunk(struct nand_chip *chip,
>  }
>  
>  /*
> - * Check a chunk is correct or not according to hardware ECC engine.
> + * Check if a chunk is correct or not according to the hardware ECC engine.
>   * mtd->ecc_stats.corrected is updated, as well as max_bitflips, however
>   * mtd->ecc_stats.failure is not, the function will instead return a non-zero
>   * value indicating that a check on the emptyness of the subpage must be
> - * performed before declaring the subpage corrupted.
> + * performed before actually declaring the subpage as "corrupted".
>   */
> -static int marvell_nfc_hw_ecc_correct(struct nand_chip *chip,
> -				      unsigned int *max_bitflips)
> +static int marvell_nfc_hw_ecc_check_bitflips(struct nand_chip *chip,
> +					     unsigned int *max_bitflips)
>  {
>  	struct mtd_info *mtd = nand_to_mtd(chip);
>  	struct marvell_nfc *nfc = to_marvell_nfc(chip->controller);
> @@ -1053,7 +1053,7 @@ static int marvell_nfc_hw_ecc_hmg_read_page(struct nand_chip *chip, u8 *buf,
>  	marvell_nfc_enable_hw_ecc(chip);
>  	marvell_nfc_hw_ecc_hmg_do_read_page(chip, buf, chip->oob_poi, false,
>  					    page);
> -	ret = marvell_nfc_hw_ecc_correct(chip, &max_bitflips);
> +	ret = marvell_nfc_hw_ecc_check_bitflips(chip, &max_bitflips);
>  	marvell_nfc_disable_hw_ecc(chip);
>  
>  	if (!ret)
> @@ -1336,7 +1336,7 @@ static int marvell_nfc_hw_ecc_bch_read_page(struct nand_chip *chip,
>  		/* Read the chunk and detect number of bitflips */
>  		marvell_nfc_hw_ecc_bch_read_chunk(chip, chunk, data, data_len,
>  						  spare, spare_len, page);
> -		ret = marvell_nfc_hw_ecc_correct(chip, &max_bitflips);
> +		ret = marvell_nfc_hw_ecc_check_bitflips(chip, &max_bitflips);
>  		if (ret)
>  			failure_mask |= BIT(chunk);
>  
> @@ -1358,10 +1358,9 @@ static int marvell_nfc_hw_ecc_bch_read_page(struct nand_chip *chip,
>  	 */
>  
>  	/*
> -	 * In case there is any subpage read error reported by ->correct(), we
> -	 * usually re-read only ECC bytes in raw mode and check if the whole
> -	 * page is empty. In this case, it is normal that the ECC check failed
> -	 * and we just ignore the error.
> +	 * In case there is any subpage read error, we usually re-read only ECC
> +	 * bytes in raw mode and check if the whole page is empty. In this case,
> +	 * it is normal that the ECC check failed and we just ignore the error.
>  	 *
>  	 * However, it has been empirically observed that for some layouts (e.g
>  	 * 2k page, 8b strength per 512B chunk), the controller tries to correct


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/