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

[Stephen Boyd <sboyd@kernel.org>]

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 guess this is only a problem if clk_core_init_rate_req() starts
setting min/max? 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.