* [PATCH] mtd: spinand: Wait after erase in spinand_markbad @ 2018-12-21 11:58 Emil Lenngren 2019-01-05 13:58 ` Boris Brezillon 0 siblings, 1 reply; 9+ messages in thread From: Emil Lenngren @ 2018-12-21 11:58 UTC (permalink / raw) To: linux-mtd Cc: Emil Lenngren, Boris Brezillon, Miquel Raynal, Richard Weinberger, David Woodhouse, Brian Norris, Marek Vasut 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@gmail.com> --- 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); + memset(spinand->oobbuf, 0, 2); return spinand_write_page(spinand, &req); } -- 2.17.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] mtd: spinand: Wait after erase in spinand_markbad 2018-12-21 11:58 [PATCH] mtd: spinand: Wait after erase in spinand_markbad Emil Lenngren @ 2019-01-05 13:58 ` Boris Brezillon 2019-01-06 6:01 ` Emil Lenngren 0 siblings, 1 reply; 9+ messages in thread From: Boris Brezillon @ 2019-01-05 13:58 UTC (permalink / raw) To: Emil Lenngren Cc: linux-mtd, Marek Vasut, Richard Weinberger, Boris Brezillon, Miquel Raynal, Brian Norris, David Woodhouse On Fri, 21 Dec 2018 12:58:14 +0100 Emil Lenngren <emil.lenngren@gmail.com> 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@gmail.com> > --- > 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. > memset(spinand->oobbuf, 0, 2); > return spinand_write_page(spinand, &req); > } ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] mtd: spinand: Wait after erase in spinand_markbad 2019-01-05 13:58 ` Boris Brezillon @ 2019-01-06 6:01 ` Emil Lenngren 2019-01-06 8:10 ` Boris Brezillon 0 siblings, 1 reply; 9+ messages in thread From: Emil Lenngren @ 2019-01-06 6:01 UTC (permalink / raw) To: Boris Brezillon Cc: linux-mtd, Marek Vasut, Richard Weinberger, Boris Brezillon, Miquel Raynal, Brian Norris, David Woodhouse Hi, Den lör 5 jan. 2019 kl 05:59 skrev Boris Brezillon <bbrezillon@kernel.org>: > > On Fri, 21 Dec 2018 12:58:14 +0100 > Emil Lenngren <emil.lenngren@gmail.com> 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@gmail.com> > > --- > > 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. 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. At the same time, it's generally disallowed to write the same page twice... But in the end I also think the best way is to avoid the erase operation and simply write 0 0 as a raw write. > > memset(spinand->oobbuf, 0, 2); > > return spinand_write_page(spinand, &req); > > } > /Emil ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] mtd: spinand: Wait after erase in spinand_markbad 2019-01-06 6:01 ` Emil Lenngren @ 2019-01-06 8:10 ` Boris Brezillon 2019-02-18 11:27 ` Emil Lenngren 0 siblings, 1 reply; 9+ messages in thread From: Boris Brezillon @ 2019-01-06 8:10 UTC (permalink / raw) To: Emil Lenngren Cc: linux-mtd, Marek Vasut, Richard Weinberger, Boris Brezillon, Miquel Raynal, Brian Norris, David Woodhouse On Sat, 5 Jan 2019 22:01:44 -0800 Emil Lenngren <emil.lenngren@gmail.com> wrote: > Hi, > > Den lör 5 jan. 2019 kl 05:59 skrev Boris Brezillon <bbrezillon@kernel.org>: > > > > On Fri, 21 Dec 2018 12:58:14 +0100 > > Emil Lenngren <emil.lenngren@gmail.com> 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@gmail.com> > > > --- > > > 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). > > But in the end I also think the best way is to avoid the erase > operation and simply write 0 0 as a raw write. I don't know why the erase op was added to nand_block_markbad_lowlevel() in the first place but I don't want to risk breaking platforms that might depend on this behavior. It's different for SPI NAND: the subsystem has just been created and I think we should get rid of this erase call until someone explicitly asks for it with a good explanation on why this is needed. > > > > memset(spinand->oobbuf, 0, 2); > > > return spinand_write_page(spinand, &req); > > > } > > > > /Emil ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] mtd: spinand: Wait after erase in spinand_markbad 2019-01-06 8:10 ` Boris Brezillon @ 2019-02-18 11:27 ` Emil Lenngren 2019-02-20 7:59 ` Boris Brezillon 2019-03-04 11:23 ` Miquel Raynal 0 siblings, 2 replies; 9+ messages in thread From: Emil Lenngren @ 2019-02-18 11:27 UTC (permalink / raw) To: Boris Brezillon Cc: Boris Brezillon, Richard Weinberger, Marek Vasut, linux-mtd, Miquel Raynal, Brian Norris, David Woodhouse Hi, Den sön 6 jan. 2019 kl 09:11 skrev Boris Brezillon <bbrezillon@kernel.org>: > > On Sat, 5 Jan 2019 22:01:44 -0800 > Emil Lenngren <emil.lenngren@gmail.com> wrote: > > > Hi, > > > > Den lör 5 jan. 2019 kl 05:59 skrev Boris Brezillon <bbrezillon@kernel.org>: > > > > > > On Fri, 21 Dec 2018 12:58:14 +0100 > > > Emil Lenngren <emil.lenngren@gmail.com> 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@gmail.com> > > > > --- > > > > 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/ ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] mtd: spinand: Wait after erase in spinand_markbad 2019-02-18 11:27 ` Emil Lenngren @ 2019-02-20 7:59 ` Boris Brezillon 2019-03-04 11:23 ` Miquel Raynal 1 sibling, 0 replies; 9+ messages in thread From: Boris Brezillon @ 2019-02-20 7:59 UTC (permalink / raw) To: Emil Lenngren Cc: Marek Vasut, Richard Weinberger, Boris Brezillon, linux-mtd, Miquel Raynal, Brian Norris, David Woodhouse On Mon, 18 Feb 2019 12:27:25 +0100 Emil Lenngren <emil.lenngren@gmail.com> wrote: > > > 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? We should definitely disable the ECC engine when reading/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...). Well, you could just ignore the ECC result, but that's better to simply read the BBM in raw mode. ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] mtd: spinand: Wait after erase in spinand_markbad 2019-02-18 11:27 ` Emil Lenngren 2019-02-20 7:59 ` Boris Brezillon @ 2019-03-04 11:23 ` Miquel Raynal 2019-03-04 11:55 ` Emil Lenngren 1 sibling, 1 reply; 9+ messages in thread From: Miquel Raynal @ 2019-03-04 11:23 UTC (permalink / raw) To: Emil Lenngren Cc: Boris Brezillon, Richard Weinberger, Boris Brezillon, Marek Vasut, linux-mtd, Brian Norris, David Woodhouse Hi Emil, Emil Lenngren <emil.lenngren@gmail.com> wrote on Mon, 18 Feb 2019 12:27:25 +0100: > Hi, > > Den sön 6 jan. 2019 kl 09:11 skrev Boris Brezillon <bbrezillon@kernel.org>: > > > > On Sat, 5 Jan 2019 22:01:44 -0800 > > Emil Lenngren <emil.lenngren@gmail.com> wrote: > > > > > Hi, > > > > > > Den lör 5 jan. 2019 kl 05:59 skrev Boris Brezillon <bbrezillon@kernel.org>: > > > > > > > > On Fri, 21 Dec 2018 12:58:14 +0100 > > > > Emil Lenngren <emil.lenngren@gmail.com> 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@gmail.com> > > > > > --- > > > > > 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/ ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] mtd: spinand: Wait after erase in spinand_markbad 2019-03-04 11:23 ` Miquel Raynal @ 2019-03-04 11:55 ` Emil Lenngren 2019-03-04 12:50 ` Miquel Raynal 0 siblings, 1 reply; 9+ messages in thread From: Emil Lenngren @ 2019-03-04 11:55 UTC (permalink / raw) To: Miquel Raynal Cc: Boris Brezillon, Richard Weinberger, Boris Brezillon, Marek Vasut, linux-mtd, Brian Norris, David Woodhouse Hi, Den mån 4 mars 2019 kl 12:23 skrev Miquel Raynal <miquel.raynal@bootlin.com>: > > Hi Emil, > > Emil Lenngren <emil.lenngren@gmail.com> wrote on Mon, 18 Feb 2019 > 12:27:25 +0100: > > > Hi, > > > > Den sön 6 jan. 2019 kl 09:11 skrev Boris Brezillon <bbrezillon@kernel.org>: > > > > > > On Sat, 5 Jan 2019 22:01:44 -0800 > > > Emil Lenngren <emil.lenngren@gmail.com> wrote: > > > > > > > Hi, > > > > > > > > Den lör 5 jan. 2019 kl 05:59 skrev Boris Brezillon <bbrezillon@kernel.org>: > > > > > > > > > > On Fri, 21 Dec 2018 12:58:14 +0100 > > > > > Emil Lenngren <emil.lenngren@gmail.com> 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@gmail.com> > > > > > > --- > > > > > > 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? 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. /Emil ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] mtd: spinand: Wait after erase in spinand_markbad 2019-03-04 11:55 ` Emil Lenngren @ 2019-03-04 12:50 ` Miquel Raynal 0 siblings, 0 replies; 9+ messages in thread From: Miquel Raynal @ 2019-03-04 12:50 UTC (permalink / raw) To: Emil Lenngren Cc: Boris Brezillon, Richard Weinberger, Boris Brezillon, Marek Vasut, linux-mtd, Brian Norris, David Woodhouse Hi Emil, Emil Lenngren <emil.lenngren@gmail.com> 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@bootlin.com>: > > > > Hi Emil, > > > > Emil Lenngren <emil.lenngren@gmail.com> wrote on Mon, 18 Feb 2019 > > 12:27:25 +0100: > > > > > Hi, > > > > > > Den sön 6 jan. 2019 kl 09:11 skrev Boris Brezillon <bbrezillon@kernel.org>: > > > > > > > > On Sat, 5 Jan 2019 22:01:44 -0800 > > > > Emil Lenngren <emil.lenngren@gmail.com> wrote: > > > > > > > > > Hi, > > > > > > > > > > Den lör 5 jan. 2019 kl 05:59 skrev Boris Brezillon <bbrezillon@kernel.org>: > > > > > > > > > > > > On Fri, 21 Dec 2018 12:58:14 +0100 > > > > > > Emil Lenngren <emil.lenngren@gmail.com> 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@gmail.com> > > > > > > > --- > > > > > > > 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/ ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-03-04 12:50 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-12-21 11:58 [PATCH] mtd: spinand: Wait after erase in spinand_markbad Emil Lenngren 2019-01-05 13:58 ` Boris Brezillon 2019-01-06 6:01 ` Emil Lenngren 2019-01-06 8:10 ` Boris Brezillon 2019-02-18 11:27 ` Emil Lenngren 2019-02-20 7:59 ` Boris Brezillon 2019-03-04 11:23 ` Miquel Raynal 2019-03-04 11:55 ` Emil Lenngren 2019-03-04 12:50 ` Miquel Raynal
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).