Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20221101024736.1509207-1-shawn.guo@linaro.org> <20221101024736.1509207-2-shawn.guo@linaro.org>
In-Reply-To: <20221101024736.1509207-2-shawn.guo@linaro.org>
From:   Ulf Hansson <ulf.hansson@linaro.org>
Date:   Tue, 1 Nov 2022 11:23:53 +0100
Message-ID: <CAPDyKFoyoMvfLcEKj2OBBky8KZc3jBp8y_UfYqpGH9ETfSJ5yQ@mail.gmail.com>
Subject: Re: [PATCH v2 1/3] PM: domains: Pass generic PM noirq hooks to genpd_finish_suspend()
To:     Shawn Guo <shawn.guo@linaro.org>
Cc:     "Rafael J . Wysocki" <rafael@kernel.org>,
        Kevin Hilman <khilman@kernel.org>,
        Len Brown <len.brown@intel.com>, Pavel Machek <pavel@ucw.cz>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Tue, 1 Nov 2022 at 03:47, Shawn Guo <shawn.guo@linaro.org> wrote:
>
> While argument `poweroff` works fine for genpd_finish_suspend() to handle
> distinction between suspend and poweroff, it won't scale if we want to
> use it for freeze as well.  Pass generic PM noirq hooks as arguments
> instead, so that the function can possibly cover freeze case too.
>
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>

Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>

Kind regards
Uffe


> ---
>  drivers/base/power/domain.c | 25 +++++++++++++------------
>  1 file changed, 13 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 6471b559230e..54f6b0dd35fb 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -1189,12 +1189,15 @@ static int genpd_prepare(struct device *dev)
>   * genpd_finish_suspend - Completion of suspend or hibernation of device in an
>   *   I/O pm domain.
>   * @dev: Device to suspend.
> - * @poweroff: Specifies if this is a poweroff_noirq or suspend_noirq callback.
> + * @suspend_noirq: Generic suspend_noirq callback.
> + * @resume_noirq: Generic resume_noirq callback.
>   *
>   * Stop the device and remove power from the domain if all devices in it have
>   * been stopped.
>   */
> -static int genpd_finish_suspend(struct device *dev, bool poweroff)
> +static int genpd_finish_suspend(struct device *dev,
> +                               int (*suspend_noirq)(struct device *dev),
> +                               int (*resume_noirq)(struct device *dev))
>  {
>         struct generic_pm_domain *genpd;
>         int ret = 0;
> @@ -1203,10 +1206,7 @@ static int genpd_finish_suspend(struct device *dev, bool poweroff)
>         if (IS_ERR(genpd))
>                 return -EINVAL;
>
> -       if (poweroff)
> -               ret = pm_generic_poweroff_noirq(dev);
> -       else
> -               ret = pm_generic_suspend_noirq(dev);
> +       ret = suspend_noirq(dev);
>         if (ret)
>                 return ret;
>
> @@ -1217,10 +1217,7 @@ static int genpd_finish_suspend(struct device *dev, bool poweroff)
>             !pm_runtime_status_suspended(dev)) {
>                 ret = genpd_stop_dev(genpd, dev);
>                 if (ret) {
> -                       if (poweroff)
> -                               pm_generic_restore_noirq(dev);
> -                       else
> -                               pm_generic_resume_noirq(dev);
> +                       resume_noirq(dev);
>                         return ret;
>                 }
>         }
> @@ -1244,7 +1241,9 @@ static int genpd_suspend_noirq(struct device *dev)
>  {
>         dev_dbg(dev, "%s()\n", __func__);
>
> -       return genpd_finish_suspend(dev, false);
> +       return genpd_finish_suspend(dev,
> +                                   pm_generic_suspend_noirq,
> +                                   pm_generic_resume_noirq);
>  }
>
>  /**
> @@ -1353,7 +1352,9 @@ static int genpd_poweroff_noirq(struct device *dev)
>  {
>         dev_dbg(dev, "%s()\n", __func__);
>
> -       return genpd_finish_suspend(dev, true);
> +       return genpd_finish_suspend(dev,
> +                                   pm_generic_poweroff_noirq,
> +                                   pm_generic_restore_noirq);
>  }
>
>  /**
> --
> 2.25.1
>