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 |