Re: [PATCH v3] mmc: sdhci-of-dwcmshc: add PD workaround on RK3576

Ulf Hansson
2025-04-29T10:14:38Z

On Wed, 23 Apr 2025 at 09:54, Nicolas Frattaroli
<nicolas.frattaroli@collabora.com> wrote:
>
> RK3576's power domains have a peculiar design where the PD_NVM power
> domain, of which the sdhci controller is a part, seemingly does not have
> idempotent runtime disable/enable. The end effect is that if PD_NVM gets
> turned off by the generic power domain logic because all the devices
> depending on it are suspended, then the next time the sdhci device is
> unsuspended, it'll hang the SoC as soon as it tries accessing the CQHCI
> registers.
>
> RK3576's UFS support needed a new dev_pm_genpd_rpm_always_on function
> added to the generic power domains API to handle what appears to be a
> similar hardware design.
>
> Use this new function to ask for the same treatment in the sdhci
> controller by giving rk3576 its own platform data with its own postinit
> function. The benefit of doing this instead of marking the power domains
> always on in the power domain core is that we only do this if we know
> the platform we're running on actually uses the sdhci controller. For
> others, keeping PD_NVM always on would be a waste, as they won't run
> into this specific issue. The only other IP in PD_NVM that could be
> affected is FSPI0. If it gets a mainline driver, it will probably want
> to do the same thing.
>
> Acked-by: Adrian Hunter <adrian.hunter@intel.com>
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>

Applied for next, thanks!

Kind regards
Uffe

> ---
> Changes in v3:
> - Reword comment and commit message to correct that this is not a
> silicon bug, but seemingly intentional design with regards to runtime
> power management.
> - Link to v2: https://lore.kernel.org/r/20250412-rk3576-emmc-fix-v2-1-830e653ad4f0@collabora.com
>
> Changes in v2:
> - Rewrite patch to use dev_pm_genpd_rpm_always_on in sdhci driver
> instead, after Ulf Hansson made me aware of its existence
> - Link to v1: https://lore.kernel.org/r/20250408-rk3576-emmc-fix-v1-1-3009828b1b31@collabora.com
> ---
> drivers/mmc/host/sdhci-of-dwcmshc.c | 40 +++++++++++++++++++++++++++++++++++++
> 1 file changed, 40 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c b/drivers/mmc/host/sdhci-of-dwcmshc.c
> index 09b9ab15e4995f0bddf57dd309c010c849be40d9..a20d03fdd6a93ecc5229c71f825bade5ac730370 100644
> --- a/drivers/mmc/host/sdhci-of-dwcmshc.c
> +++ b/drivers/mmc/host/sdhci-of-dwcmshc.c
> @@ -17,6 +17,7 @@
> #include <linux/module.h>
> #include <linux/of.h>
> #include <linux/platform_device.h>
> +#include <linux/pm_domain.h>
> #include <linux/pm_runtime.h>
> #include <linux/reset.h>
> #include <linux/sizes.h>
> @@ -745,6 +746,29 @@ static void dwcmshc_rk35xx_postinit(struct sdhci_host *host, struct dwcmshc_priv
> }
> }
>
> +static void dwcmshc_rk3576_postinit(struct sdhci_host *host, struct dwcmshc_priv *dwc_priv)
> +{
> + struct device *dev = mmc_dev(host->mmc);
> + int ret;
> +
> + /*
> + * This works around the design of the RK3576's power domains, which
> + * makes the PD_NVM power domain, which the sdhci controller on the
> + * RK3576 is in, never come back the same way once it's run-time
> + * suspended once. This can happen during early kernel boot if no driver
> + * is using either PD_NVM or its child power domain PD_SDGMAC for a
> + * short moment, leading to it being turned off to save power. By
> + * keeping it on, sdhci suspending won't lead to PD_NVM becoming a
> + * candidate for getting turned off.
> + */
> + ret = dev_pm_genpd_rpm_always_on(dev, true);
> + if (ret && ret != -EOPNOTSUPP)
> + dev_warn(dev, "failed to set PD rpm always on, SoC may hang later: %pe\n",
> + ERR_PTR(ret));
> +
> + dwcmshc_rk35xx_postinit(host, dwc_priv);
> +}
> +
> static int th1520_execute_tuning(struct sdhci_host *host, u32 opcode)
> {
> struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> @@ -1176,6 +1200,18 @@ static const struct dwcmshc_pltfm_data sdhci_dwcmshc_rk35xx_pdata = {
> .postinit = dwcmshc_rk35xx_postinit,
> };
>
> +static const struct dwcmshc_pltfm_data sdhci_dwcmshc_rk3576_pdata = {
> + .pdata = {
> + .ops = &sdhci_dwcmshc_rk35xx_ops,
> + .quirks = SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN |
> + SDHCI_QUIRK_BROKEN_TIMEOUT_VAL,
> + .quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN |
> + SDHCI_QUIRK2_CLOCK_DIV_ZERO_BROKEN,
> + },
> + .init = dwcmshc_rk35xx_init,
> + .postinit = dwcmshc_rk3576_postinit,
> +};
> +
> static const struct dwcmshc_pltfm_data sdhci_dwcmshc_th1520_pdata = {
> .pdata = {
> .ops = &sdhci_dwcmshc_th1520_ops,
> @@ -1274,6 +1310,10 @@ static const struct of_device_id sdhci_dwcmshc_dt_ids[] = {
> .compatible = "rockchip,rk3588-dwcmshc",
> .data = &sdhci_dwcmshc_rk35xx_pdata,
> },
> + {
> + .compatible = "rockchip,rk3576-dwcmshc",
> + .data = &sdhci_dwcmshc_rk3576_pdata,
> + },
> {
> .compatible = "rockchip,rk3568-dwcmshc",
> .data = &sdhci_dwcmshc_rk35xx_pdata,
>
> ---
> base-commit: f34da179a4517854b2ffbe4bce8c3405bd9be04e
> change-id: 20250317-rk3576-emmc-fix-7dc81a627422
>
> Best regards,
> --
> Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
>