[PATCH v3 10/24] gpio: reinforce desc->flags handling

Bartosz Golaszewski
2024-02-08T09:59:37Z

We now removed the gpio_lock spinlock and modified the places
previously protected by it to handle desc->flags access in a consistent
way. Let's improve other places that were previously unprotected by
reading the flags field of gpio_desc once and using the stored value for
logic consistency. If we need to modify the field, let's also write it
back once with a consistent value resulting from the function's logic.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/gpio/gpiolib.c | 42 +++++++++++++++++++++++++-----------------
1 file changed, 25 insertions(+), 17 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index d0de492acd4c..5e098a7816bd 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -336,18 +336,20 @@ static int gpiochip_find_base_unlocked(int ngpio)
int gpiod_get_direction(struct gpio_desc *desc)
{
struct gpio_chip *gc;
+ unsigned long flags;
unsigned int offset;
int ret;

gc = gpiod_to_chip(desc);
offset = gpio_chip_hwgpio(desc);
+ flags = READ_ONCE(desc->flags);

/*
* Open drain emulation using input mode may incorrectly report
* input here, fix that up.
*/
- if (test_bit(FLAG_OPEN_DRAIN, &desc->flags) &&
- test_bit(FLAG_IS_OUT, &desc->flags))
+ if (test_bit(FLAG_OPEN_DRAIN, &flags) &&
+ test_bit(FLAG_IS_OUT, &flags))
return 0;

if (!gc->get_direction)
@@ -361,7 +363,8 @@ int gpiod_get_direction(struct gpio_desc *desc)
if (ret > 0)
ret = 1;

- assign_bit(FLAG_IS_OUT, &desc->flags, !ret);
+ assign_bit(FLAG_IS_OUT, &flags, !ret);
+ WRITE_ONCE(desc->flags, flags);

return ret;
}
@@ -751,9 +754,6 @@ static void gpiochip_machine_hog(struct gpio_chip *gc, struct gpiod_hog *hog)
return;
}

- if (test_bit(FLAG_IS_HOGGED, &desc->flags))
- return;
-
rv = gpiod_hog(desc, hog->line_name, hog->lflags, hog->dflags);
if (rv)
gpiod_err(desc, "%s: unable to hog GPIO line (%s:%u): %d\n",
@@ -2528,13 +2528,16 @@ static int gpio_set_config(struct gpio_desc *desc, enum pin_config_param mode)
static int gpio_set_bias(struct gpio_desc *desc)
{
enum pin_config_param bias;
+ unsigned long flags;
unsigned int arg;

- if (test_bit(FLAG_BIAS_DISABLE, &desc->flags))
+ flags = READ_ONCE(desc->flags);
+
+ if (test_bit(FLAG_BIAS_DISABLE, &flags))
bias = PIN_CONFIG_BIAS_DISABLE;
- else if (test_bit(FLAG_PULL_UP, &desc->flags))
+ else if (test_bit(FLAG_PULL_UP, &flags))
bias = PIN_CONFIG_BIAS_PULL_UP;
- else if (test_bit(FLAG_PULL_DOWN, &desc->flags))
+ else if (test_bit(FLAG_PULL_DOWN, &flags))
bias = PIN_CONFIG_BIAS_PULL_DOWN;
else
return 0;
@@ -2700,24 +2703,28 @@ EXPORT_SYMBOL_GPL(gpiod_direction_output_raw);
*/
int gpiod_direction_output(struct gpio_desc *desc, int value)
{
+ unsigned long flags;
int ret;

VALIDATE_DESC(desc);
- if (test_bit(FLAG_ACTIVE_LOW, &desc->flags))
+
+ flags = READ_ONCE(desc->flags);
+
+ if (test_bit(FLAG_ACTIVE_LOW, &flags))
value = !value;
else
value = !!value;

/* GPIOs used for enabled IRQs shall not be set as output */
- if (test_bit(FLAG_USED_AS_IRQ, &desc->flags) &&
- test_bit(FLAG_IRQ_IS_ENABLED, &desc->flags)) {
+ if (test_bit(FLAG_USED_AS_IRQ, &flags) &&
+ test_bit(FLAG_IRQ_IS_ENABLED, &flags)) {
gpiod_err(desc,
"%s: tried to set a GPIO tied to an IRQ as output\n",
__func__);
return -EIO;
}

- if (test_bit(FLAG_OPEN_DRAIN, &desc->flags)) {
+ if (test_bit(FLAG_OPEN_DRAIN, &flags)) {
/* First see if we can enable open drain in hardware */
ret = gpio_set_config(desc, PIN_CONFIG_DRIVE_OPEN_DRAIN);
if (!ret)
@@ -2727,7 +2734,7 @@ int gpiod_direction_output(struct gpio_desc *desc, int value)
ret = gpiod_direction_input(desc);
goto set_output_flag;
}
- } else if (test_bit(FLAG_OPEN_SOURCE, &desc->flags)) {
+ } else if (test_bit(FLAG_OPEN_SOURCE, &flags)) {
ret = gpio_set_config(desc, PIN_CONFIG_DRIVE_OPEN_SOURCE);
if (!ret)
goto set_output_value;
@@ -4424,21 +4431,22 @@ int gpiod_hog(struct gpio_desc *desc, const char *name,
int hwnum;
int ret;

+ if (test_and_set_bit(FLAG_IS_HOGGED, &desc->flags))
+ return 0;
+
gc = gpiod_to_chip(desc);
hwnum = gpio_chip_hwgpio(desc);

local_desc = gpiochip_request_own_desc(gc, hwnum, name,
lflags, dflags);
if (IS_ERR(local_desc)) {
+ clear_bit(FLAG_IS_HOGGED, &desc->flags);
ret = PTR_ERR(local_desc);
pr_err("requesting hog GPIO %s (chip %s, offset %d) failed, %d\n",
name, gc->label, hwnum, ret);
return ret;
}

- /* Mark GPIO as hogged so it can be identified and removed later */
- set_bit(FLAG_IS_HOGGED, &desc->flags);
-
gpiod_dbg(desc, "hogged as %s%s\n",
(dflags & GPIOD_FLAGS_BIT_DIR_OUT) ? "output" : "input",
(dflags & GPIOD_FLAGS_BIT_DIR_OUT) ?
--
2.40.1