* [PATCH v2] mmc: sdhci-of-dwcmshc: add reset control support in suspend/resume
@ 2026-05-05 13:37 Artem Shimko
2026-05-05 14:05 ` Adrian Hunter
0 siblings, 1 reply; 3+ messages in thread
From: Artem Shimko @ 2026-05-05 13:37 UTC (permalink / raw)
To: Jisheng.Zhang, hehuan1, andriy.shevchenko, yifeng.zhao,
Adrian Hunter, Ulf Hansson, Philipp Zabel
Cc: Artem Shimko, linux-mmc, linux-kernel
The driver currently handles reset control only during initializations,
but does not manage the controller reset signal during system
suspend/resume. This can leave the hardware in an undefined state
after resume. Additionally, the reset control is stored in vendor
structs making it inaccessible for power management operations.
Move the reset control to dwcmshc_priv structure and add proper
assertion during suspend and deassertion during resume. The
needs_reset_on_pm flag to control whether reset operations are
performed during power management transitions, as not all platforms
require or support reset handling in suspend/resume path. This ensures the
controller is properly reset before entering low power state and correctly
reinitialized upon wake.
Signed-off-by: Artem Shimko <a.shimko.dev@gmail.com>
---
Hi,
Move reset control from vendor-specific private structures to the
common dwcmshc_priv. Introduce a needs_reset_on_pm flag — platforms
that require reset during suspend/resume set it to true. When the
flag is false (the default for all existing platforms), the PM path
behaves exactly as before, preserving full backward compatibility.
Rockchip RK35xx and EIC7700 do not set the flag; their suspend/resume
behaviour is unchanged.
--
Regards,
Artem
ChangeLog:
v1:
* https://lore.kernel.org/all/20260415123411.437450-1-a.shimko.dev@gmail.com/T/#u
v2:
* Added needs_reset_on_pm flag. The flag defaults to false,
guaranteeing no functional change for existing platforms.
drivers/mmc/host/sdhci-of-dwcmshc.c | 47 ++++++++++++++++++-----------
1 file changed, 29 insertions(+), 18 deletions(-)
diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c b/drivers/mmc/host/sdhci-of-dwcmshc.c
index 0b2158a7e409..cf9be59f79c5 100644
--- a/drivers/mmc/host/sdhci-of-dwcmshc.c
+++ b/drivers/mmc/host/sdhci-of-dwcmshc.c
@@ -279,12 +279,10 @@
#define PHY_DELAY_CODE_SD 0x55
struct rk35xx_priv {
- struct reset_control *reset;
u8 txclk_tapnum;
};
struct eic7700_priv {
- struct reset_control *reset;
unsigned int drive_impedance;
};
@@ -297,6 +295,8 @@ struct k230_priv {
struct dwcmshc_priv {
struct clk *bus_clk;
+ struct reset_control *reset;
+ bool needs_reset_on_pm;
int vendor_specific_area1; /* P_VENDOR_SPECIFIC_AREA1 reg */
int vendor_specific_area2; /* P_VENDOR_SPECIFIC_AREA2 reg */
@@ -887,16 +887,15 @@ static void rk35xx_sdhci_reset(struct sdhci_host *host, u8 mask)
{
struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
struct dwcmshc_priv *dwc_priv = sdhci_pltfm_priv(pltfm_host);
- struct rk35xx_priv *priv = dwc_priv->priv;
u32 extra = sdhci_readl(host, DECMSHC_EMMC_MISC_CON);
if ((host->mmc->caps2 & MMC_CAP2_CQE) && (mask & SDHCI_RESET_ALL))
cqhci_deactivate(host->mmc);
- if (mask & SDHCI_RESET_ALL && priv->reset) {
- reset_control_assert(priv->reset);
+ if (mask & SDHCI_RESET_ALL && dwc_priv->reset) {
+ reset_control_assert(dwc_priv->reset);
udelay(1);
- reset_control_deassert(priv->reset);
+ reset_control_deassert(dwc_priv->reset);
}
sdhci_reset(host, mask);
@@ -916,9 +915,9 @@ static int dwcmshc_rk35xx_init(struct device *dev, struct sdhci_host *host,
if (!priv)
return -ENOMEM;
- priv->reset = devm_reset_control_array_get_optional_exclusive(mmc_dev(host->mmc));
- if (IS_ERR(priv->reset)) {
- err = PTR_ERR(priv->reset);
+ dwc_priv->reset = devm_reset_control_array_get_optional_exclusive(mmc_dev(host->mmc));
+ if (IS_ERR(dwc_priv->reset)) {
+ err = PTR_ERR(dwc_priv->reset);
dev_err(mmc_dev(host->mmc), "failed to get reset control %d\n", err);
return err;
}
@@ -1504,24 +1503,24 @@ static void sdhci_eic7700_reset(struct sdhci_host *host, u8 mask)
sdhci_eic7700_config_phy(host);
}
-static int sdhci_eic7700_reset_init(struct device *dev, struct eic7700_priv *priv)
+static int sdhci_eic7700_reset_init(struct device *dev, struct dwcmshc_priv *dwc_priv)
{
int ret;
- priv->reset = devm_reset_control_array_get_optional_exclusive(dev);
- if (IS_ERR(priv->reset)) {
- ret = PTR_ERR(priv->reset);
+ dwc_priv->reset = devm_reset_control_array_get_optional_exclusive(dev);
+ if (IS_ERR(dwc_priv->reset)) {
+ ret = PTR_ERR(dwc_priv->reset);
dev_err(dev, "failed to get reset control %d\n", ret);
return ret;
}
- ret = reset_control_assert(priv->reset);
+ ret = reset_control_assert(dwc_priv->reset);
if (ret) {
dev_err(dev, "Failed to assert reset signals: %d\n", ret);
return ret;
}
usleep_range(2000, 2100);
- ret = reset_control_deassert(priv->reset);
+ ret = reset_control_deassert(dwc_priv->reset);
if (ret) {
dev_err(dev, "Failed to deassert reset signals: %d\n", ret);
return ret;
@@ -1780,7 +1779,7 @@ static int eic7700_init(struct device *dev, struct sdhci_host *host, struct dwcm
dwc_priv->priv = priv;
- ret = sdhci_eic7700_reset_init(dev, dwc_priv->priv);
+ ret = sdhci_eic7700_reset_init(dev, dwc_priv);
if (ret) {
dev_err(dev, "failed to reset\n");
return ret;
@@ -2563,6 +2562,9 @@ static int dwcmshc_suspend(struct device *dev)
if (ret)
return ret;
+ if (priv->needs_reset_on_pm)
+ reset_control_assert(priv->reset);
+
clk_disable_unprepare(pltfm_host->clk);
if (!IS_ERR(priv->bus_clk))
clk_disable_unprepare(priv->bus_clk);
@@ -2593,18 +2595,27 @@ static int dwcmshc_resume(struct device *dev)
if (ret)
goto disable_bus_clk;
+ if (priv->needs_reset_on_pm) {
+ ret = reset_control_deassert(priv->reset);
+ if (ret)
+ goto disable_other_clks;
+ }
+
ret = sdhci_resume_host(host);
if (ret)
- goto disable_other_clks;
+ goto assert_reset;
if (host->mmc->caps2 & MMC_CAP2_CQE) {
ret = cqhci_resume(host->mmc);
if (ret)
- goto disable_other_clks;
+ goto assert_reset;
}
return 0;
+assert_reset:
+ if (priv->needs_reset_on_pm)
+ reset_control_assert(priv->reset);
disable_other_clks:
clk_bulk_disable_unprepare(priv->num_other_clks, priv->other_clks);
disable_bus_clk:
--
2.43.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2] mmc: sdhci-of-dwcmshc: add reset control support in suspend/resume
2026-05-05 13:37 [PATCH v2] mmc: sdhci-of-dwcmshc: add reset control support in suspend/resume Artem Shimko
@ 2026-05-05 14:05 ` Adrian Hunter
2026-05-05 14:10 ` Artem Shimko
0 siblings, 1 reply; 3+ messages in thread
From: Adrian Hunter @ 2026-05-05 14:05 UTC (permalink / raw)
To: Artem Shimko, Jisheng.Zhang, hehuan1, andriy.shevchenko,
yifeng.zhao, Ulf Hansson, Philipp Zabel
Cc: linux-mmc, linux-kernel
On 05/05/2026 16:37, Artem Shimko wrote:
> The driver currently handles reset control only during initializations,
> but does not manage the controller reset signal during system
> suspend/resume. This can leave the hardware in an undefined state
> after resume. Additionally, the reset control is stored in vendor
> structs making it inaccessible for power management operations.
>
> Move the reset control to dwcmshc_priv structure and add proper
> assertion during suspend and deassertion during resume. The
> needs_reset_on_pm flag to control whether reset operations are
> performed during power management transitions, as not all platforms
> require or support reset handling in suspend/resume path. This ensures the
> controller is properly reset before entering low power state and correctly
> reinitialized upon wake.
>
> Signed-off-by: Artem Shimko <a.shimko.dev@gmail.com>
Changes cannot be accepted unless there is a user for new
functionality i.e. you must add support for a device that
has needs_reset_on_pm set to true.
> ---
>
> Hi,
>
> Move reset control from vendor-specific private structures to the
> common dwcmshc_priv. Introduce a needs_reset_on_pm flag — platforms
> that require reset during suspend/resume set it to true. When the
> flag is false (the default for all existing platforms), the PM path
> behaves exactly as before, preserving full backward compatibility.
>
> Rockchip RK35xx and EIC7700 do not set the flag; their suspend/resume
> behaviour is unchanged.
>
> --
> Regards,
> Artem
>
> ChangeLog:
> v1:
> * https://lore.kernel.org/all/20260415123411.437450-1-a.shimko.dev@gmail.com/T/#u
> v2:
> * Added needs_reset_on_pm flag. The flag defaults to false,
> guaranteeing no functional change for existing platforms.
>
> drivers/mmc/host/sdhci-of-dwcmshc.c | 47 ++++++++++++++++++-----------
> 1 file changed, 29 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c b/drivers/mmc/host/sdhci-of-dwcmshc.c
> index 0b2158a7e409..cf9be59f79c5 100644
> --- a/drivers/mmc/host/sdhci-of-dwcmshc.c
> +++ b/drivers/mmc/host/sdhci-of-dwcmshc.c
> @@ -279,12 +279,10 @@
> #define PHY_DELAY_CODE_SD 0x55
>
> struct rk35xx_priv {
> - struct reset_control *reset;
> u8 txclk_tapnum;
> };
>
> struct eic7700_priv {
> - struct reset_control *reset;
> unsigned int drive_impedance;
> };
>
> @@ -297,6 +295,8 @@ struct k230_priv {
>
> struct dwcmshc_priv {
> struct clk *bus_clk;
> + struct reset_control *reset;
> + bool needs_reset_on_pm;
> int vendor_specific_area1; /* P_VENDOR_SPECIFIC_AREA1 reg */
> int vendor_specific_area2; /* P_VENDOR_SPECIFIC_AREA2 reg */
>
> @@ -887,16 +887,15 @@ static void rk35xx_sdhci_reset(struct sdhci_host *host, u8 mask)
> {
> struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> struct dwcmshc_priv *dwc_priv = sdhci_pltfm_priv(pltfm_host);
> - struct rk35xx_priv *priv = dwc_priv->priv;
> u32 extra = sdhci_readl(host, DECMSHC_EMMC_MISC_CON);
>
> if ((host->mmc->caps2 & MMC_CAP2_CQE) && (mask & SDHCI_RESET_ALL))
> cqhci_deactivate(host->mmc);
>
> - if (mask & SDHCI_RESET_ALL && priv->reset) {
> - reset_control_assert(priv->reset);
> + if (mask & SDHCI_RESET_ALL && dwc_priv->reset) {
> + reset_control_assert(dwc_priv->reset);
> udelay(1);
> - reset_control_deassert(priv->reset);
> + reset_control_deassert(dwc_priv->reset);
> }
>
> sdhci_reset(host, mask);
> @@ -916,9 +915,9 @@ static int dwcmshc_rk35xx_init(struct device *dev, struct sdhci_host *host,
> if (!priv)
> return -ENOMEM;
>
> - priv->reset = devm_reset_control_array_get_optional_exclusive(mmc_dev(host->mmc));
> - if (IS_ERR(priv->reset)) {
> - err = PTR_ERR(priv->reset);
> + dwc_priv->reset = devm_reset_control_array_get_optional_exclusive(mmc_dev(host->mmc));
> + if (IS_ERR(dwc_priv->reset)) {
> + err = PTR_ERR(dwc_priv->reset);
> dev_err(mmc_dev(host->mmc), "failed to get reset control %d\n", err);
> return err;
> }
> @@ -1504,24 +1503,24 @@ static void sdhci_eic7700_reset(struct sdhci_host *host, u8 mask)
> sdhci_eic7700_config_phy(host);
> }
>
> -static int sdhci_eic7700_reset_init(struct device *dev, struct eic7700_priv *priv)
> +static int sdhci_eic7700_reset_init(struct device *dev, struct dwcmshc_priv *dwc_priv)
> {
> int ret;
>
> - priv->reset = devm_reset_control_array_get_optional_exclusive(dev);
> - if (IS_ERR(priv->reset)) {
> - ret = PTR_ERR(priv->reset);
> + dwc_priv->reset = devm_reset_control_array_get_optional_exclusive(dev);
> + if (IS_ERR(dwc_priv->reset)) {
> + ret = PTR_ERR(dwc_priv->reset);
> dev_err(dev, "failed to get reset control %d\n", ret);
> return ret;
> }
>
> - ret = reset_control_assert(priv->reset);
> + ret = reset_control_assert(dwc_priv->reset);
> if (ret) {
> dev_err(dev, "Failed to assert reset signals: %d\n", ret);
> return ret;
> }
> usleep_range(2000, 2100);
> - ret = reset_control_deassert(priv->reset);
> + ret = reset_control_deassert(dwc_priv->reset);
> if (ret) {
> dev_err(dev, "Failed to deassert reset signals: %d\n", ret);
> return ret;
> @@ -1780,7 +1779,7 @@ static int eic7700_init(struct device *dev, struct sdhci_host *host, struct dwcm
>
> dwc_priv->priv = priv;
>
> - ret = sdhci_eic7700_reset_init(dev, dwc_priv->priv);
> + ret = sdhci_eic7700_reset_init(dev, dwc_priv);
> if (ret) {
> dev_err(dev, "failed to reset\n");
> return ret;
> @@ -2563,6 +2562,9 @@ static int dwcmshc_suspend(struct device *dev)
> if (ret)
> return ret;
>
> + if (priv->needs_reset_on_pm)
> + reset_control_assert(priv->reset);
> +
> clk_disable_unprepare(pltfm_host->clk);
> if (!IS_ERR(priv->bus_clk))
> clk_disable_unprepare(priv->bus_clk);
> @@ -2593,18 +2595,27 @@ static int dwcmshc_resume(struct device *dev)
> if (ret)
> goto disable_bus_clk;
>
> + if (priv->needs_reset_on_pm) {
> + ret = reset_control_deassert(priv->reset);
> + if (ret)
> + goto disable_other_clks;
> + }
> +
> ret = sdhci_resume_host(host);
> if (ret)
> - goto disable_other_clks;
> + goto assert_reset;
>
> if (host->mmc->caps2 & MMC_CAP2_CQE) {
> ret = cqhci_resume(host->mmc);
> if (ret)
> - goto disable_other_clks;
> + goto assert_reset;
> }
>
> return 0;
>
> +assert_reset:
> + if (priv->needs_reset_on_pm)
> + reset_control_assert(priv->reset);
> disable_other_clks:
> clk_bulk_disable_unprepare(priv->num_other_clks, priv->other_clks);
> disable_bus_clk:
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] mmc: sdhci-of-dwcmshc: add reset control support in suspend/resume
2026-05-05 14:05 ` Adrian Hunter
@ 2026-05-05 14:10 ` Artem Shimko
0 siblings, 0 replies; 3+ messages in thread
From: Artem Shimko @ 2026-05-05 14:10 UTC (permalink / raw)
To: Adrian Hunter
Cc: Jisheng.Zhang, hehuan1, andriy.shevchenko, yifeng.zhao,
Ulf Hansson, Philipp Zabel, linux-mmc, linux-kernel
Hi Adrian,
On Tue, May 5, 2026 at 5:05 PM Adrian Hunter <adrian.hunter@intel.com> wrote:
> Changes cannot be accepted unless there is a user for new
> functionality i.e. you must add support for a device that
> has needs_reset_on_pm set to true.
Understood, thank you for the clarification. Unfortunately I cannot
upstream the device that would set needs_reset_on_pm to true at this
time.
Please drop this patch. I will keep it in our internal tree for now,
and resubmit together with the corresponding device support when
that becomes possible.
Regards,
Artem
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-05-05 14:10 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-05 13:37 [PATCH v2] mmc: sdhci-of-dwcmshc: add reset control support in suspend/resume Artem Shimko
2026-05-05 14:05 ` Adrian Hunter
2026-05-05 14:10 ` Artem Shimko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox