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

[Stephen Boyd <sboyd@kernel.org>]

Quoting Maxime Ripard (2023-05-05 04:25:09)
> The single parent clock in our kunit tests implements a mux with a
> set_parent hook, but doesn't provide a determine_rate implementation.
> 
> This is not entirely unexpected, since its whole purpose it to have a
> single parent. When determine_rate is missing, and since
> CLK_SET_RATE_PARENT is set for all its instances, the default behaviour
> of the framework will be to forward it to the current parent.
> 
> This is totally fine as far as the tests are concerned, but we'll start
> to mandate a determine_rate implementation when set_parent is set, so
> let's fill it with __clk_mux_determine_rate() which will have the same
> behavior.
> 
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> ---
>  drivers/clk/clk_test.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/drivers/clk/clk_test.c b/drivers/clk/clk_test.c
> index b3ed3b0e4c31..a154ec9d0111 100644
> --- a/drivers/clk/clk_test.c
> +++ b/drivers/clk/clk_test.c
> @@ -104,6 +104,23 @@ static const struct clk_ops clk_dummy_minimize_rate_ops = {
>  };
>  
>  static const struct clk_ops clk_dummy_single_parent_ops = {
> +       /*
> +        * FIXME: Even though we should probably be able to use

Are we ever going to fix this?

> +        * __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.
> +        */
> +       .determine_rate = __clk_mux_determine_rate_closest,
>         .set_parent = clk_dummy_single_set_parent,
>         .get_parent = clk_dummy_single_get_parent,