Discussion:
[PATCH V2 0/3] Adding UHS support for dw_mmc driver
Yuvaraj Kumar C D
2014-08-22 13:47:49 UTC
Permalink
This series adds UHS support for dw_mmc driver.
Patch[1] reworks the handling of vmmc and vqmmc regulators by mmc core
regulator API's.

Patch[2] was taken from chrome tree originally developed by Doug Anderson.
Comments recieved for this patch to remove extra state machine for CMD11
handling is dropped since it's very much required to set the
SDMMC_CMD_VOLT_SWITCH bit(28) for the proper working of clock update after
the first VOLT_SWITCH interrupt occurs during the voltage switch scenario.
Though its not mentioned in Synopsys data book, its essential to complete
the voltage switchingi sequence.

Patch[3] handles the case where in some boards uses built-in CD line for
card detection and connected to a same voltage domain as of the IO rails.
This version adds the changes for the concerned expressed by Doug Anderson
in prevoius version.

Doug Anderson (1):
[2].mmc: dw_mmc: Support voltage changes

Yuvaraj Kumar C D (2):
[1]. mmc: dw_mmc: use mmc_regulator_get_supply to handle regulators
[3]. mmc: dw_mmc: Dont cut off vqmmc and vmmc

drivers/mmc/core/core.c | 16 ++-
drivers/mmc/core/debugfs.c | 3 +
drivers/mmc/host/dw_mmc-exynos.c | 12 ++
drivers/mmc/host/dw_mmc.c | 231 ++++++++++++++++++++++++++++++--------
drivers/mmc/host/dw_mmc.h | 7 +-
include/linux/mmc/dw_mmc.h | 4 +-
include/linux/mmc/host.h | 2 +
7 files changed, 225 insertions(+), 50 deletions(-)
--
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Yuvaraj Kumar C D
2014-08-22 13:47:50 UTC
Permalink
This patch makes use of mmc_regulator_get_supply() to handle
the vmmc and vqmmc regulators.Also it moves the code handling
the these regulators to dw_mci_set_ios().It turned on the vmmc
and vqmmc during MMC_POWER_UP and MMC_POWER_ON,and turned off
during MMC_POWER_OFF.

Signed-off-by: Yuvaraj Kumar C D <***@samsung.com>
---
changes from v1:
1.Used mmc_regulator_set_ocr() instead of regulator_enable() for vmmc.
2.Turned on vmmc and vqmmc during MMC_POWER_UP.
3. Removed the flags DW_MMC_CARD_POWERED and DW_MMC_IO_POWERED which
added during the initial version of this patch.
4. Added error message, if it failed to turn on regulator's.

drivers/mmc/host/dw_mmc.c | 72 +++++++++++++++++++++-----------------------
include/linux/mmc/dw_mmc.h | 2 +-
2 files changed, 36 insertions(+), 38 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 7f227e9..aadb0d6 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -936,6 +936,7 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
struct dw_mci_slot *slot = mmc_priv(mmc);
const struct dw_mci_drv_data *drv_data = slot->host->drv_data;
u32 regs;
+ int ret;

switch (ios->bus_width) {
case MMC_BUS_WIDTH_4:
@@ -974,12 +975,38 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)

switch (ios->power_mode) {
case MMC_POWER_UP:
+ if (!IS_ERR(mmc->supply.vmmc)) {
+ ret = mmc_regulator_set_ocr(mmc, mmc->supply.vmmc,
+ ios->vdd);
+ if (ret) {
+ dev_err(slot->host->dev,
+ "failed to enable vmmc regulator\n");
+ /*return, if failed turn on vmmc*/
+ return;
+ }
+ }
+ if (!IS_ERR(mmc->supply.vqmmc) && !slot->host->vqmmc_enabled) {
+ ret = regulator_enable(mmc->supply.vqmmc);
+ if (ret < 0)
+ dev_err(slot->host->dev,
+ "failed to enable vqmmc regulator\n");
+ else
+ slot->host->vqmmc_enabled = true;
+ }
set_bit(DW_MMC_CARD_NEED_INIT, &slot->flags);
regs = mci_readl(slot->host, PWREN);
regs |= (1 << slot->id);
mci_writel(slot->host, PWREN, regs);
break;
case MMC_POWER_OFF:
+ if (!IS_ERR(mmc->supply.vmmc))
+ mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, 0);
+
+ if (!IS_ERR(mmc->supply.vqmmc) && slot->host->vqmmc_enabled) {
+ regulator_disable(mmc->supply.vqmmc);
+ slot->host->vqmmc_enabled = false;
+ }
+
regs = mci_readl(slot->host, PWREN);
regs &= ~(1 << slot->id);
mci_writel(slot->host, PWREN, regs);
@@ -2110,7 +2137,13 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
mmc->f_max = freq[1];
}

- mmc->ocr_avail = MMC_VDD_32_33 | MMC_VDD_33_34;
+ /*if there are external regulators, get them*/
+ ret = mmc_regulator_get_supply(mmc);
+ if (ret == -EPROBE_DEFER)
+ goto err_setup_bus;
+
+ if (!mmc->ocr_avail)
+ mmc->ocr_avail = MMC_VDD_32_33 | MMC_VDD_33_34;

if (host->pdata->caps)
mmc->caps = host->pdata->caps;
@@ -2176,7 +2209,7 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)

err_setup_bus:
mmc_free_host(mmc);
- return -EINVAL;
+ return ret;
}

static void dw_mci_cleanup_slot(struct dw_mci_slot *slot, unsigned int id)
@@ -2469,24 +2502,6 @@ int dw_mci_probe(struct dw_mci *host)
}
}

- host->vmmc = devm_regulator_get_optional(host->dev, "vmmc");
- if (IS_ERR(host->vmmc)) {
- ret = PTR_ERR(host->vmmc);
- if (ret == -EPROBE_DEFER)
- goto err_clk_ciu;
-
- dev_info(host->dev, "no vmmc regulator found: %d\n", ret);
- host->vmmc = NULL;
- } else {
- ret = regulator_enable(host->vmmc);
- if (ret) {
- if (ret != -EPROBE_DEFER)
- dev_err(host->dev,
- "regulator_enable fail: %d\n", ret);
- goto err_clk_ciu;
- }
- }
-
host->quirks = host->pdata->quirks;

spin_lock_init(&host->lock);
@@ -2630,8 +2645,6 @@ err_workqueue:
err_dmaunmap:
if (host->use_dma && host->dma_ops->exit)
host->dma_ops->exit(host);
- if (host->vmmc)
- regulator_disable(host->vmmc);

err_clk_ciu:
if (!IS_ERR(host->ciu_clk))
@@ -2667,9 +2680,6 @@ void dw_mci_remove(struct dw_mci *host)
if (host->use_dma && host->dma_ops->exit)
host->dma_ops->exit(host);

- if (host->vmmc)
- regulator_disable(host->vmmc);
-
if (!IS_ERR(host->ciu_clk))
clk_disable_unprepare(host->ciu_clk);

