[PATCH] gpio: shared: propagate configuration to pinctrl

Bartosz Golaszewski posted 1 patch 2 weeks, 3 days ago
drivers/gpio/gpiolib-shared.c | 16 +++++++++++-----
drivers/gpio/gpiolib.c        |  4 ++--
drivers/gpio/gpiolib.h        |  2 ++
3 files changed, 15 insertions(+), 7 deletions(-)
[PATCH] gpio: shared: propagate configuration to pinctrl
Posted by Bartosz Golaszewski 2 weeks, 3 days ago
Just toggling the descriptor's "requested" flag is not enough. We need
to properly request it in order to potentially propagate any
configuration to pinctrl via the .request() callback.

We must not take the reference to the device at this point (the device
is not ready but we're also requesting the device's own descriptor) so
make the _commit() variants of request and free functions available to
GPIO core in order to use them instead of their regular counterparts.

This fixes an audio issue reported on one of the Qualcomm platforms.

Fixes: a060b8c511ab ("gpiolib: implement low-level, shared GPIO support")
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
 drivers/gpio/gpiolib-shared.c | 16 +++++++++++-----
 drivers/gpio/gpiolib.c        |  4 ++--
 drivers/gpio/gpiolib.h        |  2 ++
 3 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/drivers/gpio/gpiolib-shared.c b/drivers/gpio/gpiolib-shared.c
index 17343fdc9758..9e6544203439 100644
--- a/drivers/gpio/gpiolib-shared.c
+++ b/drivers/gpio/gpiolib-shared.c
@@ -515,7 +515,7 @@ int gpio_device_setup_shared(struct gpio_device *gdev)
 {
 	struct gpio_shared_entry *entry;
 	struct gpio_shared_ref *ref;
-	unsigned long *flags;
+	struct gpio_desc *desc;
 	int ret;
 
 	list_for_each_entry(entry, &gpio_shared_list, list) {
@@ -543,15 +543,17 @@ int gpio_device_setup_shared(struct gpio_device *gdev)
 		if (list_count_nodes(&entry->refs) <= 1)
 			continue;
 
-		flags = &gdev->descs[entry->offset].flags;
+		desc = &gdev->descs[entry->offset];
 
-		__set_bit(GPIOD_FLAG_SHARED, flags);
+		__set_bit(GPIOD_FLAG_SHARED, &desc->flags);
 		/*
 		 * Shared GPIOs are not requested via the normal path. Make
 		 * them inaccessible to anyone even before we register the
 		 * chip.
 		 */
-		__set_bit(GPIOD_FLAG_REQUESTED, flags);
+		ret = gpiod_request_commit(desc, "shared");
+		if (ret)
+			return ret;
 
 		pr_debug("GPIO %u owned by %s is shared by multiple consumers\n",
 			 entry->offset, gpio_device_get_label(gdev));
@@ -562,8 +564,10 @@ int gpio_device_setup_shared(struct gpio_device *gdev)
 				 ref->con_id ?: "(none)");
 
 			ret = gpio_shared_make_adev(gdev, entry, ref);
-			if (ret)
+			if (ret) {
+				gpiod_free_commit(desc);
 				return ret;
+			}
 		}
 	}
 
@@ -579,6 +583,8 @@ void gpio_device_teardown_shared(struct gpio_device *gdev)
 		if (!device_match_fwnode(&gdev->dev, entry->fwnode))
 			continue;
 
+		gpiod_free_commit(&gdev->descs[entry->offset]);
+
 		list_for_each_entry(ref, &entry->refs, list) {
 			guard(mutex)(&ref->lock);
 
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index fe2d107b0a84..1578cf3a8c74 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -2453,7 +2453,7 @@ EXPORT_SYMBOL_GPL(gpiochip_remove_pin_ranges);
  * on each other, and help provide better diagnostics in debugfs.
  * They're called even less than the "set direction" calls.
  */
-static int gpiod_request_commit(struct gpio_desc *desc, const char *label)
+int gpiod_request_commit(struct gpio_desc *desc, const char *label)
 {
 	unsigned int offset;
 	int ret;
@@ -2515,7 +2515,7 @@ int gpiod_request(struct gpio_desc *desc, const char *label)
 	return ret;
 }
 
-static void gpiod_free_commit(struct gpio_desc *desc)
+void gpiod_free_commit(struct gpio_desc *desc)
 {
 	unsigned long flags;
 
diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
index 77f6f2936dc2..3abb90385829 100644
--- a/drivers/gpio/gpiolib.h
+++ b/drivers/gpio/gpiolib.h
@@ -244,7 +244,9 @@ DEFINE_CLASS(gpio_chip_guard,
 	     struct gpio_desc *desc)
 
 int gpiod_request(struct gpio_desc *desc, const char *label);
+int gpiod_request_commit(struct gpio_desc *desc, const char *label);
 void gpiod_free(struct gpio_desc *desc);
+void gpiod_free_commit(struct gpio_desc *desc);
 
 static inline int gpiod_request_user(struct gpio_desc *desc, const char *label)
 {
-- 
2.47.3
Re: [PATCH] gpio: shared: propagate configuration to pinctrl
Posted by Bartosz Golaszewski 2 weeks, 2 days ago
On Tue, 20 Jan 2026 16:49:13 +0100, Bartosz Golaszewski wrote:
> Just toggling the descriptor's "requested" flag is not enough. We need
> to properly request it in order to potentially propagate any
> configuration to pinctrl via the .request() callback.
> 
> We must not take the reference to the device at this point (the device
> is not ready but we're also requesting the device's own descriptor) so
> make the _commit() variants of request and free functions available to
> GPIO core in order to use them instead of their regular counterparts.
> 
> [...]

Applied, thanks!

[1/1] gpio: shared: propagate configuration to pinctrl
      commit: 4918cc05137cb347686462923ab3fd249ef7899d

Best regards,
-- 
Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
Re: [PATCH] gpio: shared: propagate configuration to pinctrl
Posted by Linus Walleij 2 weeks, 2 days ago
On Tue, Jan 20, 2026 at 4:49 PM Bartosz Golaszewski
<bartosz.golaszewski@oss.qualcomm.com> wrote:

> Just toggling the descriptor's "requested" flag is not enough. We need
> to properly request it in order to potentially propagate any
> configuration to pinctrl via the .request() callback.
>
> We must not take the reference to the device at this point (the device
> is not ready but we're also requesting the device's own descriptor) so
> make the _commit() variants of request and free functions available to
> GPIO core in order to use them instead of their regular counterparts.
>
> This fixes an audio issue reported on one of the Qualcomm platforms.
>
> Fixes: a060b8c511ab ("gpiolib: implement low-level, shared GPIO support")
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>

I don't quite understand all semantics but I guess it works
like this:  gpiod_request_commit() is only called once, during
proxy device creation.

If I understand it correct:
Reviewed-by: Linus Walleij <linusw@kernel.org>

Yours,
Linus Walleij
Re: [PATCH] gpio: shared: propagate configuration to pinctrl
Posted by Bartosz Golaszewski 2 weeks, 2 days ago
On Wed, Jan 21, 2026 at 1:10 PM Linus Walleij <linusw@kernel.org> wrote:
>
> On Tue, Jan 20, 2026 at 4:49 PM Bartosz Golaszewski
> <bartosz.golaszewski@oss.qualcomm.com> wrote:
>
> > Just toggling the descriptor's "requested" flag is not enough. We need
> > to properly request it in order to potentially propagate any
> > configuration to pinctrl via the .request() callback.
> >
> > We must not take the reference to the device at this point (the device
> > is not ready but we're also requesting the device's own descriptor) so
> > make the _commit() variants of request and free functions available to
> > GPIO core in order to use them instead of their regular counterparts.
> >
> > This fixes an audio issue reported on one of the Qualcomm platforms.
> >
> > Fixes: a060b8c511ab ("gpiolib: implement low-level, shared GPIO support")
> > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
>
> I don't quite understand all semantics but I guess it works
> like this:  gpiod_request_commit() is only called once, during
> proxy device creation.
>
> If I understand it correct:
> Reviewed-by: Linus Walleij <linusw@kernel.org>
>

Yes, we effectively hog the pin and allow access to it only though the proxy.

Bartosz
Re: [PATCH] gpio: shared: propagate configuration to pinctrl
Posted by Ravi Hothi 2 weeks, 2 days ago

On 1/20/2026 9:19 PM, Bartosz Golaszewski wrote:
> Just toggling the descriptor's "requested" flag is not enough. We need
> to properly request it in order to potentially propagate any
> configuration to pinctrl via the .request() callback.
> 
> We must not take the reference to the device at this point (the device
> is not ready but we're also requesting the device's own descriptor) so
> make the _commit() variants of request and free functions available to
> GPIO core in order to use them instead of their regular counterparts.
> 
> This fixes an audio issue reported on one of the Qualcomm platforms.

Thanks for the patch, Issue got fixed now.
Tested-by: Ravi Hothi <ravi.hothi@oss.qualcomm.com>

Regards,
Ravi Hothi

> 
> Fixes: a060b8c511ab ("gpiolib: implement low-level, shared GPIO support")
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
> ---
>   drivers/gpio/gpiolib-shared.c | 16 +++++++++++-----
>   drivers/gpio/gpiolib.c        |  4 ++--
>   drivers/gpio/gpiolib.h        |  2 ++
>   3 files changed, 15 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpio/gpiolib-shared.c b/drivers/gpio/gpiolib-shared.c
> index 17343fdc9758..9e6544203439 100644
> --- a/drivers/gpio/gpiolib-shared.c
> +++ b/drivers/gpio/gpiolib-shared.c
> @@ -515,7 +515,7 @@ int gpio_device_setup_shared(struct gpio_device *gdev)
>   {
>   	struct gpio_shared_entry *entry;
>   	struct gpio_shared_ref *ref;
> -	unsigned long *flags;
> +	struct gpio_desc *desc;
>   	int ret;
>   
>   	list_for_each_entry(entry, &gpio_shared_list, list) {
> @@ -543,15 +543,17 @@ int gpio_device_setup_shared(struct gpio_device *gdev)
>   		if (list_count_nodes(&entry->refs) <= 1)
>   			continue;
>   
> -		flags = &gdev->descs[entry->offset].flags;
> +		desc = &gdev->descs[entry->offset];
>   
> -		__set_bit(GPIOD_FLAG_SHARED, flags);
> +		__set_bit(GPIOD_FLAG_SHARED, &desc->flags);
>   		/*
>   		 * Shared GPIOs are not requested via the normal path. Make
>   		 * them inaccessible to anyone even before we register the
>   		 * chip.
>   		 */
> -		__set_bit(GPIOD_FLAG_REQUESTED, flags);
> +		ret = gpiod_request_commit(desc, "shared");
> +		if (ret)
> +			return ret;
>   
>   		pr_debug("GPIO %u owned by %s is shared by multiple consumers\n",
>   			 entry->offset, gpio_device_get_label(gdev));
> @@ -562,8 +564,10 @@ int gpio_device_setup_shared(struct gpio_device *gdev)
>   				 ref->con_id ?: "(none)");
>   
>   			ret = gpio_shared_make_adev(gdev, entry, ref);
> -			if (ret)
> +			if (ret) {
> +				gpiod_free_commit(desc);
>   				return ret;
> +			}
>   		}
>   	}
>   
> @@ -579,6 +583,8 @@ void gpio_device_teardown_shared(struct gpio_device *gdev)
>   		if (!device_match_fwnode(&gdev->dev, entry->fwnode))
>   			continue;
>   
> +		gpiod_free_commit(&gdev->descs[entry->offset]);
> +
>   		list_for_each_entry(ref, &entry->refs, list) {
>   			guard(mutex)(&ref->lock);
>   
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index fe2d107b0a84..1578cf3a8c74 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -2453,7 +2453,7 @@ EXPORT_SYMBOL_GPL(gpiochip_remove_pin_ranges);
>    * on each other, and help provide better diagnostics in debugfs.
>    * They're called even less than the "set direction" calls.
>    */
> -static int gpiod_request_commit(struct gpio_desc *desc, const char *label)
> +int gpiod_request_commit(struct gpio_desc *desc, const char *label)
>   {
>   	unsigned int offset;
>   	int ret;
> @@ -2515,7 +2515,7 @@ int gpiod_request(struct gpio_desc *desc, const char *label)
>   	return ret;
>   }
>   
> -static void gpiod_free_commit(struct gpio_desc *desc)
> +void gpiod_free_commit(struct gpio_desc *desc)
>   {
>   	unsigned long flags;
>   
> diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
> index 77f6f2936dc2..3abb90385829 100644
> --- a/drivers/gpio/gpiolib.h
> +++ b/drivers/gpio/gpiolib.h
> @@ -244,7 +244,9 @@ DEFINE_CLASS(gpio_chip_guard,
>   	     struct gpio_desc *desc)
>   
>   int gpiod_request(struct gpio_desc *desc, const char *label);
> +int gpiod_request_commit(struct gpio_desc *desc, const char *label);
>   void gpiod_free(struct gpio_desc *desc);
> +void gpiod_free_commit(struct gpio_desc *desc);
>   
>   static inline int gpiod_request_user(struct gpio_desc *desc, const char *label)
>   {