[PATCH v5 01/20] mmc: sdhci: Add helper functions for UHS modes

[Jaehoon Chung]

Hi Aswath,

On 2/3/21 3:06 PM, Aswath Govindraju wrote:
> Hi Jaehoon,
> 
> On 02/02/21 3:40 am, Jaehoon Chung wrote:
>> Hi Aswath,
>>
>> On 1/29/21 11:47 PM, Aswath Govindraju wrote:
>>> From: Faiz Abbas <faiz_abbas at ti.com>
>>>
>>> Add a set_voltage() function which handles the switch from 3.3V to 1.8V
>>> for SD card UHS modes.
>>>
>>> Signed-off-by: Faiz Abbas <faiz_abbas at ti.com>
>>> Signed-off-by: Aswath Govindraju <a-govindraju at ti.com>
>>> ---
>>>  drivers/mmc/sdhci.c | 73 +++++++++++++++++++++++++++++++++++++++++++++
>>>  include/sdhci.h     | 10 +++++++
>>>  2 files changed, 83 insertions(+)
>>>
>>> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
>>> index 06289343124e..276b6a08e571 100644
>>> --- a/drivers/mmc/sdhci.c
>>> +++ b/drivers/mmc/sdhci.c
>>> @@ -20,6 +20,7 @@
>>>  #include <linux/delay.h>
>>>  #include <linux/dma-mapping.h>
>>>  #include <phys2bus.h>
>>> +#include <power/regulator.h>
>>>  
>>>  static void sdhci_reset(struct sdhci_host *host, u8 mask)
>>>  {
>>> @@ -509,6 +510,78 @@ void sdhci_set_uhs_timing(struct sdhci_host *host)
>>>  	sdhci_writew(host, reg, SDHCI_HOST_CONTROL2);
>>>  }
>>>  
>>> +static void sdhci_set_voltage(struct sdhci_host *host)
>>> +{
>>> +	if (IS_ENABLED(CONFIG_MMC_IO_VOLTAGE)) {
>>> +		struct mmc *mmc = (struct mmc *)host->mmc;
>>> +		u32 ctrl;
>>> +
>>> +		ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
>>> +
>>> +		switch (mmc->signal_voltage) {
>>> +		case MMC_SIGNAL_VOLTAGE_330:
>>> +#if CONFIG_IS_ENABLED(DM_REGULATOR)
>>> +			if (mmc->vqmmc_supply) {
>>> +				if (regulator_set_enable_if_allowed(mmc->vqmmc_supply, false)) {
>>> +					pr_err("failed to disable vqmmc-supply\n");
>>> +					return;
>>> +				}
>>> +
>>> +				if (regulator_set_value(mmc->vqmmc_supply, 3300000)) {
>>> +					pr_err("failed to set vqmmc-voltage to 3.3V\n");
>>> +					return;
>>> +				}
>>> +
>>> +				if (regulator_set_enable_if_allowed(mmc->vqmmc_supply, true)) {
>>> +					pr_err("failed to enable vqmmc-supply\n");
>>> +					return;
>>> +				}
>>> +			}
>>> +#endif
>>> +			/* 3.3V regulator output should be stable within 5 ms */
>>> +			mdelay(5);
>>> +			if (IS_SD(mmc)) {
>>> +				ctrl &= ~SDHCI_CTRL_VDD_180;
>>> +				sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2);
>>> +			}
>>
>> According to Specification, after Signal bit was changed to 0 from 1, it needs to wait for stable within 5ms.
>> Isn't it right that mdelay(5) locates at here?
>>
> 
> I searched the spec for this and found the following line in it,
> 
> ```
> If this status is not supported, Host Driver should take more than 5ms for
> stable time of host voltage regulator from changing 1.8V Signaling
> Enable. Specific Host Driver may use a specific time, which is provided by
> Host System, instead of using 5ms.
> 
> ```
> 
> This is from the row of "Host Regulator Voltage Stable" in table
> describing "Present state register", "PartA2_SD
> Host_Controller_Simplified_Specification_Ver4.20" document, page 66.
> 
> The status mentioned is about host regulator voltage stable. Yes I can
> see that it has been mentioned that host driver has to wait for 5ms
> after changing the 1.8V signal enable bit. In that case I suppose the
> same has to be done at [1] as well right ?

Right, i think that it needs to be stable within 5ms.

One more, does it needs to check whether card is SD or not?
I think that it doesn't need to check it. Because eMMC needs to switch 1.8V when its mode is upper than DDR, doesn't it?

Best Regards,
Jaehoon Chung

> 
> 
> 
>>> +			break;
>>> +		case MMC_SIGNAL_VOLTAGE_180:
>>> +#if CONFIG_IS_ENABLED(DM_REGULATOR)
>>> +			if (mmc->vqmmc_supply) {
>>> +				if (regulator_set_enable_if_allowed(mmc->vqmmc_supply, false)) {
>>> +					pr_err("failed to disable vqmmc-supply\n");
>>> +					return;
>>> +				}
>>> +
>>> +				if (regulator_set_value(mmc->vqmmc_supply, 1800000)) {
>>> +					pr_err("failed to set vqmmc-voltage to 1.8V\n");
>>> +					return;
>>> +				}
>>> +
>>> +				if (regulator_set_enable_if_allowed(mmc->vqmmc_supply, true)) {
>>> +					pr_err("failed to enable vqmmc-supply\n");
>>> +					return;
>>> +				}
>>> +			}
>>> +#endif
>>> +			if (IS_SD(mmc)) {
>>> +				ctrl |= SDHCI_CTRL_VDD_180;
>>> +				sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2);
>>> +			}
> 
> [1]
> 
> For reference, I have also attached the page from where the above line
> was taken.
> 
> Thank you for pointing it out. I will post a re-spin fixing it.
> 
> Thanks,
> Aswath
> 
> 
>>> +			break;
>>> +		default:
>>> +			/* No signal voltage switch required */
>>> +			return;
>>> +		}
>>> +	}
>>> +}
>>> +
>>> +void sdhci_set_control_reg(struct sdhci_host *host)
>>> +{
>>> +	sdhci_set_voltage(host);
>>> +	sdhci_set_uhs_timing(host);
>>> +}
>>> +
>>>  #ifdef CONFIG_DM_MMC
>>>  static int sdhci_set_ios(struct udevice *dev)
>>>  {
>>> diff --git a/include/sdhci.h b/include/sdhci.h
>>> index 3e5a64981857..0ae9471ad749 100644
>>> --- a/include/sdhci.h
>>> +++ b/include/sdhci.h
>>> @@ -491,6 +491,16 @@ void sdhci_set_uhs_timing(struct sdhci_host *host);
>>>  /* Export the operations to drivers */
>>>  int sdhci_probe(struct udevice *dev);
>>>  int sdhci_set_clock(struct mmc *mmc, unsigned int clock);
>>> +
>>> +/**
>>> + * sdhci_set_control_reg - Set control registers
>>> + *
>>> + * This is used set up control registers for voltage level and UHS speed
>>> + * mode.
>>> + *
>>> + * @host: SDHCI host structure
>>> + */
>>> +void sdhci_set_control_reg(struct sdhci_host *host);
>>>  extern const struct dm_mmc_ops sdhci_ops;
>>>  #else
>>>  #endif
>>>
>>
>