Skip to contentSkip to conversation

mmc: implemented eMMC4.4 enhanced area feature

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

v3 of 4 Superseded Latest is v4 →

lore
Share & tools
lore
#0 lore raw

PATCH v3 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

---
 Documentation/ABI/testing/sysfs-devices-mmc |   19 ++++++
 drivers/mmc/core/mmc.c                      |   80 +++++++++++++++++++++++++++
 include/linux/mmc/card.h                    |    3 +
 include/linux/mmc/mmc.h                     |    3 +
 4 files changed, 105 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..6e759ae
@@ -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 -EINVAL. Unit Byte. Format decimal.
+
+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 -EINVAL. Unit KByte. Format decimal.
drivers/mmc/core/mmc.c +80 -0

104 lines not on this page · raw message

include/linux/mmc/card.h +3 -0
index 8ce0827..736697f 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 en */
+	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..264ba54 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 33 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

---
 Documentation/ABI/testing/sysfs-devices-mmc |   19 ++++++
 drivers/mmc/core/mmc.c                      |   80 +++++++++++++++++++++++++++
 include/linux/mmc/card.h                    |    3 +
 include/linux/mmc/mmc.h                     |    3 +
 4 files changed, 105 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..6e759ae
@@ -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 -EINVAL. Unit Byte. Format decimal.

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 -EINVAL. Unit KByte. Format decimal.

This is still wrapped at > 80 columns.

Quoted 117 lines
drivers/mmc/core/mmc.c +80 -0
index 16006ef..5ab44bb 100644
@@ -302,6 +302,48 @@ static int mmc_read_ext_csd(struct mmc_card *card)
 	}
 
 	if (card->ext_csd.rev >= 4) {
+		/*
+		 * Enhanced area feature support
+		 * check whether eMMC card has the Enhanced area enabled.
+		 * 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 is enabled
+			 */
+			card->ext_csd.enhanced_area_en = 1;
+			/*
+			 * calculate the enhanced data area offset, unit B
+			 */
+			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;
+			/*
+			 * calculate 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;
+		} else {
+			/*
+			 * If not enabled enhanced area, disable these
+			 * device attributes
+			 */
+			card->ext_csd.enhanced_area_offset = -EINVAL;
+			card->ext_csd.enhanced_area_size = -EINVAL;
+		}
 		card->ext_csd.sec_trim_mult =
 			ext_csd[EXT_CSD_SEC_TRIM_MULT];
 		card->ext_csd.sec_erase_mult =
@@ -336,6 +378,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, "%lld\n",
+		card->ext_csd.enhanced_area_offset);
+MMC_DEV_ATTR(enhanced_area_size, "%d\n", card->ext_csd.enhanced_area_size);
 
 static struct attribute *mmc_std_attrs[] = {
 	&dev_attr_cid.attr,
@@ -349,6 +394,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 +531,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 every time
+	 * 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
+			 * will try to enable ERASE_GROUP_DEF
+			 * during next time reinit
+			 */
+			card->ext_csd.enhanced_area_offset = -EINVAL;
+			card->ext_csd.enhanced_area_size = -EINVAL;
+		} 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..736697f 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 en */
+	loff_t		enhanced_area_offset;	/* enhanced area addr */
+	size_t		enhanced_area_size;		/* enhanced area size */

Now enhanced_area_en is aligned correctly, but the other two aren't.

This adds a warning:

drivers/mmc/core/mmc.c: In function ‘mmc_enhanced_area_size_show’:
drivers/mmc/core/mmc.c:383:261: warning: format ‘%d’ expects type ‘int’, but argument 3 has type ‘size_t’

If you really want size_t, you should use %zu for it -- see Documentation/printk-formats.txt. (Declaring it as unsigned int would work fine too.)

Oh, and let's mention the units of _offset and _size in the comment in card.h.

Quoted 17 lines
include/linux/mmc/card.h +0 -0
 };
 
 struct sd_scr {
include/linux/mmc/mmc.h +2 -0
index 612301f..264ba54 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 */

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

Quoted 7 lines
include/linux/mmc/mmc.h +1 -0
@@ -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
#2 lore raw

Hi Chris,

Quoted 21 lines

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

Quoted 4 lines
Documentation/ABI/testing/sysfs-devices-mmc new file +1 -0
@@ -0,0 +1,19 @@
+What:

/sys/devices/.../mmc_host/mmcX/mmcX:XXXX/enhanced_area_offset

Documentation/ABI/testing/sysfs-devices-mmc

Quoted 4 lines
Documentation/ABI/testing/sysfs-devices-mmc new file +4 -0
+Date:		January 2011
+Contact:	Chuanxiao Dong <[email protected]>
+Description:
+		Enhanced area is a new feature defined in eMMC4.4 standard.eMMC4.4

or

Documentation/ABI/testing/sysfs-devices-mmc

Quoted 10 lines
Documentation/ABI/testing/sysfs-devices-mmc new file +10 -0
+		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 -EINVAL. Unit Byte. Format decimal.
+
+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

Quoted 6 lines

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

Quoted 4 lines
Documentation/ABI/testing/sysfs-devices-mmc new file +4 -0
+		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 -EINVAL. Unit KByte. Format decimal.

This is still wrapped at > 80 columns.

Sorry for that. I have set the textwidth=80 for vim, not understand why this is still wrapped at >80 columns... I know checkpatch.pl can help us to find the style problem, and is there some other script which can help find the style problem of Document or how can I avoid such errors? Chris, thank you for your kind patience to help me find these stupid errors again...

Quoted 20 lines

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

Quoted 11 lines
include/linux/mmc/card.h +3 -0
index 8ce0827..736697f 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 en */
+	loff_t		enhanced_area_offset;	/* enhanced area addr */
+	size_t		enhanced_area_size;		/* enhanced area size */

Now enhanced_area_en is aligned correctly, but the other two aren't.

This adds a warning:

drivers/mmc/core/mmc.c: In function ‘mmc_enhanced_area_size_show’:
drivers/mmc/core/mmc.c:383:261: warning: format ‘%d’ expects type ‘int’, but argument 3 has type ‘size_t’

Quoted 6 lines

If you really want size_t, you should use %zu for it -- see Documentation/printk-formats.txt. (Declaring it as unsigned int would work fine too.)

Oh, and let's mention the units of _offset and _size in the comment in card.h.

Chris, you mean the type of _offset and _size? I think the 64bits number for _offset is enough, also the 32bits number for _size since the units of _size is Kbytes. Any suggestions on that?

Quoted 31 lines
Quoted 18 lines
include/linux/mmc/card.h +0 -0
 };
 
 struct sd_scr {
include/linux/mmc/mmc.h +2 -0
index 612301f..264ba54 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 */

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

Quoted 7 lines
include/linux/mmc/mmc.h +1 -0
@@ -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,

--
Chris Ball <[email protected]> <http://printf.net/>
One Laptop Per Child

#3 lore raw

Hi Chuanxiao,

Quoted 22 lines · Chuanxiao

Hi Chris,

Quoted 21 lines

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

Quoted 4 lines
Documentation/ABI/testing/sysfs-devices-mmc new file +1 -0
@@ -0,0 +1,19 @@
+What:

/sys/devices/.../mmc_host/mmcX/mmcX:XXXX/enhanced_area_offset

Documentation/ABI/testing/sysfs-devices-mmc

Quoted 4 lines
Documentation/ABI/testing/sysfs-devices-mmc new file +4 -0
+Date:		January 2011
+Contact:	Chuanxiao Dong <[email protected]>
+Description:
+		Enhanced area is a new feature defined in eMMC4.4 standard.eMMC4.4

or

Documentation/ABI/testing/sysfs-devices-mmc

Quoted 10 lines
Documentation/ABI/testing/sysfs-devices-mmc new file +10 -0
+		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 -EINVAL. Unit Byte. Format decimal.
+
+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

Quoted 7 lines
Quoted 6 lines

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

Quoted 4 lines
Documentation/ABI/testing/sysfs-devices-mmc new file +4 -0
+		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 -EINVAL. Unit KByte. Format decimal.

This is still wrapped at > 80 columns.

Sorry for that. I have set the textwidth=80 for vim, not understand why this is still wrapped at >80 columns... I know checkpatch.pl can help us to find the style problem, and is there some other script which can help find the style problem of Document or how can I avoid such errors?

checkpatch.pl will warn about >80 chars, so that should be fine on its own.

Quoted 4 lines
Quoted 2 lines

Oh, and let's mention the units of _offset and _size in the comment in card.h.

Chris, you mean the type of _offset and _size? I think the 64bits number for _offset is enough, also the 32bits number for _size since the units of _size is Kbytes. Any suggestions on that?

Ah, I wasn't clear -- I meant the bytes/kilobytes units, not the data type sizes. e.g.:

+ size_t enhanced_area_size; /* Units: KB */

Thanks,

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

Hi Chris,

Quoted 14 lines

/sys/devices/.../mmc_host/mmcX/mmcX:XXXX/enhanced_area_size

Quoted 4 lines
Quoted 4 lines

Documentation/ABI/testing/sysfs-devices-mmc

Quoted 4 lines
Documentation/ABI/testing/sysfs-devices-mmc new file +4 -0
+Date:		January 2011
+Contact:	Chuanxiao Dong <[email protected]>
+Description:
+		Enhanced area is a new feature defined in eMMC4.4 standard.

eMMC4.4

Quoted 3 lines
Quoted 3 lines

or

Review

Quoted 2 lines
(quoted diff) +2 -0
+		later card can support such feature. This kind of area can help to
+		improve the card performance. If the feature is enabled, this

attribute

Quoted 4 lines
Quoted 4 lines

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

Quoted 2 lines
Documentation/ABI/testing/sysfs-devices-mmc new file +2 -0
+		will indicate the size of enhanced data area. If not, this attribute
+		will be -EINVAL. Unit KByte. Format decimal.

This is still wrapped at > 80 columns.

Quoted 6 lines
Quoted 1 line

Sorry for that. I have set the textwidth=80 for vim, not understand why this is still

wrapped at >80 columns... I know checkpatch.pl can help us to find the style problem, and is there some other script which can help find the style problem of Document or how can I avoid such errors?

checkpatch.pl will warn about >80 chars, so that should be fine on its own.

I have used the checkpatch.pl to check this patch, and no error or warning found....

Quoted 4 lines
Quoted 4 lines
Quoted 3 lines

Oh, and let's mention the units of _offset and _size in the comment in card.h.

Quoted 8 lines
Quoted 1 line

Chris, you mean the type of _offset and _size? I think the 64bits number for

_offset is enough, also the 32bits number for _size since the units of _size is Kbytes. Any suggestions on that?

Ah, I wasn't clear -- I meant the bytes/kilobytes units, not the data type sizes. e.g.:

+ size_t enhanced_area_size; /* Units: KB */

I think the units of _offset and _size should be easy for user to understand, so just choose the Byte for _offset and Kbyte for _size...
And as spec said, the _size is (ENH_SIZE_MULT_2 << 16 + ENH_SIZE_MULT_1 << 8 + ENH_SIZE_MULT_0) x HC_WP_GRP_SIZE x HC_ERASE_GPR_SIZE x 512kBytes. It must be Kbytes aligned. That is another reason to choose it. What do you think?

Thanks
Chuanxiao

#5 lore raw

Hi Chuanxiao,

Quoted 1 line · Chuanxiao

I have used the checkpatch.pl to check this patch, and no error or warning found....

Oh, you're right! I wonder if that's because it's in a .txt file rather than a source file. Not sure how to get it to warn about this, then.

Quoted 6 lines
Quoted 4 lines

Ah, I wasn't clear -- I meant the bytes/kilobytes units, not the data type sizes. e.g.:

+ size_t enhanced_area_size; /* Units: KB */

I think the units of _offset and _size should be easy for user to understand, so just choose the Byte for _offset and Kbyte for _size...
And as spec said, the _size is (ENH_SIZE_MULT_2 << 16 + ENH_SIZE_MULT_1 << 8 + ENH_SIZE_MULT_0) x HC_WP_GRP_SIZE x HC_ERASE_GPR_SIZE x 512kBytes. It must be Kbytes aligned. That is another reason to choose it.

I agree that we we should use these units, I'm just arguing that we might as well mention them in the comment area. The other card.h definitions mention their unit sizes too. At the moment your comments for card.h simply repeat the variable name, which isn't useful -- why not make them repeat some useful and not-necessarily-obvious information instead?

Thanks,

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

Hi Chris,

Quoted 27 lines

-----Original Message-----
From: Chris Ball [mailto:[email protected]]
Sent: Friday, January 21, 2011 11:28 PM
To: Dong, Chuanxiao
Cc: [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH v3 1/1]mmc: implemented eMMC4.4 enhanced area feature

Hi Chuanxiao,

Quoted 1 line · Chuanxiao

I have used the checkpatch.pl to check this patch, and no error or warning

found....

Oh, you're right! I wonder if that's because it's in a .txt file rather than a source file. Not sure how to get it to warn about this, then.

Quoted 5 lines
Quoted 4 lines

Ah, I wasn't clear -- I meant the bytes/kilobytes units, not the data type sizes. e.g.:

+ size_t enhanced_area_size; /* Units: KB */

I think the units of _offset and _size should be easy for user to understand, so just

choose the Byte for _offset and Kbyte for _size...

Quoted 1 line

And as spec said, the _size is (ENH_SIZE_MULT_2 << 16 + ENH_SIZE_MULT_1 <<

8 + ENH_SIZE_MULT_0) x HC_WP_GRP_SIZE x HC_ERASE_GPR_SIZE x 512kBytes. It must be Kbytes aligned. That is another reason to choose it.

Quoted 6 lines

I agree that we we should use these units, I'm just arguing that we might as well mention them in the comment area. The other card.h definitions mention their unit sizes too. At the moment your comments for card.h simply repeat the variable name, which isn't useful -- why not make them repeat some useful and not-necessarily-obvious information instead?

Sure, Chris. I will take care of such things in my future submissions. Of cause including the next v4 enhanced area feature patch. :) I also remember to fix the compile warnings. Thanks a lot, Chris.

Thanks
Chuanxiao