* [RESEND PATCH v2] mtd: nand_bbt: scan for next free bbt block if writing bbt fails [not found] <1436284803-16081-1-git-send-email-xander.huff@ni.com> @ 2015-07-22 19:42 ` Xander Huff 2015-08-11 21:55 ` [RESEND RESEND " Xander Huff 0 siblings, 1 reply; 7+ messages in thread From: Xander Huff @ 2015-07-22 19:42 UTC (permalink / raw) To: dwmw2, computersforpeace Cc: linux-mtd, linux-kernel, jeff.westfahl, jaeden.amero, joshc, Ben Shelton, xander.huff From: Ben Shelton <ben.shelton@ni.com> If erasing or writing the BBT fails, we should mark the current BBT block as bad and use the BBT descriptor to scan for the next available unused block in the BBT. We should only return a failure if there isn't any space left. Based on original code implemented by Jeff Westfahl <jeff.westfahl@ni.com>. Signed-off-by: Ben Shelton <ben.shelton@ni.com> Reviewed-by: Jaeden Amero <jaeden.amero@ni.com> Suggested-by: Jeff Westfahl <jeff.westfahl@ni.com> Signed-off-by: Xander Huff <xander.huff@ni.com> --- This v2 patch is in reply to comments from Brian Norris on 7/22/13. See the following links for context: http://lists.infradead.org/pipermail/linux-mtd/2013-July/047596.html http://patchwork.ozlabs.org/patch/244324/ --- drivers/mtd/nand/nand_base.c | 4 ++++ drivers/mtd/nand/nand_bbt.c | 34 ++++++++++++++++++++++++++++++++-- include/linux/mtd/nand.h | 7 +++++++ 3 files changed, 43 insertions(+), 2 deletions(-) diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c index ceb68ca..48299dc 100644 --- a/drivers/mtd/nand/nand_base.c +++ b/drivers/mtd/nand/nand_base.c @@ -2761,6 +2761,7 @@ int nand_erase_nand(struct mtd_info *mtd, struct erase_info *instr, pr_debug("%s: device is write protected!\n", __func__); instr->state = MTD_ERASE_FAILED; + instr->priv = NAND_ERASE_WRITE_PROTECTED; goto erase_exit; } @@ -2776,6 +2777,7 @@ int nand_erase_nand(struct mtd_info *mtd, struct erase_info *instr, pr_warn("%s: attempt to erase a bad block at page 0x%08x\n", __func__, page); instr->state = MTD_ERASE_FAILED; + instr->priv = NAND_ERASE_BAD_BLOCK; goto erase_exit; } @@ -2802,6 +2804,7 @@ int nand_erase_nand(struct mtd_info *mtd, struct erase_info *instr, pr_debug("%s: failed erase, page 0x%08x\n", __func__, page); instr->state = MTD_ERASE_FAILED; + instr->priv = NAND_ERASE_BLOCK_ERASE_FAILED; instr->fail_addr = ((loff_t)page << chip->page_shift); goto erase_exit; @@ -2819,6 +2822,7 @@ int nand_erase_nand(struct mtd_info *mtd, struct erase_info *instr, } } instr->state = MTD_ERASE_DONE; + instr->priv = NAND_ERASE_OK; erase_exit: diff --git a/drivers/mtd/nand/nand_bbt.c b/drivers/mtd/nand/nand_bbt.c index 63a1a36..09f9e62 100644 --- a/drivers/mtd/nand/nand_bbt.c +++ b/drivers/mtd/nand/nand_bbt.c @@ -662,6 +662,7 @@ static int write_bbt(struct mtd_info *mtd, uint8_t *buf, page = td->pages[chip]; goto write; } +next: /* * Automatic placement of the bad block table. Search direction @@ -787,13 +788,42 @@ static int write_bbt(struct mtd_info *mtd, uint8_t *buf, einfo.addr = to; einfo.len = 1 << this->bbt_erase_shift; res = nand_erase_nand(mtd, &einfo, 1); - if (res < 0) + if (res == -EIO && einfo.state == MTD_ERASE_FAILED + && einfo.priv == NAND_ERASE_BLOCK_ERASE_FAILED) { + /* This block is bad. Mark it as such and see if + * there's another block available in the BBT area. */ + int block = page >> + (this->bbt_erase_shift - this->page_shift); + pr_info("nand_bbt: failed to erase block %d when writing BBT\n", + block); + bbt_mark_entry(this, block, BBT_BLOCK_WORN); + + res = this->block_markbad(mtd, block); + if (res) + pr_warn("nand_bbt: error %d while marking block %d bad\n", + res, block); + goto next; + } else if (res < 0) goto outerr; res = scan_write_bbt(mtd, to, len, buf, td->options & NAND_BBT_NO_OOB ? NULL : &buf[len]); - if (res < 0) + if (res == -EIO) { + /* This block is bad. Mark it as such and see if + * there's another block available in the BBT area. */ + int block = page >> + (this->bbt_erase_shift - this->page_shift); + pr_info("nand_bbt: failed to erase block %d when writing BBT\n", + block); + bbt_mark_entry(this, block, BBT_BLOCK_WORN); + + res = this->block_markbad(mtd, block); + if (res) + pr_warn("nand_bbt: error %d while marking block %d bad\n", + res, block); + goto next; + } else if (res < 0) goto outerr; pr_info("Bad block table written to 0x%012llx, version 0x%02X\n", diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h index 272f429..86e11f6 100644 --- a/include/linux/mtd/nand.h +++ b/include/linux/mtd/nand.h @@ -1030,4 +1030,11 @@ struct nand_sdr_timings { /* get timing characteristics from ONFI timing mode. */ const struct nand_sdr_timings *onfi_async_timing_mode_to_sdr_timings(int mode); + +/* reasons for erase failures */ +#define NAND_ERASE_OK 0 +#define NAND_ERASE_WRITE_PROTECTED 1 +#define NAND_ERASE_BAD_BLOCK 2 +#define NAND_ERASE_BLOCK_ERASE_FAILED 3 + #endif /* __LINUX_MTD_NAND_H */ -- 1.9.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [RESEND RESEND PATCH v2] mtd: nand_bbt: scan for next free bbt block if writing bbt fails 2015-07-22 19:42 ` [RESEND PATCH v2] mtd: nand_bbt: scan for next free bbt block if writing bbt fails Xander Huff @ 2015-08-11 21:55 ` Xander Huff 2015-08-25 17:49 ` [RESEND RESEND " Xander Huff 0 siblings, 1 reply; 7+ messages in thread From: Xander Huff @ 2015-08-11 21:55 UTC (permalink / raw) To: dwmw2, computersforpeace Cc: linux-mtd, linux-kernel, jeff.westfahl, jaeden.amero, joshc, Ben Shelton, xander.huff From: Ben Shelton <ben.shelton@ni.com> If erasing or writing the BBT fails, we should mark the current BBT block as bad and use the BBT descriptor to scan for the next available unused block in the BBT. We should only return a failure if there isn't any space left. Based on original code implemented by Jeff Westfahl <jeff.westfahl@ni.com>. Signed-off-by: Ben Shelton <ben.shelton@ni.com> Reviewed-by: Jaeden Amero <jaeden.amero@ni.com> Suggested-by: Jeff Westfahl <jeff.westfahl@ni.com> Signed-off-by: Xander Huff <xander.huff@ni.com> --- This v2 patch is in reply to comments from Brian Norris on 7/22/13. See the following links for context: http://lists.infradead.org/pipermail/linux-mtd/2013-July/047596.html http://patchwork.ozlabs.org/patch/244324/ --- drivers/mtd/nand/nand_base.c | 4 ++++ drivers/mtd/nand/nand_bbt.c | 34 ++++++++++++++++++++++++++++++++-- include/linux/mtd/nand.h | 7 +++++++ 3 files changed, 43 insertions(+), 2 deletions(-) diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c index ceb68ca..48299dc 100644 --- a/drivers/mtd/nand/nand_base.c +++ b/drivers/mtd/nand/nand_base.c @@ -2761,6 +2761,7 @@ int nand_erase_nand(struct mtd_info *mtd, struct erase_info *instr, pr_debug("%s: device is write protected!\n", __func__); instr->state = MTD_ERASE_FAILED; + instr->priv = NAND_ERASE_WRITE_PROTECTED; goto erase_exit; } @@ -2776,6 +2777,7 @@ int nand_erase_nand(struct mtd_info *mtd, struct erase_info *instr, pr_warn("%s: attempt to erase a bad block at page 0x%08x\n", __func__, page); instr->state = MTD_ERASE_FAILED; + instr->priv = NAND_ERASE_BAD_BLOCK; goto erase_exit; } @@ -2802,6 +2804,7 @@ int nand_erase_nand(struct mtd_info *mtd, struct erase_info *instr, pr_debug("%s: failed erase, page 0x%08x\n", __func__, page); instr->state = MTD_ERASE_FAILED; + instr->priv = NAND_ERASE_BLOCK_ERASE_FAILED; instr->fail_addr = ((loff_t)page << chip->page_shift); goto erase_exit; @@ -2819,6 +2822,7 @@ int nand_erase_nand(struct mtd_info *mtd, struct erase_info *instr, } } instr->state = MTD_ERASE_DONE; + instr->priv = NAND_ERASE_OK; erase_exit: diff --git a/drivers/mtd/nand/nand_bbt.c b/drivers/mtd/nand/nand_bbt.c index 63a1a36..09f9e62 100644 --- a/drivers/mtd/nand/nand_bbt.c +++ b/drivers/mtd/nand/nand_bbt.c @@ -662,6 +662,7 @@ static int write_bbt(struct mtd_info *mtd, uint8_t *buf, page = td->pages[chip]; goto write; } +next: /* * Automatic placement of the bad block table. Search direction @@ -787,13 +788,42 @@ static int write_bbt(struct mtd_info *mtd, uint8_t *buf, einfo.addr = to; einfo.len = 1 << this->bbt_erase_shift; res = nand_erase_nand(mtd, &einfo, 1); - if (res < 0) + if (res == -EIO && einfo.state == MTD_ERASE_FAILED + && einfo.priv == NAND_ERASE_BLOCK_ERASE_FAILED) { + /* This block is bad. Mark it as such and see if + * there's another block available in the BBT area. */ + int block = page >> + (this->bbt_erase_shift - this->page_shift); + pr_info("nand_bbt: failed to erase block %d when writing BBT\n", + block); + bbt_mark_entry(this, block, BBT_BLOCK_WORN); + + res = this->block_markbad(mtd, block); + if (res) + pr_warn("nand_bbt: error %d while marking block %d bad\n", + res, block); + goto next; + } else if (res < 0) goto outerr; res = scan_write_bbt(mtd, to, len, buf, td->options & NAND_BBT_NO_OOB ? NULL : &buf[len]); - if (res < 0) + if (res == -EIO) { + /* This block is bad. Mark it as such and see if + * there's another block available in the BBT area. */ + int block = page >> + (this->bbt_erase_shift - this->page_shift); + pr_info("nand_bbt: failed to erase block %d when writing BBT\n", + block); + bbt_mark_entry(this, block, BBT_BLOCK_WORN); + + res = this->block_markbad(mtd, block); + if (res) + pr_warn("nand_bbt: error %d while marking block %d bad\n", + res, block); + goto next; + } else if (res < 0) goto outerr; pr_info("Bad block table written to 0x%012llx, version 0x%02X\n", diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h index 272f429..86e11f6 100644 --- a/include/linux/mtd/nand.h +++ b/include/linux/mtd/nand.h @@ -1030,4 +1030,11 @@ struct nand_sdr_timings { /* get timing characteristics from ONFI timing mode. */ const struct nand_sdr_timings *onfi_async_timing_mode_to_sdr_timings(int mode); + +/* reasons for erase failures */ +#define NAND_ERASE_OK 0 +#define NAND_ERASE_WRITE_PROTECTED 1 +#define NAND_ERASE_BAD_BLOCK 2 +#define NAND_ERASE_BLOCK_ERASE_FAILED 3 + #endif /* __LINUX_MTD_NAND_H */ -- 1.9.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [RESEND RESEND RESEND PATCH v2] mtd: nand_bbt: scan for next free bbt block if writing bbt fails 2015-08-11 21:55 ` [RESEND RESEND " Xander Huff @ 2015-08-25 17:49 ` Xander Huff 2015-08-25 18:34 ` Brian Norris 0 siblings, 1 reply; 7+ messages in thread From: Xander Huff @ 2015-08-25 17:49 UTC (permalink / raw) To: dwmw2, computersforpeace Cc: linux-mtd, linux-kernel, jeff.westfahl, jaeden.amero, joshc, Ben Shelton, xander.huff From: Ben Shelton <ben.shelton@ni.com> If erasing or writing the BBT fails, we should mark the current BBT block as bad and use the BBT descriptor to scan for the next available unused block in the BBT. We should only return a failure if there isn't any space left. Based on original code implemented by Jeff Westfahl <jeff.westfahl@ni.com>. Signed-off-by: Ben Shelton <ben.shelton@ni.com> Reviewed-by: Jaeden Amero <jaeden.amero@ni.com> Suggested-by: Jeff Westfahl <jeff.westfahl@ni.com> Signed-off-by: Xander Huff <xander.huff@ni.com> --- This v2 patch is in reply to comments from Brian Norris on 7/22/13. See the following links for context: http://lists.infradead.org/pipermail/linux-mtd/2013-July/047596.html http://patchwork.ozlabs.org/patch/244324/ --- drivers/mtd/nand/nand_base.c | 4 ++++ drivers/mtd/nand/nand_bbt.c | 34 ++++++++++++++++++++++++++++++++-- include/linux/mtd/nand.h | 7 +++++++ 3 files changed, 43 insertions(+), 2 deletions(-) diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c index ceb68ca..48299dc 100644 --- a/drivers/mtd/nand/nand_base.c +++ b/drivers/mtd/nand/nand_base.c @@ -2761,6 +2761,7 @@ int nand_erase_nand(struct mtd_info *mtd, struct erase_info *instr, pr_debug("%s: device is write protected!\n", __func__); instr->state = MTD_ERASE_FAILED; + instr->priv = NAND_ERASE_WRITE_PROTECTED; goto erase_exit; } @@ -2776,6 +2777,7 @@ int nand_erase_nand(struct mtd_info *mtd, struct erase_info *instr, pr_warn("%s: attempt to erase a bad block at page 0x%08x\n", __func__, page); instr->state = MTD_ERASE_FAILED; + instr->priv = NAND_ERASE_BAD_BLOCK; goto erase_exit; } @@ -2802,6 +2804,7 @@ int nand_erase_nand(struct mtd_info *mtd, struct erase_info *instr, pr_debug("%s: failed erase, page 0x%08x\n", __func__, page); instr->state = MTD_ERASE_FAILED; + instr->priv = NAND_ERASE_BLOCK_ERASE_FAILED; instr->fail_addr = ((loff_t)page << chip->page_shift); goto erase_exit; @@ -2819,6 +2822,7 @@ int nand_erase_nand(struct mtd_info *mtd, struct erase_info *instr, } } instr->state = MTD_ERASE_DONE; + instr->priv = NAND_ERASE_OK; erase_exit: diff --git a/drivers/mtd/nand/nand_bbt.c b/drivers/mtd/nand/nand_bbt.c index 63a1a36..09f9e62 100644 --- a/drivers/mtd/nand/nand_bbt.c +++ b/drivers/mtd/nand/nand_bbt.c @@ -662,6 +662,7 @@ static int write_bbt(struct mtd_info *mtd, uint8_t *buf, page = td->pages[chip]; goto write; } +next: /* * Automatic placement of the bad block table. Search direction @@ -787,13 +788,42 @@ static int write_bbt(struct mtd_info *mtd, uint8_t *buf, einfo.addr = to; einfo.len = 1 << this->bbt_erase_shift; res = nand_erase_nand(mtd, &einfo, 1); - if (res < 0) + if (res == -EIO && einfo.state == MTD_ERASE_FAILED + && einfo.priv == NAND_ERASE_BLOCK_ERASE_FAILED) { + /* This block is bad. Mark it as such and see if + * there's another block available in the BBT area. */ + int block = page >> + (this->bbt_erase_shift - this->page_shift); + pr_info("nand_bbt: failed to erase block %d when writing BBT\n", + block); + bbt_mark_entry(this, block, BBT_BLOCK_WORN); + + res = this->block_markbad(mtd, block); + if (res) + pr_warn("nand_bbt: error %d while marking block %d bad\n", + res, block); + goto next; + } else if (res < 0) goto outerr; res = scan_write_bbt(mtd, to, len, buf, td->options & NAND_BBT_NO_OOB ? NULL : &buf[len]); - if (res < 0) + if (res == -EIO) { + /* This block is bad. Mark it as such and see if + * there's another block available in the BBT area. */ + int block = page >> + (this->bbt_erase_shift - this->page_shift); + pr_info("nand_bbt: failed to erase block %d when writing BBT\n", + block); + bbt_mark_entry(this, block, BBT_BLOCK_WORN); + + res = this->block_markbad(mtd, block); + if (res) + pr_warn("nand_bbt: error %d while marking block %d bad\n", + res, block); + goto next; + } else if (res < 0) goto outerr; pr_info("Bad block table written to 0x%012llx, version 0x%02X\n", diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h index 272f429..86e11f6 100644 --- a/include/linux/mtd/nand.h +++ b/include/linux/mtd/nand.h @@ -1030,4 +1030,11 @@ struct nand_sdr_timings { /* get timing characteristics from ONFI timing mode. */ const struct nand_sdr_timings *onfi_async_timing_mode_to_sdr_timings(int mode); + +/* reasons for erase failures */ +#define NAND_ERASE_OK 0 +#define NAND_ERASE_WRITE_PROTECTED 1 +#define NAND_ERASE_BAD_BLOCK 2 +#define NAND_ERASE_BLOCK_ERASE_FAILED 3 + #endif /* __LINUX_MTD_NAND_H */ -- 1.9.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [RESEND RESEND RESEND PATCH v2] mtd: nand_bbt: scan for next free bbt block if writing bbt fails 2015-08-25 17:49 ` [RESEND RESEND " Xander Huff @ 2015-08-25 18:34 ` Brian Norris 2015-08-26 15:57 ` Bean Huo 霍斌斌 (beanhuo) 0 siblings, 1 reply; 7+ messages in thread From: Brian Norris @ 2015-08-25 18:34 UTC (permalink / raw) To: Xander Huff Cc: dwmw2, linux-mtd, linux-kernel, jeff.westfahl, jaeden.amero, joshc, Ben Shelton, Bean Huo 霍斌斌 (beanhuo) + Bean, who is looking at refactoring this driver. Perhaps he can review this. On Tue, Aug 25, 2015 at 12:49:26PM -0500, Xander Huff wrote: > From: Ben Shelton <ben.shelton@ni.com> > > If erasing or writing the BBT fails, we should mark the current BBT > block as bad and use the BBT descriptor to scan for the next available > unused block in the BBT. We should only return a failure if there isn't > any space left. > > Based on original code implemented by Jeff Westfahl > <jeff.westfahl@ni.com>. > > Signed-off-by: Ben Shelton <ben.shelton@ni.com> > Reviewed-by: Jaeden Amero <jaeden.amero@ni.com> > Suggested-by: Jeff Westfahl <jeff.westfahl@ni.com> > Signed-off-by: Xander Huff <xander.huff@ni.com> > --- > This v2 patch is in reply to comments from Brian Norris on 7/22/13. See > the following links for context: > http://lists.infradead.org/pipermail/linux-mtd/2013-July/047596.html > http://patchwork.ozlabs.org/patch/244324/ Wow, a blast from the past... > --- > drivers/mtd/nand/nand_base.c | 4 ++++ > drivers/mtd/nand/nand_bbt.c | 34 ++++++++++++++++++++++++++++++++-- > include/linux/mtd/nand.h | 7 +++++++ > 3 files changed, 43 insertions(+), 2 deletions(-) > > diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c > index ceb68ca..48299dc 100644 > --- a/drivers/mtd/nand/nand_base.c > +++ b/drivers/mtd/nand/nand_base.c > @@ -2761,6 +2761,7 @@ int nand_erase_nand(struct mtd_info *mtd, struct erase_info *instr, > pr_debug("%s: device is write protected!\n", > __func__); > instr->state = MTD_ERASE_FAILED; > + instr->priv = NAND_ERASE_WRITE_PROTECTED; I'm pretty sure this is an illegal overloading of the 'priv' field. Notice how ioctl(MEMERASE64) uses this field in mtdchard.c. So I suspect you've broken char access to /dev/mtdX. Try 'flash_erase' from mtd-utils, for instance. > goto erase_exit; > } > > @@ -2776,6 +2777,7 @@ int nand_erase_nand(struct mtd_info *mtd, struct erase_info *instr, > pr_warn("%s: attempt to erase a bad block at page 0x%08x\n", > __func__, page); > instr->state = MTD_ERASE_FAILED; > + instr->priv = NAND_ERASE_BAD_BLOCK; > goto erase_exit; > } > > @@ -2802,6 +2804,7 @@ int nand_erase_nand(struct mtd_info *mtd, struct erase_info *instr, > pr_debug("%s: failed erase, page 0x%08x\n", > __func__, page); > instr->state = MTD_ERASE_FAILED; > + instr->priv = NAND_ERASE_BLOCK_ERASE_FAILED; > instr->fail_addr = > ((loff_t)page << chip->page_shift); > goto erase_exit; > @@ -2819,6 +2822,7 @@ int nand_erase_nand(struct mtd_info *mtd, struct erase_info *instr, > } > } > instr->state = MTD_ERASE_DONE; > + instr->priv = NAND_ERASE_OK; > > erase_exit: > > diff --git a/drivers/mtd/nand/nand_bbt.c b/drivers/mtd/nand/nand_bbt.c > index 63a1a36..09f9e62 100644 > --- a/drivers/mtd/nand/nand_bbt.c > +++ b/drivers/mtd/nand/nand_bbt.c > @@ -662,6 +662,7 @@ static int write_bbt(struct mtd_info *mtd, uint8_t *buf, > page = td->pages[chip]; > goto write; > } > +next: > > /* > * Automatic placement of the bad block table. Search direction > @@ -787,13 +788,42 @@ static int write_bbt(struct mtd_info *mtd, uint8_t *buf, > einfo.addr = to; > einfo.len = 1 << this->bbt_erase_shift; > res = nand_erase_nand(mtd, &einfo, 1); > - if (res < 0) > + if (res == -EIO && einfo.state == MTD_ERASE_FAILED > + && einfo.priv == NAND_ERASE_BLOCK_ERASE_FAILED) { Do you actually need that last condition? What's wrong with the first two? > + /* This block is bad. Mark it as such and see if > + * there's another block available in the BBT area. */ > + int block = page >> > + (this->bbt_erase_shift - this->page_shift); > + pr_info("nand_bbt: failed to erase block %d when writing BBT\n", > + block); > + bbt_mark_entry(this, block, BBT_BLOCK_WORN); > + > + res = this->block_markbad(mtd, block); > + if (res) > + pr_warn("nand_bbt: error %d while marking block %d bad\n", > + res, block); > + goto next; > + } else if (res < 0) > goto outerr; > > res = scan_write_bbt(mtd, to, len, buf, > td->options & NAND_BBT_NO_OOB ? NULL : > &buf[len]); > - if (res < 0) > + if (res == -EIO) { > + /* This block is bad. Mark it as such and see if > + * there's another block available in the BBT area. */ > + int block = page >> > + (this->bbt_erase_shift - this->page_shift); > + pr_info("nand_bbt: failed to erase block %d when writing BBT\n", > + block); > + bbt_mark_entry(this, block, BBT_BLOCK_WORN); > + > + res = this->block_markbad(mtd, block); > + if (res) > + pr_warn("nand_bbt: error %d while marking block %d bad\n", > + res, block); > + goto next; > + } else if (res < 0) > goto outerr; > > pr_info("Bad block table written to 0x%012llx, version 0x%02X\n", > diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h > index 272f429..86e11f6 100644 > --- a/include/linux/mtd/nand.h > +++ b/include/linux/mtd/nand.h > @@ -1030,4 +1030,11 @@ struct nand_sdr_timings { > > /* get timing characteristics from ONFI timing mode. */ > const struct nand_sdr_timings *onfi_async_timing_mode_to_sdr_timings(int mode); > + > +/* reasons for erase failures */ > +#define NAND_ERASE_OK 0 > +#define NAND_ERASE_WRITE_PROTECTED 1 > +#define NAND_ERASE_BAD_BLOCK 2 > +#define NAND_ERASE_BLOCK_ERASE_FAILED 3 Why exactly do you need these statuses? I thought the existing error codes were sufficient.. > + > #endif /* __LINUX_MTD_NAND_H */ Brian ^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [RESEND RESEND RESEND PATCH v2] mtd: nand_bbt: scan for next free bbt block if writing bbt fails 2015-08-25 18:34 ` Brian Norris @ 2015-08-26 15:57 ` Bean Huo 霍斌斌 (beanhuo) 2015-08-27 0:07 ` Brian Norris 0 siblings, 1 reply; 7+ messages in thread From: Bean Huo 霍斌斌 (beanhuo) @ 2015-08-26 15:57 UTC (permalink / raw) To: Brian Norris, Xander Huff Cc: dwmw2@infradead.org, linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org, jeff.westfahl@ni.com, jaeden.amero@ni.com, joshc@ni.com, Ben Shelton > + Bean, who is looking at refactoring this driver. Perhaps he can review > this. > > On Tue, Aug 25, 2015 at 12:49:26PM -0500, Xander Huff wrote: > > From: Ben Shelton <ben.shelton@ni.com> > > > > If erasing or writing the BBT fails, we should mark the current BBT > > block as bad and use the BBT descriptor to scan for the next available > > unused block in the BBT. We should only return a failure if there > > isn't any space left. > > > > Based on original code implemented by Jeff Westfahl > > <jeff.westfahl@ni.com>. > > > > Signed-off-by: Ben Shelton <ben.shelton@ni.com> > > Reviewed-by: Jaeden Amero <jaeden.amero@ni.com> > > Suggested-by: Jeff Westfahl <jeff.westfahl@ni.com> > > Signed-off-by: Xander Huff <xander.huff@ni.com> > > --- > > This v2 patch is in reply to comments from Brian Norris on 7/22/13. > > See the following links for context: > > http://lists.infradead.org/pipermail/linux-mtd/2013-July/047596.html > > http://patchwork.ozlabs.org/patch/244324/ > > Wow, a blast from the past... > > > --- > > drivers/mtd/nand/nand_base.c | 4 ++++ drivers/mtd/nand/nand_bbt.c > > | 34 ++++++++++++++++++++++++++++++++-- > > include/linux/mtd/nand.h | 7 +++++++ > > 3 files changed, 43 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/mtd/nand/nand_base.c > > b/drivers/mtd/nand/nand_base.c index ceb68ca..48299dc 100644 > > --- a/drivers/mtd/nand/nand_base.c > > +++ b/drivers/mtd/nand/nand_base.c > > @@ -2761,6 +2761,7 @@ int nand_erase_nand(struct mtd_info *mtd, struct > erase_info *instr, > > pr_debug("%s: device is write protected!\n", > > __func__); > > instr->state = MTD_ERASE_FAILED; > > + instr->priv = NAND_ERASE_WRITE_PROTECTED; > Here will overload the 'priv' field of mtdchar.c Please go thought mtdchar_ioctl() function. > I'm pretty sure this is an illegal overloading of the 'priv' field. > Notice how ioctl(MEMERASE64) uses this field in mtdchard.c. So I suspect > you've broken char access to /dev/mtdX. Try 'flash_erase' from mtd-utils, for > instance. > > > goto erase_exit; > > } > > > > @@ -2776,6 +2777,7 @@ int nand_erase_nand(struct mtd_info *mtd, struct > erase_info *instr, > > pr_warn("%s: attempt to erase a bad block at page > 0x%08x\n", > > __func__, page); > > instr->state = MTD_ERASE_FAILED; > > + instr->priv = NAND_ERASE_BAD_BLOCK; > > goto erase_exit; > > } > > > > @@ -2802,6 +2804,7 @@ int nand_erase_nand(struct mtd_info *mtd, struct > erase_info *instr, > > pr_debug("%s: failed erase, page 0x%08x\n", > > __func__, page); > > instr->state = MTD_ERASE_FAILED; > > + instr->priv = NAND_ERASE_BLOCK_ERASE_FAILED; > > instr->fail_addr = > > ((loff_t)page << chip->page_shift); > > goto erase_exit; > > @@ -2819,6 +2822,7 @@ int nand_erase_nand(struct mtd_info *mtd, struct > erase_info *instr, > > } > > } > > instr->state = MTD_ERASE_DONE; > > + instr->priv = NAND_ERASE_OK; > > > > erase_exit: > > > > diff --git a/drivers/mtd/nand/nand_bbt.c b/drivers/mtd/nand/nand_bbt.c > > index 63a1a36..09f9e62 100644 > > --- a/drivers/mtd/nand/nand_bbt.c > > +++ b/drivers/mtd/nand/nand_bbt.c > > @@ -662,6 +662,7 @@ static int write_bbt(struct mtd_info *mtd, uint8_t *buf, > > page = td->pages[chip]; > > goto write; > > } > > +next: > > > > /* > > * Automatic placement of the bad block table. Search direction > @@ > > -787,13 +788,42 @@ static int write_bbt(struct mtd_info *mtd, uint8_t *buf, > > einfo.addr = to; > > einfo.len = 1 << this->bbt_erase_shift; > > res = nand_erase_nand(mtd, &einfo, 1); > > - if (res < 0) > > + if (res == -EIO && einfo.state == MTD_ERASE_FAILED > > + && einfo.priv == NAND_ERASE_BLOCK_ERASE_FAILED) { > > Do you actually need that last condition? What's wrong with the first two? > > > + /* This block is bad. Mark it as such and see if > > + * there's another block available in the BBT area. */ > > + int block = page >> > > + (this->bbt_erase_shift - this->page_shift); > > + pr_info("nand_bbt: failed to erase block %d when writing > BBT\n", > > + block); > > + bbt_mark_entry(this, block, BBT_BLOCK_WORN); > > + > > + res = this->block_markbad(mtd, block); > > + if (res) > > + pr_warn("nand_bbt: error %d while marking block %d > bad\n", > > + res, block); > > + goto next; > > + } else if (res < 0) > > goto outerr; For my knowledge , we don't directly mark this block be a bad block, Just like ubi layer,this block also need to further testing and verify if It is real bad block.right? > > > > res = scan_write_bbt(mtd, to, len, buf, > > td->options & NAND_BBT_NO_OOB ? NULL : > > &buf[len]); > > - if (res < 0) > > + if (res == -EIO) { > > + /* This block is bad. Mark it as such and see if > > + * there's another block available in the BBT area. */ > > + int block = page >> > > + (this->bbt_erase_shift - this->page_shift); > > + pr_info("nand_bbt: failed to erase block %d when writing > BBT\n", > > + block); > > + bbt_mark_entry(this, block, BBT_BLOCK_WORN); > > + > > + res = this->block_markbad(mtd, block); > > + if (res) > > + pr_warn("nand_bbt: error %d while marking block %d > bad\n", > > + res, block); > > + goto next; > > + } else if (res < 0) > > goto outerr; > > > > pr_info("Bad block table written to 0x%012llx, version 0x%02X\n", > > diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h index > > 272f429..86e11f6 100644 > > --- a/include/linux/mtd/nand.h > > +++ b/include/linux/mtd/nand.h > > @@ -1030,4 +1030,11 @@ struct nand_sdr_timings { > > > > /* get timing characteristics from ONFI timing mode. */ const struct > > nand_sdr_timings *onfi_async_timing_mode_to_sdr_timings(int mode); > > + > > +/* reasons for erase failures */ > > +#define NAND_ERASE_OK 0 > > +#define NAND_ERASE_WRITE_PROTECTED 1 > > +#define NAND_ERASE_BAD_BLOCK 2 > > +#define NAND_ERASE_BLOCK_ERASE_FAILED 3 > > Why exactly do you need these statuses? I thought the existing error codes > were sufficient.. > > > + > > #endif /* __LINUX_MTD_NAND_H */ > > Brian ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RESEND RESEND RESEND PATCH v2] mtd: nand_bbt: scan for next free bbt block if writing bbt fails 2015-08-26 15:57 ` Bean Huo 霍斌斌 (beanhuo) @ 2015-08-27 0:07 ` Brian Norris 2015-09-04 21:20 ` Xander Huff 0 siblings, 1 reply; 7+ messages in thread From: Brian Norris @ 2015-08-27 0:07 UTC (permalink / raw) To: Bean Huo 霍斌斌 (beanhuo) Cc: Xander Huff, dwmw2@infradead.org, linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org, jeff.westfahl@ni.com, jaeden.amero@ni.com, joshc@ni.com, Ben Shelton, Richard Weinberger, Peter Pan 潘栋 (peterpandong) On Wed, Aug 26, 2015 at 03:57:00PM +0000, Bean Huo 霍斌斌 (beanhuo) wrote: > > On Tue, Aug 25, 2015 at 12:49:26PM -0500, Xander Huff wrote: > > > diff --git a/drivers/mtd/nand/nand_bbt.c b/drivers/mtd/nand/nand_bbt.c > > > index 63a1a36..09f9e62 100644 > > > --- a/drivers/mtd/nand/nand_bbt.c > > > +++ b/drivers/mtd/nand/nand_bbt.c > > > -787,13 +788,42 @@ static int write_bbt(struct mtd_info *mtd, uint8_t *buf, > > > einfo.addr = to; > > > einfo.len = 1 << this->bbt_erase_shift; > > > res = nand_erase_nand(mtd, &einfo, 1); > > > - if (res < 0) > > > + if (res == -EIO && einfo.state == MTD_ERASE_FAILED > > > + && einfo.priv == NAND_ERASE_BLOCK_ERASE_FAILED) { > > > > Do you actually need that last condition? What's wrong with the first two? > > > > > + /* This block is bad. Mark it as such and see if > > > + * there's another block available in the BBT area. */ > > > + int block = page >> > > > + (this->bbt_erase_shift - this->page_shift); > > > + pr_info("nand_bbt: failed to erase block %d when writing > > BBT\n", > > > + block); > > > + bbt_mark_entry(this, block, BBT_BLOCK_WORN); > > > + > > > + res = this->block_markbad(mtd, block); > > > + if (res) > > > + pr_warn("nand_bbt: error %d while marking block %d > > bad\n", > > > + res, block); > > > + goto next; > > > + } else if (res < 0) > > > goto outerr; > > > For my knowledge , we don't directly mark this block be a bad block, > Just like ubi layer,this block also need to further testing and verify if > It is real bad block.right? That's a good point...we might want some kind of separate function for a torture test. Might look at UBI's torture_peb() for inspiration. > > > > > > res = scan_write_bbt(mtd, to, len, buf, > > > td->options & NAND_BBT_NO_OOB ? NULL : > > > &buf[len]); > > > - if (res < 0) > > > + if (res == -EIO) { > > > + /* This block is bad. Mark it as such and see if > > > + * there's another block available in the BBT area. */ > > > + int block = page >> > > > + (this->bbt_erase_shift - this->page_shift); > > > + pr_info("nand_bbt: failed to erase block %d when writing > > BBT\n", > > > + block); > > > + bbt_mark_entry(this, block, BBT_BLOCK_WORN); > > > + > > > + res = this->block_markbad(mtd, block); > > > + if (res) > > > + pr_warn("nand_bbt: error %d while marking block %d > > bad\n", > > > + res, block); > > > + goto next; > > > + } else if (res < 0) > > > goto outerr; > > > > > > pr_info("Bad block table written to 0x%012llx, version 0x%02X\n", Brian ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RESEND RESEND RESEND PATCH v2] mtd: nand_bbt: scan for next free bbt block if writing bbt fails 2015-08-27 0:07 ` Brian Norris @ 2015-09-04 21:20 ` Xander Huff 0 siblings, 0 replies; 7+ messages in thread From: Xander Huff @ 2015-09-04 21:20 UTC (permalink / raw) To: Brian Norris, "Bean Huo 霍斌斌 (beanhuo)" Cc: dwmw2@infradead.org, linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org, jeff.westfahl@ni.com, jaeden.amero@ni.com, joshc@ni.com, Ben Shelton, Richard Weinberger, "Peter Pan 潘栋 (peterpandong)", nathan.sullivan On 8/26/2015 7:07 PM, Brian Norris wrote: > On Wed, Aug 26, 2015 at 03:57:00PM +0000, Bean Huo 霍斌斌 (beanhuo) wrote: >>> On Tue, Aug 25, 2015 at 12:49:26PM -0500, Xander Huff wrote: > >>>> diff --git a/drivers/mtd/nand/nand_bbt.c b/drivers/mtd/nand/nand_bbt.c >>>> index 63a1a36..09f9e62 100644 >>>> --- a/drivers/mtd/nand/nand_bbt.c >>>> +++ b/drivers/mtd/nand/nand_bbt.c > >>>> -787,13 +788,42 @@ static int write_bbt(struct mtd_info *mtd, uint8_t *buf, >>>> einfo.addr = to; >>>> einfo.len = 1 << this->bbt_erase_shift; >>>> res = nand_erase_nand(mtd, &einfo, 1); >>>> - if (res < 0) >>>> + if (res == -EIO && einfo.state == MTD_ERASE_FAILED >>>> + && einfo.priv == NAND_ERASE_BLOCK_ERASE_FAILED) { >>> >>> Do you actually need that last condition? What's wrong with the first two? >>> The intent of the extra condition is to distinguish from other erase failures due to write protection or an already known bad block. We don't want to mark a write protected block as bad simply because we failed to erase it, for example. >>>> + /* This block is bad. Mark it as such and see if >>>> + * there's another block available in the BBT area. */ >>>> + int block = page >> >>>> + (this->bbt_erase_shift - this->page_shift); >>>> + pr_info("nand_bbt: failed to erase block %d when writing >>> BBT\n", >>>> + block); >>>> + bbt_mark_entry(this, block, BBT_BLOCK_WORN); >>>> + >>>> + res = this->block_markbad(mtd, block); >>>> + if (res) >>>> + pr_warn("nand_bbt: error %d while marking block %d >>> bad\n", >>>> + res, block); >>>> + goto next; >>>> + } else if (res < 0) >>>> goto outerr; >> >> >> For my knowledge , we don't directly mark this block be a bad block, >> Just like ubi layer,this block also need to further testing and verify if >> It is real bad block.right? > > That's a good point...we might want some kind of separate function for a > torture test. Might look at UBI's torture_peb() for inspiration. > Hmm, I'll look into this. Any performance concerns if we're torturing for every potential bad block we come across? >>>> >>>> res = scan_write_bbt(mtd, to, len, buf, >>>> td->options & NAND_BBT_NO_OOB ? NULL : >>>> &buf[len]); >>>> - if (res < 0) >>>> + if (res == -EIO) { >>>> + /* This block is bad. Mark it as such and see if >>>> + * there's another block available in the BBT area. */ >>>> + int block = page >> >>>> + (this->bbt_erase_shift - this->page_shift); >>>> + pr_info("nand_bbt: failed to erase block %d when writing >>> BBT\n", >>>> + block); >>>> + bbt_mark_entry(this, block, BBT_BLOCK_WORN); >>>> + >>>> + res = this->block_markbad(mtd, block); >>>> + if (res) >>>> + pr_warn("nand_bbt: error %d while marking block %d >>> bad\n", >>>> + res, block); >>>> + goto next; >>>> + } else if (res < 0) >>>> goto outerr; >>>> >>>> pr_info("Bad block table written to 0x%012llx, version 0x%02X\n",> >>>> diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h index >>>> 272f429..86e11f6 100644 >>>> --- a/include/linux/mtd/nand.h >>>> +++ b/include/linux/mtd/nand.h >>>> @@ -1030,4 +1030,11 @@ struct nand_sdr_timings { >>>> >>>> /* get timing characteristics from ONFI timing mode. */ const struct >>>> nand_sdr_timings *onfi_async_timing_mode_to_sdr_timings(int mode); >>>> + >>>> +/* reasons for erase failures */ >>>> +#define NAND_ERASE_OK 0 >>>> +#define NAND_ERASE_WRITE_PROTECTED 1 >>>> +#define NAND_ERASE_BAD_BLOCK 2 >>>> +#define NAND_ERASE_BLOCK_ERASE_FAILED 3 >>> >>> Why exactly do you need these statuses? I thought the existing error codes >>> were sufficient.. This goes along with why we were originally using the 'priv' field. Lots of places use MTD_ERASE_FAILED to see if an erase failed, but we want to check specifically what type of erase failure occurred. Do ya'll have any suggestions on how better to accomplish this? I'm thinking maybe adding a new member to the erase_info struct like 'fail_info' to get this information to nand_bbt. -- Xander Huff Staff Software Engineer National Instruments ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2015-09-04 21:20 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1436284803-16081-1-git-send-email-xander.huff@ni.com>
2015-07-22 19:42 ` [RESEND PATCH v2] mtd: nand_bbt: scan for next free bbt block if writing bbt fails Xander Huff
2015-08-11 21:55 ` [RESEND RESEND " Xander Huff
2015-08-25 17:49 ` [RESEND RESEND " Xander Huff
2015-08-25 18:34 ` Brian Norris
2015-08-26 15:57 ` Bean Huo 霍斌斌 (beanhuo)
2015-08-27 0:07 ` Brian Norris
2015-09-04 21:20 ` Xander Huff
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox