> 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