@@ -2686,9 +2696,6 @@ EXPORT_SYMBOL(dw_mci_remove);
*/
int dw_mci_suspend(struct dw_mci *host)
{
- if (host->vmmc)
- regulator_disable(host->vmmc);
-
return 0;
}
EXPORT_SYMBOL(dw_mci_suspend);
@@ -2697,15 +2704,6 @@ int dw_mci_resume(struct dw_mci *host)
{
int i, ret;

- if (host->vmmc) {
- ret = regulator_enable(host->vmmc);
- if (ret) {
- dev_err(host->dev,
- "failed to enable regulator: %d\n", ret);
- return ret;
- }
- }
-
if (!dw_mci_ctrl_reset(host, SDMMC_CTRL_ALL_RESET_FLAGS)) {
ret = -ENODEV;
return ret;
diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h
index 29ce014..84e2827 100644
--- a/include/linux/mmc/dw_mmc.h
+++ b/include/linux/mmc/dw_mmc.h
@@ -188,7 +188,7 @@ struct dw_mci {
/* Workaround flags */
u32 quirks;

- struct regulator *vmmc; /* Power regulator */
+ bool vqmmc_enabled;
unsigned long irq_flags; /* IRQ flags */
int irq;
};
--
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Jaehoon Chung
2014-08-25 12:32:02 UTC
Permalink
Post by Yuvaraj Kumar C D
This patch makes use of mmc_regulator_get_supply() to handle
the vmmc and vqmmc regulators.Also it moves the code handling
the these regulators to dw_mci_set_ios().It turned on the vmmc
and vqmmc during MMC_POWER_UP and MMC_POWER_ON,and turned off
during MMC_POWER_OFF.
---
1.Used mmc_regulator_set_ocr() instead of regulator_enable() for vmmc.
2.Turned on vmmc and vqmmc during MMC_POWER_UP.
3. Removed the flags DW_MMC_CARD_POWERED and DW_MMC_IO_POWERED which
added during the initial version of this patch.
4. Added error message, if it failed to turn on regulator's.
drivers/mmc/host/dw_mmc.c | 72 +++++++++++++++++++++-----------------------
include/linux/mmc/dw_mmc.h | 2 +-
2 files changed, 36 insertions(+), 38 deletions(-)
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 7f227e9..aadb0d6 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -936,6 +936,7 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
struct dw_mci_slot *slot = mmc_priv(mmc);
const struct dw_mci_drv_data *drv_data = slot->host->drv_data;
u32 regs;
+ int ret;
switch (ios->bus_width) {
@@ -974,12 +975,38 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
switch (ios->power_mode) {
+ if (!IS_ERR(mmc->supply.vmmc)) {
+ ret = mmc_regulator_set_ocr(mmc, mmc->supply.vmmc,
+ ios->vdd);
+ if (ret) {
+ dev_err(slot->host->dev,
+ "failed to enable vmmc regulator\n");
+ /*return, if failed turn on vmmc*/
+ return;
+ }
+ }
+ if (!IS_ERR(mmc->supply.vqmmc) && !slot->host->vqmmc_enabled) {
Can't use the regulator_is_enabled() instead of "slot->host->vqmmc_enabled"?

Best Regards,
Jaehoon Chung
Post by Yuvaraj Kumar C D
+ ret = regulator_enable(mmc->supply.vqmmc);
+ if (ret < 0)
+ dev_err(slot->host->dev,
+ "failed to enable vqmmc regulator\n");
+ else
+ slot->host->vqmmc_enabled = true;
+ }
set_bit(DW_MMC_CARD_NEED_INIT, &slot->flags);
regs = mci_readl(slot->host, PWREN);
regs |= (1 << slot->id);
mci_writel(slot->host, PWREN, regs);
break;
+ if (!IS_ERR(mmc->supply.vmmc))
+ mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, 0);
+
+ if (!IS_ERR(mmc->supply.vqmmc) && slot->host->vqmmc_enabled) {
+ regulator_disable(mmc->supply.vqmmc);
+ slot->host->vqmmc_enabled = false;
+ }
+
regs = mci_readl(slot->host, PWREN);
regs &= ~(1 << slot->id);
mci_writel(slot->host, PWREN, regs);
@@ -2110,7 +2137,13 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
mmc->f_max = freq[1];
}
- mmc->ocr_avail = MMC_VDD_32_33 | MMC_VDD_33_34;
+ /*if there are external regulators, get them*/
+ ret = mmc_regulator_get_supply(mmc);
+ if (ret == -EPROBE_DEFER)
+ goto err_setup_bus;
+
+ if (!mmc->ocr_avail)
+ mmc->ocr_avail = MMC_VDD_32_33 | MMC_VDD_33_34;
if (host->pdata->caps)
mmc->caps = host->pdata->caps;
@@ -2176,7 +2209,7 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
mmc_free_host(mmc);
- return -EINVAL;
+ return ret;
}
static void dw_mci_cleanup_slot(struct dw_mci_slot *slot, unsigned int id)
@@ -2469,24 +2502,6 @@ int dw_mci_probe(struct dw_mci *host)
}
}
- host->vmmc = devm_regulator_get_optional(host->dev, "vmmc");
- if (IS_ERR(host->vmmc)) {
- ret = PTR_ERR(host->vmmc);
- if (ret == -EPROBE_DEFER)
- goto err_clk_ciu;
-
- dev_info(host->dev, "no vmmc regulator found: %d\n", ret);
- host->vmmc = NULL;
- } else {
- ret = regulator_enable(host->vmmc);
- if (ret) {
- if (ret != -EPROBE_DEFER)
- dev_err(host->dev,
- "regulator_enable fail: %d\n", ret);
- goto err_clk_ciu;
- }
- }
-
host->quirks = host->pdata->quirks;
spin_lock_init(&host->lock);
if (host->use_dma && host->dma_ops->exit)
host->dma_ops->exit(host);
- if (host->vmmc)
- regulator_disable(host->vmmc);
if (!IS_ERR(host->ciu_clk))
@@ -2667,9 +2680,6 @@ void dw_mci_remove(struct dw_mci *host)
if (host->use_dma && host->dma_ops->exit)
host->dma_ops->exit(host);
- if (host->vmmc)
- regulator_disable(host->vmmc);
-
if (!IS_ERR(host->ciu_clk))
clk_disable_unprepare(host->ciu_clk);
@@ -2686,9 +2696,6 @@ EXPORT_SYMBOL(dw_mci_remove);
*/
int dw_mci_suspend(struct dw_mci *host)
{
- if (host->vmmc)
- regulator_disable(host->vmmc);
-
return 0;
}
EXPORT_SYMBOL(dw_mci_suspend);
@@ -2697,15 +2704,6 @@ int dw_mci_resume(struct dw_mci *host)
{
int i, ret;
- if (host->vmmc) {
- ret = regulator_enable(host->vmmc);
- if (ret) {
- dev_err(host->dev,
- "failed to enable regulator: %d\n", ret);
- return ret;
- }
- }
-
if (!dw_mci_ctrl_reset(host, SDMMC_CTRL_ALL_RESET_FLAGS)) {
ret = -ENODEV;
return ret;
diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h
index 29ce014..84e2827 100644
--- a/include/linux/mmc/dw_mmc.h
+++ b/include/linux/mmc/dw_mmc.h
@@ -188,7 +188,7 @@ struct dw_mci {
/* Workaround flags */
u32 quirks;
- struct regulator *vmmc; /* Power regulator */
+ bool vqmmc_enabled;
unsigned long irq_flags; /* IRQ flags */
int irq;
};
Doug Anderson
2014-08-25 15:06:50 UTC
Permalink
Jaehoon,
Post by Jaehoon Chung
Post by Yuvaraj Kumar C D
This patch makes use of mmc_regulator_get_supply() to handle
the vmmc and vqmmc regulators.Also it moves the code handling
the these regulators to dw_mci_set_ios().It turned on the vmmc
and vqmmc during MMC_POWER_UP and MMC_POWER_ON,and turned off
during MMC_POWER_OFF.
---
1.Used mmc_regulator_set_ocr() instead of regulator_enable() for vmmc.
2.Turned on vmmc and vqmmc during MMC_POWER_UP.
3. Removed the flags DW_MMC_CARD_POWERED and DW_MMC_IO_POWERED which
added during the initial version of this patch.
4. Added error message, if it failed to turn on regulator's.
drivers/mmc/host/dw_mmc.c | 72 +++++++++++++++++++++-----------------------
include/linux/mmc/dw_mmc.h | 2 +-
2 files changed, 36 insertions(+), 38 deletions(-)
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 7f227e9..aadb0d6 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -936,6 +936,7 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
struct dw_mci_slot *slot = mmc_priv(mmc);
const struct dw_mci_drv_data *drv_data = slot->host->drv_data;
u32 regs;
+ int ret;
switch (ios->bus_width) {
@@ -974,12 +975,38 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
switch (ios->power_mode) {
+ if (!IS_ERR(mmc->supply.vmmc)) {
+ ret = mmc_regulator_set_ocr(mmc, mmc->supply.vmmc,
+ ios->vdd);
+ if (ret) {
+ dev_err(slot->host->dev,
+ "failed to enable vmmc regulator\n");
+ /*return, if failed turn on vmmc*/
+ return;
+ }
+ }
+ if (!IS_ERR(mmc->supply.vqmmc) && !slot->host->vqmmc_enabled) {
Can't use the regulator_is_enabled() instead of "slot->host->vqmmc_enabled"?
I think we mentioned this before, but regulator_is_enabled() won't do
what you want with shared regulators since they are refcounted. You
need to keep track of whether you've enabled the regulator yourself.

-Doug
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Ulf Hansson
2014-08-29 11:34:44 UTC
Permalink
Post by Yuvaraj Kumar C D
This patch makes use of mmc_regulator_get_supply() to handle
the vmmc and vqmmc regulators.Also it moves the code handling
the these regulators to dw_mci_set_ios().It turned on the vmmc
and vqmmc during MMC_POWER_UP and MMC_POWER_ON,and turned off
during MMC_POWER_OFF.
Thanks! Applied for next.

Kind regards
Uffe
Post by Yuvaraj Kumar C D
---
1.Used mmc_regulator_set_ocr() instead of regulator_enable() for vmmc.
2.Turned on vmmc and vqmmc during MMC_POWER_UP.
3. Removed the flags DW_MMC_CARD_POWERED and DW_MMC_IO_POWERED which
added during the initial version of this patch.
4. Added error message, if it failed to turn on regulator's.
drivers/mmc/host/dw_mmc.c | 72 +++++++++++++++++++++-----------------------
include/linux/mmc/dw_mmc.h | 2 +-
2 files changed, 36 insertions(+), 38 deletions(-)
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 7f227e9..aadb0d6 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -936,6 +936,7 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
struct dw_mci_slot *slot = mmc_priv(mmc);
const struct dw_mci_drv_data *drv_data = slot->host->drv_data;
u32 regs;
+ int ret;
switch (ios->bus_width) {
@@ -974,12 +975,38 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
switch (ios->power_mode) {
+ if (!IS_ERR(mmc->supply.vmmc)) {
+ ret = mmc_regulator_set_ocr(mmc, mmc->supply.vmmc,
+ ios->vdd);
+ if (ret) {
+ dev_err(slot->host->dev,
+ "failed to enable vmmc regulator\n");
+ /*return, if failed turn on vmmc*/
+ return;
+ }
+ }
+ if (!IS_ERR(mmc->supply.vqmmc) && !slot->host->vqmmc_enabled) {
+ ret = regulator_enable(mmc->supply.vqmmc);
+ if (ret < 0)
+ dev_err(slot->host->dev,
+ "failed to enable vqmmc regulator\n");
+ else
+ slot->host->vqmmc_enabled = true;
+ }
set_bit(DW_MMC_CARD_NEED_INIT, &slot->flags);
regs = mci_readl(slot->host, PWREN);
regs |= (1 << slot->id);
mci_writel(slot->host, PWREN, regs);
break;
+ if (!IS_ERR(mmc->supply.vmmc))
+ mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, 0);
+
+ if (!IS_ERR(mmc->supply.vqmmc) && slot->host->vqmmc_enabled) {
+ regulator_disable(mmc->supply.vqmmc);
+ slot->host->vqmmc_enabled = false;
+ }
+
regs = mci_readl(slot->host, PWREN);
regs &= ~(1 << slot->id);
mci_writel(slot->host, PWREN, regs);
@@ -2110,7 +2137,13 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
mmc->f_max = freq[1];
}
- mmc->ocr_avail = MMC_VDD_32_33 | MMC_VDD_33_34;
+ /*if there are external regulators, get them*/
+ ret = mmc_regulator_get_supply(mmc);
+ if (ret == -EPROBE_DEFER)
+ goto err_setup_bus;
+
+ if (!mmc->ocr_avail)
+ mmc->ocr_avail = MMC_VDD_32_33 | MMC_VDD_33_34;
if (host->pdata->caps)
mmc->caps = host->pdata->caps;
@@ -2176,7 +2209,7 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
mmc_free_host(mmc);
- return -EINVAL;
+ return ret;
}
static void dw_mci_cleanup_slot(struct dw_mci_slot *slot, unsigned int id)
@@ -2469,24 +2502,6 @@ int dw_mci_probe(struct dw_mci *host)
}
}
- host->vmmc = devm_regulator_get_optional(host->dev, "vmmc");
- if (IS_ERR(host->vmmc)) {
- ret = PTR_ERR(host->vmmc);
- if (ret == -EPROBE_DEFER)
- goto err_clk_ciu;
-
- dev_info(host->dev, "no vmmc regulator found: %d\n", ret);
- host->vmmc = NULL;
- } else {
- ret = regulator_enable(host->vmmc);
- if (ret) {
- if (ret != -EPROBE_DEFER)
- dev_err(host->dev,
- "regulator_enable fail: %d\n", ret);
- goto err_clk_ciu;
- }
- }
-
host->quirks = host->pdata->quirks;
spin_lock_init(&host->lock);
if (host->use_dma && host->dma_ops->exit)
host->dma_ops->exit(host);
- if (host->vmmc)
- regulator_disable(host->vmmc);
if (!IS_ERR(host->ciu_clk))
@@ -2667,9 +2680,6 @@ void dw_mci_remove(struct dw_mci *host)
if (host->use_dma && host->dma_ops->exit)
host->dma_ops->exit(host);
- if (host->vmmc)
- regulator_disable(host->vmmc);
-
if (!IS_ERR(host->ciu_clk))
clk_disable_unprepare(host->ciu_clk);
@@ -2686,9 +2696,6 @@ EXPORT_SYMBOL(dw_mci_remove);
*/
int dw_mci_suspend(struct dw_mci *host)
{
- if (host->vmmc)
- regulator_disable(host->vmmc);
-
return 0;
}
EXPORT_SYMBOL(dw_mci_suspend);
@@ -2697,15 +2704,6 @@ int dw_mci_resume(struct dw_mci *host)
{
int i, ret;
- if (host->vmmc) {
- ret = regulator_enable(host->vmmc);
- if (ret) {
- dev_err(host->dev,
- "failed to enable regulator: %d\n", ret);
- return ret;
- }
- }
-
if (!dw_mci_ctrl_reset(host, SDMMC_CTRL_ALL_RESET_FLAGS)) {
ret = -ENODEV;
return ret;
diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h
index 29ce014..84e2827 100644
--- a/include/linux/mmc/dw_mmc.h
+++ b/include/linux/mmc/dw_mmc.h
@@ -188,7 +188,7 @@ struct dw_mci {
/* Workaround flags */
u32 quirks;
- struct regulator *vmmc; /* Power regulator */
+ bool vqmmc_enabled;
unsigned long irq_flags; /* IRQ flags */
int irq;
};
--
1.7.10.4
Bartlomiej Zolnierkiewicz
2014-09-29 12:31:48 UTC
Permalink
Hi,
Post by Ulf Hansson
Post by Yuvaraj Kumar C D
This patch makes use of mmc_regulator_get_supply() to handle
the vmmc and vqmmc regulators.Also it moves the code handling
the these regulators to dw_mci_set_ios().It turned on the vmmc
and vqmmc during MMC_POWER_UP and MMC_POWER_ON,and turned off
during MMC_POWER_OFF.
Thanks! Applied for next.
Unfortunately this patch breaks mmc1 card (Kingston 32GB micro SDHC)
detection on Exynos5420 Arndale Octa for me:

[ 10.797979] dwmmc_exynos 12220000.mmc: no support for card's volts
[ 10.797998] mmc1: error -22 whilst initialising SD card

Without the patch:

[ 10.866926] mmc_host mmc1: Bus speed (slot 0) = 50000000Hz (slot req 50000000Hz, actual 50000000HZ div = 0)
[ 10.866977] mmc1: new high speed SDHC card at address 1234
[ 10.868730] mmcblk1: mmc1:1234 SA32G 29.3 GiB
[ 10.915054] mmcblk1: p1 p2 p3

The config is attached (exynos_defconfig doesn't work correctly for
this board yet).

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
Post by Ulf Hansson
Kind regards
Uffe
Post by Yuvaraj Kumar C D
---
1.Used mmc_regulator_set_ocr() instead of regulator_enable() for vmmc.
2.Turned on vmmc and vqmmc during MMC_POWER_UP.
3. Removed the flags DW_MMC_CARD_POWERED and DW_MMC_IO_POWERED which
added during the initial version of this patch.
4. Added error message, if it failed to turn on regulator's.
drivers/mmc/host/dw_mmc.c | 72 +++++++++++++++++++++-----------------------
include/linux/mmc/dw_mmc.h | 2 +-
2 files changed, 36 insertions(+), 38 deletions(-)
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 7f227e9..aadb0d6 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -936,6 +936,7 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
struct dw_mci_slot *slot = mmc_priv(mmc);
const struct dw_mci_drv_data *drv_data = slot->host->drv_data;
u32 regs;
+ int ret;
switch (ios->bus_width) {
@@ -974,12 +975,38 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
switch (ios->power_mode) {
+ if (!IS_ERR(mmc->supply.vmmc)) {
+ ret = mmc_regulator_set_ocr(mmc, mmc->supply.vmmc,
+ ios->vdd);
+ if (ret) {
+ dev_err(slot->host->dev,
+ "failed to enable vmmc regulator\n");
+ /*return, if failed turn on vmmc*/
+ return;
+ }
+ }
+ if (!IS_ERR(mmc->supply.vqmmc) && !slot->host->vqmmc_enabled) {
+ ret = regulator_enable(mmc->supply.vqmmc);
+ if (ret < 0)
+ dev_err(slot->host->dev,
+ "failed to enable vqmmc regulator\n");
+ else
+ slot->host->vqmmc_enabled = true;
+ }
set_bit(DW_MMC_CARD_NEED_INIT, &slot->flags);
regs = mci_readl(slot->host, PWREN);
regs |= (1 << slot->id);
mci_writel(slot->host, PWREN, regs);
break;
+ if (!IS_ERR(mmc->supply.vmmc))
+ mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, 0);
+
+ if (!IS_ERR(mmc->supply.vqmmc) && slot->host->vqmmc_enabled) {
+ regulator_disable(mmc->supply.vqmmc);
+ slot->host->vqmmc_enabled = false;
+ }
+
regs = mci_readl(slot->host, PWREN);
regs &= ~(1 << slot->id);
mci_writel(slot->host, PWREN, regs);
@@ -2110,7 +2137,13 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
mmc->f_max = freq[1];
}
- mmc->ocr_avail = MMC_VDD_32_33 | MMC_VDD_33_34;
+ /*if there are external regulators, get them*/
+ ret = mmc_regulator_get_supply(mmc);
+ if (ret == -EPROBE_DEFER)
+ goto err_setup_bus;
+
+ if (!mmc->ocr_avail)
+ mmc->ocr_avail = MMC_VDD_32_33 | MMC_VDD_33_34;
if (host->pdata->caps)
mmc->caps = host->pdata->caps;
@@ -2176,7 +2209,7 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
mmc_free_host(mmc);
- return -EINVAL;
+ return ret;
}
static void dw_mci_cleanup_slot(struct dw_mci_slot *slot, unsigned int id)
@@ -2469,24 +2502,6 @@ int dw_mci_probe(struct dw_mci *host)
}
}
- host->vmmc = devm_regulator_get_optional(host->dev, "vmmc");
- if (IS_ERR(host->vmmc)) {
- ret = PTR_ERR(host->vmmc);
- if (ret == -EPROBE_DEFER)
- goto err_clk_ciu;
-
- dev_info(host->dev, "no vmmc regulator found: %d\n", ret);
- host->vmmc = NULL;
- } else {
- ret = regulator_enable(host->vmmc);
- if (ret) {
- if (ret != -EPROBE_DEFER)
- dev_err(host->dev,
- "regulator_enable fail: %d\n", ret);
- goto err_clk_ciu;
- }
- }
-
host->quirks = host->pdata->quirks;
spin_lock_init(&host->lock);
if (host->use_dma && host->dma_ops->exit)
host->dma_ops->exit(host);
- if (host->vmmc)
- regulator_disable(host->vmmc);
if (!IS_ERR(host->ciu_clk))
@@ -2667,9 +2680,6 @@ void dw_mci_remove(struct dw_mci *host)
if (host->use_dma && host->dma_ops->exit)
host->dma_ops->exit(host);
- if (host->vmmc)
- regulator_disable(host->vmmc);
-
if (!IS_ERR(host->ciu_clk))
clk_disable_unprepare(host->ciu_clk);
@@ -2686,9 +2696,6 @@ EXPORT_SYMBOL(dw_mci_remove);
*/
int dw_mci_suspend(struct dw_mci *host)
{
- if (host->vmmc)
- regulator_disable(host->vmmc);
-
return 0;
}
EXPORT_SYMBOL(dw_mci_suspend);
@@ -2697,15 +2704,6 @@ int dw_mci_resume(struct dw_mci *host)
{
int i, ret;
- if (host->vmmc) {
- ret = regulator_enable(host->vmmc);
- if (ret) {
- dev_err(host->dev,
- "failed to enable regulator: %d\n", ret);
- return ret;
- }
- }
-
if (!dw_mci_ctrl_reset(host, SDMMC_CTRL_ALL_RESET_FLAGS)) {
ret = -ENODEV;
return ret;
diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h
index 29ce014..84e2827 100644
--- a/include/linux/mmc/dw_mmc.h
+++ b/include/linux/mmc/dw_mmc.h
@@ -188,7 +188,7 @@ struct dw_mci {
/* Workaround flags */
u32 quirks;
- struct regulator *vmmc; /* Power regulator */
+ bool vqmmc_enabled;
unsigned long irq_flags; /* IRQ flags */
int irq;
};
--
1.7.10.4
Jaehoon Chung
2014-09-30 05:23:44 UTC
Permalink
Hi
Post by Bartlomiej Zolnierkiewicz
Hi,
Post by Ulf Hansson
Post by Yuvaraj Kumar C D
This patch makes use of mmc_regulator_get_supply() to handle
the vmmc and vqmmc regulators.Also it moves the code handling
the these regulators to dw_mci_set_ios().It turned on the vmmc
and vqmmc during MMC_POWER_UP and MMC_POWER_ON,and turned off
during MMC_POWER_OFF.
Thanks! Applied for next.
Unfortunately this patch breaks mmc1 card (Kingston 32GB micro SDHC)
[ 10.797979] dwmmc_exynos 12220000.mmc: no support for card's volts
[ 10.797998] mmc1: error -22 whilst initialising SD card
OCR value is not matched. Which values are supported about the mmc_host's value and card's value?
Could you share the value?

Best Regards,
Jaehoon Chung
Post by Bartlomiej Zolnierkiewicz
[ 10.866926] mmc_host mmc1: Bus speed (slot 0) = 50000000Hz (slot req 50000000Hz, actual 50000000HZ div = 0)
[ 10.866977] mmc1: new high speed SDHC card at address 1234
[ 10.868730] mmcblk1: mmc1:1234 SA32G 29.3 GiB
[ 10.915054] mmcblk1: p1 p2 p3
The config is attached (exynos_defconfig doesn't work correctly for
this board yet).
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
Post by Ulf Hansson
Kind regards
Uffe
Post by Yuvaraj Kumar C D
---
1.Used mmc_regulator_set_ocr() instead of regulator_enable() for vmmc.
2.Turned on vmmc and vqmmc during MMC_POWER_UP.
3. Removed the flags DW_MMC_CARD_POWERED and DW_MMC_IO_POWERED which
added during the initial version of this patch.
4. Added error message, if it failed to turn on regulator's.
drivers/mmc/host/dw_mmc.c | 72 +++++++++++++++++++++-----------------------
include/linux/mmc/dw_mmc.h | 2 +-
2 files changed, 36 insertions(+), 38 deletions(-)
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 7f227e9..aadb0d6 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -936,6 +936,7 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
struct dw_mci_slot *slot = mmc_priv(mmc);
const struct dw_mci_drv_data *drv_data = slot->host->drv_data;
u32 regs;
+ int ret;
switch (ios->bus_width) {
@@ -974,12 +975,38 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
switch (ios->power_mode) {
+ if (!IS_ERR(mmc->supply.vmmc)) {
+ ret = mmc_regulator_set_ocr(mmc, mmc->supply.vmmc,
+ ios->vdd);
+ if (ret) {
+ dev_err(slot->host->dev,
+ "failed to enable vmmc regulator\n");
+ /*return, if failed turn on vmmc*/
+ return;
+ }
+ }
+ if (!IS_ERR(mmc->supply.vqmmc) && !slot->host->vqmmc_enabled) {
+ ret = regulator_enable(mmc->supply.vqmmc);
+ if (ret < 0)
+ dev_err(slot->host->dev,
+ "failed to enable vqmmc regulator\n");
+ else
+ slot->host->vqmmc_enabled = true;
+ }
set_bit(DW_MMC_CARD_NEED_INIT, &slot->flags);
regs = mci_readl(slot->host, PWREN);
regs |= (1 << slot->id);
mci_writel(slot->host, PWREN, regs);
break;
+ if (!IS_ERR(mmc->supply.vmmc))
+ mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, 0);
+
+ if (!IS_ERR(mmc->supply.vqmmc) && slot->host->vqmmc_enabled) {
+ regulator_disable(mmc->supply.vqmmc);
+ slot->host->vqmmc_enabled = false;
+ }
+
regs = mci_readl(slot->host, PWREN);
regs &= ~(1 << slot->id);
mci_writel(slot->host, PWREN, regs);
@@ -2110,7 +2137,13 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
mmc->f_max = freq[1];
}
- mmc->ocr_avail = MMC_VDD_32_33 | MMC_VDD_33_34;
+ /*if there are external regulators, get them*/
+ ret = mmc_regulator_get_supply(mmc);
+ if (ret == -EPROBE_DEFER)
+ goto err_setup_bus;
+
+ if (!mmc->ocr_avail)
+ mmc->ocr_avail = MMC_VDD_32_33 | MMC_VDD_33_34;
if (host->pdata->caps)
mmc->caps = host->pdata->caps;
@@ -2176,7 +2209,7 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
mmc_free_host(mmc);
- return -EINVAL;
+ return ret;
}
static void dw_mci_cleanup_slot(struct dw_mci_slot *slot, unsigned int id)
@@ -2469,24 +2502,6 @@ int dw_mci_probe(struct dw_mci *host)
}
}
- host->vmmc = devm_regulator_get_optional(host->dev, "vmmc");
- if (IS_ERR(host->vmmc)) {
- ret = PTR_ERR(host->vmmc);
- if (ret == -EPROBE_DEFER)
- goto err_clk_ciu;
-
- dev_info(host->dev, "no vmmc regulator found: %d\n", ret);
- host->vmmc = NULL;
- } else {
- ret = regulator_enable(host->vmmc);
- if (ret) {
- if (ret != -EPROBE_DEFER)
- dev_err(host->dev,
- "regulator_enable fail: %d\n", ret);
- goto err_clk_ciu;
- }
- }
-
host->quirks = host->pdata->quirks;
spin_lock_init(&host->lock);
if (host->use_dma && host->dma_ops->exit)
host->dma_ops->exit(host);
- if (host->vmmc)
- regulator_disable(host->vmmc);
if (!IS_ERR(host->ciu_clk))
@@ -2667,9 +2680,6 @@ void dw_mci_remove(struct dw_mci *host)
if (host->use_dma && host->dma_ops->exit)
host->dma_ops->exit(host);
- if (host->vmmc)
- regulator_disable(host->vmmc);
-
if (!IS_ERR(host->ciu_clk))
clk_disable_unprepare(host->ciu_clk);
@@ -2686,9 +2696,6 @@ EXPORT_SYMBOL(dw_mci_remove);
*/
int dw_mci_suspend(struct dw_mci *host)
{
- if (host->vmmc)
- regulator_disable(host->vmmc);
-
return 0;
}
EXPORT_SYMBOL(dw_mci_suspend);
@@ -2697,15 +2704,6 @@ int dw_mci_resume(struct dw_mci *host)
{
int i, ret;
- if (host->vmmc) {
- ret = regulator_enable(host->vmmc);
- if (ret) {
- dev_err(host->dev,
- "failed to enable regulator: %d\n", ret);
- return ret;
- }
- }
-
if (!dw_mci_ctrl_reset(host, SDMMC_CTRL_ALL_RESET_FLAGS)) {
ret = -ENODEV;
return ret;
diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h
index 29ce014..84e2827 100644
--- a/include/linux/mmc/dw_mmc.h
+++ b/include/linux/mmc/dw_mmc.h
@@ -188,7 +188,7 @@ struct dw_mci {
/* Workaround flags */
u32 quirks;
- struct regulator *vmmc; /* Power regulator */
+ bool vqmmc_enabled;
unsigned long irq_flags; /* IRQ flags */
int irq;
};
--
1.7.10.4
Bartlomiej Zolnierkiewicz
2014-10-01 13:57:54 UTC
Permalink
Hi,
Post by Jaehoon Chung
Hi
Post by Bartlomiej Zolnierkiewicz
Hi,
Post by Ulf Hansson
Post by Yuvaraj Kumar C D
This patch makes use of mmc_regulator_get_supply() to handle
the vmmc and vqmmc regulators.Also it moves the code handling
the these regulators to dw_mci_set_ios().It turned on the vmmc
and vqmmc during MMC_POWER_UP and MMC_POWER_ON,and turned off
during MMC_POWER_OFF.
Thanks! Applied for next.
Unfortunately this patch breaks mmc1 card (Kingston 32GB micro SDHC)
[ 10.797979] dwmmc_exynos 12220000.mmc: no support for card's volts
[ 10.797998] mmc1: error -22 whilst initialising SD card
OCR value is not matched. Which values are supported about the mmc_host's value and card's value?
Could you share the value?
Sure. I've added dev_info()s at the beginning of mmc_select_voltage():

+ dev_warn(mmc_dev(host), "card's volts: 0x%0x\n", ocr);
+ dev_warn(mmc_dev(host), "host's volts: 0x%0x\n", host->ocr_avail);

and got following results:

[ 10.851678] dwmmc_exynos 12220000.mmc: card's volts: 0xff8000
[ 10.851691] dwmmc_exynos 12220000.mmc: host's volts: 0x80

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
Post by Jaehoon Chung
Best Regards,
Jaehoon Chung
Post by Bartlomiej Zolnierkiewicz
[ 10.866926] mmc_host mmc1: Bus speed (slot 0) = 50000000Hz (slot req 50000000Hz, actual 50000000HZ div = 0)
[ 10.866977] mmc1: new high speed SDHC card at address 1234
[ 10.868730] mmcblk1: mmc1:1234 SA32G 29.3 GiB
[ 10.915054] mmcblk1: p1 p2 p3
The config is attached (exynos_defconfig doesn't work correctly for
this board yet).
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
Post by Ulf Hansson
Kind regards
Uffe
Post by Yuvaraj Kumar C D
---
1.Used mmc_regulator_set_ocr() instead of regulator_enable() for vmmc.
2.Turned on vmmc and vqmmc during MMC_POWER_UP.
3. Removed the flags DW_MMC_CARD_POWERED and DW_MMC_IO_POWERED which
added during the initial version of this patch.
4. Added error message, if it failed to turn on regulator's.
drivers/mmc/host/dw_mmc.c | 72 +++++++++++++++++++++-----------------------
include/linux/mmc/dw_mmc.h | 2 +-
2 files changed, 36 insertions(+), 38 deletions(-)
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 7f227e9..aadb0d6 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -936,6 +936,7 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
struct dw_mci_slot *slot = mmc_priv(mmc);
const struct dw_mci_drv_data *drv_data = slot->host->drv_data;
u32 regs;
+ int ret;
switch (ios->bus_width) {
@@ -974,12 +975,38 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
switch (ios->power_mode) {
+ if (!IS_ERR(mmc->supply.vmmc)) {
+ ret = mmc_regulator_set_ocr(mmc, mmc->supply.vmmc,
+ ios->vdd);
+ if (ret) {
+ dev_err(slot->host->dev,
+ "failed to enable vmmc regulator\n");
+ /*return, if failed turn on vmmc*/
+ return;
+ }
+ }
+ if (!IS_ERR(mmc->supply.vqmmc) && !slot->host->vqmmc_enabled) {
+ ret = regulator_enable(mmc->supply.vqmmc);
+ if (ret < 0)
+ dev_err(slot->host->dev,
+ "failed to enable vqmmc regulator\n");
+ else
+ slot->host->vqmmc_enabled = true;
+ }
set_bit(DW_MMC_CARD_NEED_INIT, &slot->flags);
regs = mci_readl(slot->host, PWREN);
regs |= (1 << slot->id);
mci_writel(slot->host, PWREN, regs);
break;
+ if (!IS_ERR(mmc->supply.vmmc))
+ mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, 0);
+
+ if (!IS_ERR(mmc->supply.vqmmc) && slot->host->vqmmc_enabled) {
+ regulator_disable(mmc->supply.vqmmc);
+ slot->host->vqmmc_enabled = false;
+ }
+
regs = mci_readl(slot->host, PWREN);
regs &= ~(1 << slot->id);
mci_writel(slot->host, PWREN, regs);
@@ -2110,7 +2137,13 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
mmc->f_max = freq[1];
}
- mmc->ocr_avail = MMC_VDD_32_33 | MMC_VDD_33_34;
+ /*if there are external regulators, get them*/
+ ret = mmc_regulator_get_supply(mmc);
+ if (ret == -EPROBE_DEFER)
+ goto err_setup_bus;
+
+ if (!mmc->ocr_avail)
+ mmc->ocr_avail = MMC_VDD_32_33 | MMC_VDD_33_34;
if (host->pdata->caps)
mmc->caps = host->pdata->caps;
@@ -2176,7 +2209,7 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
mmc_free_host(mmc);
- return -EINVAL;
+ return ret;
}
static void dw_mci_cleanup_slot(struct dw_mci_slot *slot, unsigned int id)
@@ -2469,24 +2502,6 @@ int dw_mci_probe(struct dw_mci *host)
}
}
- host->vmmc = devm_regulator_get_optional(host->dev, "vmmc");
- if (IS_ERR(host->vmmc)) {
- ret = PTR_ERR(host->vmmc);
- if (ret == -EPROBE_DEFER)
- goto err_clk_ciu;
-
- dev_info(host->dev, "no vmmc regulator found: %d\n", ret);
- host->vmmc = NULL;
- } else {
- ret = regulator_enable(host->vmmc);
- if (ret) {
- if (ret != -EPROBE_DEFER)
- dev_err(host->dev,
- "regulator_enable fail: %d\n", ret);
- goto err_clk_ciu;
- }
- }
-
host->quirks = host->pdata->quirks;
spin_lock_init(&host->lock);
if (host->use_dma && host->dma_ops->exit)
host->dma_ops->exit(host);
- if (host->vmmc)
- regulator_disable(host->vmmc);
if (!IS_ERR(host->ciu_clk))
@@ -2667,9 +2680,6 @@ void dw_mci_remove(struct dw_mci *host)
if (host->use_dma && host->dma_ops->exit)
host->dma_ops->exit(host);
- if (host->vmmc)
- regulator_disable(host->vmmc);
-
if (!IS_ERR(host->ciu_clk))
clk_disable_unprepare(host->ciu_clk);
@@ -2686,9 +2696,6 @@ EXPORT_SYMBOL(dw_mci_remove);
*/
int dw_mci_suspend(struct dw_mci *host)
{
- if (host->vmmc)
- regulator_disable(host->vmmc);
-
return 0;
}
EXPORT_SYMBOL(dw_mci_suspend);
@@ -2697,15 +2704,6 @@ int dw_mci_resume(struct dw_mci *host)
{
int i, ret;
- if (host->vmmc) {
- ret = regulator_enable(host->vmmc);
- if (ret) {
- dev_err(host->dev,
- "failed to enable regulator: %d\n", ret);
- return ret;
- }
- }
-
if (!dw_mci_ctrl_reset(host, SDMMC_CTRL_ALL_RESET_FLAGS)) {
ret = -ENODEV;
return ret;
diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h
index 29ce014..84e2827 100644
--- a/include/linux/mmc/dw_mmc.h
+++ b/include/linux/mmc/dw_mmc.h
@@ -188,7 +188,7 @@ struct dw_mci {
/* Workaround flags */
u32 quirks;
- struct regulator *vmmc; /* Power regulator */
+ bool vqmmc_enabled;
unsigned long irq_flags; /* IRQ flags */
int irq;
};
--
1.7.10.4
Bartlomiej Zolnierkiewicz
2014-10-01 14:14:32 UTC
Permalink
Post by Bartlomiej Zolnierkiewicz
Hi,
Post by Jaehoon Chung
Hi
Post by Bartlomiej Zolnierkiewicz
Hi,
Post by Ulf Hansson
Post by Yuvaraj Kumar C D
This patch makes use of mmc_regulator_get_supply() to handle
the vmmc and vqmmc regulators.Also it moves the code handling
the these regulators to dw_mci_set_ios().It turned on the vmmc
and vqmmc during MMC_POWER_UP and MMC_POWER_ON,and turned off
during MMC_POWER_OFF.
Thanks! Applied for next.
Unfortunately this patch breaks mmc1 card (Kingston 32GB micro SDHC)
[ 10.797979] dwmmc_exynos 12220000.mmc: no support for card's volts
[ 10.797998] mmc1: error -22 whilst initialising SD card
OCR value is not matched. Which values are supported about the mmc_host's value and card's value?
Could you share the value?
+ dev_warn(mmc_dev(host), "card's volts: 0x%0x\n", ocr);
+ dev_warn(mmc_dev(host), "host's volts: 0x%0x\n", host->ocr_avail);
[ 10.851678] dwmmc_exynos 12220000.mmc: card's volts: 0xff8000
[ 10.851691] dwmmc_exynos 12220000.mmc: host's volts: 0x80
Data for the working kernel:

[ 10.856214] dwmmc_exynos 12220000.mmc: card's volts: 0xff8000
[ 10.856227] dwmmc_exynos 12220000.mmc: host's volts: 0x300000

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
Post by Bartlomiej Zolnierkiewicz
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
Post by Jaehoon Chung
Best Regards,
Jaehoon Chung
Post by Bartlomiej Zolnierkiewicz
[ 10.866926] mmc_host mmc1: Bus speed (slot 0) = 50000000Hz (slot req 50000000Hz, actual 50000000HZ div = 0)
[ 10.866977] mmc1: new high speed SDHC card at address 1234
[ 10.868730] mmcblk1: mmc1:1234 SA32G 29.3 GiB
[ 10.915054] mmcblk1: p1 p2 p3
The config is attached (exynos_defconfig doesn't work correctly for
this board yet).
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
Post by Ulf Hansson
Kind regards
Uffe
Post by Yuvaraj Kumar C D
---
1.Used mmc_regulator_set_ocr() instead of regulator_enable() for vmmc.
2.Turned on vmmc and vqmmc during MMC_POWER_UP.
3. Removed the flags DW_MMC_CARD_POWERED and DW_MMC_IO_POWERED which
added during the initial version of this patch.
4. Added error message, if it failed to turn on regulator's.
drivers/mmc/host/dw_mmc.c | 72 +++++++++++++++++++++-----------------------
include/linux/mmc/dw_mmc.h | 2 +-
2 files changed, 36 insertions(+), 38 deletions(-)
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 7f227e9..aadb0d6 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -936,6 +936,7 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
struct dw_mci_slot *slot = mmc_priv(mmc);
const struct dw_mci_drv_data *drv_data = slot->host->drv_data;
u32 regs;
+ int ret;
switch (ios->bus_width) {
@@ -974,12 +975,38 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
switch (ios->power_mode) {
+ if (!IS_ERR(mmc->supply.vmmc)) {
+ ret = mmc_regulator_set_ocr(mmc, mmc->supply.vmmc,
+ ios->vdd);
+ if (ret) {
+ dev_err(slot->host->dev,
+ "failed to enable vmmc regulator\n");
+ /*return, if failed turn on vmmc*/
+ return;
+ }
+ }
+ if (!IS_ERR(mmc->supply.vqmmc) && !slot->host->vqmmc_enabled) {
+ ret = regulator_enable(mmc->supply.vqmmc);
+ if (ret < 0)
+ dev_err(slot->host->dev,
+ "failed to enable vqmmc regulator\n");
+ else
+ slot->host->vqmmc_enabled = true;
+ }
set_bit(DW_MMC_CARD_NEED_INIT, &slot->flags);
regs = mci_readl(slot->host, PWREN);
regs |= (1 << slot->id);
mci_writel(slot->host, PWREN, regs);
break;
+ if (!IS_ERR(mmc->supply.vmmc))
+ mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, 0);
+
+ if (!IS_ERR(mmc->supply.vqmmc) && slot->host->vqmmc_enabled) {
+ regulator_disable(mmc->supply.vqmmc);
+ slot->host->vqmmc_enabled = false;
+ }
+
regs = mci_readl(slot->host, PWREN);
regs &= ~(1 << slot->id);
mci_writel(slot->host, PWREN, regs);
@@ -2110,7 +2137,13 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
mmc->f_max = freq[1];
}
- mmc->ocr_avail = MMC_VDD_32_33 | MMC_VDD_33_34;
+ /*if there are external regulators, get them*/
+ ret = mmc_regulator_get_supply(mmc);
+ if (ret == -EPROBE_DEFER)
+ goto err_setup_bus;
+
+ if (!mmc->ocr_avail)
+ mmc->ocr_avail = MMC_VDD_32_33 | MMC_VDD_33_34;
if (host->pdata->caps)
mmc->caps = host->pdata->caps;
@@ -2176,7 +2209,7 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
mmc_free_host(mmc);
- return -EINVAL;
+ return ret;
}
static void dw_mci_cleanup_slot(struct dw_mci_slot *slot, unsigned int id)
@@ -2469,24 +2502,6 @@ int dw_mci_probe(struct dw_mci *host)
}
}
- host->vmmc = devm_regulator_get_optional(host->dev, "vmmc");
- if (IS_ERR(host->vmmc)) {
- ret = PTR_ERR(host->vmmc);
- if (ret == -EPROBE_DEFER)
- goto err_clk_ciu;
-
- dev_info(host->dev, "no vmmc regulator found: %d\n", ret);
- host->vmmc = NULL;
- } else {
- ret = regulator_enable(host->vmmc);
- if (ret) {
- if (ret != -EPROBE_DEFER)
- dev_err(host->dev,
- "regulator_enable fail: %d\n", ret);
- goto err_clk_ciu;
- }
- }
-
host->quirks = host->pdata->quirks;
spin_lock_init(&host->lock);
if (host->use_dma && host->dma_ops->exit)
host->dma_ops->exit(host);
- if (host->vmmc)
- regulator_disable(host->vmmc);
if (!IS_ERR(host->ciu_clk))
@@ -2667,9 +2680,6 @@ void dw_mci_remove(struct dw_mci *host)
if (host->use_dma && host->dma_ops->exit)
host->dma_ops->exit(host);
- if (host->vmmc)
- regulator_disable(host->vmmc);
-
if (!IS_ERR(host->ciu_clk))
clk_disable_unprepare(host->ciu_clk);
@@ -2686,9 +2696,6 @@ EXPORT_SYMBOL(dw_mci_remove);
*/
int dw_mci_suspend(struct dw_mci *host)
{
- if (host->vmmc)
- regulator_disable(host->vmmc);
-
return 0;
}
EXPORT_SYMBOL(dw_mci_suspend);
@@ -2697,15 +2704,6 @@ int dw_mci_resume(struct dw_mci *host)
{
int i, ret;
- if (host->vmmc) {
- ret = regulator_enable(host->vmmc);
- if (ret) {
- dev_err(host->dev,
- "failed to enable regulator: %d\n", ret);
- return ret;
- }
- }
-
if (!dw_mci_ctrl_reset(host, SDMMC_CTRL_ALL_RESET_FLAGS)) {
ret = -ENODEV;
return ret;
diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h
index 29ce014..84e2827 100644
--- a/include/linux/mmc/dw_mmc.h
+++ b/include/linux/mmc/dw_mmc.h
@@ -188,7 +188,7 @@ struct dw_mci {
/* Workaround flags */
u32 quirks;
- struct regulator *vmmc; /* Power regulator */
+ bool vqmmc_enabled;
unsigned long irq_flags; /* IRQ flags */
int irq;
};
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Doug Anderson
2014-09-30 17:22:34 UTC
Permalink
Bartlomiej

On Mon, Sep 29, 2014 at 5:31 AM, Bartlomiej Zolnierkiewicz
Post by Bartlomiej Zolnierkiewicz
Hi,
Post by Ulf Hansson
Post by Yuvaraj Kumar C D
This patch makes use of mmc_regulator_get_supply() to handle
the vmmc and vqmmc regulators.Also it moves the code handling
the these regulators to dw_mci_set_ios().It turned on the vmmc
and vqmmc during MMC_POWER_UP and MMC_POWER_ON,and turned off
during MMC_POWER_OFF.
Thanks! Applied for next.
Unfortunately this patch breaks mmc1 card (Kingston 32GB micro SDHC)
[ 10.797979] dwmmc_exynos 12220000.mmc: no support for card's volts
[ 10.797998] mmc1: error -22 whilst initialising SD card
[ 10.866926] mmc_host mmc1: Bus speed (slot 0) = 50000000Hz (slot req 50000000Hz, actual 50000000HZ div = 0)
[ 10.866977] mmc1: new high speed SDHC card at address 1234
[ 10.868730] mmcblk1: mmc1:1234 SA32G 29.3 GiB
[ 10.915054] mmcblk1: p1 p2 p3
The config is attached (exynos_defconfig doesn't work correctly for
this board yet).
Yup, this is an expected behavior, unfortunately. This was talked
about extensively during the review of this patch series.

I believe that patch #3 in Yuvaraj's series would fix your problem.
Specifically <https://patchwork.kernel.org/patch/4763891/>.


The current summary of this issue is (Ulf, please correct me if I got
anything wrong):

1. If nothing else, Yuvaraj's patch should probably be split in two.
One half should be the MMC core half that I originally sent Yuvaraj.
I just rebased and re-uploaed it at
<https://chromium-review.googlesource.com/220560> in case you're
curious. The second half should be the dw_mmc piece that Yuvaraj
wrote.

2. Ulf has indicated that he thinks that the MMC core change (and thus
Yuvaraj's patch) is ugly and not necessary. He advocates instead
using the MMC_CAP_NEEDS_POLL on all affected platforms. That means
we'll turn on the power every second, check for the card, then turn
off power.

3. I'm still of the opinion that the MMC core change isn't _that_
ugly. Given that there are a large number of systems affected (across
at least two SoC vendors) and that it would be nice if those systems
didn't have to poll, I'd still be happy if the MMC core change could
go in. ...but I'm a pragmatist and know that the polling isn't
_terrible_, so if that's the way we need to go then so be it.

--

My understanding was that Yuvaraj was going to spin his patch to use a
similar type of scheme to autodetect affected SoCs (looking for SoCs
known to have this problem where the platform data shows them using
the built-in card detect) and then enable MMC_CAP_NEEDS_POLL. I
haven't seen a patch from him, so maybe you could post this up?

Note: the reason why exynos5250-snow and some other boards aren't
affected yet is that the regulator is simply not specified in the
device tree (it's just left always on).

---

-Doug
Bartlomiej Zolnierkiewicz
2014-10-01 13:06:41 UTC
Permalink
Hi,
Post by Bartlomiej Zolnierkiewicz
Bartlomiej
On Mon, Sep 29, 2014 at 5:31 AM, Bartlomiej Zolnierkiewicz
Post by Bartlomiej Zolnierkiewicz
Hi,
Post by Ulf Hansson
Post by Yuvaraj Kumar C D
This patch makes use of mmc_regulator_get_supply() to handle
the vmmc and vqmmc regulators.Also it moves the code handling
the these regulators to dw_mci_set_ios().It turned on the vmmc
and vqmmc during MMC_POWER_UP and MMC_POWER_ON,and turned off
during MMC_POWER_OFF.
Thanks! Applied for next.
Unfortunately this patch breaks mmc1 card (Kingston 32GB micro SDHC)
[ 10.797979] dwmmc_exynos 12220000.mmc: no support for card's volts
[ 10.797998] mmc1: error -22 whilst initialising SD card
[ 10.866926] mmc_host mmc1: Bus speed (slot 0) = 50000000Hz (slot req 50000000Hz, actual 50000000HZ div = 0)
[ 10.866977] mmc1: new high speed SDHC card at address 1234
[ 10.868730] mmcblk1: mmc1:1234 SA32G 29.3 GiB
[ 10.915054] mmcblk1: p1 p2 p3
The config is attached (exynos_defconfig doesn't work correctly for
this board yet).
Yup, this is an expected behavior, unfortunately. This was talked
about extensively during the review of this patch series.
Do you mean that a patch with a known regression has been merged
into next branch of mmc tree? It would be quite sad if it would be
true.
Post by Bartlomiej Zolnierkiewicz
I believe that patch #3 in Yuvaraj's series would fix your problem.
Specifically <https://patchwork.kernel.org/patch/4763891/>.
Unfortunately this patch doesn't fix the problem (there is no longer
a lockup on regulators initialization but -22 error is still present).
Post by Bartlomiej Zolnierkiewicz
The current summary of this issue is (Ulf, please correct me if I got
1. If nothing else, Yuvaraj's patch should probably be split in two.
One half should be the MMC core half that I originally sent Yuvaraj.
I just rebased and re-uploaed it at
<https://chromium-review.googlesource.com/220560> in case you're
curious. The second half should be the dw_mmc piece that Yuvaraj
wrote.
2. Ulf has indicated that he thinks that the MMC core change (and thus
Yuvaraj's patch) is ugly and not necessary. He advocates instead
using the MMC_CAP_NEEDS_POLL on all affected platforms. That means
we'll turn on the power every second, check for the card, then turn
off power.
3. I'm still of the opinion that the MMC core change isn't _that_
ugly. Given that there are a large number of systems affected (across
It also doesn't look that bad for me and well, when the hardware is
quirky then the resulting code can't be esthetically perfect..
Post by Bartlomiej Zolnierkiewicz
at least two SoC vendors) and that it would be nice if those systems
didn't have to poll, I'd still be happy if the MMC core change could
go in. ...but I'm a pragmatist and know that the polling isn't
_terrible_, so if that's the way we need to go then so be it.
--
My understanding was that Yuvaraj was going to spin his patch to use a
similar type of scheme to autodetect affected SoCs (looking for SoCs
known to have this problem where the platform data shows them using
the built-in card detect) and then enable MMC_CAP_NEEDS_POLL. I
haven't seen a patch from him, so maybe you could post this up?
I worry that I have too little time and MMC code expertise to do this.
Post by Bartlomiej Zolnierkiewicz
Note: the reason why exynos5250-snow and some other boards aren't
affected yet is that the regulator is simply not specified in the
device tree (it's just left always on).
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Doug Anderson
2014-10-01 15:38:19 UTC
Permalink
Hi,

On Wed, Oct 1, 2014 at 6:06 AM, Bartlomiej Zolnierkiewicz
Post by Bartlomiej Zolnierkiewicz
Hi,
Post by Bartlomiej Zolnierkiewicz
Bartlomiej
On Mon, Sep 29, 2014 at 5:31 AM, Bartlomiej Zolnierkiewicz
Post by Bartlomiej Zolnierkiewicz
Hi,
Post by Ulf Hansson
Post by Yuvaraj Kumar C D
This patch makes use of mmc_regulator_get_supply() to handle
the vmmc and vqmmc regulators.Also it moves the code handling
the these regulators to dw_mci_set_ios().It turned on the vmmc
and vqmmc during MMC_POWER_UP and MMC_POWER_ON,and turned off
during MMC_POWER_OFF.
Thanks! Applied for next.
Unfortunately this patch breaks mmc1 card (Kingston 32GB micro SDHC)
[ 10.797979] dwmmc_exynos 12220000.mmc: no support for card's volts
[ 10.797998] mmc1: error -22 whilst initialising SD card
[ 10.866926] mmc_host mmc1: Bus speed (slot 0) = 50000000Hz (slot req 50000000Hz, actual 50000000HZ div = 0)
[ 10.866977] mmc1: new high speed SDHC card at address 1234
[ 10.868730] mmcblk1: mmc1:1234 SA32G 29.3 GiB
[ 10.915054] mmcblk1: p1 p2 p3
The config is attached (exynos_defconfig doesn't work correctly for
this board yet).
Yup, this is an expected behavior, unfortunately. This was talked
about extensively during the review of this patch series.
Do you mean that a patch with a known regression has been merged
into next branch of mmc tree? It would be quite sad if it would be
true.
...so looking at your "dts" file, it looks like this _isn't_ your
problem. Your "vmmc" regulator is listed as "always on", so I believe
that you're OK. Your regulator probably _shouldn't_ be "always on"
(it prevents power cycling the SD card) but right now it's good that
it is...

If any board has a "vmmc" that is not listed as always on then it
would regress with the first two patches applied (and without the
3rd), but there are none that I personally know of that do.
Yuvaraj Kumar C D
2014-08-22 13:47:51 UTC
Permalink
From: Doug Anderson <***@chromium.org>

For UHS cards we need the ability to switch voltages from 3.3V to
1.8V. Add support to the dw_mmc driver to handle this. Note that
dw_mmc needs a little bit of extra code since the interface needs a
special bit programmed to the CMD register while CMD11 is progressing.
This means adding a few extra states to the state machine to track.

Signed-off-by: Doug Anderson <***@chromium.org>
Signed-off-by: Yuvaraj Kumar C D <***@samsung.com>
---
changes since v1:
1. Added error message and return error in case of regulator_set_voltage() fail.
2. changed dw_mci_cmd_interrupt(host,pending | SDMMC_INT_CMD_DONE)
to dw_mci_cmd_interrupt(host,pending).
3. Removed unnecessary comments.

drivers/mmc/host/dw_mmc.c | 134 +++++++++++++++++++++++++++++++++++++++++---
drivers/mmc/host/dw_mmc.h | 5 +-
include/linux/mmc/dw_mmc.h | 2 +
3 files changed, 131 insertions(+), 10 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index aadb0d6..f20b4b8 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -29,6 +29,7 @@
#include <linux/irq.h>
#include <linux/mmc/host.h>
#include <linux/mmc/mmc.h>
+#include <linux/mmc/sd.h>
#include <linux/mmc/sdio.h>
#include <linux/mmc/dw_mmc.h>
#include <linux/bitops.h>
@@ -234,10 +235,13 @@ err:
}
#endif /* defined(CONFIG_DEBUG_FS) */

+static void mci_send_cmd(struct dw_mci_slot *slot, u32 cmd, u32 arg);
+
static u32 dw_mci_prepare_command(struct mmc_host *mmc, struct mmc_command *cmd)
{
struct mmc_data *data;
struct dw_mci_slot *slot = mmc_priv(mmc);
+ struct dw_mci *host = slot->host;
const struct dw_mci_drv_data *drv_data = slot->host->drv_data;
u32 cmdr;
cmd->error = -EINPROGRESS;
@@ -253,6 +257,31 @@ static u32 dw_mci_prepare_command(struct mmc_host *mmc, struct mmc_command *cmd)
else if (cmd->opcode != MMC_SEND_STATUS && cmd->data)
cmdr |= SDMMC_CMD_PRV_DAT_WAIT;

+ if (cmd->opcode == SD_SWITCH_VOLTAGE) {
+ u32 clk_en_a;
+
+ /* Special bit makes CMD11 not die */
+ cmdr |= SDMMC_CMD_VOLT_SWITCH;
+
+ /* Change state to continue to handle CMD11 weirdness */
+ WARN_ON(slot->host->state != STATE_SENDING_CMD);
+ slot->host->state = STATE_SENDING_CMD11;
+
+ /*
+ * We need to disable clock stop while doing voltage switch
+ * according to Voltage Switch Normal Scenario.
+ * It's assumed that by the next time the CLKENA is updated
+ * (when we set the clock next) that the voltage change will
+ * be over, so we don't bother setting any bits to synchronize
+ * with dw_mci_setup_bus().
+ */
+ clk_en_a = mci_readl(host, CLKENA);
+ clk_en_a &= ~(SDMMC_CLKEN_LOW_PWR << slot->id);
+ mci_writel(host, CLKENA, clk_en_a);
+ mci_send_cmd(slot, SDMMC_CMD_UPD_CLK |
+ SDMMC_CMD_PRV_DAT_WAIT, 0);
+ }
+
if (cmd->flags & MMC_RSP_PRESENT) {
/* We expect a response, so set this bit */
cmdr |= SDMMC_CMD_RESP_EXP;
@@ -775,11 +804,15 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot, bool force_clkinit)
unsigned int clock = slot->clock;
u32 div;
u32 clk_en_a;
+ u32 sdmmc_cmd_bits = SDMMC_CMD_UPD_CLK | SDMMC_CMD_PRV_DAT_WAIT;
+
+ /* We must continue to set bit 28 in CMD until the change is complete */
+ if (host->state == STATE_WAITING_CMD11_DONE)
+ sdmmc_cmd_bits |= SDMMC_CMD_VOLT_SWITCH;

if (!clock) {
mci_writel(host, CLKENA, 0);
- mci_send_cmd(slot,
- SDMMC_CMD_UPD_CLK | SDMMC_CMD_PRV_DAT_WAIT, 0);
+ mci_send_cmd(slot, sdmmc_cmd_bits, 0);
} else if (clock != host->current_speed || force_clkinit) {
div = host->bus_hz / clock;
if (host->bus_hz % clock && host->bus_hz > clock)
@@ -803,15 +836,13 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot, bool force_clkinit)
mci_writel(host, CLKSRC, 0);

/* inform CIU */
- mci_send_cmd(slot,
- SDMMC_CMD_UPD_CLK | SDMMC_CMD_PRV_DAT_WAIT, 0);
+ mci_send_cmd(slot, sdmmc_cmd_bits, 0);

/* set clock to desired speed */
mci_writel(host, CLKDIV, div);

/* inform CIU */
- mci_send_cmd(slot,
- SDMMC_CMD_UPD_CLK | SDMMC_CMD_PRV_DAT_WAIT, 0);
+ mci_send_cmd(slot, sdmmc_cmd_bits, 0);

/* enable clock; only low power if no SDIO */
clk_en_a = SDMMC_CLKEN_ENABLE << slot->id;
@@ -820,8 +851,7 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot, bool force_clkinit)
mci_writel(host, CLKENA, clk_en_a);

/* inform CIU */
- mci_send_cmd(slot,
- SDMMC_CMD_UPD_CLK | SDMMC_CMD_PRV_DAT_WAIT, 0);
+ mci_send_cmd(slot, sdmmc_cmd_bits, 0);

/* keep the clock with reflecting clock dividor */
slot->__clk_old = clock << div;
@@ -897,6 +927,17 @@ static void dw_mci_queue_request(struct dw_mci *host, struct dw_mci_slot *slot,

slot->mrq = mrq;

+ if (host->state == STATE_WAITING_CMD11_DONE) {
+ dev_warn(&slot->mmc->class_dev,
+ "Voltage change didn't complete\n");
+ /*
+ * this case isn't expected to happen, so we can
+ * either crash here or just try to continue on
+ * in the closest possible state
+ */
+ host->state = STATE_IDLE;
+ }
+
if (host->state == STATE_IDLE) {
host->state = STATE_SENDING_CMD;
dw_mci_start_request(host, slot);
@@ -973,6 +1014,9 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
/* Slot specific timing and width adjustment */
dw_mci_setup_bus(slot, false);

+ if (slot->host->state == STATE_WAITING_CMD11_DONE && ios->clock != 0)
+ slot->host->state = STATE_IDLE;
+
switch (ios->power_mode) {
case MMC_POWER_UP:
if (!IS_ERR(mmc->supply.vmmc)) {
@@ -1016,6 +1060,59 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
}
}

+static int dw_mci_card_busy(struct mmc_host *mmc)
+{
+ struct dw_mci_slot *slot = mmc_priv(mmc);
+ u32 status;
+
+ /*
+ * Check the busy bit which is low when DAT[3:0]
+ * (the data lines) are 0000
+ */
+ status = mci_readl(slot->host, STATUS);
+
+ return !!(status & SDMMC_STATUS_BUSY);
+}
+
+static int dw_mci_switch_voltage(struct mmc_host *mmc, struct mmc_ios *ios)
+{
+ struct dw_mci_slot *slot = mmc_priv(mmc);
+ struct dw_mci *host = slot->host;
+ u32 uhs;
+ u32 v18 = SDMMC_UHS_18V << slot->id;
+ int min_uv, max_uv;
+ int ret;
+
+ /*
+ * Program the voltage. Note that some instances of dw_mmc may use
+ * the UHS_REG for this. For other instances (like exynos) the UHS_REG
+ * does no harm but you need to set the regulator directly. Try both.
+ */
+ uhs = mci_readl(host, UHS_REG);
+ if (ios->signal_voltage == MMC_SIGNAL_VOLTAGE_330) {
+ min_uv = 2700000;
+ max_uv = 3600000;
+ uhs &= ~v18;
+ } else {
+ min_uv = 1700000;
+ max_uv = 1950000;
+ uhs |= v18;
+ }
+ if (!IS_ERR(mmc->supply.vqmmc)) {
+ ret = regulator_set_voltage(mmc->supply.vqmmc, min_uv, max_uv);
+
+ if (ret) {
+ dev_err(&mmc->class_dev,
+ "Regulator set error %d: %d - %d\n",
+ ret, min_uv, max_uv);
+ return ret;
+ }
+ }
+ mci_writel(host, UHS_REG, uhs);
+
+ return 0;
+}
+
static int dw_mci_get_ro(struct mmc_host *mmc)
{
int read_only;
@@ -1158,6 +1255,9 @@ static const struct mmc_host_ops dw_mci_ops = {
.get_cd = dw_mci_get_cd,
.enable_sdio_irq = dw_mci_enable_sdio_irq,
.execute_tuning = dw_mci_execute_tuning,
+ .card_busy = dw_mci_card_busy,
+ .start_signal_voltage_switch = dw_mci_switch_voltage,
+
};

static void dw_mci_request_end(struct dw_mci *host, struct mmc_request *mrq)
@@ -1181,7 +1281,11 @@ static void dw_mci_request_end(struct dw_mci *host, struct mmc_request *mrq)
dw_mci_start_request(host, slot);
} else {
dev_vdbg(host->dev, "list empty\n");
- host->state = STATE_IDLE;
+
+ if (host->state == STATE_SENDING_CMD11)
+ host->state = STATE_WAITING_CMD11_DONE;
+ else
+ host->state = STATE_IDLE;
}

spin_unlock(&host->lock);
@@ -1292,8 +1396,10 @@ static void dw_mci_tasklet_func(unsigned long priv)

switch (state) {
case STATE_IDLE:
+ case STATE_WAITING_CMD11_DONE:
break;

+ case STATE_SENDING_CMD11:
case STATE_SENDING_CMD:
if (!test_and_clear_bit(EVENT_CMD_COMPLETE,
&host->pending_events))
@@ -1894,6 +2000,14 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id)
}

if (pending) {
+ /* Check volt switch first, since it can look like an error */
+ if ((host->state == STATE_SENDING_CMD11) &&
+ (pending & SDMMC_INT_VOLT_SWITCH)) {
+ mci_writel(host, RINTSTS, SDMMC_INT_VOLT_SWITCH);
+ pending &= ~SDMMC_INT_VOLT_SWITCH;
+ dw_mci_cmd_interrupt(host, pending);
+ }
+
if (pending & DW_MCI_CMD_ERROR_FLAGS) {
mci_writel(host, RINTSTS, DW_MCI_CMD_ERROR_FLAGS);
host->cmd_status = pending;
@@ -1999,7 +2113,9 @@ static void dw_mci_work_routine_card(struct work_struct *work)

switch (host->state) {
case STATE_IDLE:
+ case STATE_WAITING_CMD11_DONE:
break;
+ case STATE_SENDING_CMD11:
case STATE_SENDING_CMD:
mrq->cmd->error = -ENOMEDIUM;
if (!mrq->data)
diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
index 08fd956..01b99e8 100644
--- a/drivers/mmc/host/dw_mmc.h
+++ b/drivers/mmc/host/dw_mmc.h
@@ -99,6 +99,7 @@
#define SDMMC_INT_HLE BIT(12)
#define SDMMC_INT_FRUN BIT(11)
#define SDMMC_INT_HTO BIT(10)
+#define SDMMC_INT_VOLT_SWITCH BIT(10) /* overloads bit 10! */
#define SDMMC_INT_DRTO BIT(9)
#define SDMMC_INT_RTO BIT(8)
#define SDMMC_INT_DCRC BIT(7)
@@ -113,6 +114,7 @@
/* Command register defines */
#define SDMMC_CMD_START BIT(31)
#define SDMMC_CMD_USE_HOLD_REG BIT(29)
+#define SDMMC_CMD_VOLT_SWITCH BIT(28)
#define SDMMC_CMD_CCS_EXP BIT(23)
#define SDMMC_CMD_CEATA_RD BIT(22)
#define SDMMC_CMD_UPD_CLK BIT(21)
@@ -130,6 +132,7 @@
/* Status register defines */
#define SDMMC_GET_FCNT(x) (((x)>>17) & 0x1FFF)
#define SDMMC_STATUS_DMA_REQ BIT(31)
+#define SDMMC_STATUS_BUSY BIT(9)
/* FIFOTH register defines */
#define SDMMC_SET_FIFOTH(m, r, t) (((m) & 0x7) << 28 | \
((r) & 0xFFF) << 16 | \
@@ -150,7 +153,7 @@
#define SDMMC_GET_VERID(x) ((x) & 0xFFFF)
/* Card read threshold */
#define SDMMC_SET_RD_THLD(v, x) (((v) & 0x1FFF) << 16 | (x))
-
+#define SDMMC_UHS_18V BIT(0)
/* All ctrl reset bits */
#define SDMMC_CTRL_ALL_RESET_FLAGS \
(SDMMC_CTRL_RESET | SDMMC_CTRL_FIFO_RESET | SDMMC_CTRL_DMA_RESET)
diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h
index 84e2827..0013669 100644
--- a/include/linux/mmc/dw_mmc.h
+++ b/include/linux/mmc/dw_mmc.h
@@ -26,6 +26,8 @@ enum dw_mci_state {
STATE_DATA_BUSY,
STATE_SENDING_STOP,
STATE_DATA_ERROR,
+ STATE_SENDING_CMD11,
+ STATE_WAITING_CMD11_DONE,
};

enum {
--
1.7.10.4
Ulf Hansson
2014-08-22 15:35:14 UTC
Permalink
Post by Yuvaraj Kumar C D
For UHS cards we need the ability to switch voltages from 3.3V to
1.8V. Add support to the dw_mmc driver to handle this. Note that
dw_mmc needs a little bit of extra code since the interface needs a
special bit programmed to the CMD register while CMD11 is progressing.
This means adding a few extra states to the state machine to track.
---
1. Added error message and return error in case of regulator_set_voltage() fail.
2. changed dw_mci_cmd_interrupt(host,pending | SDMMC_INT_CMD_DONE)
to dw_mci_cmd_interrupt(host,pending).
3. Removed unnecessary comments.
drivers/mmc/host/dw_mmc.c | 134 +++++++++++++++++++++++++++++++++++++++++---
drivers/mmc/host/dw_mmc.h | 5 +-
include/linux/mmc/dw_mmc.h | 2 +
3 files changed, 131 insertions(+), 10 deletions(-)
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index aadb0d6..f20b4b8 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -29,6 +29,7 @@
#include <linux/irq.h>
#include <linux/mmc/host.h>
#include <linux/mmc/mmc.h>
+#include <linux/mmc/sd.h>
#include <linux/mmc/sdio.h>
#include <linux/mmc/dw_mmc.h>
#include <linux/bitops.h>
}
#endif /* defined(CONFIG_DEBUG_FS) */
+static void mci_send_cmd(struct dw_mci_slot *slot, u32 cmd, u32 arg);
+
static u32 dw_mci_prepare_command(struct mmc_host *mmc, struct mmc_command *cmd)
{
struct mmc_data *data;
struct dw_mci_slot *slot = mmc_priv(mmc);
+ struct dw_mci *host = slot->host;
const struct dw_mci_drv_data *drv_data = slot->host->drv_data;
u32 cmdr;
cmd->error = -EINPROGRESS;
@@ -253,6 +257,31 @@ static u32 dw_mci_prepare_command(struct mmc_host *mmc, struct mmc_command *cmd)
else if (cmd->opcode != MMC_SEND_STATUS && cmd->data)
cmdr |= SDMMC_CMD_PRV_DAT_WAIT;
+ if (cmd->opcode == SD_SWITCH_VOLTAGE) {
+ u32 clk_en_a;
+
+ /* Special bit makes CMD11 not die */
+ cmdr |= SDMMC_CMD_VOLT_SWITCH;
+
+ /* Change state to continue to handle CMD11 weirdness */
+ WARN_ON(slot->host->state != STATE_SENDING_CMD);
+ slot->host->state = STATE_SENDING_CMD11;
+
+ /*
+ * We need to disable clock stop while doing voltage switch
+ * according to Voltage Switch Normal Scenario.
+ * It's assumed that by the next time the CLKENA is updated
+ * (when we set the clock next) that the voltage change will
+ * be over, so we don't bother setting any bits to synchronize
+ * with dw_mci_setup_bus().
+ */
I don't know the details about the dw_mmc controller, but normally a
host driver is expected to gate the clock from it's ->set_ios
callback, when the clk frequency are set to 0.

Could you elaborate on why dw_mmc can't do that, but need to handle
this from here?

Kind regards
Uffe
Post by Yuvaraj Kumar C D
+ clk_en_a = mci_readl(host, CLKENA);
+ clk_en_a &= ~(SDMMC_CLKEN_LOW_PWR << slot->id);
+ mci_writel(host, CLKENA, clk_en_a);
+ mci_send_cmd(slot, SDMMC_CMD_UPD_CLK |
+ SDMMC_CMD_PRV_DAT_WAIT, 0);
+ }
+
if (cmd->flags & MMC_RSP_PRESENT) {
/* We expect a response, so set this bit */
cmdr |= SDMMC_CMD_RESP_EXP;
@@ -775,11 +804,15 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot, bool force_clkinit)
unsigned int clock = slot->clock;
u32 div;
u32 clk_en_a;
+ u32 sdmmc_cmd_bits = SDMMC_CMD_UPD_CLK | SDMMC_CMD_PRV_DAT_WAIT;
+
+ /* We must continue to set bit 28 in CMD until the change is complete */
+ if (host->state == STATE_WAITING_CMD11_DONE)
+ sdmmc_cmd_bits |= SDMMC_CMD_VOLT_SWITCH;
if (!clock) {
mci_writel(host, CLKENA, 0);
- mci_send_cmd(slot,
- SDMMC_CMD_UPD_CLK | SDMMC_CMD_PRV_DAT_WAIT, 0);
+ mci_send_cmd(slot, sdmmc_cmd_bits, 0);
} else if (clock != host->current_speed || force_clkinit) {
div = host->bus_hz / clock;
if (host->bus_hz % clock && host->bus_hz > clock)
@@ -803,15 +836,13 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot, bool force_clkinit)
mci_writel(host, CLKSRC, 0);
/* inform CIU */
- mci_send_cmd(slot,
- SDMMC_CMD_UPD_CLK | SDMMC_CMD_PRV_DAT_WAIT, 0);
+ mci_send_cmd(slot, sdmmc_cmd_bits, 0);
/* set clock to desired speed */
mci_writel(host, CLKDIV, div);
/* inform CIU */
- mci_send_cmd(slot,
- SDMMC_CMD_UPD_CLK | SDMMC_CMD_PRV_DAT_WAIT, 0);
+ mci_send_cmd(slot, sdmmc_cmd_bits, 0);
/* enable clock; only low power if no SDIO */
clk_en_a = SDMMC_CLKEN_ENABLE << slot->id;
@@ -820,8 +851,7 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot, bool force_clkinit)
mci_writel(host, CLKENA, clk_en_a);
/* inform CIU */
- mci_send_cmd(slot,
- SDMMC_CMD_UPD_CLK | SDMMC_CMD_PRV_DAT_WAIT, 0);
+ mci_send_cmd(slot, sdmmc_cmd_bits, 0);
/* keep the clock with reflecting clock dividor */
slot->__clk_old = clock << div;
@@ -897,6 +927,17 @@ static void dw_mci_queue_request(struct dw_mci *host, struct dw_mci_slot *slot,
slot->mrq = mrq;
+ if (host->state == STATE_WAITING_CMD11_DONE) {
+ dev_warn(&slot->mmc->class_dev,
+ "Voltage change didn't complete\n");
+ /*
+ * this case isn't expected to happen, so we can
+ * either crash here or just try to continue on
+ * in the closest possible state
+ */
+ host->state = STATE_IDLE;
+ }
+
if (host->state == STATE_IDLE) {
host->state = STATE_SENDING_CMD;
dw_mci_start_request(host, slot);
@@ -973,6 +1014,9 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
/* Slot specific timing and width adjustment */
dw_mci_setup_bus(slot, false);
+ if (slot->host->state == STATE_WAITING_CMD11_DONE && ios->clock != 0)
+ slot->host->state = STATE_IDLE;
+
switch (ios->power_mode) {
if (!IS_ERR(mmc->supply.vmmc)) {
@@ -1016,6 +1060,59 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
}
}
+static int dw_mci_card_busy(struct mmc_host *mmc)
+{
+ struct dw_mci_slot *slot = mmc_priv(mmc);
+ u32 status;
+
+ /*
+ * Check the busy bit which is low when DAT[3:0]
+ * (the data lines) are 0000
+ */
+ status = mci_readl(slot->host, STATUS);
+
+ return !!(status & SDMMC_STATUS_BUSY);
+}
+
+static int dw_mci_switch_voltage(struct mmc_host *mmc, struct mmc_ios *ios)
+{
+ struct dw_mci_slot *slot = mmc_priv(mmc);
+ struct dw_mci *host = slot->host;
+ u32 uhs;
+ u32 v18 = SDMMC_UHS_18V << slot->id;
+ int min_uv, max_uv;
+ int ret;
+
+ /*
+ * Program the voltage. Note that some instances of dw_mmc may use
+ * the UHS_REG for this. For other instances (like exynos) the UHS_REG
+ * does no harm but you need to set the regulator directly. Try both.
+ */
+ uhs = mci_readl(host, UHS_REG);
+ if (ios->signal_voltage == MMC_SIGNAL_VOLTAGE_330) {
+ min_uv = 2700000;
+ max_uv = 3600000;
+ uhs &= ~v18;
+ } else {
+ min_uv = 1700000;
+ max_uv = 1950000;
+ uhs |= v18;
+ }
+ if (!IS_ERR(mmc->supply.vqmmc)) {
+ ret = regulator_set_voltage(mmc->supply.vqmmc, min_uv, max_uv);
+
+ if (ret) {
+ dev_err(&mmc->class_dev,
+ "Regulator set error %d: %d - %d\n",
+ ret, min_uv, max_uv);
+ return ret;
+ }
+ }
+ mci_writel(host, UHS_REG, uhs);
+
+ return 0;
+}
+
static int dw_mci_get_ro(struct mmc_host *mmc)
{
int read_only;
@@ -1158,6 +1255,9 @@ static const struct mmc_host_ops dw_mci_ops = {
.get_cd = dw_mci_get_cd,
.enable_sdio_irq = dw_mci_enable_sdio_irq,
.execute_tuning = dw_mci_execute_tuning,
+ .card_busy = dw_mci_card_busy,
+ .start_signal_voltage_switch = dw_mci_switch_voltage,
+
};
static void dw_mci_request_end(struct dw_mci *host, struct mmc_request *mrq)
@@ -1181,7 +1281,11 @@ static void dw_mci_request_end(struct dw_mci *host, struct mmc_request *mrq)
dw_mci_start_request(host, slot);
} else {
dev_vdbg(host->dev, "list empty\n");
- host->state = STATE_IDLE;
+
+ if (host->state == STATE_SENDING_CMD11)
+ host->state = STATE_WAITING_CMD11_DONE;
+ else
+ host->state = STATE_IDLE;
}
spin_unlock(&host->lock);
@@ -1292,8 +1396,10 @@ static void dw_mci_tasklet_func(unsigned long priv)
switch (state) {
break;
if (!test_and_clear_bit(EVENT_CMD_COMPLETE,
&host->pending_events))
@@ -1894,6 +2000,14 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id)
}
if (pending) {
+ /* Check volt switch first, since it can look like an error */
+ if ((host->state == STATE_SENDING_CMD11) &&
+ (pending & SDMMC_INT_VOLT_SWITCH)) {
+ mci_writel(host, RINTSTS, SDMMC_INT_VOLT_SWITCH);
+ pending &= ~SDMMC_INT_VOLT_SWITCH;
+ dw_mci_cmd_interrupt(host, pending);
+ }
+
if (pending & DW_MCI_CMD_ERROR_FLAGS) {
mci_writel(host, RINTSTS, DW_MCI_CMD_ERROR_FLAGS);
host->cmd_status = pending;
@@ -1999,7 +2113,9 @@ static void dw_mci_work_routine_card(struct work_struct *work)
switch (host->state) {
break;
mrq->cmd->error = -ENOMEDIUM;
if (!mrq->data)
diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
index 08fd956..01b99e8 100644
--- a/drivers/mmc/host/dw_mmc.h
+++ b/drivers/mmc/host/dw_mmc.h
@@ -99,6 +99,7 @@
#define SDMMC_INT_HLE BIT(12)
#define SDMMC_INT_FRUN BIT(11)
#define SDMMC_INT_HTO BIT(10)
+#define SDMMC_INT_VOLT_SWITCH BIT(10) /* overloads bit 10! */
#define SDMMC_INT_DRTO BIT(9)
#define SDMMC_INT_RTO BIT(8)
#define SDMMC_INT_DCRC BIT(7)
@@ -113,6 +114,7 @@
/* Command register defines */
#define SDMMC_CMD_START BIT(31)
#define SDMMC_CMD_USE_HOLD_REG BIT(29)
+#define SDMMC_CMD_VOLT_SWITCH BIT(28)
#define SDMMC_CMD_CCS_EXP BIT(23)
#define SDMMC_CMD_CEATA_RD BIT(22)
#define SDMMC_CMD_UPD_CLK BIT(21)
@@ -130,6 +132,7 @@
/* Status register defines */
#define SDMMC_GET_FCNT(x) (((x)>>17) & 0x1FFF)
#define SDMMC_STATUS_DMA_REQ BIT(31)
+#define SDMMC_STATUS_BUSY BIT(9)
/* FIFOTH register defines */
#define SDMMC_SET_FIFOTH(m, r, t) (((m) & 0x7) << 28 | \
((r) & 0xFFF) << 16 | \
@@ -150,7 +153,7 @@
#define SDMMC_GET_VERID(x) ((x) & 0xFFFF)
/* Card read threshold */
#define SDMMC_SET_RD_THLD(v, x) (((v) & 0x1FFF) << 16 | (x))
-
+#define SDMMC_UHS_18V BIT(0)
/* All ctrl reset bits */
#define SDMMC_CTRL_ALL_RESET_FLAGS \
(SDMMC_CTRL_RESET | SDMMC_CTRL_FIFO_RESET | SDMMC_CTRL_DMA_RESET)
diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h
index 84e2827..0013669 100644
--- a/include/linux/mmc/dw_mmc.h
+++ b/include/linux/mmc/dw_mmc.h
@@ -26,6 +26,8 @@ enum dw_mci_state {
STATE_DATA_BUSY,
STATE_SENDING_STOP,
STATE_DATA_ERROR,
+ STATE_SENDING_CMD11,
+ STATE_WAITING_CMD11_DONE,
};
enum {
--
1.7.10.4
Doug Anderson
2014-08-22 20:38:52 UTC
Permalink
Ulf,
Post by Ulf Hansson
Post by Yuvaraj Kumar C D
For UHS cards we need the ability to switch voltages from 3.3V to
1.8V. Add support to the dw_mmc driver to handle this. Note that
dw_mmc needs a little bit of extra code since the interface needs a
special bit programmed to the CMD register while CMD11 is progressing.
This means adding a few extra states to the state machine to track.
---
1. Added error message and return error in case of regulator_set_voltage() fail.
2. changed dw_mci_cmd_interrupt(host,pending | SDMMC_INT_CMD_DONE)
to dw_mci_cmd_interrupt(host,pending).
3. Removed unnecessary comments.
drivers/mmc/host/dw_mmc.c | 134 +++++++++++++++++++++++++++++++++++++++++---
drivers/mmc/host/dw_mmc.h | 5 +-
include/linux/mmc/dw_mmc.h | 2 +
3 files changed, 131 insertions(+), 10 deletions(-)
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index aadb0d6..f20b4b8 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -29,6 +29,7 @@
#include <linux/irq.h>
#include <linux/mmc/host.h>
#include <linux/mmc/mmc.h>
+#include <linux/mmc/sd.h>
#include <linux/mmc/sdio.h>
#include <linux/mmc/dw_mmc.h>
#include <linux/bitops.h>
}
#endif /* defined(CONFIG_DEBUG_FS) */
+static void mci_send_cmd(struct dw_mci_slot *slot, u32 cmd, u32 arg);
+
static u32 dw_mci_prepare_command(struct mmc_host *mmc, struct mmc_command *cmd)
{
struct mmc_data *data;
struct dw_mci_slot *slot = mmc_priv(mmc);
+ struct dw_mci *host = slot->host;
const struct dw_mci_drv_data *drv_data = slot->host->drv_data;
u32 cmdr;
cmd->error = -EINPROGRESS;
@@ -253,6 +257,31 @@ static u32 dw_mci_prepare_command(struct mmc_host *mmc, struct mmc_command *cmd)
else if (cmd->opcode != MMC_SEND_STATUS && cmd->data)
cmdr |= SDMMC_CMD_PRV_DAT_WAIT;
+ if (cmd->opcode == SD_SWITCH_VOLTAGE) {
+ u32 clk_en_a;
+
+ /* Special bit makes CMD11 not die */
+ cmdr |= SDMMC_CMD_VOLT_SWITCH;
+
+ /* Change state to continue to handle CMD11 weirdness */
+ WARN_ON(slot->host->state != STATE_SENDING_CMD);
+ slot->host->state = STATE_SENDING_CMD11;
+
+ /*
+ * We need to disable clock stop while doing voltage switch
+ * according to Voltage Switch Normal Scenario.
+ * It's assumed that by the next time the CLKENA is updated
+ * (when we set the clock next) that the voltage change will
+ * be over, so we don't bother setting any bits to synchronize
+ * with dw_mci_setup_bus().
+ */
I don't know the details about the dw_mmc controller, but normally a
host driver is expected to gate the clock from it's ->set_ios
callback, when the clk frequency are set to 0.
Could you elaborate on why dw_mmc can't do that, but need to handle
this from here?
Let's see, it's been a while since I wrote this...


So dw_mmc has a special feature where the controller itself will
automatically stop the MMC Card clock when nothing is going on. This
is called "low power" mode. I'm not super familiar with other MMC
drivers, I get the impression that this is a special dw_mmc feature
and not common to other controllers. I think other drivers have
aggressive runtime PM to get similar power savings.

The dw_mmc auto clock gating can wreck total havoc on the voltage
change procedure, since part of the way that the host and card
communicate is that the host is supposed to stop its clock when it
gets to a certain phase of the voltage switch sequence. If the
controller is stopping the clock for us then it can confuse the card.

The dw_mmc manual says that before starting a voltage change you must
turn off low power mode. That's what we're doing here.


The comment above refers to the fact dw_mci_setup_bus() will
unconditionally turn low power mode back on for us when called with a
non-zero clock. If dw_mci_setup_bus() might be called with a non-zero
clock during the voltage change then this would be disaster (low power
mode would be back on!). ...but the clock will always be zero during
the voltage change and when it's finally non-zero then it's the last
stage of the voltage change and we can go back to low power mode.


Possibly the above was not super clear from the comment (even for
those familiar with the oddities of dw_mmc). If you want me to try to
rewrite the comment, let me know.


-Doug
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Ulf Hansson
2014-08-25 08:31:14 UTC
Permalink
Post by Doug Anderson
Ulf,
Post by Ulf Hansson
Post by Yuvaraj Kumar C D
For UHS cards we need the ability to switch voltages from 3.3V to
1.8V. Add support to the dw_mmc driver to handle this. Note that
dw_mmc needs a little bit of extra code since the interface needs a
special bit programmed to the CMD register while CMD11 is progressing.
This means adding a few extra states to the state machine to track.
---
1. Added error message and return error in case of regulator_set_voltage() fail.
2. changed dw_mci_cmd_interrupt(host,pending | SDMMC_INT_CMD_DONE)
to dw_mci_cmd_interrupt(host,pending).
3. Removed unnecessary comments.
drivers/mmc/host/dw_mmc.c | 134 +++++++++++++++++++++++++++++++++++++++++---
drivers/mmc/host/dw_mmc.h | 5 +-
include/linux/mmc/dw_mmc.h | 2 +
3 files changed, 131 insertions(+), 10 deletions(-)
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index aadb0d6..f20b4b8 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -29,6 +29,7 @@
#include <linux/irq.h>
#include <linux/mmc/host.h>
#include <linux/mmc/mmc.h>
+#include <linux/mmc/sd.h>
#include <linux/mmc/sdio.h>
#include <linux/mmc/dw_mmc.h>
#include <linux/bitops.h>
}
#endif /* defined(CONFIG_DEBUG_FS) */
+static void mci_send_cmd(struct dw_mci_slot *slot, u32 cmd, u32 arg);
+
static u32 dw_mci_prepare_command(struct mmc_host *mmc, struct mmc_command *cmd)
{
struct mmc_data *data;
struct dw_mci_slot *slot = mmc_priv(mmc);
+ struct dw_mci *host = slot->host;
const struct dw_mci_drv_data *drv_data = slot->host->drv_data;
u32 cmdr;
cmd->error = -EINPROGRESS;
@@ -253,6 +257,31 @@ static u32 dw_mci_prepare_command(struct mmc_host *mmc, struct mmc_command *cmd)
else if (cmd->opcode != MMC_SEND_STATUS && cmd->data)
cmdr |= SDMMC_CMD_PRV_DAT_WAIT;
+ if (cmd->opcode == SD_SWITCH_VOLTAGE) {
+ u32 clk_en_a;
+
+ /* Special bit makes CMD11 not die */
+ cmdr |= SDMMC_CMD_VOLT_SWITCH;
+
+ /* Change state to continue to handle CMD11 weirdness */
+ WARN_ON(slot->host->state != STATE_SENDING_CMD);
+ slot->host->state = STATE_SENDING_CMD11;
+
+ /*
+ * We need to disable clock stop while doing voltage switch
+ * according to Voltage Switch Normal Scenario.
+ * It's assumed that by the next time the CLKENA is updated
+ * (when we set the clock next) that the voltage change will
+ * be over, so we don't bother setting any bits to synchronize
+ * with dw_mci_setup_bus().
+ */
I don't know the details about the dw_mmc controller, but normally a
host driver is expected to gate the clock from it's ->set_ios
callback, when the clk frequency are set to 0.
Could you elaborate on why dw_mmc can't do that, but need to handle
this from here?
Let's see, it's been a while since I wrote this...
So dw_mmc has a special feature where the controller itself will
automatically stop the MMC Card clock when nothing is going on. This
is called "low power" mode. I'm not super familiar with other MMC
drivers, I get the impression that this is a special dw_mmc feature
and not common to other controllers. I think other drivers have
aggressive runtime PM to get similar power savings.
I see.

I am familiar with such "low power" mode features, there are certainly
other controllers supporting such as well. My experience tells me,
it's hard to get things right for all corner cases. The voltage switch
behaviour is just one of these, then you have SDIO irq etc.

Instead of using the controller HW, yes you may implement clock gating
through runtime PM in the host driver.
Post by Doug Anderson
The dw_mmc auto clock gating can wreck total havoc on the voltage
change procedure, since part of the way that the host and card
communicate is that the host is supposed to stop its clock when it
gets to a certain phase of the voltage switch sequence. If the
controller is stopping the clock for us then it can confuse the card.
The dw_mmc manual says that before starting a voltage change you must
turn off low power mode. That's what we're doing here.
The comment above refers to the fact dw_mci_setup_bus() will
unconditionally turn low power mode back on for us when called with a
non-zero clock. If dw_mci_setup_bus() might be called with a non-zero
clock during the voltage change then this would be disaster (low power
mode would be back on!). ...but the clock will always be zero during
the voltage change and when it's finally non-zero then it's the last
stage of the voltage change and we can go back to low power mode.
Possibly the above was not super clear from the comment (even for
those familiar with the oddities of dw_mmc). If you want me to try to
rewrite the comment, let me know.
I appreciate an updated comment, it's nice to know what goes on. :-)

Kind regards
Uffe
Doug Anderson
2014-08-25 20:59:39 UTC
Permalink
Ulf,
Post by Ulf Hansson
Post by Doug Anderson
Ulf,
Post by Ulf Hansson
Post by Yuvaraj Kumar C D
For UHS cards we need the ability to switch voltages from 3.3V to
1.8V. Add support to the dw_mmc driver to handle this. Note that
dw_mmc needs a little bit of extra code since the interface needs a
special bit programmed to the CMD register while CMD11 is progressing.
This means adding a few extra states to the state machine to track.
---
1. Added error message and return error in case of regulator_set_voltage() fail.
2. changed dw_mci_cmd_interrupt(host,pending | SDMMC_INT_CMD_DONE)
to dw_mci_cmd_interrupt(host,pending).
3. Removed unnecessary comments.
drivers/mmc/host/dw_mmc.c | 134 +++++++++++++++++++++++++++++++++++++++++---
drivers/mmc/host/dw_mmc.h | 5 +-
include/linux/mmc/dw_mmc.h | 2 +
3 files changed, 131 insertions(+), 10 deletions(-)
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index aadb0d6..f20b4b8 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -29,6 +29,7 @@
#include <linux/irq.h>
#include <linux/mmc/host.h>
#include <linux/mmc/mmc.h>
+#include <linux/mmc/sd.h>
#include <linux/mmc/sdio.h>
#include <linux/mmc/dw_mmc.h>
#include <linux/bitops.h>
}
#endif /* defined(CONFIG_DEBUG_FS) */
+static void mci_send_cmd(struct dw_mci_slot *slot, u32 cmd, u32 arg);
+
static u32 dw_mci_prepare_command(struct mmc_host *mmc, struct mmc_command *cmd)
{
struct mmc_data *data;
struct dw_mci_slot *slot = mmc_priv(mmc);
+ struct dw_mci *host = slot->host;
const struct dw_mci_drv_data *drv_data = slot->host->drv_data;
u32 cmdr;
cmd->error = -EINPROGRESS;
@@ -253,6 +257,31 @@ static u32 dw_mci_prepare_command(struct mmc_host *mmc, struct mmc_command *cmd)
else if (cmd->opcode != MMC_SEND_STATUS && cmd->data)
cmdr |= SDMMC_CMD_PRV_DAT_WAIT;
+ if (cmd->opcode == SD_SWITCH_VOLTAGE) {
+ u32 clk_en_a;
+
+ /* Special bit makes CMD11 not die */
+ cmdr |= SDMMC_CMD_VOLT_SWITCH;
+
+ /* Change state to continue to handle CMD11 weirdness */
+ WARN_ON(slot->host->state != STATE_SENDING_CMD);
+ slot->host->state = STATE_SENDING_CMD11;
+
+ /*
+ * We need to disable clock stop while doing voltage switch
+ * according to Voltage Switch Normal Scenario.
+ * It's assumed that by the next time the CLKENA is updated
+ * (when we set the clock next) that the voltage change will
+ * be over, so we don't bother setting any bits to synchronize
+ * with dw_mci_setup_bus().
+ */
I don't know the details about the dw_mmc controller, but normally a
host driver is expected to gate the clock from it's ->set_ios
callback, when the clk frequency are set to 0.
Could you elaborate on why dw_mmc can't do that, but need to handle
this from here?
Let's see, it's been a while since I wrote this...
So dw_mmc has a special feature where the controller itself will
automatically stop the MMC Card clock when nothing is going on. This
is called "low power" mode. I'm not super familiar with other MMC
drivers, I get the impression that this is a special dw_mmc feature
and not common to other controllers. I think other drivers have
aggressive runtime PM to get similar power savings.
I see.
I am familiar with such "low power" mode features, there are certainly
other controllers supporting such as well. My experience tells me,
it's hard to get things right for all corner cases. The voltage switch
behaviour is just one of these, then you have SDIO irq etc.
Instead of using the controller HW, yes you may implement clock gating
through runtime PM in the host driver.
Post by Doug Anderson
The dw_mmc auto clock gating can wreck total havoc on the voltage
change procedure, since part of the way that the host and card
communicate is that the host is supposed to stop its clock when it
gets to a certain phase of the voltage switch sequence. If the
controller is stopping the clock for us then it can confuse the card.
The dw_mmc manual says that before starting a voltage change you must
turn off low power mode. That's what we're doing here.
The comment above refers to the fact dw_mci_setup_bus() will
unconditionally turn low power mode back on for us when called with a
non-zero clock. If dw_mci_setup_bus() might be called with a non-zero
clock during the voltage change then this would be disaster (low power
mode would be back on!). ...but the clock will always be zero during
the voltage change and when it's finally non-zero then it's the last
stage of the voltage change and we can go back to low power mode.
Possibly the above was not super clear from the comment (even for
those familiar with the oddities of dw_mmc). If you want me to try to
rewrite the comment, let me know.
I appreciate an updated comment, it's nice to know what goes on. :-)
OK, how about:

/*
* We need to disable low power mode (automatic clock stop)
* while doing voltage switch so we don't confuse the card,
* since stopping the clock is a specific part of the UHS
* voltage change dance.
*
* Note that low power mode (SDMMC_CLKEN_LOW_PWR) will be
* unconditionally turned back on in dw_mci_setup_bus() if it's
* ever called with a non-zero clock. That shouldn't happen
* until the voltage change is all done.
*/

Yuvaraj: can you include that in the next patch set you send out?
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Ulf Hansson
2014-08-29 11:43:16 UTC
Permalink
Post by Doug Anderson
Ulf,
Post by Ulf Hansson
Post by Doug Anderson
Ulf,
Post by Ulf Hansson
Post by Yuvaraj Kumar C D
For UHS cards we need the ability to switch voltages from 3.3V to
1.8V. Add support to the dw_mmc driver to handle this. Note that
dw_mmc needs a little bit of extra code since the interface needs a
special bit programmed to the CMD register while CMD11 is progressing.
This means adding a few extra states to the state machine to track.
---
1. Added error message and return error in case of regulator_set_voltage() fail.
2. changed dw_mci_cmd_interrupt(host,pending | SDMMC_INT_CMD_DONE)
to dw_mci_cmd_interrupt(host,pending).
3. Removed unnecessary comments.
drivers/mmc/host/dw_mmc.c | 134 +++++++++++++++++++++++++++++++++++++++++---
drivers/mmc/host/dw_mmc.h | 5 +-
include/linux/mmc/dw_mmc.h | 2 +
3 files changed, 131 insertions(+), 10 deletions(-)
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index aadb0d6..f20b4b8 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -29,6 +29,7 @@
#include <linux/irq.h>
#include <linux/mmc/host.h>
#include <linux/mmc/mmc.h>
+#include <linux/mmc/sd.h>
#include <linux/mmc/sdio.h>
#include <linux/mmc/dw_mmc.h>
#include <linux/bitops.h>
}
#endif /* defined(CONFIG_DEBUG_FS) */
+static void mci_send_cmd(struct dw_mci_slot *slot, u32 cmd, u32 arg);
+
static u32 dw_mci_prepare_command(struct mmc_host *mmc, struct mmc_command *cmd)
{
struct mmc_data *data;
struct dw_mci_slot *slot = mmc_priv(mmc);
+ struct dw_mci *host = slot->host;
const struct dw_mci_drv_data *drv_data = slot->host->drv_data;
u32 cmdr;
cmd->error = -EINPROGRESS;
@@ -253,6 +257,31 @@ static u32 dw_mci_prepare_command(struct mmc_host *mmc, struct mmc_command *cmd)
else if (cmd->opcode != MMC_SEND_STATUS && cmd->data)
cmdr |= SDMMC_CMD_PRV_DAT_WAIT;
+ if (cmd->opcode == SD_SWITCH_VOLTAGE) {
+ u32 clk_en_a;
+
+ /* Special bit makes CMD11 not die */
+ cmdr |= SDMMC_CMD_VOLT_SWITCH;
+
+ /* Change state to continue to handle CMD11 weirdness */
+ WARN_ON(slot->host->state != STATE_SENDING_CMD);
+ slot->host->state = STATE_SENDING_CMD11;
+
+ /*
+ * We need to disable clock stop while doing voltage switch
+ * according to Voltage Switch Normal Scenario.
+ * It's assumed that by the next time the CLKENA is updated
+ * (when we set the clock next) that the voltage change will
+ * be over, so we don't bother setting any bits to synchronize
+ * with dw_mci_setup_bus().
+ */
I don't know the details about the dw_mmc controller, but normally a
host driver is expected to gate the clock from it's ->set_ios
callback, when the clk frequency are set to 0.
Could you elaborate on why dw_mmc can't do that, but need to handle
this from here?
Let's see, it's been a while since I wrote this...
So dw_mmc has a special feature where the controller itself will
automatically stop the MMC Card clock when nothing is going on. This
is called "low power" mode. I'm not super familiar with other MMC
drivers, I get the impression that this is a special dw_mmc feature
and not common to other controllers. I think other drivers have
aggressive runtime PM to get similar power savings.
I see.
I am familiar with such "low power" mode features, there are certainly
other controllers supporting such as well. My experience tells me,
it's hard to get things right for all corner cases. The voltage switch
behaviour is just one of these, then you have SDIO irq etc.
Instead of using the controller HW, yes you may implement clock gating
through runtime PM in the host driver.
Post by Doug Anderson
The dw_mmc auto clock gating can wreck total havoc on the voltage
change procedure, since part of the way that the host and card
communicate is that the host is supposed to stop its clock when it
gets to a certain phase of the voltage switch sequence. If the
controller is stopping the clock for us then it can confuse the card.
The dw_mmc manual says that before starting a voltage change you must
turn off low power mode. That's what we're doing here.
The comment above refers to the fact dw_mci_setup_bus() will
unconditionally turn low power mode back on for us when called with a
non-zero clock. If dw_mci_setup_bus() might be called with a non-zero
clock during the voltage change then this would be disaster (low power
mode would be back on!). ...but the clock will always be zero during
the voltage change and when it's finally non-zero then it's the last
stage of the voltage change and we can go back to low power mode.
Possibly the above was not super clear from the comment (even for
those familiar with the oddities of dw_mmc). If you want me to try to
rewrite the comment, let me know.
I appreciate an updated comment, it's nice to know what goes on. :-)
/*
* We need to disable low power mode (automatic clock stop)
* while doing voltage switch so we don't confuse the card,
* since stopping the clock is a specific part of the UHS
* voltage change dance.
*
* Note that low power mode (SDMMC_CLKEN_LOW_PWR) will be
* unconditionally turned back on in dw_mci_setup_bus() if it's
* ever called with a non-zero clock. That shouldn't happen
* until the voltage change is all done.
*/
Yuvaraj: can you include that in the next patch set you send out?
Thanks! Applied for next!

I took the liberty to change to the clarified comment from Doug above.

Kind regards
Uffe
Bartlomiej Zolnierkiewicz
2014-09-29 12:49:50 UTC
Permalink
Hi,
Post by Ulf Hansson
Post by Doug Anderson
Ulf,
Post by Ulf Hansson
Post by Doug Anderson
Ulf,
Post by Ulf Hansson
Post by Yuvaraj Kumar C D
For UHS cards we need the ability to switch voltages from 3.3V to
1.8V. Add support to the dw_mmc driver to handle this. Note that
dw_mmc needs a little bit of extra code since the interface needs a
special bit programmed to the CMD register while CMD11 is progressing.
This means adding a few extra states to the state machine to track.
---
1. Added error message and return error in case of regulator_set_voltage() fail.
2. changed dw_mci_cmd_interrupt(host,pending | SDMMC_INT_CMD_DONE)
to dw_mci_cmd_interrupt(host,pending).
3. Removed unnecessary comments.
drivers/mmc/host/dw_mmc.c | 134 +++++++++++++++++++++++++++++++++++++++++---
drivers/mmc/host/dw_mmc.h | 5 +-
include/linux/mmc/dw_mmc.h | 2 +
3 files changed, 131 insertions(+), 10 deletions(-)
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index aadb0d6..f20b4b8 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -29,6 +29,7 @@
#include <linux/irq.h>
#include <linux/mmc/host.h>
#include <linux/mmc/mmc.h>
+#include <linux/mmc/sd.h>
#include <linux/mmc/sdio.h>
#include <linux/mmc/dw_mmc.h>
#include <linux/bitops.h>
}
#endif /* defined(CONFIG_DEBUG_FS) */
+static void mci_send_cmd(struct dw_mci_slot *slot, u32 cmd, u32 arg);
+
static u32 dw_mci_prepare_command(struct mmc_host *mmc, struct mmc_command *cmd)
{
struct mmc_data *data;
struct dw_mci_slot *slot = mmc_priv(mmc);
+ struct dw_mci *host = slot->host;
const struct dw_mci_drv_data *drv_data = slot->host->drv_data;
u32 cmdr;
cmd->error = -EINPROGRESS;
@@ -253,6 +257,31 @@ static u32 dw_mci_prepare_command(struct mmc_host *mmc, struct mmc_command *cmd)
else if (cmd->opcode != MMC_SEND_STATUS && cmd->data)
cmdr |= SDMMC_CMD_PRV_DAT_WAIT;
+ if (cmd->opcode == SD_SWITCH_VOLTAGE) {
+ u32 clk_en_a;
+
+ /* Special bit makes CMD11 not die */
+ cmdr |= SDMMC_CMD_VOLT_SWITCH;
+
+ /* Change state to continue to handle CMD11 weirdness */
+ WARN_ON(slot->host->state != STATE_SENDING_CMD);
+ slot->host->state = STATE_SENDING_CMD11;
+
+ /*
+ * We need to disable clock stop while doing voltage switch
+ * according to Voltage Switch Normal Scenario.
+ * It's assumed that by the next time the CLKENA is updated
+ * (when we set the clock next) that the voltage change will
+ * be over, so we don't bother setting any bits to synchronize
+ * with dw_mci_setup_bus().
+ */
I don't know the details about the dw_mmc controller, but normally a
host driver is expected to gate the clock from it's ->set_ios
callback, when the clk frequency are set to 0.
Could you elaborate on why dw_mmc can't do that, but need to handle
this from here?
Let's see, it's been a while since I wrote this...
So dw_mmc has a special feature where the controller itself will
automatically stop the MMC Card clock when nothing is going on. This
is called "low power" mode. I'm not super familiar with other MMC
drivers, I get the impression that this is a special dw_mmc feature
and not common to other controllers. I think other drivers have
aggressive runtime PM to get similar power savings.
I see.
I am familiar with such "low power" mode features, there are certainly
other controllers supporting such as well. My experience tells me,
it's hard to get things right for all corner cases. The voltage switch
behaviour is just one of these, then you have SDIO irq etc.
Instead of using the controller HW, yes you may implement clock gating
through runtime PM in the host driver.
Post by Doug Anderson
The dw_mmc auto clock gating can wreck total havoc on the voltage
change procedure, since part of the way that the host and card
communicate is that the host is supposed to stop its clock when it
gets to a certain phase of the voltage switch sequence. If the
controller is stopping the clock for us then it can confuse the card.
The dw_mmc manual says that before starting a voltage change you must
turn off low power mode. That's what we're doing here.
The comment above refers to the fact dw_mci_setup_bus() will
unconditionally turn low power mode back on for us when called with a
non-zero clock. If dw_mci_setup_bus() might be called with a non-zero
clock during the voltage change then this would be disaster (low power
mode would be back on!). ...but the clock will always be zero during
the voltage change and when it's finally non-zero then it's the last
stage of the voltage change and we can go back to low power mode.
Possibly the above was not super clear from the comment (even for
those familiar with the oddities of dw_mmc). If you want me to try to
rewrite the comment, let me know.
I appreciate an updated comment, it's nice to know what goes on. :-)
/*
* We need to disable low power mode (automatic clock stop)
* while doing voltage switch so we don't confuse the card,
* since stopping the clock is a specific part of the UHS
* voltage change dance.
*
* Note that low power mode (SDMMC_CLKEN_LOW_PWR) will be
* unconditionally turned back on in dw_mci_setup_bus() if it's
* ever called with a non-zero clock. That shouldn't happen
* until the voltage change is all done.
*/
Yuvaraj: can you include that in the next patch set you send out?
Thanks! Applied for next!
I took the liberty to change to the clarified comment from Doug above.
This patch casuses a boot hang during regulators initizalization on
Exynos5420 Arndale Octa board.

The kernel config used can was posted in other thread:
http://www.mail-archive.com/linux-samsung-***@vger.kernel.org/msg37210.html

IOW I need to revert both

"mmc: dw_mmc: Support voltage changes"

and

"mmc: dw_mmc: use mmc_regulator_get_supply to handle regulators"

patches to make next branch of mmc-uh tree work on my setup.

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Yuvaraj Kumar C D
2014-08-22 13:47:52 UTC
Permalink
Exynos 5250 and 5420 based boards uses built-in CD# line for card
detection.But unfortunately CD# line is on the same voltage rails
as of I/O voltage rails. When we cut off vqmmc,the consequent card
detection will break in these boards.

These hosts (obviously) need to keep vqmmc (and thus vmmc) on all the
time, even when the mmc core tells them to power off. However, one
problem is that these cards won't properly handle mmc_power_cycle().
That's needed to handle error cases when trying to switch voltages
(see 0797e5f mmc:core: Fixup signal voltage switch).

This patch adds a new MMC_POWER_OFF_HARD mode when it's doing a power
cycle. This mode differs from the normal MMC_POWER_OFF mode in that
the mmc core will promise to power the slot back on before it expects
the host to detect card insertion or removal.

Also if we let alone the vqmmc turned on when vmmc turned off, the
card could have half way powered and this can damage the card. So
this patch adds a check so that, if the board used the built-in
card detection mechanism i.e through CDETECT, it will not turned
down vqmmc and vmmc both.

Signed-off-by: Yuvaraj Kumar C D <***@samsung.com>
Signed-off-by: Doug Anderson <***@chromium.org>
---
changes from v1:
1.added a new MMC_POWER_OFF_HARD mode as per Doug Anderson's suggestion.
2.added dw_mci_exynos_post_init() to perform the host specific post
initialisation.
3.added a new flag MMC_CAP2_CD_NEEDS_POWER for host->caps2.

drivers/mmc/core/core.c | 16 ++++++++++++++--
drivers/mmc/core/debugfs.c | 3 +++
drivers/mmc/host/dw_mmc-exynos.c | 12 ++++++++++++
drivers/mmc/host/dw_mmc.c | 25 +++++++++++++++++++++++++
drivers/mmc/host/dw_mmc.h | 2 ++
include/linux/mmc/host.h | 2 ++
6 files changed, 58 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 68f5f4b..79ced36 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -1564,9 +1564,9 @@ void mmc_power_up(struct mmc_host *host, u32 ocr)
mmc_host_clk_release(host);
}

-void mmc_power_off(struct mmc_host *host)
+void _mmc_power_off(struct mmc_host *host, unsigned char power_mode)
{
- if (host->ios.power_mode == MMC_POWER_OFF)
+ if (host->ios.power_mode == power_mode)
return;

mmc_host_clk_hold(host);
@@ -1579,6 +1579,7 @@ void mmc_power_off(struct mmc_host *host)
host->ios.chip_select = MMC_CS_DONTCARE;
}
host->ios.power_mode = MMC_POWER_OFF;
+ host->ios.power_mode = power_mode;
host->ios.bus_width = MMC_BUS_WIDTH_1;
host->ios.timing = MMC_TIMING_LEGACY;
mmc_set_ios(host);
@@ -1593,9 +1594,20 @@ void mmc_power_off(struct mmc_host *host)
mmc_host_clk_release(host);
}

+void mmc_power_off(struct mmc_host *host)
+{
+ _mmc_power_off(host, MMC_POWER_OFF);
+}
+
void mmc_power_cycle(struct mmc_host *host, u32 ocr)
{
mmc_power_off(host);
+ /* If host normally ignores MMC_POWER_OFF, tell it to pay attention */
+ if (host->caps2 & MMC_CAP2_CD_NEEDS_POWER)
+ _mmc_power_off(host, MMC_POWER_OFF_HARD);
+ else
+ _mmc_power_off(host, MMC_POWER_OFF);
+
/* Wait at least 1 ms according to SD spec */
mmc_delay(1);
mmc_power_up(host, ocr);
diff --git a/drivers/mmc/core/debugfs.c b/drivers/mmc/core/debugfs.c
index 91eb162..3d9c5a3 100644
--- a/drivers/mmc/core/debugfs.c
+++ b/drivers/mmc/core/debugfs.c
@@ -108,6 +108,9 @@ static int mmc_ios_show(struct seq_file *s, void *data)
case MMC_POWER_ON:
str = "on";
break;
+ case MMC_POWER_OFF_HARD:
+ str = "hard off";
+ break;
default:
str = "invalid";
break;
diff --git a/drivers/mmc/host/dw_mmc-exynos.c b/drivers/mmc/host/dw_mmc-exynos.c
index 0fbc53a..4e26049 100644
--- a/drivers/mmc/host/dw_mmc-exynos.c
+++ b/drivers/mmc/host/dw_mmc-exynos.c
@@ -17,6 +17,7 @@
#include <linux/mmc/mmc.h>
#include <linux/of.h>
#include <linux/of_gpio.h>
+#include <linux/mmc/slot-gpio.h>
#include <linux/slab.h>

#include "dw_mmc.h"
@@ -217,6 +218,16 @@ static void dw_mci_exynos_set_ios(struct dw_mci *host, struct mmc_ios *ios)
}
}

+static void dw_mci_exynos_post_init(struct dw_mci_slot *slot)
+{
+ struct dw_mci_board *brd = slot->host->pdata;
+ struct mmc_host *mmc = slot->mmc;
+
+ if (!(brd->quirks & DW_MCI_QUIRK_BROKEN_CARD_DETECTION) &&
+ IS_ERR_VALUE(mmc_gpio_get_cd(mmc)))
+ mmc->caps2 |= MMC_CAP2_CD_NEEDS_POWER;
+}
+
static int dw_mci_exynos_parse_dt(struct dw_mci *host)
{
struct dw_mci_exynos_priv_data *priv;
@@ -399,6 +410,7 @@ static const struct dw_mci_drv_data exynos_drv_data = {
.prepare_command = dw_mci_exynos_prepare_command,
.set_ios = dw_mci_exynos_set_ios,
.parse_dt = dw_mci_exynos_parse_dt,
+ .post_init = dw_mci_exynos_post_init,
.execute_tuning = dw_mci_exynos_execute_tuning,
};

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index f20b4b8..6f2c681 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -972,6 +972,22 @@ static void dw_mci_request(struct mmc_host *mmc, struct mmc_request *mrq)
spin_unlock_bh(&host->lock);
}

+/*
+ * some of the boards use controller CD line for card detection.Unfortunately
+ * CD line is bind to the same volatge domain as of the IO lines.If we turn off
+ * IO voltage domain, CD line wont work.
+ * Return true when controller CD line is used for card detection or return
+ * false.
+ */
+static bool dw_mci_builtin_cd(struct dw_mci_slot *slot)
+{
+ struct dw_mci_board *brd = slot->host->pdata;
+ struct mmc_host *mmc = slot->mmc;
+
+ return (!(brd->quirks & DW_MCI_QUIRK_BROKEN_CARD_DETECTION) &&
+ IS_ERR_VALUE(mmc_gpio_get_cd(mmc))) ? 1 : 0;
+}
+
static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
{
struct dw_mci_slot *slot = mmc_priv(mmc);
@@ -1043,6 +1059,10 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
mci_writel(slot->host, PWREN, regs);
break;
case MMC_POWER_OFF:
+ if (dw_mci_builtin_cd(slot) &&
+ !test_bit(DW_MMC_CARD_PRESENT, &slot->flags))
+ return;
+
if (!IS_ERR(mmc->supply.vmmc))
mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, 0);

@@ -1055,6 +1075,8 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
regs &= ~(1 << slot->id);
mci_writel(slot->host, PWREN, regs);
break;
+ case MMC_POWER_OFF_HARD:
+ break;
default:
break;
}
@@ -2310,6 +2332,9 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
else
clear_bit(DW_MMC_CARD_PRESENT, &slot->flags);

+ if (drv_data && drv_data->post_init)
+ drv_data->post_init(slot);
+
ret = mmc_add_host(mmc);
if (ret)
goto err_setup_bus;
diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
index 01b99e8..a3c2628 100644
--- a/drivers/mmc/host/dw_mmc.h
+++ b/drivers/mmc/host/dw_mmc.h
@@ -250,6 +250,7 @@ struct dw_mci_tuning_data {
* @prepare_command: handle CMD register extensions.
* @set_ios: handle bus specific extensions.
* @parse_dt: parse implementation specific device tree properties.
+ * @post_init: implementation specific post initialization.
* @execute_tuning: implementation specific tuning procedure.
*
* Provide controller implementation specific extensions. The usage of this
@@ -263,6 +264,7 @@ struct dw_mci_drv_data {
void (*prepare_command)(struct dw_mci *host, u32 *cmdr);
void (*set_ios)(struct dw_mci *host, struct mmc_ios *ios);
int (*parse_dt)(struct dw_mci *host);
+ void (*post_init)(struct dw_mci_slot *slot);
int (*execute_tuning)(struct dw_mci_slot *slot, u32 opcode,
struct dw_mci_tuning_data *tuning_data);
};
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 4cbf614..5eb24ff 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -42,6 +42,7 @@ struct mmc_ios {
#define MMC_POWER_OFF 0
#define MMC_POWER_UP 1
#define MMC_POWER_ON 2
+#define MMC_POWER_OFF_HARD 3

unsigned char bus_width; /* data bus width */

@@ -283,6 +284,7 @@ struct mmc_host {
#define MMC_CAP2_HS400 (MMC_CAP2_HS400_1_8V | \
MMC_CAP2_HS400_1_2V)
#define MMC_CAP2_SDIO_IRQ_NOTHREAD (1 << 17)
+#define MMC_CAP2_CD_NEEDS_POWER (1 << 18) /* Card detect needs power */

mmc_pm_flag_t pm_caps; /* supported pm features */
--
1.7.10.4
Ulf Hansson
2014-08-22 15:31:50 UTC
Permalink
Post by Yuvaraj Kumar C D
Exynos 5250 and 5420 based boards uses built-in CD# line for card
detection.But unfortunately CD# line is on the same voltage rails
as of I/O voltage rails. When we cut off vqmmc,the consequent card
detection will break in these boards.
I am not sure I follow here.

Is the card detect mechanism handled internally by the dw_mmc controller?

I thought HW engineers long time ago realized that this should be done
separately on a GPIO line to be able to save power while waiting for a
card to be inserted. But that's not case then?
Post by Yuvaraj Kumar C D
These hosts (obviously) need to keep vqmmc (and thus vmmc) on all the
time, even when the mmc core tells them to power off. However, one
problem is that these cards won't properly handle mmc_power_cycle().
That's needed to handle error cases when trying to switch voltages
(see 0797e5f mmc:core: Fixup signal voltage switch).
This patch adds a new MMC_POWER_OFF_HARD mode when it's doing a power
cycle. This mode differs from the normal MMC_POWER_OFF mode in that
the mmc core will promise to power the slot back on before it expects
the host to detect card insertion or removal.
Also if we let alone the vqmmc turned on when vmmc turned off, the
card could have half way powered and this can damage the card. So
this patch adds a check so that, if the board used the built-in
card detection mechanism i.e through CDETECT, it will not turned
down vqmmc and vmmc both.
Why does vmmc needs to be enabled when there are no card inserted?
That can't be right?

Kind regards
Uffe
Post by Yuvaraj Kumar C D
---
1.added a new MMC_POWER_OFF_HARD mode as per Doug Anderson's suggestion.
2.added dw_mci_exynos_post_init() to perform the host specific post
initialisation.
3.added a new flag MMC_CAP2_CD_NEEDS_POWER for host->caps2.
drivers/mmc/core/core.c | 16 ++++++++++++++--
drivers/mmc/core/debugfs.c | 3 +++
drivers/mmc/host/dw_mmc-exynos.c | 12 ++++++++++++
drivers/mmc/host/dw_mmc.c | 25 +++++++++++++++++++++++++
drivers/mmc/host/dw_mmc.h | 2 ++
include/linux/mmc/host.h | 2 ++
6 files changed, 58 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 68f5f4b..79ced36 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -1564,9 +1564,9 @@ void mmc_power_up(struct mmc_host *host, u32 ocr)
mmc_host_clk_release(host);
}
-void mmc_power_off(struct mmc_host *host)
+void _mmc_power_off(struct mmc_host *host, unsigned char power_mode)
{
- if (host->ios.power_mode == MMC_POWER_OFF)
+ if (host->ios.power_mode == power_mode)
return;
mmc_host_clk_hold(host);
@@ -1579,6 +1579,7 @@ void mmc_power_off(struct mmc_host *host)
host->ios.chip_select = MMC_CS_DONTCARE;
}
host->ios.power_mode = MMC_POWER_OFF;
+ host->ios.power_mode = power_mode;
host->ios.bus_width = MMC_BUS_WIDTH_1;
host->ios.timing = MMC_TIMING_LEGACY;
mmc_set_ios(host);
@@ -1593,9 +1594,20 @@ void mmc_power_off(struct mmc_host *host)
mmc_host_clk_release(host);
}
+void mmc_power_off(struct mmc_host *host)
+{
+ _mmc_power_off(host, MMC_POWER_OFF);
+}
+
void mmc_power_cycle(struct mmc_host *host, u32 ocr)
{
mmc_power_off(host);
+ /* If host normally ignores MMC_POWER_OFF, tell it to pay attention */
+ if (host->caps2 & MMC_CAP2_CD_NEEDS_POWER)
+ _mmc_power_off(host, MMC_POWER_OFF_HARD);
+ else
+ _mmc_power_off(host, MMC_POWER_OFF);
+
/* Wait at least 1 ms according to SD spec */
mmc_delay(1);
mmc_power_up(host, ocr);
diff --git a/drivers/mmc/core/debugfs.c b/drivers/mmc/core/debugfs.c
index 91eb162..3d9c5a3 100644
--- a/drivers/mmc/core/debugfs.c
+++ b/drivers/mmc/core/debugfs.c
@@ -108,6 +108,9 @@ static int mmc_ios_show(struct seq_file *s, void *data)
str = "on";
break;
+ str = "hard off";
+ break;
str = "invalid";
break;
diff --git a/drivers/mmc/host/dw_mmc-exynos.c b/drivers/mmc/host/dw_mmc-exynos.c
index 0fbc53a..4e26049 100644
--- a/drivers/mmc/host/dw_mmc-exynos.c
+++ b/drivers/mmc/host/dw_mmc-exynos.c
@@ -17,6 +17,7 @@
#include <linux/mmc/mmc.h>
#include <linux/of.h>
#include <linux/of_gpio.h>
+#include <linux/mmc/slot-gpio.h>
#include <linux/slab.h>
#include "dw_mmc.h"
@@ -217,6 +218,16 @@ static void dw_mci_exynos_set_ios(struct dw_mci *host, struct mmc_ios *ios)
}
}
+static void dw_mci_exynos_post_init(struct dw_mci_slot *slot)
+{
+ struct dw_mci_board *brd = slot->host->pdata;
+ struct mmc_host *mmc = slot->mmc;
+
+ if (!(brd->quirks & DW_MCI_QUIRK_BROKEN_CARD_DETECTION) &&
+ IS_ERR_VALUE(mmc_gpio_get_cd(mmc)))
+ mmc->caps2 |= MMC_CAP2_CD_NEEDS_POWER;
+}
+
static int dw_mci_exynos_parse_dt(struct dw_mci *host)
{
struct dw_mci_exynos_priv_data *priv;
@@ -399,6 +410,7 @@ static const struct dw_mci_drv_data exynos_drv_data = {
.prepare_command = dw_mci_exynos_prepare_command,
.set_ios = dw_mci_exynos_set_ios,
.parse_dt = dw_mci_exynos_parse_dt,
+ .post_init = dw_mci_exynos_post_init,
.execute_tuning = dw_mci_exynos_execute_tuning,
};
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index f20b4b8..6f2c681 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -972,6 +972,22 @@ static void dw_mci_request(struct mmc_host *mmc, struct mmc_request *mrq)
spin_unlock_bh(&host->lock);
}
+/*
+ * some of the boards use controller CD line for card detection.Unfortunately
+ * CD line is bind to the same volatge domain as of the IO lines.If we turn off
+ * IO voltage domain, CD line wont work.
+ * Return true when controller CD line is used for card detection or return
+ * false.
+ */
+static bool dw_mci_builtin_cd(struct dw_mci_slot *slot)
+{
+ struct dw_mci_board *brd = slot->host->pdata;
+ struct mmc_host *mmc = slot->mmc;
+
+ return (!(brd->quirks & DW_MCI_QUIRK_BROKEN_CARD_DETECTION) &&
+ IS_ERR_VALUE(mmc_gpio_get_cd(mmc))) ? 1 : 0;
+}
+
static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
{
struct dw_mci_slot *slot = mmc_priv(mmc);
@@ -1043,6 +1059,10 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
mci_writel(slot->host, PWREN, regs);
break;
+ if (dw_mci_builtin_cd(slot) &&
+ !test_bit(DW_MMC_CARD_PRESENT, &slot->flags))
+ return;
+
if (!IS_ERR(mmc->supply.vmmc))
mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, 0);
@@ -1055,6 +1075,8 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
regs &= ~(1 << slot->id);
mci_writel(slot->host, PWREN, regs);
break;
+ break;
break;
}
@@ -2310,6 +2332,9 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
else
clear_bit(DW_MMC_CARD_PRESENT, &slot->flags);
+ if (drv_data && drv_data->post_init)
+ drv_data->post_init(slot);
+
ret = mmc_add_host(mmc);
if (ret)
goto err_setup_bus;
diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
index 01b99e8..a3c2628 100644
--- a/drivers/mmc/host/dw_mmc.h
+++ b/drivers/mmc/host/dw_mmc.h
@@ -250,6 +250,7 @@ struct dw_mci_tuning_data {
*
* Provide controller implementation specific extensions. The usage of this
@@ -263,6 +264,7 @@ struct dw_mci_drv_data {
void (*prepare_command)(struct dw_mci *host, u32 *cmdr);
void (*set_ios)(struct dw_mci *host, struct mmc_ios *ios);
int (*parse_dt)(struct dw_mci *host);
+ void (*post_init)(struct dw_mci_slot *slot);
int (*execute_tuning)(struct dw_mci_slot *slot, u32 opcode,
struct dw_mci_tuning_data *tuning_data);
};
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 4cbf614..5eb24ff 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -42,6 +42,7 @@ struct mmc_ios {
#define MMC_POWER_OFF 0
#define MMC_POWER_UP 1
#define MMC_POWER_ON 2
+#define MMC_POWER_OFF_HARD 3
unsigned char bus_width; /* data bus width */
@@ -283,6 +284,7 @@ struct mmc_host {
#define MMC_CAP2_HS400 (MMC_CAP2_HS400_1_8V | \
MMC_CAP2_HS400_1_2V)
#define MMC_CAP2_SDIO_IRQ_NOTHREAD (1 << 17)
+#define MMC_CAP2_CD_NEEDS_POWER (1 << 18) /* Card detect needs power */
mmc_pm_flag_t pm_caps; /* supported pm features */
--
1.7.10.4
Sonny Rao
2014-08-22 18:27:38 UTC
Permalink
Post by Ulf Hansson
Post by Yuvaraj Kumar C D
Exynos 5250 and 5420 based boards uses built-in CD# line for card
detection.But unfortunately CD# line is on the same voltage rails
as of I/O voltage rails. When we cut off vqmmc,the consequent card
detection will break in these boards.
I am not sure I follow here.
Is the card detect mechanism handled internally by the dw_mmc controller?
Yes
Post by Ulf Hansson
I thought HW engineers long time ago realized that this should be done
separately on a GPIO line to be able to save power while waiting for a
card to be inserted. But that's not case then?
At least in my limited experience, this seems to be common among SoC
vendors who are using dw_mmc, as we've seen this elsewhere as well and
after seeing it here we know that we need to ignore the CD pin that's
routed to dw_mmc and use a separately powered GPIO on the board, but
still there are probably many SoCs/boards which are doing it this way.
Post by Ulf Hansson
Post by Yuvaraj Kumar C D
These hosts (obviously) need to keep vqmmc (and thus vmmc) on all the
time, even when the mmc core tells them to power off. However, one
problem is that these cards won't properly handle mmc_power_cycle().
That's needed to handle error cases when trying to switch voltages
(see 0797e5f mmc:core: Fixup signal voltage switch).
This patch adds a new MMC_POWER_OFF_HARD mode when it's doing a power
cycle. This mode differs from the normal MMC_POWER_OFF mode in that
the mmc core will promise to power the slot back on before it expects
the host to detect card insertion or removal.
This patch is based off of one that Doug wrote (sent privately to
Yuvaraj) which just modifies the MMC core, and should be split into
two patches.
One that modifies the mmc core and one that implements this in dw_mmc.
Post by Ulf Hansson
Post by Yuvaraj Kumar C D
Also if we let alone the vqmmc turned on when vmmc turned off, the
card could have half way powered and this can damage the card. So
this patch adds a check so that, if the board used the built-in
card detection mechanism i.e through CDETECT, it will not turned
down vqmmc and vmmc both.
Why does vmmc needs to be enabled when there are no card inserted?
That can't be right?
I think this is because we don't want to send power to a card over the
I/O pins but not the vcc pin, even for a little while. We also asked
some SD card manufacturers about whether it was okay to leave vqmmc on
and they recommended not doing this, because there's a risk of damage
to the card.

So, in this (broken) environment, we basically just keep both vmmc and
vqmmc on all the time unless mmc core is asking the driver to power
cycle the card through MMC_POWER_OFF and MMC_POWER_ON modes.

Does that help?
Post by Ulf Hansson
Kind regards
Uffe
Post by Yuvaraj Kumar C D
---
1.added a new MMC_POWER_OFF_HARD mode as per Doug Anderson's suggestion.
2.added dw_mci_exynos_post_init() to perform the host specific post
initialisation.
3.added a new flag MMC_CAP2_CD_NEEDS_POWER for host->caps2.
drivers/mmc/core/core.c | 16 ++++++++++++++--
drivers/mmc/core/debugfs.c | 3 +++
drivers/mmc/host/dw_mmc-exynos.c | 12 ++++++++++++
drivers/mmc/host/dw_mmc.c | 25 +++++++++++++++++++++++++
drivers/mmc/host/dw_mmc.h | 2 ++
include/linux/mmc/host.h | 2 ++
6 files changed, 58 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 68f5f4b..79ced36 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -1564,9 +1564,9 @@ void mmc_power_up(struct mmc_host *host, u32 ocr)
mmc_host_clk_release(host);
}
-void mmc_power_off(struct mmc_host *host)
+void _mmc_power_off(struct mmc_host *host, unsigned char power_mode)
{
- if (host->ios.power_mode == MMC_POWER_OFF)
+ if (host->ios.power_mode == power_mode)
return;
mmc_host_clk_hold(host);
@@ -1579,6 +1579,7 @@ void mmc_power_off(struct mmc_host *host)
host->ios.chip_select = MMC_CS_DONTCARE;
}
host->ios.power_mode = MMC_POWER_OFF;
+ host->ios.power_mode = power_mode;
host->ios.bus_width = MMC_BUS_WIDTH_1;
host->ios.timing = MMC_TIMING_LEGACY;
mmc_set_ios(host);
@@ -1593,9 +1594,20 @@ void mmc_power_off(struct mmc_host *host)
mmc_host_clk_release(host);
}
+void mmc_power_off(struct mmc_host *host)
+{
+ _mmc_power_off(host, MMC_POWER_OFF);
+}
+
void mmc_power_cycle(struct mmc_host *host, u32 ocr)
{
mmc_power_off(host);
+ /* If host normally ignores MMC_POWER_OFF, tell it to pay attention */
+ if (host->caps2 & MMC_CAP2_CD_NEEDS_POWER)
+ _mmc_power_off(host, MMC_POWER_OFF_HARD);
+ else
+ _mmc_power_off(host, MMC_POWER_OFF);
+
/* Wait at least 1 ms according to SD spec */
mmc_delay(1);
mmc_power_up(host, ocr);
diff --git a/drivers/mmc/core/debugfs.c b/drivers/mmc/core/debugfs.c
index 91eb162..3d9c5a3 100644
--- a/drivers/mmc/core/debugfs.c
+++ b/drivers/mmc/core/debugfs.c
@@ -108,6 +108,9 @@ static int mmc_ios_show(struct seq_file *s, void *data)
str = "on";
break;
+ str = "hard off";
+ break;
str = "invalid";
break;
diff --git a/drivers/mmc/host/dw_mmc-exynos.c b/drivers/mmc/host/dw_mmc-exynos.c
index 0fbc53a..4e26049 100644
--- a/drivers/mmc/host/dw_mmc-exynos.c
+++ b/drivers/mmc/host/dw_mmc-exynos.c
@@ -17,6 +17,7 @@
#include <linux/mmc/mmc.h>
#include <linux/of.h>
#include <linux/of_gpio.h>
+#include <linux/mmc/slot-gpio.h>
#include <linux/slab.h>
#include "dw_mmc.h"
@@ -217,6 +218,16 @@ static void dw_mci_exynos_set_ios(struct dw_mci *host, struct mmc_ios *ios)
}
}
+static void dw_mci_exynos_post_init(struct dw_mci_slot *slot)
+{
+ struct dw_mci_board *brd = slot->host->pdata;
+ struct mmc_host *mmc = slot->mmc;
+
+ if (!(brd->quirks & DW_MCI_QUIRK_BROKEN_CARD_DETECTION) &&
+ IS_ERR_VALUE(mmc_gpio_get_cd(mmc)))
+ mmc->caps2 |= MMC_CAP2_CD_NEEDS_POWER;
+}
+
static int dw_mci_exynos_parse_dt(struct dw_mci *host)
{
struct dw_mci_exynos_priv_data *priv;
@@ -399,6 +410,7 @@ static const struct dw_mci_drv_data exynos_drv_data = {
.prepare_command = dw_mci_exynos_prepare_command,
.set_ios = dw_mci_exynos_set_ios,
.parse_dt = dw_mci_exynos_parse_dt,
+ .post_init = dw_mci_exynos_post_init,
.execute_tuning = dw_mci_exynos_execute_tuning,
};
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index f20b4b8..6f2c681 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -972,6 +972,22 @@ static void dw_mci_request(struct mmc_host *mmc, struct mmc_request *mrq)
spin_unlock_bh(&host->lock);
}
+/*
+ * some of the boards use controller CD line for card detection.Unfortunately
+ * CD line is bind to the same volatge domain as of the IO lines.If we turn off
+ * IO voltage domain, CD line wont work.
+ * Return true when controller CD line is used for card detection or return
+ * false.
+ */
+static bool dw_mci_builtin_cd(struct dw_mci_slot *slot)
+{
+ struct dw_mci_board *brd = slot->host->pdata;
+ struct mmc_host *mmc = slot->mmc;
+
+ return (!(brd->quirks & DW_MCI_QUIRK_BROKEN_CARD_DETECTION) &&
+ IS_ERR_VALUE(mmc_gpio_get_cd(mmc))) ? 1 : 0;
+}
+
static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
{
struct dw_mci_slot *slot = mmc_priv(mmc);
@@ -1043,6 +1059,10 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
mci_writel(slot->host, PWREN, regs);
break;
+ if (dw_mci_builtin_cd(slot) &&
+ !test_bit(DW_MMC_CARD_PRESENT, &slot->flags))
+ return;
+
if (!IS_ERR(mmc->supply.vmmc))
mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, 0);
@@ -1055,6 +1075,8 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
regs &= ~(1 << slot->id);
mci_writel(slot->host, PWREN, regs);
break;
+ break;
break;
}
@@ -2310,6 +2332,9 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
else
clear_bit(DW_MMC_CARD_PRESENT, &slot->flags);
+ if (drv_data && drv_data->post_init)
+ drv_data->post_init(slot);
+
ret = mmc_add_host(mmc);
if (ret)
goto err_setup_bus;
diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
index 01b99e8..a3c2628 100644
--- a/drivers/mmc/host/dw_mmc.h
+++ b/drivers/mmc/host/dw_mmc.h
@@ -250,6 +250,7 @@ struct dw_mci_tuning_data {
*
* Provide controller implementation specific extensions. The usage of this
@@ -263,6 +264,7 @@ struct dw_mci_drv_data {
void (*prepare_command)(struct dw_mci *host, u32 *cmdr);
void (*set_ios)(struct dw_mci *host, struct mmc_ios *ios);
int (*parse_dt)(struct dw_mci *host);
+ void (*post_init)(struct dw_mci_slot *slot);
int (*execute_tuning)(struct dw_mci_slot *slot, u32 opcode,
struct dw_mci_tuning_data *tuning_data);
};
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 4cbf614..5eb24ff 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -42,6 +42,7 @@ struct mmc_ios {
#define MMC_POWER_OFF 0
#define MMC_POWER_UP 1
#define MMC_POWER_ON 2
+#define MMC_POWER_OFF_HARD 3
unsigned char bus_width; /* data bus width */
@@ -283,6 +284,7 @@ struct mmc_host {
#define MMC_CAP2_HS400 (MMC_CAP2_HS400_1_8V | \
MMC_CAP2_HS400_1_2V)
#define MMC_CAP2_SDIO_IRQ_NOTHREAD (1 << 17)
+#define MMC_CAP2_CD_NEEDS_POWER (1 << 18) /* Card detect needs power */
mmc_pm_flag_t pm_caps; /* supported pm features */
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Ulf Hansson
2014-08-25 08:13:28 UTC
Permalink
Post by Sonny Rao
Post by Ulf Hansson
Post by Yuvaraj Kumar C D
Exynos 5250 and 5420 based boards uses built-in CD# line for card
detection.But unfortunately CD# line is on the same voltage rails
as of I/O voltage rails. When we cut off vqmmc,the consequent card
detection will break in these boards.
I am not sure I follow here.
Is the card detect mechanism handled internally by the dw_mmc controller?
Yes
Just out of curiosity.

Do you know how the power to the actual dw_mmc controller is handled?
I expect it to be SoC specific and I am guessing power domain
regulators may be involved!?
Post by Sonny Rao
Post by Ulf Hansson
I thought HW engineers long time ago realized that this should be done
separately on a GPIO line to be able to save power while waiting for a
card to be inserted. But that's not case then?
At least in my limited experience, this seems to be common among SoC
vendors who are using dw_mmc, as we've seen this elsewhere as well and
after seeing it here we know that we need to ignore the CD pin that's
routed to dw_mmc and use a separately powered GPIO on the board, but
still there are probably many SoCs/boards which are doing it this way.
Post by Ulf Hansson
Post by Yuvaraj Kumar C D
These hosts (obviously) need to keep vqmmc (and thus vmmc) on all the
time, even when the mmc core tells them to power off. However, one
problem is that these cards won't properly handle mmc_power_cycle().
That's needed to handle error cases when trying to switch voltages
(see 0797e5f mmc:core: Fixup signal voltage switch).
This patch adds a new MMC_POWER_OFF_HARD mode when it's doing a power
cycle. This mode differs from the normal MMC_POWER_OFF mode in that
the mmc core will promise to power the slot back on before it expects
the host to detect card insertion or removal.
This patch is based off of one that Doug wrote (sent privately to
Yuvaraj) which just modifies the MMC core, and should be split into
two patches.
One that modifies the mmc core and one that implements this in dw_mmc.
I looked at the mmc core parts, it seems like the wrong approach.

I think you shall be able use MMC_CAP_NEEDS_POLL, to handle this
broken card detect mechanism. We even have a DT binding for that,
"broken-cd".

Kind regards
Uffe
Jaehoon Chung
2014-08-25 08:50:15 UTC
Permalink
Post by Ulf Hansson
Post by Sonny Rao
Post by Ulf Hansson
Post by Yuvaraj Kumar C D
Exynos 5250 and 5420 based boards uses built-in CD# line for card
detection.But unfortunately CD# line is on the same voltage rails
as of I/O voltage rails. When we cut off vqmmc,the consequent card
detection will break in these boards.
I didn't know that use CD# line for card detect.
And if CD# voltage rails and I/O voltage rail are same voltage, it doesn't make sense.
Which card is used with same voltages? (eMMC? SD? SDIO?)

Well, I have checked Exynos5250 and 5420, but it looks like not same rails.
Post by Ulf Hansson
Post by Sonny Rao
Post by Ulf Hansson
I am not sure I follow here.
Is the card detect mechanism handled internally by the dw_mmc controller?
Yes
What card detect mechanism?
Post by Ulf Hansson
Just out of curiosity.
Do you know how the power to the actual dw_mmc controller is handled?
I expect it to be SoC specific and I am guessing power domain
regulators may be involved!?
Post by Sonny Rao
Post by Ulf Hansson
I thought HW engineers long time ago realized that this should be done
separately on a GPIO line to be able to save power while waiting for a
card to be inserted. But that's not case then?
At least in my limited experience, this seems to be common among SoC
vendors who are using dw_mmc, as we've seen this elsewhere as well and
after seeing it here we know that we need to ignore the CD pin that's
routed to dw_mmc and use a separately powered GPIO on the board, but
still there are probably many SoCs/boards which are doing it this way.
Post by Ulf Hansson
Post by Yuvaraj Kumar C D
These hosts (obviously) need to keep vqmmc (and thus vmmc) on all the
time, even when the mmc core tells them to power off. However, one
problem is that these cards won't properly handle mmc_power_cycle().
That's needed to handle error cases when trying to switch voltages
(see 0797e5f mmc:core: Fixup signal voltage switch).
This patch adds a new MMC_POWER_OFF_HARD mode when it's doing a power
cycle. This mode differs from the normal MMC_POWER_OFF mode in that
the mmc core will promise to power the slot back on before it expects
the host to detect card insertion or removal.
This patch is based off of one that Doug wrote (sent privately to
Yuvaraj) which just modifies the MMC core, and should be split into
two patches.
One that modifies the mmc core and one that implements this in dw_mmc.
I looked at the mmc core parts, it seems like the wrong approach.
I think you shall be able use MMC_CAP_NEEDS_POLL, to handle this
broken card detect mechanism. We even have a DT binding for that,
"broken-cd".
I agreed with Ulf's opinion.

Best Regards,
Jaehoon Chung
Post by Ulf Hansson
Kind regards
Uffe
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Doug Anderson
2014-08-25 15:25:02 UTC
Permalink
Jaehoon,
Post by Jaehoon Chung
Post by Ulf Hansson
Post by Yuvaraj Kumar C D
Exynos 5250 and 5420 based boards uses built-in CD# line for card
detection.But unfortunately CD# line is on the same voltage rails
as of I/O voltage rails. When we cut off vqmmc,the consequent card
detection will break in these boards.
I didn't know that use CD# line for card detect.
And if CD# voltage rails and I/O voltage rail are same voltage, it doesn't make sense.
Which card is used with same voltages? (eMMC? SD? SDIO?)
Well, I have checked Exynos5250 and 5420, but it looks like not same rails.
I'm not sure I totally understood what you said. In my manual I have
a table titled "Table 2-1 Exynos 5420 Pin List". Look in this table
for XMMC2CDN and XMMC2DATA_0. Look to the right of the table and
you'll see the power domain. For both it shows VDDQ_MMC2. If that
doesn't mean that the two are in the same voltage domain then I don't
know what does. Can you point to any examples where they have
different voltage domains?

I agree that what exynos does is not sensible, but that's what it is.
Post by Jaehoon Chung
Post by Ulf Hansson
I am not sure I follow here.
Is the card detect mechanism handled internally by the dw_mmc controller?
Yes
What card detect mechanism?
The dw_mmc controller has a way to read the card detect, right?
That's internal to the controller. The alternative would be to use a
generic GPIO for card detect.
Jaehoon Chung
2014-08-27 03:48:17 UTC
Permalink
Hi, Doug,
Post by Doug Anderson
Jaehoon,
Post by Jaehoon Chung
Post by Ulf Hansson
Post by Yuvaraj Kumar C D
Exynos 5250 and 5420 based boards uses built-in CD# line for card
detection.But unfortunately CD# line is on the same voltage rails
as of I/O voltage rails. When we cut off vqmmc,the consequent card
detection will break in these boards.
I didn't know that use CD# line for card detect.
And if CD# voltage rails and I/O voltage rail are same voltage, it doesn't make sense.
Which card is used with same voltages? (eMMC? SD? SDIO?)
Well, I have checked Exynos5250 and 5420, but it looks like not same rails.
I'm not sure I totally understood what you said. In my manual I have
a table titled "Table 2-1 Exynos 5420 Pin List". Look in this table
for XMMC2CDN and XMMC2DATA_0. Look to the right of the table and
you'll see the power domain. For both it shows VDDQ_MMC2. If that
doesn't mean that the two are in the same voltage domain then I don't
know what does. Can you point to any examples where they have
different voltage domains?
I think you're mis-understanding for it.
Right, It's described at exynos5420, but it's not connected.
Exynos4 series are also described, but we used the broken card detection scheme and power used one of "always-on" powers.
Because Card-detection rail need to enable as "always-on".

We don't need to consider this. I checked the circuit, this patch didn't need.

exynos5 also used the gpio-pin for card-detection. And we can use the slot-gpio API.

Best Regards,
Jaehoon Chung
Post by Doug Anderson
I agree that what exynos does is not sensible, but that's what it is.
Post by Jaehoon Chung
Post by Ulf Hansson
I am not sure I follow here.
Is the card detect mechanism handled internally by the dw_mmc controller?
Yes
What card detect mechanism?
The dw_mmc controller has a way to read the card detect, right?
That's internal to the controller. The alternative would be to use a
generic GPIO for card detect.
Doug Anderson
2014-08-27 04:14:11 UTC
Permalink
Jaehoon,
Post by Jaehoon Chung
Hi, Doug,
Post by Doug Anderson
Jaehoon,
Post by Jaehoon Chung
Post by Yuvaraj Kumar C D
Exynos 5250 and 5420 based boards uses built-in CD# line for card
detection.But unfortunately CD# line is on the same voltage rails
as of I/O voltage rails. When we cut off vqmmc,the consequent card
detection will break in these boards.
I didn't know that use CD# line for card detect.
And if CD# voltage rails and I/O voltage rail are same voltage, it doesn't make sense.
Which card is used with same voltages? (eMMC? SD? SDIO?)
Well, I have checked Exynos5250 and 5420, but it looks like not same rails.
I'm not sure I totally understood what you said. In my manual I have
a table titled "Table 2-1 Exynos 5420 Pin List". Look in this table
for XMMC2CDN and XMMC2DATA_0. Look to the right of the table and
you'll see the power domain. For both it shows VDDQ_MMC2. If that
doesn't mean that the two are in the same voltage domain then I don't
know what does. Can you point to any examples where they have
different voltage domains?
I think you're mis-understanding for it.
Right, It's described at exynos5420, but it's not connected.
"It's not connected". What do you mean? If I were to guess I'd say
that on some particular board you're looking at they don't happen to
use the "CD" pin for card detect. If this is what you mean, it
doesn't help me. exynos5420-peach-pit does use the CD pin for card
detect. You can look at the DTS file and confirm it.

...or are you saying that the CD pin somehow changes voltage domains
when configured as a GPIO? I find that very hard to believe. What
voltage domain does it go to? If it goes to a 1.8V voltage domain
then that would be bad when vqmmc was 3.3V. If it goes to a 3.3V
voltage domain then that would be bad when vqmmc was 1.8V. Remember
that externally we've got a pull up to vqmmc.

Even if somehow magically we can read the card detect pin with vqmmc
off, we have an external pullup on this line that goes directly to the
"vqmmc" power rail. If the vqmmc power rail is off then this external
pull up would not work and would actually act as an external pull down
if you could somehow configure the internal line to be a pullup.
Post by Jaehoon Chung
Exynos4 series are also described, but we used the broken card detection scheme and power used one of "always-on" powers.
Because Card-detection rail need to enable as "always-on".
We don't need to consider this. I checked the circuit, this patch didn't need.
exynos5 also used the gpio-pin for card-detection. And we can use the slot-gpio API.
When you say "exynos5", what do you mean here? Do you mean the smdk
for 5250, or something else?
Jaehoon Chung
2014-08-27 04:47:02 UTC
Permalink
Doug,
Post by Doug Anderson
Jaehoon,
Post by Jaehoon Chung
Hi, Doug,
Post by Doug Anderson
Jaehoon,
Post by Jaehoon Chung
Post by Yuvaraj Kumar C D
Exynos 5250 and 5420 based boards uses built-in CD# line for card
detection.But unfortunately CD# line is on the same voltage rails
as of I/O voltage rails. When we cut off vqmmc,the consequent card
detection will break in these boards.
I didn't know that use CD# line for card detect.
And if CD# voltage rails and I/O voltage rail are same voltage, it doesn't make sense.
Which card is used with same voltages? (eMMC? SD? SDIO?)
Well, I have checked Exynos5250 and 5420, but it looks like not same rails.
I'm not sure I totally understood what you said. In my manual I have
a table titled "Table 2-1 Exynos 5420 Pin List". Look in this table
for XMMC2CDN and XMMC2DATA_0. Look to the right of the table and
you'll see the power domain. For both it shows VDDQ_MMC2. If that
doesn't mean that the two are in the same voltage domain then I don't
know what does. Can you point to any examples where they have
different voltage domains?
I think you're mis-understanding for it.
Right, It's described at exynos5420, but it's not connected.
"It's not connected". What do you mean? If I were to guess I'd say
that on some particular board you're looking at they don't happen to
use the "CD" pin for card detect. If this is what you mean, it
doesn't help me. exynos5420-peach-pit does use the CD pin for card
detect. You can look at the DTS file and confirm it.
I didn't know how exynos5420-peach-pit's circuit is configured.
But i guess that almost all exynos5 boards are configured with the similar circuit.

At Almost all Exynos5 board, CD-pin is used, but not included in Same power domain.
(CD-pin is external card-detect pin. - like XEINT_# pin)
You mentioned CD# and DATA# lines is used the same power domain, right?
In Circuit (not exynos5420-peach-pit), DATA# line and CMD/CLK(vqmmc) is same power supply, and vdd is used other power supply.
Not use the CD# pin, used the XEINT_# pin.
So i think we don't need to consider the CD#.
If exynos5420-peach-pit board is used the CD#-pin, then our discussion can be changed.
Your commit message looks like all exynos5250/5420 board are used CD# line.
Post by Doug Anderson
...or are you saying that the CD pin somehow changes voltage domains
when configured as a GPIO? I find that very hard to believe. What
voltage domain does it go to? If it goes to a 1.8V voltage domain
then that would be bad when vqmmc was 3.3V. If it goes to a 3.3V
voltage domain then that would be bad when vqmmc was 1.8V. Remember
that externally we've got a pull up to vqmmc.
It is used with XEINT_# pin instead of CD# pin.
As i mentioned above, if exynos5420-peach-pit is used CD# line and not used XEINT_# pin,
my point is meaningless. :)

Is exynos5420-peach-pit board used with CD#pin, not XEINT_# pin?

Best Regards,
Jaehoon Chung
Post by Doug Anderson
Even if somehow magically we can read the card detect pin with vqmmc
off, we have an external pullup on this line that goes directly to the
"vqmmc" power rail. If the vqmmc power rail is off then this external
pull up would not work and would actually act as an external pull down
if you could somehow configure the internal line to be a pullup.
Post by Jaehoon Chung
Exynos4 series are also described, but we used the broken card detection scheme and power used one of "always-on" powers.
Because Card-detection rail need to enable as "always-on".
We don't need to consider this. I checked the circuit, this patch didn't need.
exynos5 also used the gpio-pin for card-detection. And we can use the slot-gpio API.
When you say "exynos5", what do you mean here? Do you mean the smdk
for 5250, or something else?
Doug Anderson
2014-08-27 15:49:27 UTC
Permalink
Jaehoon,
Post by Jaehoon Chung
Doug,
Post by Doug Anderson
Jaehoon,
Post by Jaehoon Chung
Hi, Doug,
Post by Doug Anderson
Jaehoon,
Post by Jaehoon Chung
Post by Yuvaraj Kumar C D
Exynos 5250 and 5420 based boards uses built-in CD# line for card
detection.But unfortunately CD# line is on the same voltage rails
as of I/O voltage rails. When we cut off vqmmc,the consequent card
detection will break in these boards.
I didn't know that use CD# line for card detect.
And if CD# voltage rails and I/O voltage rail are same voltage, it doesn't make sense.
Which card is used with same voltages? (eMMC? SD? SDIO?)
Well, I have checked Exynos5250 and 5420, but it looks like not same rails.
I'm not sure I totally understood what you said. In my manual I have
a table titled "Table 2-1 Exynos 5420 Pin List". Look in this table
for XMMC2CDN and XMMC2DATA_0. Look to the right of the table and
you'll see the power domain. For both it shows VDDQ_MMC2. If that
doesn't mean that the two are in the same voltage domain then I don't
know what does. Can you point to any examples where they have
different voltage domains?
I think you're mis-understanding for it.
Right, It's described at exynos5420, but it's not connected.
"It's not connected". What do you mean? If I were to guess I'd say
that on some particular board you're looking at they don't happen to
use the "CD" pin for card detect. If this is what you mean, it
doesn't help me. exynos5420-peach-pit does use the CD pin for card
detect. You can look at the DTS file and confirm it.
I didn't know how exynos5420-peach-pit's circuit is configured.
But i guess that almost all exynos5 boards are configured with the similar circuit.
At Almost all Exynos5 board, CD-pin is used, but not included in Same power domain.
(CD-pin is external card-detect pin. - like XEINT_# pin)
You mentioned CD# and DATA# lines is used the same power domain, right?
In Circuit (not exynos5420-peach-pit), DATA# line and CMD/CLK(vqmmc) is same power supply, and vdd is used other power supply.
Not use the CD# pin, used the XEINT_# pin.
So i think we don't need to consider the CD#.
If exynos5420-peach-pit board is used the CD#-pin, then our discussion can be changed.
Maybe on your board you have CD connected to a "gpx" line. ...but not
mine. The guys who designed our hardware followed the SMDK5420
reference schematics which connect the SD card slot card detect to
"gpc2_2", which is the card detect pin.

See "arch/arm/boot/dts/exynos5420-smdk5420.dts", specifically noting
the lack of a GPIO card detect and the inclusion of "sd2_cd"

***@12220000 {
status = "okay";
card-detect-delay = <200>;
samsung,dw-mshc-ciu-div = <3>;
samsung,dw-mshc-sdr-timing = <2 3>;
samsung,dw-mshc-ddr-timing = <1 2>;
pinctrl-names = "default";
pinctrl-0 = <&sd2_clk &sd2_cmd &sd2_cd &sd2_bus4>;
bus-width = <4>;
cap-sd-highspeed;
};

See "arch/arm/boot/dts/exynos5420-peach-pit.dts" too:

&mmc_2 {
status = "okay";
num-slots = <1>;
cap-sd-highspeed;
card-detect-delay = <200>;
clock-frequency = <400000000>;
samsung,dw-mshc-ciu-div = <3>;
samsung,dw-mshc-sdr-timing = <2 3>;
samsung,dw-mshc-ddr-timing = <1 2>;
pinctrl-names = "default";
pinctrl-0 = <&sd2_clk &sd2_cmd &sd2_cd &sd2_bus4>;
bus-width = <4>;
};


Here, see this ASCII art that shows how some lines are hooked up on
peach-pit. You might need to paste this into something with a
fixed-width font.

+--------------------
| Exynos 5420
|
|
P2.8V_LOUT4 ---------|- VDDQ_MMC2 (AK7)
| |
| |
+-+- 10K res -+----|- XMMC2CDN (AK6)
| | |
| | |
| | |
| Ext CD |
| |
+-- 10K res-+--+---|- XMMC2CMD (AK8)
|
|
Ext CMD

You can see from the above that the external card detect signal (that
goes to the connector) named "Ext CD" is pulled up to the same voltage
as the external CMD signal (that also goes to the connector). This is
vqmmc. If we turn off vqmmc then the 10K resistor will (I think) act
as a pull down, or in the best case it will be floating.

Said another way: we can't read card detect when vqmmc is off.
Post by Jaehoon Chung
Your commit message looks like all exynos5250/5420 board are used CD# line.
The commit message should be clearer, agreed. I think I asked Yuvaraj
to make sure that the code only invoked this quirk on exynos and only
if a GPIO was not used for card detect. Yuvaraj: can you make it more
obvious that not all exynos5250/5420 boards need this, only those that
use the "official" card detect line?
Post by Jaehoon Chung
Post by Doug Anderson
...or are you saying that the CD pin somehow changes voltage domains
when configured as a GPIO? I find that very hard to believe. What
voltage domain does it go to? If it goes to a 1.8V voltage domain
then that would be bad when vqmmc was 3.3V. If it goes to a 3.3V
voltage domain then that would be bad when vqmmc was 1.8V. Remember
that externally we've got a pull up to vqmmc.
It is used with XEINT_# pin instead of CD# pin.
As i mentioned above, if exynos5420-peach-pit is used CD# line and not used XEINT_# pin,
my point is meaningless. :)
Is exynos5420-peach-pit board used with CD#pin, not XEINT_# pin?
Yes. It is using CD#. Do you remove your objections to this patch,
then (once the commit message is clearer)?

-Doug
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Yuvaraj Kumar
2014-08-28 04:54:35 UTC
Permalink
Post by Doug Anderson
Jaehoon,
Post by Jaehoon Chung
Doug,
Post by Doug Anderson
Jaehoon,
Post by Jaehoon Chung
Hi, Doug,
Post by Doug Anderson
Jaehoon,
Post by Jaehoon Chung
Post by Yuvaraj Kumar C D
Exynos 5250 and 5420 based boards uses built-in CD# line for card
detection.But unfortunately CD# line is on the same voltage rails
as of I/O voltage rails. When we cut off vqmmc,the consequent card
detection will break in these boards.
I didn't know that use CD# line for card detect.
And if CD# voltage rails and I/O voltage rail are same voltage, it doesn't make sense.
Which card is used with same voltages? (eMMC? SD? SDIO?)
Well, I have checked Exynos5250 and 5420, but it looks like not same rails.
I'm not sure I totally understood what you said. In my manual I have
a table titled "Table 2-1 Exynos 5420 Pin List". Look in this table
for XMMC2CDN and XMMC2DATA_0. Look to the right of the table and
you'll see the power domain. For both it shows VDDQ_MMC2. If that
doesn't mean that the two are in the same voltage domain then I don't
know what does. Can you point to any examples where they have
different voltage domains?
I think you're mis-understanding for it.
Right, It's described at exynos5420, but it's not connected.
"It's not connected". What do you mean? If I were to guess I'd say
that on some particular board you're looking at they don't happen to
use the "CD" pin for card detect. If this is what you mean, it
doesn't help me. exynos5420-peach-pit does use the CD pin for card
detect. You can look at the DTS file and confirm it.
I didn't know how exynos5420-peach-pit's circuit is configured.
But i guess that almost all exynos5 boards are configured with the similar circuit.
At Almost all Exynos5 board, CD-pin is used, but not included in Same power domain.
(CD-pin is external card-detect pin. - like XEINT_# pin)
You mentioned CD# and DATA# lines is used the same power domain, right?
In Circuit (not exynos5420-peach-pit), DATA# line and CMD/CLK(vqmmc) is same power supply, and vdd is used other power supply.
Not use the CD# pin, used the XEINT_# pin.
So i think we don't need to consider the CD#.
If exynos5420-peach-pit board is used the CD#-pin, then our discussion can be changed.
Maybe on your board you have CD connected to a "gpx" line. ...but not
mine. The guys who designed our hardware followed the SMDK5420
reference schematics which connect the SD card slot card detect to
"gpc2_2", which is the card detect pin.
See "arch/arm/boot/dts/exynos5420-smdk5420.dts", specifically noting
the lack of a GPIO card detect and the inclusion of "sd2_cd"
status = "okay";
card-detect-delay = <200>;
samsung,dw-mshc-ciu-div = <3>;
samsung,dw-mshc-sdr-timing = <2 3>;
samsung,dw-mshc-ddr-timing = <1 2>;
pinctrl-names = "default";
pinctrl-0 = <&sd2_clk &sd2_cmd &sd2_cd &sd2_bus4>;
bus-width = <4>;
cap-sd-highspeed;
};
&mmc_2 {
status = "okay";
num-slots = <1>;
cap-sd-highspeed;
card-detect-delay = <200>;
clock-frequency = <400000000>;
samsung,dw-mshc-ciu-div = <3>;
samsung,dw-mshc-sdr-timing = <2 3>;
samsung,dw-mshc-ddr-timing = <1 2>;
pinctrl-names = "default";
pinctrl-0 = <&sd2_clk &sd2_cmd &sd2_cd &sd2_bus4>;
bus-width = <4>;
};
Here, see this ASCII art that shows how some lines are hooked up on
peach-pit. You might need to paste this into something with a
fixed-width font.
+--------------------
| Exynos 5420
|
|
P2.8V_LOUT4 ---------|- VDDQ_MMC2 (AK7)
| |
| |
+-+- 10K res -+----|- XMMC2CDN (AK6)
| | |
| | |
| | |
| Ext CD |
| |
+-- 10K res-+--+---|- XMMC2CMD (AK8)
|
|
Ext CMD
You can see from the above that the external card detect signal (that
goes to the connector) named "Ext CD" is pulled up to the same voltage
as the external CMD signal (that also goes to the connector). This is
vqmmc. If we turn off vqmmc then the 10K resistor will (I think) act
as a pull down, or in the best case it will be floating.
Said another way: we can't read card detect when vqmmc is off.
Post by Jaehoon Chung
Your commit message looks like all exynos5250/5420 board are used CD# line.
The commit message should be clearer, agreed. I think I asked Yuvaraj
to make sure that the code only invoked this quirk on exynos and only
if a GPIO was not used for card detect. Yuvaraj: can you make it more
obvious that not all exynos5250/5420 boards need this, only those that
use the "official" card detect line?
if (!(brd->quirks & DW_MCI_QUIRK_BROKEN_CARD_DETECTION) &&
IS_ERR_VALUE(mmc_gpio_get_cd(mmc)))
This check will not enable the quirk, if a GPIO was used for the card
detect.Did I miss something?
I will change the commit message to exclude "all exynos5250/5420 boards".
Post by Doug Anderson
Post by Jaehoon Chung
Post by Doug Anderson
...or are you saying that the CD pin somehow changes voltage domains
when configured as a GPIO? I find that very hard to believe. What
voltage domain does it go to? If it goes to a 1.8V voltage domain
then that would be bad when vqmmc was 3.3V. If it goes to a 3.3V
voltage domain then that would be bad when vqmmc was 1.8V. Remember
that externally we've got a pull up to vqmmc.
It is used with XEINT_# pin instead of CD# pin.
As i mentioned above, if exynos5420-peach-pit is used CD# line and not used XEINT_# pin,
my point is meaningless. :)
Is exynos5420-peach-pit board used with CD#pin, not XEINT_# pin?
Yes. It is using CD#. Do you remove your objections to this patch,
then (once the commit message is clearer)?
-Doug
Jaehoon Chung
2014-08-28 08:43:54 UTC
Permalink
Post by Doug Anderson
Jaehoon,
Post by Jaehoon Chung
Doug,
Post by Doug Anderson
Jaehoon,
Post by Jaehoon Chung
Hi, Doug,
Post by Doug Anderson
Jaehoon,
Post by Jaehoon Chung
Post by Yuvaraj Kumar C D
Exynos 5250 and 5420 based boards uses built-in CD# line for card
detection.But unfortunately CD# line is on the same voltage rails
as of I/O voltage rails. When we cut off vqmmc,the consequent card
detection will break in these boards.
I didn't know that use CD# line for card detect.
And if CD# voltage rails and I/O voltage rail are same voltage, it doesn't make sense.
Which card is used with same voltages? (eMMC? SD? SDIO?)
Well, I have checked Exynos5250 and 5420, but it looks like not same rails.
I'm not sure I totally understood what you said. In my manual I have
a table titled "Table 2-1 Exynos 5420 Pin List". Look in this table
for XMMC2CDN and XMMC2DATA_0. Look to the right of the table and
you'll see the power domain. For both it shows VDDQ_MMC2. If that
doesn't mean that the two are in the same voltage domain then I don't
know what does. Can you point to any examples where they have
different voltage domains?
I think you're mis-understanding for it.
Right, It's described at exynos5420, but it's not connected.
"It's not connected". What do you mean? If I were to guess I'd say
that on some particular board you're looking at they don't happen to
use the "CD" pin for card detect. If this is what you mean, it
doesn't help me. exynos5420-peach-pit does use the CD pin for card
detect. You can look at the DTS file and confirm it.
I didn't know how exynos5420-peach-pit's circuit is configured.
But i guess that almost all exynos5 boards are configured with the similar circuit.
At Almost all Exynos5 board, CD-pin is used, but not included in Same power domain.
(CD-pin is external card-detect pin. - like XEINT_# pin)
You mentioned CD# and DATA# lines is used the same power domain, right?
In Circuit (not exynos5420-peach-pit), DATA# line and CMD/CLK(vqmmc) is same power supply, and vdd is used other power supply.
Not use the CD# pin, used the XEINT_# pin.
So i think we don't need to consider the CD#.
If exynos5420-peach-pit board is used the CD#-pin, then our discussion can be changed.
Maybe on your board you have CD connected to a "gpx" line. ...but not
mine. The guys who designed our hardware followed the SMDK5420
reference schematics which connect the SD card slot card detect to
"gpc2_2", which is the card detect pin.
See "arch/arm/boot/dts/exynos5420-smdk5420.dts", specifically noting
the lack of a GPIO card detect and the inclusion of "sd2_cd"
status = "okay";
card-detect-delay = <200>;
samsung,dw-mshc-ciu-div = <3>;
samsung,dw-mshc-sdr-timing = <2 3>;
samsung,dw-mshc-ddr-timing = <1 2>;
pinctrl-names = "default";
pinctrl-0 = <&sd2_clk &sd2_cmd &sd2_cd &sd2_bus4>;
bus-width = <4>;
cap-sd-highspeed;
};
&mmc_2 {
status = "okay";
num-slots = <1>;
cap-sd-highspeed;
card-detect-delay = <200>;
clock-frequency = <400000000>;
samsung,dw-mshc-ciu-div = <3>;
samsung,dw-mshc-sdr-timing = <2 3>;
samsung,dw-mshc-ddr-timing = <1 2>;
pinctrl-names = "default";
pinctrl-0 = <&sd2_clk &sd2_cmd &sd2_cd &sd2_bus4>;
bus-width = <4>;
};
Here, see this ASCII art that shows how some lines are hooked up on
peach-pit. You might need to paste this into something with a
fixed-width font.
+--------------------
| Exynos 5420
|
|
P2.8V_LOUT4 ---------|- VDDQ_MMC2 (AK7)
| |
| |
+-+- 10K res -+----|- XMMC2CDN (AK6)
| | |
| | |
| | |
| Ext CD |
| |
+-- 10K res-+--+---|- XMMC2CMD (AK8)
|
|
Ext CMD
You can see from the above that the external card detect signal (that
goes to the connector) named "Ext CD" is pulled up to the same voltage
as the external CMD signal (that also goes to the connector). This is
vqmmc. If we turn off vqmmc then the 10K resistor will (I think) act
as a pull down, or in the best case it will be floating.
Said another way: we can't read card detect when vqmmc is off.
If that's the case, it makes sense. But i wonder why designed like that.
Anyway, then we need to consider that controls the vqmmc power for card-detection.

But if polling system uses, it seems to detect the card.
Polling is the method that sends the status command.
At that time, we can notice whether card is insert/remove. Is it impossible?
Post by Doug Anderson
Post by Jaehoon Chung
Your commit message looks like all exynos5250/5420 board are used CD# line.
The commit message should be clearer, agreed. I think I asked Yuvaraj
to make sure that the code only invoked this quirk on exynos and only
if a GPIO was not used for card detect. Yuvaraj: can you make it more
obvious that not all exynos5250/5420 boards need this, only those that
use the "official" card detect line?
Post by Jaehoon Chung
Post by Doug Anderson
...or are you saying that the CD pin somehow changes voltage domains
when configured as a GPIO? I find that very hard to believe. What
voltage domain does it go to? If it goes to a 1.8V voltage domain
then that would be bad when vqmmc was 3.3V. If it goes to a 3.3V
voltage domain then that would be bad when vqmmc was 1.8V. Remember
that externally we've got a pull up to vqmmc.
It is used with XEINT_# pin instead of CD# pin.
As i mentioned above, if exynos5420-peach-pit is used CD# line and not used XEINT_# pin,
my point is meaningless. :)
Is exynos5420-peach-pit board used with CD#pin, not XEINT_# pin?
Yes. It is using CD#. Do you remove your objections to this patch,
then (once the commit message is clearer)?
Sure.
And I will also effort to find it, if we can find the more generic approach.

Best Regards,
Jaehoon Chung
Post by Doug Anderson
-Doug
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
More majordomo info at http://vger.kernel.org/majordomo-info.html
Doug Anderson
2014-08-28 15:52:13 UTC
Permalink
Jaehoon,
Post by Jaehoon Chung
Post by Doug Anderson
Jaehoon,
Post by Jaehoon Chung
Doug,
Post by Doug Anderson
Jaehoon,
Post by Jaehoon Chung
Hi, Doug,
Post by Doug Anderson
Jaehoon,
Post by Jaehoon Chung
Post by Yuvaraj Kumar C D
Exynos 5250 and 5420 based boards uses built-in CD# line for card
detection.But unfortunately CD# line is on the same voltage rails
as of I/O voltage rails. When we cut off vqmmc,the consequent card
detection will break in these boards.
I didn't know that use CD# line for card detect.
And if CD# voltage rails and I/O voltage rail are same voltage, it doesn't make sense.
Which card is used with same voltages? (eMMC? SD? SDIO?)
Well, I have checked Exynos5250 and 5420, but it looks like not same rails.
I'm not sure I totally understood what you said. In my manual I have
a table titled "Table 2-1 Exynos 5420 Pin List". Look in this table
for XMMC2CDN and XMMC2DATA_0. Look to the right of the table and
you'll see the power domain. For both it shows VDDQ_MMC2. If that
doesn't mean that the two are in the same voltage domain then I don't
know what does. Can you point to any examples where they have
different voltage domains?
I think you're mis-understanding for it.
Right, It's described at exynos5420, but it's not connected.
"It's not connected". What do you mean? If I were to guess I'd say
that on some particular board you're looking at they don't happen to
use the "CD" pin for card detect. If this is what you mean, it
doesn't help me. exynos5420-peach-pit does use the CD pin for card
detect. You can look at the DTS file and confirm it.
I didn't know how exynos5420-peach-pit's circuit is configured.
But i guess that almost all exynos5 boards are configured with the similar circuit.
At Almost all Exynos5 board, CD-pin is used, but not included in Same power domain.
(CD-pin is external card-detect pin. - like XEINT_# pin)
You mentioned CD# and DATA# lines is used the same power domain, right?
In Circuit (not exynos5420-peach-pit), DATA# line and CMD/CLK(vqmmc) is same power supply, and vdd is used other power supply.
Not use the CD# pin, used the XEINT_# pin.
So i think we don't need to consider the CD#.
If exynos5420-peach-pit board is used the CD#-pin, then our discussion can be changed.
Maybe on your board you have CD connected to a "gpx" line. ...but not
mine. The guys who designed our hardware followed the SMDK5420
reference schematics which connect the SD card slot card detect to
"gpc2_2", which is the card detect pin.
See "arch/arm/boot/dts/exynos5420-smdk5420.dts", specifically noting
the lack of a GPIO card detect and the inclusion of "sd2_cd"
status = "okay";
card-detect-delay = <200>;
samsung,dw-mshc-ciu-div = <3>;
samsung,dw-mshc-sdr-timing = <2 3>;
samsung,dw-mshc-ddr-timing = <1 2>;
pinctrl-names = "default";
pinctrl-0 = <&sd2_clk &sd2_cmd &sd2_cd &sd2_bus4>;
bus-width = <4>;
cap-sd-highspeed;
};
&mmc_2 {
status = "okay";
num-slots = <1>;
cap-sd-highspeed;
card-detect-delay = <200>;
clock-frequency = <400000000>;
samsung,dw-mshc-ciu-div = <3>;
samsung,dw-mshc-sdr-timing = <2 3>;
samsung,dw-mshc-ddr-timing = <1 2>;
pinctrl-names = "default";
pinctrl-0 = <&sd2_clk &sd2_cmd &sd2_cd &sd2_bus4>;
bus-width = <4>;
};
Here, see this ASCII art that shows how some lines are hooked up on
peach-pit. You might need to paste this into something with a
fixed-width font.
+--------------------
| Exynos 5420
|
|
P2.8V_LOUT4 ---------|- VDDQ_MMC2 (AK7)
| |
| |
+-+- 10K res -+----|- XMMC2CDN (AK6)
| | |
| | |
| | |
| Ext CD |
| |
+-- 10K res-+--+---|- XMMC2CMD (AK8)
|
|
Ext CMD
You can see from the above that the external card detect signal (that
goes to the connector) named "Ext CD" is pulled up to the same voltage
as the external CMD signal (that also goes to the connector). This is
vqmmc. If we turn off vqmmc then the 10K resistor will (I think) act
as a pull down, or in the best case it will be floating.
Said another way: we can't read card detect when vqmmc is off.
If that's the case, it makes sense. But i wonder why designed like that.
Anyway, then we need to consider that controls the vqmmc power for card-detection.
Maybe you can send a message to the SoC designers at Samsung not to
design the chip incorrectly in the future?
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Doug Anderson
2014-08-25 15:20:40 UTC
Permalink
Ulf,
Post by Ulf Hansson
Post by Sonny Rao
Post by Ulf Hansson
Post by Yuvaraj Kumar C D
Exynos 5250 and 5420 based boards uses built-in CD# line for card
detection.But unfortunately CD# line is on the same voltage rails
as of I/O voltage rails. When we cut off vqmmc,the consequent card
detection will break in these boards.
I am not sure I follow here.
Is the card detect mechanism handled internally by the dw_mmc controller?
Yes
Just out of curiosity.
Do you know how the power to the actual dw_mmc controller is handled?
I expect it to be SoC specific and I am guessing power domain
regulators may be involved!?
You can likely read the dw_mmc registers when vqmmc is off. Is that
what you're asking? Certainly if vqmmc is not powered then the lines
themselves will be useless, won't they? The "vqmmc" supply goes to
the "VDDQ_MMC2" pin on 5420. In my 5420 user manual, I see that
"clk", "cmd", "cd", "datN", "wp" and "biuvr" pins are all in this same
voltage (VDDQ_MMC2) domain. Can you really read a pin without
powering that part of the SoC?
Post by Ulf Hansson
Post by Sonny Rao
Post by Ulf Hansson
I thought HW engineers long time ago realized that this should be done
separately on a GPIO line to be able to save power while waiting for a
card to be inserted. But that's not case then?
At least in my limited experience, this seems to be common among SoC
vendors who are using dw_mmc, as we've seen this elsewhere as well and
after seeing it here we know that we need to ignore the CD pin that's
routed to dw_mmc and use a separately powered GPIO on the board, but
still there are probably many SoCs/boards which are doing it this way.
Post by Ulf Hansson
Post by Yuvaraj Kumar C D
These hosts (obviously) need to keep vqmmc (and thus vmmc) on all the
time, even when the mmc core tells them to power off. However, one
problem is that these cards won't properly handle mmc_power_cycle().
That's needed to handle error cases when trying to switch voltages
(see 0797e5f mmc:core: Fixup signal voltage switch).
This patch adds a new MMC_POWER_OFF_HARD mode when it's doing a power
cycle. This mode differs from the normal MMC_POWER_OFF mode in that
the mmc core will promise to power the slot back on before it expects
the host to detect card insertion or removal.
This patch is based off of one that Doug wrote (sent privately to
Yuvaraj) which just modifies the MMC core, and should be split into
two patches.
One that modifies the mmc core and one that implements this in dw_mmc.
I looked at the mmc core parts, it seems like the wrong approach.
I think you shall be able use MMC_CAP_NEEDS_POLL, to handle this
broken card detect mechanism. We even have a DT binding for that,
"broken-cd".
I don't think this is possible, but let me explain why I think so and
you can correct me.

The voltage domain of the "card detect" pin on the SoC is vqmmc,
right? That means that you won't be able to read the pin without
turning on vqmmc. Even if you could read the pin without turning on
vqmmc, the pullup on this line is connected to vqmmc too. ...so if
vqmmc is off then there's no pulup and you can't use card detect.

Are you suggesting that we should flip the voltage of vqmmc (and thus
vmmc to prevent damaging the card) during polling? That seems ugly.


One other thing to mention: we didn't find any power savings by
actually turning off vmmc and vqmmc when there was no card inserted.
There's no current running through the lines when there is no card
inserted and apparently everything is efficient enough that there was
no problem.

-Doug
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Ulf Hansson
2014-08-26 07:37:29 UTC
Permalink
Post by Doug Anderson
Ulf,
Post by Ulf Hansson
Post by Sonny Rao
Post by Ulf Hansson
Post by Yuvaraj Kumar C D
Exynos 5250 and 5420 based boards uses built-in CD# line for card
detection.But unfortunately CD# line is on the same voltage rails
as of I/O voltage rails. When we cut off vqmmc,the consequent card
detection will break in these boards.
I am not sure I follow here.
Is the card detect mechanism handled internally by the dw_mmc controller?
Yes
Just out of curiosity.
Do you know how the power to the actual dw_mmc controller is handled?
I expect it to be SoC specific and I am guessing power domain
regulators may be involved!?
You can likely read the dw_mmc registers when vqmmc is off. Is that
what you're asking? Certainly if vqmmc is not powered then the lines
themselves will be useless, won't they? The "vqmmc" supply goes to
the "VDDQ_MMC2" pin on 5420. In my 5420 user manual, I see that
"clk", "cmd", "cd", "datN", "wp" and "biuvr" pins are all in this same
voltage (VDDQ_MMC2) domain. Can you really read a pin without
powering that part of the SoC?
Can you verify that you can't re-route these signals to GPIOs on your
Exynos board, especially for wp and cd?

That would mean, you don't need the internal logic of the dw_mmc to
get either card detect or write protect, which would solve your
problem.
Post by Doug Anderson
Post by Ulf Hansson
Post by Sonny Rao
Post by Ulf Hansson
I thought HW engineers long time ago realized that this should be done
separately on a GPIO line to be able to save power while waiting for a
card to be inserted. But that's not case then?
At least in my limited experience, this seems to be common among SoC
vendors who are using dw_mmc, as we've seen this elsewhere as well and
after seeing it here we know that we need to ignore the CD pin that's
routed to dw_mmc and use a separately powered GPIO on the board, but
still there are probably many SoCs/boards which are doing it this way.
Post by Ulf Hansson
Post by Yuvaraj Kumar C D
These hosts (obviously) need to keep vqmmc (and thus vmmc) on all the
time, even when the mmc core tells them to power off. However, one
problem is that these cards won't properly handle mmc_power_cycle().
That's needed to handle error cases when trying to switch voltages
(see 0797e5f mmc:core: Fixup signal voltage switch).
This patch adds a new MMC_POWER_OFF_HARD mode when it's doing a power
cycle. This mode differs from the normal MMC_POWER_OFF mode in that
the mmc core will promise to power the slot back on before it expects
the host to detect card insertion or removal.
This patch is based off of one that Doug wrote (sent privately to
Yuvaraj) which just modifies the MMC core, and should be split into
two patches.
One that modifies the mmc core and one that implements this in dw_mmc.
I looked at the mmc core parts, it seems like the wrong approach.
I think you shall be able use MMC_CAP_NEEDS_POLL, to handle this
broken card detect mechanism. We even have a DT binding for that,
"broken-cd".
I don't think this is possible, but let me explain why I think so and
you can correct me.
The voltage domain of the "card detect" pin on the SoC is vqmmc,
right? That means that you won't be able to read the pin without
turning on vqmmc. Even if you could read the pin without turning on
vqmmc, the pullup on this line is connected to vqmmc too. ...so if
vqmmc is off then there's no pulup and you can't use card detect.
Are you suggesting that we should flip the voltage of vqmmc (and thus
vmmc to prevent damaging the card) during polling? That seems ugly.
I am not sure I follow to understand the problem. All I am saying is:
From your ->set_ios() when you get MMC_POWER_UP, enable vcc and vccq.
From your ->set_ios() when you get MMC_POWER_OFF, disable vccq and vcc.

The rest is taken care of from mmc core, when trying to initialize the card.
Post by Doug Anderson
One other thing to mention: we didn't find any power savings by
actually turning off vmmc and vqmmc when there was no card inserted.
There's no current running through the lines when there is no card
inserted and apparently everything is efficient enough that there was
no problem.
On what level of current leakage did you measure?

In an system PM suspend state we are chasing uA; I find it hard that
no leakage is added when the power is enabled. Anyway, let's leave
that as a separate discussion. :-)

Kind regards
Uffe
Doug Anderson
2014-08-26 20:32:39 UTC
Permalink
Ulf,
Post by Ulf Hansson
Post by Doug Anderson
Ulf,
Post by Ulf Hansson
Post by Sonny Rao
Post by Ulf Hansson
Post by Yuvaraj Kumar C D
Exynos 5250 and 5420 based boards uses built-in CD# line for card
detection.But unfortunately CD# line is on the same voltage rails
as of I/O voltage rails. When we cut off vqmmc,the consequent card
detection will break in these boards.
I am not sure I follow here.
Is the card detect mechanism handled internally by the dw_mmc controller?
Yes
Just out of curiosity.
Do you know how the power to the actual dw_mmc controller is handled?
I expect it to be SoC specific and I am guessing power domain
regulators may be involved!?
You can likely read the dw_mmc registers when vqmmc is off. Is that
what you're asking? Certainly if vqmmc is not powered then the lines
themselves will be useless, won't they? The "vqmmc" supply goes to
the "VDDQ_MMC2" pin on 5420. In my 5420 user manual, I see that
"clk", "cmd", "cd", "datN", "wp" and "biuvr" pins are all in this same
voltage (VDDQ_MMC2) domain. Can you really read a pin without
powering that part of the SoC?
Can you verify that you can't re-route these signals to GPIOs on your
Exynos board, especially for wp and cd?
Verified.
Post by Ulf Hansson
That would mean, you don't need the internal logic of the dw_mmc to
get either card detect or write protect, which would solve your
problem.
Right. ...but since I can't re-route, I can't use this to solve my problem.
Post by Ulf Hansson
Post by Doug Anderson
Post by Ulf Hansson
Post by Sonny Rao
Post by Ulf Hansson
I thought HW engineers long time ago realized that this should be done
separately on a GPIO line to be able to save power while waiting for a
card to be inserted. But that's not case then?
At least in my limited experience, this seems to be common among SoC
vendors who are using dw_mmc, as we've seen this elsewhere as well and
after seeing it here we know that we need to ignore the CD pin that's
routed to dw_mmc and use a separately powered GPIO on the board, but
still there are probably many SoCs/boards which are doing it this way.
Post by Ulf Hansson
Post by Yuvaraj Kumar C D
These hosts (obviously) need to keep vqmmc (and thus vmmc) on all the
time, even when the mmc core tells them to power off. However, one
problem is that these cards won't properly handle mmc_power_cycle().
That's needed to handle error cases when trying to switch voltages
(see 0797e5f mmc:core: Fixup signal voltage switch).
This patch adds a new MMC_POWER_OFF_HARD mode when it's doing a power
cycle. This mode differs from the normal MMC_POWER_OFF mode in that
the mmc core will promise to power the slot back on before it expects
the host to detect card insertion or removal.
This patch is based off of one that Doug wrote (sent privately to
Yuvaraj) which just modifies the MMC core, and should be split into
two patches.
One that modifies the mmc core and one that implements this in dw_mmc.
I looked at the mmc core parts, it seems like the wrong approach.
I think you shall be able use MMC_CAP_NEEDS_POLL, to handle this
broken card detect mechanism. We even have a DT binding for that,
"broken-cd".
I don't think this is possible, but let me explain why I think so and
you can correct me.
The voltage domain of the "card detect" pin on the SoC is vqmmc,
right? That means that you won't be able to read the pin without
turning on vqmmc. Even if you could read the pin without turning on
vqmmc, the pullup on this line is connected to vqmmc too. ...so if
vqmmc is off then there's no pulup and you can't use card detect.
Are you suggesting that we should flip the voltage of vqmmc (and thus
vmmc to prevent damaging the card) during polling? That seems ugly.
From your ->set_ios() when you get MMC_POWER_UP, enable vcc and vccq.
From your ->set_ios() when you get MMC_POWER_OFF, disable vccq and vcc.
The rest is taken care of from mmc core, when trying to initialize the card.
We must not be on the same page, so I'll put all my assumptions in
super detail and we can see what's different...


So if I have "MMC_CAP_NEEDS_POLL" set, the MMC core will poll things,
right? ...and you are suggesting that I could solve my vqmmc vs. card
detect problem by using MMC_CAP_NEEDS_POLL, right?


Let's think about the case when no card is inserted. When no card is
inserted the core will call set_ios() with MMC_POWER_OFF, right?

...and we want that to turn off vmmc.
...and if we turn off vmmc, we should turn off vqmmc.

Now we've got vmmc off and vqmmc off and on this board we can't read
the card detect line, right?

Now, we've got MMC_CAP_NEEDS_POLL, so dw_mmc will periodically be
called to check the card detect line, but with vmmc and vqmmc off. It
will be unable to return a sensible value without actually turning on
vmmc and vqmmc.


...so with that context, I'll ask my questoin again:

Are you suggesting that we should flip the voltage of vqmmc (and thus
vmmc to prevent damaging the card) during polling? That seems ugly.
Post by Ulf Hansson
Post by Doug Anderson
One other thing to mention: we didn't find any power savings by
actually turning off vmmc and vqmmc when there was no card inserted.
There's no current running through the lines when there is no card
inserted and apparently everything is efficient enough that there was
no problem.
On what level of current leakage did you measure?
It was not measurable during normal running (S0).
Post by Ulf Hansson
In an system PM suspend state we are chasing uA; I find it hard that
no leakage is added when the power is enabled. Anyway, let's leave
that as a separate discussion. :-)
During system PM state we actually _can_ turn power off. We don't
need to detect card insertions at that time (card detect is not a
wakeup source on this board).

-Doug
Ulf Hansson
2014-08-27 11:17:34 UTC
Permalink
Hi Doug,

[snip]
Post by Doug Anderson
Post by Ulf Hansson
Post by Doug Anderson
Post by Ulf Hansson
I think you shall be able use MMC_CAP_NEEDS_POLL, to handle this
broken card detect mechanism. We even have a DT binding for that,
"broken-cd".
I don't think this is possible, but let me explain why I think so and
you can correct me.
The voltage domain of the "card detect" pin on the SoC is vqmmc,
right? That means that you won't be able to read the pin without
turning on vqmmc. Even if you could read the pin without turning on
vqmmc, the pullup on this line is connected to vqmmc too. ...so if
vqmmc is off then there's no pulup and you can't use card detect.
Are you suggesting that we should flip the voltage of vqmmc (and thus
vmmc to prevent damaging the card) during polling? That seems ugly.
From your ->set_ios() when you get MMC_POWER_UP, enable vcc and vccq.
From your ->set_ios() when you get MMC_POWER_OFF, disable vccq and vcc.
The rest is taken care of from mmc core, when trying to initialize the card.
We must not be on the same page, so I'll put all my assumptions in
super detail and we can see what's different...
So if I have "MMC_CAP_NEEDS_POLL" set, the MMC core will poll things,
right? ...and you are suggesting that I could solve my vqmmc vs. card
detect problem by using MMC_CAP_NEEDS_POLL, right?
Yes.
Post by Doug Anderson
Let's think about the case when no card is inserted. When no card is
inserted the core will call set_ios() with MMC_POWER_OFF, right?
...and we want that to turn off vmmc.
...and if we turn off vmmc, we should turn off vqmmc.
Correct. At MMC_POWER_OFF, all host drivers shall turn off all the
possible powers that goes to the card. I am just telling this to make
sure, we don't think this is a dw_mmc specific thing.
Post by Doug Anderson
Now we've got vmmc off and vqmmc off and on this board we can't read
the card detect line, right?
Got it. :-)
Post by Doug Anderson
Now, we've got MMC_CAP_NEEDS_POLL, so dw_mmc will periodically be
called to check the card detect line, but with vmmc and vqmmc off. It
will be unable to return a sensible value without actually turning on
vmmc and vqmmc.
Currently MMC_CAP_NEEDS_POLL mean the mmc rescan work will reschedule
itself with HZ interval. This to check for card insert/removal.

Now, mmc_rescan() will, if present, invoke host's ->get_cd() callback
to check whether it's worth to continue initialization sequence or if
it should re-schedule itself again.

If your host driver can distinguish whether a card is inserted, which
in this the are when vccq is turned off, your ->get_cd() callback need
to return 0. That will allow mmc_rescan() to continue it's
initialization sequence and do mmc_power_up().
Post by Doug Anderson
Are you suggesting that we should flip the voltage of vqmmc (and thus
vmmc to prevent damaging the card) during polling? That seems ugly.
Nope.

Kind regards
Uffe
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Ulf Hansson
2014-08-27 11:20:35 UTC
Permalink
Post by Ulf Hansson
Hi Doug,
[snip]
Post by Doug Anderson
Post by Ulf Hansson
Post by Doug Anderson
Post by Ulf Hansson
I think you shall be able use MMC_CAP_NEEDS_POLL, to handle this
broken card detect mechanism. We even have a DT binding for that,
"broken-cd".
I don't think this is possible, but let me explain why I think so and
you can correct me.
The voltage domain of the "card detect" pin on the SoC is vqmmc,
right? That means that you won't be able to read the pin without
turning on vqmmc. Even if you could read the pin without turning on
vqmmc, the pullup on this line is connected to vqmmc too. ...so if
vqmmc is off then there's no pulup and you can't use card detect.
Are you suggesting that we should flip the voltage of vqmmc (and thus
vmmc to prevent damaging the card) during polling? That seems ugly.
From your ->set_ios() when you get MMC_POWER_UP, enable vcc and vccq.
From your ->set_ios() when you get MMC_POWER_OFF, disable vccq and vcc.
The rest is taken care of from mmc core, when trying to initialize the card.
We must not be on the same page, so I'll put all my assumptions in
super detail and we can see what's different...
So if I have "MMC_CAP_NEEDS_POLL" set, the MMC core will poll things,
right? ...and you are suggesting that I could solve my vqmmc vs. card
detect problem by using MMC_CAP_NEEDS_POLL, right?
Yes.
Post by Doug Anderson
Let's think about the case when no card is inserted. When no card is
inserted the core will call set_ios() with MMC_POWER_OFF, right?
...and we want that to turn off vmmc.
...and if we turn off vmmc, we should turn off vqmmc.
Correct. At MMC_POWER_OFF, all host drivers shall turn off all the
possible powers that goes to the card. I am just telling this to make
sure, we don't think this is a dw_mmc specific thing.
Post by Doug Anderson
Now we've got vmmc off and vqmmc off and on this board we can't read
the card detect line, right?
Got it. :-)
Post by Doug Anderson
Now, we've got MMC_CAP_NEEDS_POLL, so dw_mmc will periodically be
called to check the card detect line, but with vmmc and vqmmc off. It
will be unable to return a sensible value without actually turning on
vmmc and vqmmc.
Currently MMC_CAP_NEEDS_POLL mean the mmc rescan work will reschedule
itself with HZ interval. This to check for card insert/removal.
Now, mmc_rescan() will, if present, invoke host's ->get_cd() callback
to check whether it's worth to continue initialization sequence or if
it should re-schedule itself again.
If your host driver can distinguish whether a card is inserted, which
in this the are when vccq is turned off, your ->get_cd() callback need
/s /the /case
Post by Ulf Hansson
to return 0. That will allow mmc_rescan() to continue it's
/s /return 0 /return 1
Post by Ulf Hansson
initialization sequence and do mmc_power_up().
Post by Doug Anderson
Are you suggesting that we should flip the voltage of vqmmc (and thus
vmmc to prevent damaging the card) during polling? That seems ugly.
Nope.
Kind regards
Uffe
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Doug Anderson
2014-08-27 15:52:50 UTC
Permalink
Ulf,
Post by Ulf Hansson
Post by Doug Anderson
Now, we've got MMC_CAP_NEEDS_POLL, so dw_mmc will periodically be
called to check the card detect line, but with vmmc and vqmmc off. It
will be unable to return a sensible value without actually turning on
vmmc and vqmmc.
Currently MMC_CAP_NEEDS_POLL mean the mmc rescan work will reschedule
itself with HZ interval. This to check for card insert/removal.
Now, mmc_rescan() will, if present, invoke host's ->get_cd() callback
to check whether it's worth to continue initialization sequence or if
it should re-schedule itself again.
If your host driver can distinguish whether a card is inserted, which
in this case are when vccq is turned off, your ->get_cd() callback need
to return 1. That will allow mmc_rescan() to continue it's
initialization sequence and do mmc_power_up().
As per my other email, we can't tell whether a card is inserted when
vqmmc is off.

Does this mean that you have removed your objections to a solution
like Yuvaraj has posted? We still need to break it into two patches
(the core part and the dwmmc part), but otherwise is it OK? I can
post the original patch I sent Yuvaraj if it's helpful (or he can just
include my patch as part 1 of his series).

-Doug
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Ulf Hansson
2014-08-28 07:25:55 UTC
Permalink
Post by Doug Anderson
Ulf,
Post by Ulf Hansson
Post by Doug Anderson
Now, we've got MMC_CAP_NEEDS_POLL, so dw_mmc will periodically be
called to check the card detect line, but with vmmc and vqmmc off. It
will be unable to return a sensible value without actually turning on
vmmc and vqmmc.
Currently MMC_CAP_NEEDS_POLL mean the mmc rescan work will reschedule
itself with HZ interval. This to check for card insert/removal.
Now, mmc_rescan() will, if present, invoke host's ->get_cd() callback
to check whether it's worth to continue initialization sequence or if
it should re-schedule itself again.
If your host driver can distinguish whether a card is inserted, which
in this case are when vccq is turned off, your ->get_cd() callback need
to return 1. That will allow mmc_rescan() to continue it's
initialization sequence and do mmc_power_up().
As per my other email, we can't tell whether a card is inserted when
vqmmc is off.
I understand.

The solution I proposed above, is:

1) Enable MMC_CAP_NEEDS_POLL.
2) Make ->get_cd() return 1, when you can't distinguish if the card is inserted.

That will solve this issue and without messing up the mmc core.
Post by Doug Anderson
Does this mean that you have removed your objections to a solution
like Yuvaraj has posted? We still need to break it into two patches
(the core part and the dwmmc part), but otherwise is it OK? I can
post the original patch I sent Yuvaraj if it's helpful (or he can just
include my patch as part 1 of his series).
No. This can entirely be fixed in the host driver. As suggested above.

Kind regards
Uffe
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Doug Anderson
2014-08-28 15:50:25 UTC
Permalink
Ulf,
Post by Ulf Hansson
Post by Doug Anderson
Ulf,
Post by Ulf Hansson
Post by Doug Anderson
Now, we've got MMC_CAP_NEEDS_POLL, so dw_mmc will periodically be
called to check the card detect line, but with vmmc and vqmmc off. It
will be unable to return a sensible value without actually turning on
vmmc and vqmmc.
Currently MMC_CAP_NEEDS_POLL mean the mmc rescan work will reschedule
itself with HZ interval. This to check for card insert/removal.
Now, mmc_rescan() will, if present, invoke host's ->get_cd() callback
to check whether it's worth to continue initialization sequence or if
it should re-schedule itself again.
If your host driver can distinguish whether a card is inserted, which
in this case are when vccq is turned off, your ->get_cd() callback need
to return 1. That will allow mmc_rescan() to continue it's
initialization sequence and do mmc_power_up().
As per my other email, we can't tell whether a card is inserted when
vqmmc is off.
I understand.
1) Enable MMC_CAP_NEEDS_POLL.
2) Make ->get_cd() return 1, when you can't distinguish if the card is inserted.
That will solve this issue and without messing up the mmc core.
Ah, I see! So every 1 second, we'll do the following:

1. Call get_cd(), which returns 1.

2. MMC core will power everything up (turn on all regulators) for MMC.

3. We'll start to initialize a card.

4. We'll notice that, oops, there's no card there.

5. MMC core will shut things down again.


That seems awfully expensive to do every second when the card detect
line actually does work (as long as you keep power lines). Is the
patch to separate out the concepts of "power off because no card is
inserted" and "power off because we're power cycling the card" really
bad enough to warrant forcing us to use the above?

I'm not an EE, but cycling regulators on and off every second doesn't
seem super ideal, but maybe they're designed for it?


Personally, I'd be tempted to just leave the power on all the time and
if a card somehow needs to be power cycled (because UHS negotiation
failed?) then that card just isn't supported. This could be done in
the dts by saying that the regulator is "always on" and no need to
pollute any source files.


-Doug
Sonny Rao
2014-08-28 17:50:28 UTC
Permalink
Post by Doug Anderson
Ulf,
Post by Ulf Hansson
Post by Doug Anderson
Ulf,
Post by Ulf Hansson
Post by Doug Anderson
Now, we've got MMC_CAP_NEEDS_POLL, so dw_mmc will periodically be
called to check the card detect line, but with vmmc and vqmmc off. It
will be unable to return a sensible value without actually turning on
vmmc and vqmmc.
Currently MMC_CAP_NEEDS_POLL mean the mmc rescan work will reschedule
itself with HZ interval. This to check for card insert/removal.
Now, mmc_rescan() will, if present, invoke host's ->get_cd() callback
to check whether it's worth to continue initialization sequence or if
it should re-schedule itself again.
If your host driver can distinguish whether a card is inserted, which
in this case are when vccq is turned off, your ->get_cd() callback need
to return 1. That will allow mmc_rescan() to continue it's
initialization sequence and do mmc_power_up().
As per my other email, we can't tell whether a card is inserted when
vqmmc is off.
I understand.
1) Enable MMC_CAP_NEEDS_POLL.
2) Make ->get_cd() return 1, when you can't distinguish if the card is inserted.
That will solve this issue and without messing up the mmc core.
1. Call get_cd(), which returns 1.
2. MMC core will power everything up (turn on all regulators) for MMC.
3. We'll start to initialize a card.
4. We'll notice that, oops, there's no card there.
5. MMC core will shut things down again.
That seems awfully expensive to do every second when the card detect
line actually does work (as long as you keep power lines). Is the
patch to separate out the concepts of "power off because no card is
inserted" and "power off because we're power cycling the card" really
bad enough to warrant forcing us to use the above?
I'm not an EE, but cycling regulators on and off every second doesn't
seem super ideal, but maybe they're designed for it?
Personally, I'd be tempted to just leave the power on all the time and
if a card somehow needs to be power cycled (because UHS negotiation
failed?) then that card just isn't supported. This could be done in
the dts by saying that the regulator is "always on" and no need to
pollute any source files.
Yeah, power cycling the regulators constantly doesn't seem like a
great idea, we can ask the EEs what they think.

This scheme of leaving them on all the time would prevent us from
being able to actually power cycle the card, which I think is required
by the spec in case UHS negotiation fails.
Post by Doug Anderson
-Doug
Doug Anderson
2014-08-29 04:08:10 UTC
Permalink
Hi,
Post by Sonny Rao
Post by Doug Anderson
Ulf,
Post by Ulf Hansson
Post by Doug Anderson
Ulf,
Post by Ulf Hansson
Post by Doug Anderson
Now, we've got MMC_CAP_NEEDS_POLL, so dw_mmc will periodically be
called to check the card detect line, but with vmmc and vqmmc off. It
will be unable to return a sensible value without actually turning on
vmmc and vqmmc.
Currently MMC_CAP_NEEDS_POLL mean the mmc rescan work will reschedule
itself with HZ interval. This to check for card insert/removal.
Now, mmc_rescan() will, if present, invoke host's ->get_cd() callback
to check whether it's worth to continue initialization sequence or if
it should re-schedule itself again.
If your host driver can distinguish whether a card is inserted, which
in this case are when vccq is turned off, your ->get_cd() callback need
to return 1. That will allow mmc_rescan() to continue it's
initialization sequence and do mmc_power_up().
As per my other email, we can't tell whether a card is inserted when
vqmmc is off.
I understand.
1) Enable MMC_CAP_NEEDS_POLL.
2) Make ->get_cd() return 1, when you can't distinguish if the card is inserted.
That will solve this issue and without messing up the mmc core.
1. Call get_cd(), which returns 1.
2. MMC core will power everything up (turn on all regulators) for MMC.
3. We'll start to initialize a card.
4. We'll notice that, oops, there's no card there.
5. MMC core will shut things down again.
That seems awfully expensive to do every second when the card detect
line actually does work (as long as you keep power lines). Is the
patch to separate out the concepts of "power off because no card is
inserted" and "power off because we're power cycling the card" really
bad enough to warrant forcing us to use the above?
I'm not an EE, but cycling regulators on and off every second doesn't
seem super ideal, but maybe they're designed for it?
Personally, I'd be tempted to just leave the power on all the time and
if a card somehow needs to be power cycled (because UHS negotiation
failed?) then that card just isn't supported. This could be done in
the dts by saying that the regulator is "always on" and no need to
pollute any source files.
Yeah, power cycling the regulators constantly doesn't seem like a
great idea, we can ask the EEs what they think.
This scheme of leaving them on all the time would prevent us from
being able to actually power cycle the card, which I think is required
by the spec in case UHS negotiation fails.
OK, fair enough. I guess polling is less bad than nor supporting card
power cycling. ...it still feels like adding this quirk to the core
isn't super ugly, but if everyone is against it...


-Doug
Jaehoon Chung
2014-08-27 03:55:09 UTC
Permalink
Hi.
Post by Doug Anderson
Ulf,
Post by Ulf Hansson
Post by Sonny Rao
Post by Ulf Hansson
Post by Yuvaraj Kumar C D
Exynos 5250 and 5420 based boards uses built-in CD# line for card
detection.But unfortunately CD# line is on the same voltage rails
as of I/O voltage rails. When we cut off vqmmc,the consequent card
detection will break in these boards.
I am not sure I follow here.
Is the card detect mechanism handled internally by the dw_mmc controller?
Yes
Just out of curiosity.
Do you know how the power to the actual dw_mmc controller is handled?
I expect it to be SoC specific and I am guessing power domain
regulators may be involved!?
You can likely read the dw_mmc registers when vqmmc is off. Is that
what you're asking? Certainly if vqmmc is not powered then the lines
themselves will be useless, won't they? The "vqmmc" supply goes to
the "VDDQ_MMC2" pin on 5420. In my 5420 user manual, I see that
"clk", "cmd", "cd", "datN", "wp" and "biuvr" pins are all in this same
voltage (VDDQ_MMC2) domain. Can you really read a pin without
powering that part of the SoC?
It's not correct.
At TRM, described as same voltage domain. But CD-pin is used with "always-on" power.
In circuit, CD# pin is disconnected.
Post by Doug Anderson
Post by Ulf Hansson
Post by Sonny Rao
Post by Ulf Hansson
I thought HW engineers long time ago realized that this should be done
separately on a GPIO line to be able to save power while waiting for a
card to be inserted. But that's not case then?
At least in my limited experience, this seems to be common among SoC
vendors who are using dw_mmc, as we've seen this elsewhere as well and
after seeing it here we know that we need to ignore the CD pin that's
routed to dw_mmc and use a separately powered GPIO on the board, but
still there are probably many SoCs/boards which are doing it this way.
Post by Ulf Hansson
Post by Yuvaraj Kumar C D
These hosts (obviously) need to keep vqmmc (and thus vmmc) on all the
time, even when the mmc core tells them to power off. However, one
problem is that these cards won't properly handle mmc_power_cycle().
That's needed to handle error cases when trying to switch voltages
(see 0797e5f mmc:core: Fixup signal voltage switch).
This patch adds a new MMC_POWER_OFF_HARD mode when it's doing a power
cycle. This mode differs from the normal MMC_POWER_OFF mode in that
the mmc core will promise to power the slot back on before it expects
the host to detect card insertion or removal.
This patch is based off of one that Doug wrote (sent privately to
Yuvaraj) which just modifies the MMC core, and should be split into
two patches.
One that modifies the mmc core and one that implements this in dw_mmc.
I looked at the mmc core parts, it seems like the wrong approach.
I think you shall be able use MMC_CAP_NEEDS_POLL, to handle this
broken card detect mechanism. We even have a DT binding for that,
"broken-cd".
I don't think this is possible, but let me explain why I think so and
you can correct me.
Exynos series is using the external gpio-cd concept. So it need not to use MMC_CAP_NEEDS_POLL.
Can use the slot-gpio API. In my exynos5 board, it's working fine with the slot-gpio API.

