From mboxrd@z Thu Jan  1 00:00:00 1970
From: Simon Glass <sjg@chromium.org>
Date: Sat, 23 Jan 2021 19:03:20 -0700
Subject: [PATCH v3 01/20] mmc: sdhci: Add helper functions for UHS modes
In-Reply-To: <20210121124052.3454-2-a-govindraju@ti.com>
References: <20210121124052.3454-1-a-govindraju@ti.com>
 <20210121124052.3454-2-a-govindraju@ti.com>
Message-ID: <CAPnjgZ08VKW7+9Sm+UEnDRrHF5ponSogzOud1jr7KJNy=eWcXw@mail.gmail.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Hi Aswath,

On Thu, 21 Jan 2021 at 05:41, Aswath Govindraju <a-govindraju@ti.com> wrote:
>
> From: Faiz Abbas <faiz_abbas@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@ti.com>
> Signed-off-by: Aswath Govindraju <a-govindraju@ti.com>
> ---
>  drivers/mmc/sdhci.c | 80 +++++++++++++++++++++++++++++++++++++++++++++
>  include/sdhci.h     |  1 +
>  2 files changed, 81 insertions(+)
>
> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
> index 06289343124e..e7e3e22d1a7e 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,85 @@ void sdhci_set_uhs_timing(struct sdhci_host *host)
>         sdhci_writew(host, reg, SDHCI_HOST_CONTROL2);
>  }
>
> +#if CONFIG_IS_ENABLED(MMC_IO_VOLTAGE)
> +static void sdhci_set_voltage(struct sdhci_host *host)
> +{
> +       struct mmc *mmc = (struct mmc *)host->mmc;
> +       u32 ctrl;
> +       int ret;
> +
> +       ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
> +
> +       switch (mmc->signal_voltage) {
> +       case MMC_SIGNAL_VOLTAGE_330:
> +#if CONFIG_IS_ENABLED(DM_REGULATOR)

Can you use if(), please? You should see a patman warning about this.

> +               if (mmc->vqmmc_supply) {
> +                       ret = regulator_set_enable(mmc->vqmmc_supply, false);
> +                       if (ret) {
> +                               pr_err("failed to disable vqmmc-supply: %d\n", ret);
> +                               return;
> +                       }
> +
> +                       ret = regulator_set_value(mmc->vqmmc_supply, 3300000);
> +                       if (ret) {
> +                               pr_err("failed to set vqmmc-voltage to 3.3V: %d\n", ret);
> +                               return;
> +                       }
> +
> +                       ret = regulator_set_enable(mmc->vqmmc_supply, true);
> +                       if (ret) {
> +                               pr_err("failed to enable vqmmc-supply: %d\n", ret);
> +                               return;
> +                       }
> +               }
> +#endif
> +               mdelay(5);
> +               if (IS_SD(mmc)) {
> +                       ctrl &= ~SDHCI_CTRL_VDD_180;
> +                       sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2);
> +               }
> +               break;
> +       case MMC_SIGNAL_VOLTAGE_180:
> +#if CONFIG_IS_ENABLED(DM_REGULATOR)
> +               if (mmc->vqmmc_supply) {
> +                       regulator_set_enable(mmc->vqmmc_supply, false);
> +                       if (ret) {
> +                               pr_err("failed to disable vqmmc-supply: %d\n", ret);
> +                               return;
> +                       }
> +
> +                       regulator_set_value(mmc->vqmmc_supply, 1800000);
> +                       if (ret) {
> +                               pr_err("failed to set vqmmc-voltage to 1.8V: %d\n", ret);
> +                               return;
> +                       }
> +
> +                       regulator_set_enable(mmc->vqmmc_supply, true);
> +                       if (ret) {
> +                               pr_err("failed to enable vqmmc-supply: %d\n", ret);
> +                               return;
> +                       }
> +               }
> +#endif
> +               if (IS_SD(mmc)) {
> +                       ctrl |= SDHCI_CTRL_VDD_180;
> +                       sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2);
> +               }
> +               break;
> +       default:
> +               /* No signal voltage switch required */
> +               return;
> +       }
> +}
> +#else
> +static void sdhci_set_voltage(struct sdhci_host *host) { }
> +#endif
> +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..0f820c6d2669 100644
> --- a/include/sdhci.h
> +++ b/include/sdhci.h
> @@ -491,6 +491,7 @@ 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);
> +void sdhci_set_control_reg(struct sdhci_host *host);

This needs a comment. What does it do?

>  extern const struct dm_mmc_ops sdhci_ops;
>  #else
>  #endif
> --
> 2.17.1
>

Regards,
Simon