linux-kernel - Re: [PATCH 4/5] clk: Add flag to prevent frequency changes when walking subtrees

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: <87bjx2tf3y.fsf@bootlin.com>
Date: Mon, 23 Dec 2024 19:43:13 +0100
From: Miquel Raynal <miquel.raynal@...tlin.com>
To: Maxime Ripard <mripard@...hat.com>
Cc: Abel Vesa <abelvesa@...nel.org>,  Peng Fan <peng.fan@....com>,  Michael
 Turquette <mturquette@...libre.com>,  Stephen Boyd <sboyd@...nel.org>,
  Shawn Guo <shawnguo@...nel.org>,  Sascha Hauer <s.hauer@...gutronix.de>,
  Pengutronix Kernel Team <kernel@...gutronix.de>,  Fabio Estevam
 <festevam@...il.com>,  Ying Liu <victor.liu@....com>,  Marek Vasut
 <marex@...x.de>,  Laurent Pinchart <laurent.pinchart@...asonboard.com>,
  linux-clk@...r.kernel.org,  imx@...ts.linux.dev,
  linux-arm-kernel@...ts.infradead.org,  linux-kernel@...r.kernel.org,
  dri-devel@...ts.freedesktop.org,  Abel Vesa <abel.vesa@...aro.org>,
  Herve Codina <herve.codina@...tlin.com>,  Luca Ceresoli
 <luca.ceresoli@...tlin.com>,  Thomas Petazzoni
 <thomas.petazzoni@...tlin.com>,  Ian Ray <ian.ray@...com>
Subject: Re: [PATCH 4/5] clk: Add flag to prevent frequency changes when
 walking subtrees

Hi Maxime,

On 17/12/2024 at 13:47:53 +01, Maxime Ripard <mripard@...hat.com> wrote:

> On Thu, Nov 21, 2024 at 06:41:14PM +0100, Miquel Raynal wrote:
>> There are mainly two ways to change a clock frequency.
>
> There's much more than that :)

"mainly"

Or maybe I should have added "on purpose".

>
> Off the top of my head, setting/clearing a min/max rate and changing the
> parent might also result in a rate change.
>
> And then, the firmware might get involved too.
>
>> The active way requires calling ->set_rate() in order to ask "on
>> purpose" for a frequency change. Otherwise, a clock can passively see
>> its frequency being updated depending on upstream clock frequency
>> changes. In most cases it is fine to just accept the new upstream
>> frequency - which by definition will have an impact on downstream
>> frequencies if we do not recalculate internal divisors. But there are
>> cases where, upon an upstream frequency change, we would like to
>> maintain a specific rate.
>
> Why is clk_set_rate_exclusive not enough?

I am trying to protect these rate changes from subtree walks, I don't
see where setting an exclusive rate would have an effect? But I might be
overlooking something, definitely.

...

>> @@ -2272,7 +2271,13 @@ static void clk_calc_subtree(struct clk_core *core)
>>  {
>>  	struct clk_core *child;
>>  
>> -	core->new_rate = clk_recalc(core, core->parent->new_rate);
>> +	if (core->flags & CLK_NO_RATE_CHANGE_DURING_PROPAGATION) {
>> +		core->new_rate = clk_determine(core, core->rate);
>> +		if (!core->new_rate)
>> +			core->new_rate = clk_recalc(core, core->parent->new_rate);
>> +	} else {
>> +		core->new_rate = clk_recalc(core, core->parent->new_rate);
>> +	}
>
> Sorry, it's not clear to me how it works. How will the parent clocks
> will get notified to adjust their dividers in that scenario? Also, what
> if they can't?

The idea is: if the flag is set, instead of accepting the new upstream
rate and recalculate the downstream rate based on a previously set
divider value, we change our divider value to match the same frequency
as before. But if we cannot, then we just keep the old way.

Cheers,
Miquèl