[PATCH 0/2] SD current limit setting fix

[PATCH 0/2] SD current limit setting fix

[Aaron Lu]

The following 2 patches fix setting of SD current limit

Aaron Lu (2):
  mmc: core: Simplify and fix for SD switch processing
  mmc: sd: Fix sd current limit setting

 drivers/mmc/core/sd.c    | 144 +++++++++++++++++++++++------------------------
 drivers/mmc/host/sdhci.c |  28 +++++++--
 include/linux/mmc/host.h |  16 ++++--
 3 files changed, 105 insertions(+), 83 deletions(-)

-- 
1.7.11.1.3.g4c8a9db

[PATCH 1/2] mmc: core: Simplify and fix for SD switch processing

[Aaron Lu]

In mmc_read_switch, just do a one time mode 0 switch command to get the
support bits information, no need to do multiple times as the support
bits do not change with different arguments.

And no need to check current limit support bits, as these bits is
fixed according to the signal voltage. If the signal voltage is 1.8V,
the support bits would be 0xf and if the signal voltage is 3.3V, the
support bits would be 0x01. We will check host's ability to set the
current limit.

Signed-off-by: Aaron Lu <aaron.lu@amd.com>
---
 drivers/mmc/core/sd.c | 94 ++++++++++++---------------------------------------
 1 file changed, 22 insertions(+), 72 deletions(-)

diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
index b0b9e37..ae72d6e 100644
--- a/drivers/mmc/core/sd.c
+++ b/drivers/mmc/core/sd.c
@@ -290,8 +290,12 @@ static int mmc_read_switch(struct mmc_card *card)
 		return -ENOMEM;
 	}
 
-	/* Find out the supported Bus Speed Modes. */
-	err = mmc_sd_switch(card, 0, 0, 1, status);
+	/*
+	 * Find out the card's support bits with a mode 0 operation.
+	 * The argument does not matter, as the support bits do not
+	 * change with the arguments.
+	 */
+	err = mmc_sd_switch(card, 0, 0, 0, status);
 	if (err) {
 		/*
 		 * If the host or the card can't do the switch,
@@ -312,46 +316,8 @@ static int mmc_read_switch(struct mmc_card *card)
 
 	if (card->scr.sda_spec3) {
 		card->sw_caps.sd3_bus_mode = status[13];
-
-		/* Find out Driver Strengths supported by the card */
-		err = mmc_sd_switch(card, 0, 2, 1, status);
-		if (err) {
-			/*
-			 * If the host or the card can't do the switch,
-			 * fail more gracefully.
-			 */
-			if (err != -EINVAL && err != -ENOSYS && err != -EFAULT)
-				goto out;
-
-			pr_warning("%s: problem reading "
-				"Driver Strength.\n",
-				mmc_hostname(card->host));
-			err = 0;
-
-			goto out;
-		}
-
+		/* Driver Strengths supported by the card */
 		card->sw_caps.sd3_drv_type = status[9];
-
-		/* Find out Current Limits supported by the card */
-		err = mmc_sd_switch(card, 0, 3, 1, status);
-		if (err) {
-			/*
-			 * If the host or the card can't do the switch,
-			 * fail more gracefully.
-			 */
-			if (err != -EINVAL && err != -ENOSYS && err != -EFAULT)
-				goto out;
-
-			pr_warning("%s: problem reading "
-				"Current Limit.\n",
-				mmc_hostname(card->host));
-			err = 0;
-
-			goto out;
-		}
-
-		card->sw_caps.sd3_curr_limit = status[7];
 	}
 
 out:
@@ -560,41 +526,24 @@ static int sd_set_current_limit(struct mmc_card *card, u8 *status)
 	 * Current limit switch is only defined for SDR50, SDR104, and DDR50
 	 * bus speed modes. For other bus speed modes, we do not change the
 	 * current limit.
+	 * We only check host's capability here, if we set a limit that is
+	 * higher than the card's maximum current, the card will be using its
+	 * maximum current, e.g. if the card's maximum current is 300ma, and
+	 * when we set current limit to 200ma, the card will draw 200ma, and
+	 * when we set current limit to 400/600/800ma, the card will draw its
+	 * maximum 300ma from the host.
 	 */
 	if ((card->sd_bus_speed == UHS_SDR50_BUS_SPEED) ||
 	    (card->sd_bus_speed == UHS_SDR104_BUS_SPEED) ||
 	    (card->sd_bus_speed == UHS_DDR50_BUS_SPEED)) {
-		if (card->host->caps & MMC_CAP_MAX_CURRENT_800) {
-			if (card->sw_caps.sd3_curr_limit & SD_MAX_CURRENT_800)
-				current_limit = SD_SET_CURRENT_LIMIT_800;
-			else if (card->sw_caps.sd3_curr_limit &
-					SD_MAX_CURRENT_600)
-				current_limit = SD_SET_CURRENT_LIMIT_600;
-			else if (card->sw_caps.sd3_curr_limit &
-					SD_MAX_CURRENT_400)
-				current_limit = SD_SET_CURRENT_LIMIT_400;
-			else if (card->sw_caps.sd3_curr_limit &
-					SD_MAX_CURRENT_200)
-				current_limit = SD_SET_CURRENT_LIMIT_200;
-		} else if (card->host->caps & MMC_CAP_MAX_CURRENT_600) {
-			if (card->sw_caps.sd3_curr_limit & SD_MAX_CURRENT_600)
-				current_limit = SD_SET_CURRENT_LIMIT_600;
-			else if (card->sw_caps.sd3_curr_limit &
-					SD_MAX_CURRENT_400)
-				current_limit = SD_SET_CURRENT_LIMIT_400;
-			else if (card->sw_caps.sd3_curr_limit &
-					SD_MAX_CURRENT_200)
-				current_limit = SD_SET_CURRENT_LIMIT_200;
-		} else if (card->host->caps & MMC_CAP_MAX_CURRENT_400) {
-			if (card->sw_caps.sd3_curr_limit & SD_MAX_CURRENT_400)
-				current_limit = SD_SET_CURRENT_LIMIT_400;
-			else if (card->sw_caps.sd3_curr_limit &
-					SD_MAX_CURRENT_200)
-				current_limit = SD_SET_CURRENT_LIMIT_200;
-		} else if (card->host->caps & MMC_CAP_MAX_CURRENT_200) {
-			if (card->sw_caps.sd3_curr_limit & SD_MAX_CURRENT_200)
-				current_limit = SD_SET_CURRENT_LIMIT_200;
-		}
+		if (card->host->caps & MMC_CAP_MAX_CURRENT_800)
+			current_limit = SD_SET_CURRENT_LIMIT_800;
+		else if (card->host->caps & MMC_CAP_MAX_CURRENT_600)
+			current_limit = SD_SET_CURRENT_LIMIT_600;
+		else if (card->host->caps & MMC_CAP_MAX_CURRENT_400)
+			current_limit = SD_SET_CURRENT_LIMIT_400;
+		else if (card->host->caps & MMC_CAP_MAX_CURRENT_200)
+			current_limit = SD_SET_CURRENT_LIMIT_200;
 	}
 
 	if (current_limit != SD_SET_CURRENT_NO_CHANGE) {
@@ -607,6 +556,7 @@ static int sd_set_current_limit(struct mmc_card *card, u8 *status)
 				mmc_hostname(card->host));
 
 	}
+
 	return 0;
 }
 
-- 
1.7.11.1.3.g4c8a9db

[PATCH 2/2] mmc: sd: Fix sd current limit setting

[Aaron Lu]

Host has different current capabilities at different voltages, we need
to record these settings seperately. Before set current limit for the sd
card, find out the current voltage first and then find out the current
capabilities of the host to set the limit.

Signed-off-by: Aaron Lu <aaron.lu@amd.com>
---
 drivers/mmc/core/sd.c    | 58 ++++++++++++++++++++++++++++++++++++++++++------
 drivers/mmc/host/sdhci.c | 28 +++++++++++++++++++----
 include/linux/mmc/host.h | 16 +++++++++----
 3 files changed, 87 insertions(+), 15 deletions(-)

diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
index ae72d6e..4b4cf4d 100644
--- a/drivers/mmc/core/sd.c
+++ b/drivers/mmc/core/sd.c
@@ -521,11 +521,39 @@ static int sd_set_current_limit(struct mmc_card *card, u8 *status)
 {
 	int current_limit = SD_SET_CURRENT_NO_CHANGE;
 	int err;
+	u8 voltage;
 
 	/*
 	 * Current limit switch is only defined for SDR50, SDR104, and DDR50
 	 * bus speed modes. For other bus speed modes, we do not change the
 	 * current limit.
+	 */
+	if ((card->sd_bus_speed != UHS_SDR50_BUS_SPEED) &&
+	    (card->sd_bus_speed != UHS_SDR104_BUS_SPEED) &&
+	    (card->sd_bus_speed != UHS_DDR50_BUS_SPEED))
+		return 0;
+
+	/*
+	 * Host has different current capabilities when operating at
+	 * different voltages, so find out the current voltage first.
+	 */
+	switch (1 << card->host->ios.vdd) {
+		case MMC_VDD_165_195:
+			voltage = 0; /* host's voltage is 1.8V */
+			break;
+		case MMC_VDD_29_30:
+		case MMC_VDD_30_31:
+			voltage = 1; /* host's voltage is 3.0V */
+			break;
+		case MMC_VDD_32_33:
+		case MMC_VDD_33_34:
+			voltage = 2; /* host's voltage is 3.3V */
+			break;
+		default:
+			BUG(); /* host's voltage is invalid */
+	}
+
+	/*
 	 * We only check host's capability here, if we set a limit that is
 	 * higher than the card's maximum current, the card will be using its
 	 * maximum current, e.g. if the card's maximum current is 300ma, and
@@ -533,16 +561,32 @@ static int sd_set_current_limit(struct mmc_card *card, u8 *status)
 	 * when we set current limit to 400/600/800ma, the card will draw its
 	 * maximum 300ma from the host.
 	 */
-	if ((card->sd_bus_speed == UHS_SDR50_BUS_SPEED) ||
-	    (card->sd_bus_speed == UHS_SDR104_BUS_SPEED) ||
-	    (card->sd_bus_speed == UHS_DDR50_BUS_SPEED)) {
-		if (card->host->caps & MMC_CAP_MAX_CURRENT_800)
+	if (voltage == 0) {
+		if (card->host->caps & MMC_CAP_MAX_CURRENT_800_180)
+			current_limit = SD_SET_CURRENT_LIMIT_800;
+		else if (card->host->caps & MMC_CAP_MAX_CURRENT_600_180)
+			current_limit = SD_SET_CURRENT_LIMIT_600;
+		else if (card->host->caps & MMC_CAP_MAX_CURRENT_400_180)
+			current_limit = SD_SET_CURRENT_LIMIT_400;
+		else if (card->host->caps & MMC_CAP_MAX_CURRENT_200_180)
+			current_limit = SD_SET_CURRENT_LIMIT_200;
+	} else if (voltage == 1) {
+		if (card->host->caps & MMC_CAP_MAX_CURRENT_800_300)
+			current_limit = SD_SET_CURRENT_LIMIT_800;
+		else if (card->host->caps & MMC_CAP_MAX_CURRENT_600_300)
+			current_limit = SD_SET_CURRENT_LIMIT_600;
+		else if (card->host->caps & MMC_CAP_MAX_CURRENT_400_300)
+			current_limit = SD_SET_CURRENT_LIMIT_400;
+		else if (card->host->caps & MMC_CAP_MAX_CURRENT_200_300)
+			current_limit = SD_SET_CURRENT_LIMIT_200;
+	} else if (voltage == 2) {
+		if (card->host->caps & MMC_CAP_MAX_CURRENT_800_330)
 			current_limit = SD_SET_CURRENT_LIMIT_800;
-		else if (card->host->caps & MMC_CAP_MAX_CURRENT_600)
+		else if (card->host->caps & MMC_CAP_MAX_CURRENT_600_330)
 			current_limit = SD_SET_CURRENT_LIMIT_600;
-		else if (card->host->caps & MMC_CAP_MAX_CURRENT_400)
+		else if (card->host->caps & MMC_CAP_MAX_CURRENT_400_330)
 			current_limit = SD_SET_CURRENT_LIMIT_400;
-		else if (card->host->caps & MMC_CAP_MAX_CURRENT_200)
+		else if (card->host->caps & MMC_CAP_MAX_CURRENT_200_330)
 			current_limit = SD_SET_CURRENT_LIMIT_200;
 	}
 
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index caba999..00c2cbb 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2897,6 +2897,16 @@ int sdhci_add_host(struct sdhci_host *host)
 
 		if (max_current_330 > 150)
 			mmc->caps |= MMC_CAP_SET_XPC_330;
+
+		/* Maximum current capabilities of the host at 3.3V */
+		if (max_current_330 >= 800)
+			mmc->caps |= MMC_CAP_MAX_CURRENT_800_330;
+		else if (max_current_330 >= 600)
+			mmc->caps |= MMC_CAP_MAX_CURRENT_600_330;
+		else if (max_current_330 >= 400)
+			mmc->caps |= MMC_CAP_MAX_CURRENT_400_330;
+		else if (max_current_330 >= 200)
+			mmc->caps |= MMC_CAP_MAX_CURRENT_200_330;
 	}
 	if (caps[0] & SDHCI_CAN_VDD_300) {
 		int max_current_300;
@@ -2910,6 +2920,16 @@ int sdhci_add_host(struct sdhci_host *host)
 
 		if (max_current_300 > 150)
 			mmc->caps |= MMC_CAP_SET_XPC_300;
+
+		/* Maximum current capabilities of the host at 3.0V */
+		if (max_current_300 >= 800)
+			mmc->caps |= MMC_CAP_MAX_CURRENT_800_300;
+		else if (max_current_300 >= 600)
+			mmc->caps |= MMC_CAP_MAX_CURRENT_600_300;
+		else if (max_current_300 >= 400)
+			mmc->caps |= MMC_CAP_MAX_CURRENT_400_300;
+		else if (max_current_300 >= 200)
+			mmc->caps |= MMC_CAP_MAX_CURRENT_200_300;
 	}
 	if (caps[0] & SDHCI_CAN_VDD_180) {
 		int max_current_180;
@@ -2926,13 +2946,13 @@ int sdhci_add_host(struct sdhci_host *host)
 
 		/* Maximum current capabilities of the host at 1.8V */
 		if (max_current_180 >= 800)
-			mmc->caps |= MMC_CAP_MAX_CURRENT_800;
+			mmc->caps |= MMC_CAP_MAX_CURRENT_800_180;
 		else if (max_current_180 >= 600)
-			mmc->caps |= MMC_CAP_MAX_CURRENT_600;
+			mmc->caps |= MMC_CAP_MAX_CURRENT_600_180;
 		else if (max_current_180 >= 400)
-			mmc->caps |= MMC_CAP_MAX_CURRENT_400;
+			mmc->caps |= MMC_CAP_MAX_CURRENT_400_180;
 		else if (max_current_180 >= 200)
-			mmc->caps |= MMC_CAP_MAX_CURRENT_200;
+			mmc->caps |= MMC_CAP_MAX_CURRENT_200_180;
 	}
 
 	mmc->ocr_avail = ocr_avail;
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 65c64ee..ca84ffb 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -238,10 +238,10 @@ struct mmc_host {
 #define MMC_CAP_DRIVER_TYPE_A	(1 << 23)	/* Host supports Driver Type A */
 #define MMC_CAP_DRIVER_TYPE_C	(1 << 24)	/* Host supports Driver Type C */
 #define MMC_CAP_DRIVER_TYPE_D	(1 << 25)	/* Host supports Driver Type D */
-#define MMC_CAP_MAX_CURRENT_200	(1 << 26)	/* Host max current limit is 200mA */
-#define MMC_CAP_MAX_CURRENT_400	(1 << 27)	/* Host max current limit is 400mA */
-#define MMC_CAP_MAX_CURRENT_600	(1 << 28)	/* Host max current limit is 600mA */
-#define MMC_CAP_MAX_CURRENT_800	(1 << 29)	/* Host max current limit is 800mA */
+#define MMC_CAP_MAX_CURRENT_200_180 (1 << 26)	/* Host max current limit is 200mA at 1.8V */
+#define MMC_CAP_MAX_CURRENT_400_180 (1 << 27)	/* Host max current limit is 400mA at 1.8V */
+#define MMC_CAP_MAX_CURRENT_600_180 (1 << 28)	/* Host max current limit is 600mA at 1.8V */
+#define MMC_CAP_MAX_CURRENT_800_180 (1 << 29)	/* Host max current limit is 800mA at 1.8V */
 #define MMC_CAP_CMD23		(1 << 30)	/* CMD23 supported. */
 #define MMC_CAP_HW_RESET	(1 << 31)	/* Hardware reset */
 
@@ -261,6 +261,14 @@ struct mmc_host {
 #define MMC_CAP2_HC_ERASE_SZ	(1 << 9)	/* High-capacity erase size */
 #define MMC_CAP2_CD_ACTIVE_HIGH	(1 << 10)	/* Card-detect signal active high */
 #define MMC_CAP2_RO_ACTIVE_HIGH	(1 << 11)	/* Write-protect signal active high */
+#define MMC_CAP_MAX_CURRENT_200_300 (1 << 12)	/* Host max current limit is 200mA at 3.0V */
+#define MMC_CAP_MAX_CURRENT_400_300 (1 << 13)	/* Host max current limit is 400mA at 3.0V */
+#define MMC_CAP_MAX_CURRENT_600_300 (1 << 14)	/* Host max current limit is 600mA at 3.0V */
+#define MMC_CAP_MAX_CURRENT_800_300 (1 << 15)	/* Host max current limit is 800mA at 3.0V */
+#define MMC_CAP_MAX_CURRENT_200_330 (1 << 16)	/* Host max current limit is 200mA at 3.3V */
+#define MMC_CAP_MAX_CURRENT_400_330 (1 << 17)	/* Host max current limit is 400mA at 3.3V */
+#define MMC_CAP_MAX_CURRENT_600_330 (1 << 18)	/* Host max current limit is 600mA at 3.3V */
+#define MMC_CAP_MAX_CURRENT_800_330 (1 << 19)	/* Host max current limit is 800mA at 3.3V */
 
 	mmc_pm_flag_t		pm_caps;	/* supported pm features */
 	unsigned int        power_notify_type;
-- 
1.7.11.1.3.g4c8a9db

Re: [PATCH 2/2] mmc: sd: Fix sd current limit setting

[Philip Rakity]

Hi Arron,

One minor comment -- okay without the change but clearer IMHO with the change.

Reviewed-by: Philip Rakity <prakity@marvell.com>

Philip


On Jul 2, 2012, at 11:16 PM, Aaron Lu wrote:

> Host has different current capabilities at different voltages, we need
> to record these settings seperately. Before set current limit for the sd
> card, find out the current voltage first and then find out the current
> capabilities of the host to set the limit.
> 
> Signed-off-by: Aaron Lu <aaron.lu@amd.com>
> ---
> drivers/mmc/core/sd.c    | 58 ++++++++++++++++++++++++++++++++++++++++++------
> drivers/mmc/host/sdhci.c | 28 +++++++++++++++++++----
> include/linux/mmc/host.h | 16 +++++++++----
> 3 files changed, 87 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
> index ae72d6e..4b4cf4d 100644
> --- a/drivers/mmc/core/sd.c
> +++ b/drivers/mmc/core/sd.c
> @@ -521,11 +521,39 @@ static int sd_set_current_limit(struct mmc_card *card, u8 *status)
> {
> 	int current_limit = SD_SET_CURRENT_NO_CHANGE;
> 	int err;
> +	u8 voltage;
	u32 voltage;
> 
> 	/*
> 	 * Current limit switch is only defined for SDR50, SDR104, and DDR50
> 	 * bus speed modes. For other bus speed modes, we do not change the
> 	 * current limit.
> +	 */
> +	if ((card->sd_bus_speed != UHS_SDR50_BUS_SPEED) &&
> +	    (card->sd_bus_speed != UHS_SDR104_BUS_SPEED) &&
> +	    (card->sd_bus_speed != UHS_DDR50_BUS_SPEED))
> +		return 0;
> +
> +	/*
> +	 * Host has different current capabilities when operating at
> +	 * different voltages, so find out the current voltage first.
> +	 */
> +	switch (1 << card->host->ios.vdd) {
> +		case MMC_VDD_165_195:
> +			voltage = 0; /* host's voltage is 1.8V */

It is a little clearly if you do this or make voltage 0, 1, 2 an enum for the routine.
otherwise
	reviewed-by: Philip Rakity <prakity@marvell.com>
			voltage = MMC_VDD_165_195;
> 
> +			break;
> +		case MMC_VDD_29_30:
> +		case MMC_VDD_30_31:
> +			voltage = 1; /* host's voltage is 3.0V */
			voltage = MMC_VDD_29_30 | MMC_VDD_30_31;
> 
> +			break;
> +		case MMC_VDD_32_33:
> +		case MMC_VDD_33_34:
> +			voltage = 2; /* host's voltage is 3.3V */
			voltage = MMC_VDD_32_33 | MMC_VDD_33_34;
> 
> +			break;
> +		default:
> +			BUG(); /* host's voltage is invalid */
> +	}
> +
> +	/*
> 	 * We only check host's capability here, if we set a limit that is
> 	 * higher than the card's maximum current, the card will be using its
> 	 * maximum current, e.g. if the card's maximum current is 300ma, and
> @@ -533,16 +561,32 @@ static int sd_set_current_limit(struct mmc_card *card, u8 *status)
> 	 * when we set current limit to 400/600/800ma, the card will draw its
> 	 * maximum 300ma from the host.
> 	 */
> -	if ((card->sd_bus_speed == UHS_SDR50_BUS_SPEED) ||
> -	    (card->sd_bus_speed == UHS_SDR104_BUS_SPEED) ||
> -	    (card->sd_bus_speed == UHS_DDR50_BUS_SPEED)) {
> -		if (card->host->caps & MMC_CAP_MAX_CURRENT_800)
> +	if (voltage == 0) {
	if (voltage == MMC_VDD_165_195) { 
> +		if (card->host->caps & MMC_CAP_MAX_CURRENT_800_180)
> +			current_limit = SD_SET_CURRENT_LIMIT_800;
> +		else if (card->host->caps & MMC_CAP_MAX_CURRENT_600_180)
> +			current_limit = SD_SET_CURRENT_LIMIT_600;
> +		else if (card->host->caps & MMC_CAP_MAX_CURRENT_400_180)
> +			current_limit = SD_SET_CURRENT_LIMIT_400;
> +		else if (card->host->caps & MMC_CAP_MAX_CURRENT_200_180)
> +			current_limit = SD_SET_CURRENT_LIMIT_200;
> +	} else if (voltage == 1)
	} else if (voltage == ( MMC_VDD_29_30 | MMC_VDD_30_31)

	etc ?
> {
> +		if (card->host->caps & MMC_CAP_MAX_CURRENT_800_300)
> +			current_limit = SD_SET_CURRENT_LIMIT_800;
> +		else if (card->host->caps & MMC_CAP_MAX_CURRENT_600_300)
> +			current_limit = SD_SET_CURRENT_LIMIT_600;
> +		else if (card->host->caps & MMC_CAP_MAX_CURRENT_400_300)
> +			current_limit = SD_SET_CURRENT_LIMIT_400;
> +		else if (card->host->caps & MMC_CAP_MAX_CURRENT_200_300)
> +			current_limit = SD_SET_CURRENT_LIMIT_200;
> +	} else if (voltage == 2) {
> +		if (card->host->caps & MMC_CAP_MAX_CURRENT_800_330)
> 			current_limit = SD_SET_CURRENT_LIMIT_800;
> -		else if (card->host->caps & MMC_CAP_MAX_CURRENT_600)
> +		else if (card->host->caps & MMC_CAP_MAX_CURRENT_600_330)
> 			current_limit = SD_SET_CURRENT_LIMIT_600;
> -		else if (card->host->caps & MMC_CAP_MAX_CURRENT_400)
> +		else if (card->host->caps & MMC_CAP_MAX_CURRENT_400_330)
> 			current_limit = SD_SET_CURRENT_LIMIT_400;
> -		else if (card->host->caps & MMC_CAP_MAX_CURRENT_200)
> +		else if (card->host->caps & MMC_CAP_MAX_CURRENT_200_330)
> 			current_limit = SD_SET_CURRENT_LIMIT_200;
> 	}
> 
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index caba999..00c2cbb 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -2897,6 +2897,16 @@ int sdhci_add_host(struct sdhci_host *host)
> 
> 		if (max_current_330 > 150)
> 			mmc->caps |= MMC_CAP_SET_XPC_330;
> +
> +		/* Maximum current capabilities of the host at 3.3V */
> +		if (max_current_330 >= 800)
> +			mmc->caps |= MMC_CAP_MAX_CURRENT_800_330;
> +		else if (max_current_330 >= 600)
> +			mmc->caps |= MMC_CAP_MAX_CURRENT_600_330;
> +		else if (max_current_330 >= 400)
> +			mmc->caps |= MMC_CAP_MAX_CURRENT_400_330;
> +		else if (max_current_330 >= 200)
> +			mmc->caps |= MMC_CAP_MAX_CURRENT_200_330;
> 	}
> 	if (caps[0] & SDHCI_CAN_VDD_300) {
> 		int max_current_300;
> @@ -2910,6 +2920,16 @@ int sdhci_add_host(struct sdhci_host *host)
> 
> 		if (max_current_300 > 150)
> 			mmc->caps |= MMC_CAP_SET_XPC_300;
> +
> +		/* Maximum current capabilities of the host at 3.0V */
> +		if (max_current_300 >= 800)
> +			mmc->caps |= MMC_CAP_MAX_CURRENT_800_300;
> +		else if (max_current_300 >= 600)
> +			mmc->caps |= MMC_CAP_MAX_CURRENT_600_300;
> +		else if (max_current_300 >= 400)
> +			mmc->caps |= MMC_CAP_MAX_CURRENT_400_300;
> +		else if (max_current_300 >= 200)
> +			mmc->caps |= MMC_CAP_MAX_CURRENT_200_300;
> 	}
> 	if (caps[0] & SDHCI_CAN_VDD_180) {
> 		int max_current_180;
> @@ -2926,13 +2946,13 @@ int sdhci_add_host(struct sdhci_host *host)
> 
> 		/* Maximum current capabilities of the host at 1.8V */
> 		if (max_current_180 >= 800)
> -			mmc->caps |= MMC_CAP_MAX_CURRENT_800;
> +			mmc->caps |= MMC_CAP_MAX_CURRENT_800_180;
> 		else if (max_current_180 >= 600)
> -			mmc->caps |= MMC_CAP_MAX_CURRENT_600;
> +			mmc->caps |= MMC_CAP_MAX_CURRENT_600_180;
> 		else if (max_current_180 >= 400)
> -			mmc->caps |= MMC_CAP_MAX_CURRENT_400;
> +			mmc->caps |= MMC_CAP_MAX_CURRENT_400_180;
> 		else if (max_current_180 >= 200)
> -			mmc->caps |= MMC_CAP_MAX_CURRENT_200;
> +			mmc->caps |= MMC_CAP_MAX_CURRENT_200_180;
> 	}
> 
> 	mmc->ocr_avail = ocr_avail;
> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index 65c64ee..ca84ffb 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -238,10 +238,10 @@ struct mmc_host {
> #define MMC_CAP_DRIVER_TYPE_A	(1 << 23)	/* Host supports Driver Type A */
> #define MMC_CAP_DRIVER_TYPE_C	(1 << 24)	/* Host supports Driver Type C */
> #define MMC_CAP_DRIVER_TYPE_D	(1 << 25)	/* Host supports Driver Type D */
> -#define MMC_CAP_MAX_CURRENT_200	(1 << 26)	/* Host max current limit is 200mA */
> -#define MMC_CAP_MAX_CURRENT_400	(1 << 27)	/* Host max current limit is 400mA */
> -#define MMC_CAP_MAX_CURRENT_600	(1 << 28)	/* Host max current limit is 600mA */
> -#define MMC_CAP_MAX_CURRENT_800	(1 << 29)	/* Host max current limit is 800mA */
> +#define MMC_CAP_MAX_CURRENT_200_180 (1 << 26)	/* Host max current limit is 200mA at 1.8V */
> +#define MMC_CAP_MAX_CURRENT_400_180 (1 << 27)	/* Host max current limit is 400mA at 1.8V */
> +#define MMC_CAP_MAX_CURRENT_600_180 (1 << 28)	/* Host max current limit is 600mA at 1.8V */
> +#define MMC_CAP_MAX_CURRENT_800_180 (1 << 29)	/* Host max current limit is 800mA at 1.8V */
> #define MMC_CAP_CMD23		(1 << 30)	/* CMD23 supported. */
> #define MMC_CAP_HW_RESET	(1 << 31)	/* Hardware reset */
> 
> @@ -261,6 +261,14 @@ struct mmc_host {
> #define MMC_CAP2_HC_ERASE_SZ	(1 << 9)	/* High-capacity erase size */
> #define MMC_CAP2_CD_ACTIVE_HIGH	(1 << 10)	/* Card-detect signal active high */
> #define MMC_CAP2_RO_ACTIVE_HIGH	(1 << 11)	/* Write-protect signal active high */
> +#define MMC_CAP_MAX_CURRENT_200_300 (1 << 12)	/* Host max current limit is 200mA at 3.0V */
> +#define MMC_CAP_MAX_CURRENT_400_300 (1 << 13)	/* Host max current limit is 400mA at 3.0V */
> +#define MMC_CAP_MAX_CURRENT_600_300 (1 << 14)	/* Host max current limit is 600mA at 3.0V */
> +#define MMC_CAP_MAX_CURRENT_800_300 (1 << 15)	/* Host max current limit is 800mA at 3.0V */
> +#define MMC_CAP_MAX_CURRENT_200_330 (1 << 16)	/* Host max current limit is 200mA at 3.3V */
> +#define MMC_CAP_MAX_CURRENT_400_330 (1 << 17)	/* Host max current limit is 400mA at 3.3V */
> +#define MMC_CAP_MAX_CURRENT_600_330 (1 << 18)	/* Host max current limit is 600mA at 3.3V */
> +#define MMC_CAP_MAX_CURRENT_800_330 (1 << 19)	/* Host max current limit is 800mA at 3.3V */
> 
> 	mmc_pm_flag_t		pm_caps;	/* supported pm features */
> 	unsigned int        power_notify_type;
> -- 
> 1.7.11.1.3.g4c8a9db
> 
> 

Re: [PATCH 1/2] mmc: core: Simplify and fix for SD switch processing

[Chris Ball]

Hi,

On Tue, Jul 03 2012, Aaron Lu wrote:
> In mmc_read_switch, just do a one time mode 0 switch command to get the
> support bits information, no need to do multiple times as the support
> bits do not change with different arguments.
>
> And no need to check current limit support bits, as these bits is
> fixed according to the signal voltage. If the signal voltage is 1.8V,
> the support bits would be 0xf and if the signal voltage is 3.3V, the
> support bits would be 0x01. We will check host's ability to set the
> current limit.
>
> Signed-off-by: Aaron Lu <aaron.lu@amd.com>

Thanks, pushed to mmc-next for 3.6.

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

Re: [PATCH 2/2] mmc: sd: Fix sd current limit setting

[Chris Ball]

Hi,

On Tue, Jul 03 2012, Aaron Lu wrote:
> Host has different current capabilities at different voltages, we need
> to record these settings seperately. Before set current limit for the sd
> card, find out the current voltage first and then find out the current
> capabilities of the host to set the limit.
>
> Signed-off-by: Aaron Lu <aaron.lu@amd.com>
> [...]
> +	switch (1 << card->host->ios.vdd) {
> +		case MMC_VDD_165_195:
> +			voltage = 0; /* host's voltage is 1.8V */
> +			break;
> +		case MMC_VDD_29_30:
> +		case MMC_VDD_30_31:
> +			voltage = 1; /* host's voltage is 3.0V */
> +			break;
> +		case MMC_VDD_32_33:
> +		case MMC_VDD_33_34:
> +			voltage = 2; /* host's voltage is 3.3V */
> +			break;
> +		default:
> +			BUG(); /* host's voltage is invalid */

Please don't call BUG() -- that could easily cause a kernel panic here
(the mmc_core module will hang), which is unjustified.  You'll need to
figure out what exactly you want to have happen when the host's voltage
is invalid, and do that without affecting other controllers that are
in use on the board.

Thanks,

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

[PATCH v2 2/2] mmc: sd: Fix sd current limit setting

[Aaron Lu]

Host has different current capabilities at different voltages, we need
to record these settings seperately. The defined voltages are 1.8/3.0/3.3.
For other voltages, we do not touch current limit setting.

Before set current limit for the sd card, find out the host's operating
voltage first and then find out the current capabilities of the host at
that voltage to set the current limit.

Signed-off-by: Aaron Lu <aaron.lu@amd.com>
---
v2:
Do not call BUG() when the host's voltage is not supported as suggested by
Chris Ball.
Do not use 0/1/2 to represent the host's voltage as suggested by Philip Rakity.

 drivers/mmc/core/sd.c    | 44 +++++++++++++++++++++++++++++++++++++-------
 drivers/mmc/host/sdhci.c | 28 ++++++++++++++++++++++++----
 include/linux/mmc/host.h | 16 ++++++++++++----
 3 files changed, 73 insertions(+), 15 deletions(-)

diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
index 8460568..312b78d 100644
--- a/drivers/mmc/core/sd.c
+++ b/drivers/mmc/core/sd.c
@@ -521,11 +521,25 @@ static int sd_set_current_limit(struct mmc_card *card, u8 *status)
 {
 	int current_limit = SD_SET_CURRENT_NO_CHANGE;
 	int err;
+	u32 voltage;
 
 	/*
 	 * Current limit switch is only defined for SDR50, SDR104, and DDR50
 	 * bus speed modes. For other bus speed modes, we do not change the
 	 * current limit.
+	 */
+	if ((card->sd_bus_speed != UHS_SDR50_BUS_SPEED) &&
+	    (card->sd_bus_speed != UHS_SDR104_BUS_SPEED) &&
+	    (card->sd_bus_speed != UHS_DDR50_BUS_SPEED))
+		return 0;
+
+	/*
+	 * Host has different current capabilities when operating at
+	 * different voltages, so find out the current voltage first.
+	 */
+	voltage = 1 << card->host->ios.vdd;
+
+	/*
 	 * We only check host's capability here, if we set a limit that is
 	 * higher than the card's maximum current, the card will be using its
 	 * maximum current, e.g. if the card's maximum current is 300ma, and
@@ -533,16 +547,32 @@ static int sd_set_current_limit(struct mmc_card *card, u8 *status)
 	 * when we set current limit to 400/600/800ma, the card will draw its
 	 * maximum 300ma from the host.
 	 */
-	if ((card->sd_bus_speed == UHS_SDR50_BUS_SPEED) ||
-	    (card->sd_bus_speed == UHS_SDR104_BUS_SPEED) ||
-	    (card->sd_bus_speed == UHS_DDR50_BUS_SPEED)) {
-		if (card->host->caps & MMC_CAP_MAX_CURRENT_800)
+	if (voltage == MMC_VDD_165_195) {
+		if (card->host->caps & MMC_CAP_MAX_CURRENT_800_180)
+			current_limit = SD_SET_CURRENT_LIMIT_800;
+		else if (card->host->caps & MMC_CAP_MAX_CURRENT_600_180)
+			current_limit = SD_SET_CURRENT_LIMIT_600;
+		else if (card->host->caps & MMC_CAP_MAX_CURRENT_400_180)
+			current_limit = SD_SET_CURRENT_LIMIT_400;
+		else if (card->host->caps & MMC_CAP_MAX_CURRENT_200_180)
+			current_limit = SD_SET_CURRENT_LIMIT_200;
+	} else if (voltage & (MMC_VDD_29_30 | MMC_VDD_30_31)) {
+		if (card->host->caps & MMC_CAP_MAX_CURRENT_800_300)
+			current_limit = SD_SET_CURRENT_LIMIT_800;
+		else if (card->host->caps & MMC_CAP_MAX_CURRENT_600_300)
+			current_limit = SD_SET_CURRENT_LIMIT_600;
+		else if (card->host->caps & MMC_CAP_MAX_CURRENT_400_300)
+			current_limit = SD_SET_CURRENT_LIMIT_400;
+		else if (card->host->caps & MMC_CAP_MAX_CURRENT_200_300)
+			current_limit = SD_SET_CURRENT_LIMIT_200;
+	} else if (voltage & (MMC_VDD_32_33 | MMC_VDD_33_34)) {
+		if (card->host->caps & MMC_CAP_MAX_CURRENT_800_330)
 			current_limit = SD_SET_CURRENT_LIMIT_800;
-		else if (card->host->caps & MMC_CAP_MAX_CURRENT_600)
+		else if (card->host->caps & MMC_CAP_MAX_CURRENT_600_330)
 			current_limit = SD_SET_CURRENT_LIMIT_600;
-		else if (card->host->caps & MMC_CAP_MAX_CURRENT_400)
+		else if (card->host->caps & MMC_CAP_MAX_CURRENT_400_330)
 			current_limit = SD_SET_CURRENT_LIMIT_400;
-		else if (card->host->caps & MMC_CAP_MAX_CURRENT_200)
+		else if (card->host->caps & MMC_CAP_MAX_CURRENT_200_330)
 			current_limit = SD_SET_CURRENT_LIMIT_200;
 	}
 
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 3ec4182..d89e97c 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2913,6 +2913,16 @@ int sdhci_add_host(struct sdhci_host *host)
 
 		if (max_current_330 > 150)
 			mmc->caps |= MMC_CAP_SET_XPC_330;
+
+		/* Maximum current capabilities of the host at 3.3V */
+		if (max_current_330 >= 800)
+			mmc->caps |= MMC_CAP_MAX_CURRENT_800_330;
+		else if (max_current_330 >= 600)
+			mmc->caps |= MMC_CAP_MAX_CURRENT_600_330;
+		else if (max_current_330 >= 400)
+			mmc->caps |= MMC_CAP_MAX_CURRENT_400_330;
+		else if (max_current_330 >= 200)
+			mmc->caps |= MMC_CAP_MAX_CURRENT_200_330;
 	}
 	if (caps[0] & SDHCI_CAN_VDD_300) {
 		int max_current_300;
@@ -2926,6 +2936,16 @@ int sdhci_add_host(struct sdhci_host *host)
 
 		if (max_current_300 > 150)
 			mmc->caps |= MMC_CAP_SET_XPC_300;
+
+		/* Maximum current capabilities of the host at 3.0V */
+		if (max_current_300 >= 800)
+			mmc->caps |= MMC_CAP_MAX_CURRENT_800_300;
+		else if (max_current_300 >= 600)
+			mmc->caps |= MMC_CAP_MAX_CURRENT_600_300;
+		else if (max_current_300 >= 400)
+			mmc->caps |= MMC_CAP_MAX_CURRENT_400_300;
+		else if (max_current_300 >= 200)
+			mmc->caps |= MMC_CAP_MAX_CURRENT_200_300;
 	}
 	if (caps[0] & SDHCI_CAN_VDD_180) {
 		int max_current_180;
@@ -2942,13 +2962,13 @@ int sdhci_add_host(struct sdhci_host *host)
 
 		/* Maximum current capabilities of the host at 1.8V */
 		if (max_current_180 >= 800)
-			mmc->caps |= MMC_CAP_MAX_CURRENT_800;
+			mmc->caps |= MMC_CAP_MAX_CURRENT_800_180;
 		else if (max_current_180 >= 600)
-			mmc->caps |= MMC_CAP_MAX_CURRENT_600;
+			mmc->caps |= MMC_CAP_MAX_CURRENT_600_180;
 		else if (max_current_180 >= 400)
-			mmc->caps |= MMC_CAP_MAX_CURRENT_400;
+			mmc->caps |= MMC_CAP_MAX_CURRENT_400_180;
 		else if (max_current_180 >= 200)
-			mmc->caps |= MMC_CAP_MAX_CURRENT_200;
+			mmc->caps |= MMC_CAP_MAX_CURRENT_200_180;
 	}
 
 	mmc->ocr_avail = ocr_avail;
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 65c64ee..ca84ffb 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -238,10 +238,10 @@ struct mmc_host {
 #define MMC_CAP_DRIVER_TYPE_A	(1 << 23)	/* Host supports Driver Type A */
 #define MMC_CAP_DRIVER_TYPE_C	(1 << 24)	/* Host supports Driver Type C */
 #define MMC_CAP_DRIVER_TYPE_D	(1 << 25)	/* Host supports Driver Type D */
-#define MMC_CAP_MAX_CURRENT_200	(1 << 26)	/* Host max current limit is 200mA */
-#define MMC_CAP_MAX_CURRENT_400	(1 << 27)	/* Host max current limit is 400mA */
-#define MMC_CAP_MAX_CURRENT_600	(1 << 28)	/* Host max current limit is 600mA */
-#define MMC_CAP_MAX_CURRENT_800	(1 << 29)	/* Host max current limit is 800mA */
+#define MMC_CAP_MAX_CURRENT_200_180 (1 << 26)	/* Host max current limit is 200mA at 1.8V */
+#define MMC_CAP_MAX_CURRENT_400_180 (1 << 27)	/* Host max current limit is 400mA at 1.8V */
+#define MMC_CAP_MAX_CURRENT_600_180 (1 << 28)	/* Host max current limit is 600mA at 1.8V */
+#define MMC_CAP_MAX_CURRENT_800_180 (1 << 29)	/* Host max current limit is 800mA at 1.8V */
 #define MMC_CAP_CMD23		(1 << 30)	/* CMD23 supported. */
 #define MMC_CAP_HW_RESET	(1 << 31)	/* Hardware reset */
 
@@ -261,6 +261,14 @@ struct mmc_host {
 #define MMC_CAP2_HC_ERASE_SZ	(1 << 9)	/* High-capacity erase size */
 #define MMC_CAP2_CD_ACTIVE_HIGH	(1 << 10)	/* Card-detect signal active high */
 #define MMC_CAP2_RO_ACTIVE_HIGH	(1 << 11)	/* Write-protect signal active high */
+#define MMC_CAP_MAX_CURRENT_200_300 (1 << 12)	/* Host max current limit is 200mA at 3.0V */
+#define MMC_CAP_MAX_CURRENT_400_300 (1 << 13)	/* Host max current limit is 400mA at 3.0V */
+#define MMC_CAP_MAX_CURRENT_600_300 (1 << 14)	/* Host max current limit is 600mA at 3.0V */
+#define MMC_CAP_MAX_CURRENT_800_300 (1 << 15)	/* Host max current limit is 800mA at 3.0V */
+#define MMC_CAP_MAX_CURRENT_200_330 (1 << 16)	/* Host max current limit is 200mA at 3.3V */
+#define MMC_CAP_MAX_CURRENT_400_330 (1 << 17)	/* Host max current limit is 400mA at 3.3V */
+#define MMC_CAP_MAX_CURRENT_600_330 (1 << 18)	/* Host max current limit is 600mA at 3.3V */
+#define MMC_CAP_MAX_CURRENT_800_330 (1 << 19)	/* Host max current limit is 800mA at 3.3V */
 
 	mmc_pm_flag_t		pm_caps;	/* supported pm features */
 	unsigned int        power_notify_type;
-- 
1.7.11.1.3.g4c8a9db

Re: [PATCH v2 2/2] mmc: sd: Fix sd current limit setting

[Philip Rakity]

Reviewed-by: Philip Rakity <prakity@Marvell.com>

On Jul 3, 2012, at 10:31 PM, Aaron Lu wrote:

> Host has different current capabilities at different voltages, we need
> to record these settings seperately. The defined voltages are 1.8/3.0/3.3.
> For other voltages, we do not touch current limit setting.
> 
> Before set current limit for the sd card, find out the host's operating
> voltage first and then find out the current capabilities of the host at
> that voltage to set the current limit.
> 
> Signed-off-by: Aaron Lu <aaron.lu@amd.com>
> ---
> v2:
> Do not call BUG() when the host's voltage is not supported as suggested by
> Chris Ball.
> Do not use 0/1/2 to represent the host's voltage as suggested by Philip Rakity.
> 
> drivers/mmc/core/sd.c    | 44 +++++++++++++++++++++++++++++++++++++-------
> drivers/mmc/host/sdhci.c | 28 ++++++++++++++++++++++++----
> include/linux/mmc/host.h | 16 ++++++++++++----
> 3 files changed, 73 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
> index 8460568..312b78d 100644
> --- a/drivers/mmc/core/sd.c
> +++ b/drivers/mmc/core/sd.c
> @@ -521,11 +521,25 @@ static int sd_set_current_limit(struct mmc_card *card, u8 *status)
> {
> 	int current_limit = SD_SET_CURRENT_NO_CHANGE;
> 	int err;
> +	u32 voltage;
> 
> 	/*
> 	 * Current limit switch is only defined for SDR50, SDR104, and DDR50
> 	 * bus speed modes. For other bus speed modes, we do not change the
> 	 * current limit.
> +	 */
> +	if ((card->sd_bus_speed != UHS_SDR50_BUS_SPEED) &&
> +	    (card->sd_bus_speed != UHS_SDR104_BUS_SPEED) &&
> +	    (card->sd_bus_speed != UHS_DDR50_BUS_SPEED))
> +		return 0;
> +
> +	/*
> +	 * Host has different current capabilities when operating at
> +	 * different voltages, so find out the current voltage first.
> +	 */
> +	voltage = 1 << card->host->ios.vdd;
> +
> +	/*
> 	 * We only check host's capability here, if we set a limit that is
> 	 * higher than the card's maximum current, the card will be using its
> 	 * maximum current, e.g. if the card's maximum current is 300ma, and
> @@ -533,16 +547,32 @@ static int sd_set_current_limit(struct mmc_card *card, u8 *status)
> 	 * when we set current limit to 400/600/800ma, the card will draw its
> 	 * maximum 300ma from the host.
> 	 */
> -	if ((card->sd_bus_speed == UHS_SDR50_BUS_SPEED) ||
> -	    (card->sd_bus_speed == UHS_SDR104_BUS_SPEED) ||
> -	    (card->sd_bus_speed == UHS_DDR50_BUS_SPEED)) {
> -		if (card->host->caps & MMC_CAP_MAX_CURRENT_800)
> +	if (voltage == MMC_VDD_165_195) {
> +		if (card->host->caps & MMC_CAP_MAX_CURRENT_800_180)
> +			current_limit = SD_SET_CURRENT_LIMIT_800;
> +		else if (card->host->caps & MMC_CAP_MAX_CURRENT_600_180)
> +			current_limit = SD_SET_CURRENT_LIMIT_600;
> +		else if (card->host->caps & MMC_CAP_MAX_CURRENT_400_180)
> +			current_limit = SD_SET_CURRENT_LIMIT_400;
> +		else if (card->host->caps & MMC_CAP_MAX_CURRENT_200_180)
> +			current_limit = SD_SET_CURRENT_LIMIT_200;
> +	} else if (voltage & (MMC_VDD_29_30 | MMC_VDD_30_31)) {
> +		if (card->host->caps & MMC_CAP_MAX_CURRENT_800_300)
> +			current_limit = SD_SET_CURRENT_LIMIT_800;
> +		else if (card->host->caps & MMC_CAP_MAX_CURRENT_600_300)
> +			current_limit = SD_SET_CURRENT_LIMIT_600;
> +		else if (card->host->caps & MMC_CAP_MAX_CURRENT_400_300)
> +			current_limit = SD_SET_CURRENT_LIMIT_400;
> +		else if (card->host->caps & MMC_CAP_MAX_CURRENT_200_300)
> +			current_limit = SD_SET_CURRENT_LIMIT_200;
> +	} else if (voltage & (MMC_VDD_32_33 | MMC_VDD_33_34)) {
> +		if (card->host->caps & MMC_CAP_MAX_CURRENT_800_330)
> 			current_limit = SD_SET_CURRENT_LIMIT_800;
> -		else if (card->host->caps & MMC_CAP_MAX_CURRENT_600)
> +		else if (card->host->caps & MMC_CAP_MAX_CURRENT_600_330)
> 			current_limit = SD_SET_CURRENT_LIMIT_600;
> -		else if (card->host->caps & MMC_CAP_MAX_CURRENT_400)
> +		else if (card->host->caps & MMC_CAP_MAX_CURRENT_400_330)
> 			current_limit = SD_SET_CURRENT_LIMIT_400;
> -		else if (card->host->caps & MMC_CAP_MAX_CURRENT_200)
> +		else if (card->host->caps & MMC_CAP_MAX_CURRENT_200_330)
> 			current_limit = SD_SET_CURRENT_LIMIT_200;
> 	}
> 
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 3ec4182..d89e97c 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -2913,6 +2913,16 @@ int sdhci_add_host(struct sdhci_host *host)
> 
> 		if (max_current_330 > 150)
> 			mmc->caps |= MMC_CAP_SET_XPC_330;
> +
> +		/* Maximum current capabilities of the host at 3.3V */
> +		if (max_current_330 >= 800)
> +			mmc->caps |= MMC_CAP_MAX_CURRENT_800_330;
> +		else if (max_current_330 >= 600)
> +			mmc->caps |= MMC_CAP_MAX_CURRENT_600_330;
> +		else if (max_current_330 >= 400)
> +			mmc->caps |= MMC_CAP_MAX_CURRENT_400_330;
> +		else if (max_current_330 >= 200)
> +			mmc->caps |= MMC_CAP_MAX_CURRENT_200_330;
> 	}
> 	if (caps[0] & SDHCI_CAN_VDD_300) {
> 		int max_current_300;
> @@ -2926,6 +2936,16 @@ int sdhci_add_host(struct sdhci_host *host)
> 
> 		if (max_current_300 > 150)
> 			mmc->caps |= MMC_CAP_SET_XPC_300;
> +
> +		/* Maximum current capabilities of the host at 3.0V */
> +		if (max_current_300 >= 800)
> +			mmc->caps |= MMC_CAP_MAX_CURRENT_800_300;
> +		else if (max_current_300 >= 600)
> +			mmc->caps |= MMC_CAP_MAX_CURRENT_600_300;
> +		else if (max_current_300 >= 400)
> +			mmc->caps |= MMC_CAP_MAX_CURRENT_400_300;
> +		else if (max_current_300 >= 200)
> +			mmc->caps |= MMC_CAP_MAX_CURRENT_200_300;
> 	}
> 	if (caps[0] & SDHCI_CAN_VDD_180) {
> 		int max_current_180;
> @@ -2942,13 +2962,13 @@ int sdhci_add_host(struct sdhci_host *host)
> 
> 		/* Maximum current capabilities of the host at 1.8V */
> 		if (max_current_180 >= 800)
> -			mmc->caps |= MMC_CAP_MAX_CURRENT_800;
> +			mmc->caps |= MMC_CAP_MAX_CURRENT_800_180;
> 		else if (max_current_180 >= 600)
> -			mmc->caps |= MMC_CAP_MAX_CURRENT_600;
> +			mmc->caps |= MMC_CAP_MAX_CURRENT_600_180;
> 		else if (max_current_180 >= 400)
> -			mmc->caps |= MMC_CAP_MAX_CURRENT_400;
> +			mmc->caps |= MMC_CAP_MAX_CURRENT_400_180;
> 		else if (max_current_180 >= 200)
> -			mmc->caps |= MMC_CAP_MAX_CURRENT_200;
> +			mmc->caps |= MMC_CAP_MAX_CURRENT_200_180;
> 	}
> 
> 	mmc->ocr_avail = ocr_avail;
> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index 65c64ee..ca84ffb 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -238,10 +238,10 @@ struct mmc_host {
> #define MMC_CAP_DRIVER_TYPE_A	(1 << 23)	/* Host supports Driver Type A */
> #define MMC_CAP_DRIVER_TYPE_C	(1 << 24)	/* Host supports Driver Type C */
> #define MMC_CAP_DRIVER_TYPE_D	(1 << 25)	/* Host supports Driver Type D */
> -#define MMC_CAP_MAX_CURRENT_200	(1 << 26)	/* Host max current limit is 200mA */
> -#define MMC_CAP_MAX_CURRENT_400	(1 << 27)	/* Host max current limit is 400mA */
> -#define MMC_CAP_MAX_CURRENT_600	(1 << 28)	/* Host max current limit is 600mA */
> -#define MMC_CAP_MAX_CURRENT_800	(1 << 29)	/* Host max current limit is 800mA */
> +#define MMC_CAP_MAX_CURRENT_200_180 (1 << 26)	/* Host max current limit is 200mA at 1.8V */
> +#define MMC_CAP_MAX_CURRENT_400_180 (1 << 27)	/* Host max current limit is 400mA at 1.8V */
> +#define MMC_CAP_MAX_CURRENT_600_180 (1 << 28)	/* Host max current limit is 600mA at 1.8V */
> +#define MMC_CAP_MAX_CURRENT_800_180 (1 << 29)	/* Host max current limit is 800mA at 1.8V */
> #define MMC_CAP_CMD23		(1 << 30)	/* CMD23 supported. */
> #define MMC_CAP_HW_RESET	(1 << 31)	/* Hardware reset */
> 
> @@ -261,6 +261,14 @@ struct mmc_host {
> #define MMC_CAP2_HC_ERASE_SZ	(1 << 9)	/* High-capacity erase size */
> #define MMC_CAP2_CD_ACTIVE_HIGH	(1 << 10)	/* Card-detect signal active high */
> #define MMC_CAP2_RO_ACTIVE_HIGH	(1 << 11)	/* Write-protect signal active high */
> +#define MMC_CAP_MAX_CURRENT_200_300 (1 << 12)	/* Host max current limit is 200mA at 3.0V */
> +#define MMC_CAP_MAX_CURRENT_400_300 (1 << 13)	/* Host max current limit is 400mA at 3.0V */
> +#define MMC_CAP_MAX_CURRENT_600_300 (1 << 14)	/* Host max current limit is 600mA at 3.0V */
> +#define MMC_CAP_MAX_CURRENT_800_300 (1 << 15)	/* Host max current limit is 800mA at 3.0V */
> +#define MMC_CAP_MAX_CURRENT_200_330 (1 << 16)	/* Host max current limit is 200mA at 3.3V */
> +#define MMC_CAP_MAX_CURRENT_400_330 (1 << 17)	/* Host max current limit is 400mA at 3.3V */
> +#define MMC_CAP_MAX_CURRENT_600_330 (1 << 18)	/* Host max current limit is 600mA at 3.3V */
> +#define MMC_CAP_MAX_CURRENT_800_330 (1 << 19)	/* Host max current limit is 800mA at 3.3V */
> 
> 	mmc_pm_flag_t		pm_caps;	/* supported pm features */
> 	unsigned int        power_notify_type;
> -- 
> 1.7.11.1.3.g4c8a9db
> 

Re: [PATCH v2 2/2] mmc: sd: Fix sd current limit setting

[Chris Ball]

Hi,

On Wed, Jul 04 2012, Aaron Lu wrote:
> Host has different current capabilities at different voltages, we need
> to record these settings seperately. The defined voltages are 1.8/3.0/3.3.
> For other voltages, we do not touch current limit setting.
>
> Before set current limit for the sd card, find out the host's operating
> voltage first and then find out the current capabilities of the host at
> that voltage to set the current limit.
>
> Signed-off-by: Aaron Lu <aaron.lu@amd.com>
> ---
> v2:
> Do not call BUG() when the host's voltage is not supported as suggested by
> Chris Ball.
> Do not use 0/1/2 to represent the host's voltage as suggested by Philip Rakity.

Thanks, pushed to mmc-next for 3.6.

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

Re: [PATCH 2/2] mmc: sd: Fix sd current limit setting

[Girish K S]

On 3 July 2012 11:46, Aaron Lu <aaron.lu@amd.com> wrote:
> Host has different current capabilities at different voltages, we need
> to record these settings seperately. Before set current limit for the sd
> card, find out the current voltage first and then find out the current
> capabilities of the host to set the limit.
>
> Signed-off-by: Aaron Lu <aaron.lu@amd.com>
> ---
>  drivers/mmc/core/sd.c    | 58 ++++++++++++++++++++++++++++++++++++++++++------
>  drivers/mmc/host/sdhci.c | 28 +++++++++++++++++++----
>  include/linux/mmc/host.h | 16 +++++++++----
>  3 files changed, 87 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
> index ae72d6e..4b4cf4d 100644
> --- a/drivers/mmc/core/sd.c
> +++ b/drivers/mmc/core/sd.c
> @@ -521,11 +521,39 @@ static int sd_set_current_limit(struct mmc_card *card, u8 *status)
>  {
>         int current_limit = SD_SET_CURRENT_NO_CHANGE;
>         int err;
> +       u8 voltage;
>
>         /*
>          * Current limit switch is only defined for SDR50, SDR104, and DDR50
>          * bus speed modes. For other bus speed modes, we do not change the
>          * current limit.
> +        */
> +       if ((card->sd_bus_speed != UHS_SDR50_BUS_SPEED) &&
> +           (card->sd_bus_speed != UHS_SDR104_BUS_SPEED) &&
> +           (card->sd_bus_speed != UHS_DDR50_BUS_SPEED))
> +               return 0;
> +
> +       /*
> +        * Host has different current capabilities when operating at
> +        * different voltages, so find out the current voltage first.
> +        */
> +       switch (1 << card->host->ios.vdd) {
> +               case MMC_VDD_165_195:
> +                       voltage = 0; /* host's voltage is 1.8V */
> +                       break;
> +               case MMC_VDD_29_30:
> +               case MMC_VDD_30_31:
> +                       voltage = 1; /* host's voltage is 3.0V */
> +                       break;
> +               case MMC_VDD_32_33:
> +               case MMC_VDD_33_34:
> +                       voltage = 2; /* host's voltage is 3.3V */
> +                       break;
> +               default:
> +                       BUG(); /* host's voltage is invalid */
> +       }
> +
> +       /*
>          * We only check host's capability here, if we set a limit that is
>          * higher than the card's maximum current, the card will be using its
>          * maximum current, e.g. if the card's maximum current is 300ma, and
> @@ -533,16 +561,32 @@ static int sd_set_current_limit(struct mmc_card *card, u8 *status)
>          * when we set current limit to 400/600/800ma, the card will draw its
>          * maximum 300ma from the host.
>          */
> -       if ((card->sd_bus_speed == UHS_SDR50_BUS_SPEED) ||
> -           (card->sd_bus_speed == UHS_SDR104_BUS_SPEED) ||
> -           (card->sd_bus_speed == UHS_DDR50_BUS_SPEED)) {
> -               if (card->host->caps & MMC_CAP_MAX_CURRENT_800)
> +       if (voltage == 0) {
> +               if (card->host->caps & MMC_CAP_MAX_CURRENT_800_180)
> +                       current_limit = SD_SET_CURRENT_LIMIT_800;
> +               else if (card->host->caps & MMC_CAP_MAX_CURRENT_600_180)
> +                       current_limit = SD_SET_CURRENT_LIMIT_600;
> +               else if (card->host->caps & MMC_CAP_MAX_CURRENT_400_180)
> +                       current_limit = SD_SET_CURRENT_LIMIT_400;
> +               else if (card->host->caps & MMC_CAP_MAX_CURRENT_200_180)
> +                       current_limit = SD_SET_CURRENT_LIMIT_200;
> +       } else if (voltage == 1) {
> +               if (card->host->caps & MMC_CAP_MAX_CURRENT_800_300)
> +                       current_limit = SD_SET_CURRENT_LIMIT_800;
> +               else if (card->host->caps & MMC_CAP_MAX_CURRENT_600_300)
> +                       current_limit = SD_SET_CURRENT_LIMIT_600;
> +               else if (card->host->caps & MMC_CAP_MAX_CURRENT_400_300)
> +                       current_limit = SD_SET_CURRENT_LIMIT_400;
> +               else if (card->host->caps & MMC_CAP_MAX_CURRENT_200_300)
> +                       current_limit = SD_SET_CURRENT_LIMIT_200;
> +       } else if (voltage == 2) {
> +               if (card->host->caps & MMC_CAP_MAX_CURRENT_800_330)
>                         current_limit = SD_SET_CURRENT_LIMIT_800;
> -               else if (card->host->caps & MMC_CAP_MAX_CURRENT_600)
> +               else if (card->host->caps & MMC_CAP_MAX_CURRENT_600_330)
>                         current_limit = SD_SET_CURRENT_LIMIT_600;
> -               else if (card->host->caps & MMC_CAP_MAX_CURRENT_400)
> +               else if (card->host->caps & MMC_CAP_MAX_CURRENT_400_330)
>                         current_limit = SD_SET_CURRENT_LIMIT_400;
> -               else if (card->host->caps & MMC_CAP_MAX_CURRENT_200)
> +               else if (card->host->caps & MMC_CAP_MAX_CURRENT_200_330)
>                         current_limit = SD_SET_CURRENT_LIMIT_200;
>         }
>
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index caba999..00c2cbb 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -2897,6 +2897,16 @@ int sdhci_add_host(struct sdhci_host *host)
>
>                 if (max_current_330 > 150)
>                         mmc->caps |= MMC_CAP_SET_XPC_330;
> +
> +               /* Maximum current capabilities of the host at 3.3V */
> +               if (max_current_330 >= 800)
> +                       mmc->caps |= MMC_CAP_MAX_CURRENT_800_330;
> +               else if (max_current_330 >= 600)
> +                       mmc->caps |= MMC_CAP_MAX_CURRENT_600_330;
> +               else if (max_current_330 >= 400)
> +                       mmc->caps |= MMC_CAP_MAX_CURRENT_400_330;
> +               else if (max_current_330 >= 200)
> +                       mmc->caps |= MMC_CAP_MAX_CURRENT_200_330;
>         }
>         if (caps[0] & SDHCI_CAN_VDD_300) {
>                 int max_current_300;
> @@ -2910,6 +2920,16 @@ int sdhci_add_host(struct sdhci_host *host)
>
>                 if (max_current_300 > 150)
>                         mmc->caps |= MMC_CAP_SET_XPC_300;
> +
> +               /* Maximum current capabilities of the host at 3.0V */
> +               if (max_current_300 >= 800)
> +                       mmc->caps |= MMC_CAP_MAX_CURRENT_800_300;
> +               else if (max_current_300 >= 600)
> +                       mmc->caps |= MMC_CAP_MAX_CURRENT_600_300;
> +               else if (max_current_300 >= 400)
> +                       mmc->caps |= MMC_CAP_MAX_CURRENT_400_300;
> +               else if (max_current_300 >= 200)
> +                       mmc->caps |= MMC_CAP_MAX_CURRENT_200_300;
>         }
>         if (caps[0] & SDHCI_CAN_VDD_180) {
>                 int max_current_180;
> @@ -2926,13 +2946,13 @@ int sdhci_add_host(struct sdhci_host *host)
>
>                 /* Maximum current capabilities of the host at 1.8V */
>                 if (max_current_180 >= 800)
> -                       mmc->caps |= MMC_CAP_MAX_CURRENT_800;
> +                       mmc->caps |= MMC_CAP_MAX_CURRENT_800_180;
>                 else if (max_current_180 >= 600)
> -                       mmc->caps |= MMC_CAP_MAX_CURRENT_600;
> +                       mmc->caps |= MMC_CAP_MAX_CURRENT_600_180;
>                 else if (max_current_180 >= 400)
> -                       mmc->caps |= MMC_CAP_MAX_CURRENT_400;
> +                       mmc->caps |= MMC_CAP_MAX_CURRENT_400_180;
>                 else if (max_current_180 >= 200)
> -                       mmc->caps |= MMC_CAP_MAX_CURRENT_200;
> +                       mmc->caps |= MMC_CAP_MAX_CURRENT_200_180;
>         }
>
>         mmc->ocr_avail = ocr_avail;
> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index 65c64ee..ca84ffb 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -238,10 +238,10 @@ struct mmc_host {
>  #define MMC_CAP_DRIVER_TYPE_A  (1 << 23)       /* Host supports Driver Type A */
>  #define MMC_CAP_DRIVER_TYPE_C  (1 << 24)       /* Host supports Driver Type C */
>  #define MMC_CAP_DRIVER_TYPE_D  (1 << 25)       /* Host supports Driver Type D */
> -#define MMC_CAP_MAX_CURRENT_200        (1 << 26)       /* Host max current limit is 200mA */
> -#define MMC_CAP_MAX_CURRENT_400        (1 << 27)       /* Host max current limit is 400mA */
> -#define MMC_CAP_MAX_CURRENT_600        (1 << 28)       /* Host max current limit is 600mA */
> -#define MMC_CAP_MAX_CURRENT_800        (1 << 29)       /* Host max current limit is 800mA */
> +#define MMC_CAP_MAX_CURRENT_200_180 (1 << 26)  /* Host max current limit is 200mA at 1.8V */
> +#define MMC_CAP_MAX_CURRENT_400_180 (1 << 27)  /* Host max current limit is 400mA at 1.8V */
> +#define MMC_CAP_MAX_CURRENT_600_180 (1 << 28)  /* Host max current limit is 600mA at 1.8V */
> +#define MMC_CAP_MAX_CURRENT_800_180 (1 << 29)  /* Host max current limit is 800mA at 1.8V */
>  #define MMC_CAP_CMD23          (1 << 30)       /* CMD23 supported. */
>  #define MMC_CAP_HW_RESET       (1 << 31)       /* Hardware reset */
>
> @@ -261,6 +261,14 @@ struct mmc_host {
>  #define MMC_CAP2_HC_ERASE_SZ   (1 << 9)        /* High-capacity erase size */
>  #define MMC_CAP2_CD_ACTIVE_HIGH        (1 << 10)       /* Card-detect signal active high */
>  #define MMC_CAP2_RO_ACTIVE_HIGH        (1 << 11)       /* Write-protect signal active high */
> +#define MMC_CAP_MAX_CURRENT_200_300 (1 << 12)  /* Host max current limit is 200mA at 3.0V */
> +#define MMC_CAP_MAX_CURRENT_400_300 (1 << 13)  /* Host max current limit is 400mA at 3.0V */
> +#define MMC_CAP_MAX_CURRENT_600_300 (1 << 14)  /* Host max current limit is 600mA at 3.0V */
> +#define MMC_CAP_MAX_CURRENT_800_300 (1 << 15)  /* Host max current limit is 800mA at 3.0V */
> +#define MMC_CAP_MAX_CURRENT_200_330 (1 << 16)  /* Host max current limit is 200mA at 3.3V */
> +#define MMC_CAP_MAX_CURRENT_400_330 (1 << 17)  /* Host max current limit is 400mA at 3.3V */
> +#define MMC_CAP_MAX_CURRENT_600_330 (1 << 18)  /* Host max current limit is 600mA at 3.3V */
> +#define MMC_CAP_MAX_CURRENT_800_330 (1 << 19)  /* Host max current limit is 800mA at 3.3V */
sorry for seeing this late.
The macro name should be **_CAP2_***. This is already merged in
chris's mmc-next branch,
Chris,
Is it possible to change the macro name in the mmc-next branch for this patch?

>
>         mmc_pm_flag_t           pm_caps;        /* supported pm features */
>         unsigned int        power_notify_type;
> --
> 1.7.11.1.3.g4c8a9db
>
>
> --
> 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 2/2] mmc: sd: Fix sd current limit setting

[Aaron Lu]

On Tue, Jul 17, 2012 at 03:04:05PM +0530, Girish K S wrote:
> > @@ -261,6 +261,14 @@ struct mmc_host {
> >  #define MMC_CAP2_HC_ERASE_SZ   (1 << 9)        /* High-capacity erase size */
> >  #define MMC_CAP2_CD_ACTIVE_HIGH        (1 << 10)       /* Card-detect signal active high */
> >  #define MMC_CAP2_RO_ACTIVE_HIGH        (1 << 11)       /* Write-protect signal active high */
> > +#define MMC_CAP_MAX_CURRENT_200_300 (1 << 12)  /* Host max current limit is 200mA at 3.0V */
> > +#define MMC_CAP_MAX_CURRENT_400_300 (1 << 13)  /* Host max current limit is 400mA at 3.0V */
> > +#define MMC_CAP_MAX_CURRENT_600_300 (1 << 14)  /* Host max current limit is 600mA at 3.0V */
> > +#define MMC_CAP_MAX_CURRENT_800_300 (1 << 15)  /* Host max current limit is 800mA at 3.0V */
> > +#define MMC_CAP_MAX_CURRENT_200_330 (1 << 16)  /* Host max current limit is 200mA at 3.3V */
> > +#define MMC_CAP_MAX_CURRENT_400_330 (1 << 17)  /* Host max current limit is 400mA at 3.3V */
> > +#define MMC_CAP_MAX_CURRENT_600_330 (1 << 18)  /* Host max current limit is 600mA at 3.3V */
> > +#define MMC_CAP_MAX_CURRENT_800_330 (1 << 19)  /* Host max current limit is 800mA at 3.3V */
> sorry for seeing this late.
> The macro name should be **_CAP2_***.

Thanks for pointing this out, I'll be more careful next time.

> This is already merged in chris's mmc-next branch,
> Chris,
> Is it possible to change the macro name in the mmc-next branch for this patch?

Chris, please let me know how you want to deal with this and I'll be
glad to do whatever I can to fix this. Sorry for the trouble.

-Aaron

Re: [PATCH 2/2] mmc: sd: Fix sd current limit setting

[Chris Ball]

Hi,

On Tue, Jul 17 2012, Aaron Lu wrote:
>> sorry for seeing this late.
>> The macro name should be **_CAP2_***.
>
> Chris, please let me know how you want to deal with this and I'll be
> glad to do whatever I can to fix this. Sorry for the trouble.

No worries, I can rebase it in.  Mind sending a patch on top of current
mmc-next for me to squash on top of the previous one?

Thanks,

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

Re: [PATCH 2/2] mmc: sd: Fix sd current limit setting

[Aaron Lu]

Hi,

On Tue, Jul 17, 2012 at 11:43:26AM -0400, Chris Ball wrote:
> >
> > Chris, please let me know how you want to deal with this and I'll be
> > glad to do whatever I can to fix this. Sorry for the trouble.
> 
> No worries, I can rebase it in.  Mind sending a patch on top of current
> mmc-next for me to squash on top of the previous one?

Is the following patch OK? This is based on top of current mmc-next with
the previous one in tree. Not sure if this is what you want though.

diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
index 312b78d..ec03d15 100644
--- a/drivers/mmc/core/sd.c
+++ b/drivers/mmc/core/sd.c
@@ -557,22 +557,22 @@ static int sd_set_current_limit(struct mmc_card *card, u8 *status)
 		else if (card->host->caps & MMC_CAP_MAX_CURRENT_200_180)
 			current_limit = SD_SET_CURRENT_LIMIT_200;
 	} else if (voltage & (MMC_VDD_29_30 | MMC_VDD_30_31)) {
-		if (card->host->caps & MMC_CAP_MAX_CURRENT_800_300)
+		if (card->host->caps2 & MMC_CAP2_MAX_CURRENT_800_300)
 			current_limit = SD_SET_CURRENT_LIMIT_800;
-		else if (card->host->caps & MMC_CAP_MAX_CURRENT_600_300)
+		else if (card->host->caps2 & MMC_CAP2_MAX_CURRENT_600_300)
 			current_limit = SD_SET_CURRENT_LIMIT_600;
-		else if (card->host->caps & MMC_CAP_MAX_CURRENT_400_300)
+		else if (card->host->caps2 & MMC_CAP2_MAX_CURRENT_400_300)
 			current_limit = SD_SET_CURRENT_LIMIT_400;
-		else if (card->host->caps & MMC_CAP_MAX_CURRENT_200_300)
+		else if (card->host->caps2 & MMC_CAP2_MAX_CURRENT_200_300)
 			current_limit = SD_SET_CURRENT_LIMIT_200;
 	} else if (voltage & (MMC_VDD_32_33 | MMC_VDD_33_34)) {
-		if (card->host->caps & MMC_CAP_MAX_CURRENT_800_330)
+		if (card->host->caps2 & MMC_CAP2_MAX_CURRENT_800_330)
 			current_limit = SD_SET_CURRENT_LIMIT_800;
-		else if (card->host->caps & MMC_CAP_MAX_CURRENT_600_330)
+		else if (card->host->caps2 & MMC_CAP2_MAX_CURRENT_600_330)
 			current_limit = SD_SET_CURRENT_LIMIT_600;
-		else if (card->host->caps & MMC_CAP_MAX_CURRENT_400_330)
+		else if (card->host->caps2 & MMC_CAP2_MAX_CURRENT_400_330)
 			current_limit = SD_SET_CURRENT_LIMIT_400;
-		else if (card->host->caps & MMC_CAP_MAX_CURRENT_200_330)
+		else if (card->host->caps2 & MMC_CAP2_MAX_CURRENT_200_330)
 			current_limit = SD_SET_CURRENT_LIMIT_200;
 	}
 
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 455a093..29d4357 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2920,13 +2920,13 @@ int sdhci_add_host(struct sdhci_host *host)
 
 		/* Maximum current capabilities of the host at 3.3V */
 		if (max_current_330 >= 800)
-			mmc->caps |= MMC_CAP_MAX_CURRENT_800_330;
+			mmc->caps2 |= MMC_CAP2_MAX_CURRENT_800_330;
 		else if (max_current_330 >= 600)
-			mmc->caps |= MMC_CAP_MAX_CURRENT_600_330;
+			mmc->caps2 |= MMC_CAP2_MAX_CURRENT_600_330;
 		else if (max_current_330 >= 400)
-			mmc->caps |= MMC_CAP_MAX_CURRENT_400_330;
+			mmc->caps2 |= MMC_CAP2_MAX_CURRENT_400_330;
 		else if (max_current_330 >= 200)
-			mmc->caps |= MMC_CAP_MAX_CURRENT_200_330;
+			mmc->caps2 |= MMC_CAP2_MAX_CURRENT_200_330;
 	}
 	if (caps[0] & SDHCI_CAN_VDD_300) {
 		int max_current_300;
@@ -2943,13 +2943,13 @@ int sdhci_add_host(struct sdhci_host *host)
 
 		/* Maximum current capabilities of the host at 3.0V */
 		if (max_current_300 >= 800)
-			mmc->caps |= MMC_CAP_MAX_CURRENT_800_300;
+			mmc->caps2 |= MMC_CAP2_MAX_CURRENT_800_300;
 		else if (max_current_300 >= 600)
-			mmc->caps |= MMC_CAP_MAX_CURRENT_600_300;
+			mmc->caps2 |= MMC_CAP2_MAX_CURRENT_600_300;
 		else if (max_current_300 >= 400)
-			mmc->caps |= MMC_CAP_MAX_CURRENT_400_300;
+			mmc->caps2 |= MMC_CAP2_MAX_CURRENT_400_300;
 		else if (max_current_300 >= 200)
-			mmc->caps |= MMC_CAP_MAX_CURRENT_200_300;
+			mmc->caps2 |= MMC_CAP2_MAX_CURRENT_200_300;
 	}
 	if (caps[0] & SDHCI_CAN_VDD_180) {
 		int max_current_180;
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 79d8921..4a40312 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -261,14 +261,14 @@ struct mmc_host {
 #define MMC_CAP2_HC_ERASE_SZ	(1 << 9)	/* High-capacity erase size */
 #define MMC_CAP2_CD_ACTIVE_HIGH	(1 << 10)	/* Card-detect signal active high */
 #define MMC_CAP2_RO_ACTIVE_HIGH	(1 << 11)	/* Write-protect signal active high */
-#define MMC_CAP_MAX_CURRENT_200_300 (1 << 12)	/* Host max current limit is 200mA at 3.0V */
-#define MMC_CAP_MAX_CURRENT_400_300 (1 << 13)	/* Host max current limit is 400mA at 3.0V */
-#define MMC_CAP_MAX_CURRENT_600_300 (1 << 14)	/* Host max current limit is 600mA at 3.0V */
-#define MMC_CAP_MAX_CURRENT_800_300 (1 << 15)	/* Host max current limit is 800mA at 3.0V */
-#define MMC_CAP_MAX_CURRENT_200_330 (1 << 16)	/* Host max current limit is 200mA at 3.3V */
-#define MMC_CAP_MAX_CURRENT_400_330 (1 << 17)	/* Host max current limit is 400mA at 3.3V */
-#define MMC_CAP_MAX_CURRENT_600_330 (1 << 18)	/* Host max current limit is 600mA at 3.3V */
-#define MMC_CAP_MAX_CURRENT_800_330 (1 << 19)	/* Host max current limit is 800mA at 3.3V */
+#define MMC_CAP2_MAX_CURRENT_200_300 (1 << 12)	/* Host max current limit is 200mA at 3.0V */
+#define MMC_CAP2_MAX_CURRENT_400_300 (1 << 13)	/* Host max current limit is 400mA at 3.0V */
+#define MMC_CAP2_MAX_CURRENT_600_300 (1 << 14)	/* Host max current limit is 600mA at 3.0V */
+#define MMC_CAP2_MAX_CURRENT_800_300 (1 << 15)	/* Host max current limit is 800mA at 3.0V */
+#define MMC_CAP2_MAX_CURRENT_200_330 (1 << 16)	/* Host max current limit is 200mA at 3.3V */
+#define MMC_CAP2_MAX_CURRENT_400_330 (1 << 17)	/* Host max current limit is 400mA at 3.3V */
+#define MMC_CAP2_MAX_CURRENT_600_330 (1 << 18)	/* Host max current limit is 600mA at 3.3V */
+#define MMC_CAP2_MAX_CURRENT_800_330 (1 << 19)	/* Host max current limit is 800mA at 3.3V */
 #define MMC_CAP2_PACKED_RD	    (1 << 20)	/* Allow packed read */
 #define MMC_CAP2_PACKED_WR	    (1 << 21)	/* Allow packed write */
 #define MMC_CAP2_PACKED_CMD	(MMC_CAP2_PACKED_RD | \



Thanks,
Aaron

Re: [PATCH 2/2] mmc: sd: Fix sd current limit setting

[Chris Ball]

Hi Aaron,

On Wed, Jul 18 2012, Aaron Lu wrote:
> Is the following patch OK? This is based on top of current mmc-next with
> the previous one in tree. Not sure if this is what you want though.

Yes, that's perfect; squashed into the original patch and pushed out
to mmc-next.  Thanks!

Having there be so many MAX_CURRENT defines -- and having them be
split in the middle between CAP_ and CAP2_ -- is starting to feel
a bit awkward.  Does anyone have ideas on how that might be tidied
up, since we have an opportunity to come up with a better plan
before this gets merged soon?

Thanks,

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

Re: [PATCH 2/2] mmc: sd: Fix sd current limit setting

[Aaron Lu]

Hi Chris,

On Wed, Jul 18, 2012 at 01:28:40AM -0400, Chris Ball wrote:
> Hi Aaron,
> 
> On Wed, Jul 18 2012, Aaron Lu wrote:
> > Is the following patch OK? This is based on top of current mmc-next with
> > the previous one in tree. Not sure if this is what you want though.
> 
> Yes, that's perfect; squashed into the original patch and pushed out
> to mmc-next.  Thanks!
> 
> Having there be so many MAX_CURRENT defines -- and having them be
> split in the middle between CAP_ and CAP2_ -- is starting to feel
> a bit awkward.

I agree.

> Does anyone have ideas on how that might be tidied up,
> since we have an opportunity to come up with a better plan
> before this gets merged soon?

What about we add three fields in mmc_host to store the max current
value for 3.3v/3.0v/1.8v and use that when needed instead of the cap
setting of the host?

I've prepared the following code, please check if this is better than
the current one:

diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
index 312b78d..2232004 100644
--- a/drivers/mmc/core/sd.c
+++ b/drivers/mmc/core/sd.c
@@ -517,11 +517,36 @@ static int sd_set_bus_speed_mode(struct mmc_card *card, u8 *status)
 	return 0;
 }
 
+/* Get host's max current setting at its current voltage */
+static u32 sd_get_host_max_current(struct mmc_host *host)
+{
+	u32 voltage, max_current;
+
+	voltage = 1 << host->ios.vdd;
+	switch (voltage) {
+	case MMC_VDD_165_195:
+		max_current = host->max_current_180;
+		break;
+	case MMC_VDD_29_30:
+	case MMC_VDD_30_31:
+		max_current = host->max_current_300;
+		break;
+	case MMC_VDD_32_33:
+	case MMC_VDD_33_34:
+		max_current = host->max_current_330;
+		break;
+	default:
+		max_current = 0;
+	}
+
+	return max_current;
+}
+
 static int sd_set_current_limit(struct mmc_card *card, u8 *status)
 {
 	int current_limit = SD_SET_CURRENT_NO_CHANGE;
 	int err;
-	u32 voltage;
+	u32 max_current;
 
 	/*
 	 * Current limit switch is only defined for SDR50, SDR104, and DDR50
@@ -535,9 +560,9 @@ static int sd_set_current_limit(struct mmc_card *card, u8 *status)
 
 	/*
 	 * Host has different current capabilities when operating at
-	 * different voltages, so find out the current voltage first.
+	 * different voltages, so find out its max current first.
 	 */
-	voltage = 1 << card->host->ios.vdd;
+	max_current = sd_get_host_max_current(card->host);
 
 	/*
 	 * We only check host's capability here, if we set a limit that is
@@ -547,34 +572,15 @@ static int sd_set_current_limit(struct mmc_card *card, u8 *status)
 	 * when we set current limit to 400/600/800ma, the card will draw its
 	 * maximum 300ma from the host.
 	 */
-	if (voltage == MMC_VDD_165_195) {
-		if (card->host->caps & MMC_CAP_MAX_CURRENT_800_180)
-			current_limit = SD_SET_CURRENT_LIMIT_800;
-		else if (card->host->caps & MMC_CAP_MAX_CURRENT_600_180)
-			current_limit = SD_SET_CURRENT_LIMIT_600;
-		else if (card->host->caps & MMC_CAP_MAX_CURRENT_400_180)
-			current_limit = SD_SET_CURRENT_LIMIT_400;
-		else if (card->host->caps & MMC_CAP_MAX_CURRENT_200_180)
-			current_limit = SD_SET_CURRENT_LIMIT_200;
-	} else if (voltage & (MMC_VDD_29_30 | MMC_VDD_30_31)) {
-		if (card->host->caps & MMC_CAP_MAX_CURRENT_800_300)
-			current_limit = SD_SET_CURRENT_LIMIT_800;
-		else if (card->host->caps & MMC_CAP_MAX_CURRENT_600_300)
-			current_limit = SD_SET_CURRENT_LIMIT_600;
-		else if (card->host->caps & MMC_CAP_MAX_CURRENT_400_300)
-			current_limit = SD_SET_CURRENT_LIMIT_400;
-		else if (card->host->caps & MMC_CAP_MAX_CURRENT_200_300)
-			current_limit = SD_SET_CURRENT_LIMIT_200;
-	} else if (voltage & (MMC_VDD_32_33 | MMC_VDD_33_34)) {
-		if (card->host->caps & MMC_CAP_MAX_CURRENT_800_330)
-			current_limit = SD_SET_CURRENT_LIMIT_800;
-		else if (card->host->caps & MMC_CAP_MAX_CURRENT_600_330)
-			current_limit = SD_SET_CURRENT_LIMIT_600;
-		else if (card->host->caps & MMC_CAP_MAX_CURRENT_400_330)
-			current_limit = SD_SET_CURRENT_LIMIT_400;
-		else if (card->host->caps & MMC_CAP_MAX_CURRENT_200_330)
-			current_limit = SD_SET_CURRENT_LIMIT_200;
-	}
+
+	if (max_current >= 800)
+		current_limit = SD_SET_CURRENT_LIMIT_800;
+	else if (max_current >= 600)
+		current_limit = SD_SET_CURRENT_LIMIT_600;
+	else if (max_current >= 400)
+		current_limit = SD_SET_CURRENT_LIMIT_400;
+	else if (max_current >= 200)
+		current_limit = SD_SET_CURRENT_LIMIT_200;
 
 	if (current_limit != SD_SET_CURRENT_NO_CHANGE) {
 		err = mmc_sd_switch(card, 1, 3, current_limit, status);
@@ -707,6 +713,7 @@ struct device_type sd_type = {
 int mmc_sd_get_cid(struct mmc_host *host, u32 ocr, u32 *cid, u32 *rocr)
 {
 	int err;
+	u32 max_current;
 
 	/*
 	 * Since we're changing the OCR value, we seem to
@@ -734,9 +741,12 @@ int mmc_sd_get_cid(struct mmc_host *host, u32 ocr, u32 *cid, u32 *rocr)
 	    MMC_CAP_UHS_SDR50 | MMC_CAP_UHS_SDR104 | MMC_CAP_UHS_DDR50))
 		ocr |= SD_OCR_S18R;
 
-	/* If the host can supply more than 150mA, XPC should be set to 1. */
-	if (host->caps & (MMC_CAP_SET_XPC_330 | MMC_CAP_SET_XPC_300 |
-	    MMC_CAP_SET_XPC_180))
+	/*
+	 * If the host can supply more than 150mA at current voltage,
+	 * XPC should be set to 1.
+	 */
+	max_current = sd_get_host_max_current(host);
+	if (max_current > 150)
 		ocr |= SD_OCR_XPC;
 
 try_again:
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 455a093..a72ad30 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2906,73 +2906,30 @@ int sdhci_add_host(struct sdhci_host *host)
 	}
 
 	if (caps[0] & SDHCI_CAN_VDD_330) {
-		int max_current_330;
-
 		ocr_avail |= MMC_VDD_32_33 | MMC_VDD_33_34;
 
-		max_current_330 = ((max_current_caps &
+		mmc->max_current_330 = ((max_current_caps &
 				   SDHCI_MAX_CURRENT_330_MASK) >>
 				   SDHCI_MAX_CURRENT_330_SHIFT) *
 				   SDHCI_MAX_CURRENT_MULTIPLIER;
 
-		if (max_current_330 > 150)
-			mmc->caps |= MMC_CAP_SET_XPC_330;
-
-		/* Maximum current capabilities of the host at 3.3V */
-		if (max_current_330 >= 800)
-			mmc->caps |= MMC_CAP_MAX_CURRENT_800_330;
-		else if (max_current_330 >= 600)
-			mmc->caps |= MMC_CAP_MAX_CURRENT_600_330;
-		else if (max_current_330 >= 400)
-			mmc->caps |= MMC_CAP_MAX_CURRENT_400_330;
-		else if (max_current_330 >= 200)
-			mmc->caps |= MMC_CAP_MAX_CURRENT_200_330;
 	}
 	if (caps[0] & SDHCI_CAN_VDD_300) {
-		int max_current_300;
-
 		ocr_avail |= MMC_VDD_29_30 | MMC_VDD_30_31;
 
-		max_current_300 = ((max_current_caps &
+		mmc->max_current_300 = ((max_current_caps &
 				   SDHCI_MAX_CURRENT_300_MASK) >>
 				   SDHCI_MAX_CURRENT_300_SHIFT) *
 				   SDHCI_MAX_CURRENT_MULTIPLIER;
 
-		if (max_current_300 > 150)
-			mmc->caps |= MMC_CAP_SET_XPC_300;
-
-		/* Maximum current capabilities of the host at 3.0V */
-		if (max_current_300 >= 800)
-			mmc->caps |= MMC_CAP_MAX_CURRENT_800_300;
-		else if (max_current_300 >= 600)
-			mmc->caps |= MMC_CAP_MAX_CURRENT_600_300;
-		else if (max_current_300 >= 400)
-			mmc->caps |= MMC_CAP_MAX_CURRENT_400_300;
-		else if (max_current_300 >= 200)
-			mmc->caps |= MMC_CAP_MAX_CURRENT_200_300;
 	}
 	if (caps[0] & SDHCI_CAN_VDD_180) {
-		int max_current_180;
-
 		ocr_avail |= MMC_VDD_165_195;
 
-		max_current_180 = ((max_current_caps &
+		mmc->max_current_180 = ((max_current_caps &
 				   SDHCI_MAX_CURRENT_180_MASK) >>
 				   SDHCI_MAX_CURRENT_180_SHIFT) *
 				   SDHCI_MAX_CURRENT_MULTIPLIER;
-
-		if (max_current_180 > 150)
-			mmc->caps |= MMC_CAP_SET_XPC_180;
-
-		/* Maximum current capabilities of the host at 1.8V */
-		if (max_current_180 >= 800)
-			mmc->caps |= MMC_CAP_MAX_CURRENT_800_180;
-		else if (max_current_180 >= 600)
-			mmc->caps |= MMC_CAP_MAX_CURRENT_600_180;
-		else if (max_current_180 >= 400)
-			mmc->caps |= MMC_CAP_MAX_CURRENT_400_180;
-		else if (max_current_180 >= 200)
-			mmc->caps |= MMC_CAP_MAX_CURRENT_200_180;
 	}
 
 	mmc->ocr_avail = ocr_avail;
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 79d8921..8d2c052 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -189,6 +189,9 @@ struct mmc_host {
 	u32			ocr_avail_sd;	/* SD-specific OCR */
 	u32			ocr_avail_mmc;	/* MMC-specific OCR */
 	struct notifier_block	pm_notify;
+	u32			max_current_330;
+	u32			max_current_300;
+	u32			max_current_180;
 
 #define MMC_VDD_165_195		0x00000080	/* VDD voltage 1.65 - 1.95 */
 #define MMC_VDD_20_21		0x00000100	/* VDD voltage 2.0 ~ 2.1 */
@@ -232,16 +235,9 @@ struct mmc_host {
 #define MMC_CAP_UHS_SDR50	(1 << 17)	/* Host supports UHS SDR50 mode */
 #define MMC_CAP_UHS_SDR104	(1 << 18)	/* Host supports UHS SDR104 mode */
 #define MMC_CAP_UHS_DDR50	(1 << 19)	/* Host supports UHS DDR50 mode */
-#define MMC_CAP_SET_XPC_330	(1 << 20)	/* Host supports >150mA current at 3.3V */
-#define MMC_CAP_SET_XPC_300	(1 << 21)	/* Host supports >150mA current at 3.0V */
-#define MMC_CAP_SET_XPC_180	(1 << 22)	/* Host supports >150mA current at 1.8V */
 #define MMC_CAP_DRIVER_TYPE_A	(1 << 23)	/* Host supports Driver Type A */
 #define MMC_CAP_DRIVER_TYPE_C	(1 << 24)	/* Host supports Driver Type C */
 #define MMC_CAP_DRIVER_TYPE_D	(1 << 25)	/* Host supports Driver Type D */
-#define MMC_CAP_MAX_CURRENT_200_180 (1 << 26)	/* Host max current limit is 200mA at 1.8V */
-#define MMC_CAP_MAX_CURRENT_400_180 (1 << 27)	/* Host max current limit is 400mA at 1.8V */
-#define MMC_CAP_MAX_CURRENT_600_180 (1 << 28)	/* Host max current limit is 600mA at 1.8V */
-#define MMC_CAP_MAX_CURRENT_800_180 (1 << 29)	/* Host max current limit is 800mA at 1.8V */
 #define MMC_CAP_CMD23		(1 << 30)	/* CMD23 supported. */
 #define MMC_CAP_HW_RESET	(1 << 31)	/* Hardware reset */
 
@@ -261,14 +257,6 @@ struct mmc_host {
 #define MMC_CAP2_HC_ERASE_SZ	(1 << 9)	/* High-capacity erase size */
 #define MMC_CAP2_CD_ACTIVE_HIGH	(1 << 10)	/* Card-detect signal active high */
 #define MMC_CAP2_RO_ACTIVE_HIGH	(1 << 11)	/* Write-protect signal active high */
-#define MMC_CAP_MAX_CURRENT_200_300 (1 << 12)	/* Host max current limit is 200mA at 3.0V */
-#define MMC_CAP_MAX_CURRENT_400_300 (1 << 13)	/* Host max current limit is 400mA at 3.0V */
-#define MMC_CAP_MAX_CURRENT_600_300 (1 << 14)	/* Host max current limit is 600mA at 3.0V */
-#define MMC_CAP_MAX_CURRENT_800_300 (1 << 15)	/* Host max current limit is 800mA at 3.0V */
-#define MMC_CAP_MAX_CURRENT_200_330 (1 << 16)	/* Host max current limit is 200mA at 3.3V */
-#define MMC_CAP_MAX_CURRENT_400_330 (1 << 17)	/* Host max current limit is 400mA at 3.3V */
-#define MMC_CAP_MAX_CURRENT_600_330 (1 << 18)	/* Host max current limit is 600mA at 3.3V */
-#define MMC_CAP_MAX_CURRENT_800_330 (1 << 19)	/* Host max current limit is 800mA at 3.3V */
 #define MMC_CAP2_PACKED_RD	    (1 << 20)	/* Allow packed read */
 #define MMC_CAP2_PACKED_WR	    (1 << 21)	/* Allow packed write */
 #define MMC_CAP2_PACKED_CMD	(MMC_CAP2_PACKED_RD | \


Thanks,
Aaron

Re: [PATCH 2/2] mmc: sd: Fix sd current limit setting

[Chris Ball]

Hi,

On Wed, Jul 18 2012, Aaron Lu wrote:
> I've prepared the following code, please check if this is better than
> the current one:

I think the diffstat speaks for itself.  :-)

 drivers/mmc/core/sd.c    |   77 ++++++++++++++++++++++++++--------------------
 drivers/mmc/host/sdhci.c |   51 ++----------------------------
 include/linux/mmc/host.h |   22 +++----------
 3 files changed, 51 insertions(+), 99 deletions(-)

Girish/Philip, does this look good to you too?  If so, I'll squash it
into the original patch.

Thanks!

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

Re: [PATCH 2/2] mmc: sd: Fix sd current limit setting

[Philip Rakity]

It is much cleaner solution then before !  I like it.

Reviewed-by: Philip Rakity <prakity@marvell.com>

On Jul 18, 2012, at 7:41 PM, Chris Ball wrote:

> Hi,
> 
> On Wed, Jul 18 2012, Aaron Lu wrote:
>> I've prepared the following code, please check if this is better than
>> the current one:
> 
> I think the diffstat speaks for itself.  :-)
> 
> drivers/mmc/core/sd.c    |   77 ++++++++++++++++++++++++++--------------------
> drivers/mmc/host/sdhci.c |   51 ++----------------------------
> include/linux/mmc/host.h |   22 +++----------
> 3 files changed, 51 insertions(+), 99 deletions(-)
> 
> Girish/Philip, does this look good to you too?  If so, I'll squash it
> into the original patch.
> 
> Thanks!
> 
> - Chris.
> -- 
> Chris Ball   <cjb@laptop.org>   <http://printf.net/>
> One Laptop Per Child

Re: [PATCH 2/2] mmc: sd: Fix sd current limit setting

[Girish K S]

On 18 July 2012 11:52, Aaron Lu <aaron.lu@amd.com> wrote:
> Hi Chris,
>
> On Wed, Jul 18, 2012 at 01:28:40AM -0400, Chris Ball wrote:
>> Hi Aaron,
>>
>> On Wed, Jul 18 2012, Aaron Lu wrote:
>> > Is the following patch OK? This is based on top of current mmc-next with
>> > the previous one in tree. Not sure if this is what you want though.
>>
>> Yes, that's perfect; squashed into the original patch and pushed out
>> to mmc-next.  Thanks!
>>
>> Having there be so many MAX_CURRENT defines -- and having them be
>> split in the middle between CAP_ and CAP2_ -- is starting to feel
>> a bit awkward.
>
> I agree.
>
>> Does anyone have ideas on how that might be tidied up,
>> since we have an opportunity to come up with a better plan
>> before this gets merged soon?
>
> What about we add three fields in mmc_host to store the max current
> value for 3.3v/3.0v/1.8v and use that when needed instead of the cap
> setting of the host?
>
> I've prepared the following code, please check if this is better than
> the current one:
>
> diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
> index 312b78d..2232004 100644
> --- a/drivers/mmc/core/sd.c
> +++ b/drivers/mmc/core/sd.c
> @@ -517,11 +517,36 @@ static int sd_set_bus_speed_mode(struct mmc_card *card, u8 *status)
>         return 0;
>  }
>
> +/* Get host's max current setting at its current voltage */
> +static u32 sd_get_host_max_current(struct mmc_host *host)
> +{
> +       u32 voltage, max_current;
> +
> +       voltage = 1 << host->ios.vdd;
> +       switch (voltage) {
> +       case MMC_VDD_165_195:
> +               max_current = host->max_current_180;
> +               break;
> +       case MMC_VDD_29_30:
> +       case MMC_VDD_30_31:
> +               max_current = host->max_current_300;
> +               break;
> +       case MMC_VDD_32_33:
> +       case MMC_VDD_33_34:
> +               max_current = host->max_current_330;
> +               break;
> +       default:
> +               max_current = 0;
> +       }
> +
> +       return max_current;
> +}
> +
>  static int sd_set_current_limit(struct mmc_card *card, u8 *status)
>  {
>         int current_limit = SD_SET_CURRENT_NO_CHANGE;
>         int err;
> -       u32 voltage;
> +       u32 max_current;
>
>         /*
>          * Current limit switch is only defined for SDR50, SDR104, and DDR50
> @@ -535,9 +560,9 @@ static int sd_set_current_limit(struct mmc_card *card, u8 *status)
>
>         /*
>          * Host has different current capabilities when operating at
> -        * different voltages, so find out the current voltage first.
> +        * different voltages, so find out its max current first.
>          */
> -       voltage = 1 << card->host->ios.vdd;
> +       max_current = sd_get_host_max_current(card->host);
>
>         /*
>          * We only check host's capability here, if we set a limit that is
> @@ -547,34 +572,15 @@ static int sd_set_current_limit(struct mmc_card *card, u8 *status)
>          * when we set current limit to 400/600/800ma, the card will draw its
>          * maximum 300ma from the host.
>          */
> -       if (voltage == MMC_VDD_165_195) {
> -               if (card->host->caps & MMC_CAP_MAX_CURRENT_800_180)
> -                       current_limit = SD_SET_CURRENT_LIMIT_800;
> -               else if (card->host->caps & MMC_CAP_MAX_CURRENT_600_180)
> -                       current_limit = SD_SET_CURRENT_LIMIT_600;
> -               else if (card->host->caps & MMC_CAP_MAX_CURRENT_400_180)
> -                       current_limit = SD_SET_CURRENT_LIMIT_400;
> -               else if (card->host->caps & MMC_CAP_MAX_CURRENT_200_180)
> -                       current_limit = SD_SET_CURRENT_LIMIT_200;
> -       } else if (voltage & (MMC_VDD_29_30 | MMC_VDD_30_31)) {
> -               if (card->host->caps & MMC_CAP_MAX_CURRENT_800_300)
> -                       current_limit = SD_SET_CURRENT_LIMIT_800;
> -               else if (card->host->caps & MMC_CAP_MAX_CURRENT_600_300)
> -                       current_limit = SD_SET_CURRENT_LIMIT_600;
> -               else if (card->host->caps & MMC_CAP_MAX_CURRENT_400_300)
> -                       current_limit = SD_SET_CURRENT_LIMIT_400;
> -               else if (card->host->caps & MMC_CAP_MAX_CURRENT_200_300)
> -                       current_limit = SD_SET_CURRENT_LIMIT_200;
> -       } else if (voltage & (MMC_VDD_32_33 | MMC_VDD_33_34)) {
> -               if (card->host->caps & MMC_CAP_MAX_CURRENT_800_330)
> -                       current_limit = SD_SET_CURRENT_LIMIT_800;
> -               else if (card->host->caps & MMC_CAP_MAX_CURRENT_600_330)
> -                       current_limit = SD_SET_CURRENT_LIMIT_600;
> -               else if (card->host->caps & MMC_CAP_MAX_CURRENT_400_330)
> -                       current_limit = SD_SET_CURRENT_LIMIT_400;
> -               else if (card->host->caps & MMC_CAP_MAX_CURRENT_200_330)
> -                       current_limit = SD_SET_CURRENT_LIMIT_200;
> -       }
> +
> +       if (max_current >= 800)
> +               current_limit = SD_SET_CURRENT_LIMIT_800;
> +       else if (max_current >= 600)
> +               current_limit = SD_SET_CURRENT_LIMIT_600;
> +       else if (max_current >= 400)
> +               current_limit = SD_SET_CURRENT_LIMIT_400;
> +       else if (max_current >= 200)
> +               current_limit = SD_SET_CURRENT_LIMIT_200;
>
>         if (current_limit != SD_SET_CURRENT_NO_CHANGE) {
>                 err = mmc_sd_switch(card, 1, 3, current_limit, status);
> @@ -707,6 +713,7 @@ struct device_type sd_type = {
>  int mmc_sd_get_cid(struct mmc_host *host, u32 ocr, u32 *cid, u32 *rocr)
>  {
>         int err;
> +       u32 max_current;
>
>         /*
>          * Since we're changing the OCR value, we seem to
> @@ -734,9 +741,12 @@ int mmc_sd_get_cid(struct mmc_host *host, u32 ocr, u32 *cid, u32 *rocr)
>             MMC_CAP_UHS_SDR50 | MMC_CAP_UHS_SDR104 | MMC_CAP_UHS_DDR50))
>                 ocr |= SD_OCR_S18R;
>
> -       /* If the host can supply more than 150mA, XPC should be set to 1. */
> -       if (host->caps & (MMC_CAP_SET_XPC_330 | MMC_CAP_SET_XPC_300 |
> -           MMC_CAP_SET_XPC_180))
> +       /*
> +        * If the host can supply more than 150mA at current voltage,
> +        * XPC should be set to 1.
> +        */
> +       max_current = sd_get_host_max_current(host);
> +       if (max_current > 150)
>                 ocr |= SD_OCR_XPC;
>
>  try_again:
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 455a093..a72ad30 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -2906,73 +2906,30 @@ int sdhci_add_host(struct sdhci_host *host)
>         }
>
>         if (caps[0] & SDHCI_CAN_VDD_330) {
> -               int max_current_330;
> -
>                 ocr_avail |= MMC_VDD_32_33 | MMC_VDD_33_34;
>
> -               max_current_330 = ((max_current_caps &
> +               mmc->max_current_330 = ((max_current_caps &
>                                    SDHCI_MAX_CURRENT_330_MASK) >>
>                                    SDHCI_MAX_CURRENT_330_SHIFT) *
>                                    SDHCI_MAX_CURRENT_MULTIPLIER;
>
> -               if (max_current_330 > 150)
> -                       mmc->caps |= MMC_CAP_SET_XPC_330;
> -
> -               /* Maximum current capabilities of the host at 3.3V */
> -               if (max_current_330 >= 800)
> -                       mmc->caps |= MMC_CAP_MAX_CURRENT_800_330;
> -               else if (max_current_330 >= 600)
> -                       mmc->caps |= MMC_CAP_MAX_CURRENT_600_330;
> -               else if (max_current_330 >= 400)
> -                       mmc->caps |= MMC_CAP_MAX_CURRENT_400_330;
> -               else if (max_current_330 >= 200)
> -                       mmc->caps |= MMC_CAP_MAX_CURRENT_200_330;
>         }
>         if (caps[0] & SDHCI_CAN_VDD_300) {
> -               int max_current_300;
> -
>                 ocr_avail |= MMC_VDD_29_30 | MMC_VDD_30_31;
>
> -               max_current_300 = ((max_current_caps &
> +               mmc->max_current_300 = ((max_current_caps &
>                                    SDHCI_MAX_CURRENT_300_MASK) >>
>                                    SDHCI_MAX_CURRENT_300_SHIFT) *
>                                    SDHCI_MAX_CURRENT_MULTIPLIER;
>
> -               if (max_current_300 > 150)
> -                       mmc->caps |= MMC_CAP_SET_XPC_300;
> -
> -               /* Maximum current capabilities of the host at 3.0V */
> -               if (max_current_300 >= 800)
> -                       mmc->caps |= MMC_CAP_MAX_CURRENT_800_300;
> -               else if (max_current_300 >= 600)
> -                       mmc->caps |= MMC_CAP_MAX_CURRENT_600_300;
> -               else if (max_current_300 >= 400)
> -                       mmc->caps |= MMC_CAP_MAX_CURRENT_400_300;
> -               else if (max_current_300 >= 200)
> -                       mmc->caps |= MMC_CAP_MAX_CURRENT_200_300;
>         }
>         if (caps[0] & SDHCI_CAN_VDD_180) {
> -               int max_current_180;
> -
>                 ocr_avail |= MMC_VDD_165_195;
>
> -               max_current_180 = ((max_current_caps &
> +               mmc->max_current_180 = ((max_current_caps &
>                                    SDHCI_MAX_CURRENT_180_MASK) >>
>                                    SDHCI_MAX_CURRENT_180_SHIFT) *
>                                    SDHCI_MAX_CURRENT_MULTIPLIER;
> -
> -               if (max_current_180 > 150)
> -                       mmc->caps |= MMC_CAP_SET_XPC_180;
> -
> -               /* Maximum current capabilities of the host at 1.8V */
> -               if (max_current_180 >= 800)
> -                       mmc->caps |= MMC_CAP_MAX_CURRENT_800_180;
> -               else if (max_current_180 >= 600)
> -                       mmc->caps |= MMC_CAP_MAX_CURRENT_600_180;
> -               else if (max_current_180 >= 400)
> -                       mmc->caps |= MMC_CAP_MAX_CURRENT_400_180;
> -               else if (max_current_180 >= 200)
> -                       mmc->caps |= MMC_CAP_MAX_CURRENT_200_180;
>         }
>
>         mmc->ocr_avail = ocr_avail;
> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index 79d8921..8d2c052 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -189,6 +189,9 @@ struct mmc_host {
>         u32                     ocr_avail_sd;   /* SD-specific OCR */
>         u32                     ocr_avail_mmc;  /* MMC-specific OCR */
>         struct notifier_block   pm_notify;
> +       u32                     max_current_330;
> +       u32                     max_current_300;
> +       u32                     max_current_180;
>
>  #define MMC_VDD_165_195                0x00000080      /* VDD voltage 1.65 - 1.95 */
>  #define MMC_VDD_20_21          0x00000100      /* VDD voltage 2.0 ~ 2.1 */
> @@ -232,16 +235,9 @@ struct mmc_host {
>  #define MMC_CAP_UHS_SDR50      (1 << 17)       /* Host supports UHS SDR50 mode */
>  #define MMC_CAP_UHS_SDR104     (1 << 18)       /* Host supports UHS SDR104 mode */
>  #define MMC_CAP_UHS_DDR50      (1 << 19)       /* Host supports UHS DDR50 mode */
> -#define MMC_CAP_SET_XPC_330    (1 << 20)       /* Host supports >150mA current at 3.3V */
> -#define MMC_CAP_SET_XPC_300    (1 << 21)       /* Host supports >150mA current at 3.0V */
> -#define MMC_CAP_SET_XPC_180    (1 << 22)       /* Host supports >150mA current at 1.8V */
>  #define MMC_CAP_DRIVER_TYPE_A  (1 << 23)       /* Host supports Driver Type A */
>  #define MMC_CAP_DRIVER_TYPE_C  (1 << 24)       /* Host supports Driver Type C */
>  #define MMC_CAP_DRIVER_TYPE_D  (1 << 25)       /* Host supports Driver Type D */
> -#define MMC_CAP_MAX_CURRENT_200_180 (1 << 26)  /* Host max current limit is 200mA at 1.8V */
> -#define MMC_CAP_MAX_CURRENT_400_180 (1 << 27)  /* Host max current limit is 400mA at 1.8V */
> -#define MMC_CAP_MAX_CURRENT_600_180 (1 << 28)  /* Host max current limit is 600mA at 1.8V */
> -#define MMC_CAP_MAX_CURRENT_800_180 (1 << 29)  /* Host max current limit is 800mA at 1.8V */
>  #define MMC_CAP_CMD23          (1 << 30)       /* CMD23 supported. */
>  #define MMC_CAP_HW_RESET       (1 << 31)       /* Hardware reset */
>
> @@ -261,14 +257,6 @@ struct mmc_host {
>  #define MMC_CAP2_HC_ERASE_SZ   (1 << 9)        /* High-capacity erase size */
>  #define MMC_CAP2_CD_ACTIVE_HIGH        (1 << 10)       /* Card-detect signal active high */
>  #define MMC_CAP2_RO_ACTIVE_HIGH        (1 << 11)       /* Write-protect signal active high */
> -#define MMC_CAP_MAX_CURRENT_200_300 (1 << 12)  /* Host max current limit is 200mA at 3.0V */
> -#define MMC_CAP_MAX_CURRENT_400_300 (1 << 13)  /* Host max current limit is 400mA at 3.0V */
> -#define MMC_CAP_MAX_CURRENT_600_300 (1 << 14)  /* Host max current limit is 600mA at 3.0V */
> -#define MMC_CAP_MAX_CURRENT_800_300 (1 << 15)  /* Host max current limit is 800mA at 3.0V */
> -#define MMC_CAP_MAX_CURRENT_200_330 (1 << 16)  /* Host max current limit is 200mA at 3.3V */
> -#define MMC_CAP_MAX_CURRENT_400_330 (1 << 17)  /* Host max current limit is 400mA at 3.3V */
> -#define MMC_CAP_MAX_CURRENT_600_330 (1 << 18)  /* Host max current limit is 600mA at 3.3V */
> -#define MMC_CAP_MAX_CURRENT_800_330 (1 << 19)  /* Host max current limit is 800mA at 3.3V */
>  #define MMC_CAP2_PACKED_RD         (1 << 20)   /* Allow packed read */
>  #define MMC_CAP2_PACKED_WR         (1 << 21)   /* Allow packed write */
>  #define MMC_CAP2_PACKED_CMD    (MMC_CAP2_PACKED_RD | \
>
Looks like a better solution than earlier one.
Reviewed By: Girish K S <girish.shivananjappa@linaro.org>

Chris,
How about the holes created by removal of caps2 macros. Will they be
adjusted to follow the sequence or left as it is for future use.
>
> Thanks,
> Aaron
>

Re: [PATCH 2/2] mmc: sd: Fix sd current limit setting

[Chris Ball]

Hi,

On Thu, Jul 19 2012, Girish K S wrote:
> Looks like a better solution than earlier one.
> Reviewed By: Girish K S <girish.shivananjappa@linaro.org>

Thanks.

> Chris,
> How about the holes created by removal of caps2 macros. Will they be
> adjusted to follow the sequence or left as it is for future use.

There actually isn't a hole in caps2, since yesterday I told Seungwon
and Maya that I think the packed-write patches are not ready for 3.6 and
probably should wait another cycle:

http://marc.info/?l=linux-mmc&m=134259666416161&w=2

I'll be removing them from mmc-next, unless something changes.
There aren't any other caps creating a hole in caps2.

There's a hole in caps1, though.  I think we'll just wait for the next
patches that want caps to take the place of the hole in caps1 without
renumbering any extant caps.  Let me know if you disagree.

Thanks,

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

Re: [PATCH 2/2] mmc: sd: Fix sd current limit setting

[Chris Ball]

Hi,

On Wed, Jul 18 2012, Chris Ball wrote:
> Girish/Philip, does this look good to you too?  If so, I'll squash it
> into the original patch.

Thanks, everyone -- squashed in and pushed out to mmc-next.

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