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

Thanks,
Miquèl

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