[PATCH v1] clk: tegra: Adjust callbacks in tegra_clock_pm

Rafael J. Wysocki posted 1 patch 1 month ago
There is a newer version of this series
drivers/clk/tegra/clk-device.c |    2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH v1] clk: tegra: Adjust callbacks in tegra_clock_pm
Posted by Rafael J. Wysocki 1 month ago
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

System suspend and resume callbacks run after the core has bumped
up the runtime PM usage counters of all devices, so these callbacks
need not worry about runtime PM reference counting.

Accordingly, to eliminate useless overhead related to runtime PM
usage counter manipulation, set the suspend callback pointer in
tegra_clock_pm to pm_runtime_resume() and do not set the resume
callback in it at all.

This will also facilitate a planned change of the pm_runtime_put()
return type to void in the future.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

This patch is requisite for converting pm_runtime_put() into a void
function.

If you decide to pick it up, please let me know.

Otherwise, an ACK or equivalent will be appreciated, but also the lack
of specific criticism will be eventually regarded as consent.

---
 drivers/clk/tegra/clk-device.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/drivers/clk/tegra/clk-device.c
+++ b/drivers/clk/tegra/clk-device.c
@@ -175,7 +175,7 @@ unreg_clk:
  * perspective since voltage is kept at a nominal level during suspend anyways.
  */
 static const struct dev_pm_ops tegra_clock_pm = {
-	SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_resume_and_get, pm_runtime_put)
+	SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_resume, NULL)
 };
 
 static const struct of_device_id tegra_clock_match[] = {
Re: [PATCH v1] clk: tegra: Adjust callbacks in tegra_clock_pm
Posted by Jon Hunter 1 month ago
Hi Rafael,

On 04/01/2026 11:53, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> System suspend and resume callbacks run after the core has bumped
> up the runtime PM usage counters of all devices, so these callbacks
> need not worry about runtime PM reference counting.
> 
> Accordingly, to eliminate useless overhead related to runtime PM
> usage counter manipulation, set the suspend callback pointer in
> tegra_clock_pm to pm_runtime_resume() and do not set the resume
> callback in it at all.
> 
> This will also facilitate a planned change of the pm_runtime_put()
> return type to void in the future.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> 
> This patch is requisite for converting pm_runtime_put() into a void
> function.
> 
> If you decide to pick it up, please let me know.
> 
> Otherwise, an ACK or equivalent will be appreciated, but also the lack
> of specific criticism will be eventually regarded as consent.
> 
> ---
>   drivers/clk/tegra/clk-device.c |    2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> --- a/drivers/clk/tegra/clk-device.c
> +++ b/drivers/clk/tegra/clk-device.c
> @@ -175,7 +175,7 @@ unreg_clk:
>    * perspective since voltage is kept at a nominal level during suspend anyways.
>    */
>   static const struct dev_pm_ops tegra_clock_pm = {
> -	SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_resume_and_get, pm_runtime_put)
> +	SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_resume, NULL)
>   };
>   
>   static const struct of_device_id tegra_clock_match[] = {


I gave this a quick test and this is causing a suspend regression on
Tegra20 and Tegra30 that use this driver. Looking at the console log
on Tegra20 I see the following errors ...

  tegra-clock tegra_clk_sclk: PM: dpm_run_callback(): pm_runtime_resume returns 1
  tegra-clock tegra_clk_sclk: PM: failed to suspend: error 1
  PM: Some devices failed to suspend, or early wake event detected

Jon

-- 
nvpublic
Re: [PATCH v1] clk: tegra: Adjust callbacks in tegra_clock_pm
Posted by Rafael J. Wysocki 1 month ago
Hi Jon,

On Tue, Jan 6, 2026 at 11:36 AM Jon Hunter <jonathanh@nvidia.com> wrote:
>
> Hi Rafael,
>
> On 04/01/2026 11:53, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >
> > System suspend and resume callbacks run after the core has bumped
> > up the runtime PM usage counters of all devices, so these callbacks
> > need not worry about runtime PM reference counting.
> >
> > Accordingly, to eliminate useless overhead related to runtime PM
> > usage counter manipulation, set the suspend callback pointer in
> > tegra_clock_pm to pm_runtime_resume() and do not set the resume
> > callback in it at all.
> >
> > This will also facilitate a planned change of the pm_runtime_put()
> > return type to void in the future.
> >
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > ---
> >
> > This patch is requisite for converting pm_runtime_put() into a void
> > function.
> >
> > If you decide to pick it up, please let me know.
> >
> > Otherwise, an ACK or equivalent will be appreciated, but also the lack
> > of specific criticism will be eventually regarded as consent.
> >
> > ---
> >   drivers/clk/tegra/clk-device.c |    2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > --- a/drivers/clk/tegra/clk-device.c
> > +++ b/drivers/clk/tegra/clk-device.c
> > @@ -175,7 +175,7 @@ unreg_clk:
> >    * perspective since voltage is kept at a nominal level during suspend anyways.
> >    */
> >   static const struct dev_pm_ops tegra_clock_pm = {
> > -     SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_resume_and_get, pm_runtime_put)
> > +     SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_resume, NULL)
> >   };
> >
> >   static const struct of_device_id tegra_clock_match[] = {
>
>
> I gave this a quick test and this is causing a suspend regression on
> Tegra20 and Tegra30 that use this driver. Looking at the console log
> on Tegra20 I see the following errors ...
>
>   tegra-clock tegra_clk_sclk: PM: dpm_run_callback(): pm_runtime_resume returns 1

Of course, it needs a wrapper.

>   tegra-clock tegra_clk_sclk: PM: failed to suspend: error 1
>   PM: Some devices failed to suspend, or early wake event detected

Thanks for reporting!