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 <kyle.roesch...@ni.com> 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
> <jeff.westf...@ni.com>.
> 
> Signed-off-by: Kyle Roeschley <kyle.roesch...@ni.com>
> Suggested-by: Jeff Westfahl <jeff.westf...@ni.com>
> ---
> 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

Reply via email to