diff mbox series

mmc-utils: Add basic erase error check

Message ID ca4d29de093e4e29a2222c4d94a9367f@hyperstone.com (mailing list archive)
State New, archived
Headers show
Series mmc-utils: Add basic erase error check | expand

Commit Message

Christian Loehle Jan. 16, 2023, 10:49 a.m. UTC
Check for erase specific R1 errors so e.g. an OOR erase is not
reported as successful when it never executed.

There could be checks for more error bits but R1_ERASE_SEQ_ERROR
on CMD38 should catch all that are reported by hardware anyway.

Signed-off-by: Christian Loehle <cloehle@hyperstone.com>
---
-v2: Remove unneeded error bit checking
 mmc_cmds.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

Comments

Avri Altman Jan. 16, 2023, 11:10 a.m. UTC | #1
> Check for erase specific R1 errors so e.g. an OOR erase is not reported as
> successful when it never executed.
> 
> There could be checks for more error bits but R1_ERASE_SEQ_ERROR on
> CMD38 should catch all that are reported by hardware anyway.
> 
> Signed-off-by: Christian Loehle <cloehle@hyperstone.com>
> ---
> -v2: Remove unneeded error bit checking
>  mmc_cmds.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/mmc_cmds.c b/mmc_cmds.c
> index e6d3273..97fdb59 100644
> --- a/mmc_cmds.c
> +++ b/mmc_cmds.c
> @@ -54,7 +54,6 @@
>  #define WPTYPE_PWRON 2
>  #define WPTYPE_PERM 3
> 
> -
>  int read_extcsd(int fd, __u8 *ext_csd)
>  {
>         int ret = 0;
> @@ -2668,6 +2667,18 @@ static int erase(int dev_fd, __u32 argin, __u32 start,
> __u32 end)
>         if (ret)
>                 perror("Erase multi-cmd ioctl");
> 
> +       /* Does not work for SPI cards */
> +       if (multi_cmd->cmds[0].response[0] & R1_ERASE_PARAM) {
> +               fprintf(stderr, "Erase start response: 0x%08x\n",
> +                               multi_cmd->cmds[0].response[0]);
> +               ret = -EIO;
> +       }
Your concluding remark in your V1:
" I think ERASE_PARAM should be checked for CMD36, ERASE_SEQ_ERROR for CMD38, I'm fine with removing ERASE_RESET check for the patch, it is caught at CMD38 ERASE_SEQ_ERROR anyway.
What do you say?"

But you are testing R1_ERASE_PARAM in CMD35?
Honestly, I don't really know, just want to verify that you changed your mind.

Thanks,
Avri

> +       if (multi_cmd->cmds[2].response[0] & R1_ERASE_SEQ_ERROR) {
> +               fprintf(stderr, "Erase response: 0x%08x\n",
> +                               multi_cmd->cmds[2].response[0]);
> +               ret = -EIO;
> +       }
> +
>         free(multi_cmd);
>         return ret;
>  }
> --
> 2.37.3
> 
> 
> Hyperstone GmbH | Reichenaustr. 39a  | 78467 Konstanz Managing Director:
> Dr. Jan Peter Berns.
> Commercial register of local courts: Freiburg HRB381782
diff mbox series

Patch

diff --git a/mmc_cmds.c b/mmc_cmds.c
index e6d3273..97fdb59 100644
--- a/mmc_cmds.c
+++ b/mmc_cmds.c
@@ -54,7 +54,6 @@ 
 #define WPTYPE_PWRON 2
 #define WPTYPE_PERM 3
 
-
 int read_extcsd(int fd, __u8 *ext_csd)
 {
 	int ret = 0;
@@ -2668,6 +2667,18 @@  static int erase(int dev_fd, __u32 argin, __u32 start, __u32 end)
 	if (ret)
 		perror("Erase multi-cmd ioctl");
 
+	/* Does not work for SPI cards */
+	if (multi_cmd->cmds[0].response[0] & R1_ERASE_PARAM) {
+		fprintf(stderr, "Erase start response: 0x%08x\n",
+				multi_cmd->cmds[0].response[0]);
+		ret = -EIO;
+	}
+	if (multi_cmd->cmds[2].response[0] & R1_ERASE_SEQ_ERROR) {
+		fprintf(stderr, "Erase response: 0x%08x\n",
+				multi_cmd->cmds[2].response[0]);
+		ret = -EIO;
+	}
+
 	free(multi_cmd);
 	return ret;
 }