[PATCH 0/4] dmaengine: sh: rz-dmac: Add runtime PM support

[PATCH 0/4] dmaengine: sh: rz-dmac: Add runtime PM support

[Tommaso Merciai]

Dear All,

This patch series adds runtime PM support to the rz-dmac driver.
It also combines common code from rzg2l_cpg_assert() and rzg2l_cpg_deassert()
into a new __rzg2l_cpg_assert() helper to avoid code duplication,
and reworks __rzg2l_cpg_assert()/__rzv2h_cpg_assert() to return the state of
the assert/deassert operation.

Thanks & Regards,
Tommaso

Tommaso Merciai (4):
  clk: renesas: rzg2l: Simplify rzg2l_cpg_assert() and
    rzg2l_cpg_deassert()
  clk: renesas: rzg2l: Re-assert reset on deassert timeout
  clk: renesas: rzv2h: Re-assert reset on deassert timeout
  dmaengine: sh: rz-dmac: Add runtime PM support

 drivers/clk/renesas/rzg2l-cpg.c | 54 ++++++++++++++-----------------
 drivers/clk/renesas/rzv2h-cpg.c | 14 ++++++--
 drivers/dma/sh/rz-dmac.c        | 57 +++++++++++++++++++++++++++++----
 3 files changed, 85 insertions(+), 40 deletions(-)

-- 
2.43.0

[PATCH 1/4] clk: renesas: rzg2l: Simplify rzg2l_cpg_assert() and rzg2l_cpg_deassert()

[Tommaso Merciai]

Combine common code from rzg2l_cpg_assert() and rzg2l_cpg_deassert() into a
new __rzg2l_cpg_assert() helper to avoid code duplication. This reduces
maintenance effort and improves code clarity.

Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
 drivers/clk/renesas/rzg2l-cpg.c | 45 ++++++++++++---------------------
 1 file changed, 16 insertions(+), 29 deletions(-)

