gpio: reinforce desc->flags handling
authorBartosz Golaszewski <bartosz.golaszewski@linaro.org>
Tue, 23 Jan 2024 13:26:09 +0000 (14:26 +0100)
committerBartosz Golaszewski <bartosz.golaszewski@linaro.org>
Mon, 12 Feb 2024 09:50:49 +0000 (10:50 +0100)
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>
Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
drivers/gpio/gpiolib.c

index 04b4636baff18909ed82f7031cd61518a34c055e..cfbaf47b47d26d9944ee5c027e2934433952e776 100644 (file)
@@ -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) ?