diff mbox series

[2/7] mtd/nand: try to erase bad blocks only if scrub flag is provided

Message ID 20221006031501.110290-2-mikhail.kshevetskiy@iopsys.eu
State Rejected, archived
Delegated to: Dario Binacchi
Headers show
Series [1/7] mtd: replace name of 'rfree' field with 'free' in struct mtd_ooblayout_ops to better match it's description | expand

Commit Message

Mikhail Kshevetskiy Oct. 6, 2022, 3:14 a.m. UTC
From: Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu>

'mtd erase' command should not erase bad blocks. To force bad block erasing
there is 'mtd erase.dontskipbad' command (this command sets 'scrub' flag
to true in the erase_info structure). Unfortunately nand layer ignore
scrub flag and try to erases bad blocks unconditionally. This is wrong.

Add checks to allow bad block erasing only if scrub flag is set.

Signed-off-by: Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu>
---
 drivers/mtd/nand/core.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Comments

Mikhail Kshevetskiy Oct. 6, 2022, 3:52 p.m. UTC | #1
On Thu, 6 Oct 2022 08:56:08 +0200
Michael Nazzareno Trimarchi <michael@amarulasolutions.com> wrote:

> [External email]
> 
> 
> 
> 
> 
> Hi
> 
> On Thu, Oct 6, 2022 at 5:15 AM <mikhail.kshevetskiy@iopsys.eu> wrote:
> >
> > From: Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu>
> >
> > 'mtd erase' command should not erase bad blocks. To force bad block erasing
> > there is 'mtd erase.dontskipbad' command (this command sets 'scrub' flag
> > to true in the erase_info structure). Unfortunately nand layer ignore
> > scrub flag and try to erases bad blocks unconditionally. This is wrong.
> >
> > Add checks to allow bad block erasing only if scrub flag is set.
> >
> > Signed-off-by: Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu>
> > ---
> >  drivers/mtd/nand/core.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/mtd/nand/core.c b/drivers/mtd/nand/core.c
> > index 99c29670c7..a4fb7602c9 100644
> > --- a/drivers/mtd/nand/core.c
> > +++ b/drivers/mtd/nand/core.c
> > @@ -174,7 +174,10 @@ int nanddev_mtd_erase(struct mtd_info *mtd, struct
> > erase_info *einfo) nanddev_offs_to_pos(nand, einfo->addr + einfo->len - 1,
> > &last); while (nanddev_pos_cmp(&pos, &last) <= 0) {
> >                 schedule();
> > -               ret = nanddev_erase(nand, &pos);
> > +               if (!einfo->scrub && nanddev_isbad(nand, &pos))
> 
> The nandev_erase already check it here:
> 
> if (nanddev_isbad(nand, pos) || nanddev_isreserved(nand, pos)) {
>

no it does not work. see nanddev_erase() code

if block is bad or reserverved, than warning is printed, than block is erased.

 
> > +                       ret = -EIO;
> > +               else
> > +                       ret = nanddev_erase(nand, &pos);
> 
> erase opt should already take in account scrub.
> 
> Please extend the problem
> 
> Michael
> >                 if (ret) {
> >                         einfo->fail_addr = nanddev_pos_to_offs(nand, &pos);
> >
> > --
> > 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&amp;data=05%7C01%7Cmikhail.kshevetskiy%40iopsys.eu%7C0271cbe11c804a8dfde608daa767e01b%7C7ff78d652de440f586750569e5c7a65d%7C0%7C0%7C638006361837818885%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=ZmdqQp%2FXl4XC7yFKmFEYWocIEsqQGYk8b2UI9i2cibo%3D&amp;reserved=0
Michael Nazzareno Trimarchi Oct. 6, 2022, 4:03 p.m. UTC | #2
On Thu, Oct 6, 2022 at 5:52 PM Mikhail Kshevetskiy
<mikhail.kshevetskiy@iopsys.eu> wrote:
>
> On Thu, 6 Oct 2022 08:56:08 +0200
> Michael Nazzareno Trimarchi <michael@amarulasolutions.com> wrote:
>
> > [External email]
> >
> >
> >
> >
> >
> > Hi
> >
> > On Thu, Oct 6, 2022 at 5:15 AM <mikhail.kshevetskiy@iopsys.eu> wrote:
> > >
> > > From: Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu>
> > >
> > > 'mtd erase' command should not erase bad blocks. To force bad block erasing
> > > there is 'mtd erase.dontskipbad' command (this command sets 'scrub' flag
> > > to true in the erase_info structure). Unfortunately nand layer ignore
> > > scrub flag and try to erases bad blocks unconditionally. This is wrong.
> > >
> > > Add checks to allow bad block erasing only if scrub flag is set.
> > >
> > > Signed-off-by: Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu>
> > > ---
> > >  drivers/mtd/nand/core.c | 5 ++++-
> > >  1 file changed, 4 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/mtd/nand/core.c b/drivers/mtd/nand/core.c
> > > index 99c29670c7..a4fb7602c9 100644
> > > --- a/drivers/mtd/nand/core.c
> > > +++ b/drivers/mtd/nand/core.c
> > > @@ -174,7 +174,10 @@ int nanddev_mtd_erase(struct mtd_info *mtd, struct
> > > erase_info *einfo) nanddev_offs_to_pos(nand, einfo->addr + einfo->len - 1,
> > > &last); while (nanddev_pos_cmp(&pos, &last) <= 0) {
> > >                 schedule();
> > > -               ret = nanddev_erase(nand, &pos);
> > > +               if (!einfo->scrub && nanddev_isbad(nand, &pos))
> >
> > The nandev_erase already check it here:
> >
> > if (nanddev_isbad(nand, pos) || nanddev_isreserved(nand, pos)) {
> >
>
> no it does not work. see nanddev_erase() code
>

Let me re-formulate it. What execution path are you taking into account?

The nand are erased using the cmd/nand interface and the erase command
there calls nand_erase_opts that take in account it.

Michael



The nand is erased using the
> if block is bad or reserverved, than warning is printed, than block is erased.
>
>
> > > +                       ret = -EIO;
> > > +               else
> > > +                       ret = nanddev_erase(nand, &pos);
> >
> > erase opt should already take in account scrub.
> >
> > Please extend the problem
> >
> > Michael
> > >                 if (ret) {
> > >                         einfo->fail_addr = nanddev_pos_to_offs(nand, &pos);
> > >
> > > --
> > > 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&amp;data=05%7C01%7Cmikhail.kshevetskiy%40iopsys.eu%7C0271cbe11c804a8dfde608daa767e01b%7C7ff78d652de440f586750569e5c7a65d%7C0%7C0%7C638006361837818885%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=ZmdqQp%2FXl4XC7yFKmFEYWocIEsqQGYk8b2UI9i2cibo%3D&amp;reserved=0
Mikhail Kshevetskiy Oct. 6, 2022, 4:17 p.m. UTC | #3
On Thu, 6 Oct 2022 18:03:17 +0200
Michael Nazzareno Trimarchi <michael@amarulasolutions.com> wrote:

> [External email]
> 
> 
> 
> 
> 
> On Thu, Oct 6, 2022 at 5:52 PM Mikhail Kshevetskiy
> <mikhail.kshevetskiy@iopsys.eu> wrote:
> >
> > On Thu, 6 Oct 2022 08:56:08 +0200
> > Michael Nazzareno Trimarchi <michael@amarulasolutions.com> wrote:
> >
> > > [External email]
> > >
> > >
> > >
> > >
> > >
> > > Hi
> > >
> > > On Thu, Oct 6, 2022 at 5:15 AM <mikhail.kshevetskiy@iopsys.eu> wrote:
> > > >
> > > > From: Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu>
> > > >
> > > > 'mtd erase' command should not erase bad blocks. To force bad block
> > > > erasing there is 'mtd erase.dontskipbad' command (this command sets
> > > > 'scrub' flag to true in the erase_info structure). Unfortunately nand
> > > > layer ignore scrub flag and try to erases bad blocks unconditionally.
> > > > This is wrong.
> > > >
> > > > Add checks to allow bad block erasing only if scrub flag is set.
> > > >
> > > > Signed-off-by: Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu>
> > > > ---
> > > >  drivers/mtd/nand/core.c | 5 ++++-
> > > >  1 file changed, 4 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/mtd/nand/core.c b/drivers/mtd/nand/core.c
> > > > index 99c29670c7..a4fb7602c9 100644
> > > > --- a/drivers/mtd/nand/core.c
> > > > +++ b/drivers/mtd/nand/core.c
> > > > @@ -174,7 +174,10 @@ int nanddev_mtd_erase(struct mtd_info *mtd, struct
> > > > erase_info *einfo) nanddev_offs_to_pos(nand, einfo->addr + einfo->len -
> > > > 1, &last); while (nanddev_pos_cmp(&pos, &last) <= 0) {
> > > >                 schedule();
> > > > -               ret = nanddev_erase(nand, &pos);
> > > > +               if (!einfo->scrub && nanddev_isbad(nand, &pos))
> > >
> > > The nandev_erase already check it here:
> > >
> > > if (nanddev_isbad(nand, pos) || nanddev_isreserved(nand, pos)) {
> > >
> >
> > no it does not work. see nanddev_erase() code
> >
> 
> Let me re-formulate it. What execution path are you taking into account?
> 
> The nand are erased using the cmd/nand interface and the erase command
> there calls nand_erase_opts that take in account it.

spi-nand flash

cmd/mtd.c -> do_mtd_erase() -> .... -> nanddev_mtd_erase() -> nanddev_erase()


> 
> Michael
> 
> 
> 
> The nand is erased using the
> > if block is bad or reserverved, than warning is printed, than block is
> > erased.
> >
> >
> > > > +                       ret = -EIO;
> > > > +               else
> > > > +                       ret = nanddev_erase(nand, &pos);
> > >
> > > erase opt should already take in account scrub.
> > >
> > > Please extend the problem
> > >
> > > Michael
> > > >                 if (ret) {
> > > >                         einfo->fail_addr = nanddev_pos_to_offs(nand,
> > > > &pos);
> > > >
> > > > --
> > > > 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&amp;data=05%7C01%7Cmikhail.kshevetskiy%40iopsys.eu%7C220e94cc3ad54f5ccca108daa7b44fe6%7C7ff78d652de440f586750569e5c7a65d%7C0%7C0%7C638006690117535825%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=zTEEwU%2BKnLWNDq5xs77MmD6SI%2F31bRf3fEx%2Fyaa65%2FU%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%7C220e94cc3ad54f5ccca108daa7b44fe6%7C7ff78d652de440f586750569e5c7a65d%7C0%7C0%7C638006690117535825%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=zTEEwU%2BKnLWNDq5xs77MmD6SI%2F31bRf3fEx%2Fyaa65%2FU%3D&amp;reserved=0
Michael Nazzareno Trimarchi Oct. 6, 2022, 6:09 p.m. UTC | #4
Hi Mikhail

On Thu, Oct 6, 2022 at 6:17 PM Mikhail Kshevetskiy
<mikhail.kshevetskiy@iopsys.eu> wrote:
>
> On Thu, 6 Oct 2022 18:03:17 +0200
> Michael Nazzareno Trimarchi <michael@amarulasolutions.com> wrote:
>
> > [External email]
> >
> >
> >
> >
> >
> > On Thu, Oct 6, 2022 at 5:52 PM Mikhail Kshevetskiy
> > <mikhail.kshevetskiy@iopsys.eu> wrote:
> > >
> > > On Thu, 6 Oct 2022 08:56:08 +0200
> > > Michael Nazzareno Trimarchi <michael@amarulasolutions.com> wrote:
> > >
> > > > [External email]
> > > >
> > > >
> > > >
> > > >
> > > >
> > > > Hi
> > > >
> > > > On Thu, Oct 6, 2022 at 5:15 AM <mikhail.kshevetskiy@iopsys.eu> wrote:
> > > > >
> > > > > From: Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu>
> > > > >
> > > > > 'mtd erase' command should not erase bad blocks. To force bad block
> > > > > erasing there is 'mtd erase.dontskipbad' command (this command sets
> > > > > 'scrub' flag to true in the erase_info structure). Unfortunately nand
> > > > > layer ignore scrub flag and try to erases bad blocks unconditionally.
> > > > > This is wrong.
> > > > >
> > > > > Add checks to allow bad block erasing only if scrub flag is set.
> > > > >
> > > > > Signed-off-by: Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu>
> > > > > ---
> > > > >  drivers/mtd/nand/core.c | 5 ++++-
> > > > >  1 file changed, 4 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/drivers/mtd/nand/core.c b/drivers/mtd/nand/core.c
> > > > > index 99c29670c7..a4fb7602c9 100644
> > > > > --- a/drivers/mtd/nand/core.c
> > > > > +++ b/drivers/mtd/nand/core.c
> > > > > @@ -174,7 +174,10 @@ int nanddev_mtd_erase(struct mtd_info *mtd, struct
> > > > > erase_info *einfo) nanddev_offs_to_pos(nand, einfo->addr + einfo->len -
> > > > > 1, &last); while (nanddev_pos_cmp(&pos, &last) <= 0) {
> > > > >                 schedule();
> > > > > -               ret = nanddev_erase(nand, &pos);
> > > > > +               if (!einfo->scrub && nanddev_isbad(nand, &pos))
> > > >
> > > > The nandev_erase already check it here:
> > > >
> > > > if (nanddev_isbad(nand, pos) || nanddev_isreserved(nand, pos)) {
> > > >
> > >
> > > no it does not work. see nanddev_erase() code
> > >
> >
> > Let me re-formulate it. What execution path are you taking into account?
> >
> > The nand are erased using the cmd/nand interface and the erase command
> > there calls nand_erase_opts that take in account it.
>
> spi-nand flash
>
> cmd/mtd.c -> do_mtd_erase() -> .... -> nanddev_mtd_erase() -> nanddev_erase()
>

Ok now it's clear. I'm thinking and i have create a patch if we can
use somenthing like this

Not tested


@@ -393,7 +394,7 @@ out_put_mtd:
 static int do_mtd_erase(struct cmd_tbl *cmdtp, int flag, int argc,
                        char *const argv[])
 {
-       struct erase_info erase_op = {};
+       nand_erase_options_t opts = {};
        struct mtd_info *mtd;
        u64 off, len;
        bool scrub;
@@ -431,26 +432,11 @@ static int do_mtd_erase(struct cmd_tbl *cmdtp,
int flag, int argc,
        printf("Erasing 0x%08llx ... 0x%08llx (%d eraseblock(s))\n",
               off, off + len - 1, mtd_div_by_eb(len, mtd));

-       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 (ret) {
-                       /* Abort if its not a bad block error */
-                       if (ret != -EIO)
-                               break;
-                       printf("Skipping bad block at 0x%08llx\n",
-                              erase_op.addr);
-               }
-
-               len -= mtd->erasesize;
-               erase_op.addr += mtd->erasesize;
-       }
+       opts.offset = off;
+       opts.length = len;
+       opts.scrub = scrub;

+       ret = nand_erase_opts(mtd, &opts);
        if (ret && ret != -EIO)
                ret = CMD_RET_FAILURE;
        else

Michael


>
> >
> > Michael
> >
> >
> >
> > The nand is erased using the
> > > if block is bad or reserverved, than warning is printed, than block is
> > > erased.
> > >
> > >
> > > > > +                       ret = -EIO;
> > > > > +               else
> > > > > +                       ret = nanddev_erase(nand, &pos);
> > > >
> > > > erase opt should already take in account scrub.
> > > >
> > > > Please extend the problem
> > > >
> > > > Michael
> > > > >                 if (ret) {
> > > > >                         einfo->fail_addr = nanddev_pos_to_offs(nand,
> > > > > &pos);
> > > > >
> > > > > --
> > > > > 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&amp;data=05%7C01%7Cmikhail.kshevetskiy%40iopsys.eu%7C220e94cc3ad54f5ccca108daa7b44fe6%7C7ff78d652de440f586750569e5c7a65d%7C0%7C0%7C638006690117535825%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=zTEEwU%2BKnLWNDq5xs77MmD6SI%2F31bRf3fEx%2Fyaa65%2FU%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%7C220e94cc3ad54f5ccca108daa7b44fe6%7C7ff78d652de440f586750569e5c7a65d%7C0%7C0%7C638006690117535825%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=zTEEwU%2BKnLWNDq5xs77MmD6SI%2F31bRf3fEx%2Fyaa65%2FU%3D&amp;reserved=0



--
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
michael@amarulasolutions.com
Mikhail Kshevetskiy Oct. 10, 2022, 1:32 p.m. UTC | #5
On Thu, 6 Oct 2022 20:09:11 +0200
Michael Nazzareno Trimarchi <michael@amarulasolutions.com> wrote:

> [External email]
> 
> 
> 
> 
> 
> Hi Mikhail
> 
> On Thu, Oct 6, 2022 at 6:17 PM Mikhail Kshevetskiy
> <mikhail.kshevetskiy@iopsys.eu> wrote:
> >
> > On Thu, 6 Oct 2022 18:03:17 +0200
> > Michael Nazzareno Trimarchi <michael@amarulasolutions.com> wrote:
> >
> > > [External email]
> > >
> > >
> > >
> > >
> > >
> > > On Thu, Oct 6, 2022 at 5:52 PM Mikhail Kshevetskiy
> > > <mikhail.kshevetskiy@iopsys.eu> wrote:
> > > >
> > > > On Thu, 6 Oct 2022 08:56:08 +0200
> > > > Michael Nazzareno Trimarchi <michael@amarulasolutions.com> wrote:
> > > >
> > > > > [External email]
> > > > >
> > > > >
> > > > >
> > > > >
> > > > >
> > > > > Hi
> > > > >
> > > > > On Thu, Oct 6, 2022 at 5:15 AM <mikhail.kshevetskiy@iopsys.eu> wrote:
> > > > > >
> > > > > > From: Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu>
> > > > > >
> > > > > > 'mtd erase' command should not erase bad blocks. To force bad block
> > > > > > erasing there is 'mtd erase.dontskipbad' command (this command sets
> > > > > > 'scrub' flag to true in the erase_info structure). Unfortunately
> > > > > > nand layer ignore scrub flag and try to erases bad blocks
> > > > > > unconditionally. This is wrong.
> > > > > >
> > > > > > Add checks to allow bad block erasing only if scrub flag is set.
> > > > > >
> > > > > > Signed-off-by: Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu>
> > > > > > ---
> > > > > >  drivers/mtd/nand/core.c | 5 ++++-
> > > > > >  1 file changed, 4 insertions(+), 1 deletion(-)
> > > > > >
> > > > > > diff --git a/drivers/mtd/nand/core.c b/drivers/mtd/nand/core.c
> > > > > > index 99c29670c7..a4fb7602c9 100644
> > > > > > --- a/drivers/mtd/nand/core.c
> > > > > > +++ b/drivers/mtd/nand/core.c
> > > > > > @@ -174,7 +174,10 @@ int nanddev_mtd_erase(struct mtd_info *mtd,
> > > > > > struct erase_info *einfo) nanddev_offs_to_pos(nand, einfo->addr +
> > > > > > einfo->len - 1, &last); while (nanddev_pos_cmp(&pos, &last) <= 0) {
> > > > > >                 schedule();
> > > > > > -               ret = nanddev_erase(nand, &pos);
> > > > > > +               if (!einfo->scrub && nanddev_isbad(nand, &pos))
> > > > >
> > > > > The nandev_erase already check it here:
> > > > >
> > > > > if (nanddev_isbad(nand, pos) || nanddev_isreserved(nand, pos)) {
> > > > >
> > > >
> > > > no it does not work. see nanddev_erase() code
> > > >
> > >
> > > Let me re-formulate it. What execution path are you taking into account?
> > >
> > > The nand are erased using the cmd/nand interface and the erase command
> > > there calls nand_erase_opts that take in account it.
> >
> > spi-nand flash
> >
> > cmd/mtd.c -> do_mtd_erase() -> .... -> nanddev_mtd_erase() ->
> > nanddev_erase()
> >
> 
> Ok now it's clear. I'm thinking and i have create a patch if we can
> use somenthing like this
> 
> Not tested
> 
> 
> @@ -393,7 +394,7 @@ out_put_mtd:
>  static int do_mtd_erase(struct cmd_tbl *cmdtp, int flag, int argc,
>                         char *const argv[])
>  {
> -       struct erase_info erase_op = {};
> +       nand_erase_options_t opts = {};
>         struct mtd_info *mtd;
>         u64 off, len;
>         bool scrub;
> @@ -431,26 +432,11 @@ static int do_mtd_erase(struct cmd_tbl *cmdtp,
> int flag, int argc,
>         printf("Erasing 0x%08llx ... 0x%08llx (%d eraseblock(s))\n",
>                off, off + len - 1, mtd_div_by_eb(len, mtd));
> 
> -       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 (ret) {
> -                       /* Abort if its not a bad block error */
> -                       if (ret != -EIO)
> -                               break;
> -                       printf("Skipping bad block at 0x%08llx\n",
> -                              erase_op.addr);
> -               }
> -
> -               len -= mtd->erasesize;
> -               erase_op.addr += mtd->erasesize;
> -       }
> +       opts.offset = off;
> +       opts.length = len;
> +       opts.scrub = scrub;
> 
> +       ret = nand_erase_opts(mtd, &opts);
>         if (ret && ret != -EIO)
>                 ret = CMD_RET_FAILURE;
>         else
> 
> Michael

no, we can't do it.

cmd/mtd.c can be used without CONFIG_MTD_RAW_NAND and I think your fix will
break mtd command for non-nand flashes.

spi-nand flashes directly registers itself with mtd layer (nand_register()
function is not used), so it much more simple use cmd/mtd than cmd/nand.



> 
> 
> >
> > >
> > > Michael
> > >
> > >
> > >
> > > The nand is erased using the
> > > > if block is bad or reserverved, than warning is printed, than block is
> > > > erased.
> > > >
> > > >
> > > > > > +                       ret = -EIO;
> > > > > > +               else
> > > > > > +                       ret = nanddev_erase(nand, &pos);
> > > > >
> > > > > erase opt should already take in account scrub.
> > > > >
> > > > > Please extend the problem
> > > > >
> > > > > Michael
> > > > > >                 if (ret) {
> > > > > >                         einfo->fail_addr = nanddev_pos_to_offs(nand,
> > > > > > &pos);
> > > > > >
> > > > > > --
> > > > > > 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&amp;data=05%7C01%7Cmikhail.kshevetskiy%40iopsys.eu%7C315a2da790414f5cb22108daa7c5e5e2%7C7ff78d652de440f586750569e5c7a65d%7C0%7C0%7C638006765651063852%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=ouhzZ4pEofl7llK8OQHpatkrdksZozIgzY5IY9P%2BIL8%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%7C315a2da790414f5cb22108daa7c5e5e2%7C7ff78d652de440f586750569e5c7a65d%7C0%7C0%7C638006765651063852%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=ouhzZ4pEofl7llK8OQHpatkrdksZozIgzY5IY9P%2BIL8%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%7C315a2da790414f5cb22108daa7c5e5e2%7C7ff78d652de440f586750569e5c7a65d%7C0%7C0%7C638006765651063852%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=ouhzZ4pEofl7llK8OQHpatkrdksZozIgzY5IY9P%2BIL8%3D&amp;reserved=0


Maybe it's better accept my change and fix nand_erase_opts() instead ?
Bad block table is autocleaned in nanddev_erase() function.

diff --git a/drivers/mtd/nand/raw/nand_util.c b/drivers/mtd/nand/raw/nand_util.c
index b2345dca7f..b35571dfd5 100644
--- a/drivers/mtd/nand/raw/nand_util.c
+++ b/drivers/mtd/nand/raw/nand_util.c
@@ -75,6 +75,7 @@ int nand_erase_opts(struct mtd_info *mtd,
 	erase.mtd = mtd;
 	erase.len = mtd->erasesize;
 	erase.addr = opts->offset;
+	erase.scrub = opts->scrub;
 	erase_length = lldiv(opts->length + mtd->erasesize - 1,
 			     mtd->erasesize);
 
@@ -82,23 +83,6 @@ int nand_erase_opts(struct mtd_info *mtd,
 	cleanmarker.nodetype = cpu_to_je16(JFFS2_NODETYPE_CLEANMARKER);
 	cleanmarker.totlen = cpu_to_je32(8);
 
-	/* scrub option allows to erase badblock. To prevent internal
-	 * check from erase() method, set block check method to dummy
-	 * and disable bad block table while erasing.
-	 */
-	if (opts->scrub) {
-		erase.scrub = opts->scrub;
-		/*
-		 * We don't need the bad block table anymore...
-		 * after scrub, there are no bad blocks left!
-		 */
-		if (chip->bbt) {
-			kfree(chip->bbt);
-		}
-		chip->bbt = NULL;
-		chip->options &= ~NAND_BBT_SCANNED;
-	}
-
 	for (erased_length = 0;
 	     erased_length < erase_length;
 	     erase.addr += mtd->erasesize) {
@@ -109,34 +93,24 @@ int nand_erase_opts(struct mtd_info *mtd,
 			puts("Size of erase exceeds limit\n");
 			return -EFBIG;
 		}
-		if (!opts->scrub) {
-			int ret = mtd_block_isbad(mtd, erase.addr);
-			if (ret > 0) {
+
+		erased_length++;
+
+		result = mtd_erase(mtd, &erase);
+		if (result != 0) {
+			if (!opts->scrub && result == -EIO) {
 				if (!opts->quiet)
 					printf("\rSkipping bad block at  "
 					       "0x%08llx                 "
 					       "                         \n",
 					       erase.addr);
 
-				if (!opts->spread)
-					erased_length++;
-
-				continue;
-
-			} else if (ret < 0) {
-				printf("\n%s: MTD get bad block failed: %d\n",
-				       mtd_device,
-				       ret);
-				return -1;
+				if (opts->spread)
+					erased_length--;
+			} else {
+				printf("\n%s: MTD Erase failure: %d\n",
+				       mtd_device, result);
 			}
-		}
-
-		erased_length++;
-
-		result = mtd_erase(mtd, &erase);
-		if (result != 0) {
-			printf("\n%s: MTD Erase failure: %d\n",
-			       mtd_device, result);
 			continue;
 		}
 
---
Mikhail Kshevetskiy
diff mbox series

Patch

diff --git a/drivers/mtd/nand/core.c b/drivers/mtd/nand/core.c
index 99c29670c7..a4fb7602c9 100644
--- a/drivers/mtd/nand/core.c
+++ b/drivers/mtd/nand/core.c
@@ -174,7 +174,10 @@  int nanddev_mtd_erase(struct mtd_info *mtd, struct erase_info *einfo)
 	nanddev_offs_to_pos(nand, einfo->addr + einfo->len - 1, &last);
 	while (nanddev_pos_cmp(&pos, &last) <= 0) {
 		schedule();
-		ret = nanddev_erase(nand, &pos);
+		if (!einfo->scrub && nanddev_isbad(nand, &pos))
+			ret = -EIO;
+		else
+			ret = nanddev_erase(nand, &pos);
 		if (ret) {
 			einfo->fail_addr = nanddev_pos_to_offs(nand, &pos);