[RFC PATCH] pinctrl: aspeed: Handle GPIOZ6 pass-through strap on GPIO requests

[RFC PATCH] pinctrl: aspeed: Handle GPIOZ6 pass-through strap on GPIO requests

[Prasanth Kumar Padarthi]

On the ASRock Rack B650D4U, requesting GPIOP3 through gpiolib does
not provide working host power control when the GPIOZ6 pass-through
strap in SCU51C[9] is set.

With the existing GPIO request path, SCU4BC reads zero, but a host
power-on request through x86-power-control times out waiting for
power-good:

  SCU4BC = 0x00000000
  SCU510 = 0x00000000
  SCU51C = 0x00001e80

The AST2600 A3 datasheet describes SCU51C[9] as an additional
pass-through enable for GPIOP0/P1, GPIOP2/P3 and GPIOP4/P5 when
SCU510[30] is clear.

A diagnostic experiment that cleared only SCU51C[9], preserving all
other bits, allowed the same GPIO-only power-control implementation
to power on the host:

  SCU51C: 0x00001e80 -> 0x00001c80

SCU4BC and SCU510 remained zero. No per-operation THRU1 switching
or additional userspace delay was required.

Add an AST2600-specific GPIO request callback that performs the
existing GPIO mux configuration and clears SCU51C[9] for GPIOP0-P5
when SCU510[30] is clear. Check register access errors and verify
that the strap bit cleared. Other GPIOs retain the existing path.

This RFC handles only the GPIOZ6 strap source. It leaves SCU51C
unchanged when SCU510[30] is set and returns -EOPNOTSUPP for
GPIOP0-P5 requests when SCU510[28] is set.

Hardware testing on B650D4U with this pinctrl candidate confirmed:

  - Remote power-on: power-good asserted.
  - Remote graceful power-off: power-good deasserted.
  - Physical power-button power-on.
  - Physical power-button shutdown.

These tests used x86-power-control with software button forwarding.
The earlier board-specific SoC driver, its DT node, custom sysfs
interface and diagnostic boot parameter were not used.

This replaces the approach explored in the withdrawn B650D4U
power-passthrough RFC. The new proposal changes the existing pinctrl
driver and introduces no DT binding or userspace interface.

Feedback is requested on two unresolved issues:

  1. SCU51C[9] is shared by all three GPIOP pass-through pairs.
     Clearing it for one GPIO request may affect forwarding on
     another pair. How should that shared control be represented
     and coordinated with existing mux ownership?

  2. The SCU510[28] enable path has not been validated. Returning
     -EOPNOTSUPP is a limitation of this candidate, not a proposed
     complete solution for every AST2600 configuration.

Is the GPIO request callback the appropriate place to handle this
strap interaction, or should it be modeled through the existing
signal-expression mechanism?


Signed-off-by: Prasanth Kumar Padarthi <prasanth.padarthi10@gmail.com>
---
diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c b/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
--- a/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
+++ b/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
@@ -40,6 +40,7 @@
 #define SCU4D8		0x4D8 /* Multi-function Pin Control #23 */
 #define SCU500		0x500 /* Hardware Strap 1 */
 #define SCU510		0x510 /* Hardware Strap 2 */
+#define SCU51C		0x51c /* Hardware Strap 3 */
 #define SCU610		0x610 /* Disable GPIO Internal Pull-Down #0 */
 #define SCU614		0x614 /* Disable GPIO Internal Pull-Down #1 */
 #define SCU618		0x618 /* Disable GPIO Internal Pull-Down #2 */
@@ -2747,12 +2748,56 @@
 	.nconfmaps = ARRAY_SIZE(aspeed_g6_pin_config_map),
 };
 
+/*
+ * GPIOP0-P5 have a global strap-controlled pass-through enable in addition
+ * to their individual SCU4BC mux bits. SCU51C is ordinary RW, unlike the
+ * write-one-set SCU510 register. This candidate handles the GPIOZ6 strap
+ * source only; the SCU510[28] enable path needs separate validation.
+ */
+static int aspeed_g6_gpio_request_enable(struct pinctrl_dev *pctldev,
+					 struct pinctrl_gpio_range *range,
+					 unsigned int offset)
+{
+	struct aspeed_pinctrl_data *pdata = pinctrl_dev_get_drvdata(pctldev);
+	struct regmap *scu = pdata->pinmux.maps[ASPEED_IP_SCU];
+	unsigned int strap2, strap3;
+	int ret;
+
+	if (offset < AB22 || offset > AB23)
+		return aspeed_gpio_request_enable(pctldev, range, offset);
+
+	ret = regmap_read(scu, SCU510, &strap2);
+	if (ret)
+		return ret;
+	if (strap2 & BIT(28))
+		return -EOPNOTSUPP;
+
+	ret = aspeed_gpio_request_enable(pctldev, range, offset);
+	if (ret)
+		return ret;
+
+	/* In dedicated-strap mode, SCU51C[9] does not enable pass-through. */
+	if (strap2 & BIT(30))
+		return 0;
+
+	ret = regmap_update_bits(scu, SCU51C, BIT(9), 0);
+	if (ret)
+		return ret;
+	ret = regmap_read(scu, SCU51C, &strap3);
+	if (ret)
+		return ret;
+	if (strap3 & BIT(9))
+		return -EPERM;
+
+	return 0;
+}
+
 static const struct pinmux_ops aspeed_g6_pinmux_ops = {
 	.get_functions_count = aspeed_pinmux_get_fn_count,
 	.get_function_name = aspeed_pinmux_get_fn_name,
 	.get_function_groups = aspeed_pinmux_get_fn_groups,
 	.set_mux = aspeed_pinmux_set_mux,
-	.gpio_request_enable = aspeed_gpio_request_enable,
+	.gpio_request_enable = aspeed_g6_gpio_request_enable,
 	.strict = true,
 };
 

[RFC PATCH v2 0/1] AST2600 global GPIOP passthrough handover experiment

[Prasanth Kumar Padarthi]

Hi,

This RFC v2 explores a different implementation from the October 15
GPIO-request callback. It is not presented as a completed generic fix.

The intended software model remains ordinary GPIO power control with
x86-power-control software button forwarding. No board-specific sysfs ABI,
Devicetree binding, or per-operation userspace SCU switching is introduced.

Changes from v1:
- Move shared-enable handling before pinctrl registration.
- Preserve global THRU selections in per-pin SCU4BC controls first.
- Handle the SCU510[28] source using SCU514 rather than rejecting it outright.
- If handover fails, block affected GPIO/mux requests while attempting to
  retain unrelated pinctrl functionality. Check all group pins before muxing.

Problem and observations
------------------------
On B650D4U, a baseline with SCU4BC=0, SCU510=0, SCU51C=0x1e80 logs
a 200 ms power pulse but times out waiting for power-good after 8 seconds.
The earlier callback candidate clears SCU51C[9], permitting power-on.
The current implementation transfers the global selection to local mux bits
before registering consumers; observed post-daemon state is:

                    Before power request       After power request
  SCU41C                 0x000000c0                 0x000000c0
  SCU4BC                 0x30000000                 0x30000000
  SCU510                 0x00000000                 0x00000000
  SCU51C                 0x00001c80                 0x00001c80

Current implementation journal excerpts (boot-relative seconds):
  182.265233 power-on request event received
  182.267277 control-power-n set to 0
  182.467530 control-power-n released
  182.496345 power OK assert event received
  182.496404 Moving to On

The logged pulse lasts 200.253 ms; power-good arrives 231.112 ms after
the request. The operator confirms physical power-on, including after
complete AC removal. The supplied timestamped trace is not separately
identified as the AC-removal run. P4/P5 local THRU selections remain set;
actual forwarding and signal continuity have not been measured.

Runtime SHA256 matches the earlier A/B tests:
  eb98e896b67c2742db5ea127a40eecc6d23659a69a635ba9e2c5495bb32bd8bb
    /usr/bin/power-control
  47d323408e754c692f9f8764f2369e8bdbe76f56c59b804f52428a6229a7b4d3
    /usr/share/x86-power-control/power-config-host0.json

The locally patched x86-power-control permits omitted optional GPIO
configuration entries. A comparison patch removes the earlier custom sysfs
passthrough switching and 300 us delay, restoring the original GPIO pulse
helper, including its masked-output handling. The configured power pulse is
200 ms; software button forwarding is enabled. This is not a logging-only
change or an unmodified upstream binary. The same executable/configuration
hashes were captured in the baseline and candidate tests. These source
changes are not intended to introduce different GPIO request flags; the
captured hashes identify the tested executables rather than independently
proving their build provenance.

The image build completed successfully. The kernel base is the local
OpenBMC revision 68da063267662da17b990c3384d5beceab1cdc80, whose
Makefile identifies Linux 6.18.24 (VERSION=6, PATCHLEVEL=18, SUBLEVEL=24).
It is not claimed to be the current pinctrl maintainer base, and equivalence
of its Aspeed pinctrl code to an upstream stable release has not been
verified. Runtime release: 6.18.24-68da063-patch-157ea07.

Open questions and validation limits
-----------------------------------
1. Is local/global THRU equivalence valid during the transfer, including
   PWM priority and signal continuity? Only steady-state selections were
   checked; no electrical measurements or mixed-pair functional tests exist.
2. Is mandatory normalization at probe acceptable, including platforms that
   never request GPIOP? Would an ownership-aware request path be preferable?
3. What shared serialization should protect the complete handover against
   other SCU writers, including concurrent driver probes? Would a common
   SCU-level locking mechanism be appropriate, and which writers must
   participate? A pinctrl-private mutex would not serialize independent
   drivers, and regmap_fields alone does not make a multi-register sequence
   atomic. Regmap locking currently covers individual accesses only.
4. How should partial handover and subsequent registration/hog failure be
   handled? Current restricted mode does not guarantee registration if a
   controller hog itself requests a blocked P0-P5 group.

Protected-write/fault paths, PWM coexistence and SCU510[28] behavior have
not been hardware tested. All presented register captures are after daemon
GPIO initialization, not a direct trace of the handover. Exact flashed-image
hashes and standalone raw evidence archive references are not yet attached.
BMC NotReady, missing BootProgress and LPC snoop failures remain separate
port issues; successful power-on is not full platform qualification.

Feedback on this implementation direction and its ownership assumptions
is requested before extending it or treating it as a generic solution.

Previous RFC Message-ID:
20261015035118.9495-1-prasanth.padarthi10@gmail.com

Regards,
Prasanth

Prasanth Kumar Padarthi (1):
  pinctrl: aspeed: Hand over global GPIOP passthrough to local mux
    controls

 drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c | 147 ++++++++++++++++++++++++++++-
 1 file changed, 143 insertions(+), 4 deletions(-)

-- 
2.47.3

[RFC PATCH v2] pinctrl: aspeed: Hand over global GPIOP passthrough to local mux controls

[Prasanth Kumar Padarthi]

On B650D4U, GPIO-only host power control fails when SCU51C[9]
selects GPIOP passthrough even after the normal GPIO request path
clears the per-pin SCU4BC selections. The THRU expressions currently
model the local controls but not the shared strap source.

Experiment with transferring globally selected GPIOP0-P5 THRU roles
to SCU4BC[29:24] before registering the controller, then clear the
active shared sources. Leave PWM selection and the strap-source
selector unchanged. Subsequent requests use the existing mux helpers.
Clear SCU510[28] through SCU514 and verify the shared enables cleared.

If handover fails, record an instance-local error and reject GPIO and
mux requests involving P0-P5 before changing their mux state, while
attempting to register the controller for unrelated pins. A P0-P5
controller hog can still cause registration to fail. Retain prepared
local selections after partial failure rather than blindly restoring
a shared enable.

This RFC explores probe-time normalization rather than the previous
request callback. Mandatory initialization policy, signal continuity,
concurrent SCU writers and post-handover registration failure remain
open design questions.

On the tested B650D4U, the new image powers on the host with unchanged
GPIO-only power-control userspace. SCU4BC remains 0x30000000 and
SCU51C remains 0x1c80 before and after the request. The operator also
reports successful power-on after complete AC removal. Actual P4/P5
forwarding, transition glitches, PWM coexistence, SCU510[28] hardware
handling and restricted failure paths have not been validated.

