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

[PATCH v3] 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 step 6~8,
(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).

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.

But current code checks if this bit is cleared by controller
right after set it. On some platforms found the bit is not
cleared right away and host reports "1.8V regulator output
did not became stable" and 5ms delay can help.

Follow the spec and add 5ms delay to make sure the 1.8V Signal Enable
bit is cleared.

Signed-off-by: Zhoujie Wu <zjwu@marvell.com>
---
 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 v3] mmc: sdhci: wait 5ms after set 1.8V signal enable

[Adrian Hunter]

On 15/12/17 21:28, Zhoujie Wu wrote:
> According to SD spec 3.00 3.6.1 signal voltage switch
> procedure step 6~8,
> (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).
> 
> 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.
> 
> But current code checks if this bit is cleared by controller
> right after set it. On some platforms found the bit is not
> cleared right away and host reports "1.8V regulator output
> did not became stable" and 5ms delay can help.

The check is for the bit set not cleared.

> 
> Follow the spec and add 5ms delay to make sure the 1.8V Signal Enable
> bit is cleared.
> 
> Signed-off-by: Zhoujie Wu <zjwu@marvell.com>
> ---
>  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);

We have managed without this delay for a while and I am reluctant to add
delays that might anyway be specific to the controller.  What do you think
about implementing ->voltage_switch() and putting the delay there?

> +
>  		/* 1.8V regulator output should be stable within 5 ms */
>  		ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
>  		if (ctrl & SDHCI_CTRL_VDD_180)
> 

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

[Zhoujie Wu]

On 12/18/2017 05:19 AM, Adrian Hunter wrote:
> External Email
>
> ----------------------------------------------------------------------
> On 15/12/17 21:28, Zhoujie Wu wrote:
>> According to SD spec 3.00 3.6.1 signal voltage switch
>> procedure step 6~8,
>> (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).
>>
>> 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.
>>
>> But current code checks if this bit is cleared by controller
>> right after set it. On some platforms found the bit is not
>> cleared right away and host reports "1.8V regulator output
>> did not became stable" and 5ms delay can help.
> The check is for the bit set not cleared.
I will correct the comments.
>> Follow the spec and add 5ms delay to make sure the 1.8V Signal Enable
>> bit is cleared.
>>
>> Signed-off-by: Zhoujie Wu <zjwu@marvell.com>
>> ---
>>   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);
> We have managed without this delay for a while and I am reluctant to add
> delays that might anyway be specific to the controller.  What do you think
> about implementing ->voltage_switch() and putting the delay there?
Ok to me, will send out the updated version. Thanks.
>
>> +
>>   		/* 1.8V regulator output should be stable within 5 ms */
>>   		ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
>>   		if (ctrl & SDHCI_CTRL_VDD_180)
>>