| Message ID | 20211016105022.303413-2-martin.blumenstingl@googlemail.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | Fix clk-composite to support .determine_rate | expand |
Quoting Martin Blumenstingl (2021-10-16 03:50:21) > Commit 69a00fb3d69706 ("clk: divider: Implement and wire up > .determine_rate by default") switches clk_divider_ops to implement > .determine_rate by default. This breaks composite clocks with multiple > parents because clk-composite.c does not use the special handling for > mux + divider combinations anymore (that was restricted to rate clocks > which only implement .round_rate, but not .determine_rate). > > Alex reports: > 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 ...) > > 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. > > Restore the original behavior by changing the priority of the conditions > inside clk-composite.c. Now the special rate + mux case (with rate_ops > having a .round_rate - which is still the case for the default > clk_divider_ops) is preferred over rate_ops which have .determine_rate > defined (and not further considering the mux). > > Fixes: 69a00fb3d69706 ("clk: divider: Implement and wire up .determine_rate by default") > Reported-by: Alex Bee <knaerzche@gmail.com> > Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> > --- Applied to clk-fixes
Hi Stephen, Am 18.10.21 um 22:01 schrieb Stephen Boyd: > Quoting Martin Blumenstingl (2021-10-16 03:50:21) >> Commit 69a00fb3d69706 ("clk: divider: Implement and wire up >> .determine_rate by default") switches clk_divider_ops to implement >> .determine_rate by default. This breaks composite clocks with multiple >> parents because clk-composite.c does not use the special handling for >> mux + divider combinations anymore (that was restricted to rate clocks >> which only implement .round_rate, but not .determine_rate). >> >> Alex reports: >> 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 ...) >> >> 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. >> >> Restore the original behavior by changing the priority of the conditions >> inside clk-composite.c. Now the special rate + mux case (with rate_ops >> having a .round_rate - which is still the case for the default >> clk_divider_ops) is preferred over rate_ops which have .determine_rate >> defined (and not further considering the mux). >> >> Fixes: 69a00fb3d69706 ("clk: divider: Implement and wire up .determine_rate by default") >> Reported-by: Alex Bee <knaerzche@gmail.com> >> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> >> --- > > Applied to clk-fixes any chance this can make it in 5.15? Regards, Alex >
Quoting Alex Bee (2021-10-27 15:47:03) > > > > Applied to clk-fixes > > any chance this can make it in 5.15? > That's the plan.
diff --git a/drivers/clk/clk-composite.c b/drivers/clk/clk-composite.c index c7b97fb0051b..ba8d4d8cf8dd 100644 --- a/drivers/clk/clk-composite.c +++ b/drivers/clk/clk-composite.c @@ -58,11 +58,8 @@ static int clk_composite_determine_rate(struct clk_hw *hw, long rate; int i; - if (rate_hw && rate_ops && rate_ops->determine_rate) { - __clk_hw_set_clk(rate_hw, hw); - return rate_ops->determine_rate(rate_hw, req); - } else if (rate_hw && rate_ops && rate_ops->round_rate && - mux_hw && mux_ops && mux_ops->set_parent) { + if (rate_hw && rate_ops && rate_ops->round_rate && + mux_hw && mux_ops && mux_ops->set_parent) { req->best_parent_hw = NULL; if (clk_hw_get_flags(hw) & CLK_SET_RATE_NO_REPARENT) { @@ -107,6 +104,9 @@ static int clk_composite_determine_rate(struct clk_hw *hw, req->rate = best_rate; return 0; + } else if (rate_hw && rate_ops && rate_ops->determine_rate) { + __clk_hw_set_clk(rate_hw, hw); + return rate_ops->determine_rate(rate_hw, req); } else if (mux_hw && mux_ops && mux_ops->determine_rate) { __clk_hw_set_clk(mux_hw, hw); return mux_ops->determine_rate(mux_hw, req);
Commit 69a00fb3d69706 ("clk: divider: Implement and wire up .determine_rate by default") switches clk_divider_ops to implement .determine_rate by default. This breaks composite clocks with multiple parents because clk-composite.c does not use the special handling for mux + divider combinations anymore (that was restricted to rate clocks which only implement .round_rate, but not .determine_rate). Alex reports: 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 ...) 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. Restore the original behavior by changing the priority of the conditions inside clk-composite.c. Now the special rate + mux case (with rate_ops having a .round_rate - which is still the case for the default clk_divider_ops) is preferred over rate_ops which have .determine_rate defined (and not further considering the mux). Fixes: 69a00fb3d69706 ("clk: divider: Implement and wire up .determine_rate by default") Reported-by: Alex Bee <knaerzche@gmail.com> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> --- drivers/clk/clk-composite.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)