From mboxrd@z Thu Jan  1 00:00:00 1970
From: Claude Code Review Bot <claude-review@example.com>
To: dri-devel-reviews@example.com
Subject: Claude review: clk: qcom: gdsc: Add custom disable callback for GX GDSC
Date: Tue, 28 Apr 2026 15:05:23 +1000
Message-ID: <review-patch1-20260427-gfx-clk-fixes-v2-1-797e54b3d464@oss.qualcomm.com>
In-Reply-To: <20260427-gfx-clk-fixes-v2-1-797e54b3d464@oss.qualcomm.com>
References: <20260427-gfx-clk-fixes-v2-0-797e54b3d464@oss.qualcomm.com> <20260427-gfx-clk-fixes-v2-1-797e54b3d464@oss.qualcomm.com>
X-Mailer: Claude Code Patch Reviewer
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0

Patch Review

**Author:** Jagadeesh Kona

This patch adds `gdsc_gx_disable()` which mirrors the existing `gdsc_gx_do_nothing_enable()` pattern. The logic is:
- If `synced_poweroff` is set (recovery), allow the real `gdsc_disable()` path.
- Otherwise, only disable the regulator supply and skip HW toggling.

```c
+int gdsc_gx_disable(struct generic_pm_domain *domain)
+{
+	struct gdsc *sc = domain_to_gdsc(domain);
+
+	if (domain->synced_poweroff)
+		return gdsc_disable(domain);
+
+	/* Remove parent-supply placed in enable */
+	if (sc->rsupply)
+		return regulator_disable(sc->rsupply);
+
+	return 0;
+}
```

This looks correct and symmetric with `gdsc_gx_do_nothing_enable()`, which does `regulator_enable(sc->rsupply)`. The `synced_poweroff` flag is a clean GenPD mechanism for communicating the recovery intent.

No issues found.

---

---
Generated by Claude Code Patch Reviewer