From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from down.free-electrons.com ([37.187.137.238] helo=mail.free-electrons.com) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1b3Ngl-00071K-Lg for linux-mtd@lists.infradead.org; Thu, 19 May 2016 13:08:33 +0000 Date: Thu, 19 May 2016 15:08:07 +0200 From: Boris Brezillon To: Kyle Roeschley Cc: richard@nod.at, dwmw2@infradead.org, computersforpeace@gmail.com, beanhuo@micron.com, linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org, nathan.sullivan@ni.com, xander.huff@ni.com, peterpansjtu@gmail.com Subject: Re: [RESEND PATCH v4] mtd: nand_bbt: scan for next free bbt block if writing bbt fails Message-ID: <20160519150807.7345634d@bbrezillon> In-Reply-To: <1463609401-19861-1-git-send-email-kyle.roeschley@ni.com> References: <1463609401-19861-1-git-send-email-kyle.roeschley@ni.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Hi, Sorry for the delay but I'm waiting for 4.7-rc1 before taking new NAND patches, and decided to use this time to work on other stuff. On Wed, 18 May 2016 17:10:01 -0500 Kyle Roeschley wrote: > 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 > . > > Signed-off-by: Kyle Roeschley > Suggested-by: Jeff Westfahl > --- > v4: Don't ignore write protection while marking bad BBT blocks > Correctly call block_markbad > Minor cleanups > > v3: Don't overload mtd->priv > Keep nand_erase_nand from erroring on protected BBT blocks > > v2: Mark OOB area in each block as well as BBT > Avoid marking read-only, bad address, or known bad blocks as bad > > drivers/mtd/nand/nand_bbt.c | 41 +++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 39 insertions(+), 2 deletions(-) > > diff --git a/drivers/mtd/nand/nand_bbt.c b/drivers/mtd/nand/nand_bbt.c > index 2fbb523..dfc68e0 100644 > --- a/drivers/mtd/nand/nand_bbt.c > +++ b/drivers/mtd/nand/nand_bbt.c > @@ -663,6 +663,7 @@ static int write_bbt(struct mtd_info *mtd, uint8_t *buf, > goto write; > } > > +next: > /* > * Automatic placement of the bad block table. Search direction > * top -> down? > @@ -787,14 +788,50 @@ 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) { > + /* > + * 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, to); > + if (res) > + pr_warn("nand_bbt: error %d while marking block %d bad\n", > + res, block); > + td->pages[chip] = -1; > + goto next; I'd like to have other feedback on this approach before taking a decision. Brian, Richard, any comments? > + } else if (res) { > 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 write block %d when writing BBT\n", > + block); > + bbt_mark_entry(this, block, BBT_BLOCK_WORN); > + > + res = this->block_markbad(mtd, to); > + if (res) > + pr_warn("nand_bbt: error %d while marking block %d bad\n", > + res, block); > + td->pages[chip] = -1; I see twice the same block of code, probably a good candidate for factorization ;-). > + goto next; > + } else if (res) { > goto outerr; > + } > > pr_info("Bad block table written to 0x%012llx, version 0x%02X\n", > (unsigned long long)to, td->version[chip]); Best Regards, Boris -- Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com