| Message ID | 20220408174832.303915-1-huobean@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v1] mmc-utils: Add General command CMD56 read support | expand |
> From: Bean Huo <beanhuo@micron.com> > > Micron eMMC offers an additional set of commands that go beyond the JEDEC > Device Health Report. These additional DEVICE HEALTH commands are > implemented using the generic CMD56 command and they return a significant > amount of useful information about the status of the NAND device. Such as bad > block counters, block erase counters ,etc. For more information, refer to TN-FC- > 32: e·MMC Device Health Report. > > Since the CMD56 is specified in JEDEC, to make CMD56 universal used and let > more users of mmc-utils get the benefit for this. I add CMD56 read, and let the > user to input the CMD56 argument, also, here I didn't add data parsing, just print > raw data, since it is vendor-specific. > > Signed-off-by: Bean Huo <beanhuo@micron.com> > --- > mmc.c | 7 +++++++ > mmc.h | 2 ++ > mmc_cmds.c | 52 > ++++++++++++++++++++++++++++++++++++++++++++++++++++ > mmc_cmds.h | 1 + > 4 files changed, 62 insertions(+) > > diff --git a/mmc.c b/mmc.c > index eb2638b12271..ea465e59fdf3 100644 > --- a/mmc.c > +++ b/mmc.c > @@ -237,6 +237,13 @@ static struct Command commands[] = { > "secure-trim1 | secure-trim2 | trim \n", > NULL > }, > + { do_general_cmd_read, -2, > + "gen_cmd read", "<arg> <device>\n" > + "Send GEN_CMD (CMD56) with specific <arg> to <device> to read > vendor\n" > + "specific data. <arg> must be 32 bits length hex number prefixed with > 0x/0X.\n\n" > + "NOTE!: Because this option is only used to read, the bit0 in <arg> > must be 1", > + NULL > + }, > { 0, 0, 0, 0 } > }; > > diff --git a/mmc.h b/mmc.h > index 25d6864ac76f..b621374a1ed1 100644 > --- a/mmc.h > +++ b/mmc.h > @@ -41,6 +41,8 @@ > [1] Discard Enable > [0] Identify Write Blocks for > Erase (or TRIM Enable) R1b */ > +#define MMC_GEN_CMD 56 /* adtc [31:1] stuff bits. > + [0]: RD/WR1 R1 */ > > #define R1_OUT_OF_RANGE (1 << 31) /* er, c */ > #define R1_ADDRESS_ERROR (1 << 30) /* erx, c */ > diff --git a/mmc_cmds.c b/mmc_cmds.c > index 49d3e324d266..6e006b10d4fd 100644 > --- a/mmc_cmds.c > +++ b/mmc_cmds.c > @@ -2981,3 +2981,55 @@ out: > return ret; > #endif > } > + > +int do_general_cmd_read(int nargs, char **argv) { > + int dev_fd; > + __u8 buf[512]; > + __u32 arg; > + char *device; > + char *endptr; > + int ret = -EINVAL, i; > + struct mmc_ioc_cmd idata; > + > + if (nargs != 3) { > + fprintf(stderr, "Usage: gen_cmd read <arg> </path/to/mmcblkX>\n"); > + exit(1); > + } > + > + device = argv[2]; > + dev_fd = open(device, O_RDWR); > + if (dev_fd < 0) { > + perror("device open failed"); > + exit(1); > + } > + arg = strtol(argv[1], &endptr, 16); > + if (errno != 0 || *endptr != '\0' || !arg&0x1) { > + fprintf(stderr, "Wrong ARG, it should be Hex number and bit0 must be > 1\n"); > + goto out; > + } One line space > + memset(&idata, 0, sizeof(idata)); > + idata.write_flag = 0; > + idata.opcode = MMC_GEN_CMD; > + idata.arg = arg; Practically arg is not needed, because bits[1..31] are meaningless. Maybe just set: idata.arg = 1; Thanks, Avri
On Sat, 2022-04-09 at 09:03 +0000, Avri Altman wrote: > > + memset(&idata, 0, sizeof(idata)); > > + idata.write_flag = 0; > > + idata.opcode = MMC_GEN_CMD; > > + idata.arg = arg; > Practically arg is not needed, because bits[1..31] are meaningless. > Maybe just set: idata.arg = 1; > > Thanks, > Avri Thanks for your review. you are right arg is not needed according to eMMC spec. But as I mentioned in the commit message, for the universal use. we need it for the vendor-specific arg. I will change it that its arg will be 1 by default if the user doesn't specific arg. Kind regards, Bean
> On Sat, 2022-04-09 at 09:03 +0000, Avri Altman wrote: > > > + memset(&idata, 0, sizeof(idata)); > > > + idata.write_flag = 0; > > > + idata.opcode = MMC_GEN_CMD; > > > + idata.arg = arg; > > Practically arg is not needed, because bits[1..31] are meaningless. > > Maybe just set: idata.arg = 1; > > > > Thanks, > > Avri > Thanks for your review. > > you are right arg is not needed according to eMMC spec. But as I mentioned > in the commit message, for the universal use. we need it for the vendor- > specific arg. I will change it that its arg will be 1 by default if the user doesn't > specific arg. Ack. Thanks, Avri > > Kind regards, > Bean > >
Tested-by: Rossler, Jakob (Nokia - DE/Ulm) <jakob.rossler@nokia.com> Best regards Jakob -----Original Message----- From: Avri Altman <Avri.Altman@wdc.com> Sent: Monday, April 11, 2022 12:05 PM To: Bean Huo <huobean@gmail.com>; ulf.hansson@linaro.org; adrian.hunter@intel.com Cc: linux-mmc@vger.kernel.org; linux-kernel@vger.kernel.org; Rossler, Jakob (Nokia - DE/Ulm) <jakob.rossler@nokia.com>; sperson@micron.com; Bean Huo <beanhuo@micron.com> Subject: RE: [PATCH v1] mmc-utils: Add General command CMD56 read support > On Sat, 2022-04-09 at 09:03 +0000, Avri Altman wrote: > > > + memset(&idata, 0, sizeof(idata)); > > > + idata.write_flag = 0; > > > + idata.opcode = MMC_GEN_CMD; > > > + idata.arg = arg; > > Practically arg is not needed, because bits[1..31] are meaningless. > > Maybe just set: idata.arg = 1; > > > > Thanks, > > Avri > Thanks for your review. > > you are right arg is not needed according to eMMC spec. But as I > mentioned in the commit message, for the universal use. we need it for > the vendor- specific arg. I will change it that its arg will be 1 by > default if the user doesn't specific arg. Ack. Thanks, Avri > > Kind regards, > Bean > >
diff --git a/mmc.c b/mmc.c index eb2638b12271..ea465e59fdf3 100644 --- a/mmc.c +++ b/mmc.c @@ -237,6 +237,13 @@ static struct Command commands[] = { "secure-trim1 | secure-trim2 | trim \n", NULL }, + { do_general_cmd_read, -2, + "gen_cmd read", "<arg> <device>\n" + "Send GEN_CMD (CMD56) with specific <arg> to <device> to read vendor\n" + "specific data. <arg> must be 32 bits length hex number prefixed with 0x/0X.\n\n" + "NOTE!: Because this option is only used to read, the bit0 in <arg> must be 1", + NULL + }, { 0, 0, 0, 0 } }; diff --git a/mmc.h b/mmc.h index 25d6864ac76f..b621374a1ed1 100644 --- a/mmc.h +++ b/mmc.h @@ -41,6 +41,8 @@ [1] Discard Enable [0] Identify Write Blocks for Erase (or TRIM Enable) R1b */ +#define MMC_GEN_CMD 56 /* adtc [31:1] stuff bits. + [0]: RD/WR1 R1 */ #define R1_OUT_OF_RANGE (1 << 31) /* er, c */ #define R1_ADDRESS_ERROR (1 << 30) /* erx, c */ diff --git a/mmc_cmds.c b/mmc_cmds.c index 49d3e324d266..6e006b10d4fd 100644 --- a/mmc_cmds.c +++ b/mmc_cmds.c @@ -2981,3 +2981,55 @@ out: return ret; #endif } + +int do_general_cmd_read(int nargs, char **argv) +{ + int dev_fd; + __u8 buf[512]; + __u32 arg; + char *device; + char *endptr; + int ret = -EINVAL, i; + struct mmc_ioc_cmd idata; + + if (nargs != 3) { + fprintf(stderr, "Usage: gen_cmd read <arg> </path/to/mmcblkX>\n"); + exit(1); + } + + device = argv[2]; + dev_fd = open(device, O_RDWR); + if (dev_fd < 0) { + perror("device open failed"); + exit(1); + } + arg = strtol(argv[1], &endptr, 16); + if (errno != 0 || *endptr != '\0' || !arg&0x1) { + fprintf(stderr, "Wrong ARG, it should be Hex number and bit0 must be 1\n"); + goto out; + } + memset(&idata, 0, sizeof(idata)); + idata.write_flag = 0; + idata.opcode = MMC_GEN_CMD; + idata.arg = arg; + idata.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_ADTC; + idata.blksz = 512; + idata.blocks = 1; + mmc_ioc_cmd_set_data(idata, buf); + + ret = ioctl(dev_fd, MMC_IOC_CMD, &idata); + if (ret) { + perror("ioctl"); + goto out; + } + + printf("Data:\n"); + for (i = 0; i < 512; i++) { + printf("%2x ", buf[i]); + if ((i + 1) % 16 == 0) + printf("\n"); + } +out: + close(dev_fd); + return ret; +} diff --git a/mmc_cmds.h b/mmc_cmds.h index 8331ab2373fd..0f7c0041f753 100644 --- a/mmc_cmds.h +++ b/mmc_cmds.h @@ -46,3 +46,4 @@ int do_read_scr(int argc, char **argv); 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);