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: <cca286f5-bb43-4914-864c-b5e5c73270c8@samsung.com>
Date: Thu, 23 Oct 2025 14:17:29 +0200
From: Marek Szyprowski <m.szyprowski@...sung.com>
To: Ulf Hansson <ulf.hansson@...aro.org>, 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 23.10.2025 12:02, Ulf Hansson wrote:
> 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.

Well, yes, this works too:

if ((of_device_is_compatible(np, "samsung,exynos4210-pd") &&
     (strstr(pd->pd.name, "LCD") || strstr(pd->pd.name, "DISP"))))
          exynos_pd_power_off(&pd->pd);

>> 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.

It is not a matter of broken reset at all. It is a matter of software 
configuration and the lack of 'protocol' to pass the information that 
the display controller is configured to display splash screen from the 
system memory at given address and newly instantiated drivers must to be 
aware of that.

Turning display-related power domain off simply resets all that 
configuration, so drivers can start from good known 'unconfigured' state.

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