Re: [PATCH 16/22] clk: Remove redundant clk_core_init_rate_req() call

Maxime Ripard
2022-04-23T07:45:05Z

On Fri, Apr 22, 2022 at 09:02:48PM -0700, Stephen Boyd wrote:
> Quoting Maxime Ripard (2022-04-08 02:10:31)
> > Since all the users of clk_core_round_rate_nolock() will now properly
> > initialize, there's no need for it to initialize the request itself.
>
> Probably this needs to be combined with the previous patch.
>
> >
> > This is even dangerous, as if the clock cannot change its rate by itself
> > and has CLK_SET_RATE_PARENT, clk_core_round_rate_nolock() will call
> > itself with the parent clock but the client clk_rate_request structure.
> >
>
> I think the next sentence is part of the single sentence paragraph
> above.
>
> > We will then reinitialize the child request with the parent context
> > (parent, boundaries, etc.), which is an issue if the parent ever changes
> > its own parent or parent rate.
>
> The parent of the parent can't be the parent of the child, i.e. itself.

I already explained this a bit more in my answer to patch 15.

> I guess this is only a problem if clk_core_init_rate_req() starts
> setting min/max?

The bugs I were seeing were mostly about the parent related infos
(best_parent_rate and best_parent_hw) leaking into the child request.

> We want to leave those members unchanged so that the
> rate request can flow up through the tree and be modified when rounding
> rates from a grandchild. That's why the child req is passed up to the
> parent if the child can't round itself. The boundary of the child is
> moved to the parent.
>
> Definitely the rate should be clamped through a parent to the
> grandparent taking into account any of their constraints. Perhaps the
> bug is that __clk_determine_rate() doesn't clamp to boundaries like
> clk_hw_round_rate() does by calling clk_core_get_boundaries() and then
> mixing in the new requests boundaries.

That should work fine though, I've added some tests to make sure this is
working properly after a few fixes
(clk_test_single_parent_mux_set_range_round_rate_child_smaller in patch
5, clk_test_single_parent_mux_set_range_round_rate_parent_only and
clk_test_single_parent_mux_set_range_round_rate_parent_smaller in patch 19)

Maxime