[v1] clk: renesas: cpg-mssr: Add module reset support for RZ/T2H

[PATCH] clk: renesas: cpg-mssr: Add module reset support for RZ/T2H

Posted by Prabhakar 1 month, 4 weeks ago

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Add support for module reset handling on the RZ/T2H SoC. Unlike earlier
CPG/MSSR variants, RZ/T2H uses a unified set of Module Reset Control
Registers (MRCR) where both reset and deassert actions are done via
read-modify-write (RMW) to the same register.

Introduce a new MRCR offset table (mrcr_for_rzt2h) for RZ/T2H and assign
it to both reset_regs and reset_clear_regs. For RZ/T2H, set
rcdev.nr_resets based on the number of MRCR registers rather than the
number of module clocks.

Update the reset/assert/deassert/status operations to perform RMW when
handling RZ/T2H-specific layout. This enables proper reset sequencing for
modules on RZ/T2H without affecting the behavior of other supported SoCs.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 drivers/clk/renesas/renesas-cpg-mssr.c | 40 ++++++++++++++++++++++++--
 1 file changed, 37 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/renesas/renesas-cpg-mssr.c b/drivers/clk/renesas/renesas-cpg-mssr.c
index 5ff6ee1f7d4b..d299c2bb6100 100644
--- a/drivers/clk/renesas/renesas-cpg-mssr.c
+++ b/drivers/clk/renesas/renesas-cpg-mssr.c
@@ -137,6 +137,22 @@ static const u16 srcr_for_gen4[] = {
 	0x2C60, 0x2C64, 0x2C68, 0x2C6C, 0x2C70, 0x2C74,
 };
 
+static const u16 mrcr_for_rzt2h[] = {
+	0x240,	/* MRCTLA */
+	0x244,	/* Reserved */
+	0x248,	/* Reserved */
+	0x24C,	/* Reserved */
+	0x250,	/* MRCTLE */
+	0x254,	/* Reserved */
+	0x258,	/* Reserved */
+	0x25C,	/* Reserved */
+	0x260,	/* MRCTLI */
+	0x264,	/* Reserved */
+	0x268,	/* Reserved */
+	0x26C,	/* Reserved */
+	0x270,	/* MRCTLM */
+};
+
 /*
  * Software Reset Clearing Register offsets
  */
@@ -686,12 +702,16 @@ static int cpg_mssr_reset(struct reset_controller_dev *rcdev,
 
 	dev_dbg(priv->dev, "reset %u%02u\n", reg, bit);
 
+	if (priv->reg_layout == CLK_REG_LAYOUT_RZ_T2H)
+		bitmask = readl(priv->pub.base0 + priv->reset_regs[reg]) | bitmask;
 	/* Reset module */
 	writel(bitmask, priv->pub.base0 + priv->reset_regs[reg]);
 
 	/* Wait for at least one cycle of the RCLK clock (@ ca. 32 kHz) */
 	udelay(35);
 
+	if (priv->reg_layout == CLK_REG_LAYOUT_RZ_T2H)
+		bitmask = readl(priv->pub.base0 + priv->reset_clear_regs[reg]) & ~bitmask;
 	/* Release module from reset state */
 	writel(bitmask, priv->pub.base0 + priv->reset_clear_regs[reg]);
 
@@ -707,6 +727,8 @@ static int cpg_mssr_assert(struct reset_controller_dev *rcdev, unsigned long id)
 
 	dev_dbg(priv->dev, "assert %u%02u\n", reg, bit);
 
+	if (priv->reg_layout == CLK_REG_LAYOUT_RZ_T2H)
+		bitmask = readl(priv->pub.base0 + priv->reset_regs[reg]) | bitmask;
 	writel(bitmask, priv->pub.base0 + priv->reset_regs[reg]);
 	return 0;
 }
@@ -721,6 +743,8 @@ static int cpg_mssr_deassert(struct reset_controller_dev *rcdev,
 
 	dev_dbg(priv->dev, "deassert %u%02u\n", reg, bit);
 
+	if (priv->reg_layout == CLK_REG_LAYOUT_RZ_T2H)
+		bitmask = readl(priv->pub.base0 + priv->reset_regs[reg]) & ~bitmask;
 	writel(bitmask, priv->pub.base0 + priv->reset_clear_regs[reg]);
 	return 0;
 }
@@ -764,7 +788,16 @@ static int cpg_mssr_reset_controller_register(struct cpg_mssr_priv *priv)
 	priv->rcdev.of_node = priv->dev->of_node;
 	priv->rcdev.of_reset_n_cells = 1;
 	priv->rcdev.of_xlate = cpg_mssr_reset_xlate;