diff --git a/drivers/clk/renesas/rzg2l-cpg.c b/drivers/clk/renesas/rzg2l-cpg.c
index b508823f9723..1c648c0baf7c 100644
--- a/drivers/clk/renesas/rzg2l-cpg.c
+++ b/drivers/clk/renesas/rzg2l-cpg.c
@@ -1638,8 +1638,8 @@ rzg2l_cpg_register_mod_clk(const struct rzg2l_mod_clk *mod,
 
 #define rcdev_to_priv(x)	container_of(x, struct rzg2l_cpg_priv, rcdev)
 
-static int rzg2l_cpg_assert(struct reset_controller_dev *rcdev,
-			    unsigned long id)
+static int __rzg2l_cpg_assert(struct reset_controller_dev *rcdev,
+			      unsigned long id, bool assert)
 {
 	struct rzg2l_cpg_priv *priv = rcdev_to_priv(rcdev);
 	const struct rzg2l_cpg_info *info = priv->info;
@@ -1647,9 +1647,13 @@ static int rzg2l_cpg_assert(struct reset_controller_dev *rcdev,
 	u32 mask = BIT(info->resets[id].bit);
 	s8 monbit = info->resets[id].monbit;
 	u32 value = mask << 16;
+	int ret;
 
-	dev_dbg(rcdev->dev, "assert id:%ld offset:0x%x\n", id, CLK_RST_R(reg));
+	dev_dbg(rcdev->dev, "%s id:%ld offset:0x%x\n",
+		assert ? "assert" : "deassert", id, CLK_RST_R(reg));
 
+	if (!assert)
+		value |= mask;
 	writel(value, priv->base + CLK_RST_R(reg));
 
 	if (info->has_clk_mon_regs) {
@@ -1664,37 +1668,20 @@ static int rzg2l_cpg_assert(struct reset_controller_dev *rcdev,
 	}
 
 	return readl_poll_timeout_atomic(priv->base + reg, value,
-					 value & mask, 10, 200);
+					 assert ? (value & mask) : !(value & mask),
+					 10, 200);
+}
+
+static int rzg2l_cpg_assert(struct reset_controller_dev *rcdev,
+			    unsigned long id)
+{
+	return __rzg2l_cpg_assert(rcdev, id, true);
 }
 
 static int rzg2l_cpg_deassert(struct reset_controller_dev *rcdev,
 			      unsigned long id)
 {
-	struct rzg2l_cpg_priv *priv = rcdev_to_priv(rcdev);
-	const struct rzg2l_cpg_info *info = priv->info;
-	unsigned int reg = info->resets[id].off;
-	u32 mask = BIT(info->resets[id].bit);
-	s8 monbit = info->resets[id].monbit;
-	u32 value = (mask << 16) | mask;
-
-	dev_dbg(rcdev->dev, "deassert id:%ld offset:0x%x\n", id,
-		CLK_RST_R(reg));
-
-	writel(value, priv->base + CLK_RST_R(reg));
-
-	if (info->has_clk_mon_regs) {
-		reg = CLK_MRST_R(reg);
-	} else if (monbit >= 0) {
-		reg = CPG_RST_MON;
-		mask = BIT(monbit);
-	} else {
-		/* Wait for at least one cycle of the RCLK clock (@ ca. 32 kHz) */
-		udelay(35);
-		return 0;
-	}
-
-	return readl_poll_timeout_atomic(priv->base + reg, value,
-					 !(value & mask), 10, 200);
+	return __rzg2l_cpg_assert(rcdev, id, false);
 }
 
 static int rzg2l_cpg_reset(struct reset_controller_dev *rcdev,
-- 
2.43.0

[PATCH 2/4] clk: renesas: rzg2l: Re-assert reset on deassert timeout

[Tommaso Merciai]

Prevent issues during reset deassertion by re-asserting the reset if a
timeout occurs when trying to deassert. This ensures the reset line is in a
known state and improves reliability for hardware that may not immediately
clear the reset monitor bit.

Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
 drivers/clk/renesas/rzg2l-cpg.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/renesas/rzg2l-cpg.c b/drivers/clk/renesas/rzg2l-cpg.c
index 1c648c0baf7c..6ed90368cf94 100644
--- a/drivers/clk/renesas/rzg2l-cpg.c
+++ b/drivers/clk/renesas/rzg2l-cpg.c
@@ -1667,9 +1667,16 @@ static int __rzg2l_cpg_assert(struct reset_controller_dev *rcdev,
 		return 0;
 	}
 
-	return readl_poll_timeout_atomic(priv->base + reg, value,
-					 assert ? (value & mask) : !(value & mask),
-					 10, 200);
+	ret = readl_poll_timeout_atomic(priv->base + reg, value,
+					assert ? (value & mask) : !(value & mask),
+					10, 200);
+	if (ret && !assert) {
+		dev_warn(rcdev->dev, "deassert timeout, re-asserting reset id %ld\n", id);
+		value = mask << 16;
+		writel(value, priv->base + CLK_RST_R(info->resets[id].off));
+	}
+
+	return ret;
 }
 
 static int rzg2l_cpg_assert(struct reset_controller_dev *rcdev,
-- 
2.43.0

[PATCH 3/4] clk: renesas: rzv2h: Re-assert reset on deassert timeout

[Tommaso Merciai]

Prevent issues during reset deassertion by re-asserting the reset if a
timeout occurs when trying to deassert. This ensures the reset line is in a
known state and improves reliability for hardware that may not immediately
clear the reset monitor bit.

Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
 drivers/clk/renesas/rzv2h-cpg.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/renesas/rzv2h-cpg.c b/drivers/clk/renesas/rzv2h-cpg.c
index 58ccbae0f904..469f40894c43 100644
--- a/drivers/clk/renesas/rzv2h-cpg.c
+++ b/drivers/clk/renesas/rzv2h-cpg.c
@@ -854,6 +854,7 @@ static int __rzv2h_cpg_assert(struct reset_controller_dev *rcdev,
 	u32 mask = BIT(priv->resets[id].reset_bit);
 	u8 monbit = priv->resets[id].mon_bit;
 	u32 value = mask << 16;
+	int ret;
 
 	dev_dbg(rcdev->dev, "%s id:%ld offset:0x%x\n",
 		assert ? "assert" : "deassert", id, reg);
@@ -865,9 +866,16 @@ static int __rzv2h_cpg_assert(struct reset_controller_dev *rcdev,
 	reg = GET_RST_MON_OFFSET(priv->resets[id].mon_index);
 	mask = BIT(monbit);
 
-	return readl_poll_timeout_atomic(priv->base + reg, value,
-					 assert ? (value & mask) : !(value & mask),
-					 10, 200);
+	ret = readl_poll_timeout_atomic(priv->base + reg, value,
+					assert ? (value & mask) : !(value & mask),
+					10, 200);
+	if (ret && !assert) {
+		dev_warn(rcdev->dev, "deassert timeout, re-asserting reset id %ld\n", id);
+		value = mask << 16;
+		writel(value, priv->base + GET_RST_OFFSET(priv->resets[id].reset_index));
+	}
+
+	return ret;
 }
 
 static int rzv2h_cpg_assert(struct reset_controller_dev *rcdev,
-- 
2.43.0

[PATCH 4/4] dmaengine: sh: rz-dmac: Add runtime PM support

[Tommaso Merciai]

Enable runtime power management in the rz-dmac driver by adding suspend and
resume callbacks. This ensures the driver can correctly assert and deassert
the reset control and manage power state transitions during suspend and
resume. Adding runtime PM support allows the DMA controller to reduce power
consumption when idle and maintain correct operation across system sleep
states, addressing the previous lack of dynamic power management in the
driver.

Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
---
 drivers/dma/sh/rz-dmac.c | 57 +++++++++++++++++++++++++++++++++++-----
 1 file changed, 50 insertions(+), 7 deletions(-)

diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c
index 1f687b08d6b8..2f06bdb7ce3b 100644
--- a/drivers/dma/sh/rz-dmac.c
+++ b/drivers/dma/sh/rz-dmac.c
@@ -437,6 +437,17 @@ static int rz_dmac_xfer_desc(struct rz_dmac_chan *chan)
  * DMA engine operations
  */
 
+static void rz_dmac_chan_init_all(struct rz_dmac *dmac)
+{
+	unsigned int i;
+
+	rz_dmac_writel(dmac, DCTRL_DEFAULT, CHANNEL_0_7_COMMON_BASE + DCTRL);
+	rz_dmac_writel(dmac, DCTRL_DEFAULT, CHANNEL_8_15_COMMON_BASE + DCTRL);
+
+	for (i = 0; i < dmac->n_channels; i++)
+		rz_dmac_ch_writel(&dmac->channels[i], CHCTRL_DEFAULT, CHCTRL, 1);
+}
+
 static int rz_dmac_alloc_chan_resources(struct dma_chan *chan)
 {
 	struct rz_dmac_chan *channel = to_rz_dmac_chan(chan);
@@ -970,10 +981,6 @@ static int rz_dmac_probe(struct platform_device *pdev)
 		goto err_pm_disable;
 	}
 
-	ret = reset_control_deassert(dmac->rstc);
-	if (ret)
-		goto err_pm_runtime_put;
-
 	for (i = 0; i < dmac->n_channels; i++) {
 		ret = rz_dmac_chan_probe(dmac, &dmac->channels[i], i);
 		if (ret < 0)
@@ -1028,8 +1035,6 @@ static int rz_dmac_probe(struct platform_device *pdev)
 				  channel->lmdesc.base_dma);
 	}
 
-	reset_control_assert(dmac->rstc);
-err_pm_runtime_put:
 	pm_runtime_put(&pdev->dev);
 err_pm_disable:
 	pm_runtime_disable(&pdev->dev);
@@ -1052,13 +1057,50 @@ static void rz_dmac_remove(struct platform_device *pdev)
 				  channel->lmdesc.base,
 				  channel->lmdesc.base_dma);
 	}
-	reset_control_assert(dmac->rstc);
 	pm_runtime_put(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
 
 	platform_device_put(dmac->icu.pdev);
 }
 
+static int rz_dmac_runtime_suspend(struct device *dev)
+{
+	struct rz_dmac *dmac = dev_get_drvdata(dev);
+
+	return reset_control_assert(dmac->rstc);
+}
+
+static int rz_dmac_runtime_resume(struct device *dev)
+{
+	struct rz_dmac *dmac = dev_get_drvdata(dev);
+
+	return reset_control_deassert(dmac->rstc);
+}
+
+static int rz_dmac_resume(struct device *dev)
+{
+	struct rz_dmac *dmac = dev_get_drvdata(dev);
+	int ret;
+
+	ret = pm_runtime_force_resume(dev);
+	if (ret)
+		return ret;
+
+	rz_dmac_chan_init_all(dmac);
+
+	return 0;
+}
+
+static const struct dev_pm_ops rz_dmac_pm_ops = {
+	/*
+	 * TODO for system sleep/resume:
+	 *   - Wait for the current transfer to complete and stop the device,
+	 *   - Resume transfers, if any.
+	 */
+	NOIRQ_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend, rz_dmac_resume)
+	RUNTIME_PM_OPS(rz_dmac_runtime_suspend, rz_dmac_runtime_resume, NULL)
+};
+
 static const struct of_device_id of_rz_dmac_match[] = {
 	{ .compatible = "renesas,r9a09g057-dmac", },
 	{ .compatible = "renesas,rz-dmac", },
@@ -1068,6 +1110,7 @@ MODULE_DEVICE_TABLE(of, of_rz_dmac_match);
 
 static struct platform_driver rz_dmac_driver = {
 	.driver		= {
+		.pm	= pm_ptr(&rz_dmac_pm_ops),
 		.name	= "rz-dmac",
 		.of_match_table = of_rz_dmac_match,
 	},
-- 
2.43.0

Re: [PATCH 1/4] clk: renesas: rzg2l: Simplify rzg2l_cpg_assert() and rzg2l_cpg_deassert()

[Geert Uytterhoeven]

Hi Tommaso,

On Wed, 20 Aug 2025 at 12:05, Tommaso Merciai
<tommaso.merciai.xr@bp.renesas.com> wrote:
> Combine common code from rzg2l_cpg_assert() and rzg2l_cpg_deassert() into a
> new __rzg2l_cpg_assert() helper to avoid code duplication. This reduces
> maintenance effort and improves code clarity.
>
> Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>

Thanks for your patch!

> --- a/drivers/clk/renesas/rzg2l-cpg.c
> +++ b/drivers/clk/renesas/rzg2l-cpg.c

> @@ -1664,37 +1668,20 @@ static int rzg2l_cpg_assert(struct reset_controller_dev *rcdev,
>         }
>
>         return readl_poll_timeout_atomic(priv->base + reg, value,
> -                                        value & mask, 10, 200);
> +                                        assert ? (value & mask) : !(value & mask),

This can be simplified to "assert == !!(value & mask)".
Do you like that?

> +                                        10, 200);
> +}

The rest LGTM, so
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

Re: [PATCH 2/4] clk: renesas: rzg2l: Re-assert reset on deassert timeout

[Geert Uytterhoeven]

Hi Tommaso,

On Wed, 20 Aug 2025 at 12:05, Tommaso Merciai
<tommaso.merciai.xr@bp.renesas.com> wrote:
> Prevent issues during reset deassertion by re-asserting the reset if a
> timeout occurs when trying to deassert. This ensures the reset line is in a
> known state and improves reliability for hardware that may not immediately
> clear the reset monitor bit.
>
> Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>

Thanks for your patch!

> --- a/drivers/clk/renesas/rzg2l-cpg.c
> +++ b/drivers/clk/renesas/rzg2l-cpg.c
> @@ -1667,9 +1667,16 @@ static int __rzg2l_cpg_assert(struct reset_controller_dev *rcdev,
>                 return 0;
>         }
>
> -       return readl_poll_timeout_atomic(priv->base + reg, value,
> -                                        assert ? (value & mask) : !(value & mask),
> -                                        10, 200);
> +       ret = readl_poll_timeout_atomic(priv->base + reg, value,
> +                                       assert ? (value & mask) : !(value & mask),
> +                                       10, 200);
> +       if (ret && !assert) {
> +               dev_warn(rcdev->dev, "deassert timeout, re-asserting reset id %ld\n", id);
> +               value = mask << 16;
> +               writel(value, priv->base + CLK_RST_R(info->resets[id].off));
> +       }

Is this an issue you've seen during actual use?
Would it make sense to print warnings on assertion timeouts, too?

> +
> +       return ret;
>  }

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

Re: [PATCH 3/4] clk: renesas: rzv2h: Re-assert reset on deassert timeout

[Geert Uytterhoeven]

Hi Tommaso,

On Wed, 20 Aug 2025 at 12:05, Tommaso Merciai
<tommaso.merciai.xr@bp.renesas.com> wrote:
> Prevent issues during reset deassertion by re-asserting the reset if a
> timeout occurs when trying to deassert. This ensures the reset line is in a
> known state and improves reliability for hardware that may not immediately
> clear the reset monitor bit.
>
> Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>

Thanks for your patch!

> --- a/drivers/clk/renesas/rzv2h-cpg.c
> +++ b/drivers/clk/renesas/rzv2h-cpg.c
> @@ -865,9 +866,16 @@ static int __rzv2h_cpg_assert(struct reset_controller_dev *rcdev,
>         reg = GET_RST_MON_OFFSET(priv->resets[id].mon_index);
>         mask = BIT(monbit);
>
> -       return readl_poll_timeout_atomic(priv->base + reg, value,
> -                                        assert ? (value & mask) : !(value & mask),
> -                                        10, 200);
> +       ret = readl_poll_timeout_atomic(priv->base + reg, value,
> +                                       assert ? (value & mask) : !(value & mask),
> +                                       10, 200);
> +       if (ret && !assert) {
> +               dev_warn(rcdev->dev, "deassert timeout, re-asserting reset id %ld\n", id);
> +               value = mask << 16;
> +               writel(value, priv->base + GET_RST_OFFSET(priv->resets[id].reset_index));
> +       }

Same questions as for the previous patch:
Is this an issue you've seen during actual use?
Would it make sense to print warnings on assertion timeouts, too?

> +
> +       return ret;
>  }
>
>  static int rzv2h_cpg_assert(struct reset_controller_dev *rcdev,

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

Re: [PATCH 1/4] clk: renesas: rzg2l: Simplify rzg2l_cpg_assert() and rzg2l_cpg_deassert()

[Tommaso Merciai]

Hi Geert,
Thanks for your review!

On Tue, Sep 02, 2025 at 02:12:50PM +0200, Geert Uytterhoeven wrote:
> Hi Tommaso,
> 
> On Wed, 20 Aug 2025 at 12:05, Tommaso Merciai
> <tommaso.merciai.xr@bp.renesas.com> wrote:
> > Combine common code from rzg2l_cpg_assert() and rzg2l_cpg_deassert() into a
> > new __rzg2l_cpg_assert() helper to avoid code duplication. This reduces
> > maintenance effort and improves code clarity.
> >
> > Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> 
> Thanks for your patch!
> 
> > --- a/drivers/clk/renesas/rzg2l-cpg.c
> > +++ b/drivers/clk/renesas/rzg2l-cpg.c
> 
> > @@ -1664,37 +1668,20 @@ static int rzg2l_cpg_assert(struct reset_controller_dev *rcdev,
> >         }
> >
> >         return readl_poll_timeout_atomic(priv->base + reg, value,
> > -                                        value & mask, 10, 200);
> > +                                        assert ? (value & mask) : !(value & mask),
> 
> This can be simplified to "assert == !!(value & mask)".
> Do you like that?

Yes, thanks.
Looks good to me.

Thanks & Regards,
Tommaso

> 
> > +                                        10, 200);
> > +}
> 
> The rest LGTM, so
> 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

Re: [PATCH 2/4] clk: renesas: rzg2l: Re-assert reset on deassert timeout

[Tommaso Merciai]

Hi Geert,
Thank you for your review!

On Tue, Sep 02, 2025 at 02:18:17PM +0200, Geert Uytterhoeven wrote:
> Hi Tommaso,
> 
> On Wed, 20 Aug 2025 at 12:05, Tommaso Merciai
> <tommaso.merciai.xr@bp.renesas.com> wrote:
> > Prevent issues during reset deassertion by re-asserting the reset if a
> > timeout occurs when trying to deassert. This ensures the reset line is in a
> > known state and improves reliability for hardware that may not immediately
> > clear the reset monitor bit.
> >
> > Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> 
> Thanks for your patch!
> 
> > --- a/drivers/clk/renesas/rzg2l-cpg.c
> > +++ b/drivers/clk/renesas/rzg2l-cpg.c
> > @@ -1667,9 +1667,16 @@ static int __rzg2l_cpg_assert(struct reset_controller_dev *rcdev,
> >                 return 0;
> >         }
> >
> > -       return readl_poll_timeout_atomic(priv->base + reg, value,
> > -                                        assert ? (value & mask) : !(value & mask),
> > -                                        10, 200);
> > +       ret = readl_poll_timeout_atomic(priv->base + reg, value,
> > +                                       assert ? (value & mask) : !(value & mask),
> > +                                       10, 200);
> > +       if (ret && !assert) {
> > +               dev_warn(rcdev->dev, "deassert timeout, re-asserting reset id %ld\n", id);
> > +               value = mask << 16;
> > +               writel(value, priv->base + CLK_RST_R(info->resets[id].off));
> > +       }
> 
> Is this an issue you've seen during actual use?
> Would it make sense to print warnings on assertion timeouts, too?

I haven’t observed any assertion timeout issues during actual use,
so maybe printing warnings on assertion may not be necessary.
What do you think?

Thanks & Regards,
Tommaso

> 
> > +
> > +       return ret;
> >  }
> 
> 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

Re: [PATCH 3/4] clk: renesas: rzv2h: Re-assert reset on deassert timeout

[Tommaso Merciai]

Hi Geert,
Thanks for your review!

On Tue, Sep 02, 2025 at 02:19:16PM +0200, Geert Uytterhoeven wrote:
> Hi Tommaso,
> 
> On Wed, 20 Aug 2025 at 12:05, Tommaso Merciai
> <tommaso.merciai.xr@bp.renesas.com> wrote:
> > Prevent issues during reset deassertion by re-asserting the reset if a
> > timeout occurs when trying to deassert. This ensures the reset line is in a
> > known state and improves reliability for hardware that may not immediately
> > clear the reset monitor bit.
> >
> > Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> 
> Thanks for your patch!
> 
> > --- a/drivers/clk/renesas/rzv2h-cpg.c
> > +++ b/drivers/clk/renesas/rzv2h-cpg.c
> > @@ -865,9 +866,16 @@ static int __rzv2h_cpg_assert(struct reset_controller_dev *rcdev,
> >         reg = GET_RST_MON_OFFSET(priv->resets[id].mon_index);
> >         mask = BIT(monbit);
> >
> > -       return readl_poll_timeout_atomic(priv->base + reg, value,
> > -                                        assert ? (value & mask) : !(value & mask),
> > -                                        10, 200);
> > +       ret = readl_poll_timeout_atomic(priv->base + reg, value,
> > +                                       assert ? (value & mask) : !(value & mask),
> > +                                       10, 200);
> > +       if (ret && !assert) {
> > +               dev_warn(rcdev->dev, "deassert timeout, re-asserting reset id %ld\n", id);
> > +               value = mask << 16;
> > +               writel(value, priv->base + GET_RST_OFFSET(priv->resets[id].reset_index));
> > +       }
> 
> Same questions as for the previous patch:
> Is this an issue you've seen during actual use?
> Would it make sense to print warnings on assertion timeouts, too?

Same here.

Thank & Regards,
Tommaso

> 
> > +
> > +       return ret;
> >  }
> >
> >  static int rzv2h_cpg_assert(struct reset_controller_dev *rcdev,
> 
> 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

Re: [PATCH 2/4] clk: renesas: rzg2l: Re-assert reset on deassert timeout

[Geert Uytterhoeven]

Hi Tommaso,

On Tue, 2 Sept 2025 at 15:51, Tommaso Merciai
<tommaso.merciai.xr@bp.renesas.com> wrote:
> On Tue, Sep 02, 2025 at 02:18:17PM +0200, Geert Uytterhoeven wrote:
> > On Wed, 20 Aug 2025 at 12:05, Tommaso Merciai
> > <tommaso.merciai.xr@bp.renesas.com> wrote:
> > > Prevent issues during reset deassertion by re-asserting the reset if a
> > > timeout occurs when trying to deassert. This ensures the reset line is in a
> > > known state and improves reliability for hardware that may not immediately
> > > clear the reset monitor bit.
> > >
> > > Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> >
> > > --- a/drivers/clk/renesas/rzg2l-cpg.c
> > > +++ b/drivers/clk/renesas/rzg2l-cpg.c
> > > @@ -1667,9 +1667,16 @@ static int __rzg2l_cpg_assert(struct reset_controller_dev *rcdev,
> > >                 return 0;
> > >         }
> > >
> > > -       return readl_poll_timeout_atomic(priv->base + reg, value,
> > > -                                        assert ? (value & mask) : !(value & mask),
> > > -                                        10, 200);
> > > +       ret = readl_poll_timeout_atomic(priv->base + reg, value,
> > > +                                       assert ? (value & mask) : !(value & mask),
> > > +                                       10, 200);
> > > +       if (ret && !assert) {
> > > +               dev_warn(rcdev->dev, "deassert timeout, re-asserting reset id %ld\n", id);
> > > +               value = mask << 16;
> > > +               writel(value, priv->base + CLK_RST_R(info->resets[id].off));
> > > +       }
> >
> > Is this an issue you've seen during actual use?
> > Would it make sense to print warnings on assertion timeouts, too?
>
> I haven’t observed any assertion timeout issues during actual use,
> so maybe printing warnings on assertion may not be necessary.
> What do you think?

Have you seen deassertion timeouts?
I would rather not print a warning.  The error code would be propagated
up anyway.

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 2/4] clk: renesas: rzg2l: Re-assert reset on deassert timeout

[Tommaso Merciai]

Hi Geert,

On Tue, Sep 02, 2025 at 04:16:12PM +0200, Geert Uytterhoeven wrote:
> Hi Tommaso,
> 
> On Tue, 2 Sept 2025 at 15:51, Tommaso Merciai
> <tommaso.merciai.xr@bp.renesas.com> wrote:
> > On Tue, Sep 02, 2025 at 02:18:17PM +0200, Geert Uytterhoeven wrote:
> > > On Wed, 20 Aug 2025 at 12:05, Tommaso Merciai
> > > <tommaso.merciai.xr@bp.renesas.com> wrote:
> > > > Prevent issues during reset deassertion by re-asserting the reset if a
> > > > timeout occurs when trying to deassert. This ensures the reset line is in a
> > > > known state and improves reliability for hardware that may not immediately
> > > > clear the reset monitor bit.
> > > >
> > > > Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> > >
> > > > --- a/drivers/clk/renesas/rzg2l-cpg.c
> > > > +++ b/drivers/clk/renesas/rzg2l-cpg.c
> > > > @@ -1667,9 +1667,16 @@ static int __rzg2l_cpg_assert(struct reset_controller_dev *rcdev,
> > > >                 return 0;
> > > >         }
> > > >
> > > > -       return readl_poll_timeout_atomic(priv->base + reg, value,
> > > > -                                        assert ? (value & mask) : !(value & mask),
> > > > -                                        10, 200);
> > > > +       ret = readl_poll_timeout_atomic(priv->base + reg, value,
> > > > +                                       assert ? (value & mask) : !(value & mask),
> > > > +                                       10, 200);
> > > > +       if (ret && !assert) {
> > > > +               dev_warn(rcdev->dev, "deassert timeout, re-asserting reset id %ld\n", id);
> > > > +               value = mask << 16;
> > > > +               writel(value, priv->base + CLK_RST_R(info->resets[id].off));
> > > > +       }
> > >
> > > Is this an issue you've seen during actual use?
> > > Would it make sense to print warnings on assertion timeouts, too?
> >
> > I haven’t observed any assertion timeout issues during actual use,
> > so maybe printing warnings on assertion may not be necessary.
> > What do you think?
> 
> Have you seen deassertion timeouts?

