Hi Paul, On Wed, Apr 05, 2023 at 03:04:05PM +0200, Paul Cercueil wrote: > Le mardi 04 avril 2023 à 12:11 +0200, Maxime Ripard a écrit : > > The Ingenic CGU clocks 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. However, the most likely candidate to > > trigger that parent change is a call to clk_set_rate(), with > > determine_rate() figuring out which parent is the best suited for a > > given rate. > > > > The other trigger would be a call to clk_set_parent(), but it's far > > less > > used, and it doesn't look like there's any obvious user for that > > clock. > > > > So, the set_parent hook is effectively unused, possibly because of an > > oversight. However, it could also be an explicit decision by the > > original author to avoid any reparenting but through an explicit call > > to > > clk_set_parent(). > > As I said in the v2 (IIRC), clk_set_parent() is used when re-parenting > from the device tree. Yep, it's indeed an oversight in the commit log. > > The driver does implement round_rate() though, which means that we > > can > > change the rate of the clock, but we will never get to change the > > parent. > > > > However, It's hard to tell whether it's been done on purpose or not. > > > > Since we'll start mandating a determine_rate() implementation, let's > > convert the round_rate() implementation to a determine_rate(), which > > will also make the current behavior explicit. And if it was an > > oversight, the clock behaviour can be adjusted later on. > > So just to be sure, this patch won't make clk_set_rate() automatically > switch parents, right? > > Allowing automatic re-parenting sounds like a huge can of worms... The behaviour is strictly equivalent before that patch and after: the driver will not reparent on a rate change. It just makes it explicit. Maxime
Attachment:
signature.asc
Description: PGP signature
