[U-Boot] [PATCH 00/18] Support for eMMC partitioning and related fixes

[U-Boot] [PATCH 00/18] Support for eMMC partitioning and related fixes

[Diego Santa Cruz]

I have the need to hardware partition eMMC devices from U-Boot along
with setting enhanced and reliable write attributes.

This series of patches adds this support to U-Boot via a new mmc
API, a few new members of struct mmc and a new mmc sub-command. It
also features several fixes to the eMMC hardware partition support. I
have tested this with Micron eMMC 4.41 parts and it is working as
expected.

I am new to sending patches to the U-Boot mailing list, let alone
working with git, so please excuse any mishaps.

Diego Santa Cruz (18):
  mmc: show hardware partition sizes in mmcinfo output
  mmc: extend mmcinfo to show enhanced partition attribute
  mmc: make eMMC general purpose partition numbering match spec
  mmc: skip mmcinfo partition info processing for eMMC < 4.41
  mmc: incomplete test to switch to high-capacity group size
    definitions
  mmc: computation of eMMC GP partition size was missing 512 KiB factor
  mmc: read the size of eMMC enhanced user data area
  mmc: display size and start of eMMC enhanced user data area in
    mmcinfo
  mmc: fix erase_grp_size computation with high-capacity size
    definition
  mmc: read the high capacity WP group size for eMMC
  mmc: show the erase group size and HC WP group size in mmcinfo output
  mmc: eMMC partitioning data is not effective till partitioning
    completed
  mmc: the ext_csd data may be used during init even if reading failed
  mmc: add API to do eMMC hardware partitioning
  mmc: add mmc hwpartition sub-command to do eMMC hardware partitioning
  mmc: extend the mmc hardware partitioning API with write reliability
  mmc: extend the mmc hwpartition sub-command to change write
    reliability
  mmc: extend mmcinfo output to show partition write reliability
    settings

 common/cmd_mmc.c  |  207 ++++++++++++++++++++++++++++++++++++++-
 drivers/mmc/mmc.c |  283 +++++++++++++++++++++++++++++++++++++++++++++++++----
 include/mmc.h     |   47 +++++++++-
 3 files changed, 515 insertions(+), 22 deletions(-)

[U-Boot] [PATCH 01/18] mmc: show hardware partition sizes in mmcinfo output

[Diego Santa Cruz]

There is currently no command that will provide an overview of the hardware
partitions present on an eMMC device, one has to switch to every partition
via "mmc dev" and run mmcinfo for each to get the partition's capacity.
This commit adds a few lines of output to mmcinfo with the sizes of the
present partitions, like this:

Device: OMAP SD/MMC
Manufacturer ID: fe
OEM: 14e
Name: MMC16
Tran Speed: 52000000
Rd Block Len: 512
MMC version 4.41
High Capacity: Yes
Capacity: 13.8 GiB
Bus Width: 4-bit
User Capacity: 13.8 GiB
Boot Capacity: 16 MiB
RPMB Capacity: 128 KiB
GP1 Capacity: 64 MiB
GP2 Capacity: 64 MiB
---
 common/cmd_mmc.c |   17 +++++++++++++++++
 1 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/common/cmd_mmc.c b/common/cmd_mmc.c
index 4286e26..1a23b28 100644
--- a/common/cmd_mmc.c
+++ b/common/cmd_mmc.c
@@ -73,6 +73,8 @@ U_BOOT_CMD(
 
 static void print_mmcinfo(struct mmc *mmc)
 {
+	int i;
+
 	printf("Device: %s\n", mmc->cfg->name);
 	printf("Manufacturer ID: %x\n", mmc->cid[0] >> 24);
 	printf("OEM: %x\n", (mmc->cid[0] >> 8) & 0xffff);
@@ -91,6 +93,21 @@ static void print_mmcinfo(struct mmc *mmc)
 	print_size(mmc->capacity, "\n");
 
 	printf("Bus Width: %d-bit\n", mmc->bus_width);
+
+	if (!IS_SD(mmc) && (mmc->version >= MMC_VERSION_4)) {
+		puts("User Capacity: ");
+		print_size(mmc->capacity_user, "\n");
+		puts("Boot Capacity: ");
+		print_size(mmc->capacity_boot, "\n");
+		puts("RPMB Capacity: ");
+		print_size(mmc->capacity_rpmb, "\n");
+		for (i = 0; i < ARRAY_SIZE(mmc->capacity_gp); i++) {
+			if (mmc->capacity_gp[i]) {
+				printf("GP%i Capacity: ", i);
+				print_size(mmc->capacity_gp[i], "\n");
+			}
+		}
+	}
 }
 static struct mmc *init_mmc_device(int dev, bool force_init)
 {
-- 
1.7.1

[U-Boot] [PATCH 02/18] mmc: extend mmcinfo to show enhanced partition attribute

[Diego Santa Cruz]

This extends the mmcinfo command's output to show which eMMC partitions
have the enhanced attribute set. Note that the eMMC spec says that
if the enhanced attribute is supported then the boot and RPMB
partitions are of the enhanced type.

The output of mmcinfo becomes:
Device: OMAP SD/MMC
Manufacturer ID: fe
OEM: 14e
Name: MMC16
Tran Speed: 52000000
Rd Block Len: 512
MMC version 4.41
High Capacity: Yes
Capacity: 13.8 GiB
Bus Width: 4-bit
User Capacity: 13.8 GiB ENH
Boot Capacity: 16 MiB ENH
RPMB Capacity: 128 KiB ENH
GP1 Capacity: 64 MiB ENH
GP2 Capacity: 64 MiB ENH
---
 common/cmd_mmc.c  |   14 ++++++++++----
 drivers/mmc/mmc.c |    3 +++
 include/mmc.h     |    6 ++++++
 3 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/common/cmd_mmc.c b/common/cmd_mmc.c
index 1a23b28..56c8680 100644
--- a/common/cmd_mmc.c
+++ b/common/cmd_mmc.c
@@ -95,16 +95,22 @@ static void print_mmcinfo(struct mmc *mmc)
 	printf("Bus Width: %d-bit\n", mmc->bus_width);
 
 	if (!IS_SD(mmc) && (mmc->version >= MMC_VERSION_4)) {
+		bool has_enh = (mmc->part_support & ENHNCD_SUPPORT) != 0;
 		puts("User Capacity: ");
-		print_size(mmc->capacity_user, "\n");
+		print_size(mmc->capacity_user,
+			   has_enh && (mmc->part_attr & EXT_CSD_ENH_USR) ?
+			   " ENH\n" : "\n");
 		puts("Boot Capacity: ");
-		print_size(mmc->capacity_boot, "\n");
+		print_size(mmc->capacity_boot, has_enh ? " ENH\n" : "\n");
 		puts("RPMB Capacity: ");
-		print_size(mmc->capacity_rpmb, "\n");
+		print_size(mmc->capacity_rpmb, has_enh ? " ENH\n" : "\n");
 		for (i = 0; i < ARRAY_SIZE(mmc->capacity_gp); i++) {
+			bool is_enh = has_enh &&
+				(mmc->part_attr & EXT_CSD_ENH_GP(i));
 			if (mmc->capacity_gp[i]) {
 				printf("GP%i Capacity: ", i);
-				print_size(mmc->capacity_gp[i], "\n");
+				print_size(mmc->capacity_gp[i],
+					   is_enh ? " ENH\n" : "\n");
 			}
 		}
 	}
diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index 44a4feb..89f33da 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -1032,9 +1032,12 @@ static int mmc_startup(struct mmc *mmc)
 		}
 
 		/* store the partition info of emmc */
+		mmc->part_support = ext_csd[EXT_CSD_PARTITIONING_SUPPORT];
 		if ((ext_csd[EXT_CSD_PARTITIONING_SUPPORT] & PART_SUPPORT) ||
 		    ext_csd[EXT_CSD_BOOT_MULT])
 			mmc->part_config = ext_csd[EXT_CSD_PART_CONF];
+		if (ext_csd[EXT_CSD_PARTITIONING_SUPPORT] & ENHNCD_SUPPORT)
+			mmc->part_attr = ext_csd[EXT_CSD_PARTITIONS_ATTRIBUTE];
 
 		mmc->capacity_boot = ext_csd[EXT_CSD_BOOT_MULT] << 17;
 
diff --git a/include/mmc.h b/include/mmc.h
index d74a190..739845f 100644
--- a/include/mmc.h
+++ b/include/mmc.h
@@ -197,6 +197,9 @@
 #define EXT_CSD_BOOT_BUS_WIDTH_RESET(x)	(x << 2)
 #define EXT_CSD_BOOT_BUS_WIDTH_WIDTH(x)	(x)
 
+#define EXT_CSD_ENH_USR		(1 << 0)	/* user data area is enhanced */
+#define EXT_CSD_ENH_GP(x)	(1 << ((x)+1))	/* GP part (x+1) is enhanced */
+
 #define R1_ILLEGAL_COMMAND		(1 << 22)
 #define R1_APP_CMD			(1 << 5)
 
@@ -220,6 +223,7 @@
 #define MMCPART_NOAVAILABLE	(0xff)
 #define PART_ACCESS_MASK	(0x7)
 #define PART_SUPPORT		(0x1)
+#define ENHNCD_SUPPORT		(0x2)
 #define PART_ENH_ATTRIB		(0x1f)
 
 /* Maximum block size for MMC */
@@ -298,6 +302,8 @@ struct mmc {
 	uint csd[4];
 	uint cid[4];
 	ushort rca;
+	u8 part_support;
+	u8 part_attr;
 	char part_config;
 	char part_num;
 	uint tran_speed;
-- 
1.7.1

[U-Boot] [PATCH 03/18] mmc: make eMMC general purpose partition numbering match spec

[Diego Santa Cruz]

The eMMC spec numbers general purpose partitions starting at 1, but
the mmcinfo output follows the internal numbering which starts at 0.
Make the mmcinfo command output number partitions as in the eMMC
spec to avoid confusion.
---
 common/cmd_mmc.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/common/cmd_mmc.c b/common/cmd_mmc.c
index 56c8680..414fac6 100644
--- a/common/cmd_mmc.c
+++ b/common/cmd_mmc.c
@@ -108,7 +108,7 @@ static void print_mmcinfo(struct mmc *mmc)
 			bool is_enh = has_enh &&
 				(mmc->part_attr & EXT_CSD_ENH_GP(i));
 			if (mmc->capacity_gp[i]) {
-				printf("GP%i Capacity: ", i);
+				printf("GP%i Capacity: ", i+1);
 				print_size(mmc->capacity_gp[i],
 					   is_enh ? " ENH\n" : "\n");
 			}
-- 
1.7.1

[U-Boot] [PATCH 04/18] mmc: skip mmcinfo partition info processing for eMMC < 4.41

[Diego Santa Cruz]

eMMC partitions are defined as of eMMC 4.41, but mmcinfo process
partition info for eMMC >= 4.0, change it to do it for >= 4.41
---
 common/cmd_mmc.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/common/cmd_mmc.c b/common/cmd_mmc.c
index 414fac6..bc02273 100644
--- a/common/cmd_mmc.c
+++ b/common/cmd_mmc.c
@@ -94,7 +94,7 @@ static void print_mmcinfo(struct mmc *mmc)
 
 	printf("Bus Width: %d-bit\n", mmc->bus_width);
 
-	if (!IS_SD(mmc) && (mmc->version >= MMC_VERSION_4)) {
+	if (!IS_SD(mmc) && (mmc->version >= MMC_VERSION_4_41)) {
 		bool has_enh = (mmc->part_support & ENHNCD_SUPPORT) != 0;
 		puts("User Capacity: ");
 		print_size(mmc->capacity_user,
-- 
1.7.1

[U-Boot] [PATCH 05/18] mmc: incomplete test to switch to high-capacity group size definitions

[Diego Santa Cruz]

The eMMC spec mandates that the high-capacity group size definitions
should be enabled when the device is partitioned (by setting
ERASE_GROUP_DEF in EXT_CSD). The current test to determine when this is
required misses a few cases. In particular a device may have been
partitioned without setting the enhanced attribute on any partition
or partitioning may be completed without creating any extra partitions.

This change moves the code to set ERASE_GROUP_DEF to after reading
all partition information. It is also enabled when
PARTITIONING_SETTING_COMPLETED is set as it is necessary to enable
ERASE_GROUP_DEF before setting that bit, so it means that the user
previously switched to the high capacity definitions.
---
 drivers/mmc/mmc.c |   51 +++++++++++++++++++++++++++++----------------------
 include/mmc.h     |    2 ++
 2 files changed, 31 insertions(+), 22 deletions(-)

diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index 89f33da..c045f9e 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -818,6 +818,7 @@ static int mmc_startup(struct mmc *mmc)
 	ALLOC_CACHE_ALIGN_BUFFER(u8, ext_csd, MMC_MAX_BLOCK_LEN);
 	ALLOC_CACHE_ALIGN_BUFFER(u8, test_csd, MMC_MAX_BLOCK_LEN);
 	int timeout = 1000;
+	bool has_parts = false;
 
 #ifdef CONFIG_MMC_SPI_CRC_ON
 	if (mmc_host_is_spi(mmc)) { /* enable CRC check for spi */
@@ -1003,13 +1004,40 @@ static int mmc_startup(struct mmc *mmc)
 			break;
 		}
 
+		/* store the partition info of emmc */
+		mmc->part_support = ext_csd[EXT_CSD_PARTITIONING_SUPPORT];
+		if ((ext_csd[EXT_CSD_PARTITIONING_SUPPORT] & PART_SUPPORT) ||
+		    ext_csd[EXT_CSD_BOOT_MULT])
+			mmc->part_config = ext_csd[EXT_CSD_PART_CONF];
+		if (ext_csd[EXT_CSD_PARTITIONING_SUPPORT] & ENHNCD_SUPPORT)
+			mmc->part_attr = ext_csd[EXT_CSD_PARTITIONS_ATTRIBUTE];
+
+		mmc->capacity_boot = ext_csd[EXT_CSD_BOOT_MULT] << 17;
+
+		mmc->capacity_rpmb = ext_csd[EXT_CSD_RPMB_MULT] << 17;
+
+		for (i = 0; i < 4; i++) {
+			int idx = EXT_CSD_GP_SIZE_MULT + i * 3;
+			mmc->capacity_gp[i] = (ext_csd[idx + 2] << 16) +
+				(ext_csd[idx + 1] << 8) + ext_csd[idx];
+			mmc->capacity_gp[i] *=
+				ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE];
+			mmc->capacity_gp[i] *= ext_csd[EXT_CSD_HC_WP_GRP_SIZE];
+			if (mmc->capacity_gp[i])
+				has_parts = true;
+		}
+
 		/*
 		 * Host needs to enable ERASE_GRP_DEF bit if device is
 		 * partitioned. This bit will be lost every time after a reset
 		 * or power off. This will affect erase size.
 		 */
+		if (ext_csd[EXT_CSD_PART_SETTING_COMPLETED] & PART_SETTING_COMPLETED)
+			has_parts = true;
 		if ((ext_csd[EXT_CSD_PARTITIONING_SUPPORT] & PART_SUPPORT) &&
-		    (ext_csd[EXT_CSD_PARTITIONS_ATTRIBUTE] & PART_ENH_ATTRIB)) {
+		    (ext_csd[EXT_CSD_PARTITIONS_ATTRIBUTE] & PART_ENH_ATTRIB))
+			has_parts = true;
+		if (has_parts) {
 			err = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL,
 				EXT_CSD_ERASE_GROUP_DEF, 1);
 
@@ -1030,27 +1058,6 @@ static int mmc_startup(struct mmc *mmc)
 			mmc->erase_grp_size = (erase_gsz + 1)
 				* (erase_gmul + 1);
 		}
-
-		/* store the partition info of emmc */
-		mmc->part_support = ext_csd[EXT_CSD_PARTITIONING_SUPPORT];
-		if ((ext_csd[EXT_CSD_PARTITIONING_SUPPORT] & PART_SUPPORT) ||
-		    ext_csd[EXT_CSD_BOOT_MULT])
-			mmc->part_config = ext_csd[EXT_CSD_PART_CONF];
-		if (ext_csd[EXT_CSD_PARTITIONING_SUPPORT] & ENHNCD_SUPPORT)
-			mmc->part_attr = ext_csd[EXT_CSD_PARTITIONS_ATTRIBUTE];
-
-		mmc->capacity_boot = ext_csd[EXT_CSD_BOOT_MULT] << 17;
-
-		mmc->capacity_rpmb = ext_csd[EXT_CSD_RPMB_MULT] << 17;
-
-		for (i = 0; i < 4; i++) {
-			int idx = EXT_CSD_GP_SIZE_MULT + i * 3;
-			mmc->capacity_gp[i] = (ext_csd[idx + 2] << 16) +
-				(ext_csd[idx + 1] << 8) + ext_csd[idx];
-			mmc->capacity_gp[i] *=
-				ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE];
-			mmc->capacity_gp[i] *= ext_csd[EXT_CSD_HC_WP_GRP_SIZE];
-		}
 	}
 
 	err = mmc_set_capacity(mmc, mmc->part_num);
diff --git a/include/mmc.h b/include/mmc.h
index 739845f..160e227 100644
--- a/include/mmc.h
+++ b/include/mmc.h
@@ -147,6 +147,7 @@
  * EXT_CSD fields
  */
 #define EXT_CSD_GP_SIZE_MULT		143	/* R/W */
+#define EXT_CSD_PART_SETTING_COMPLETED	155	/* R/W */
 #define EXT_CSD_PARTITIONS_ATTRIBUTE	156	/* R/W */
 #define EXT_CSD_PARTITIONING_SUPPORT	160	/* RO */
 #define EXT_CSD_RST_N_FUNCTION		162	/* R/W */
@@ -220,6 +221,7 @@
 #define MMC_RSP_R6	(MMC_RSP_PRESENT|MMC_RSP_CRC|MMC_RSP_OPCODE)
 #define MMC_RSP_R7	(MMC_RSP_PRESENT|MMC_RSP_CRC|MMC_RSP_OPCODE)
 
+#define PART_SETTING_COMPLETED	(1 << 0)
 #define MMCPART_NOAVAILABLE	(0xff)
 #define PART_ACCESS_MASK	(0x7)
 #define PART_SUPPORT		(0x1)
-- 
1.7.1

[U-Boot] [PATCH 06/18] mmc: computation of eMMC GP partition size was missing 512 KiB factor

[Diego Santa Cruz]

---
 drivers/mmc/mmc.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index c045f9e..b088fd7 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -1023,6 +1023,7 @@ static int mmc_startup(struct mmc *mmc)
 			mmc->capacity_gp[i] *=
 				ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE];
 			mmc->capacity_gp[i] *= ext_csd[EXT_CSD_HC_WP_GRP_SIZE];
