[PATCH] mmc: Report firmware version for eMMC 5.0 devices.

Discussion:

Gwendal Grignou

2014-10-15 23:04:48 UTC

For eMMC 5.0 compliant device, firmware version is stored in ext_csd.
Report firmware as a 64bit hexa decimal. Vendor can use hexa or ascii
string to report firmware version.
Also add FFU related EXT_CSD register and note if the device is FFU capable.

Signed-off-by: Gwendal Grignou <***@chromium.org>
---
drivers/mmc/core/mmc.c | 24 +++++++++++++++++++++++-
include/linux/mmc/card.h | 2 ++
include/linux/mmc/mmc.h | 10 ++++++++++
3 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 1eda8dd..a0663cf 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -603,6 +603,25 @@ static int mmc_read_ext_csd(struct mmc_card *card, u8 *ext_csd)
card->ext_csd.data_sector_size = 512;
}

+ /* eMMC v5 or later */
+ if (card->ext_csd.rev >= 7) {
+ u64 fwrev;
+
+ memcpy(&fwrev, &ext_csd[EXT_CSD_FIRMWARE_VERSION],
+ sizeof(fwrev));
+ card->ext_csd.ffu_capable =
+ ((ext_csd[EXT_CSD_SUPPORTED_MODE] & 1) == 1) &&
+ ((ext_csd[EXT_CSD_FW_CONFIG] & 1) == 0);
+ /*
+ * Firmware version can be a string or a hexa decimal number.
+ * In case of string, the first byte should be printed first.
+ * in case of hexadecimal, we can assume the first byte is the
+ * more significant, therefore we print it first as well.
+ */
+ card->ext_csd.fwrev = be64_to_cpu(fwrev);
+ } else {
+ card->ext_csd.ffu_capable = false;
+ }
out:
return err;
}
@@ -697,7 +716,9 @@ MMC_DEV_ATTR(csd, "%08x%08x%08x%08x\n", card->raw_csd[0], card->raw_csd[1],
MMC_DEV_ATTR(date, "%02d/%04d\n", card->cid.month, card->cid.year);
MMC_DEV_ATTR(erase_size, "%u\n", card->erase_size << 9);
MMC_DEV_ATTR(preferred_erase_size, "%u\n", card->pref_erase << 9);
-MMC_DEV_ATTR(fwrev, "0x%x\n", card->cid.fwrev);
+MMC_DEV_ATTR(fwrev, "0x%llx\n", (card->ext_csd.rev >= 7 ?
+ card->ext_csd.fwrev : card->cid.fwrev));
+MMC_DEV_ATTR(ffu_capable, "0x%x\n", card->ext_csd.ffu_capable);
MMC_DEV_ATTR(hwrev, "0x%x\n", card->cid.hwrev);
MMC_DEV_ATTR(manfid, "0x%06x\n", card->cid.manfid);
MMC_DEV_ATTR(name, "%s\n", card->cid.prod_name);
@@ -717,6 +738,7 @@ static struct attribute *mmc_std_attrs[] = {
&dev_attr_erase_size.attr,
&dev_attr_preferred_erase_size.attr,
&dev_attr_fwrev.attr,
+ &dev_attr_ffu_capable.attr,
&dev_attr_hwrev.attr,
&dev_attr_manfid.attr,
&dev_attr_name.attr,
diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
index d424b9d..67bae22 100644
--- a/include/linux/mmc/card.h
+++ b/include/linux/mmc/card.h
@@ -87,6 +87,8 @@ struct mmc_ext_csd {
unsigned int data_tag_unit_size; /* DATA TAG UNIT size */
unsigned int boot_ro_lock; /* ro lock support */
bool boot_ro_lockable;
+ bool ffu_capable; /* FFU support */
+ u64 fwrev; /* Firmware version */
u8 raw_exception_status; /* 54 */
u8 raw_partition_support; /* 160 */
u8 raw_rpmb_size_mult; /* 168 */
diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
index 64ec963..9216519 100644
--- a/include/linux/mmc/mmc.h
+++ b/include/linux/mmc/mmc.h
@@ -272,6 +272,9 @@ struct _mmc_csd {
* EXT_CSD fields
*/

+#define EXT_CSD_FFU_STATUS 26 /* R */
+#define EXT_CSD_MODE_OPERATION_CODES 29 /* W */
+#define EXT_CSD_MODE_CONFIG 30 /* R/W */
#define EXT_CSD_FLUSH_CACHE 32 /* W */
#define EXT_CSD_CACHE_CTRL 33 /* R/W */
#define EXT_CSD_POWER_OFF_NOTIFICATION 34 /* R/W */
@@ -290,6 +293,7 @@ struct _mmc_csd {
#define EXT_CSD_SANITIZE_START 165 /* W */
#define EXT_CSD_WR_REL_PARAM 166 /* RO */
#define EXT_CSD_RPMB_MULT 168 /* RO */
+#define EXT_CSD_FW_CONFIG 169 /* R/W */
#define EXT_CSD_BOOT_WP 173 /* R/W */
#define EXT_CSD_ERASE_GROUP_DEF 175 /* R/W */
#define EXT_CSD_PART_CONFIG 179 /* R/W */
@@ -326,6 +330,12 @@ struct _mmc_csd {
#define EXT_CSD_GENERIC_CMD6_TIME 248 /* RO */
#define EXT_CSD_CACHE_SIZE 249 /* RO, 4 bytes */
#define EXT_CSD_PWR_CL_DDR_200_360 253 /* RO */
+#define EXT_CSD_FIRMWARE_VERSION 254 /* RO, 8 bytes */
+#define EXT_CSD_NUM_OF_FW_SEC_PROG 302 /* RO, 4 bytes */
+#define EXT_CSD_FFU_ARG 487 /* RO, 4 bytes */
+#define EXT_CSD_OPERATION_CODE_TIMEOUT 491 /* RO */
+#define EXT_CSD_FFU_FEATURES 492 /* RO */
+#define EXT_CSD_SUPPORTED_MODE 493 /* RO */
#define EXT_CSD_TAG_UNIT_SIZE 498 /* RO */
#define EXT_CSD_DATA_TAG_SUPPORT 499 /* RO */
#define EXT_CSD_MAX_PACKED_WRITES 500 /* RO */

--
2.1.0.rc2.206.gedb03e5

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Ulf Hansson

2014-10-16 13:28:16 UTC

Permalink

Post by Gwendal Grignou
For eMMC 5.0 compliant device, firmware version is stored in ext_csd.
Report firmware as a 64bit hexa decimal. Vendor can use hexa or ascii
string to report firmware version.
Also add FFU related EXT_CSD register and note if the device is FFU capable.

Nitpick: Please prefix patch with "mmc: core:"

Post by Gwendal Grignou
---
drivers/mmc/core/mmc.c | 24 +++++++++++++++++++++++-
include/linux/mmc/card.h | 2 ++
include/linux/mmc/mmc.h | 10 ++++++++++
3 files changed, 35 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 1eda8dd..a0663cf 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -603,6 +603,25 @@ static int mmc_read_ext_csd(struct mmc_card *card, u8 *ext_csd)
card->ext_csd.data_sector_size = 512;
}
+ /* eMMC v5 or later */
+ if (card->ext_csd.rev >= 7) {
+ u64 fwrev;
+
+ memcpy(&fwrev, &ext_csd[EXT_CSD_FIRMWARE_VERSION],
+ sizeof(fwrev));
+ card->ext_csd.ffu_capable =
+ ((ext_csd[EXT_CSD_SUPPORTED_MODE] & 1) == 1) &&
+ ((ext_csd[EXT_CSD_FW_CONFIG] & 1) == 0);

Please convert the above statement into this:

card->ext_csd.ffu_capable = ext_csd[EXT_CSD_SUPPORTED_MODE] & 0x1 &&
!(ext_csd[EXT_CSD_FW_CONFIG] & 0x1)

Post by Gwendal Grignou
+ /*
+ * Firmware version can be a string or a hexa decimal number.
+ * In case of string, the first byte should be printed first.
+ * in case of hexadecimal, we can assume the first byte is the
+ * more significant, therefore we print it first as well.

I had a quick look in the eMMC 5.0 spec. I can't find were the above
is stated, or maybe what you are saying is just a consequence of that
there are some pieces missing in the spec? :-)

Anyway, I wonder if just should skip the conversion and keep the raw format.

Post by Gwendal Grignou
+ */
+ card->ext_csd.fwrev = be64_to_cpu(fwrev);
+ } else {

The "else" isn't needed, ffu_capable's default value is already false.

Post by Gwendal Grignou
+ card->ext_csd.ffu_capable = false;
+ }
return err;
}
@@ -697,7 +716,9 @@ MMC_DEV_ATTR(csd, "%08x%08x%08x%08x\n", card->raw_csd[0], card->raw_csd[1],
MMC_DEV_ATTR(date, "%02d/%04d\n", card->cid.month, card->cid.year);
MMC_DEV_ATTR(erase_size, "%u\n", card->erase_size << 9);
MMC_DEV_ATTR(preferred_erase_size, "%u\n", card->pref_erase << 9);
-MMC_DEV_ATTR(fwrev, "0x%x\n", card->cid.fwrev);
+MMC_DEV_ATTR(fwrev, "0x%llx\n", (card->ext_csd.rev >= 7 ?
+ card->ext_csd.fwrev : card->cid.fwrev));

cid.fwrev is not an u64.

Post by Gwendal Grignou
+MMC_DEV_ATTR(ffu_capable, "0x%x\n", card->ext_csd.ffu_capable);
MMC_DEV_ATTR(hwrev, "0x%x\n", card->cid.hwrev);
MMC_DEV_ATTR(manfid, "0x%06x\n", card->cid.manfid);
MMC_DEV_ATTR(name, "%s\n", card->cid.prod_name);
@@ -717,6 +738,7 @@ static struct attribute *mmc_std_attrs[] = {
&dev_attr_erase_size.attr,
&dev_attr_preferred_erase_size.attr,
&dev_attr_fwrev.attr,
+ &dev_attr_ffu_capable.attr,
&dev_attr_hwrev.attr,
&dev_attr_manfid.attr,
&dev_attr_name.attr,
diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
index d424b9d..67bae22 100644
--- a/include/linux/mmc/card.h
+++ b/include/linux/mmc/card.h
@@ -87,6 +87,8 @@ struct mmc_ext_csd {
unsigned int data_tag_unit_size; /* DATA TAG UNIT size */
unsigned int boot_ro_lock; /* ro lock support */
bool boot_ro_lockable;
+ bool ffu_capable; /* FFU support */
+ u64 fwrev; /* Firmware version */
u8 raw_exception_status; /* 54 */
u8 raw_partition_support; /* 160 */
u8 raw_rpmb_size_mult; /* 168 */
diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
index 64ec963..9216519 100644
--- a/include/linux/mmc/mmc.h
+++ b/include/linux/mmc/mmc.h
@@ -272,6 +272,9 @@ struct _mmc_csd {
* EXT_CSD fields
*/
+#define EXT_CSD_FFU_STATUS 26 /* R */
+#define EXT_CSD_MODE_OPERATION_CODES 29 /* W */
+#define EXT_CSD_MODE_CONFIG 30 /* R/W */
#define EXT_CSD_FLUSH_CACHE 32 /* W */
#define EXT_CSD_CACHE_CTRL 33 /* R/W */
#define EXT_CSD_POWER_OFF_NOTIFICATION 34 /* R/W */
@@ -290,6 +293,7 @@ struct _mmc_csd {
#define EXT_CSD_SANITIZE_START 165 /* W */
#define EXT_CSD_WR_REL_PARAM 166 /* RO */
#define EXT_CSD_RPMB_MULT 168 /* RO */
+#define EXT_CSD_FW_CONFIG 169 /* R/W */
#define EXT_CSD_BOOT_WP 173 /* R/W */
#define EXT_CSD_ERASE_GROUP_DEF 175 /* R/W */
#define EXT_CSD_PART_CONFIG 179 /* R/W */
@@ -326,6 +330,12 @@ struct _mmc_csd {
#define EXT_CSD_GENERIC_CMD6_TIME 248 /* RO */
#define EXT_CSD_CACHE_SIZE 249 /* RO, 4 bytes */
#define EXT_CSD_PWR_CL_DDR_200_360 253 /* RO */
+#define EXT_CSD_FIRMWARE_VERSION 254 /* RO, 8 bytes */
+#define EXT_CSD_NUM_OF_FW_SEC_PROG 302 /* RO, 4 bytes */
+#define EXT_CSD_FFU_ARG 487 /* RO, 4 bytes */
+#define EXT_CSD_OPERATION_CODE_TIMEOUT 491 /* RO */
+#define EXT_CSD_FFU_FEATURES 492 /* RO */
+#define EXT_CSD_SUPPORTED_MODE 493 /* RO */
#define EXT_CSD_TAG_UNIT_SIZE 498 /* RO */
#define EXT_CSD_DATA_TAG_SUPPORT 499 /* RO */
#define EXT_CSD_MAX_PACKED_WRITES 500 /* RO */
--
2.1.0.rc2.206.gedb03e5

Nitpick: You are adding a few EXT_CSD defines that's not required by
this patch. Normally I would prefer to keep things separate, could you
maybe split those pieces?

Kind regards
Uffe
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Gwendal Grignou

2014-10-16 18:09:56 UTC

Permalink

Post by Ulf Hansson

Nitpick: Please prefix patch with "mmc: core:"

card->ext_csd.ffu_capable = ext_csd[EXT_CSD_SUPPORTED_MODE] & 0x1 &&
!(ext_csd[EXT_CSD_FW_CONFIG] & 0x1)

I had a quick look in the eMMC 5.0 spec. I can't find were the above
is stated, or maybe what you are saying is just a consequence of that
there are some pieces missing in the spec? :-)

The only info in the spec I found is:
7.4.26 FIRMWARE_VERSION [261-254]
This field provides the device firmware version.

And that's it. The spec does not specify if it is a string, which
character set is allowed and so on.
I rework the patch to print it is as an hex buffer, without the weird
conversion.

Post by Ulf Hansson
Anyway, I wonder if just should skip the conversion and keep the raw format.

Post by Gwendal Grignou
+ */
+ card->ext_csd.fwrev = be64_to_cpu(fwrev);
+ } else {

The "else" isn't needed, ffu_capable's default value is already false.

cid.fwrev is not an u64.

Nitpick: You are adding a few EXT_CSD defines that's not required by
this patch. Normally I would prefer to keep things separate, could you
maybe split those pieces?
Kind regards
Uffe

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html