Re: [PATCH RFC v1 3/3] clk: meson: pll: switch to determine_rate for the PLL ops

[Jerome Brunet]

On Mon 17 May 2021 at 22:37, Martin Blumenstingl <martin.blumenstingl@xxxxxxxxxxxxxx> wrote:

> This increases the maxmium supported frequency on 32-bit systems from
> 2^31 (signed long as used by clk_ops.round_rate, maximum value:
> approx. 2.14GHz) to 2^32 (unsigned long as used by
> clk_ops.determine_rate, maximum value: approx. 4.29GHz).
> On Meson8/8b/8m2 the HDMI PLL and it's OD (post-dividers) are
> capable of running at up to 2.97GHz. So switch the divider
> implementation in clk-regmap to clk_ops.determine_rate to support these
> higher frequencies on 32-bit systems.
>
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@xxxxxxxxxxxxxx>

Looks good. I see no reason to keep this one as RFC.
I can take it directly if this is OK with you ?

> ---
>  drivers/clk/meson/clk-pll.c | 26 +++++++++++++++-----------
>  1 file changed, 15 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c
> index 49f27fe53213..9e55617bc3b4 100644
> --- a/drivers/clk/meson/clk-pll.c
> +++ b/drivers/clk/meson/clk-pll.c
> @@ -242,8 +242,8 @@ static int meson_clk_get_pll_settings(unsigned long rate,
>  	return best ? 0 : -EINVAL;
>  }
>  
> -static long meson_clk_pll_round_rate(struct clk_hw *hw, unsigned long rate,
> -				     unsigned long *parent_rate)
> +static int meson_clk_pll_determine_rate(struct clk_hw *hw,
> +					struct clk_rate_request *req)
>  {
>  	struct clk_regmap *clk = to_clk_regmap(hw);
>  	struct meson_clk_pll_data *pll = meson_clk_pll_data(clk);
> @@ -251,22 +251,26 @@ static long meson_clk_pll_round_rate(struct clk_hw *hw, unsigned long rate,
>  	unsigned long round;
>  	int ret;
>  
> -	ret = meson_clk_get_pll_settings(rate, *parent_rate, &m, &n, pll);
> +	ret = meson_clk_get_pll_settings(req->rate, req->best_parent_rate,
> +					 &m, &n, pll);
>  	if (ret)
> -		return meson_clk_pll_recalc_rate(hw, *parent_rate);
> +		return ret;
>  
> -	round = __pll_params_to_rate(*parent_rate, m, n, 0, pll);
> +	round = __pll_params_to_rate(req->best_parent_rate, m, n, 0, pll);
>  
> -	if (!MESON_PARM_APPLICABLE(&pll->frac) || rate == round)
> -		return round;
> +	if (!MESON_PARM_APPLICABLE(&pll->frac) || req->rate == round) {
> +		req->rate = round;
> +		return 0;
> +	}
>  
>  	/*
>  	 * The rate provided by the setting is not an exact match, let's
>  	 * try to improve the result using the fractional parameter
>  	 */
> -	frac = __pll_params_with_frac(rate, *parent_rate, m, n, pll);
> +	frac = __pll_params_with_frac(req->rate, req->best_parent_rate, m, n, pll);
> +	req->rate = __pll_params_to_rate(req->best_parent_rate, m, n, frac, pll);
>  
> -	return __pll_params_to_rate(*parent_rate, m, n, frac, pll);
> +	return 0;
>  }
>  
>  static int meson_clk_pll_wait_lock(struct clk_hw *hw)
> @@ -419,7 +423,7 @@ static int meson_clk_pll_set_rate(struct clk_hw *hw, unsigned long rate,
>   */
>  const struct clk_ops meson_clk_pcie_pll_ops = {
>  	.recalc_rate	= meson_clk_pll_recalc_rate,
> -	.round_rate	= meson_clk_pll_round_rate,
> +	.determine_rate	= meson_clk_pll_determine_rate,
>  	.is_enabled	= meson_clk_pll_is_enabled,
>  	.enable		= meson_clk_pcie_pll_enable,
>  	.disable	= meson_clk_pll_disable
> @@ -429,7 +433,7 @@ EXPORT_SYMBOL_GPL(meson_clk_pcie_pll_ops);
>  const struct clk_ops meson_clk_pll_ops = {
>  	.init		= meson_clk_pll_init,
>  	.recalc_rate	= meson_clk_pll_recalc_rate,
> -	.round_rate	= meson_clk_pll_round_rate,
> +	.determine_rate	= meson_clk_pll_determine_rate,
>  	.set_rate	= meson_clk_pll_set_rate,
>  	.is_enabled	= meson_clk_pll_is_enabled,
>  	.enable		= meson_clk_pll_enable,