* [PATCH 2/2] mmc: core: Power cycle card on voltage switch fail
@ 2012-09-20 18:42 Johan Rudholm
2012-10-23 20:39 ` Chris Ball
0 siblings, 1 reply; 4+ messages in thread
From: Johan Rudholm @ 2012-09-20 18:42 UTC (permalink / raw)
To: linux-mmc, Chris Ball; +Cc: Per Forlin, Ulf Hansson, Johan Rudholm
Signed-off-by: Johan Rudholm <johan.rudholm@stericsson.com>
---
drivers/mmc/core/core.c | 2 +-
drivers/mmc/core/core.h | 1 +
drivers/mmc/core/sd.c | 8 ++++++--
3 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 3779431..671986a 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -1346,7 +1346,7 @@ static void mmc_poweroff_notify(struct mmc_host *host)
* If a host does all the power sequencing itself, ignore the
* initial MMC_POWER_UP stage.
*/
-static void mmc_power_up(struct mmc_host *host)
+void mmc_power_up(struct mmc_host *host)
{
int bit;
diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h
index 3bdafbc..5a5170d 100644
--- a/drivers/mmc/core/core.h
+++ b/drivers/mmc/core/core.h
@@ -45,6 +45,7 @@ int mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage,
void mmc_set_timing(struct mmc_host *host, unsigned int timing);
void mmc_set_driver_type(struct mmc_host *host, unsigned int drv_type);
void mmc_power_off(struct mmc_host *host);
+void mmc_power_up(struct mmc_host *host);
static inline void mmc_delay(unsigned int ms)
{
diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
index 74972c2..9a165451 100644
--- a/drivers/mmc/core/sd.c
+++ b/drivers/mmc/core/sd.c
@@ -720,6 +720,7 @@ int mmc_sd_get_cid(struct mmc_host *host, u32 ocr, u32 *cid, u32 *rocr)
* state. We wait 1ms to give cards time to
* respond.
*/
+try_again:
mmc_go_idle(host);
/*
@@ -748,7 +749,6 @@ int mmc_sd_get_cid(struct mmc_host *host, u32 ocr, u32 *cid, u32 *rocr)
if (max_current > 150)
ocr |= SD_OCR_XPC;
-try_again:
err = mmc_send_app_op_cond(host, ocr, rocr);
if (err)
return err;
@@ -761,7 +761,11 @@ try_again:
((*rocr & 0x41000000) == 0x41000000)) {
err = mmc_set_signal_voltage(host, MMC_SIGNAL_VOLTAGE_180, true);
if (err) {
- ocr &= ~SD_OCR_S18R;
+ /* Power cycle card */
+ pr_warning("%s: Signal voltage switch failed, "
+ "power cycling card\n", mmc_hostname(host));
+ mmc_power_off(host);
+ mmc_power_up(host);
goto try_again;
}
}
--
1.7.10
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 2/2] mmc: core: Power cycle card on voltage switch fail
2012-09-20 18:42 [PATCH 2/2] mmc: core: Power cycle card on voltage switch fail Johan Rudholm
@ 2012-10-23 20:39 ` Chris Ball
2012-10-23 22:48 ` Daniel Drake
0 siblings, 1 reply; 4+ messages in thread
From: Chris Ball @ 2012-10-23 20:39 UTC (permalink / raw)
To: Johan Rudholm; +Cc: linux-mmc, Per Forlin, Ulf Hansson, dsd
Hi Johan,
On Thu, Sep 20 2012, Johan Rudholm wrote:
> Signed-off-by: Johan Rudholm <johan.rudholm@stericsson.com>
> ---
> drivers/mmc/core/core.c | 2 +-
> drivers/mmc/core/core.h | 1 +
> drivers/mmc/core/sd.c | 8 ++++++--
> 3 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index 3779431..671986a 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -1346,7 +1346,7 @@ static void mmc_poweroff_notify(struct mmc_host *host)
> * If a host does all the power sequencing itself, ignore the
> * initial MMC_POWER_UP stage.
> */
> -static void mmc_power_up(struct mmc_host *host)
> +void mmc_power_up(struct mmc_host *host)
> {
> int bit;
>
> diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h
> index 3bdafbc..5a5170d 100644
> --- a/drivers/mmc/core/core.h
> +++ b/drivers/mmc/core/core.h
> @@ -45,6 +45,7 @@ int mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage,
> void mmc_set_timing(struct mmc_host *host, unsigned int timing);
> void mmc_set_driver_type(struct mmc_host *host, unsigned int drv_type);
> void mmc_power_off(struct mmc_host *host);
> +void mmc_power_up(struct mmc_host *host);
>
> static inline void mmc_delay(unsigned int ms)
> {
> diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
> index 74972c2..9a165451 100644
> --- a/drivers/mmc/core/sd.c
> +++ b/drivers/mmc/core/sd.c
> @@ -720,6 +720,7 @@ int mmc_sd_get_cid(struct mmc_host *host, u32 ocr, u32 *cid, u32 *rocr)
> * state. We wait 1ms to give cards time to
> * respond.
> */
> +try_again:
> mmc_go_idle(host);
>
> /*
> @@ -748,7 +749,6 @@ int mmc_sd_get_cid(struct mmc_host *host, u32 ocr, u32 *cid, u32 *rocr)
> if (max_current > 150)
> ocr |= SD_OCR_XPC;
>
> -try_again:
> err = mmc_send_app_op_cond(host, ocr, rocr);
> if (err)
> return err;
> @@ -761,7 +761,11 @@ try_again:
> ((*rocr & 0x41000000) == 0x41000000)) {
> err = mmc_set_signal_voltage(host, MMC_SIGNAL_VOLTAGE_180, true);
> if (err) {
> - ocr &= ~SD_OCR_S18R;
> + /* Power cycle card */
> + pr_warning("%s: Signal voltage switch failed, "
> + "power cycling card\n", mmc_hostname(host));
> + mmc_power_off(host);
> + mmc_power_up(host);
> goto try_again;
> }
> }
Hasn't the card already been power-cycled inside mmc_set_signal_voltage's
call to sdhci_do_start_signal_voltage_switch()?
/*
* If we are here, that means the switch to 1.8V signaling
* failed. We power cycle the card, and retry initialization
* sequence by setting S18R to 0.
*/
pwr = sdhci_readb(host, SDHCI_POWER_CONTROL);
pwr &= ~SDHCI_POWER_ON;
sdhci_writeb(host, pwr, SDHCI_POWER_CONTROL);
(If you're going to do it here instead, should we remove this code from
sdhci.c?)
Dan, maybe you could see if this patch (there's a 1/2 patch too) solves
your UHS problem?
Thanks,
- Chris.
--
Chris Ball <cjb@laptop.org> <http://printf.net/>
One Laptop Per Child
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 2/2] mmc: core: Power cycle card on voltage switch fail
2012-10-23 20:39 ` Chris Ball
@ 2012-10-23 22:48 ` Daniel Drake
2012-10-24 10:56 ` Johan Rudholm
0 siblings, 1 reply; 4+ messages in thread
From: Daniel Drake @ 2012-10-23 22:48 UTC (permalink / raw)
To: Chris Ball; +Cc: Johan Rudholm, linux-mmc, Per Forlin, Ulf Hansson
[-- Attachment #1: Type: text/plain, Size: 1491 bytes --]
On Tue, Oct 23, 2012 at 2:39 PM, Chris Ball <cjb@laptop.org> wrote:
> Dan, maybe you could see if this patch (there's a 1/2 patch too) solves
> your UHS problem?
I tested [RFC/PATCH] mmc: core: Fixup signal voltage switch
https://patchwork.kernel.org/patch/1514691/
This was previously failing on both XO-1.75 (kernel 3.0) and XO-4
(kernel 3.5) - the 1.8V switch would fail, but it would not
successfully switch back to 3.3V.
Testing on XO-4, it worked fine: the 1.8V failed switch was detected,
it came back as 3.3V and everything was fine.
Testing on XO-1.75, it didn't work: it thought that the 1.8V switch
was successful so it left it at that, then the card reacted in a very
unstable manner (failed/retried reads, huge amount of kernel log spam,
etc).
So I came up with the attached sdhci patch. That fixes the XO-1.75
case, which now correctly detects the 1.8V switch failure and goes to
3.3V. However, that broke XO-4, which now just does:
mmc2: Signal voltage switch failed, power cycling card (retries = 10)
mmc2: error -110 whilst initialising SD card
(no more time to debug exactly whats happening)
All tests with the same 32GB SD storage card.
I wonder if this difference in behaviour is related to the difference
in kernel versions on each platform (3.0 vs 3.5). I would like to test
with 3.5 or newer running on both, but this requires a bit of setup
work for the XO-1.75 first (long story). And I'm out of time at this
point :( this was a borrowed SD card.
Daniel
[-- Attachment #2: 0001-update-sdhci-for-voltage-changing-fixes.txt --]
[-- Type: text/plain, Size: 3675 bytes --]
From 664afe4de295d3783d335a737ce239cebf5885f2 Mon Sep 17 00:00:00 2001
From: Daniel Drake <dsd@laptop.org>
Date: Tue, 23 Oct 2012 15:58:33 -0600
Subject: [PATCH] update sdhci for voltage changing fixes
---
drivers/mmc/host/sdhci.c | 72 +++++++++---------------------------------------
1 file changed, 13 insertions(+), 59 deletions(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 07a5346..1955406 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1587,9 +1587,7 @@ static void sdhci_enable_sdio_irq(struct mmc_host *mmc, int enable)
static int sdhci_do_start_signal_voltage_switch(struct sdhci_host *host,
struct mmc_ios *ios)
{
- u8 pwr;
- u16 clk, ctrl;
- u32 present_state;
+ u16 ctrl;
/*
* Signal Voltage Switching is only applicable for Host Controllers
@@ -1622,62 +1620,9 @@ static int sdhci_do_start_signal_voltage_switch(struct sdhci_host *host,
}
} else if (!(ctrl & SDHCI_CTRL_VDD_180) &&
(ios->signal_voltage == MMC_SIGNAL_VOLTAGE_180)) {
- /* Stop SDCLK */
- clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL);
- clk &= ~SDHCI_CLOCK_CARD_EN;
- sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
-
- /* Check whether DAT[3:0] is 0000 */
- present_state = sdhci_readl(host, SDHCI_PRESENT_STATE);
- if (!((present_state & SDHCI_DATA_LVL_MASK) >>
- SDHCI_DATA_LVL_SHIFT)) {
- /*
- * Enable 1.8V Signal Enable in the Host Control2
- * register
- */
- ctrl |= SDHCI_CTRL_VDD_180;
- sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2);
-
- /* Wait for 5ms */
- usleep_range(5000, 5500);
-
- ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
- if (ctrl & SDHCI_CTRL_VDD_180) {
- /* Provide SDCLK again and wait for 1ms*/
- clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL);
- clk |= SDHCI_CLOCK_CARD_EN;
- sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
- usleep_range(1000, 1500);
-
- /*
- * If DAT[3:0] level is 1111b, then the card
- * was successfully switched to 1.8V signaling.
- */
- present_state = sdhci_readl(host,
- SDHCI_PRESENT_STATE);
- if ((present_state & SDHCI_DATA_LVL_MASK) ==
- SDHCI_DATA_LVL_MASK)
- return 0;
- }
- }
-
- /*
- * If we are here, that means the switch to 1.8V signaling
- * failed. We power cycle the card, and retry initialization
- * sequence by setting S18R to 0.
- */
- pwr = sdhci_readb(host, SDHCI_POWER_CONTROL);
- pwr &= ~SDHCI_POWER_ON;
- sdhci_writeb(host, pwr, SDHCI_POWER_CONTROL);
-
- /* Wait for 1ms as per the spec */
- usleep_range(1000, 1500);
- pwr |= SDHCI_POWER_ON;
- sdhci_writeb(host, pwr, SDHCI_POWER_CONTROL);
-
- pr_info(DRIVER_NAME ": Switching to 1.8V signalling "
- "voltage failed, retrying with S18R set to 0\n");
- return -EAGAIN;
+ ctrl |= SDHCI_CTRL_VDD_180;
+ sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2);
+ return 0;
} else
/* No signal voltage switch required */
return 0;
@@ -1931,11 +1876,20 @@ static void sdhci_enable_preset_value(struct mmc_host *mmc, bool enable)
sdhci_runtime_pm_put(host);
}
+static int sdhci_card_busy(struct mmc_host *mmc, int keep_busy)
+{
+ struct sdhci_host *host = mmc_priv(mmc);
+ u32 present_state = sdhci_readl(host, SDHCI_PRESENT_STATE);
+
+ return (present_state & SDHCI_DATA_LVL_MASK) != SDHCI_DATA_LVL_MASK;
+}
+
static const struct mmc_host_ops sdhci_ops = {
.request = sdhci_request,
.set_ios = sdhci_set_ios,
.get_ro = sdhci_get_ro,
.hw_reset = sdhci_hw_reset,
+ .card_busy = sdhci_card_busy,
.enable_sdio_irq = sdhci_enable_sdio_irq,
.start_signal_voltage_switch = sdhci_start_signal_voltage_switch,
.execute_tuning = sdhci_execute_tuning,
--
1.7.11.7
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 2/2] mmc: core: Power cycle card on voltage switch fail
2012-10-23 22:48 ` Daniel Drake
@ 2012-10-24 10:56 ` Johan Rudholm
0 siblings, 0 replies; 4+ messages in thread
From: Johan Rudholm @ 2012-10-24 10:56 UTC (permalink / raw)
To: Daniel Drake
Cc: Chris Ball, Johan Rudholm, linux-mmc, Per Forlin, Ulf Hansson
Hi Chris and Daniel,
Chris: Thank you for commenting on this patch! Yes, the sdhci power
cycle code needs to be removed just as Daniel's patch does, and as I
suggested in another patch: [RFC] mmc: sdhci: Let core handle UHS
switch failure (https://patchwork.kernel.org/patch/1517211/).
Daniel: Thank you for testing the patch! Comments below.
2012/10/24 Daniel Drake <dsd@laptop.org>:
> On Tue, Oct 23, 2012 at 2:39 PM, Chris Ball <cjb@laptop.org> wrote:
>> Dan, maybe you could see if this patch (there's a 1/2 patch too) solves
>> your UHS problem?
>
> I tested [RFC/PATCH] mmc: core: Fixup signal voltage switch
> https://patchwork.kernel.org/patch/1514691/
>
> This was previously failing on both XO-1.75 (kernel 3.0) and XO-4
> (kernel 3.5) - the 1.8V switch would fail, but it would not
> successfully switch back to 3.3V.
Can you enlighten me a bit; what is XO? :) Host controller hardware?
> Testing on XO-4, it worked fine: the 1.8V failed switch was detected,
> it came back as 3.3V and everything was fine.
Ok, so since you didn't have the card_busy function at this point, the
failure was detected by the sdhci code, right? It power-cycles the
card, returns -EAGAIN, the new code in mmc_sd_get_cid will try 10
times and then come back to 3.3V. Is this what happened, did you get
this print?
pr_warning("%s: Skipping voltage switch\n",
mmc_hostname(host));
> Testing on XO-1.75, it didn't work: it thought that the 1.8V switch
> was successful so it left it at that, then the card reacted in a very
> unstable manner (failed/retried reads, huge amount of kernel log spam,
> etc).
This points to that the way of checking if the card is busy or not may
not work on XO-1.75? But then it seems strange that the card was
semi-usable...
> So I came up with the attached sdhci patch. That fixes the XO-1.75
> case, which now correctly detects the 1.8V switch failure and goes to
> 3.3V. However, that broke XO-4, which now just does:
> mmc2: Signal voltage switch failed, power cycling card (retries = 10)
> mmc2: error -110 whilst initialising SD card
> (no more time to debug exactly whats happening)
Ok, so failure is detected in mmc_set_signal_voltage by your card_busy
function, good. Then the card is power cycled, and should be
initialized again, but this fails, it's probably mmc_send_app_op_cond
which returns -110. Maybe the power cycle fails?
So the strange thing here is that on kernel 3.5, without my patch,
1.8V switch fails and fall-back to 3.3V fails. With my patch, 1.8V
fails and fall-back to 3.3V succeeds. But, when we move the detection
and error-handling to my patch (the upper layer), the fall-back fails.
I'm thinking of a couple of things that could have gone wrong here...
If you used v2 of the patch, is assumes that the signal voltage is
reset to 3.30 V in mmc_power_up, but this was introduced quite
recently, in v3.6 (mmc: core: reset signal voltage on power up), but
you used v1, right? So then there are at least two other differences:
1) The way the card is power cycled.
sdhci toggles the SDHCI_POWER_ON bit in SDHCI_POWER_CONTROL, but
my patch calls mmc_power_off / mmc_power_up.
Do you know if mmc_power_off / up is a good way to power-cycle the card?
2) The way the clock is stopped.
sdhci clears SDHCI_CLOCK_CARD_EN in SDHCI_CLOCK_CONTROL, my patch
sets ios.clock = 0 and calls mmc_set_ios.
But maybe this is not the issue, since the 1.8V switch fails in all cases.
I don't really know how to proceed. Do you have the complete dmesgs
from the 3.5 kernel: with my patch and without your patch + with my
patch and with your patch? With these I could try to do a deeper
analysis.
By the way, in your patch
0001-update-sdhci-for-voltage-changing-fixes.txt you don't check if
the signal voltage switch succeeds electrically:
ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
if (ctrl & SDHCI_CTRL_VDD_180) {
but maybe we can assume that this will be fine after the delay of 5 ms
in mmc_set_signal_voltage? Also in sdhci_card_busy you don't do
runtime_pm_get before you check the busy status, but since the busy
check returns busy, perhaps this is not the issue here either.
> All tests with the same 32GB SD storage card.
>
> I wonder if this difference in behaviour is related to the difference
> in kernel versions on each platform (3.0 vs 3.5). I would like to test
> with 3.5 or newer running on both, but this requires a bit of setup
> work for the XO-1.75 first (long story). And I'm out of time at this
> point :( this was a borrowed SD card.
Ok, I hope you'll be able to get time and SD-card in the future. :)
Btw, what brand was the SD-card?
Kind regards, Johan
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2012-10-24 10:56 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-09-20 18:42 [PATCH 2/2] mmc: core: Power cycle card on voltage switch fail Johan Rudholm
2012-10-23 20:39 ` Chris Ball
2012-10-23 22:48 ` Daniel Drake
2012-10-24 10:56 ` Johan Rudholm
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.