Hi Prabhakar,
On Thu, 7 Aug 2025 at 18:44, Prabhakar <prabhakar.csengg@xxxxxxxxx> wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
>
> 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@xxxxxxxxxxxxxx>
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@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
