All of lore.kernel.org
 help / color / mirror / Atom feed
From: Miquel Raynal <miquel.raynal@bootlin.com>
To: "Stefan Riedmüller" <S.Riedmueller@phytec.de>
Cc: "festevam@gmail.com" <festevam@gmail.com>,
	"guillaume.tucker@collabora.com" <guillaume.tucker@collabora.com>,
	"kernel@pengutronix.de" <kernel@pengutronix.de>,
	"linux-mtd@lists.infradead.org" <linux-mtd@lists.infradead.org>
Subject: Re: imx27: No space left to write bad block table
Date: Mon, 19 Apr 2021 17:36:08 +0200	[thread overview]
Message-ID: <20210419173608.434f4b8d@xps13> (raw)
In-Reply-To: <b6c62efc5b24bc0e04f222095573285f0c977216.camel@phytec.de>

Hi Stefan,

> > Interesting. Maybe I overlooked the below commit when applying. Indeed,
> > BBT may be considered as bad blocks, so I wonder if the below change is
> > valid now...
> > 
> > Guillaume, would you have a way to revert this patch on top of
> > linux-next? Stefan, would you mind giving more details on the testing
> > procedure?  
> 
> I have tested this on an i.MX 6 by simulating two bad BBT blocks by simply
> returning -EIO in nand_erase_nand when the block to be erased is one of the
> first two BBT blocks.
> 
> I have seen this once on a customer board but were not able to reproduce it
> anymore, thus the simulation of the two bad blocks.
> 
> Without the patch below new versions of the BBT can no longer be written to
> the first two blocks reserved for the BBT but they are still evaluated to read
> the BBT from during boot due the lack of a test if these blocks are bad. So
> changes to the BBT after these two blocks turn bad are only kept and used
> until the next reboot where again the old version of the two worn blocks is
> used as a basis.
> 
> I tried to use the same mechanism that is used to identify bad blocks during a
> scan for bad blocks. But maybe I missed something there? Or were my
> assumptions wrong in the first place?

Honestly I don't know what is wrong exactly in this patch.

We will revert the commit as it clearly breaks something fundamental
and the merge window is too close to adopt a hackish attitude.

I would propose the following tests with your board:
- Hack the core to allow yourself to access bad blocks from userspace
  for testing purposes.
- With the below commit, you should have the same behavior than
  reported by Fabio.
- Revert the commit.
- Manually change the bad block markers (nanddump, flash_erase,
  nandwrite) to declare the two tables bad. Reboot and observe if there
  are any issues. You can try to work from there.

> > ---8<---
> > 
> > commit bd9c9fe2ad04546940f4a9979d679e62cae6aa51
> > Author: Stefan Riedmueller <s.riedmueller@phytec.de>
> > Date:   Thu Mar 25 11:23:37 2021 +0100
> > 
> >     mtd: rawnand: bbt: Skip bad blocks when searching for the BBT in NAND
> >     
> >     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.
> >     
> >     Otherwise in very rare cases where two BBT blocks wear out it might
> >     happen that an obsolete BBT is used instead of a newer available
> >     version.
> >     
> >     Signed-off-by: Stefan Riedmueller <s.riedmueller@phytec.de>
> >     Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> >     Link: 
> > https://lore.kernel.org/linux-mtd/20210325102337.481172-1-s.riedmueller@phytec.de
> > 
> > diff --git a/drivers/mtd/nand/raw/nand_bbt.c
> > b/drivers/mtd/nand/raw/nand_bbt.c
> > index dced32a126d9..6e25a5ce5ba9 100644
> > --- a/drivers/mtd/nand/raw/nand_bbt.c
> > +++ b/drivers/mtd/nand/raw/nand_bbt.c
> > @@ -525,6 +525,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;
> > @@ -560,6 +561,10 @@ 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)) {
> > 
> > 
> > Thanks,
> > Miquèl  

Thanks,
Miquèl

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

  reply	other threads:[~2021-04-19 15:37 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-17 15:59 imx27: No space left to write bad block table Fabio Estevam
2021-04-19  6:37 ` Miquel Raynal
2021-04-19 11:47   ` Fabio Estevam
2021-04-19 12:27     ` Miquel Raynal
2021-04-19 12:41       ` Fabio Estevam
2021-04-19 12:48         ` Fabio Estevam
2021-04-19 13:01           ` Fabio Estevam
2021-04-19 13:40           ` Miquel Raynal
2021-04-19 13:56             ` Fabio Estevam
2021-04-19 13:04       ` Stefan Riedmüller
2021-04-19 15:36         ` Miquel Raynal [this message]
2021-04-20  6:26           ` Stefan Riedmüller
2021-04-21 20:44             ` Guillaume Tucker
2021-04-21 23:29               ` Fabio Estevam
2021-04-22 13:16                 ` Guillaume Tucker
2021-04-22 13:28                   ` Fabio Estevam
2021-04-23 21:04                     ` Fabio Estevam
2021-04-26 15:53           ` Stefan Riedmüller
2021-05-04  8:34             ` Miquel Raynal
2021-05-10  8:38               ` Stefan Riedmüller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210419173608.434f4b8d@xps13 \
    --to=miquel.raynal@bootlin.com \
    --cc=S.Riedmueller@phytec.de \
    --cc=festevam@gmail.com \
    --cc=guillaume.tucker@collabora.com \
    --cc=kernel@pengutronix.de \
    --cc=linux-mtd@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.