Hi Patrick

On 11/28/22 10:22, 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().
> 
> Signed-off-by: Patrick Delaunay <patrick.delau...@foss.st.com>
> ---
> 
>  drivers/dfu/dfu_mtd.c | 26 ++++++++++++++++++--------
>  1 file changed, 18 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/dfu/dfu_mtd.c b/drivers/dfu/dfu_mtd.c
> index c7075f12eca9..4fb02c4c806c 100644
> --- a/drivers/dfu/dfu_mtd.c
> +++ b/drivers/dfu/dfu_mtd.c
> @@ -91,22 +91,32 @@ 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 */
> +                     if (mtd_block_isbad(mtd, off)) {
> +                             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;
>               }
>       }

Reviewed-by: Patrice Chotard <patrice.chot...@foss.st.com>

Thanks
Patrice

Reply via email to