diff mbox series

mmc-utils: Add softreset command for issuing CMD0

Message ID fee2ce51a200442f90bea0668466a2b1@hyperstone.com (mailing list archive)
State New, archived
Headers show
Series mmc-utils: Add softreset command for issuing CMD0 | expand

Commit Message

Christian Loehle July 18, 2022, 12:17 p.m. UTC
CMD0 may be used to see if the hardware can handle a UHS card
that completed the voltage switch. If a UHS card has problems
coming back up after CMD0 your hardware may not support a hard
reset properly.

Signed-off-by: Christian Loehle <cloehle@hyperstone.com>
---
 mmc.c      |  5 +++++
 mmc.h      |  3 +++
 mmc_cmds.c | 30 ++++++++++++++++++++++++++++++
 mmc_cmds.h |  1 +
 4 files changed, 39 insertions(+)

Comments

Avri Altman July 18, 2022, 12:24 p.m. UTC | #1
Hi,

> 
> CMD0 may be used to see if the hardware can handle a UHS card
> that completed the voltage switch. If a UHS card has problems
> coming back up after CMD0 your hardware may not support a hard
> reset properly.
I think it's a useful addition, not just for that reason.
Specifically to test hw-reset support, I think you should use the GO_PRE_IDLE_STATE option.

> 
> Signed-off-by: Christian Loehle <cloehle@hyperstone.com>
> ---
>  mmc.c      |  5 +++++
>  mmc.h      |  3 +++
>  mmc_cmds.c | 30 ++++++++++++++++++++++++++++++
>  mmc_cmds.h |  1 +
>  4 files changed, 39 insertions(+)
> 
> diff --git a/mmc.c b/mmc.c
> index 6c56387..ba2c883 100644
> --- a/mmc.c
> +++ b/mmc.c
> @@ -245,6 +245,11 @@ static struct Command commands[] = {
>                 "be 1.",
>         NULL
>         },
> +       { do_soft_reset, -1,
> +         "softreset", "<device>\n"
> +         "Issues a CMD0 softreset, e.g. for testing if hardware reset for UHS
> works\n\n",
> +         NULL
> +       },
>         { 0, 0, 0, 0 }
>  };
> 
> diff --git a/mmc.h b/mmc.h
> index daff62c..9796d2e 100644
> --- a/mmc.h
> +++ b/mmc.h
> @@ -21,6 +21,7 @@
>  #include <linux/mmc/ioctl.h>
> 
>  /* From kernel linux/mmc/mmc.h */
> +#define MMC_GO_IDLE_STATE         0   /* bc                          */
>  #define MMC_SWITCH             6       /* ac   [31:0] See below        R1b */
>  #define MMC_SEND_EXT_CSD       8       /* adtc                         R1  */
>  #define MMC_SEND_STATUS                13      /* ac   [31:16] RCA        R1  */
> @@ -226,6 +227,7 @@
> 
> 
>  /* From kernel linux/mmc/core.h */
> +#define MMC_RSP_NONE   0                       /* no response */
>  #define MMC_RSP_PRESENT        (1 << 0)
>  #define MMC_RSP_136    (1 << 1)                /* 136 bit response */
>  #define MMC_RSP_CRC    (1 << 2)                /* expect valid crc */
> @@ -234,6 +236,7 @@
> 
>  #define MMC_CMD_AC     (0 << 5)
>  #define MMC_CMD_ADTC   (1 << 5)
> +#define MMC_CMD_BC     (2 << 5)
> 
>  #define MMC_RSP_SPI_S1 (1 << 7)                /* one status byte */
>  #define MMC_RSP_SPI_BUSY (1 << 10)             /* card may send busy */
> diff --git a/mmc_cmds.c b/mmc_cmds.c
> index 12b7802..c027cfa 100644
> --- a/mmc_cmds.c
> +++ b/mmc_cmds.c
> @@ -3039,3 +3039,33 @@ out:
>         close(dev_fd);
>         return ret;
>  }
> +
> +int do_soft_reset(int nargs, char **argv)
> +{
> +       int fd;
> +       char *device;
> +       struct mmc_ioc_cmd idata;
> +
> +       if (nargs != 2) {
> +               fprintf(stderr, "Usage: mmc status softreset
> </path/to/mmcblkX>\n");
> +               exit(1);
> +       }
> +
> +       device = argv[1];
> +
> +       fd = open(device, O_RDWR);
> +       if (fd < 0) {
> +               perror("open");
> +               exit(1);
> +       }
> +
> +       memset(&idata, 0, sizeof(idata));
> +       idata.opcode = MMC_GO_IDLE_STATE;
> +       idata.flags = MMC_RSP_NONE | MMC_CMD_BC;
How about adding all 3 flavors of soft-reset: GO_IDLE_STATE, GO_PRE_IDLE_STATE, and BOOT_INITIATION.
There can be 3 different command, that calls the same __soft_reset helper, or whatever.
What do you think?  

Thanks,
Avri

> +
> +       /* No need to check for error, it is expected */
> +       ioctl(fd, MMC_IOC_CMD, &idata);
> +       close(fd);
> +
> +       return 0;
> +}
> diff --git a/mmc_cmds.h b/mmc_cmds.h
> index 0f7c004..c112a95 100644
> --- a/mmc_cmds.h
> +++ b/mmc_cmds.h
> @@ -47,3 +47,4 @@ int do_read_cid(int argc, char **argv);
>  int do_read_csd(int argc, char **argv);
>  int do_erase(int nargs, char **argv);
>  int do_general_cmd_read(int nargs, char **argv);
> +int do_soft_reset(int nargs, char **argv);
> --
> 2.36.1
> 
> Hyperstone GmbH | Reichenaustr. 39a  | 78467 Konstanz
> Managing Director: Dr. Jan Peter Berns.
> Commercial register of local courts: Freiburg HRB381782
Christian Loehle July 19, 2022, 8:07 a.m. UTC | #2
>> CMD0 may be used to see if the hardware can handle a UHS card
>> that completed the voltage switch. If a UHS card has problems
>> coming back up after CMD0 your hardware may not support a hard
>> reset properly.
>I think it's a useful addition, not just for that reason.
>Specifically to test hw-reset support, I think you should use the GO_PRE_IDLE_STATE option.

Makes sense to add both, I can do that.

>
>> +
>> +       memset(&idata, 0, sizeof(idata));
>> +       idata.opcode = MMC_GO_IDLE_STATE;
>> +       idata.flags = MMC_RSP_NONE | MMC_CMD_BC;
>How about adding all 3 flavors of soft-reset: GO_IDLE_STATE, GO_PRE_IDLE_STATE, and BOOT_INITIATION.
>There can be 3 different command, that calls the same __soft_reset helper, or whatever.
>What do you think?  

I'm fine with GO_IDLE_STATE and GO_PRE_IDLE_STATE, but BOOT_INITATION Im not sure about, what purpose would it serve?
If the intention is to be able to get the boot operation done with multi_cmd of GO_PRE_IDLE_STATE and BOOT_INITATION and return the data then that would at least be some more testing effort before I resend.
I have only done the boot operations completely in the driver so far.
Furthermore I think many (most?) host controllers (or our drivers) seem to have issues with the transfer size I'd expect of a normal boot operation (i.e 4-32MB).

Hyperstone GmbH | Reichenaustr. 39a  | 78467 Konstanz
Managing Director: Dr. Jan Peter Berns.
Commercial register of local courts: Freiburg HRB381782
Avri Altman July 19, 2022, 8:42 a.m. UTC | #3
> 
> >> CMD0 may be used to see if the hardware can handle a UHS card that
> >> completed the voltage switch. If a UHS card has problems coming back
> >> up after CMD0 your hardware may not support a hard reset properly.
> >I think it's a useful addition, not just for that reason.
> >Specifically to test hw-reset support, I think you should use the
> GO_PRE_IDLE_STATE option.
> 
> Makes sense to add both, I can do that.
> 
> >
> >> +
> >> +       memset(&idata, 0, sizeof(idata));
> >> +       idata.opcode = MMC_GO_IDLE_STATE;
> >> +       idata.flags = MMC_RSP_NONE | MMC_CMD_BC;
> >How about adding all 3 flavors of soft-reset: GO_IDLE_STATE,
> GO_PRE_IDLE_STATE, and BOOT_INITIATION.
> >There can be 3 different command, that calls the same __soft_reset helper,
> or whatever.
> >What do you think?
> 
> I'm fine with GO_IDLE_STATE and GO_PRE_IDLE_STATE, but
> BOOT_INITATION Im not sure about, what purpose would it serve?
We can move with these 2 to start with.

> If the intention is to be able to get the boot operation done with multi_cmd
> of GO_PRE_IDLE_STATE and BOOT_INITATION and return the data then that
> would at least be some more testing effort before I resend.
> I have only done the boot operations completely in the driver so far.
> Furthermore I think many (most?) host controllers (or our drivers) seem to
> have issues with the transfer size I'd expect of a normal boot operation (i.e 4-
> 32MB).
You can add it later (or not) when you find the time.

