[PATCH] mmc: eMMC bus width may not work on all platforms

[PATCH] mmc: eMMC bus width may not work on all platforms

[Philip Rakity]

CMD19 -- The offical way to validate bus widths from the
JEDEC spec does not work on all platforms.  Some platforms
that use PCI/PCIe to connect their SD controllers are known
to fail.

If the quirk MMC_BUS_WIDTH_TEST is not defined we try
to figure out the bus width by reading the ext_csd at different
bus widths and compare this against the ext_csd read in 1 bit
mode.  If no ext_csd is available we default to 1 bit operations.

Code has been tested on mmp2 against 8 bit eMMC and Transcend 2GB
card that is known to not work in 4 bit mode.  The physical
pins on the card are not present to support 4 bit operation.

Signed-off-by: Philip Rakity <prakity@marvell.com>
---
 drivers/mmc/core/mmc.c |  135 ++++++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 125 insertions(+), 10 deletions(-)

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index baab027..42b5045 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -174,14 +174,17 @@ static int mmc_decode_csd(struct mmc_card *card)
 }
 
 /*
- * Read and decode extended CSD.
+ * Read extended CSD.
  */
-static int mmc_read_ext_csd(struct mmc_card *card)
+static int mmc_get_ext_csd(struct mmc_card *card, u8 **new_ext_csd)
 {
 	int err;
 	u8 *ext_csd;
 
 	BUG_ON(!card);
+	BUG_ON(!new_ext_csd);
+
+	*new_ext_csd = NULL;
 
 	if (card->csd.mmca_vsn < CSD_SPEC_VER_4)
 		return 0;
@@ -199,12 +202,15 @@ static int mmc_read_ext_csd(struct mmc_card *card)
 
 	err = mmc_send_ext_csd(card, ext_csd);
 	if (err) {
+		kfree(ext_csd);
+		*new_ext_csd = NULL;
+
 		/* If the host or the card can't do the switch,
 		 * fail more gracefully. */
 		if ((err != -EINVAL)
 		 && (err != -ENOSYS)
 		 && (err != -EFAULT))
-			goto out;
+			return err;
 
 		/*
 		 * High capacity cards should have this "magic" size
@@ -222,9 +228,23 @@ static int mmc_read_ext_csd(struct mmc_card *card)
 				mmc_hostname(card->host));
 			err = 0;
 		}
+	} else
+		*new_ext_csd = ext_csd;
 
-		goto out;
-	}
+	return err;
+}
+
+/*
+ * Decode extended CSD.
+ */
+static int mmc_read_ext_csd(struct mmc_card *card, u8 *ext_csd)
+{
+	int err = 0;
+
+	BUG_ON(!card);
+
+	if (!ext_csd)
+		return 0;
 
 	/* Version is coded in the CSD_STRUCTURE byte in the EXT_CSD register */
 	if (card->csd.structure == 3) {
@@ -372,8 +392,92 @@ static int mmc_read_ext_csd(struct mmc_card *card)
 		card->erased_byte = 0x0;
 
 out:
+	return err;
+}
+
+static inline void mmc_free_ext_csd(u8 *ext_csd)
+{
 	kfree(ext_csd);
+}
+
+static int mmc_cmp_ext_csd(u8 *ext_csd, u8 *bw_ext_csd, unsigned bus_width)
+{
+	if (ext_csd == NULL || bw_ext_csd == NULL)
+		return bus_width != MMC_BUS_WIDTH_1;
+
+	if (bus_width == MMC_BUS_WIDTH_1)
+		return 0;
+
+	/* only compare read only fields */
 
+	if (ext_csd[EXT_CSD_PARTITION_SUPPORT] !=
+		bw_ext_csd[EXT_CSD_PARTITION_SUPPORT])
+			return -1;
+
+	if (ext_csd[EXT_CSD_ERASED_MEM_CONT] !=
+		bw_ext_csd[EXT_CSD_ERASED_MEM_CONT])
+			return -2;
+
+	if (ext_csd[EXT_CSD_REV] !=
+		bw_ext_csd[EXT_CSD_REV])
+			return -3;
+
+	if (ext_csd[EXT_CSD_STRUCTURE] !=
+		bw_ext_csd[EXT_CSD_STRUCTURE])
+			return -4;
+
+	if (ext_csd[EXT_CSD_CARD_TYPE] !=
+		bw_ext_csd[EXT_CSD_CARD_TYPE])
+			return -5;
+
+	if (ext_csd[EXT_CSD_S_A_TIMEOUT] !=
+		bw_ext_csd[EXT_CSD_S_A_TIMEOUT])
+			return -6;
+
+	if (ext_csd[EXT_CSD_HC_WP_GRP_SIZE] !=
+		bw_ext_csd[EXT_CSD_HC_WP_GRP_SIZE])
+			return -7;
+
+	if (ext_csd[EXT_CSD_ERASE_TIMEOUT_MULT] !=
+		bw_ext_csd[EXT_CSD_ERASE_TIMEOUT_MULT])
+			return -8;
+
+	if (ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE] !=
+		bw_ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE])
+			return -9;
+
+	if (ext_csd[EXT_CSD_SEC_TRIM_MULT] !=
+		bw_ext_csd[EXT_CSD_SEC_TRIM_MULT])
+			return -10;
+
+	if (ext_csd[EXT_CSD_SEC_ERASE_MULT] !=
+		bw_ext_csd[EXT_CSD_SEC_ERASE_MULT])
+			return -11;
+
+	if (ext_csd[EXT_CSD_SEC_FEATURE_SUPPORT] !=
+		bw_ext_csd[EXT_CSD_SEC_FEATURE_SUPPORT])
+			return -12;
+
+	if (ext_csd[EXT_CSD_TRIM_MULT] !=
+		bw_ext_csd[EXT_CSD_TRIM_MULT])
+			return -13;
+
+	return memcmp(&ext_csd[EXT_CSD_SEC_CNT],
+			&bw_ext_csd[EXT_CSD_SEC_CNT],
+			4);
+}
+
+static int mmc_compare_ext_csds(struct mmc_card *card, u8 *ext_csd,
+			unsigned bus_width)
+{
+	u8 *bw_ext_csd;
+	int err;
+
+	err = mmc_get_ext_csd(card, &bw_ext_csd);
+	if (!err)
+		err = mmc_cmp_ext_csd(ext_csd, bw_ext_csd, bus_width);
+
+	mmc_free_ext_csd(bw_ext_csd);
 	return err;
 }
 
@@ -438,6 +542,7 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
 	u32 cid[4];
 	unsigned int max_dtr;
 	u32 rocr;
+	u8 *ext_csd = NULL;
 
 	BUG_ON(!host);
 	WARN_ON(!host->claimed);
@@ -536,7 +641,11 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
 		/*
 		 * Fetch and process extended CSD.
 		 */
-		err = mmc_read_ext_csd(card);
+
+		err = mmc_get_ext_csd(card, &ext_csd);
+		if (err)
+			goto free_card;
+		err = mmc_read_ext_csd(card, ext_csd);
 		if (err)
 			goto free_card;
 
@@ -676,14 +785,18 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
 					 0);
 			if (!err) {
 				mmc_set_bus_width(card->host, bus_width);
+
 				/*
 				 * If controller can't handle bus width test,
-				 * use the highest bus width to maintain
-				 * compatibility with previous MMC behavior.
+				 * compare ext_csd previously read in 1 bit mode
+				 * against ext_csd at new bus width
 				 */
 				if (!(host->caps & MMC_CAP_BUS_WIDTH_TEST))
-					break;
-				err = mmc_bus_test(card, bus_width);
+					err = mmc_compare_ext_csds(card,
+						ext_csd,
+						bus_width);
+				else
+					err = mmc_bus_test(card, bus_width);
 				if (!err)
 					break;
 			}
