On 03/02/21 11:36 am, 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 ?
>
>
>
>>> + 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.
>
If there was a problem accessing the attachment. Here is a link to
download the document,
https://www.sdcard.org/downloads/pls/
Thanks,
Aswath
> 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
>>>
>>
>
