Re: [PATCH 1/2] clk: sunxi-ng: nkm: consider alternative parent rates when finding rate

[Maxime Ripard]

On Mon, Jun 05, 2023 at 09:07:44PM +0200, Frank Oltmanns wrote:
> In case the CLK_SET_RATE_PARENT flag is set, consider using a different
> parent rate when determining a new rate.
> 
> To find the best match for the requested rate, perform the following
> steps for each NKM combination:
>  - calculate the optimal parent rate,
>  - find the best parent rate that the parent clock actually supports
>  - use that parent rate to calculate the effective rate.
> 
> In case the clk does not support setting the parent rate, use the same
> algorithm as before.
> 
> Signed-off-by: Frank Oltmanns <frank@xxxxxxxxxxxx>
> ---
>  drivers/clk/sunxi-ng/ccu_nkm.c | 40 ++++++++++++++++++++++++++--------
>  1 file changed, 31 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/clk/sunxi-ng/ccu_nkm.c b/drivers/clk/sunxi-ng/ccu_nkm.c
> index a0978a50edae..c71e237226f2 100644
> --- a/drivers/clk/sunxi-ng/ccu_nkm.c
> +++ b/drivers/clk/sunxi-ng/ccu_nkm.c
> @@ -16,10 +16,10 @@ struct _ccu_nkm {
>  	unsigned long	m, min_m, max_m;
>  };
>  
> -static unsigned long ccu_nkm_find_best(unsigned long parent, unsigned long rate,
> -				       struct _ccu_nkm *nkm)
> +static unsigned long ccu_nkm_find_best(unsigned long *parent, unsigned long rate,
> +				       struct _ccu_nkm *nkm, struct clk_hw *parent_hw)
>  {
> -	unsigned long best_rate = 0;
> +	unsigned long best_rate = 0, best_parent_rate = 0, tmp_parent = *parent;
>  	unsigned long best_n = 0, best_k = 0, best_m = 0;
>  	unsigned long _n, _k, _m;
>  
> @@ -28,12 +28,29 @@ static unsigned long ccu_nkm_find_best(unsigned long parent, unsigned long rate,
>  			for (_m = nkm->min_m; _m <= nkm->max_m; _m++) {
>  				unsigned long tmp_rate;
>  
> -				tmp_rate = parent * _n * _k / _m;
> +				if (parent_hw) {

NKM clocks always have a parent

You should test if the CLK_SET_RATE_PARENT flag is set.

> +					// We must round up the desired parent rate, because the
> +					// rounding down happens when calculating tmp_rate. If we
> +					// round down also here, we'd round down twice.
> +					unsigned long optimal_parent =
> +							(rate * _m + (_n * _k - 1)) / _n / _k;

I assume the addition of n * k - 1 is to round up, but I'm not sure we
should hack around like that.

You should compute the ideal parent rate for a given set of timings, and
then just call round_rate on it. If the parent wants to round it one way
or another, that's the parent concern.

Maxime

Attachment: signature.asc
Description: PGP signature