Re: [PATCH 5.15 399/511] clk: imx: pll14xx: dynamically configure PLL for 393216000/361267200Hz

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Greg,

On 17.09.23 21:13, Greg Kroah-Hartman wrote:
> 5.15-stable review patch.  If anyone has any objections, please let me know.
> 
> ------------------
> 
> From: Ahmad Fatoum <a.fatoum@xxxxxxxxxxxxxx>
> 
> commit 72d00e560d10665e6139c9431956a87ded6e9880 upstream.
> 
> 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: stable@xxxxxxxxxxxxxxx # v5.18+

Patch is only correct for v5.18 onward. Please drop for v5.15.

Is there another syntax that we should've used instead that your tools
pick up?

Thanks,
Ahmad

> Signed-off-by: Ahmad Fatoum <a.fatoum@xxxxxxxxxxxxxx>
> Signed-off-by: Marco Felsch <m.felsch@xxxxxxxxxxxxxx>
> Link: https://lore.kernel.org/r/20230807084744.1184791-2-m.felsch@xxxxxxxxxxxxxx
> Signed-off-by: Abel Vesa <abel.vesa@xxxxxxxxxx>
> Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> ---
>  drivers/clk/imx/clk-pll14xx.c |    2 --
>  1 file changed, 2 deletions(-)
> 
> --- a/drivers/clk/imx/clk-pll14xx.c
> +++ b/drivers/clk/imx/clk-pll14xx.c
> @@ -60,8 +60,6 @@ static const struct imx_pll14xx_rate_tab
>  	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 = {
> 
> 
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |




[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux