[PATCH v2] mmc: sdhci: wait 5ms after set 1.8V signal enable

[PATCH v2] mmc: sdhci: wait 5ms after set 1.8V signal enable

[Zhoujie Wu]

According to SD spec 3.00 3.6.1 signal voltage switch
procedure, host should wait 5ms after set 1.8V signal
enable bit in Host Control 2 register and check if 1.8V
is stable or not.

On some cards, found without the delay would see "1.8V
regulator output did not became stable" err log.

Signed-off-by: Zhoujie Wu <zjwu@marvell.com>
---
v2: removed undeliverable cc email address.

 drivers/mmc/host/sdhci.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index e9290a3..fe5f208 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1933,6 +1933,9 @@ int sdhci_start_signal_voltage_switch(struct mmc_host *mmc,
 		if (host->ops->voltage_switch)
 			host->ops->voltage_switch(host);
 
+		/* Wait for 5ms */
+		usleep_range(5000, 5500);
+
 		/* 1.8V regulator output should be stable within 5 ms */
 		ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
 		if (ctrl & SDHCI_CTRL_VDD_180)
-- 
1.9.1

Re: [PATCH v2] mmc: sdhci: wait 5ms after set 1.8V signal enable

[Ulf Hansson]

On 14 December 2017 at 00:48, Zhoujie Wu <zjwu@marvell.com> wrote:
> According to SD spec 3.00 3.6.1 signal voltage switch
> procedure, host should wait 5ms after set 1.8V signal
> enable bit in Host Control 2 register and check if 1.8V
> is stable or not.
>

The core already deals with this. See mmc_set_uhs_voltage() and
mmc_host_set_uhs_voltage().

We even have a retry mechanism in place.

> On some cards, found without the delay would see "1.8V
> regulator output did not became stable" err log.

Seems like you need to fix the regulator instead.

BTW, is this a regression? No?

[...]

Kind regards
Uffe

Re: [EXT] Re: [PATCH v2] mmc: sdhci: wait 5ms after set 1.8V signal enable

[Zhoujie Wu]

Hi Ulf,

On 12/14/2017 12:41 AM, Ulf Hansson wrote:
> External Email
>
> ----------------------------------------------------------------------
> On 14 December 2017 at 00:48, Zhoujie Wu <zjwu@marvell.com> wrote:
>> According to SD spec 3.00 3.6.1 signal voltage switch
>> procedure, host should wait 5ms after set 1.8V signal
>> enable bit in Host Control 2 register and check if 1.8V
>> is stable or not.
>>
> The core already deals with this. See mmc_set_uhs_voltage() and
> mmc_host_set_uhs_voltage().
>
> We even have a retry mechanism in place.
I checked these functions, they just make sure when do signal voltage 
switch, gate the clock to SD card 5ms and wait 1ms before check data lines.
This patch is following below spec requirement when switching signal 
voltage to 1.8V, wait 5ms after set 1.8V signal Enable bit in Host 
control 2 register.
Current code will set 1.8V Signal Enable bit and check if this bit is 
cleared by host right after that,  the bit is not cleared and host 
report "1.8V regulator output did not became stable".

You can find the steps at SD spec 3.0 3.6.1.
(6) Set 1.8V Signal Enable in the Host Control 2 register.
(7) Wait 5ms. 1.8V voltage regulator shall be stable within this period.
(8) If 1.8V Signal Enable is cleared by Host Controller, go to step (12).
>
>> On some cards, found without the delay would see "1.8V
>> regulator output did not became stable" err log.
> Seems like you need to fix the regulator instead.
>
> BTW, is this a regression? No?
It is not regulator related, the regulator already output 1.8V and add 
delay after regulator set voltage didn't help.
The only solution is to add 5ms after set 1.8V signal enable bit .
Not a regression issue, we recently found it when enabling uhs on some 
platforms since they didn't enable it previously.
>
> [...]
>
> Kind regards
> Uffe

Re: [EXT] Re: [PATCH v2] mmc: sdhci: wait 5ms after set 1.8V signal enable

[Ulf Hansson]

On 14 December 2017 at 20:04, Zhoujie Wu <zjwu@marvell.com> wrote:
> Hi Ulf,
>
> On 12/14/2017 12:41 AM, Ulf Hansson wrote:
>>
>> External Email
>>
>> ----------------------------------------------------------------------
>> On 14 December 2017 at 00:48, Zhoujie Wu <zjwu@marvell.com> wrote:
>>>
>>> According to SD spec 3.00 3.6.1 signal voltage switch
>>> procedure, host should wait 5ms after set 1.8V signal
>>> enable bit in Host Control 2 register and check if 1.8V
>>> is stable or not.
>>>
>> The core already deals with this. See mmc_set_uhs_voltage() and
>> mmc_host_set_uhs_voltage().
>>
>> We even have a retry mechanism in place.
>
> I checked these functions, they just make sure when do signal voltage
> switch, gate the clock to SD card 5ms and wait 1ms before check data lines.
> This patch is following below spec requirement when switching signal voltage
> to 1.8V, wait 5ms after set 1.8V signal Enable bit in Host control 2
> register.

Right, so this is an SDHCI controller requirement. Could you perhaps
clarify that in the changlog, then I  am happy.

> Current code will set 1.8V Signal Enable bit and check if this bit is
> cleared by host right after that,  the bit is not cleared and host report
> "1.8V regulator output did not became stable".
>
> You can find the steps at SD spec 3.0 3.6.1.
> (6) Set 1.8V Signal Enable in the Host Control 2 register.
> (7) Wait 5ms. 1.8V voltage regulator shall be stable within this period.
> (8) If 1.8V Signal Enable is cleared by Host Controller, go to step (12).
>>
>>
>>> On some cards, found without the delay would see "1.8V
>>> regulator output did not became stable" err log.
>>
>> Seems like you need to fix the regulator instead.
>>
>> BTW, is this a regression? No?
>
> It is not regulator related, the regulator already output 1.8V and add delay
> after regulator set voltage didn't help.
> The only solution is to add 5ms after set 1.8V signal enable bit .
> Not a regression issue, we recently found it when enabling uhs on some
> platforms since they didn't enable it previously.

I see, thanks for clarifying.

Kind regards
Uffe

Re: [EXT] Re: [PATCH v2] mmc: sdhci: wait 5ms after set 1.8V signal enable

[Zhoujie Wu]

On 12/14/2017 11:05 PM, Ulf Hansson wrote:
> On 14 December 2017 at 20:04, Zhoujie Wu <zjwu@marvell.com> wrote:
>> Hi Ulf,
>>
>> On 12/14/2017 12:41 AM, Ulf Hansson wrote:
>>> External Email
>>>
>>> ----------------------------------------------------------------------
>>> On 14 December 2017 at 00:48, Zhoujie Wu <zjwu@marvell.com> wrote:
>>>> According to SD spec 3.00 3.6.1 signal voltage switch
>>>> procedure, host should wait 5ms after set 1.8V signal
>>>> enable bit in Host Control 2 register and check if 1.8V
>>>> is stable or not.
>>>>
>>> The core already deals with this. See mmc_set_uhs_voltage() and
>>> mmc_host_set_uhs_voltage().
>>>
>>> We even have a retry mechanism in place.
>> I checked these functions, they just make sure when do signal voltage
>> switch, gate the clock to SD card 5ms and wait 1ms before check data lines.
>> This patch is following below spec requirement when switching signal voltage
>> to 1.8V, wait 5ms after set 1.8V signal Enable bit in Host control 2
>> register.
> Right, so this is an SDHCI controller requirement. Could you perhaps
> clarify that in the changlog, then I  am happy.
Sure, I will give more details in comment to avoid confusing:)
>> Current code will set 1.8V Signal Enable bit and check if this bit is
>> cleared by host right after that,  the bit is not cleared and host report
>> "1.8V regulator output did not became stable".
>>
>> You can find the steps at SD spec 3.0 3.6.1.
>> (6) Set 1.8V Signal Enable in the Host Control 2 register.
>> (7) Wait 5ms. 1.8V voltage regulator shall be stable within this period.
>> (8) If 1.8V Signal Enable is cleared by Host Controller, go to step (12).
>>>
>>>> On some cards, found without the delay would see "1.8V
>>>> regulator output did not became stable" err log.
>>> Seems like you need to fix the regulator instead.
>>>
>>> BTW, is this a regression? No?
>> It is not regulator related, the regulator already output 1.8V and add delay
>> after regulator set voltage didn't help.
>> The only solution is to add 5ms after set 1.8V signal enable bit .
>> Not a regression issue, we recently found it when enabling uhs on some
>> platforms since they didn't enable it previously.
> I see, thanks for clarifying.
>
> Kind regards
> Uffe