[PATCH v2] cmd: mtd: try to erase bad blocks only if scrub flag is provided

[Dario Binacchi]

Hi Mikhail,

On Mon, Oct 24, 2022 at 10:27 AM Michael Nazzareno Trimarchi <
michael at amarulasolutions.com> wrote:

> Hi
>
> On Mon, Oct 24, 2022 at 10:20 AM Mikhail Kshevetskiy
> <mikhail.kshevetskiy at iopsys.eu> wrote:
> >
> >
> > On 24.10.2022 10:49, Michael Nazzareno Trimarchi wrote:
> > > [External email]
> > >
> > >
> > >
> > >
> > >
> > > Hi Mikhail
> > >
> > > On Mon, Oct 24, 2022 at 9:37 AM Mikhail Kshevetskiy
> > > <mikhail.kshevetskiy at 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 at amarulasolutions.com> for
> his patch.
> > >>
> > >> Signed-off-by: Mikhail Kshevetskiy <mikhail.kshevetskiy at 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;
> > >> +                       }
> > >> +               }
> > >>
> > >> +               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
> > >>
> > > As I stated in a different email. Please re-post with the right
> sign-off
> > >
> > > Michael
> >
> > There is an issue with Dario Binacchi patch. Please see my comments for
> > his patch.
> >
> > I just suggest a fix. I am sorry if I do it wrong way.
> >
> > Mikhail
>
> There are two ways I can see:
> Repost the v3 starting from Dario one and improve the commit message
> for him. You can add your signed off and tested by


And please add these tags:

Suggested-by: Michael Trimarchi <michael at amarulasolutions.com>
Co-developed-by: Michael Trimarchi <michael at amarulasolutions.com>
Signed-off-by: Michael Trimarchi <michael at amarulasolutions.com>
Co-developed-by: Dario Binacchi <dario.binacchi at amarulasolutions.com>
Signed-off-by: Dario Binacchi <dario.binacchi at amarulasolutions.com>

Thanks and regards,
Dario


> or
> wait Dario to resend with all the suggestion you have made
>
> We think that you rise a correct problem and you help us to
> understand. We just decided to fix on uboot level and not core level.
>
> Michael
>
>
> >
> > > --
> > > Michael Nazzareno Trimarchi
> > > Co-Founder & Chief Executive Officer
> > > M. +39 347 913 2170
> > > michael at amarulasolutions.com
> > > __________________________________
> > >
> > > Amarula Solutions BV
> > > Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
> > > T. +31 (0)85 111 9172
> > > info at amarulasolutions.com
> > >
> https://eur03.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.amarulasolutions.com%2F&data=05%7C01%7Cmikhail.kshevetskiy%40iopsys.eu%7Ca0041129e25a4456725708dab594462c%7C7ff78d652de440f586750569e5c7a65d%7C0%7C0%7C638021945688614410%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=HASzL7T4QF8Kea8t6Dt7hQYnHGs1poC9gRPZ6N3SPEI%3D&reserved=0
>
>
>
> --
> Michael Nazzareno Trimarchi
> Co-Founder & Chief Executive Officer
> M. +39 347 913 2170
> michael at amarulasolutions.com
> __________________________________
>
> Amarula Solutions BV
> Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
> T. +31 (0)85 111 9172
> info at amarulasolutions.com
> www.amarulasolutions.com
>


-- 

*Dario Binacchi*

Embedded Linux Developer

dario.binacchi at amarulasolutions.com

__________________________________


*Amarula Solutions SRL*

Via Le Canevare 30, 31100 Treviso, Veneto, IT

T. +39 042 243 5310
info at amarulasolutions.com

www.amarulasolutions.com