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

[Maxime Ripard]

On Wed, Sep 17, 2025 at 08:24:47AM -0700, Kevin Hilman wrote:
> Michael Walle <mwalle@xxxxxxxxxx> 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.

It doesn't really help that the CCF doesn't seem to agree on if it
should do that in the first place :)

In the original clk API definition, you're not supposed to call
clk_get_rate() when the clock is disabled.

https://elixir.bootlin.com/linux/v6.16.8/source/include/linux/clk.h#L746

However, it's been allowed by the CCF since forever:

https://elixir.bootlin.com/linux/v6.16.8/source/drivers/clk/clk.c#L1986

But then, some drivers will return 0 as a valid value, and not an error
code (whatever 0Hz for a clock means).

It's kind of a mess, and very regression prone, so I don't really expect
it to change anytime soon.

Maxime

Attachment: signature.asc
Description: PGP signature