@@ -730,12 +843,14 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
 	if (!oldcard)
 		host->card = card;
 
+	mmc_free_ext_csd(ext_csd);
 	return 0;
 
 free_card:
 	if (!oldcard)
 		mmc_remove_card(card);
 err:
+	mmc_free_ext_csd(ext_csd);
 
 	return err;
 }
-- 
1.7.0.4

RE: [PATCH] mmc: eMMC bus width may not work on all platforms

[Mok, Tze Siong]

Hi Chris,

The following patch https://patchwork.kernel.org/patch/792162/ is tested using Intel EG20T PCH, the Transcend MMC 1bit card can now be detected, read and write to the card successfully. 
Note : Need to add MMC_CAP_BUS_WIDTH_TEST caps into the SD host controller HW platform code in order to work.

Tested-by: tze.siong.mok@intel.com


-----Original Message-----
From: Philip Rakity [mailto:prakity@marvell.com] 
Sent: Wednesday, May 18, 2011 2:41 AM
To: linux-mmc@vger.kernel.org
Cc: Chris Ball; Mok, Tze Siong; Tomoya MORINAGA; Takashi Iwai; Mark Brown
Subject: [PATCH] mmc: eMMC bus width may not work on all platforms

CMD19 -- The offical way to validate bus widths from the
JEDEC spec does not work on all platforms.  Some platforms
that use PCI/PCIe to connect their SD controllers are known
to fail.

If the quirk MMC_BUS_WIDTH_TEST is not defined we try
to figure out the bus width by reading the ext_csd at different
bus widths and compare this against the ext_csd read in 1 bit
mode.  If no ext_csd is available we default to 1 bit operations.

Code has been tested on mmp2 against 8 bit eMMC and Transcend 2GB
card that is known to not work in 4 bit mode.  The physical
pins on the card are not present to support 4 bit operation.

Signed-off-by: Philip Rakity <prakity@marvell.com>
---
 drivers/mmc/core/mmc.c |  135 ++++++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 125 insertions(+), 10 deletions(-)

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index baab027..42b5045 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -174,14 +174,17 @@ static int mmc_decode_csd(struct mmc_card *card)
 }
 
 /*
- * Read and decode extended CSD.
+ * Read extended CSD.
  */
-static int mmc_read_ext_csd(struct mmc_card *card)
+static int mmc_get_ext_csd(struct mmc_card *card, u8 **new_ext_csd)
 {
 	int err;
 	u8 *ext_csd;
 
 	BUG_ON(!card);
+	BUG_ON(!new_ext_csd);
+
+	*new_ext_csd = NULL;
 
 	if (card->csd.mmca_vsn < CSD_SPEC_VER_4)
 		return 0;
@@ -199,12 +202,15 @@ static int mmc_read_ext_csd(struct mmc_card *card)
 
 	err = mmc_send_ext_csd(card, ext_csd);
 	if (err) {
+		kfree(ext_csd);
+		*new_ext_csd = NULL;
+
 		/* If the host or the card can't do the switch,
 		 * fail more gracefully. */
 		if ((err != -EINVAL)
 		 && (err != -ENOSYS)
 		 && (err != -EFAULT))
-			goto out;
+			return err;
 
 		/*
 		 * High capacity cards should have this "magic" size
@@ -222,9 +228,23 @@ static int mmc_read_ext_csd(struct mmc_card *card)
 				mmc_hostname(card->host));
 			err = 0;
 		}
+	} else
+		*new_ext_csd = ext_csd;
 
-		goto out;
-	}
+	return err;
+}
+
+/*
+ * Decode extended CSD.
+ */
+static int mmc_read_ext_csd(struct mmc_card *card, u8 *ext_csd)
+{
+	int err = 0;
+
+	BUG_ON(!card);
+
+	if (!ext_csd)
+		return 0;
 
 	/* Version is coded in the CSD_STRUCTURE byte in the EXT_CSD register */
 	if (card->csd.structure == 3) {
@@ -372,8 +392,92 @@ static int mmc_read_ext_csd(struct mmc_card *card)
 		card->erased_byte = 0x0;
 
 out:
+	return err;
+}
+
+static inline void mmc_free_ext_csd(u8 *ext_csd)
+{
 	kfree(ext_csd);
+}
+
+static int mmc_cmp_ext_csd(u8 *ext_csd, u8 *bw_ext_csd, unsigned bus_width)
+{
+	if (ext_csd == NULL || bw_ext_csd == NULL)
+		return bus_width != MMC_BUS_WIDTH_1;
+
+	if (bus_width == MMC_BUS_WIDTH_1)
+		return 0;
+
+	/* only compare read only fields */
 
+	if (ext_csd[EXT_CSD_PARTITION_SUPPORT] !=
+		bw_ext_csd[EXT_CSD_PARTITION_SUPPORT])
+			return -1;
+
+	if (ext_csd[EXT_CSD_ERASED_MEM_CONT] !=
+		bw_ext_csd[EXT_CSD_ERASED_MEM_CONT])
+			return -2;
+
+	if (ext_csd[EXT_CSD_REV] !=
+		bw_ext_csd[EXT_CSD_REV])
+			return -3;
+
+	if (ext_csd[EXT_CSD_STRUCTURE] !=
+		bw_ext_csd[EXT_CSD_STRUCTURE])
+			return -4;
+
+	if (ext_csd[EXT_CSD_CARD_TYPE] !=
+		bw_ext_csd[EXT_CSD_CARD_TYPE])
+			return -5;
+
+	if (ext_csd[EXT_CSD_S_A_TIMEOUT] !=
+		bw_ext_csd[EXT_CSD_S_A_TIMEOUT])
+			return -6;
+
+	if (ext_csd[EXT_CSD_HC_WP_GRP_SIZE] !=
+		bw_ext_csd[EXT_CSD_HC_WP_GRP_SIZE])
+			return -7;
+
+	if (ext_csd[EXT_CSD_ERASE_TIMEOUT_MULT] !=
+		bw_ext_csd[EXT_CSD_ERASE_TIMEOUT_MULT])
+			return -8;
+
+	if (ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE] !=
+		bw_ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE])
+			return -9;
+
+	if (ext_csd[EXT_CSD_SEC_TRIM_MULT] !=
+		bw_ext_csd[EXT_CSD_SEC_TRIM_MULT])
+			return -10;
+
+	if (ext_csd[EXT_CSD_SEC_ERASE_MULT] !=
+		bw_ext_csd[EXT_CSD_SEC_ERASE_MULT])
+			return -11;
+
+	if (ext_csd[EXT_CSD_SEC_FEATURE_SUPPORT] !=
+		bw_ext_csd[EXT_CSD_SEC_FEATURE_SUPPORT])
+			return -12;
+
+	if (ext_csd[EXT_CSD_TRIM_MULT] !=
+		bw_ext_csd[EXT_CSD_TRIM_MULT])
+			return -13;
+
+	return memcmp(&ext_csd[EXT_CSD_SEC_CNT],
+			&bw_ext_csd[EXT_CSD_SEC_CNT],
+			4);
+}
+
+static int mmc_compare_ext_csds(struct mmc_card *card, u8 *ext_csd,
+			unsigned bus_width)
+{
+	u8 *bw_ext_csd;
+	int err;
+
+	err = mmc_get_ext_csd(card, &bw_ext_csd);
+	if (!err)
+		err = mmc_cmp_ext_csd(ext_csd, bw_ext_csd, bus_width);
+
+	mmc_free_ext_csd(bw_ext_csd);
 	return err;
 }
 
@@ -438,6 +542,7 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
 	u32 cid[4];
 	unsigned int max_dtr;
 	u32 rocr;
+	u8 *ext_csd = NULL;
 
 	BUG_ON(!host);
 	WARN_ON(!host->claimed);
@@ -536,7 +641,11 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
 		/*
 		 * Fetch and process extended CSD.
 		 */
-		err = mmc_read_ext_csd(card);
+
+		err = mmc_get_ext_csd(card, &ext_csd);
+		if (err)
+			goto free_card;
+		err = mmc_read_ext_csd(card, ext_csd);
 		if (err)
 			goto free_card;
 
@@ -676,14 +785,18 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
 					 0);
 			if (!err) {
 				mmc_set_bus_width(card->host, bus_width);
+
 				/*
 				 * If controller can't handle bus width test,
-				 * use the highest bus width to maintain
-				 * compatibility with previous MMC behavior.
+				 * compare ext_csd previously read in 1 bit mode
+				 * against ext_csd at new bus width
 				 */
 				if (!(host->caps & MMC_CAP_BUS_WIDTH_TEST))
-					break;
-				err = mmc_bus_test(card, bus_width);
+					err = mmc_compare_ext_csds(card,
+						ext_csd,
+						bus_width);
+				else
+					err = mmc_bus_test(card, bus_width);
 				if (!err)
 					break;
 			}