Signed-off-by: Prasanth Kumar Padarthi <prasanth.padarthi10@gmail.com>
---
 drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c | 147 ++++++++++++++++++++-
 1 file changed, 143 insertions(+), 4 deletions(-)

diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c b/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
index b0c7e4f6df9c..54396f2ba91b 100644
--- a/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
+++ b/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
@@ -10,6 +10,7 @@
 #include <linux/platform_device.h>
 #include <linux/pinctrl/pinctrl.h>
 #include <linux/pinctrl/pinmux.h>
+#include <linux/slab.h>
 #include <linux/string.h>
 #include <linux/types.h>
 
@@ -40,6 +41,8 @@
 #define SCU4D8		0x4D8 /* Multi-function Pin Control #23 */
 #define SCU500		0x500 /* Hardware Strap 1 */
 #define SCU510		0x510 /* Hardware Strap 2 */
+#define SCU514		0x514 /* Hardware Strap 2 Clear */
+#define SCU51C		0x51c /* Hardware Strap 3 */
 #define SCU610		0x610 /* Disable GPIO Internal Pull-Down #0 */
 #define SCU614		0x614 /* Disable GPIO Internal Pull-Down #1 */
 #define SCU618		0x618 /* Disable GPIO Internal Pull-Down #2 */
@@ -2747,12 +2750,56 @@ static struct aspeed_pinctrl_data aspeed_g6_pinctrl_data = {
 	.nconfmaps = ARRAY_SIZE(aspeed_g6_pin_config_map),
 };
 
+struct aspeed_g6_probe_data {
+	struct aspeed_pinctrl_data pdata;
+	int thru_error;
+};
+
+static bool aspeed_g6_is_thru_pin(unsigned int pin)
+{
+	return pin >= AB22 && pin <= AB23;
+}
+
+static int aspeed_g6_gpio_request_enable(struct pinctrl_dev *pctldev,
+					 struct pinctrl_gpio_range *range,
+					 unsigned int offset)
+{
+	struct aspeed_pinctrl_data *pdata = pinctrl_dev_get_drvdata(pctldev);
+	struct aspeed_g6_probe_data *g6 =
+		container_of(pdata, struct aspeed_g6_probe_data, pdata);
+
+	if (g6->thru_error && aspeed_g6_is_thru_pin(offset))
+		return g6->thru_error;
+
+	return aspeed_gpio_request_enable(pctldev, range, offset);
+}
+
+static int aspeed_g6_set_mux(struct pinctrl_dev *pctldev,
+			     unsigned int function, unsigned int group)
+{
+	struct aspeed_pinctrl_data *pdata = pinctrl_dev_get_drvdata(pctldev);
+	struct aspeed_g6_probe_data *g6 =
+		container_of(pdata, struct aspeed_g6_probe_data, pdata);
+	const struct aspeed_pin_group *pgroup = &pdata->pinmux.groups[group];
+	unsigned int i;
+
+	/* Check the entire group before the shared helper changes any pin. */
+	if (g6->thru_error) {
+		for (i = 0; i < pgroup->npins; i++) {
+			if (aspeed_g6_is_thru_pin(pgroup->pins[i]))
+				return g6->thru_error;
+		}
+	}
+
+	return aspeed_pinmux_set_mux(pctldev, function, group);
+}
+
 static const struct pinmux_ops aspeed_g6_pinmux_ops = {
 	.get_functions_count = aspeed_pinmux_get_fn_count,
 	.get_function_name = aspeed_pinmux_get_fn_name,
 	.get_function_groups = aspeed_pinmux_get_fn_groups,
-	.set_mux = aspeed_pinmux_set_mux,
-	.gpio_request_enable = aspeed_gpio_request_enable,
+	.set_mux = aspeed_g6_set_mux,
+	.gpio_request_enable = aspeed_g6_gpio_request_enable,
 	.strict = true,
 };
 
@@ -2782,15 +2829,107 @@ static const struct pinctrl_desc aspeed_g6_pinctrl_desc = {
 	.confops = &aspeed_g6_conf_ops,
 };
 
+/*
+ * Experimental: transfer the global GPIOP0-P5 THRU selection to the
+ * individual mux controls before registering any pinctrl consumers.
+ * PWM selection is higher priority and is deliberately left unchanged.
+ *
+ * Do not undo the local selections on error: a shared enable may already
+ * have cleared. Keeping the equivalent local selections preserves routing
+ * while rejecting affected requests, rather than granting an unsafe GPIO.
+ * Signal continuity during this transfer still needs hardware validation.
+ * Running before pinctrl registration excludes this controller's consumers,
+ * not independent SCU writers. Regmap locks individual accesses, not this
+ * sequence; coordination with other writers remains an RFC design question.
+ */
+static int aspeed_g6_thru_handover(struct regmap *scu)
+{
+	const u32 local_mask = GENMASK(29, 24);
+	unsigned int strap2, strap3, local;
+	int ret;
+
+	ret = regmap_read(scu, SCU510, &strap2);
+	if (ret)
+		return ret;
+	ret = regmap_read(scu, SCU51C, &strap3);
+	if (ret)
+		return ret;
+
+	if (!(strap2 & BIT(28)) &&
+	    ((strap2 & BIT(30)) || !(strap3 & BIT(9))))
+		return 0;
+
+	/* Preserve every globally selected role, including under PWM. */
+	ret = regmap_update_bits(scu, SCU4BC, local_mask, local_mask);
+	if (ret)
+		return ret;
+	ret = regmap_read(scu, SCU4BC, &local);
+	if (ret)
+		return ret;
+	if ((local & local_mask) != local_mask)
+		return -EPERM;
+
+	/* Leave the inactive GPIOZ6 source alone in dedicated-strap mode. */
+	if (!(strap2 & BIT(30)) && (strap3 & BIT(9))) {
+		ret = regmap_update_bits(scu, SCU51C, BIT(9), 0);
+		if (ret)
+			return ret;
+	}
+
+	if (strap2 & BIT(28)) {
+		/* SCU510 is write-one-set; do not use an ordinary RW clear. */
+		ret = regmap_write(scu, SCU514, BIT(28));
+		if (ret)
+			return ret;
+	}
+
+	ret = regmap_read(scu, SCU510, &strap2);
+	if (ret)
+		return ret;
+	ret = regmap_read(scu, SCU51C, &strap3);
+	if (ret)
+		return ret;
+	if ((strap2 & BIT(28)) ||
+	    (!(strap2 & BIT(30)) && (strap3 & BIT(9))))
+		return -EPERM;
+
+	return 0;
+}
+
 static int aspeed_g6_pinctrl_probe(struct platform_device *pdev)
 {
-	int i;
+	struct aspeed_g6_probe_data *g6;
+	struct regmap *scu;
+	int i, ret;
+
+	if (!pdev->dev.parent)
+		return -ENODEV;
+	scu = syscon_node_to_regmap(pdev->dev.parent->of_node);
+	if (IS_ERR(scu))
+		return dev_err_probe(&pdev->dev, PTR_ERR(scu),
+				     "No SCU regmap for THRU handover\n");
+
+	g6 = devm_kzalloc(&pdev->dev, sizeof(*g6), GFP_KERNEL);
+	if (!g6)
+		return -ENOMEM;
+	/* Copy per-device state; referenced mapping tables remain shared. */
+	memcpy(&g6->pdata, &aspeed_g6_pinctrl_data, sizeof(g6->pdata));
+
+	ret = aspeed_g6_thru_handover(scu);
+	if (ret == -EPROBE_DEFER)
+		return ret;
+	if (ret) {
+		g6->thru_error = ret;
+		dev_warn(&pdev->dev,
+			 "THRU handover failed (%d); GPIOP0-P5 mux requests blocked\n",
+			 ret);
+	}
 
 	for (i = 0; i < ARRAY_SIZE(aspeed_g6_pins); i++)
 		aspeed_g6_pins[i].number = i;
 
 	return aspeed_pinctrl_probe(pdev, &aspeed_g6_pinctrl_desc,
-			&aspeed_g6_pinctrl_data);
+			&g6->pdata);
 }
 
 static const struct of_device_id aspeed_g6_pinctrl_of_match[] = {
-- 
2.47.3