From: Shengjiu Wang <shengjiu.wang@nxp.com>

The fvco frequency range is between 1600MHz and 3200MHz, without
this constraint the fvco may out of range, the real output
frequency is no accurate.

Aslo correct the name for fvco and fout clock.

Fixes: b09c68dc57c9 ("clk: imx: pll14xx: Support dynamic rates")
Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
Acked-by: Jacky Bai <ping.bai@nxp.com>
Tested-by: Chancel Liu <chancel.liu@nxp.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/clk/imx/clk-pll14xx.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/imx/clk-pll14xx.c b/drivers/clk/imx/clk-pll14xx.c
index d63564dbb12c..55812bfb9ec2 100644
--- a/drivers/clk/imx/clk-pll14xx.c
+++ b/drivers/clk/imx/clk-pll14xx.c
@@ -131,7 +131,7 @@ static void imx_pll14xx_calc_settings(struct clk_pll14xx *pll, unsigned long rat
 {
 	u32 pll_div_ctl0, pll_div_ctl1;
 	int mdiv, pdiv, sdiv, kdiv;
-	long fout, rate_min, rate_max, dist, best = LONG_MAX;
+	long fvco, fout, rate_min, rate_max, dist, best = LONG_MAX;
 	const struct imx_pll14xx_rate_table *tt;
 
 	/*
@@ -144,6 +144,8 @@ static void imx_pll14xx_calc_settings(struct clk_pll14xx *pll, unsigned long rat
 	 *
 	 * fvco = (m * 65536 + k) * prate / (p * 65536)
 	 * fout = (m * 65536 + k) * prate / (p * 65536) / (1 << sdiv)
+	 *
+	 * e) 1600MHz <= fvco <= 3200MHz
 	 */
 
 	/* First try if we can get the desired rate from one of the static entries */
@@ -193,6 +195,10 @@ static void imx_pll14xx_calc_settings(struct clk_pll14xx *pll, unsigned long rat
 			kdiv = pll1443x_calc_kdiv(mdiv, pdiv, sdiv, rate, prate);
 			fout = pll14xx_calc_rate(pll, mdiv, pdiv, sdiv, kdiv, prate);
 
+			fvco = fout << sdiv;
+
+			if (fvco < 1600000000 || fvco > 3200000000)
+				continue;
 			/* best match */
 			dist = abs((long)rate - (long)fout);
 			if (dist < best) {

-- 
2.37.1

On Fri, May 10, 2024 at 4:14 AM Peng Fan (OSS) <peng.fan@oss.nxp.com> wrote:
>
> From: Shengjiu Wang <shengjiu.wang@nxp.com>
>
> The fvco frequency range is between 1600MHz and 3200MHz, without
> this constraint the fvco may out of range, the real output
> frequency is no accurate.
>
> Aslo correct the name for fvco and fout clock.
>
> Fixes: b09c68dc57c9 ("clk: imx: pll14xx: Support dynamic rates")
> Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> Acked-by: Jacky Bai <ping.bai@nxp.com>
> Tested-by: Chancel Liu <chancel.liu@nxp.com>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  drivers/clk/imx/clk-pll14xx.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/clk/imx/clk-pll14xx.c b/drivers/clk/imx/clk-pll14xx.c
> index d63564dbb12c..55812bfb9ec2 100644
> --- a/drivers/clk/imx/clk-pll14xx.c
> +++ b/drivers/clk/imx/clk-pll14xx.c
> @@ -131,7 +131,7 @@ static void imx_pll14xx_calc_settings(struct clk_pll14xx *pll, unsigned long rat
>  {
>         u32 pll_div_ctl0, pll_div_ctl1;
>         int mdiv, pdiv, sdiv, kdiv;
> -       long fout, rate_min, rate_max, dist, best = LONG_MAX;
> +       long fvco, fout, rate_min, rate_max, dist, best = LONG_MAX;
>         const struct imx_pll14xx_rate_table *tt;
>
>         /*
> @@ -144,6 +144,8 @@ static void imx_pll14xx_calc_settings(struct clk_pll14xx *pll, unsigned long rat
>          *
>          * fvco = (m * 65536 + k) * prate / (p * 65536)
>          * fout = (m * 65536 + k) * prate / (p * 65536) / (1 << sdiv)
> +        *
> +        * e) 1600MHz <= fvco <= 3200MHz
>          */
>
>         /* First try if we can get the desired rate from one of the static entries */
> @@ -193,6 +195,10 @@ static void imx_pll14xx_calc_settings(struct clk_pll14xx *pll, unsigned long rat
>                         kdiv = pll1443x_calc_kdiv(mdiv, pdiv, sdiv, rate, prate);
>                         fout = pll14xx_calc_rate(pll, mdiv, pdiv, sdiv, kdiv, prate);
>
> +                       fvco = fout << sdiv;

The calculation of fvco here does not match the comment above where
fvco = (m * 65536 + k) * prate / (p * 65536)


> +
> +                       if (fvco < 1600000000 || fvco > 3200000000)
> +                               continue;
>                         /* best match */
>                         dist = abs((long)rate - (long)fout);
>                         if (dist < best) {
>
> --
> 2.37.1
>
>

On 10/05/2024 11.19, Peng Fan (OSS) wrote:
> 
> Aslo correct the name for fvco and fout clock.

Btw., that part of the commit log is misleading. The patch does no such
thing, as that part is already done in f52f00069888 that went into v6.8.

Please don't mindlessly cherry-pick stuff from the NXP fork and fixup
stuff so it builds and just ignore whether the commit log still makes
sense.

Rasmus

On 10/05/2024 11.19, Peng Fan (OSS) wrote:
> From: Shengjiu Wang <shengjiu.wang@nxp.com>
> 
> The fvco frequency range is between 1600MHz and 3200MHz, without
> this constraint the fvco may out of range, the real output
> frequency is no accurate.

Could you please point everybody in the direction of where that
requirement is stated? The imx8mp reference manual, for example, merely
lists constraints for p, m, s and k.


>  
>  	/* First try if we can get the desired rate from one of the static entries */
> @@ -193,6 +195,10 @@ static void imx_pll14xx_calc_settings(struct clk_pll14xx *pll, unsigned long rat
>  			kdiv = pll1443x_calc_kdiv(mdiv, pdiv, sdiv, rate, prate);
>  			fout = pll14xx_calc_rate(pll, mdiv, pdiv, sdiv, kdiv, prate);
>  
> +			fvco = fout << sdiv;
> +
> +			if (fvco < 1600000000 || fvco > 3200000000)
> +				continue;

If this is really a necessary constraint, it seems that one could just
up-front compute the only possible value of s, or at least change the
logic so that one loops over a smaller range of possible values of s.

Rasmus