On Mar 16, 2011, at 7:32 AM, Nath, Arindam wrote:
> Hi Philip,
>
>
>> -----Original Message-----
>> From: Philip Rakity [mailto:prak...@marvell.com]
>> Sent: Wednesday, March 16, 2011 7:56 PM
>> To: Nath, Arindam
>> Cc: c...@laptop.org; zhangfei....@gmail.com; subha...@codeaurora.org;
>> linux-mmc@vger.kernel.org; Su, Henry; Lu, Aaron; anath....@gmail.com
>> Subject: Re: [PATCH v2 07/12] mmc: sd: set current limit for uhs cards
>>
>>
>> On Mar 4, 2011, at 3:32 AM, Arindam Nath wrote:
>>
>>> We decide on the current limit to be set for the card based on the
>>> Capability of Host Controller to provide current at 1.8V signalling,
>>> and the maximum current limit of the card as indicated by CMD6
>>> mode 0. We then set the current limit for the card using CMD6 mode 1.
>>>
>>> Signed-off-by: Arindam Nath <arindam.n...@amd.com>
>>> ---
>>> drivers/mmc/core/sd.c | 45
>> +++++++++++++++++++++++++++++++++++++++++++++
>>> drivers/mmc/host/sdhci.c | 24 ++++++++++++++++++++++++
>>> include/linux/mmc/card.h | 9 +++++++++
>>> include/linux/mmc/host.h | 1 +
>>> 4 files changed, 79 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
>>> index ec0d8e6..df98a2c 100644
>>> --- a/drivers/mmc/core/sd.c
>>> +++ b/drivers/mmc/core/sd.c
>>> @@ -550,6 +550,46 @@ static int sd_set_bus_speed_mode(struct mmc_card
>> *card, u8 *status)
>>> return 0;
>>> }
>>>
>>> +static int sd_set_current_limit(struct mmc_card *card, u8 *status)
>>> +{
>>> + struct mmc_host *host = card->host;
>>> + int mmc_host_max_current_180, current_limit;
>>> + int err;
>>> +
>>> + /* sanity check */
>>> + if (!host->ops->get_max_current_180)
>>> + return 0;
>>
>> a better name would be get_max_current rather than get_max_current_180
>
> The Max Current Capabilities register reports maximum currents for 1.8V, 3.0V
> and 3.3V. Since we are only interested in the maximum current at 1.8V, so I
> have added *_180 to the variable names to make it explicit.
understand but that is the usage now and if we need to extend the code the name
becomes misleading.
>
>>
>> do you want a test for get_max_current_180 < 400 ? and return 0 have
>> it do the switch
>> by setting the value ?
>
> As mentioned in the Physical Layer spec v3.01, <400mA falls under the default
> current limit, so we don't need to set it in case any or all of the above
> conditions fail.
if future cards have memory or power is not removed they will they retain the
old setting ? if so better to explicitly initialize.
>
> Thanks,
> Arindam
>
>>> +
>>> + /* Maximum current supported by host at 1.8V */
>>> + mmc_host_max_current_180 = host->ops->get_max_current_180(host);
>>> +
>>> + if (mmc_host_max_current_180 >= 800) {
>>> + if (card->sw_caps.uhs_curr_limit & SD_MAX_CURRENT_800)
>>> + current_limit = SD_SET_CURRENT_LIMIT_800;
>>> + else if (card->sw_caps.uhs_curr_limit & SD_MAX_CURRENT_600)
>>> + current_limit = SD_SET_CURRENT_LIMIT_600;
>>> + else if (card->sw_caps.uhs_curr_limit & SD_MAX_CURRENT_400)
>>> + current_limit = SD_SET_CURRENT_LIMIT_400;
>>> + } else if (mmc_host_max_current_180 >= 600) {
>>> + if (card->sw_caps.uhs_curr_limit & SD_MAX_CURRENT_600)
>>> + current_limit = SD_SET_CURRENT_LIMIT_600;
>>> + else if (card->sw_caps.uhs_curr_limit & SD_MAX_CURRENT_400)
>>> + current_limit = SD_SET_CURRENT_LIMIT_400;
>>> + } else if (mmc_host_max_current_180 >= 400)
>>> + if (card->sw_caps.uhs_curr_limit & SD_MAX_CURRENT_400)
>>> + current_limit = SD_SET_CURRENT_LIMIT_400;
>>
>> or
>> else
>> current_limit = SD_SET_CURRENT_LIMIT_200;
>>> +
>>> + err = mmc_sd_switch(card, 1, 3, current_limit, status);
>>> + if (err)
>>> + return err;
>>> +
>>> + if (((status[15] >> 4) & 0x0F) != current_limit)
>>> + printk(KERN_WARNING "%s: Problem setting current limit!\n",
>>> + mmc_hostname(card->host));
>>> +
>>> + return 0;
>>> +}
>>> +
>>> /*
>>> * UHS-I specific initialization procedure
>>> */
>>> @@ -590,6 +630,11 @@ static int mmc_sd_init_uhs_card(struct mmc_card
>> *card)
>>>
>>> /* Set bus speed mode of the card */
>>> err = sd_set_bus_speed_mode(card, status);
>>> + if (err)
>>> + goto out;
>>> +
>>> + /* Set current limit for the card */
>>> + err = sd_set_current_limit(card, status);
>>>
>>> out:
>>> kfree(status);
>>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>>> index f127fa2..245cc39 100644
>>> --- a/drivers/mmc/host/sdhci.c
>>> +++ b/drivers/mmc/host/sdhci.c
>>> @@ -1462,12 +1462,36 @@ static int
>> sdhci_start_signal_voltage_switch(struct mmc_host *mmc)
>>> return -EAGAIN;
>>> }
>>>
>>
>> better name is sdhci_get_max_current
>>
>>> +static int sdhci_get_max_current_180(struct mmc_host *mmc)
>>> +{
>>> + struct sdhci_host *host;
>>> + u32 max_current_caps;
>>> + unsigned long flags;
>>> + int max_current_180;
>>> +
>>> + host = mmc_priv(mmc);
>>> +
>>> + spin_lock_irqsave(&host->lock, flags);
>>> +
>>> + max_current_caps = sdhci_readl(host, SDHCI_MAX_CURRENT);
>>> +
>>> + spin_unlock_irqrestore(&host->lock, flags);
>>> +
>>> + /* Maximum current is 4 times the register value for 1.8V */
>>> + max_current_180 = ((max_current_caps &
>> SDHCI_MAX_CURRENT_180_MASK) >>
>>> + SDHCI_MAX_CURRENT_180_SHIFT) *
>>> + SDHCI_MAX_CURRENT_MULTIPLIER;
>>
>> SDHCI_MAX_CURRENT_SHIFT is better name.
>>
>>> +
>>> + return max_current_180;
>>> +}
>>> +
>>> static const struct mmc_host_ops sdhci_ops = {
>>> .request = sdhci_request,
>>> .set_ios = sdhci_set_ios,
>>> .get_ro = sdhci_get_ro,
>>> .enable_sdio_irq = sdhci_enable_sdio_irq,
>>> .start_signal_voltage_switch =
>> sdhci_start_signal_voltage_switch,
>>> + .get_max_current_180 = sdhci_get_max_current_180,
>>
>> .get_max_current = sdhci_get_max_current;
>>> };
>>>
>>>
>> /**********************************************************************
>> *******\
>>> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
>>> index 0b24c41..a6811ae 100644
>>> --- a/include/linux/mmc/card.h
>>> +++ b/include/linux/mmc/card.h
>>> @@ -98,6 +98,15 @@ struct sd_switch_caps {
>>> #define SD_DRIVER_TYPE_C 0x04
>>> #define SD_DRIVER_TYPE_D 0x08
>>> unsigned int uhs_curr_limit;
>>> +#define SD_SET_CURRENT_LIMIT_200 0
>>> +#define SD_SET_CURRENT_LIMIT_400 1
>>> +#define SD_SET_CURRENT_LIMIT_600 2
>>> +#define SD_SET_CURRENT_LIMIT_800 3
>>> +
>>> +#define SD_MAX_CURRENT_200 (1 << SD_SET_CURRENT_LIMIT_200)
>>> +#define SD_MAX_CURRENT_400 (1 << SD_SET_CURRENT_LIMIT_400)
>>> +#define SD_MAX_CURRENT_600 (1 << SD_SET_CURRENT_LIMIT_600)
>>> +#define SD_MAX_CURRENT_800 (1 << SD_SET_CURRENT_LIMIT_800)
>>> };
>>>
>>> struct sdio_cccr {
>>> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
>>> index 4dfff6d..e84cd05 100644
>>> --- a/include/linux/mmc/host.h
>>> +++ b/include/linux/mmc/host.h
>>> @@ -128,6 +128,7 @@ struct mmc_host_ops {
>>> void (*init_card)(struct mmc_host *host, struct mmc_card *card);
>>>
>>> int (*start_signal_voltage_switch)(struct mmc_host *host);
>>> + int (*get_max_current_180)(struct mmc_host *mmc);
>>> };
>>>
>>> struct mmc_card;
>>> --
>>> 1.7.1
>>>
>>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
