[PATCH v3] mtd: rawnand: mxc: Move the ECC engine initialization to the right place

[PATCH v3] mtd: rawnand: mxc: Move the ECC engine initialization to the right place

[Fabio Estevam]

No ECC initialization should happen during the host controller probe.

In fact, we need the probe function to call nand_scan() in order to:

- identify the device, its capabilities and constraints (nand_scan_ident())
- configure the ECC engine accordingly (->attach_chip())
- scan its content and prepare the core (nand_scan_tail())

Moving these lines to mxcnd_attach_chip() fixes a regression caused by
a previous commit supposed to clarify these steps.

When moving the ECC initialization from probe() to attach(), get rid
of the pdata usage to determine the engine type and let the core decide
instead.

Tested on a imx27-pdk board.

Reported-by: Fabio Estevam <festevam@gmail.com>
Co-developed-by: Miquel Raynal <miquel.raynal@bootlin.com>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Signed-off-by: Fabio Estevam <festevam@gmail.com>
---
Changes since v2:
- Remove pdata check in attach() and let the core determine the engine type

 drivers/mtd/nand/raw/mxc_nand.c | 17 +++++------------
 1 file changed, 5 insertions(+), 12 deletions(-)

diff --git a/drivers/mtd/nand/raw/mxc_nand.c b/drivers/mtd/nand/raw/mxc_nand.c
index d4200eb2ad32..684c51e5e60d 100644
--- a/drivers/mtd/nand/raw/mxc_nand.c
+++ b/drivers/mtd/nand/raw/mxc_nand.c
@@ -1681,6 +1681,11 @@ static int mxcnd_attach_chip(struct nand_chip *chip)
 	struct mxc_nand_host *host = nand_get_controller_data(chip);
 	struct device *dev = mtd->dev.parent;
 
+	chip->ecc.bytes = host->devtype_data->eccbytes;
+	host->eccsize = host->devtype_data->eccsize;
+	chip->ecc.size = 512;
+	mtd_set_ooblayout(mtd, host->devtype_data->ooblayout);
+
 	switch (chip->ecc.engine_type) {
 	case NAND_ECC_ENGINE_TYPE_ON_HOST:
 		chip->ecc.read_page = mxc_nand_read_page;
@@ -1836,19 +1841,7 @@ static int mxcnd_probe(struct platform_device *pdev)
 	if (host->devtype_data->axi_offset)
 		host->regs_axi = host->base + host->devtype_data->axi_offset;
 
-	this->ecc.bytes = host->devtype_data->eccbytes;
-	host->eccsize = host->devtype_data->eccsize;
-
 	this->legacy.select_chip = host->devtype_data->select_chip;
-	this->ecc.size = 512;
-	mtd_set_ooblayout(mtd, host->devtype_data->ooblayout);
-
-	if (host->pdata.hw_ecc) {
-		this->ecc.engine_type = NAND_ECC_ENGINE_TYPE_ON_HOST;
-	} else {
-		this->ecc.engine_type = NAND_ECC_ENGINE_TYPE_SOFT;
-		this->ecc.algo = NAND_ECC_ALGO_HAMMING;
-	}
 
 	/* NAND bus width determines access functions used by upper layer */
 	if (host->pdata.width == 2)
-- 
2.17.1


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

Re: [PATCH v3] mtd: rawnand: mxc: Move the ECC engine initialization to the right place

[Fabio Estevam]

Hi Miquel,

On Fri, Oct 16, 2020 at 6:36 PM Fabio Estevam <festevam@gmail.com> wrote:
>
> No ECC initialization should happen during the host controller probe.
>
> In fact, we need the probe function to call nand_scan() in order to:
>
> - identify the device, its capabilities and constraints (nand_scan_ident())
> - configure the ECC engine accordingly (->attach_chip())
> - scan its content and prepare the core (nand_scan_tail())
>
> Moving these lines to mxcnd_attach_chip() fixes a regression caused by
> a previous commit supposed to clarify these steps.
>
> When moving the ECC initialization from probe() to attach(), get rid
> of the pdata usage to determine the engine type and let the core decide
> instead.
>
> Tested on a imx27-pdk board.
>
> Reported-by: Fabio Estevam <festevam@gmail.com>
> Co-developed-by: Miquel Raynal <miquel.raynal@bootlin.com>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> Signed-off-by: Fabio Estevam <festevam@gmail.com>

Would you like me to resend it with the tag below?

Fixes: d7157ff49a5b ("mtd: rawnand: Use the ECC framework user input
parsing bits")

This commit is in Linus' tree now.

Thanks

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

Re: [PATCH v3] mtd: rawnand: mxc: Move the ECC engine initialization to the right place

[Sascha Hauer]

On Fri, Oct 16, 2020 at 06:36:13PM -0300, Fabio Estevam wrote:
> No ECC initialization should happen during the host controller probe.
> 
> In fact, we need the probe function to call nand_scan() in order to:
> 
> - identify the device, its capabilities and constraints (nand_scan_ident())
> - configure the ECC engine accordingly (->attach_chip())
> - scan its content and prepare the core (nand_scan_tail())
> 
> Moving these lines to mxcnd_attach_chip() fixes a regression caused by
> a previous commit supposed to clarify these steps.
> 
> When moving the ECC initialization from probe() to attach(), get rid
> of the pdata usage to determine the engine type and let the core decide
> instead.
> 
> Tested on a imx27-pdk board.
> 
> Reported-by: Fabio Estevam <festevam@gmail.com>
> Co-developed-by: Miquel Raynal <miquel.raynal@bootlin.com>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> Signed-off-by: Fabio Estevam <festevam@gmail.com>

I gave it a test on a Phytec phyCARD board, this fixes it.

Tested-by: Sascha Hauer <s.hauer@pengutronix.de>

Sascha

> ---
> Changes since v2:
> - Remove pdata check in attach() and let the core determine the engine type
> 
>  drivers/mtd/nand/raw/mxc_nand.c | 17 +++++------------
>  1 file changed, 5 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/mxc_nand.c b/drivers/mtd/nand/raw/mxc_nand.c
> index d4200eb2ad32..684c51e5e60d 100644
> --- a/drivers/mtd/nand/raw/mxc_nand.c
> +++ b/drivers/mtd/nand/raw/mxc_nand.c
> @@ -1681,6 +1681,11 @@ static int mxcnd_attach_chip(struct nand_chip *chip)
>  	struct mxc_nand_host *host = nand_get_controller_data(chip);
>  	struct device *dev = mtd->dev.parent;
>  
> +	chip->ecc.bytes = host->devtype_data->eccbytes;
> +	host->eccsize = host->devtype_data->eccsize;
> +	chip->ecc.size = 512;
> +	mtd_set_ooblayout(mtd, host->devtype_data->ooblayout);
> +
>  	switch (chip->ecc.engine_type) {
>  	case NAND_ECC_ENGINE_TYPE_ON_HOST:
>  		chip->ecc.read_page = mxc_nand_read_page;
> @@ -1836,19 +1841,7 @@ static int mxcnd_probe(struct platform_device *pdev)
>  	if (host->devtype_data->axi_offset)
>  		host->regs_axi = host->base + host->devtype_data->axi_offset;
>  
> -	this->ecc.bytes = host->devtype_data->eccbytes;
> -	host->eccsize = host->devtype_data->eccsize;
> -
>  	this->legacy.select_chip = host->devtype_data->select_chip;
> -	this->ecc.size = 512;
> -	mtd_set_ooblayout(mtd, host->devtype_data->ooblayout);
> -
> -	if (host->pdata.hw_ecc) {
> -		this->ecc.engine_type = NAND_ECC_ENGINE_TYPE_ON_HOST;
> -	} else {
> -		this->ecc.engine_type = NAND_ECC_ENGINE_TYPE_SOFT;
> -		this->ecc.algo = NAND_ECC_ALGO_HAMMING;
> -	}
>  
>  	/* NAND bus width determines access functions used by upper layer */
>  	if (host->pdata.width == 2)
> -- 
> 2.17.1
> 
> 
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
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/

Re: [PATCH v3] mtd: rawnand: mxc: Move the ECC engine initialization to the right place

[Martin Kaiser]

Thus wrote Sascha Hauer (s.hauer@pengutronix.de):

> On Fri, Oct 16, 2020 at 06:36:13PM -0300, Fabio Estevam wrote:
> > No ECC initialization should happen during the host controller probe.

> > In fact, we need the probe function to call nand_scan() in order to:

> > - identify the device, its capabilities and constraints (nand_scan_ident())
> > - configure the ECC engine accordingly (->attach_chip())
> > - scan its content and prepare the core (nand_scan_tail())

> > Moving these lines to mxcnd_attach_chip() fixes a regression caused by
> > a previous commit supposed to clarify these steps.

> > When moving the ECC initialization from probe() to attach(), get rid
> > of the pdata usage to determine the engine type and let the core decide
> > instead.

> > Tested on a imx27-pdk board.

> > Reported-by: Fabio Estevam <festevam@gmail.com>
> > Co-developed-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > Signed-off-by: Fabio Estevam <festevam@gmail.com>

> I gave it a test on a Phytec phyCARD board, this fixes it.

> Tested-by: Sascha Hauer <s.hauer@pengutronix.de>

same here on my imx25 system with this flash chip

[    2.044024] nand: Toshiba NAND 256MiB 1,8V 8-bit
[    2.048721] nand: 256 MiB, SLC, erase size: 128 KiB, page size: 2048, OOB size: 128

Tested-by: Martin Kaiser <martin@kaiser.cx>

I would be helptful if the fix showed up in linux-next soon...

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

Re: [PATCH v3] mtd: rawnand: mxc: Move the ECC engine initialization to the right place

[Miquel Raynal]

On Fri, 2020-10-16 at 21:36:13 UTC, Fabio Estevam wrote:
> No ECC initialization should happen during the host controller probe.
> 
> In fact, we need the probe function to call nand_scan() in order to:
> 
> - identify the device, its capabilities and constraints (nand_scan_ident())
> - configure the ECC engine accordingly (->attach_chip())
> - scan its content and prepare the core (nand_scan_tail())
> 
> Moving these lines to mxcnd_attach_chip() fixes a regression caused by
> a previous commit supposed to clarify these steps.
> 
> When moving the ECC initialization from probe() to attach(), get rid
> of the pdata usage to determine the engine type and let the core decide
> instead.
> 
> Tested on a imx27-pdk board.
> 
> Reported-by: Fabio Estevam <festevam@gmail.com>
> Co-developed-by: Miquel Raynal <miquel.raynal@bootlin.com>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> Signed-off-by: Fabio Estevam <festevam@gmail.com>
> Tested-by: Sascha Hauer <s.hauer@pengutronix.de>
> Tested-by: Martin Kaiser <martin@kaiser.cx>

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git mtd/fixes, thanks.

Miquel

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

Re: [PATCH v3] mtd: rawnand: mxc: Move the ECC engine initialization to the right place

[Miquel Raynal]

Hello,

Miquel Raynal <miquel.raynal@bootlin.com> wrote on Mon, 26 Oct 2020
18:45:32 +0100:

> On Fri, 2020-10-16 at 21:36:13 UTC, Fabio Estevam wrote:
> > No ECC initialization should happen during the host controller probe.
> > 
> > In fact, we need the probe function to call nand_scan() in order to:
> > 
> > - identify the device, its capabilities and constraints (nand_scan_ident())
> > - configure the ECC engine accordingly (->attach_chip())
> > - scan its content and prepare the core (nand_scan_tail())
> > 
> > Moving these lines to mxcnd_attach_chip() fixes a regression caused by
> > a previous commit supposed to clarify these steps.
> > 
> > When moving the ECC initialization from probe() to attach(), get rid
> > of the pdata usage to determine the engine type and let the core decide
> > instead.
> > 
> > Tested on a imx27-pdk board.
> > 
> > Reported-by: Fabio Estevam <festevam@gmail.com>
> > Co-developed-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > Signed-off-by: Fabio Estevam <festevam@gmail.com>
> > Tested-by: Sascha Hauer <s.hauer@pengutronix.de>
> > Tested-by: Martin Kaiser <martin@kaiser.cx>  

This patch should be in next by tomorrow+.

I moved my SoB to the bottom as Fabio is now the author of the patch
and added the Fixes tag.

Cheers,
Miquèl

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