linux-kernel - Re: [PATCH v3 1/5] clk: Respect CLK_OPS_PARENT

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: <176962688200.4027.9869545980331892869@lazor>
Date: Wed, 28 Jan 2026 12:01:22 -0700
From: Stephen Boyd <sboyd@...nel.org>
To: Mark Brown <broonie@...nel.org>, Nicolas Frattaroli <nicolas.frattaroli@...labora.com>
Cc: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>, Michael Turquette <mturquette@...libre.com>, Dong Aisheng <aisheng.dong@....com>, Matthias Brugger <matthias.bgg@...il.com>, Yassine Oudjana <y.oudjana@...tonmail.com>, Laura Nao <laura.nao@...labora.com>, Nícolas F. R. A. Prado <nfraprado@...labora.com>, Chia-I Wu <olvaffe@...il.com>, Chen-Yu Tsai <wenst@...omium.org>, kernel@...labora.com, linux-clk@...r.kernel.org, linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org, linux-mediatek@...ts.infradead.org
Subject: Re: [PATCH v3 1/5] clk: Respect CLK_OPS_PARENT_ENABLE during recalc

Quoting Nicolas Frattaroli (2026-01-28 09:04:55)
> On Wednesday, 28 January 2026 15:47:08 Central European Standard Time Mark Brown wrote:
> > 
> > For the Avenger96 that says:
> > 
> > [    0.347816] __clk_core_init: enabling parent ck_hse for ck_per
> > [    0.352230] __clk_core_init: disabling parent ck_hse for ck_per
> > [    0.358207] __clk_core_init: enabling parent pll1_p for ck_mpu
> > [    0.364005] __clk_core_init: disabling parent pll1_p for ck_mpu
> > 
> > https://lava.sirena.org.uk/scheduler/job/2412562#L563
> > 
> 
> Okay, on this one, there may be a problem in the clock tree.
> ck_mpu is marked CLK_IS_CRITICAL, but its parent, pll1_p, is not. Clock
> core doesn't seem to check whether any children of a clock are marked as
> critical before disabling it.
> 
> I'm not super familiar with the intended semantics of critical clocks.
> If we need to manually mark all parents of critical clocks as critical
> as well, then a (potentially partial) fix for the Avenger96 might be:
> 

Marking parents critical hasn't been strictly necessary so far because
we've been relying on the prepare/enable count on a critical child to
keep the parent prepared/enabled.

> 
> I just looked for CLK_IS_CRITICAL clocks in that file that have the
> CLK_OPS_PARENT_ENABLE flag, and marked their PLL parents as critical
> as well.
> 
> An alternate approach would be to skip the parent enable/disable pair
> in the problematic patch in __clk_core_init for clocks that are marked
> as critical, because if the parent wasn't on for a critical clock then
> we wouldn't be in that function, we would be dead.

The parent may not be known yet in __clk_core_init() because we lazily
find parents. Putting it another way, we can get to the recalc_rate()
call in __clk_core_init() for a clk with CLK_OPS_PARENT_ENABLE when the
parent pointer is NULL. This hasn't been a problem because when we adopt
the orphan clk the rate is recalculated (see
clk_core_reparent_orphans_nolock()).

I don't see an easy way out of this problem because in general we need
to know a clk with CLK_OPS_PARENT_ENABLE is parented enough to be able
to read the registers when calling clk_ops like recalc_rate() (and
probably get_parent() as well meaning that clk_op can't touch
hardware?). Maybe the simplest approach is to skip any sort of hardware
access when this flag is set during setup and reparenting.

BTW, I don't know what this patch is actually fixing, so I'm going to
revert it. When it is resubmitted please describe the problem you're
seeing.