From: adrian.hunter@intel•com (Adrian Hunter)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH] ARM: tegra: beaver: allow SD card voltage to be changed
Date: Tue, 14 Jun 2016 09:20:14 +0300 [thread overview]
Message-ID: <575FA21E.80309@intel.com> (raw)
In-Reply-To: <575E897A.5080508@nvidia.com>
On 13/06/16 13:22, Jon Hunter wrote:
> Adding Adrian and Ulf ...
>
> On 19/05/16 15:29, Jon Hunter wrote:
>>
>> On 13/05/16 18:27, Thierry Reding wrote:
>>> * PGP Signed by an unknown key
>>>
>>> On Fri, May 13, 2016 at 09:25:31AM +0200, Lucas Stach wrote:
>>>> Am Montag, den 29.02.2016, 22:01 +0100 schrieb Lucas Stach:
>>>>> This allows to switch the card signal voltage level to 1.8V,
>>>>> which is needed for any ultra high speed modes to work.
>>>>>
>>>>> Signed-off-by: Lucas Stach <dev@lynxeye•de>
>>>>> ---
>>>>> This needs the SDMMC memcomp pad calibration patches I just
>>>>> sent out to be applied, otherwise the card voltage change will
>>>>> fail with a message in the kernel log and a fall back to
>>>>> high speed operation.
>>>>
>>>> The patches this one depends on have been applied for some time now.
>>>> Please pick up this patch.
>>>
>>> My understanding is that UHS modes currently cause problems on Beaver.
>>> What I don't understand about that is how it will even try those modes
>>> if the voltage regulator can't be set to 1.8 V? Shouldn't that actively
>>> prevent those modes from even being attempted?
>>
>> Looking at the sdhci code, if the regulator is missing then we still
>> attempt to place the controller is 1.8V mode ...
>>
>> static int sdhci_start_signal_voltage_switch(struct mmc_host *mmc,
>> struct mmc_ios *ios)
>> {
>>
>> ...
>>
>> case MMC_SIGNAL_VOLTAGE_180:
>> if (!IS_ERR(mmc->supply.vqmmc)) {
>> ret = regulator_set_voltage(mmc->supply.vqmmc,
>> 1700000, 1950000);
>> if (ret) {
>> pr_warn("%s: Switching to 1.8V signalling voltage failed\n",
>> mmc_hostname(mmc));
>> return -EIO;
>> }
>> }
>>
>> /*
>> * Enable 1.8V Signal Enable in the Host Control2
>> * register
>> */
>> ctrl |= SDHCI_CTRL_VDD_180;
>> sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2);
>>
>> /* Some controller need to do more when switching */
>> if (host->ops->voltage_switch)
>> host->ops->voltage_switch(host);
>>
>> /* 1.8V regulator output should be stable within 5 ms */
>> ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
>> if (ctrl & SDHCI_CTRL_VDD_180)
>> return 0;
>>
>> pr_warn("%s: 1.8V regulator output did not became stable\n",
>> mmc_hostname(mmc));
>>
>> return -EAGAIN;
>>
>> Ideally, the above *should* fail if the regulator is missing. However, what
>> I have found, is that in my case, even though the regulator is missing, the
>> above succeeds and the host thinks we are operating at 1.8V even though we
>> are still at 3.3V! It seems that this does not happen with all SD cards that
>> support UHS.
>>
>> This patch resolves the problems I am seeing on beaver with SD card
>> initialisation failing. I am surprised this is not causing problems for
>> others?
>
> Adrian, Ulf, per the above, I have found that on a Tegra30 beaver board,
> if we enable UHS-I modes for Tegra30 but the device-tree for the board
> is missing the regulator to select 1.8V mode operation, then the above
> code sequence may still return success (ie. SDHCI_CTRL_VDD_180 bit is
> set in SDHCI_HOST_CONTROL2) even though we have not changed the voltage.
> This leads to other problems later on during SD initialisation.
>
> Would you expect that an SDHCI controller should fail to set the
> SDHCI_CTRL_VDD_180 bit in the SDHCI_HOST_CONTROL2 register if we did not
> change the voltage?
What is meant to happen is that sdhci should wait 5ms and then check
SDHCI_CTRL_VDD_180 - which it used to do but then someone took the 5ms wait
away.
In any case, if you are using a regulator there is no knowing what sdhci is
meant to do.
>
> We want to ensure that Tegra devices do not attempt to switch the UHS-I
> modes if the regulator is not present and it is not clear to me if there
> is a problem with the Tegra SDHCI controller or how this should be handled.
If the driver doesn't support UHS-I modes then it must remove the cap flags.
next prev parent reply other threads:[~2016-06-14 6:20 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-02-29 21:01 [PATCH] ARM: tegra: beaver: allow SD card voltage to be changed Lucas Stach
2016-05-13 7:25 ` Lucas Stach
2016-05-13 17:27 ` Thierry Reding
2016-05-13 19:08 ` Lucas Stach
2016-05-19 14:29 ` Jon Hunter
2016-06-13 10:22 ` Jon Hunter
2016-06-14 6:20 ` Adrian Hunter [this message]
2016-06-14 8:23 ` Jon Hunter
2016-06-14 10:05 ` Adrian Hunter
2016-06-14 14:19 ` Jon Hunter
2016-06-24 9:14 ` Jon Hunter
2016-06-28 13:27 ` Adrian Hunter
2016-05-19 14:31 ` Jon Hunter
2016-05-19 16:12 ` Stephen Warren
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=575FA21E.80309@intel.com \
--to=adrian.hunter@intel$(echo .)com \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox