RE: [PATCH v1] mmc-utils: Add General command CMD56 read support

[Avri Altman]

> From: Bean Huo <beanhuo@xxxxxxxxxx>
> 
> 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@xxxxxxxxxx>
> ---
>  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