Merge tag 'gpio-fixes-for-v6.8-rc1' of git://git.kernel.org/pub/scm/linux/kernel...
authorLinus Torvalds <torvalds@linux-foundation.org>
Fri, 19 Jan 2024 01:03:51 +0000 (17:03 -0800)
committerLinus Torvalds <torvalds@linux-foundation.org>
Fri, 19 Jan 2024 01:03:51 +0000 (17:03 -0800)
Pull gpio fixes from Bartosz Golaszewski:
 "Apart from some regular driver fixes there's a relatively big revert
  of the locking changes that were introduced to GPIOLIB in this merge
  window.

  This is because it turned out that some legacy GPIO interfaces - that
  need to translate a number from the global GPIO numberspace to the
  address of the relevant descriptor, thus running a GPIO device lookup
  and taking the GPIO device list lock - are still used in old code from
  atomic context resulting in "scheduling while atomic" errors.

  I'll try to make the read-only part of the list access entirely
  lockless using SRCU but this will take some time so let's go back to
  the old global spinlock for now.

  Summary:

   - revert the changes aiming to use a read-write semaphore to protect
     the list of GPIO devices due to calls to legacy API taking that
     lock from atomic context in old code

   - fix inverted logic in DEFINE_FREE() for GPIO device references

   - check the return value of bgpio_init() in gpio-mlxbf3

   - fix node address in the DT bindings example for gpio-xilinx

   - fix signedness bug in gpio-rtd

   - fix kernel-doc warnings in gpio-en7523"

* tag 'gpio-fixes-for-v6.8-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux:
  gpiolib: revert the attempt to protect the GPIO device list with an rwsem
  gpio: EN7523: fix kernel-doc warnings
  gpiolib: Fix scope-based gpio_device refcounting
  gpio: mlxbf3: add an error code check in mlxbf3_gpio_probe
  dt-bindings: gpio: xilinx: Fix node address in gpio
  gpio: rtd: Fix signedness bug in probe

Documentation/devicetree/bindings/gpio/xlnx,gpio-xilinx.yaml
drivers/gpio/gpio-en7523.c
drivers/gpio/gpio-mlxbf3.c
drivers/gpio/gpio-rtd.c
drivers/gpio/gpiolib-sysfs.c
drivers/gpio/gpiolib-sysfs.h
drivers/gpio/gpiolib.c
drivers/gpio/gpiolib.h
include/linux/gpio/driver.h

index c1060e5fcef3a95c4bf2ef55e897c6c09b790d03..d3d8a2e143ed25dee5634ae9539c413c4f51f865 100644 (file)
@@ -126,7 +126,7 @@ examples:
   - |
     #include <dt-bindings/interrupt-controller/arm-gic.h>
 
