On Thu Sep 25, 2025 at 6:43 AM CDT, Maxime Ripard wrote: > 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 <[email protected]> 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
Thanks Maxime. I'll take this as a hint to stop digging for the moment. Right now, treating unprepared clocks as untrusted / uncached makes sense and shouldn't be too much of a performance issue. - Randolph