Best Regards,
Jaehoon Chung
Post by Doug Anderson
The voltage domain of the "card detect" pin on the SoC is vqmmc,
right? That means that you won't be able to read the pin without
turning on vqmmc. Even if you could read the pin without turning on
vqmmc, the pullup on this line is connected to vqmmc too. ...so if
vqmmc is off then there's no pulup and you can't use card detect.
Are you suggesting that we should flip the voltage of vqmmc (and thus
vmmc to prevent damaging the card) during polling? That seems ugly.
One other thing to mention: we didn't find any power savings by
actually turning off vmmc and vqmmc when there was no card inserted.
There's no current running through the lines when there is no card
inserted and apparently everything is efficient enough that there was
no problem.
-Doug
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Bartlomiej Zolnierkiewicz
2014-10-01 14:00:34 UTC
Permalink
Hi,
Since I am out of station, i dont have an access to my work set up.
Can you send me the dts entries of sd crad and their corresponding regulator entries?
...
***@12200000 {
status = "okay";
broken-cd;
supports-highspeed;
card-detect-delay = <200>;
samsung,dw-mshc-ciu-div = <3>;
samsung,dw-mshc-sdr-timing = <0 4>;
samsung,dw-mshc-ddr-timing = <0 2>;
pinctrl-names = "default";
pinctrl-0 = <&sd0_clk &sd0_cmd &sd0_bus4 &sd0_bus8>;
vmmc-supply = <&ldo10_reg>;

***@0 {
reg = <0>;
bus-width = <8>;
};
};