-        gpio@e000a000 {
+        gpio@a0020000 {
             compatible = "xlnx,xps-gpio-1.00.a";
             reg = <0xa0020000 0x10000>;
             #gpio-cells = <2>;
index f836a8db4c1d21f8c44370246e8ad97bfedb064f..69834db2c1cf26be379c0deca38dda889202f706 100644 (file)
 #define AIROHA_GPIO_MAX                32
 
 /**
- * airoha_gpio_ctrl - Airoha GPIO driver data
+ * struct airoha_gpio_ctrl - Airoha GPIO driver data
  * @gc: Associated gpio_chip instance.
  * @data: The data register.
- * @dir0: The direction register for the lower 16 pins.
- * @dir1: The direction register for the higher 16 pins.
+ * @dir: [0] The direction register for the lower 16 pins.
+ * [1]: The direction register for the higher 16 pins.
  * @output: The output enable register.
  */
 struct airoha_gpio_ctrl {
index 7a3e1760fc5b7d1a754d25d05df1bb30fbbad396..d5906d419b0ab996a5286d8cc411929385bf2c4b 100644 (file)
@@ -215,6 +215,8 @@ static int mlxbf3_gpio_probe(struct platform_device *pdev)
                        gs->gpio_clr_io + MLXBF_GPIO_FW_DATA_OUT_CLEAR,
                        gs->gpio_set_io + MLXBF_GPIO_FW_OUTPUT_ENABLE_SET,
                        gs->gpio_clr_io + MLXBF_GPIO_FW_OUTPUT_ENABLE_CLEAR, 0);
+       if (ret)
+               return dev_err_probe(dev, ret, "%s: bgpio_init() failed", __func__);
 
        gc->request = gpiochip_generic_request;
        gc->free = gpiochip_generic_free;
index a7939bd0aa566e94ac5093e7fc494b7535623b9a..bf7f008f58d703347cba14f35c19f5798ee3a949 100644 (file)
@@ -525,18 +525,21 @@ static int rtd_gpio_probe(struct platform_device *pdev)
        struct device *dev = &pdev->dev;
        struct gpio_irq_chip *irq_chip;
        struct rtd_gpio *data;
+       int ret;
 
        data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
        if (!data)
                return -ENOMEM;
 
-       data->irqs[0] = platform_get_irq(pdev, 0);
-       if (data->irqs[0] < 0)
-               return data->irqs[0];
+       ret = platform_get_irq(pdev, 0);
+       if (ret < 0)
+               return ret;
+       data->irqs[0] = ret;
 
-       data->irqs[1] = platform_get_irq(pdev, 1);
-       if (data->irqs[1] < 0)
-               return data->irqs[1];
+       ret = platform_get_irq(pdev, 1);
+       if (ret < 0)
+               return ret;
+       data->irqs[1] = ret;
 
        data->info = device_get_match_data(dev);
        if (!data->info)
index 4dbf298bb5dda0f3fadc58843707d593a1adbc5c..6bf5332136e5a9d4bd0b4c52618fa05241ccaa00 100644 (file)
@@ -768,25 +768,6 @@ int gpiochip_sysfs_register(struct gpio_device *gdev)
        return 0;
 }
 
-int gpiochip_sysfs_register_all(void)
-{
-       struct gpio_device *gdev;
-       int ret;
-
-       guard(rwsem_read)(&gpio_devices_sem);
-
-       list_for_each_entry(gdev, &gpio_devices, list) {
-               if (gdev->mockdev)
-                       continue;
-
-               ret = gpiochip_sysfs_register(gdev);
-               if (ret)
-                       return ret;
-       }
-
-       return 0;
-}
-
 void gpiochip_sysfs_unregister(struct gpio_device *gdev)
 {
        struct gpio_desc *desc;
@@ -811,7 +792,9 @@ void gpiochip_sysfs_unregister(struct gpio_device *gdev)
 
 static int __init gpiolib_sysfs_init(void)
 {
-       int status;
+       int             status;
+       unsigned long   flags;
+       struct gpio_device *gdev;
 
        status = class_register(&gpio_class);
        if (status < 0)
@@ -823,6 +806,26 @@ static int __init gpiolib_sysfs_init(void)
         * We run before arch_initcall() so chip->dev nodes can have
         * registered, and so arch_initcall() can always gpiod_export().
         */
-       return gpiochip_sysfs_register_all();
+       spin_lock_irqsave(&gpio_lock, flags);
+       list_for_each_entry(gdev, &gpio_devices, list) {
+               if (gdev->mockdev)
+                       continue;
+
+               /*
+                * TODO we yield gpio_lock here because
+                * gpiochip_sysfs_register() acquires a mutex. This is unsafe
+                * and needs to be fixed.
+                *
+                * Also it would be nice to use gpio_device_find() here so we
+                * can keep gpio_chips local to gpiolib.c, but the yield of
+                * gpio_lock prevents us from doing this.
+                */
+               spin_unlock_irqrestore(&gpio_lock, flags);
+               status = gpiochip_sysfs_register(gdev);
+               spin_lock_irqsave(&gpio_lock, flags);
+       }
+       spin_unlock_irqrestore(&gpio_lock, flags);
+
+       return status;
 }
 postcore_initcall(gpiolib_sysfs_init);
index ab157cec0b4bec5ae89249fedbd8374e1cd0d91e..b794b396d6a52588c93839fbba062eb160236ca4 100644 (file)
@@ -8,7 +8,6 @@ struct gpio_device;
 #ifdef CONFIG_GPIO_SYSFS
 
 int gpiochip_sysfs_register(struct gpio_device *gdev);
-int gpiochip_sysfs_register_all(void);
 void gpiochip_sysfs_unregister(struct gpio_device *gdev);
 
 #else
@@ -18,11 +17,6 @@ static inline int gpiochip_sysfs_register(struct gpio_device *gdev)
        return 0;
 }
 
-static inline int gpiochip_sysfs_register_all(void)
-{
-       return 0;
-}
-
 static inline void gpiochip_sysfs_unregister(struct gpio_device *gdev)
 {
 }
index 4c93cf73a8260569de5287a4b2ae231dc54dbab6..44c8f5743a2416087b523e973967e993e8a192a1 100644 (file)
@@ -2,7 +2,6 @@
 
 #include <linux/acpi.h>
 #include <linux/bitmap.h>
-#include <linux/cleanup.h>
 #include <linux/compat.h>
 #include <linux/debugfs.h>
 #include <linux/device.h>
@@ -16,7 +15,6 @@
 #include <linux/kernel.h>
 #include <linux/list.h>
 #include <linux/module.h>
-#include <linux/mutex.h>
 #include <linux/of.h>
 #include <linux/pinctrl/consumer.h>
 #include <linux/seq_file.h>
@@ -83,9 +81,7 @@ DEFINE_SPINLOCK(gpio_lock);
 
 static DEFINE_MUTEX(gpio_lookup_lock);
 static LIST_HEAD(gpio_lookup_list);
-
 LIST_HEAD(gpio_devices);
-DECLARE_RWSEM(gpio_devices_sem);
 
 static DEFINE_MUTEX(gpio_machine_hogs_mutex);
 static LIST_HEAD(gpio_machine_hogs);
@@ -117,15 +113,20 @@ static inline void desc_set_label(struct gpio_desc *d, const char *label)
 struct gpio_desc *gpio_to_desc(unsigned gpio)
 {
        struct gpio_device *gdev;
+       unsigned long flags;
+
+       spin_lock_irqsave(&gpio_lock, flags);
 
-       scoped_guard(rwsem_read, &gpio_devices_sem) {
-               list_for_each_entry(gdev, &gpio_devices, list) {
-                       if (gdev->base <= gpio &&
-                           gdev->base + gdev->ngpio > gpio)
-                               return &gdev->descs[gpio - gdev->base];
+       list_for_each_entry(gdev, &gpio_devices, list) {
+               if (gdev->base <= gpio &&
+                   gdev->base + gdev->ngpio > gpio) {
+                       spin_unlock_irqrestore(&gpio_lock, flags);
+                       return &gdev->descs[gpio - gdev->base];
                }
        }
 
+       spin_unlock_irqrestore(&gpio_lock, flags);
+
        if (!gpio_is_valid(gpio))
                pr_warn("invalid GPIO %d\n", gpio);
 
@@ -398,21 +399,26 @@ static int gpiodev_add_to_list_unlocked(struct gpio_device *gdev)
 static struct gpio_desc *gpio_name_to_desc(const char * const name)
 {
        struct gpio_device *gdev;
+       unsigned long flags;
 
        if (!name)
                return NULL;
 
-       guard(rwsem_read)(&gpio_devices_sem);
+       spin_lock_irqsave(&gpio_lock, flags);
 
        list_for_each_entry(gdev, &gpio_devices, list) {
                struct gpio_desc *desc;
 
                for_each_gpio_desc(gdev->chip, desc) {
-                       if (desc->name && !strcmp(desc->name, name))
+                       if (desc->name && !strcmp(desc->name, name)) {
+                               spin_unlock_irqrestore(&gpio_lock, flags);
                                return desc;
+                       }
                }
        }
 
+       spin_unlock_irqrestore(&gpio_lock, flags);
+
        return NULL;
 }
 
@@ -807,6 +813,7 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
                               struct lock_class_key *request_key)
 {
        struct gpio_device *gdev;
+       unsigned long flags;
        unsigned int i;
        int base = 0;
        int ret = 0;
@@ -871,46 +878,49 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
 
        gdev->ngpio = gc->ngpio;
 
-       scoped_guard(rwsem_write, &gpio_devices_sem) {
-               /*
-                * TODO: this allocates a Linux GPIO number base in the global
-                * GPIO numberspace for this chip. In the long run we want to
-                * get *rid* of this numberspace and use only descriptors, but
-                * it may be a pipe dream. It will not happen before we get rid
-                * of the sysfs interface anyways.
-                */
-               base = gc->base;
+       spin_lock_irqsave(&gpio_lock, flags);
 
+       /*
+        * TODO: this allocates a Linux GPIO number base in the global
+        * GPIO numberspace for this chip. In the long run we want to
+        * get *rid* of this numberspace and use only descriptors, but
+        * it may be a pipe dream. It will not happen before we get rid
+        * of the sysfs interface anyways.
+        */
+       base = gc->base;
+       if (base < 0) {
+               base = gpiochip_find_base_unlocked(gc->ngpio);
                if (base < 0) {
-                       base = gpiochip_find_base_unlocked(gc->ngpio);
-                       if (base < 0) {
-                               ret = base;
-                               base = 0;
-                               goto err_free_label;
-                       }
-                       /*
-                        * TODO: it should not be necessary to reflect the assigned
-                        * base outside of the GPIO subsystem. Go over drivers and
-                        * see if anyone makes use of this, else drop this and assign
-                        * a poison instead.
-                        */
-                       gc->base = base;
-               } else {
-                       dev_warn(&gdev->dev,
-                                "Static allocation of GPIO base is deprecated, use dynamic allocation.\n");
-               }
-               gdev->base = base;
-
-               ret = gpiodev_add_to_list_unlocked(gdev);
-               if (ret) {
-                       chip_err(gc, "GPIO integer space overlap, cannot add chip\n");
+                       spin_unlock_irqrestore(&gpio_lock, flags);
+                       ret = base;
+                       base = 0;
                        goto err_free_label;
                }
+               /*
+                * TODO: it should not be necessary to reflect the assigned
+                * base outside of the GPIO subsystem. Go over drivers and
+                * see if anyone makes use of this, else drop this and assign
+                * a poison instead.
+                */
+               gc->base = base;
+       } else {
+               dev_warn(&gdev->dev,
+                        "Static allocation of GPIO base is deprecated, use dynamic allocation.\n");
+       }
+       gdev->base = base;
 
-               for (i = 0; i < gc->ngpio; i++)
-                       gdev->descs[i].gdev = gdev;
+       ret = gpiodev_add_to_list_unlocked(gdev);
+       if (ret) {
+               spin_unlock_irqrestore(&gpio_lock, flags);
+               chip_err(gc, "GPIO integer space overlap, cannot add chip\n");
+               goto err_free_label;
        }
 
+       for (i = 0; i < gc->ngpio; i++)
+               gdev->descs[i].gdev = gdev;
+
+       spin_unlock_irqrestore(&gpio_lock, flags);
+
        BLOCKING_INIT_NOTIFIER_HEAD(&gdev->line_state_notifier);
        BLOCKING_INIT_NOTIFIER_HEAD(&gdev->device_notifier);
        init_rwsem(&gdev->sem);
@@ -1001,8 +1011,9 @@ err_free_gpiochip_mask:
                goto err_print_message;
        }
 err_remove_from_list:
-       scoped_guard(rwsem_write, &gpio_devices_sem)
-               list_del(&gdev->list);
+       spin_lock_irqsave(&gpio_lock, flags);
+       list_del(&gdev->list);
+       spin_unlock_irqrestore(&gpio_lock, flags);
 err_free_label:
        kfree_const(gdev->label);
 err_free_descs:
@@ -1065,7 +1076,7 @@ void gpiochip_remove(struct gpio_chip *gc)
                dev_crit(&gdev->dev,
                         "REMOVING GPIOCHIP WITH GPIOS STILL REQUESTED\n");
 
-       scoped_guard(rwsem_write, &gpio_devices_sem)
+       scoped_guard(spinlock_irqsave, &gpio_lock)
                list_del(&gdev->list);
 
        /*
@@ -1114,7 +1125,7 @@ struct gpio_device *gpio_device_find(void *data,
         */
        might_sleep();
 
-       guard(rwsem_read)(&gpio_devices_sem);
+       guard(spinlock_irqsave)(&gpio_lock);
 
        list_for_each_entry(gdev, &gpio_devices, list) {
                if (gdev->chip && match(gdev->chip, data))
@@ -4725,33 +4736,35 @@ static void gpiolib_dbg_show(struct seq_file *s, struct gpio_device *gdev)
 
 static void *gpiolib_seq_start(struct seq_file *s, loff_t *pos)
 {
+       unsigned long flags;
        struct gpio_device *gdev = NULL;
        loff_t index = *pos;
 
        s->private = "";
 
-       guard(rwsem_read)(&gpio_devices_sem);
-
-       list_for_each_entry(gdev, &gpio_devices, list) {
-               if (index-- == 0)
+       spin_lock_irqsave(&gpio_lock, flags);
+       list_for_each_entry(gdev, &gpio_devices, list)
+               if (index-- == 0) {
+                       spin_unlock_irqrestore(&gpio_lock, flags);
                        return gdev;
-       }
+               }
+       spin_unlock_irqrestore(&gpio_lock, flags);
 
        return NULL;
 }
 
 static void *gpiolib_seq_next(struct seq_file *s, void *v, loff_t *pos)
 {
+       unsigned long flags;
        struct gpio_device *gdev = v;
        void *ret = NULL;
 
-       scoped_guard(rwsem_read, &gpio_devices_sem) {
-               if (list_is_last(&gdev->list, &gpio_devices))
-                       ret = NULL;
-               else
-                       ret = list_first_entry(&gdev->list, struct gpio_device,
-                                              list);
-       }
+       spin_lock_irqsave(&gpio_lock, flags);
+       if (list_is_last(&gdev->list, &gpio_devices))
+               ret = NULL;
+       else
+               ret = list_first_entry(&gdev->list, struct gpio_device, list);
+       spin_unlock_irqrestore(&gpio_lock, flags);
 
        s->private = "\n";
        ++*pos;
index 97df54abf57ac0f2a740d72bf37965b299d87927..a4a2520b5f31cced763696970a452ab1fff0e4bf 100644 (file)
@@ -15,7 +15,6 @@
 #include <linux/gpio/consumer.h> /* for enum gpiod_flags */
 #include <linux/gpio/driver.h>
 #include <linux/module.h>
-#include <linux/mutex.h>
 #include <linux/notifier.h>
 #include <linux/rwsem.h>
 
@@ -137,7 +136,6 @@ int gpiod_set_transitory(struct gpio_desc *desc, bool transitory);
 
 extern spinlock_t gpio_lock;
 extern struct list_head gpio_devices;
-extern struct rw_semaphore gpio_devices_sem;
 
 void gpiod_line_state_notify(struct gpio_desc *desc, unsigned long action);
 
index e846bd4e7559bb54bba7ffb2dcda3dc8e0099e16..9a5c6c76e6533385dbb32de98abfd330c8736585 100644 (file)
@@ -635,7 +635,7 @@ struct gpio_device *gpio_device_get(struct gpio_device *gdev);
 void gpio_device_put(struct gpio_device *gdev);
 
 DEFINE_FREE(gpio_device_put, struct gpio_device *,
-           if (IS_ERR_OR_NULL(_T)) gpio_device_put(_T));
+           if (!IS_ERR_OR_NULL(_T)) gpio_device_put(_T))
 
 struct device *gpio_device_to_device(struct gpio_device *gdev);