Quoting Maxime Ripard (2023-03-29 12:50:49)
On Wed, Mar 22, 2023 at 04:31:04PM -0700, Stephen Boyd wrote:
The clk_set_parent() path is valid for those cases. Probably nobody cares about determine_rate because they don't set rates on these clks. Some drivers even explicitly left out determine_rate()/round_rate() because they didn't want to have some other clk round up to the mux and change the parent.
Eventually we want drivers to migrate to determine_rate op so we can get rid of the round_rate op and save a pointer (we're so greedy). It's been 10 years though, and that hasn't been done. Sigh! I can see value in this series from the angle of migrating, but adding a determine_rate op when there isn't a round_rate op makes it hard to reason about. What if something copies the clk_ops or sets a different flag? Now we've just added parent changing support to clk_set_rate(). What if the clk has CLK_SET_RATE_PARENT flag set? Now we're going to ask the parent clk to change rate. Fun bugs.
TL;DR: If the set_parent op exists but determine_rate/round_rate doesn't then the clk is a mux that doesn't want to support clk_set_rate(). Make a new mux function that's the contents of the CLK_SET_RATE_NO_REPARENT branch in clk_mux_determine_rate_flags() and call that directly from the clk_ops so it is clear what's happening, clk_hw_mux_same_parent_determine_rate() or something with a better name. Otherwise migrate the explicit determine_rate op to this new function and don't set the flag.
It may be possible to entirely remove the CLK_SET_RATE_NO_REPARENT flag with this design, if the determine_rate clk_op can call the inner wrapper function instead of __clk_mux_determine_rate*() (those underscores are awful, we should just prefix them with clk_hw_mux_*() and live happier). That should be another patch series.
Sorry but it's not really clear to me what you expect in the v2 of this series (if you even expect one). It looks that you don't like the assignment-if-missing idea Mark suggested, but should I just rebase/resend or did you expect something else?
Yes, we want explicit code. Just rebase & resend. Don't add a determine_rate if there isn't a round_rate. Don't add more users of CLK_SET_RATE_NO_REPARENT. Instead, make an explicit determine_rate function for that. If you want to work on the removal of CLK_SET_RATE_NO_REPARENT go for it. Otherwise I'll take care of it after this series.