[PATCH v2] clk: keystone: don't cache clock rate

[PATCH v2] clk: keystone: don't cache clock rate

[a-christidis]

From: Michael Walle <mwalle@kernel.org>

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>
Reviewed-by: Nishanth Menon <nm@ti.com>
Signed-off-by: Antonios Christidis <a-christidis@ti.com>
---
v2:
- Resent as part of series [1], separated from series per reviewer feedback [2]
- Original patch was sent here [3]
    
[1]: https://lore.kernel.org/all/20260224-gpu_dts-v1-5-cc5ddffe140c@ti.com/
[2]: https://lore.kernel.org/all/20260225010507.flvt775fs5kfe7ez@unknotted/
[3]: https://lore.kernel.org/all/20260109120728.2wku6akxof2ddn4h@tastiness/
---
 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);

---
base-commit: 17c7841d09ee7d33557fd075562d9289b6018c90
change-id: 20260507-clk-sci-175f398ecdc0

Best regards,
-- 
Antonios Christidis <a-christidis@ti.com>

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

[Brian Masney]

On Thu, May 07, 2026 at 11:09:34AM -0500, a-christidis@ti.com wrote:
> From: Michael Walle <mwalle@kernel.org>
> 
> 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>
> Reviewed-by: Nishanth Menon <nm@ti.com>
> Signed-off-by: Antonios Christidis <a-christidis@ti.com>

Reviewed-by: Brian Masney <bmasney@redhat.com>

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

[Nishanth Menon]

On 10:36-20260511, Brian Masney wrote:
> On Thu, May 07, 2026 at 11:09:34AM -0500, a-christidis@ti.com wrote:
> > From: Michael Walle <mwalle@kernel.org>
> > 
> > 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>
> > Reviewed-by: Nishanth Menon <nm@ti.com>
> > Signed-off-by: Antonios Christidis <a-christidis@ti.com>
> 
> Reviewed-by: Brian Masney <bmasney@redhat.com>
> 

Brian,

Could you clarify if I need to take it via my tree to arnd or if this
patch will go via the clk tree?

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

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

[Brian Masney]

Hi Nishanth,

On Tue, May 12, 2026 at 05:48:48AM -0500, Nishanth Menon wrote:
> On 10:36-20260511, Brian Masney wrote:
> > On Thu, May 07, 2026 at 11:09:34AM -0500, a-christidis@ti.com wrote:
> > > From: Michael Walle <mwalle@kernel.org>
> > > 
> > > 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>
> > > Reviewed-by: Nishanth Menon <nm@ti.com>
> > > Signed-off-by: Antonios Christidis <a-christidis@ti.com>
> > 
> > Reviewed-by: Brian Masney <bmasney@redhat.com>
> > 
> 
> Brian,
> 
> Could you clarify if I need to take it via my tree to arnd or if this
> patch will go via the clk tree?

Sorry, I'm not sure what Stephen prefers here. An argument could be made
for either approach. I would just go with whatever you have done in the
past.

Brian

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

[Nishanth Menon]

On 10:51-20260512, Brian Masney wrote:
> Hi Nishanth,
> > Could you clarify if I need to take it via my tree to arnd or if this
> > patch will go via the clk tree?
> 
> Sorry, I'm not sure what Stephen prefers here. An argument could be made
> for either approach. I would just go with whatever you have done in the
> past.

Thanks Brian for the review.

This usually will go via Stephen. Will wait, just trying to make sure
there is no change in expectations (given this patch missed a window).

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