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

[Michael Nazzareno Trimarchi]

Hi Mikhail

On Mon, Oct 24, 2022 at 10:34 AM Mikhail Kshevetskiy
<mikhail.kshevetskiy at iopsys.eu> wrote:
>
>
> On 24.10.2022 11:27, Michael Nazzareno Trimarchi wrote:
> > [External email]
> >
> >
> >
> >
> >
> > 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 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
>
> Is there a good way to apply Dario patch? It was sent as HTML message.
>
> I can fix message body and apply it, but it will be better if Dario
> resend it as normal clear text.

We will respin with your fix on Dario patch, your tested-by and your signed off

Michael

>
>
> Mikhail
>
> PS: I send patch for a second time because first time it was sent in
> HTML format.
>
> >
> >>> --
> >>> 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%7C2c68f94e89c247e2886f08dab5999790%7C7ff78d652de440f586750569e5c7a65d%7C0%7C0%7C638021968536553670%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=GUYTrPb1VWhNN0EfEgV15jtuZxTldBB4qBpUsl%2Bd8H0%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
> > https://eur03.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.amarulasolutions.com%2F&data=05%7C01%7Cmikhail.kshevetskiy%40iopsys.eu%7C2c68f94e89c247e2886f08dab5999790%7C7ff78d652de440f586750569e5c7a65d%7C0%7C0%7C638021968536553670%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=GUYTrPb1VWhNN0EfEgV15jtuZxTldBB4qBpUsl%2Bd8H0%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