***@12220000 {
status = "okay";
supports-highspeed;
card-detect-delay = <200>;
samsung,dw-mshc-ciu-div = <3>;
samsung,dw-mshc-sdr-timing = <2 3>;
samsung,dw-mshc-ddr-timing = <1 2>;
pinctrl-names = "default";
pinctrl-0 = <&sd2_clk &sd2_cmd &sd2_cd &sd2_bus4>;
vmmc-supply = <&ldo10_reg>;

***@0 {
reg = <0>;
bus-width = <4>;
};
};
...
ldo10_reg: LDO10 {
regulator-name = "PVDD_PRE_1V8";
regulator-min-microvolt = <1800000>;
regulator-max-microvolt = <1800000>;
regulator-always-on;
};
...

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
Doug Anderson
2014-10-01 16:04:09 UTC
Permalink
Hi,

On Wed, Oct 1, 2014 at 7:00 AM, Bartlomiej Zolnierkiewicz
Post by Doug Anderson
Hi,
Since I am out of station, i dont have an access to my work set up.
Can you send me the dts entries of sd crad and their corresponding regulator entries?
...
status = "okay";
broken-cd;
supports-highspeed;
card-detect-delay = <200>;
samsung,dw-mshc-ciu-div = <3>;
samsung,dw-mshc-sdr-timing = <0 4>;
samsung,dw-mshc-ddr-timing = <0 2>;
pinctrl-names = "default";
pinctrl-0 = <&sd0_clk &sd0_cmd &sd0_bus4 &sd0_bus8>;
vmmc-supply = <&ldo10_reg>;
reg = <0>;
bus-width = <8>;
};
};
status = "okay";
supports-highspeed;
card-detect-delay = <200>;
samsung,dw-mshc-ciu-div = <3>;
samsung,dw-mshc-sdr-timing = <2 3>;
samsung,dw-mshc-ddr-timing = <1 2>;
pinctrl-names = "default";
pinctrl-0 = <&sd2_clk &sd2_cmd &sd2_cd &sd2_bus4>;
vmmc-supply = <&ldo10_reg>;
reg = <0>;
bus-width = <4>;
};
};
...
ldo10_reg: LDO10 {
regulator-name = "PVDD_PRE_1V8";
regulator-min-microvolt = <1800000>;
regulator-max-microvolt = <1800000>;
regulator-always-on;
};
I don't have schematics for Arndale Octa, but the above is really
fishy. "vmmc" shouldn't be 1.8V. That's the general power signal to
MMC and should be 2.7V - 3.6V. "vqmmc" could be 1.8V in certain
situations, but my understanding is that for maximum compatibility it
should at least start out identical to "vmmc" (and later go down to
1.7V - 1.95V).

