[PATCH v3 2/2] dfu: mtd: mark bad the MTD block on erase error

[Dario Binacchi]

Hello Patrick,

On Mon, Jun 05, 2023 at 09:52:08AM +0200, Patrick Delaunay wrote:
> In the MTD DFU backend, it is needed to mark the NAND block bad when the
> erase failed with the -EIO error, as it is done in UBI and JFFS2 code.
> 
> This operation is not done in the MTD framework, but the bad block
> tag (in BBM or in BBT) is required to avoid to write data on this block
> in the next DFU_OP_WRITE loop in mtd_block_op(): the code skip the bad
> blocks, tested by mtd_block_isbad().
> 
> Without this patch, when the NAND block become bad on DFU write operation
> - low probability on new NAND - the DFU write operation will always failed
> because the failing block is never marked bad.
> 
> This patch also adds a test to avoid to request an erase operation on a
> block already marked bad; this test is not performed in MTD framework
> in mtd_erase().
> 
> Reviewed-by: Michael Trimarchi <michael at amarulasolutions.com>
> Signed-off-by: Patrick Delaunay <patrick.delaunay at foss.st.com>

Applied to nand-next,
thanks and regards,

Dario

> ---
> 
> Changes in v3:
> - Split serie with trace fix and support of bad block in MTD erase
> - Fix trace for "bbt reserved" when mtd_block_isbad return 2
> 
> Changes in v2:
> - fix mtd_block_isbad offset parameter for erase check
> - Add trace when bad block are skipped in erase loop
> - Add remaining byte in trace "Limit reached"
> 
>  drivers/dfu/dfu_mtd.c | 30 ++++++++++++++++++++++--------
>  1 file changed, 22 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/dfu/dfu_mtd.c b/drivers/dfu/dfu_mtd.c
> index b764f091786d..271d485d1c9a 100644
> --- a/drivers/dfu/dfu_mtd.c
> +++ b/drivers/dfu/dfu_mtd.c
> @@ -91,22 +91,36 @@ static int mtd_block_op(enum dfu_op op, struct dfu_entity *dfu,
>  				return -EIO;
>  			}
>  
> +			/* Skip the block if it is bad, don't erase it again */
> +			ret = mtd_block_isbad(mtd, erase_op.addr);
> +			if (ret) {
> +				printf("Skipping %s at 0x%08llx\n",
> +				       ret == 1 ? "bad block" : "bbt reserved",
> +				       erase_op.addr);
> +				erase_op.addr += mtd->erasesize;
> +				continue;
> +			}
> +
>  			ret = mtd_erase(mtd, &erase_op);
>  
>  			if (ret) {
> -				/* Abort if its not a bad block error */
> -				if (ret != -EIO) {
> -					printf("Failure while erasing at offset 0x%llx\n",
> -					       erase_op.fail_addr);
> -					return 0;
> +				/* If this is not -EIO, we have no idea what to do. */
> +				if (ret == -EIO) {
> +					printf("Marking bad block at 0x%08llx (%d)\n",
> +					       erase_op.fail_addr, ret);
> +					ret = mtd_block_markbad(mtd, erase_op.addr);
> +				}
> +				/* Abort if it is not -EIO or can't mark bad */
> +				if (ret) {
> +					printf("Failure while erasing at offset 0x%llx (%d)\n",
> +					       erase_op.fail_addr, ret);
> +					return ret;
>  				}
> -				printf("Skipping bad block at 0x%08llx\n",
> -				       erase_op.addr);
>  			} else {
>  				remaining -= mtd->erasesize;
>  			}
>  
> -			/* Continue erase behind bad block */
> +			/* Continue erase behind the current block */
>  			erase_op.addr += mtd->erasesize;
>  		}
>  	}