diff mbox series

[4/5] mmc: sdhci-omap: Implement PM runtime functions

Message ID 20210930065733.31943-5-tony@atomide.com (mailing list archive)
State New, archived
Headers show
Series More SoCs for sdhci-omap to deprecate omap_hsmmc | expand

Commit Message

Tony Lindgren Sept. 30, 2021, 6:57 a.m. UTC
Implement PM runtime functions and enable MMC_CAP_AGGRESSIVE_PM.

Note that we save context in probe to avoid restoring invalid context
on the first resume. For system suspend, we have the new PM runtime
functions do most of the work.

Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 drivers/mmc/host/sdhci-omap.c | 66 +++++++++++++++++++++++++++++------
 1 file changed, 56 insertions(+), 10 deletions(-)

Comments

Ulf Hansson Oct. 8, 2021, 2:43 p.m. UTC | #1
On Thu, 30 Sept 2021 at 08:57, Tony Lindgren <tony@atomide.com> wrote:
>
> Implement PM runtime functions and enable MMC_CAP_AGGRESSIVE_PM.

I suggest you split this change into two pieces. MMC_CAP_AGGRESSIVE_PM
is about enabling runtime PM management for the eMMC/SD card device,
which is perfectly fine to use independently of whether runtime PM is
supported for the host device.

>
> Note that we save context in probe to avoid restoring invalid context
> on the first resume. For system suspend, we have the new PM runtime
> functions do most of the work.
>
> Signed-off-by: Tony Lindgren <tony@atomide.com>
> ---
>  drivers/mmc/host/sdhci-omap.c | 66 +++++++++++++++++++++++++++++------
>  1 file changed, 56 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-omap.c b/drivers/mmc/host/sdhci-omap.c
> --- a/drivers/mmc/host/sdhci-omap.c
> +++ b/drivers/mmc/host/sdhci-omap.c
> @@ -117,6 +117,9 @@ struct sdhci_omap_host {
>
>         struct pinctrl          *pinctrl;
>         struct pinctrl_state    **pinctrl_state;
> +       unsigned long           context_valid:1;
> +       unsigned long           is_runtime_suspended:1;
> +       unsigned long           needs_resume:1;
>         bool                    is_tuning;
>
>         /* Offset for omap specific registers from base */
> @@ -1207,6 +1210,8 @@ static const struct soc_device_attribute sdhci_omap_soc_devices[] = {
>         }
>  };
>
> +static void sdhci_omap_context_save(struct sdhci_omap_host *omap_host);
> +
>  static int sdhci_omap_probe(struct platform_device *pdev)
>  {
>         int ret;
> @@ -1338,6 +1343,8 @@ static int sdhci_omap_probe(struct platform_device *pdev)
>         /* R1B responses is required to properly manage HW busy detection. */
>         mmc->caps |= MMC_CAP_NEED_RSP_BUSY;
>
> +       mmc->caps |= MMC_CAP_AGGRESSIVE_PM;
> +
>         ret = sdhci_setup_host(host);
>         if (ret)
>                 goto err_put_sync;
> @@ -1350,6 +1357,11 @@ static int sdhci_omap_probe(struct platform_device *pdev)
>         if (ret)
>                 goto err_cleanup_host;
>
> +       sdhci_omap_context_save(omap_host);
> +       omap_host->context_valid = 1;

Looks like you can remove this flag, it's not being used.

> +
> +       pm_runtime_put_sync(dev);

I recommend to use the PM runtime autosuspend feature, as to avoid an
initial latency for every I/O request to the host driver. The mmc core
already supports that, see mmc_release_host().

The typical default timeout value for autosuspend, is usually set
~50-200ms, by host drivers (if I recall correctly).

> +
>         return 0;
>
>  err_cleanup_host:
> @@ -1371,6 +1383,7 @@ static int sdhci_omap_remove(struct platform_device *pdev)
>         struct device *dev = &pdev->dev;
>         struct sdhci_host *host = platform_get_drvdata(pdev);
>
> +       pm_runtime_get_sync(dev);
>         sdhci_remove_host(host, true);
>         pm_runtime_put_sync(dev);

There is no guarantee that this triggers a call to
->sdhci_omap_runtime_suspend(), which I guess is what we want.
Userspace via sysfs may have increase the RPM usage count
(pm_runtime_forbid(), for example.

To address this, I would call pm_runtime_disable() first and then
explicitly put the device into low power state, rather than relying on
runtime PM to do it. Another option could be to use
pm_runtime_force_suspend().

>         pm_runtime_disable(dev);
> @@ -1402,42 +1415,75 @@ static void sdhci_omap_context_restore(struct sdhci_omap_host *omap_host)
>         sdhci_omap_writel(omap_host, SDHCI_OMAP_ISE, omap_host->ise);
>  }
>
> -static int __maybe_unused sdhci_omap_suspend(struct device *dev)
> +static int __maybe_unused sdhci_omap_runtime_suspend(struct device *dev)
>  {
>         struct sdhci_host *host = dev_get_drvdata(dev);
>         struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>         struct sdhci_omap_host *omap_host = sdhci_pltfm_priv(pltfm_host);
>
> -       sdhci_suspend_host(host);
> -

Shouldn't you call sdhci_runtime_suspend_host() somewhere here?

>         sdhci_omap_context_save(omap_host);
>
>         pinctrl_pm_select_idle_state(dev);
>
> -       pm_runtime_force_suspend(dev);
> +       omap_host->is_runtime_suspended = 1;
>
>         return 0;
>  }
>
> -static int __maybe_unused sdhci_omap_resume(struct device *dev)
> +static int __maybe_unused sdhci_omap_runtime_resume(struct device *dev)
>  {
>         struct sdhci_host *host = dev_get_drvdata(dev);
>         struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>         struct sdhci_omap_host *omap_host = sdhci_pltfm_priv(pltfm_host);
>
> -       pm_runtime_force_resume(dev);
> -
>         pinctrl_pm_select_default_state(dev);
>
> -       sdhci_omap_context_restore(omap_host);
> +       if (omap_host->context_valid)
> +               sdhci_omap_context_restore(omap_host);
> +
> +       omap_host->is_runtime_suspended = 0;

Shouldn't you call sdhci_runtime_resume_host() somewhere here?

> +
> +       return 0;
> +}
> +
> +static int __maybe_unused sdhci_omap_suspend(struct device *dev)
> +{
> +       struct sdhci_host *host = dev_get_drvdata(dev);
> +       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +       struct sdhci_omap_host *omap_host = sdhci_pltfm_priv(pltfm_host);
> +
> +       if (omap_host->is_runtime_suspended)
> +               return 0;

So if the host is already runtime suspended, it's okay to just leave it as is?

In a way that sounds like you could call pm_runtime_force_suspend()
instead, assuming the sdhci_omap_runtime_suspend() can be extended to
do the right thing for system suspend as well.

It looks a bit odd that sdhci_suspend_host() is called only when the
host is runtime resumed. Perhaps you can elaborate a bit more on why
this is, so I can understand better what you want to achieve here.

> +
> +       sdhci_suspend_host(host);
> +       sdhci_omap_runtime_suspend(dev);
> +       omap_host->needs_resume = 1;
>
> +       return 0;
> +}
> +
> +static int __maybe_unused sdhci_omap_resume(struct device *dev)
> +{
> +       struct sdhci_host *host = dev_get_drvdata(dev);
> +       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +       struct sdhci_omap_host *omap_host = sdhci_pltfm_priv(pltfm_host);
> +
> +       if (!omap_host->needs_resume)
> +               return 0;
> +
> +       sdhci_omap_runtime_resume(dev);
>         sdhci_resume_host(host);
> +       omap_host->needs_resume = 0;
>
>         return 0;
>  }
>  #endif
> -static SIMPLE_DEV_PM_OPS(sdhci_omap_dev_pm_ops, sdhci_omap_suspend,
> -                        sdhci_omap_resume);
> +
> +static const struct dev_pm_ops sdhci_omap_dev_pm_ops = {
> +       SET_RUNTIME_PM_OPS(sdhci_omap_runtime_suspend,
> +                          sdhci_omap_runtime_resume, NULL)
> +       SET_SYSTEM_SLEEP_PM_OPS(sdhci_omap_suspend, sdhci_omap_resume)
> +};
>
>  static struct platform_driver sdhci_omap_driver = {
>         .probe = sdhci_omap_probe,
> --
> 2.33.0

Kind regards
Uffe
Tony Lindgren Oct. 11, 2021, 5:23 a.m. UTC | #2
* Ulf Hansson <ulf.hansson@linaro.org> [211008 14:44]:
> On Thu, 30 Sept 2021 at 08:57, Tony Lindgren <tony@atomide.com> wrote:
> >
> > Implement PM runtime functions and enable MMC_CAP_AGGRESSIVE_PM.
> 
> I suggest you split this change into two pieces. MMC_CAP_AGGRESSIVE_PM
> is about enabling runtime PM management for the eMMC/SD card device,
> which is perfectly fine to use independently of whether runtime PM is
> supported for the host device.

OK

> > @@ -1350,6 +1357,11 @@ static int sdhci_omap_probe(struct platform_device *pdev)
> >         if (ret)
> >                 goto err_cleanup_host;
> >
> > +       sdhci_omap_context_save(omap_host);
> > +       omap_host->context_valid = 1;
> 
> Looks like you can remove this flag, it's not being used.

Hmm I think it is needed as otherwise we end up trying to restore
an invalid context on probe on the first pm_runtime_get(). Do you
have some nicer solution for that in mind?

> > +
> > +       pm_runtime_put_sync(dev);
> 
> I recommend to use the PM runtime autosuspend feature, as to avoid an
> initial latency for every I/O request to the host driver. The mmc core
> already supports that, see mmc_release_host().
> 
> The typical default timeout value for autosuspend, is usually set
> ~50-200ms, by host drivers (if I recall correctly).

OK I have a patch to also enable autosuspend too, I'll add that
too for the next revision.

> > @@ -1371,6 +1383,7 @@ static int sdhci_omap_remove(struct platform_device *pdev)
> >         struct device *dev = &pdev->dev;
> >         struct sdhci_host *host = platform_get_drvdata(pdev);
> >
> > +       pm_runtime_get_sync(dev);
> >         sdhci_remove_host(host, true);
> >         pm_runtime_put_sync(dev);
> 
> There is no guarantee that this triggers a call to
> ->sdhci_omap_runtime_suspend(), which I guess is what we want.
> Userspace via sysfs may have increase the RPM usage count
> (pm_runtime_forbid(), for example.
> 
> To address this, I would call pm_runtime_disable() first and then
> explicitly put the device into low power state, rather than relying on
> runtime PM to do it. Another option could be to use
> pm_runtime_force_suspend().

OK I'll take a look.

> > @@ -1402,42 +1415,75 @@ static void sdhci_omap_context_restore(struct sdhci_omap_host *omap_host)
> >         sdhci_omap_writel(omap_host, SDHCI_OMAP_ISE, omap_host->ise);
> >  }
> >
> > -static int __maybe_unused sdhci_omap_suspend(struct device *dev)
> > +static int __maybe_unused sdhci_omap_runtime_suspend(struct device *dev)
> >  {
> >         struct sdhci_host *host = dev_get_drvdata(dev);
> >         struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> >         struct sdhci_omap_host *omap_host = sdhci_pltfm_priv(pltfm_host);
> >
> > -       sdhci_suspend_host(host);
> > -
> 
> Shouldn't you call sdhci_runtime_suspend_host() somewhere here?

I'm pretty sure I tried, but runtime resume did not seem to work after
doing that.. I'll take a look again.

> > +static int __maybe_unused sdhci_omap_suspend(struct device *dev)
> > +{
> > +       struct sdhci_host *host = dev_get_drvdata(dev);
> > +       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> > +       struct sdhci_omap_host *omap_host = sdhci_pltfm_priv(pltfm_host);
> > +
> > +       if (omap_host->is_runtime_suspended)
> > +               return 0;
> 
> So if the host is already runtime suspended, it's okay to just leave it as is?

Ideally yeah there should not be anything left to do for suspesnd at
that point. But sounds like I may be missing something.

> In a way that sounds like you could call pm_runtime_force_suspend()
> instead, assuming the sdhci_omap_runtime_suspend() can be extended to
> do the right thing for system suspend as well.

OK I'll check.

> It looks a bit odd that sdhci_suspend_host() is called only when the
> host is runtime resumed. Perhaps you can elaborate a bit more on why
> this is, so I can understand better what you want to achieve here.

I guess I'm not clear on what's left for sdhci_suspend_host() to do if
the host is already runtime suspended :)

Regards,

Tony
Ulf Hansson Oct. 12, 2021, 9:05 a.m. UTC | #3
On Mon, 11 Oct 2021 at 07:23, Tony Lindgren <tony@atomide.com> wrote:
>
> * Ulf Hansson <ulf.hansson@linaro.org> [211008 14:44]:
> > On Thu, 30 Sept 2021 at 08:57, Tony Lindgren <tony@atomide.com> wrote:
> > >
> > > Implement PM runtime functions and enable MMC_CAP_AGGRESSIVE_PM.
> >
> > I suggest you split this change into two pieces. MMC_CAP_AGGRESSIVE_PM
> > is about enabling runtime PM management for the eMMC/SD card device,
> > which is perfectly fine to use independently of whether runtime PM is
> > supported for the host device.
>
> OK
>
> > > @@ -1350,6 +1357,11 @@ static int sdhci_omap_probe(struct platform_device *pdev)
> > >         if (ret)
> > >                 goto err_cleanup_host;
> > >
> > > +       sdhci_omap_context_save(omap_host);
> > > +       omap_host->context_valid = 1;
> >
> > Looks like you can remove this flag, it's not being used.
>
> Hmm I think it is needed as otherwise we end up trying to restore
> an invalid context on probe on the first pm_runtime_get(). Do you
> have some nicer solution for that in mind?

Right, I didn't notice that, my apologies.

In any case, an option is to bring the device into full power, without
calling pm_runtime_resume_and_get() from ->probe(). In principle,
running the same operations as the ->runtime_resume() callback does,
except for restoring the context then. When this is done, the
following calls to runtime PM should do the trick (I extended it to
support autosuspend as well):

pm_runtime_get_noresume()
pm_runtime_set_active()
pm_runtime_set_autosuspend_delay()
pm_runtime_use_autosuspend()
pm_runtime_enable()

Note that, this means that the omaps PM domain's ->runtime_resume()
callback doesn't get invoked when powering on the device for the first
time. Can this be a problem?

>
> > > +
> > > +       pm_runtime_put_sync(dev);
> >
> > I recommend to use the PM runtime autosuspend feature, as to avoid an
> > initial latency for every I/O request to the host driver. The mmc core
> > already supports that, see mmc_release_host().
> >
> > The typical default timeout value for autosuspend, is usually set
> > ~50-200ms, by host drivers (if I recall correctly).
>
> OK I have a patch to also enable autosuspend too, I'll add that
> too for the next revision.
>
> > > @@ -1371,6 +1383,7 @@ static int sdhci_omap_remove(struct platform_device *pdev)
> > >         struct device *dev = &pdev->dev;
> > >         struct sdhci_host *host = platform_get_drvdata(pdev);
> > >
> > > +       pm_runtime_get_sync(dev);
> > >         sdhci_remove_host(host, true);
> > >         pm_runtime_put_sync(dev);
> >
> > There is no guarantee that this triggers a call to
> > ->sdhci_omap_runtime_suspend(), which I guess is what we want.
> > Userspace via sysfs may have increase the RPM usage count
> > (pm_runtime_forbid(), for example.
> >
> > To address this, I would call pm_runtime_disable() first and then
> > explicitly put the device into low power state, rather than relying on
> > runtime PM to do it. Another option could be to use
> > pm_runtime_force_suspend().
>
> OK I'll take a look.
>
> > > @@ -1402,42 +1415,75 @@ static void sdhci_omap_context_restore(struct sdhci_omap_host *omap_host)
> > >         sdhci_omap_writel(omap_host, SDHCI_OMAP_ISE, omap_host->ise);
> > >  }
> > >
> > > -static int __maybe_unused sdhci_omap_suspend(struct device *dev)
> > > +static int __maybe_unused sdhci_omap_runtime_suspend(struct device *dev)
> > >  {
> > >         struct sdhci_host *host = dev_get_drvdata(dev);
> > >         struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> > >         struct sdhci_omap_host *omap_host = sdhci_pltfm_priv(pltfm_host);
> > >
> > > -       sdhci_suspend_host(host);
> > > -
> >
> > Shouldn't you call sdhci_runtime_suspend_host() somewhere here?
>
> I'm pretty sure I tried, but runtime resume did not seem to work after
> doing that.. I'll take a look again.
>
> > > +static int __maybe_unused sdhci_omap_suspend(struct device *dev)
> > > +{
> > > +       struct sdhci_host *host = dev_get_drvdata(dev);
> > > +       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> > > +       struct sdhci_omap_host *omap_host = sdhci_pltfm_priv(pltfm_host);
> > > +
> > > +       if (omap_host->is_runtime_suspended)
> > > +               return 0;
> >
> > So if the host is already runtime suspended, it's okay to just leave it as is?
>
> Ideally yeah there should not be anything left to do for suspesnd at
> that point. But sounds like I may be missing something.
>
> > In a way that sounds like you could call pm_runtime_force_suspend()
> > instead, assuming the sdhci_omap_runtime_suspend() can be extended to
> > do the right thing for system suspend as well.
>
> OK I'll check.
>
> > It looks a bit odd that sdhci_suspend_host() is called only when the
> > host is runtime resumed. Perhaps you can elaborate a bit more on why
> > this is, so I can understand better what you want to achieve here.
>
> I guess I'm not clear on what's left for sdhci_suspend_host() to do if
> the host is already runtime suspended :)

I think what boils down to that is that, sdhci_suspend|resume_host()
adds some special treatment for system wakeups (for SDIO irqs). I am
not sure whether you may need that.

Some host drivers doesn't use sdhci_suspend|resume_host, but sticks to
the sdhci_runtime_suspend|resume()_host() functions. Like
sdhci-sprd.c, for example.

Kind regards
Uffe
Tony Lindgren Oct. 12, 2021, 9:18 a.m. UTC | #4
* Ulf Hansson <ulf.hansson@linaro.org> [211012 09:07]:
> On Mon, 11 Oct 2021 at 07:23, Tony Lindgren <tony@atomide.com> wrote:
> >
> > * Ulf Hansson <ulf.hansson@linaro.org> [211008 14:44]:
> > > On Thu, 30 Sept 2021 at 08:57, Tony Lindgren <tony@atomide.com> wrote:
> > > >
> > > > Implement PM runtime functions and enable MMC_CAP_AGGRESSIVE_PM.
> > >
> > > I suggest you split this change into two pieces. MMC_CAP_AGGRESSIVE_PM
> > > is about enabling runtime PM management for the eMMC/SD card device,
> > > which is perfectly fine to use independently of whether runtime PM is
> > > supported for the host device.
> >
> > OK
> >
> > > > @@ -1350,6 +1357,11 @@ static int sdhci_omap_probe(struct platform_device *pdev)
> > > >         if (ret)
> > > >                 goto err_cleanup_host;
> > > >
> > > > +       sdhci_omap_context_save(omap_host);
> > > > +       omap_host->context_valid = 1;
> > >
> > > Looks like you can remove this flag, it's not being used.
> >
> > Hmm I think it is needed as otherwise we end up trying to restore
> > an invalid context on probe on the first pm_runtime_get(). Do you
> > have some nicer solution for that in mind?
> 
> Right, I didn't notice that, my apologies.
> 
> In any case, an option is to bring the device into full power, without
> calling pm_runtime_resume_and_get() from ->probe(). In principle,
> running the same operations as the ->runtime_resume() callback does,
> except for restoring the context then. When this is done, the
> following calls to runtime PM should do the trick (I extended it to
> support autosuspend as well):
> 
> pm_runtime_get_noresume()
> pm_runtime_set_active()
> pm_runtime_set_autosuspend_delay()
> pm_runtime_use_autosuspend()
> pm_runtime_enable()
> 
> Note that, this means that the omaps PM domain's ->runtime_resume()
> callback doesn't get invoked when powering on the device for the first
> time. Can this be a problem?

Yeah I think that would be a problem as the register access won't work :)

I changed things around to initialize omap_host->con = -ENODEV and then
check it on resume. Not an ideal solution but avoids the extra flag.

> > > > @@ -1402,42 +1415,75 @@ static void sdhci_omap_context_restore(struct sdhci_omap_host *omap_host)
> > > >         sdhci_omap_writel(omap_host, SDHCI_OMAP_ISE, omap_host->ise);
> > > >  }
> > > >
> > > > -static int __maybe_unused sdhci_omap_suspend(struct device *dev)
> > > > +static int __maybe_unused sdhci_omap_runtime_suspend(struct device *dev)
> > > >  {
> > > >         struct sdhci_host *host = dev_get_drvdata(dev);
> > > >         struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> > > >         struct sdhci_omap_host *omap_host = sdhci_pltfm_priv(pltfm_host);
> > > >
> > > > -       sdhci_suspend_host(host);
> > > > -
> > >
> > > Shouldn't you call sdhci_runtime_suspend_host() somewhere here?
> >
> > I'm pretty sure I tried, but runtime resume did not seem to work after
> > doing that.. I'll take a look again.

Confusion solved for this one and it's working now.. Maybe I tried
calling sdhci_suspend_host() instead of sdhci_runtime_suspend_host()
earlier who knows.

> > > It looks a bit odd that sdhci_suspend_host() is called only when the
> > > host is runtime resumed. Perhaps you can elaborate a bit more on why
> > > this is, so I can understand better what you want to achieve here.
> >
> > I guess I'm not clear on what's left for sdhci_suspend_host() to do if
> > the host is already runtime suspended :)
> 
> I think what boils down to that is that, sdhci_suspend|resume_host()
> adds some special treatment for system wakeups (for SDIO irqs). I am
> not sure whether you may need that.

OK, will check.

> Some host drivers doesn't use sdhci_suspend|resume_host, but sticks to
> the sdhci_runtime_suspend|resume()_host() functions. Like
> sdhci-sprd.c, for example.

Hmm so is there some helper to figure out if the mmc host is active
and has a card?

Seems we could skip sdhci_suspend/resume for the inactive mmc host
instances.

Regards,

Tony
diff mbox series

Patch

diff --git a/drivers/mmc/host/sdhci-omap.c b/drivers/mmc/host/sdhci-omap.c
--- a/drivers/mmc/host/sdhci-omap.c
+++ b/drivers/mmc/host/sdhci-omap.c
@@ -117,6 +117,9 @@  struct sdhci_omap_host {
 
 	struct pinctrl		*pinctrl;
 	struct pinctrl_state	**pinctrl_state;
+	unsigned long		context_valid:1;
+	unsigned long		is_runtime_suspended:1;
+	unsigned long		needs_resume:1;
 	bool			is_tuning;
 
 	/* Offset for omap specific registers from base */
@@ -1207,6 +1210,8 @@  static const struct soc_device_attribute sdhci_omap_soc_devices[] = {
 	}
 };
 
+static void sdhci_omap_context_save(struct sdhci_omap_host *omap_host);
+
 static int sdhci_omap_probe(struct platform_device *pdev)
 {
 	int ret;
@@ -1338,6 +1343,8 @@  static int sdhci_omap_probe(struct platform_device *pdev)
 	/* R1B responses is required to properly manage HW busy detection. */
 	mmc->caps |= MMC_CAP_NEED_RSP_BUSY;
 
+	mmc->caps |= MMC_CAP_AGGRESSIVE_PM;
+
 	ret = sdhci_setup_host(host);
 	if (ret)
 		goto err_put_sync;
@@ -1350,6 +1357,11 @@  static int sdhci_omap_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_cleanup_host;
 
+	sdhci_omap_context_save(omap_host);
+	omap_host->context_valid = 1;
+
+	pm_runtime_put_sync(dev);
+
 	return 0;
 
 err_cleanup_host:
@@ -1371,6 +1383,7 @@  static int sdhci_omap_remove(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct sdhci_host *host = platform_get_drvdata(pdev);
 
+	pm_runtime_get_sync(dev);
 	sdhci_remove_host(host, true);
 	pm_runtime_put_sync(dev);
 	pm_runtime_disable(dev);
@@ -1402,42 +1415,75 @@  static void sdhci_omap_context_restore(struct sdhci_omap_host *omap_host)
 	sdhci_omap_writel(omap_host, SDHCI_OMAP_ISE, omap_host->ise);
 }
 
-static int __maybe_unused sdhci_omap_suspend(struct device *dev)
+static int __maybe_unused sdhci_omap_runtime_suspend(struct device *dev)
 {
 	struct sdhci_host *host = dev_get_drvdata(dev);
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
 	struct sdhci_omap_host *omap_host = sdhci_pltfm_priv(pltfm_host);
 
-	sdhci_suspend_host(host);
-
 	sdhci_omap_context_save(omap_host);
 
 	pinctrl_pm_select_idle_state(dev);
 
-	pm_runtime_force_suspend(dev);
+	omap_host->is_runtime_suspended = 1;
 
 	return 0;
 }
 
-static int __maybe_unused sdhci_omap_resume(struct device *dev)
+static int __maybe_unused sdhci_omap_runtime_resume(struct device *dev)
 {
 	struct sdhci_host *host = dev_get_drvdata(dev);
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
 	struct sdhci_omap_host *omap_host = sdhci_pltfm_priv(pltfm_host);
 
-	pm_runtime_force_resume(dev);
-
 	pinctrl_pm_select_default_state(dev);
 
-	sdhci_omap_context_restore(omap_host);
+	if (omap_host->context_valid)
+		sdhci_omap_context_restore(omap_host);
+
+	omap_host->is_runtime_suspended = 0;
+
+	return 0;
+}
+
+static int __maybe_unused sdhci_omap_suspend(struct device *dev)
+{
+	struct sdhci_host *host = dev_get_drvdata(dev);
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_omap_host *omap_host = sdhci_pltfm_priv(pltfm_host);
+
+	if (omap_host->is_runtime_suspended)
+		return 0;
+
+	sdhci_suspend_host(host);
+	sdhci_omap_runtime_suspend(dev);
+	omap_host->needs_resume = 1;
 
+	return 0;
+}
+
+static int __maybe_unused sdhci_omap_resume(struct device *dev)
+{
+	struct sdhci_host *host = dev_get_drvdata(dev);
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_omap_host *omap_host = sdhci_pltfm_priv(pltfm_host);
+
+	if (!omap_host->needs_resume)
+		return 0;
+
+	sdhci_omap_runtime_resume(dev);
 	sdhci_resume_host(host);
+	omap_host->needs_resume = 0;
 
 	return 0;
 }
 #endif
-static SIMPLE_DEV_PM_OPS(sdhci_omap_dev_pm_ops, sdhci_omap_suspend,
-			 sdhci_omap_resume);
+
+static const struct dev_pm_ops sdhci_omap_dev_pm_ops = {
+	SET_RUNTIME_PM_OPS(sdhci_omap_runtime_suspend,
+			   sdhci_omap_runtime_resume, NULL)
+	SET_SYSTEM_SLEEP_PM_OPS(sdhci_omap_suspend, sdhci_omap_resume)
+};
 
 static struct platform_driver sdhci_omap_driver = {
 	.probe = sdhci_omap_probe,