Hi Prabhakar, On Mon, 10 Mar 2025 at 19:22, Lad, Prabhakar <prabhakar.csengg@xxxxxxxxx> wrote: > On Sun, Mar 9, 2025 at 9:14 PM Prabhakar <prabhakar.csengg@xxxxxxxxx> wrote: > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx> > > > > Some RZ/V2H(P) SoC variants do not have a GPU, resulting in PLLGPU being > > disabled by default in TF-A. Add support for enabling PLL clocks in the > > RZ/V2H(P) CPG driver to manage this. > > > > Introduce `is_enabled` and `enable` callbacks to handle PLL state > > transitions. With the `enable` callback, PLLGPU will be turned ON only > > when the GPU node is enabled; otherwise, it will remain off. Define new > > macros for PLL standby and monitor registers to facilitate this process. > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx> > > --- > > v1->v2 > > - Updated macros to get PLL offsets > > - Switched to readl_poll_timeout_atomic() and updated the timeout Thanks for the update! Reviewed-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx> > > --- a/drivers/clk/renesas/rzv2h-cpg.c > > +++ b/drivers/clk/renesas/rzv2h-cpg.c > > +static int rzv2h_cpg_pll_clk_enable(struct clk_hw *hw) > > +{ > > + struct pll_clk *pll_clk = to_pll(hw); > > + struct rzv2h_cpg_priv *priv = pll_clk->priv; > > + struct pll pll = pll_clk->pll; > > + u32 stby_offset; > > + u32 mon_offset; > > + u32 val; > > + int ret; > > + > > + if (rzv2h_cpg_pll_clk_is_enabled(hw)) > > + return 0; > > + > > + stby_offset = CPG_PLL_STBY(pll.offset); > > + mon_offset = CPG_PLL_MON(pll.offset); > > + > > + writel(CPG_PLL_STBY_RESETB_WEN | CPG_PLL_STBY_RESETB, > > + priv->base + stby_offset); > > + > > + /* ensure PLL is in normal mode */ > > + ret = readl_poll_timeout_atomic(priv->base + mon_offset, val, > > + (val & (CPG_PLL_MON_RESETB | CPG_PLL_MON_LOCK)) == > > + (CPG_PLL_MON_RESETB | CPG_PLL_MON_LOCK), 10, 100); > This timeout didnt work when I power cycled after a complete shutdown overnight. > > I will update the timeout as below, this Ive made sure the below delay > works OK after complete shutdown. > > /* > * Ensure PLL enters into normal mode > * > * Note: There is no HW information about the worst case latency. > * > * Since this value might be dependent on external xtal rate, pll > * rate or even the other emulation clocks rate, use 2000 as a > * "super" safe value. > */ > ret = readl_poll_timeout_atomic(priv->base + mon_offset, val, > (val & > (CPG_PLL_MON_RESETB | CPG_PLL_MON_LOCK)) == > > (CPG_PLL_MON_RESETB | CPG_PLL_MON_LOCK), 200, 2000); > > Please let me know shall I send v3 with this change or wait for your review. I can incorporate this fix while queuing in renesas-clk for v6.16. But, please explain what is "the other emulation clocks rate"? Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds