Re: [PATCH v2 1/3] PM: domains: Pass generic PM noirq hooks to genpd_finish_suspend()

[Ulf Hansson]

On Tue, 1 Nov 2022 at 03:47, Shawn Guo <shawn.guo@xxxxxxxxxx> 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@xxxxxxxxxx>

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

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
>