Re: [RFC PATCH 3/3] clk: imx: clk-fracn-gppll: Support dynamic rates

[Sascha Hauer <s.hauer@pengutronix.de>]

Hi Alexander,

On Mon, Feb 10, 2025 at 05:00:11PM +0100, Alexander Stein wrote:
> The fracn gppll PLL so far only supports rates from a rate table passed
> during initialization. Calculating PLL settings dynamically helps audio
> applications to get their desired rates, so support for this is added
> in this patch.
> 
> The strategy to get to the PLL setting for a rate is:
> 
> - The rate table is searched for suitable rates, so for standard rates the
>   same settings are used as without this patch
> - Then try to only adjust mfn, on fractional PLLs only, which specifies
>   the fractional part of the PLL. This setting can be changed without
>   glitches on the output and is therefore preferred

I wonder if this part is worth it. There might be cases in which a
glitch free switch is required, but without being able to enforce
a glitch free switch we can't rely on it.

Also this makes the result depend on the current PLL settings, so the
result is no longer reproducible. I.e. switching from a fari away
frequency to the desired frequency might yield in different settings
than switching from a nearby frequency to the desired frequency.

Finally I think the glitch free switch doesn't work currently, because
the PLL is fully disabled and re-enabled unconditionally in
clk_fracn_gppll_set_rate().

That said, glitch free switching would be great to have sometimes.

> - As a last resort the best settings are calculated dynamically
> 
> Implementation is inspired by commit b09c68dc57c9d ("clk: imx: pll14xx:
> Support dynamic rates")
> 
> Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> ---

...

> +			if (pll->flags & CLK_FRACN_GPPLL_FRACN) {
> +				if (!dist) {
> +					/* Disable fractional part upon exact match */
> +					mfd = 1;
> +					mfn = 0;
> +				} else {
> +					mfd = 100;
> +					mfd = clamp(mfd, PLL_MFD_MIN, PLL_MFN_MAX);

With mfd = 100 this clamp looks like a no-op. Do we need this?

Sascha

-- 
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 |