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>
---
v3->v4:
- Renamed cpg_mrcr_set_bit() to cpg_mrcr_set_reset_state() for clarity.
- Updated the parameters in cpg_mrcr_set_reset_state().

v2->v3:
- Simplifed the code by adding a common function cpg_mrcr_set_bit() to handle
  set/clear of bits with options for verify and dummy reads.
- Added a macro for the number of dummy reads required.

v1->v2:
- Added cpg_mrcr_reset_ops for RZ/T2H specific handling
- Updated commit message
---
 drivers/clk/renesas/renesas-cpg-mssr.c | 111 ++++++++++++++++++++++++-
 1 file changed, 107 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/renesas/renesas-cpg-mssr.c b/drivers/clk/renesas/renesas-cpg-mssr.c
index de1cf7ba45b7..fcb2c3c22f87 100644
--- a/drivers/clk/renesas/renesas-cpg-mssr.c
+++ b/drivers/clk/renesas/renesas-cpg-mssr.c
@@ -40,6 +40,8 @@
 #define WARN_DEBUG(x)	do { } while (0)
 #endif
 
+#define RZT2H_RESET_REG_READ_COUNT	7
+
 /*
  * Module Standby and Software Reset register offets.
  *
@@ -137,6 +139,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 +754,72 @@ static int cpg_mssr_status(struct reset_controller_dev *rcdev,
 	return !!(readl(priv->pub.base0 + priv->reset_regs[reg]) & bitmask);
 }
 
+static int cpg_mrcr_set_reset_state(struct reset_controller_dev *rcdev,
+				    unsigned long id, bool set)
+{
+	struct cpg_mssr_priv *priv = rcdev_to_priv(rcdev);
+	unsigned int reg = id / 32;
+	unsigned int bit = id % 32;
+	u32 bitmask = BIT(bit);
+	void __iomem *reg_addr;
+	unsigned long flags;
+	unsigned int i;
+	u32 val;
+
+	dev_dbg(priv->dev, "%s %u%02u\n", set ? "assert" : "deassert", reg, bit);
+
+	spin_lock_irqsave(&priv->pub.rmw_lock, flags);
+
+	reg_addr = priv->pub.base0 + priv->reset_regs[reg];
+	/* Read current value and modify */
+	val = readl(reg_addr);
+	if (set)
+		val |= bitmask;
+	else
+		val &= ~bitmask;
+	writel(val, reg_addr);
+
+	/*
+	 * For secure processing after release from a module reset, dummy read
+	 * the same register at least seven times.
+	 */
+	for (i = 0; !set && i < RZT2H_RESET_REG_READ_COUNT; i++)
+		readl(reg_addr);
+
+	/* Verify the operation */
+	val = readl(reg_addr);
+	if ((set && !(bitmask & val)) || (!set && (bitmask & val))) {
+		dev_err(priv->dev, "Reset register %u%02u operation failed\n", 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_reset(struct reset_controller_dev *rcdev, unsigned long id)
+{
+	int ret;
+
+	ret = cpg_mrcr_set_reset_state(rcdev, id, true);
+	if (ret)
+		return ret;
+
+	return cpg_mrcr_set_reset_state(rcdev, id, false);
+}
+
+static int cpg_mrcr_assert(struct reset_controller_dev *rcdev, unsigned long id)
+{
+	return cpg_mrcr_set_reset_state(rcdev, id, true);
+}
+
+static int cpg_mrcr_deassert(struct reset_controller_dev *rcdev, unsigned long id)
+{
+	return cpg_mrcr_set_reset_state(rcdev, id, false);
+}
+
 static const struct reset_control_ops cpg_mssr_reset_ops = {
 	.reset = cpg_mssr_reset,
 	.assert = cpg_mssr_assert,
@@ -743,6 +827,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 +851,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);
 }
 
@@ -1169,6 +1272,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;
@@ -1265,8 +1369,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

Hi Prabhakar,

On Mon, 29 Sept 2025 at 13:23, 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 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>
> ---
> v3->v4:
> - Renamed cpg_mrcr_set_bit() to cpg_mrcr_set_reset_state() for clarity.
> - Updated the parameters in cpg_mrcr_set_reset_state().

Thanks for the update!

> --- a/drivers/clk/renesas/renesas-cpg-mssr.c
> +++ b/drivers/clk/renesas/renesas-cpg-mssr.c
> @@ -736,6 +754,72 @@ static int cpg_mssr_status(struct reset_controller_dev *rcdev,
>         return !!(readl(priv->pub.base0 + priv->reset_regs[reg]) & bitmask);
>  }
>
> +static int cpg_mrcr_set_reset_state(struct reset_controller_dev *rcdev,
> +                                   unsigned long id, bool set)
> +{
> +       struct cpg_mssr_priv *priv = rcdev_to_priv(rcdev);
> +       unsigned int reg = id / 32;
> +       unsigned int bit = id % 32;
> +       u32 bitmask = BIT(bit);
> +       void __iomem *reg_addr;
> +       unsigned long flags;
> +       unsigned int i;
> +       u32 val;
> +
> +       dev_dbg(priv->dev, "%s %u%02u\n", set ? "assert" : "deassert", reg, bit);
> +
> +       spin_lock_irqsave(&priv->pub.rmw_lock, flags);
> +
> +       reg_addr = priv->pub.base0 + priv->reset_regs[reg];
> +       /* Read current value and modify */
> +       val = readl(reg_addr);
> +       if (set)
> +               val |= bitmask;
> +       else
> +               val &= ~bitmask;
> +       writel(val, reg_addr);
> +
> +       /*
> +        * For secure processing after release from a module reset, dummy read
> +        * the same register at least seven times.

This comment is waiting to become out-of-sync with the actual value...

> +        */
> +       for (i = 0; !set && i < RZT2H_RESET_REG_READ_COUNT; i++)
> +               readl(reg_addr);
> +
> +       /* Verify the operation */
> +       val = readl(reg_addr);
> +       if ((set && !(bitmask & val)) || (!set && (bitmask & val))) {

Perhaps just "set == !(bitmask & val)"? Or is that too obscure?

> +               dev_err(priv->dev, "Reset register %u%02u operation failed\n", reg, bit);
> +               spin_unlock_irqrestore(&priv->pub.rmw_lock, flags);
> +               return -EIO;
> +       }
> +
> +       spin_unlock_irqrestore(&priv->pub.rmw_lock, flags);
> +
> +       return 0;
> +}

Regardless:
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

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 Mon, Oct 13, 2025 at 4:46 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Prabhakar,
>
> On Mon, 29 Sept 2025 at 13:23, 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 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>
> > ---
> > v3->v4:
> > - Renamed cpg_mrcr_set_bit() to cpg_mrcr_set_reset_state() for clarity.
> > - Updated the parameters in cpg_mrcr_set_reset_state().
>
> Thanks for the update!
>
> > --- a/drivers/clk/renesas/renesas-cpg-mssr.c
> > +++ b/drivers/clk/renesas/renesas-cpg-mssr.c
> > @@ -736,6 +754,72 @@ static int cpg_mssr_status(struct reset_controller_dev *rcdev,
> >         return !!(readl(priv->pub.base0 + priv->reset_regs[reg]) & bitmask);
> >  }
> >
> > +static int cpg_mrcr_set_reset_state(struct reset_controller_dev *rcdev,
> > +                                   unsigned long id, bool set)
> > +{
> > +       struct cpg_mssr_priv *priv = rcdev_to_priv(rcdev);
> > +       unsigned int reg = id / 32;
> > +       unsigned int bit = id % 32;
> > +       u32 bitmask = BIT(bit);
> > +       void __iomem *reg_addr;
> > +       unsigned long flags;
> > +       unsigned int i;
> > +       u32 val;
> > +
> > +       dev_dbg(priv->dev, "%s %u%02u\n", set ? "assert" : "deassert", reg, bit);
> > +
> > +       spin_lock_irqsave(&priv->pub.rmw_lock, flags);
> > +
> > +       reg_addr = priv->pub.base0 + priv->reset_regs[reg];
> > +       /* Read current value and modify */
> > +       val = readl(reg_addr);
> > +       if (set)
> > +               val |= bitmask;
> > +       else
> > +               val &= ~bitmask;
> > +       writel(val, reg_addr);
> > +
> > +       /*
> > +        * For secure processing after release from a module reset, dummy read
> > +        * the same register at least seven times.
>
> This comment is waiting to become out-of-sync with the actual value...
>
For the reset operation no, for this I would like to keep this as is.
But for the MSTP registers I will be adding a delay. Or did I
misunderstand something?

> > +        */
> > +       for (i = 0; !set && i < RZT2H_RESET_REG_READ_COUNT; i++)
> > +               readl(reg_addr);
> > +
> > +       /* Verify the operation */
> > +       val = readl(reg_addr);
> > +       if ((set && !(bitmask & val)) || (!set && (bitmask & val))) {
>
> Perhaps just "set == !(bitmask & val)"? Or is that too obscure?
>
Ok, I will update it to the above in v5.

Cheers,
Prabhakar
> > +               dev_err(priv->dev, "Reset register %u%02u operation failed\n", reg, bit);
> > +               spin_unlock_irqrestore(&priv->pub.rmw_lock, flags);
> > +               return -EIO;
> > +       }
> > +
> > +       spin_unlock_irqrestore(&priv->pub.rmw_lock, flags);
> > +
> > +       return 0;
> > +}
>
> Regardless:
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
>
> 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 Prabhakar,

On Mon, 13 Oct 2025 at 18:45, Lad, Prabhakar <prabhakar.csengg@gmail.com> wrote:
> On Mon, Oct 13, 2025 at 4:46 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Mon, 29 Sept 2025 at 13:23, 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 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>
> > > ---
> > > v3->v4:
> > > - Renamed cpg_mrcr_set_bit() to cpg_mrcr_set_reset_state() for clarity.
> > > - Updated the parameters in cpg_mrcr_set_reset_state().
> >
> > Thanks for the update!
> >
> > > --- a/drivers/clk/renesas/renesas-cpg-mssr.c
> > > +++ b/drivers/clk/renesas/renesas-cpg-mssr.c
> > > @@ -736,6 +754,72 @@ static int cpg_mssr_status(struct reset_controller_dev *rcdev,
> > >         return !!(readl(priv->pub.base0 + priv->reset_regs[reg]) & bitmask);
> > >  }
> > >
> > > +static int cpg_mrcr_set_reset_state(struct reset_controller_dev *rcdev,
> > > +                                   unsigned long id, bool set)
> > > +{
> > > +       struct cpg_mssr_priv *priv = rcdev_to_priv(rcdev);
> > > +       unsigned int reg = id / 32;
> > > +       unsigned int bit = id % 32;
> > > +       u32 bitmask = BIT(bit);
> > > +       void __iomem *reg_addr;
> > > +       unsigned long flags;
> > > +       unsigned int i;
> > > +       u32 val;
> > > +
> > > +       dev_dbg(priv->dev, "%s %u%02u\n", set ? "assert" : "deassert", reg, bit);
> > > +
> > > +       spin_lock_irqsave(&priv->pub.rmw_lock, flags);
> > > +
> > > +       reg_addr = priv->pub.base0 + priv->reset_regs[reg];
> > > +       /* Read current value and modify */
> > > +       val = readl(reg_addr);
> > > +       if (set)
> > > +               val |= bitmask;
> > > +       else
> > > +               val &= ~bitmask;
> > > +       writel(val, reg_addr);
> > > +
> > > +       /*
> > > +        * For secure processing after release from a module reset, dummy read
> > > +        * the same register at least seven times.
> >
> > This comment is waiting to become out-of-sync with the actual value...
> >
> For the reset operation no, for this I would like to keep this as is.
> But for the MSTP registers I will be adding a delay. Or did I
> misunderstand something?

How to make sure both "#define RZT2H_RESET_REG_READ_COUNT 7" and
"seven" are updated together?

    /*
     * For secure processing after release from a module reset, one must
     * perform multiple dummy reads of the same register.
     */

>
> > > +        */
> > > +       for (i = 0; !set && i < RZT2H_RESET_REG_READ_COUNT; i++)
> > > +               readl(reg_addr);
> > > +
> > > +       /* Verify the operation */
> > > +       val = readl(reg_addr);
> > > +       if ((set && !(bitmask & val)) || (!set && (bitmask & val))) {
> >
> > Perhaps just "set == !(bitmask & val)"? Or is that too obscure?
> >
> Ok, I will update it to the above in v5.

No need to resend yet, I could make these changes while applying
(when Philipp is happy).

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,

On Tue, Oct 14, 2025 at 7:46 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Prabhakar,
>
> On Mon, 13 Oct 2025 at 18:45, Lad, Prabhakar <prabhakar.csengg@gmail.com> wrote:
> > On Mon, Oct 13, 2025 at 4:46 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > On Mon, 29 Sept 2025 at 13:23, 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 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>
> > > > ---
> > > > v3->v4:
> > > > - Renamed cpg_mrcr_set_bit() to cpg_mrcr_set_reset_state() for clarity.
> > > > - Updated the parameters in cpg_mrcr_set_reset_state().
> > >
> > > Thanks for the update!
> > >
> > > > --- a/drivers/clk/renesas/renesas-cpg-mssr.c
> > > > +++ b/drivers/clk/renesas/renesas-cpg-mssr.c
> > > > @@ -736,6 +754,72 @@ static int cpg_mssr_status(struct reset_controller_dev *rcdev,
> > > >         return !!(readl(priv->pub.base0 + priv->reset_regs[reg]) & bitmask);
> > > >  }
> > > >
> > > > +static int cpg_mrcr_set_reset_state(struct reset_controller_dev *rcdev,
> > > > +                                   unsigned long id, bool set)
> > > > +{
> > > > +       struct cpg_mssr_priv *priv = rcdev_to_priv(rcdev);
> > > > +       unsigned int reg = id / 32;
> > > > +       unsigned int bit = id % 32;
> > > > +       u32 bitmask = BIT(bit);
> > > > +       void __iomem *reg_addr;
> > > > +       unsigned long flags;
> > > > +       unsigned int i;
> > > > +       u32 val;
> > > > +
> > > > +       dev_dbg(priv->dev, "%s %u%02u\n", set ? "assert" : "deassert", reg, bit);
> > > > +
> > > > +       spin_lock_irqsave(&priv->pub.rmw_lock, flags);
> > > > +
> > > > +       reg_addr = priv->pub.base0 + priv->reset_regs[reg];
> > > > +       /* Read current value and modify */
> > > > +       val = readl(reg_addr);
> > > > +       if (set)
> > > > +               val |= bitmask;
> > > > +       else
> > > > +               val &= ~bitmask;
> > > > +       writel(val, reg_addr);
> > > > +
> > > > +       /*
> > > > +        * For secure processing after release from a module reset, dummy read
> > > > +        * the same register at least seven times.
> > >
> > > This comment is waiting to become out-of-sync with the actual value...
> > >
> > For the reset operation no, for this I would like to keep this as is.
> > But for the MSTP registers I will be adding a delay. Or did I
> > misunderstand something?
>
> How to make sure both "#define RZT2H_RESET_REG_READ_COUNT 7" and
> "seven" are updated together?
>
>     /*
>      * For secure processing after release from a module reset, one must
>      * perform multiple dummy reads of the same register.
>      */
>
Ahh got you now, thanks for the clarification.

> >
> > > > +        */
> > > > +       for (i = 0; !set && i < RZT2H_RESET_REG_READ_COUNT; i++)
> > > > +               readl(reg_addr);
> > > > +
> > > > +       /* Verify the operation */
> > > > +       val = readl(reg_addr);
> > > > +       if ((set && !(bitmask & val)) || (!set && (bitmask & val))) {
> > >
> > > Perhaps just "set == !(bitmask & val)"? Or is that too obscure?
> > >
> > Ok, I will update it to the above in v5.
>
> No need to resend yet, I could make these changes while applying
> (when Philipp is happy).
>
Ok, thank you.


Cheers,
Prabhakar