Re: [PATCH v4 07/68] clk: test: Add a determine_rate hook

[Stephen Boyd <sboyd@kernel.org>]

Quoting Maxime Ripard (2023-06-13 01:21:59)
> On Thu, Jun 08, 2023 at 06:41:56PM -0700, Stephen Boyd wrote:
> > Quoting Maxime Ripard (2023-05-05 04:25:09)
> 
> > > +        * __clk_mux_determine_rate() here, if we use it and call
> > > +        * clk_round_rate() or clk_set_rate() with a rate lower than
> > > +        * what all the parents can provide, it will return -EINVAL.
> > > +        *
> > > +        * This is due to the fact that it has the undocumented
> > > +        * behaviour to always pick up the closest rate higher than the
> > > +        * requested rate. If we get something lower, it thus considers
> > > +        * that it's not acceptable and will return an error.
> > > +        *
> > > +        * It's somewhat inconsistent and creates a weird threshold
> > > +        * between rates above the parent rate which would be rounded to
> > > +        * what the parent can provide, but rates below will simply
> > > +        * return an error.
> > > +        */
> 
> I guess it's mostly a policy decision: __clk_mux_determine_rate() always
> has been returning rates that are lower or equal to the target rate.
> 
> If no parent can provide one, then the obvious solution is to error out,
> which creates the inconsistency mentioned above.
> 
> Another solution would be to pick up a parent by default (the current
> one maybe?) but then we could return a rate higher than the target rate
> which breaks what we've been doing so far.
> 
> I'm not sure if one is better than the other.

We should pick a rounding policy that we want for the test. The test
shouldn't be checking the rounding policy. It should be checking that
the code under test does what is expected when the rounding policy is
what we choose.

If __clk_mux_determine_rate() returns an error when the rate is lower
than the parent supports then we should test that as well and make sure
it does return an error in this case. We can directly call
__clk_mux_determine_rate() after registering the clks so that the
function is isolated.

We should also be testing what the clk API will do if a determine_rate()
clk_op returns an error. And what it will do if the clk has multiple
parents, etc. I'm mostly confused that it's marked as a FIXME when the
test should be doing wacky things to test various pieces of code.