@@ -730,12 +843,14 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
 	if (!oldcard)
 		host->card = card;
 
+	mmc_free_ext_csd(ext_csd);
 	return 0;
 
 free_card:
 	if (!oldcard)
 		mmc_remove_card(card);
 err:
+	mmc_free_ext_csd(ext_csd);
 
 	return err;
 }
-- 
1.7.0.4

Re: [PATCH] mmc: eMMC bus width may not work on all platforms

[Chris Ball]

Hi,

On Sat, May 21 2011, Mok, Tze Siong wrote:
> The following patch https://patchwork.kernel.org/patch/792162/ is tested using Intel EG20T PCH, the Transcend MMC 1bit card can now be detected, read and write to the card successfully. 
> Note : Need to add MMC_CAP_BUS_WIDTH_TEST caps into the SD host controller HW platform code in order to work.
>
> Tested-by: tze.siong.mok@intel.com

Great, thank you.  Philip, a few comments:

> +static int mmc_cmp_ext_csd(u8 *ext_csd, u8 *bw_ext_csd, unsigned bus_width)
> +{
> +	if (ext_csd == NULL || bw_ext_csd == NULL)
> +		return bus_width != MMC_BUS_WIDTH_1;
> +
> +	if (bus_width == MMC_BUS_WIDTH_1)
> +		return 0;
> +
> +	/* only compare read only fields */
>  
> +	if (ext_csd[EXT_CSD_PARTITION_SUPPORT] !=
> +		bw_ext_csd[EXT_CSD_PARTITION_SUPPORT])
> +			return -1;
> +
> +	if (ext_csd[EXT_CSD_ERASED_MEM_CONT] !=
> +		bw_ext_csd[EXT_CSD_ERASED_MEM_CONT])
> +			return -2;
> +
> +	if (ext_csd[EXT_CSD_REV] !=
> +		bw_ext_csd[EXT_CSD_REV])
> +			return -3;
> +
> +	if (ext_csd[EXT_CSD_STRUCTURE] !=
> +		bw_ext_csd[EXT_CSD_STRUCTURE])
> +			return -4;
> +
> +	if (ext_csd[EXT_CSD_CARD_TYPE] !=
> +		bw_ext_csd[EXT_CSD_CARD_TYPE])
> +			return -5;
> +
> +	if (ext_csd[EXT_CSD_S_A_TIMEOUT] !=
> +		bw_ext_csd[EXT_CSD_S_A_TIMEOUT])
> +			return -6;
> +
> +	if (ext_csd[EXT_CSD_HC_WP_GRP_SIZE] !=
> +		bw_ext_csd[EXT_CSD_HC_WP_GRP_SIZE])
> +			return -7;
> +
> +	if (ext_csd[EXT_CSD_ERASE_TIMEOUT_MULT] !=
> +		bw_ext_csd[EXT_CSD_ERASE_TIMEOUT_MULT])
> +			return -8;
> +
> +	if (ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE] !=
> +		bw_ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE])
> +			return -9;
> +
> +	if (ext_csd[EXT_CSD_SEC_TRIM_MULT] !=
> +		bw_ext_csd[EXT_CSD_SEC_TRIM_MULT])
> +			return -10;
> +
> +	if (ext_csd[EXT_CSD_SEC_ERASE_MULT] !=
> +		bw_ext_csd[EXT_CSD_SEC_ERASE_MULT])
> +			return -11;
> +
> +	if (ext_csd[EXT_CSD_SEC_FEATURE_SUPPORT] !=
> +		bw_ext_csd[EXT_CSD_SEC_FEATURE_SUPPORT])
> +			return -12;
> +
> +	if (ext_csd[EXT_CSD_TRIM_MULT] !=
> +		bw_ext_csd[EXT_CSD_TRIM_MULT])
> +			return -13;

Hm, I think people reading dmesg are going to interpret these as errnos,
which they're ambiguous with.  Is returning a different number for each
condition important?

Perhaps just pick one errno to return, have a single long conditional,
and if we're going to fail all of mmc_init_card() because of an error
here, add a printk explaining the situation to this function?

> +
> +	return memcmp(&ext_csd[EXT_CSD_SEC_CNT],
> +			&bw_ext_csd[EXT_CSD_SEC_CNT],
> +			4);
> +}
> +
> +static int mmc_compare_ext_csds(struct mmc_card *card, u8 *ext_csd,
> +			unsigned bus_width)
> +{
> +	u8 *bw_ext_csd;
> +	int err;
> +
> +	err = mmc_get_ext_csd(card, &bw_ext_csd);
> +	if (!err)
> +		err = mmc_cmp_ext_csd(ext_csd, bw_ext_csd, bus_width);
> +
> +	mmc_free_ext_csd(bw_ext_csd);
>  	return err;
>  }

mmc_compare_ext_csds() and mmc_cmp_ext_csd() don't seem like they have
a strong reason for existing as separate functions -- perhaps collapse
them both into a single mmc_compare_ext_csds()?

Thanks!

- Chris.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child

Re: [PATCH] mmc: eMMC bus width may not work on all platforms

[Philip Rakity]

On May 21, 2011, at 6:20 PM, Chris Ball wrote:

> Hi,
> 
> On Sat, May 21 2011, Mok, Tze Siong wrote:
>> The following patch https://patchwork.kernel.org/patch/792162/ is tested using Intel EG20T PCH, the Transcend MMC 1bit card can now be detected, read and write to the card successfully. 
>> Note : Need to add MMC_CAP_BUS_WIDTH_TEST caps into the SD host controller HW platform code in order to work.
>> 
>> Tested-by: tze.siong.mok@intel.com
> 
> Great, thank you.  Philip, a few comments:
> 
>> +static int mmc_cmp_ext_csd(u8 *ext_csd, u8 *bw_ext_csd, unsigned bus_width)
>> +{
>> +	if (ext_csd == NULL || bw_ext_csd == NULL)
>> +		return bus_width != MMC_BUS_WIDTH_1;
>> +
>> +	if (bus_width == MMC_BUS_WIDTH_1)
>> +		return 0;
>> +
>> +	/* only compare read only fields */
>> 
>> +	if (ext_csd[EXT_CSD_PARTITION_SUPPORT] !=
>> +		bw_ext_csd[EXT_CSD_PARTITION_SUPPORT])
>> +			return -1;
>> +
>> +	if (ext_csd[EXT_CSD_ERASED_MEM_CONT] !=
>> +		bw_ext_csd[EXT_CSD_ERASED_MEM_CONT])
>> +			return -2;
>> +
>> +	if (ext_csd[EXT_CSD_REV] !=
>> +		bw_ext_csd[EXT_CSD_REV])
>> +			return -3;
>> +
>> +	if (ext_csd[EXT_CSD_STRUCTURE] !=
>> +		bw_ext_csd[EXT_CSD_STRUCTURE])
>> +			return -4;
>> +
>> +	if (ext_csd[EXT_CSD_CARD_TYPE] !=
>> +		bw_ext_csd[EXT_CSD_CARD_TYPE])
>> +			return -5;
>> +
>> +	if (ext_csd[EXT_CSD_S_A_TIMEOUT] !=
>> +		bw_ext_csd[EXT_CSD_S_A_TIMEOUT])
>> +			return -6;
>> +
>> +	if (ext_csd[EXT_CSD_HC_WP_GRP_SIZE] !=
>> +		bw_ext_csd[EXT_CSD_HC_WP_GRP_SIZE])
>> +			return -7;
>> +
>> +	if (ext_csd[EXT_CSD_ERASE_TIMEOUT_MULT] !=
>> +		bw_ext_csd[EXT_CSD_ERASE_TIMEOUT_MULT])
>> +			return -8;
>> +
>> +	if (ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE] !=
>> +		bw_ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE])
>> +			return -9;
>> +
>> +	if (ext_csd[EXT_CSD_SEC_TRIM_MULT] !=
>> +		bw_ext_csd[EXT_CSD_SEC_TRIM_MULT])
>> +			return -10;
>> +
>> +	if (ext_csd[EXT_CSD_SEC_ERASE_MULT] !=
>> +		bw_ext_csd[EXT_CSD_SEC_ERASE_MULT])
>> +			return -11;
>> +
>> +	if (ext_csd[EXT_CSD_SEC_FEATURE_SUPPORT] !=
>> +		bw_ext_csd[EXT_CSD_SEC_FEATURE_SUPPORT])
>> +			return -12;
>> +
>> +	if (ext_csd[EXT_CSD_TRIM_MULT] !=
>> +		bw_ext_csd[EXT_CSD_TRIM_MULT])
>> +			return -13;
> 
> Hm, I think people reading dmesg are going to interpret these as errnos,
> which they're ambiguous with.  Is returning a different number for each
> condition important?
> 

No -- was just a way for me to debug -- not important

> Perhaps just pick one errno to return, have a single long conditional,
> and if we're going to fail all of mmc_init_card() because of an error
> here, add a printk explaining the situation to this function?

which errro would you suggest. 

 I do not want to have a long conditional unless
this is very important.  Hard to follow.  (see below)

> 
>> +
>> +	return memcmp(&ext_csd[EXT_CSD_SEC_CNT],
>> +			&bw_ext_csd[EXT_CSD_SEC_CNT],
>> +			4);
>> +}
>> +
>> +static int mmc_compare_ext_csds(struct mmc_card *card, u8 *ext_csd,
>> +			unsigned bus_width)
>> +{
>> +	u8 *bw_ext_csd;
>> +	int err;
>> +
>> +	err = mmc_get_ext_csd(card, &bw_ext_csd);
>> +	if (!err)
>> +		err = mmc_cmp_ext_csd(ext_csd, bw_ext_csd, bus_width);

would it be better to add
		if (err)
			err = ERRNO WE WANT TO USE;

>> +
>> +	mmc_free_ext_csd(bw_ext_csd);
>> 	return err;
>> }
> 
> mmc_compare_ext_csds() and mmc_cmp_ext_csd() don't seem like they have
> a strong reason for existing as separate functions -- perhaps collapse
> them both into a single mmc_compare_ext_csds()?

I can do this.

> 
> Thanks!
> 
> - Chris.
> -- 
> Chris Ball   <cjb@laptop.org>   <http://printf.net/>
> One Laptop Per Child
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH V2] mmc: eMMC bus width may not work on all platforms

[Philip Rakity]

Note:  I cannot test test for a few days.  Following Chris suggestion posting to list.

