lkml.org 
[lkml]   [2014]   [Nov]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: Possible regression with commit 52221610d
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@nvidia.com> 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.



\
 
 \ /
  Last update: 2014-11-04 10:21    [W:0.101 / U:0.600 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site