| Message ID | e2b6431104a6a822840d4e7b984d7aa23048a0af.1539103468.git.jamesnuss@nanometrics.ca (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | fix bugs in write_reliability and enh_area set commands + more extcsd parsing | expand |
> +++ b/mmc_cmds.c > @@ -1758,8 +1758,15 @@ int do_read_extcsd(int nargs, char **argv) > } > > if (ext_csd_rev >= 7) { > - printf("eMMC Firmware Version: %s\n", > - (char*)&ext_csd[EXT_CSD_FIRMWARE_VERSION]); > + printf("Firmware Version: > 0x%02x%02x%02x%02x%02x%02x%02x%02x\n", > + ext_csd[EXT_CSD_FIRMWARE_VERSION_7], > + ext_csd[EXT_CSD_FIRMWARE_VERSION_6], > + ext_csd[EXT_CSD_FIRMWARE_VERSION_5], > + ext_csd[EXT_CSD_FIRMWARE_VERSION_4], > + ext_csd[EXT_CSD_FIRMWARE_VERSION_3], > + ext_csd[EXT_CSD_FIRMWARE_VERSION_2], > + ext_csd[EXT_CSD_FIRMWARE_VERSION_1], > + ext_csd[EXT_CSD_FIRMWARE_VERSION_0]); ExtCSD[261:254] is an ASCII string, just add a terminating null. Thanks, Avri
On Wed, Oct 10, 2018 at 4:43 AM Avri Altman <Avri.Altman@wdc.com> wrote: > > > > +++ b/mmc_cmds.c > > @@ -1758,8 +1758,15 @@ int do_read_extcsd(int nargs, char **argv) > > } > > > > if (ext_csd_rev >= 7) { > > - printf("eMMC Firmware Version: %s\n", > > - (char*)&ext_csd[EXT_CSD_FIRMWARE_VERSION]); > > + printf("Firmware Version: > > 0x%02x%02x%02x%02x%02x%02x%02x%02x\n", > > + ext_csd[EXT_CSD_FIRMWARE_VERSION_7], > > + ext_csd[EXT_CSD_FIRMWARE_VERSION_6], > > + ext_csd[EXT_CSD_FIRMWARE_VERSION_5], > > + ext_csd[EXT_CSD_FIRMWARE_VERSION_4], > > + ext_csd[EXT_CSD_FIRMWARE_VERSION_3], > > + ext_csd[EXT_CSD_FIRMWARE_VERSION_2], > > + ext_csd[EXT_CSD_FIRMWARE_VERSION_1], > > + ext_csd[EXT_CSD_FIRMWARE_VERSION_0]); > ExtCSD[261:254] is an ASCII string, just add a terminating null. Unfortunately I found two different manufacturers which put non-printable characters in this 8-byte field. So I don't think it can be treated as ASCII in all cases. Printing out the hex value seemed liked the most comprehensive solution. > > Thanks, > Avri > --
> > On Wed, Oct 10, 2018 at 4:43 AM Avri Altman <Avri.Altman@wdc.com> wrote: > > > > > > > +++ b/mmc_cmds.c > > > @@ -1758,8 +1758,15 @@ int do_read_extcsd(int nargs, char **argv) > > > } > > > > > > if (ext_csd_rev >= 7) { > > > - printf("eMMC Firmware Version: %s\n", > > > - (char*)&ext_csd[EXT_CSD_FIRMWARE_VERSION]); > > > + printf("Firmware Version: > > > 0x%02x%02x%02x%02x%02x%02x%02x%02x\n", > > > + ext_csd[EXT_CSD_FIRMWARE_VERSION_7], > > > + ext_csd[EXT_CSD_FIRMWARE_VERSION_6], > > > + ext_csd[EXT_CSD_FIRMWARE_VERSION_5], > > > + ext_csd[EXT_CSD_FIRMWARE_VERSION_4], > > > + ext_csd[EXT_CSD_FIRMWARE_VERSION_3], > > > + ext_csd[EXT_CSD_FIRMWARE_VERSION_2], > > > + ext_csd[EXT_CSD_FIRMWARE_VERSION_1], > > > + ext_csd[EXT_CSD_FIRMWARE_VERSION_0]); > > ExtCSD[261:254] is an ASCII string, just add a terminating null. > > Unfortunately I found two different manufacturers which put > non-printable characters in this 8-byte field. So I don't think it can > be treated as ASCII in all cases. Printing out the hex value seemed > liked the most comprehensive solution. NAK with prejudice. This interfere with the output that we/our clients expects.
diff --git a/mmc.h b/mmc.h index 5d8a7e3..86e209a 100644 --- a/mmc.h +++ b/mmc.h @@ -59,7 +59,14 @@ #define EXT_CSD_OPTIMAL_READ_SIZE 266 /* RO */ #define EXT_CSD_OPTIMAL_WRITE_SIZE 265 /* RO */ #define EXT_CSD_OPTIMAL_TRIM_UNIT_SIZE 264 /* RO */ -#define EXT_CSD_FIRMWARE_VERSION 254 /* RO */ +#define EXT_CSD_FIRMWARE_VERSION_7 261 /* RO */ +#define EXT_CSD_FIRMWARE_VERSION_6 260 /* RO */ +#define EXT_CSD_FIRMWARE_VERSION_5 259 /* RO */ +#define EXT_CSD_FIRMWARE_VERSION_4 258 /* RO */ +#define EXT_CSD_FIRMWARE_VERSION_3 257 /* RO */ +#define EXT_CSD_FIRMWARE_VERSION_2 256 /* RO */ +#define EXT_CSD_FIRMWARE_VERSION_1 255 /* RO */ +#define EXT_CSD_FIRMWARE_VERSION_0 254 /* RO */ #define EXT_CSD_CACHE_SIZE_3 252 #define EXT_CSD_CACHE_SIZE_2 251 #define EXT_CSD_CACHE_SIZE_1 250 diff --git a/mmc_cmds.c b/mmc_cmds.c index 97ea111..45aa4c0 100644 --- a/mmc_cmds.c +++ b/mmc_cmds.c @@ -1758,8 +1758,15 @@ int do_read_extcsd(int nargs, char **argv) } if (ext_csd_rev >= 7) { - printf("eMMC Firmware Version: %s\n", - (char*)&ext_csd[EXT_CSD_FIRMWARE_VERSION]); + printf("Firmware Version: 0x%02x%02x%02x%02x%02x%02x%02x%02x\n", + ext_csd[EXT_CSD_FIRMWARE_VERSION_7], + ext_csd[EXT_CSD_FIRMWARE_VERSION_6], + ext_csd[EXT_CSD_FIRMWARE_VERSION_5], + ext_csd[EXT_CSD_FIRMWARE_VERSION_4], + ext_csd[EXT_CSD_FIRMWARE_VERSION_3], + ext_csd[EXT_CSD_FIRMWARE_VERSION_2], + ext_csd[EXT_CSD_FIRMWARE_VERSION_1], + ext_csd[EXT_CSD_FIRMWARE_VERSION_0]); printf("eMMC Life Time Estimation A [EXT_CSD_DEVICE_LIFE_TIME_EST_TYP_A]: 0x%02x\n", ext_csd[EXT_CSD_DEVICE_LIFE_TIME_EST_TYP_A]); printf("eMMC Life Time Estimation B [EXT_CSD_DEVICE_LIFE_TIME_EST_TYP_B]: 0x%02x\n",
The FIRMWARE_VERSION field is 8-bytes in size and contains non-printable characters. Treat this field as binary and print individual byte values in hex Signed-off-by: James Nuss <jamesnuss@nanometrics.ca> --- mmc.h | 9 ++++++++- mmc_cmds.c | 11 +++++++++-- 2 files changed, 17 insertions(+), 3 deletions(-)