This series updates SDHCI to use the common regulator infrastructure that mmc
core provides.
Tim Kryger (2):
mmc: sdhci: Use supplies in common mmc_host struct
mmc: sdhci: Use common mmc_regulator_get_supply
drivers/mmc/host/sdhci.c | 96 +++++++++++++++++----------------------------
include/linux/mmc/sdhci.h | 3 --
2 files changed, 35 insertions(+), 64 deletions(-)
--
1.7.9.5
Replace some buggy code with a call to mmc_regulator_get_supply.
When external regulator provides VDD, ocr_avail is set directly based on
the supported voltage range. This allows for the use of regulators that
can't provide exactly 1.8v, 3.0v, or 3.3v. Commit cec2e21 had attempted
to address this by relaxing the range checks in the SDHCI driver but it
set the capabilities as an intermediate step which meant ocr_avail could
get bits set for unsupported voltages.
Signed-off-by: Tim Kryger <[email protected]>
---
drivers/mmc/host/sdhci.c | 67 +++++++++++++---------------------------------
1 file changed, 18 insertions(+), 49 deletions(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 16fba66..2d081d8 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1438,7 +1438,8 @@ static void sdhci_do_set_ios(struct sdhci_host *host, struct mmc_ios *ios)
if (host->flags & SDHCI_DEVICE_DEAD) {
spin_unlock_irqrestore(&host->lock, flags);
- if (mmc->supply.vmmc && ios->power_mode == MMC_POWER_OFF)
+ if (!IS_ERR(mmc->supply.vmmc) &&
+ ios->power_mode == MMC_POWER_OFF)
mmc_regulator_set_ocr(host->mmc, mmc->supply.vmmc, 0);
return;
}
@@ -1464,7 +1465,7 @@ static void sdhci_do_set_ios(struct sdhci_host *host, struct mmc_ios *ios)
else
vdd_bit = sdhci_set_power(host, ios->vdd);
- if (mmc->supply.vmmc && vdd_bit != -1) {
+ if (!IS_ERR(mmc->supply.vmmc) && vdd_bit != -1) {
spin_unlock_irqrestore(&host->lock, flags);
mmc_regulator_set_ocr(host->mmc, mmc->supply.vmmc, vdd_bit);
spin_lock_irqsave(&host->lock, flags);
@@ -1762,7 +1763,7 @@ static int sdhci_do_start_signal_voltage_switch(struct sdhci_host *host,
ctrl &= ~SDHCI_CTRL_VDD_180;
sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2);
- if (mmc->supply.vqmmc) {
+ if (!IS_ERR(mmc->supply.vqmmc)) {
ret = regulator_set_voltage(mmc->supply.vqmmc, 2700000,
3600000);
if (ret) {
@@ -1784,7 +1785,7 @@ static int sdhci_do_start_signal_voltage_switch(struct sdhci_host *host,
return -EAGAIN;
case MMC_SIGNAL_VOLTAGE_180:
- if (mmc->supply.vqmmc) {
+ if (!IS_ERR(mmc->supply.vqmmc)) {
ret = regulator_set_voltage(mmc->supply.vqmmc,
1700000, 1950000);
if (ret) {
@@ -1814,7 +1815,7 @@ static int sdhci_do_start_signal_voltage_switch(struct sdhci_host *host,
return -EAGAIN;
case MMC_SIGNAL_VOLTAGE_120:
- if (mmc->supply.vqmmc) {
+ if (!IS_ERR(mmc->supply.vqmmc)) {
ret = regulator_set_voltage(mmc->supply.vqmmc, 1100000,
1300000);
if (ret) {
@@ -2979,15 +2980,12 @@ int sdhci_add_host(struct sdhci_host *host)
!(host->mmc->caps & MMC_CAP_NONREMOVABLE))
mmc->caps |= MMC_CAP_NEEDS_POLL;
+ /* If there are external regulators, get them */
+ if (mmc_regulator_get_supply(mmc) == -EPROBE_DEFER)
+ return -EPROBE_DEFER;
+
/* If vqmmc regulator and no 1.8V signalling, then there's no UHS */
- mmc->supply.vqmmc = regulator_get_optional(mmc_dev(mmc), "vqmmc");
- if (IS_ERR_OR_NULL(mmc->supply.vqmmc)) {
- if (PTR_ERR(mmc->supply.vqmmc) < 0) {
- pr_info("%s: no vqmmc regulator found\n",
- mmc_hostname(mmc));
- mmc->supply.vqmmc = NULL;
- }
- } else {
+ if (!IS_ERR(mmc->supply.vqmmc)) {
ret = regulator_enable(mmc->supply.vqmmc);
if (!regulator_is_supported_voltage(mmc->supply.vqmmc, 1700000,
1950000))
@@ -3058,34 +3056,6 @@ int sdhci_add_host(struct sdhci_host *host)
ocr_avail = 0;
- mmc->supply.vmmc = regulator_get_optional(mmc_dev(mmc), "vmmc");
- if (IS_ERR_OR_NULL(mmc->supply.vmmc)) {
- if (PTR_ERR(mmc->supply.vmmc) < 0) {
- pr_info("%s: no vmmc regulator found\n",
- mmc_hostname(mmc));
- mmc->supply.vmmc = NULL;
- }
- }
-
-#ifdef CONFIG_REGULATOR
- /*
- * Voltage range check makes sense only if regulator reports
- * any voltage value.
- */
- if (mmc->supply.vmmc && regulator_get_voltage(mmc->supply.vmmc) > 0) {
- ret = regulator_is_supported_voltage(mmc->supply.vmmc, 2700000,
- 3600000);
- if ((ret <= 0) || (!(caps[0] & SDHCI_CAN_VDD_330)))
- caps[0] &= ~SDHCI_CAN_VDD_330;
- if ((ret <= 0) || (!(caps[0] & SDHCI_CAN_VDD_300)))
- caps[0] &= ~SDHCI_CAN_VDD_300;
- ret = regulator_is_supported_voltage(mmc->supply.vmmc, 1700000,
- 1950000);
- if ((ret <= 0) || (!(caps[0] & SDHCI_CAN_VDD_180)))
- caps[0] &= ~SDHCI_CAN_VDD_180;
- }
-#endif /* CONFIG_REGULATOR */
-
/*
* According to SD Host Controller spec v3.00, if the Host System
* can afford more than 150mA, Host Driver should set XPC to 1. Also
@@ -3094,7 +3064,7 @@ int sdhci_add_host(struct sdhci_host *host)
* value.
*/
max_current_caps = sdhci_readl(host, SDHCI_MAX_CURRENT);
- if (!max_current_caps && mmc->supply.vmmc) {
+ if (!max_current_caps && !IS_ERR(mmc->supply.vmmc)) {
u32 curr = regulator_get_current_limit(mmc->supply.vmmc);
if (curr > 0) {
@@ -3135,8 +3105,11 @@ int sdhci_add_host(struct sdhci_host *host)
SDHCI_MAX_CURRENT_MULTIPLIER;
}
+ if (mmc->ocr_avail)
+ ocr_avail &= mmc->ocr_avail;
+
if (host->ocr_mask)
- ocr_avail = host->ocr_mask;
+ ocr_avail &= host->ocr_mask;
mmc->ocr_avail = ocr_avail;
mmc->ocr_avail_sdio = ocr_avail;
@@ -3330,15 +3303,11 @@ void sdhci_remove_host(struct sdhci_host *host, int dead)
tasklet_kill(&host->card_tasklet);
tasklet_kill(&host->finish_tasklet);
- if (mmc->supply.vmmc) {
+ if (!IS_ERR(mmc->supply.vmmc))
regulator_disable(mmc->supply.vmmc);
- regulator_put(mmc->supply.vmmc);
- }
- if (mmc->supply.vqmmc) {
+ if (!IS_ERR(mmc->supply.vqmmc))
regulator_disable(mmc->supply.vqmmc);
- regulator_put(mmc->supply.vqmmc);
- }
kfree(host->adma_desc);
kfree(host->align_buffer);
--
1.7.9.5
Switch the common SDHCI code over to use mmc_host's regulator pointers
and remove the ones in the sdhci_host structure.
Signed-off-by: Tim Kryger <[email protected]>
---
drivers/mmc/host/sdhci.c | 71 ++++++++++++++++++++++++---------------------
include/linux/mmc/sdhci.h | 3 --
2 files changed, 38 insertions(+), 36 deletions(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 9a79fc4..16fba66 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1429,6 +1429,7 @@ static void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq)
static void sdhci_do_set_ios(struct sdhci_host *host, struct mmc_ios *ios)
{
+ struct mmc_host *mmc = host->mmc;
unsigned long flags;
int vdd_bit = -1;
u8 ctrl;
@@ -1437,8 +1438,8 @@ static void sdhci_do_set_ios(struct sdhci_host *host, struct mmc_ios *ios)
if (host->flags & SDHCI_DEVICE_DEAD) {
spin_unlock_irqrestore(&host->lock, flags);
- if (host->vmmc && ios->power_mode == MMC_POWER_OFF)
- mmc_regulator_set_ocr(host->mmc, host->vmmc, 0);
+ if (mmc->supply.vmmc && ios->power_mode == MMC_POWER_OFF)
+ mmc_regulator_set_ocr(host->mmc, mmc->supply.vmmc, 0);
return;
}
@@ -1463,9 +1464,9 @@ static void sdhci_do_set_ios(struct sdhci_host *host, struct mmc_ios *ios)
else
vdd_bit = sdhci_set_power(host, ios->vdd);
- if (host->vmmc && vdd_bit != -1) {
+ if (mmc->supply.vmmc && vdd_bit != -1) {
spin_unlock_irqrestore(&host->lock, flags);
- mmc_regulator_set_ocr(host->mmc, host->vmmc, vdd_bit);
+ mmc_regulator_set_ocr(host->mmc, mmc->supply.vmmc, vdd_bit);
spin_lock_irqsave(&host->lock, flags);
}
@@ -1742,6 +1743,7 @@ static void sdhci_enable_sdio_irq(struct mmc_host *mmc, int enable)
static int sdhci_do_start_signal_voltage_switch(struct sdhci_host *host,
struct mmc_ios *ios)
{
+ struct mmc_host *mmc = host->mmc;
u16 ctrl;
int ret;
@@ -1760,8 +1762,9 @@ static int sdhci_do_start_signal_voltage_switch(struct sdhci_host *host,
ctrl &= ~SDHCI_CTRL_VDD_180;
sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2);
- if (host->vqmmc) {
- ret = regulator_set_voltage(host->vqmmc, 2700000, 3600000);
+ if (mmc->supply.vqmmc) {
+ ret = regulator_set_voltage(mmc->supply.vqmmc, 2700000,
+ 3600000);
if (ret) {
pr_warning("%s: Switching to 3.3V signalling voltage "
" failed\n", mmc_hostname(host->mmc));
@@ -1781,8 +1784,8 @@ static int sdhci_do_start_signal_voltage_switch(struct sdhci_host *host,
return -EAGAIN;
case MMC_SIGNAL_VOLTAGE_180:
- if (host->vqmmc) {
- ret = regulator_set_voltage(host->vqmmc,
+ if (mmc->supply.vqmmc) {
+ ret = regulator_set_voltage(mmc->supply.vqmmc,
1700000, 1950000);
if (ret) {
pr_warning("%s: Switching to 1.8V signalling voltage "
@@ -1811,8 +1814,9 @@ static int sdhci_do_start_signal_voltage_switch(struct sdhci_host *host,
return -EAGAIN;
case MMC_SIGNAL_VOLTAGE_120:
- if (host->vqmmc) {
- ret = regulator_set_voltage(host->vqmmc, 1100000, 1300000);
+ if (mmc->supply.vqmmc) {
+ ret = regulator_set_voltage(mmc->supply.vqmmc, 1100000,
+ 1300000);
if (ret) {
pr_warning("%s: Switching to 1.2V signalling voltage "
" failed\n", mmc_hostname(host->mmc));
@@ -2976,24 +2980,24 @@ int sdhci_add_host(struct sdhci_host *host)
mmc->caps |= MMC_CAP_NEEDS_POLL;
/* If vqmmc regulator and no 1.8V signalling, then there's no UHS */
- host->vqmmc = regulator_get_optional(mmc_dev(mmc), "vqmmc");
- if (IS_ERR_OR_NULL(host->vqmmc)) {
- if (PTR_ERR(host->vqmmc) < 0) {
+ mmc->supply.vqmmc = regulator_get_optional(mmc_dev(mmc), "vqmmc");
+ if (IS_ERR_OR_NULL(mmc->supply.vqmmc)) {
+ if (PTR_ERR(mmc->supply.vqmmc) < 0) {
pr_info("%s: no vqmmc regulator found\n",
mmc_hostname(mmc));
- host->vqmmc = NULL;
+ mmc->supply.vqmmc = NULL;
}
} else {
- ret = regulator_enable(host->vqmmc);
- if (!regulator_is_supported_voltage(host->vqmmc, 1700000,
- 1950000))
+ ret = regulator_enable(mmc->supply.vqmmc);
+ if (!regulator_is_supported_voltage(mmc->supply.vqmmc, 1700000,
+ 1950000))
caps[1] &= ~(SDHCI_SUPPORT_SDR104 |
SDHCI_SUPPORT_SDR50 |
SDHCI_SUPPORT_DDR50);
if (ret) {
pr_warn("%s: Failed to enable vqmmc regulator: %d\n",
mmc_hostname(mmc), ret);
- host->vqmmc = NULL;
+ mmc->supply.vqmmc = NULL;
}
}
@@ -3054,12 +3058,12 @@ int sdhci_add_host(struct sdhci_host *host)
ocr_avail = 0;
- host->vmmc = regulator_get_optional(mmc_dev(mmc), "vmmc");
- if (IS_ERR_OR_NULL(host->vmmc)) {
- if (PTR_ERR(host->vmmc) < 0) {
+ mmc->supply.vmmc = regulator_get_optional(mmc_dev(mmc), "vmmc");
+ if (IS_ERR_OR_NULL(mmc->supply.vmmc)) {
+ if (PTR_ERR(mmc->supply.vmmc) < 0) {
pr_info("%s: no vmmc regulator found\n",
mmc_hostname(mmc));
- host->vmmc = NULL;
+ mmc->supply.vmmc = NULL;
}
}
@@ -3068,14 +3072,14 @@ int sdhci_add_host(struct sdhci_host *host)
* Voltage range check makes sense only if regulator reports
* any voltage value.
*/
- if (host->vmmc && regulator_get_voltage(host->vmmc) > 0) {
- ret = regulator_is_supported_voltage(host->vmmc, 2700000,
+ if (mmc->supply.vmmc && regulator_get_voltage(mmc->supply.vmmc) > 0) {
+ ret = regulator_is_supported_voltage(mmc->supply.vmmc, 2700000,
3600000);
if ((ret <= 0) || (!(caps[0] & SDHCI_CAN_VDD_330)))
caps[0] &= ~SDHCI_CAN_VDD_330;
if ((ret <= 0) || (!(caps[0] & SDHCI_CAN_VDD_300)))
caps[0] &= ~SDHCI_CAN_VDD_300;
- ret = regulator_is_supported_voltage(host->vmmc, 1700000,
+ ret = regulator_is_supported_voltage(mmc->supply.vmmc, 1700000,
1950000);
if ((ret <= 0) || (!(caps[0] & SDHCI_CAN_VDD_180)))
caps[0] &= ~SDHCI_CAN_VDD_180;
@@ -3090,8 +3094,8 @@ int sdhci_add_host(struct sdhci_host *host)
* value.
*/
max_current_caps = sdhci_readl(host, SDHCI_MAX_CURRENT);
- if (!max_current_caps && host->vmmc) {
- u32 curr = regulator_get_current_limit(host->vmmc);
+ if (!max_current_caps && mmc->supply.vmmc) {
+ u32 curr = regulator_get_current_limit(mmc->supply.vmmc);
if (curr > 0) {
/* convert to SDHCI_MAX_CURRENT format */
@@ -3288,6 +3292,7 @@ EXPORT_SYMBOL_GPL(sdhci_add_host);
void sdhci_remove_host(struct sdhci_host *host, int dead)
{
+ struct mmc_host *mmc = host->mmc;
unsigned long flags;
if (dead) {
@@ -3325,14 +3330,14 @@ void sdhci_remove_host(struct sdhci_host *host, int dead)
tasklet_kill(&host->card_tasklet);
tasklet_kill(&host->finish_tasklet);
- if (host->vmmc) {
- regulator_disable(host->vmmc);
- regulator_put(host->vmmc);
+ if (mmc->supply.vmmc) {
+ regulator_disable(mmc->supply.vmmc);
+ regulator_put(mmc->supply.vmmc);
}
- if (host->vqmmc) {
- regulator_disable(host->vqmmc);
- regulator_put(host->vqmmc);
+ if (mmc->supply.vqmmc) {
+ regulator_disable(mmc->supply.vqmmc);
+ regulator_put(mmc->supply.vqmmc);
}
kfree(host->adma_desc);
diff --git a/include/linux/mmc/sdhci.h b/include/linux/mmc/sdhci.h
index 7be12b8..0e3c3f8 100644
--- a/include/linux/mmc/sdhci.h
+++ b/include/linux/mmc/sdhci.h
@@ -108,9 +108,6 @@ struct sdhci_host {
const struct sdhci_ops *ops; /* Low level hw interface */
- struct regulator *vmmc; /* Power regulator (vmmc) */
- struct regulator *vqmmc; /* Signaling regulator (vccq) */
-
/* Internal data */
struct mmc_host *mmc; /* MMC structure */
u64 dma_mask; /* custom DMA mask */
--
1.7.9.5
On 18 April 2014 02:46, Tim Kryger <[email protected]> wrote:
> This series updates SDHCI to use the common regulator infrastructure that mmc
> core provides.
>
> Tim Kryger (2):
> mmc: sdhci: Use supplies in common mmc_host struct
> mmc: sdhci: Use common mmc_regulator_get_supply
Both patches looks good, but I would prefer if you could squash them
into one patch.
Kind regards
Ulf Hansson
>
> drivers/mmc/host/sdhci.c | 96 +++++++++++++++++----------------------------
> include/linux/mmc/sdhci.h | 3 --
> 2 files changed, 35 insertions(+), 64 deletions(-)
>
> --
> 1.7.9.5
>
On Thu, Apr 24, 2014 at 1:34 AM, Ulf Hansson <[email protected]> wrote:
> On 18 April 2014 02:46, Tim Kryger <[email protected]> wrote:
>> This series updates SDHCI to use the common regulator infrastructure that mmc
>> core provides.
>>
>> Tim Kryger (2):
>> mmc: sdhci: Use supplies in common mmc_host struct
>> mmc: sdhci: Use common mmc_regulator_get_supply
>
> Both patches looks good, but I would prefer if you could squash them
> into one patch.
Sure.