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

Nishanth Menon
2025-12-30T20:12:47Z

On 13:47-20251223, Michael Walle wrote:
> 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. Thus, disable the cache
> altogether.
>
> Signed-off-by: Michael Walle <mwalle@kernel.org>
> Reviewed-by: Kevin Hilman <khilman@baylibre.com>
> Reviewed-by: Randolph Sapp <rs@ti.com>
> ---
> drivers/clk/keystone/sci-clk.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/clk/keystone/sci-clk.c b/drivers/clk/keystone/sci-clk.c
> index 9d5071223f4c..0a1565fdbb3b 100644
> --- a/drivers/clk/keystone/sci-clk.c
> +++ b/drivers/clk/keystone/sci-clk.c
> @@ -333,6 +333,14 @@ static int _sci_clk_build(struct sci_clk_provider *provider,
>
> init.ops = &sci_clk_ops;
> init.num_parents = sci_clk->num_parents;
> +
> + /*
> + * A clock rate query to the SCI firmware will return 0 if either the
> + * clock itself is disabled or the attached device/consumer is disabled.
> + * This makes it inherently unsuitable for the caching of the clk
> + * framework.
> + */
> + init.flags = CLK_GET_RATE_NOCACHE;
> sci_clk->hw.init = &init;
>
> ret = devm_clk_hw_register(provider->dev, &sci_clk->hw);
> --
> 2.47.3
>

Reviewed-by: Nishanth Menon <nm@ti.com>

I wish there was a better scheme, but inherently, just like SCMI and
other systems where power management co-processor controls clocks, there
is no real feasible caching scheme I can think of. I wonder if Stephen
or others have a thought on this?

That said, I wonder if we need fixes tag to this? I am sure there are
other clocks susceptible to this as well. I wonder if
commit 3c13933c6033 ("clk: keystone: sci-clk: add support for
dynamically probing clocks") is the appropriate tag?

--
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3 1A34 DDB5 849D 1736 249D