[PATCH] soc/tegra: pmc: Ensure power-domains are in a known state

[PATCH] soc/tegra: pmc: Ensure power-domains are in a known state

[Jon Hunter]

After commit 13a4b7fb6260 ("pmdomain: core: Leave powered-on genpds on
until late_initcall_sync") was applied, the Tegra210 Jetson TX1 board
failed to boot. Looking into this issue, before this commit was applied,
if any of the Tegra power-domains were in 'on' state when the kernel
booted, they were being turned off by the genpd core before any driver
had chance to request them. This was purely by luck and a consequence of
the power-domains being turned off earlier during boot. After this
commit was applied, any power-domains in the 'on' state are kept on for
longer during boot and therefore, may never transitioned to the off
state before they are requested/used. The hang on the Tegra210 Jetson
TX1 is caused because devices in some power-domains are accessed without
the power-domain being turned off and on, indicating that the
power-domain is not in a completely on state.

From reviewing the Tegra PMC driver code, if a power-domain is in the
'on' state there is no guarantee that all the necessary clocks
associated with the power-domain are on and even if they are they would
not have been requested via the clock framework and so could be turned
off later. Some power-domains also have a 'clamping' register that needs
to be configured as well. In short, if a power-domain is already 'on' it
is difficult to know if it has been configured correctly. Given that the
power-domains happened to be switched off during boot previously, to
ensure that they are in a good known state on boot, fix this by
switching off any power-domains that are on initially when registering
the power-domains with the genpd framework.

Note that commit 05cfb988a4d0 ("soc/tegra: pmc: Initialise resets
associated with a power partition") updated the
tegra_powergate_of_get_resets() function to pass the 'off' to ensure
that the resets for the power-domain are in the correct state on boot.
However, now that we may power off a domain on boot, if it is on, it is
better to move this logic into the tegra_powergate_add() function so
that there is a single place where we are handling the initial state of
the power-domain.

Fixes: a38045121bf4 ("soc/tegra: pmc: Add generic PM domain support")
Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
---
Please note the commit hash 13a4b7fb6260 is based upon the current hash
in -next. I have used the initial commit that added genpd support for
Tegra in the fixes-tag because this issue has really existed since this
was implemented.

 drivers/soc/tegra/pmc.c | 51 +++++++++++++++++++++++------------------
 1 file changed, 29 insertions(+), 22 deletions(-)

diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
index 2a5f24ee858c..034a2a535a1e 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -1232,7 +1232,7 @@ static int tegra_powergate_of_get_clks(struct tegra_powergate *pg,
 }
 
 static int tegra_powergate_of_get_resets(struct tegra_powergate *pg,
-					 struct device_node *np, bool off)
+					 struct device_node *np)
 {
 	struct device *dev = pg->pmc->dev;
 	int err;
@@ -1247,22 +1247,6 @@ static int tegra_powergate_of_get_resets(struct tegra_powergate *pg,
 	err = reset_control_acquire(pg->reset);
 	if (err < 0) {
 		pr_err("failed to acquire resets: %d\n", err);
-		goto out;
-	}
-
-	if (off) {
-		err = reset_control_assert(pg->reset);
-	} else {
-		err = reset_control_deassert(pg->reset);
-		if (err < 0)
-			goto out;
-
-		reset_control_release(pg->reset);
-	}
-
-out:
-	if (err) {
-		reset_control_release(pg->reset);
 		reset_control_put(pg->reset);
 	}
 
@@ -1308,20 +1292,43 @@ static int tegra_powergate_add(struct tegra_pmc *pmc, struct device_node *np)
 		goto set_available;
 	}
 
-	err = tegra_powergate_of_get_resets(pg, np, off);
+	err = tegra_powergate_of_get_resets(pg, np);
 	if (err < 0) {
 		dev_err(dev, "failed to get resets for %pOFn: %d\n", np, err);
 		goto remove_clks;
 	}
 
-	if (!IS_ENABLED(CONFIG_PM_GENERIC_DOMAINS)) {
-		if (off)
-			WARN_ON(tegra_powergate_power_up(pg, true));
+	/*
+	 * If the power-domain is off, then ensure the resets are asserted.
+	 * If the power-domain is on, then power down to ensure that when is
+	 * it turned on the power-domain, clocks and resets are all in the
+	 * expected state.
+	 */
+	if (off) {
+		err = reset_control_assert(pg->reset);
+		if (err) {
+			pr_err("failed to assert resets: %d\n", err);
+			goto remove_resets;
+		}
+	} else {
+		err = tegra_powergate_power_down(pg);
+		if (err) {
+			dev_err(dev, "failed to turn off PM domain %s: %d\n",
+				pg->genpd.name, err);
+			goto remove_resets;
+		}
+	}
 
+	/*
+	 * If PM_GENERIC_DOMAINS is not enabled, power-on
+	 * the domain and skip the genpd registration.
+	 */
+	if (!IS_ENABLED(CONFIG_PM_GENERIC_DOMAINS)) {
+		WARN_ON(tegra_powergate_power_up(pg, true));
 		goto remove_resets;
 	}
 
-	err = pm_genpd_init(&pg->genpd, NULL, off);
+	err = pm_genpd_init(&pg->genpd, NULL, true);
 	if (err < 0) {
 		dev_err(dev, "failed to initialise PM domain %pOFn: %d\n", np,
 		       err);
-- 
2.43.0

Re: [PATCH] soc/tegra: pmc: Ensure power-domains are in a known state

[Ulf Hansson]

On Thu, 31 Jul 2025 at 14:18, Jon Hunter <jonathanh@nvidia.com> wrote:
>
> After commit 13a4b7fb6260 ("pmdomain: core: Leave powered-on genpds on
> until late_initcall_sync") was applied, the Tegra210 Jetson TX1 board
> failed to boot. Looking into this issue, before this commit was applied,
> if any of the Tegra power-domains were in 'on' state when the kernel
> booted, they were being turned off by the genpd core before any driver
> had chance to request them. This was purely by luck and a consequence of
> the power-domains being turned off earlier during boot. After this
> commit was applied, any power-domains in the 'on' state are kept on for
> longer during boot and therefore, may never transitioned to the off
> state before they are requested/used. The hang on the Tegra210 Jetson
> TX1 is caused because devices in some power-domains are accessed without
> the power-domain being turned off and on, indicating that the
> power-domain is not in a completely on state.
>
> From reviewing the Tegra PMC driver code, if a power-domain is in the
> 'on' state there is no guarantee that all the necessary clocks
> associated with the power-domain are on and even if they are they would
> not have been requested via the clock framework and so could be turned
> off later. Some power-domains also have a 'clamping' register that needs
> to be configured as well. In short, if a power-domain is already 'on' it
> is difficult to know if it has been configured correctly. Given that the
> power-domains happened to be switched off during boot previously, to
> ensure that they are in a good known state on boot, fix this by
> switching off any power-domains that are on initially when registering
> the power-domains with the genpd framework.
>
> Note that commit 05cfb988a4d0 ("soc/tegra: pmc: Initialise resets
> associated with a power partition") updated the
> tegra_powergate_of_get_resets() function to pass the 'off' to ensure
> that the resets for the power-domain are in the correct state on boot.
> However, now that we may power off a domain on boot, if it is on, it is
> better to move this logic into the tegra_powergate_add() function so
> that there is a single place where we are handling the initial state of
> the power-domain.
>
> Fixes: a38045121bf4 ("soc/tegra: pmc: Add generic PM domain support")
> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>

Thanks for looking into this!

I have picked this up via my pmdomain tree and applied it as a fix
with a stable tag. Please let me know if you prefer to take this via
your soc tree instead.

That said, I guess we have some use-cases on Tegra where it actually
would make sense to allow powered-on PM-domains to stay on during
boot. Although, at this point, it seems better to deal with those on a
case by case basis, as improvements on top.

Kind regards
Uffe


> ---
> Please note the commit hash 13a4b7fb6260 is based upon the current hash
> in -next. I have used the initial commit that added genpd support for
> Tegra in the fixes-tag because this issue has really existed since this
> was implemented.
>
>  drivers/soc/tegra/pmc.c | 51 +++++++++++++++++++++++------------------
>  1 file changed, 29 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
> index 2a5f24ee858c..034a2a535a1e 100644
> --- a/drivers/soc/tegra/pmc.c
> +++ b/drivers/soc/tegra/pmc.c
> @@ -1232,7 +1232,7 @@ static int tegra_powergate_of_get_clks(struct tegra_powergate *pg,
>  }
>
>  static int tegra_powergate_of_get_resets(struct tegra_powergate *pg,
> -                                        struct device_node *np, bool off)
> +                                        struct device_node *np)
>  {
>         struct device *dev = pg->pmc->dev;
>         int err;
> @@ -1247,22 +1247,6 @@ static int tegra_powergate_of_get_resets(struct tegra_powergate *pg,
>         err = reset_control_acquire(pg->reset);
>         if (err < 0) {
>                 pr_err("failed to acquire resets: %d\n", err);
> -               goto out;
> -       }
> -
> -       if (off) {
> -               err = reset_control_assert(pg->reset);
> -       } else {
> -               err = reset_control_deassert(pg->reset);
> -               if (err < 0)
> -                       goto out;
> -
> -               reset_control_release(pg->reset);
> -       }
> -
> -out:
> -       if (err) {
> -               reset_control_release(pg->reset);
>                 reset_control_put(pg->reset);
>         }
>
> @@ -1308,20 +1292,43 @@ static int tegra_powergate_add(struct tegra_pmc *pmc, struct device_node *np)
>                 goto set_available;
>         }
>
> -       err = tegra_powergate_of_get_resets(pg, np, off);
> +       err = tegra_powergate_of_get_resets(pg, np);
>         if (err < 0) {
>                 dev_err(dev, "failed to get resets for %pOFn: %d\n", np, err);
>                 goto remove_clks;
>         }
>
> -       if (!IS_ENABLED(CONFIG_PM_GENERIC_DOMAINS)) {
> -               if (off)
> -                       WARN_ON(tegra_powergate_power_up(pg, true));
> +       /*
> +        * If the power-domain is off, then ensure the resets are asserted.
> +        * If the power-domain is on, then power down to ensure that when is
> +        * it turned on the power-domain, clocks and resets are all in the
> +        * expected state.
> +        */
> +       if (off) {
> +               err = reset_control_assert(pg->reset);
> +               if (err) {
> +                       pr_err("failed to assert resets: %d\n", err);
> +                       goto remove_resets;
> +               }
> +       } else {
> +               err = tegra_powergate_power_down(pg);
> +               if (err) {
> +                       dev_err(dev, "failed to turn off PM domain %s: %d\n",
> +                               pg->genpd.name, err);
> +                       goto remove_resets;
> +               }
> +       }
>
> +       /*
> +        * If PM_GENERIC_DOMAINS is not enabled, power-on
> +        * the domain and skip the genpd registration.
> +        */
> +       if (!IS_ENABLED(CONFIG_PM_GENERIC_DOMAINS)) {
> +               WARN_ON(tegra_powergate_power_up(pg, true));
>                 goto remove_resets;
>         }
>
> -       err = pm_genpd_init(&pg->genpd, NULL, off);
> +       err = pm_genpd_init(&pg->genpd, NULL, true);
>         if (err < 0) {
>                 dev_err(dev, "failed to initialise PM domain %pOFn: %d\n", np,
>                        err);
> --
> 2.43.0
>

Re: [PATCH] soc/tegra: pmc: Ensure power-domains are in a known state

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 3870 bytes --]

On Mon, Aug 11, 2025 at 12:37:25PM +0200, Ulf Hansson wrote:
> On Thu, 31 Jul 2025 at 14:18, Jon Hunter <jonathanh@nvidia.com> wrote:
> >
> > After commit 13a4b7fb6260 ("pmdomain: core: Leave powered-on genpds on
> > until late_initcall_sync") was applied, the Tegra210 Jetson TX1 board
> > failed to boot. Looking into this issue, before this commit was applied,
> > if any of the Tegra power-domains were in 'on' state when the kernel
> > booted, they were being turned off by the genpd core before any driver
> > had chance to request them. This was purely by luck and a consequence of
> > the power-domains being turned off earlier during boot. After this
> > commit was applied, any power-domains in the 'on' state are kept on for
> > longer during boot and therefore, may never transitioned to the off
> > state before they are requested/used. The hang on the Tegra210 Jetson
> > TX1 is caused because devices in some power-domains are accessed without
> > the power-domain being turned off and on, indicating that the
> > power-domain is not in a completely on state.
> >
> > From reviewing the Tegra PMC driver code, if a power-domain is in the
> > 'on' state there is no guarantee that all the necessary clocks
> > associated with the power-domain are on and even if they are they would
> > not have been requested via the clock framework and so could be turned
> > off later. Some power-domains also have a 'clamping' register that needs
> > to be configured as well. In short, if a power-domain is already 'on' it
> > is difficult to know if it has been configured correctly. Given that the
> > power-domains happened to be switched off during boot previously, to
> > ensure that they are in a good known state on boot, fix this by
> > switching off any power-domains that are on initially when registering
> > the power-domains with the genpd framework.
> >
> > Note that commit 05cfb988a4d0 ("soc/tegra: pmc: Initialise resets
> > associated with a power partition") updated the
> > tegra_powergate_of_get_resets() function to pass the 'off' to ensure
> > that the resets for the power-domain are in the correct state on boot.
> > However, now that we may power off a domain on boot, if it is on, it is
> > better to move this logic into the tegra_powergate_add() function so
> > that there is a single place where we are handling the initial state of
> > the power-domain.
> >
> > Fixes: a38045121bf4 ("soc/tegra: pmc: Add generic PM domain support")
> > Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
> 
> Thanks for looking into this!
> 
> I have picked this up via my pmdomain tree and applied it as a fix
> with a stable tag. Please let me know if you prefer to take this via
> your soc tree instead.
> 
> That said, I guess we have some use-cases on Tegra where it actually
> would make sense to allow powered-on PM-domains to stay on during
> boot. Although, at this point, it seems better to deal with those on a
> case by case basis, as improvements on top.

We're actually running into one of these cases right now. This happens
for display hardware where we have simple-framebuffer device tree nodes
that are meant to allow a seamless transition from the firmware's early
framebuffer to the Linux framebuffer. But since Jon's patch disables all
of the power domains, the seamless transition no longer works.

I suppose we could argue that seamless display is less important than
systems booting, so I'm inclined to say we want to keep this patch to
fix the earlier regression and then apply a fix on top to address the
issue with the early display.

Do you have any thoughts on how to deal with specific power domains that
should remain powered on during boot? Ideally it would be something
standard, but worst case we can also special-case in the Tegra PMC
driver.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]