All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v3 5/8] clk: sunxi-ng: nkm: Support finding closest rate
@ 2023-07-03 20:19 kernel test robot
  0 siblings, 0 replies; 15+ messages in thread
From: kernel test robot @ 2023-07-03 20:19 UTC (permalink / raw)
  To: oe-kbuild; +Cc: lkp, Dan Carpenter

BCC: lkp@intel.com
CC: oe-kbuild-all@lists.linux.dev
In-Reply-To: <20230702-pll-mipi_set_rate_parent-v3-5-46dcb8aa9cbc@oltmanns.dev>
References: <20230702-pll-mipi_set_rate_parent-v3-5-46dcb8aa9cbc@oltmanns.dev>
TO: Frank Oltmanns <frank@oltmanns.dev>
TO: Maxime Ripard <maxime@cerno.tech>
TO: Michael Turquette <mturquette@baylibre.com>
TO: Stephen Boyd <sboyd@kernel.org>
TO: "Chen-Yu Tsai" <wens@csie.org>
TO: Jernej Skrabec <jernej.skrabec@gmail.com>
TO: Samuel Holland <samuel@sholland.org>
TO: Andre Przywara <andre.przywara@arm.com>
TO: Roman Beranek <me@crly.cz>
CC: linux-clk@vger.kernel.org
CC: linux-arm-kernel@lists.infradead.org
CC: linux-sunxi@lists.linux.dev
CC: linux-kernel@vger.kernel.org
CC: Frank Oltmanns <frank@oltmanns.dev>

Hi Frank,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 6995e2de6891c724bfeb2db33d7b87775f913ad1]

url:    https://github.com/intel-lab-lkp/linux/commits/Frank-Oltmanns/clk-sunxi-ng-nkm-consider-alternative-parent-rates-when-determining-rate/20230703-015726
base:   6995e2de6891c724bfeb2db33d7b87775f913ad1
patch link:    https://lore.kernel.org/r/20230702-pll-mipi_set_rate_parent-v3-5-46dcb8aa9cbc%40oltmanns.dev
patch subject: [PATCH v3 5/8] clk: sunxi-ng: nkm: Support finding closest rate
:::::: branch date: 26 hours ago
:::::: commit date: 26 hours ago
config: riscv-randconfig-m031-20230703 (https://download.01.org/0day-ci/archive/20230704/202307040351.19rZm4kd-lkp@intel.com/config)
compiler: riscv64-linux-gcc (GCC) 12.3.0
reproduce: (https://download.01.org/0day-ci/archive/20230704/202307040351.19rZm4kd-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <error27@gmail.com>
| Closes: https://lore.kernel.org/r/202307040351.19rZm4kd-lkp@intel.com/

smatch warnings:
drivers/clk/sunxi-ng/ccu_nkm.c:46 ccu_nkm_find_best_with_parent_adj() error: uninitialized symbol 'tmp_diff'.
drivers/clk/sunxi-ng/ccu_nkm.c:93 ccu_nkm_find_best() error: uninitialized symbol 'tmp_diff'.

vim +/tmp_diff +46 drivers/clk/sunxi-ng/ccu_nkm.c

df6561e60244c0 Maxime Ripard  2016-06-29   19  
8a97d9f6cbb457 Frank Oltmanns 2023-07-02   20  static unsigned long ccu_nkm_find_best_with_parent_adj(unsigned long *parent, unsigned long rate,
8e5b4aea92912f Frank Oltmanns 2023-07-02   21  						       struct _ccu_nkm *nkm, struct clk_hw *phw,
8e5b4aea92912f Frank Oltmanns 2023-07-02   22  						       unsigned long features)
8a97d9f6cbb457 Frank Oltmanns 2023-07-02   23  {
8e5b4aea92912f Frank Oltmanns 2023-07-02   24  	unsigned long best_rate = 0, best_parent_rate = 0, tmp_parent = *parent;
8e5b4aea92912f Frank Oltmanns 2023-07-02   25  	unsigned long best_diff = ULONG_MAX;
8a97d9f6cbb457 Frank Oltmanns 2023-07-02   26  	unsigned long best_n = 0, best_k = 0, best_m = 0;
8a97d9f6cbb457 Frank Oltmanns 2023-07-02   27  	unsigned long _n, _k, _m;
8a97d9f6cbb457 Frank Oltmanns 2023-07-02   28  
8a97d9f6cbb457 Frank Oltmanns 2023-07-02   29  	for (_k = nkm->min_k; _k <= nkm->max_k; _k++) {
8a97d9f6cbb457 Frank Oltmanns 2023-07-02   30  		for (_n = nkm->min_n; _n <= nkm->max_n; _n++) {
8a97d9f6cbb457 Frank Oltmanns 2023-07-02   31  			for (_m = nkm->min_m; _m <= nkm->max_m; _m++) {
8a97d9f6cbb457 Frank Oltmanns 2023-07-02   32  				unsigned long tmp_rate;
8e5b4aea92912f Frank Oltmanns 2023-07-02   33  				unsigned long tmp_diff;
8a97d9f6cbb457 Frank Oltmanns 2023-07-02   34  
8a97d9f6cbb457 Frank Oltmanns 2023-07-02   35  				tmp_parent = clk_hw_round_rate(phw, rate * _m / (_n * _k));
8a97d9f6cbb457 Frank Oltmanns 2023-07-02   36  
8a97d9f6cbb457 Frank Oltmanns 2023-07-02   37  				tmp_rate = tmp_parent * _n * _k / _m;
8e5b4aea92912f Frank Oltmanns 2023-07-02   38  
8e5b4aea92912f Frank Oltmanns 2023-07-02   39  				if (features & CCU_FEATURE_CLOSEST_RATE) {
8e5b4aea92912f Frank Oltmanns 2023-07-02   40  					tmp_diff = rate > tmp_rate ?
8e5b4aea92912f Frank Oltmanns 2023-07-02   41  						   rate - tmp_rate :
8e5b4aea92912f Frank Oltmanns 2023-07-02   42  						   tmp_rate - rate;
8e5b4aea92912f Frank Oltmanns 2023-07-02   43  				} else {
8a97d9f6cbb457 Frank Oltmanns 2023-07-02   44  					if (tmp_rate > rate)
8a97d9f6cbb457 Frank Oltmanns 2023-07-02   45  						continue;
8e5b4aea92912f Frank Oltmanns 2023-07-02  @46  					tmp_diff = rate - tmp_diff;
8e5b4aea92912f Frank Oltmanns 2023-07-02   47  				}
8a97d9f6cbb457 Frank Oltmanns 2023-07-02   48  
8e5b4aea92912f Frank Oltmanns 2023-07-02   49  				if (tmp_diff < best_diff) {
8a97d9f6cbb457 Frank Oltmanns 2023-07-02   50  					best_rate = tmp_rate;
8a97d9f6cbb457 Frank Oltmanns 2023-07-02   51  					best_parent_rate = tmp_parent;
8e5b4aea92912f Frank Oltmanns 2023-07-02   52  					best_diff = tmp_diff;
8a97d9f6cbb457 Frank Oltmanns 2023-07-02   53  					best_n = _n;
8a97d9f6cbb457 Frank Oltmanns 2023-07-02   54  					best_k = _k;
8a97d9f6cbb457 Frank Oltmanns 2023-07-02   55  					best_m = _m;
8a97d9f6cbb457 Frank Oltmanns 2023-07-02   56  				}
8a97d9f6cbb457 Frank Oltmanns 2023-07-02   57  			}
8a97d9f6cbb457 Frank Oltmanns 2023-07-02   58  		}
8a97d9f6cbb457 Frank Oltmanns 2023-07-02   59  	}
8a97d9f6cbb457 Frank Oltmanns 2023-07-02   60  
8a97d9f6cbb457 Frank Oltmanns 2023-07-02   61  	nkm->n = best_n;
8a97d9f6cbb457 Frank Oltmanns 2023-07-02   62  	nkm->k = best_k;
8a97d9f6cbb457 Frank Oltmanns 2023-07-02   63  	nkm->m = best_m;
8a97d9f6cbb457 Frank Oltmanns 2023-07-02   64  
8a97d9f6cbb457 Frank Oltmanns 2023-07-02   65  	*parent = best_parent_rate;
8a97d9f6cbb457 Frank Oltmanns 2023-07-02   66  
8a97d9f6cbb457 Frank Oltmanns 2023-07-02   67  	return best_rate;
8a97d9f6cbb457 Frank Oltmanns 2023-07-02   68  }
8a97d9f6cbb457 Frank Oltmanns 2023-07-02   69  
657f477a89acb2 Samuel Holland 2022-12-31   70  static unsigned long ccu_nkm_find_best(unsigned long parent, unsigned long rate,
8e5b4aea92912f Frank Oltmanns 2023-07-02   71  				       struct _ccu_nkm *nkm, unsigned long features)
df6561e60244c0 Maxime Ripard  2016-06-29   72  {
df6561e60244c0 Maxime Ripard  2016-06-29   73  	unsigned long best_rate = 0;
8e5b4aea92912f Frank Oltmanns 2023-07-02   74  	unsigned long best_diff = ULONG_MAX;
df6561e60244c0 Maxime Ripard  2016-06-29   75  	unsigned long best_n = 0, best_k = 0, best_m = 0;
df6561e60244c0 Maxime Ripard  2016-06-29   76  	unsigned long _n, _k, _m;
df6561e60244c0 Maxime Ripard  2016-06-29   77  
6e0d50daa97f4b Maxime Ripard  2016-09-29   78  	for (_k = nkm->min_k; _k <= nkm->max_k; _k++) {
6e0d50daa97f4b Maxime Ripard  2016-09-29   79  		for (_n = nkm->min_n; _n <= nkm->max_n; _n++) {
6e0d50daa97f4b Maxime Ripard  2016-09-29   80  			for (_m = nkm->min_m; _m <= nkm->max_m; _m++) {
df6561e60244c0 Maxime Ripard  2016-06-29   81  				unsigned long tmp_rate;
8e5b4aea92912f Frank Oltmanns 2023-07-02   82  				unsigned long tmp_diff;
df6561e60244c0 Maxime Ripard  2016-06-29   83  
df6561e60244c0 Maxime Ripard  2016-06-29   84  				tmp_rate = parent * _n * _k / _m;
df6561e60244c0 Maxime Ripard  2016-06-29   85  
8e5b4aea92912f Frank Oltmanns 2023-07-02   86  				if (features & CCU_FEATURE_CLOSEST_RATE) {
8e5b4aea92912f Frank Oltmanns 2023-07-02   87  					tmp_diff = rate > tmp_rate ?
8e5b4aea92912f Frank Oltmanns 2023-07-02   88  						   rate - tmp_rate :
8e5b4aea92912f Frank Oltmanns 2023-07-02   89  						   tmp_rate - rate;
8e5b4aea92912f Frank Oltmanns 2023-07-02   90  				} else {
df6561e60244c0 Maxime Ripard  2016-06-29   91  					if (tmp_rate > rate)
df6561e60244c0 Maxime Ripard  2016-06-29   92  						continue;
8e5b4aea92912f Frank Oltmanns 2023-07-02  @93  					tmp_diff = rate - tmp_diff;
8e5b4aea92912f Frank Oltmanns 2023-07-02   94  				}
8e5b4aea92912f Frank Oltmanns 2023-07-02   95  
8e5b4aea92912f Frank Oltmanns 2023-07-02   96  				if (tmp_diff < best_diff) {
df6561e60244c0 Maxime Ripard  2016-06-29   97  					best_rate = tmp_rate;
8e5b4aea92912f Frank Oltmanns 2023-07-02   98  					best_diff = tmp_diff;
df6561e60244c0 Maxime Ripard  2016-06-29   99  					best_n = _n;
df6561e60244c0 Maxime Ripard  2016-06-29  100  					best_k = _k;
df6561e60244c0 Maxime Ripard  2016-06-29  101  					best_m = _m;
df6561e60244c0 Maxime Ripard  2016-06-29  102  				}
df6561e60244c0 Maxime Ripard  2016-06-29  103  			}
ee28648cb2b4d4 Maxime Ripard  2016-09-29  104  		}
ee28648cb2b4d4 Maxime Ripard  2016-09-29  105  	}
df6561e60244c0 Maxime Ripard  2016-06-29  106  
df6561e60244c0 Maxime Ripard  2016-06-29  107  	nkm->n = best_n;
df6561e60244c0 Maxime Ripard  2016-06-29  108  	nkm->k = best_k;
df6561e60244c0 Maxime Ripard  2016-06-29  109  	nkm->m = best_m;
657f477a89acb2 Samuel Holland 2022-12-31  110  
657f477a89acb2 Samuel Holland 2022-12-31  111  	return best_rate;
df6561e60244c0 Maxime Ripard  2016-06-29  112  }
df6561e60244c0 Maxime Ripard  2016-06-29  113  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 5/8] clk: sunxi-ng: nkm: Support finding closest rate
  2023-07-03  8:59         ` Frank Oltmanns
@ 2023-07-03 11:36           ` Maxime Ripard
  -1 siblings, 0 replies; 15+ messages in thread
From: Maxime Ripard @ 2023-07-03 11:36 UTC (permalink / raw)
  To: Frank Oltmanns
  Cc: Michael Turquette, Stephen Boyd, Chen-Yu Tsai, Jernej Skrabec,
	Samuel Holland, Andre Przywara, Roman Beranek, linux-clk,
	linux-arm-kernel, linux-sunxi, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 5561 bytes --]