My first thought would be to just remove the "vmmc-supply" from your
DTS. I think we could land that and pick it back easily. That will
get you something working and won't introduce any regressions because:
1. MMC core will give you a dummy regulator
2. The code will default to assuming that vmmc is 3.3V, which is what
it used to do anyway.
3. The only referenced regulator is always on anyway.

Separately you could specify a proper vmmc and maybe even a vqmmc.

On SMDK5420 I see this for the SD card (mmc2):
* vmmc should be "VDD_SD_2V8". From LDO19.
* vqmmc should be "VDDQ_MMC2_AP". From LDO13.

OK, I dug up the Arndale schematics. For mmc2:
* vmmc should be PVDD_TFLASH_2V8. That's LDO19.
* vqmmc (hooked up to VDDQ_MMC2): PVDD_APIO_MMCOFF_2V8. LDO13 just like SMDK.

...sadly it looks like Anrdale has a schematics problem that prevents
you from doing UHS. I see that the data lines are pulled up to
PVDD_TFLASH_2V8 (vmmc), not pulled up to PVDD_APIO_MMCOFF_2V8 (vqmmc).
I think that means that if you ever lower vqmmc to 1.8V (as needed for
UHS) then you'll still be pulling up to 2.8V. That's not good. You
should probably make sure that both LDO13 and LDO19 are listed as
being exactly 2.8V.


