linux-kernel - Re: [PATCH 2/3] clk: keystone: don't cache clock rate

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] [day] [month] [year] [list]

Message-ID: <20250925-elephant-of-absolute-prowess-a97fcd@penduick>
Date: Thu, 25 Sep 2025 13:43:18 +0200
From: Maxime Ripard <mripard@...nel.org>
To: Randolph Sapp <rs@...com>
Cc: Kevin Hilman <khilman@...nel.org>, Michael Walle <mwalle@...nel.org>, 
	Frank Binns <frank.binns@...tec.com>, Matt Coster <matt.coster@...tec.com>, 
	Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, Thomas Zimmermann <tzimmermann@...e.de>, 
	David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>, Rob Herring <robh@...nel.org>, 
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, Nishanth Menon <nm@...com>, 
	Vignesh Raghavendra <vigneshr@...com>, Tero Kristo <kristo@...nel.org>, 
	Santosh Shilimkar <ssantosh@...nel.org>, Michael Turquette <mturquette@...libre.com>, 
	Stephen Boyd <sboyd@...nel.org>, Andrew Davis <afd@...com>, dri-devel@...ts.freedesktop.org, 
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, 
	linux-arm-kernel@...ts.infradead.org, linux-clk@...r.kernel.org
Subject: Re: [PATCH 2/3] clk: keystone: don't cache clock rate

On Wed, Sep 24, 2025 at 09:26:17PM -0500, Randolph Sapp wrote:
> On Wed Sep 17, 2025 at 10:24 AM CDT, Kevin Hilman wrote:
> > Michael Walle <mwalle@...nel.org> writes:
> >
> >> The TISCI firmware will return 0 if the clock or consumer is not
> >> enabled although there is a stored value in the firmware. IOW a call to
> >> set rate will work but at get rate will always return 0 if the clock is
> >> disabled.
> >> The clk framework will try to cache the clock rate when it's requested
> >> by a consumer. If the clock or consumer is not enabled at that point,
> >> the cached value is 0, which is wrong.
> >
> > Hmm, it also seems wrong to me that the clock framework would cache a
> > clock rate when it's disabled.  On platforms with clocks that may have
> > shared management (eg. TISCI or other platforms using SCMI) it's
> > entirely possible that when Linux has disabled a clock, some other
> > entity may have changed it.
> >
> > Could another solution here be to have the clk framework only cache when
> > clocks are enabled?
> 
> So I looked into that. There are still about 34 clock operations that are
> functionally uncached, but it does seem more logical than treating everything as
> uncached.
> 
> Side note, why would someone even want to read the rate of an unprepared clock?
> I dumped some debug info for all the clocks tripping this new uncached path.
> Seems weird that it's even happening this often. Even weirder that it's
> apparently happening 3 times to cpu0's core clock on the board I'm currently
> testing.

The short, unsatisfying, answer is that the API explicitly allowed it so far.

It's also somewhat natural when you have a functional rate to set it up
before enabling it and the logic driven by it so that you would avoid a
rate change, or something like a cycle where you would enable, shut
down, reparent, enable the clock again.

In such a case, we would either need the cache, or to read the rate, to
know if we have to change the clock rate in the first place.

Maxime

Download attachment "signature.asc" of type "application/pgp-signature" (274 bytes)