-	priv->rcdev.nr_resets = priv->num_mod_clks;
+
+	/*
+	 * RZ/T2H (and family) has the Module Reset Control Registers
+	 * which allows control resets of certain modules.
+	 * The number of resets is not equal to the number of module clocks.
+	 */
+	if (priv->reg_layout == CLK_REG_LAYOUT_RZ_T2H)
+		priv->rcdev.nr_resets = ARRAY_SIZE(mrcr_for_rzt2h) * 32;
+	else
+		priv->rcdev.nr_resets = priv->num_mod_clks;
 	return devm_reset_controller_register(priv->dev, &priv->rcdev);
 }
 
@@ -1166,6 +1199,8 @@ static int __init cpg_mssr_common_init(struct device *dev,
 		priv->control_regs = stbcr;
 	} else if (priv->reg_layout == CLK_REG_LAYOUT_RZ_T2H) {
 		priv->control_regs = mstpcr_for_rzt2h;
+		priv->reset_regs = mrcr_for_rzt2h;
+		priv->reset_clear_regs = mrcr_for_rzt2h;
 	} else if (priv->reg_layout == CLK_REG_LAYOUT_RCAR_GEN4) {
 		priv->status_regs = mstpsr_for_gen4;
 		priv->control_regs = mstpcr_for_gen4;
@@ -1262,8 +1297,7 @@ static int __init cpg_mssr_probe(struct platform_device *pdev)
 		goto reserve_exit;
 
 	/* Reset Controller not supported for Standby Control SoCs */
-	if (priv->reg_layout == CLK_REG_LAYOUT_RZ_A ||
-	    priv->reg_layout == CLK_REG_LAYOUT_RZ_T2H)
+	if (priv->reg_layout == CLK_REG_LAYOUT_RZ_A)
 		goto reserve_exit;
 
 	error = cpg_mssr_reset_controller_register(priv);
-- 
2.50.1

Re: [PATCH] clk: renesas: cpg-mssr: Add module reset support for RZ/T2H

Posted by Geert Uytterhoeven 1 month, 3 weeks ago

Hi Prabhakar,

On Thu, 7 Aug 2025 at 18:44, Prabhakar <prabhakar.csengg@gmail.com> wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Add support for module reset handling on the RZ/T2H SoC. Unlike earlier
> CPG/MSSR variants, RZ/T2H uses a unified set of Module Reset Control
> Registers (MRCR) where both reset and deassert actions are done via
> read-modify-write (RMW) to the same register.
>
> Introduce a new MRCR offset table (mrcr_for_rzt2h) for RZ/T2H and assign
> it to both reset_regs and reset_clear_regs. For RZ/T2H, set
> rcdev.nr_resets based on the number of MRCR registers rather than the
> number of module clocks.
>
> Update the reset/assert/deassert/status operations to perform RMW when
> handling RZ/T2H-specific layout. This enables proper reset sequencing for
> modules on RZ/T2H without affecting the behavior of other supported SoCs.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Thanks for your patch!

> --- a/drivers/clk/renesas/renesas-cpg-mssr.c
> +++ b/drivers/clk/renesas/renesas-cpg-mssr.c
> @@ -137,6 +137,22 @@ static const u16 srcr_for_gen4[] = {
>         0x2C60, 0x2C64, 0x2C68, 0x2C6C, 0x2C70, 0x2C74,
>  };
>
> +static const u16 mrcr_for_rzt2h[] = {
> +       0x240,  /* MRCTLA */
> +       0x244,  /* Reserved */
> +       0x248,  /* Reserved */
> +       0x24C,  /* Reserved */
> +       0x250,  /* MRCTLE */
> +       0x254,  /* Reserved */
> +       0x258,  /* Reserved */
> +       0x25C,  /* Reserved */
> +       0x260,  /* MRCTLI */
> +       0x264,  /* Reserved */
> +       0x268,  /* Reserved */
> +       0x26C,  /* Reserved */
> +       0x270,  /* MRCTLM */
> +};
> +
>  /*
>   * Software Reset Clearing Register offsets
>   */
> @@ -686,12 +702,16 @@ static int cpg_mssr_reset(struct reset_controller_dev *rcdev,
>
>         dev_dbg(priv->dev, "reset %u%02u\n", reg, bit);
>
> +       if (priv->reg_layout == CLK_REG_LAYOUT_RZ_T2H)
> +               bitmask = readl(priv->pub.base0 + priv->reset_regs[reg]) | bitmask;

bitmask |= ...

>         /* Reset module */
>         writel(bitmask, priv->pub.base0 + priv->reset_regs[reg]);

However, you should hold the &priv->pub.rmw_lock spinlock everywhere
you do an RMW operation.

>         /* Wait for at least one cycle of the RCLK clock (@ ca. 32 kHz) */
>         udelay(35);

Please read "6.5.1 Notes on Module Reset Control Register Operation"...

> +       if (priv->reg_layout == CLK_REG_LAYOUT_RZ_T2H)
> +               bitmask = readl(priv->pub.base0 + priv->reset_clear_regs[reg]) & ~bitmask;
>         /* Release module from reset state */
>         writel(bitmask, priv->pub.base0 + priv->reset_clear_regs[reg]);

Hence I think it would be worthwhile to have your own struct
reset_control_ops mrcr_reset_ops with its own set of callbacks.

> @@ -764,7 +788,16 @@ static int cpg_mssr_reset_controller_register(struct cpg_mssr_priv *priv)
>         priv->rcdev.of_node = priv->dev->of_node;
>         priv->rcdev.of_reset_n_cells = 1;
>         priv->rcdev.of_xlate = cpg_mssr_reset_xlate;
> -       priv->rcdev.nr_resets = priv->num_mod_clks;
> +
> +       /*
> +        * RZ/T2H (and family) has the Module Reset Control Registers
> +        * which allows control resets of certain modules.
> +        * The number of resets is not equal to the number of module clocks.
> +        */
> +       if (priv->reg_layout == CLK_REG_LAYOUT_RZ_T2H)
> +               priv->rcdev.nr_resets = ARRAY_SIZE(mrcr_for_rzt2h) * 32;
> +       else
> +               priv->rcdev.nr_resets = priv->num_mod_clks;

I guess this is fine for now, but probably we will have to introduce
a proper nr_resets field when the next RZ/T* SoC is introduced...

>         return devm_reset_controller_register(priv->dev, &priv->rcdev);
>  }
>
> @@ -1166,6 +1199,8 @@ static int __init cpg_mssr_common_init(struct device *dev,
>                 priv->control_regs = stbcr;
>         } else if (priv->reg_layout == CLK_REG_LAYOUT_RZ_T2H) {
>                 priv->control_regs = mstpcr_for_rzt2h;
> +               priv->reset_regs = mrcr_for_rzt2h;
> +               priv->reset_clear_regs = mrcr_for_rzt2h;

With its own set of reset callbacks directly accessing mrcr_for_rzt2h,
this could be dropped.  However, if you want to keep on using
cpg_mssr_status(), you do have to keep the assignment to
priv->reset_regs.

>         } else if (priv->reg_layout == CLK_REG_LAYOUT_RCAR_GEN4) {
>                 priv->status_regs = mstpsr_for_gen4;
>                 priv->control_regs = mstpcr_for_gen4;

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

Re: [PATCH] clk: renesas: cpg-mssr: Add module reset support for RZ/T2H

Posted by Lad, Prabhakar 1 month, 2 weeks ago

Hi Geert,

Thank you for the review.

On Fri, Aug 8, 2025 at 11:14 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Prabhakar,
>
> On Thu, 7 Aug 2025 at 18:44, Prabhakar <prabhakar.csengg@gmail.com> wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > Add support for module reset handling on the RZ/T2H SoC. Unlike earlier
> > CPG/MSSR variants, RZ/T2H uses a unified set of Module Reset Control
> > Registers (MRCR) where both reset and deassert actions are done via
> > read-modify-write (RMW) to the same register.
> >
> > Introduce a new MRCR offset table (mrcr_for_rzt2h) for RZ/T2H and assign
> > it to both reset_regs and reset_clear_regs. For RZ/T2H, set
> > rcdev.nr_resets based on the number of MRCR registers rather than the
> > number of module clocks.
> >
> > Update the reset/assert/deassert/status operations to perform RMW when
> > handling RZ/T2H-specific layout. This enables proper reset sequencing for
> > modules on RZ/T2H without affecting the behavior of other supported SoCs.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Thanks for your patch!
>
> > --- a/drivers/clk/renesas/renesas-cpg-mssr.c
> > +++ b/drivers/clk/renesas/renesas-cpg-mssr.c
> > @@ -137,6 +137,22 @@ static const u16 srcr_for_gen4[] = {
> >         0x2C60, 0x2C64, 0x2C68, 0x2C6C, 0x2C70, 0x2C74,
> >  };
> >
> > +static const u16 mrcr_for_rzt2h[] = {
> > +       0x240,  /* MRCTLA */
> > +       0x244,  /* Reserved */
> > +       0x248,  /* Reserved */
> > +       0x24C,  /* Reserved */
> > +       0x250,  /* MRCTLE */
> > +       0x254,  /* Reserved */
> > +       0x258,  /* Reserved */
> > +       0x25C,  /* Reserved */
> > +       0x260,  /* MRCTLI */
> > +       0x264,  /* Reserved */
> > +       0x268,  /* Reserved */
> > +       0x26C,  /* Reserved */
> > +       0x270,  /* MRCTLM */
> > +};
> > +
> >  /*
> >   * Software Reset Clearing Register offsets
> >   */
> > @@ -686,12 +702,16 @@ static int cpg_mssr_reset(struct reset_controller_dev *rcdev,
> >
> >         dev_dbg(priv->dev, "reset %u%02u\n", reg, bit);
> >
> > +       if (priv->reg_layout == CLK_REG_LAYOUT_RZ_T2H)
> > +               bitmask = readl(priv->pub.base0 + priv->reset_regs[reg]) | bitmask;
>
> bitmask |= ...
>
> >         /* Reset module */
> >         writel(bitmask, priv->pub.base0 + priv->reset_regs[reg]);
>
> However, you should hold the &priv->pub.rmw_lock spinlock everywhere
> you do an RMW operation.
>
Ok.

> >         /* Wait for at least one cycle of the RCLK clock (@ ca. 32 kHz) */
> >         udelay(35);
>
> Please read "6.5.1 Notes on Module Reset Control Register Operation"...
>
I had missed that...

> > +       if (priv->reg_layout == CLK_REG_LAYOUT_RZ_T2H)
> > +               bitmask = readl(priv->pub.base0 + priv->reset_clear_regs[reg]) & ~bitmask;
> >         /* Release module from reset state */
> >         writel(bitmask, priv->pub.base0 + priv->reset_clear_regs[reg]);
>
> Hence I think it would be worthwhile to have your own struct
> reset_control_ops mrcr_reset_ops with its own set of callbacks.
>
Ok, I will add a new mrcr_reset_ops for T2H which will adhere to
"6.5.1 Notes on Module Reset Control Register Operation" restrictions
with spinlock held.

> > @@ -764,7 +788,16 @@ static int cpg_mssr_reset_controller_register(struct cpg_mssr_priv *priv)
> >         priv->rcdev.of_node = priv->dev->of_node;
> >         priv->rcdev.of_reset_n_cells = 1;
> >         priv->rcdev.of_xlate = cpg_mssr_reset_xlate;
> > -       priv->rcdev.nr_resets = priv->num_mod_clks;
> > +
> > +       /*
> > +        * RZ/T2H (and family) has the Module Reset Control Registers
> > +        * which allows control resets of certain modules.
> > +        * The number of resets is not equal to the number of module clocks.
> > +        */
> > +       if (priv->reg_layout == CLK_REG_LAYOUT_RZ_T2H)
> > +               priv->rcdev.nr_resets = ARRAY_SIZE(mrcr_for_rzt2h) * 32;
> > +       else
> > +               priv->rcdev.nr_resets = priv->num_mod_clks;
>
> I guess this is fine for now, but probably we will have to introduce
> a proper nr_resets field when the next RZ/T* SoC is introduced...
>
Thanks.

> >         return devm_reset_controller_register(priv->dev, &priv->rcdev);
> >  }
> >
> > @@ -1166,6 +1199,8 @@ static int __init cpg_mssr_common_init(struct device *dev,
> >                 priv->control_regs = stbcr;
> >         } else if (priv->reg_layout == CLK_REG_LAYOUT_RZ_T2H) {
> >                 priv->control_regs = mstpcr_for_rzt2h;
> > +               priv->reset_regs = mrcr_for_rzt2h;
> > +               priv->reset_clear_regs = mrcr_for_rzt2h;
>
> With its own set of reset callbacks directly accessing mrcr_for_rzt2h,
> this could be dropped.  However, if you want to keep on using
> cpg_mssr_status(), you do have to keep the assignment to
> priv->reset_regs.
>
To reuse cpg_mssr_status() I'll keep ` priv->reset_regs =
mrcr_for_rzt2h;` for now and drop the later.

Cheers,
Prabhakar