On 18 July 2012 11:52, Aaron Lu <aaron.lu@xxxxxxx> 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@xxxxxxxxxx>
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
>
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