Thanks,
Avri

> 
> 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.c b/mmc.c
index 6c56387..ba2c883 100644
--- a/mmc.c
+++ b/mmc.c
@@ -245,6 +245,11 @@  static struct Command commands[] = {
 		"be 1.",
 	NULL
 	},
+	{ do_soft_reset, -1,
+	  "softreset", "<device>\n"
+	  "Issues a CMD0 softreset, e.g. for testing if hardware reset for UHS works\n\n",
+	  NULL
+	},
 	{ 0, 0, 0, 0 }
 };
 
diff --git a/mmc.h b/mmc.h
index daff62c..9796d2e 100644
--- a/mmc.h
+++ b/mmc.h
@@ -21,6 +21,7 @@ 
 #include <linux/mmc/ioctl.h>
 
 /* From kernel linux/mmc/mmc.h */
+#define MMC_GO_IDLE_STATE         0   /* bc                          */
 #define MMC_SWITCH		6	/* ac	[31:0] See below	R1b */
 #define MMC_SEND_EXT_CSD	8	/* adtc				R1  */
 #define MMC_SEND_STATUS		13	/* ac   [31:16] RCA        R1  */
@@ -226,6 +227,7 @@ 
 
 
 /* From kernel linux/mmc/core.h */
+#define MMC_RSP_NONE	0			/* no response */
 #define MMC_RSP_PRESENT	(1 << 0)
 #define MMC_RSP_136	(1 << 1)		/* 136 bit response */
 #define MMC_RSP_CRC	(1 << 2)		/* expect valid crc */