On Mon, Jul 03, 2023 at 10:59:43AM +0200, Frank Oltmanns wrote:
> 
> 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.

The main issue will be probing and mocking. Those clocks are meant to be
probed through the device tree and expect to have underlying registers
accessible.

We would need some way to mock / prevent any register access, while
still registering a clock with its device tree node, parent, etc. for
the tests to be meaningful. And that's not going to be an easy thing to
do :)

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 5/8] clk: sunxi-ng: nkm: Support finding closest rate
@ 2023-07-03 11:36           ` Maxime Ripard
  0 siblings, 0 replies; 15+ messages in thread
From: Maxime Ripard @ 2023-07-03 11:36 UTC (permalink / raw)
  To: Frank Oltmanns
  Cc: Michael Turquette, Stephen Boyd, Chen-Yu Tsai, Jernej Skrabec,
	Samuel Holland, Andre Przywara, Roman Beranek, linux-clk,
	linux-arm-kernel, linux-sunxi, linux-kernel


[-- Attachment #1.1: Type: text/plain, Size: 5561 bytes --]

On Mon, Jul 03, 2023 at 10:59:43AM +0200, Frank Oltmanns wrote:
> 
> 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.

The main issue will be probing and mocking. Those clocks are meant to be
probed through the device tree and expect to have underlying registers
accessible.

We would need some way to mock / prevent any register access, while
still registering a clock with its device tree node, parent, etc. for
the tests to be meaningful. And that's not going to be an easy thing to
do :)

Maxime

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 5/8] clk: sunxi-ng: nkm: Support finding closest rate
  2023-07-03  7:25       ` Maxime Ripard
@ 2023-07-03  8:59         ` Frank Oltmanns
  -1 siblings, 0 replies; 15+ messages in thread
From: Frank Oltmanns @ 2023-07-03  8:59 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Michael Turquette, Stephen Boyd, Chen-Yu Tsai, Jernej Skrabec,
	Samuel Holland, Andre Przywara, Roman Beranek, linux-clk,
	linux-arm-kernel, linux-sunxi, linux-kernel


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]]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 5/8] clk: sunxi-ng: nkm: Support finding closest rate
@ 2023-07-03  8:59         ` Frank Oltmanns
  0 siblings, 0 replies; 15+ messages in thread
From: Frank Oltmanns @ 2023-07-03  8:59 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Michael Turquette, Stephen Boyd, Chen-Yu Tsai, Jernej Skrabec,
	Samuel Holland, Andre Przywara, Roman Beranek, linux-clk,
	linux-arm-kernel, linux-sunxi, linux-kernel


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]]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 5/8] clk: sunxi-ng: nkm: Support finding closest rate
  2023-07-02 17:55   ` Frank Oltmanns
@ 2023-07-03  7:33     ` Maxime Ripard
  -1 siblings, 0 replies; 15+ messages in thread
