Commit 65a828ba authored by Bartosz Golaszewski's avatar Bartosz Golaszewski
Browse files

gpiolib: use a mutex to protect the list of GPIO devices 



The global list of GPIO devices is never modified or accessed from
atomic context so it's fine to protect it using a mutex. Add a new
global lock dedicated to the gpio_devices list and use it whenever
accessing or modifying it.

Signed-off-by: default avatarBartosz Golaszewski <bartosz.golaszewski@linaro.org>
Reviewed-by: default avatarAndy Shevchenko <andriy.shevchenko@linux.intel.com>
parent f95fd4ac

drivers/gpio/gpiolib-sysfs.c

+21 −24
Original line number Diff line number Diff line
@@ -766,6 +766,25 @@ int gpiochip_sysfs_register(struct gpio_device *gdev) 
	return 0;

}



int gpiochip_sysfs_register_all(void)

{

	struct gpio_device *gdev;

	int ret;



	guard(mutex)(&gpio_devices_lock);



	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;
@@ -791,8 +810,6 @@ void gpiochip_sysfs_unregister(struct gpio_device *gdev) 
static int __init gpiolib_sysfs_init(void)

{

	int status;

	unsigned long	flags;

	struct gpio_device *gdev;



	status = class_register(&gpio_class);

	if (status < 0)
@@ -804,26 +821,6 @@ 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().

	 */

	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;

	return gpiochip_sysfs_register_all();

}

postcore_initcall(gpiolib_sysfs_init);

drivers/gpio/gpiolib-sysfs.h

+6 −0
Original line number Diff line number Diff line
@@ -8,6 +8,7 @@ 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
@@ -17,6 +18,11 @@ 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

+61 −76
Original line number Diff line number Diff line
@@ -2,6 +2,7 @@ 


#include <linux/acpi.h>

#include <linux/bitmap.h>

#include <linux/cleanup.h>

#include <linux/compat.h>

#include <linux/debugfs.h>

#include <linux/device.h>
@@ -15,6 +16,7 @@ 
#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>
@@ -94,7 +96,9 @@ DEFINE_SPINLOCK(gpio_lock); 


static DEFINE_MUTEX(gpio_lookup_lock);

static LIST_HEAD(gpio_lookup_list);



LIST_HEAD(gpio_devices);

DEFINE_MUTEX(gpio_devices_lock);



static DEFINE_MUTEX(gpio_machine_hogs_mutex);

static LIST_HEAD(gpio_machine_hogs);
@@ -126,20 +130,15 @@ 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(mutex, &gpio_devices_lock) {

		list_for_each_entry(gdev, &gpio_devices, list) {

			if (gdev->base <= gpio &&

		    gdev->base + gdev->ngpio > gpio) {

			spin_unlock_irqrestore(&gpio_lock, flags);

			    gdev->base + gdev->ngpio > gpio)

				return &gdev->descs[gpio - gdev->base];

		}

	}



	spin_unlock_irqrestore(&gpio_lock, flags);



	if (!gpio_is_valid(gpio))

		pr_warn("invalid GPIO %d\n", gpio);
@@ -412,25 +411,20 @@ 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;



	spin_lock_irqsave(&gpio_lock, flags);

	guard(mutex)(&gpio_devices_lock);



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

				spin_unlock_irqrestore(&gpio_lock, flags);

			if (desc->name && !strcmp(desc->name, name))

				return desc;

		}

	}

	}



	spin_unlock_irqrestore(&gpio_lock, flags);



	return NULL;

}
@@ -669,11 +663,9 @@ EXPORT_SYMBOL_GPL(gpiochip_line_is_valid); 
static void gpiodev_release(struct device *dev)

{

	struct gpio_device *gdev = to_gpio_device(dev);

	unsigned long flags;



	spin_lock_irqsave(&gpio_lock, flags);

	scoped_guard(mutex, &gpio_devices_lock)

		list_del(&gdev->list);

	spin_unlock_irqrestore(&gpio_lock, flags);



	ida_free(&gpio_ida, gdev->id);

	kfree_const(gdev->label);
@@ -831,7 +823,6 @@ 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;
@@ -896,8 +887,7 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data, 


	gdev->ngpio = gc->ngpio;



	spin_lock_irqsave(&gpio_lock, flags);



	scoped_guard(mutex, &gpio_devices_lock) {

		/*

		 * TODO: this allocates a Linux GPIO number base in the global

		 * GPIO numberspace for this chip. In the long run we want to
@@ -906,10 +896,10 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data, 
		 * of the sysfs interface anyways.

		 */

		base = gc->base;



		if (base < 0) {

			base = gpiochip_find_base_unlocked(gc->ngpio);

			if (base < 0) {

			spin_unlock_irqrestore(&gpio_lock, flags);

				ret = base;

				base = 0;

				goto err_free_label;
@@ -929,15 +919,13 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data, 


		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);
@@ -1029,9 +1017,8 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data, 
		goto err_print_message;

	}

err_remove_from_list:

	spin_lock_irqsave(&gpio_lock, flags);

	scoped_guard(mutex, &gpio_devices_lock)

		list_del(&gdev->list);

	spin_unlock_irqrestore(&gpio_lock, flags);

err_free_label:

	kfree_const(gdev->label);

err_free_descs:
@@ -1140,7 +1127,7 @@ struct gpio_device *gpio_device_find(void *data, 
	 */

	might_sleep();



	guard(spinlock_irqsave)(&gpio_lock);

	guard(mutex)(&gpio_devices_lock);



	list_for_each_entry(gdev, &gpio_devices, list) {

		if (gdev->chip && match(gdev->chip, data))
@@ -4756,35 +4743,33 @@ 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 = "";



	spin_lock_irqsave(&gpio_lock, flags);

	list_for_each_entry(gdev, &gpio_devices, list)

		if (index-- == 0) {

			spin_unlock_irqrestore(&gpio_lock, flags);

	guard(mutex)(&gpio_devices_lock);



	list_for_each_entry(gdev, &gpio_devices, list) {

		if (index-- == 0)

			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;



	spin_lock_irqsave(&gpio_lock, flags);

	scoped_guard(mutex, &gpio_devices_lock) {

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

			ret = list_first_entry(&gdev->list, struct gpio_device,

					       list);

	}



	s->private = "\n";

	++*pos;

drivers/gpio/gpiolib.h

+2 −0
Original line number Diff line number Diff line
@@ -15,6 +15,7 @@ 
#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>
@@ -136,6 +137,7 @@ int gpiod_set_transitory(struct gpio_desc *desc, bool transitory); 


extern spinlock_t gpio_lock;

extern struct list_head gpio_devices;

extern struct mutex gpio_devices_lock;



void gpiod_line_state_notify(struct gpio_desc *desc, unsigned long action);