Anyway, the above has (obviously) not been tested and is just based on
a casual browsing of schematics. Please let me know how it goes.



-Doug
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Bartlomiej Zolnierkiewicz
2014-10-02 16:06:51 UTC
Permalink
Hi,
Post by Doug Anderson
Hi,
On Wed, Oct 1, 2014 at 7:00 AM, Bartlomiej Zolnierkiewicz
Post by Doug Anderson
Hi,
Since I am out of station, i dont have an access to my work set up.
Can you send me the dts entries of sd crad and their corresponding regulator entries?
...
status = "okay";
broken-cd;
supports-highspeed;
card-detect-delay = <200>;
samsung,dw-mshc-ciu-div = <3>;
samsung,dw-mshc-sdr-timing = <0 4>;
samsung,dw-mshc-ddr-timing = <0 2>;
pinctrl-names = "default";
pinctrl-0 = <&sd0_clk &sd0_cmd &sd0_bus4 &sd0_bus8>;
vmmc-supply = <&ldo10_reg>;
reg = <0>;
bus-width = <8>;
};
};
status = "okay";
supports-highspeed;
card-detect-delay = <200>;
samsung,dw-mshc-ciu-div = <3>;
samsung,dw-mshc-sdr-timing = <2 3>;
samsung,dw-mshc-ddr-timing = <1 2>;
pinctrl-names = "default";
pinctrl-0 = <&sd2_clk &sd2_cmd &sd2_cd &sd2_bus4>;
vmmc-supply = <&ldo10_reg>;
reg = <0>;
bus-width = <4>;
};
};
...
ldo10_reg: LDO10 {
regulator-name = "PVDD_PRE_1V8";
regulator-min-microvolt = <1800000>;
regulator-max-microvolt = <1800000>;
regulator-always-on;
};
I don't have schematics for Arndale Octa, but the above is really
fishy. "vmmc" shouldn't be 1.8V. That's the general power signal to
MMC and should be 2.7V - 3.6V. "vqmmc" could be 1.8V in certain
situations, but my understanding is that for maximum compatibility it
should at least start out identical to "vmmc" (and later go down to
1.7V - 1.95V).
My first thought would be to just remove the "vmmc-supply" from your
DTS. I think we could land that and pick it back easily. That will
1. MMC core will give you a dummy regulator
2. The code will default to assuming that vmmc is 3.3V, which is what
it used to do anyway.
3. The only referenced regulator is always on anyway.
Separately you could specify a proper vmmc and maybe even a vqmmc.
* vmmc should be "VDD_SD_2V8". From LDO19.
* vqmmc should be "VDDQ_MMC2_AP". From LDO13.
* vmmc should be PVDD_TFLASH_2V8. That's LDO19.
* vqmmc (hooked up to VDDQ_MMC2): PVDD_APIO_MMCOFF_2V8. LDO13 just like SMDK.
...sadly it looks like Anrdale has a schematics problem that prevents
you from doing UHS. I see that the data lines are pulled up to
PVDD_TFLASH_2V8 (vmmc), not pulled up to PVDD_APIO_MMCOFF_2V8 (vqmmc).
I think that means that if you ever lower vqmmc to 1.8V (as needed for
UHS) then you'll still be pulling up to 2.8V. That's not good. You
should probably make sure that both LDO13 and LDO19 are listed as
being exactly 2.8V.
Anyway, the above has (obviously) not been tested and is just based on
a casual browsing of schematics. Please let me know how it goes.
Removing the "vmmc-supply" entry fixes the problem. Using LDO13 for vmmc
and LDO19 for vqmmc also works fine. I'll post a patch fixing the mmc2 DT
entry to Kukjin in a few minutes. Thank you for your help.

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Loading...