From mboxrd@z Thu Jan  1 00:00:00 1970
From: Boris Brezillon <boris.brezillon@bootlin.com>
Date: Mon, 6 Aug 2018 22:06:13 +0200
Subject: [U-Boot] [PATCH 2/4] mtd: nand: Don't abort erase operation
 when a bad block is detected
In-Reply-To: <20180806151253.31205-2-sr@denx.de>
References: <20180806151253.31205-1-sr@denx.de>
 <20180806151253.31205-2-sr@denx.de>
Message-ID: <20180806220613.7d134ea5@bbrezillon>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Hi Stefan,

On Mon,  6 Aug 2018 17:12:51 +0200
Stefan Roese <sr@denx.de> wrote:

> It was noticed, that the erase command (mtd erase spi-nand0) aborts upon
> the first bad block. With this change, bad blocks are now skipped and
> the erase operation will continue.
> 

That's not what the raw NAND framework does [1], and I'm almost sure
MTD users expect erase ops to stop when a bad block is found and "skip
bad block" was not explicitly requested.

I'd suggest moving to an approach where cmd/mtd.c erases blocks one by
one and checks the status of each block (with mtd_block_isbad()) before
calling mtd_erase(). Alternatively, we could add a 'bool skipbad' field
to struct erase_info, but that means getting away from Linux
implementation (which is already the case since uboot has an extra
"int scrub" field).

Regards,

Boris

[1]https://elixir.bootlin.com/u-boot/v2018.09-rc1/source/drivers/mtd/nand/nand_base.c#L2911

> Signed-off-by: Stefan Roese <sr@denx.de>
> Cc: Miquel Raynal <miquel.raynal@bootlin.com>
> Cc: Boris Brezillon <boris.brezillon@bootlin.com>
> Cc: Jagan Teki <jagan@openedev.com>
> ---
>  drivers/mtd/nand/core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/nand/core.c b/drivers/mtd/nand/core.c
> index 0b793695cc..888f765b90 100644
> --- a/drivers/mtd/nand/core.c
> +++ b/drivers/mtd/nand/core.c
> @@ -162,7 +162,7 @@ int nanddev_mtd_erase(struct mtd_info *mtd, struct erase_info *einfo)
>  	nanddev_offs_to_pos(nand, einfo->addr + einfo->len - 1, &last);
>  	while (nanddev_pos_cmp(&pos, &last) <= 0) {
>  		ret = nanddev_erase(nand, &pos);
> -		if (ret) {
> +		if (ret && ret != -EIO) {
>  			einfo->fail_addr = nanddev_pos_to_offs(nand, &pos);
>  
>  			return ret;