Re: [PATCH 1/2] clk: spacemit: re-enable PLLs after init

"Troy Mitchell" <[email protected]>
Newsgroups dev.linux.lists.spacemit,org.infradead.lists.linux-riscv,org.kernel.vger.linux-clk,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
> I don't think this patch makes sense.
>
> If this PLL is essential for some components of the systems to work,
> there's no chance to re-program it safely after the kernel gets started,
> since all the PLLs in K1 and K3 are specified as CLK_SET_RATE_GATE.
>
> If the essential clocks could be re-parented glitchlessly, registering
> a notifier and re-parenting it before the PLL disables might be a
> solution. In this case, we need to delay the operation in
> ccu_pll*_init() until clock registration fully completes.
>
> Otherwise, re-enabling the PLL seems totally extra here, they could be
> enabled on the fly when it's going to be used.

You are right. In both hunks, ccu_pll*_disable() has already gated the
PLL before the newly added ccu_pll*_enable() can run:

> @@ -148,7 +148,7 @@ static int ccu_pll_init(struct clk_hw *hw)
> 	ccu_pll_disable(hw);
> 	ccu_pll_update_param(pll, &pll->config.rate_tbl[0]);
>
> -	return 0;
> +	return ccu_pll_enable(hw);
>  }

> [...]
>
> @@ -252,7 +252,7 @@ static int ccu_plla_init(struct clk_hw *hw)
> 	ccu_plla_disable(hw);
> 	ccu_plla_update_param(pll, &pll->config.rate_tbl[0]);
>
> -	return 0;
> +	return ccu_plla_enable(hw);
>  }

Re-enabling the PLL afterwards therefore does not make reprogramming
safe for a live consumer. I will drop this patch, "[PATCH 1/2]
clk: spacemit: re-enable PLLs after init", from v2.

To avoid delaying the 1.8 GHz rate-table update while this broader issue
is investigated, I will resend the current "[PATCH 2/2] clk: spacemit:
k3: add 1.8 GHz PLL5 and PLL8 rates" as a standalone patch. It only adds
the firmware-selected 1.8 GHz settings to the rate tables.

The existing problem of reprogramming an unmatched PLL from
ccu_pll*_init() needs a broader solution and is independent of the rate
table update. I will address this issue in a separate patch series.

                                            - Troy
signature.asc (application/pgp-signature, 248 B)
-----BEGIN PGP SIGNATURE-----

iIMEABYKACsWIQSL4Ay2cExaPXAQcU2YCe+A+TM0LwUCanmO2g0caUB0cm95LXku
b3JnAAoJEJgJ74D5MzQvCy8A/3AS+h5VxUd4a+hzLwgWTitmdjKz5AQxKY9ffa9g
0PEaAP46TDFaDGtlJxZlgxExHbHVjyTZ7P44F33VqpOtqO4bBw==
=9C3d
-----END PGP SIGNATURE-----
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.