version 2
---
return -EINVAL when compare of ext_csd_fails
combine two compare routines into one per suggestion
from Chris.

---
version 1

CMD19 -- The offical way to validate bus widths from the
JEDEC spec does not work on all platforms.  Some platforms
that use PCI/PCIe to connect their SD controllers are known
to fail.

If the quirk MMC_BUS_WIDTH_TEST is not defined we try
to figure out the bus width by reading the ext_csd at different
bus widths and compare this against the ext_csd read in 1 bit
mode.  If no ext_csd is available we default to 1 bit operations.

Code has been tested on mmp2 against 8 bit eMMC and Transcend 2GB
card that is known to not work in 4 bit mode.  The physical
pins on the card are not present to support 4 bit operation.

Signed-off-by: Philip Rakity <prakity@marvell.com>
---
 drivers/mmc/core/mmc.c |  113 +++++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 103 insertions(+), 10 deletions(-)

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index baab027..6db91d6 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -174,14 +174,17 @@ static int mmc_decode_csd(struct mmc_card *card)
 }
 
 /*
- * Read and decode extended CSD.
+ * Read extended CSD.
  */
-static int mmc_read_ext_csd(struct mmc_card *card)
+static int mmc_get_ext_csd(struct mmc_card *card, u8 **new_ext_csd)
 {
 	int err;
 	u8 *ext_csd;
 
 	BUG_ON(!card);
+	BUG_ON(!new_ext_csd);
+
+	*new_ext_csd = NULL;
 
 	if (card->csd.mmca_vsn < CSD_SPEC_VER_4)
 		return 0;
@@ -199,12 +202,15 @@ static int mmc_read_ext_csd(struct mmc_card *card)
 
 	err = mmc_send_ext_csd(card, ext_csd);
 	if (err) {
+		kfree(ext_csd);
+		*new_ext_csd = NULL;
+
 		/* If the host or the card can't do the switch,
 		 * fail more gracefully. */
 		if ((err != -EINVAL)
 		 && (err != -ENOSYS)
 		 && (err != -EFAULT))
-			goto out;
+			return err;
 
 		/*
 		 * High capacity cards should have this "magic" size
@@ -222,9 +228,23 @@ static int mmc_read_ext_csd(struct mmc_card *card)
 				mmc_hostname(card->host));
 			err = 0;
 		}
+	} else
+		*new_ext_csd = ext_csd;
 
-		goto out;
-	}
+	return err;
+}
+
+/*
+ * Decode extended CSD.
+ */
+static int mmc_read_ext_csd(struct mmc_card *card, u8 *ext_csd)
+{
+	int err = 0;
+
+	BUG_ON(!card);
+
+	if (!ext_csd)
+		return 0;
 
 	/* Version is coded in the CSD_STRUCTURE byte in the EXT_CSD register */
 	if (card->csd.structure == 3) {
@@ -372,8 +392,70 @@ static int mmc_read_ext_csd(struct mmc_card *card)
 		card->erased_byte = 0x0;
 
 out:
+	return err;
+}
+
+static inline void mmc_free_ext_csd(u8 *ext_csd)
+{
 	kfree(ext_csd);
+}
+
 
+static int mmc_compare_ext_csds(struct mmc_card *card, u8 *ext_csd,
+			unsigned bus_width)
+{
+	u8 *bw_ext_csd;
+	int err;
+
+	err = mmc_get_ext_csd(card, &bw_ext_csd);
+	if (err)
+		return err;
+
+	if ((ext_csd == NULL || bw_ext_csd == NULL)) {
+		if (bus_width != MMC_BUS_WIDTH_1)
+			err = -EINVAL;
+		goto out;
+	}
+
+	if (bus_width == MMC_BUS_WIDTH_1)
+		goto out;
+
+	/* only compare read only fields */
+
+	err =	(!(ext_csd[EXT_CSD_PARTITION_SUPPORT] ==
+			bw_ext_csd[EXT_CSD_PARTITION_SUPPORT]) &&
+		(ext_csd[EXT_CSD_ERASED_MEM_CONT] ==
+			bw_ext_csd[EXT_CSD_ERASED_MEM_CONT]) &&
+		(ext_csd[EXT_CSD_REV] ==
+			bw_ext_csd[EXT_CSD_REV]) &&
+		(ext_csd[EXT_CSD_STRUCTURE] ==
+			bw_ext_csd[EXT_CSD_STRUCTURE]) &&
+		(ext_csd[EXT_CSD_CARD_TYPE] ==
+			bw_ext_csd[EXT_CSD_CARD_TYPE]) &&
+		(ext_csd[EXT_CSD_S_A_TIMEOUT] ==
+			bw_ext_csd[EXT_CSD_S_A_TIMEOUT]) &&
+		(ext_csd[EXT_CSD_HC_WP_GRP_SIZE] ==
+			bw_ext_csd[EXT_CSD_HC_WP_GRP_SIZE]) &&
+		(ext_csd[EXT_CSD_ERASE_TIMEOUT_MULT] ==
+			bw_ext_csd[EXT_CSD_ERASE_TIMEOUT_MULT]) &&
+		(ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE] ==
+			bw_ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE]) &&
+		(ext_csd[EXT_CSD_SEC_TRIM_MULT] ==
+			bw_ext_csd[EXT_CSD_SEC_TRIM_MULT]) &&
+		(ext_csd[EXT_CSD_SEC_ERASE_MULT] ==
+			bw_ext_csd[EXT_CSD_SEC_ERASE_MULT]) &&
+		(ext_csd[EXT_CSD_SEC_FEATURE_SUPPORT] ==
+			bw_ext_csd[EXT_CSD_SEC_FEATURE_SUPPORT]) &&
+		(ext_csd[EXT_CSD_TRIM_MULT] ==
+			bw_ext_csd[EXT_CSD_TRIM_MULT]) &&
+		memcmp(&ext_csd[EXT_CSD_SEC_CNT],
+			&bw_ext_csd[EXT_CSD_SEC_CNT],
+			4) != 0);
+	if (err)
+		err = -EINVAL;
+
+out:
+	mmc_free_ext_csd(bw_ext_csd);
 	return err;
 }

Re: [PATCH V2] mmc: eMMC bus width may not work on all platforms

[Chris Ball]

Hi Philip,

On Sun, May 22 2011, Philip Rakity wrote:
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index baab027..6db91d6 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -174,14 +174,17 @@ static int mmc_decode_csd(struct mmc_card *card)
>  }
>  
>  /*
> - * Read and decode extended CSD.
> + * Read extended CSD.
>   */
> -static int mmc_read_ext_csd(struct mmc_card *card)
> +static int mmc_read_ext_csd(struct mmc_card *card, u8 *ext_csd)

This one doesn't work -- you're missing the half of the patch that hooks
up the rest of mmc.c to the new code, which was in v1.  As-is, we get:

drivers/mmc/core/mmc.c: In function ‘mmc_init_card’:
drivers/mmc/core/mmc.c:621:3: error: too few arguments to function ‘mmc_read_ext_csd’
drivers/mmc/core/mmc.c:240:12: note: declared here
drivers/mmc/core/mmc.c: At top level:
drivers/mmc/core/mmc.c:404:12: warning: ‘mmc_compare_ext_csds’ defined but not used [-Wunused-function]

- Chris.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child

Re: [PATCH V2] mmc: eMMC bus width may not work on all platforms

[Philip Rakity]

resend 

Note:  I cannot test test for a few days.  Following Chris
suggestion posting to list.

version 2
---
return -EINVAL when compare of ext_csd_fails
combine two compare routines into one per suggestion
from Chris.

---
version 1

CMD19 -- The offical way to validate bus widths from the
JEDEC spec does not work on all platforms.  Some platforms
that use PCI/PCIe to connect their SD controllers are known
to fail.

If the quirk MMC_BUS_WIDTH_TEST is not defined we try
to figure out the bus width by reading the ext_csd at different
bus widths and compare this against the ext_csd read in 1 bit
mode.  If no ext_csd is available we default to 1 bit operations.

Code has been tested on mmp2 against 8 bit eMMC and Transcend 2GB
card that is known to not work in 4 bit mode.  The physical
pins on the card are not present to support 4 bit operation.

Signed-off-by: Philip Rakity <prakity@marvell.com>
---
 drivers/mmc/core/mmc.c |  113 +++++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 103 insertions(+), 10 deletions(-)

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index baab027..6db91d6 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -174,14 +174,17 @@ static int mmc_decode_csd(struct mmc_card *card)
 }
 
 /*
- * Read and decode extended CSD.
+ * Read extended CSD.
  */
-static int mmc_read_ext_csd(struct mmc_card *card)
+static int mmc_get_ext_csd(struct mmc_card *card, u8 **new_ext_csd)
 {
 	int err;
 	u8 *ext_csd;
 
 	BUG_ON(!card);
+	BUG_ON(!new_ext_csd);
+
+	*new_ext_csd = NULL;
 
 	if (card->csd.mmca_vsn < CSD_SPEC_VER_4)
 		return 0;
@@ -199,12 +202,15 @@ static int mmc_read_ext_csd(struct mmc_card *card)
 
 	err = mmc_send_ext_csd(card, ext_csd);
 	if (err) {
+		kfree(ext_csd);
+		*new_ext_csd = NULL;
+
 		/* If the host or the card can't do the switch,
 		 * fail more gracefully. */
 		if ((err != -EINVAL)
 		 && (err != -ENOSYS)
 		 && (err != -EFAULT))
-			goto out;
+			return err;
 
 		/*
 		 * High capacity cards should have this "magic" size
@@ -222,9 +228,23 @@ static int mmc_read_ext_csd(struct mmc_card *card)
 				mmc_hostname(card->host));
 			err = 0;
 		}
+	} else
+		*new_ext_csd = ext_csd;
 
-		goto out;
-	}
+	return err;
+}
+
+/*
+ * Decode extended CSD.
+ */
+static int mmc_read_ext_csd(struct mmc_card *card, u8 *ext_csd)
+{
+	int err = 0;
+
+	BUG_ON(!card);
+
+	if (!ext_csd)
+		return 0;
 
 	/* Version is coded in the CSD_STRUCTURE byte in the EXT_CSD register */
 	if (card->csd.structure == 3) {
@@ -372,8 +392,70 @@ static int mmc_read_ext_csd(struct mmc_card *card)
 		card->erased_byte = 0x0;
 
 out:
+	return err;
+}
+
+static inline void mmc_free_ext_csd(u8 *ext_csd)
+{
 	kfree(ext_csd);
+}
+
 
+static int mmc_compare_ext_csds(struct mmc_card *card, u8 *ext_csd,
+			unsigned bus_width)
+{
+	u8 *bw_ext_csd;
+	int err;
+
+	err = mmc_get_ext_csd(card, &bw_ext_csd);
+	if (err)
+		return err;
+
+	if ((ext_csd == NULL || bw_ext_csd == NULL)) {
+		if (bus_width != MMC_BUS_WIDTH_1)
+			err = -EINVAL;
+		goto out;
+	}
+
+	if (bus_width == MMC_BUS_WIDTH_1)
+		goto out;
+
+	/* only compare read only fields */
+
+	err =	(!(ext_csd[EXT_CSD_PARTITION_SUPPORT] ==
+			bw_ext_csd[EXT_CSD_PARTITION_SUPPORT]) &&
+		(ext_csd[EXT_CSD_ERASED_MEM_CONT] ==
+			bw_ext_csd[EXT_CSD_ERASED_MEM_CONT]) &&
+		(ext_csd[EXT_CSD_REV] ==
+			bw_ext_csd[EXT_CSD_REV]) &&
+		(ext_csd[EXT_CSD_STRUCTURE] ==
+			bw_ext_csd[EXT_CSD_STRUCTURE]) &&
+		(ext_csd[EXT_CSD_CARD_TYPE] ==
+			bw_ext_csd[EXT_CSD_CARD_TYPE]) &&
+		(ext_csd[EXT_CSD_S_A_TIMEOUT] ==
+			bw_ext_csd[EXT_CSD_S_A_TIMEOUT]) &&
+		(ext_csd[EXT_CSD_HC_WP_GRP_SIZE] ==
+			bw_ext_csd[EXT_CSD_HC_WP_GRP_SIZE]) &&
+		(ext_csd[EXT_CSD_ERASE_TIMEOUT_MULT] ==
+			bw_ext_csd[EXT_CSD_ERASE_TIMEOUT_MULT]) &&
+		(ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE] ==
+			bw_ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE]) &&
+		(ext_csd[EXT_CSD_SEC_TRIM_MULT] ==
+			bw_ext_csd[EXT_CSD_SEC_TRIM_MULT]) &&
+		(ext_csd[EXT_CSD_SEC_ERASE_MULT] ==
+			bw_ext_csd[EXT_CSD_SEC_ERASE_MULT]) &&
+		(ext_csd[EXT_CSD_SEC_FEATURE_SUPPORT] ==
+			bw_ext_csd[EXT_CSD_SEC_FEATURE_SUPPORT]) &&
+		(ext_csd[EXT_CSD_TRIM_MULT] ==
+			bw_ext_csd[EXT_CSD_TRIM_MULT]) &&
+		memcmp(&ext_csd[EXT_CSD_SEC_CNT],
+			&bw_ext_csd[EXT_CSD_SEC_CNT],
+			4) != 0);
+	if (err)
+		err = -EINVAL;
+
+out:
+	mmc_free_ext_csd(bw_ext_csd);
 	return err;
 }
 
@@ -438,6 +520,7 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
 	u32 cid[4];
 	unsigned int max_dtr;
 	u32 rocr;
+	u8 *ext_csd = NULL;
 
 	BUG_ON(!host);
 	WARN_ON(!host->claimed);
@@ -536,7 +619,11 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
 		/*
 		 * Fetch and process extended CSD.
 		 */
-		err = mmc_read_ext_csd(card);
+
+		err = mmc_get_ext_csd(card, &ext_csd);
+		if (err)
+			goto free_card;
+		err = mmc_read_ext_csd(card, ext_csd);
 		if (err)
 			goto free_card;
 
@@ -676,14 +763,18 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
 					 0);
 			if (!err) {
 				mmc_set_bus_width(card->host, bus_width);
+
 				/*
 				 * If controller can't handle bus width test,
-				 * use the highest bus width to maintain
-				 * compatibility with previous MMC behavior.
+				 * compare ext_csd previously read in 1 bit mode
+				 * against ext_csd at new bus width
 				 */
 				if (!(host->caps & MMC_CAP_BUS_WIDTH_TEST))
-					break;
-				err = mmc_bus_test(card, bus_width);
+					err = mmc_compare_ext_csds(card,
+						ext_csd,
+						bus_width);
+				else
+					err = mmc_bus_test(card, bus_width);
 				if (!err)
 					break;
 			}
