| Message ID | 20230522215310.2038669-1-ejo@pengutronix.de |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] mmc-utils: introduce optional --verify argument for 'extcsd write' | expand |
-----Original Message----- From: Enrico Jorns <ejo@pengutronix.de> Sent: Monday, May 22, 2023 11:53 PM To: linux-mmc@vger.kernel.org Cc: Avri Altman <avri.altman@wdc.com>; Ulf Hansson <ulf.hansson@linaro.org>; ejo@pengutronix.de Subject: [PATCH 1/2] mmc-utils: introduce optional --verify argument for 'extcsd write' > Registers can be write-once but ioctl does not necessarily return with an error. Thus it is a good idea to allow verifying the data written. But it will return with an error soon, I'm working on it, anyway I think it's not a bad idea. User needs to be aware that not all indices can be '--verify'ed, though. > > Signed-off-by: Enrico Jorns <ejo@pengutronix.de> > --- > mmc.c | 7 ++++--- > mmc_cmds.c | 28 ++++++++++++++++++++++++++-- > 2 files changed, 30 insertions(+), 5 deletions(-) > > diff --git a/mmc.c b/mmc.c > index 795b4e3..3f813b4 100644 > --- a/mmc.c > +++ b/mmc.c > @@ -56,9 +56,10 @@ static struct Command commands[] = { > "Print extcsd data from <device>.", > NULL > }, > - { do_write_extcsd, 3, > - "extcsd write", "<offset> <value> <device>\n" > - "Write <value> at offset <offset> to <device>'s extcsd.", > + { do_write_extcsd, -3, > + "extcsd write", "<offset> <value> <device> [--verify]\n" > + "Write <value> at offset <offset> to <device>'s extcsd.\n" > + " --verify Verify data written", > NULL > }, > { do_writeprotect_boot_get, -1, > diff --git a/mmc_cmds.c b/mmc_cmds.c > index df66986..154020e 100644 > --- a/mmc_cmds.c > +++ b/mmc_cmds.c > @@ -1986,9 +1986,10 @@ int do_write_extcsd(int nargs, char **argv) > int fd, ret; > int offset, value; > char *device; > + int verify = 0; > > - if (nargs != 4) { > - fprintf(stderr, "Usage: mmc extcsd write <offset> <value> </path/to/mmcblkX>\n"); > + if (nargs != 4 && nargs != 5) { > + fprintf(stderr, "Usage: mmc extcsd write <offset> <value> > +</path/to/mmcblkX> [--verify]\n"); > exit(1); > } > > @@ -1996,6 +1997,14 @@ int do_write_extcsd(int nargs, char **argv) > value = strtol(argv[2], NULL, 0); > device = argv[3]; > > + if (nargs == 5) { > + if (strcmp(argv[4], "--verify") == 0) { > + verify = 1; > + } else { > + fprintf(stderr, "Unknown argument: '%s'\n", argv[4]); > + } > + } > + > fd = open(device, O_RDWR); > if (fd < 0) { > perror("open"); > @@ -2010,6 +2019,21 @@ int do_write_extcsd(int nargs, char **argv) > exit(1); > } > > + if (verify) { > + __u8 ext_csd[512]; > + > + ret = read_extcsd(fd, ext_csd); > + if (ret) { > + fprintf(stderr, "Could not read EXT_CSD from %s\n", device); I think this error message should indicate that the write already happened. > + exit(1); > + } > + > + if (ext_csd[offset] != value) { > + fprintf(stderr, "Verification failed: expected 0x%x, got 0x%x\n", value, ext_csd[offset]); > + exit(1); > + } > + } > + > return ret; > } > > -- > 2.39.2 > Hyperstone GmbH | Reichenaustr. 39a | 78467 Konstanz Managing Director: Dr. Jan Peter Berns. Commercial register of local courts: Freiburg HRB381782
> Registers can be write-once but ioctl does not necessarily return with an > error. Thus it is a good idea to allow verifying the data written. Yeah - I find this approach more practical, than e.g. analyze the R1b response. > > Signed-off-by: Enrico Jorns <ejo@pengutronix.de> Aked-by: Avri Altman <avri.altman@wdc.com> > --- > mmc.c | 7 ++++--- > mmc_cmds.c | 28 ++++++++++++++++++++++++++-- > 2 files changed, 30 insertions(+), 5 deletions(-) > > diff --git a/mmc.c b/mmc.c > index 795b4e3..3f813b4 100644 > --- a/mmc.c > +++ b/mmc.c > @@ -56,9 +56,10 @@ static struct Command commands[] = { > "Print extcsd data from <device>.", > NULL > }, > - { do_write_extcsd, 3, > - "extcsd write", "<offset> <value> <device>\n" > - "Write <value> at offset <offset> to <device>'s extcsd.", > + { do_write_extcsd, -3, > + "extcsd write", "<offset> <value> <device> [--verify]\n" > + "Write <value> at offset <offset> to <device>'s extcsd.\n" > + " --verify Verify data written", > NULL > }, > { do_writeprotect_boot_get, -1, > diff --git a/mmc_cmds.c b/mmc_cmds.c > index df66986..154020e 100644 > --- a/mmc_cmds.c > +++ b/mmc_cmds.c > @@ -1986,9 +1986,10 @@ int do_write_extcsd(int nargs, char **argv) > int fd, ret; > int offset, value; > char *device; > + int verify = 0; > > - if (nargs != 4) { > - fprintf(stderr, "Usage: mmc extcsd write <offset> <value> > </path/to/mmcblkX>\n"); > + if (nargs != 4 && nargs != 5) { > + fprintf(stderr, "Usage: mmc extcsd write <offset> > + <value> </path/to/mmcblkX> [--verify]\n"); > exit(1); > } > > @@ -1996,6 +1997,14 @@ int do_write_extcsd(int nargs, char **argv) > value = strtol(argv[2], NULL, 0); > device = argv[3]; > > + if (nargs == 5) { > + if (strcmp(argv[4], "--verify") == 0) { > + verify = 1; > + } else { > + fprintf(stderr, "Unknown argument: '%s'\n", argv[4]); > + } > + } > + > fd = open(device, O_RDWR); > if (fd < 0) { > perror("open"); > @@ -2010,6 +2019,21 @@ int do_write_extcsd(int nargs, char **argv) > exit(1); > } > > + if (verify) { > + __u8 ext_csd[512]; > + > + ret = read_extcsd(fd, ext_csd); > + if (ret) { > + fprintf(stderr, "Could not read EXT_CSD from %s\n", device); > + exit(1); > + } > + > + if (ext_csd[offset] != value) { > + fprintf(stderr, "Verification failed: expected 0x%x, got 0x%x\n", > value, ext_csd[offset]); > + exit(1); > + } > + } > + > return ret; > } > > -- > 2.39.2
diff --git a/mmc.c b/mmc.c index 795b4e3..3f813b4 100644 --- a/mmc.c +++ b/mmc.c @@ -56,9 +56,10 @@ static struct Command commands[] = { "Print extcsd data from <device>.", NULL }, - { do_write_extcsd, 3, - "extcsd write", "<offset> <value> <device>\n" - "Write <value> at offset <offset> to <device>'s extcsd.", + { do_write_extcsd, -3, + "extcsd write", "<offset> <value> <device> [--verify]\n" + "Write <value> at offset <offset> to <device>'s extcsd.\n" + " --verify Verify data written", NULL }, { do_writeprotect_boot_get, -1, diff --git a/mmc_cmds.c b/mmc_cmds.c index df66986..154020e 100644 --- a/mmc_cmds.c +++ b/mmc_cmds.c @@ -1986,9 +1986,10 @@ int do_write_extcsd(int nargs, char **argv) int fd, ret; int offset, value; char *device; + int verify = 0; - if (nargs != 4) { - fprintf(stderr, "Usage: mmc extcsd write <offset> <value> </path/to/mmcblkX>\n"); + if (nargs != 4 && nargs != 5) { + fprintf(stderr, "Usage: mmc extcsd write <offset> <value> </path/to/mmcblkX> [--verify]\n"); exit(1); } @@ -1996,6 +1997,14 @@ int do_write_extcsd(int nargs, char **argv) value = strtol(argv[2], NULL, 0); device = argv[3]; + if (nargs == 5) { + if (strcmp(argv[4], "--verify") == 0) { + verify = 1; + } else { + fprintf(stderr, "Unknown argument: '%s'\n", argv[4]); + } + } + fd = open(device, O_RDWR); if (fd < 0) { perror("open"); @@ -2010,6 +2019,21 @@ int do_write_extcsd(int nargs, char **argv) exit(1); } + if (verify) { + __u8 ext_csd[512]; + + ret = read_extcsd(fd, ext_csd); + if (ret) { + fprintf(stderr, "Could not read EXT_CSD from %s\n", device); + exit(1); + } + + if (ext_csd[offset] != value) { + fprintf(stderr, "Verification failed: expected 0x%x, got 0x%x\n", value, ext_csd[offset]); + exit(1); + } + } + return ret; }
Registers can be write-once but ioctl does not necessarily return with an error. Thus it is a good idea to allow verifying the data written. Signed-off-by: Enrico Jorns <ejo@pengutronix.de> --- mmc.c | 7 ++++--- mmc_cmds.c | 28 ++++++++++++++++++++++++++-- 2 files changed, 30 insertions(+), 5 deletions(-)