public inbox for linux-mtd@lists.infradead.org
 help / color / mirror / Atom feed
From: "Stefan Riedmüller" <S.Riedmueller@phytec.de>
To: "miquel.raynal@bootlin.com" <miquel.raynal@bootlin.com>
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: Tue, 20 Apr 2021 06:26:05 +0000	[thread overview]
Message-ID: <13112dfe42b99e0c5ef340d9382bb76014bb870c.camel@phytec.de> (raw)
In-Reply-To: <20210419173608.434f4b8d@xps13>

Hi Miquel,

On Mon, 2021-04-19 at 17:36 +0200, Miquel Raynal wrote:
> 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.

Thanks for the input! I will follow your suggestions and let you guys know my
findings.

Regards,
Stefan

> 
> > > ---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-20  6:27 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
2021-04-20  6:26           ` Stefan Riedmüller [this message]
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=13112dfe42b99e0c5ef340d9382bb76014bb870c.camel@phytec.de \
    --to=s.riedmueller@phytec.de \
    --cc=festevam@gmail.com \
    --cc=guillaume.tucker@collabora.com \
    --cc=kernel@pengutronix.de \
    --cc=linux-mtd@lists.infradead.org \
    --cc=miquel.raynal@bootlin.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox