linux-mmc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Ulf Hansson <ulf.hansson@linaro.org>
To: Tony Lindgren <tony@atomide.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>,
	Chunyan Zhang <zhang.chunyan@linaro.org>,
	Faiz Abbas <faiz_abbas@ti.com>,
	Kishon Vijay Abraham I <kishon@ti.com>,
	Santosh Shilimkar <ssantosh@kernel.org>,
	linux-mmc <linux-mmc@vger.kernel.org>,
	linux-omap <linux-omap@vger.kernel.org>,
	Rob Herring <robh@kernel.org>, DTML <devicetree@vger.kernel.org>
Subject: Re: [PATCH 4/6] mmc: sdhci-omap: Implement PM runtime functions
Date: Tue, 12 Oct 2021 17:16:26 +0200	[thread overview]
Message-ID: <CAPDyKFq_jiYPrm_kcprijFfcceVVGnZpkG+4ZY_XSBXJnCT0LA@mail.gmail.com> (raw)
In-Reply-To: <20211012103750.38328-5-tony@atomide.com>

On Tue, 12 Oct 2021 at 12:38, Tony Lindgren <tony@atomide.com> wrote:
>
> Implement PM runtime functions and enable autosuspend.
>
> 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 | 78 ++++++++++++++++++++++++++++-------
>  1 file changed, 63 insertions(+), 15 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
> @@ -1207,6 +1207,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;
> @@ -1252,6 +1254,7 @@ static int sdhci_omap_probe(struct platform_device *pdev)
>         omap_host->timing = MMC_TIMING_LEGACY;
>         omap_host->flags = data->flags;
>         omap_host->omap_offset = data->omap_offset;
> +       omap_host->con = -EINVAL;
>         host->ioaddr += offset;
>         host->mapbase = regs->start + offset;
>
> @@ -1302,6 +1305,8 @@ static int sdhci_omap_probe(struct platform_device *pdev)
>          * SYSCONFIG register of omap devices. The callback will be invoked
>          * as part of pm_runtime_get_sync.
>          */
> +       pm_runtime_use_autosuspend(dev);
> +       pm_runtime_set_autosuspend_delay(dev, 50);
>         pm_runtime_enable(dev);
>         ret = pm_runtime_resume_and_get(dev);
>         if (ret) {
> @@ -1312,7 +1317,7 @@ static int sdhci_omap_probe(struct platform_device *pdev)
>         ret = sdhci_omap_set_capabilities(host);
>         if (ret) {
>                 dev_err(dev, "failed to set system capabilities\n");
> -               goto err_put_sync;
> +               goto err_rpm_put;
>         }
>
>         host->mmc_host_ops.start_signal_voltage_switch =
> @@ -1340,7 +1345,7 @@ static int sdhci_omap_probe(struct platform_device *pdev)
>
>         ret = sdhci_setup_host(host);
>         if (ret)
> -               goto err_put_sync;
> +               goto err_rpm_put;
>
>         ret = sdhci_omap_config_iodelay_pinctrl_state(omap_host);
>         if (ret)
> @@ -1350,15 +1355,21 @@ static int sdhci_omap_probe(struct platform_device *pdev)
>         if (ret)
>                 goto err_cleanup_host;
>
> +       sdhci_omap_context_save(omap_host);

Calling sdhci_omap_context_save() here looks unnecessary. The device
is already runtime resumed at this point.

In other words, sdhci_omap_context_save() will be called from the
->runtime_suspend() callback, next time the device becomes runtime
suspended. That should be sufficient, right?

> +
> +       pm_runtime_mark_last_busy(dev);
> +       pm_runtime_put_autosuspend(dev);
> +
>         return 0;
>
>  err_cleanup_host:
>         sdhci_cleanup_host(host);
>
> -err_put_sync:
> -       pm_runtime_put_sync(dev);
> -
> +err_rpm_put:
> +       pm_runtime_mark_last_busy(dev);
> +       pm_runtime_put_autosuspend(dev);
>  err_rpm_disable:
> +       pm_runtime_dont_use_autosuspend(dev);
>         pm_runtime_disable(dev);
>
>  err_pltfm_free:
> @@ -1371,8 +1382,12 @@ 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_dont_use_autosuspend(dev);
>         pm_runtime_put_sync(dev);
> +       /* Ensure device gets idled despite userspace sysfs config */
> +       pm_runtime_force_suspend(dev);
>         pm_runtime_disable(dev);

The call to pm_runtime_disable() can be removed, as that is taken care
of in pm_runtime_force_suspend().

>         sdhci_pltfm_free(pdev);
>
> @@ -1402,42 +1417,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_runtime_suspend_host(host);
>
>         sdhci_omap_context_save(omap_host);
>
>         pinctrl_pm_select_idle_state(dev);
>
> -       pm_runtime_force_suspend(dev);
> -
>         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->con != -EINVAL)
> +               sdhci_omap_context_restore(omap_host);
> +
> +       sdhci_runtime_resume_host(host, 0);
> +
> +       return 0;
> +}
> +
> +static int __maybe_unused sdhci_omap_suspend(struct device *dev)
> +{
> +       struct sdhci_host *host = dev_get_drvdata(dev);
> +       int err;
> +
> +       /* Enable for configuring wakeups, paired in resume */
> +       err = pm_runtime_resume_and_get(dev);
> +       if (err < 0)
> +               return err;
> +
> +       sdhci_suspend_host(host);

As far as I can tell, sdhci_suspend_host() doesn't really make sense
for the omap variant. What you need, is to put the device into the
same low power state as "runtime suspend", that should be sufficient.

The system wakeup will be armed (and later then disarmed) by the PM
core, when it calls device_wakeup_arm_wake_irqs() from the
dpm_suspend_noirq() phase.

In other words, pointing the system suspend/resume callbacks to
pm_runtime_force_suspend|resume() should work fine, I think.

> +
> +       return pm_runtime_force_suspend(dev);
> +}
> +
> +static int __maybe_unused sdhci_omap_resume(struct device *dev)
> +{
> +       struct sdhci_host *host = dev_get_drvdata(dev);
> +       int err;
> +
> +       err = pm_runtime_force_resume(dev);
> +       if (err < 0)
> +               dev_warn(dev, "force resume failed: %i\n", err);
>
>         sdhci_resume_host(host);
>
> +       /* Balance pm_runtime_resume_and_get() done in suspend */
> +       pm_runtime_put(dev);
> +
>         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

  parent reply	other threads:[~2021-10-12 15:17 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-12 10:37 [PATCHv3 0/6] More SoCs for sdhci-omap to deprecate omap_hsmmc Tony Lindgren
2021-10-12 10:37 ` [PATCH 1/6] dt-bindings: sdhci-omap: Update binding for legacy SoCs Tony Lindgren
2021-10-12 10:37 ` [PATCH 2/6] mmc: sdhci-omap: Handle voltages to add support omap4 Tony Lindgren
2021-10-12 14:16   ` kernel test robot
2021-10-12 10:37 ` [PATCH 3/6] mmc: sdhci-omap: Add omap_offset to support omap3 and earlier Tony Lindgren
2021-10-12 10:37 ` [PATCH 4/6] mmc: sdhci-omap: Implement PM runtime functions Tony Lindgren
2021-10-12 15:11   ` kernel test robot
2021-10-12 15:16   ` Ulf Hansson [this message]
2021-10-15  9:34     ` Tony Lindgren
2021-10-12 10:37 ` [PATCH 5/6] sdhci: omap: Enable aggressive PM Tony Lindgren
2021-10-12 14:14   ` Ulf Hansson
2021-10-12 10:37 ` [PATCH 6/6] mmc: sdhci-omap: Configure optional wakeirq Tony Lindgren
2021-10-12 15:02   ` Ulf Hansson
2021-10-15 10:47 [PATCHv4 0/6] More SoCs for sdhci-omap to deprecate omap_hsmmc Tony Lindgren
2021-10-15 10:47 ` [PATCH 4/6] mmc: sdhci-omap: Implement PM runtime functions Tony Lindgren

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAPDyKFq_jiYPrm_kcprijFfcceVVGnZpkG+4ZY_XSBXJnCT0LA@mail.gmail.com \
    --to=ulf.hansson@linaro.org \
    --cc=adrian.hunter@intel.com \
    --cc=devicetree@vger.kernel.org \
    --cc=faiz_abbas@ti.com \
    --cc=kishon@ti.com \
    --cc=linux-mmc@vger.kernel.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=robh@kernel.org \
    --cc=ssantosh@kernel.org \
    --cc=tony@atomide.com \
    --cc=zhang.chunyan@linaro.org \
    --subject='Re: [PATCH 4/6] mmc: sdhci-omap: Implement PM runtime functions' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).