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

[Emil Lenngren <emil.lenngren@xxxxxxxxx>]

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

/Emil

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