* [PATCH 0/2] Fix sharing a PMIC GPIO between multiple consumers
@ 2026-09-15 1:44 Shawn Guo
2026-09-15 1:44 ` [PATCH 1/2] gpio: shared-proxy: track direction instead of reading it back Shawn Guo
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Shawn Guo @ 2026-09-15 1:44 UTC (permalink / raw)
To: Linus Walleij
Cc: Bartosz Golaszewski, Bjorn Andersson, Yu Zhang, linux-gpio,
linux-arm-msm, linux-kernel, Shawn Guo
On the Qualcomm Nord boards, three discrete regulators feeding a WCN7850
module are gated by a single PMIC GPIO (pmau0102_e GPIO6), so the board DT
describes three regulator-fixed nodes pointing at the same GPIO. Only the
first of them probes:
reg-fixed-voltage regulator-wcn-core-vm-1p35: setup of GPIO (default) failed: -1
reg-fixed-voltage regulator-wcn-core-vm-1p35: error -EPERM: can't get GPIO
platform wcn7850-pmu: deferred probe pending: supplier regulator-wcn-core-vh-1p95 not ready
ARCH_QCOM selects HAVE_SHARED_GPIOS, so gpiolib creates shared GPIO proxies
for the line rather than taking the legacy nonexclusive path. The failure
turns out to have two independent causes, one on each side of that
interface.
On the provider side, neither of spmi-gpio's direction callbacks clears the
opposite buffer: .direction_output() only sets output_enabled and
.direction_input() only sets input_enabled. A pad left in DIGITAL_INPUT
mode by the bootloader therefore ends up in DIGITAL_INPUT_OUTPUT mode on
the first direction change, after which .get_direction() reports
GPIO_LINE_DIRECTION_IN even though the driver is driving the line. The
same asymmetry means gpiod_direction_input() never stops driving a pad that
was previously an output, which is a bug in its own right.
On the consumer side, gpio_shared_proxy_direction_output() reads the
direction back for every requester after the first and refuses with -EPERM
if it sees an input. The readback is not needed, since the proxy is the
only entity configuring the line and already knows which direction it
asked for. It is also not robust: a provider without .get_direction() at
all makes gpiochip_get_direction() return -EOPNOTSUPP under a WARN_ON,
which the proxy propagates just the same.
Either patch on its own is enough to make the board work; both are sent
because both describe real bugs. Patch 1 is what makes shared GPIOs work
irrespective of the provider, so I would consider it the more important of
the two. There is no build or apply dependency between them and they
touch different subsystems, so they can go through separate trees.
Shawn Guo (2):
gpio: shared-proxy: track direction instead of reading it back
pinctrl: qcom: spmi-gpio: make direction changes exclusive
drivers/gpio/gpio-shared-proxy.c | 43 ++++++++++++++----------
drivers/gpio/gpiolib-shared.c | 1 +
drivers/gpio/gpiolib-shared.h | 1 +
drivers/pinctrl/qcom/pinctrl-spmi-gpio.c | 16 +++++----
4 files changed, 37 insertions(+), 24 deletions(-)
--
2.43.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/2] gpio: shared-proxy: track direction instead of reading it back
2026-09-15 1:44 [PATCH 0/2] Fix sharing a PMIC GPIO between multiple consumers Shawn Guo
@ 2026-09-15 1:44 ` Shawn Guo
2026-09-16 8:48 ` Bartosz Golaszewski
2026-09-16 9:00 ` Bartosz Golaszewski
2026-09-15 1:44 ` [PATCH 2/2] pinctrl: qcom: spmi-gpio: make direction changes exclusive Shawn Guo
2026-09-16 8:49 ` [PATCH 0/2] Fix sharing a PMIC GPIO between multiple consumers Bartosz Golaszewski
2 siblings, 2 replies; 10+ messages in thread
From: Shawn Guo @ 2026-09-15 1:44 UTC (permalink / raw)
To: Linus Walleij
Cc: Bartosz Golaszewski, Bjorn Andersson, Yu Zhang, linux-gpio,
linux-arm-msm, linux-kernel, Shawn Guo
On boards where more than one consumer shares a Qualcomm SPMI PMIC GPIO,
all but the first consumer fail to configure the line as output:
reg-fixed-voltage regulator-wcn-core-vm-1p35: setup of GPIO (default) failed: -1
reg-fixed-voltage regulator-wcn-core-vm-1p35: error -EPERM: can't get GPIO
The first consumer takes the usecnt == 1 branch and really does set the
underlying line to output. Later consumers instead read the direction
back with gpiod_get_direction() and refuse with -EPERM if it reports
input. pmic_gpio_get_direction() derives the direction from the pad's
input buffer (pad->input_enabled), which pmic_gpio_direction_output()
never clears, so a pad whose input buffer is enabled at power-up reports
input forever and the proxy rejects every consumer after the first.
The readback is not needed: the proxy is the only entity configuring the
line, so it already knows which direction it asked for. Record that in
struct gpio_shared_desc and compare against the recorded value. Keep a
readback in the get_direction() callback for the case where no proxy has
configured the line yet, and reset the recorded direction once the last
user goes away so that the next requester establishes it again.
Assisted-by: LLM
Fixes: e992d54c6f97 ("gpio: shared-proxy: implement the shared GPIO proxy driver")
Signed-off-by: Shawn Guo <shengchao.guo@oss.qualcomm.com>
---
drivers/gpio/gpio-shared-proxy.c | 43 +++++++++++++++++++-------------
drivers/gpio/gpiolib-shared.c | 1 +
drivers/gpio/gpiolib-shared.h | 1 +
3 files changed, 27 insertions(+), 18 deletions(-)
diff --git a/drivers/gpio/gpio-shared-proxy.c b/drivers/gpio/gpio-shared-proxy.c
index bc69b8729d19..a8f0d08c6d9f 100644
--- a/drivers/gpio/gpio-shared-proxy.c
+++ b/drivers/gpio/gpio-shared-proxy.c
@@ -116,6 +116,8 @@ static void gpio_shared_proxy_free(struct gpio_chip *gc, unsigned int offset)
}
proxy->shared_desc->usecnt--;
+ if (!shared_desc->usecnt)
+ shared_desc->dir = -1;
dev_dbg(proxy->dev, "Shared GPIO freed, number of users: %u\n",
proxy->shared_desc->usecnt);
@@ -155,22 +157,24 @@ static int gpio_shared_proxy_direction_input(struct gpio_chip *gc,
struct gpio_shared_proxy_data *proxy = gpiochip_get_data(gc);
struct gpio_shared_desc *shared_desc = proxy->shared_desc;
struct gpio_desc *desc = shared_desc->desc;
- int dir;
+ int ret;
guard(mutex)(&shared_desc->mutex);
- if (shared_desc->usecnt == 1) {
+ if (shared_desc->usecnt == 1 || shared_desc->dir < 0) {
dev_dbg(proxy->dev,
- "Only one user of this shared GPIO, allowing to set direction to input\n");
+ "Setting the direction of the shared GPIO to input\n");
- return gpiod_direction_input(desc);
- }
+ ret = gpiod_direction_input(desc);
+ if (ret)
+ return ret;
- dir = gpiod_get_direction(desc);
- if (dir < 0)
- return dir;
+ shared_desc->dir = GPIO_LINE_DIRECTION_IN;
+
+ return 0;
+ }
- if (dir == GPIO_LINE_DIRECTION_OUT) {
+ if (shared_desc->dir == GPIO_LINE_DIRECTION_OUT) {
dev_dbg(proxy->dev,
"Shared GPIO's direction already set to output, refusing to change\n");
return -EPERM;
@@ -185,19 +189,20 @@ static int gpio_shared_proxy_direction_output(struct gpio_chip *gc,
struct gpio_shared_proxy_data *proxy = gpiochip_get_data(gc);
struct gpio_shared_desc *shared_desc = proxy->shared_desc;
struct gpio_desc *desc = shared_desc->desc;
- int ret, dir;
+ int ret;
guard(mutex)(&shared_desc->mutex);
- if (shared_desc->usecnt == 1) {
+ if (shared_desc->usecnt == 1 || shared_desc->dir < 0) {
dev_dbg(proxy->dev,
- "Only one user of this shared GPIO, allowing to set direction to output with value '%s'\n",
+ "Setting the direction of the shared GPIO to output with value '%s'\n",
str_high_low(value));
ret = gpiod_direction_output(desc, value);
if (ret)
return ret;
+ shared_desc->dir = GPIO_LINE_DIRECTION_OUT;
shared_desc->def_val = value;
shared_desc->votecnt = 0;
proxy->voted_change = false;
@@ -205,11 +210,7 @@ static int gpio_shared_proxy_direction_output(struct gpio_chip *gc,
return 0;
}
- dir = gpiod_get_direction(desc);
- if (dir < 0)
- return dir;
-
- if (dir == GPIO_LINE_DIRECTION_IN) {
+ if (shared_desc->dir == GPIO_LINE_DIRECTION_IN) {
dev_dbg(proxy->dev,
"Shared GPIO's direction already set to input, refusing to change\n");
return -EPERM;
@@ -240,8 +241,14 @@ static int gpio_shared_proxy_get_direction(struct gpio_chip *gc,
unsigned int offset)
{
struct gpio_shared_proxy_data *proxy = gpiochip_get_data(gc);
+ struct gpio_shared_desc *shared_desc = proxy->shared_desc;
+
+ guard(mutex)(&shared_desc->mutex);
+
+ if (shared_desc->dir < 0)
+ return gpiod_get_direction(shared_desc->desc);
- return gpiod_get_direction(proxy->shared_desc->desc);
+ return shared_desc->dir;
}
static int gpio_shared_proxy_to_irq(struct gpio_chip *gc, unsigned int offset)
diff --git a/drivers/gpio/gpiolib-shared.c b/drivers/gpio/gpiolib-shared.c
index 5f9623e40b0f..8267b5089244 100644
--- a/drivers/gpio/gpiolib-shared.c
+++ b/drivers/gpio/gpiolib-shared.c
@@ -661,6 +661,7 @@ gpiod_shared_desc_create(struct gpio_shared_entry *entry)
}
shared_desc->desc = &gdev->descs[entry->offset];
+ shared_desc->dir = -1;
mutex_init(&shared_desc->mutex);
return shared_desc;
diff --git a/drivers/gpio/gpiolib-shared.h b/drivers/gpio/gpiolib-shared.h
index 618756f6c6aa..0ef3bc7a7916 100644
--- a/drivers/gpio/gpiolib-shared.h
+++ b/drivers/gpio/gpiolib-shared.h
@@ -43,6 +43,7 @@ struct gpio_shared_desc {
unsigned int usecnt;
unsigned int votecnt;
int def_val;
+ int dir; /* GPIO_LINE_DIRECTION_* as configured by the proxies, -1 if unset */
struct mutex mutex; /* serializes all proxy operations on this descriptor */
};
--
2.43.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/2] pinctrl: qcom: spmi-gpio: make direction changes exclusive
2026-09-15 1:44 [PATCH 0/2] Fix sharing a PMIC GPIO between multiple consumers Shawn Guo
2026-09-15 1:44 ` [PATCH 1/2] gpio: shared-proxy: track direction instead of reading it back Shawn Guo
@ 2026-09-15 1:44 ` Shawn Guo
2026-09-15 13:11 ` Neil Armstrong
2026-09-16 8:49 ` [PATCH 0/2] Fix sharing a PMIC GPIO between multiple consumers Bartosz Golaszewski
2 siblings, 1 reply; 10+ messages in thread
From: Shawn Guo @ 2026-09-15 1:44 UTC (permalink / raw)
To: Linus Walleij
Cc: Bartosz Golaszewski, Bjorn Andersson, Yu Zhang, linux-gpio,
linux-arm-msm, linux-kernel, Shawn Guo
pmic_gpio_populate() seeds pad->input_enabled and pad->output_enabled from
the hardware MODE_CTL register, so a pad left in DIGITAL_INPUT or
DIGITAL_INPUT_OUTPUT mode by the bootloader starts out with the input
buffer enabled. Neither direction callback clears the opposite buffer:
.direction_output() only packs PIN_CONFIG_LEVEL, which sets
output_enabled, and .direction_input() only packs PIN_CONFIG_INPUT_ENABLE,
which sets input_enabled. Requesting either direction on such a pad
therefore programs MODE_DIGITAL_INPUT_OUTPUT rather than the requested
direction.
That silently breaks both directions. After gpiod_direction_input() the
pad keeps driving the line, since the output buffer is never disabled.
And after gpiod_direction_output() pmic_gpio_get_direction() still reports
GPIO_LINE_DIRECTION_IN, because it cannot tell plain input from
input+output, which makes gpiolib consider the line an input while the
driver is driving it. On a board where several regulator-fixed nodes
share one PMIC GPIO the shared GPIO proxy reads that direction back and
rejects every consumer after the first:
reg-fixed-voltage regulator-wcn-core-vm-1p35: setup of GPIO (default) failed: -1
reg-fixed-voltage regulator-wcn-core-vm-1p35: error -EPERM: can't get GPIO
Pack the opposite buffer's PIN_CONFIG_*_ENABLE along with the requested
direction so that the resulting MODE_CTL is DIGITAL_INPUT or
DIGITAL_OUTPUT, never both. pmic_gpio_config_set() programs the registers
once after walking all configs, so this stays a single register write.
Pads that are genuinely bidirectional can still be described that way
through pinconf, which is the interface that has always been able to
express it; the gpiolib direction callbacks now mean what gpiolib says
they mean.
Assisted-by: LLM
Fixes: eadff3024472 ("pinctrl: Qualcomm SPMI PMIC GPIO pin controller driver")
Signed-off-by: Shawn Guo <shengchao.guo@oss.qualcomm.com>
---
drivers/pinctrl/qcom/pinctrl-spmi-gpio.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
index f6dc43e27b38..eb4431591331 100644
--- a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
+++ b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
@@ -741,22 +741,26 @@ static int pmic_gpio_get_direction(struct gpio_chip *chip, unsigned pin)
static int pmic_gpio_direction_input(struct gpio_chip *chip, unsigned pin)
{
struct pmic_gpio_state *state = gpiochip_get_data(chip);
- unsigned long config;
+ unsigned long configs[2];
- config = pinconf_to_config_packed(PIN_CONFIG_INPUT_ENABLE, 1);
+ configs[0] = pinconf_to_config_packed(PIN_CONFIG_OUTPUT_ENABLE, 0);
+ configs[1] = pinconf_to_config_packed(PIN_CONFIG_INPUT_ENABLE, 1);
- return pmic_gpio_config_set(state->ctrl, pin, &config, 1);
+ return pmic_gpio_config_set(state->ctrl, pin, configs,
+ ARRAY_SIZE(configs));
}
static int pmic_gpio_direction_output(struct gpio_chip *chip,
unsigned pin, int val)
{
struct pmic_gpio_state *state = gpiochip_get_data(chip);
- unsigned long config;
+ unsigned long configs[2];
- config = pinconf_to_config_packed(PIN_CONFIG_LEVEL, val);
+ configs[0] = pinconf_to_config_packed(PIN_CONFIG_INPUT_ENABLE, 0);
+ configs[1] = pinconf_to_config_packed(PIN_CONFIG_LEVEL, val);
- return pmic_gpio_config_set(state->ctrl, pin, &config, 1);
+ return pmic_gpio_config_set(state->ctrl, pin, configs,
+ ARRAY_SIZE(configs));
}
static int pmic_gpio_get(struct gpio_chip *chip, unsigned pin)
--
2.43.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] pinctrl: qcom: spmi-gpio: make direction changes exclusive
2026-09-15 1:44 ` [PATCH 2/2] pinctrl: qcom: spmi-gpio: make direction changes exclusive Shawn Guo
@ 2026-09-15 13:11 ` Neil Armstrong
2026-09-18 1:11 ` Shawn Guo
0 siblings, 1 reply; 10+ messages in thread
From: Neil Armstrong @ 2026-09-15 13:11 UTC (permalink / raw)
To: Shawn Guo, Linus Walleij
Cc: Bartosz Golaszewski, Bjorn Andersson, Yu Zhang, linux-gpio,
linux-arm-msm, linux-kernel
On 9/15/26 03:44, Shawn Guo wrote:
> pmic_gpio_populate() seeds pad->input_enabled and pad->output_enabled from
> the hardware MODE_CTL register, so a pad left in DIGITAL_INPUT or
> DIGITAL_INPUT_OUTPUT mode by the bootloader starts out with the input
> buffer enabled. Neither direction callback clears the opposite buffer:
> .direction_output() only packs PIN_CONFIG_LEVEL, which sets
> output_enabled, and .direction_input() only packs PIN_CONFIG_INPUT_ENABLE,
> which sets input_enabled. Requesting either direction on such a pad
> therefore programs MODE_DIGITAL_INPUT_OUTPUT rather than the requested
> direction.
>
> That silently breaks both directions. After gpiod_direction_input() the
> pad keeps driving the line, since the output buffer is never disabled.
> And after gpiod_direction_output() pmic_gpio_get_direction() still reports
> GPIO_LINE_DIRECTION_IN, because it cannot tell plain input from
> input+output, which makes gpiolib consider the line an input while the
> driver is driving it. On a board where several regulator-fixed nodes
> share one PMIC GPIO the shared GPIO proxy reads that direction back and
> rejects every consumer after the first:
>
> reg-fixed-voltage regulator-wcn-core-vm-1p35: setup of GPIO (default) failed: -1
> reg-fixed-voltage regulator-wcn-core-vm-1p35: error -EPERM: can't get GPIO
>
> Pack the opposite buffer's PIN_CONFIG_*_ENABLE along with the requested
> direction so that the resulting MODE_CTL is DIGITAL_INPUT or
> DIGITAL_OUTPUT, never both. pmic_gpio_config_set() programs the registers
> once after walking all configs, so this stays a single register write.
>
> Pads that are genuinely bidirectional can still be described that way
> through pinconf, which is the interface that has always been able to
> express it; the gpiolib direction callbacks now mean what gpiolib says
> they mean.
>
> Assisted-by: LLM
> Fixes: eadff3024472 ("pinctrl: Qualcomm SPMI PMIC GPIO pin controller driver")
You should add:
Fixes: 263447532463 ("pinctrl: qcom: spmi-gpio: implement .get_direction()")
Since my change added the get_direction callback.
Personally I would prefer patch 1 instead of this change.
Neil
> Signed-off-by: Shawn Guo <shengchao.guo@oss.qualcomm.com>
> ---
> drivers/pinctrl/qcom/pinctrl-spmi-gpio.c | 16 ++++++++++------
> 1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
> index f6dc43e27b38..eb4431591331 100644
> --- a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
> +++ b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
> @@ -741,22 +741,26 @@ static int pmic_gpio_get_direction(struct gpio_chip *chip, unsigned pin)
> static int pmic_gpio_direction_input(struct gpio_chip *chip, unsigned pin)
> {
> struct pmic_gpio_state *state = gpiochip_get_data(chip);
> - unsigned long config;
> + unsigned long configs[2];
>
> - config = pinconf_to_config_packed(PIN_CONFIG_INPUT_ENABLE, 1);
> + configs[0] = pinconf_to_config_packed(PIN_CONFIG_OUTPUT_ENABLE, 0);
> + configs[1] = pinconf_to_config_packed(PIN_CONFIG_INPUT_ENABLE, 1);
>
> - return pmic_gpio_config_set(state->ctrl, pin, &config, 1);
> + return pmic_gpio_config_set(state->ctrl, pin, configs,
> + ARRAY_SIZE(configs));
> }
>
> static int pmic_gpio_direction_output(struct gpio_chip *chip,
> unsigned pin, int val)
> {
> struct pmic_gpio_state *state = gpiochip_get_data(chip);
> - unsigned long config;
> + unsigned long configs[2];
>
> - config = pinconf_to_config_packed(PIN_CONFIG_LEVEL, val);
> + configs[0] = pinconf_to_config_packed(PIN_CONFIG_INPUT_ENABLE, 0);
> + configs[1] = pinconf_to_config_packed(PIN_CONFIG_LEVEL, val);
>
> - return pmic_gpio_config_set(state->ctrl, pin, &config, 1);
> + return pmic_gpio_config_set(state->ctrl, pin, configs,
> + ARRAY_SIZE(configs));
> }
>
> static int pmic_gpio_get(struct gpio_chip *chip, unsigned pin)
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] gpio: shared-proxy: track direction instead of reading it back
2026-09-15 1:44 ` [PATCH 1/2] gpio: shared-proxy: track direction instead of reading it back Shawn Guo
@ 2026-09-16 8:48 ` Bartosz Golaszewski
2026-09-16 9:00 ` Bartosz Golaszewski
1 sibling, 0 replies; 10+ messages in thread
From: Bartosz Golaszewski @ 2026-09-16 8:48 UTC (permalink / raw)
To: Shawn Guo
Cc: Linus Walleij, Bartosz Golaszewski, Bjorn Andersson, Yu Zhang,
linux-gpio, linux-arm-msm, linux-kernel
On Tue, 15 Sep 2026 03:44:46 +0200, Shawn Guo
<shengchao.guo@oss.qualcomm.com> said:
> On boards where more than one consumer shares a Qualcomm SPMI PMIC GPIO,
> all but the first consumer fail to configure the line as output:
>
> reg-fixed-voltage regulator-wcn-core-vm-1p35: setup of GPIO (default) failed: -1
> reg-fixed-voltage regulator-wcn-core-vm-1p35: error -EPERM: can't get GPIO
>
> The first consumer takes the usecnt == 1 branch and really does set the
> underlying line to output. Later consumers instead read the direction
> back with gpiod_get_direction() and refuse with -EPERM if it reports
> input. pmic_gpio_get_direction() derives the direction from the pad's
> input buffer (pad->input_enabled), which pmic_gpio_direction_output()
> never clears, so a pad whose input buffer is enabled at power-up reports
> input forever and the proxy rejects every consumer after the first.
>
Do we still even need this with patch 2/2 applied?
Bart
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/2] Fix sharing a PMIC GPIO between multiple consumers
2026-09-15 1:44 [PATCH 0/2] Fix sharing a PMIC GPIO between multiple consumers Shawn Guo
2026-09-15 1:44 ` [PATCH 1/2] gpio: shared-proxy: track direction instead of reading it back Shawn Guo
2026-09-15 1:44 ` [PATCH 2/2] pinctrl: qcom: spmi-gpio: make direction changes exclusive Shawn Guo
@ 2026-09-16 8:49 ` Bartosz Golaszewski
2 siblings, 0 replies; 10+ messages in thread
From: Bartosz Golaszewski @ 2026-09-16 8:49 UTC (permalink / raw)
To: Shawn Guo
Cc: Linus Walleij, Bartosz Golaszewski, Bjorn Andersson, Yu Zhang,
linux-gpio, linux-arm-msm, linux-kernel
On Tue, 15 Sep 2026 03:44:45 +0200, Shawn Guo
<shengchao.guo@oss.qualcomm.com> said:
> On the Qualcomm Nord boards, three discrete regulators feeding a WCN7850
> module are gated by a single PMIC GPIO (pmau0102_e GPIO6), so the board DT
> describes three regulator-fixed nodes pointing at the same GPIO. Only the
> first of them probes:
>
> reg-fixed-voltage regulator-wcn-core-vm-1p35: setup of GPIO (default) failed: -1
> reg-fixed-voltage regulator-wcn-core-vm-1p35: error -EPERM: can't get GPIO
> platform wcn7850-pmu: deferred probe pending: supplier regulator-wcn-core-vh-1p95 not ready
>
> ARCH_QCOM selects HAVE_SHARED_GPIOS, so gpiolib creates shared GPIO proxies
> for the line rather than taking the legacy nonexclusive path. The failure
> turns out to have two independent causes, one on each side of that
> interface.
>
> On the provider side, neither of spmi-gpio's direction callbacks clears the
> opposite buffer: .direction_output() only sets output_enabled and
> .direction_input() only sets input_enabled. A pad left in DIGITAL_INPUT
> mode by the bootloader therefore ends up in DIGITAL_INPUT_OUTPUT mode on
> the first direction change, after which .get_direction() reports
> GPIO_LINE_DIRECTION_IN even though the driver is driving the line. The
> same asymmetry means gpiod_direction_input() never stops driving a pad that
> was previously an output, which is a bug in its own right.
>
> On the consumer side, gpio_shared_proxy_direction_output() reads the
> direction back for every requester after the first and refuses with -EPERM
> if it sees an input. The readback is not needed, since the proxy is the
> only entity configuring the line and already knows which direction it
> asked for. It is also not robust: a provider without .get_direction() at
> all makes gpiochip_get_direction() return -EOPNOTSUPP under a WARN_ON,
> which the proxy propagates just the same.
>
> Either patch on its own is enough to make the board work; both are sent
> because both describe real bugs. Patch 1 is what makes shared GPIOs work
> irrespective of the provider, so I would consider it the more important of
> the two. There is no build or apply dependency between them and they
> touch different subsystems, so they can go through separate trees.
>
I responded to patch 1/2 before reading this bit. Ok, I'm not against 1/2,
I'll queue it for fixes.
Bart
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] gpio: shared-proxy: track direction instead of reading it back
2026-09-15 1:44 ` [PATCH 1/2] gpio: shared-proxy: track direction instead of reading it back Shawn Guo
2026-09-16 8:48 ` Bartosz Golaszewski
@ 2026-09-16 9:00 ` Bartosz Golaszewski
2026-09-18 14:43 ` Shawn Guo
1 sibling, 1 reply; 10+ messages in thread
From: Bartosz Golaszewski @ 2026-09-16 9:00 UTC (permalink / raw)
To: Shawn Guo
Cc: Bartosz Golaszewski, Bjorn Andersson, Yu Zhang, linux-gpio,
linux-arm-msm, linux-kernel, Linus Walleij
On Tue, 15 Sep 2026 03:44:46 +0200, Shawn Guo
<shengchao.guo@oss.qualcomm.com> said:
> On boards where more than one consumer shares a Qualcomm SPMI PMIC GPIO,
> all but the first consumer fail to configure the line as output:
>
> reg-fixed-voltage regulator-wcn-core-vm-1p35: setup of GPIO (default) failed: -1
> reg-fixed-voltage regulator-wcn-core-vm-1p35: error -EPERM: can't get GPIO
>
> The first consumer takes the usecnt == 1 branch and really does set the
> underlying line to output. Later consumers instead read the direction
> back with gpiod_get_direction() and refuse with -EPERM if it reports
> input. pmic_gpio_get_direction() derives the direction from the pad's
> input buffer (pad->input_enabled), which pmic_gpio_direction_output()
> never clears, so a pad whose input buffer is enabled at power-up reports
> input forever and the proxy rejects every consumer after the first.
>
> The readback is not needed: the proxy is the only entity configuring the
> line, so it already knows which direction it asked for. Record that in
> struct gpio_shared_desc and compare against the recorded value. Keep a
> readback in the get_direction() callback for the case where no proxy has
> configured the line yet, and reset the recorded direction once the last
> user goes away so that the next requester establishes it again.
>
> Assisted-by: LLM
> Fixes: e992d54c6f97 ("gpio: shared-proxy: implement the shared GPIO proxy driver")
> Signed-off-by: Shawn Guo <shengchao.guo@oss.qualcomm.com>
> ---
I think sashiko is correct about the GPIOD_ASIS use-case. I think this is why
I used an unconditional read-back in the first place actually.
Bart
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] pinctrl: qcom: spmi-gpio: make direction changes exclusive
2026-09-15 13:11 ` Neil Armstrong
@ 2026-09-18 1:11 ` Shawn Guo
2026-09-18 8:05 ` Bartosz Golaszewski
0 siblings, 1 reply; 10+ messages in thread
From: Shawn Guo @ 2026-09-18 1:11 UTC (permalink / raw)
To: Neil Armstrong
Cc: Linus Walleij, Bartosz Golaszewski, Bjorn Andersson, Yu Zhang,
linux-gpio, linux-arm-msm, linux-kernel
On Tue, Sep 15, 2026 at 03:11:48PM +0200, Neil Armstrong wrote:
> On 9/15/26 03:44, Shawn Guo wrote:
> > pmic_gpio_populate() seeds pad->input_enabled and pad->output_enabled from
> > the hardware MODE_CTL register, so a pad left in DIGITAL_INPUT or
> > DIGITAL_INPUT_OUTPUT mode by the bootloader starts out with the input
> > buffer enabled. Neither direction callback clears the opposite buffer:
> > .direction_output() only packs PIN_CONFIG_LEVEL, which sets
> > output_enabled, and .direction_input() only packs PIN_CONFIG_INPUT_ENABLE,
> > which sets input_enabled. Requesting either direction on such a pad
> > therefore programs MODE_DIGITAL_INPUT_OUTPUT rather than the requested
> > direction.
> >
> > That silently breaks both directions. After gpiod_direction_input() the
> > pad keeps driving the line, since the output buffer is never disabled.
> > And after gpiod_direction_output() pmic_gpio_get_direction() still reports
> > GPIO_LINE_DIRECTION_IN, because it cannot tell plain input from
> > input+output, which makes gpiolib consider the line an input while the
> > driver is driving it. On a board where several regulator-fixed nodes
> > share one PMIC GPIO the shared GPIO proxy reads that direction back and
> > rejects every consumer after the first:
> >
> > reg-fixed-voltage regulator-wcn-core-vm-1p35: setup of GPIO (default) failed: -1
> > reg-fixed-voltage regulator-wcn-core-vm-1p35: error -EPERM: can't get GPIO
> >
> > Pack the opposite buffer's PIN_CONFIG_*_ENABLE along with the requested
> > direction so that the resulting MODE_CTL is DIGITAL_INPUT or
> > DIGITAL_OUTPUT, never both. pmic_gpio_config_set() programs the registers
> > once after walking all configs, so this stays a single register write.
> >
> > Pads that are genuinely bidirectional can still be described that way
> > through pinconf, which is the interface that has always been able to
> > express it; the gpiolib direction callbacks now mean what gpiolib says
> > they mean.
> >
> > Assisted-by: LLM
> > Fixes: eadff3024472 ("pinctrl: Qualcomm SPMI PMIC GPIO pin controller driver")
>
> You should add:
> Fixes: 263447532463 ("pinctrl: qcom: spmi-gpio: implement .get_direction()")
>
> Since my change added the get_direction callback.
Indeed!
>
> Personally I would prefer patch 1 instead of this change.
As I mentioned in the cover letter, both patches are fixing real bugs
existing at different level. So I prefer to merge both. But it's a call
up to Bartosz.
Shawn
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] pinctrl: qcom: spmi-gpio: make direction changes exclusive
2026-09-18 1:11 ` Shawn Guo
@ 2026-09-18 8:05 ` Bartosz Golaszewski
0 siblings, 0 replies; 10+ messages in thread
From: Bartosz Golaszewski @ 2026-09-18 8:05 UTC (permalink / raw)
To: Shawn Guo
Cc: Linus Walleij, Bartosz Golaszewski, Bjorn Andersson, Yu Zhang,
linux-gpio, linux-arm-msm, linux-kernel, Neil Armstrong
On Fri, 18 Sep 2026 03:11:15 +0200, Shawn Guo
<shengchao.guo@oss.qualcomm.com> said:
>>
>> Personally I would prefer patch 1 instead of this change.
>
> As I mentioned in the cover letter, both patches are fixing real bugs
> existing at different level. So I prefer to merge both. But it's a call
> up to Bartosz.
>
I'd take both but I'm not sure if the ASIS case for shared proxy can be
addressed without reading back the direction.
Bart
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] gpio: shared-proxy: track direction instead of reading it back
2026-09-16 9:00 ` Bartosz Golaszewski
@ 2026-09-18 14:43 ` Shawn Guo
0 siblings, 0 replies; 10+ messages in thread
From: Shawn Guo @ 2026-09-18 14:43 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Bjorn Andersson, Yu Zhang, linux-gpio, linux-arm-msm,
linux-kernel, Linus Walleij
On Wed, Sep 16, 2026 at 04:00:01AM -0500, Bartosz Golaszewski wrote:
> On Tue, 15 Sep 2026 03:44:46 +0200, Shawn Guo
> <shengchao.guo@oss.qualcomm.com> said:
> > On boards where more than one consumer shares a Qualcomm SPMI PMIC GPIO,
> > all but the first consumer fail to configure the line as output:
> >
> > reg-fixed-voltage regulator-wcn-core-vm-1p35: setup of GPIO (default) failed: -1
> > reg-fixed-voltage regulator-wcn-core-vm-1p35: error -EPERM: can't get GPIO
> >
> > The first consumer takes the usecnt == 1 branch and really does set the
> > underlying line to output. Later consumers instead read the direction
> > back with gpiod_get_direction() and refuse with -EPERM if it reports
> > input. pmic_gpio_get_direction() derives the direction from the pad's
> > input buffer (pad->input_enabled), which pmic_gpio_direction_output()
> > never clears, so a pad whose input buffer is enabled at power-up reports
> > input forever and the proxy rejects every consumer after the first.
> >
> > The readback is not needed: the proxy is the only entity configuring the
> > line, so it already knows which direction it asked for. Record that in
> > struct gpio_shared_desc and compare against the recorded value. Keep a
> > readback in the get_direction() callback for the case where no proxy has
> > configured the line yet, and reset the recorded direction once the last
> > user goes away so that the next requester establishes it again.
> >
> > Assisted-by: LLM
> > Fixes: e992d54c6f97 ("gpio: shared-proxy: implement the shared GPIO proxy driver")
> > Signed-off-by: Shawn Guo <shengchao.guo@oss.qualcomm.com>
> > ---
>
> I think sashiko is correct about the GPIOD_ASIS use-case.
I didn't receive any comments from sashiko. Could you point me to it?
Shawn
> I think this is why
> I used an unconditional read-back in the first place actually.
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2026-09-18 14:44 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-15 1:44 [PATCH 0/2] Fix sharing a PMIC GPIO between multiple consumers Shawn Guo
2026-09-15 1:44 ` [PATCH 1/2] gpio: shared-proxy: track direction instead of reading it back Shawn Guo
2026-09-16 8:48 ` Bartosz Golaszewski
2026-09-16 9:00 ` Bartosz Golaszewski
2026-09-18 14:43 ` Shawn Guo
2026-09-15 1:44 ` [PATCH 2/2] pinctrl: qcom: spmi-gpio: make direction changes exclusive Shawn Guo
2026-09-15 13:11 ` Neil Armstrong
2026-09-18 1:11 ` Shawn Guo
2026-09-18 8:05 ` Bartosz Golaszewski
2026-09-16 8:49 ` [PATCH 0/2] Fix sharing a PMIC GPIO between multiple consumers Bartosz Golaszewski
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®