[PATCH 1/2] clk: renesas: rzg2l: Deassert reset on assert timeout

Biju posted 2 patches 2 months ago
There is a newer version of this series
[PATCH 1/2] clk: renesas: rzg2l: Deassert reset on assert timeout
Posted by Biju 2 months ago
From: Biju Das <biju.das.jz@bp.renesas.com>

If the assert() fails due to timeout error, set the reset register bit
back to deasserted state. This change is needed especially for handling
assert error in suspend() callback that expect the device to be in
operational state in case of failure.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 drivers/clk/renesas/rzg2l-cpg.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/renesas/rzg2l-cpg.c b/drivers/clk/renesas/rzg2l-cpg.c
index 64d1ef6e4c94..751f0340854f 100644
--- a/drivers/clk/renesas/rzg2l-cpg.c
+++ b/drivers/clk/renesas/rzg2l-cpg.c
@@ -1669,8 +1669,11 @@ static int __rzg2l_cpg_assert(struct reset_controller_dev *rcdev,
 
 	ret = readl_poll_timeout_atomic(priv->base + reg, value,
 					assert == !!(value & mask), 10, 200);
-	if (ret && !assert) {
+	if (ret) {
 		value = mask << 16;
+		if (assert)
+			value |= mask;
+
 		writel(value, priv->base + CLK_RST_R(info->resets[id].off));
 	}
 
-- 
2.43.0
Re: [PATCH 1/2] clk: renesas: rzg2l: Deassert reset on assert timeout
Posted by Geert Uytterhoeven 1 month ago
Hi Biju,

On Mon, 8 Dec 2025 at 11:14, Biju <biju.das.au@gmail.com> wrote:
> From: Biju Das <biju.das.jz@bp.renesas.com>
>
> If the assert() fails due to timeout error, set the reset register bit
> back to deasserted state. This change is needed especially for handling
> assert error in suspend() callback that expect the device to be in
> operational state in case of failure.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

Thanks for your patch!

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

However, I am wondering what you think about the alternative below?

> --- a/drivers/clk/renesas/rzg2l-cpg.c
> +++ b/drivers/clk/renesas/rzg2l-cpg.c
> @@ -1669,8 +1669,11 @@ static int __rzg2l_cpg_assert(struct reset_controller_dev *rcdev,
>
>         ret = readl_poll_timeout_atomic(priv->base + reg, value,
>                                         assert == !!(value & mask), 10, 200);

If this loop would use its own "u32 mon" instead of reusing "value"...

> -       if (ret && !assert) {
> +       if (ret) {

... then "value" would still have the wanted state here...

>                 value = mask << 16;
> +               if (assert)
> +                       value |= mask;
> +

... and you can just switch back to the old state using:

    value ^= mask;

>                 writel(value, priv->base + CLK_RST_R(info->resets[id].off));
>         }
>

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