[v2,2/2] clk: imx: pll14xx: dynamically configure PLL for 393216000/361267200Hz

Commit Message

Marco Felsch Aug. 7, 2023, 8:47 a.m. UTC

From: Ahmad Fatoum <[e-mail]>

Since commit b09c68dc57c9 ("clk: imx: pll14xx: Support dynamic rates"),
the driver has the ability to dynamically compute PLL parameters to
approximate the requested rates. This is not always used, because the
logic is as follows:

  - Check if the target rate is hardcoded in the frequency table
  - Check if varying only kdiv is possible, so switch over is glitch free
  - Compute rate dynamically by iterating over pdiv range

If we skip the frequency table for the 1443x PLL, we find that the
computed values differ to the hardcoded ones. This can be valid if the
hardcoded values guarantee for example an earlier lock-in or if the
divisors are chosen, so that other important rates are more likely to
be reached glitch-free.

For rates (393216000 and 361267200, this doesn't seem to be the case:
They are only approximated by existing parameters (393215995 and
361267196 Hz, respectively) and they aren't reachable glitch-free from
other hardcoded frequencies. Dropping them from the table allows us
to lock-in to these frequencies exactly.

This is immediately noticeable because they are the assigned-clock-rates
for IMX8MN_AUDIO_PLL1 and IMX8MN_AUDIO_PLL2, respectively and a look
into clk_summary so far showed that they were a few Hz short of the target:

imx8mn-board:~# grep audio_pll[12]_out /sys/kernel/debug/clk/clk_summary
audio_pll2_out           0        0        0   361267196 0     0  50000   N
audio_pll1_out           1        1        0   393215995 0     0  50000   Y

and afterwards:

imx8mn-board:~# grep audio_pll[12]_out /sys/kernel/debug/clk/clk_summary
audio_pll2_out           0        0        0   361267200 0     0  50000   N
audio_pll1_out           1        1        0   393216000 0     0  50000   Y

This change is equivalent to adding following hardcoded values:

  /*               rate     mdiv  pdiv  sdiv   kdiv */
  PLL_1443X_RATE(393216000, 655,    5,    3,  23593),
  PLL_1443X_RATE(361267200, 497,   33,    0, -16882),

Fixes: 053a4ffe2988 ("clk: imx: imx8mm: fix audio pll setting")
Cc: [e-mail] # v5.18+
Signed-off-by: Ahmad Fatoum <[e-mail]>
---
v2:
- new patch

 drivers/clk/imx/clk-pll14xx.c | 2 --
 1 file changed, 2 deletions(-)

Comments

Marco Felsch Aug. 7, 2023, 8:59 a.m. UTC | #1

Hi,

On 23-08-07, Marco Felsch wrote:
> From: Ahmad Fatoum <[e-mail]>
> 
> Since commit b09c68dc57c9 ("clk: imx: pll14xx: Support dynamic rates"),
> the driver has the ability to dynamically compute PLL parameters to
> approximate the requested rates. This is not always used, because the
> logic is as follows:
> 
>   - Check if the target rate is hardcoded in the frequency table
>   - Check if varying only kdiv is possible, so switch over is glitch free
>   - Compute rate dynamically by iterating over pdiv range
> 
> If we skip the frequency table for the 1443x PLL, we find that the
> computed values differ to the hardcoded ones. This can be valid if the
> hardcoded values guarantee for example an earlier lock-in or if the
> divisors are chosen, so that other important rates are more likely to
> be reached glitch-free.
> 
> For rates (393216000 and 361267200, this doesn't seem to be the case:
> They are only approximated by existing parameters (393215995 and
> 361267196 Hz, respectively) and they aren't reachable glitch-free from
> other hardcoded frequencies. Dropping them from the table allows us
> to lock-in to these frequencies exactly.
> 
> This is immediately noticeable because they are the assigned-clock-rates
> for IMX8MN_AUDIO_PLL1 and IMX8MN_AUDIO_PLL2, respectively and a look
> into clk_summary so far showed that they were a few Hz short of the target:
> 
> imx8mn-board:~# grep audio_pll[12]_out /sys/kernel/debug/clk/clk_summary
> audio_pll2_out           0        0        0   361267196 0     0  50000   N
> audio_pll1_out           1        1        0   393215995 0     0  50000   Y
> 
> and afterwards:
> 
> imx8mn-board:~# grep audio_pll[12]_out /sys/kernel/debug/clk/clk_summary
> audio_pll2_out           0        0        0   361267200 0     0  50000   N
> audio_pll1_out           1        1        0   393216000 0     0  50000   Y
> 
> This change is equivalent to adding following hardcoded values:
> 
>   /*               rate     mdiv  pdiv  sdiv   kdiv */
>   PLL_1443X_RATE(393216000, 655,    5,    3,  23593),
>   PLL_1443X_RATE(361267200, 497,   33,    0, -16882),
> 
> Fixes: 053a4ffe2988 ("clk: imx: imx8mm: fix audio pll setting")
> Cc: [e-mail] # v5.18+
> Signed-off-by: Ahmad Fatoum <[e-mail]>

I forgot to add my s-o-b, if b4 can collect this I will do it this way:

Signed-off-by: Marco Felsch <[e-mail]>

Regards,
  Marco

> ---
> v2:
> - new patch
> 
>  drivers/clk/imx/clk-pll14xx.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/clk/imx/clk-pll14xx.c b/drivers/clk/imx/clk-pll14xx.c
> index dc6bc21dff41..0d58d85c375e 100644
> --- a/drivers/clk/imx/clk-pll14xx.c
> +++ b/drivers/clk/imx/clk-pll14xx.c
> @@ -64,8 +64,6 @@ static const struct imx_pll14xx_rate_table imx_pll1443x_tbl[] = {
>  	PLL_1443X_RATE(650000000U, 325, 3, 2, 0),
>  	PLL_1443X_RATE(594000000U, 198, 2, 2, 0),
>  	PLL_1443X_RATE(519750000U, 173, 2, 2, 16384),
> -	PLL_1443X_RATE(393216000U, 262, 2, 3, 9437),
> -	PLL_1443X_RATE(361267200U, 361, 3, 3, 17511),
>  };
>  
>  struct imx_pll14xx_clk imx_1443x_pll = {
> -- 
> 2.39.2
> 
> 
>

Adam Ford Aug. 8, 2023, 12:19 p.m. UTC | #2

On Mon, Aug 7, 2023 at 3:47 AM Marco Felsch <[e-mail]> wrote:
>
> From: Ahmad Fatoum <[e-mail]>
>
> Since commit b09c68dc57c9 ("clk: imx: pll14xx: Support dynamic rates"),
> the driver has the ability to dynamically compute PLL parameters to
> approximate the requested rates. This is not always used, because the
> logic is as follows:
>
>   - Check if the target rate is hardcoded in the frequency table
>   - Check if varying only kdiv is possible, so switch over is glitch free
>   - Compute rate dynamically by iterating over pdiv range
>
> If we skip the frequency table for the 1443x PLL, we find that the
> computed values differ to the hardcoded ones. This can be valid if the
> hardcoded values guarantee for example an earlier lock-in or if the
> divisors are chosen, so that other important rates are more likely to
> be reached glitch-free.
>
> For rates (393216000 and 361267200, this doesn't seem to be the case:
> They are only approximated by existing parameters (393215995 and
> 361267196 Hz, respectively) and they aren't reachable glitch-free from
> other hardcoded frequencies. Dropping them from the table allows us
> to lock-in to these frequencies exactly.
>
> This is immediately noticeable because they are the assigned-clock-rates
> for IMX8MN_AUDIO_PLL1 and IMX8MN_AUDIO_PLL2, respectively and a look
> into clk_summary so far showed that they were a few Hz short of the target:
>
> imx8mn-board:~# grep audio_pll[12]_out /sys/kernel/debug/clk/clk_summary
> audio_pll2_out           0        0        0   361267196 0     0  50000   N
> audio_pll1_out           1        1        0   393215995 0     0  50000   Y
>
> and afterwards:
>
> imx8mn-board:~# grep audio_pll[12]_out /sys/kernel/debug/clk/clk_summary
> audio_pll2_out           0        0        0   361267200 0     0  50000   N
> audio_pll1_out           1        1        0   393216000 0     0  50000   Y
>
> This change is equivalent to adding following hardcoded values:
>
>   /*               rate     mdiv  pdiv  sdiv   kdiv */
>   PLL_1443X_RATE(393216000, 655,    5,    3,  23593),
>   PLL_1443X_RATE(361267200, 497,   33,    0, -16882),
>
> Fixes: 053a4ffe2988 ("clk: imx: imx8mm: fix audio pll setting")
> Cc: [e-mail] # v5.18+
> Signed-off-by: Ahmad Fatoum <[e-mail]>
> ---
> v2:
> - new patch
>
>  drivers/clk/imx/clk-pll14xx.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/drivers/clk/imx/clk-pll14xx.c b/drivers/clk/imx/clk-pll14xx.c
> index dc6bc21dff41..0d58d85c375e 100644
> --- a/drivers/clk/imx/clk-pll14xx.c
> +++ b/drivers/clk/imx/clk-pll14xx.c
> @@ -64,8 +64,6 @@ static const struct imx_pll14xx_rate_table imx_pll1443x_tbl[] = {
>         PLL_1443X_RATE(650000000U, 325, 3, 2, 0),
>         PLL_1443X_RATE(594000000U, 198, 2, 2, 0),
>         PLL_1443X_RATE(519750000U, 173, 2, 2, 16384),
> -       PLL_1443X_RATE(393216000U, 262, 2, 3, 9437),
> -       PLL_1443X_RATE(361267200U, 361, 3, 3, 17511),

Part of me wonders why we need the look-up table at all if the driver
has been fixed to achieve better rates.  I don't know if there is a
significant time in calculating the numbers as compared to the time it
takes to search the table.

adam
>  };
>
>  struct imx_pll14xx_clk imx_1443x_pll = {
> --
> 2.39.2
>

Ahmad Fatoum Aug. 8, 2023, 1:03 p.m. UTC | #3

On 08.08.23 14:19, Adam Ford wrote:
> On Mon, Aug 7, 2023 at 3:47 AM Marco Felsch <[e-mail]> wrote:
>>
>> From: Ahmad Fatoum <[e-mail]>
>>
>> Since commit b09c68dc57c9 ("clk: imx: pll14xx: Support dynamic rates"),
>> the driver has the ability to dynamically compute PLL parameters to
>> approximate the requested rates. This is not always used, because the
>> logic is as follows:
>>
>>   - Check if the target rate is hardcoded in the frequency table
>>   - Check if varying only kdiv is possible, so switch over is glitch free
>>   - Compute rate dynamically by iterating over pdiv range
>>
>> If we skip the frequency table for the 1443x PLL, we find that the
>> computed values differ to the hardcoded ones. This can be valid if the
>> hardcoded values guarantee for example an earlier lock-in or if the
>> divisors are chosen, so that other important rates are more likely to
>> be reached glitch-free.
>>
>> For rates (393216000 and 361267200, this doesn't seem to be the case:
>> They are only approximated by existing parameters (393215995 and
>> 361267196 Hz, respectively) and they aren't reachable glitch-free from
>> other hardcoded frequencies. Dropping them from the table allows us
>> to lock-in to these frequencies exactly.
>>
>> This is immediately noticeable because they are the assigned-clock-rates
>> for IMX8MN_AUDIO_PLL1 and IMX8MN_AUDIO_PLL2, respectively and a look
>> into clk_summary so far showed that they were a few Hz short of the target:
>>
>> imx8mn-board:~# grep audio_pll[12]_out /sys/kernel/debug/clk/clk_summary
>> audio_pll2_out           0        0        0   361267196 0     0  50000   N
>> audio_pll1_out           1        1        0   393215995 0     0  50000   Y
>>
>> and afterwards:
>>
>> imx8mn-board:~# grep audio_pll[12]_out /sys/kernel/debug/clk/clk_summary
>> audio_pll2_out           0        0        0   361267200 0     0  50000   N
>> audio_pll1_out           1        1        0   393216000 0     0  50000   Y
>>
>> This change is equivalent to adding following hardcoded values:
>>
>>   /*               rate     mdiv  pdiv  sdiv   kdiv */
>>   PLL_1443X_RATE(393216000, 655,    5,    3,  23593),
>>   PLL_1443X_RATE(361267200, 497,   33,    0, -16882),
>>
>> Fixes: 053a4ffe2988 ("clk: imx: imx8mm: fix audio pll setting")
>> Cc: [e-mail] # v5.18+
>> Signed-off-by: Ahmad Fatoum <[e-mail]>
>> ---
>> v2:
>> - new patch
>>
>>  drivers/clk/imx/clk-pll14xx.c | 2 --
>>  1 file changed, 2 deletions(-)
>>
>> diff --git a/drivers/clk/imx/clk-pll14xx.c b/drivers/clk/imx/clk-pll14xx.c
>> index dc6bc21dff41..0d58d85c375e 100644
>> --- a/drivers/clk/imx/clk-pll14xx.c
>> +++ b/drivers/clk/imx/clk-pll14xx.c
>> @@ -64,8 +64,6 @@ static const struct imx_pll14xx_rate_table imx_pll1443x_tbl[] = {
>>         PLL_1443X_RATE(650000000U, 325, 3, 2, 0),
>>         PLL_1443X_RATE(594000000U, 198, 2, 2, 0),
>>         PLL_1443X_RATE(519750000U, 173, 2, 2, 16384),
>> -       PLL_1443X_RATE(393216000U, 262, 2, 3, 9437),
>> -       PLL_1443X_RATE(361267200U, 361, 3, 3, 17511),
> 
> Part of me wonders why we need the look-up table at all if the driver
> has been fixed to achieve better rates. 

The look-up table achieves a different (worse) rate only for the two setpoints
that are dropped in this patch.

> I don't know if there is a
> significant time in calculating the numbers as compared to the time it
> takes to search the table.

The speed of calculation is negligible. Differently chosen parameters
may affect the speed of lock-in or allow faster switch to other interesting
frequencies. I also think we might be able to drop the table, but that should
be a different patch with a different justification as both ways
would achieve the same rate, but with different parameters.

If anybody from NXP could shed some light on how the existing parameters
were initially chosen that would be most useful.

Cheers,
Ahmad


> 
> adam
>>  };
>>
>>  struct imx_pll14xx_clk imx_1443x_pll = {
>> --
>> 2.39.2
>>
>

Peng Fan Aug. 9, 2023, 1:22 a.m. UTC | #4

> Subject: Re: [PATCH v2 2/2] clk: imx: pll14xx: dynamically configure PLL for
> 393216000/361267200Hz
> 
> On 08.08.23 14:19, Adam Ford wrote:
> > On Mon, Aug 7, 2023 at 3:47 AM Marco Felsch <[e-mail]>
> wrote:
> >>
> >> From: Ahmad Fatoum <[e-mail]>
> >>
> >> Since commit b09c68dc57c9 ("clk: imx: pll14xx: Support dynamic
> >> rates"), the driver has the ability to dynamically compute PLL
> >> parameters to approximate the requested rates. This is not always
> >> used, because the logic is as follows:
> >>
> >>   - Check if the target rate is hardcoded in the frequency table
> >>   - Check if varying only kdiv is possible, so switch over is glitch free
> >>   - Compute rate dynamically by iterating over pdiv range
> >>
> >> If we skip the frequency table for the 1443x PLL, we find that the
> >> computed values differ to the hardcoded ones. This can be valid if
> >> the hardcoded values guarantee for example an earlier lock-in or if
> >> the divisors are chosen, so that other important rates are more
> >> likely to be reached glitch-free.
> >>
> >> For rates (393216000 and 361267200, this doesn't seem to be the case:
> >> They are only approximated by existing parameters (393215995 and
> >> 361267196 Hz, respectively) and they aren't reachable glitch-free
> >> from other hardcoded frequencies. Dropping them from the table allows
> >> us to lock-in to these frequencies exactly.
> >>
> >> This is immediately noticeable because they are the
> >> assigned-clock-rates for IMX8MN_AUDIO_PLL1 and
> IMX8MN_AUDIO_PLL2,
> >> respectively and a look into clk_summary so far showed that they were a
> few Hz short of the target:
> >>
> >> imx8mn-board:~# grep audio_pll[12]_out
> /sys/kernel/debug/clk/clk_summary
> >> audio_pll2_out           0        0        0   361267196 0     0  50000   N
> >> audio_pll1_out           1        1        0   393215995 0     0  50000   Y
> >>
> >> and afterwards:
> >>
> >> imx8mn-board:~# grep audio_pll[12]_out
> /sys/kernel/debug/clk/clk_summary
> >> audio_pll2_out           0        0        0   361267200 0     0  50000   N
> >> audio_pll1_out           1        1        0   393216000 0     0  50000   Y
> >>
> >> This change is equivalent to adding following hardcoded values:
> >>
> >>   /*               rate     mdiv  pdiv  sdiv   kdiv */
> >>   PLL_1443X_RATE(393216000, 655,    5,    3,  23593),
> >>   PLL_1443X_RATE(361267200, 497,   33,    0, -16882),
> >>
> >> Fixes: 053a4ffe2988 ("clk: imx: imx8mm: fix audio pll setting")
> >> Cc: [e-mail] # v5.18+
> >> Signed-off-by: Ahmad Fatoum <[e-mail]>
> >> ---
> >> v2:
> >> - new patch
> >>
> >>  drivers/clk/imx/clk-pll14xx.c | 2 --
> >>  1 file changed, 2 deletions(-)
> >>
> >> diff --git a/drivers/clk/imx/clk-pll14xx.c
> >> b/drivers/clk/imx/clk-pll14xx.c index dc6bc21dff41..0d58d85c375e
> >> 100644
> >> --- a/drivers/clk/imx/clk-pll14xx.c
> >> +++ b/drivers/clk/imx/clk-pll14xx.c
> >> @@ -64,8 +64,6 @@ static const struct imx_pll14xx_rate_table
> imx_pll1443x_tbl[] = {
> >>         PLL_1443X_RATE(650000000U, 325, 3, 2, 0),
> >>         PLL_1443X_RATE(594000000U, 198, 2, 2, 0),
> >>         PLL_1443X_RATE(519750000U, 173, 2, 2, 16384),
> >> -       PLL_1443X_RATE(393216000U, 262, 2, 3, 9437),
> >> -       PLL_1443X_RATE(361267200U, 361, 3, 3, 17511),
> >
> > Part of me wonders why we need the look-up table at all if the driver
> > has been fixed to achieve better rates.
> 
> The look-up table achieves a different (worse) rate only for the two
> setpoints that are dropped in this patch.
> 
> > I don't know if there is a
> > significant time in calculating the numbers as compared to the time it
> > takes to search the table.
> 
> The speed of calculation is negligible. Differently chosen parameters may
> affect the speed of lock-in or allow faster switch to other interesting
> frequencies. I also think we might be able to drop the table, but that should
> be a different patch with a different justification as both ways would
> achieve the same rate, but with different parameters.
> 
> If anybody from NXP could shed some light on how the existing parameters
> were initially chosen that would be most useful.

No specific reason, just bit lazy to write the calculation algorithm,
LUT table would be easier to understand and check.

If the runtime calculation algorithm works well and not violate VCO
or spec, it should be fine to drop LUT.

Regards,
Peng.

> 
> Cheers,
> Ahmad
> 
> 
> >
> > adam
> >>  };
> >>
> >>  struct imx_pll14xx_clk imx_1443x_pll = {
> >> --
> >> 2.39.2
> >>
> >
> 
> --
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       |
> https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.
> pengutronix.de%2F&data=05%7C01%7Cpeng.fan%40nxp.com%7C5c46a083
> 87714c77639608db980fe775%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0
> %7C0%7C638270966291144836%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiM
> C4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000
> %7C%7C%7C&sdata=lOGHpEcwJCTMOHHRacVqyBic3tvXiz7FmDX9l3oh2yM
> %3D&reserved=0  |
> 31137 Hildesheim, Germany                  | Phone: [phone]    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   [phone]-5555 |

Adam Ford Aug. 9, 2023, 1:18 p.m. UTC | #5

On Tue, Aug 8, 2023 at 8:22 PM Peng Fan <[e-mail]> wrote:
>
> > Subject: Re: [PATCH v2 2/2] clk: imx: pll14xx: dynamically configure PLL for
> > 393216000/361267200Hz
> >
> > On 08.08.23 14:19, Adam Ford wrote:
> > > On Mon, Aug 7, 2023 at 3:47 AM Marco Felsch <[e-mail]>
> > wrote:
> > >>
> > >> From: Ahmad Fatoum <[e-mail]>
> > >>
> > >> Since commit b09c68dc57c9 ("clk: imx: pll14xx: Support dynamic
> > >> rates"), the driver has the ability to dynamically compute PLL
> > >> parameters to approximate the requested rates. This is not always
> > >> used, because the logic is as follows:
> > >>
> > >>   - Check if the target rate is hardcoded in the frequency table
> > >>   - Check if varying only kdiv is possible, so switch over is glitch free
> > >>   - Compute rate dynamically by iterating over pdiv range
> > >>
> > >> If we skip the frequency table for the 1443x PLL, we find that the
> > >> computed values differ to the hardcoded ones. This can be valid if
> > >> the hardcoded values guarantee for example an earlier lock-in or if
> > >> the divisors are chosen, so that other important rates are more
> > >> likely to be reached glitch-free.
> > >>
> > >> For rates (393216000 and 361267200, this doesn't seem to be the case:
> > >> They are only approximated by existing parameters (393215995 and
> > >> 361267196 Hz, respectively) and they aren't reachable glitch-free
> > >> from other hardcoded frequencies. Dropping them from the table allows
> > >> us to lock-in to these frequencies exactly.
> > >>
> > >> This is immediately noticeable because they are the
> > >> assigned-clock-rates for IMX8MN_AUDIO_PLL1 and
> > IMX8MN_AUDIO_PLL2,
> > >> respectively and a look into clk_summary so far showed that they were a
> > few Hz short of the target:
> > >>
> > >> imx8mn-board:~# grep audio_pll[12]_out
> > /sys/kernel/debug/clk/clk_summary
> > >> audio_pll2_out           0        0        0   361267196 0     0  50000   N
> > >> audio_pll1_out           1        1        0   393215995 0     0  50000   Y
> > >>
> > >> and afterwards:
> > >>
> > >> imx8mn-board:~# grep audio_pll[12]_out
> > /sys/kernel/debug/clk/clk_summary
> > >> audio_pll2_out           0        0        0   361267200 0     0  50000   N
> > >> audio_pll1_out           1        1        0   393216000 0     0  50000   Y
> > >>
> > >> This change is equivalent to adding following hardcoded values:
> > >>
> > >>   /*               rate     mdiv  pdiv  sdiv   kdiv */
> > >>   PLL_1443X_RATE(393216000, 655,    5,    3,  23593),
> > >>   PLL_1443X_RATE(361267200, 497,   33,    0, -16882),
> > >>
> > >> Fixes: 053a4ffe2988 ("clk: imx: imx8mm: fix audio pll setting")
> > >> Cc: [e-mail] # v5.18+
> > >> Signed-off-by: Ahmad Fatoum <[e-mail]>
> > >> ---
> > >> v2:
> > >> - new patch
> > >>
> > >>  drivers/clk/imx/clk-pll14xx.c | 2 --
> > >>  1 file changed, 2 deletions(-)
> > >>
> > >> diff --git a/drivers/clk/imx/clk-pll14xx.c
> > >> b/drivers/clk/imx/clk-pll14xx.c index dc6bc21dff41..0d58d85c375e
> > >> 100644
> > >> --- a/drivers/clk/imx/clk-pll14xx.c
> > >> +++ b/drivers/clk/imx/clk-pll14xx.c
> > >> @@ -64,8 +64,6 @@ static const struct imx_pll14xx_rate_table
> > imx_pll1443x_tbl[] = {
> > >>         PLL_1443X_RATE(650000000U, 325, 3, 2, 0),
> > >>         PLL_1443X_RATE(594000000U, 198, 2, 2, 0),
> > >>         PLL_1443X_RATE(519750000U, 173, 2, 2, 16384),
> > >> -       PLL_1443X_RATE(393216000U, 262, 2, 3, 9437),
> > >> -       PLL_1443X_RATE(361267200U, 361, 3, 3, 17511),
> > >
> > > Part of me wonders why we need the look-up table at all if the driver
> > > has been fixed to achieve better rates.
> >
> > The look-up table achieves a different (worse) rate only for the two
> > setpoints that are dropped in this patch.
> >
> > > I don't know if there is a
> > > significant time in calculating the numbers as compared to the time it
> > > takes to search the table.
> >
> > The speed of calculation is negligible. Differently chosen parameters may
> > affect the speed of lock-in or allow faster switch to other interesting
> > frequencies. I also think we might be able to drop the table, but that should
> > be a different patch with a different justification as both ways would
> > achieve the same rate, but with different parameters.
> >
> > If anybody from NXP could shed some light on how the existing parameters
> > were initially chosen that would be most useful.
>
> No specific reason, just bit lazy to write the calculation algorithm,
> LUT table would be easier to understand and check.
>
> If the runtime calculation algorithm works well and not violate VCO
> or spec, it should be fine to drop LUT.

Hopefully the algorithm doesn't violate the VCO, but I'll run some
checks to verify it doesn't, and I'll verify  what values are
generated for those other frequencies remaining in the LUT when I have
time (unless someone beats me to it).  If nothing else, it will help
shrink the code a bit.

adam
>
> Regards,
> Peng.
>
> >
> > Cheers,
> > Ahmad
> >
> >
> > >
> > > adam
> > >>  };
> > >>
> > >>  struct imx_pll14xx_clk imx_1443x_pll = {
> > >> --
> > >> 2.39.2
> > >>
> > >
> >
> > --
> > Pengutronix e.K.                           |                             |
> > Steuerwalder Str. 21                       |
> > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.
> > pengutronix.de%2F&data=05%7C01%7Cpeng.fan%40nxp.com%7C5c46a083
> > 87714c77639608db980fe775%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0
> > %7C0%7C638270966291144836%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiM
> > C4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000
> > %7C%7C%7C&sdata=lOGHpEcwJCTMOHHRacVqyBic3tvXiz7FmDX9l3oh2yM
> > %3D&reserved=0  |
> > 31137 Hildesheim, Germany                  | Phone: [phone]    |
> > Amtsgericht Hildesheim, HRA 2686           | Fax:   [phone]-5555 |
>

Patch

diff --git a/drivers/clk/imx/clk-pll14xx.c b/drivers/clk/imx/clk-pll14xx.c
index dc6bc21dff41..0d58d85c375e 100644
--- a/drivers/clk/imx/clk-pll14xx.c
+++ b/drivers/clk/imx/clk-pll14xx.c
@@ -64,8 +64,6 @@  static const struct imx_pll14xx_rate_table imx_pll1443x_tbl[] = {
 	PLL_1443X_RATE(650000000U, 325, 3, 2, 0),
 	PLL_1443X_RATE(594000000U, 198, 2, 2, 0),
 	PLL_1443X_RATE(519750000U, 173, 2, 2, 16384),
-	PLL_1443X_RATE(393216000U, 262, 2, 3, 9437),
-	PLL_1443X_RATE(361267200U, 361, 3, 3, 17511),
 };
 
 struct imx_pll14xx_clk imx_1443x_pll = {