Re: [PATCH] mtd: spinand: Wait after erase in spinand_markbad

[Miquel Raynal <miquel.raynal@xxxxxxxxxxx>]

Hi Emil,

Emil Lenngren <emil.lenngren@xxxxxxxxx> wrote on Mon, 4 Mar 2019
12:55:30 +0100:

> Hi,
> 
> Den mån 4 mars 2019 kl 12:23 skrev Miquel Raynal <miquel.raynal@xxxxxxxxxxx>:
> >
> > Hi Emil,
> >
> > Emil Lenngren <emil.lenngren@xxxxxxxxx> wrote on Mon, 18 Feb 2019
> > 12:27:25 +0100:
> >  
> > > Hi,
> > >
> > > Den sön 6 jan. 2019 kl 09:11 skrev Boris Brezillon <bbrezillon@xxxxxxxxxx>:  
> > > >
> > > > On Sat, 5 Jan 2019 22:01:44 -0800
> > > > Emil Lenngren <emil.lenngren@xxxxxxxxx> wrote:
> > > >  
> > > > > Hi,
> > > > >
> > > > > Den lör 5 jan. 2019 kl 05:59 skrev Boris Brezillon <bbrezillon@xxxxxxxxxx>:  
> > > > > >
> > > > > > On Fri, 21 Dec 2018 12:58:14 +0100
> > > > > > Emil Lenngren <emil.lenngren@xxxxxxxxx> wrote:
> > > > > >  
> > > > > > > SPI NAND flashes don't accept new commands while an erase is ongoing.
> > > > > > > Make sure to wait until the device is ready before writing the marker.
> > > > > > >
> > > > > > > Just as with the erase op, no error check is performed since we want
> > > > > > > to continue writing the marker even if the erase fails.
> > > > > > >
> > > > > > > Signed-off-by: Emil Lenngren <emil.lenngren@xxxxxxxxx>
> > > > > > > ---
> > > > > > >  drivers/mtd/nand/spi/core.c | 2 ++
> > > > > > >  1 file changed, 2 insertions(+)
> > > > > > >
> > > > > > > diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
> > > > > > > index 479c2f2cf1..c2724d34e6 100644
> > > > > > > --- a/drivers/mtd/nand/spi/core.c
> > > > > > > +++ b/drivers/mtd/nand/spi/core.c
> > > > > > > @@ -685,6 +685,8 @@ static int spinand_markbad(struct nand_device *nand, const struct nand_pos *pos)
> > > > > > >
> > > > > > >       spinand_erase_op(spinand, pos);
> > > > > > >
> > > > > > > +     spinand_wait(spinand, NULL);
> > > > > > > +  
> > > > > >
> > > > > > After thinking a bit more about it, I think we should simply write the
> > > > > > BBM and skip the erase operation. Marking a block bad is just about
> > > > > > writing 0 to the first 2 bytes of the OOB area, and we don't need the
> > > > > > block to be erased to do that.
> > > > > >  
> > > > >
> > > > > I compared with the raw and implementation in
> > > > > nand_block_markbad_lowlevel, it also erases first, ignoring a
> > > > > potential error.  
> > > >
> > > > Yes, and I think this implementation was inspired by the rawnand one,
> > > > but I'm not sure the rawnand implem is correct.
> > > >  
> > > > >
> > > > > On the other hand, a common spi flash chip MX35LF1GE4AB states in the
> > > > > datasheet that it's not recommended to erase a bad block, but no
> > > > > reason why.  
> > > >
> > > > Because the erase might succeed and reset the BBM to 0xff, thus marking
> > > > the block good even if it's unreliable.
> > > >  
> > > > > At the same time, it's generally disallowed to write the
> > > > > same page twice...  
> > > >
> > > > That's only if you care about the data you write to the page. Marking a
> > > > block bad is just about setting the BBM to 0x0, which should always work
> > > > even if the page you're writing to has already been written, simply
> > > > because a 1 -> 0 cell transition does not require an erase (only a 0 ->
> > > > 1 transition does).  
> > >
> > > Should the BBM be written with or without ECC? Now it uses whatever
> > > mode was used in the last operation. Also for reading, I see the
> > > "spinand_isbad" function sets .mode to MTD_OPS_RAW but that field
> > > doesn't seem to be inspected, again using the same ECC mode as in the
> > > last operation.
> > > Isn't it best to use the non-ECC mode for both reading and writing the
> > > BBM? If we would use ECC mode and overwrite the page when writing the
> > > BBM marker, I guess the controller needs to store a second error
> > > correction code (overwriting the previous one), which will probably
> > > lead to ECC failure if later read in ECC-mode (and who knows if it
> > > will "correct" the zero bits to ones...).
> > >  
> >
> > I suppose writing the BBM without correction is what we want.  
> 
> I was also thinking, is it a good or bad idea to move the BBM
> writing/reading to NAND core? I mean, as I understand, both parallel
> nand and spi nand use the same logic to handle the BBM (at the
> beginning of the OOB area). Or is there some specific treatment that
> must be done individually for spi and parallel flash chips?

I don't think there is a difference. Moving it to the NAND core is
probably a good choice. However, as far as I remember, there is no BBT
support in the SPI-NAND core yet.

> 
> After reading the BBM info of some datasheets, it appears the variance
> lies between different manufacturers, not the technology used
> (parallel or spi). For example, Toshiba fills a whole bad block with
> 00h and instructs to read any byte in any page and check if it
> contains 00h. Macronix on the other hand instructs to check the first
> byte of the OOB area in the first and second page to see they contain
> 00h. SmartMedia instructs to check the 6th byte of the OOB area of the
> first page and see if it contains at least two zero bits.

I think the BBM was supposed to be at the 6th bytes of the OOB area for
small page NAND devices (probably the case of the SmartMedia datasheet
you read), otherwise it is supposed to be located at the 1st (and 2nd?)
byte(s) which would work for both Toshiba and Macronix parts, for
instance.


Thanks,
Miquèl

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