Skip to contentSkip to conversation

mmc: implemented eMMC4.4 enhanced area feature

PATCH Superseded Chuanxiao Dong 2 participants 5 messages Jan 13, 2011 – Jan 20, 2011 MMC, SD and SDIO

v2 of 4 Superseded Latest is v4 →

lore
Share & tools
lore

Chuanxiao Dong

author
#0 lore raw

PATCH v2 1/1 mmc: implemented eMMC4.4 enhanced area feature

Enhanced area feature is a new feature defined in eMMC4.4 standard. This kind of area can help to improve the performance.

MMC driver will read out the enhanced area offset and size and add them to be device attributes. The feature enabling should be done in manufactory. To use this feature, bit ERASE_GRP_DEF should also be set.

Documentation/ABI/testing/sysfs-devices-mmc described the two new attributes

Signed-off-by Chuanxiao Dong <[email protected]>
---
 Documentation/ABI/testing/sysfs-devices-mmc |   19 +++++++
 drivers/mmc/core/mmc.c                      |   73 +++++++++++++++++++++++++++
 include/linux/mmc/card.h                    |    3 +
 include/linux/mmc/mmc.h                     |    3 +
 4 files changed, 98 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-devices-mmc
Documentation/ABI/testing/sysfs-devices-mmc new file +19 -0
new file mode 100644
index 0000000..f0fb053
@@ -0,0 +1,19 @@
+What:		/sys/devices/.../mmc_host/mmcX/mmcX:XXXX/enhanced_area_offset
+Date:		January 2011
+Contact:	Chuanxiao Dong <[email protected]>
+Description:
+		Enhanced area is a new feature defined in eMMC4.4 standard.
+		eMMC4.4 or later card can support such feature. This kind of area
+		can help to improve the card performance. If the feature is enabled,
+		this attribute will indicate the start address of enhanced data
+		area. If not, this attribute will be 0. Unit Byte.
+
+What:		/sys/devices/.../mmc_host/mmcX/mmcX:XXXX/enhanced_area_size
+Date:		January 2011
+Contact:	Chuanxiao Dong <[email protected]>
+Description:
+		Enhanced area is a new feature defined in eMMC4.4 standard.
+		eMMC4.4 or later card can support such feature. This kind of area
+		can help to improve the card performance. If the feature is enabled,
+		this attribute will indicate the size of enhanced data area. If not,
+		this attribute will be 0. Unit KBytes.
drivers/mmc/core/mmc.c +73 -0

97 lines not on this page · raw message

include/linux/mmc/card.h +3 -0
index 8ce0827..cc9a07e 100644
@@ -54,6 +54,9 @@ struct mmc_ext_csd {
 	unsigned int		sec_trim_mult;	/* Secure trim multiplier  */
 	unsigned int		sec_erase_mult;	/* Secure erase multiplier */
 	unsigned int		trim_timeout;		/* In milliseconds */
+	bool		enhanced_area_en; /* enhanced area enable bit */
+	loff_t		enhanced_area_offset; /* enhanced area addr */
+	size_t		enhanced_area_size; /* enhanced area size */
 };
 
 struct sd_scr {
include/linux/mmc/mmc.h +3 -0
index 612301f..c50ede6 100644
@@ -253,6 +253,8 @@ struct _mmc_csd {
  * EXT_CSD fields
  */
 
+#define EXT_CSD_PARTITION_ATTRIBUTE 156	/* R/W */
+#define EXT_CSD_PARTITION_SUPPORT	160	/* RO */
 #define EXT_CSD_ERASE_GROUP_DEF		175	/* R/W */
 #define EXT_CSD_ERASED_MEM_CONT		181	/* RO */
 #define EXT_CSD_BUS_WIDTH		183	/* R/W */

@@ -262,6 +264,7 @@ struct _mmc_csd {
 #define EXT_CSD_CARD_TYPE		196	/* RO */
 #define EXT_CSD_SEC_CNT			212	/* RO, 4 bytes */
 #define EXT_CSD_S_A_TIMEOUT		217	/* RO */
+#define EXT_CSD_HC_WP_GRP_SIZE		221	/* RO */
 #define EXT_CSD_ERASE_TIMEOUT_MULT	223	/* RO */
 #define EXT_CSD_HC_ERASE_GRP_SIZE	224	/* RO */
 #define EXT_CSD_SEC_TRIM_MULT		229	/* RO */
Signature · 1 line
1.6.6.1
#1 lore raw

Hi Chuanxiao,

Quoted 10 lines · Chuanxiao Dong

Enhanced area feature is a new feature defined in eMMC4.4 standard. This kind of area can help to improve the performance.

MMC driver will read out the enhanced area offset and size and add them to be device attributes. The feature enabling should be done in manufactory. To use this feature, bit ERASE_GRP_DEF should also be set.

Documentation/ABI/testing/sysfs-devices-mmc described the two new attributes

Signed-off-by Chuanxiao Dong <[email protected]>

Thanks for the explanation of your use case, this patch looks good. Just a few more comments:

Quoted 23 lines
---
 Documentation/ABI/testing/sysfs-devices-mmc |   19 +++++++
 drivers/mmc/core/mmc.c                      |   73 +++++++++++++++++++++++++++
 include/linux/mmc/card.h                    |    3 +
 include/linux/mmc/mmc.h                     |    3 +
 4 files changed, 98 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-devices-mmc
Documentation/ABI/testing/sysfs-devices-mmc new file +9 -0
new file mode 100644
index 0000000..f0fb053
@@ -0,0 +1,19 @@
+What:		/sys/devices/.../mmc_host/mmcX/mmcX:XXXX/enhanced_area_offset
+Date:		January 2011
+Contact:	Chuanxiao Dong <[email protected]>
+Description:
+		Enhanced area is a new feature defined in eMMC4.4 standard.
+		eMMC4.4 or later card can support such feature. This kind of area
+		can help to improve the card performance. If the feature is enabled,
+		this attribute will indicate the start address of enhanced data
+		area. If not, this attribute will be 0. Unit Byte.

Documentation/ABI/testing/sysfs-devices-mmc line 19

Quoted 10 lines
Documentation/ABI/testing/sysfs-devices-mmc new file +10 -0
+
+What:		/sys/devices/.../mmc_host/mmcX/mmcX:XXXX/enhanced_area_size
+Date:		January 2011
+Contact:	Chuanxiao Dong <[email protected]>
+Description:
+		Enhanced area is a new feature defined in eMMC4.4 standard.
+		eMMC4.4 or later card can support such feature. This kind of area
+		can help to improve the card performance. If the feature is enabled,
+		this attribute will indicate the size of enhanced data area. If not,
+		this attribute will be 0. Unit KBytes.

Please wrap the above at 80 chars.

drivers/mmc/core/mmc.c static int mmc_read_ext_csd(struct mmc_card *card) · line 307

Quoted 11 lines
drivers/mmc/core/mmc.c +3 -0
index 16006ef..c8b5b3b 100644
@@ -302,6 +302,41 @@ static int mmc_read_ext_csd(struct mmc_card *card)
 	}
 
 	if (card->ext_csd.rev >= 4) {
+		/*
+		 * Enhanced area feature support
+		 * check whether eMMC card is enabled enhanced area,

check whether eMMC card has the Enhanced Area enabled

drivers/mmc/core/mmc.c static int mmc_read_ext_csd(struct mmc_card *card) · line 319

Quoted 12 lines
drivers/mmc/core/mmc.c +12 -0
+		 * if so, export enhanced area offset and size to
+		 * user by adding sysfs interface
+		 */
+		if ((ext_csd[EXT_CSD_PARTITION_SUPPORT] & 0x2) &&
+				(ext_csd[EXT_CSD_PARTITION_ATTRIBUTE] & 0x1)) {
+			u8 hc_erase_grp_sz =
+				ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE];
+			u8 hc_wp_grp_sz =
+				ext_csd[EXT_CSD_HC_WP_GRP_SIZE];
+			/*
+			 * set a flag to identify whether the enhanced
+			 * user data are enabled

enhanced user data area is enabled

drivers/mmc/core/mmc.c static int mmc_read_ext_csd(struct mmc_card *card) · line 323

Quoted 4 lines
drivers/mmc/core/mmc.c +4 -0
+			 */
+			card->ext_csd.enhanced_area_en = 1;
+			/*
+			 * caculate the enhanced data area offset, unit B

calculate

Quoted 20 lines
drivers/mmc/core/mmc.c +16 -0
+			 */
+			card->ext_csd.enhanced_area_offset =
+				(ext_csd[139] << 24) + (ext_csd[138] << 16) +
+				(ext_csd[137] << 8) + ext_csd[136];
+			if (mmc_card_blockaddr(card))
+				card->ext_csd.enhanced_area_offset <<= 9;
+			/*
+			 * caculate the enhanced data area size, unit KB
+			 */
+			card->ext_csd.enhanced_area_size =
+				(ext_csd[142] << 16) + (ext_csd[141] << 8) +
+				ext_csd[140];
+			card->ext_csd.enhanced_area_size *=
+				(size_t)(hc_erase_grp_sz * hc_wp_grp_sz);
+			card->ext_csd.enhanced_area_size <<= 9;
+		}
 		card->ext_csd.sec_trim_mult =
 			ext_csd[EXT_CSD_SEC_TRIM_MULT];
 		card->ext_csd.sec_erase_mult =

drivers/mmc/core/mmc.c MMC_DEV_ATTR(manfid, "0x%06x\n", card->cid.manfid); · line 376

Quoted 6 lines
drivers/mmc/core/mmc.c +3 -0
@@ -336,6 +371,9 @@ MMC_DEV_ATTR(manfid, "0x%06x\n", card->cid.manfid);
 MMC_DEV_ATTR(name, "%s\n", card->cid.prod_name);
 MMC_DEV_ATTR(oemid, "0x%04x\n", card->cid.oemid);
 MMC_DEV_ATTR(serial, "0x%08x\n", card->cid.serial);
+MMC_DEV_ATTR(enhanced_area_offset, "0x%llxB\n",
+		card->ext_csd.enhanced_area_offset);
+MMC_DEV_ATTR(enhanced_area_size, "0x%xKB\n", card->ext_csd.enhanced_area_size);

I think these should probably be printed in decimal instead of hex.

Hm, this way the sysfs files will show up regardless of whether we have an eMMC device or an SD card, which is very confusing. Can you not check whether enhanced_area_en == 1 before create these files?

Quoted 27 lines
drivers/mmc/core/mmc.c +13 -0
 static struct attribute *mmc_std_attrs[] = {
 	&dev_attr_cid.attr,

@@ -349,6 +387,8 @@ static struct attribute *mmc_std_attrs[] = {
 	&dev_attr_name.attr,
 	&dev_attr_oemid.attr,
 	&dev_attr_serial.attr,
+	&dev_attr_enhanced_area_offset.attr,
+	&dev_attr_enhanced_area_size.attr,
 	NULL,
 };
 

@@ -484,6 +524,39 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
 	}
 
 	/*
+	 * If enhanced_area_en is TRUE, host need to enable
+	 * ERASE_GRP_DEF bit.
+	 * ERASE_GRP_DEF bit will be lost everytime
+	 * after a reset or power off.
+	 */
+	if (card->ext_csd.enhanced_area_en) {
+		err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
+				EXT_CSD_ERASE_GROUP_DEF, 1);
+
+		if (err && err != -EBADMSG)
+			goto free_card;

drivers/mmc/core/mmc.c static int mmc_init_card(struct mmc_host *host, u32 ocr, · line 547

Quoted 10 lines
drivers/mmc/core/mmc.c +10 -0
+
+		if (err) {
+			err = 0;
+			/*
+			 * Just disable enhanced area off & sz
+			 * will try to enable ERASE_GROUP_DEF
+			 * during next time reinit
+			 */
+			card->ext_csd.enhanced_area_offset = 0;
+			card->ext_csd.enhanced_area_size = 0;

It makes more sense to me to return -EINVAL instead of 0 if we know nothing about the enhanced_area. What do you think?

Quoted 20 lines
drivers/mmc/core/mmc.c +12 -0
+		} else {
+			card->ext_csd.erase_group_def = 1;
+			/*
+			 * enable ERASE_GRP_DEF successfully.
+			 * This will affect the erase size, so
+			 * here need to reset erase size
+			 */
+			mmc_set_erase_size(card);
+		}
+	}
+
+	/*
 	 * Activate high speed (if supported)
 	 */
 	if ((card->ext_csd.hs_max_dtr != 0) &&
include/linux/mmc/card.h +0 -0
index 8ce0827..cc9a07e 100644

include/linux/mmc/card.h struct mmc_ext_csd { · line 59

Quoted 6 lines
include/linux/mmc/card.h +3 -0
@@ -54,6 +54,9 @@ struct mmc_ext_csd {
 	unsigned int		sec_trim_mult;	/* Secure trim multiplier  */
 	unsigned int		sec_erase_mult;	/* Secure erase multiplier */
 	unsigned int		trim_timeout;		/* In milliseconds */
+	bool		enhanced_area_en; /* enhanced area enable bit */
+	loff_t		enhanced_area_offset; /* enhanced area addr */
+	size_t		enhanced_area_size; /* enhanced area size */

Think you need another tab to get these to line up.

include/linux/mmc/mmc.h struct _mmc_csd { · line 256

Quoted 12 lines
include/linux/mmc/card.h +0 -0
 };
 
 struct sd_scr {
include/linux/mmc/mmc.h +1 -0
index 612301f..c50ede6 100644
@@ -253,6 +253,8 @@ struct _mmc_csd {
  * EXT_CSD fields
  */
 
+#define EXT_CSD_PARTITION_ATTRIBUTE 156	/* R/W */

This isn't lined up properly either.

include/linux/mmc/mmc.h struct _mmc_csd { · line 270

Quoted 12 lines
include/linux/mmc/mmc.h +2 -0
+#define EXT_CSD_PARTITION_SUPPORT	160	/* RO */
 #define EXT_CSD_ERASE_GROUP_DEF		175	/* R/W */
 #define EXT_CSD_ERASED_MEM_CONT		181	/* RO */
 #define EXT_CSD_BUS_WIDTH		183	/* R/W */

@@ -262,6 +264,7 @@ struct _mmc_csd {
 #define EXT_CSD_CARD_TYPE		196	/* RO */
 #define EXT_CSD_SEC_CNT			212	/* RO, 4 bytes */
 #define EXT_CSD_S_A_TIMEOUT		217	/* RO */
+#define EXT_CSD_HC_WP_GRP_SIZE		221	/* RO */
 #define EXT_CSD_ERASE_TIMEOUT_MULT	223	/* RO */
 #define EXT_CSD_HC_ERASE_GRP_SIZE	224	/* RO */
 #define EXT_CSD_SEC_TRIM_MULT		229	/* RO */

Thanks,

Signature · 2 lines
Chris Ball   <[email protected]>   <http://printf.net/>
One Laptop Per Child
author
#2 lore raw

Hi Chris,
Thanks for comments. I will fix the typo and line warp errors in the next submissions. :)

Quoted 8 lines

drivers/mmc/core/mmc.c MMC_DEV_ATTR(manfid, "0x%06x\n", card->cid.manfid);

Quoted 6 lines

MMC_DEV_ATTR(name, "%s\n", card->cid.prod_name); MMC_DEV_ATTR(oemid, "0x%04x\n", card->cid.oemid); MMC_DEV_ATTR(serial, "0x%08x\n", card->cid.serial);

drivers/mmc/core/mmc.c +3 -0
+MMC_DEV_ATTR(enhanced_area_offset, "0x%llxB\n",
+		card->ext_csd.enhanced_area_offset);
+MMC_DEV_ATTR(enhanced_area_size, "0x%xKB\n",

card->ext_csd.enhanced_area_size);

Quoted 6 lines

I think these should probably be printed in decimal instead of hex.

Hm, this way the sysfs files will show up regardless of whether we have an eMMC device or an SD card, which is very confusing. Can you not check whether enhanced_area_en == 1 before create these files?

SD card will not use this code when initializing. So I think these files only are created for eMMC card. If the eMMC card doesn't support enhanced area, can we use the same way like below scenario, exporting some value like -EINVAL to user? What do you think?

Quoted 33 lines

drivers/mmc/core/mmc.c static int mmc_init_card(struct mmc_host *host, u32

Quoted 13 lines

static struct attribute *mmc_std_attrs[] = {

drivers/mmc/core/mmc.c +2 -0
 	&dev_attr_cid.attr,

@@ -349,6 +387,8 @@ static struct attribute *mmc_std_attrs[] = {
 	&dev_attr_name.attr,
 	&dev_attr_oemid.attr,
 	&dev_attr_serial.attr,
+	&dev_attr_enhanced_area_offset.attr,
+	&dev_attr_enhanced_area_size.attr,
 	NULL,
 };
 

@@ -484,6 +524,39 @@ static int mmc_init_card(struct mmc_host *host, u32

ocr,

Quoted 19 lines
drivers/mmc/core/mmc.c +16 -0
 	}
 
 	/*
+	 * If enhanced_area_en is TRUE, host need to enable
+	 * ERASE_GRP_DEF bit.
+	 * ERASE_GRP_DEF bit will be lost everytime
+	 * after a reset or power off.
+	 */
+	if (card->ext_csd.enhanced_area_en) {
+		err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
+				EXT_CSD_ERASE_GROUP_DEF, 1);
+
+		if (err && err != -EBADMSG)
+			goto free_card;
+
+		if (err) {
+			err = 0;
+			/*
+			 * Just disable enhanced area off & sz
Quoted 8 lines

drivers/mmc/core/mmc.c static int mmc_init_card(struct mmc_host *host, u32 ocr, · line 547

Quoted 5 lines
drivers/mmc/core/mmc.c +5 -0
+			 * will try to enable ERASE_GROUP_DEF
+			 * during next time reinit
+			 */
+			card->ext_csd.enhanced_area_offset = 0;
+			card->ext_csd.enhanced_area_size = 0;

It makes more sense to me to return -EINVAL instead of 0 if we know nothing about the enhanced_area. What do you think?

Agree. This can let user know the enhanced area offset and size are invalid argument.

Thanks
Chuanxiao

#3 lore raw

Hi Chuanxiao,

Quoted 1 line · Chuanxiao
Quoted 1 line

I think these should probably be printed in decimal instead of hex.

(Will assume you agree with this, let me know if not.)

Quoted 5 lines
Quoted 4 lines

Hm, this way the sysfs files will show up regardless of whether we have an eMMC device or an SD card, which is very confusing. Can you not check whether enhanced_area_en == 1 before create these files?

SD card will not use this code when initializing. So I think these files only are created for eMMC card. If the eMMC card doesn't support enhanced area, can we use the same way like below scenario, exporting some value like -EINVAL to user? What do you think?

Oh, makes sense -- I agree, let's just use -EINVAL, and update the sysfs ABI doc to mention that. Thanks!

Signature · 2 lines
Chris Ball   <[email protected]>   <http://printf.net/>
One Laptop Per Child
author
#4 lore raw

Hi Chris,

Quoted 5 lines · Chuanxiao
Quoted 1 line
Quoted 1 line

I think these should probably be printed in decimal instead of hex.

(Will assume you agree with this, let me know if not.)

Agree with this. Sorry for missing it.

Quoted 5 lines
Quoted 5 lines
Quoted 5 lines

Hm, this way the sysfs files will show up regardless of whether we have an eMMC device or an SD card, which is very confusing. Can you not check whether enhanced_area_en == 1 before create these files?

Quoted 7 lines
Quoted 1 line

SD card will not use this code when initializing. So I think these files only are

created for eMMC card. If the eMMC card doesn't support enhanced area, can we use the same way like below scenario, exporting some value like -EINVAL to user? What do you think?

Oh, makes sense -- I agree, let's just use -EINVAL, and update the sysfs ABI doc to mention that. Thanks!

Cool! I will fix these and then resubmit the new patch. :)

Thanks
Chuanxiao