[PATCH AUTOSEL 6.19-6.18] clk: renesas: rzg2l: Deassert reset on assert timeout

[Sasha Levin]

From: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>

[ Upstream commit 0b0201f259e1158a875c5fd01adf318ae5d32352 ]

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@xxxxxxxxxxxxxx>
Reviewed-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>
Link: https://patch.msgid.link/20260108123433.104464-2-biju.das.jz@xxxxxxxxxxxxxx
Signed-off-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>
Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>
---

LLM Generated explanations, may be completely bogus:

The suspend/resume directly in this driver doesn't call assert. The
commit message mentions "suspend() callback" — this likely refers to
peripheral device drivers that use reset_control_assert() during their
own suspend callbacks, which then calls `rzg2l_cpg_assert()` through the
reset_control_ops.

## Summary of Analysis

### What the commit fixes:
1. **Variable shadowing bug**: `readl_poll_timeout_atomic` overwrites
   `value`, which is then needed for the error recovery path. The patch
   introduces a separate `mon` variable for the poll output, keeping
   `value` intact.
2. **Missing error recovery on assert timeout**: The original code only
   rolls back on deassert failure. If assert fails (e.g., during
   suspend), the device is left in an asserted (reset) state with no
   recovery, making the device non-operational even though the suspend
   callback returns an error. The fix uses `value ^= mask` to undo
   either direction of the operation.

### Dependencies:
This commit has **hard dependencies** on two v6.18 commits:
- `341b0f1b6cba5` - introduces `__rzg2l_cpg_assert()` helper
- `f8c5f0dc77d86` - introduces the deassert-only rollback code that this
  commit modifies

Without these two prerequisite commits, the patch cannot apply to any
stable tree older than v6.18.

### Risk Assessment:
- **Scope**: Small, contained change in one function (~10 lines changed)
- **Risk**: Low — only affects error paths in Renesas RZ/G2L clock/reset
  driver
- **Correctness**: The XOR approach is elegant and correct for both
  assert and deassert cases
- **Review**: Has "Reviewed-by: Geert Uytterhoeven" (Renesas clk
  subsystem maintainer)

### Stable Tree Applicability:
The fix only applies to v6.18.y since its prerequisites were merged in
v6.18-rc1. For older stable trees (6.12.y, 6.6.y, etc.), the code
structure is completely different (separate functions, no rollback
logic), so this specific bug doesn't exist there.

## Verification

- **git log master** showed the prerequisite commits 341b0f1b6cba5 and
  f8c5f0dc77d86 are only in v6.18+
- **git tag --contains** confirmed both prerequisites are in v6.18-rc1+,
  not in any earlier release
- **git show v6.12:drivers/clk/renesas/rzg2l-cpg.c** confirmed older
  stable trees have separate assert/deassert functions without the
  `__rzg2l_cpg_assert()` helper and without any error rollback logic
- **Read of current file** confirmed the code matches the "before" state
  of the diff
- **Code analysis** verified the XOR-based rollback logic is correct:
  when asserting, `value = mask << 16` (no lower mask), XOR with mask
  adds it → deasserts; when deasserting, `value = (mask << 16) | mask`,
  XOR with mask removes it → re-asserts
- **Variable shadowing confirmed**: `readl_poll_timeout_atomic` modifies
  the `value` parameter in-place, so using it for both the initial write
  value and poll output is a real bug (though in practice the old code's
  `if (ret && !assert)` branch reassigned `value`, masking the issue for
  deassert)

## Decision

This is a legitimate bug fix that:
1. Fixes a real issue where assert failure during suspend leaves
   hardware in a non-operational state
2. Also fixes a variable shadowing bug (reusing `value` for two
   different purposes)
3. Is small, contained, reviewed by the subsystem maintainer
4. Meets stable kernel criteria

However, it has hard dependencies on two refactoring/feature commits
from v6.18 that are not in any current stable tree. The only applicable
stable tree would be 6.18.y. Given that this is a genuine fix for an
error recovery path that matters during suspend/resume, and it's
applicable to 6.18.y, it should be backported.

**YES**

 drivers/clk/renesas/rzg2l-cpg.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/renesas/rzg2l-cpg.c b/drivers/clk/renesas/rzg2l-cpg.c
index 64d1ef6e4c943..c20ea1212b360 100644
--- a/drivers/clk/renesas/rzg2l-cpg.c
+++ b/drivers/clk/renesas/rzg2l-cpg.c
@@ -1647,6 +1647,7 @@ 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;
+	u32 mon;
 	int ret;
 
 	dev_dbg(rcdev->dev, "%s id:%ld offset:0x%x\n",
@@ -1667,10 +1668,10 @@ static int __rzg2l_cpg_assert(struct reset_controller_dev *rcdev,
 		return 0;
 	}
 
-	ret = readl_poll_timeout_atomic(priv->base + reg, value,
-					assert == !!(value & mask), 10, 200);
-	if (ret && !assert) {
-		value = mask << 16;
+	ret = readl_poll_timeout_atomic(priv->base + reg, mon,
+					assert == !!(mon & mask), 10, 200);
+	if (ret) {
+		value ^= mask;
 		writel(value, priv->base + CLK_RST_R(info->resets[id].off));
 	}
 
-- 
2.51.0