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_ab...@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_ab...@ti.com> >>>> Signed-off-by: Aswath Govindraju <a-govindr...@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 >>>> >>> >> >