| Message ID | 20201016213613.1450-1-festevam@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v3] mtd: rawnand: mxc: Move the ECC engine initialization to the right place | expand |
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
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 > > >
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...
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
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
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)