On 04/04/2023 11:11, Maxime Ripard wrote: > The Versatile sp810 "timerclken" clock implements a mux with a > set_parent hook, but doesn't provide a determine_rate implementation. > > This is a bit odd, since set_parent() is there to, as its name implies, > change the parent of a clock. Explanation of this mystery is pretty simple - the original patch: commit 6e973d2c438502dcf956e76305258ba7d1c7d1d3 Author: Pawel Moll <pawel.moll@xxxxxxx> Date: Thu Apr 18 18:23:22 2013 +0100 clk: vexpress: Add separate SP810 driver predates introduction of determine_rate to clk_ops... commit 71472c0c06cf9a3d1540762ea205654c584e3bc4 Author: James Hogan <jhogan@xxxxxxxxxx> Date: Mon Jul 29 12:25:00 2013 +0100 clk: add support for clock reparent on set_rate and clearly no one (the author included ;-) bothered to have another look at this side of the driver. > And if it was an oversight, then we are at least explicit about our > behavior now and it can be further refined down the line. It's been one hell of a memory lane trip, but my recollection suggest that the main goal of the driver was simply initialisation of the mux to select the 1MHz parent, because the other option - 32kHz - just didn't make any sense whatsoever. And that would be the case on every single platform using SP810 I know (or at least: knew), so it's seems to me that making the state permanent, as you're suggesting (or I think you're suggesting?) it's the right thing to do. Thanks! Paweł
