| Message ID | 20230605095147.v3.2.I20e8d74ea2ff0a99c6c741846b46af89c4ee136a@changeid |
|---|---|
| State | Accepted |
| Commit | be0da1257f189c09604b01bc04a7e8411bf18e5c |
| Delegated to: | Dario Binacchi |
| Headers | show |
| Series | dfu: mtd: mark bad the MTD block on erase error | expand |
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@amarulasolutions.com> > Signed-off-by: Patrick Delaunay <patrick.delaunay@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; > } > }
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; } }