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 reset_regs. For this SoC, the number of resets is based on the
number of MRCR registers rather than the number of module clocks. Also
add cpg_mrcr_reset_ops to implement reset, assert, and deassert using RMW
while holding the spinlock. This follows the RZ/T2H requirements, where
processing after releasing a module reset must be secured by performing
seven dummy reads of the same register, and where a module that is reset
and released again must ensure the target bit in the Module Reset Control
Register is set to 1.

Update the reset controller registration to select cpg_mrcr_reset_ops for
RZ/T2H, while keeping the existing cpg_mssr_reset_ops for other SoCs.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
v1->v2:
- Added cpg_mrcr_reset_ops for RZ/T2H specific handling
- Updated commit message
---
 drivers/clk/renesas/renesas-cpg-mssr.c | 139 ++++++++++++++++++++++++-
 1 file changed, 135 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/renesas/renesas-cpg-mssr.c b/drivers/clk/renesas/renesas-cpg-mssr.c
index 5ff6ee1f7d4b..77a298b50c9c 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
  */
@@ -736,6 +752,102 @@ static int cpg_mssr_status(struct reset_controller_dev *rcdev,
 	return !!(readl(priv->pub.base0 + priv->reset_regs[reg]) & bitmask);
 }
 
+static int cpg_mrcr_reset(struct reset_controller_dev *rcdev,
+			  unsigned long id)
+{
+	struct cpg_mssr_priv *priv = rcdev_to_priv(rcdev);
+	unsigned int reg = id / 32;
+	unsigned int bit = id % 32;
+	u32 bitmask = BIT(bit);
+	unsigned long flags;
+	unsigned int i;
+
+	dev_dbg(priv->dev, "reset %u%02u\n", reg, bit);
+
+	spin_lock_irqsave(&priv->pub.rmw_lock, flags);
+	/* Reset module */
+	bitmask |= readl(priv->pub.base0 + priv->reset_regs[reg]);
+	writel(bitmask, priv->pub.base0 + priv->reset_regs[reg]);
+
+	/* Ensure module reset control register is set */
+	if (!(bitmask & readl(priv->pub.base0 + priv->reset_regs[reg]))) {
+		dev_err(priv->dev, "Reset register %u%02u is not set\n",
+			readl(priv->pub.base0 + priv->reset_regs[reg]), bit);
+		spin_unlock_irqrestore(&priv->pub.rmw_lock, flags);
+		return -EIO;
+	}
+
+	/* Release module from reset state */
+	bitmask = readl(priv->pub.base0 + priv->reset_regs[reg]) & ~bitmask;
+	writel(bitmask, priv->pub.base0 + priv->reset_regs[reg]);
+
+	/*
+	 * To secure processing after release from a module reset, dummy read
+	 * the same register at least seven times.
+	 */
+	for (i = 0; i < 7; i++)
+		readl(priv->pub.base0 + priv->reset_regs[reg]);
+
+	spin_unlock_irqrestore(&priv->pub.rmw_lock, flags);
+
+	return 0;
+}
+
+static int cpg_mrcr_assert(struct reset_controller_dev *rcdev, unsigned long id)
+{
+	struct cpg_mssr_priv *priv = rcdev_to_priv(rcdev);
+	unsigned int reg = id / 32;
+	unsigned int bit = id % 32;
+	u32 bitmask = BIT(bit);
+	unsigned long flags;
+
+	dev_dbg(priv->dev, "assert %u%02u\n", reg, bit);
+
+	spin_lock_irqsave(&priv->pub.rmw_lock, flags);
+	bitmask |= readl(priv->pub.base0 + priv->reset_regs[reg]);
+	writel(bitmask, priv->pub.base0 + priv->reset_regs[reg]);
+
+	/* Ensure module reset control register is set */
+	if (!(bitmask & readl(priv->pub.base0 + priv->reset_regs[reg]))) {
+		dev_err(priv->dev, "Reset register %u%02u is not set\n",
+			readl(priv->pub.base0 + priv->reset_regs[reg]), bit);
+		spin_unlock_irqrestore(&priv->pub.rmw_lock, flags);
+		return -EIO;
+	}
+
+	spin_unlock_irqrestore(&priv->pub.rmw_lock, flags);
+
+	return 0;
+}
+
+static int cpg_mrcr_deassert(struct reset_controller_dev *rcdev,
+			     unsigned long id)
+{
+	struct cpg_mssr_priv *priv = rcdev_to_priv(rcdev);
+	unsigned int reg = id / 32;
+	unsigned int bit = id % 32;
+	u32 bitmask = BIT(bit);
+	unsigned long flags;
+	unsigned int i;
+
+	dev_dbg(priv->dev, "deassert %u%02u\n", reg, bit);
+
+	spin_lock_irqsave(&priv->pub.rmw_lock, flags);
+	bitmask = readl(priv->pub.base0 + priv->reset_regs[reg]) & ~bitmask;
+	writel(bitmask, priv->pub.base0 + priv->reset_regs[reg]);
+
+	/*
+	 * To secure processing after release from a module reset, dummy read
+	 * the same register at least seven times.
+	 */
+	for (i = 0; i < 7; i++)
+		readl(priv->pub.base0 + priv->reset_regs[reg]);
+
+	spin_unlock_irqrestore(&priv->pub.rmw_lock, flags);
+
+	return 0;
+}
+
 static const struct reset_control_ops cpg_mssr_reset_ops = {
 	.reset = cpg_mssr_reset,
 	.assert = cpg_mssr_assert,
@@ -743,6 +855,13 @@ static const struct reset_control_ops cpg_mssr_reset_ops = {
 	.status = cpg_mssr_status,
 };
 
+static const struct reset_control_ops cpg_mrcr_reset_ops = {
+	.reset = cpg_mrcr_reset,
+	.assert = cpg_mrcr_assert,
+	.deassert = cpg_mrcr_deassert,
+	.status = cpg_mssr_status,
+};
+
 static int cpg_mssr_reset_xlate(struct reset_controller_dev *rcdev,
 				const struct of_phandle_args *reset_spec)
 {
@@ -760,11 +879,23 @@ static int cpg_mssr_reset_xlate(struct reset_controller_dev *rcdev,
 
 static int cpg_mssr_reset_controller_register(struct cpg_mssr_priv *priv)
 {
-	priv->rcdev.ops = &cpg_mssr_reset_ops;
+	/*
+	 * 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.ops = &cpg_mrcr_reset_ops;
+		priv->rcdev.nr_resets = ARRAY_SIZE(mrcr_for_rzt2h) * 32;
+	} else {
+		priv->rcdev.ops = &cpg_mssr_reset_ops;
+		priv->rcdev.nr_resets = priv->num_mod_clks;
+	}
+
 	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;
+
 	return devm_reset_controller_register(priv->dev, &priv->rcdev);
 }
 
@@ -1166,6 +1297,7 @@ 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;
 	} else if (priv->reg_layout == CLK_REG_LAYOUT_RCAR_GEN4) {
 		priv->status_regs = mstpsr_for_gen4;
 		priv->control_regs = mstpcr_for_gen4;
@@ -1262,8 +1394,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.51.0

On Mi, 2025-08-20 at 21:48 +0100, Prabhakar 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 reset_regs. For this SoC, the number of resets is based on the
> number of MRCR registers rather than the number of module clocks. Also
> add cpg_mrcr_reset_ops to implement reset, assert, and deassert using RMW
> while holding the spinlock. This follows the RZ/T2H requirements, where
> processing after releasing a module reset must be secured by performing
> seven dummy reads of the same register, and where a module that is reset
> and released again must ensure the target bit in the Module Reset Control
> Register is set to 1.
> 
> Update the reset controller registration to select cpg_mrcr_reset_ops for
> RZ/T2H, while keeping the existing cpg_mssr_reset_ops for other SoCs.
> 
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
> v1->v2:
> - Added cpg_mrcr_reset_ops for RZ/T2H specific handling
> - Updated commit message
> ---
>  drivers/clk/renesas/renesas-cpg-mssr.c | 139 ++++++++++++++++++++++++-
>  1 file changed, 135 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/clk/renesas/renesas-cpg-mssr.c b/drivers/clk/renesas/renesas-cpg-mssr.c
> index 5ff6ee1f7d4b..77a298b50c9c 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 */
> +};

Does each of these registers contain 32 reset controls?

Why are reserved registers in this list? It looks like the driver
allows poking around in reserved registers.

> +
>  /*
>   * Software Reset Clearing Register offsets
>   */
> @@ -736,6 +752,102 @@ static int cpg_mssr_status(struct reset_controller_dev *rcdev,
>  	return !!(readl(priv->pub.base0 + priv->reset_regs[reg]) & bitmask);
>  }
>  
> +static int cpg_mrcr_reset(struct reset_controller_dev *rcdev,
> +			  unsigned long id)
> +{
> +	struct cpg_mssr_priv *priv = rcdev_to_priv(rcdev);
> +	unsigned int reg = id / 32;
> +	unsigned int bit = id % 32;
> +	u32 bitmask = BIT(bit);
> +	unsigned long flags;
> +	unsigned int i;
> +
> +	dev_dbg(priv->dev, "reset %u%02u\n", reg, bit);
> +
> +	spin_lock_irqsave(&priv->pub.rmw_lock, flags);
> +	/* Reset module */
> +	bitmask |= readl(priv->pub.base0 + priv->reset_regs[reg]);
> +	writel(bitmask, priv->pub.base0 + priv->reset_regs[reg]);
> +
> +	/* Ensure module reset control register is set */
> +	if (!(bitmask & readl(priv->pub.base0 + priv->reset_regs[reg]))) {

Can this actually happen on this hardware? Under which circumstances?

> +		dev_err(priv->dev, "Reset register %u%02u is not set\n",
> +			readl(priv->pub.base0 + priv->reset_regs[reg]), bit);

Why read the register again? Could it have changed in the meantime?
Maybe it would be better to store the value that was actually checked
and print the variable here.

> +		spin_unlock_irqrestore(&priv->pub.rmw_lock, flags);
> +		return -EIO;
> +	}
> +
> +	/* Release module from reset state */
> +	bitmask = readl(priv->pub.base0 + priv->reset_regs[reg]) & ~bitmask;
> +	writel(bitmask, priv->pub.base0 + priv->reset_regs[reg]);
> +
> +	/*
> +	 * To secure processing after release from a module reset, dummy read
> +	 * the same register at least seven times.

Why 7? Is this documented in a reference manual?

> +	 */
> +	for (i = 0; i < 7; i++)
> +		readl(priv->pub.base0 + priv->reset_regs[reg]);

It would be better to define a macro and use it in both places instead
of the magic number.

regards
Philipp

Hi Philipp,

Thank you for the review.

On Fri, Aug 29, 2025 at 10:46 AM Philipp Zabel <p.zabel@pengutronix.de> wrote:
>
> On Mi, 2025-08-20 at 21:48 +0100, Prabhakar 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 reset_regs. For this SoC, the number of resets is based on the
> > number of MRCR registers rather than the number of module clocks. Also
> > add cpg_mrcr_reset_ops to implement reset, assert, and deassert using RMW
> > while holding the spinlock. This follows the RZ/T2H requirements, where
> > processing after releasing a module reset must be secured by performing
> > seven dummy reads of the same register, and where a module that is reset
> > and released again must ensure the target bit in the Module Reset Control
> > Register is set to 1.
> >
> > Update the reset controller registration to select cpg_mrcr_reset_ops for
> > RZ/T2H, while keeping the existing cpg_mssr_reset_ops for other SoCs.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > ---
> > v1->v2:
> > - Added cpg_mrcr_reset_ops for RZ/T2H specific handling
> > - Updated commit message
> > ---
> >  drivers/clk/renesas/renesas-cpg-mssr.c | 139 ++++++++++++++++++++++++-
> >  1 file changed, 135 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/clk/renesas/renesas-cpg-mssr.c b/drivers/clk/renesas/renesas-cpg-mssr.c
> > index 5ff6ee1f7d4b..77a298b50c9c 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 */
> > +};
>
> Does each of these registers contain 32 reset controls?
>
On RZ/T2H SoC for registers MRCTLA-MRCTLM none of them have 32 rest
controls in them; For the bits which does not have rest controls they
are marked as reserved.

> Why are reserved registers in this list? It looks like the driver
> allows poking around in reserved registers.
>
The HW manual currently lists registers MRCTLA/E/I/M, for the rest of
the registers I have marked them as reserved to avoid breakage i.e. in
future if MRCTLC register is exposed we won't have any breakage.

MRCTLA   -  0
MRCTLE   -  400
MRCTLI    -  800
MRCTLM  -  1200


For example, MRCTLE[0] controls GMAC0 PCLKH reset; this is represented
as 400 in DT.

This driver uses `4` to get the offset from  mrcr_for_rzt2h array and
`00` is the bit to be updated. So suppose in future if MRCTLD is
exposed we just use that in DT.

> > +
> >  /*
> >   * Software Reset Clearing Register offsets
> >   */
> > @@ -736,6 +752,102 @@ static int cpg_mssr_status(struct reset_controller_dev *rcdev,
> >       return !!(readl(priv->pub.base0 + priv->reset_regs[reg]) & bitmask);
> >  }
> >
> > +static int cpg_mrcr_reset(struct reset_controller_dev *rcdev,
> > +                       unsigned long id)
> > +{
> > +     struct cpg_mssr_priv *priv = rcdev_to_priv(rcdev);
> > +     unsigned int reg = id / 32;
> > +     unsigned int bit = id % 32;
> > +     u32 bitmask = BIT(bit);
> > +     unsigned long flags;
> > +     unsigned int i;
> > +
> > +     dev_dbg(priv->dev, "reset %u%02u\n", reg, bit);
> > +
> > +     spin_lock_irqsave(&priv->pub.rmw_lock, flags);
> > +     /* Reset module */
> > +     bitmask |= readl(priv->pub.base0 + priv->reset_regs[reg]);
> > +     writel(bitmask, priv->pub.base0 + priv->reset_regs[reg]);
> > +
> > +     /* Ensure module reset control register is set */
> > +     if (!(bitmask & readl(priv->pub.base0 + priv->reset_regs[reg]))) {
>
> Can this actually happen on this hardware? Under which circumstances?
>
This check is based on the HW manual,

"When module is reset once and released again, make sure that the
target bit of module reset control register is set to 1 by reading the
register before releasing from a module reset. Then release from a
module reset."

> > +             dev_err(priv->dev, "Reset register %u%02u is not set\n",
> > +                     readl(priv->pub.base0 + priv->reset_regs[reg]), bit);
>
> Why read the register again? Could it have changed in the meantime?
> Maybe it would be better to store the value that was actually checked
> and print the variable here.
>
OK.

> > +             spin_unlock_irqrestore(&priv->pub.rmw_lock, flags);
> > +             return -EIO;
> > +     }
> > +
> > +     /* Release module from reset state */
> > +     bitmask = readl(priv->pub.base0 + priv->reset_regs[reg]) & ~bitmask;
> > +     writel(bitmask, priv->pub.base0 + priv->reset_regs[reg]);
> > +
> > +     /*
> > +      * To secure processing after release from a module reset, dummy read
> > +      * the same register at least seven times.
>
> Why 7? Is this documented in a reference manual?
>
This is again based on the HW manual,

"To secure processing after release from a module reset, dummy read
the same register at least seven times after writing to initiate
release from the module reset, and only then proceed with the
subsequent processing."

> > +      */
> > +     for (i = 0; i < 7; i++)
> > +             readl(priv->pub.base0 + priv->reset_regs[reg]);
>
> It would be better to define a macro and use it in both places instead
> of the magic number.
>
OK.

Cheers,
Prabhakar