linux-kernel - Re: [PATCH v3 08/10] pmdomain: samsung: selectively handle enforced sync

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAPDyKFrCS1PGwPeZd2ahZ=wKXCqPj93qAJ7V-ELELLA_OwgdSw@mail.gmail.com>
Date: Thu, 23 Oct 2025 12:02:48 +0200
From: Ulf Hansson <ulf.hansson@...aro.org>
To: Marek Szyprowski <m.szyprowski@...sung.com>, Krzysztof Kozlowski <krzk@...nel.org>, 
	Rob Herring <robh@...nel.org>
Cc: André Draszik <andre.draszik@...aro.org>, 
	Alim Akhtar <alim.akhtar@...sung.com>, Conor Dooley <conor+dt@...nel.org>, 
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Peter Griffin <peter.griffin@...aro.org>, 
	Tudor Ambarus <tudor.ambarus@...aro.org>, Will McVicker <willmcvicker@...gle.com>, 
	kernel-team@...roid.com, linux-arm-kernel@...ts.infradead.org, 
	linux-samsung-soc@...r.kernel.org, devicetree@...r.kernel.org, 
	linux-kernel@...r.kernel.org, linux-pm@...r.kernel.org
Subject: Re: [PATCH v3 08/10] pmdomain: samsung: selectively handle enforced sync_state

On Wed, 22 Oct 2025 at 20:39, Marek Szyprowski <m.szyprowski@...sung.com> wrote:
>
> On 22.10.2025 13:06, Ulf Hansson wrote:
> > On Thu, 16 Oct 2025 at 17:58, André Draszik <andre.draszik@...aro.org> wrote:
> >> Unconditionally calling of_genpd_sync_state() causes issues on
> >> platforms with child domains as the parent domain will be turned off
> >> before the child domain was even registered during boot.
> >>
> >> This in particular is an issue for the upcoming Google gs101 support -
> >> all operations on child domains registered after the parent domain
> >> misbehave.
> >>
> >> Add a flag to the probe data to be able to sync_state conditionally
> >> only, and enable that flag on the two platforms currently supported by
> >> this driver.
> >>
> >> Signed-off-by: André Draszik <andre.draszik@...aro.org>
> >>
> >> ---
> >> v2:
> >> * use bool for need_early_sync_state (Krzysztof)
> >> ---
> >>   drivers/pmdomain/samsung/exynos-pm-domains.c | 5 ++++-
> >>   1 file changed, 4 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/pmdomain/samsung/exynos-pm-domains.c b/drivers/pmdomain/samsung/exynos-pm-domains.c
> >> index 638d286b57f716140b2401092415644a6805870e..15a1582aa92103a07335eb681600d9415369fefd 100644
> >> --- a/drivers/pmdomain/samsung/exynos-pm-domains.c
> >> +++ b/drivers/pmdomain/samsung/exynos-pm-domains.c
> >> @@ -20,6 +20,7 @@
> >>   struct exynos_pm_domain_config {
> >>          /* Value for LOCAL_PWR_CFG and STATUS fields for each domain */
> >>          u32 local_pwr_cfg;
> >> +       bool need_early_sync_state;
> >>   };
> >>
> >>   /*
> >> @@ -69,10 +70,12 @@ static int exynos_pd_power_off(struct generic_pm_domain *domain)
> >>
> >>   static const struct exynos_pm_domain_config exynos4210_cfg = {
> >>          .local_pwr_cfg          = 0x7,
> >> +       .need_early_sync_state  = true,
> >>   };
> >>
> >>   static const struct exynos_pm_domain_config exynos5433_cfg = {
> >>          .local_pwr_cfg          = 0xf,
> >> +       .need_early_sync_state  = true,
> >>   };
> >>
> >>   static const struct of_device_id exynos_pm_domain_of_match[] = {
> >> @@ -179,7 +182,7 @@ static int exynos_pd_probe(struct platform_device *pdev)
> >>           * reset during boot. As a temporary hack to manage this, let's enforce
> >>           * a sync_state.
> >>           */
> >> -       if (!ret)
> >> +       if (pm_domain_cfg->need_early_sync_state && !ret)
> >>                  of_genpd_sync_state(np);
> > The call to of_genpd_sync_state() was intended as a temporary solution here.
> >
> > Potentially, if we would be able to distinguish what PM domain that is
> > causing the problem on the Exynos platforms, we could set
> > GENPD_FLAG_NO_STAY_ON for that genpd instead.
>
> Well, this of_genpd_sync_state() "workaround" has to be applied only to
> the power domain of the display controller device. It can be replaced by
> the following check on the legacy Exynos systems:
>
> if (IS_ENABLED(CONFIG_ARM) &&
> of_device_is_compatible(np, "samsung,exynos4210-pd") &&
> (strstr(pd->pd.name, "LCD") || strstr(pd->pd.name, "DISP")))
> pd->pd.flags = GENPD_FLAG_NO_STAY_ON;

Oh wait, perhaps better to just power-off these PM domains before
calling pm_genpd_init(), if that can be done safely?

At least that would guarantee the reset to happen before the display
driver gets probed. Instead of relying on genpd_power_off_unused()
(late_initcall_sync) to do it.

>
> I assume that this information cannot be coded in device tree to make it
> somehow generic...

Right, in principle we would need a new DT property for a power-domain
provider, like "broken-hw-reset", because we don't have a reset-line
to pull.

>
> Best regards
> --
> Marek Szyprowski, PhD
> Samsung R&D Institute Poland
>

Kind regards
Uffe