| Message ID | 20250309211402.80886-4-prabhakar.mahadev-lad.rj@bp.renesas.com (mailing list archive) |
|---|---|
| State | Awaiting Upstream, archived |
| Headers | show |
| Series | clk: renesas: Add GE3D clock/reset entries for RZ/V2H(P) SoC | expand |
Hi Geert, On Sun, Mar 9, 2025 at 9:14 PM Prabhakar <prabhakar.csengg@gmail.com> wrote: > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > 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@bp.renesas.com> > --- > v1->v2 > - Updated macros to get PLL offsets > - Switched to readl_poll_timeout_atomic() and updated the timeout > --- > drivers/clk/renesas/rzv2h-cpg.c | 49 +++++++++++++++++++++++++++++++++ > 1 file changed, 49 insertions(+) > > diff --git a/drivers/clk/renesas/rzv2h-cpg.c b/drivers/clk/renesas/rzv2h-cpg.c > index e489ce28ae63..76ad037b4361 100644 > --- a/drivers/clk/renesas/rzv2h-cpg.c > +++ b/drivers/clk/renesas/rzv2h-cpg.c > @@ -44,12 +44,18 @@ > #define CPG_BUS_1_MSTOP (0xd00) > #define CPG_BUS_MSTOP(m) (CPG_BUS_1_MSTOP + ((m) - 1) * 4) > > +#define CPG_PLL_STBY(x) ((x)) > +#define CPG_PLL_STBY_RESETB BIT(0) > +#define CPG_PLL_STBY_RESETB_WEN BIT(16) > #define CPG_PLL_CLK1(x) ((x) + 0x004) > #define KDIV(val) ((s16)FIELD_GET(GENMASK(31, 16), (val))) > #define MDIV(val) FIELD_GET(GENMASK(15, 6), (val)) > #define PDIV(val) FIELD_GET(GENMASK(5, 0), (val)) > #define CPG_PLL_CLK2(x) ((x) + 0x008) > #define SDIV(val) FIELD_GET(GENMASK(2, 0), (val)) > +#define CPG_PLL_MON(x) ((x) + 0x010) > +#define CPG_PLL_MON_RESETB BIT(0) > +#define CPG_PLL_MON_LOCK BIT(4) > > #define DDIV_DIVCTL_WEN(shift) BIT((shift) + 16) > > @@ -141,6 +147,47 @@ struct ddiv_clk { > > #define to_ddiv_clock(_div) container_of(_div, struct ddiv_clk, div) > > +static int rzv2h_cpg_pll_clk_is_enabled(struct clk_hw *hw) > +{ > + struct pll_clk *pll_clk = to_pll(hw); > + struct rzv2h_cpg_priv *priv = pll_clk->priv; > + u32 val = readl(priv->base + CPG_PLL_MON(pll_clk->pll.offset)); > + > + /* Ensure both RESETB and LOCK bits are set */ > + return (val & (CPG_PLL_MON_RESETB | CPG_PLL_MON_LOCK)) == > + (CPG_PLL_MON_RESETB | CPG_PLL_MON_LOCK); > +} > + > +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. Cheers, Prabhakar
Hi Prabhakar, On Mon, 10 Mar 2025 at 19:22, Lad, Prabhakar <prabhakar.csengg@gmail.com> wrote: > On Sun, Mar 9, 2025 at 9:14 PM Prabhakar <prabhakar.csengg@gmail.com> wrote: > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > > > 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@bp.renesas.com> > > --- > > 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@glider.be> > > --- 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@linux-m68k.org 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
Hi Geert, Thank you for the review. On Fri, Mar 14, 2025 at 1:04 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > Hi Prabhakar, > > On Mon, 10 Mar 2025 at 19:22, Lad, Prabhakar <prabhakar.csengg@gmail.com> wrote: > > On Sun, Mar 9, 2025 at 9:14 PM Prabhakar <prabhakar.csengg@gmail.com> wrote: > > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > > > > > 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@bp.renesas.com> > > > --- > > > 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@glider.be> > > > > --- 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"? > I got carried away referring to R-Car code, let's drop the `or even the other emulation clocks rate`. Thank you for taking care of it. Cheers, Prabhakar
diff --git a/drivers/clk/renesas/rzv2h-cpg.c b/drivers/clk/renesas/rzv2h-cpg.c index e489ce28ae63..76ad037b4361 100644 --- a/drivers/clk/renesas/rzv2h-cpg.c +++ b/drivers/clk/renesas/rzv2h-cpg.c @@ -44,12 +44,18 @@ #define CPG_BUS_1_MSTOP (0xd00) #define CPG_BUS_MSTOP(m) (CPG_BUS_1_MSTOP + ((m) - 1) * 4) +#define CPG_PLL_STBY(x) ((x)) +#define CPG_PLL_STBY_RESETB BIT(0) +#define CPG_PLL_STBY_RESETB_WEN BIT(16) #define CPG_PLL_CLK1(x) ((x) + 0x004) #define KDIV(val) ((s16)FIELD_GET(GENMASK(31, 16), (val))) #define MDIV(val) FIELD_GET(GENMASK(15, 6), (val)) #define PDIV(val) FIELD_GET(GENMASK(5, 0), (val)) #define CPG_PLL_CLK2(x) ((x) + 0x008) #define SDIV(val) FIELD_GET(GENMASK(2, 0), (val)) +#define CPG_PLL_MON(x) ((x) + 0x010) +#define CPG_PLL_MON_RESETB BIT(0) +#define CPG_PLL_MON_LOCK BIT(4) #define DDIV_DIVCTL_WEN(shift) BIT((shift) + 16) @@ -141,6 +147,47 @@ struct ddiv_clk { #define to_ddiv_clock(_div) container_of(_div, struct ddiv_clk, div) +static int rzv2h_cpg_pll_clk_is_enabled(struct clk_hw *hw) +{ + struct pll_clk *pll_clk = to_pll(hw); + struct rzv2h_cpg_priv *priv = pll_clk->priv; + u32 val = readl(priv->base + CPG_PLL_MON(pll_clk->pll.offset)); + + /* Ensure both RESETB and LOCK bits are set */ + return (val & (CPG_PLL_MON_RESETB | CPG_PLL_MON_LOCK)) == + (CPG_PLL_MON_RESETB | CPG_PLL_MON_LOCK); +} + +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); + if (ret) + dev_err(priv->dev, "Failed to enable PLL 0x%x/%pC\n", + stby_offset, hw->clk); + + return ret; +} + static unsigned long rzv2h_cpg_pll_clk_recalc_rate(struct clk_hw *hw, unsigned long parent_rate) { @@ -163,6 +210,8 @@ static unsigned long rzv2h_cpg_pll_clk_recalc_rate(struct clk_hw *hw, } static const struct clk_ops rzv2h_cpg_pll_ops = { + .is_enabled = rzv2h_cpg_pll_clk_is_enabled, + .enable = rzv2h_cpg_pll_clk_enable, .recalc_rate = rzv2h_cpg_pll_clk_recalc_rate, };