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

[Aswath Govindraju]

Hi Jaehoon,

On 04/02/21 4:23 am, Jaehoon Chung wrote:
> 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?
> 

Voltage switching is not allowed for eMMC and to ensure it, the above
mentioned check is done. eMMC operates at the same voltage from the start.

Thanks,
Aswath

>>
>>>> +			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
>>>>
>>>
>>
>