Hi Tim, thanks for your reply!
On 11/04/2014 02:28 PM, Tim Kryger wrote:
On Mon, Nov 3, 2014 at 7:05 PM, Alexandre Courbot <acourbot@xxxxxxxxxx> wrote:
Hi guys,
On the NVIDIA shield (tegra114-roth) platform, I have noticed that MMC
stopped working completely on recent kernels. MMC devices will not show up
and the message "mmc1: Controller never released inhibit bit(s)." shows up
repeatedly in the console.
After bisecting I tracked commit 52221610dd84dc3e9196554f0292ca9e8ab3541d
("mmc: sdhci: Improve external VDD regulator support") as the one that
introduced this issue, which seems somehow surprising to me since it has
been around for a while and nobody else complained about this AFAICT.
I'm not too familiar with the Nvidia Shield so can you please confirm
the following?
The controller in the Tegra114 is SDHCI compliant and as such
sdhci_tegra_probe calls sdhci_add_host. External regulators are
sought in sdhci_add_host with a call to mmc_regulator_get_supply.
This is correct.
Since no external regulators are specified in tegra114.dtsi or
tegra114-roth.dts, mmc->supply.vmmc and mmc->supply.vqmmc are set to
-ENODEV.
Actually 2 of the MMC nodes in tegra114-roth.dts (for SD card and eMMC)
have a vmmc-supply property, so for two of them at least
mmc->supply.vmmc is a valid pointer.
The following diff solves the issue for me, however I don't know whether it
also reverts the intended purpose of the initial patch:
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index ada1a3ea3a87..615701bb8ea3 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1235,13 +1235,6 @@ static void sdhci_set_power(struct sdhci_host *host,
unsigned char mode,
struct mmc_host *mmc = host->mmc;
u8 pwr = 0;
- if (!IS_ERR(mmc->supply.vmmc)) {
- spin_unlock_irq(&host->lock);
- mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd);
- spin_lock_irq(&host->lock);
- return;
- }
-
if (mode != MMC_POWER_OFF) {
switch (1 << vdd) {
case MMC_VDD_165_195:
@@ -1300,6 +1293,12 @@ static void sdhci_set_power(struct sdhci_host *host,
unsigned char mode,
if (host->quirks & SDHCI_QUIRK_DELAY_AFTER_POWER)
mdelay(10);
}
+
+ if (!IS_ERR(mmc->supply.vmmc)) {
+ spin_unlock_irq(&host->lock);
+ mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd);
+ spin_lock_irq(&host->lock);
+ }
}
Does this look like the right approach? If not, would you have any
suggestion as to how to solve this problem?
The patch you proposed would break Exynos4210 so I don't think it is
appropriate.
Do you understand why this code block is executed on your hardware? I
wouldn't expect it.
As explained above, vmmc is a valid pointer for 2 instances of the MMC
controller. Interestingly, if I just remove the "return" line in the
IS_ERR() block (without moving it around), the issue also seems to be fixed.
Can you provide the relevant parts of the log before the problem occurs?
There is not much unfortunately ; the only relevant log I have is this:
[ 12.246022] mmc2: Timeout waiting for hardware interrupt.
[ 12.264990] mmc2: Controller never released inhibit bit(s).
Some hardware interrupt timed out. I don't know much about the MMC
subsystem. but could it be because initially the controller is not in a
powered-on state, and that return statement causes the function to leave
it unpowered?
Thanks,
Alex.
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