@@ -234,6 +236,7 @@ 
 
 #define MMC_CMD_AC	(0 << 5)
 #define MMC_CMD_ADTC	(1 << 5)
+#define MMC_CMD_BC	(2 << 5)
 
 #define MMC_RSP_SPI_S1	(1 << 7)		/* one status byte */
 #define MMC_RSP_SPI_BUSY (1 << 10)		/* card may send busy */
diff --git a/mmc_cmds.c b/mmc_cmds.c
index 12b7802..c027cfa 100644
--- a/mmc_cmds.c
+++ b/mmc_cmds.c
@@ -3039,3 +3039,33 @@  out:
 	close(dev_fd);
 	return ret;
 }
+
+int do_soft_reset(int nargs, char **argv)
+{
+	int fd;
+	char *device;
+	struct mmc_ioc_cmd idata;
+
+	if (nargs != 2) {
+		fprintf(stderr, "Usage: mmc status softreset </path/to/mmcblkX>\n");
+		exit(1);
+	}
+
+	device = argv[1];
+
+	fd = open(device, O_RDWR);
+	if (fd < 0) {
+		perror("open");
+		exit(1);
+	}
+
+	memset(&idata, 0, sizeof(idata));
+	idata.opcode = MMC_GO_IDLE_STATE;
+	idata.flags = MMC_RSP_NONE | MMC_CMD_BC;
+
+	/* No need to check for error, it is expected */
+	ioctl(fd, MMC_IOC_CMD, &idata);
+	close(fd);
+
+	return 0;
+}
diff --git a/mmc_cmds.h b/mmc_cmds.h
index 0f7c004..c112a95 100644
--- a/mmc_cmds.h
+++ b/mmc_cmds.h
@@ -47,3 +47,4 @@  int do_read_cid(int argc, char **argv);
 int do_read_csd(int argc, char **argv);
 int do_erase(int nargs, char **argv);
 int do_general_cmd_read(int nargs, char **argv);
+int do_soft_reset(int nargs, char **argv);