Re: [PATCH] clk: do not trust cached rates for disabled clocks

[Randolph Sapp <rs@ti.com>]

On Thu Oct 23, 2025 at 3:36 AM CDT, Maxime Ripard wrote:
> On Wed, Oct 22, 2025 at 06:18:22PM -0500, Randolph Sapp wrote:
>> On Wed Oct 22, 2025 at 1:23 AM CDT, Michael Walle wrote:
>> > Hi,
>> >
>> >>> Am I correct in my assumption about running clk_get_rate on unprepared clocks
>> >>> though? (That it shouldn't be allowed or, if it is, that the result shouldn't be
>> >>> cached.)
>> >>>
>> >> Any follow up to this Michael? I wanted to be sure this was something the
>> >> subsystem should allow before I look into further workarounds.
>> >
>> > I don't know. I'm not that familar with the ccs. My first reaction
>> > was that it's asymmetrical that a .set is allowed (and works for
>> > tisci) and that .get is not allowed. That way you can't read the
>> > hardware clock (or think of a fixed clock, where you want to get the
>> > frequency) before enabling it. I could imagine some devices which
>> > needs to be configured first, before you might turn the clock on.
>> >
>> > OTOH Maxime pointed out the comment in the kdoc of clk_get_rate()
>> > which clearly states that it's only valid if the clock is on.
>> >
>> > -michael
>> 
>> Yeah, I still don't like the way we handle clock in firmware but I've already
>> been shut down on that front.
>> 
>> Regardless, there are quite a few drivers right now that use clk_get_rate prior
>> to preparing the clock. If the kdoc reports clk_get_rate is only valid if the
>> clock is enabled, should we report a proper warning when this occurs?
>
> It's more complicated than that.
>
> The clock API documentation mentions that, and the CCF is one
> implementation of that API. It's now the dominant implementation, but
> the CCF itself never mentioned or required it.
>
> And thus drivers started to rely on the CCF behaviour.
>
> Plus, leaving the documentation part aside, being able to call
> clk_get_rate when the clock is disabled has value.
>
> How can you setup a clock before enabling it to avoid any frequency
> change while the device operates otherwise?

Why would enabling a clock change it's rate unless the current rate wasn't
actually valid? I can only see a change explicitly occurring if the clock parent
has decided that the associated rate was not acceptable.

If some device always resets the rate when enabled, isn't that already
problematic?

> Or how do you make sure the clock is in its operating range and thus the
> device *can* operate?

Well, if enabling a clock doesn't change it's rate there's nothing stopping
people from enabling the clock prior to getting the rate.

> There's a reason people have started using it. And it might be
> abstracted away by the firmware in some cases, but not all platforms use
> a firmware, so you can't rely on that either.
>
> Maxime

Thanks for taking the time to talk with me about this. I assume there is some
specific thing that violates my understanding of how this should work, but I
feel like things are too loosely defined as is.

- Randolph