diff mbox series

[05/22] gpio: don't set label from irq helpers

Message ID 20240130124828.14678-6-brgl@bgdev.pl
State Superseded
Headers show
Series gpio: rework locking and object life-time control | expand

Commit Message

Bartosz Golaszewski Jan. 30, 2024, 12:48 p.m. UTC 
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

We will soon serialize access to the descriptor label using SRCU. The
write-side of the protection will require calling synchronize_srcu()
which must not be called from atomic context. We have two irq helpers:
gpiochip_lock_as_irq() and gpiochip_unlock_as_irq() that set the label
if the GPIO is not requested but is being used as interrupt. They are
called with a spinlock held from the interrupt subsystem.

They must not do it if we are to use SRCU so instead let's move the
special corner case to a dedicated getter.

Let's use the flags of the descriptor to figure out whether we should
use the special "interrupt" label.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 drivers/gpio/gpiolib.c | 21 ++++++++-------------
 1 file changed, 8 insertions(+), 13 deletions(-)

Comments

Linus Walleij Jan. 31, 2024, 7:38 p.m. UTC | #1 
On Tue, Jan 30, 2024 at 1:48 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:

> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>
> We will soon serialize access to the descriptor label using SRCU. The
> write-side of the protection will require calling synchronize_srcu()
> which must not be called from atomic context. We have two irq helpers:
> gpiochip_lock_as_irq() and gpiochip_unlock_as_irq() that set the label
> if the GPIO is not requested but is being used as interrupt. They are
> called with a spinlock held from the interrupt subsystem.
>
> They must not do it if we are to use SRCU so instead let's move the
> special corner case to a dedicated getter.
>
> Let's use the flags of the descriptor to figure out whether we should
> use the special "interrupt" label.
>
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

I would refine the commit message: what you do IIUC is that you
simply avoid modifying a string, the label isn't set anymore,
instead a const string is returned and it is selected from the
state of an atomic variable (ha! smart!) and that is how the
atomicity is achieved.

Anyway:
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij
diff mbox series

Patch

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index d0a2f014dacd..4e6b26b3febb 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -107,7 +107,14 @@  static bool gpiolib_initialized;
 
 const char *gpiod_get_label(struct gpio_desc *desc)
 {
-	return desc->label;
+	unsigned long flags;
+
+	flags = READ_ONCE(desc->flags);
+	if (test_bit(FLAG_USED_AS_IRQ, &flags) &&
+	    !test_bit(FLAG_REQUESTED, &flags))
+		return "interrupt";
+
+	return test_bit(FLAG_REQUESTED, &flags) ? desc->label : NULL;
 }
 
 static inline void desc_set_label(struct gpio_desc *d, const char *label)
@@ -3590,14 +3597,6 @@  int gpiochip_lock_as_irq(struct gpio_chip *gc, unsigned int offset)
 	set_bit(FLAG_USED_AS_IRQ, &desc->flags);
 	set_bit(FLAG_IRQ_IS_ENABLED, &desc->flags);
 
-	/*
-	 * If the consumer has not set up a label (such as when the
-	 * IRQ is referenced from .to_irq()) we set up a label here
-	 * so it is clear this is used as an interrupt.
-	 */
-	if (!desc->label)
-		desc_set_label(desc, "interrupt");
-
 	return 0;
 }
 EXPORT_SYMBOL_GPL(gpiochip_lock_as_irq);
@@ -3620,10 +3619,6 @@  void gpiochip_unlock_as_irq(struct gpio_chip *gc, unsigned int offset)
 
 	clear_bit(FLAG_USED_AS_IRQ, &desc->flags);
 	clear_bit(FLAG_IRQ_IS_ENABLED, &desc->flags);
-
-	/* If we only had this marking, erase it */
-	if (desc->label && !strcmp(desc->label, "interrupt"))
-		desc_set_label(desc, NULL);
 }
 EXPORT_SYMBOL_GPL(gpiochip_unlock_as_irq);