| Message ID | CAMLZHHRdz-==wgqvkoest61eaAdSHWTA0x-JTBgLWdO3B2gKJQ@mail.gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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,