I haven’t seen any deassertion timeouts either.

> I would rather not print a warning.  The error code would be propagated
> up anyway.

If for you is ok.
I will drop dev_warn() on both:

	- __rzg2l_cpg_assert()
	- __rzv2h_cpg_assert()

in v2.
Please gently le me know.

Thanks & Regards,
Tommaso

> 
> 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 2/4] clk: renesas: rzg2l: Re-assert reset on deassert timeout

[Geert Uytterhoeven]

Hi Tommaso,

On Tue, 2 Sept 2025 at 16:26, Tommaso Merciai
<tommaso.merciai.xr@bp.renesas.com> wrote:
> On Tue, Sep 02, 2025 at 04:16:12PM +0200, Geert Uytterhoeven wrote:
> > On Tue, 2 Sept 2025 at 15:51, Tommaso Merciai
> > <tommaso.merciai.xr@bp.renesas.com> wrote:
> > > On Tue, Sep 02, 2025 at 02:18:17PM +0200, Geert Uytterhoeven wrote:
> > > > On Wed, 20 Aug 2025 at 12:05, Tommaso Merciai
> > > > <tommaso.merciai.xr@bp.renesas.com> wrote:
> > > > > Prevent issues during reset deassertion by re-asserting the reset if a
> > > > > timeout occurs when trying to deassert. This ensures the reset line is in a
> > > > > known state and improves reliability for hardware that may not immediately
> > > > > clear the reset monitor bit.
> > > > >
> > > > > Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> > > >
> > > > > --- a/drivers/clk/renesas/rzg2l-cpg.c
> > > > > +++ b/drivers/clk/renesas/rzg2l-cpg.c
> > > > > @@ -1667,9 +1667,16 @@ static int __rzg2l_cpg_assert(struct reset_controller_dev *rcdev,
> > > > >                 return 0;
> > > > >         }
> > > > >
> > > > > -       return readl_poll_timeout_atomic(priv->base + reg, value,
> > > > > -                                        assert ? (value & mask) : !(value & mask),
> > > > > -                                        10, 200);
> > > > > +       ret = readl_poll_timeout_atomic(priv->base + reg, value,
> > > > > +                                       assert ? (value & mask) : !(value & mask),
> > > > > +                                       10, 200);
> > > > > +       if (ret && !assert) {
> > > > > +               dev_warn(rcdev->dev, "deassert timeout, re-asserting reset id %ld\n", id);
> > > > > +               value = mask << 16;
> > > > > +               writel(value, priv->base + CLK_RST_R(info->resets[id].off));
> > > > > +       }
> > > >
> > > > Is this an issue you've seen during actual use?
> > > > Would it make sense to print warnings on assertion timeouts, too?
> > >
> > > I haven’t observed any assertion timeout issues during actual use,
> > > so maybe printing warnings on assertion may not be necessary.
> > > What do you think?
> >
> > Have you seen deassertion timeouts?
>
> I haven’t seen any deassertion timeouts either.
>
> > I would rather not print a warning.  The error code would be propagated
> > up anyway.
>
> If for you is ok.
> I will drop dev_warn() on both:
>
>         - __rzg2l_cpg_assert()
>         - __rzv2h_cpg_assert()
>
> in v2.
> Please gently le me know.

Please drop dev_warn() on both. Thanks!

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