Re: [PATCH v1 1/6] clk: divider: Implement and wire up .determine_rate by default

[Martin Blumenstingl <martin.blumenstingl@googlemail.com>]

Hi Alex,

On Thu, Oct 14, 2021 at 11:55 AM Alex Bee <knaerzche@gmail.com> wrote:
[...]
> This breaks lot of clocks for Rockchip which intensively uses
> composites,  i.e. those clocks will always stay at the initial parent,
> which in some cases  is the XTAL clock and I strongly guess it is the
> same for other platforms,  which use composite clocks having more than
> one parent (e.g. mediatek, ti ...)
Sorry for that and thanks for bisecting this!

> Example (RK3399)
> clk_sdio is set (initialized) with XTAL (24 MHz) as parent in u-boot.
> It will always stay at this parent, even if the mmc driver sets a rate
> of  200 MHz (fails, as the nature of things), which should switch it to
> any of its possible parent PLLs defined in
> mux_pll_src_cpll_gpll_npll_ppll_upll_24m_p (see clk-rk3399.c)  - which
> never happens.
My question to Stephen et. al. is: where is the correct place to solve this?
What I came up with so far (in no particular order):
1) not using clk-composite from clock drivers and letting CCF take
care of re-parenting clocks as needed (and as specified with
CLK_SET_RATE_NO_REPARENT)
2) clk-composite.c: extending the logic so "rate" clocks with
.determine_rate include the existing logic which only applies to
.round_rate (which means clk-composite.c is then responsible for
finding the best possible parent clock)
3) clk-divider.c: extending the logic here to account for clk_hws with
multiple parents

For 3) I am wondering whether this would even work because it seems
that clk-composite uses multiple struct clk_hw.
Letting the divider handle multiple parents means it would need to
know about the information which is only available in mux_hw - whereas
clk-composite currently passes rate_hw (struct clk_hw for the
divider).

I am happy to work on a patch for this if I can get some help with
testing (since I don't have any board with Rockchip SoC).

> Reverting this commit makes it work again: Unless there is a quick and
> obvious fix for that, I guess this should be done for 5.15 - even if the
> real issue is somewhere else.
Reverting this patch is fine from the Amlogic SoC point of view.
The main goal was to clean up / improve the CCF code.
Nothing (that I am aware of) is going to break in Amlogic land if we
revert this.



Best regards,
Martin