[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,
 };
 

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] pinctrl: aspeed: clobbering shared SCU51C[9] strap bit breaks pass-through
- [High] pinctrl: aspeed: unconditionally rejecting GPIO requests based on SCU510[28]
- [Medium] pinctrl: aspeed: missing hardware mux rollback on error path
--

commit fc751bac2dfdee809f65ba4da118ede6fdbe6aaf
Author: Prasanth Kumar Padarthi <prasanth.padarthi10@gmail.com>

pinctrl: aspeed: Handle GPIOZ6 pass-through strap on GPIO requests

This commit introduces an AST2600-specific GPIO request callback to handle
the GPIOZ6 pass-through strap in SCU51C[9] for GPIOP0-P5. It clears this
shared strap bit to allow host power control on boards like the B650D4U,
while also explicitly rejecting requests when SCU510[28] is set.

> diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c b/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
> index 85d6640ff4c4e..6e58feb48bf59 100644
> --- a/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
> +++ b/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c

[ ... ]

> @@ -2743,12 +2744,56 @@ static struct aspeed_pinctrl_data aspeed_g6_pinctrl_data = {
> +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;

[Severity: High]
Does this unconditionally reject standard GPIO requests based on an unhandled
strap condition? 

Systems where the SCU510[28] strap is set will no longer be able to request
GPIOP0-P5 as standard GPIOs. Any standard GPIO consumer on such hardware will
fail, breaking backwards compatibility.

> +
> +	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);

[Severity: High]
Does this clear a shared hardware enable bit for all three pairs without
refcounting? 

The SCU51C[9] bit globally controls the pass-through feature for three
distinct pin pairs (GPIOP0/P1, GPIOP2/P3, and GPIOP4/P5). Modifying it
unconditionally for a single pin request might destructively clobber
concurrent pass-through usage for the other unconnected pins.

> +	if (ret)
> +		return ret;

[Severity: Medium]
If regmap_update_bits() fails here, does this return an error without
unwinding the active pinmux modification made earlier by
aspeed_gpio_request_enable()?

This could leave the pin's hardware multiplexer configured for GPIO, while
the pinctrl subsystem considers the request failed and the pin unreserved,
creating a state mismatch between hardware and software.

> +	ret = regmap_read(scu, SCU51C, &strap3);
> +	if (ret)
> +		return ret;

[Severity: Medium]
Similarly, if this regmap_read() fails, does this also leak the hardware mux
state?

> +	if (strap3 & BIT(9))
> +		return -EPERM;

[Severity: Medium]
Could this error path also miss unwinding the hardware mux state from
aspeed_gpio_request_enable()?

> +
> +	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,

[Severity: High]
Since this structure lacks a .gpio_disable_free callback, is there any
mechanism to ensure the shared strap modification is reversed upon release?

Without it, the SCU51C[9] bit is never restored when the GPIO is freed,
which could cause a permanent system-wide functional loss for pass-through
features until a reboot.

>  	.strict = true,
>  };

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261015035118.9495-1-prasanth.padarthi10@gmail.com?part=1

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

[Prasanth]

Thanks for the review. The shared SCU51C[9] control and SCU510[28]
rejection are known limitations called out in this RFC. I agree that
the post-mux error paths also require attention.

The hardware result establishes that clearing SCU51C[9] permits
ordinary GPIO power control on B650D4U; it does not establish that
this callback implementation is suitable across AST2600 platforms. I
will investigate coordinated handling of the shared strap and per-pair
mux state before proposing a revision.

On Thu, Oct 15, 2026 at 9:31 AM <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> - [High] pinctrl: aspeed: clobbering shared SCU51C[9] strap bit breaks pass-through
> - [High] pinctrl: aspeed: unconditionally rejecting GPIO requests based on SCU510[28]
> - [Medium] pinctrl: aspeed: missing hardware mux rollback on error path
> --
>
> commit fc751bac2dfdee809f65ba4da118ede6fdbe6aaf
> Author: Prasanth Kumar Padarthi <prasanth.padarthi10@gmail.com>
>
> pinctrl: aspeed: Handle GPIOZ6 pass-through strap on GPIO requests
>
> This commit introduces an AST2600-specific GPIO request callback to handle
> the GPIOZ6 pass-through strap in SCU51C[9] for GPIOP0-P5. It clears this
> shared strap bit to allow host power control on boards like the B650D4U,
> while also explicitly rejecting requests when SCU510[28] is set.
>
> > diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c b/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
> > index 85d6640ff4c4e..6e58feb48bf59 100644
> > --- a/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
> > +++ b/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
>
> [ ... ]
>
> > @@ -2743,12 +2744,56 @@ static struct aspeed_pinctrl_data aspeed_g6_pinctrl_data = {
> > +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;
>
> [Severity: High]
> Does this unconditionally reject standard GPIO requests based on an unhandled
> strap condition?
>
> Systems where the SCU510[28] strap is set will no longer be able to request
> GPIOP0-P5 as standard GPIOs. Any standard GPIO consumer on such hardware will
> fail, breaking backwards compatibility.
>
> > +
> > +     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);
>
> [Severity: High]
> Does this clear a shared hardware enable bit for all three pairs without
> refcounting?
>
> The SCU51C[9] bit globally controls the pass-through feature for three
> distinct pin pairs (GPIOP0/P1, GPIOP2/P3, and GPIOP4/P5). Modifying it
> unconditionally for a single pin request might destructively clobber
> concurrent pass-through usage for the other unconnected pins.
>
> > +     if (ret)
> > +             return ret;
>
> [Severity: Medium]
> If regmap_update_bits() fails here, does this return an error without
> unwinding the active pinmux modification made earlier by
> aspeed_gpio_request_enable()?
>
> This could leave the pin's hardware multiplexer configured for GPIO, while
> the pinctrl subsystem considers the request failed and the pin unreserved,
> creating a state mismatch between hardware and software.
>
> > +     ret = regmap_read(scu, SCU51C, &strap3);
> > +     if (ret)
> > +             return ret;
>
> [Severity: Medium]
> Similarly, if this regmap_read() fails, does this also leak the hardware mux
> state?
>
> > +     if (strap3 & BIT(9))
> > +             return -EPERM;
>
> [Severity: Medium]
> Could this error path also miss unwinding the hardware mux state from
> aspeed_gpio_request_enable()?
>
> > +
> > +     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,
>
> [Severity: High]
> Since this structure lacks a .gpio_disable_free callback, is there any
> mechanism to ensure the shared strap modification is reversed upon release?
>
> Without it, the SCU51C[9] bit is never restored when the GPIO is freed,
> which could cause a permanent system-wide functional loss for pass-through
> features until a reboot.
>
> >       .strict = true,
> >  };
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20261015035118.9495-1-prasanth.padarthi10@gmail.com?part=1