drivers/mmc/host/sdhci-sprd.c | 62 +++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+)
With cat regulator_summary, we found that vqmmc was not shutting
down after the card was pulled.
cat /sys/kernel/debug/regulator/regulator_summary
1.before fix
1)Insert SD card
vddsdio 1 1 0 unknown 3500mV 0mA 1200mV 3750mV
71100000.mmc-vqmmc 1 0mA 3500mV 3600mV
2)Pull out the SD card
vddsdio 1 1 0 unknown 3500mV 0mA 1200mV 3750mV
71100000.mmc-vqmmc 1 0mA 3500mV 3600mV
2.after fix
1)Insert SD cardt
vddsdio 1 1 0 unknown 3500mV 0mA 1200mV 3750mV
71100000.mmc-vqmmc 1 0mA 3500mV 3600mV
2)Pull out the SD card
vddsdio 0 1 0 unknown 3500mV 0mA 1200mV 3750mV
71100000.mmc-vqmmc 0 0mA 3500mV 3600mV
Fixes: fb8bd90f83c4 ("mmc: sdhci-sprd: Add Spreadtrum's initial host controller")
Signed-off-by: Wenchao Chen <wenchao.chen@unisoc.com>
---
drivers/mmc/host/sdhci-sprd.c | 62 +++++++++++++++++++++++++++++++++++
1 file changed, 62 insertions(+)
diff --git a/drivers/mmc/host/sdhci-sprd.c b/drivers/mmc/host/sdhci-sprd.c
index 6b84ba27e6ab..b5f22e48e571 100644
--- a/drivers/mmc/host/sdhci-sprd.c
+++ b/drivers/mmc/host/sdhci-sprd.c
@@ -90,6 +90,7 @@ struct sdhci_sprd_host {
u32 base_rate;
int flags; /* backup of host attribute */
u32 phy_delay[MMC_TIMING_MMC_HS400 + 2];
+ u8 power_mode;
};
enum sdhci_sprd_tuning_type {
@@ -416,12 +417,71 @@ static void sdhci_sprd_request_done(struct sdhci_host *host,
mmc_request_done(host->mmc, mrq);
}
+static void sdhci_sprd_signal_voltage_switch(struct sdhci_host *host, bool enable)
+{
+ const char *name = mmc_hostname(host->mmc);
+ bool on;
+
+ if (IS_ERR(host->mmc->supply.vqmmc))
+ return;
+
+ on = regulator_is_enabled(host->mmc->supply.vqmmc);
+ if (!(on ^ enable))
+ return;
+
+ if (enable) {
+ if (regulator_enable(host->mmc->supply.vqmmc))
+ pr_err("%s: signal voltage enable fail!\n", name);
+ else if (regulator_is_enabled(host->mmc->supply.vqmmc))
+ pr_debug("%s: signal voltage enable success!\n", name);
+ else
+ pr_err("%s: signal voltage enable hw fail!\n", name);
+ } else {
+ if (regulator_disable(host->mmc->supply.vqmmc))
+ pr_err("%s: signal voltage disable fail\n", name);
+ else if (!regulator_is_enabled(host->mmc->supply.vqmmc))
+ pr_debug("%s: signal voltage disable success!\n", name);
+ else
+ pr_err("%s: signal voltage disable hw fail\n", name);
+ }
+}
+
+static void sdhci_sprd_set_power(struct sdhci_host *host, unsigned char mode,
+ unsigned short vdd)
+{
+ struct sdhci_sprd_host *sprd_host = TO_SPRD_HOST(host);
+ struct mmc_host *mmc = host->mmc;
+
+ if (sprd_host->power_mode == mmc->ios.power_mode)
+ return;
+
+ switch (mode) {
+ case MMC_POWER_OFF:
+ sdhci_sprd_signal_voltage_switch(host, false);
+ if (!IS_ERR(mmc->supply.vmmc))
+ mmc_regulator_set_ocr(host->mmc, mmc->supply.vmmc, 0);
+ break;
+ case MMC_POWER_ON:
+ case MMC_POWER_UP:
+ if (!IS_ERR(mmc->supply.vmmc))
+ mmc_regulator_set_ocr(host->mmc, mmc->supply.vmmc, vdd);
+
+ /* waiting for mmc->supply.vmmc to stabilize */
+ usleep_range(200, 250);
+ sdhci_sprd_signal_voltage_switch(host, true);
+ break;
+ }
+
+ sprd_host->power_mode = mmc->ios.power_mode;
+}
+
static struct sdhci_ops sdhci_sprd_ops = {
.read_l = sdhci_sprd_readl,
.write_l = sdhci_sprd_writel,
.write_w = sdhci_sprd_writew,
.write_b = sdhci_sprd_writeb,
.set_clock = sdhci_sprd_set_clock,
+ .set_power = sdhci_sprd_set_power,
.get_max_clock = sdhci_sprd_get_max_clock,
.get_min_clock = sdhci_sprd_get_min_clock,
.set_bus_width = sdhci_set_bus_width,
@@ -805,6 +865,8 @@ static int sdhci_sprd_probe(struct platform_device *pdev)
sprd_host->version = ((host->version & SDHCI_VENDOR_VER_MASK) >>
SDHCI_VENDOR_VER_SHIFT);
+ sprd_host->power_mode = MMC_POWER_OFF;
+
pm_runtime_get_noresume(&pdev->dev);
pm_runtime_set_active(&pdev->dev);
pm_runtime_enable(&pdev->dev);
--
2.17.1On Thu, 9 Nov 2023 at 07:37, Wenchao Chen <wenchao.chen@unisoc.com> wrote:
>
> With cat regulator_summary, we found that vqmmc was not shutting
> down after the card was pulled.
>
> cat /sys/kernel/debug/regulator/regulator_summary
> 1.before fix
> 1)Insert SD card
> vddsdio 1 1 0 unknown 3500mV 0mA 1200mV 3750mV
> 71100000.mmc-vqmmc 1 0mA 3500mV 3600mV
>
> 2)Pull out the SD card
> vddsdio 1 1 0 unknown 3500mV 0mA 1200mV 3750mV
> 71100000.mmc-vqmmc 1 0mA 3500mV 3600mV
>
> 2.after fix
> 1)Insert SD cardt
> vddsdio 1 1 0 unknown 3500mV 0mA 1200mV 3750mV
> 71100000.mmc-vqmmc 1 0mA 3500mV 3600mV
>
> 2)Pull out the SD card
> vddsdio 0 1 0 unknown 3500mV 0mA 1200mV 3750mV
> 71100000.mmc-vqmmc 0 0mA 3500mV 3600mV
>
> Fixes: fb8bd90f83c4 ("mmc: sdhci-sprd: Add Spreadtrum's initial host controller")
> Signed-off-by: Wenchao Chen <wenchao.chen@unisoc.com>
> ---
> drivers/mmc/host/sdhci-sprd.c | 62 +++++++++++++++++++++++++++++++++++
> 1 file changed, 62 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-sprd.c b/drivers/mmc/host/sdhci-sprd.c
> index 6b84ba27e6ab..b5f22e48e571 100644
> --- a/drivers/mmc/host/sdhci-sprd.c
> +++ b/drivers/mmc/host/sdhci-sprd.c
> @@ -90,6 +90,7 @@ struct sdhci_sprd_host {
> u32 base_rate;
> int flags; /* backup of host attribute */
> u32 phy_delay[MMC_TIMING_MMC_HS400 + 2];
> + u8 power_mode;
> };
>
> enum sdhci_sprd_tuning_type {
> @@ -416,12 +417,71 @@ static void sdhci_sprd_request_done(struct sdhci_host *host,
> mmc_request_done(host->mmc, mrq);
> }
>
> +static void sdhci_sprd_signal_voltage_switch(struct sdhci_host *host, bool enable)
> +{
> + const char *name = mmc_hostname(host->mmc);
> + bool on;
> +
> + if (IS_ERR(host->mmc->supply.vqmmc))
> + return;
> +
> + on = regulator_is_enabled(host->mmc->supply.vqmmc);
> + if (!(on ^ enable))
> + return;
> +
> + if (enable) {
> + if (regulator_enable(host->mmc->supply.vqmmc))
> + pr_err("%s: signal voltage enable fail!\n", name);
> + else if (regulator_is_enabled(host->mmc->supply.vqmmc))
> + pr_debug("%s: signal voltage enable success!\n", name);
> + else
> + pr_err("%s: signal voltage enable hw fail!\n", name);
> + } else {
> + if (regulator_disable(host->mmc->supply.vqmmc))
> + pr_err("%s: signal voltage disable fail\n", name);
> + else if (!regulator_is_enabled(host->mmc->supply.vqmmc))
> + pr_debug("%s: signal voltage disable success!\n", name);
> + else
> + pr_err("%s: signal voltage disable hw fail\n", name);
> + }
This looks rather noisy to me.
How about converting into using mmc_regulator_enable|disable_vqmmc()
instead - and then simply drop the entire function above.
> +}
> +
> +static void sdhci_sprd_set_power(struct sdhci_host *host, unsigned char mode,
> + unsigned short vdd)
> +{
> + struct sdhci_sprd_host *sprd_host = TO_SPRD_HOST(host);
> + struct mmc_host *mmc = host->mmc;
> +
> + if (sprd_host->power_mode == mmc->ios.power_mode)
> + return;
> +
> + switch (mode) {
> + case MMC_POWER_OFF:
> + sdhci_sprd_signal_voltage_switch(host, false);
> + if (!IS_ERR(mmc->supply.vmmc))
> + mmc_regulator_set_ocr(host->mmc, mmc->supply.vmmc, 0);
> + break;
> + case MMC_POWER_ON:
> + case MMC_POWER_UP:
This means that a usleep_range() below is being executed for both
MMC_POWER_ON and MMC_POWER_UP, even if we actually didn't change the
regulators. I assume that isn't needed and nor the intent, right?
> + if (!IS_ERR(mmc->supply.vmmc))
> + mmc_regulator_set_ocr(host->mmc, mmc->supply.vmmc, vdd);
> +
> + /* waiting for mmc->supply.vmmc to stabilize */
> + usleep_range(200, 250);
> + sdhci_sprd_signal_voltage_switch(host, true);
> + break;
> + }
> +
> + sprd_host->power_mode = mmc->ios.power_mode;
If we convert to use the helper functions
mmc_regulator_enable|disable_vqmmc(), you shouldn't need to use
sprd_host->power_mode, as that is already taken care of by the
helpers.
> +}
> +
> static struct sdhci_ops sdhci_sprd_ops = {
> .read_l = sdhci_sprd_readl,
> .write_l = sdhci_sprd_writel,
> .write_w = sdhci_sprd_writew,
> .write_b = sdhci_sprd_writeb,
> .set_clock = sdhci_sprd_set_clock,
> + .set_power = sdhci_sprd_set_power,
> .get_max_clock = sdhci_sprd_get_max_clock,
> .get_min_clock = sdhci_sprd_get_min_clock,
> .set_bus_width = sdhci_set_bus_width,
> @@ -805,6 +865,8 @@ static int sdhci_sprd_probe(struct platform_device *pdev)
> sprd_host->version = ((host->version & SDHCI_VENDOR_VER_MASK) >>
> SDHCI_VENDOR_VER_SHIFT);
>
> + sprd_host->power_mode = MMC_POWER_OFF;
> +
> pm_runtime_get_noresume(&pdev->dev);
> pm_runtime_set_active(&pdev->dev);
> pm_runtime_enable(&pdev->dev);
Kind regards
Uffe
© 2016 - 2025 Red Hat, Inc.