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

'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(-)
  

Hi Mikhail

On Mon, Oct 24, 2022 at 9:37 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;
> +                       }
> +               }
>
> +               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
  

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@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;
>> +                       }
>> +               }
>>
>> +               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

> --
> 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&amp;data=05%7C01%7Cmikhail.kshevetskiy%40iopsys.eu%7Ca0041129e25a4456725708dab594462c%7C7ff78d652de440f586750569e5c7a65d%7C0%7C0%7C638021945688614410%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=HASzL7T4QF8Kea8t6Dt7hQYnHGs1poC9gRPZ6N3SPEI%3D&amp;reserved=0
  

Hi

On Mon, Oct 24, 2022 at 10:20 AM Mikhail Kshevetskiy
<mikhail.kshevetskiy@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@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;
> >> +                       }
> >> +               }
> >>
> >> +               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


>
> > --
> > 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&amp;data=05%7C01%7Cmikhail.kshevetskiy%40iopsys.eu%7Ca0041129e25a4456725708dab594462c%7C7ff78d652de440f586750569e5c7a65d%7C0%7C0%7C638021945688614410%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=HASzL7T4QF8Kea8t6Dt7hQYnHGs1poC9gRPZ6N3SPEI%3D&amp;reserved=0
  

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@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@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;
>>>> +                       }
>>>> +               }
>>>>
>>>> +               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.


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@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&amp;data=05%7C01%7Cmikhail.kshevetskiy%40iopsys.eu%7C2c68f94e89c247e2886f08dab5999790%7C7ff78d652de440f586750569e5c7a65d%7C0%7C0%7C638021968536553670%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=GUYTrPb1VWhNN0EfEgV15jtuZxTldBB4qBpUsl%2Bd8H0%3D&amp;reserved=0
>
>
> --
> 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&amp;data=05%7C01%7Cmikhail.kshevetskiy%40iopsys.eu%7C2c68f94e89c247e2886f08dab5999790%7C7ff78d652de440f586750569e5c7a65d%7C0%7C0%7C638021968536553670%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=GUYTrPb1VWhNN0EfEgV15jtuZxTldBB4qBpUsl%2Bd8H0%3D&amp;reserved=0
  

Hi Mikhail

On Mon, Oct 24, 2022 at 10:34 AM Mikhail Kshevetskiy
<mikhail.kshevetskiy@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@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@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;
> >>>> +                       }
> >>>> +               }
> >>>>
> >>>> +               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@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&amp;data=05%7C01%7Cmikhail.kshevetskiy%40iopsys.eu%7C2c68f94e89c247e2886f08dab5999790%7C7ff78d652de440f586750569e5c7a65d%7C0%7C0%7C638021968536553670%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=GUYTrPb1VWhNN0EfEgV15jtuZxTldBB4qBpUsl%2Bd8H0%3D&amp;reserved=0
> >
> >
> > --
> > 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&amp;data=05%7C01%7Cmikhail.kshevetskiy%40iopsys.eu%7C2c68f94e89c247e2886f08dab5999790%7C7ff78d652de440f586750569e5c7a65d%7C0%7C0%7C638021968536553670%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=GUYTrPb1VWhNN0EfEgV15jtuZxTldBB4qBpUsl%2Bd8H0%3D&amp;reserved=0
  

Hi Mikhail,

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

> Hi
>
> On Mon, Oct 24, 2022 at 10:20 AM Mikhail Kshevetskiy
> <mikhail.kshevetskiy@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@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;
> > >> +                       }
> > >> +               }
> > >>
> > >> +               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@amarulasolutions.com>
Co-developed-by: Michael Trimarchi <michael@amarulasolutions.com>
Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
Co-developed-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
Signed-off-by: Dario Binacchi <dario.binacchi@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@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&amp;data=05%7C01%7Cmikhail.kshevetskiy%40iopsys.eu%7Ca0041129e25a4456725708dab594462c%7C7ff78d652de440f586750569e5c7a65d%7C0%7C0%7C638021945688614410%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=HASzL7T4QF8Kea8t6Dt7hQYnHGs1poC9gRPZ6N3SPEI%3D&amp;reserved=0
>
>
>
> --
> 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
> www.amarulasolutions.com
>