cmd: mtd: check if a block has to be skipped or erased
diff mbox series

Message ID 20221021152929.3598415-1-dario.binacchi@amarulasolutions.com
State New
Headers show
Series
  • cmd: mtd: check if a block has to be skipped or erased
Related show

Commit Message

Dario Binacchi Oct. 21, 2022, 3:29 p.m. UTC 
As reported by patch [1], the `mtd erase' command should not erase bad
blocks.
To force bad block erasing you have to use the `mtd erase.dontskipbad'
command.

This patch tries to fix the same issue without modifying code taken
from the linux kernel, in order to make further upgrades easier.

[1] https://lore.kernel.org/all/20221006031501.110290-2-mikhail.kshevetskiy@iopsys.eu/
Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
Suggested-by: Michael Trimarchi <michael@amarulasolutions.com>
Co-developed-by: Michael Trimarchi <michael@amarulasolutions.com>
Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
Cc: Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu>

---

 cmd/mtd.c | 28 ++++++++++++++++++++--------
 1 file changed, 20 insertions(+), 8 deletions(-)

Comments

'Krzysztof Kozlowski' via Amarula Linux Oct. 24, 2022, 6:04 a.m. UTC | #1 
On 21.10.2022 18:29, Dario Binacchi wrote:
> [External email]
>
>
>
>
>
> As reported by patch [1], the `mtd erase' command should not erase bad
> blocks.
> To force bad block erasing you have to use the `mtd erase.dontskipbad'
> command.
>
> This patch tries to fix the same issue without modifying code taken
> from the linux kernel, in order to make further upgrades easier.
>
> [1] https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fall%2F20221006031501.110290-2-mikhail.kshevetskiy%40iopsys.eu%2F&amp;data=05%7C01%7Cmikhail.kshevetskiy%40iopsys.eu%7C914960325cd74fb7338208dab37913e4%7C7ff78d652de440f586750569e5c7a65d%7C0%7C0%7C638019629853388501%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=6WBTjvbfll%2Boiz5B8q0o%2Bec0bWKCBf5RimWU1hZe61E%3D&amp;reserved=0
> Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
> Suggested-by: Michael Trimarchi <michael@amarulasolutions.com>
> Co-developed-by: Michael Trimarchi <michael@amarulasolutions.com>
> Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
> Cc: Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu>
>
> ---
>
>   cmd/mtd.c | 28 ++++++++++++++++++++--------
>   1 file changed, 20 insertions(+), 8 deletions(-)
>
> diff --git a/cmd/mtd.c b/cmd/mtd.c
> index ad5cc9827d55..3330a428c018 100644
> --- a/cmd/mtd.c
> +++ b/cmd/mtd.c
> @@ -434,19 +434,31 @@ 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;
> +                       }
>
> -               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);
> +                       if (ret > 0) {
> +                               printf("Skipping bad block at 0x%08llx\n",
> +                                      erase_op.addr);
> +                               ret = 0;
> +                               len -= mtd->erasesize;
> +                               erase_op.addr += mtd->erasesize;
> +                               continue;
> +                       }
>                  }
>
> +               ret = mtd_erase(mtd, &erase_op);
> +               if (ret)
> +                       break;
> +

You should check for bad block here, otherwise  bad block not marked in 
BBT will terminate erasing with CMD_RET_SUCCESS result.

>                  len -= mtd->erasesize;
>                  erase_op.addr += mtd->erasesize;
>          }
> --
> 2.32.0
>
'Krzysztof Kozlowski' via Amarula Linux Oct. 24, 2022, 9:12 a.m. UTC | #2 
Signed-off-by: Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu>

On 21.10.2022 18:29, Dario Binacchi wrote:
> [External email]
>
>
>
>
>
> As reported by patch [1], the `mtd erase' command should not erase bad
> blocks.
> To force bad block erasing you have to use the `mtd erase.dontskipbad'
> command.
>
> This patch tries to fix the same issue without modifying code taken
> from the linux kernel, in order to make further upgrades easier.
>
> [1] https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fall%2F20221006031501.110290-2-mikhail.kshevetskiy%40iopsys.eu%2F&amp;data=05%7C01%7Cmikhail.kshevetskiy%40iopsys.eu%7C914960325cd74fb7338208dab37913e4%7C7ff78d652de440f586750569e5c7a65d%7C0%7C0%7C638019629853388501%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=6WBTjvbfll%2Boiz5B8q0o%2Bec0bWKCBf5RimWU1hZe61E%3D&amp;reserved=0
> Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
> Suggested-by: Michael Trimarchi <michael@amarulasolutions.com>
> Co-developed-by: Michael Trimarchi <michael@amarulasolutions.com>
> Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
> Cc: Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu>
>
> ---
>
>  cmd/mtd.c | 28 ++++++++++++++++++++--------
>  1 file changed, 20 insertions(+), 8 deletions(-)
>
> diff --git a/cmd/mtd.c b/cmd/mtd.c
> index ad5cc9827d55..3330a428c018 100644
> --- a/cmd/mtd.c
> +++ b/cmd/mtd.c
> @@ -434,19 +434,31 @@ 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;
> +                       }
>
> -               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);
> +                       if (ret > 0) {
> +                               printf("Skipping bad block at 0x%08llx\n",
> +                                      erase_op.addr);
> +                               ret = 0;
> +                               len -= mtd->erasesize;
> +                               erase_op.addr += mtd->erasesize;
> +                               continue;
> +                       }
>                 }
>
> +               ret = mtd_erase(mtd, &erase_op);
> +               if (ret)
> +                       break;
> +
>                 len -= mtd->erasesize;
>                 erase_op.addr += mtd->erasesize;
>         }
> --
> 2.32.0
>

Patch
diff mbox series 

diff --git a/cmd/mtd.c b/cmd/mtd.c
index ad5cc9827d55..3330a428c018 100644
--- a/cmd/mtd.c
+++ b/cmd/mtd.c
@@ -434,19 +434,31 @@  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;
+			}
 
-		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);
+			if (ret > 0) {
+				printf("Skipping bad block at 0x%08llx\n",
+				       erase_op.addr);
+				ret = 0;
+				len -= mtd->erasesize;
+				erase_op.addr += mtd->erasesize;
+				continue;
+			}
 		}
 
+		ret = mtd_erase(mtd, &erase_op);
+		if (ret)
+			break;
+
 		len -= mtd->erasesize;
 		erase_op.addr += mtd->erasesize;
 	}