[PATCH 1/2] nand: nand-mxs: Fix marking BBT blocks as bad

[PATCH 1/2] nand: nand-mxs: Fix marking BBT blocks as bad

[Stefan Riedmueller]

Currently the nand-mxs driver uses a hook function to the
mtd->_block_markbad function to allow write access to the OOB bytes only
if it is to mark a block as bad.

This hook is not called when a Bad Block Table block is marked bad since
this routine directly calls nand_markbad_bbm.

The chip->legacy.block_markbad hook gives a driver the ability to
implement a custom block_markbad function. Since this is used by
nand_markbad_bbm, if it exists, replace the mtd->_block_markbad hook by
a chip->legacy.block_markbad function. The gpmi-nand Linux implementation
of this driver uses the same mechanism.

This fixes an issue where marking Bad Block Table blocks as bad fails
with:
	NXS NAND: Writing OOB isn't supported

Tested on PHYTEC phyCORE-i.MX 6Q.

Signed-off-by: Stefan Riedmueller <s.riedmueller@phytec.de>
---
 drivers/mtd/nand/nand_mxs.c | 87 +++++++++++++++----------------------
 1 file changed, 34 insertions(+), 53 deletions(-)

diff --git a/drivers/mtd/nand/nand_mxs.c b/drivers/mtd/nand/nand_mxs.c
index 434da49d3ea9..96ae71364efb 100644
--- a/drivers/mtd/nand/nand_mxs.c
+++ b/drivers/mtd/nand/nand_mxs.c
@@ -215,7 +215,6 @@ struct mxs_nand_info {
 	uint8_t		*data_buf;
 	uint8_t		*oob_buf;
 
-	uint8_t		marking_block_bad;
 	uint8_t		raw_oob_mode;
 
 	/* Functions with altered behaviour */
@@ -223,8 +222,6 @@ struct mxs_nand_info {
 				loff_t from, struct mtd_oob_ops *ops);
 	int		(*hooked_write_oob)(struct mtd_info *mtd,
 				loff_t to, struct mtd_oob_ops *ops);
-	int		(*hooked_block_markbad)(struct mtd_info *mtd,
-				loff_t ofs);
 
 	/* DMA descriptors */
 	struct mxs_dma_desc	**desc;
@@ -1073,27 +1070,6 @@ static int mxs_nand_hook_write_oob(struct mtd_info *mtd, loff_t to,
 	return ret;
 }
 
-/*
- * Mark a block bad in NAND.
- *
- * This function is a veneer that replaces the function originally installed by
- * the NAND Flash MTD code.
- */
-static int mxs_nand_hook_block_markbad(struct mtd_info *mtd, loff_t ofs)
-{
-	struct nand_chip *chip = mtd_to_nand(mtd);
-	struct mxs_nand_info *nand_info = chip->priv;
-	int ret;
-
-	nand_info->marking_block_bad = 1;
-
-	ret = nand_info->hooked_block_markbad(mtd, ofs);
-
-	nand_info->marking_block_bad = 0;
-
-	return ret;
-}
-
 /*
  * There are several places in this driver where we have to handle the OOB and
  * block marks. This is the function where things are the most complicated, so
@@ -1177,36 +1153,14 @@ static int mxs_nand_ecc_read_oob(struct nand_chip *chip, int page)
  */
 static int mxs_nand_ecc_write_oob(struct nand_chip *chip, int page)
 {
-	struct mtd_info *mtd = nand_to_mtd(chip);
-	struct mxs_nand_info *nand_info = chip->priv;
-	int column;
-	uint8_t block_mark = 0;
-
 	/*
 	 * There are fundamental incompatibilities between the i.MX GPMI NFC and
 	 * the NAND Flash MTD model that make it essentially impossible to write
 	 * the out-of-band bytes.
-	 *
-	 * We permit *ONE* exception. If the *intent* of writing the OOB is to
-	 * mark a block bad, we can do that.
 	 */
 
-	if (!nand_info->marking_block_bad) {
-		printf("NXS NAND: Writing OOB isn't supported\n");
-		return -EIO;
-	}
-
-	column = nand_info->version == GPMI_VERSION_TYPE_MX23 ? 0 : mtd->writesize;
-	/* Write the block mark. */
-	chip->legacy.cmdfunc(chip, NAND_CMD_SEQIN, column, page);
-	chip->legacy.write_buf(chip, &block_mark, 1);
-	chip->legacy.cmdfunc(chip, NAND_CMD_PAGEPROG, -1, -1);
-
-	/* Check if it worked. */
-	if (chip->legacy.waitfunc(chip) & NAND_STATUS_FAIL)
-		return -EIO;
-
-	return 0;
+	printf("MXS NAND: Writing OOB isn't supported\n");
+	return -EIO;
 }
 
 /*
@@ -1227,6 +1181,37 @@ static int mxs_nand_block_bad(struct nand_chip *chip , loff_t ofs)
 	return 0;
 }
 
+/*
+ * Mark a block as bad in NAND.
+ */
+static int mxs_nand_block_markbad(struct nand_chip *chip , loff_t ofs)
+{
+	struct mtd_info *mtd = nand_to_mtd(chip);
+	struct mxs_nand_info *nand_info = chip->priv;
+	int column, page, chipnr, status;
+	uint8_t block_mark = 0;
+
+	chipnr = (int)(ofs >> chip->chip_shift);
+	nand_select_target(chip, chipnr);
+
+	column = nand_info->version == GPMI_VERSION_TYPE_MX23 ? 0 : mtd->writesize;
+	page = (int)(ofs >> chip->page_shift);
+	/* Write the block mark. */
+	chip->legacy.cmdfunc(chip, NAND_CMD_SEQIN, column, page);
+	chip->legacy.write_buf(chip, &block_mark, 1);
+	chip->legacy.cmdfunc(chip, NAND_CMD_PAGEPROG, -1, -1);
+
+	/* Check if it worked. */
+	status = chip->legacy.waitfunc(chip);
+
+	nand_deselect_target(chip);
+
+	if (status & NAND_STATUS_FAIL)
+		return -EIO;
+
+	return 0;
+}
+
 /*
  * Nominally, the purpose of this function is to look for or create the bad
  * block table. In fact, since the we call this function at the very end of
@@ -1273,11 +1258,6 @@ static int mxs_nand_scan_bbt(struct nand_chip *chip)
 		mtd->_write_oob = mxs_nand_hook_write_oob;
 	}
 
-	if (mtd->_block_markbad != mxs_nand_hook_block_markbad) {
-		nand_info->hooked_block_markbad = mtd->_block_markbad;
-		mtd->_block_markbad = mxs_nand_hook_block_markbad;
-	}
-
 	/* We use the reference implementation for bad block management. */
 	return nand_create_bbt(chip);
 }
@@ -2201,6 +2181,7 @@ static int mxs_nand_probe(struct device_d *dev)
 	chip->legacy.dev_ready		= mxs_nand_device_ready;
 	chip->legacy.select_chip	= mxs_nand_select_chip;
 	chip->legacy.block_bad		= mxs_nand_block_bad;
+	chip->legacy.block_markbad	= mxs_nand_block_markbad;
 
 	chip->legacy.read_byte		= mxs_nand_read_byte;
 
-- 
2.25.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 2/2] mtd: nand_bbt: Skip bad blocks when searching for the BBT in NAND

[Stefan Riedmueller]

The blocks containing the bad block table can become bad as well. So
make sure to skip any blocks that are marked bad when searching for the
bad block table.

Signed-off-by: Stefan Riedmueller <s.riedmueller@phytec.de>
---
 drivers/mtd/nand/nand_bbt.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/mtd/nand/nand_bbt.c b/drivers/mtd/nand/nand_bbt.c
index a86b5b2da38e..0b48373e6a0b 100644
--- a/drivers/mtd/nand/nand_bbt.c
+++ b/drivers/mtd/nand/nand_bbt.c
@@ -528,6 +528,7 @@ static int search_bbt(struct nand_chip *this, uint8_t *buf,
 {
 	u64 targetsize = nanddev_target_size(&this->base);
 	struct mtd_info *mtd = nand_to_mtd(this);
+	struct nand_bbt_descr *bd = this->badblock_pattern;
 	int i, chips;
 	int startblock, block, dir;
 	int scanlen = mtd->writesize + mtd->oobsize;
@@ -563,6 +564,9 @@ static int search_bbt(struct nand_chip *this, uint8_t *buf,
 			int actblock = startblock + dir * block;
 			loff_t offs = (loff_t)actblock << this->bbt_erase_shift;
 
+			/* Check if block is marked bad */
+			if (scan_block_fast(this, bd, offs, buf))
+				continue;
 			/* Read first page */
 			scan_read(this, buf, offs, mtd->writesize, td);
 			if (!check_pattern(buf, scanlen, mtd->writesize, td)) {
-- 
2.25.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 2/2] mtd: nand_bbt: Skip bad blocks when searching for the BBT in NAND

[Sascha Hauer]

Hi Stefan,

On Tue, Mar 16, 2021 at 01:36:26PM +0100, Stefan Riedmueller wrote:
> The blocks containing the bad block table can become bad as well. So
> make sure to skip any blocks that are marked bad when searching for the
> bad block table.

The file is a more or less direct copy from the kernel. Does the problem
exist in the kernel as well?

Sascha

> 
> Signed-off-by: Stefan Riedmueller <s.riedmueller@phytec.de>
> ---
>  drivers/mtd/nand/nand_bbt.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/mtd/nand/nand_bbt.c b/drivers/mtd/nand/nand_bbt.c
> index a86b5b2da38e..0b48373e6a0b 100644
> --- a/drivers/mtd/nand/nand_bbt.c
> +++ b/drivers/mtd/nand/nand_bbt.c
> @@ -528,6 +528,7 @@ static int search_bbt(struct nand_chip *this, uint8_t *buf,
>  {
>  	u64 targetsize = nanddev_target_size(&this->base);
>  	struct mtd_info *mtd = nand_to_mtd(this);
> +	struct nand_bbt_descr *bd = this->badblock_pattern;
>  	int i, chips;
>  	int startblock, block, dir;
>  	int scanlen = mtd->writesize + mtd->oobsize;
> @@ -563,6 +564,9 @@ static int search_bbt(struct nand_chip *this, uint8_t *buf,
>  			int actblock = startblock + dir * block;
>  			loff_t offs = (loff_t)actblock << this->bbt_erase_shift;
>  
> +			/* Check if block is marked bad */
> +			if (scan_block_fast(this, bd, offs, buf))
> +				continue;
>  			/* Read first page */
>  			scan_read(this, buf, offs, mtd->writesize, td);
>  			if (!check_pattern(buf, scanlen, mtd->writesize, td)) {
> -- 
> 2.25.1
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox
> 

-- 
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 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 2/2] mtd: nand_bbt: Skip bad blocks when searching for the BBT in NAND

[Stefan Riedmüller]

Hi Sascha,

On Wed, 2021-03-17 at 10:41 +0100, Sascha Hauer wrote:
> Hi Stefan,
> 
> On Tue, Mar 16, 2021 at 01:36:26PM +0100, Stefan Riedmueller wrote:
> > The blocks containing the bad block table can become bad as well. So
> > make sure to skip any blocks that are marked bad when searching for the
> > bad block table.
> 
> The file is a more or less direct copy from the kernel. Does the problem
> exist in the kernel as well?

>From my point of view I would say yes. I have just made a small test Issue is
shown in the kernel as well. It is a very raw case where two BBT blocks need
to be worn out but on the other hand this fix should not have that much of an
impact.

I will send a similar patch to the kernel.

Stefan

> 
> Sascha
> 
> > Signed-off-by: Stefan Riedmueller <s.riedmueller@phytec.de>
> > ---
> >  drivers/mtd/nand/nand_bbt.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/drivers/mtd/nand/nand_bbt.c b/drivers/mtd/nand/nand_bbt.c
> > index a86b5b2da38e..0b48373e6a0b 100644
> > --- a/drivers/mtd/nand/nand_bbt.c
> > +++ b/drivers/mtd/nand/nand_bbt.c
> > @@ -528,6 +528,7 @@ static int search_bbt(struct nand_chip *this, uint8_t
> > *buf,
> >  {
> >  	u64 targetsize = nanddev_target_size(&this->base);
> >  	struct mtd_info *mtd = nand_to_mtd(this);
> > +	struct nand_bbt_descr *bd = this->badblock_pattern;
> >  	int i, chips;
> >  	int startblock, block, dir;
> >  	int scanlen = mtd->writesize + mtd->oobsize;
> > @@ -563,6 +564,9 @@ static int search_bbt(struct nand_chip *this, uint8_t
> > *buf,
> >  			int actblock = startblock + dir * block;
> >  			loff_t offs = (loff_t)actblock << this-
> > >bbt_erase_shift;
> >  
> > +			/* Check if block is marked bad */
> > +			if (scan_block_fast(this, bd, offs, buf))
> > +				continue;
> >  			/* Read first page */
> >  			scan_read(this, buf, offs, mtd->writesize, td);
> >  			if (!check_pattern(buf, scanlen, mtd->writesize, td))
> > {
> > -- 
> > 2.25.1
> > 
> > 
> > _______________________________________________
> > barebox mailing list
> > barebox@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/barebox
> > 
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 2/2] mtd: nand_bbt: Skip bad blocks when searching for the BBT in NAND

[sha]

On Wed, Mar 17, 2021 at 05:14:05PM +0000, Stefan Riedmüller wrote:
> Hi Sascha,
> 
> On Wed, 2021-03-17 at 10:41 +0100, Sascha Hauer wrote:
> > Hi Stefan,
> > 
> > On Tue, Mar 16, 2021 at 01:36:26PM +0100, Stefan Riedmueller wrote:
> > > The blocks containing the bad block table can become bad as well. So
> > > make sure to skip any blocks that are marked bad when searching for the
> > > bad block table.
> > 
> > The file is a more or less direct copy from the kernel. Does the problem
> > exist in the kernel as well?
> 
> From my point of view I would say yes. I have just made a small test Issue is
> shown in the kernel as well. It is a very raw case where two BBT blocks need
> to be worn out but on the other hand this fix should not have that much of an
> impact.
> 
> I will send a similar patch to the kernel.

I'd like to delay this one until it is accepted in the kernel. I do not
fully understand the consequences of this patch, but hopefully on the
mtd list there is someone.

Sascha

-- 
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 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 2/2] mtd: nand_bbt: Skip bad blocks when searching for the BBT in NAND

[Stefan Riedmüller]

Hi Sascha,

here is the link to the kernel patch.
Sorry, I forgot to put you in cc.

https://patchwork.ozlabs.org/project/linux-mtd/patch/20210325102337.481172-1-s.riedmueller@phytec.de/

Stefan

On Mon, 2021-03-22 at 06:59 +0100, sha@pengutronix.de wrote:
> On Wed, Mar 17, 2021 at 05:14:05PM +0000, Stefan Riedmüller wrote:
> > Hi Sascha,
> > 
> > On Wed, 2021-03-17 at 10:41 +0100, Sascha Hauer wrote:
> > > Hi Stefan,
> > > 
> > > On Tue, Mar 16, 2021 at 01:36:26PM +0100, Stefan Riedmueller wrote:
> > > > The blocks containing the bad block table can become bad as well. So
> > > > make sure to skip any blocks that are marked bad when searching for
> > > > the
> > > > bad block table.
> > > 
> > > The file is a more or less direct copy from the kernel. Does the problem
> > > exist in the kernel as well?
> > 
> > From my point of view I would say yes. I have just made a small test Issue
> > is
> > shown in the kernel as well. It is a very raw case where two BBT blocks
> > need
> > to be worn out but on the other hand this fix should not have that much of
> > an
> > impact.
> > 
> > I will send a similar patch to the kernel.
> 
> I'd like to delay this one until it is accepted in the kernel. I do not
> fully understand the consequences of this patch, but hopefully on the
> mtd list there is someone.
> 
> Sascha
> 
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 2/2] mtd: nand_bbt: Skip bad blocks when searching for the BBT in NAND

[Stefan Riedmüller]

Hi Sascha,

just wanted to let you know that the kernel patch has been accepted and is
included in the pull request for 5.13.

https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git/log/?h=nand/for-5.13

Regards,
Stefan

On Mon, 2021-03-29 at 07:20 +0000, Stefan Riedmüller wrote:
> Hi Sascha,
> 
> here is the link to the kernel patch.
> Sorry, I forgot to put you in cc.
> 
> https://patchwork.ozlabs.org/project/linux-mtd/patch/20210325102337.481172-1-s.riedmueller@phytec.de/
> 
> Stefan
> 
> On Mon, 2021-03-22 at 06:59 +0100, sha@pengutronix.de wrote:
> > On Wed, Mar 17, 2021 at 05:14:05PM +0000, Stefan Riedmüller wrote:
> > > Hi Sascha,
> > > 
> > > On Wed, 2021-03-17 at 10:41 +0100, Sascha Hauer wrote:
> > > > Hi Stefan,
> > > > 
> > > > On Tue, Mar 16, 2021 at 01:36:26PM +0100, Stefan Riedmueller wrote:
> > > > > The blocks containing the bad block table can become bad as well. So
> > > > > make sure to skip any blocks that are marked bad when searching for
> > > > > the
> > > > > bad block table.
> > > > 
> > > > The file is a more or less direct copy from the kernel. Does the
> > > > problem
> > > > exist in the kernel as well?
> > > 
> > > From my point of view I would say yes. I have just made a small test
> > > Issue
> > > is
> > > shown in the kernel as well. It is a very raw case where two BBT blocks
> > > need
> > > to be worn out but on the other hand this fix should not have that much
> > > of
> > > an
> > > impact.
> > > 
> > > I will send a similar patch to the kernel.
> > 
> > I'd like to delay this one until it is accepted in the kernel. I do not
> > fully understand the consequences of this patch, but hopefully on the
> > mtd list there is someone.
> > 
> > Sascha
> > 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 2/2] mtd: nand_bbt: Skip bad blocks when searching for the BBT in NAND

[sha]

0.08

Hi Stefan,

On Wed, Apr 14, 2021 at 01:18:39PM +0000, Stefan Riedmüller wrote:
> Hi Sascha,
> 
> just wanted to let you know that the kernel patch has been accepted and is
> included in the pull request for 5.13.

Thanks for the effort. I just merged it into barebox.

Sascha


> 
> https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git/log/?h=nand/for-5.13
> 
> Regards,
> Stefan
> 
> On Mon, 2021-03-29 at 07:20 +0000, Stefan Riedmüller wrote:
> > Hi Sascha,
> > 
> > here is the link to the kernel patch.
> > Sorry, I forgot to put you in cc.
> > 
> > https://patchwork.ozlabs.org/project/linux-mtd/patch/20210325102337.481172-1-s.riedmueller@phytec.de/
> > 
> > Stefan
> > 
> > On Mon, 2021-03-22 at 06:59 +0100, sha@pengutronix.de wrote:
> > > On Wed, Mar 17, 2021 at 05:14:05PM +0000, Stefan Riedmüller wrote:
> > > > Hi Sascha,
> > > > 
> > > > On Wed, 2021-03-17 at 10:41 +0100, Sascha Hauer wrote:
> > > > > Hi Stefan,
> > > > > 
> > > > > On Tue, Mar 16, 2021 at 01:36:26PM +0100, Stefan Riedmueller wrote:
> > > > > > The blocks containing the bad block table can become bad as well. So
> > > > > > make sure to skip any blocks that are marked bad when searching for
> > > > > > the
> > > > > > bad block table.
> > > > > 
> > > > > The file is a more or less direct copy from the kernel. Does the
> > > > > problem
> > > > > exist in the kernel as well?
> > > > 
> > > > From my point of view I would say yes. I have just made a small test
> > > > Issue
> > > > is
> > > > shown in the kernel as well. It is a very raw case where two BBT blocks
> > > > need
> > > > to be worn out but on the other hand this fix should not have that much
> > > > of
> > > > an
> > > > impact.
> > > > 
> > > > I will send a similar patch to the kernel.
> > > 
> > > I'd like to delay this one until it is accepted in the kernel. I do not
> > > fully understand the consequences of this patch, but hopefully on the
> > > mtd list there is someone.
> > > 
> > > Sascha
> > > 
> > _______________________________________________
> > barebox mailing list
> > barebox@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/barebox

-- 
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 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 2/2] mtd: nand_bbt: Skip bad blocks when searching for the BBT in NAND

[Stefan Riedmüller]

Hi Sascha,

unfortunately this Patch has caused a regression on imx27 in Linux and will be
reverted for now. I would suggest to do the same here until a proper solution
is found.

http://lists.infradead.org/pipermail/linux-mtd/2021-April/086124.html

Sorry for the trouble!

Regards,
Stefan


On Wed, 2021-04-14 at 16:36 +0200, sha@pengutronix.de wrote:
> 0.08
> 
> Hi Stefan,
> 
> On Wed, Apr 14, 2021 at 01:18:39PM +0000, Stefan Riedmüller wrote:
> > Hi Sascha,
> > 
> > just wanted to let you know that the kernel patch has been accepted and is
> > included in the pull request for 5.13.
> 
> Thanks for the effort. I just merged it into barebox.
> 
> Sascha
> 
> 
> > https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git/log/?h=nand/for-5.13
> > 
> > Regards,
> > Stefan
> > 
> > On Mon, 2021-03-29 at 07:20 +0000, Stefan Riedmüller wrote:
> > > Hi Sascha,
> > > 
> > > here is the link to the kernel patch.
> > > Sorry, I forgot to put you in cc.
> > > 
> > > https://patchwork.ozlabs.org/project/linux-mtd/patch/20210325102337.481172-1-s.riedmueller@phytec.de/
> > > 
> > > Stefan
> > > 
> > > On Mon, 2021-03-22 at 06:59 +0100, sha@pengutronix.de wrote:
> > > > On Wed, Mar 17, 2021 at 05:14:05PM +0000, Stefan Riedmüller wrote:
> > > > > Hi Sascha,
> > > > > 
> > > > > On Wed, 2021-03-17 at 10:41 +0100, Sascha Hauer wrote:
> > > > > > Hi Stefan,
> > > > > > 
> > > > > > On Tue, Mar 16, 2021 at 01:36:26PM +0100, Stefan Riedmueller
> > > > > > wrote:
> > > > > > > The blocks containing the bad block table can become bad as
> > > > > > > well. So
> > > > > > > make sure to skip any blocks that are marked bad when searching
> > > > > > > for
> > > > > > > the
> > > > > > > bad block table.
> > > > > > 
> > > > > > The file is a more or less direct copy from the kernel. Does the
> > > > > > problem
> > > > > > exist in the kernel as well?
> > > > > 
> > > > > From my point of view I would say yes. I have just made a small test
> > > > > Issue
> > > > > is
> > > > > shown in the kernel as well. It is a very raw case where two BBT
> > > > > blocks
> > > > > need
> > > > > to be worn out but on the other hand this fix should not have that
> > > > > much
> > > > > of
> > > > > an
> > > > > impact.
> > > > > 
> > > > > I will send a similar patch to the kernel.
> > > > 
> > > > I'd like to delay this one until it is accepted in the kernel. I do
> > > > not
> > > > fully understand the consequences of this patch, but hopefully on the
> > > > mtd list there is someone.
> > > > 
> > > > Sascha
> > > > 
> > > _______________________________________________
> > > barebox mailing list
> > > barebox@lists.infradead.org
> > > http://lists.infradead.org/mailman/listinfo/barebox
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 2/2] mtd: nand_bbt: Skip bad blocks when searching for the BBT in NAND

[sha]

On Tue, Apr 20, 2021 at 06:29:47AM +0000, Stefan Riedmüller wrote:
> Hi Sascha,
> 
> unfortunately this Patch has caused a regression on imx27 in Linux and will be
> reverted for now. I would suggest to do the same here until a proper solution
> is found.

Did that.

> 
> http://lists.infradead.org/pipermail/linux-mtd/2021-April/086124.html
> 
> Sorry for the trouble!

Don't worry. This is what testing is good for, so let's be glad it was
caught.

Sascha

-- 
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 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 1/2] nand: nand-mxs: Fix marking BBT blocks as bad

[Sascha Hauer]

On Tue, Mar 16, 2021 at 01:36:25PM +0100, Stefan Riedmueller wrote:
> Currently the nand-mxs driver uses a hook function to the
> mtd->_block_markbad function to allow write access to the OOB bytes only
> if it is to mark a block as bad.
> 
> This hook is not called when a Bad Block Table block is marked bad since
> this routine directly calls nand_markbad_bbm.
> 
> The chip->legacy.block_markbad hook gives a driver the ability to
> implement a custom block_markbad function. Since this is used by
> nand_markbad_bbm, if it exists, replace the mtd->_block_markbad hook by
> a chip->legacy.block_markbad function. The gpmi-nand Linux implementation
> of this driver uses the same mechanism.
> 
> This fixes an issue where marking Bad Block Table blocks as bad fails
> with:
> 	NXS NAND: Writing OOB isn't supported
> 
> Tested on PHYTEC phyCORE-i.MX 6Q.
> 
> Signed-off-by: Stefan Riedmueller <s.riedmueller@phytec.de>
> ---
>  drivers/mtd/nand/nand_mxs.c | 87 +++++++++++++++----------------------
>  1 file changed, 34 insertions(+), 53 deletions(-)

Applied this one, thanks

Sascha

> 
> diff --git a/drivers/mtd/nand/nand_mxs.c b/drivers/mtd/nand/nand_mxs.c
> index 434da49d3ea9..96ae71364efb 100644
> --- a/drivers/mtd/nand/nand_mxs.c
> +++ b/drivers/mtd/nand/nand_mxs.c
> @@ -215,7 +215,6 @@ struct mxs_nand_info {
>  	uint8_t		*data_buf;
>  	uint8_t		*oob_buf;
>  
> -	uint8_t		marking_block_bad;
>  	uint8_t		raw_oob_mode;
>  
>  	/* Functions with altered behaviour */
> @@ -223,8 +222,6 @@ struct mxs_nand_info {
>  				loff_t from, struct mtd_oob_ops *ops);
>  	int		(*hooked_write_oob)(struct mtd_info *mtd,
>  				loff_t to, struct mtd_oob_ops *ops);
> -	int		(*hooked_block_markbad)(struct mtd_info *mtd,
> -				loff_t ofs);
>  
>  	/* DMA descriptors */
>  	struct mxs_dma_desc	**desc;
> @@ -1073,27 +1070,6 @@ static int mxs_nand_hook_write_oob(struct mtd_info *mtd, loff_t to,
>  	return ret;
>  }
>  
> -/*
> - * Mark a block bad in NAND.
> - *
> - * This function is a veneer that replaces the function originally installed by
> - * the NAND Flash MTD code.
> - */
> -static int mxs_nand_hook_block_markbad(struct mtd_info *mtd, loff_t ofs)
> -{
> -	struct nand_chip *chip = mtd_to_nand(mtd);
> -	struct mxs_nand_info *nand_info = chip->priv;
> -	int ret;
> -
> -	nand_info->marking_block_bad = 1;
> -
> -	ret = nand_info->hooked_block_markbad(mtd, ofs);
> -
> -	nand_info->marking_block_bad = 0;
> -
> -	return ret;
> -}
> -
>  /*
>   * There are several places in this driver where we have to handle the OOB and
>   * block marks. This is the function where things are the most complicated, so
> @@ -1177,36 +1153,14 @@ static int mxs_nand_ecc_read_oob(struct nand_chip *chip, int page)
>   */
>  static int mxs_nand_ecc_write_oob(struct nand_chip *chip, int page)
>  {
> -	struct mtd_info *mtd = nand_to_mtd(chip);
> -	struct mxs_nand_info *nand_info = chip->priv;
> -	int column;
> -	uint8_t block_mark = 0;
> -
>  	/*
>  	 * There are fundamental incompatibilities between the i.MX GPMI NFC and
>  	 * the NAND Flash MTD model that make it essentially impossible to write
>  	 * the out-of-band bytes.
> -	 *
> -	 * We permit *ONE* exception. If the *intent* of writing the OOB is to
> -	 * mark a block bad, we can do that.
>  	 */
>  
> -	if (!nand_info->marking_block_bad) {
> -		printf("NXS NAND: Writing OOB isn't supported\n");
> -		return -EIO;
> -	}
> -
> -	column = nand_info->version == GPMI_VERSION_TYPE_MX23 ? 0 : mtd->writesize;
> -	/* Write the block mark. */
> -	chip->legacy.cmdfunc(chip, NAND_CMD_SEQIN, column, page);
> -	chip->legacy.write_buf(chip, &block_mark, 1);
> -	chip->legacy.cmdfunc(chip, NAND_CMD_PAGEPROG, -1, -1);
> -
> -	/* Check if it worked. */
> -	if (chip->legacy.waitfunc(chip) & NAND_STATUS_FAIL)
> -		return -EIO;
> -
> -	return 0;
> +	printf("MXS NAND: Writing OOB isn't supported\n");
> +	return -EIO;
>  }
>  
>  /*
> @@ -1227,6 +1181,37 @@ static int mxs_nand_block_bad(struct nand_chip *chip , loff_t ofs)
>  	return 0;
>  }
>  
> +/*
> + * Mark a block as bad in NAND.
> + */
> +static int mxs_nand_block_markbad(struct nand_chip *chip , loff_t ofs)
> +{
> +	struct mtd_info *mtd = nand_to_mtd(chip);
> +	struct mxs_nand_info *nand_info = chip->priv;
> +	int column, page, chipnr, status;
> +	uint8_t block_mark = 0;
> +
> +	chipnr = (int)(ofs >> chip->chip_shift);
> +	nand_select_target(chip, chipnr);
> +
> +	column = nand_info->version == GPMI_VERSION_TYPE_MX23 ? 0 : mtd->writesize;
> +	page = (int)(ofs >> chip->page_shift);
> +	/* Write the block mark. */
> +	chip->legacy.cmdfunc(chip, NAND_CMD_SEQIN, column, page);
> +	chip->legacy.write_buf(chip, &block_mark, 1);
> +	chip->legacy.cmdfunc(chip, NAND_CMD_PAGEPROG, -1, -1);
> +
> +	/* Check if it worked. */
> +	status = chip->legacy.waitfunc(chip);
> +
> +	nand_deselect_target(chip);
> +
> +	if (status & NAND_STATUS_FAIL)
> +		return -EIO;
> +
> +	return 0;
> +}
> +
>  /*
>   * Nominally, the purpose of this function is to look for or create the bad
>   * block table. In fact, since the we call this function at the very end of
> @@ -1273,11 +1258,6 @@ static int mxs_nand_scan_bbt(struct nand_chip *chip)
>  		mtd->_write_oob = mxs_nand_hook_write_oob;
>  	}
>  
> -	if (mtd->_block_markbad != mxs_nand_hook_block_markbad) {
> -		nand_info->hooked_block_markbad = mtd->_block_markbad;
> -		mtd->_block_markbad = mxs_nand_hook_block_markbad;
> -	}
> -
>  	/* We use the reference implementation for bad block management. */
>  	return nand_create_bbt(chip);
>  }
> @@ -2201,6 +2181,7 @@ static int mxs_nand_probe(struct device_d *dev)
>  	chip->legacy.dev_ready		= mxs_nand_device_ready;
>  	chip->legacy.select_chip	= mxs_nand_select_chip;
>  	chip->legacy.block_bad		= mxs_nand_block_bad;
> +	chip->legacy.block_markbad	= mxs_nand_block_markbad;
>  
>  	chip->legacy.read_byte		= mxs_nand_read_byte;
>  
> -- 
> 2.25.1
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox
> 

-- 
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 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox