| Date | Thu, 6 Apr 2023 16:21:15 +0100 | Subject | Re: [PATCH v3 36/65] clk: versatile: sp810: Add a determine_rate hook | From | Pawel Moll <> |
| |
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@arm.com> 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@kernel.org> 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ł
|