| Message ID | 20230804233015.173957-1-limings@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v6] mmc: sdhci-of-dwcmshc: Add runtime PM operations | expand |
On Sat, 5 Aug 2023 at 01:30, Liming Sun <limings@nvidia.com> wrote: > > This commit implements the runtime PM operations to disable eMMC > card clock when idle. > > Reviewed-by: David Thompson <davthompson@nvidia.com> > Signed-off-by: Liming Sun <limings@nvidia.com> > --- > v5->v6: > - Address Adrian's more comments and add coordination between > runtime PM and system PM; > v4->v5: > - Address Adrian's comment to move the pm_enable to the end to > avoid race; > v3->v4: > - Fix compiling reported by 'kernel test robot'; > v2->v3: > - Revise the commit message; > v1->v2: > Updates for comments from Ulf: > - Make the runtime PM logic generic for sdhci-of-dwcmshc; > v1: Initial version. > --- > drivers/mmc/host/sdhci-of-dwcmshc.c | 72 ++++++++++++++++++++++++++++- > 1 file changed, 70 insertions(+), 2 deletions(-) > > diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c b/drivers/mmc/host/sdhci-of-dwcmshc.c > index e68cd87998c8..aaf66358f626 100644 > --- a/drivers/mmc/host/sdhci-of-dwcmshc.c > +++ b/drivers/mmc/host/sdhci-of-dwcmshc.c > @@ -15,6 +15,7 @@ > #include <linux/module.h> > #include <linux/of.h> > #include <linux/of_device.h> > +#include <linux/pm_runtime.h> > #include <linux/reset.h> > #include <linux/sizes.h> > > @@ -548,9 +549,13 @@ static int dwcmshc_probe(struct platform_device *pdev) > > host->mmc->caps |= MMC_CAP_WAIT_WHILE_BUSY; > > + pm_runtime_get_noresume(dev); > + pm_runtime_set_active(dev); > + pm_runtime_enable(dev); > + > err = sdhci_setup_host(host); > if (err) > - goto err_clk; > + goto err_rpm; > > if (rk_priv) > dwcmshc_rk35xx_postinit(host, priv); > @@ -559,10 +564,15 @@ static int dwcmshc_probe(struct platform_device *pdev) > if (err) > goto err_setup_host; > > + pm_runtime_put_sync(dev); > + The async pm_runtime_put() is probably sufficient - and it would allow the probe to complete a bit "sooner". > return 0; > > err_setup_host: > sdhci_cleanup_host(host); > +err_rpm: > + pm_runtime_disable(dev); > + pm_runtime_put_noidle(dev); > err_clk: > clk_disable_unprepare(pltfm_host->clk); > clk_disable_unprepare(priv->bus_clk); > @@ -606,6 +616,12 @@ static int dwcmshc_suspend(struct device *dev) > if (ret) > return ret; > > + ret = pm_runtime_force_suspend(dev); Can dwcmshc_suspend() be called for a device that may be attached to the ACPI PM domain? > + if (ret) { > + sdhci_resume_host(host); > + return ret; > + } > + > clk_disable_unprepare(pltfm_host->clk); > if (!IS_ERR(priv->bus_clk)) > clk_disable_unprepare(priv->bus_clk); > @@ -625,6 +641,10 @@ static int dwcmshc_resume(struct device *dev) > struct rk35xx_priv *rk_priv = priv->priv; > int ret; > > + ret = pm_runtime_force_resume(dev); > + if (ret) > + return ret; > + > ret = clk_prepare_enable(pltfm_host->clk); > if (ret) > return ret; > @@ -646,7 +666,55 @@ static int dwcmshc_resume(struct device *dev) > } > #endif > > -static SIMPLE_DEV_PM_OPS(dwcmshc_pmops, dwcmshc_suspend, dwcmshc_resume); > +#ifdef CONFIG_PM > + > +static void dwcmshc_enable_card_clk(struct sdhci_host *host) > +{ > + u16 ctrl; > + > + ctrl = sdhci_readw(host, SDHCI_CLOCK_CONTROL); > + if ((ctrl & SDHCI_CLOCK_INT_EN) && !(ctrl & SDHCI_CLOCK_CARD_EN)) { > + ctrl |= SDHCI_CLOCK_CARD_EN; > + sdhci_writew(host, ctrl, SDHCI_CLOCK_CONTROL); > + } > +} > + > +static void dwcmshc_disable_card_clk(struct sdhci_host *host) > +{ > + u16 ctrl; > + > + ctrl = sdhci_readw(host, SDHCI_CLOCK_CONTROL); > + if (ctrl & SDHCI_CLOCK_CARD_EN) { > + ctrl &= ~SDHCI_CLOCK_CARD_EN; > + sdhci_writew(host, ctrl, SDHCI_CLOCK_CONTROL); > + } > +} > + > +static int dwcmshc_runtime_suspend(struct device *dev) > +{ > + struct sdhci_host *host = dev_get_drvdata(dev); > + > + dwcmshc_disable_card_clk(host); > + > + return 0; > +} > + > +static int dwcmshc_runtime_resume(struct device *dev) > +{ > + struct sdhci_host *host = dev_get_drvdata(dev); > + > + dwcmshc_enable_card_clk(host); > + > + return 0; > +} > + > +#endif > + > +static const struct dev_pm_ops dwcmshc_pmops = { > + SET_SYSTEM_SLEEP_PM_OPS(dwcmshc_suspend, dwcmshc_resume) > + SET_RUNTIME_PM_OPS(dwcmshc_runtime_suspend, > + dwcmshc_runtime_resume, NULL) > +}; > > static struct platform_driver sdhci_dwcmshc_driver = { > .driver = { > -- Kind regards Uffe
> -----Original Message----- > From: Ulf Hansson <ulf.hansson@linaro.org> > Sent: Tuesday, August 8, 2023 5:40 AM > To: Liming Sun <limings@nvidia.com> > Cc: Adrian Hunter <adrian.hunter@intel.com>; David Thompson > <davthompson@nvidia.com>; Shawn Lin <shawn.lin@rock-chips.com>; linux- > mmc@vger.kernel.org; linux-kernel@vger.kernel.org > Subject: Re: [PATCH v6] mmc: sdhci-of-dwcmshc: Add runtime PM operations > > On Sat, 5 Aug 2023 at 01:30, Liming Sun <limings@nvidia.com> wrote: > > > > This commit implements the runtime PM operations to disable eMMC > > card clock when idle. > > > > Reviewed-by: David Thompson <davthompson@nvidia.com> > > Signed-off-by: Liming Sun <limings@nvidia.com> > > --- > > v5->v6: > > - Address Adrian's more comments and add coordination between > > runtime PM and system PM; > > v4->v5: > > - Address Adrian's comment to move the pm_enable to the end to > > avoid race; > > v3->v4: > > - Fix compiling reported by 'kernel test robot'; > > v2->v3: > > - Revise the commit message; > > v1->v2: > > Updates for comments from Ulf: > > - Make the runtime PM logic generic for sdhci-of-dwcmshc; > > v1: Initial version. > > --- > > drivers/mmc/host/sdhci-of-dwcmshc.c | 72 > ++++++++++++++++++++++++++++- > > 1 file changed, 70 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c b/drivers/mmc/host/sdhci- > of-dwcmshc.c > > index e68cd87998c8..aaf66358f626 100644 > > --- a/drivers/mmc/host/sdhci-of-dwcmshc.c > > +++ b/drivers/mmc/host/sdhci-of-dwcmshc.c > > @@ -15,6 +15,7 @@ > > #include <linux/module.h> > > #include <linux/of.h> > > #include <linux/of_device.h> > > +#include <linux/pm_runtime.h> > > #include <linux/reset.h> > > #include <linux/sizes.h> > > > > @@ -548,9 +549,13 @@ static int dwcmshc_probe(struct platform_device > *pdev) > > > > host->mmc->caps |= MMC_CAP_WAIT_WHILE_BUSY; > > > > + pm_runtime_get_noresume(dev); > > + pm_runtime_set_active(dev); > > + pm_runtime_enable(dev); > > + > > err = sdhci_setup_host(host); > > if (err) > > - goto err_clk; > > + goto err_rpm; > > > > if (rk_priv) > > dwcmshc_rk35xx_postinit(host, priv); > > @@ -559,10 +564,15 @@ static int dwcmshc_probe(struct platform_device > *pdev) > > if (err) > > goto err_setup_host; > > > > + pm_runtime_put_sync(dev); > > + > > The async pm_runtime_put() is probably sufficient - and it would allow > the probe to complete a bit "sooner". Sure, will test and update the line in v7. > > > return 0; > > > > err_setup_host: > > sdhci_cleanup_host(host); > > +err_rpm: > > + pm_runtime_disable(dev); > > + pm_runtime_put_noidle(dev); > > err_clk: > > clk_disable_unprepare(pltfm_host->clk); > > clk_disable_unprepare(priv->bus_clk); > > @@ -606,6 +616,12 @@ static int dwcmshc_suspend(struct device *dev) > > if (ret) > > return ret; > > > > + ret = pm_runtime_force_suspend(dev); > > Can dwcmshc_suspend() be called for a device that may be attached to > the ACPI PM domain? BlueField SoC is the only chip that uses ACPI in this driver for now and it doesn't support System Sleep. Thus, the dwcmshc_suspend() won't be called. But I guess it might be possible when other new chip support is added into this driver. Is it a concern? > > > + if (ret) { > > + sdhci_resume_host(host); > > + return ret; > > + } > > + > > clk_disable_unprepare(pltfm_host->clk); > > if (!IS_ERR(priv->bus_clk)) > > clk_disable_unprepare(priv->bus_clk); > > @@ -625,6 +641,10 @@ static int dwcmshc_resume(struct device *dev) > > struct rk35xx_priv *rk_priv = priv->priv; > > int ret; > > > > + ret = pm_runtime_force_resume(dev); > > + if (ret) > > + return ret; > > + > > ret = clk_prepare_enable(pltfm_host->clk); > > if (ret) > > return ret; > > @@ -646,7 +666,55 @@ static int dwcmshc_resume(struct device *dev) > > } > > #endif > > > > -static SIMPLE_DEV_PM_OPS(dwcmshc_pmops, dwcmshc_suspend, > dwcmshc_resume); > > +#ifdef CONFIG_PM > > + > > +static void dwcmshc_enable_card_clk(struct sdhci_host *host) > > +{ > > + u16 ctrl; > > + > > + ctrl = sdhci_readw(host, SDHCI_CLOCK_CONTROL); > > + if ((ctrl & SDHCI_CLOCK_INT_EN) && !(ctrl & SDHCI_CLOCK_CARD_EN)) > { > > + ctrl |= SDHCI_CLOCK_CARD_EN; > > + sdhci_writew(host, ctrl, SDHCI_CLOCK_CONTROL); > > + } > > +} > > + > > +static void dwcmshc_disable_card_clk(struct sdhci_host *host) > > +{ > > + u16 ctrl; > > + > > + ctrl = sdhci_readw(host, SDHCI_CLOCK_CONTROL); > > + if (ctrl & SDHCI_CLOCK_CARD_EN) { > > + ctrl &= ~SDHCI_CLOCK_CARD_EN; > > + sdhci_writew(host, ctrl, SDHCI_CLOCK_CONTROL); > > + } > > +} > > + > > +static int dwcmshc_runtime_suspend(struct device *dev) > > +{ > > + struct sdhci_host *host = dev_get_drvdata(dev); > > + > > + dwcmshc_disable_card_clk(host); > > + > > + return 0; > > +} > > + > > +static int dwcmshc_runtime_resume(struct device *dev) > > +{ > > + struct sdhci_host *host = dev_get_drvdata(dev); > > + > > + dwcmshc_enable_card_clk(host); > > + > > + return 0; > > +} > > + > > +#endif > > + > > +static const struct dev_pm_ops dwcmshc_pmops = { > > + SET_SYSTEM_SLEEP_PM_OPS(dwcmshc_suspend, dwcmshc_resume) > > + SET_RUNTIME_PM_OPS(dwcmshc_runtime_suspend, > > + dwcmshc_runtime_resume, NULL) > > +}; > > > > static struct platform_driver sdhci_dwcmshc_driver = { > > .driver = { > > -- > > Kind regards > Uffe
On Tue, 8 Aug 2023 at 15:21, Liming Sun <limings@nvidia.com> wrote: > > > > > -----Original Message----- > > From: Ulf Hansson <ulf.hansson@linaro.org> > > Sent: Tuesday, August 8, 2023 5:40 AM > > To: Liming Sun <limings@nvidia.com> > > Cc: Adrian Hunter <adrian.hunter@intel.com>; David Thompson > > <davthompson@nvidia.com>; Shawn Lin <shawn.lin@rock-chips.com>; linux- > > mmc@vger.kernel.org; linux-kernel@vger.kernel.org > > Subject: Re: [PATCH v6] mmc: sdhci-of-dwcmshc: Add runtime PM operations > > > > On Sat, 5 Aug 2023 at 01:30, Liming Sun <limings@nvidia.com> wrote: > > > > > > This commit implements the runtime PM operations to disable eMMC > > > card clock when idle. > > > > > > Reviewed-by: David Thompson <davthompson@nvidia.com> > > > Signed-off-by: Liming Sun <limings@nvidia.com> > > > --- > > > v5->v6: > > > - Address Adrian's more comments and add coordination between > > > runtime PM and system PM; > > > v4->v5: > > > - Address Adrian's comment to move the pm_enable to the end to > > > avoid race; > > > v3->v4: > > > - Fix compiling reported by 'kernel test robot'; > > > v2->v3: > > > - Revise the commit message; > > > v1->v2: > > > Updates for comments from Ulf: > > > - Make the runtime PM logic generic for sdhci-of-dwcmshc; > > > v1: Initial version. > > > --- > > > drivers/mmc/host/sdhci-of-dwcmshc.c | 72 > > ++++++++++++++++++++++++++++- > > > 1 file changed, 70 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c b/drivers/mmc/host/sdhci- > > of-dwcmshc.c > > > index e68cd87998c8..aaf66358f626 100644 > > > --- a/drivers/mmc/host/sdhci-of-dwcmshc.c > > > +++ b/drivers/mmc/host/sdhci-of-dwcmshc.c > > > @@ -15,6 +15,7 @@ > > > #include <linux/module.h> > > > #include <linux/of.h> > > > #include <linux/of_device.h> > > > +#include <linux/pm_runtime.h> > > > #include <linux/reset.h> > > > #include <linux/sizes.h> > > > > > > @@ -548,9 +549,13 @@ static int dwcmshc_probe(struct platform_device > > *pdev) > > > > > > host->mmc->caps |= MMC_CAP_WAIT_WHILE_BUSY; > > > > > > + pm_runtime_get_noresume(dev); > > > + pm_runtime_set_active(dev); > > > + pm_runtime_enable(dev); > > > + > > > err = sdhci_setup_host(host); > > > if (err) > > > - goto err_clk; > > > + goto err_rpm; > > > > > > if (rk_priv) > > > dwcmshc_rk35xx_postinit(host, priv); > > > @@ -559,10 +564,15 @@ static int dwcmshc_probe(struct platform_device > > *pdev) > > > if (err) > > > goto err_setup_host; > > > > > > + pm_runtime_put_sync(dev); > > > + > > > > The async pm_runtime_put() is probably sufficient - and it would allow > > the probe to complete a bit "sooner". > > Sure, will test and update the line in v7. > > > > > > return 0; > > > > > > err_setup_host: > > > sdhci_cleanup_host(host); > > > +err_rpm: > > > + pm_runtime_disable(dev); > > > + pm_runtime_put_noidle(dev); > > > err_clk: > > > clk_disable_unprepare(pltfm_host->clk); > > > clk_disable_unprepare(priv->bus_clk); > > > @@ -606,6 +616,12 @@ static int dwcmshc_suspend(struct device *dev) > > > if (ret) > > > return ret; > > > > > > + ret = pm_runtime_force_suspend(dev); > > > > Can dwcmshc_suspend() be called for a device that may be attached to > > the ACPI PM domain? > > BlueField SoC is the only chip that uses ACPI in this driver for now and it doesn't support System Sleep. Thus, the dwcmshc_suspend() won't be called. But I guess it might be possible when other new chip support is added into this driver. Is it a concern? The ACPI PM domain doesn't support drivers using pm_runtime_force_suspend|resume(). Unless that has been changed without me knowing about it. Anyway, it looks like it shouldn't be a problem at this point. We can also add separate callbacks for other SoCs, if that comes into play going forward. [...] Kind regards Uffe
> -----Original Message----- > From: Ulf Hansson <ulf.hansson@linaro.org> > Sent: Tuesday, August 8, 2023 9:57 AM > To: Liming Sun <limings@nvidia.com> > Cc: Adrian Hunter <adrian.hunter@intel.com>; David Thompson > <davthompson@nvidia.com>; Shawn Lin <shawn.lin@rock-chips.com>; linux- > mmc@vger.kernel.org; linux-kernel@vger.kernel.org > Subject: Re: [PATCH v6] mmc: sdhci-of-dwcmshc: Add runtime PM operations > > On Tue, 8 Aug 2023 at 15:21, Liming Sun <limings@nvidia.com> wrote: > > > > > > > > > -----Original Message----- > > > From: Ulf Hansson <ulf.hansson@linaro.org> > > > Sent: Tuesday, August 8, 2023 5:40 AM > > > To: Liming Sun <limings@nvidia.com> > > > Cc: Adrian Hunter <adrian.hunter@intel.com>; David Thompson > > > <davthompson@nvidia.com>; Shawn Lin <shawn.lin@rock-chips.com>; > linux- > > > mmc@vger.kernel.org; linux-kernel@vger.kernel.org > > > Subject: Re: [PATCH v6] mmc: sdhci-of-dwcmshc: Add runtime PM > operations > > > > > > On Sat, 5 Aug 2023 at 01:30, Liming Sun <limings@nvidia.com> wrote: > > > > > > > > This commit implements the runtime PM operations to disable eMMC > > > > card clock when idle. > > > > > > > > Reviewed-by: David Thompson <davthompson@nvidia.com> > > > > Signed-off-by: Liming Sun <limings@nvidia.com> > > > > --- > > > > v5->v6: > > > > - Address Adrian's more comments and add coordination between > > > > runtime PM and system PM; > > > > v4->v5: > > > > - Address Adrian's comment to move the pm_enable to the end to > > > > avoid race; > > > > v3->v4: > > > > - Fix compiling reported by 'kernel test robot'; > > > > v2->v3: > > > > - Revise the commit message; > > > > v1->v2: > > > > Updates for comments from Ulf: > > > > - Make the runtime PM logic generic for sdhci-of-dwcmshc; > > > > v1: Initial version. > > > > --- > > > > drivers/mmc/host/sdhci-of-dwcmshc.c | 72 > > > ++++++++++++++++++++++++++++- > > > > 1 file changed, 70 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c > b/drivers/mmc/host/sdhci- > > > of-dwcmshc.c > > > > index e68cd87998c8..aaf66358f626 100644 > > > > --- a/drivers/mmc/host/sdhci-of-dwcmshc.c > > > > +++ b/drivers/mmc/host/sdhci-of-dwcmshc.c > > > > @@ -15,6 +15,7 @@ > > > > #include <linux/module.h> > > > > #include <linux/of.h> > > > > #include <linux/of_device.h> > > > > +#include <linux/pm_runtime.h> > > > > #include <linux/reset.h> > > > > #include <linux/sizes.h> > > > > > > > > @@ -548,9 +549,13 @@ static int dwcmshc_probe(struct > platform_device > > > *pdev) > > > > > > > > host->mmc->caps |= MMC_CAP_WAIT_WHILE_BUSY; > > > > > > > > + pm_runtime_get_noresume(dev); > > > > + pm_runtime_set_active(dev); > > > > + pm_runtime_enable(dev); > > > > + > > > > err = sdhci_setup_host(host); > > > > if (err) > > > > - goto err_clk; > > > > + goto err_rpm; > > > > > > > > if (rk_priv) > > > > dwcmshc_rk35xx_postinit(host, priv); > > > > @@ -559,10 +564,15 @@ static int dwcmshc_probe(struct > platform_device > > > *pdev) > > > > if (err) > > > > goto err_setup_host; > > > > > > > > + pm_runtime_put_sync(dev); > > > > + > > > > > > The async pm_runtime_put() is probably sufficient - and it would allow > > > the probe to complete a bit "sooner". > > > > Sure, will test and update the line in v7. > > > > > > > > > return 0; > > > > > > > > err_setup_host: > > > > sdhci_cleanup_host(host); > > > > +err_rpm: > > > > + pm_runtime_disable(dev); > > > > + pm_runtime_put_noidle(dev); > > > > err_clk: > > > > clk_disable_unprepare(pltfm_host->clk); > > > > clk_disable_unprepare(priv->bus_clk); > > > > @@ -606,6 +616,12 @@ static int dwcmshc_suspend(struct device *dev) > > > > if (ret) > > > > return ret; > > > > > > > > + ret = pm_runtime_force_suspend(dev); > > > > > > Can dwcmshc_suspend() be called for a device that may be attached to > > > the ACPI PM domain? > > > > BlueField SoC is the only chip that uses ACPI in this driver for now and it > doesn't support System Sleep. Thus, the dwcmshc_suspend() won't be called. > But I guess it might be possible when other new chip support is added into this > driver. Is it a concern? > > The ACPI PM domain doesn't support drivers using > pm_runtime_force_suspend|resume(). Unless that has been changed > without me knowing about it. > > Anyway, it looks like it shouldn't be a problem at this point. We can > also add separate callbacks for other SoCs, if that comes into play > going forward. Thanks. So no further changes for now. Posted v7 (for the other comment). > > [...] > > Kind regards > Uffe
On Sat, 5 Aug 2023 at 01:30, Liming Sun <limings@nvidia.com> wrote: > > This commit implements the runtime PM operations to disable eMMC > card clock when idle. > > Reviewed-by: David Thompson <davthompson@nvidia.com> > Signed-off-by: Liming Sun <limings@nvidia.com> > --- > v5->v6: > - Address Adrian's more comments and add coordination between > runtime PM and system PM; > v4->v5: > - Address Adrian's comment to move the pm_enable to the end to > avoid race; > v3->v4: > - Fix compiling reported by 'kernel test robot'; > v2->v3: > - Revise the commit message; > v1->v2: > Updates for comments from Ulf: > - Make the runtime PM logic generic for sdhci-of-dwcmshc; > v1: Initial version. > --- > drivers/mmc/host/sdhci-of-dwcmshc.c | 72 ++++++++++++++++++++++++++++- > 1 file changed, 70 insertions(+), 2 deletions(-) > > diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c b/drivers/mmc/host/sdhci-of-dwcmshc.c > index e68cd87998c8..aaf66358f626 100644 > --- a/drivers/mmc/host/sdhci-of-dwcmshc.c > +++ b/drivers/mmc/host/sdhci-of-dwcmshc.c > @@ -15,6 +15,7 @@ > #include <linux/module.h> > #include <linux/of.h> > #include <linux/of_device.h> > +#include <linux/pm_runtime.h> > #include <linux/reset.h> > #include <linux/sizes.h> > > @@ -548,9 +549,13 @@ static int dwcmshc_probe(struct platform_device *pdev) > > host->mmc->caps |= MMC_CAP_WAIT_WHILE_BUSY; > > + pm_runtime_get_noresume(dev); > + pm_runtime_set_active(dev); > + pm_runtime_enable(dev); > + > err = sdhci_setup_host(host); > if (err) > - goto err_clk; > + goto err_rpm; > > if (rk_priv) > dwcmshc_rk35xx_postinit(host, priv); > @@ -559,10 +564,15 @@ static int dwcmshc_probe(struct platform_device *pdev) > if (err) > goto err_setup_host; > > + pm_runtime_put_sync(dev); > + > return 0; > > err_setup_host: > sdhci_cleanup_host(host); > +err_rpm: > + pm_runtime_disable(dev); > + pm_runtime_put_noidle(dev); > err_clk: > clk_disable_unprepare(pltfm_host->clk); > clk_disable_unprepare(priv->bus_clk); > @@ -606,6 +616,12 @@ static int dwcmshc_suspend(struct device *dev) > if (ret) > return ret; > sdhci_suspend_host() is called a few lines above, which requires the host to be runtime resumed when called. This may not be the case here. To fix this there are in principle two options: 1) Call a pm_runtime_get_sync() around here and a corresponding pm_runtime_put_noidle() somewhere after you have called pm_runtime_force_suspend(). 2) Move away from using sdhci_suspend|resume_host(), but use sdhci_runtime_suspend|resume_host() instead. This requires some additional changes, but with the benefit that we can avoid runtime resuming the device upfront. In this case, sdhci_runtime_suspend|resume_host() should be called from dwcmshc_runtime_suspend|resume(). Moreover, we either need to move the entire clock mgmt from dwcmshc_suspend|resume() into the dwcmshc_runtime_suspend|resume() or call pm_runtime_get_noresume() before calling pm_runtime_force_suspend() and pm_runtime_put_noidle() just after it. > + ret = pm_runtime_force_suspend(dev); > + if (ret) { > + sdhci_resume_host(host); > + return ret; > + } > + > clk_disable_unprepare(pltfm_host->clk); > if (!IS_ERR(priv->bus_clk)) > clk_disable_unprepare(priv->bus_clk); > @@ -625,6 +641,10 @@ static int dwcmshc_resume(struct device *dev) > struct rk35xx_priv *rk_priv = priv->priv; > int ret; > > + ret = pm_runtime_force_resume(dev); > + if (ret) > + return ret; > + > ret = clk_prepare_enable(pltfm_host->clk); > if (ret) > return ret; > @@ -646,7 +666,55 @@ static int dwcmshc_resume(struct device *dev) > } > #endif > > -static SIMPLE_DEV_PM_OPS(dwcmshc_pmops, dwcmshc_suspend, dwcmshc_resume); > +#ifdef CONFIG_PM > + > +static void dwcmshc_enable_card_clk(struct sdhci_host *host) > +{ > + u16 ctrl; > + > + ctrl = sdhci_readw(host, SDHCI_CLOCK_CONTROL); > + if ((ctrl & SDHCI_CLOCK_INT_EN) && !(ctrl & SDHCI_CLOCK_CARD_EN)) { > + ctrl |= SDHCI_CLOCK_CARD_EN; > + sdhci_writew(host, ctrl, SDHCI_CLOCK_CONTROL); > + } > +} > + > +static void dwcmshc_disable_card_clk(struct sdhci_host *host) > +{ > + u16 ctrl; > + > + ctrl = sdhci_readw(host, SDHCI_CLOCK_CONTROL); > + if (ctrl & SDHCI_CLOCK_CARD_EN) { > + ctrl &= ~SDHCI_CLOCK_CARD_EN; > + sdhci_writew(host, ctrl, SDHCI_CLOCK_CONTROL); > + } > +} > + > +static int dwcmshc_runtime_suspend(struct device *dev) > +{ > + struct sdhci_host *host = dev_get_drvdata(dev); > + > + dwcmshc_disable_card_clk(host); > + > + return 0; > +} > + > +static int dwcmshc_runtime_resume(struct device *dev) > +{ > + struct sdhci_host *host = dev_get_drvdata(dev); > + > + dwcmshc_enable_card_clk(host); > + > + return 0; > +} > + > +#endif > + > +static const struct dev_pm_ops dwcmshc_pmops = { > + SET_SYSTEM_SLEEP_PM_OPS(dwcmshc_suspend, dwcmshc_resume) > + SET_RUNTIME_PM_OPS(dwcmshc_runtime_suspend, > + dwcmshc_runtime_resume, NULL) > +}; > > static struct platform_driver sdhci_dwcmshc_driver = { > .driver = { Kind regards Uffe
diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c b/drivers/mmc/host/sdhci-of-dwcmshc.c index e68cd87998c8..aaf66358f626 100644 --- a/drivers/mmc/host/sdhci-of-dwcmshc.c +++ b/drivers/mmc/host/sdhci-of-dwcmshc.c @@ -15,6 +15,7 @@ #include <linux/module.h> #include <linux/of.h> #include <linux/of_device.h> +#include <linux/pm_runtime.h> #include <linux/reset.h> #include <linux/sizes.h> @@ -548,9 +549,13 @@ static int dwcmshc_probe(struct platform_device *pdev) host->mmc->caps |= MMC_CAP_WAIT_WHILE_BUSY; + pm_runtime_get_noresume(dev); + pm_runtime_set_active(dev); + pm_runtime_enable(dev); + err = sdhci_setup_host(host); if (err) - goto err_clk; + goto err_rpm; if (rk_priv) dwcmshc_rk35xx_postinit(host, priv); @@ -559,10 +564,15 @@ static int dwcmshc_probe(struct platform_device *pdev) if (err) goto err_setup_host; + pm_runtime_put_sync(dev); + return 0; err_setup_host: sdhci_cleanup_host(host); +err_rpm: + pm_runtime_disable(dev); + pm_runtime_put_noidle(dev); err_clk: clk_disable_unprepare(pltfm_host->clk); clk_disable_unprepare(priv->bus_clk); @@ -606,6 +616,12 @@ static int dwcmshc_suspend(struct device *dev) if (ret) return ret; + ret = pm_runtime_force_suspend(dev); + if (ret) { + sdhci_resume_host(host); + return ret; + } + clk_disable_unprepare(pltfm_host->clk); if (!IS_ERR(priv->bus_clk)) clk_disable_unprepare(priv->bus_clk); @@ -625,6 +641,10 @@ static int dwcmshc_resume(struct device *dev) struct rk35xx_priv *rk_priv = priv->priv; int ret; + ret = pm_runtime_force_resume(dev); + if (ret) + return ret; + ret = clk_prepare_enable(pltfm_host->clk); if (ret) return ret; @@ -646,7 +666,55 @@ static int dwcmshc_resume(struct device *dev) } #endif -static SIMPLE_DEV_PM_OPS(dwcmshc_pmops, dwcmshc_suspend, dwcmshc_resume); +#ifdef CONFIG_PM + +static void dwcmshc_enable_card_clk(struct sdhci_host *host) +{ + u16 ctrl; + + ctrl = sdhci_readw(host, SDHCI_CLOCK_CONTROL); + if ((ctrl & SDHCI_CLOCK_INT_EN) && !(ctrl & SDHCI_CLOCK_CARD_EN)) { + ctrl |= SDHCI_CLOCK_CARD_EN; + sdhci_writew(host, ctrl, SDHCI_CLOCK_CONTROL); + } +} + +static void dwcmshc_disable_card_clk(struct sdhci_host *host) +{ + u16 ctrl; + + ctrl = sdhci_readw(host, SDHCI_CLOCK_CONTROL); + if (ctrl & SDHCI_CLOCK_CARD_EN) { + ctrl &= ~SDHCI_CLOCK_CARD_EN; + sdhci_writew(host, ctrl, SDHCI_CLOCK_CONTROL); + } +} + +static int dwcmshc_runtime_suspend(struct device *dev) +{ + struct sdhci_host *host = dev_get_drvdata(dev); + + dwcmshc_disable_card_clk(host); + + return 0; +} + +static int dwcmshc_runtime_resume(struct device *dev) +{ + struct sdhci_host *host = dev_get_drvdata(dev); + + dwcmshc_enable_card_clk(host); + + return 0; +} + +#endif + +static const struct dev_pm_ops dwcmshc_pmops = { + SET_SYSTEM_SLEEP_PM_OPS(dwcmshc_suspend, dwcmshc_resume) + SET_RUNTIME_PM_OPS(dwcmshc_runtime_suspend, + dwcmshc_runtime_resume, NULL) +}; static struct platform_driver sdhci_dwcmshc_driver = { .driver = {