+			mmc->capacity_gp[i] <<= 19;
 			if (mmc->capacity_gp[i])
 				has_parts = true;
 		}
-- 
1.7.1

[U-Boot] [PATCH 07/18] mmc: read the size of eMMC enhanced user data area

[Diego Santa Cruz]

This modification reads the size of the eMMC enhanced user data area
upon initialization of an mmc device, it will be used later by
mmcinfo.
---
 drivers/mmc/mmc.c |   15 +++++++++++++++
 include/mmc.h     |    4 ++++
 2 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index b088fd7..b0b4c8e 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -1028,6 +1028,21 @@ static int mmc_startup(struct mmc *mmc)
 				has_parts = true;
 		}
 
+		mmc->enh_user_size =
+			(ext_csd[EXT_CSD_ENH_SIZE_MULT+2] << 16) +
+			(ext_csd[EXT_CSD_ENH_SIZE_MULT+1] << 8) +
+			ext_csd[EXT_CSD_ENH_SIZE_MULT];
+		mmc->enh_user_size *= ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE];
+		mmc->enh_user_size *= ext_csd[EXT_CSD_HC_WP_GRP_SIZE];
+		mmc->enh_user_size <<= 19;
+		mmc->enh_user_start =
+			(ext_csd[EXT_CSD_ENH_START_ADDR+3] << 24) +
+			(ext_csd[EXT_CSD_ENH_START_ADDR+2] << 16) +
+			(ext_csd[EXT_CSD_ENH_START_ADDR+1] << 8) +
+			ext_csd[EXT_CSD_ENH_START_ADDR];
+		if (mmc->high_capacity)
+			mmc->enh_user_start <<= 9;
+
 		/*
 		 * Host needs to enable ERASE_GRP_DEF bit if device is
 		 * partitioned. This bit will be lost every time after a reset
diff --git a/include/mmc.h b/include/mmc.h
index 160e227..8500949 100644
--- a/include/mmc.h
+++ b/include/mmc.h
@@ -146,6 +146,8 @@
 /*
  * EXT_CSD fields
  */
+#define EXT_CSD_ENH_START_ADDR		136	/* R/W */
+#define EXT_CSD_ENH_SIZE_MULT		140	/* R/W */
 #define EXT_CSD_GP_SIZE_MULT		143	/* R/W */
 #define EXT_CSD_PART_SETTING_COMPLETED	155	/* R/W */
 #define EXT_CSD_PARTITIONS_ATTRIBUTE	156	/* R/W */
@@ -317,6 +319,8 @@ struct mmc {
 	u64 capacity_boot;
 	u64 capacity_rpmb;
 	u64 capacity_gp[4];
+	u64 enh_user_start;
+	u64 enh_user_size;
 	block_dev_desc_t block_dev;
 	char op_cond_pending;	/* 1 if we are waiting on an op_cond command */
 	char init_in_progress;	/* 1 if we have done mmc_start_init() */
-- 
1.7.1

[U-Boot] [PATCH 08/18] mmc: display size and start of eMMC enhanced user data area in mmcinfo

[Diego Santa Cruz]

This adds output to show the eMMC enhanced user data area size and offset
along with the partition sizes in mmcinfo's output.
---
 common/cmd_mmc.c |   11 ++++++++---
 1 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/common/cmd_mmc.c b/common/cmd_mmc.c
index bc02273..aa5ba0e 100644
--- a/common/cmd_mmc.c
+++ b/common/cmd_mmc.c
@@ -96,10 +96,15 @@ static void print_mmcinfo(struct mmc *mmc)
 
 	if (!IS_SD(mmc) && (mmc->version >= MMC_VERSION_4_41)) {
 		bool has_enh = (mmc->part_support & ENHNCD_SUPPORT) != 0;
+		bool usr_enh = has_enh && (mmc->part_attr & EXT_CSD_ENH_USR);
 		puts("User Capacity: ");
-		print_size(mmc->capacity_user,
-			   has_enh && (mmc->part_attr & EXT_CSD_ENH_USR) ?
-			   " ENH\n" : "\n");
+		print_size(mmc->capacity_user, usr_enh ? " ENH\n" : "\n");
+		if (usr_enh) {
+			puts("User Enhanced Start: ");
+			print_size(mmc->enh_user_start, "\n");
+			puts("User Enhanced Size: ");
+			print_size(mmc->enh_user_size, "\n");
+		}
 		puts("Boot Capacity: ");
 		print_size(mmc->capacity_boot, has_enh ? " ENH\n" : "\n");
 		puts("RPMB Capacity: ");
-- 
1.7.1

[U-Boot] [PATCH 09/18] mmc: fix erase_grp_size computation with high-capacity size definition

[Diego Santa Cruz]

The erase_grp_size in struct mmc is to be a size in 512-byte sectors
but the code used to compute it for eMMC when EXT_CSD_ERASE_GROUP_DEF is
enabled computed it as bytes, leading to erase sizes and alignment
much larger than what is actually required by the mmc device.
---
 drivers/mmc/mmc.c |    3 +--
 include/mmc.h     |    2 +-
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index b0b4c8e..cfba0aa 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -1064,8 +1064,7 @@ static int mmc_startup(struct mmc *mmc)
 
 			/* Read out group size from ext_csd */
 			mmc->erase_grp_size =
-				ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE] *
-					MMC_MAX_BLOCK_LEN * 1024;
+				ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE] * 1024;
 		} else {
 			/* Calculate the group size from the csd value. */
 			int erase_gsz, erase_gmul;
diff --git a/include/mmc.h b/include/mmc.h
index 8500949..fcfe9c9 100644
--- a/include/mmc.h
+++ b/include/mmc.h
@@ -313,7 +313,7 @@ struct mmc {
 	uint tran_speed;
 	uint read_bl_len;
 	uint write_bl_len;
-	uint erase_grp_size;
+	uint erase_grp_size;	/* in 512-byte sectors */
 	u64 capacity;
 	u64 capacity_user;
 	u64 capacity_boot;
-- 
1.7.1

[U-Boot] [PATCH 10/18] mmc: read the high capacity WP group size for eMMC

[Diego Santa Cruz]

Read the eMMC high capacity write protect group size at mmc device
initialization. This is useful to correctly partition an eMMC device,
as partitions need to be aligned to this size.
---
 drivers/mmc/mmc.c |    6 ++++++
 include/mmc.h     |    1 +
 2 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index cfba0aa..3879899 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -1061,7 +1061,9 @@ static int mmc_startup(struct mmc *mmc)
 				return err;
 			else
 				ext_csd[EXT_CSD_ERASE_GROUP_DEF] = 1;
+		}
 
+		if (ext_csd[EXT_CSD_ERASE_GROUP_DEF] & 0x01) {
 			/* Read out group size from ext_csd */
 			mmc->erase_grp_size =
 				ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE] * 1024;
@@ -1073,6 +1075,10 @@ static int mmc_startup(struct mmc *mmc)
 			mmc->erase_grp_size = (erase_gsz + 1)
 				* (erase_gmul + 1);
 		}
+
+		mmc->hc_wp_grp_size = 1024
+			* ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE]
+			* ext_csd[EXT_CSD_HC_WP_GRP_SIZE];
 	}
 
 	err = mmc_set_capacity(mmc, mmc->part_num);
diff --git a/include/mmc.h b/include/mmc.h
index fcfe9c9..bf08bec 100644
--- a/include/mmc.h
+++ b/include/mmc.h
@@ -314,6 +314,7 @@ struct mmc {
 	uint read_bl_len;
 	uint write_bl_len;
 	uint erase_grp_size;	/* in 512-byte sectors */
+	uint hc_wp_grp_size;	/* in 512-byte sectors */
 	u64 capacity;
 	u64 capacity_user;
 	u64 capacity_boot;
-- 
1.7.1

[U-Boot] [PATCH 11/18] mmc: show the erase group size and HC WP group size in mmcinfo output

[Diego Santa Cruz]

This adds the erase group size and high-capacity WP group size to
mmcinfo's output. The erase group size is necessary to properly align
erase requests on eMMC. The high-capacity WP group size is necessary
to properly align partitions on eMMC.
---
 common/cmd_mmc.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/common/cmd_mmc.c b/common/cmd_mmc.c
index aa5ba0e..7b7167c 100644
--- a/common/cmd_mmc.c
+++ b/common/cmd_mmc.c
@@ -94,9 +94,16 @@ static void print_mmcinfo(struct mmc *mmc)
 
 	printf("Bus Width: %d-bit\n", mmc->bus_width);
 
+	puts("Erase Group Size: ");
+	print_size(((u64)mmc->erase_grp_size) << 9, "\n");
+
 	if (!IS_SD(mmc) && (mmc->version >= MMC_VERSION_4_41)) {
 		bool has_enh = (mmc->part_support & ENHNCD_SUPPORT) != 0;
 		bool usr_enh = has_enh && (mmc->part_attr & EXT_CSD_ENH_USR);
+
+		puts("HC WP Group Size: ");
+		print_size(((u64)mmc->hc_wp_grp_size) << 9, "\n");
+
 		puts("User Capacity: ");
 		print_size(mmc->capacity_user, usr_enh ? " ENH\n" : "\n");
 		if (usr_enh) {
@@ -105,10 +112,13 @@ static void print_mmcinfo(struct mmc *mmc)
 			puts("User Enhanced Size: ");
 			print_size(mmc->enh_user_size, "\n");
 		}
+
 		puts("Boot Capacity: ");
 		print_size(mmc->capacity_boot, has_enh ? " ENH\n" : "\n");
+
 		puts("RPMB Capacity: ");
 		print_size(mmc->capacity_rpmb, has_enh ? " ENH\n" : "\n");
+
 		for (i = 0; i < ARRAY_SIZE(mmc->capacity_gp); i++) {
 			bool is_enh = has_enh &&
 				(mmc->part_attr & EXT_CSD_ENH_GP(i));
-- 
1.7.1

[U-Boot] [PATCH 12/18] mmc: eMMC partitioning data is not effective till partitioning completed

[Diego Santa Cruz]

The eMMC spec says that partitioning is only effective after the
PARTITION_SETTING_COMPLETED is set in EXT_CSD (and a power cycle was done,
but that we cannot know). Thus the partition sizes and attributes should
be ignored when that bit is not set, otherwise the various capacities
are not coherent (e.g., the user data capacity will be that of the
unpartitioned device while partition sizes would be non-zero).

Prescence of non-zero partitioning data is nevertheless still used to
activate the high-capacity size definitions (EXT_CSD_ERASE_GROUP_DEF)
as it is necessary to set that to write any of the partitioning fields
in EXT_CSD, so having partitioning data means someone previously
activated that and we should keep it activated.
---
 drivers/mmc/mmc.c |   53 ++++++++++++++++++++++++++++++++++-------------------
 1 files changed, 34 insertions(+), 19 deletions(-)

diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index 3879899..f545576 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -819,6 +819,7 @@ static int mmc_startup(struct mmc *mmc)
 	ALLOC_CACHE_ALIGN_BUFFER(u8, test_csd, MMC_MAX_BLOCK_LEN);
 	int timeout = 1000;
 	bool has_parts = false;
+	bool part_completed;
 
 #ifdef CONFIG_MMC_SPI_CRC_ON
 	if (mmc_host_is_spi(mmc)) { /* enable CRC check for spi */
@@ -1004,12 +1005,21 @@ static int mmc_startup(struct mmc *mmc)
 			break;
 		}
 
+		/* The partition data may be non-zero but it is only
+		 * effective if PARTITION_SETTING_COMPLETED is set in
+		 * EXT_CSD, so ignore any data if this bit is not set,
+		 * except for enabling the high-capacity group size
+		 * definition (see below). */
+		part_completed = !!(ext_csd[EXT_CSD_PART_SETTING_COMPLETED] &
+				    PART_SETTING_COMPLETED);
+
 		/* store the partition info of emmc */
 		mmc->part_support = ext_csd[EXT_CSD_PARTITIONING_SUPPORT];
 		if ((ext_csd[EXT_CSD_PARTITIONING_SUPPORT] & PART_SUPPORT) ||
 		    ext_csd[EXT_CSD_BOOT_MULT])
 			mmc->part_config = ext_csd[EXT_CSD_PART_CONF];
-		if (ext_csd[EXT_CSD_PARTITIONING_SUPPORT] & ENHNCD_SUPPORT)
+		if (part_completed &&
+		    (ext_csd[EXT_CSD_PARTITIONING_SUPPORT] & ENHNCD_SUPPORT))
 			mmc->part_attr = ext_csd[EXT_CSD_PARTITIONS_ATTRIBUTE];
 
 		mmc->capacity_boot = ext_csd[EXT_CSD_BOOT_MULT] << 17;
@@ -1018,37 +1028,42 @@ static int mmc_startup(struct mmc *mmc)
 
 		for (i = 0; i < 4; i++) {
 			int idx = EXT_CSD_GP_SIZE_MULT + i * 3;
-			mmc->capacity_gp[i] = (ext_csd[idx + 2] << 16) +
+			uint mult = (ext_csd[idx + 2] << 16) +
 				(ext_csd[idx + 1] << 8) + ext_csd[idx];
+			if (mult)
+				has_parts = true;
+			if (!part_completed)
+				continue;
+			mmc->capacity_gp[i] = mult;
 			mmc->capacity_gp[i] *=
 				ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE];
 			mmc->capacity_gp[i] *= ext_csd[EXT_CSD_HC_WP_GRP_SIZE];
 			mmc->capacity_gp[i] <<= 19;
-			if (mmc->capacity_gp[i])
-				has_parts = true;
 		}
 
-		mmc->enh_user_size =
-			(ext_csd[EXT_CSD_ENH_SIZE_MULT+2] << 16) +
-			(ext_csd[EXT_CSD_ENH_SIZE_MULT+1] << 8) +
-			ext_csd[EXT_CSD_ENH_SIZE_MULT];
-		mmc->enh_user_size *= ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE];
-		mmc->enh_user_size *= ext_csd[EXT_CSD_HC_WP_GRP_SIZE];
-		mmc->enh_user_size <<= 19;
-		mmc->enh_user_start =
-			(ext_csd[EXT_CSD_ENH_START_ADDR+3] << 24) +
-			(ext_csd[EXT_CSD_ENH_START_ADDR+2] << 16) +
-			(ext_csd[EXT_CSD_ENH_START_ADDR+1] << 8) +
-			ext_csd[EXT_CSD_ENH_START_ADDR];
-		if (mmc->high_capacity)
-			mmc->enh_user_start <<= 9;
+		if (part_completed) {
+			mmc->enh_user_size =
+				(ext_csd[EXT_CSD_ENH_SIZE_MULT+2] << 16) +
+				(ext_csd[EXT_CSD_ENH_SIZE_MULT+1] << 8) +
+				ext_csd[EXT_CSD_ENH_SIZE_MULT];
+			mmc->enh_user_size *= ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE];
+			mmc->enh_user_size *= ext_csd[EXT_CSD_HC_WP_GRP_SIZE];
+			mmc->enh_user_size <<= 19;
+			mmc->enh_user_start =
+				(ext_csd[EXT_CSD_ENH_START_ADDR+3] << 24) +
+				(ext_csd[EXT_CSD_ENH_START_ADDR+2] << 16) +
+				(ext_csd[EXT_CSD_ENH_START_ADDR+1] << 8) +
+				ext_csd[EXT_CSD_ENH_START_ADDR];
+			if (mmc->high_capacity)
+				mmc->enh_user_start <<= 9;
+		}
 
 		/*
 		 * Host needs to enable ERASE_GRP_DEF bit if device is
 		 * partitioned. This bit will be lost every time after a reset
 		 * or power off. This will affect erase size.
 		 */
-		if (ext_csd[EXT_CSD_PART_SETTING_COMPLETED] & PART_SETTING_COMPLETED)
+		if (part_completed)
 			has_parts = true;
 		if ((ext_csd[EXT_CSD_PARTITIONING_SUPPORT] & PART_SUPPORT) &&
 		    (ext_csd[EXT_CSD_PARTITIONS_ATTRIBUTE] & PART_ENH_ATTRIB))
-- 
1.7.1

[U-Boot] [PATCH 13/18] mmc: the ext_csd data may be used during init even if reading failed

[Diego Santa Cruz]

The mmc_startup() function uses the ext_csd data even if reading it
from the mmc device failed. This bug was introduced in commit
bc897b1d4d86597311430dbe7b3e6c807c8c53e5. We now bail out if
reading it fails, this should not be a problem as ext_csd was
introduced in MMC 4.0 and this code is conditional on MMC >= 4.0.
---
 drivers/mmc/mmc.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index f545576..8f6cbd5 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -972,7 +972,9 @@ static int mmc_startup(struct mmc *mmc)
 	if (!IS_SD(mmc) && (mmc->version >= MMC_VERSION_4)) {
 		/* check  ext_csd version and capacity */
 		err = mmc_send_ext_csd(mmc, ext_csd);
-		if (!err && (ext_csd[EXT_CSD_REV] >= 2)) {
+		if (err)
+			return err;
+		if (ext_csd[EXT_CSD_REV] >= 2) {
 			/*
 			 * According to the JEDEC Standard, the value of
 			 * ext_csd's capacity is valid if the value is more
-- 
1.7.1

[U-Boot] [PATCH 14/18] mmc: add API to do eMMC hardware partitioning

[Diego Santa Cruz]

This adds an API to do hardware partitioning on eMMC devices. The
new mmc_hwpart_config() function does the partitioning in one go.
As the different attributes and partitioning options on eMMC may
be interdependent validation has to be done based on the complete
partitioning configuration. The function accepts three modes:

- MMC_HWPART_CONF_CHECK: just validates that the configuration
  is valid.
- MMC_HWPART_CONF_SET: validates and sets all the fields in
  EXT_CSD but without setting the "partitioning completed" bit,
  and thus is reversible.
- MMC_HWPART_CONF_COMPLETE: does everything and is thus not
  reversible.
---
 drivers/mmc/mmc.c |  154 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 include/mmc.h     |   18 ++++++
 2 files changed, 172 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index 8f6cbd5..311097f 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -605,6 +605,160 @@ int mmc_switch_part(int dev_num, unsigned int part_num)
 	return ret;
 }
 
+int mmc_hwpart_config(struct mmc *mmc,
+		      const struct mmc_hwpart_conf *conf,
+		      enum mmc_hwpart_conf_mode mode)
+{
+	u8 part_attrs = 0;
+	u32 enh_size_mult;
+	u32 enh_start_addr;
+	u32 gp_size_mult[4];
+	u32 max_enh_size_mult;
+	u32 tot_enh_size_mult = 0;
+	int i, pidx, err;
+	ALLOC_CACHE_ALIGN_BUFFER(u8, ext_csd, MMC_MAX_BLOCK_LEN);
+
+	if (mode < MMC_HWPART_CONF_CHECK || mode > MMC_HWPART_CONF_COMPLETE)
+		return -EINVAL;
+
+	if (IS_SD(mmc) || (mmc->version < MMC_VERSION_4_41)) {
+		printf("eMMC >= 4.4 required for enhanced user data area\n");
+		return -EMEDIUMTYPE;
+	}
+
+	if ( ! (mmc->part_support & PART_SUPPORT) ) {
+		printf("Card does not support partitioning\n");
+		return -EMEDIUMTYPE;
+	}
+
+	if ( ! mmc->hc_wp_grp_size ) {
+		printf("Card does not define HC WP group size\n");
+		return -EMEDIUMTYPE;
+	}
+
+	/* check partition alignment and total enhanced size */
+	if (conf->user_enh_size) {
+		if (conf->user_enh_size % mmc->hc_wp_grp_size ||
+		    conf->user_enh_start % mmc->hc_wp_grp_size) {
+			printf("User data enhanced area not HC WP group "
+			       "size aligned\n");
+			return -EINVAL;
+		}
+		part_attrs |= EXT_CSD_ENH_USR;
+		enh_size_mult = conf->user_enh_size / mmc->hc_wp_grp_size;
+		if (mmc->high_capacity) {
+			enh_start_addr = conf->user_enh_start;
+		} else {
+			enh_start_addr = (conf->user_enh_start << 9);
+		}
+	} else {
+		enh_size_mult = 0;
+		enh_start_addr = 0;
+	}
+	tot_enh_size_mult += enh_size_mult;
+
+	for (pidx = 0; pidx < 4; pidx++) {
+		if (conf->gp_part[pidx].size % mmc->hc_wp_grp_size) {
+			printf("GP%i partition not HC WP group size "
+			       "aligned\n", pidx+1);
+			return -EINVAL;
+		}
+		gp_size_mult[pidx] = conf->gp_part[pidx].size / mmc->hc_wp_grp_size;
+		if (conf->gp_part[pidx].size && conf->gp_part[pidx].enhanced) {
+			part_attrs |= EXT_CSD_ENH_GP(pidx);
+			tot_enh_size_mult += gp_size_mult[pidx];
+		}
+	}
+
+	if (part_attrs && ! (mmc->part_support & ENHNCD_SUPPORT)) {
+		printf("Card does not support enhanced attribute\n");
+		return -EMEDIUMTYPE;
+	}
+
+	err = mmc_send_ext_csd(mmc, ext_csd);
+	if (err)
+		return err;
+
+	max_enh_size_mult =
+		(ext_csd[EXT_CSD_MAX_ENH_SIZE_MULT+2] << 16) +
+		(ext_csd[EXT_CSD_MAX_ENH_SIZE_MULT+1] << 8) +
+		ext_csd[EXT_CSD_MAX_ENH_SIZE_MULT];
+	if (tot_enh_size_mult > max_enh_size_mult) {
+		printf("Total enhanced size exceeds maximum (%u > %u)\n",
+		       tot_enh_size_mult, max_enh_size_mult);
+		return -EMEDIUMTYPE;
+	}
+
+	if (ext_csd[EXT_CSD_PART_SETTING_COMPLETED] & PART_SETTING_COMPLETED) {
+		printf("Card already partitioned\n");
+		return -EPERM;
+	}
+
+	if (mode == MMC_HWPART_CONF_CHECK)
+		return 0;
+
+	/* Partitioning requires high-capacity size definitions */
+	if ( ! (ext_csd[EXT_CSD_ERASE_GROUP_DEF] & 0x01) ) {
+		err = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL,
+				 EXT_CSD_ERASE_GROUP_DEF, 1);
+
+		if (err)
+			return err;
+
+		ext_csd[EXT_CSD_ERASE_GROUP_DEF] = 1;
+
+		/* update erase group size to be high-capacity */
+		mmc->erase_grp_size =
+			ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE] * 1024;
+
+	}
+
+	/* all OK, write the configuration */
+	for (i = 0; i < 4; i++) {
+		err = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL,
+				 EXT_CSD_ENH_START_ADDR+i,
+				 (enh_start_addr >> (i*8)) & 0xFF);
+		if (err)
+			return err;
+	}
+	for (i = 0; i < 3; i++) {
+		err = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL,
+				 EXT_CSD_ENH_SIZE_MULT+i,
+				 (enh_size_mult >> (i*8)) & 0xFF);
+		if (err)
+			return err;
+	}
+	for (pidx = 0; pidx < 4; pidx++) {
+		for (i = 0; i < 3; i++) {
+			err = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL,
+					 EXT_CSD_GP_SIZE_MULT+pidx*3+i,
+					 (gp_size_mult[pidx] >> (i*8)) & 0xFF);
+			if (err)
+				return err;
+		}
+	}
+	err = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL,
+			 EXT_CSD_PARTITIONS_ATTRIBUTE, part_attrs);
+	if (err)
+		return err;
+
+	if (mode == MMC_HWPART_CONF_SET)
+		return 0;
+
+	/* Setting PART_SETTING_COMPLETED confirms the partition
+	 * configuration but it only becomes effective after power
+	 * cycle, so we do not adjust the partition related settings
+	 * in the mmc struct. */
+
+	err = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL,
+			 EXT_CSD_PART_SETTING_COMPLETED,
+			 PART_SETTING_COMPLETED);
+	if (err)
+		return err;
+
+	return 0;
+}
+
 int mmc_getcd(struct mmc *mmc)
 {
 	int cd;
diff --git a/include/mmc.h b/include/mmc.h
index bf08bec..7892880 100644
--- a/include/mmc.h
+++ b/include/mmc.h
@@ -151,6 +151,7 @@
 #define EXT_CSD_GP_SIZE_MULT		143	/* R/W */
 #define EXT_CSD_PART_SETTING_COMPLETED	155	/* R/W */
 #define EXT_CSD_PARTITIONS_ATTRIBUTE	156	/* R/W */
+#define EXT_CSD_MAX_ENH_SIZE_MULT	157	/* R */
 #define EXT_CSD_PARTITIONING_SUPPORT	160	/* RO */
 #define EXT_CSD_RST_N_FUNCTION		162	/* R/W */
 #define EXT_CSD_RPMB_MULT		168	/* RO */
@@ -329,6 +330,21 @@ struct mmc {
 	uint op_cond_response;	/* the response byte from the last op_cond */
 };
 
+struct mmc_hwpart_conf {
+	uint user_enh_start;	/* in 512-byte sectors */
+	uint user_enh_size;	/* in 512-byte sectors, if 0 no enhanced user data */
+	struct {
+		uint size;	/* in 512-byte sectors */
+		int enhanced;
+	} gp_part[4];
+};
+
+enum mmc_hwpart_conf_mode {
+	MMC_HWPART_CONF_CHECK,
+	MMC_HWPART_CONF_SET,
+	MMC_HWPART_CONF_COMPLETE,
+};
+
 int mmc_register(struct mmc *mmc);
 struct mmc *mmc_create(const struct mmc_config *cfg, void *priv);
 void mmc_destroy(struct mmc *mmc);
@@ -341,6 +357,8 @@ int mmc_set_dev(int dev_num);
 void print_mmc_devices(char separator);
 int get_mmc_num(void);
 int mmc_switch_part(int dev_num, unsigned int part_num);
+int mmc_hwpart_config(struct mmc *mmc, const struct mmc_hwpart_conf *conf,
+		      enum mmc_hwpart_conf_mode mode);
 int mmc_getcd(struct mmc *mmc);
 int board_mmc_getcd(struct mmc *mmc);
 int mmc_getwp(struct mmc *mmc);
-- 
1.7.1

[U-Boot] [PATCH 15/18] mmc: add mmc hwpartition sub-command to do eMMC hardware partitioning

[Diego Santa Cruz]

Adds the mmc hwpartition sub-command to perform eMMC hardware
partitioning on an mmc device. The number of arguments can be
large for a complex partitioning, but as the partitioning has
to be done in one go it is difficult to make it simpler.
---
 common/cmd_mmc.c |   92 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 91 insertions(+), 1 deletions(-)

diff --git a/common/cmd_mmc.c b/common/cmd_mmc.c
index 7b7167c..3943fb7 100644
--- a/common/cmd_mmc.c
+++ b/common/cmd_mmc.c
@@ -481,6 +481,90 @@ static int do_mmc_list(cmd_tbl_t *cmdtp, int flag,
 	print_mmc_devices('\n');
 	return CMD_RET_SUCCESS;
 }
+
+static int do_mmc_hwpartition(cmd_tbl_t *cmdtp, int flag,
+			      int argc, char * const argv[])
+{
+	struct mmc *mmc;
+	struct mmc_hwpart_conf pconf = { };
+	enum mmc_hwpart_conf_mode mode = MMC_HWPART_CONF_CHECK;
+	int i, pidx;
+
+	mmc = init_mmc_device(curr_device, false);
+	if (!mmc)
+		return CMD_RET_FAILURE;
+
+	if (argc < 1)
+		return CMD_RET_USAGE;
+	i = 1;
+	while (i < argc) {
+		if (!strcmp(argv[i], "userenh")) {
+			if (i + 2 >= argc)
+				return CMD_RET_USAGE;
+			pconf.user_enh_start =
+				simple_strtoul(argv[i+1], NULL, 10);
+			pconf.user_enh_size =
+				simple_strtoul(argv[i+2], NULL, 10);
+			i += 3;
+		} else if (!strncmp(argv[i], "gp", 2) &&
+			   strlen(argv[i]) == 3 &&
+			   argv[i][2] >= '1' && argv[i][2] <= '4') {
+			if (i + 1 >= argc)
+				return CMD_RET_USAGE;
+			pidx = argv[i][2] - '1';
+			pconf.gp_part[pidx].size =
+				simple_strtoul(argv[i+1], NULL, 10);
+			if (i + 2 < argc && !strcmp(argv[i+2], "enh")) {
+				pconf.gp_part[pidx].enhanced = 1;
+				i += 3;
+			} else {
+				pconf.gp_part[pidx].enhanced = 0;
+				i += 2;
+			}
+		} else if (!strcmp(argv[i], "check")) {
+			mode = MMC_HWPART_CONF_CHECK;
+			i++;
+		} else if (!strcmp(argv[i], "set")) {
+			mode = MMC_HWPART_CONF_SET;
+			i++;
+		} else if (!strcmp(argv[i], "complete")) {
+			mode = MMC_HWPART_CONF_COMPLETE;
+			i++;
+		} else {
+			return CMD_RET_USAGE;
+		}
+	}
+
+	puts("Partition configuration:\n");
+	if (pconf.user_enh_size) {
+		puts("\tUser Enhanced Start: ");
+		print_size(((u64)pconf.user_enh_start) << 9, "\n");
+		puts("\tUser Enhanced Size: ");
+		print_size(((u64)pconf.user_enh_size) << 9, "\n");
+	} else {
+		puts("\tNo enhanced user data area\n");
+	}
+	for (pidx = 0; pidx < 4; pidx++) {
+		if (pconf.gp_part[pidx].size) {
+			printf("\tGP%i Capacity: ", pidx+1);
+			print_size(((u64)pconf.gp_part[pidx].size) << 9,
+				   pconf.gp_part[pidx].enhanced ?
+				   " ENH\n" : "\n");
+		} else {
+			printf("\tNo GP%i partition\n", pidx+1);
+		}
+	}
+
+	if (!mmc_hwpart_config(mmc, &pconf, mode)) {
+		if (mode == MMC_HWPART_CONF_COMPLETE)
+			puts("Partitioning successful, "
+			     "power-cycle to make effective\n");
+		return CMD_RET_SUCCESS;
+	} else {
+		return CMD_RET_FAILURE;
+	}
+}
+
 #ifdef CONFIG_SUPPORT_EMMC_BOOT
 static int do_mmc_bootbus(cmd_tbl_t *cmdtp, int flag,
 			  int argc, char * const argv[])
@@ -638,6 +722,7 @@ static cmd_tbl_t cmd_mmc[] = {
 	U_BOOT_CMD_MKENT(part, 1, 1, do_mmc_part, "", ""),
 	U_BOOT_CMD_MKENT(dev, 3, 0, do_mmc_dev, "", ""),
 	U_BOOT_CMD_MKENT(list, 1, 1, do_mmc_list, "", ""),
+	U_BOOT_CMD_MKENT(hwpartition, 17, 0, do_mmc_hwpartition, "", ""),
 #ifdef CONFIG_SUPPORT_EMMC_BOOT
 	U_BOOT_CMD_MKENT(bootbus, 5, 0, do_mmc_bootbus, "", ""),
 	U_BOOT_CMD_MKENT(bootpart-resize, 4, 0, do_mmc_boot_resize, "", ""),
@@ -677,7 +762,7 @@ static int do_mmcops(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 }
 
 U_BOOT_CMD(
-	mmc, 7, 1, do_mmcops,
+	mmc, 18, 1, do_mmcops,
 	"MMC sub system",
 	"info - display info of the current MMC device\n"
 	"mmc read addr blk# cnt\n"
@@ -687,6 +772,11 @@ U_BOOT_CMD(
 	"mmc part - lists available partition on current mmc device\n"
 	"mmc dev [dev] [part] - show or set current mmc device [partition]\n"
 	"mmc list - lists available devices\n"
+	"mmc hwpartition [args...] - does hardware partitioning\n"
+	"  arguments (sizes in 512-byte blocks):\n"
+	"    [userenh start cnt] - sets enhanced user data area\n"
+	"    [gp1|gp2|gp3|gp4 cnt [enh]] - general purpose partition\n"
+	"    [check|set|complete] - mode, complete set partitioning completed\n"
 #ifdef CONFIG_SUPPORT_EMMC_BOOT
 	"mmc bootbus dev boot_bus_width reset_boot_bus_width boot_mode\n"
 	" - Set the BOOT_BUS_WIDTH field of the specified device\n"
-- 
1.7.1

[U-Boot] [PATCH 16/18] mmc: extend the mmc hardware partitioning API with write reliability

[Diego Santa Cruz]

The eMMC partition write reliability settings are to be set while
partitioning a device, as per the eMMC spec, so changes to these
attributes needs to be done in the hardware partitioning API.
This commit adds such support.
---
 common/cmd_mmc.c  |   21 +++++++++++----------
 drivers/mmc/mmc.c |   51 +++++++++++++++++++++++++++++++++++++++++++++------
 include/mmc.h     |   19 ++++++++++++++++---
 3 files changed, 72 insertions(+), 19 deletions(-)

diff --git a/common/cmd_mmc.c b/common/cmd_mmc.c
index 3943fb7..c0a4a3e 100644
--- a/common/cmd_mmc.c
+++ b/common/cmd_mmc.c
@@ -501,9 +501,10 @@ static int do_mmc_hwpartition(cmd_tbl_t *cmdtp, int flag,
 		if (!strcmp(argv[i], "userenh")) {
 			if (i + 2 >= argc)
 				return CMD_RET_USAGE;
-			pconf.user_enh_start =
+			memset(&pconf.user, 0, sizeof(pconf.user));
+			pconf.user.enh_start =
 				simple_strtoul(argv[i+1], NULL, 10);
-			pconf.user_enh_size =
+			pconf.user.enh_size =
 				simple_strtoul(argv[i+2], NULL, 10);
 			i += 3;
 		} else if (!strncmp(argv[i], "gp", 2) &&
@@ -512,14 +513,14 @@ static int do_mmc_hwpartition(cmd_tbl_t *cmdtp, int flag,
 			if (i + 1 >= argc)
 				return CMD_RET_USAGE;
 			pidx = argv[i][2] - '1';
+			memset(&pconf.gp_part[pidx], 0,
+			       sizeof(pconf.gp_part[pidx]));
 			pconf.gp_part[pidx].size =
 				simple_strtoul(argv[i+1], NULL, 10);
-			if (i + 2 < argc && !strcmp(argv[i+2], "enh")) {
+			i += 2;
+			if (i < argc && !strcmp(argv[i], "enh")) {
 				pconf.gp_part[pidx].enhanced = 1;
-				i += 3;
-			} else {
-				pconf.gp_part[pidx].enhanced = 0;
-				i += 2;
+				i++;
 			}
 		} else if (!strcmp(argv[i], "check")) {
 			mode = MMC_HWPART_CONF_CHECK;
@@ -536,11 +537,11 @@ static int do_mmc_hwpartition(cmd_tbl_t *cmdtp, int flag,
 	}
 
 	puts("Partition configuration:\n");
-	if (pconf.user_enh_size) {
+	if (pconf.user.enh_size) {
 		puts("\tUser Enhanced Start: ");
-		print_size(((u64)pconf.user_enh_start) << 9, "\n");
+		print_size(((u64)pconf.user.enh_start) << 9, "\n");
 		puts("\tUser Enhanced Size: ");
-		print_size(((u64)pconf.user_enh_size) << 9, "\n");
+		print_size(((u64)pconf.user.enh_size) << 9, "\n");
 	} else {
 		puts("\tNo enhanced user data area\n");
 	}
diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index 311097f..7cd21f2 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -615,6 +615,7 @@ int mmc_hwpart_config(struct mmc *mmc,
 	u32 gp_size_mult[4];
 	u32 max_enh_size_mult;
 	u32 tot_enh_size_mult = 0;
+	u8 wr_rel_set;
 	int i, pidx, err;
 	ALLOC_CACHE_ALIGN_BUFFER(u8, ext_csd, MMC_MAX_BLOCK_LEN);
 
@@ -637,19 +638,19 @@ int mmc_hwpart_config(struct mmc *mmc,
 	}
 
 	/* check partition alignment and total enhanced size */
-	if (conf->user_enh_size) {
-		if (conf->user_enh_size % mmc->hc_wp_grp_size ||
-		    conf->user_enh_start % mmc->hc_wp_grp_size) {
+	if (conf->user.enh_size) {
+		if (conf->user.enh_size % mmc->hc_wp_grp_size ||
+		    conf->user.enh_start % mmc->hc_wp_grp_size) {
 			printf("User data enhanced area not HC WP group "
 			       "size aligned\n");
 			return -EINVAL;
 		}
 		part_attrs |= EXT_CSD_ENH_USR;
-		enh_size_mult = conf->user_enh_size / mmc->hc_wp_grp_size;
+		enh_size_mult = conf->user.enh_size / mmc->hc_wp_grp_size;
 		if (mmc->high_capacity) {
-			enh_start_addr = conf->user_enh_start;
+			enh_start_addr = conf->user.enh_start;
 		} else {
-			enh_start_addr = (conf->user_enh_start << 9);
+			enh_start_addr = (conf->user.enh_start << 9);
 		}
 	} else {
 		enh_size_mult = 0;
@@ -689,6 +690,33 @@ int mmc_hwpart_config(struct mmc *mmc,
 		return -EMEDIUMTYPE;
 	}
 
+	/* The default value of EXT_CSD_WR_REL_SET is device
+	 * dependent, the values can only be changed if the
+	 * EXT_CSD_HS_CTRL_REL bit is set. The values can be
+	 * changed only once and before partitioning is completed. */
+	wr_rel_set = ext_csd[EXT_CSD_WR_REL_SET];
+	if (conf->user.wr_rel_change) {
+		if (conf->user.wr_rel_set)
+			wr_rel_set |= EXT_CSD_WR_DATA_REL_USR;
+		else
+			wr_rel_set &= ~EXT_CSD_WR_DATA_REL_USR;
+	}
+	for (pidx = 0; pidx < 4; pidx++) {
+		if (conf->gp_part[pidx].wr_rel_change) {
+			if (conf->gp_part[pidx].wr_rel_set)
+				wr_rel_set |= EXT_CSD_WR_DATA_REL_GP(pidx);
+			else
+				wr_rel_set &= ~EXT_CSD_WR_DATA_REL_GP(pidx);
+		}
+	}
+
+	if (wr_rel_set != ext_csd[EXT_CSD_WR_REL_SET] &&
+	    !(ext_csd[EXT_CSD_WR_REL_PARAM] & EXT_CSD_HS_CTRL_REL)) {
+		puts("Card does not support host controlled partition write "
+		     "reliability settings\n");
+		return -EMEDIUMTYPE;
+	}
+
 	if (ext_csd[EXT_CSD_PART_SETTING_COMPLETED] & PART_SETTING_COMPLETED) {
 		printf("Card already partitioned\n");
 		return -EPERM;
@@ -745,6 +773,17 @@ int mmc_hwpart_config(struct mmc *mmc,
 	if (mode == MMC_HWPART_CONF_SET)
 		return 0;
 
+	/* The WR_REL_SET is a write-once register but shall be
+	 * written before setting PART_SETTING_COMPLETED. As it is
+	 * write-once we can only write it when completing the
+	 * partitioning. */
+	if (wr_rel_set != ext_csd[EXT_CSD_WR_REL_SET]) {
+		err = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL,
+				 EXT_CSD_WR_REL_SET, wr_rel_set);
+		if (err)
+			return err;
+	}
+
 	/* Setting PART_SETTING_COMPLETED confirms the partition
 	 * configuration but it only becomes effective after power
 	 * cycle, so we do not adjust the partition related settings
diff --git a/include/mmc.h b/include/mmc.h
index 7892880..33595f0 100644
--- a/include/mmc.h
+++ b/include/mmc.h
@@ -154,6 +154,8 @@
 #define EXT_CSD_MAX_ENH_SIZE_MULT	157	/* R */
 #define EXT_CSD_PARTITIONING_SUPPORT	160	/* RO */
 #define EXT_CSD_RST_N_FUNCTION		162	/* R/W */
+#define EXT_CSD_WR_REL_PARAM		166	/* R */
+#define EXT_CSD_WR_REL_SET		167	/* R/W */
 #define EXT_CSD_RPMB_MULT		168	/* RO */
 #define EXT_CSD_ERASE_GROUP_DEF		175	/* R/W */
 #define EXT_CSD_BOOT_BUS_WIDTH		177
@@ -204,6 +206,11 @@
 #define EXT_CSD_ENH_USR		(1 << 0)	/* user data area is enhanced */
 #define EXT_CSD_ENH_GP(x)	(1 << ((x)+1))	/* GP part (x+1) is enhanced */
 
+#define EXT_CSD_HS_CTRL_REL	(1 << 0)	/* host controlled WR_REL_SET */
+
+#define EXT_CSD_WR_DATA_REL_USR		(1 << 0)	/* user data area WR_REL */
+#define EXT_CSD_WR_DATA_REL_GP(x)	(1 << ((x)+1))	/* GP part (x+1) WR_REL */
+
 #define R1_ILLEGAL_COMMAND		(1 << 22)
 #define R1_APP_CMD			(1 << 5)
 
@@ -331,11 +338,17 @@ struct mmc {
 };
 
 struct mmc_hwpart_conf {
-	uint user_enh_start;	/* in 512-byte sectors */
-	uint user_enh_size;	/* in 512-byte sectors, if 0 no enhanced user data */
+	struct {
+		uint enh_start;	/* in 512-byte sectors */
+		uint enh_size;	/* in 512-byte sectors, if 0 no enhanced user data */
+		unsigned wr_rel_change : 1;
+		unsigned wr_rel_set : 1;
+	} user;
 	struct {
 		uint size;	/* in 512-byte sectors */
-		int enhanced;
+		unsigned enhanced : 1;
+		unsigned wr_rel_change : 1;
+		unsigned wr_rel_set : 1;
 	} gp_part[4];
 };
 
-- 
1.7.1

[U-Boot] [PATCH 17/18] mmc: extend the mmc hwpartition sub-command to change write reliability

[Diego Santa Cruz]

This change extends the mmc hwpartition sub-command to change the
per-partition write reliability settings. It also changes the
syntax used for the enhanced user data area slightly to better
accomodate the write reliability option.
---
 common/cmd_mmc.c |  116 ++++++++++++++++++++++++++++++++++++++++++-----------
 1 files changed, 92 insertions(+), 24 deletions(-)

diff --git a/common/cmd_mmc.c b/common/cmd_mmc.c
index c0a4a3e..e226598 100644
--- a/common/cmd_mmc.c
+++ b/common/cmd_mmc.c
@@ -482,13 +482,81 @@ static int do_mmc_list(cmd_tbl_t *cmdtp, int flag,
 	return CMD_RET_SUCCESS;
 }
 
+static int parse_hwpart_user(struct mmc_hwpart_conf *pconf,
+			     int argc, char * const argv[])
+{
+	int i = 0;
+
+	memset(&pconf->user, 0, sizeof(pconf->user));
+
+	while (i < argc) {
+		if (!strcmp(argv[i], "enh")) {
+			if (i + 2 >= argc)
+				return -1;
+			pconf->user.enh_start =
+				simple_strtoul(argv[i+1], NULL, 10);
+			pconf->user.enh_size =
+				simple_strtoul(argv[i+2], NULL, 10);
+			i += 3;
+		} else if (!strcmp(argv[i], "wrrel")) {
+			if (i + 1 >= argc)
+				return -1;
+			pconf->user.wr_rel_change = 1;
+			if (!strcmp(argv[i+1], "on"))
+				pconf->user.wr_rel_set = 1;
+			else if (!strcmp(argv[i+1], "off"))
+				pconf->user.wr_rel_set = 0;
+			else
+				return -1;
+			i += 2;
+		} else {
+			break;
+		}
+	}
+	return i;
+}
+
+static int parse_hwpart_gp(struct mmc_hwpart_conf *pconf, int pidx,
+			   int argc, char * const argv[])
+{
+	int i;
+
+	memset(&pconf->gp_part[pidx], 0, sizeof(pconf->gp_part[pidx]));
+
+	if (1 >= argc)
+		return -1;
+	pconf->gp_part[pidx].size = simple_strtoul(argv[0], NULL, 10);
+
+	i = 1;
+	while (i < argc) {
+		if (!strcmp(argv[i], "enh")) {
+			pconf->gp_part[pidx].enhanced = 1;
+			i += 1;
+		} else if (!strcmp(argv[i], "wrrel")) {
+			if (i + 1 >= argc)
+				return -1;
+			pconf->gp_part[pidx].wr_rel_change = 1;
+			if (!strcmp(argv[i+1], "on"))
+				pconf->gp_part[pidx].wr_rel_set = 1;
+			else if (!strcmp(argv[i+1], "off"))
+				pconf->gp_part[pidx].wr_rel_set = 0;
+			else
+				return -1;
+			i += 2;
+		} else {
+			break;
+		}
+	}
+	return i;
+}
+
 static int do_mmc_hwpartition(cmd_tbl_t *cmdtp, int flag,
 			      int argc, char * const argv[])
 {
 	struct mmc *mmc;
 	struct mmc_hwpart_conf pconf = { };
 	enum mmc_hwpart_conf_mode mode = MMC_HWPART_CONF_CHECK;
-	int i, pidx;
+	int i, r, pidx;
 
 	mmc = init_mmc_device(curr_device, false);
 	if (!mmc)
@@ -498,30 +566,21 @@ static int do_mmc_hwpartition(cmd_tbl_t *cmdtp, int flag,
 		return CMD_RET_USAGE;
 	i = 1;
 	while (i < argc) {
-		if (!strcmp(argv[i], "userenh")) {
-			if (i + 2 >= argc)
+		if (!strcmp(argv[i], "user")) {
+			i++;
+			r = parse_hwpart_user(&pconf, argc-i, &argv[i]);
+			if (r < 0)
 				return CMD_RET_USAGE;
-			memset(&pconf.user, 0, sizeof(pconf.user));
-			pconf.user.enh_start =
-				simple_strtoul(argv[i+1], NULL, 10);
-			pconf.user.enh_size =
-				simple_strtoul(argv[i+2], NULL, 10);
-			i += 3;
+			i += r;
 		} else if (!strncmp(argv[i], "gp", 2) &&
 			   strlen(argv[i]) == 3 &&
 			   argv[i][2] >= '1' && argv[i][2] <= '4') {
-			if (i + 1 >= argc)
-				return CMD_RET_USAGE;
 			pidx = argv[i][2] - '1';
-			memset(&pconf.gp_part[pidx], 0,
-			       sizeof(pconf.gp_part[pidx]));
-			pconf.gp_part[pidx].size =
-				simple_strtoul(argv[i+1], NULL, 10);
-			i += 2;
-			if (i < argc && !strcmp(argv[i], "enh")) {
-				pconf.gp_part[pidx].enhanced = 1;
-				i++;
-			}
+			i++;
+			r = parse_hwpart_gp(&pconf, pidx, argc-i, &argv[i]);
+			if (r < 0)
+				return CMD_RET_USAGE;
+			i += r;
 		} else if (!strcmp(argv[i], "check")) {
 			mode = MMC_HWPART_CONF_CHECK;
 			i++;
@@ -545,6 +604,9 @@ static int do_mmc_hwpartition(cmd_tbl_t *cmdtp, int flag,
 	} else {
 		puts("\tNo enhanced user data area\n");
 	}
+	if (pconf.user.wr_rel_change)
+		printf("\tUser partition write reliability: %s\n",
+		       pconf.user.wr_rel_set ? "on" : "off");
 	for (pidx = 0; pidx < 4; pidx++) {
 		if (pconf.gp_part[pidx].size) {
 			printf("\tGP%i Capacity: ", pidx+1);
@@ -554,6 +616,9 @@ static int do_mmc_hwpartition(cmd_tbl_t *cmdtp, int flag,
 		} else {
 			printf("\tNo GP%i partition\n", pidx+1);
 		}
+		if (pconf.gp_part[pidx].wr_rel_change)
+			printf("\tGP%i write reliability: %s\n", pidx+1,
+			       pconf.gp_part[pidx].wr_rel_set ? "on" : "off");
 	}
 
 	if (!mmc_hwpart_config(mmc, &pconf, mode)) {
@@ -562,6 +627,7 @@ static int do_mmc_hwpartition(cmd_tbl_t *cmdtp, int flag,
 			     "power-cycle to make effective\n");
 		return CMD_RET_SUCCESS;
 	} else {
+		puts("Failed!\n");
 		return CMD_RET_FAILURE;
 	}
 }
@@ -723,7 +789,7 @@ static cmd_tbl_t cmd_mmc[] = {
 	U_BOOT_CMD_MKENT(part, 1, 1, do_mmc_part, "", ""),
 	U_BOOT_CMD_MKENT(dev, 3, 0, do_mmc_dev, "", ""),
 	U_BOOT_CMD_MKENT(list, 1, 1, do_mmc_list, "", ""),
-	U_BOOT_CMD_MKENT(hwpartition, 17, 0, do_mmc_hwpartition, "", ""),
+	U_BOOT_CMD_MKENT(hwpartition, 28, 0, do_mmc_hwpartition, "", ""),
 #ifdef CONFIG_SUPPORT_EMMC_BOOT
 	U_BOOT_CMD_MKENT(bootbus, 5, 0, do_mmc_bootbus, "", ""),
 	U_BOOT_CMD_MKENT(bootpart-resize, 4, 0, do_mmc_boot_resize, "", ""),
@@ -763,7 +829,7 @@ static int do_mmcops(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 }
 
 U_BOOT_CMD(
-	mmc, 18, 1, do_mmcops,
+	mmc, 29, 1, do_mmcops,
 	"MMC sub system",
 	"info - display info of the current MMC device\n"
 	"mmc read addr blk# cnt\n"
@@ -775,9 +841,11 @@ U_BOOT_CMD(
 	"mmc list - lists available devices\n"
 	"mmc hwpartition [args...] - does hardware partitioning\n"
 	"  arguments (sizes in 512-byte blocks):\n"
-	"    [userenh start cnt] - sets enhanced user data area\n"
-	"    [gp1|gp2|gp3|gp4 cnt [enh]] - general purpose partition\n"
+	"    [user [enh start cnt] [wrrel {on|off}]] - sets user data area attributes\n"
+	"    [gp1|gp2|gp3|gp4 cnt [enh] [wrrel {on|off}]] - general purpose partition\n"
 	"    [check|set|complete] - mode, complete set partitioning completed\n"
+	"  WARNING: Partitioning is a write-once setting once it is set to complete.\n"
+	"  Power cycling is required to initialize partitions after set to complete.\n"
 #ifdef CONFIG_SUPPORT_EMMC_BOOT
 	"mmc bootbus dev boot_bus_width reset_boot_bus_width boot_mode\n"
 	" - Set the BOOT_BUS_WIDTH field of the specified device\n"
-- 
1.7.1

[U-Boot] [PATCH 18/18] mmc: extend mmcinfo output to show partition write reliability settings

[Diego Santa Cruz]

This extends the mmcinfo hardware partition info output to show
partitions with write reliability enabled with the "WRREL" string.
If the partition does not have write reliability enabled the "WRREL"
string is omitted; this is analogous to the ehhanced attribute.

Example output:

Device: OMAP SD/MMC
Manufacturer ID: fe
OEM: 14e
Name: MMC16
Tran Speed: 52000000
Rd Block Len: 512
MMC version 4.41
High Capacity: Yes
Capacity: 13.8 GiB
Bus Width: 4-bit
Erase Group Size: 8 MiB
HC WP Group Size: 16 MiB
User Capacity: 13.8 GiB ENH WRREL
User Enhanced Start: 0 Bytes
User Enhanced Size: 512 MiB
Boot Capacity: 16 MiB ENH
RPMB Capacity: 128 KiB ENH
GP1 Capacity: 64 MiB ENH WRREL
GP2 Capacity: 64 MiB ENH WRREL
---
 common/cmd_mmc.c  |   12 ++++++++++--
 drivers/mmc/mmc.c |    2 ++
 include/mmc.h     |    1 +
 3 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/common/cmd_mmc.c b/common/cmd_mmc.c
index e226598..8ff2cdc 100644
--- a/common/cmd_mmc.c
+++ b/common/cmd_mmc.c
@@ -105,7 +105,11 @@ static void print_mmcinfo(struct mmc *mmc)
 		print_size(((u64)mmc->hc_wp_grp_size) << 9, "\n");
 
 		puts("User Capacity: ");
-		print_size(mmc->capacity_user, usr_enh ? " ENH\n" : "\n");
+		print_size(mmc->capacity_user, usr_enh ? " ENH" : "");
+		if (mmc->wr_rel_set & EXT_CSD_WR_DATA_REL_USR)
+			puts(" WRREL\n");
+		else
+			putc('\n');
 		if (usr_enh) {
 			puts("User Enhanced Start: ");
 			print_size(mmc->enh_user_start, "\n");
@@ -125,7 +129,11 @@ static void print_mmcinfo(struct mmc *mmc)
 			if (mmc->capacity_gp[i]) {
 				printf("GP%i Capacity: ", i+1);
 				print_size(mmc->capacity_gp[i],
-					   is_enh ? " ENH\n" : "\n");
+					   is_enh ? " ENH" : "");
+				if (mmc->wr_rel_set & EXT_CSD_WR_DATA_REL_GP(i))
+					puts(" WRREL\n");
+				else
+					putc('\n');
 			}
 		}
 	}
diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index 7cd21f2..9bbf90f 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -1289,6 +1289,8 @@ static int mmc_startup(struct mmc *mmc)
 		mmc->hc_wp_grp_size = 1024
 			* ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE]
 			* ext_csd[EXT_CSD_HC_WP_GRP_SIZE];
+
+		mmc->wr_rel_set = ext_csd[EXT_CSD_WR_REL_SET];
 	}
 
 	err = mmc_set_capacity(mmc, mmc->part_num);
diff --git a/include/mmc.h b/include/mmc.h
index 33595f0..dddde86 100644
--- a/include/mmc.h
+++ b/include/mmc.h
@@ -316,6 +316,7 @@ struct mmc {
 	ushort rca;
 	u8 part_support;
 	u8 part_attr;
+	u8 wr_rel_set;
 	char part_config;
 	char part_num;
 	uint tran_speed;
-- 
1.7.1

[U-Boot] [PATCH 00/18] Support for eMMC partitioning and related fixes

[Pantelis Antoniou]

Hi Diego,

This is quite a large patchset, so we?ll have to take a close look.
 
> On Nov 28, 2014, at 11:10 , Diego Santa Cruz <Diego.SantaCruz@spinetix.com> wrote:
> 
> I have the need to hardware partition eMMC devices from U-Boot along
> with setting enhanced and reliable write attributes.
> 
> This series of patches adds this support to U-Boot via a new mmc
> API, a few new members of struct mmc and a new mmc sub-command. It
> also features several fixes to the eMMC hardware partition support. I
> have tested this with Micron eMMC 4.41 parts and it is working as
> expected.
> 
> I am new to sending patches to the U-Boot mailing list, let alone
> working with git, so please excuse any mishaps.
> 

No worries.

> Diego Santa Cruz (18):
>  mmc: show hardware partition sizes in mmcinfo output
>  mmc: extend mmcinfo to show enhanced partition attribute
>  mmc: make eMMC general purpose partition numbering match spec
>  mmc: skip mmcinfo partition info processing for eMMC < 4.41
>  mmc: incomplete test to switch to high-capacity group size
>    definitions
>  mmc: computation of eMMC GP partition size was missing 512 KiB factor
>  mmc: read the size of eMMC enhanced user data area
>  mmc: display size and start of eMMC enhanced user data area in
>    mmcinfo
>  mmc: fix erase_grp_size computation with high-capacity size
>    definition
>  mmc: read the high capacity WP group size for eMMC
>  mmc: show the erase group size and HC WP group size in mmcinfo output
>  mmc: eMMC partitioning data is not effective till partitioning
>    completed
>  mmc: the ext_csd data may be used during init even if reading failed
>  mmc: add API to do eMMC hardware partitioning
>  mmc: add mmc hwpartition sub-command to do eMMC hardware partitioning
>  mmc: extend the mmc hardware partitioning API with write reliability
>  mmc: extend the mmc hwpartition sub-command to change write
>    reliability
>  mmc: extend mmcinfo output to show partition write reliability
>    settings
> 
> common/cmd_mmc.c  |  207 ++++++++++++++++++++++++++++++++++++++-
> drivers/mmc/mmc.c |  283 +++++++++++++++++++++++++++++++++++++++++++++++++----
> include/mmc.h     |   47 +++++++++-
> 3 files changed, 515 insertions(+), 22 deletions(-)
> 

Regards

? Pantelis

[U-Boot] [PATCH 00/18] Support for eMMC partitioning and related fixes

[Joakim Tjernlund]

> 
> I have the need to hardware partition eMMC devices from U-Boot along
> with setting enhanced and reliable write attributes.
> 
> This series of patches adds this support to U-Boot via a new mmc
> API, a few new members of struct mmc and a new mmc sub-command. It
> also features several fixes to the eMMC hardware partition support. I
> have tested this with Micron eMMC 4.41 parts and it is working as
> expected.

This is really great, thanks.

I do wonder(I am fairly new to eMMC) about 512B emulation.
In mmc utils there is a:
mmc disable 512B emulation <device>
        Set the eMMC data sector size to 4KB by disabling emulation on 
<device>.

I am hoping 4K size will increase performance and reliability?
Could you add this feature too to your patch set?

  Jocke

[U-Boot] [PATCH 16/18] mmc: extend the mmc hardware partitioning API with write reliability

[Pantelis Antoniou]

Hi Diego,

At first glance it?s OK, I?m not the turning of user_enh_start, _size to a structure
has a lot of benefits. It makes the diff longer.

> On Nov 28, 2014, at 11:10 , Diego Santa Cruz <Diego.SantaCruz@spinetix.com> wrote:
> 
> The eMMC partition write reliability settings are to be set while
> partitioning a device, as per the eMMC spec, so changes to these
> attributes needs to be done in the hardware partitioning API.
> This commit adds such support.
> ---
> common/cmd_mmc.c  |   21 +++++++++++----------
> drivers/mmc/mmc.c |   51 +++++++++++++++++++++++++++++++++++++++++++++------
> include/mmc.h     |   19 ++++++++++++++++---
> 3 files changed, 72 insertions(+), 19 deletions(-)
> 
> diff --git a/common/cmd_mmc.c b/common/cmd_mmc.c
> index 3943fb7..c0a4a3e 100644
> --- a/common/cmd_mmc.c
> +++ b/common/cmd_mmc.c
> @@ -501,9 +501,10 @@ static int do_mmc_hwpartition(cmd_tbl_t *cmdtp, int flag,
> 		if (!strcmp(argv[i], "userenh")) {
> 			if (i + 2 >= argc)
> 				return CMD_RET_USAGE;
> -			pconf.user_enh_start =
> +			memset(&pconf.user, 0, sizeof(pconf.user));
> +			pconf.user.enh_start =
> 				simple_strtoul(argv[i+1], NULL, 10);
> -			pconf.user_enh_size =
> +			pconf.user.enh_size =
> 				simple_strtoul(argv[i+2], NULL, 10);
> 			i += 3;
> 		} else if (!strncmp(argv[i], "gp", 2) &&
> @@ -512,14 +513,14 @@ static int do_mmc_hwpartition(cmd_tbl_t *cmdtp, int flag,
> 			if (i + 1 >= argc)
> 				return CMD_RET_USAGE;
> 			pidx = argv[i][2] - '1';
> +			memset(&pconf.gp_part[pidx], 0,
> +			       sizeof(pconf.gp_part[pidx]));
> 			pconf.gp_part[pidx].size =
> 				simple_strtoul(argv[i+1], NULL, 10);
> -			if (i + 2 < argc && !strcmp(argv[i+2], "enh")) {
> +			i += 2;
> +			if (i < argc && !strcmp(argv[i], "enh")) {
> 				pconf.gp_part[pidx].enhanced = 1;
> -				i += 3;
> -			} else {
> -				pconf.gp_part[pidx].enhanced = 0;
> -				i += 2;
> +				i++;
> 			}
> 		} else if (!strcmp(argv[i], "check")) {
> 			mode = MMC_HWPART_CONF_CHECK;
> @@ -536,11 +537,11 @@ static int do_mmc_hwpartition(cmd_tbl_t *cmdtp, int flag,
> 	}
> 
> 	puts("Partition configuration:\n");
> -	if (pconf.user_enh_size) {
> +	if (pconf.user.enh_size) {
> 		puts("\tUser Enhanced Start: ");
> -		print_size(((u64)pconf.user_enh_start) << 9, "\n");
> +		print_size(((u64)pconf.user.enh_start) << 9, "\n");
> 		puts("\tUser Enhanced Size: ");
> -		print_size(((u64)pconf.user_enh_size) << 9, "\n");
> +		print_size(((u64)pconf.user.enh_size) << 9, "\n");
> 	} else {
> 		puts("\tNo enhanced user data area\n");
> 	}
> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
> index 311097f..7cd21f2 100644
> --- a/drivers/mmc/mmc.c
> +++ b/drivers/mmc/mmc.c
> @@ -615,6 +615,7 @@ int mmc_hwpart_config(struct mmc *mmc,
> 	u32 gp_size_mult[4];
> 	u32 max_enh_size_mult;
> 	u32 tot_enh_size_mult = 0;
> +	u8 wr_rel_set;
> 	int i, pidx, err;
> 	ALLOC_CACHE_ALIGN_BUFFER(u8, ext_csd, MMC_MAX_BLOCK_LEN);
> 
> @@ -637,19 +638,19 @@ int mmc_hwpart_config(struct mmc *mmc,
> 	}
> 
> 	/* check partition alignment and total enhanced size */
> -	if (conf->user_enh_size) {
> -		if (conf->user_enh_size % mmc->hc_wp_grp_size ||
> -		    conf->user_enh_start % mmc->hc_wp_grp_size) {
> +	if (conf->user.enh_size) {
> +		if (conf->user.enh_size % mmc->hc_wp_grp_size ||
> +		    conf->user.enh_start % mmc->hc_wp_grp_size) {
> 			printf("User data enhanced area not HC WP group "
> 			       "size aligned\n");
> 			return -EINVAL;
> 		}
> 		part_attrs |= EXT_CSD_ENH_USR;
> -		enh_size_mult = conf->user_enh_size / mmc->hc_wp_grp_size;
> +		enh_size_mult = conf->user.enh_size / mmc->hc_wp_grp_size;
> 		if (mmc->high_capacity) {
> -			enh_start_addr = conf->user_enh_start;
> +			enh_start_addr = conf->user.enh_start;
> 		} else {
> -			enh_start_addr = (conf->user_enh_start << 9);
> +			enh_start_addr = (conf->user.enh_start << 9);
> 		}
> 	} else {
> 		enh_size_mult = 0;
> @@ -689,6 +690,33 @@ int mmc_hwpart_config(struct mmc *mmc,
> 		return -EMEDIUMTYPE;
> 	}
> 
> +	/* The default value of EXT_CSD_WR_REL_SET is device
> +	 * dependent, the values can only be changed if the
> +	 * EXT_CSD_HS_CTRL_REL bit is set. The values can be
> +	 * changed only once and before partitioning is completed. */
> +	wr_rel_set = ext_csd[EXT_CSD_WR_REL_SET];
> +	if (conf->user.wr_rel_change) {
> +		if (conf->user.wr_rel_set)
> +			wr_rel_set |= EXT_CSD_WR_DATA_REL_USR;
> +		else
> +			wr_rel_set &= ~EXT_CSD_WR_DATA_REL_USR;
> +	}
> +	for (pidx = 0; pidx < 4; pidx++) {
> +		if (conf->gp_part[pidx].wr_rel_change) {
> +			if (conf->gp_part[pidx].wr_rel_set)
> +				wr_rel_set |= EXT_CSD_WR_DATA_REL_GP(pidx);
> +			else
> +				wr_rel_set &= ~EXT_CSD_WR_DATA_REL_GP(pidx);
> +		}
> +	}
> +
> +	if (wr_rel_set != ext_csd[EXT_CSD_WR_REL_SET] &&
> +	    !(ext_csd[EXT_CSD_WR_REL_PARAM] & EXT_CSD_HS_CTRL_REL)) {
> +		puts("Card does not support host controlled partition write "
> +		     "reliability settings\n");
> +		return -EMEDIUMTYPE;
> +	}
> +
> 	if (ext_csd[EXT_CSD_PART_SETTING_COMPLETED] & PART_SETTING_COMPLETED) {
> 		printf("Card already partitioned\n");
> 		return -EPERM;
> @@ -745,6 +773,17 @@ int mmc_hwpart_config(struct mmc *mmc,
> 	if (mode == MMC_HWPART_CONF_SET)
> 		return 0;
> 
> +	/* The WR_REL_SET is a write-once register but shall be
> +	 * written before setting PART_SETTING_COMPLETED. As it is
> +	 * write-once we can only write it when completing the
> +	 * partitioning. */
> +	if (wr_rel_set != ext_csd[EXT_CSD_WR_REL_SET]) {
> +		err = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL,
> +				 EXT_CSD_WR_REL_SET, wr_rel_set);
> +		if (err)
> +			return err;
> +	}
> +
> 	/* Setting PART_SETTING_COMPLETED confirms the partition
> 	 * configuration but it only becomes effective after power
> 	 * cycle, so we do not adjust the partition related settings
> diff --git a/include/mmc.h b/include/mmc.h
> index 7892880..33595f0 100644
> --- a/include/mmc.h
> +++ b/include/mmc.h
> @@ -154,6 +154,8 @@
> #define EXT_CSD_MAX_ENH_SIZE_MULT	157	/* R */
> #define EXT_CSD_PARTITIONING_SUPPORT	160	/* RO */
> #define EXT_CSD_RST_N_FUNCTION		162	/* R/W */
> +#define EXT_CSD_WR_REL_PARAM		166	/* R */
> +#define EXT_CSD_WR_REL_SET		167	/* R/W */
> #define EXT_CSD_RPMB_MULT		168	/* RO */
> #define EXT_CSD_ERASE_GROUP_DEF		175	/* R/W */
> #define EXT_CSD_BOOT_BUS_WIDTH		177
> @@ -204,6 +206,11 @@
> #define EXT_CSD_ENH_USR		(1 << 0)	/* user data area is enhanced */
> #define EXT_CSD_ENH_GP(x)	(1 << ((x)+1))	/* GP part (x+1) is enhanced */
> 
> +#define EXT_CSD_HS_CTRL_REL	(1 << 0)	/* host controlled WR_REL_SET */
> +
> +#define EXT_CSD_WR_DATA_REL_USR		(1 << 0)	/* user data area WR_REL */
> +#define EXT_CSD_WR_DATA_REL_GP(x)	(1 << ((x)+1))	/* GP part (x+1) WR_REL */
> +
> #define R1_ILLEGAL_COMMAND		(1 << 22)
> #define R1_APP_CMD			(1 << 5)
> 
> @@ -331,11 +338,17 @@ struct mmc {
> };
> 
> struct mmc_hwpart_conf {
> -	uint user_enh_start;	/* in 512-byte sectors */
> -	uint user_enh_size;	/* in 512-byte sectors, if 0 no enhanced user data */
> +	struct {
> +		uint enh_start;	/* in 512-byte sectors */
> +		uint enh_size;	/* in 512-byte sectors, if 0 no enhanced user data */
> +		unsigned wr_rel_change : 1;
> +		unsigned wr_rel_set : 1;

How about user_wr_rel_change : 1; & wr_rel_set : 1;

It will cut down on the diff size.

> +	} user;
> 	struct {
> 		uint size;	/* in 512-byte sectors */
> -		int enhanced;
> +		unsigned enhanced : 1;
> +		unsigned wr_rel_change : 1;
> +		unsigned wr_rel_set : 1;
> 	} gp_part[4];
> };
> 
> -- 
> 1.7.1
> 

Regards

? Pantelis

[U-Boot] [PATCH 13/18] mmc: the ext_csd data may be used during init even if reading failed

[Pantelis Antoniou]

> On Nov 28, 2014, at 11:10 , Diego Santa Cruz <Diego.SantaCruz@spinetix.com> wrote:
> 
> The mmc_startup() function uses the ext_csd data even if reading it
> from the mmc device failed. This bug was introduced in commit
> bc897b1d4d86597311430dbe7b3e6c807c8c53e5. We now bail out if
> reading it fails, this should not be a problem as ext_csd was
> introduced in MMC 4.0 and this code is conditional on MMC >= 4.0.
> ---
> drivers/mmc/mmc.c |    4 +++-
> 1 files changed, 3 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
> index f545576..8f6cbd5 100644
> --- a/drivers/mmc/mmc.c
> +++ b/drivers/mmc/mmc.c
> @@ -972,7 +972,9 @@ static int mmc_startup(struct mmc *mmc)
> 	if (!IS_SD(mmc) && (mmc->version >= MMC_VERSION_4)) {
> 		/* check  ext_csd version and capacity */
> 		err = mmc_send_ext_csd(mmc, ext_csd);
> -		if (!err && (ext_csd[EXT_CSD_REV] >= 2)) {
> +		if (err)
> +			return err;

Hum, good catch.

> +		if (ext_csd[EXT_CSD_REV] >= 2) {
> 			/*
> 			 * According to the JEDEC Standard, the value of
> 			 * ext_csd's capacity is valid if the value is more
> -- 
> 1.7.1
> 

[U-Boot] [PATCH 00/18] Support for eMMC partitioning and related fixes

[Diego Santa Cruz]

Hi Pantelis,

[resending as message was rejected due to base64 content]

> 
> Hi Diego,
> 
> This is quite a large patchset, so we'll have to take a close look.

Feedback is appreciated. The number of patches is relatively large, but I have tried to break them in small chunks so that they are easier to review, most of them are pretty simple.

I forgot to mention, but the patches are against yesterday's u-boot.git master.

Let me know if they need some rework.

Best,

Diego

-- 
Diego Santa Cruz, PhD
Technology Architect
spinetix.com

[U-Boot] [PATCH 16/18] mmc: extend the mmc hardware partitioning API with write reliability

[Diego Santa Cruz]

Hi Pantelis,

> Hi Diego,
> 
> At first glance it's OK, I'm not the turning of user_enh_start, _size to a
> structure
> has a lot of benefits. It makes the diff longer.
>

I did that so that all the user data area members can be easily reset to zero when parsing the mmc hwpartition sub-command arguments, in particular if any more members get added in the future, as it is easy to forget to add a reset to zero when adding a new member to the main structure. Keeping all of them in a sub-structure allows to just memset the sub-structure to zero and not worry about members that may get added in the future.

> > struct mmc_hwpart_conf {
> > -	uint user_enh_start;	/* in 512-byte sectors */
> > -	uint user_enh_size;	/* in 512-byte sectors, if 0 no enhanced user data
> */
> > +	struct {
> > +		uint enh_start;	/* in 512-byte sectors */
> > +		uint enh_size;	/* in 512-byte sectors, if 0 no enhanced user
> data */
> > +		unsigned wr_rel_change : 1;
> > +		unsigned wr_rel_set : 1;
> 
> How about user_wr_rel_change : 1; & wr_rel_set : 1;
> 
> It will cut down on the diff size.
>

I am not sure I fully understand your comment. You mean not using a struct and just put them in the main struct with the user_ prefix?

Best,

Diego

-- 
Diego Santa Cruz, PhD
Technology Architect
spinetix.com


> -----Original Message-----
> From: Pantelis Antoniou [mailto:panto at antoniou-consulting.com]
> Sent: Friday, November 28, 2014 11:09 AM
> To: Diego Santa Cruz
> Cc: u-boot at lists.denx.de
> Subject: Re: [PATCH 16/18] mmc: extend the mmc hardware partitioning API with
> write reliability
> 
> > On Nov 28, 2014, at 11:10 , Diego Santa Cruz <Diego.SantaCruz@spinetix.com>
> wrote:
> >
> > The eMMC partition write reliability settings are to be set while
> > partitioning a device, as per the eMMC spec, so changes to these
> > attributes needs to be done in the hardware partitioning API.
> > This commit adds such support.
> > ---
> > common/cmd_mmc.c  |   21 +++++++++++----------
> > drivers/mmc/mmc.c |   51 +++++++++++++++++++++++++++++++++++++++++++++------
> > include/mmc.h     |   19 ++++++++++++++++---
> > 3 files changed, 72 insertions(+), 19 deletions(-)
> >
> > diff --git a/common/cmd_mmc.c b/common/cmd_mmc.c
> > index 3943fb7..c0a4a3e 100644
> > --- a/common/cmd_mmc.c
> > +++ b/common/cmd_mmc.c
> > @@ -501,9 +501,10 @@ static int do_mmc_hwpartition(cmd_tbl_t *cmdtp, int
> flag,
> > 		if (!strcmp(argv[i], "userenh")) {
> > 			if (i + 2 >= argc)
> > 				return CMD_RET_USAGE;
> > -			pconf.user_enh_start =
> > +			memset(&pconf.user, 0, sizeof(pconf.user));
> > +			pconf.user.enh_start =
> > 				simple_strtoul(argv[i+1], NULL, 10);
> > -			pconf.user_enh_size =
> > +			pconf.user.enh_size =
> > 				simple_strtoul(argv[i+2], NULL, 10);
> > 			i += 3;
> > 		} else if (!strncmp(argv[i], "gp", 2) &&
> > @@ -512,14 +513,14 @@ static int do_mmc_hwpartition(cmd_tbl_t *cmdtp, int
> flag,
> > 			if (i + 1 >= argc)
> > 				return CMD_RET_USAGE;
> > 			pidx = argv[i][2] - '1';
> > +			memset(&pconf.gp_part[pidx], 0,
> > +			       sizeof(pconf.gp_part[pidx]));
> > 			pconf.gp_part[pidx].size =
> > 				simple_strtoul(argv[i+1], NULL, 10);
> > -			if (i + 2 < argc && !strcmp(argv[i+2], "enh")) {
> > +			i += 2;
> > +			if (i < argc && !strcmp(argv[i], "enh")) {
> > 				pconf.gp_part[pidx].enhanced = 1;
> > -				i += 3;
> > -			} else {
> > -				pconf.gp_part[pidx].enhanced = 0;
> > -				i += 2;
> > +				i++;
> > 			}
> > 		} else if (!strcmp(argv[i], "check")) {
> > 			mode = MMC_HWPART_CONF_CHECK;
> > @@ -536,11 +537,11 @@ static int do_mmc_hwpartition(cmd_tbl_t *cmdtp, int
> flag,
> > 	}
> >
> > 	puts("Partition configuration:\n");
> > -	if (pconf.user_enh_size) {
> > +	if (pconf.user.enh_size) {
> > 		puts("\tUser Enhanced Start: ");
> > -		print_size(((u64)pconf.user_enh_start) << 9, "\n");
> > +		print_size(((u64)pconf.user.enh_start) << 9, "\n");
> > 		puts("\tUser Enhanced Size: ");
> > -		print_size(((u64)pconf.user_enh_size) << 9, "\n");
> > +		print_size(((u64)pconf.user.enh_size) << 9, "\n");
> > 	} else {
> > 		puts("\tNo enhanced user data area\n");
> > 	}
> > diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
> > index 311097f..7cd21f2 100644
> > --- a/drivers/mmc/mmc.c
> > +++ b/drivers/mmc/mmc.c
> > @@ -615,6 +615,7 @@ int mmc_hwpart_config(struct mmc *mmc,
> > 	u32 gp_size_mult[4];
> > 	u32 max_enh_size_mult;
> > 	u32 tot_enh_size_mult = 0;
> > +	u8 wr_rel_set;
> > 	int i, pidx, err;
> > 	ALLOC_CACHE_ALIGN_BUFFER(u8, ext_csd, MMC_MAX_BLOCK_LEN);
> >
> > @@ -637,19 +638,19 @@ int mmc_hwpart_config(struct mmc *mmc,
> > 	}
> >
> > 	/* check partition alignment and total enhanced size */
> > -	if (conf->user_enh_size) {
> > -		if (conf->user_enh_size % mmc->hc_wp_grp_size ||
> > -		    conf->user_enh_start % mmc->hc_wp_grp_size) {
> > +	if (conf->user.enh_size) {
> > +		if (conf->user.enh_size % mmc->hc_wp_grp_size ||
> > +		    conf->user.enh_start % mmc->hc_wp_grp_size) {
> > 			printf("User data enhanced area not HC WP group "
> > 			       "size aligned\n");
> > 			return -EINVAL;
> > 		}
> > 		part_attrs |= EXT_CSD_ENH_USR;
> > -		enh_size_mult = conf->user_enh_size / mmc->hc_wp_grp_size;
> > +		enh_size_mult = conf->user.enh_size / mmc->hc_wp_grp_size;
> > 		if (mmc->high_capacity) {
> > -			enh_start_addr = conf->user_enh_start;
> > +			enh_start_addr = conf->user.enh_start;
> > 		} else {
> > -			enh_start_addr = (conf->user_enh_start << 9);
> > +			enh_start_addr = (conf->user.enh_start << 9);
> > 		}
> > 	} else {
> > 		enh_size_mult = 0;
> > @@ -689,6 +690,33 @@ int mmc_hwpart_config(struct mmc *mmc,
> > 		return -EMEDIUMTYPE;
> > 	}
> >
> > +	/* The default value of EXT_CSD_WR_REL_SET is device
> > +	 * dependent, the values can only be changed if the
> > +	 * EXT_CSD_HS_CTRL_REL bit is set. The values can be
> > +	 * changed only once and before partitioning is completed. */
> > +	wr_rel_set = ext_csd[EXT_CSD_WR_REL_SET];
> > +	if (conf->user.wr_rel_change) {
> > +		if (conf->user.wr_rel_set)
> > +			wr_rel_set |= EXT_CSD_WR_DATA_REL_USR;
> > +		else
> > +			wr_rel_set &= ~EXT_CSD_WR_DATA_REL_USR;
> > +	}
> > +	for (pidx = 0; pidx < 4; pidx++) {
> > +		if (conf->gp_part[pidx].wr_rel_change) {
> > +			if (conf->gp_part[pidx].wr_rel_set)
> > +				wr_rel_set |= EXT_CSD_WR_DATA_REL_GP(pidx);
> > +			else
> > +				wr_rel_set &= ~EXT_CSD_WR_DATA_REL_GP(pidx);
> > +		}
> > +	}
> > +
> > +	if (wr_rel_set != ext_csd[EXT_CSD_WR_REL_SET] &&
> > +	    !(ext_csd[EXT_CSD_WR_REL_PARAM] & EXT_CSD_HS_CTRL_REL)) {
> > +		puts("Card does not support host controlled partition write "
> > +		     "reliability settings\n");
> > +		return -EMEDIUMTYPE;
> > +	}
> > +
> > 	if (ext_csd[EXT_CSD_PART_SETTING_COMPLETED] & PART_SETTING_COMPLETED) {
> > 		printf("Card already partitioned\n");
> > 		return -EPERM;
> > @@ -745,6 +773,17 @@ int mmc_hwpart_config(struct mmc *mmc,
> > 	if (mode == MMC_HWPART_CONF_SET)
> > 		return 0;
> >
> > +	/* The WR_REL_SET is a write-once register but shall be
> > +	 * written before setting PART_SETTING_COMPLETED. As it is
> > +	 * write-once we can only write it when completing the
> > +	 * partitioning. */
> > +	if (wr_rel_set != ext_csd[EXT_CSD_WR_REL_SET]) {
> > +		err = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL,
> > +				 EXT_CSD_WR_REL_SET, wr_rel_set);
> > +		if (err)
> > +			return err;
> > +	}
> > +
> > 	/* Setting PART_SETTING_COMPLETED confirms the partition
> > 	 * configuration but it only becomes effective after power
> > 	 * cycle, so we do not adjust the partition related settings
> > diff --git a/include/mmc.h b/include/mmc.h
> > index 7892880..33595f0 100644
> > --- a/include/mmc.h
> > +++ b/include/mmc.h
> > @@ -154,6 +154,8 @@
> > #define EXT_CSD_MAX_ENH_SIZE_MULT	157	/* R */
> > #define EXT_CSD_PARTITIONING_SUPPORT	160	/* RO */
> > #define EXT_CSD_RST_N_FUNCTION		162	/* R/W */
> > +#define EXT_CSD_WR_REL_PARAM		166	/* R */
> > +#define EXT_CSD_WR_REL_SET		167	/* R/W */
> > #define EXT_CSD_RPMB_MULT		168	/* RO */
> > #define EXT_CSD_ERASE_GROUP_DEF		175	/* R/W */
> > #define EXT_CSD_BOOT_BUS_WIDTH		177
> > @@ -204,6 +206,11 @@
> > #define EXT_CSD_ENH_USR		(1 << 0)	/* user data area is enhanced */
> > #define EXT_CSD_ENH_GP(x)	(1 << ((x)+1))	/* GP part (x+1) is
> enhanced */
> >
> > +#define EXT_CSD_HS_CTRL_REL	(1 << 0)	/* host controlled WR_REL_SET */
> > +
> > +#define EXT_CSD_WR_DATA_REL_USR		(1 << 0)	/* user data area
> WR_REL */
> > +#define EXT_CSD_WR_DATA_REL_GP(x)	(1 << ((x)+1))	/* GP part (x+1)
> WR_REL */
> > +
> > #define R1_ILLEGAL_COMMAND		(1 << 22)
> > #define R1_APP_CMD			(1 << 5)
> >
> > @@ -331,11 +338,17 @@ struct mmc {
> > };
> >
> > +	} user;
> > 	struct {
> > 		uint size;	/* in 512-byte sectors */
> > -		int enhanced;
> > +		unsigned enhanced : 1;
> > +		unsigned wr_rel_change : 1;
> > +		unsigned wr_rel_set : 1;
> > 	} gp_part[4];
> > };
> >
> > --
> > 1.7.1
> >
> 
> Regards
> 
> - Pantelis

[U-Boot] [PATCH 00/18] Support for eMMC partitioning and related fixes

[Diego Santa Cruz]

Hi,

> -----Original Message-----
> From: Joakim Tjernlund [mailto:joakim.tjernlund at transmode.se]
> Sent: Friday, November 28, 2014 11:05 AM
> To: Diego Santa Cruz
> Cc: panto at antoniou-consulting.com; u-boot at lists.denx.de
> Subject: Re: [U-Boot] [PATCH 00/18] Support for eMMC partitioning and related
> fixes
> >
> > I have the need to hardware partition eMMC devices from U-Boot along
> > with setting enhanced and reliable write attributes.
> >
> > This series of patches adds this support to U-Boot via a new mmc
> > API, a few new members of struct mmc and a new mmc sub-command. It
> > also features several fixes to the eMMC hardware partition support. I
> > have tested this with Micron eMMC 4.41 parts and it is working as
> > expected.
> 
> This is really great, thanks.

Good to know it may be useful to other people.

> 
> I do wonder(I am fairly new to eMMC) about 512B emulation.
> In mmc utils there is a:
> mmc disable 512B emulation <device>
>         Set the eMMC data sector size to 4KB by disabling emulation on
> <device>.
> 
> I am hoping 4K size will increase performance and reliability?
> Could you add this feature too to your patch set?
>

I think this was introduced in eMMC 4.51. I do not have any 4.51 devices at hand to test with but I am not sure there would be any performance benefit to issue reads and writes in the 4KB large sector size. All eMMC devices I've seen write in chunks much larger than 4 KB internally anyhow (the ones I'm using now have a superpage size of 32 KB). Writes should be aligned to the superpage size to get good performance.

Best,

Diego

-- 
Diego Santa Cruz, PhD
Technology Architect
spinetix.com

[U-Boot] [PATCH 16/18] mmc: extend the mmc hardware partitioning API with write reliability

[Pantelis Antoniou]

Hi Diego,

> On Nov 28, 2014, at 12:57 , Diego Santa Cruz <Diego.SantaCruz@spinetix.com> wrote:
> 
> Hi Pantelis,
> 
>> Hi Diego,
>> 
>> At first glance it's OK, I'm not the turning of user_enh_start, _size to a
>> structure
>> has a lot of benefits. It makes the diff longer.
>> 
> 
> I did that so that all the user data area members can be easily reset to zero when parsing the mmc hwpartition sub-command arguments, in particular if any more members get added in the future, as it is easy to forget to add a reset to zero when adding a new member to the main structure. Keeping all of them in a sub-structure allows to just memset the sub-structure to zero and not worry about members that may get added in the future.
> 
>>> struct mmc_hwpart_conf {
>>> -	uint user_enh_start;	/* in 512-byte sectors */
>>> -	uint user_enh_size;	/* in 512-byte sectors, if 0 no enhanced user data
>> */
>>> +	struct {
>>> +		uint enh_start;	/* in 512-byte sectors */
>>> +		uint enh_size;	/* in 512-byte sectors, if 0 no enhanced user
>> data */
>>> +		unsigned wr_rel_change : 1;
>>> +		unsigned wr_rel_set : 1;
>> 
>> How about user_wr_rel_change : 1; & wr_rel_set : 1;
>> 
>> It will cut down on the diff size.
>> 
> 
> I am not sure I fully understand your comment. You mean not using a struct and just put them in the main struct with the user_ prefix?
> 

I think the source of the confusion is that you?ve introduced user_enh_start, user_enh_size in one patch and then modify them
again in another. That causes diff changes to show up due to the introduction of the structure. How about declaring the structure
in the first patch and the new members in another? 

> Best,
> 
> Diego
> 

Regards

? Pantelis

> -- 
> Diego Santa Cruz, PhD
> Technology Architect
> spinetix.com
> 
> 
>> -----Original Message-----
>> From: Pantelis Antoniou [mailto:panto at antoniou-consulting.com]
>> Sent: Friday, November 28, 2014 11:09 AM
>> To: Diego Santa Cruz
>> Cc: u-boot at lists.denx.de
>> Subject: Re: [PATCH 16/18] mmc: extend the mmc hardware partitioning API with
>> write reliability
>> 
>>> On Nov 28, 2014, at 11:10 , Diego Santa Cruz <Diego.SantaCruz@spinetix.com>
>> wrote:
>>> 
>>> The eMMC partition write reliability settings are to be set while
>>> partitioning a device, as per the eMMC spec, so changes to these
>>> attributes needs to be done in the hardware partitioning API.
>>> This commit adds such support.
>>> ---
>>> common/cmd_mmc.c  |   21 +++++++++++----------
>>> drivers/mmc/mmc.c |   51 +++++++++++++++++++++++++++++++++++++++++++++------
>>> include/mmc.h     |   19 ++++++++++++++++---
>>> 3 files changed, 72 insertions(+), 19 deletions(-)
>>> 
>>> diff --git a/common/cmd_mmc.c b/common/cmd_mmc.c
>>> index 3943fb7..c0a4a3e 100644
>>> --- a/common/cmd_mmc.c
>>> +++ b/common/cmd_mmc.c
>>> @@ -501,9 +501,10 @@ static int do_mmc_hwpartition(cmd_tbl_t *cmdtp, int
>> flag,
>>> 		if (!strcmp(argv[i], "userenh")) {
>>> 			if (i + 2 >= argc)
>>> 				return CMD_RET_USAGE;
>>> -			pconf.user_enh_start =
>>> +			memset(&pconf.user, 0, sizeof(pconf.user));
>>> +			pconf.user.enh_start =
>>> 				simple_strtoul(argv[i+1], NULL, 10);
>>> -			pconf.user_enh_size =
>>> +			pconf.user.enh_size =
>>> 				simple_strtoul(argv[i+2], NULL, 10);
>>> 			i += 3;
>>> 		} else if (!strncmp(argv[i], "gp", 2) &&
>>> @@ -512,14 +513,14 @@ static int do_mmc_hwpartition(cmd_tbl_t *cmdtp, int
>> flag,
>>> 			if (i + 1 >= argc)
>>> 				return CMD_RET_USAGE;
>>> 			pidx = argv[i][2] - '1';
>>> +			memset(&pconf.gp_part[pidx], 0,
>>> +			       sizeof(pconf.gp_part[pidx]));
>>> 			pconf.gp_part[pidx].size =
>>> 				simple_strtoul(argv[i+1], NULL, 10);
>>> -			if (i + 2 < argc && !strcmp(argv[i+2], "enh")) {
>>> +			i += 2;
>>> +			if (i < argc && !strcmp(argv[i], "enh")) {
>>> 				pconf.gp_part[pidx].enhanced = 1;
>>> -				i += 3;
>>> -			} else {
>>> -				pconf.gp_part[pidx].enhanced = 0;
>>> -				i += 2;
>>> +				i++;
>>> 			}
>>> 		} else if (!strcmp(argv[i], "check")) {
>>> 			mode = MMC_HWPART_CONF_CHECK;
>>> @@ -536,11 +537,11 @@ static int do_mmc_hwpartition(cmd_tbl_t *cmdtp, int
>> flag,
>>> 	}
>>> 
>>> 	puts("Partition configuration:\n");
>>> -	if (pconf.user_enh_size) {
>>> +	if (pconf.user.enh_size) {
>>> 		puts("\tUser Enhanced Start: ");
>>> -		print_size(((u64)pconf.user_enh_start) << 9, "\n");
>>> +		print_size(((u64)pconf.user.enh_start) << 9, "\n");
>>> 		puts("\tUser Enhanced Size: ");
>>> -		print_size(((u64)pconf.user_enh_size) << 9, "\n");
>>> +		print_size(((u64)pconf.user.enh_size) << 9, "\n");
>>> 	} else {
>>> 		puts("\tNo enhanced user data area\n");
>>> 	}
>>> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
>>> index 311097f..7cd21f2 100644
>>> --- a/drivers/mmc/mmc.c
>>> +++ b/drivers/mmc/mmc.c
>>> @@ -615,6 +615,7 @@ int mmc_hwpart_config(struct mmc *mmc,
>>> 	u32 gp_size_mult[4];
>>> 	u32 max_enh_size_mult;
>>> 	u32 tot_enh_size_mult = 0;
>>> +	u8 wr_rel_set;
>>> 	int i, pidx, err;
>>> 	ALLOC_CACHE_ALIGN_BUFFER(u8, ext_csd, MMC_MAX_BLOCK_LEN);
>>> 
>>> @@ -637,19 +638,19 @@ int mmc_hwpart_config(struct mmc *mmc,
>>> 	}
>>> 
>>> 	/* check partition alignment and total enhanced size */
>>> -	if (conf->user_enh_size) {
>>> -		if (conf->user_enh_size % mmc->hc_wp_grp_size ||
>>> -		    conf->user_enh_start % mmc->hc_wp_grp_size) {
>>> +	if (conf->user.enh_size) {
>>> +		if (conf->user.enh_size % mmc->hc_wp_grp_size ||
>>> +		    conf->user.enh_start % mmc->hc_wp_grp_size) {
>>> 			printf("User data enhanced area not HC WP group "
>>> 			       "size aligned\n");
>>> 			return -EINVAL;
>>> 		}
>>> 		part_attrs |= EXT_CSD_ENH_USR;
>>> -		enh_size_mult = conf->user_enh_size / mmc->hc_wp_grp_size;
>>> +		enh_size_mult = conf->user.enh_size / mmc->hc_wp_grp_size;
>>> 		if (mmc->high_capacity) {
>>> -			enh_start_addr = conf->user_enh_start;
>>> +			enh_start_addr = conf->user.enh_start;
>>> 		} else {
>>> -			enh_start_addr = (conf->user_enh_start << 9);
>>> +			enh_start_addr = (conf->user.enh_start << 9);
>>> 		}
>>> 	} else {
>>> 		enh_size_mult = 0;
>>> @@ -689,6 +690,33 @@ int mmc_hwpart_config(struct mmc *mmc,
>>> 		return -EMEDIUMTYPE;
>>> 	}
>>> 
>>> +	/* The default value of EXT_CSD_WR_REL_SET is device
>>> +	 * dependent, the values can only be changed if the
>>> +	 * EXT_CSD_HS_CTRL_REL bit is set. The values can be
>>> +	 * changed only once and before partitioning is completed. */
>>> +	wr_rel_set = ext_csd[EXT_CSD_WR_REL_SET];
>>> +	if (conf->user.wr_rel_change) {
>>> +		if (conf->user.wr_rel_set)
>>> +			wr_rel_set |= EXT_CSD_WR_DATA_REL_USR;
>>> +		else
>>> +			wr_rel_set &= ~EXT_CSD_WR_DATA_REL_USR;
>>> +	}
>>> +	for (pidx = 0; pidx < 4; pidx++) {
>>> +		if (conf->gp_part[pidx].wr_rel_change) {
>>> +			if (conf->gp_part[pidx].wr_rel_set)
>>> +				wr_rel_set |= EXT_CSD_WR_DATA_REL_GP(pidx);
>>> +			else
>>> +				wr_rel_set &= ~EXT_CSD_WR_DATA_REL_GP(pidx);
>>> +		}
>>> +	}
>>> +
>>> +	if (wr_rel_set != ext_csd[EXT_CSD_WR_REL_SET] &&
>>> +	    !(ext_csd[EXT_CSD_WR_REL_PARAM] & EXT_CSD_HS_CTRL_REL)) {
>>> +		puts("Card does not support host controlled partition write "
>>> +		     "reliability settings\n");
>>> +		return -EMEDIUMTYPE;
>>> +	}
>>> +
>>> 	if (ext_csd[EXT_CSD_PART_SETTING_COMPLETED] & PART_SETTING_COMPLETED) {
>>> 		printf("Card already partitioned\n");
>>> 		return -EPERM;
>>> @@ -745,6 +773,17 @@ int mmc_hwpart_config(struct mmc *mmc,
>>> 	if (mode == MMC_HWPART_CONF_SET)
>>> 		return 0;
>>> 
>>> +	/* The WR_REL_SET is a write-once register but shall be
>>> +	 * written before setting PART_SETTING_COMPLETED. As it is
>>> +	 * write-once we can only write it when completing the
>>> +	 * partitioning. */
>>> +	if (wr_rel_set != ext_csd[EXT_CSD_WR_REL_SET]) {
>>> +		err = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL,
>>> +				 EXT_CSD_WR_REL_SET, wr_rel_set);
>>> +		if (err)
>>> +			return err;
>>> +	}
>>> +
>>> 	/* Setting PART_SETTING_COMPLETED confirms the partition
>>> 	 * configuration but it only becomes effective after power
>>> 	 * cycle, so we do not adjust the partition related settings
>>> diff --git a/include/mmc.h b/include/mmc.h
>>> index 7892880..33595f0 100644
>>> --- a/include/mmc.h
>>> +++ b/include/mmc.h
>>> @@ -154,6 +154,8 @@
>>> #define EXT_CSD_MAX_ENH_SIZE_MULT	157	/* R */
>>> #define EXT_CSD_PARTITIONING_SUPPORT	160	/* RO */
>>> #define EXT_CSD_RST_N_FUNCTION		162	/* R/W */
>>> +#define EXT_CSD_WR_REL_PARAM		166	/* R */
>>> +#define EXT_CSD_WR_REL_SET		167	/* R/W */
>>> #define EXT_CSD_RPMB_MULT		168	/* RO */
>>> #define EXT_CSD_ERASE_GROUP_DEF		175	/* R/W */
>>> #define EXT_CSD_BOOT_BUS_WIDTH		177
>>> @@ -204,6 +206,11 @@
>>> #define EXT_CSD_ENH_USR		(1 << 0)	/* user data area is enhanced */
>>> #define EXT_CSD_ENH_GP(x)	(1 << ((x)+1))	/* GP part (x+1) is
>> enhanced */
>>> 
>>> +#define EXT_CSD_HS_CTRL_REL	(1 << 0)	/* host controlled WR_REL_SET */
>>> +
>>> +#define EXT_CSD_WR_DATA_REL_USR		(1 << 0)	/* user data area
>> WR_REL */
>>> +#define EXT_CSD_WR_DATA_REL_GP(x)	(1 << ((x)+1))	/* GP part (x+1)
>> WR_REL */
>>> +
>>> #define R1_ILLEGAL_COMMAND		(1 << 22)
>>> #define R1_APP_CMD			(1 << 5)
>>> 
>>> @@ -331,11 +338,17 @@ struct mmc {
>>> };
>>> 
>>> +	} user;
>>> 	struct {
>>> 		uint size;	/* in 512-byte sectors */
>>> -		int enhanced;
>>> +		unsigned enhanced : 1;
>>> +		unsigned wr_rel_change : 1;
>>> +		unsigned wr_rel_set : 1;
>>> 	} gp_part[4];
>>> };
>>> 
>>> --
>>> 1.7.1
>>> 
>> 
>> Regards
>> 
>> - Pantelis
> 

[U-Boot] [PATCH 16/18] mmc: extend the mmc hardware partitioning API with write reliability

[Diego Santa Cruz]

Hi Pantelis,

> I think the source of the confusion is that you've introduced user_enh_start,
> user_enh_size in one patch and then modify them
> again in another. That causes diff changes to show up due to the introduction
> of the structure. How about declaring the structure
> in the first patch and the new members in another?
>

Indeed, I realized the shortcoming of the first approach down the road.

I can modify the patches as you propose. As I'm fairly new to git, I would appreciate if you can point me out how to efficiently modify an intermediate commit in my git repo so that I can generate a new patch series.

Thanks,

Diego

-- 
Diego Santa Cruz, PhD
Technology Architect
spinetix.com

[U-Boot] [PATCH 16/18] mmc: extend the mmc hardware partitioning API with write reliability

[Pantelis Antoniou]

Hi Diego,

> On Nov 28, 2014, at 16:27 , Diego Santa Cruz <Diego.SantaCruz@spinetix.com> wrote:
> 
> Hi Pantelis,
> 
>> I think the source of the confusion is that you've introduced user_enh_start,
>> user_enh_size in one patch and then modify them
>> again in another. That causes diff changes to show up due to the introduction
>> of the structure. How about declaring the structure
>> in the first patch and the new members in another?
>> 
> 
> Indeed, I realized the shortcoming of the first approach down the road.
> 
> I can modify the patches as you propose. As I'm fairly new to git, I would appreciate if you can point me out how to efficiently modify an intermediate commit in my git repo so that I can generate a new patch series.
> 

$ git rebase -i
<pick up the commit you want to edit and change pick to edit>
$ <make changes>
$ git add <changed-files>
$ git rebase ?continue

You can take a look at something like this:

https://www.atlassian.com/git/tutorials/rewriting-history/git-rebase

> Thanks,
> 
> Diego
> 

Regards

? Pantelis

> -- 
> Diego Santa Cruz, PhD
> Technology Architect
> spinetix.com

[U-Boot] [PATCH 16/18] mmc: extend the mmc hardware partitioning API with write reliability

[Diego Santa Cruz]

Hi Pantelis,

> 
> $ git rebase -i
> <pick up the commit you want to edit and change pick to edit>
> $ <make changes>
> $ git add <changed-files>
> $ git rebase -continue
> 
> You can take a look at something like this:
> 
> https://www.atlassian.com/git/tutorials/rewriting-history/git-rebase
>

Thanks for the instructions, I managed to clear up this confusion. I begin to grasp why git has become so popular :-)

I modified patch 14 (add API to do eMMC hardware partitioning) as suggested and modified 15 and 16 to move the bits that do not belong in 16 to either 14 or 15 as appropriate.

Any other changes you think I should make before I resend the patch series?

I guess I should resend the whole series to the mailing list for consistency.

Best,

Diego

-- 
Diego Santa Cruz, PhD
Technology Architect
spinetix.com

[U-Boot] [PATCH 00/18] Support for eMMC partitioning and related fixes

[Joakim Tjernlund]

Diego Santa Cruz <Diego.SantaCruz@spinetix.com> wrote on 2014/11/28 
12:12:56:
> 
> Hi,
> 
> > -----Original Message-----
> > From: Joakim Tjernlund [mailto:joakim.tjernlund at transmode.se]
> > Sent: Friday, November 28, 2014 11:05 AM
> > To: Diego Santa Cruz
> > Cc: panto at antoniou-consulting.com; u-boot at lists.denx.de
> > Subject: Re: [U-Boot] [PATCH 00/18] Support for eMMC partitioning and 
related
> > fixes
> > >
> > > I have the need to hardware partition eMMC devices from U-Boot along
> > > with setting enhanced and reliable write attributes.
> > >
> > > This series of patches adds this support to U-Boot via a new mmc
> > > API, a few new members of struct mmc and a new mmc sub-command. It
> > > also features several fixes to the eMMC hardware partition support. 
I
> > > have tested this with Micron eMMC 4.41 parts and it is working as
> > > expected.
> > 
> > This is really great, thanks.
> 
> Good to know it may be useful to other people.
> 
> > 
> > I do wonder(I am fairly new to eMMC) about 512B emulation.
> > In mmc utils there is a:
> > mmc disable 512B emulation <device>
> >         Set the eMMC data sector size to 4KB by disabling emulation on
> > <device>.
> > 
> > I am hoping 4K size will increase performance and reliability?
> > Could you add this feature too to your patch set?
> >
> 
> I think this was introduced in eMMC 4.51. I do not have any 4.51 devices 
at hand to test with but I am not sure there would be any performance 
benefit to issue reads and writes in the 4KB large sector size. All eMMC 
devices I've seen write in chunks much larger than 4 KB internally anyhow 
(the ones I'm using now have a superpage size of 32 KB). Writes should be 
aligned to the superpage size to get good performance.

Sorry for the delay, I guess this option is there for a reason and 
performance/reliability
is the only thing that comes to mind.
I guess Linux is happier with 4K sector sizes? 

 Jocke

[U-Boot] [PATCH 00/18] Support for eMMC partitioning and related fixes

[Joakim Tjernlund]

Diego Santa Cruz <Diego.SantaCruz@spinetix.com> wrote on 2014/11/28 
12:12:56:
> 
> Hi,
> 
> > -----Original Message-----
> > From: Joakim Tjernlund [mailto:joakim.tjernlund at transmode.se]
> > Sent: Friday, November 28, 2014 11:05 AM
> > To: Diego Santa Cruz
> > Cc: panto at antoniou-consulting.com; u-boot at lists.denx.de
> > Subject: Re: [U-Boot] [PATCH 00/18] Support for eMMC partitioning and 
related
> > fixes
> > >
> > > I have the need to hardware partition eMMC devices from U-Boot along
> > > with setting enhanced and reliable write attributes.
> > >
> > > This series of patches adds this support to U-Boot via a new mmc
> > > API, a few new members of struct mmc and a new mmc sub-command. It
> > > also features several fixes to the eMMC hardware partition support. 
I
> > > have tested this with Micron eMMC 4.41 parts and it is working as
> > > expected.
> > 
> > This is really great, thanks.
> 
> Good to know it may be useful to other people.
> 
> > 
> > I do wonder(I am fairly new to eMMC) about 512B emulation.
> > In mmc utils there is a:
> > mmc disable 512B emulation <device>
> >         Set the eMMC data sector size to 4KB by disabling emulation on
> > <device>.
> > 
> > I am hoping 4K size will increase performance and reliability?
> > Could you add this feature too to your patch set?
> >
> 
> I think this was introduced in eMMC 4.51. I do not have any 4.51 devices 
at hand to test with but I am not sure there would be any performance 
benefit to issue reads and writes in the 4KB large sector size. All eMMC 
devices I've seen write in chunks much larger than 4 KB internally anyhow 
(the ones I'm using now have a superpage size of 32 KB). Writes should be 
aligned to the superpage size to get good performance.
> 

It is in eMMC 4.5 and I asked our eMMC supplier about 4KB, not only does 
4KB increase performance somewhat,
it also has a significant impact on device life.

 Jocke

[U-Boot] [PATCH 00/18] Support for eMMC partitioning and related fixes

[Diego Santa Cruz]

> -----Original Message-----
> From: Joakim Tjernlund [mailto:joakim.tjernlund at transmode.se]
> Sent: Thursday, December 04, 2014 7:29 PM
> To: Diego Santa Cruz
> Cc: u-boot at lists.denx.de
> Subject: RE: [U-Boot] [PATCH 00/18] Support for eMMC partitioning and related
> fixes
> 
> Diego Santa Cruz <Diego.SantaCruz@spinetix.com> wrote on 2014/11/28
> 12:12:56:
> >
> > Hi,
> >
> > > -----Original Message-----
> > > From: Joakim Tjernlund [mailto:joakim.tjernlund at transmode.se]
> > > Sent: Friday, November 28, 2014 11:05 AM
> > > To: Diego Santa Cruz
> > > Cc: panto at antoniou-consulting.com; u-boot at lists.denx.de
> > > Subject: Re: [U-Boot] [PATCH 00/18] Support for eMMC partitioning and
> related
> > > fixes
> > > >
> > > > I have the need to hardware partition eMMC devices from U-Boot along
> > > > with setting enhanced and reliable write attributes.
> > > >
> > > > This series of patches adds this support to U-Boot via a new mmc
> > > > API, a few new members of struct mmc and a new mmc sub-command. It
> > > > also features several fixes to the eMMC hardware partition support.
> I
> > > > have tested this with Micron eMMC 4.41 parts and it is working as
> > > > expected.
> > >
> > > This is really great, thanks.
> >
> > Good to know it may be useful to other people.
> >
> > >
> > > I do wonder(I am fairly new to eMMC) about 512B emulation.
> > > In mmc utils there is a:
> > > mmc disable 512B emulation <device>
> > >         Set the eMMC data sector size to 4KB by disabling emulation on
> > > <device>.
> > >
> > > I am hoping 4K size will increase performance and reliability?
> > > Could you add this feature too to your patch set?
> > >
> >
> > I think this was introduced in eMMC 4.51. I do not have any 4.51 devices
> at hand to test with but I am not sure there would be any performance
> benefit to issue reads and writes in the 4KB large sector size. All eMMC
> devices I've seen write in chunks much larger than 4 KB internally anyhow
> (the ones I'm using now have a superpage size of 32 KB). Writes should be
> aligned to the superpage size to get good performance.
> >
> 
> It is in eMMC 4.5 and I asked our eMMC supplier about 4KB, not only does
> 4KB increase performance somewhat,
> it also has a significant impact on device life.
> 

Write size should always be in a multiple of the superpage size to maximize performance and life, and the superpage size will be a multiple of 4 KB for any reasonable eMMC device. Currently, as far as I can tell, the mmc code does the writes in multiple blocks, by chunks of up to CONFIG_SYS_MMC_MAX_BLK_COUNT (65535 by default) 512-byte blocks, so data is written in sufficiently big chunks as long as the mmc write command is issued properly aligned and with a proper length. The eMMC device does not have to have committed the data until the end of the multiple block write command, so it should buffer up the data to sufficiently large buffers before writing them to NAND, lifting most of the performance penalty of the 512-byte sectors and, I believe, lifting all the life penalty. For a reliable write the situation may be a bit different.

NAND chips nowadays use pages which are usually 8 KB and NAND pages must be written in their entirety, so even writing in chunks of 4 KB will incur a penalty if the write command is stopped every 4 KB. Furthermore, larger capacity devices use several NAND chips in parallel which makes the effective page size be the NAND chip size multiplied by the number of NAND chips, this is what the superpage size is.

My guess is that 4 KB page devices may arrange the ECC differently when in native mode and may also arrange their internal block mapping metadata differently as well, leading to a performance improvement.

This being said, I realized that switching to the native sector size is a one-time operation that needs to be done before hardware partitioning, so yes it would be good to add it to U-Boot. But, as I said, I do not have any 4.5 / 4.51 devices at hand to develop this feature with.

While writing this I also realized that the current choice of CONFIG_SYS_MMC_MAX_BLK_COUNT being 65535 is not good, as writing more than 65535 512-byte blocks will result in issuing non-aligned writes on the second and subsequent chunks when writing 32 MiB or more in a single command. A simple fix for this would be to set the default value for CONFIG_SYS_MMC_MAX_BLK_COUNT to 65280 (i.e. aligned to 256 KiB) so that it is aligned to any reasonable superpage size. A more elaborate fix would be for b_max to be rounded down to a multiple of the superpage size read from EXT_CSD in mmc_startup().

-- 
Diego Santa Cruz, PhD
Technology Architect
spinetix.com

[U-Boot] [PATCH 00/18] Support for eMMC partitioning and related fixes

[Joakim Tjernlund]

Diego Santa Cruz <Diego.SantaCruz@spinetix.com> wrote on 2014/12/05 
09:38:34:
> 
> > -----Original Message-----
> > From: Joakim Tjernlund [mailto:joakim.tjernlund at transmode.se]
> > Sent: Thursday, December 04, 2014 7:29 PM
> > To: Diego Santa Cruz
> > Cc: u-boot at lists.denx.de
> > Subject: RE: [U-Boot] [PATCH 00/18] Support for eMMC partitioning and 
related
> > fixes
> > 
> > Diego Santa Cruz <Diego.SantaCruz@spinetix.com> wrote on 2014/11/28
> > 12:12:56:
> > >
> > > Hi,
> > >
> > > > -----Original Message-----
> > > > From: Joakim Tjernlund [mailto:joakim.tjernlund at transmode.se]
> > > > Sent: Friday, November 28, 2014 11:05 AM
> > > > To: Diego Santa Cruz
> > > > Cc: panto at antoniou-consulting.com; u-boot at lists.denx.de
> > > > Subject: Re: [U-Boot] [PATCH 00/18] Support for eMMC partitioning 
and
> > related
> > > > fixes
> > > > >
> > > > > I have the need to hardware partition eMMC devices from U-Boot 
along
> > > > > with setting enhanced and reliable write attributes.
> > > > >
> > > > > This series of patches adds this support to U-Boot via a new mmc
> > > > > API, a few new members of struct mmc and a new mmc sub-command. 
It
> > > > > also features several fixes to the eMMC hardware partition 
support.
> > I
> > > > > have tested this with Micron eMMC 4.41 parts and it is working 
as
> > > > > expected.
> > > >
> > > > This is really great, thanks.
> > >
> > > Good to know it may be useful to other people.
> > >
> > > >
> > > > I do wonder(I am fairly new to eMMC) about 512B emulation.
> > > > In mmc utils there is a:
> > > > mmc disable 512B emulation <device>
> > > >         Set the eMMC data sector size to 4KB by disabling 
emulation on
> > > > <device>.
> > > >
> > > > I am hoping 4K size will increase performance and reliability?
> > > > Could you add this feature too to your patch set?
> > > >
> > >
> > > I think this was introduced in eMMC 4.51. I do not have any 4.51 
devices
> > at hand to test with but I am not sure there would be any performance
> > benefit to issue reads and writes in the 4KB large sector size. All 
eMMC
> > devices I've seen write in chunks much larger than 4 KB internally 
anyhow
> > (the ones I'm using now have a superpage size of 32 KB). Writes should 
be
> > aligned to the superpage size to get good performance.
> > >
> > 
> > It is in eMMC 4.5 and I asked our eMMC supplier about 4KB, not only 
does
> > 4KB increase performance somewhat,
> > it also has a significant impact on device life.
> > 
> 
> Write size should always be in a multiple of the superpage size to 
maximize performance and life, and the superpage size will be a multiple 
of 4 KB for any reasonable eMMC device. Currently, as far as I can tell, 
the mmc code does the writes in multiple blocks, by chunks of up to 
CONFIG_SYS_MMC_MAX_BLK_COUNT (65535 by default) 512-byte blocks, so data 
is written in sufficiently big chunks as long as the mmc write command is 
issued properly aligned and with a proper length. The eMMC device does not 
have to have committed the data until the end of the multiple block write 
command, so it should buffer up the data to sufficiently large buffers 
before writing them to NAND, lifting most of the performance penalty of 
the 512-byte sectors and, I believe, lifting all the life penalty. For a 
reliable write the situation may be a bit different.

Yes, reliable write is the key(which we need, doesn't everybody? :). This 
is even default for some eMMC devices.

> 
> This being said, I realized that switching to the native sector size is 
a one-time operation that needs to be done before hardware partitioning, 
so yes it would be good to add it to U-Boot. But, as I said, I do not have 
any 4.5 / 4.51 devices at hand to develop this feature with.

You could just "copy" the code from mmc-utils and do a RFC impl. with a 
big fat warning attached to it.

> 
> While writing this I also realized that the current choice of 
CONFIG_SYS_MMC_MAX_BLK_COUNT being 65535 is not good, as writing more than 
65535 512-byte blocks will result in issuing non-aligned writes on the 
second and subsequent chunks when writing 32 MiB or more in a single 
command. A simple fix for this would be to set the default value for 
CONFIG_SYS_MMC_MAX_BLK_COUNT to 65280 (i.e. aligned to 256 KiB) so that it 
is aligned to any reasonable superpage size. A more elaborate fix would be 
for b_max to be rounded down to a multiple of the superpage size read from 
EXT_CSD in mmc_startup().

yes, you could consider 4 MiB alignment as well, 
http://lwn.net/Articles/428584/

[U-Boot] [PATCH 00/18] Support for eMMC partitioning and related fixes

[Diego Santa Cruz]

> -----Original Message-----
> From: Joakim Tjernlund [mailto:joakim.tjernlund at transmode.se]
> Sent: Friday, December 05, 2014 10:46 AM
> To: Diego Santa Cruz
> Cc: u-boot at lists.denx.de
> Subject: RE: [U-Boot] [PATCH 00/18] Support for eMMC partitioning and related
> fixes
> 
[snip]
> >
> > Write size should always be in a multiple of the superpage size to
> maximize performance and life, and the superpage size will be a multiple
> of 4 KB for any reasonable eMMC device. Currently, as far as I can tell,
> the mmc code does the writes in multiple blocks, by chunks of up to
> CONFIG_SYS_MMC_MAX_BLK_COUNT (65535 by default) 512-byte blocks, so data
> is written in sufficiently big chunks as long as the mmc write command is
> issued properly aligned and with a proper length. The eMMC device does not
> have to have committed the data until the end of the multiple block write
> command, so it should buffer up the data to sufficiently large buffers
> before writing them to NAND, lifting most of the performance penalty of
> the 512-byte sectors and, I believe, lifting all the life penalty. For a
> reliable write the situation may be a bit different.
> 
> Yes, reliable write is the key(which we need, doesn't everybody? :). This
> is even default for some eMMC devices.
> 
> >
> > This being said, I realized that switching to the native sector size is
> a one-time operation that needs to be done before hardware partitioning,
> so yes it would be good to add it to U-Boot. But, as I said, I do not have
> any 4.5 / 4.51 devices at hand to develop this feature with.
> 
> You could just "copy" the code from mmc-utils and do a RFC impl. with a
> big fat warning attached to it.
> 

Then I leave it as an exercise for the reader :-) Note that after setting a 4 KB device to its native sector size you cannot issue any read nor write whose offset or size is not 4KB aligned, I do not know if all U-Boot code is currently capable of this, otherwise the mmc read / write code needs to emulate unaligned accesses.

> >
> > While writing this I also realized that the current choice of
> CONFIG_SYS_MMC_MAX_BLK_COUNT being 65535 is not good, as writing more than
> 65535 512-byte blocks will result in issuing non-aligned writes on the
> second and subsequent chunks when writing 32 MiB or more in a single
> command. A simple fix for this would be to set the default value for
> CONFIG_SYS_MMC_MAX_BLK_COUNT to 65280 (i.e. aligned to 256 KiB) so that it
> is aligned to any reasonable superpage size. A more elaborate fix would be
> for b_max to be rounded down to a multiple of the superpage size read from
> EXT_CSD in mmc_startup().
> 
> yes, you could consider 4 MiB alignment as well,
> http://lwn.net/Articles/428584/

It seems to me that 4 MiB alignment for breaking up a large write into chunks is overkill. It is the user of the eMMC that should take care to align things to 4 MiB or similar. I guess it would be even better to align the things to the write protect group or erase size.

I'll cook something up for this issue as soon as I have some spare time.

Best,

-- 
Diego Santa Cruz, PhD
Technology Architect
spinetix.com