[U-Boot] [PATCH] cmd: mtd: solve bad block support in erase command

[Miquel Raynal <miquel.raynal@bootlin.com>]

Hi Patrick,

Patrick Delaunay <patrick.delaunay@st.com> wrote on Fri, 20 Sep 2019
09:20:12 +0200:

> This patch modify the loop in mtd erase command to erase one by one
> the blocks in the requested area.
> 
> It solves issue on "mtd erase" command on nand with existing bad block,
> the command is interrupted on the first bad block with the trace:
> 	"Skipping bad block at 0xffffffffffffffff"
> 
> In MTD driver (nand/raw), when a bad block is present on the MTD
> device, the erase_op.fail_addr is not updated and we have the initial
> value MTD_FAIL_ADDR_UNKNOWN = (ULL)-1.
> 
> This case seems normal in nand_base.c:nand_erase_nand(),
> we have the 2 exit cases during the loop:
> 
> 1/ we have a bad block (nand_block_checkbad)
> 	instr->state = MTD_ERASE_FAILED
> 	loop interrupted (goto erase_exit)
> 
> 2/ if block erase failed (status & NAND_STATUS_FAIL)
> 	instr->state = MTD_ERASE_FAILED;
> 	instr->fail_addr =
> 				((loff_t)page << chip->page_shift);
> 	loop interrupted (goto erase_exit)
> 
> So erase_op.fail_addr can't be used if bad blocks were present
> in the erased area; we need to use mtd_erase only one block to detect
> and skip these existing bad blocks (as it is done in nand_util.c).
> 
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> ---
> 
> Hi,
> 
> Found a correct in the mtd erase command.
> 
> I detect the issue and test the patch on STM32MP157C-EV1 board,
> with nor and nand. We have the block table at the end of the nand
> so the 4 last blocks are marked bad.
> 
> And I try to erase all the nand with the command "mtd erase".
> 
> Before the patch:
> 
> The "nand erase" command behavior is OK:
> 
> STM32MP> nand erase 0x0 0x000040000000  
> 
> NAND erase: device 0 whole chip
> Skipping bad block at  0x3ff00000
> Skipping bad block at  0x3ff40000
> Skipping bad block at  0x3ff80000
> Skipping bad block at  0x3ffc0000
> 
> But the "mtd erase" command is not correct:
> 
> STM32MP> mtd list  
> SF: Detected mx66l51235l with page size 256 Bytes, erase size 64 KiB, total 64 MiB
> List of MTD devices:
> * nand0
>   - type: NAND flash
>   - block size: 0x40000 bytes
>   - min I/O: 0x1000 bytes
>   - OOB size: 224 bytes
>   - OOB available: 118 bytes
>   - ECC strength: 8 bits
>   - ECC step size: 512 bytes
>   - bitflip threshold: 6 bits
>   - 0x000000000000-0x000040000000 : "nand0"
>           - 0x000000000000-0x000000200000 : "fsbl"
>           - 0x000000200000-0x000000400000 : "ssbl1"
>           - 0x000000400000-0x000000600000 : "ssbl2"
>           - 0x000000600000-0x000040000000 : "UBI"
> * nor0
>   - type: NOR flash
>   - block size: 0x10000 bytes
>   - min I/O: 0x1 bytes
>   - 0x000000000000-0x000004000000 : "nor0"
>           - 0x000000000000-0x000000040000 : "fsbl1"
>           - 0x000000040000-0x000000080000 : "fsbl2"
>           - 0x000000080000-0x000000280000 : "ssbl"
>           - 0x000000280000-0x000000300000 : "u-boot-env"
>           - 0x000000300000-0x000004000000 : "nor_user"
> 
> STM32MP> mtd erase nand0 0x0 0x000040000000  
> Erasing 0x00000000 ... 0x3fffffff (4096 eraseblock(s))
> Skipping bad block at 0xffffffffffffffff
> 
> OK
> 
> The 4 bad blocks are not correctly skipped,
> the command is stopped on the first error.
> 
> After the patch, the "mtd erase" command skips the 4 bad block
> exactly as the "nand erase" command:
> 
> STM32MP> mtd erase nand0 0x000000000000 0x000040000000  
> SF: Detected mx66l51235l with page size 256 Bytes, erase size 64 KiB, total 64 MiB
> Erasing 0x00000000 ... 0x3fffffff (4096 eraseblock(s))
> Skipping bad block at 0x3ff00000
> Skipping bad block at 0x3ff40000
> Skipping bad block at 0x3ff80000
> Skipping bad block at 0x3ffc0000
> 
> Regards
> 
> Patrick
> 
> 
>  cmd/mtd.c | 24 ++++++++++++------------
>  1 file changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/cmd/mtd.c b/cmd/mtd.c
> index 1b6b8dda2b..a559b5a4a3 100644
> --- a/cmd/mtd.c
> +++ b/cmd/mtd.c
> @@ -387,7 +387,7 @@ static int do_mtd_erase(cmd_tbl_t *cmdtp, int flag, int argc,
>  	struct mtd_info *mtd;
>  	u64 off, len;
>  	bool scrub;
> -	int ret;
> +	int ret = 0;
>  
>  	if (argc < 2)
>  		return CMD_RET_USAGE;
> @@ -423,22 +423,22 @@ static int do_mtd_erase(cmd_tbl_t *cmdtp, int flag, int argc,
>  
>  	erase_op.mtd = mtd;
>  	erase_op.addr = off;
> -	erase_op.len = len;
> +	erase_op.len = mtd->erasesize;
>  	erase_op.scrub = scrub;
>  
> -	while (erase_op.len) {
> +	while (len) {
>  		ret = mtd_erase(mtd, &erase_op);
>  
> -		/* Abort if its not a bad block error */
> -		if (ret != -EIO)
> -			break;
> -
> -		printf("Skipping bad block at 0x%08llx\n", erase_op.fail_addr);
> +		if (ret) {
> +			/* Abort if its not a bad block error */
> +			if (ret != -EIO)
> +				break;
> +			printf("Skipping bad block at 0x%08llx\n",
> +			       erase_op.addr);
> +		}
>  
> -		/* Skip bad block and continue behind it */
> -		erase_op.len -= erase_op.fail_addr - erase_op.addr;
> -		erase_op.len -= mtd->erasesize;
> -		erase_op.addr = erase_op.fail_addr + mtd->erasesize;
> +		len -= mtd->erasesize;
> +		erase_op.addr += mtd->erasesize;
>  	}
>  
>  	if (ret && ret != -EIO)

Nice catch!

Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>


Thanks,
Miquèl