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

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

[Rafael J. Wysocki]

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

[Jon Hunter]

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

[Rafael J. Wysocki]

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!

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

[Rafael J. Wysocki]

On Tuesday, January 6, 2026 1:07:15 PM CET Rafael J. Wysocki wrote:
> 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.

So the patch below should work better.

---
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Subject: [PATCH v2] clk: tegra: Adjust callbacks in tegra_clock_pm

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 a wrapper around pm_runtime_resume() called
tegra_clock_suspend() 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>
---

v1 -> v2:
   * Use a wrapper around pm_runtime_resume() to avoid aborting
     system suspend when it returns 1 (which is always when the
     clock is active during system suspend).

---
 drivers/clk/tegra/clk-device.c |   13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

--- a/drivers/clk/tegra/clk-device.c
+++ b/drivers/clk/tegra/clk-device.c
@@ -174,8 +174,19 @@ unreg_clk:
  * problem. In practice this makes no difference from a power management
  * perspective since voltage is kept at a nominal level during suspend anyways.
  */
+static inline int tegra_clock_suspend(struct device *dev)
+{
+	int ret;
+
+	ret = pm_runtime_resume(dev);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
 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(tegra_clock_suspend, NULL)
 };
 
 static const struct of_device_id tegra_clock_match[] = {

Re: [PATCH v2] clk: tegra: Adjust callbacks in tegra_clock_pm

[Jon Hunter]

On 06/01/2026 12:19, Rafael J. Wysocki wrote:

...

>>> 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.
> 
> So the patch below should work better.
> 
> ---
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Subject: [PATCH v2] clk: tegra: Adjust callbacks in tegra_clock_pm
> 
> 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 a wrapper around pm_runtime_resume() called
> tegra_clock_suspend() 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>
> ---
> 
> v1 -> v2:
>     * Use a wrapper around pm_runtime_resume() to avoid aborting
>       system suspend when it returns 1 (which is always when the
>       clock is active during system suspend).
> 
> ---
>   drivers/clk/tegra/clk-device.c |   13 ++++++++++++-
>   1 file changed, 12 insertions(+), 1 deletion(-)
> 
> --- a/drivers/clk/tegra/clk-device.c
> +++ b/drivers/clk/tegra/clk-device.c
> @@ -174,8 +174,19 @@ unreg_clk:
>    * problem. In practice this makes no difference from a power management
>    * perspective since voltage is kept at a nominal level during suspend anyways.
>    */
> +static inline int tegra_clock_suspend(struct device *dev)
> +{
> +	int ret;
> +
> +	ret = pm_runtime_resume(dev);
> +	if (ret < 0)
> +		return ret;
> +
> +	return 0;
> +}
> +
>   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(tegra_clock_suspend, NULL)
>   };
>   
>   static const struct of_device_id tegra_clock_match[] = {


Thanks. This version works for me so ...

Acked-by: Jon Hunter <jonathanh@nvidia.com>
Tested-by: Jon Hunter <jonathanh@nvidia.com>

Cheers
Jon

-- 
nvpublic

Re: [PATCH v2] clk: tegra: Adjust callbacks in tegra_clock_pm

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>


On Tue, 06 Jan 2026 13:19:47 +0100, Rafael J. Wysocki wrote:
> On Tuesday, January 6, 2026 1:07:15 PM CET Rafael J. Wysocki wrote:
> > 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.
> 
> [...]

Applied, thanks!

[1/1] clk: tegra: Adjust callbacks in tegra_clock_pm
      commit: 53bf300fd4a73146882889020504e8e87cc86c7d

Best regards,
-- 
Thierry Reding <treding@nvidia.com>