| Message ID | 216fe2eb-54f4-465e-9206-ac92a91750e8@iopsys.eu |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
HI Mikhail On Mon, Oct 24, 2022 at 9:30 AM Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu> wrote: > > 'mtd erase' command should not erase bad blocks. To force bad block erasing > there is 'mtd erase.dontskipbad' command. Unfortunately nand layer erases > bad blocks unconditionally. This is wrong. > > Fix issue by adding bad block checks to do_mtd_erase() function in the case > srub flag is not provided. We can't simplify code by eliminating -EIO result > check of mtd_erase() as it will terminate erasing with CMD_RET_SUCCESS. > > Thanks to Dario Binacchi <dario.binacchi@amarulasolutions.com> for his patch. > > Signed-off-by: Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu> > --- > cmd/mtd.c | 17 +++++++++++++++-- > 1 file changed, 15 insertions(+), 2 deletions(-) > > diff --git a/cmd/mtd.c b/cmd/mtd.c > index ad5cc9827d..a314745e95 100644 > --- a/cmd/mtd.c > +++ b/cmd/mtd.c > @@ -434,11 +434,24 @@ static int do_mtd_erase(struct cmd_tbl *cmdtp, int flag, int argc, > erase_op.mtd = mtd; > erase_op.addr = off; > erase_op.len = mtd->erasesize; > - erase_op.scrub = scrub; > > while (len) { > - ret = mtd_erase(mtd, &erase_op); > + if (!scrub) { > + ret = mtd_block_isbad(mtd, erase_op.addr); > + if (ret < 0) { > + printf("Failed to get bad block at 0x%08llx\n", > + erase_op.addr); > + ret = CMD_RET_FAILURE; > + goto out_put_mtd; > + } else if (ret > 0) { > + /* simulate bad block behavior */ > + ret = -EIO; > + goto skip_block_erasing; > + } > + } > It's better to review the dario patch and we can resend with your signoff and comment too Michael > + ret = mtd_erase(mtd, &erase_op); > +skip_block_erasing: > if (ret) { > /* Abort if its not a bad block error */ > if (ret != -EIO) > -- > 2.35.1 >
On 24.10.2022 10:36, Michael Nazzareno Trimarchi wrote: > [External email] > > > > > > HI Mikhail > > On Mon, Oct 24, 2022 at 9:30 AM Mikhail Kshevetskiy > <mikhail.kshevetskiy@iopsys.eu> wrote: >> 'mtd erase' command should not erase bad blocks. To force bad block erasing >> there is 'mtd erase.dontskipbad' command. Unfortunately nand layer erases >> bad blocks unconditionally. This is wrong. >> >> Fix issue by adding bad block checks to do_mtd_erase() function in the case >> srub flag is not provided. We can't simplify code by eliminating -EIO result >> check of mtd_erase() as it will terminate erasing with CMD_RET_SUCCESS. >> >> Thanks to Dario Binacchi <dario.binacchi@amarulasolutions.com> for his patch. >> >> Signed-off-by: Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu> >> --- >> cmd/mtd.c | 17 +++++++++++++++-- >> 1 file changed, 15 insertions(+), 2 deletions(-) >> >> diff --git a/cmd/mtd.c b/cmd/mtd.c >> index ad5cc9827d..a314745e95 100644 >> --- a/cmd/mtd.c >> +++ b/cmd/mtd.c >> @@ -434,11 +434,24 @@ static int do_mtd_erase(struct cmd_tbl *cmdtp, int flag, int argc, >> erase_op.mtd = mtd; >> erase_op.addr = off; >> erase_op.len = mtd->erasesize; >> - erase_op.scrub = scrub; >> >> while (len) { >> - ret = mtd_erase(mtd, &erase_op); >> + if (!scrub) { >> + ret = mtd_block_isbad(mtd, erase_op.addr); >> + if (ret < 0) { >> + printf("Failed to get bad block at 0x%08llx\n", >> + erase_op.addr); >> + ret = CMD_RET_FAILURE; >> + goto out_put_mtd; >> + } else if (ret > 0) { >> + /* simulate bad block behavior */ >> + ret = -EIO; >> + goto skip_block_erasing; >> + } >> + } >> > It's better to review the dario patch and we can resend with your > signoff and comment too > > Michael I already do it and just suggest a better version of his patch. Mikhail >> + ret = mtd_erase(mtd, &erase_op); >> +skip_block_erasing: >> if (ret) { >> /* Abort if its not a bad block error */ >> if (ret != -EIO) >> -- >> 2.35.1 >> > > -- > Michael Nazzareno Trimarchi > Co-Founder & Chief Executive Officer > M. +39 347 913 2170 > michael@amarulasolutions.com > __________________________________ > > Amarula Solutions BV > Joop Geesinkweg 125, 1114 AB, Amsterdam, NL > T. +31 (0)85 111 9172 > info@amarulasolutions.com > https://eur03.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.amarulasolutions.com%2F&data=05%7C01%7Cmikhail.kshevetskiy%40iopsys.eu%7C934733f81ca541502e1508dab5926d99%7C7ff78d652de440f586750569e5c7a65d%7C0%7C0%7C638021937756458878%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=ydvGZ%2BeoO%2FonTzuVg3sStZZHSDFUeKxUhCrdwA48OB0%3D&reserved=0
diff --git a/cmd/mtd.c b/cmd/mtd.c index ad5cc9827d..a314745e95 100644 --- a/cmd/mtd.c +++ b/cmd/mtd.c @@ -434,11 +434,24 @@ static int do_mtd_erase(struct cmd_tbl *cmdtp, int flag, int argc, erase_op.mtd = mtd; erase_op.addr = off; erase_op.len = mtd->erasesize; - erase_op.scrub = scrub; while (len) { - ret = mtd_erase(mtd, &erase_op); + if (!scrub) { + ret = mtd_block_isbad(mtd, erase_op.addr); + if (ret < 0) { + printf("Failed to get bad block at 0x%08llx\n", + erase_op.addr); + ret = CMD_RET_FAILURE; + goto out_put_mtd; + } else if (ret > 0) { + /* simulate bad block behavior */ + ret = -EIO; + goto skip_block_erasing; + } + } + ret = mtd_erase(mtd, &erase_op); +skip_block_erasing: if (ret) { /* Abort if its not a bad block error */ if (ret != -EIO)
'mtd erase' command should not erase bad blocks. To force bad block erasing there is 'mtd erase.dontskipbad' command. Unfortunately nand layer erases bad blocks unconditionally. This is wrong. Fix issue by adding bad block checks to do_mtd_erase() function in the case srub flag is not provided. We can't simplify code by eliminating -EIO result check of mtd_erase() as it will terminate erasing with CMD_RET_SUCCESS. Thanks to Dario Binacchi <dario.binacchi@amarulasolutions.com> for his patch. Signed-off-by: Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu> --- cmd/mtd.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-)