Re: [PATCH v4 2/8] PCI: rzg3s-host: Fix runtime PM handling in the NOIRQ suspend/resume phase

[Lad, Prabhakar]

On Tue, Sep 22, 2026 at 3:57 PM Claudiu Beznea
<claudiu.beznea@xxxxxxxxxx> wrote:
>
> From: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx>
>
> The runtime PM documentation states the following:
> - During system suspend, pm_runtime_get_noresume() is called for every
>   device right before executing the subsystem-level .prepare() callback
>   (in device_prepare()). In addition, the PM core disables runtime PM for
>   every device right before executing the subsystem-level .suspend_late()
>   callback (in device_suspend_late()).
> - During system resume, pm_runtime_enable() is called for every device
>   right after executing the subsystem-level .resume_early() callback (in
>   device_resume_early()), and pm_runtime_put() is called right after
>   executing the subsystem-level .complete() callback (in
>   device_complete()).
>
> The driver's .suspend_noirq() callback is invoked after .suspend_late(),
> while .resume_noirq() is invoked before .resume_early().
>
> If:
> - the device is not part of the wake-up path, and
> - its runtime PM status is not RPM_SUSPENDED,
> the generic power domain .suspend_noirq()/.resume_noirq() callbacks
> (genpd_suspend_noirq()/genpd_resume_noirq()) invoke the driver's
> .suspend_noirq()/.resume_noirq() callbacks and call
> genpd_stop_dev()/genpd_start_dev() before and after them, respectively.
>
> Calling genpd_stop_dev()/genpd_start_dev() allows devices whose power is
> controlled by generic power domains to be powered off and on during
> system suspend and resume, even though their runtime PM usage count does
> not reach zero.
>
> Since the runtime PM usage count is incremented in device_prepare() and
> decremented in device_complete(), runtime PM operations performed from
> the driver's .suspend_noirq()/.resume_noirq() callbacks are no-ops. The
> actual power transitions are handled by the generic power domain
> .suspend_noirq()/.resume_noirq() callbacks.
>
> Moreover, attempting to runtime resume a device while runtime PM is
> disabled may return -EACCES. This may cause system resume to fail when
> resuming after a failed Root Port reset, as described in a subsequent
> patch adding hot-plug support.
>
> Remove the runtime PM calls from the driver's
> .suspend_noirq()/.resume_noirq() callbacks and rely on the generic power
> domain callbacks to power the device off and on.
>
> Fixes: 7ef502fb35b2 ("PCI: Add Renesas RZ/G3S host controller driver")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx>
> ---
>
> Changes in v4:
> - simplified the comments in rzg3s_pcie_suspend_noirq()/rzg3s_pcie_resume_noirq()
>
> Changes in v3:
> - none, this patch is new
>
>  drivers/pci/controller/pcie-rzg3s-host.c | 22 ++++++++++------------
>  1 file changed, 10 insertions(+), 12 deletions(-)
>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
Tested-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>

Cheers,
Prabhakar

> diff --git a/drivers/pci/controller/pcie-rzg3s-host.c b/drivers/pci/controller/pcie-rzg3s-host.c
> index e1105b1f2652..0ef49bb5ab1a 100644
> --- a/drivers/pci/controller/pcie-rzg3s-host.c
> +++ b/drivers/pci/controller/pcie-rzg3s-host.c
> @@ -1961,19 +1961,18 @@ static int rzg3s_pcie_suspend_noirq(struct device *dev)
>         if (ret)
>                 goto config_reinit;
>
> -       ret = pm_runtime_put_sync(dev);
> -       if (ret)
> -               goto power_resets_restore;
> +       /*
> +        * Since the power domain's genpd_suspend_noirq() will disable clocks,
> +        * there is no need to manually invoke runtime PM API here.
> +        */
>
>         ret = rzg3s_sysc_config_func(sysc, RZG3S_SYSC_FUNC_ID_RST_RSM_B, 0);
>         if (ret)
> -               goto rpm_resume;
> +               goto power_resets_restore;
>
>         return 0;
>
>         /* Restore the previous state if any error happens */
> -rpm_resume:
> -       pm_runtime_resume_and_get(dev);
>  power_resets_restore:
>         reset_control_bulk_deassert(data->num_power_resets,
>                                     host->power_resets);
> @@ -2011,13 +2010,14 @@ static int rzg3s_pcie_resume_noirq(struct device *dev)
>                         goto assert_rst_rsm_b;
>         }
>
> -       ret = pm_runtime_resume_and_get(dev);
> -       if (ret)
> -               goto assert_rst_rsm_b;
> +       /*
> +        * Since the power domain's genpd_resume_noirq() will enable clocks,
> +        * there is no need to manually invoke runtime PM API here.
> +        */
>
>         ret = rzg3s_pcie_power_resets_deassert(host);
>         if (ret)
> -               goto rpm_put;
> +               goto assert_rst_rsm_b;
>
>         ret = rzg3s_pcie_host_setup(host, rzg3s_pcie_msi_hw_setup,
>                                     rzg3s_pcie_msi_hw_teardown);
> @@ -2033,8 +2033,6 @@ static int rzg3s_pcie_resume_noirq(struct device *dev)
>  assert_power_resets:
>         reset_control_bulk_assert(data->num_power_resets,
>                                   host->power_resets);
> -rpm_put:
> -       pm_runtime_put_sync(dev);
>  assert_rst_rsm_b:
>         rzg3s_sysc_config_func(sysc, RZG3S_SYSC_FUNC_ID_RST_RSM_B, 0);
>         return ret;
> --
> 2.43.0
>
>