* [RFC PATCH] pinctrl: aspeed: Handle GPIOZ6 pass-through strap on GPIO requests
@ 2026-10-15 3:51 Prasanth Kumar Padarthi
2026-10-15 4:01 ` sashiko-bot
` (2 more replies)
0 siblings, 3 replies; 19+ messages in thread
From: Prasanth Kumar Padarthi @ 2026-10-15 3:51 UTC (permalink / raw)
To: andrew
Cc: linus.walleij, joel, brgl, linux-aspeed, openbmc, linux-gpio,
linux-arm-kernel, linux-kernel, 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,
};
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC PATCH] pinctrl: aspeed: Handle GPIOZ6 pass-through strap on GPIO requests
2026-10-15 3:51 [RFC PATCH] pinctrl: aspeed: Handle GPIOZ6 pass-through strap on GPIO requests Prasanth Kumar Padarthi
@ 2026-10-15 4:01 ` sashiko-bot
2026-10-15 4:09 ` Prasanth
2026-10-22 2:43 ` [RFC PATCH v2 0/1] AST2600 global GPIOP passthrough handover experiment Prasanth Kumar Padarthi
2026-11-04 12:19 ` [RFC PATCH] pinctrl: aspeed: Handle GPIOZ6 pass-through strap on GPIO requests Linus Walleij
2 siblings, 1 reply; 19+ messages in thread
From: sashiko-bot @ 2026-10-15 4:01 UTC (permalink / raw)
To: Prasanth Kumar Padarthi; +Cc: linux-gpio
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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC PATCH] pinctrl: aspeed: Handle GPIOZ6 pass-through strap on GPIO requests
2026-10-15 4:01 ` sashiko-bot
@ 2026-10-15 4:09 ` Prasanth
2026-10-20 12:52 ` Prasanth
0 siblings, 1 reply; 19+ messages in thread
From: Prasanth @ 2026-10-15 4:09 UTC (permalink / raw)
To: sashiko-reviews; +Cc: linux-gpio
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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC PATCH] pinctrl: aspeed: Handle GPIOZ6 pass-through strap on GPIO requests
2026-10-15 4:09 ` Prasanth
@ 2026-10-20 12:52 ` Prasanth
0 siblings, 0 replies; 19+ messages in thread
From: Prasanth @ 2026-10-20 12:52 UTC (permalink / raw)
To: sashiko-reviews; +Cc: linux-gpio
Hi,
Further B650D4U testing shows that ordinary GPIO power control fails
with SCU4BC=0, SCU510=0 and SCU51C=0x1e80. With the candidate, SCU51C
is 0x1c80 and power-on succeeds. The captured power-control executable
and configuration hashes match between runs.
A separate candidate test also succeeded after complete AC removal and
CMOS-battery removal/reinsertion.
I understand that unconditionally clearing SCU51C[9] can disrupt the
other GPIOP passthrough pairs, so the current callback is not a
complete generic solution.
I am investigating whether shared-enable routing can first be
represented through the corresponding per-pin SCU4BC controls, then
the shared enable cleared, allowing the normal GPIO request path to
select GPIO on the requested pins while preserving the other pairs.
Does the hardware support this transfer without interrupting existing
passthrough signals, including when PWM has higher priority? If so,
would normalization during pinctrl initialization or coordinated
handling in the request path be the appropriate model?
This is a proposed direction, not a validated transition sequence.
Feedback on its hardware and ownership assumptions would help before
preparing a revision.
Regards,
Prasanth
On Thu, Oct 15, 2026 at 9:39 AM Prasanth <prasanth.padarthi10@gmail.com> wrote:
>
> 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
^ permalink raw reply [flat|nested] 19+ messages in thread
* [RFC PATCH v2 0/1] AST2600 global GPIOP passthrough handover experiment
2026-10-15 3:51 [RFC PATCH] pinctrl: aspeed: Handle GPIOZ6 pass-through strap on GPIO requests Prasanth Kumar Padarthi
2026-10-15 4:01 ` sashiko-bot
@ 2026-10-22 2:43 ` Prasanth Kumar Padarthi
2026-10-22 2:43 ` [RFC PATCH v2] pinctrl: aspeed: Hand over global GPIOP passthrough to local mux controls Prasanth Kumar Padarthi
2026-11-05 15:23 ` [PATCH v3] " Prasanth Kumar Padarthi
2026-11-04 12:19 ` [RFC PATCH] pinctrl: aspeed: Handle GPIOZ6 pass-through strap on GPIO requests Linus Walleij
2 siblings, 2 replies; 19+ messages in thread
From: Prasanth Kumar Padarthi @ 2026-10-22 2:43 UTC (permalink / raw)
To: linux-aspeed
Cc: linux-gpio, openbmc, linux-kernel, krzk, tan.siewert, billy_tsai,
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
^ permalink raw reply [flat|nested] 19+ messages in thread
* [RFC PATCH v2] pinctrl: aspeed: Hand over global GPIOP passthrough to local mux controls
2026-10-22 2:43 ` [RFC PATCH v2 0/1] AST2600 global GPIOP passthrough handover experiment Prasanth Kumar Padarthi
@ 2026-10-22 2:43 ` Prasanth Kumar Padarthi
2026-11-05 15:23 ` [PATCH v3] " Prasanth Kumar Padarthi
1 sibling, 0 replies; 19+ messages in thread
From: Prasanth Kumar Padarthi @ 2026-10-22 2:43 UTC (permalink / raw)
To: linux-aspeed
Cc: linux-gpio, openbmc, linux-kernel, krzk, tan.siewert, billy_tsai,
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
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [RFC PATCH] pinctrl: aspeed: Handle GPIOZ6 pass-through strap on GPIO requests
2026-10-15 3:51 [RFC PATCH] pinctrl: aspeed: Handle GPIOZ6 pass-through strap on GPIO requests Prasanth Kumar Padarthi
2026-10-15 4:01 ` sashiko-bot
2026-10-22 2:43 ` [RFC PATCH v2 0/1] AST2600 global GPIOP passthrough handover experiment Prasanth Kumar Padarthi
@ 2026-11-04 12:19 ` Linus Walleij
2026-11-05 3:36 ` Prasanth
2 siblings, 1 reply; 19+ messages in thread
From: Linus Walleij @ 2026-11-04 12:19 UTC (permalink / raw)
To: Prasanth Kumar Padarthi
Cc: andrew, linus.walleij, joel, brgl, linux-aspeed, openbmc,
linux-gpio, linux-arm-kernel, linux-kernel
Hi Prasanth,
On Thu, Oct 15, 2026 at 5:51 AM Prasanth Kumar Padarthi
<prasanth.padarthi10@gmail.com> wrote:
> 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>
I don't quite understand the status of this patch, if it is still up for
grabs, please rebase it on v7.4-rc1 and resend so we can restart
the review.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC PATCH] pinctrl: aspeed: Handle GPIOZ6 pass-through strap on GPIO requests
2026-11-04 12:19 ` [RFC PATCH] pinctrl: aspeed: Handle GPIOZ6 pass-through strap on GPIO requests Linus Walleij
@ 2026-11-05 3:36 ` Prasanth
2026-11-05 13:27 ` Linus Walleij
0 siblings, 1 reply; 19+ messages in thread
From: Prasanth @ 2026-11-05 3:36 UTC (permalink / raw)
To: Linus Walleij
Cc: andrew, joel, brgl, linux-aspeed, openbmc, linux-gpio,
linux-arm-kernel, linux-kernel
Hi Linus,
Thanks for following up. This work is still active.
The October 15 GPIO-request callback was superseded by RFC v2,
submitted on October 22:
[RFC PATCH v2 0/1] AST2600 global GPIOP passthrough handover experiment
October 22 RFC v2 cover letter:
https://lore.kernel.org/r/20261022024314.48623-1-prasanth.padarthi10@gmail.com
V2 transfers the global pass-through selection to per-pin SCU4BC
controls before pinctrl registration. It also handles SCU510[28]
through SCU514 and restricts affected requests if handover fails.
I have retested this implementation on B650D4U with a newly built
image and GPIO-only x86-power-control. Remote power-on, remote
graceful power-off and physical-button power-on passed, with
power-good transitions recorded in the journal.
SCU4BC remained 0x30000000 and SCU51C remained 0x1c80 across the
remote power-on/off tests. Electrical continuity, mixed-pair
forwarding and fault-path behavior remain unvalidated.
I will verify the requested base, rebase the October 22 version
and resend it as RFC v3 for renewed review.
Regards,
Prasanth
On Wed, Nov 4, 2026 at 5:49 PM Linus Walleij <linusw@kernel.org> wrote:
>
> Hi Prasanth,
>
> On Thu, Oct 15, 2026 at 5:51 AM Prasanth Kumar Padarthi
> <prasanth.padarthi10@gmail.com> wrote:
>
> > 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>
>
> I don't quite understand the status of this patch, if it is still up for
> grabs, please rebase it on v7.4-rc1 and resend so we can restart
> the review.
>
> Yours,
> Linus Walleij
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC PATCH] pinctrl: aspeed: Handle GPIOZ6 pass-through strap on GPIO requests
2026-11-05 3:36 ` Prasanth
@ 2026-11-05 13:27 ` Linus Walleij
0 siblings, 0 replies; 19+ messages in thread
From: Linus Walleij @ 2026-11-05 13:27 UTC (permalink / raw)
To: Prasanth
Cc: andrew, joel, brgl, linux-aspeed, openbmc, linux-gpio,
linux-arm-kernel, linux-kernel
On Thu, Nov 5, 2026 at 4:36 AM Prasanth <prasanth.padarthi10@gmail.com> wrote:
> I will verify the requested base, rebase the October 22 version
> and resend it as RFC v3 for renewed review.
Do you have to still keep it RFC? I normally do not apply
"RFC" patches. Consider dropping the prefix.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v3] pinctrl: aspeed: Hand over global GPIOP passthrough to local mux controls
2026-10-22 2:43 ` [RFC PATCH v2 0/1] AST2600 global GPIOP passthrough handover experiment Prasanth Kumar Padarthi
2026-10-22 2:43 ` [RFC PATCH v2] pinctrl: aspeed: Hand over global GPIOP passthrough to local mux controls Prasanth Kumar Padarthi
@ 2026-11-05 15:23 ` Prasanth Kumar Padarthi
2026-11-05 15:33 ` sashiko-bot
` (2 more replies)
1 sibling, 3 replies; 19+ messages in thread
From: Prasanth Kumar Padarthi @ 2026-11-05 15:23 UTC (permalink / raw)
To: linusw
Cc: andrew, joel, brgl, tan.siewert, billy_tsai, linux-aspeed,
linux-gpio, openbmc, linux-arm-kernel, linux-kernel,
Prasanth Kumar Padarthi
On B650D4U, GPIO-only host power control fails when SCU51C[9]
enables GPIOP passthrough, even after the normal GPIO request path
clears the individual SCU4BC selections.
Transfer 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.
Clear SCU510[28] through SCU514 and verify the shared enables cleared.
If handover fails, retain an instance-local error and reject affected
GPIO and mux requests before changing their mux state. Check every
pin in a mux group before calling the shared helper. Retain prepared
local selections after partial failure rather than blindly restoring
a shared enable.
Probe-time normalization policy, concurrent SCU writers and
post-handover registration failure remain open design questions.
Electrical continuity, mixed-pair forwarding, protected-write paths,
PWM coexistence and SCU510[28] hardware handling remain unvalidated.
The implementation passed remote power-on, remote graceful power-off
and physical-button power-on tests on B650D4U with an OpenBMC Linux
6.18.24-based kernel. The rebased version was ARM compile-tested on
v7.4-rc1 using aspeed_g5_defconfig with AST2600 pinctrl enabled.
The rebased kernel has not been tested on hardware.
Signed-off-by: Prasanth Kumar Padarthi <prasanth.padarthi10@gmail.com>
---
drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c | 145 ++++++++++++++++++++-
1 file changed, 141 insertions(+), 4 deletions(-)
diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c b/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
index 85d6640ff..be22a9184 100644
--- a/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
+++ b/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
@@ -4,7 +4,10 @@
#include <linux/init.h>
#include <linux/io.h>
#include <linux/kernel.h>
+#include <linux/mfd/syscon.h>
#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/string.h>
#include <linux/pinctrl/pinctrl.h>
#include <linux/pinctrl/pinmux.h>
#include <linux/types.h>
@@ -36,6 +39,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 */
@@ -2743,12 +2748,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,
};
@@ -2778,15 +2827,103 @@ 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.
+ */
+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;
+ 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[] = {
base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
--
2.47.3
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v3] pinctrl: aspeed: Hand over global GPIOP passthrough to local mux controls
2026-11-05 15:23 ` [PATCH v3] " Prasanth Kumar Padarthi
@ 2026-11-05 15:33 ` sashiko-bot
2026-11-05 17:09 ` Prasanth
2026-11-06 2:28 ` [PATCH v4 0/2] Aspeed GPIOP handover with managed controller lifetime Prasanth Kumar Padarthi
2026-11-06 2:29 ` Prasanth Kumar Padarthi
2 siblings, 1 reply; 19+ messages in thread
From: sashiko-bot @ 2026-11-05 15:33 UTC (permalink / raw)
To: Prasanth Kumar Padarthi; +Cc: linux-gpio
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] pinctrl: aspeed: use-after-free on unbind in aspeed_g6_pinctrl_probe()
--
commit 7926ad91b4f1741a83b80da360b9a845357a5376
Author: Prasanth Kumar Padarthi <prasanth.padarthi10@gmail.com>
pinctrl: aspeed: Hand over global GPIOP passthrough to local mux controls
This commit transfers global GPIOP0-P5 passthrough selections in SCU510 and
SCU51C to per-pin mux controls in SCU4BC during probe, and blocks GPIO and
mux requests on GPIOP0-P5 if the handover fails.
> diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c b/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
> index 85d6640ff4c4e..be22a918420df 100644
> --- a/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
> +++ b/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
[ ... ]
> @@ -2778,15 +2827,103 @@ static const struct pinctrl_desc aspeed_g6_pinctrl_desc = {
[ ... ]
> 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;
> + 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);
[Severity: High]
Can this cause a use-after-free of g6 if the device is unbound?
In aspeed_g6_pinctrl_probe(), g6 is allocated with devm_kzalloc() and
&g6->pdata is passed to aspeed_pinctrl_probe(). However,
drivers/pinctrl/aspeed/pinctrl-aspeed.c:aspeed_pinctrl_probe() registers the
controller with the unmanaged pinctrl_register() helper:
drivers/pinctrl/aspeed/pinctrl-aspeed.c:aspeed_pinctrl_probe() {
...
pctl = pinctrl_register(pdesc, &pdev->dev, pdata);
...
}
Previously, aspeed_g6_pinctrl_probe() passed the static
aspeed_g6_pinctrl_data structure, which remained valid for the lifetime of
the kernel. Because aspeed_g6_pinctrl_driver does not define a .remove
callback to call pinctrl_unregister() and does not set .suppress_bind_attrs,
unbinding the platform device via sysfs frees g6 via devres while the
pinctrl_dev remains registered with driver_data pointing to &g6->pdata.
Subsequent calls to aspeed_g6_set_mux(), aspeed_g6_gpio_request_enable(),
other pinctrl callbacks, or pinctrl debugfs entries will then dereference the
freed g6 structure.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261105152348.8567-1-prasanth.padarthi10@gmail.com?part=1
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3] pinctrl: aspeed: Hand over global GPIOP passthrough to local mux controls
2026-11-05 15:33 ` sashiko-bot
@ 2026-11-05 17:09 ` Prasanth
0 siblings, 0 replies; 19+ messages in thread
From: Prasanth @ 2026-11-05 17:09 UTC (permalink / raw)
To: sashiko-reviews; +Cc: linux-gpio
Thanks for the review. I checked the shared probe helper and confirm
that it uses unmanaged pinctrl_register(), while this patch introduces
devm-allocated private data. The unbind lifetime needs to be corrected.
I will address controller cleanup and private-data lifetime in the
next revision.
On Thu, Nov 5, 2026 at 9:03 PM <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] pinctrl: aspeed: use-after-free on unbind in aspeed_g6_pinctrl_probe()
> --
>
> commit 7926ad91b4f1741a83b80da360b9a845357a5376
> Author: Prasanth Kumar Padarthi <prasanth.padarthi10@gmail.com>
>
> pinctrl: aspeed: Hand over global GPIOP passthrough to local mux controls
>
> This commit transfers global GPIOP0-P5 passthrough selections in SCU510 and
> SCU51C to per-pin mux controls in SCU4BC during probe, and blocks GPIO and
> mux requests on GPIOP0-P5 if the handover fails.
>
> > diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c b/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
> > index 85d6640ff4c4e..be22a918420df 100644
> > --- a/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
> > +++ b/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
> [ ... ]
> > @@ -2778,15 +2827,103 @@ static const struct pinctrl_desc aspeed_g6_pinctrl_desc = {
> [ ... ]
> > 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;
> > + 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);
>
> [Severity: High]
> Can this cause a use-after-free of g6 if the device is unbound?
>
> In aspeed_g6_pinctrl_probe(), g6 is allocated with devm_kzalloc() and
> &g6->pdata is passed to aspeed_pinctrl_probe(). However,
> drivers/pinctrl/aspeed/pinctrl-aspeed.c:aspeed_pinctrl_probe() registers the
> controller with the unmanaged pinctrl_register() helper:
>
> drivers/pinctrl/aspeed/pinctrl-aspeed.c:aspeed_pinctrl_probe() {
> ...
> pctl = pinctrl_register(pdesc, &pdev->dev, pdata);
> ...
> }
>
> Previously, aspeed_g6_pinctrl_probe() passed the static
> aspeed_g6_pinctrl_data structure, which remained valid for the lifetime of
> the kernel. Because aspeed_g6_pinctrl_driver does not define a .remove
> callback to call pinctrl_unregister() and does not set .suppress_bind_attrs,
> unbinding the platform device via sysfs frees g6 via devres while the
> pinctrl_dev remains registered with driver_data pointing to &g6->pdata.
>
> Subsequent calls to aspeed_g6_set_mux(), aspeed_g6_gpio_request_enable(),
> other pinctrl callbacks, or pinctrl debugfs entries will then dereference the
> freed g6 structure.
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20261105152348.8567-1-prasanth.padarthi10@gmail.com?part=1
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v4 0/2] Aspeed GPIOP handover with managed controller lifetime
2026-11-05 15:23 ` [PATCH v3] " Prasanth Kumar Padarthi
2026-11-05 15:33 ` sashiko-bot
@ 2026-11-06 2:28 ` Prasanth Kumar Padarthi
2026-11-06 2:29 ` Prasanth Kumar Padarthi
2 siblings, 0 replies; 19+ messages in thread
From: Prasanth Kumar Padarthi @ 2026-11-06 2:28 UTC (permalink / raw)
To: linusw
Cc: andrew, joel, ryan_chen, billy_tsai, brgl, tan.siewert,
linux-aspeed, linux-gpio, openbmc, linux-arm-kernel, linux-kernel,
Prasanth Kumar Padarthi
This series continues the AST2600 global-to-local GPIOP passthrough
handover work, rebased on v7.4-rc1. It adds a shared pinctrl lifetime
fix before the G6 handover change.
The registration lifetime issue was identified by automated review:
the G6 callback data is devm-allocated, while the shared helper used
unmanaged pinctrl registration. Patch 1 switches registration to
devm_pinctrl_register(), ensuring unregister runs before earlier
devres allocations are freed. Patch 2 retains the v3 handover logic.
Changes since v3:
- Add managed registration as a separate prerequisite patch.
- Keep the handover implementation unchanged.
- Add KASAN lifecycle testing in QEMU and on B650D4U hardware.
- Update validation statements to distinguish lifecycle and power tests.
Validation:
- Shared helper and G4/G5/G6 objects compile-tested with W=1 on ARM.
- G7 SOC0 object compile-tested using ARM COMPILE_TEST; this was not
an ARM64 build or G7 runtime test.
- Generic KASAN boot and one unbind/rebind cycle passed in AST2600
QEMU and on physical B650D4U hardware. Controller entries disappeared
on unbind and returned on rebind; a subsequent debugfs pins read
succeeded. No KASAN reports were observed in the captured tests.
- The final v4 source tree matches the tested v3 tree plus the
managed-registration change; only commit order and messages differ.
- Earlier remote power-on/off and physical-button power-on testing
used an OpenBMC 6.18.24-based kernel, not this v7.4-rc1 test kernel.
The unfixed runtime comparison has not been performed. Concurrent
readers, probe-failure injection, mixed-pair forwarding, protected
writes and SCU510[28] hardware handling remain unvalidated. Existing
questions about probe-time normalization policy and concurrent SCU
writers also remain open. Electrical measurements are outside the
scope of this lifecycle test.
Supplementary procedure, exact source/config and logs:
https://github.com/prasanthp20/asrock-b650d4u-bmc-support/tree/64eaccfde9ac4ec35646c0b7a8663486775b5d3b/docs/experiments/03-pinctrl-lifecycle-kasan
Previous version:
https://lore.kernel.org/r/20261105152348.8567-1-prasanth.padarthi10@gmail.com
Please review the shared lifetime fix and the GPIOP handover policy.
Prasanth Kumar Padarthi (2):
pinctrl: aspeed: Use managed pinctrl registration
pinctrl: aspeed: Hand over global GPIOP passthrough to local mux
controls
drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c | 145 ++++++++++++++++++++-
drivers/pinctrl/aspeed/pinctrl-aspeed.c | 3 +-
2 files changed, 143 insertions(+), 5 deletions(-)
base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
--
2.47.3
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v4 0/2] Aspeed GPIOP handover with managed controller lifetime
2026-11-05 15:23 ` [PATCH v3] " Prasanth Kumar Padarthi
2026-11-05 15:33 ` sashiko-bot
2026-11-06 2:28 ` [PATCH v4 0/2] Aspeed GPIOP handover with managed controller lifetime Prasanth Kumar Padarthi
@ 2026-11-06 2:29 ` Prasanth Kumar Padarthi
2026-11-06 2:29 ` [PATCH v4 1/2] pinctrl: aspeed: Use managed pinctrl registration Prasanth Kumar Padarthi
` (2 more replies)
2 siblings, 3 replies; 19+ messages in thread
From: Prasanth Kumar Padarthi @ 2026-11-06 2:29 UTC (permalink / raw)
To: linusw
Cc: andrew, joel, ryan_chen, billy_tsai, brgl, tan.siewert,
linux-aspeed, linux-gpio, openbmc, linux-arm-kernel, linux-kernel,
Prasanth Kumar Padarthi
This series continues the AST2600 global-to-local GPIOP passthrough
handover work, rebased on v7.4-rc1. It adds a shared pinctrl lifetime
fix before the G6 handover change.
The registration lifetime issue was identified by automated review:
the G6 callback data is devm-allocated, while the shared helper used
unmanaged pinctrl registration. Patch 1 switches registration to
devm_pinctrl_register(), ensuring unregister runs before earlier
devres allocations are freed. Patch 2 retains the v3 handover logic.
Changes since v3:
- Add managed registration as a separate prerequisite patch.
- Keep the handover implementation unchanged.
- Add KASAN lifecycle testing in QEMU and on B650D4U hardware.
- Update validation statements to distinguish lifecycle and power tests.
Validation:
- Shared helper and G4/G5/G6 objects compile-tested with W=1 on ARM.
- G7 SOC0 object compile-tested using ARM COMPILE_TEST; this was not
an ARM64 build or G7 runtime test.
- Generic KASAN boot and one unbind/rebind cycle passed in AST2600
QEMU and on physical B650D4U hardware. Controller entries disappeared
on unbind and returned on rebind; a subsequent debugfs pins read
succeeded. No KASAN reports were observed in the captured tests.
- The final v4 source tree matches the tested v3 tree plus the
managed-registration change; only commit order and messages differ.
- Earlier remote power-on/off and physical-button power-on testing
used an OpenBMC 6.18.24-based kernel, not this v7.4-rc1 test kernel.
The unfixed runtime comparison has not been performed. Concurrent
readers, probe-failure injection, mixed-pair forwarding, protected
writes and SCU510[28] hardware handling remain unvalidated. Existing
questions about probe-time normalization policy and concurrent SCU
writers also remain open. Electrical measurements are outside the
scope of this lifecycle test.
Supplementary procedure, exact source/config and logs:
https://github.com/prasanthp20/asrock-b650d4u-bmc-support/tree/64eaccfde9ac4ec35646c0b7a8663486775b5d3b/docs/experiments/03-pinctrl-lifecycle-kasan
Previous version:
https://lore.kernel.org/r/20261105152348.8567-1-prasanth.padarthi10@gmail.com
Please review the shared lifetime fix and the GPIOP handover policy.
Prasanth Kumar Padarthi (2):
pinctrl: aspeed: Use managed pinctrl registration
pinctrl: aspeed: Hand over global GPIOP passthrough to local mux
controls
drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c | 145 ++++++++++++++++++++-
drivers/pinctrl/aspeed/pinctrl-aspeed.c | 3 +-
2 files changed, 143 insertions(+), 5 deletions(-)
base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
--
2.47.3
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v4 1/2] pinctrl: aspeed: Use managed pinctrl registration
2026-11-06 2:29 ` Prasanth Kumar Padarthi
@ 2026-11-06 2:29 ` Prasanth Kumar Padarthi
2026-11-06 6:13 ` Andrew Jeffery
2026-11-06 2:29 ` [PATCH v4 2/2] pinctrl: aspeed: Hand over global GPIOP passthrough to local mux controls Prasanth Kumar Padarthi
2026-11-06 5:25 ` [PATCH v4 0/2] Aspeed GPIOP handover with managed controller lifetime Andrew Jeffery
2 siblings, 1 reply; 19+ messages in thread
From: Prasanth Kumar Padarthi @ 2026-11-06 2:29 UTC (permalink / raw)
To: linusw
Cc: andrew, joel, ryan_chen, billy_tsai, brgl, tan.siewert,
linux-aspeed, linux-gpio, openbmc, linux-arm-kernel, linux-kernel,
Prasanth Kumar Padarthi
The shared Aspeed probe helper registers its controller with
pinctrl_register(), but does not unregister it when the device is
unbound. Registration can therefore outlive device-managed callback
data, leaving a registered controller with pointers to freed memory.
The G6 handover change in this series embeds that data in a
device-managed allocation.
Use devm_pinctrl_register() so controller teardown runs before earlier
devres allocations are released. This also cleans up the registration
if later probe processing fails.
With this change and the G6 handover patch, one unbind/rebind cycle
passed under generic KASAN in AST2600 QEMU and on B650D4U hardware.
Controller debugfs entries disappeared on unbind and returned on
rebind; a subsequent pins read succeeded without observed KASAN
reports. The unfixed control case has not been runtime-tested.
Signed-off-by: Prasanth Kumar Padarthi <prasanth.padarthi10@gmail.com>
---
drivers/pinctrl/aspeed/pinctrl-aspeed.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed.c b/drivers/pinctrl/aspeed/pinctrl-aspeed.c
index 7e0ebf11a..d19b4d23c 100644
--- a/drivers/pinctrl/aspeed/pinctrl-aspeed.c
+++ b/drivers/pinctrl/aspeed/pinctrl-aspeed.c
@@ -461,7 +461,8 @@ int aspeed_pinctrl_probe(struct platform_device *pdev,
pdata->pinmux.maps[ASPEED_IP_SCU] = pdata->scu;
- pctl = pinctrl_register(pdesc, &pdev->dev, pdata);
+ /* Unregister before earlier devres allocations such as pdata are freed. */
+ pctl = devm_pinctrl_register(&pdev->dev, pdesc, pdata);
if (IS_ERR(pctl)) {
dev_err(&pdev->dev, "Failed to register pinctrl\n");
--
2.47.3
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v4 2/2] pinctrl: aspeed: Hand over global GPIOP passthrough to local mux controls
2026-11-06 2:29 ` Prasanth Kumar Padarthi
2026-11-06 2:29 ` [PATCH v4 1/2] pinctrl: aspeed: Use managed pinctrl registration Prasanth Kumar Padarthi
@ 2026-11-06 2:29 ` Prasanth Kumar Padarthi
2026-11-06 10:54 ` Andrew Jeffery
2026-11-06 5:25 ` [PATCH v4 0/2] Aspeed GPIOP handover with managed controller lifetime Andrew Jeffery
2 siblings, 1 reply; 19+ messages in thread
From: Prasanth Kumar Padarthi @ 2026-11-06 2:29 UTC (permalink / raw)
To: linusw
Cc: andrew, joel, ryan_chen, billy_tsai, brgl, tan.siewert,
linux-aspeed, linux-gpio, openbmc, linux-arm-kernel, linux-kernel,
Prasanth Kumar Padarthi
On B650D4U, GPIO-only host power control fails when SCU51C[9]
enables GPIOP passthrough, even after the normal GPIO request path
clears the individual SCU4BC selections.
Transfer 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.
Clear SCU510[28] through SCU514 and verify the shared enables cleared.
If handover fails, retain an instance-local error and reject affected
GPIO and mux requests before changing their mux state. Check every
pin in a mux group before calling the shared helper. Retain prepared
local selections after partial failure rather than blindly restoring
a shared enable.
Probe-time normalization policy, concurrent SCU writers and
post-handover registration failure remain open design questions.
Electrical continuity, mixed-pair forwarding, protected-write paths,
PWM coexistence and SCU510[28] hardware handling remain unvalidated.
The implementation passed remote power-on, remote graceful power-off
and physical-button power-on tests on B650D4U with an OpenBMC Linux
6.18.24-based kernel. The rebased version was ARM compile-tested on
v7.4-rc1 using aspeed_g5_defconfig with AST2600 pinctrl enabled.
With the managed-registration prerequisite, the v7.4-rc1 version
also completed one unbind/rebind cycle under generic KASAN on
B650D4U hardware and in AST2600 QEMU. Host power operations have
not been retested with the v7.4-rc1 kernel.
Signed-off-by: Prasanth Kumar Padarthi <prasanth.padarthi10@gmail.com>
---
drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c | 145 ++++++++++++++++++++-
1 file changed, 141 insertions(+), 4 deletions(-)
diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c b/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
index 85d6640ff..be22a9184 100644
--- a/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
+++ b/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
@@ -4,7 +4,10 @@
#include <linux/init.h>
#include <linux/io.h>
#include <linux/kernel.h>
+#include <linux/mfd/syscon.h>
#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/string.h>
#include <linux/pinctrl/pinctrl.h>
#include <linux/pinctrl/pinmux.h>
#include <linux/types.h>
@@ -36,6 +39,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 */
@@ -2743,12 +2748,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,
};
@@ -2778,15 +2827,103 @@ 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.
+ */
+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;
+ 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
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v4 0/2] Aspeed GPIOP handover with managed controller lifetime
2026-11-06 2:29 ` Prasanth Kumar Padarthi
2026-11-06 2:29 ` [PATCH v4 1/2] pinctrl: aspeed: Use managed pinctrl registration Prasanth Kumar Padarthi
2026-11-06 2:29 ` [PATCH v4 2/2] pinctrl: aspeed: Hand over global GPIOP passthrough to local mux controls Prasanth Kumar Padarthi
@ 2026-11-06 5:25 ` Andrew Jeffery
2 siblings, 0 replies; 19+ messages in thread
From: Andrew Jeffery @ 2026-11-06 5:25 UTC (permalink / raw)
To: Prasanth Kumar Padarthi, linusw
Cc: joel, ryan_chen, billy_tsai, brgl, tan.siewert, linux-aspeed,
linux-gpio, openbmc, linux-arm-kernel, linux-kernel
On Fri, 2026-11-06 at 07:59 +0530, Prasanth Kumar Padarthi wrote:
> This series continues the AST2600 global-to-local GPIOP passthrough
> handover work, rebased on v7.4-rc1. It adds a shared pinctrl lifetime
> fix before the G6 handover change.
>
> The registration lifetime issue was identified by automated review:
> the G6 callback data is devm-allocated, while the shared helper used
> unmanaged pinctrl registration. Patch 1 switches registration to
> devm_pinctrl_register(), ensuring unregister runs before earlier
> devres allocations are freed. Patch 2 retains the v3 handover logic.
>
> Changes since v3:
> - Add managed registration as a separate prerequisite patch.
> - Keep the handover implementation unchanged.
> - Add KASAN lifecycle testing in QEMU and on B650D4U hardware.
> - Update validation statements to distinguish lifecycle and power tests.
Can you please re-send v4 so it's not threaded under v3?
Also, I received the v4 cover letter twice. Something seems wrong with
your mail setup.
Andrew
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v4 1/2] pinctrl: aspeed: Use managed pinctrl registration
2026-11-06 2:29 ` [PATCH v4 1/2] pinctrl: aspeed: Use managed pinctrl registration Prasanth Kumar Padarthi
@ 2026-11-06 6:13 ` Andrew Jeffery
0 siblings, 0 replies; 19+ messages in thread
From: Andrew Jeffery @ 2026-11-06 6:13 UTC (permalink / raw)
To: Prasanth Kumar Padarthi, linusw
Cc: joel, ryan_chen, billy_tsai, brgl, tan.siewert, linux-aspeed,
linux-gpio, openbmc, linux-arm-kernel, linux-kernel
On Fri, 2026-11-06 at 07:59 +0530, Prasanth Kumar Padarthi wrote:
> The shared Aspeed probe helper registers its controller with
> pinctrl_register(), but does not unregister it when the device is
> unbound.
>
This is a problem in its own right. IMO the rest of the discussion can
be left out, as it depends on the subsequent change, which still needs
consideration.
> Registration can therefore outlive device-managed callback
> data, leaving a registered controller with pointers to freed memory.
> The G6 handover change in this series embeds that data in a
> device-managed allocation.
>
> Use devm_pinctrl_register() so controller teardown runs before earlier
> devres allocations are released. This also cleans up the registration
> if later probe processing fails.
>
> With this change and the G6 handover patch, one unbind/rebind cycle
> passed under generic KASAN in AST2600 QEMU and on B650D4U hardware.
IMO this sentence is enough to describe the testing, the rest reads
like unnecessary detail that tends to be generated by LLMs.
> Controller debugfs entries disappeared on unbind and returned on
> rebind; a subsequent pins read succeeded without observed KASAN
> reports. The unfixed control case has not been runtime-tested.
What is this last sentence referring to?
>
> Signed-off-by: Prasanth Kumar Padarthi <prasanth.padarthi10@gmail.com>
> ---
> drivers/pinctrl/aspeed/pinctrl-aspeed.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed.c b/drivers/pinctrl/aspeed/pinctrl-aspeed.c
> index 7e0ebf11a..d19b4d23c 100644
> --- a/drivers/pinctrl/aspeed/pinctrl-aspeed.c
> +++ b/drivers/pinctrl/aspeed/pinctrl-aspeed.c
> @@ -461,7 +461,8 @@ int aspeed_pinctrl_probe(struct platform_device *pdev,
>
> pdata->pinmux.maps[ASPEED_IP_SCU] = pdata->scu;
>
> - pctl = pinctrl_register(pdesc, &pdev->dev, pdata);
> + /* Unregister before earlier devres allocations such as pdata are freed. */
I'd rather the comment is removed.
Cheers,
Andrew
> + pctl = devm_pinctrl_register(&pdev->dev, pdesc, pdata);
>
> if (IS_ERR(pctl)) {
> dev_err(&pdev->dev, "Failed to register pinctrl\n");
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v4 2/2] pinctrl: aspeed: Hand over global GPIOP passthrough to local mux controls
2026-11-06 2:29 ` [PATCH v4 2/2] pinctrl: aspeed: Hand over global GPIOP passthrough to local mux controls Prasanth Kumar Padarthi
@ 2026-11-06 10:54 ` Andrew Jeffery
0 siblings, 0 replies; 19+ messages in thread
From: Andrew Jeffery @ 2026-11-06 10:54 UTC (permalink / raw)
To: Prasanth Kumar Padarthi, linusw, Tan Siewert
Cc: joel, ryan_chen, billy_tsai, brgl, tan.siewert, linux-aspeed,
linux-gpio, openbmc, linux-arm-kernel, linux-kernel
Hi Prasanth,
On Fri, 2026-11-06 at 07:59 +0530, Prasanth Kumar Padarthi wrote:
> On B650D4U, GPIO-only host power control fails when SCU51C[9]
> enables GPIOP passthrough, even after the normal GPIO request path
> clears the individual SCU4BC selections.
>
> Transfer 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.
> Clear SCU510[28] through SCU514 and verify the shared enables cleared.
>
> If handover fails, retain an instance-local error and reject affected
> GPIO and mux requests before changing their mux state. Check every
> pin in a mux group before calling the shared helper. Retain prepared
> local selections after partial failure rather than blindly restoring
> a shared enable.
>
> Probe-time normalization policy, concurrent SCU writers and
> post-handover registration failure remain open design questions.
> Electrical continuity, mixed-pair forwarding, protected-write paths,
> PWM coexistence and SCU510[28] hardware handling remain unvalidated.
>
> The implementation passed remote power-on, remote graceful power-off
> and physical-button power-on tests on B650D4U with an OpenBMC Linux
> 6.18.24-based kernel. The rebased version was ARM compile-tested on
> v7.4-rc1 using aspeed_g5_defconfig with AST2600 pinctrl enabled.
> With the managed-registration prerequisite, the v7.4-rc1 version
> also completed one unbind/rebind cycle under generic KASAN on
> B650D4U hardware and in AST2600 QEMU. Host power operations have
> not been retested with the v7.4-rc1 kernel.
>
> Signed-off-by: Prasanth Kumar Padarthi <prasanth.padarthi10@gmail.com>
> ---
> drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c | 145 ++++++++++++++++++++-
> 1 file changed, 141 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c b/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
> index 85d6640ff..be22a9184 100644
> --- a/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
> +++ b/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
> @@ -4,7 +4,10 @@
> #include <linux/init.h>
> #include <linux/io.h>
> #include <linux/kernel.h>
> +#include <linux/mfd/syscon.h>
> #include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/string.h>
> #include <linux/pinctrl/pinctrl.h>
> #include <linux/pinctrl/pinmux.h>
> #include <linux/types.h>
> @@ -36,6 +39,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 */
> @@ -2743,12 +2748,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,
> };
>
> @@ -2778,15 +2827,103 @@ 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.
> + */
> +static int aspeed_g6_thru_handover(struct regmap *scu)
> +{
> + const u32 local_mask = GENMASK(29, 24);
Defining a variable with the value (even though that variable is
qualified const) means it's not an integer constant expression, which
may impede some optimisations.
Further, local_mask is not a descriptive name for what's occurring
here. Please name the mask in terms of its behaviour. Perhaps:
#define GPIOP_UNGROUPED_PASSTHROUGH_MASK GENMASK(29, 24)
> + unsigned int strap2, strap3, local;
Please lets improve on the name 'local'. Perhaps: ungrouped_passthrough
> + int ret;
> +
> + ret = regmap_read(scu, SCU510, &strap2);
> + if (ret)
> + return ret;
Consider this error-handling strategy:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/pinctrl/pinctrl-gemini.c?h=v7.4-rc1#n2556
If the first read doesn't work, then we can't expect any of the further
accesses by the core aspeed pinctrl implementation to work either, so
we can just bail out entirely in the caller.
> + ret = regmap_read(scu, SCU51C, &strap3);
> + if (ret)
> + return ret;
> +
> + if (!(strap2 & BIT(28)) &&
> + ((strap2 & BIT(30)) || !(strap3 & BIT(9))))
Please give these bits named macros. This is not easy to comprehend as
it is.
Also, consider FIELD_GET() from include/linux/bitfield.h. Something
like:
if (!FIELD_GET(OTPSTRAP_ENABLE_GPIO_PASSTHROUGH, strap2) &&
(FIELD_GET(OTPSTRAP_ENABLE_DEDICATED_STRAP_PINS, strap2) ||
!(FIELD_GET(STRAP_ENABLE_GPIO_PASSTHROUGH, strap3)))
> + return 0;
> +
> + /* Preserve every globally selected role, including under PWM. */
This comment feels like a classic LLM statement that somehow manages to
be simultaneously both too abstract and too detailed, while also devoid
of context, all in 8 words.
Can we rather say that the PWM functions on these pins are higher
priority in the mux than the pass-through GPIO function, so if PWM is
already selected, then the following write does not affect the PWM
configuration?
> + 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;
Please add a comment here about the write-protection in SCUF30.
> +
> + /* Leave the inactive GPIOZ6 source alone in dedicated-strap mode. */
'dedicated strap mode' is a confusing statement because ASPEED's
documentation is confusing. I think we can drop this comment with
regard to making other improvements.
> + if (!(strap2 & BIT(30)) && (strap3 & BIT(9))) {
We've already inspected these bit values previously. Please consider
extracting them to bool variables for re-use and to improve everyone's
ability to comprehend the expression.
> + ret = regmap_update_bits(scu, SCU51C, BIT(9), 0);
Essentially, having configured the ungrouped passthrough functions
above, we can now de-configure the grouped passthrough function in
strap3 (SCU51C).
It's probably worth noting in a comment that SCU51C cannot be write-
protected.
> + 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;
This appears to be the negated expression of the earlier one, and given
we're only returning a fixed value in either case, you could rather use
the same expression again. Given it's repeated, you could extract it to
a macro or static inline function.
> +
> + 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;
> + memcpy(&g6->pdata, &aspeed_g6_pinctrl_data, sizeof(g6->pdata));
This seems unfortunate.
Have you considered other approaches?
> +
> + ret = aspeed_g6_thru_handover(scu);
Must this logic be separate to the usual set_mux path? If not, I expect
we don't need to retain thru_error.
> + if (ret == -EPROBE_DEFER)
Do you have an example of how ret could be -EPROBE_DEFER?
> + return ret;
> + if (ret) {
If this handover logic is to be integrated here, can you please test
for specific return values to separate the regmap access failure from
the write-protection failure?
Related, I've heard opinions from Tan that this patch is not necessary
to get the platform booting. Given the current complexity, I'm
interested to hear whether you can do without it.
Andrew
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2026-11-06 10:54 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-15 3:51 [RFC PATCH] pinctrl: aspeed: Handle GPIOZ6 pass-through strap on GPIO requests Prasanth Kumar Padarthi
2026-10-15 4:01 ` sashiko-bot
2026-10-15 4:09 ` Prasanth
2026-10-20 12:52 ` Prasanth
2026-10-22 2:43 ` [RFC PATCH v2 0/1] AST2600 global GPIOP passthrough handover experiment Prasanth Kumar Padarthi
2026-10-22 2:43 ` [RFC PATCH v2] pinctrl: aspeed: Hand over global GPIOP passthrough to local mux controls Prasanth Kumar Padarthi
2026-11-05 15:23 ` [PATCH v3] " Prasanth Kumar Padarthi
2026-11-05 15:33 ` sashiko-bot
2026-11-05 17:09 ` Prasanth
2026-11-06 2:28 ` [PATCH v4 0/2] Aspeed GPIOP handover with managed controller lifetime Prasanth Kumar Padarthi
2026-11-06 2:29 ` Prasanth Kumar Padarthi
2026-11-06 2:29 ` [PATCH v4 1/2] pinctrl: aspeed: Use managed pinctrl registration Prasanth Kumar Padarthi
2026-11-06 6:13 ` Andrew Jeffery
2026-11-06 2:29 ` [PATCH v4 2/2] pinctrl: aspeed: Hand over global GPIOP passthrough to local mux controls Prasanth Kumar Padarthi
2026-11-06 10:54 ` Andrew Jeffery
2026-11-06 5:25 ` [PATCH v4 0/2] Aspeed GPIOP handover with managed controller lifetime Andrew Jeffery
2026-11-04 12:19 ` [RFC PATCH] pinctrl: aspeed: Handle GPIOZ6 pass-through strap on GPIO requests Linus Walleij
2026-11-05 3:36 ` Prasanth
2026-11-05 13:27 ` Linus Walleij
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox