Messages in this thread | | | From | Frank Oltmanns <> | Subject | Re: [PATCH v3 5/8] clk: sunxi-ng: nkm: Support finding closest rate | Date | Mon, 03 Jul 2023 10:59:43 +0200 |
| |
On 2023-07-03 at 09:25:59 +0200, Maxime Ripard <maxime@cerno.tech> wrote: > [[PGP Signed Part:Undecided]] > On Mon, Jul 03, 2023 at 09:17:43AM +0200, Frank Oltmanns wrote: >> >> On 2023-07-02 at 19:55:24 +0200, Frank Oltmanns <frank@oltmanns.dev> wrote: >> > When finding the best rate for a NKM clock, consider rates that are >> > higher than the requested rate, if the CCU_FEATURE_CLOSEST_RATE flag is >> > set. >> > >> > Accommodate ccu_mux_helper_determine_rate to this change. >> > >> > Signed-off-by: Frank Oltmanns <frank@oltmanns.dev> >> > --- >> > drivers/clk/sunxi-ng/ccu_mux.c | 23 +++++++++++++++----- >> > drivers/clk/sunxi-ng/ccu_nkm.c | 48 +++++++++++++++++++++++++++++++----------- >> > 2 files changed, 54 insertions(+), 17 deletions(-) >> > >> > diff --git a/drivers/clk/sunxi-ng/ccu_mux.c b/drivers/clk/sunxi-ng/ccu_mux.c >> > index 1d557e323169..8594d6a4addd 100644 >> > --- a/drivers/clk/sunxi-ng/ccu_mux.c >> > +++ b/drivers/clk/sunxi-ng/ccu_mux.c >> > @@ -113,7 +113,7 @@ int ccu_mux_helper_determine_rate(struct ccu_common *common, >> > } >> > >> > for (i = 0; i < clk_hw_get_num_parents(hw); i++) { >> > - unsigned long tmp_rate, parent_rate; >> > + unsigned long tmp_rate, parent_rate, best_diff = ULONG_MAX; >> > struct clk_hw *parent; >> > >> > parent = clk_hw_get_parent_by_index(hw, i); >> > @@ -139,10 +139,23 @@ int ccu_mux_helper_determine_rate(struct ccu_common *common, >> > goto out; >> > } >> > >> > - if ((req->rate - tmp_rate) < (req->rate - best_rate)) { >> > - best_rate = tmp_rate; >> > - best_parent_rate = parent_rate; >> > - best_parent = parent; >> > + if (common->features & CCU_FEATURE_CLOSEST_RATE) { >> > + unsigned long tmp_diff = req->rate > tmp_rate ? >> > + req->rate - tmp_rate : >> > + tmp_rate - req->rate; >> > + >> > + if (tmp_diff < best_diff) { >> > + best_rate = tmp_rate; >> > + best_parent_rate = parent_rate; >> > + best_parent = parent; >> > + best_diff = tmp_diff; >> > + } >> > + } else { >> > + if ((req->rate - tmp_rate) < (req->rate - best_rate)) { >> > + best_rate = tmp_rate; >> > + best_parent_rate = parent_rate; >> > + best_parent = parent; >> > + } >> > } >> > } >> > >> > diff --git a/drivers/clk/sunxi-ng/ccu_nkm.c b/drivers/clk/sunxi-ng/ccu_nkm.c >> > index d83843e69c25..36d9e987e4d8 100644 >> > --- a/drivers/clk/sunxi-ng/ccu_nkm.c >> > +++ b/drivers/clk/sunxi-ng/ccu_nkm.c >> > @@ -18,9 +18,11 @@ struct _ccu_nkm { >> > }; >> > >> > static unsigned long ccu_nkm_find_best_with_parent_adj(unsigned long *parent, unsigned long rate, >> > - struct _ccu_nkm *nkm, struct clk_hw *phw) >> > + struct _ccu_nkm *nkm, struct clk_hw *phw, >> > + unsigned long features) >> > { >> > - unsigned long best_rate = 0, best_parent_rate = *parent, tmp_parent = *parent; >> > + unsigned long best_rate = 0, best_parent_rate = 0, tmp_parent = *parent; >> > + unsigned long best_diff = ULONG_MAX; >> > unsigned long best_n = 0, best_k = 0, best_m = 0; >> > unsigned long _n, _k, _m; >> > >> > @@ -28,16 +30,26 @@ static unsigned long ccu_nkm_find_best_with_parent_adj(unsigned long *parent, un >> > for (_n = nkm->min_n; _n <= nkm->max_n; _n++) { >> > for (_m = nkm->min_m; _m <= nkm->max_m; _m++) { >> > unsigned long tmp_rate; >> > + unsigned long tmp_diff; >> > >> > tmp_parent = clk_hw_round_rate(phw, rate * _m / (_n * _k)); >> > >> > tmp_rate = tmp_parent * _n * _k / _m; >> > - if (tmp_rate > rate) >> > - continue; >> > >> > - if ((rate - tmp_rate) < (rate - best_rate)) { >> > + if (features & CCU_FEATURE_CLOSEST_RATE) { >> > + tmp_diff = rate > tmp_rate ? >> > + rate - tmp_rate : >> > + tmp_rate - rate; >> > + } else { >> > + if (tmp_rate > rate) >> > + continue; >> > + tmp_diff = rate - tmp_diff; >> >> Sorry, this should of course be tmp_diff = rate - tmp_rate. I'll fix >> that in v4. Also I'll do tests on my phone where >> CCU_FEATURE_CLOSEST_RATE is not set (i.e., without PATCH 8), so see if >> it replicates the old behaviour. I'll also look into adding kunit tests, >> so that this doesn't happen again. I'm not sure if this is feasible, but >> I'll ask here for advise, if/when I encounter obstacles. > > While this would obviously be great, I don't think we have the > infrastructure just yet to allow to easily add kunit tests for entire > clocks.
I think, clk_test.c provides a good blueprint. I tried to do that for clk-fractional-divider [1], but Stephen wanted to go a different route, so I dropped it. You could look at clk_fd_test_init() in [1]. A similar approach might work for the sunxi-ng clocks. I don't see any real blockers, but maybe that's me being naive.
[1]: https://lore.kernel.org/all/20230614185521.477924-3-frank@oltmanns.dev/
Best regards, Frank
> > Maxime > > [[End of PGP Signed Part]]
| |