RE: [PATCH v2 07/12] mmc: sd: set current limit for uhs cards

["Subhash Jadavani" <subhashj@codeaurora.org>]

> -----Original Message-----
> From: Arindam Nath [mailto:anath.amd@gmail.com] On Behalf Of Arindam
> Nath
> Sent: Friday, March 04, 2011 5:03 PM
> To: cjb@laptop.org
> Cc: zhangfei.gao@gmail.com; prakity@marvell.com;
> subhashj@codeaurora.org; linux-mmc@vger.kernel.org; henry.su@amd.com;
> aaron.lu@amd.com; anath.amd@gmail.com; Arindam Nath
> Subject: [PATCH v2 07/12] mmc: sd: set current limit for uhs cards
> 
> 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.nath@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;
> +
> +	/* 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;
> +
> +	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));

This is what SD3.01 spec says:
"Current limit switch is only for SDR50, SDR104 and DDR50. Current limit
does not act on the card in SDR12 and SDR25 modes."

But in this function you are not checking for the currently selected bus
speed mode. If let's say you are in SDR12/SDR25 mode and if host supports
MAX_CURRENT_800 then this function will try to the current limit to 800 but
card won't switch to it as SDR12/SDR25 mode doen't allow the current switch.

So basically your following check will always print the warning for
SDR12/SDR25 mode.

	if (((status[15] >> 4) & 0x0F) != current_limit)
		printk(KERN_WARNING "%s: Problem setting current limit!\n",
			mmc_hostname(card->host));

So in this function you should also check the current bus speed mode. If
current bus_speed_mode is SDR12/SDR25/legacy then you should select the
current limit to 200ma and for SDR104/SDR50/DDR50, you should select it
depending on the host's Max. current limit.


> +
> +	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;
>  }
> 
> +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;
> +
> +	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,
>  };
> 
> 
> /**********************************************************************
> *******\
> 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