[PATCH v2 0/2] clk: qcom: gdsc: add MSM8x60 LEGACY_FOOTSWITCH + RPM_ALWAYS_ON support

[PATCH v2 0/2] clk: qcom: gdsc: add MSM8x60 LEGACY_FOOTSWITCH + RPM_ALWAYS_ON support

[Herman van Hazendonk]

The MSM8x60 family (MSM8260 / MSM8660 / APQ8060) uses a pre-modern
"footswitch" power-domain block (GFS) for its multimedia GDSCs that
predates the modern qcom GDSC programming model. The legacy block has
a single-register layout (ENABLE / CLAMP / RETENTION at fixed bits in
the GDSCR) instead of the modern split GDSCR / CFG_GDSCR pair, no
power-status bit (state has to be inferred from ENABLE + CLAMP), and a
fixed-delay handoff sequence with no completion register to poll.

Patch 1 adds a LEGACY_FOOTSWITCH flag plus the supporting helpers
(legacy enable / disable sequencing, gdsc_check_status fallback,
legacy clamp helpers). Patch 2 adds an RPM_ALWAYS_ON flag wiring the
PWRSTS_ON ALWAYS_ON paths to GENPD_FLAG_RPM_ALWAYS_ON; this is needed
because some MSM8x60 GDSCs are managed by RPM firmware and the kernel
must keep its genpd vote alive across runtime PM transitions.

v2 folds 2 High Sashiko findings on the v1 patchset, both vote-balance
bugs in gdsc_init() (see below). Both folds verified on HW; no
regulator-imbalance WARNs in dmesg post-fix.

Signed-off-by: Herman van Hazendonk <github.com@herrie.org>
---
Changes in v2:

Both Sashiko findings folded:

  - [High] gdsc_init() PWRSTS_ON + LEGACY_FOOTSWITCH + initial_on
    triple condition: the PWRSTS_ON block calls gdsc_enable() above,
    which on the legacy path takes a regulator_enable() vote
    unconditionally. The downstream "sync" block at line ~737 then
    voted *again* because initial_on=true, while gdsc_disable() only
    unwinds the vote once -> permanent +1 ref-count imbalance per
    probe. Skip the sync vote when
    (pwrsts == PWRSTS_ON && LEGACY_FOOTSWITCH).
    Folded into patch 1 (LEGACY_FOOTSWITCH support).

  - [High] gdsc_init() ALWAYS_ON / RPM_ALWAYS_ON block: the
    unconditional gdsc_enable() call ignored its return value and
    unconditionally set on=true. If gdsc_enable() failed (e.g. an
    internal regulator_enable failure) the subsequent
    pm_genpd_init() error path would call regulator_disable() for a
    vote that was never actually taken -> regulator ref-count
    underflow and vote leak. Propagate the gdsc_enable() return
    immediately. Folded into patch 2 (RPM_ALWAYS_ON flag).

Test results (v2, HP TouchPad APQ8060, kernel 7.1.0-rc1):

  - Boot clean. All MSM8x60 GDSCs probe and reach their expected
    initial state: vfe / ved / rot / mdp / gfx3d on (bootloader
    handoff); ijpeg / vpe / gfx2d0 / gfx2d1 off-0 (kernel-managed).

  - Camera stream stress (20x rapid pix640 + pix1280 + 60s
    unbind/rebind race on mt9m113 -> exercises vfe domain off/on
    cycles + ved bringup): zero regulator-imbalance WARNs, zero
    "Unbalanced disable" messages from regulator/core.c, zero
    clk-refcount underflow WARNs.

  - 120s concurrent multi-subsystem load (camera stream loop + dd
    to eMMC + /dev/urandom + filesystem walk): all GDSCs stay in
    expected state, no genpd warnings, USB stayed up, MDP stayed
    active. Driver still bound at the end.

  - dmesg post-load: 0 WARN, 0 BUG, 0 Oops from gdsc /
    regulator / clk frameworks. The only routine "CSIPHY1:
    usleep 10-15ms for GDSC stabilize" info lines (camss
    pre-existing chatter, not gdsc-related) are visible.

  - Kernel build clean (ARCH=arm, CONFIG_QCOM_GDSC=y,
    CONFIG_QCOM_CLK_RPM=y, CONFIG_MSM_MMCC_8660=y).

- Link to v1: https://lore.kernel.org/r/20260602050840.435933-1-github.com@herrie.org

---
Herman van Hazendonk (2):
      clk: qcom: gdsc: add LEGACY_FOOTSWITCH support for MSM8x60
      clk: qcom: gdsc: add RPM_ALWAYS_ON flag

 drivers/clk/qcom/gdsc.c | 344 ++++++++++++++++++++++++++++++++++++++++++++++--
 drivers/clk/qcom/gdsc.h |  19 +++
 2 files changed, 351 insertions(+), 12 deletions(-)
---
base-commit: 944125b4c454b58d2fe6e35f1087a932b2050dff
change-id: 20260606-submit-clk-gdsc-msm8x60-legacy-1e8d4b336eeb

Best regards,
-- 
Herman van Hazendonk <github.com@herrie.org>

[PATCH v2 1/2] clk: qcom: gdsc: add LEGACY_FOOTSWITCH support for MSM8x60

[Herman van Hazendonk]

The MSM8x60 family (MSM8260, MSM8660, APQ8060) ships an older
footswitch (FS / "GFS") block that pre-dates the GDSC programming
model the existing driver was designed around. Adding GDSC entries
for that family's MMCC power domains needs the driver to understand
the legacy register layout:

  - the CLAMP, ENABLE and RETENTION bits live in the main GDSCR
    register rather than in a separate clamp_io_ctrl;
  - there is no power-status bit, so software cannot poll for the
    transition completing and has to gate progress on a fixed
    udelay() after toggling ENABLE;
  - ENABLE is positive-logic (set to power up, clear to collapse)
    rather than the modern inverted SW_COLLAPSE semantics;
  - none of the modern wait-time / HW-trigger / SW-override fields
    are present, so gdsc_init() must skip the wait-config
    programming block entirely.

Introduce a LEGACY_FOOTSWITCH flag and the matching code paths in
gdsc_check_status(), gdsc_update_collapse_bit(), gdsc_enable(),
gdsc_disable() and gdsc_init(). The enable / disable sequences
mirror what the downstream vendor footswitch driver did on these
SoCs, with the regulator hand-off normally done by
gdsc_toggle_logic() spliced in at the matching points so a legacy
domain that has a parent regulator still gets voted up/down:

  enable:  regulator_enable -> assert resets -> set ENABLE ->
           2us settle -> deassert resets -> clear CLAMP ->
           5us settle
  disable: assert resets -> set CLAMP -> clear ENABLE ->
           regulator_disable

Every register-write failure inside the sequence walks the partial
state back to a defined endpoint before returning the error, so a
mid-sequence failure cannot strand the block in "rail powered but
clamped + in reset" (or the symmetric "clamped + reset asserted +
ENABLE still set"). The two pwrsts == PWRSTS_ON short-circuits at
the top of gdsc_enable() / gdsc_disable() are gated on
!(sc->flags & LEGACY_FOOTSWITCH) for the same reason: a legacy
footswitch declared PWRSTS_ON still has a real rail and clamp,
which the reset-only fast path would silently skip. gdsc_init()'s
PWRSTS_ON force-enable goes through gdsc_enable() for legacy
domains for the same reason.

In gdsc_init(), clear the RETENTION bit (BIT 9) before jumping to
the common state-sync block. The vendor MSM8x60 footswitch driver
does the same one-shot clear at probe for every footswitch; without
it the reset-default value is unspecified per board and a stuck-set
retention bit would leave the rail draining power while looking
collapsed in software.

This patch only adds the infrastructure; the MSM8x60 MMCC driver
that consumes it lands in a follow-up series.

Assisted-by: Claude:claude-opus-4-7 Sashiko:claude-haiku-4-5
Signed-off-by: Herman van Hazendonk <github.com@herrie.org>
---
 drivers/clk/qcom/gdsc.c | 323 ++++++++++++++++++++++++++++++++++++++++++++++--
 drivers/clk/qcom/gdsc.h |   7 ++
 2 files changed, 321 insertions(+), 9 deletions(-)

diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c
index 95aa07120245..1f80bebe88c7 100644
--- a/drivers/clk/qcom/gdsc.c
+++ b/drivers/clk/qcom/gdsc.c
@@ -27,6 +27,11 @@
 #define GMEM_CLAMP_IO_MASK	BIT(0)
 #define GMEM_RESET_MASK		BIT(4)
 
+/* Legacy MSM8x60 footswitch register bits (single register layout) */
+#define LEGACY_FS_CLAMP_MASK		BIT(5)
+#define LEGACY_FS_ENABLE_MASK		BIT(8)
+#define LEGACY_FS_RETENTION_MASK	BIT(9)
+
 /* CFG_GDSCR */
 #define GDSC_POWER_UP_COMPLETE		BIT(16)
 #define GDSC_POWER_DOWN_COMPLETE	BIT(15)
@@ -63,6 +68,32 @@ static int gdsc_check_status(struct gdsc *sc, enum gdsc_status status)
 	u32 val;
 	int ret;
 
+	/*
+	 * Legacy footswitches have no power-status bit: software has to
+	 * infer the state from the ENABLE bit it just wrote.
+	 */
+	if (sc->flags & LEGACY_FOOTSWITCH) {
+		ret = regmap_read(sc->regmap, sc->gdscr, &val);
+		if (ret)
+			return ret;
+		/*
+		 * A block with ENABLE=1 but CLAMP=1 is electrically isolated:
+		 * the rail is powered but all I/O is clamped. The downstream
+		 * vendor footswitch driver (footswitch-8x60.c) treats the block
+		 * as "ON" only when ENABLE is set AND CLAMP is clear -- mirror
+		 * that convention so callers don't mistake a clamped-but-
+		 * powered block for a fully usable one.
+		 */
+		switch (status) {
+		case GDSC_ON:
+			return (val & (LEGACY_FS_ENABLE_MASK | LEGACY_FS_CLAMP_MASK))
+			       == LEGACY_FS_ENABLE_MASK;
+		case GDSC_OFF:
+			return !(val & LEGACY_FS_ENABLE_MASK);
+		}
+		return -EINVAL;
+	}
+
 	if (sc->flags & POLL_CFG_GDSCR)
 		reg = sc->gdscr + CFG_GDSCR_OFFSET;
 	else if (sc->gds_hw_ctrl)
@@ -121,6 +152,18 @@ static int gdsc_update_collapse_bit(struct gdsc *sc, bool val)
 	u32 reg, mask;
 	int ret;
 
+	/*
+	 * Legacy footswitches do not have an inverted SW_COLLAPSE bit;
+	 * instead the same bit means ENABLE: clear to disable the rail,
+	 * set to enable it. Invert the caller's "collapse" intent.
+	 */
+	if (sc->flags & LEGACY_FOOTSWITCH) {
+		reg = sc->gdscr;
+		mask = LEGACY_FS_ENABLE_MASK;
+		return regmap_update_bits(sc->regmap, reg, mask,
+					  val ? 0 : mask);
+	}
+
 	if (sc->collapse_mask) {
 		reg = sc->collapse_ctrl;
 		mask = sc->collapse_mask;
@@ -240,6 +283,23 @@ static inline void gdsc_assert_clamp_io(struct gdsc *sc)
 			   GMEM_CLAMP_IO_MASK, 1);
 }
 
+/*
+ * Legacy MSM8x60 footswitches keep the I/O clamp bit in the main GDSCR
+ * (no separate clamp_io_ctrl register), so the helpers here use sc->gdscr.
+ */
+static inline int legacy_fs_deassert_clamp(struct gdsc *sc)
+{
+	return regmap_update_bits(sc->regmap, sc->gdscr,
+				  LEGACY_FS_CLAMP_MASK, 0);
+}
+
+static inline int legacy_fs_assert_clamp(struct gdsc *sc)
+{
+	return regmap_update_bits(sc->regmap, sc->gdscr,
+				  LEGACY_FS_CLAMP_MASK,
+				  LEGACY_FS_CLAMP_MASK);
+}
+
 static inline void gdsc_assert_reset_aon(struct gdsc *sc)
 {
 	regmap_update_bits(sc->regmap, sc->clamp_io_ctrl,
@@ -259,11 +319,121 @@ static void gdsc_retain_ff_on(struct gdsc *sc)
 static int gdsc_enable(struct generic_pm_domain *domain)
 {
 	struct gdsc *sc = domain_to_gdsc(domain);
-	int ret;
+	int ret, rc;
 
-	if (sc->pwrsts == PWRSTS_ON)
+	/*
+	 * Modern PWRSTS_ON-only GDSCs are pure reset-controllers: there
+	 * is no rail to bring up so only the reset deassert is needed.
+	 * Legacy footswitches always need the full power-up + clamp-
+	 * release sequence below, even when declared PWRSTS_ON, so they
+	 * must not take this short-circuit.
+	 */
+	if (sc->pwrsts == PWRSTS_ON && !(sc->flags & LEGACY_FOOTSWITCH))
 		return gdsc_deassert_reset(sc);
 
+	/*
+	 * Legacy MSM8x60 footswitch enable sequence:
+	 *   0. enable the parent regulator supply (if any)
+	 *   1. assert per-block resets (if SW_RESET)
+	 *   2. set ENABLE in GDSCR to power up the rail
+	 *   3. wait 2us for the rail to fully charge
+	 *   4. deassert resets
+	 *   5. clear CLAMP in GDSCR to release the I/O clamp
+	 *   6. wait 5us for clamps to release and signals to settle
+	 *
+	 * No status-bit polling -- the hardware does not expose one, so
+	 * the fixed delays below are the only safe synchronisation point.
+	 */
+	if (sc->flags & LEGACY_FOOTSWITCH) {
+		if (sc->rsupply) {
+			ret = regulator_enable(sc->rsupply);
+			if (ret < 0)
+				return ret;
+		}
+
+		if (sc->flags & SW_RESET) {
+			gdsc_assert_reset(sc);
+			/*
+			 * Wait for synchronous resets to propagate before
+			 * raising ENABLE: matches footswitch-8x60.c's
+			 * udelay(RESET_DELAY_US) between assert and enable.
+			 */
+			udelay(1);
+		}
+
+		ret = gdsc_update_collapse_bit(sc, false);
+		if (ret) {
+			/*
+			 * Power-up write failed -- release the reset we
+			 * just asserted so the block does not stay stuck
+			 * in reset for the rest of the system's lifetime,
+			 * and roll back the regulator vote we just took.
+			 */
+			if (sc->flags & SW_RESET)
+				gdsc_deassert_reset(sc);
+			if (sc->rsupply)
+				regulator_disable(sc->rsupply);
+			return ret;
+		}
+
+		udelay(2);
+
+		/*
+		 * Release the I/O clamp BEFORE deasserting resets: the
+		 * downstream vendor footswitch driver (footswitch-8x60.c)
+		 * always clears CLAMP_BIT first, then deasserts per-block
+		 * resets. This lets the block's outputs settle in a known
+		 * reset state before they become visible to consumers.
+		 */
+		ret = legacy_fs_deassert_clamp(sc);
+		if (ret) {
+			/*
+			 * Rail is already powered up; if we cannot release
+			 * the I/O clamp, collapse the rail again to avoid
+			 * leaving the block live but isolated, re-assert
+			 * the reset so the block ends in a defined
+			 * power-off state, and undo the regulator vote.
+			 * Errors from these best-effort rollback steps are
+			 * reported but do not override the original error
+			 * returned to the caller -- the secondary failure
+			 * means the hardware state is already indeterminate
+			 * and the regulator vote must still be released.
+			 */
+			rc = gdsc_update_collapse_bit(sc, true);
+			if (rc) {
+				/*
+				 * Collapse also failed: the rail is still ON.
+				 * Do NOT call regulator_disable() -- the rail
+				 * is still drawing from the supply and cutting
+				 * it while ENABLE is set risks hardware damage.
+				 * Mirror gdsc_disable()'s collapse-failure path
+				 * which deliberately skips regulator_disable()
+				 * when the rail did not collapse.
+				 */
+				pr_err("%s: rail collapse rollback failed (%d) after clamp release failure (%d); rail may be ON, regulator vote leaked\n",
+				       sc->pd.name, rc, ret);
+			} else {
+				if (sc->flags & SW_RESET)
+					gdsc_assert_reset(sc);
+				if (sc->rsupply) {
+					rc = regulator_disable(sc->rsupply);
+					if (rc)
+						pr_err("%s: regulator_disable failed (%d) in clamp-release rollback\n",
+						       sc->pd.name, rc);
+				}
+			}
+			return ret;
+		}
+
+		/* Deassert resets now that clamp is released (vendor order). */
+		if (sc->flags & SW_RESET)
+			gdsc_deassert_reset(sc);
+
+		udelay(5);
+
+		return 0;
+	}
+
 	if (sc->flags & SW_RESET) {
 		gdsc_assert_reset(sc);
 		udelay(1);
@@ -317,11 +487,91 @@ static int gdsc_enable(struct generic_pm_domain *domain)
 static int gdsc_disable(struct generic_pm_domain *domain)
 {
 	struct gdsc *sc = domain_to_gdsc(domain);
-	int ret;
+	int ret, rc;
 
-	if (sc->pwrsts == PWRSTS_ON)
+	/*
+	 * Symmetric to gdsc_enable: modern PWRSTS_ON-only GDSCs only
+	 * need a reset assert, but legacy footswitches with PWRSTS_ON
+	 * still need to clamp I/O and collapse the rail explicitly so
+	 * they must not take this short-circuit.
+	 */
+	if (sc->pwrsts == PWRSTS_ON && !(sc->flags & LEGACY_FOOTSWITCH))
 		return gdsc_assert_reset(sc);
 
+	/*
+	 * Legacy MSM8x60 footswitch disable sequence:
+	 *   1. assert per-block resets (if SW_RESET)
+	 *   2. set CLAMP in GDSCR to hold I/O at safe values across collapse
+	 *   3. clear ENABLE in GDSCR to collapse the rail
+	 *   4. drop the parent regulator vote (if any)
+	 */
+	if (sc->flags & LEGACY_FOOTSWITCH) {
+		if (sc->flags & SW_RESET) {
+			gdsc_assert_reset(sc);
+			/*
+			 * Wait for synchronous resets to propagate before
+			 * clamping I/O: footswitch-8x60.c udelay(RESET_DELAY_US)
+			 * between assert and CLAMP_BIT set.
+			 */
+			udelay(1);
+		}
+
+		ret = legacy_fs_assert_clamp(sc);
+		if (ret) {
+			/*
+			 * Clamp programming failed -- release the reset we
+			 * just asserted so the block is not stranded in
+			 * reset, then surface the error.
+			 */
+			if (sc->flags & SW_RESET)
+				gdsc_deassert_reset(sc);
+			return ret;
+		}
+
+		ret = gdsc_update_collapse_bit(sc, true);
+		if (ret) {
+			/*
+			 * Collapse failed -- the rail is still ON. Walk
+			 * back the clamp and reset so the block returns
+			 * to its enabled state rather than being stranded
+			 * in the half-disabled "clamped + reset + on"
+			 * state; the regulator vote stays in place because
+			 * the rail is still drawing from it. A secondary
+			 * failure of the clamp release is reported but
+			 * cannot override the original error: the rail is
+			 * still ON, so the caller's view ("disable failed,
+			 * leave ON") is the correct outcome regardless.
+			 */
+			rc = legacy_fs_deassert_clamp(sc);
+			if (rc)
+				pr_err("%s: clamp release rollback failed (%d) after rail collapse failure (%d); hw may be clamped+ON\n",
+				       sc->pd.name, rc, ret);
+			if (sc->flags & SW_RESET)
+				gdsc_deassert_reset(sc);
+			return ret;
+		}
+
+		if (sc->rsupply) {
+			ret = regulator_disable(sc->rsupply);
+			if (ret < 0) {
+				/*
+				 * The rail is already collapsed. Reporting
+				 * the regulator error to genpd would leave it
+				 * thinking the domain is still ON when the
+				 * silicon is in fact off; the next consumer
+				 * enable would then be no-op'd by genpd and
+				 * hit dead hardware. Better to leak the
+				 * regulator vote (visible via /sys/.../
+				 * regulator) than to corrupt genpd state.
+				 */
+				pr_err("%s: regulator_disable failed (%d) after rail collapse; vote leaked, genpd state kept consistent with silicon\n",
+				       sc->pd.name, ret);
+			}
+		}
+
+		return 0;
+	}
+
 	/* Turn off HW trigger mode if supported */
 	if (sc->flags & HW_CTRL) {
 		ret = gdsc_hwctrl(sc, false);
@@ -403,7 +653,28 @@ static bool gdsc_get_hwmode(struct generic_pm_domain *domain, struct device *dev
 static int gdsc_init(struct gdsc *sc)
 {
 	u32 mask, val;
-	int on, ret;
+	int initial_on, on, ret;
+
+	/*
+	 * Legacy MSM8x60 footswitches share none of the modern GDSC
+	 * wait-time fields and have no HW trigger / SW override bits at
+	 * all, so skip the wait-config programming and jump straight to
+	 * the common state-sync block below.
+	 *
+	 * Clear the retention bit (BIT 9) so subsequent disable actually
+	 * power-collapses the rail rather than holding state. The vendor
+	 * MSM8x60 footswitch driver does the same one-shot clear at probe
+	 * for every footswitch; without it the reset-default value is
+	 * unspecified per board and a stuck-set retention bit would leave
+	 * the rail draining power while looking collapsed in software.
+	 */
+	if (sc->flags & LEGACY_FOOTSWITCH) {
+		ret = regmap_update_bits(sc->regmap, sc->gdscr,
+					 LEGACY_FS_RETENTION_MASK, 0);
+		if (ret)
+			return ret;
+		goto skip_wait_config;
+	}
 
 	/*
 	 * Disable HW trigger: collapse/restore occur based on registers writes.
@@ -428,9 +699,31 @@ static int gdsc_init(struct gdsc *sc)
 	if (ret)
 		return ret;
 
-	/* Force gdsc ON if only ON state is supported */
+skip_wait_config:
+	/*
+	 * Sample the GDSC power state BEFORE any probe-time enable below
+	 * so the "sync the kernel state" regulator vote only runs when the
+	 * GDSC was already on at probe (bootloader handoff). For the
+	 * PWRSTS_ON / ALWAYS_ON force-enable paths, gdsc_enable() and
+	 * gdsc_toggle_logic() take the vote themselves -- re-voting from
+	 * the sync block would double-vote rsupply and leak a reference.
+	 */
+	initial_on = gdsc_check_status(sc, GDSC_ON);
+	if (initial_on < 0)
+		return initial_on;
+
+	/*
+	 * Force gdsc ON if only ON state is supported. For legacy
+	 * footswitches, gdsc_toggle_logic() would only flip the ENABLE
+	 * bit and skip the I/O-clamp release + settle delay that the
+	 * MSM8x60 power-up sequence requires; call gdsc_enable() instead
+	 * so the full legacy sequence runs.
+	 */
 	if (sc->pwrsts == PWRSTS_ON) {
-		ret = gdsc_toggle_logic(sc, GDSC_ON, false);
+		if (sc->flags & LEGACY_FOOTSWITCH)
+			ret = gdsc_enable(&sc->pd);
+		else
+			ret = gdsc_toggle_logic(sc, GDSC_ON, false);
 		if (ret)
 			return ret;
 	}
@@ -440,8 +733,20 @@ static int gdsc_init(struct gdsc *sc)
 		return on;
 
 	if (on) {
-		/* The regulator must be on, sync the kernel state */
-		if (sc->rsupply) {
+		/*
+		 * Sync the kernel regulator state only if the GDSC was
+		 * already on at probe; if we just enabled it above, the
+		 * vote was taken inside gdsc_enable() / gdsc_toggle_logic().
+		 *
+		 * Special case: PWRSTS_ON + LEGACY_FOOTSWITCH always routes
+		 * through gdsc_enable() above (lines around the PWRSTS_ON
+		 * block), which calls regulator_enable() unconditionally on
+		 * the legacy path. Skip the sync vote in that case to avoid
+		 * a double-vote that gdsc_disable() only unwinds once.
+		 */
+		if (sc->rsupply && initial_on &&
+		    !(sc->pwrsts == PWRSTS_ON &&
+		      (sc->flags & LEGACY_FOOTSWITCH))) {
 			ret = regulator_enable(sc->rsupply);
 			if (ret < 0)
 				return ret;
diff --git a/drivers/clk/qcom/gdsc.h b/drivers/clk/qcom/gdsc.h
index dd843e86c05b..13ca09f93a01 100644
--- a/drivers/clk/qcom/gdsc.h
+++ b/drivers/clk/qcom/gdsc.h
@@ -68,6 +68,13 @@ struct gdsc {
 #define RETAIN_FF_ENABLE	BIT(7)
 #define NO_RET_PERIPH	BIT(8)
 #define HW_CTRL_TRIGGER	BIT(9)
+/*
+ * Legacy MSM8x60-family footswitch (a.k.a. "GFS"). Different register layout
+ * from the modern GDSC blocks: CLAMP at bit 5, ENABLE at bit 8, RETENTION at
+ * bit 9, and there is no power-status bit so software has to assume the
+ * transition completed after a fixed delay rather than polling status.
+ */
+#define LEGACY_FOOTSWITCH	BIT(10)
 	struct reset_controller_dev	*rcdev;
 	unsigned int			*resets;
 	unsigned int			reset_count;

-- 
2.43.0

[PATCH v2 2/2] clk: qcom: gdsc: add RPM_ALWAYS_ON flag

[Herman van Hazendonk]

Some power domains need to stay powered across runtime PM even though
their clocks may still gate, and only collapse on full system suspend.
Add an RPM_ALWAYS_ON flag that maps to the existing
GENPD_FLAG_RPM_ALWAYS_ON on the underlying generic_pm_domain.

This is distinct from the existing ALWAYS_ON flag (which keeps the
domain permanently enabled and prevents collapse even during system
suspend) and from leaving the flag unset (which allows the domain to
collapse on every runtime-idle transition).

GENPD_FLAG_RPM_ALWAYS_ON, like GENPD_FLAG_ALWAYS_ON, requires the
underlying genpd to be in the ON state at pm_genpd_init() time --
the framework rejects registration otherwise. Fold RPM_ALWAYS_ON
into the gdsc_init() block that already force-enables ALWAYS_ON
GDSCs found powered down at sync time so the flag combination is
honoured consistently.

The first user is the upcoming MSM8x60 MMCC driver, which needs
RPM_ALWAYS_ON on the a2xx (Adreno 220) GFX3D footswitch: cold-cycling
the GPU rail on every runtime idle forces an a2xx_hw_init microcode
reload whose MMIO burst can stall the shared MMSS AXI fabric when it
coincides with an MDP display client-switch underrun, hard-hanging
the SoC. Letting the rail stay up during runtime PM (clocks still
gate, idle power is still saved) and only collapsing on system
suspend avoids the corner case while still allowing full power-down
during deep sleep.

Assisted-by: Claude:claude-opus-4-7 Sashiko:claude-haiku-4-5
Signed-off-by: Herman van Hazendonk <github.com@herrie.org>
---
 drivers/clk/qcom/gdsc.c | 21 ++++++++++++++++++---
 drivers/clk/qcom/gdsc.h | 12 ++++++++++++
 2 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c
index 1f80bebe88c7..0622bb23d17b 100644
--- a/drivers/clk/qcom/gdsc.c
+++ b/drivers/clk/qcom/gdsc.c
@@ -777,9 +777,22 @@ static int gdsc_init(struct gdsc *sc)
 				goto err_disable_supply;
 		}
 
-	} else if (sc->flags & ALWAYS_ON) {
-		/* If ALWAYS_ON GDSCs are not ON, turn them ON */
-		gdsc_enable(&sc->pd);
+	} else if (sc->flags & (ALWAYS_ON | RPM_ALWAYS_ON)) {
+		/*
+		 * Both GENPD_FLAG_ALWAYS_ON and GENPD_FLAG_RPM_ALWAYS_ON
+		 * require the domain to be ON at pm_genpd_init() time --
+		 * the framework rejects registration otherwise. Bring up
+		 * any such GDSC that is currently off so the genpd flags
+		 * we set below match the silicon state.
+		 *
+		 * Propagate the gdsc_enable() return so a failure here does
+		 * not silently set on=true and leak a vote through the
+		 * err_disable_supply path (which would unwind a vote that
+		 * was never actually taken).
+		 */
+		ret = gdsc_enable(&sc->pd);
+		if (ret)
+			return ret;
 		on = true;
 	}
 
@@ -790,6 +803,8 @@ static int gdsc_init(struct gdsc *sc)
 
 	if (sc->flags & ALWAYS_ON)
 		sc->pd.flags |= GENPD_FLAG_ALWAYS_ON;
+	if (sc->flags & RPM_ALWAYS_ON)
+		sc->pd.flags |= GENPD_FLAG_RPM_ALWAYS_ON;
 	if (!sc->pd.power_off)
 		sc->pd.power_off = gdsc_disable;
 	if (!sc->pd.power_on)
diff --git a/drivers/clk/qcom/gdsc.h b/drivers/clk/qcom/gdsc.h
index 13ca09f93a01..27acf20e8d68 100644
--- a/drivers/clk/qcom/gdsc.h
+++ b/drivers/clk/qcom/gdsc.h
@@ -75,6 +75,18 @@ struct gdsc {
  * transition completed after a fixed delay rather than polling status.
  */
 #define LEGACY_FOOTSWITCH	BIT(10)
+/*
+ * Keep the domain powered across runtime PM (its clocks may still gate via
+ * the clock framework) and only allow it to power-collapse on system
+ * suspend. Maps to GENPD_FLAG_RPM_ALWAYS_ON on the underlying genpd. Useful
+ * for blocks whose cold-start sequence is expensive enough that runtime
+ * power cycling causes user-visible latency or hardware corner-case bugs --
+ * e.g. the MSM8x60 a2xx (Adreno 220) graphics footswitch, whose first
+ * power-up after collapse forces a full microcode reload that can stall
+ * the shared MMSS AXI fabric when it coincides with an MDP display
+ * underrun.
+ */
+#define RPM_ALWAYS_ON	BIT(11)
 	struct reset_controller_dev	*rcdev;
 	unsigned int			*resets;
 	unsigned int			reset_count;

-- 
2.43.0

Re: [PATCH v2 1/2] clk: qcom: gdsc: add LEGACY_FOOTSWITCH support for MSM8x60

[Dmitry Baryshkov]

On Sat, Jun 06, 2026 at 02:34:26PM +0200, Herman van Hazendonk wrote:
> The MSM8x60 family (MSM8260, MSM8660, APQ8060) ships an older
> footswitch (FS / "GFS") block that pre-dates the GDSC programming
> model the existing driver was designed around. Adding GDSC entries
> for that family's MMCC power domains needs the driver to understand
> the legacy register layout:
> 
>   - the CLAMP, ENABLE and RETENTION bits live in the main GDSCR
>     register rather than in a separate clamp_io_ctrl;
>   - there is no power-status bit, so software cannot poll for the
>     transition completing and has to gate progress on a fixed
>     udelay() after toggling ENABLE;
>   - ENABLE is positive-logic (set to power up, clear to collapse)
>     rather than the modern inverted SW_COLLAPSE semantics;
>   - none of the modern wait-time / HW-trigger / SW-override fields
>     are present, so gdsc_init() must skip the wait-config
>     programming block entirely.

I think there are enough differences to mandate a separate footswitch.c
file. It can further be used by MMCC on MSM8960 and APQ8064 platforms.
It might result by some code duplication, but I think in the end it
would be easier to read (and it won't clobber the existing gdsc.c code).
Add footswitch_register() / unregister() functions and call them from
the common code. Make sure they are replaced with stubs if
CONFIG_QCOM_FOOTSWITCH is not selected.

> 
> Introduce a LEGACY_FOOTSWITCH flag and the matching code paths in
> gdsc_check_status(), gdsc_update_collapse_bit(), gdsc_enable(),
> gdsc_disable() and gdsc_init(). The enable / disable sequences
> mirror what the downstream vendor footswitch driver did on these
> SoCs, with the regulator hand-off normally done by
> gdsc_toggle_logic() spliced in at the matching points so a legacy
> domain that has a parent regulator still gets voted up/down:

-- 
With best wishes
Dmitry

Re: [PATCH v2 1/2] clk: qcom: gdsc: add LEGACY_FOOTSWITCH support for MSM8x60

[Konrad Dybcio]

On 6/6/26 5:17 PM, Dmitry Baryshkov wrote:
> On Sat, Jun 06, 2026 at 02:34:26PM +0200, Herman van Hazendonk wrote:
>> The MSM8x60 family (MSM8260, MSM8660, APQ8060) ships an older
>> footswitch (FS / "GFS") block that pre-dates the GDSC programming
>> model the existing driver was designed around. Adding GDSC entries
>> for that family's MMCC power domains needs the driver to understand
>> the legacy register layout:
>>
>>   - the CLAMP, ENABLE and RETENTION bits live in the main GDSCR
>>     register rather than in a separate clamp_io_ctrl;
>>   - there is no power-status bit, so software cannot poll for the
>>     transition completing and has to gate progress on a fixed
>>     udelay() after toggling ENABLE;
>>   - ENABLE is positive-logic (set to power up, clear to collapse)
>>     rather than the modern inverted SW_COLLAPSE semantics;
>>   - none of the modern wait-time / HW-trigger / SW-override fields
>>     are present, so gdsc_init() must skip the wait-config
>>     programming block entirely.
> 
> I think there are enough differences to mandate a separate footswitch.c
> file. It can further be used by MMCC on MSM8960 and APQ8064 platforms.
> It might result by some code duplication, but I think in the end it
> would be easier to read (and it won't clobber the existing gdsc.c code).

Yes please, gdsc.c is already a bit convoluted..

Konrad

Re: [PATCH v2 2/2] clk: qcom: gdsc: add RPM_ALWAYS_ON flag

[Konrad Dybcio]

On 6/6/26 2:34 PM, Herman van Hazendonk wrote:
> Some power domains need to stay powered across runtime PM even though
> their clocks may still gate, and only collapse on full system suspend.
> Add an RPM_ALWAYS_ON flag that maps to the existing
> GENPD_FLAG_RPM_ALWAYS_ON on the underlying generic_pm_domain.
> 
> This is distinct from the existing ALWAYS_ON flag (which keeps the
> domain permanently enabled and prevents collapse even during system
> suspend) and from leaving the flag unset (which allows the domain to
> collapse on every runtime-idle transition).
> 
> GENPD_FLAG_RPM_ALWAYS_ON, like GENPD_FLAG_ALWAYS_ON, requires the
> underlying genpd to be in the ON state at pm_genpd_init() time --
> the framework rejects registration otherwise. Fold RPM_ALWAYS_ON
> into the gdsc_init() block that already force-enables ALWAYS_ON
> GDSCs found powered down at sync time so the flag combination is
> honoured consistently.
> 
> The first user is the upcoming MSM8x60 MMCC driver, which needs
> RPM_ALWAYS_ON on the a2xx (Adreno 220) GFX3D footswitch: cold-cycling
> the GPU rail on every runtime idle forces an a2xx_hw_init microcode
> reload whose MMIO burst can stall the shared MMSS AXI fabric when it
> coincides with an MDP display client-switch underrun, hard-hanging
> the SoC. Letting the rail stay up during runtime PM (clocks still
> gate, idle power is still saved) and only collapsing on system
> suspend avoids the corner case while still allowing full power-down
> during deep sleep.

Is this something that downstream works around, or is that observed
behavior with upstream?

Konrad