@@ -730,12 +821,14 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
 	if (!oldcard)
 		host->card = card;
 
+	mmc_free_ext_csd(ext_csd);
 	return 0;
 
 free_card:
 	if (!oldcard)
 		mmc_remove_card(card);
 err:
+	mmc_free_ext_csd(ext_csd);
 
 	return err;
 }
-- 
1.7.0.4



On May 23, 2011, at 9:42 PM, Chris Ball wrote:

> Hi Philip,
> 
> On Sun, May 22 2011, Philip Rakity wrote:
>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>> index baab027..6db91d6 100644
>> --- a/drivers/mmc/core/mmc.c
>> +++ b/drivers/mmc/core/mmc.c
>> @@ -174,14 +174,17 @@ static int mmc_decode_csd(struct mmc_card *card)
>> }
>> 
>> /*
>> - * Read and decode extended CSD.
>> + * Read extended CSD.
>>  */
>> -static int mmc_read_ext_csd(struct mmc_card *card)
>> +static int mmc_read_ext_csd(struct mmc_card *card, u8 *ext_csd)
> 
> This one doesn't work -- you're missing the half of the patch that hooks
> up the rest of mmc.c to the new code, which was in v1.  As-is, we get:
> 
> drivers/mmc/core/mmc.c: In function ‘mmc_init_card’:
> drivers/mmc/core/mmc.c:621:3: error: too few arguments to function ‘mmc_read_ext_csd’
> drivers/mmc/core/mmc.c:240:12: note: declared here
> drivers/mmc/core/mmc.c: At top level:
> drivers/mmc/core/mmc.c:404:12: warning: ‘mmc_compare_ext_csds’ defined but not used [-Wunused-function]
> 
> - Chris.
> -- 
> Chris Ball   <cjb@laptop.org>   <http://printf.net/>
> One Laptop Per Child
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2] mmc: eMMC bus width may not work on all platforms

[Philip Rakity]

Chris,

Just tested the mod on brownstone.  Works.

Philip

On May 24, 2011, at 6:14 PM, Philip Rakity wrote:

> 
> resend 
> 
> Note:  I cannot test test for a few days.  Following Chris
> suggestion posting to list.
> 
> version 2
> ---
> return -EINVAL when compare of ext_csd_fails
> combine two compare routines into one per suggestion
> from Chris.
> 
> ---
> version 1
> 
> CMD19 -- The offical way to validate bus widths from the
> JEDEC spec does not work on all platforms.  Some platforms
> that use PCI/PCIe to connect their SD controllers are known
> to fail.
> 
> If the quirk MMC_BUS_WIDTH_TEST is not defined we try
> to figure out the bus width by reading the ext_csd at different
> bus widths and compare this against the ext_csd read in 1 bit
> mode.  If no ext_csd is available we default to 1 bit operations.
> 
> Code has been tested on mmp2 against 8 bit eMMC and Transcend 2GB
> card that is known to not work in 4 bit mode.  The physical
> pins on the card are not present to support 4 bit operation.
> 
> Signed-off-by: Philip Rakity <prakity@marvell.com>
> ---
> drivers/mmc/core/mmc.c |  113 +++++++++++++++++++++++++++++++++++++++++++----
> 1 files changed, 103 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index baab027..6db91d6 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -174,14 +174,17 @@ static int mmc_decode_csd(struct mmc_card *card)
> }
> 
> /*
> - * Read and decode extended CSD.
> + * Read extended CSD.
>  */
> -static int mmc_read_ext_csd(struct mmc_card *card)
> +static int mmc_get_ext_csd(struct mmc_card *card, u8 **new_ext_csd)
> {
> 	int err;
> 	u8 *ext_csd;
> 
> 	BUG_ON(!card);
> +	BUG_ON(!new_ext_csd);
> +
> +	*new_ext_csd = NULL;
> 
> 	if (card->csd.mmca_vsn < CSD_SPEC_VER_4)
> 		return 0;
> @@ -199,12 +202,15 @@ static int mmc_read_ext_csd(struct mmc_card *card)
> 
> 	err = mmc_send_ext_csd(card, ext_csd);
> 	if (err) {
> +		kfree(ext_csd);
> +		*new_ext_csd = NULL;
> +
> 		/* If the host or the card can't do the switch,
> 		 * fail more gracefully. */
> 		if ((err != -EINVAL)
> 		 && (err != -ENOSYS)
> 		 && (err != -EFAULT))
> -			goto out;
> +			return err;
> 
> 		/*
> 		 * High capacity cards should have this "magic" size
> @@ -222,9 +228,23 @@ static int mmc_read_ext_csd(struct mmc_card *card)
> 				mmc_hostname(card->host));
> 			err = 0;
> 		}
> +	} else
> +		*new_ext_csd = ext_csd;
> 
> -		goto out;
> -	}
> +	return err;
> +}
> +
> +/*
> + * Decode extended CSD.
> + */
> +static int mmc_read_ext_csd(struct mmc_card *card, u8 *ext_csd)
> +{
> +	int err = 0;
> +
> +	BUG_ON(!card);
> +
> +	if (!ext_csd)
> +		return 0;
> 
> 	/* Version is coded in the CSD_STRUCTURE byte in the EXT_CSD register */
> 	if (card->csd.structure == 3) {
> @@ -372,8 +392,70 @@ static int mmc_read_ext_csd(struct mmc_card *card)
> 		card->erased_byte = 0x0;
> 
> out:
> +	return err;
> +}
> +
> +static inline void mmc_free_ext_csd(u8 *ext_csd)
> +{
> 	kfree(ext_csd);
> +}
> +
> 
> +static int mmc_compare_ext_csds(struct mmc_card *card, u8 *ext_csd,
> +			unsigned bus_width)
> +{
> +	u8 *bw_ext_csd;
> +	int err;
> +
> +	err = mmc_get_ext_csd(card, &bw_ext_csd);
> +	if (err)
> +		return err;
> +
> +	if ((ext_csd == NULL || bw_ext_csd == NULL)) {
> +		if (bus_width != MMC_BUS_WIDTH_1)
> +			err = -EINVAL;
> +		goto out;
> +	}
> +
> +	if (bus_width == MMC_BUS_WIDTH_1)
> +		goto out;
> +
> +	/* only compare read only fields */
> +
> +	err =	(!(ext_csd[EXT_CSD_PARTITION_SUPPORT] ==
> +			bw_ext_csd[EXT_CSD_PARTITION_SUPPORT]) &&
> +		(ext_csd[EXT_CSD_ERASED_MEM_CONT] ==
> +			bw_ext_csd[EXT_CSD_ERASED_MEM_CONT]) &&
> +		(ext_csd[EXT_CSD_REV] ==
> +			bw_ext_csd[EXT_CSD_REV]) &&
> +		(ext_csd[EXT_CSD_STRUCTURE] ==
> +			bw_ext_csd[EXT_CSD_STRUCTURE]) &&
> +		(ext_csd[EXT_CSD_CARD_TYPE] ==
> +			bw_ext_csd[EXT_CSD_CARD_TYPE]) &&
> +		(ext_csd[EXT_CSD_S_A_TIMEOUT] ==
> +			bw_ext_csd[EXT_CSD_S_A_TIMEOUT]) &&
> +		(ext_csd[EXT_CSD_HC_WP_GRP_SIZE] ==
> +			bw_ext_csd[EXT_CSD_HC_WP_GRP_SIZE]) &&
> +		(ext_csd[EXT_CSD_ERASE_TIMEOUT_MULT] ==
> +			bw_ext_csd[EXT_CSD_ERASE_TIMEOUT_MULT]) &&
> +		(ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE] ==
> +			bw_ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE]) &&
> +		(ext_csd[EXT_CSD_SEC_TRIM_MULT] ==
> +			bw_ext_csd[EXT_CSD_SEC_TRIM_MULT]) &&
> +		(ext_csd[EXT_CSD_SEC_ERASE_MULT] ==
> +			bw_ext_csd[EXT_CSD_SEC_ERASE_MULT]) &&
> +		(ext_csd[EXT_CSD_SEC_FEATURE_SUPPORT] ==
> +			bw_ext_csd[EXT_CSD_SEC_FEATURE_SUPPORT]) &&
> +		(ext_csd[EXT_CSD_TRIM_MULT] ==
> +			bw_ext_csd[EXT_CSD_TRIM_MULT]) &&
> +		memcmp(&ext_csd[EXT_CSD_SEC_CNT],
> +			&bw_ext_csd[EXT_CSD_SEC_CNT],
> +			4) != 0);
> +	if (err)
> +		err = -EINVAL;
> +
> +out:
> +	mmc_free_ext_csd(bw_ext_csd);
> 	return err;
> }
> 
> @@ -438,6 +520,7 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
> 	u32 cid[4];
> 	unsigned int max_dtr;
> 	u32 rocr;
> +	u8 *ext_csd = NULL;
> 
> 	BUG_ON(!host);
> 	WARN_ON(!host->claimed);
> @@ -536,7 +619,11 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
> 		/*
> 		 * Fetch and process extended CSD.
> 		 */
> -		err = mmc_read_ext_csd(card);
> +
> +		err = mmc_get_ext_csd(card, &ext_csd);
> +		if (err)
> +			goto free_card;
> +		err = mmc_read_ext_csd(card, ext_csd);
> 		if (err)
> 			goto free_card;
> 
> @@ -676,14 +763,18 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
> 					 0);
> 			if (!err) {
> 				mmc_set_bus_width(card->host, bus_width);
> +
> 				/*
> 				 * If controller can't handle bus width test,
> -				 * use the highest bus width to maintain
> -				 * compatibility with previous MMC behavior.
> +				 * compare ext_csd previously read in 1 bit mode
> +				 * against ext_csd at new bus width
> 				 */
> 				if (!(host->caps & MMC_CAP_BUS_WIDTH_TEST))
> -					break;
> -				err = mmc_bus_test(card, bus_width);
> +					err = mmc_compare_ext_csds(card,
> +						ext_csd,
> +						bus_width);
> +				else
> +					err = mmc_bus_test(card, bus_width);
> 				if (!err)
> 					break;
> 			}
> @@ -730,12 +821,14 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
> 	if (!oldcard)
> 		host->card = card;
> 
> +	mmc_free_ext_csd(ext_csd);
> 	return 0;
> 
> free_card:
> 	if (!oldcard)
> 		mmc_remove_card(card);
> err:
> +	mmc_free_ext_csd(ext_csd);
> 
> 	return err;
> }
> -- 
> 1.7.0.4
> 
> 
> 
> On May 23, 2011, at 9:42 PM, Chris Ball wrote:
> 
>> Hi Philip,
>> 
>> On Sun, May 22 2011, Philip Rakity wrote:
>>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>>> index baab027..6db91d6 100644
>>> --- a/drivers/mmc/core/mmc.c
>>> +++ b/drivers/mmc/core/mmc.c
>>> @@ -174,14 +174,17 @@ static int mmc_decode_csd(struct mmc_card *card)
>>> }
>>> 
>>> /*
>>> - * Read and decode extended CSD.
>>> + * Read extended CSD.
>>> */
>>> -static int mmc_read_ext_csd(struct mmc_card *card)
>>> +static int mmc_read_ext_csd(struct mmc_card *card, u8 *ext_csd)
>> 
>> This one doesn't work -- you're missing the half of the patch that hooks
>> up the rest of mmc.c to the new code, which was in v1.  As-is, we get:
>> 
>> drivers/mmc/core/mmc.c: In function ‘mmc_init_card’:
>> drivers/mmc/core/mmc.c:621:3: error: too few arguments to function ‘mmc_read_ext_csd’
>> drivers/mmc/core/mmc.c:240:12: note: declared here
>> drivers/mmc/core/mmc.c: At top level:
>> drivers/mmc/core/mmc.c:404:12: warning: ‘mmc_compare_ext_csds’ defined but not used [-Wunused-function]
>> 
>> - Chris.
>> -- 
>> Chris Ball   <cjb@laptop.org>   <http://printf.net/>
>> One Laptop Per Child
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>