Merge tag 'gpio-fixes-for-v6.8-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux

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

@@ -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>;

drivers/gpio/gpio-en7523.c

@@ -12,11 +12,11 @@
 #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 {

drivers/gpio/gpio-mlxbf3.c

@@ -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;

drivers/gpio/gpio-rtd.c

@@ -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)

drivers/gpio/gpiolib-sysfs.c

@@ -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);

drivers/gpio/gpiolib-sysfs.h

@@ -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)
 {
 }

drivers/gpio/gpiolib.c

@@ -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 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
 		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;

drivers/gpio/gpiolib.h

@@ -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);
 

include/linux/gpio/driver.h

@@ -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);