linux-mtd.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3] mtd: rawnand: mxc: Move the ECC engine initialization to the right place
@ 2020-10-16 21:36 Fabio Estevam
  2020-10-18 19:42 ` Fabio Estevam
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Fabio Estevam @ 2020-10-16 21:36 UTC (permalink / raw)
  To: miquel.raynal
  Cc: vigneshr, richard, boris.brezillon, linux-mtd, kernel, han.xu,
	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/

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH v3] mtd: rawnand: mxc: Move the ECC engine initialization to the right place
  2020-10-16 21:36 [PATCH v3] mtd: rawnand: mxc: Move the ECC engine initialization to the right place Fabio Estevam
@ 2020-10-18 19:42 ` Fabio Estevam
  2020-10-19  9:13 ` Sascha Hauer
  2020-10-26 17:45 ` Miquel Raynal
  2 siblings, 0 replies; 6+ messages in thread
From: Fabio Estevam @ 2020-10-18 19:42 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Vignesh Raghavendra, Richard Weinberger, Boris Brezillon,
	linux-mtd, Sascha Hauer, Han Xu

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/

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH v3] mtd: rawnand: mxc: Move the ECC engine initialization to the right place
  2020-10-16 21:36 [PATCH v3] mtd: rawnand: mxc: Move the ECC engine initialization to the right place Fabio Estevam
  2020-10-18 19:42 ` Fabio Estevam
@ 2020-10-19  9:13 ` Sascha Hauer
  2020-10-25 18:01   ` Martin Kaiser
  2020-10-26 17:45 ` Miquel Raynal
  2 siblings, 1 reply; 6+ messages in thread
From: Sascha Hauer @ 2020-10-19  9:13 UTC (permalink / raw)
  To: Fabio Estevam
  Cc: vigneshr, richard, boris.brezillon, linux-mtd, kernel,
	miquel.raynal, han.xu

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/

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH v3] mtd: rawnand: mxc: Move the ECC engine initialization to the right place
  2020-10-19  9:13 ` Sascha Hauer
@ 2020-10-25 18:01   ` Martin Kaiser
  0 siblings, 0 replies; 6+ messages in thread
From: Martin Kaiser @ 2020-10-25 18:01 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: vigneshr, richard, boris.brezillon, linux-mtd, kernel,
	miquel.raynal, han.xu, Fabio Estevam

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/

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH v3] mtd: rawnand: mxc: Move the ECC engine initialization to the right place
  2020-10-16 21:36 [PATCH v3] mtd: rawnand: mxc: Move the ECC engine initialization to the right place Fabio Estevam
  2020-10-18 19:42 ` Fabio Estevam
  2020-10-19  9:13 ` Sascha Hauer
@ 2020-10-26 17:45 ` Miquel Raynal
  2020-10-26 17:49   ` Miquel Raynal
  2 siblings, 1 reply; 6+ messages in thread
From: Miquel Raynal @ 2020-10-26 17:45 UTC (permalink / raw)
  To: Fabio Estevam, miquel.raynal
  Cc: vigneshr, richard, boris.brezillon, linux-mtd, kernel, han.xu

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/

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH v3] mtd: rawnand: mxc: Move the ECC engine initialization to the right place
  2020-10-26 17:45 ` Miquel Raynal
@ 2020-10-26 17:49   ` Miquel Raynal
  0 siblings, 0 replies; 6+ messages in thread
From: Miquel Raynal @ 2020-10-26 17:49 UTC (permalink / raw)
  To: Fabio Estevam, miquel.raynal
  Cc: vigneshr, richard, boris.brezillon, linux-mtd, kernel, han.xu

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/

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-10-26 17:51 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-16 21:36 [PATCH v3] mtd: rawnand: mxc: Move the ECC engine initialization to the right place Fabio Estevam
2020-10-18 19:42 ` Fabio Estevam
2020-10-19  9:13 ` Sascha Hauer
2020-10-25 18:01   ` Martin Kaiser
2020-10-26 17:45 ` Miquel Raynal
2020-10-26 17:49   ` Miquel Raynal

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