From: Maxime Ripard @ 2023-07-03  7:33 UTC (permalink / raw)
  To: Frank Oltmanns
  Cc: Michael Turquette, Stephen Boyd, Chen-Yu Tsai, Jernej Skrabec,
	Samuel Holland, Andre Przywara, Roman Beranek, linux-clk,
	linux-arm-kernel, linux-sunxi, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 2623 bytes --]

On Sun, Jul 02, 2023 at 07:55:24PM +0200, Frank Oltmanns 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;
> +			}
>  		}
>  	}

Like I said in the previous patch, I think we could do something like:

bool ccu_is_better_rate(struct ccu_common *common,
     			unsigned long target_rate,
			unsigned long current_rate,
			unsigned long best_rate)
{
	if (common->features & CCU_FEATURE_CLOSEST_RATE)
		return abs(current_rate - target_rate) < abs(best_rate - target_rate);

	return current_rate <= target_rate && current_rate > best_rate;
}

Then, the code above would look like:

if (ccu_is_better_rate(common, req->rate, tmp_rate, best_rate)) {
	best_rate = tmp_rate;
	best_parent_rate = parent_rate;
	best_parent = parent;
}

It's simpler, and we can share it easily between drivers.
Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 5/8] clk: sunxi-ng: nkm: Support finding closest rate
@ 2023-07-03  7:33     ` Maxime Ripard
  0 siblings, 0 replies; 15+ messages in thread
From: Maxime Ripard @ 2023-07-03  7:33 UTC (permalink / raw)
  To: Frank Oltmanns
  Cc: Michael Turquette, Stephen Boyd, Chen-Yu Tsai, Jernej Skrabec,
	Samuel Holland, Andre Przywara, Roman Beranek, linux-clk,
	linux-arm-kernel, linux-sunxi, linux-kernel


[-- Attachment #1.1: Type: text/plain, Size: 2623 bytes --]

On Sun, Jul 02, 2023 at 07:55:24PM +0200, Frank Oltmanns 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;
> +			}
>  		}
>  	}

Like I said in the previous patch, I think we could do something like:

bool ccu_is_better_rate(struct ccu_common *common,
     			unsigned long target_rate,
			unsigned long current_rate,
			unsigned long best_rate)
{
	if (common->features & CCU_FEATURE_CLOSEST_RATE)
		return abs(current_rate - target_rate) < abs(best_rate - target_rate);

	return current_rate <= target_rate && current_rate > best_rate;
}

Then, the code above would look like:

if (ccu_is_better_rate(common, req->rate, tmp_rate, best_rate)) {
	best_rate = tmp_rate;
	best_parent_rate = parent_rate;
	best_parent = parent;
}

It's simpler, and we can share it easily between drivers.
Maxime

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 5/8] clk: sunxi-ng: nkm: Support finding closest rate
  2023-07-03  7:17     ` Frank Oltmanns
@ 2023-07-03  7:25       ` Maxime Ripard
  -1 siblings, 0 replies; 15+ messages in thread
From: Maxime Ripard @ 2023-07-03  7:25 UTC (permalink / raw)
  To: Frank Oltmanns
  Cc: Michael Turquette, Stephen Boyd, Chen-Yu Tsai, Jernej Skrabec,
	Samuel Holland, Andre Przywara, Roman Beranek, linux-clk,
	linux-arm-kernel, linux-sunxi, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 4335 bytes --]

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.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 5/8] clk: sunxi-ng: nkm: Support finding closest rate
@ 2023-07-03  7:25       ` Maxime Ripard
  0 siblings, 0 replies; 15+ messages in thread
From: Maxime Ripard @ 2023-07-03  7:25 UTC (permalink / raw)
  To: Frank Oltmanns
  Cc: Michael Turquette, Stephen Boyd, Chen-Yu Tsai, Jernej Skrabec,
	Samuel Holland, Andre Przywara, Roman Beranek, linux-clk,
	linux-arm-kernel, linux-sunxi, linux-kernel


[-- Attachment #1.1: Type: text/plain, Size: 4335 bytes --]

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.

Maxime

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 5/8] clk: sunxi-ng: nkm: Support finding closest rate
  2023-07-02 17:55   ` Frank Oltmanns
@ 2023-07-03  7:17     ` Frank Oltmanns
  -1 siblings, 0 replies; 15+ messages in thread
From: Frank Oltmanns @ 2023-07-03  7:17 UTC (permalink / raw)
  To: Frank Oltmanns
  Cc: Maxime Ripard, Michael Turquette, Stephen Boyd, Chen-Yu Tsai,
	Jernej Skrabec, Samuel Holland, Andre Przywara, Roman Beranek,
	linux-clk, linux-arm-kernel, linux-sunxi, linux-kernel


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.

Best regards,
  Frank

> +				}
> +
> +				if (tmp_diff < best_diff) {
>  					best_rate = tmp_rate;
>  					best_parent_rate = tmp_parent;
> +					best_diff = tmp_diff;
>  					best_n = _n;
>  					best_k = _k;
>  					best_m = _m;
> @@ -56,9 +68,10 @@ static unsigned long ccu_nkm_find_best_with_parent_adj(unsigned long *parent, un
>  }
>
>  static unsigned long ccu_nkm_find_best(unsigned long parent, unsigned long rate,
> -				       struct _ccu_nkm *nkm)
> +				       struct _ccu_nkm *nkm, unsigned long features)
>  {
>  	unsigned long best_rate = 0;
> +	unsigned long best_diff = ULONG_MAX;
>  	unsigned long best_n = 0, best_k = 0, best_m = 0;
>  	unsigned long _n, _k, _m;
>
> @@ -66,13 +79,23 @@ static unsigned long ccu_nkm_find_best(unsigned long parent, unsigned long rate,
>  		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_rate = 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;
> +				}
> +
> +				if (tmp_diff < best_diff) {
>  					best_rate = tmp_rate;
> +					best_diff = tmp_diff;
>  					best_n = _n;
>  					best_k = _k;
>  					best_m = _m;
> @@ -164,9 +187,10 @@ static unsigned long ccu_nkm_round_rate(struct ccu_mux_internal *mux,
>  		rate *= nkm->fixed_post_div;
>
>  	if (!clk_hw_can_set_rate_parent(&nkm->common.hw))
> -		rate = ccu_nkm_find_best(*parent_rate, rate, &_nkm);
> +		rate = ccu_nkm_find_best(*parent_rate, rate, &_nkm, nkm->common.features);
>  	else
> -		rate = ccu_nkm_find_best_with_parent_adj(parent_rate, rate, &_nkm, parent_hw);
> +		rate = ccu_nkm_find_best_with_parent_adj(parent_rate, rate, &_nkm, parent_hw,
> +							 nkm->common.features);
>
>  	if (nkm->common.features & CCU_FEATURE_FIXED_POSTDIV)
>  		rate /= nkm->fixed_post_div;
> @@ -201,7 +225,7 @@ static int ccu_nkm_set_rate(struct clk_hw *hw, unsigned long rate,
>  	_nkm.min_m = 1;
>  	_nkm.max_m = nkm->m.max ?: 1 << nkm->m.width;
>
> -	ccu_nkm_find_best(&parent_rate, rate, &_nkm);
> +	ccu_nkm_find_best(parent_rate, rate, &_nkm, nkm->common.features);
>
>  	spin_lock_irqsave(nkm->common.lock, flags);

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 5/8] clk: sunxi-ng: nkm: Support finding closest rate
@ 2023-07-03  7:17     ` Frank Oltmanns
  0 siblings, 0 replies; 15+ messages in thread
From: Frank Oltmanns @ 2023-07-03  7:17 UTC (permalink / raw)
  To: Frank Oltmanns
  Cc: Maxime Ripard, Michael Turquette, Stephen Boyd, Chen-Yu Tsai,
	Jernej Skrabec, Samuel Holland, Andre Przywara, Roman Beranek,
	linux-clk, linux-arm-kernel, linux-sunxi, linux-kernel


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.

Best regards,
  Frank

> +				}
> +
> +				if (tmp_diff < best_diff) {
>  					best_rate = tmp_rate;
>  					best_parent_rate = tmp_parent;
> +					best_diff = tmp_diff;
>  					best_n = _n;
>  					best_k = _k;
>  					best_m = _m;
> @@ -56,9 +68,10 @@ static unsigned long ccu_nkm_find_best_with_parent_adj(unsigned long *parent, un
>  }
>
>  static unsigned long ccu_nkm_find_best(unsigned long parent, unsigned long rate,
> -				       struct _ccu_nkm *nkm)
> +				       struct _ccu_nkm *nkm, unsigned long features)
>  {
>  	unsigned long best_rate = 0;
> +	unsigned long best_diff = ULONG_MAX;
>  	unsigned long best_n = 0, best_k = 0, best_m = 0;
>  	unsigned long _n, _k, _m;
>
> @@ -66,13 +79,23 @@ static unsigned long ccu_nkm_find_best(unsigned long parent, unsigned long rate,
>  		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_rate = 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;
> +				}
> +
> +				if (tmp_diff < best_diff) {
>  					best_rate = tmp_rate;
> +					best_diff = tmp_diff;
>  					best_n = _n;
>  					best_k = _k;
>  					best_m = _m;
> @@ -164,9 +187,10 @@ static unsigned long ccu_nkm_round_rate(struct ccu_mux_internal *mux,
>  		rate *= nkm->fixed_post_div;
>
>  	if (!clk_hw_can_set_rate_parent(&nkm->common.hw))
> -		rate = ccu_nkm_find_best(*parent_rate, rate, &_nkm);
> +		rate = ccu_nkm_find_best(*parent_rate, rate, &_nkm, nkm->common.features);
>  	else
> -		rate = ccu_nkm_find_best_with_parent_adj(parent_rate, rate, &_nkm, parent_hw);
> +		rate = ccu_nkm_find_best_with_parent_adj(parent_rate, rate, &_nkm, parent_hw,
> +							 nkm->common.features);
>
>  	if (nkm->common.features & CCU_FEATURE_FIXED_POSTDIV)
>  		rate /= nkm->fixed_post_div;
> @@ -201,7 +225,7 @@ static int ccu_nkm_set_rate(struct clk_hw *hw, unsigned long rate,
>  	_nkm.min_m = 1;
>  	_nkm.max_m = nkm->m.max ?: 1 << nkm->m.width;
>
> -	ccu_nkm_find_best(&parent_rate, rate, &_nkm);
> +	ccu_nkm_find_best(parent_rate, rate, &_nkm, nkm->common.features);
>
>  	spin_lock_irqsave(nkm->common.lock, flags);

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 5/8] clk: sunxi-ng: nkm: Support finding closest rate
  2023-07-02 17:55   ` Frank Oltmanns
@ 2023-07-02 20:06     ` kernel test robot
  -1 siblings, 0 replies; 15+ messages in thread
From: kernel test robot @ 2023-07-02 20:06 UTC (permalink / raw)
  To: Frank Oltmanns, Maxime Ripard, Michael Turquette, Stephen Boyd,
	Chen-Yu Tsai, Jernej Skrabec, Samuel Holland, Andre Przywara,
	Roman Beranek
  Cc: llvm, oe-kbuild-all, linux-clk, linux-arm-kernel, linux-sunxi,
	linux-kernel, Frank Oltmanns

Hi Frank,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 6995e2de6891c724bfeb2db33d7b87775f913ad1]

url:    https://github.com/intel-lab-lkp/linux/commits/Frank-Oltmanns/clk-sunxi-ng-nkm-consider-alternative-parent-rates-when-determining-rate/20230703-015726
base:   6995e2de6891c724bfeb2db33d7b87775f913ad1
patch link:    https://lore.kernel.org/r/20230702-pll-mipi_set_rate_parent-v3-5-46dcb8aa9cbc%40oltmanns.dev
patch subject: [PATCH v3 5/8] clk: sunxi-ng: nkm: Support finding closest rate
config: riscv-rv32_defconfig (https://download.01.org/0day-ci/archive/20230703/202307030302.s1bheEun-lkp@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project.git 4a5ac14ee968ff0ad5d2cc1ffa0299048db4c88a)
reproduce: (https://download.01.org/0day-ci/archive/20230703/202307030302.s1bheEun-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202307030302.s1bheEun-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/clk/sunxi-ng/ccu_nkm.c:46:24: warning: variable 'tmp_diff' is uninitialized when used here [-Wuninitialized]
      46 |                                         tmp_diff = rate - tmp_diff;
         |                                                           ^~~~~~~~
   drivers/clk/sunxi-ng/ccu_nkm.c:33:27: note: initialize the variable 'tmp_diff' to silence this warning
      33 |                                 unsigned long tmp_diff;
         |                                                       ^
         |                                                        = 0
   drivers/clk/sunxi-ng/ccu_nkm.c:93:24: warning: variable 'tmp_diff' is uninitialized when used here [-Wuninitialized]
      93 |                                         tmp_diff = rate - tmp_diff;
         |                                                           ^~~~~~~~
   drivers/clk/sunxi-ng/ccu_nkm.c:82:27: note: initialize the variable 'tmp_diff' to silence this warning
      82 |                                 unsigned long tmp_diff;
         |                                                       ^
         |                                                        = 0
   2 warnings generated.


vim +/tmp_diff +46 drivers/clk/sunxi-ng/ccu_nkm.c

    19	
    20	static unsigned long ccu_nkm_find_best_with_parent_adj(unsigned long *parent, unsigned long rate,
    21							       struct _ccu_nkm *nkm, struct clk_hw *phw,
    22							       unsigned long features)
    23	{
    24		unsigned long best_rate = 0, best_parent_rate = 0, tmp_parent = *parent;
    25		unsigned long best_diff = ULONG_MAX;
    26		unsigned long best_n = 0, best_k = 0, best_m = 0;
    27		unsigned long _n, _k, _m;
    28	
    29		for (_k = nkm->min_k; _k <= nkm->max_k; _k++) {
    30			for (_n = nkm->min_n; _n <= nkm->max_n; _n++) {
    31				for (_m = nkm->min_m; _m <= nkm->max_m; _m++) {
    32					unsigned long tmp_rate;
    33					unsigned long tmp_diff;
    34	
    35					tmp_parent = clk_hw_round_rate(phw, rate * _m / (_n * _k));
    36	
    37					tmp_rate = tmp_parent * _n * _k / _m;
    38	
    39					if (features & CCU_FEATURE_CLOSEST_RATE) {
    40						tmp_diff = rate > tmp_rate ?
    41							   rate - tmp_rate :
    42							   tmp_rate - rate;
    43					} else {
    44						if (tmp_rate > rate)
    45							continue;
  > 46						tmp_diff = rate - tmp_diff;
    47					}
    48	
    49					if (tmp_diff < best_diff) {
    50						best_rate = tmp_rate;
    51						best_parent_rate = tmp_parent;
    52						best_diff = tmp_diff;
    53						best_n = _n;
    54						best_k = _k;
    55						best_m = _m;
    56					}
    57				}
    58			}
    59		}
    60	
    61		nkm->n = best_n;
    62		nkm->k = best_k;
    63		nkm->m = best_m;
    64	
    65		*parent = best_parent_rate;
    66	
    67		return best_rate;
    68	}
    69	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 5/8] clk: sunxi-ng: nkm: Support finding closest rate
@ 2023-07-02 20:06     ` kernel test robot
  0 siblings, 0 replies; 15+ messages in thread
From: kernel test robot @ 2023-07-02 20:06 UTC (permalink / raw)
  To: Frank Oltmanns, Maxime Ripard, Michael Turquette, Stephen Boyd,
	Chen-Yu Tsai, Jernej Skrabec, Samuel Holland, Andre Przywara,
	Roman Beranek
  Cc: llvm, oe-kbuild-all, linux-clk, linux-arm-kernel, linux-sunxi,
	linux-kernel, Frank Oltmanns

Hi Frank,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 6995e2de6891c724bfeb2db33d7b87775f913ad1]

url:    https://github.com/intel-lab-lkp/linux/commits/Frank-Oltmanns/clk-sunxi-ng-nkm-consider-alternative-parent-rates-when-determining-rate/20230703-015726
base:   6995e2de6891c724bfeb2db33d7b87775f913ad1
patch link:    https://lore.kernel.org/r/20230702-pll-mipi_set_rate_parent-v3-5-46dcb8aa9cbc%40oltmanns.dev
patch subject: [PATCH v3 5/8] clk: sunxi-ng: nkm: Support finding closest rate
config: riscv-rv32_defconfig (https://download.01.org/0day-ci/archive/20230703/202307030302.s1bheEun-lkp@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project.git 4a5ac14ee968ff0ad5d2cc1ffa0299048db4c88a)
reproduce: (https://download.01.org/0day-ci/archive/20230703/202307030302.s1bheEun-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202307030302.s1bheEun-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/clk/sunxi-ng/ccu_nkm.c:46:24: warning: variable 'tmp_diff' is uninitialized when used here [-Wuninitialized]
      46 |                                         tmp_diff = rate - tmp_diff;
         |                                                           ^~~~~~~~
   drivers/clk/sunxi-ng/ccu_nkm.c:33:27: note: initialize the variable 'tmp_diff' to silence this warning
      33 |                                 unsigned long tmp_diff;
         |                                                       ^
         |                                                        = 0
   drivers/clk/sunxi-ng/ccu_nkm.c:93:24: warning: variable 'tmp_diff' is uninitialized when used here [-Wuninitialized]
      93 |                                         tmp_diff = rate - tmp_diff;
         |                                                           ^~~~~~~~
   drivers/clk/sunxi-ng/ccu_nkm.c:82:27: note: initialize the variable 'tmp_diff' to silence this warning
      82 |                                 unsigned long tmp_diff;
         |                                                       ^
         |                                                        = 0
   2 warnings generated.


vim +/tmp_diff +46 drivers/clk/sunxi-ng/ccu_nkm.c

    19	
    20	static unsigned long ccu_nkm_find_best_with_parent_adj(unsigned long *parent, unsigned long rate,
    21							       struct _ccu_nkm *nkm, struct clk_hw *phw,
    22							       unsigned long features)
    23	{
    24		unsigned long best_rate = 0, best_parent_rate = 0, tmp_parent = *parent;
    25		unsigned long best_diff = ULONG_MAX;
    26		unsigned long best_n = 0, best_k = 0, best_m = 0;
    27		unsigned long _n, _k, _m;
    28	
    29		for (_k = nkm->min_k; _k <= nkm->max_k; _k++) {
    30			for (_n = nkm->min_n; _n <= nkm->max_n; _n++) {
    31				for (_m = nkm->min_m; _m <= nkm->max_m; _m++) {
    32					unsigned long tmp_rate;
    33					unsigned long tmp_diff;
    34	
    35					tmp_parent = clk_hw_round_rate(phw, rate * _m / (_n * _k));
    36	
    37					tmp_rate = tmp_parent * _n * _k / _m;
    38	
    39					if (features & CCU_FEATURE_CLOSEST_RATE) {
    40						tmp_diff = rate > tmp_rate ?
    41							   rate - tmp_rate :
    42							   tmp_rate - rate;
    43					} else {
    44						if (tmp_rate > rate)
    45							continue;
  > 46						tmp_diff = rate - tmp_diff;
    47					}
    48	
    49					if (tmp_diff < best_diff) {
    50						best_rate = tmp_rate;
    51						best_parent_rate = tmp_parent;
    52						best_diff = tmp_diff;
    53						best_n = _n;
    54						best_k = _k;
    55						best_m = _m;
    56					}
    57				}
    58			}
    59		}
    60	
    61		nkm->n = best_n;
    62		nkm->k = best_k;
    63		nkm->m = best_m;
    64	
    65		*parent = best_parent_rate;
    66	
    67		return best_rate;
    68	}
    69	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v3 5/8] clk: sunxi-ng: nkm: Support finding closest rate
  2023-07-02 17:55 [PATCH v3 0/8] clk: sunxi-ng: Consider alternative parent rates when determining NKM clock rate Frank Oltmanns
@ 2023-07-02 17:55   ` Frank Oltmanns
  0 siblings, 0 replies; 15+ messages in thread
From: Frank Oltmanns @ 2023-07-02 17:55 UTC (permalink / raw)
  To: Maxime Ripard, Michael Turquette, Stephen Boyd, Chen-Yu Tsai,
	Jernej Skrabec, Samuel Holland, Andre Przywara, Roman Beranek
  Cc: linux-clk, linux-arm-kernel, linux-sunxi, linux-kernel, Frank Oltmanns

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;
+				}
+
+				if (tmp_diff < best_diff) {
 					best_rate = tmp_rate;
 					best_parent_rate = tmp_parent;
+					best_diff = tmp_diff;
 					best_n = _n;
 					best_k = _k;
 					best_m = _m;
@@ -56,9 +68,10 @@ static unsigned long ccu_nkm_find_best_with_parent_adj(unsigned long *parent, un
 }
 
 static unsigned long ccu_nkm_find_best(unsigned long parent, unsigned long rate,
-				       struct _ccu_nkm *nkm)
+				       struct _ccu_nkm *nkm, unsigned long features)
 {
 	unsigned long best_rate = 0;
+	unsigned long best_diff = ULONG_MAX;
 	unsigned long best_n = 0, best_k = 0, best_m = 0;
 	unsigned long _n, _k, _m;
 
@@ -66,13 +79,23 @@ static unsigned long ccu_nkm_find_best(unsigned long parent, unsigned long rate,
 		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_rate = 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;
+				}
+
+				if (tmp_diff < best_diff) {
 					best_rate = tmp_rate;
+					best_diff = tmp_diff;
 					best_n = _n;
 					best_k = _k;
 					best_m = _m;
@@ -164,9 +187,10 @@ static unsigned long ccu_nkm_round_rate(struct ccu_mux_internal *mux,
 		rate *= nkm->fixed_post_div;
 
 	if (!clk_hw_can_set_rate_parent(&nkm->common.hw))
-		rate = ccu_nkm_find_best(*parent_rate, rate, &_nkm);
+		rate = ccu_nkm_find_best(*parent_rate, rate, &_nkm, nkm->common.features);
 	else
-		rate = ccu_nkm_find_best_with_parent_adj(parent_rate, rate, &_nkm, parent_hw);
+		rate = ccu_nkm_find_best_with_parent_adj(parent_rate, rate, &_nkm, parent_hw,
+							 nkm->common.features);
 
 	if (nkm->common.features & CCU_FEATURE_FIXED_POSTDIV)
 		rate /= nkm->fixed_post_div;
@@ -201,7 +225,7 @@ static int ccu_nkm_set_rate(struct clk_hw *hw, unsigned long rate,
 	_nkm.min_m = 1;
 	_nkm.max_m = nkm->m.max ?: 1 << nkm->m.width;
 
-	ccu_nkm_find_best(&parent_rate, rate, &_nkm);
+	ccu_nkm_find_best(parent_rate, rate, &_nkm, nkm->common.features);
 
 	spin_lock_irqsave(nkm->common.lock, flags);
 

-- 
2.41.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v3 5/8] clk: sunxi-ng: nkm: Support finding closest rate
@ 2023-07-02 17:55   ` Frank Oltmanns
  0 siblings, 0 replies; 15+ messages in thread
From: Frank Oltmanns @ 2023-07-02 17:55 UTC (permalink / raw)
  To: Maxime Ripard, Michael Turquette, Stephen Boyd, Chen-Yu Tsai,
	Jernej Skrabec, Samuel Holland, Andre Przywara, Roman Beranek
  Cc: linux-clk, linux-arm-kernel, linux-sunxi, linux-kernel, Frank Oltmanns

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;
+				}
+
+				if (tmp_diff < best_diff) {
 					best_rate = tmp_rate;
 					best_parent_rate = tmp_parent;
+					best_diff = tmp_diff;
 					best_n = _n;
 					best_k = _k;
 					best_m = _m;
@@ -56,9 +68,10 @@ static unsigned long ccu_nkm_find_best_with_parent_adj(unsigned long *parent, un
 }
 
 static unsigned long ccu_nkm_find_best(unsigned long parent, unsigned long rate,
-				       struct _ccu_nkm *nkm)
+				       struct _ccu_nkm *nkm, unsigned long features)
 {
 	unsigned long best_rate = 0;
+	unsigned long best_diff = ULONG_MAX;
 	unsigned long best_n = 0, best_k = 0, best_m = 0;
 	unsigned long _n, _k, _m;
 
@@ -66,13 +79,23 @@ static unsigned long ccu_nkm_find_best(unsigned long parent, unsigned long rate,
 		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_rate = 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;
+				}
+
+				if (tmp_diff < best_diff) {
 					best_rate = tmp_rate;
+					best_diff = tmp_diff;
 					best_n = _n;
 					best_k = _k;
 					best_m = _m;
@@ -164,9 +187,10 @@ static unsigned long ccu_nkm_round_rate(struct ccu_mux_internal *mux,
 		rate *= nkm->fixed_post_div;
 
 	if (!clk_hw_can_set_rate_parent(&nkm->common.hw))
-		rate = ccu_nkm_find_best(*parent_rate, rate, &_nkm);
+		rate = ccu_nkm_find_best(*parent_rate, rate, &_nkm, nkm->common.features);
 	else
-		rate = ccu_nkm_find_best_with_parent_adj(parent_rate, rate, &_nkm, parent_hw);
+		rate = ccu_nkm_find_best_with_parent_adj(parent_rate, rate, &_nkm, parent_hw,
+							 nkm->common.features);
 
 	if (nkm->common.features & CCU_FEATURE_FIXED_POSTDIV)
 		rate /= nkm->fixed_post_div;
@@ -201,7 +225,7 @@ static int ccu_nkm_set_rate(struct clk_hw *hw, unsigned long rate,
 	_nkm.min_m = 1;
 	_nkm.max_m = nkm->m.max ?: 1 << nkm->m.width;
 
-	ccu_nkm_find_best(&parent_rate, rate, &_nkm);
+	ccu_nkm_find_best(parent_rate, rate, &_nkm, nkm->common.features);
 
 	spin_lock_irqsave(nkm->common.lock, flags);
 

-- 
2.41.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2023-07-03 20:19 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-07-03 20:19 [PATCH v3 5/8] clk: sunxi-ng: nkm: Support finding closest rate kernel test robot
  -- strict thread matches above, loose matches on Subject: below --
2023-07-02 17:55 [PATCH v3 0/8] clk: sunxi-ng: Consider alternative parent rates when determining NKM clock rate Frank Oltmanns
2023-07-02 17:55 ` [PATCH v3 5/8] clk: sunxi-ng: nkm: Support finding closest rate Frank Oltmanns
2023-07-02 17:55   ` Frank Oltmanns
2023-07-02 20:06   ` kernel test robot
2023-07-02 20:06     ` kernel test robot
2023-07-03  7:17   ` Frank Oltmanns
2023-07-03  7:17     ` Frank Oltmanns
2023-07-03  7:25     ` Maxime Ripard
2023-07-03  7:25       ` Maxime Ripard
2023-07-03  8:59       ` Frank Oltmanns
2023-07-03  8:59         ` Frank Oltmanns
2023-07-03 11:36         ` Maxime Ripard
2023-07-03 11:36           ` Maxime Ripard
2023-07-03  7:33   ` Maxime Ripard
2023-07-03  7:33     ` Maxime Ripard

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.