| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAPDyKFr+uVDYBMvsN+L9XPToaD+Wr9P=SnnXyKB6ucqQ7se=7A@mail.gmail.com>
Date: Mon, 11 Aug 2025 12:37:25 +0200
From: Ulf Hansson <ulf.hansson@...aro.org>
To: Jon Hunter <jonathanh@...dia.com>
Cc: Thierry Reding <thierry.reding@...il.com>, linux-tegra@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH] soc/tegra: pmc: Ensure power-domains are in a known state
On Thu, 31 Jul 2025 at 14:18, Jon Hunter <jonathanh@...dia.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@...dia.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
>
Powered by blists - more mailing lists