[PATCH 0/7] fix bugs in write_reliability and enh_area set commands + more extcsd parsing

[PATCH 0/7] fix bugs in write_reliability and enh_area set commands + more extcsd parsing

[James Nuss]

[PATCH 1-3] just introduce further parsing and human-readable output of eMMC 5.0+ fields in the extcsd:
  mmc-utils: interpret OPTIMAL_*_SIZE fields in extcsd
  mmc-utils: treat FIRMWARE_VERSION as binary field instead of string
  mmc-utils: interpret DEVICE_VERSION when printing extcsd

[PATCH 4-5] are critical bug fixes for the current version. This addresses concerns previously raised in threads wrt write-reliability setting:
https://www.spinics.net/lists/linux-mmc/msg41952.html
https://www.spinics.net/lists/linux-mmc/msg43171.html

[PATCH 6] is a critical bug fix for the current version to allow setting of enhanced and extended attributes on multiple partitions.

[PATCH 7] is a version up-rev:
  mmc-utils: update version number to 0.2

James Nuss (7):
  mmc-utils: interpret OPTIMAL_*_SIZE fields in extcsd
  mmc-utils: treat FIRMWARE_VERSION as binary field instead of string
  mmc-utils: interpret DEVICE_VERSION when printing extcsd
  mmc-utils: Introduce write_reliability set_register command
  mmc-utils: remove write_reliability set command
  mmc-utils: discrete commands for enhanced and extended attribute
  mmc-utils: update version number to 0.2

 mmc.c      |  52 ++++++++++---
 mmc.h      |  14 +++-
 mmc_cmds.c | 243 ++++++++++++++++++++++++++++++++++++++++++++-----------------
 mmc_cmds.h |   6 +-
 4 files changed, 235 insertions(+), 80 deletions(-)

-- 
2.7.4


-- 
This message is intended exclusively for the individual or entity to which 
it is addressed. This communication may contain information that is 
proprietary, privileged, confidential or otherwise legally exempt from 
disclosure. If you are not the named addressee, or have been inadvertently 
and erroneously referenced in the address line, you are not authorized to 
read, print, retain, copy or disseminate this message or any part of it. If 
you have received this message in error, please notify the sender 
immediately by e-mail and delete all copies of the message.

[PATCH 1/7] mmc-utils: interpret OPTIMAL_*_SIZE fields in extcsd

[James Nuss]

eMMC 5.0 introduced OPTIMAL_READ_SIZE, OPTIMAL_WRITE_SIZE and OPTIMAL
TRIM_UNIT_SIZE fields in the extcsd

Interpret these fields when reading out the extcsd with human-readable
results

Signed-off-by: James Nuss <jamesnuss@nanometrics.ca>
---
 mmc.h      |  3 +++
 mmc_cmds.c | 18 ++++++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/mmc.h b/mmc.h
index 285c1f1..5d8a7e3 100644
--- a/mmc.h
+++ b/mmc.h
@@ -56,6 +56,9 @@
 #define EXT_CSD_DEVICE_LIFE_TIME_EST_TYP_B 	269	/* RO */
 #define EXT_CSD_DEVICE_LIFE_TIME_EST_TYP_A 	268	/* RO */
 #define EXT_CSD_PRE_EOL_INFO		267	/* RO */
+#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_CACHE_SIZE_3		252
 #define EXT_CSD_CACHE_SIZE_2		251
diff --git a/mmc_cmds.c b/mmc_cmds.c
index 44623fe..97ea111 100644
--- a/mmc_cmds.c
+++ b/mmc_cmds.c
@@ -1766,6 +1766,24 @@ int do_read_extcsd(int nargs, char **argv)
 			ext_csd[EXT_CSD_DEVICE_LIFE_TIME_EST_TYP_B]);
 		printf("eMMC Pre EOL information [EXT_CSD_PRE_EOL_INFO]: 0x%02x\n",
 			ext_csd[EXT_CSD_PRE_EOL_INFO]);
+
+		reg = ext_csd[EXT_CSD_OPTIMAL_READ_SIZE];
+		printf("Minimum optimal read unit size (for the device) "
+			"[OPTIMAL_READ_SIZE]: 0x%02x\n", reg);
+		printf(" i.e. %lu KiB\n", reg * 4UL);
+
+		reg = ext_csd[EXT_CSD_OPTIMAL_WRITE_SIZE];
+		printf("Minimum optimal write unit size (for the device) "
+			"[OPTIMAL_WRITE_SIZE]: 0x%02x\n", reg);
+		printf(" i.e. %lu KiB\n", reg * 4UL);
+
+		reg = ext_csd[EXT_CSD_OPTIMAL_TRIM_UNIT_SIZE];
+		printf("Minimum optimal trim unit size (for the device) "
+			"[OPTIMAL_TRIM_UNIT_SIZE]: 0x%02x\n", reg);
+		if (reg == 0 || reg > 21)
+			printf("error: invalid OPTIMAL_TRIM_UNIT_SIZE\n");
+		else
+			printf(" i.e. %lu KiB\n", (1UL << (reg - 1)) * 4);
 	}
 
 	if (ext_csd_rev >= 8) {
-- 
2.7.4


-- 
This message is intended exclusively for the individual or entity to which 
it is addressed. This communication may contain information that is 
proprietary, privileged, confidential or otherwise legally exempt from 
disclosure. If you are not the named addressee, or have been inadvertently 
and erroneously referenced in the address line, you are not authorized to 
read, print, retain, copy or disseminate this message or any part of it. If 
you have received this message in error, please notify the sender 
immediately by e-mail and delete all copies of the message.

[PATCH 2/7] mmc-utils: treat FIRMWARE_VERSION as binary field instead of string

[James Nuss]

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(-)

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",
-- 
2.7.4


-- 
This message is intended exclusively for the individual or entity to which 
it is addressed. This communication may contain information that is 
proprietary, privileged, confidential or otherwise legally exempt from 
disclosure. If you are not the named addressee, or have been inadvertently 
and erroneously referenced in the address line, you are not authorized to 
read, print, retain, copy or disseminate this message or any part of it. If 
you have received this message in error, please notify the sender 
immediately by e-mail and delete all copies of the message.

[PATCH 3/7] mmc-utils: interpret DEVICE_VERSION when printing extcsd

[James Nuss]

The DEVICE_VERSION is a 2-byte field

Print the individual byte values in hex

Signed-off-by: James Nuss <jamesnuss@nanometrics.ca>
---
 mmc.h      | 2 ++
 mmc_cmds.c | 3 +++
 2 files changed, 5 insertions(+)

diff --git a/mmc.h b/mmc.h
index 86e209a..f25869e 100644
--- a/mmc.h
+++ b/mmc.h
@@ -59,6 +59,8 @@
 #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_DEVICE_VERSION_1	263	/* RO */
+#define EXT_CSD_DEVICE_VERSION_0	262	/* 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 */
diff --git a/mmc_cmds.c b/mmc_cmds.c
index 45aa4c0..756aa2f 100644
--- a/mmc_cmds.c
+++ b/mmc_cmds.c
@@ -1758,6 +1758,9 @@ int do_read_extcsd(int nargs, char **argv)
 	}
 
 	if (ext_csd_rev >= 7) {
+		printf("Device Version: 0x%02x%02x\n",
+			ext_csd[EXT_CSD_DEVICE_VERSION_1],
+			ext_csd[EXT_CSD_DEVICE_VERSION_0]);
 		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],
-- 
2.7.4


-- 
This message is intended exclusively for the individual or entity to which 
it is addressed. This communication may contain information that is 
proprietary, privileged, confidential or otherwise legally exempt from 
disclosure. If you are not the named addressee, or have been inadvertently 
and erroneously referenced in the address line, you are not authorized to 
read, print, retain, copy or disseminate this message or any part of it. If 
you have received this message in error, please notify the sender 
immediately by e-mail and delete all copies of the message.

[PATCH 4/7] mmc-utils: Introduce write_reliability set_register command

[James Nuss]

"write-reliability" setting must be performed on all required partitions
in one command.

Introduce new command "write_reliability set_register" to allow setting
of write-reliability for all required partitions.

This command is a one-time programmable action. It cannot not be undone
by power-cycling the unit.

Signed-off-by: James Nuss <jamesnuss@nanometrics.ca>
---
 mmc.c      |  9 +++++++++
 mmc_cmds.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 mmc_cmds.h |  1 +
 3 files changed, 69 insertions(+)

diff --git a/mmc.c b/mmc.c
index 50c9c9e..aaefd3d 100644
--- a/mmc.c
+++ b/mmc.c
@@ -102,6 +102,15 @@ static struct Command commands[] = {
 		"Enable write reliability per partition for the <device>.\nDry-run only unless -y or -c is passed.\nUse -c if more partitioning settings are still to come.\nNOTE!  This is a one-time programmable (unreversible) change.",
 	  NULL
 	},
+	{ do_write_reliability_set_register, -2,
+	  "write_reliability set_register", "<partition-mask> " "<device>\n"
+		"Set the write-reliability register (WR_REL_SET) for the <device>.\n"
+		"User area=bit0, GP1=bit1, GP2=bit2, GP3=bit3, GP4=bit4\n"
+		"e.g setting register to 0x03 will set 'User Area' and 'GP1' partitions \n"
+		"with write-reliability, and all others without write-reliability.\n"
+		"NOTE!  This is a one-time programmable (irreversible) change.",
+	  NULL
+	},
 	{ do_status_get, -1,
 	  "status get", "<device>\n"
 	  "Print the response to STATUS_SEND (CMD13).",
diff --git a/mmc_cmds.c b/mmc_cmds.c
index 756aa2f..68c73ef 100644
--- a/mmc_cmds.c
+++ b/mmc_cmds.c
@@ -1244,6 +1244,65 @@ int do_enh_area_set(int nargs, char **argv)
 	return 0;
 }
 
+int do_write_reliability_set_register(int nargs, char **argv)
+{
+	__u8 ext_csd[512];
+	int fd, ret;
+	long partition_mask;
+	char *device;
+	char *endptr;
+
+	if (nargs != 3) {
+		fprintf(stderr,"Usage: mmc write_reliability set_register <partition-mask> </path/to/mmcblkX>\n");
+		exit(1);
+	}
+
+	errno = 0;
+	partition_mask = strtol(argv[1], &endptr, 0);
+	if (errno != 0 || endptr == argv[1] || *endptr != 0 || partition_mask < 0 || partition_mask > 0x1f) {
+		fprintf(stderr, "Partition mask invalid: %s\n", argv[1]);
+		exit(1);
+	}
+
+	device = argv[2];
+	fd = open(device, O_RDWR);
+	if (fd < 0) {
+		perror("open");
+		exit(1);
+	}
+
+	ret = read_extcsd(fd, ext_csd);
+	if (ret) {
+		fprintf(stderr, "Could not read EXT_CSD from %s\n", device);
+		exit(1);
+	}
+
+	/* assert not PARTITION_SETTING_COMPLETED */
+	if (ext_csd[EXT_CSD_PARTITION_SETTING_COMPLETED])
+	{
+		fprintf(stderr, "Device is already partitioned\n");
+		exit(1);
+	}
+
+	/* assert HS_CTRL_REL */
+	if (!(ext_csd[EXT_CSD_WR_REL_PARAM] & HS_CTRL_REL)) {
+		fprintf(stderr, "Cannot set write reliability parameters, WR_REL_SET is "
+				"read-only\n");
+		exit(1);
+	}
+
+	ret = write_extcsd_value(fd, EXT_CSD_WR_REL_SET, (__u8)partition_mask);
+	if (ret) {
+		fprintf(stderr, "Could not write 0x%02x to EXT_CSD[%d] in %s\n",
+				(__u8)partition_mask, EXT_CSD_WR_REL_SET, device);
+		exit(1);
+	}
+	fprintf(stderr, "Done setting EXT_CSD_WR_REL_SET to 0x%02x on %s\n",
+		(__u8)partition_mask, device);
+
+	return 0;
+}
+
 int do_write_reliability_set(int nargs, char **argv)
 {
 	__u8 value;
diff --git a/mmc_cmds.h b/mmc_cmds.h
index 9d3246c..8ca9ac9 100644
--- a/mmc_cmds.h
+++ b/mmc_cmds.h
@@ -35,6 +35,7 @@ int do_status_get(int nargs, char **argv);
 int do_create_gp_partition(int nargs, char **argv);
 int do_enh_area_set(int nargs, char **argv);
 int do_write_reliability_set(int nargs, char **argv);
+int do_write_reliability_set_register(int nargs, char **argv);
 int do_rpmb_write_key(int nargs, char **argv);
 int do_rpmb_read_counter(int nargs, char **argv);
 int do_rpmb_read_block(int nargs, char **argv);
-- 
2.7.4


-- 
This message is intended exclusively for the individual or entity to which 
it is addressed. This communication may contain information that is 
proprietary, privileged, confidential or otherwise legally exempt from 
disclosure. If you are not the named addressee, or have been inadvertently 
and erroneously referenced in the address line, you are not authorized to 
read, print, retain, copy or disseminate this message or any part of it. If 
you have received this message in error, please notify the sender 
immediately by e-mail and delete all copies of the message.

[PATCH 5/7] mmc-utils: remove write_reliability set command

[James Nuss]

The "write_reliability set" command is dangerous since it will only set
write-reliability on one partition per command and this command is a
one-time programmable action. This makes it impossible to set
write-reliability on multiple partitions.

Remove the command so it cannot be used in the future.
Use "write_reliability set_register" instead.

Signed-off-by: James Nuss <jamesnuss@nanometrics.ca>
---
 mmc.c      |  5 -----
 mmc_cmds.c | 67 --------------------------------------------------------------
 mmc_cmds.h |  1 -
 3 files changed, 73 deletions(-)

diff --git a/mmc.c b/mmc.c
index aaefd3d..655546b 100644
--- a/mmc.c
+++ b/mmc.c
@@ -97,11 +97,6 @@ static struct Command commands[] = {
 		"Enable the enhanced user area for the <device>.\nDry-run only unless -y or -c is passed.\nUse -c if more partitioning settings are still to come.\nNOTE!  This is a one-time programmable (unreversible) change.",
 	  NULL
 	},
-	{ do_write_reliability_set, -2,
-	  "write_reliability set", "<-y|-n|-c> " "<partition> " "<device>\n"
-		"Enable write reliability per partition for the <device>.\nDry-run only unless -y or -c is passed.\nUse -c if more partitioning settings are still to come.\nNOTE!  This is a one-time programmable (unreversible) change.",
-	  NULL
-	},
 	{ do_write_reliability_set_register, -2,
 	  "write_reliability set_register", "<partition-mask> " "<device>\n"
 		"Set the write-reliability register (WR_REL_SET) for the <device>.\n"
diff --git a/mmc_cmds.c b/mmc_cmds.c
index 68c73ef..9565bc9 100644
--- a/mmc_cmds.c
+++ b/mmc_cmds.c
@@ -1303,73 +1303,6 @@ int do_write_reliability_set_register(int nargs, char **argv)
 	return 0;
 }
 
-int do_write_reliability_set(int nargs, char **argv)
-{
-	__u8 value;
-	__u8 ext_csd[512];
-	int fd, ret;
-
-	int dry_run = 1;
-	int partition;
-	char *device;
-
-	if (nargs != 4) {
-		fprintf(stderr,"Usage: mmc write_reliability set <-y|-n|-c> <partition> </path/to/mmcblkX>\n");
-		exit(1);
-	}
-
-	if (!strcmp("-y", argv[1])) {
-		dry_run = 0;
-	} else if (!strcmp("-c", argv[1])) {
-		dry_run = 2;
-	}
-
-	partition = strtol(argv[2], NULL, 10);
-	device = argv[3];
-
-	fd = open(device, O_RDWR);
-	if (fd < 0) {
-		perror("open");
-		exit(1);
-	}
-
-	ret = read_extcsd(fd, ext_csd);
-	if (ret) {
-		fprintf(stderr, "Could not read EXT_CSD from %s\n", device);
-		exit(1);
-	}
-
-	/* assert not PARTITION_SETTING_COMPLETED */
-	if (ext_csd[EXT_CSD_PARTITION_SETTING_COMPLETED])
-	{
-		printf(" Device is already partitioned\n");
-		exit(1);
-	}
-
-	/* assert HS_CTRL_REL */
-	if (!(ext_csd[EXT_CSD_WR_REL_PARAM] & HS_CTRL_REL)) {
-		printf("Cannot set write reliability parameters, WR_REL_SET is "
-				"read-only\n");
-		exit(1);
-	}
-
-	value = ext_csd[EXT_CSD_WR_REL_SET] | (1<<partition);
-	ret = write_extcsd_value(fd, EXT_CSD_WR_REL_SET, value);
-	if (ret) {
-		fprintf(stderr, "Could not write 0x%02x to EXT_CSD[%d] in %s\n",
-				value, EXT_CSD_WR_REL_SET, device);
-		exit(1);
-	}
-
-	printf("Done setting EXT_CSD_WR_REL_SET to 0x%02x on %s\n",
-		value, device);
-
-	if (set_partitioning_setting_completed(dry_run, device, fd))
-		exit(1);
-
-	return 0;
-}
-
 int do_read_extcsd(int nargs, char **argv)
 {
 	__u8 ext_csd[512], ext_csd_rev, reg;
diff --git a/mmc_cmds.h b/mmc_cmds.h
index 8ca9ac9..cf6c6fb 100644
--- a/mmc_cmds.h
+++ b/mmc_cmds.h
@@ -34,7 +34,6 @@ int do_sanitize(int nargs, char **argv);
 int do_status_get(int nargs, char **argv);
 int do_create_gp_partition(int nargs, char **argv);
 int do_enh_area_set(int nargs, char **argv);
-int do_write_reliability_set(int nargs, char **argv);
 int do_write_reliability_set_register(int nargs, char **argv);
 int do_rpmb_write_key(int nargs, char **argv);
 int do_rpmb_read_counter(int nargs, char **argv);
-- 
2.7.4


-- 
This message is intended exclusively for the individual or entity to which 
it is addressed. This communication may contain information that is 
proprietary, privileged, confidential or otherwise legally exempt from 
disclosure. If you are not the named addressee, or have been inadvertently 
and erroneously referenced in the address line, you are not authorized to 
read, print, retain, copy or disseminate this message or any part of it. If 
you have received this message in error, please notify the sender 
immediately by e-mail and delete all copies of the message.

[PATCH 6/7] mmc-utils: discrete commands for enhanced and extended attribute setting

[James Nuss]

The enhanced and extended attributes must be set on all required partitions
in a single command instead of one command per partition.

Remove the enhanced and extended attribute setting parameters from the
"gp create" and "enh_area set" commands and create new commands:
* "enh_area set_partitions"
* "enh_area set_user_data"
* "gp set_extended"

Signed-off-by: James Nuss <jamesnuss@nanometrics.ca>
---
 mmc.c      |  42 ++++++++++++---
 mmc_cmds.c | 169 ++++++++++++++++++++++++++++++++++++++++++++++---------------
 mmc_cmds.h |   4 +-
 3 files changed, 167 insertions(+), 48 deletions(-)

diff --git a/mmc.c b/mmc.c
index 655546b..f2c714c 100644
--- a/mmc.c
+++ b/mmc.c
@@ -87,14 +87,42 @@ static struct Command commands[] = {
 		"Set the eMMC data sector size to 4KB by disabling emulation on\n<device>.",
 	  NULL
 	},
-	{ do_create_gp_partition, -6,
-	  "gp create", "<-y|-n|-c> " "<length KiB> " "<partition> " "<enh_attr> " "<ext_attr> " "<device>\n"
-		"Create general purpose partition for the <device>.\nDry-run only unless -y or -c is passed.\nUse -c if more partitioning settings are still to come.\nNOTE!  This is a one-time programmable (unreversible) change.\nTo set enhanced attribute to general partition being created set\n <enh_attr> to 1 else set it to 0.\nTo set extended attribute to general partition\n set <ext_attr> to 1,2 else set it to 0",
+	{ do_create_gp_partition, 4,
+	  "gp create", "<-y|-n|-c> " "<length KiB> " "<partition> " "<device>\n"
+		"Create general purpose partition for the <device>.\n"
+		"Dry-run only unless -y or -c is passed.\n"
+		"Use -c if more partitioning settings are still to come.\n"
+		"NOTE!  This is a one-time programmable (unreversible) change.",
 	  NULL
 	},
-	{ do_enh_area_set, -4,
-	  "enh_area set", "<-y|-n|-c> " "<start KiB> " "<length KiB> " "<device>\n"
-		"Enable the enhanced user area for the <device>.\nDry-run only unless -y or -c is passed.\nUse -c if more partitioning settings are still to come.\nNOTE!  This is a one-time programmable (unreversible) change.",
+	{ do_gp_set_extended, 3,
+	  "gp set_extended", "<-y|-n|-c> " "<partition-mask> " "<device>\n"
+		"Set extended attribute on specified general purpose partitions for the <device>.\n"
+		"GP4=bit[15:12], GP3=bit[11:8], GP2=bit[7:4], GP1=bit[3:0]\n"   
+		"e.g setting <partition-mask> to 0x0120 will set extended attribute 0x02 on 'GP2'\n"
+		"  partition and extended attribute 0x01 on 'GP3' partition\n"
+		"Dry-run only unless -y or -c is passed.\n"
+		"Use -c if more partitioning settings are still to come.\n"
+		"NOTE!  This is a one-time programmable (unreversible) change.",
+	  NULL
+	},
+	{ do_enh_area_set_user_data, -4,
+	  "enh_area set_user_data", "<-y|-n|-c> " "<start KiB> " "<length KiB> " "<device>\n"
+		"Set the size and offset of the enhanced user data area for the <device>.\n"
+		"Dry-run only unless -y or -c is passed.\n"
+		"Use -c if more partitioning settings are still to come.\n"
+		"NOTE!  This is a one-time programmable (unreversible) change.",
+	  NULL
+	},
+	{ do_enh_area_set_partitions, 3,
+	  "enh_area set_partitions", "<-y|-n|-c> " "<partition-mask> " "<device>\n"
+		"Enable enhanced attribute on specified partitions for the <device>.\n"
+		"User area=bit[0], GP1=bit[1], GP2=bit[2], GP3=bit[3], GP4=bit[4]\n"
+		"e.g setting <partition-mask> to 0x03 will set 'User Area' and 'GP1' partitions \n"
+		"    with enhanced attribute, and all others without enhanced attribute.\n"
+		"Dry-run only unless -y or -c is passed.\n"
+		"Use -c if more partitioning settings are still to come.\n"
+		"NOTE!  This is a one-time programmable (unreversible) change.",
 	  NULL
 	},
 	{ do_write_reliability_set_register, -2,
@@ -102,7 +130,7 @@ static struct Command commands[] = {
 		"Set the write-reliability register (WR_REL_SET) for the <device>.\n"
 		"User area=bit0, GP1=bit1, GP2=bit2, GP3=bit3, GP4=bit4\n"
 		"e.g setting register to 0x03 will set 'User Area' and 'GP1' partitions \n"
-		"with write-reliability, and all others without write-reliability.\n"
+		"    with write-reliability, and all others without write-reliability.\n"
 		"NOTE!  This is a one-time programmable (irreversible) change.",
 	  NULL
 	},
diff --git a/mmc_cmds.c b/mmc_cmds.c
index 9565bc9..793b7fa 100644
--- a/mmc_cmds.c
+++ b/mmc_cmds.c
@@ -967,6 +967,81 @@ int check_enhanced_area_total_limit(const char * const device, int fd)
 	return 0;
 }
 
+// TODO: check if enhanced attributes set on GP partitions (only allowed either enhanced or extended attribute)
+int do_gp_set_extended(int nargs, char **argv)
+{
+	__u8 value;
+	__u8 ext_csd[512];
+	__u8 address;
+	int fd, ret;
+	char *device;
+	int dry_run = 1;
+	long partition_mask;
+	char *endptr;
+
+	if (nargs != 4) {
+		fprintf(stderr, "Usage: mmc gp set_extended <-y|-n|-c> <partition-mask> </path/to/mmcblkX>\n");
+		exit(1);
+	}
+
+	if (!strcmp("-y", argv[1])) {
+		dry_run = 0;
+        } else if (!strcmp("-c", argv[1])) {
+		dry_run = 2;
+	}
+
+	errno = 0;
+	partition_mask = strtol(argv[2], &endptr, 0);
+	if (errno != 0 || endptr == argv[2] || *endptr != 0 || partition_mask < 0 || partition_mask > 0xffff) {
+		fprintf(stderr, "Partition mask invalid: %s\n", argv[2]);
+		exit(1);
+	}
+
+	device = argv[3];
+	fd = open(device, O_RDWR);
+	if (fd < 0) {
+		perror("open");
+		exit(1);
+	}
+
+	ret = read_extcsd(fd, ext_csd);
+	if (ret) {
+		fprintf(stderr, "Could not read EXT_CSD from %s\n", device);
+		exit(1);
+	}
+
+	/* assert not PARTITION_SETTING_COMPLETED */
+	if (ext_csd[EXT_CSD_PARTITION_SETTING_COMPLETED]) {
+		printf(" Device is already partitioned\n");
+		exit(1);
+	}
+
+	value = (__u8)partition_mask;
+	address = EXT_CSD_EXT_PARTITIONS_ATTRIBUTE_0;
+	ret = write_extcsd_value(fd, address, value);
+	fprintf(stderr, "Writing 0x%02x to EXT_CSD[%d]\n", value, address);
+	if (ret) {
+		fprintf(stderr, "Could not write 0x%x to EXT_CSD[%d] in %s\n",
+			value, address, device);
+		exit(1);
+	}
+
+	value = (__u8)(partition_mask >> 8);
+	address = EXT_CSD_EXT_PARTITIONS_ATTRIBUTE_1;
+	ret = write_extcsd_value(fd, address, value);
+	fprintf(stderr, "Writing 0x%02x to EXT_CSD[%d]\n", value, address);
+	if (ret) {
+		fprintf(stderr, "Could not write 0x%x to EXT_CSD[%d] in %s\n",
+			value, address, device);
+		exit(1);
+	}
+
+	if (set_partitioning_setting_completed(dry_run, device, fd))
+		exit(1);
+
+	return 0;
+}
+
 int do_create_gp_partition(int nargs, char **argv)
 {
 	__u8 value;
@@ -975,12 +1050,12 @@ int do_create_gp_partition(int nargs, char **argv)
 	int fd, ret;
 	char *device;
 	int dry_run = 1;
-	int partition, enh_attr, ext_attr;
+	int partition;
 	unsigned int length_kib, gp_size_mult;
 	unsigned long align;
 
-	if (nargs != 7) {
-		fprintf(stderr, "Usage: mmc gp create <-y|-n|-c> <length KiB> <partition> <enh_attr> <ext_attr> </path/to/mmcblkX>\n");
+	if (nargs != 5) {
+		fprintf(stderr, "Usage: mmc gp create <-y|-n|-c> <length KiB> <partition> </path/to/mmcblkX>\n");
 		exit(1);
 	}
 
@@ -992,20 +1067,13 @@ int do_create_gp_partition(int nargs, char **argv)
 
 	length_kib = strtol(argv[2], NULL, 10);
 	partition = strtol(argv[3], NULL, 10);
-	enh_attr = strtol(argv[4], NULL, 10);
-	ext_attr = strtol(argv[5], NULL, 10);
-	device = argv[6];
+	device = argv[4];
 
 	if (partition < 1 || partition > 4) {
 		printf("Invalid gp partition number; valid range [1-4].\n");
 		exit(1);
 	}
 
-	if (enh_attr && ext_attr) {
-		printf("Not allowed to set both enhanced attribute and extended attribute\n");
-		exit(1);
-	}
-
 	fd = open(device, O_RDWR);
 	if (fd < 0) {
 		perror("open");
@@ -1060,30 +1128,52 @@ int do_create_gp_partition(int nargs, char **argv)
 		exit(1);
 	}
 
-	value = ext_csd[EXT_CSD_PARTITIONS_ATTRIBUTE];
-	if (enh_attr)
-		value |= (1 << partition);
-	else
-		value &= ~(1 << partition);
+	if (set_partitioning_setting_completed(dry_run, device, fd))
+		exit(1);
 
-	ret = write_extcsd_value(fd, EXT_CSD_PARTITIONS_ATTRIBUTE, value);
-	if (ret) {
-		fprintf(stderr, "Could not write EXT_CSD_ENH_%x to EXT_CSD[%d] in %s\n",
-			partition, EXT_CSD_PARTITIONS_ATTRIBUTE, device);
+	return 0;
+}
+
+// TODO: check if extended attributes set on GP partitions (only allowed either enhanced or extended attribute)
+int do_enh_area_set_partitions(int nargs, char **argv)
+{
+	int fd, ret;
+	char *device;
+	int dry_run = 1;
+	long partition_mask;
+	char *endptr;
+
+	if (nargs != 4) {
+		fprintf(stderr, "Usage: mmc enh_area set_partitions <-y|-n|-c> <partition-mask> </path/to/mmcblkX>\n");
 		exit(1);
 	}
 
-	address = EXT_CSD_EXT_PARTITIONS_ATTRIBUTE_0 + (partition - 1) / 2;
-	value = ext_csd[address];
-	if (ext_attr)
-		value |= (ext_attr << (4 * ((partition - 1) % 2)));
-	else
-		value &= (0xF << (4 * ((partition % 2))));
+	if (!strcmp("-y", argv[1])) {
+		dry_run = 0;
+	} else if (!strcmp("-c", argv[1])) {
+		dry_run = 2;
+	}
 
-	ret = write_extcsd_value(fd, address, value);
+	errno = 0;
+	partition_mask = strtol(argv[2], &endptr, 0);
+	if (errno != 0 || endptr == argv[2] || *endptr != 0 || partition_mask < 0 || partition_mask > 0x1f) {
+		fprintf(stderr, "Partition mask invalid: %s\n", argv[2]);
+		exit(1);
+	}
+
+	device = argv[3];
+	fd = open(device, O_RDWR);
+	if (fd < 0) {
+		perror("open");
+		exit(1);
+	}
+
+	ret = write_extcsd_value(fd, EXT_CSD_PARTITIONS_ATTRIBUTE, (__u8)partition_mask);
 	if (ret) {
-		fprintf(stderr, "Could not write 0x%x to EXT_CSD[%d] in %s\n",
-			value, address, device);
+		fprintf(stderr, "Could not write 0x%02x to "
+			"EXT_CSD[%d] in %s\n",
+		       	(__u8)partition_mask,
+			EXT_CSD_PARTITIONS_ATTRIBUTE, device);
 		exit(1);
 	}
 
@@ -1097,7 +1187,7 @@ int do_create_gp_partition(int nargs, char **argv)
 	return 0;
 }
 
-int do_enh_area_set(int nargs, char **argv)
+int do_enh_area_set_user_data(int nargs, char **argv)
 {
 	__u8 value;
 	__u8 ext_csd[512];
@@ -1108,7 +1198,7 @@ int do_enh_area_set(int nargs, char **argv)
 	unsigned long align;
 
 	if (nargs != 5) {
-		fprintf(stderr, "Usage: mmc enh_area set <-y|-n|-c> <start KiB> <length KiB> </path/to/mmcblkX>\n");
+		fprintf(stderr, "Usage: mmc enh_area set_user_data <-y|-n|-c> <start KiB> <length KiB> </path/to/mmcblkX>\n");
 		exit(1);
 	}
 
@@ -1148,6 +1238,13 @@ int do_enh_area_set(int nargs, char **argv)
 		exit(1);
 	}
 
+	/* assert enhanced attribute is set on user partition */
+	if (!(ext_csd[EXT_CSD_PARTITIONS_ATTRIBUTE] & EXT_CSD_ENH_USR))
+	{
+		fprintf(stderr, "error: User partition must be set with enhanced attribute first\n");
+		exit(1);
+	}
+
 	align = 512l * get_hc_wp_grp_size(ext_csd) * get_hc_erase_grp_size(ext_csd);
 
 	enh_size_mult = (length_kib + align/2l) / align;
@@ -1165,7 +1262,7 @@ int do_enh_area_set(int nargs, char **argv)
 		exit(1);
 	}
 
-	/* write to ENH_START_ADDR and ENH_SIZE_MULT and PARTITIONS_ATTRIBUTE's ENH_USR bit */
+	/* write to ENH_START_ADDR and ENH_SIZE_MULT */
 	value = (enh_start_addr >> 24) & 0xff;
 	ret = write_extcsd_value(fd, EXT_CSD_ENH_START_ADDR_3, value);
 	if (ret) {
@@ -1223,14 +1320,6 @@ int do_enh_area_set(int nargs, char **argv)
 			EXT_CSD_ENH_SIZE_MULT_0, device);
 		exit(1);
 	}
-	value = ext_csd[EXT_CSD_PARTITIONS_ATTRIBUTE] | EXT_CSD_ENH_USR;
-	ret = write_extcsd_value(fd, EXT_CSD_PARTITIONS_ATTRIBUTE, value);
-	if (ret) {
-		fprintf(stderr, "Could not write EXT_CSD_ENH_USR to "
-			"EXT_CSD[%d] in %s\n",
-			EXT_CSD_PARTITIONS_ATTRIBUTE, device);
-		exit(1);
-	}
 
 	ret = check_enhanced_area_total_limit(device, fd);
 	if (ret)
diff --git a/mmc_cmds.h b/mmc_cmds.h
index cf6c6fb..a8a90c2 100644
--- a/mmc_cmds.h
+++ b/mmc_cmds.h
@@ -33,7 +33,9 @@ int do_hwreset_dis(int nargs, char **argv);
 int do_sanitize(int nargs, char **argv);
 int do_status_get(int nargs, char **argv);
 int do_create_gp_partition(int nargs, char **argv);
-int do_enh_area_set(int nargs, char **argv);
+int do_gp_set_extended(int nargs, char **argv);
+int do_enh_area_set_user_data(int nargs, char **argv);
+int do_enh_area_set_partitions(int nargs, char **argv);
 int do_write_reliability_set_register(int nargs, char **argv);
 int do_rpmb_write_key(int nargs, char **argv);
 int do_rpmb_read_counter(int nargs, char **argv);
-- 
2.7.4


-- 
This message is intended exclusively for the individual or entity to which 
it is addressed. This communication may contain information that is 
proprietary, privileged, confidential or otherwise legally exempt from 
disclosure. If you are not the named addressee, or have been inadvertently 
and erroneously referenced in the address line, you are not authorized to 
read, print, retain, copy or disseminate this message or any part of it. If 
you have received this message in error, please notify the sender 
immediately by e-mail and delete all copies of the message.

[PATCH 7/7] mmc-utils: update version number to 0.2

[James Nuss]

---
 mmc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mmc.c b/mmc.c
index f2c714c..660b62e 100644
--- a/mmc.c
+++ b/mmc.c
@@ -26,7 +26,7 @@
 
 #include "mmc_cmds.h"
 
-#define MMC_VERSION	"0.1"
+#define MMC_VERSION	"0.2"
 
 #define BASIC_HELP 0
 #define ADVANCED_HELP 1
-- 
2.7.4


-- 
This message is intended exclusively for the individual or entity to which 
it is addressed. This communication may contain information that is 
proprietary, privileged, confidential or otherwise legally exempt from 
disclosure. If you are not the named addressee, or have been inadvertently 
and erroneously referenced in the address line, you are not authorized to 
read, print, retain, copy or disseminate this message or any part of it. If 
you have received this message in error, please notify the sender 
immediately by e-mail and delete all copies of the message.

RE: [PATCH 1/7] mmc-utils: interpret OPTIMAL_*_SIZE fields in extcsd

[Avri Altman]

Looks fine.

Thanks,
Avri

> eMMC 5.0 introduced OPTIMAL_READ_SIZE, OPTIMAL_WRITE_SIZE and
> OPTIMAL
> TRIM_UNIT_SIZE fields in the extcsd
> 
> Interpret these fields when reading out the extcsd with human-readable
> results
> 
> Signed-off-by: James Nuss <jamesnuss@nanometrics.ca>
Reviewed-by: Avri Altman <avri.altman@wdc.com>

RE: [PATCH 2/7] mmc-utils: treat FIRMWARE_VERSION as binary field instead of string

[Avri Altman]

> +++ 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

Re: [PATCH 2/7] mmc-utils: treat FIRMWARE_VERSION as binary field instead of string

[James Nuss]

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
>


--

-- 
This message is intended exclusively for the individual or entity to which 
it is addressed. This communication may contain information that is 
proprietary, privileged, confidential or otherwise legally exempt from 
disclosure. If you are not the named addressee, or have been inadvertently 
and erroneously referenced in the address line, you are not authorized to 
read, print, retain, copy or disseminate this message or any part of it. If 
you have received this message in error, please notify the sender 
immediately by e-mail and delete all copies of the message.

RE: [PATCH 2/7] mmc-utils: treat FIRMWARE_VERSION as binary field instead of string

[Avri Altman]

> 
> 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.