Re: [PATCH] pmdomain: core: Add tracepoints for domain state changes

[Ulf Hansson]

On Thu, Sep 24, 2026 at 5:07 PM Abel Vesa <abel.vesa@xxxxxxxxxxxxxxxx> wrote:
>
> Power domain state changes are difficult to correlate with device activity
> using snapshots of the genpd debugfs state alone.
>
> Add tracepoints for domain initialization, power-on and power-off
> operations, and performance-state changes. Record the domain name and
> initial power and performance states at initialization. For power
> operations, record the idle-state index and return value, and for
> performance-state changes, record the requested state and return value.
>
> Emit the operation tracepoints on both success and failure paths,
> including power notifier failures and performance-state rollback paths,
> to make failed transitions visible alongside successful ones.
>
> Assisted-by: LLM
> Signed-off-by: Abel Vesa <abel.vesa@xxxxxxxxxxxxxxxx>
> ---
>  drivers/pmdomain/core.c      | 27 +++++++++----
>  include/trace/events/genpd.h | 94 ++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 114 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/pmdomain/core.c b/drivers/pmdomain/core.c
> index b99c2dcf3d8f..62bd29e5918d 100644
> --- a/drivers/pmdomain/core.c
> +++ b/drivers/pmdomain/core.c
> @@ -28,6 +28,9 @@
>
>  #include <trace/events/ipi.h>
>
> +#define CREATE_TRACE_POINTS
> +#include <trace/events/genpd.h>
> +
>  /* Provides a unique ID for each genpd device */
>  static DEFINE_IDA(genpd_ida);
>
> @@ -485,7 +488,7 @@ static int _genpd_set_performance_state(struct generic_pm_domain *genpd,
>                 if (ret) {
>                         if (link)
>                                 goto rollback_parents_up;
> -                       return ret;
> +                       goto out;
>                 }
>         }
>
> @@ -499,15 +502,18 @@ static int _genpd_set_performance_state(struct generic_pm_domain *genpd,
>         }
>
>         genpd->performance_state = state;
> -       return 0;
> +       ret = 0;

Nitpick, probably nicer to initialize ret to 0 while declaring it.

> +       goto out;
>
>  rollback_parents_up:
>         list_for_each_entry_continue_reverse(link, &genpd->child_links, child_node)
>                 _genpd_rollback_parent_state(link, depth);
> -       return ret;
> +       goto out;
>  rollback_parents_down:
>         list_for_each_entry_continue(link, &genpd->child_links, child_node)
>                 _genpd_rollback_parent_state(link, depth);
> +out:
> +       trace_genpd_set_performance_state(dev_name(&genpd->dev), state, ret);
>         return ret;
>  }
>
> @@ -842,7 +848,7 @@ static int _genpd_power_on(struct generic_pm_domain *genpd, bool timed)
>                                              GENPD_NOTIFY_OFF, NULL);
>         ret = notifier_to_errno(ret);
>         if (ret)
> -               return ret;
> +               goto trace;
>
>         if (!genpd->power_on)
>                 goto out;
> @@ -873,10 +879,13 @@ static int _genpd_power_on(struct generic_pm_domain *genpd, bool timed)
>  out:
>         raw_notifier_call_chain(&genpd->power_notifiers, GENPD_NOTIFY_ON, NULL);
>         genpd->synced_poweroff = false;
> -       return 0;
> +       ret = 0;

Ditto.

> +       goto trace;
>  err:
>         raw_notifier_call_chain(&genpd->power_notifiers, GENPD_NOTIFY_OFF,
>                                 NULL);
> +trace:
> +       trace_genpd_power_on(dev_name(&genpd->dev), state_idx, ret);
>         return ret;
>  }
>
> @@ -893,7 +902,7 @@ static int _genpd_power_off(struct generic_pm_domain *genpd, bool timed)
>                                              GENPD_NOTIFY_ON, NULL);
>         ret = notifier_to_errno(ret);
>         if (ret)
> -               return ret;
> +               goto trace;
>
>         if (!genpd->power_off)
>                 goto out;
> @@ -924,9 +933,12 @@ static int _genpd_power_off(struct generic_pm_domain *genpd, bool timed)
>  out:
>         raw_notifier_call_chain(&genpd->power_notifiers, GENPD_NOTIFY_OFF,
>                                 NULL);
> -       return 0;
> +       ret = 0;

Ditto.

> +       goto trace;
>  busy:
>         raw_notifier_call_chain(&genpd->power_notifiers, GENPD_NOTIFY_ON, NULL);
> +trace:
> +       trace_genpd_power_off(dev_name(&genpd->dev), state_idx, ret);
>         return ret;
>  }

[...]

Kind regards
Uffe