Commit a81668db authored by Linus Torvalds's avatar Linus Torvalds
Browse files

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

Pull gpio fixes from Bartosz Golaszewski:
 "There are several ordinary driver fixes and a fix to a race between
  the registration of two chips that causes a crash in GPIO core.

  The bulk of the changed lines however, concerns the management of
  shared GPIOs that landed in v6.19-rc1. Enabling it for ARCH_QCOM
  enabled it in defconfig which effectively enabled it for all arm64
  platforms and exposed the code to quite a lot of testing (which is
  good, right? :)).

  As a resukt, I received a number of bug reports, which I progressively
  fixed over the course of last weeks. This explains the number of lines
  higher than what I normally aim for at this stage.

   - balance superio enter/exit calls in error path in gpio-it87

   - fix a race where we try to take the SRCU read lock of the GPIO
     device before it's been initialized causing a NULL-pointer
     dereference

   - fix handling of short-pulse interrupts in gpio-pca053x

   - fix a reference leak in error path in gpio-mpsse

   - mark the GPIO controller as sleeping (it calls sleeping functions)
     in gpio-rockchip

   - fix several issues in management of shared GPIOs"

* tag 'gpio-fixes-for-v6.19-rc5' of git://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux:
  gpio: shared: fix a false-positive sharing detection with reset-gpios
  gpiolib: fix lookup table matching
  gpio: shared: don't allocate the lookup table until we really need it
  gpio: shared: fix a race condition
  gpio: shared: assign the correct firmware node for reset-gpio use-case
  gpio: rockchip: mark the GPIO controller as sleeping
  gpio: mpsse: fix reference leak in gpio_mpsse_probe() error paths
  gpio: pca953x: handle short interrupt pulses on PCAL devices
  gpiolib: fix race condition for gdev->srcu
  gpio: shared: allow sharing a reset-gpios pin between reset-gpio and gpiolib
  gpio: shared: verify con_id when adding proxy lookup
  gpiolib: allow multiple lookup tables per consumer
  gpio: it87: balance superio enter/exit calls in error path
parents cbd4480c d578b318

drivers/gpio/gpio-it87.c

+3 −8
Original line number Diff line number Diff line
@@ -12,6 +12,7 @@ 


#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt



#include <linux/cleanup.h>

#include <linux/init.h>

#include <linux/kernel.h>

#include <linux/module.h>
@@ -241,23 +242,17 @@ static int it87_gpio_direction_out(struct gpio_chip *chip, 
	mask = 1 << (gpio_num % 8);

	group = (gpio_num / 8);



	spin_lock(&it87_gpio->lock);

	guard(spinlock)(&it87_gpio->lock);



	rc = superio_enter();

	if (rc)

		goto exit;

		return rc;



	/* set the output enable bit */

	superio_set_mask(mask, group + it87_gpio->output_base);



	rc = it87_gpio_set(chip, gpio_num, val);

	if (rc)

		goto exit;



	superio_exit();



exit:

	spin_unlock(&it87_gpio->lock);

	return rc;

}

drivers/gpio/gpio-mpsse.c

+11 −1
Original line number Diff line number Diff line
@@ -548,6 +548,13 @@ static void gpio_mpsse_ida_remove(void *data) 
	ida_free(&gpio_mpsse_ida, priv->id);

}



static void gpio_mpsse_usb_put_dev(void *data)

{

	struct mpsse_priv *priv = data;



	usb_put_dev(priv->udev);

}



static int mpsse_init_valid_mask(struct gpio_chip *chip,

				 unsigned long *valid_mask,

				 unsigned int ngpios)
@@ -592,6 +599,10 @@ static int gpio_mpsse_probe(struct usb_interface *interface, 
	INIT_LIST_HEAD(&priv->workers);



	priv->udev = usb_get_dev(interface_to_usbdev(interface));

	err = devm_add_action_or_reset(dev, gpio_mpsse_usb_put_dev, priv);

	if (err)

		return err;



	priv->intf = interface;

	priv->intf_id = interface->cur_altsetting->desc.bInterfaceNumber;
@@ -713,7 +724,6 @@ static void gpio_mpsse_disconnect(struct usb_interface *intf) 


	priv->intf = NULL;

	usb_set_intfdata(intf, NULL);

	usb_put_dev(priv->udev);

}



static struct usb_driver gpio_mpsse_driver = {

drivers/gpio/gpio-pca953x.c

+24 −1
Original line number Diff line number Diff line
@@ -943,14 +943,35 @@ static bool pca953x_irq_pending(struct pca953x_chip *chip, unsigned long *pendin 
	DECLARE_BITMAP(old_stat, MAX_LINE);

	DECLARE_BITMAP(cur_stat, MAX_LINE);

	DECLARE_BITMAP(new_stat, MAX_LINE);

	DECLARE_BITMAP(int_stat, MAX_LINE);

	DECLARE_BITMAP(trigger, MAX_LINE);

	DECLARE_BITMAP(edges, MAX_LINE);

	int ret;



	if (chip->driver_data & PCA_PCAL) {

		/* Read INT_STAT before it is cleared by the input-port read. */

		ret = pca953x_read_regs(chip, PCAL953X_INT_STAT, int_stat);

		if (ret)

			return false;

	}



	ret = pca953x_read_regs(chip, chip->regs->input, cur_stat);

	if (ret)

		return false;



	if (chip->driver_data & PCA_PCAL) {

		/* Detect short pulses via INT_STAT. */

		bitmap_and(trigger, int_stat, chip->irq_mask, gc->ngpio);



		/* Apply filter for rising/falling edge selection. */

		bitmap_replace(new_stat, chip->irq_trig_fall, chip->irq_trig_raise,

			       cur_stat, gc->ngpio);



		bitmap_and(int_stat, new_stat, trigger, gc->ngpio);

	} else {

		bitmap_zero(int_stat, gc->ngpio);

	}



	/* Remove output pins from the equation */

	pca953x_read_regs(chip, chip->regs->direction, reg_direction);
@@ -964,7 +985,8 @@ static bool pca953x_irq_pending(struct pca953x_chip *chip, unsigned long *pendin 


	if (bitmap_empty(chip->irq_trig_level_high, gc->ngpio) &&

	    bitmap_empty(chip->irq_trig_level_low, gc->ngpio)) {

		if (bitmap_empty(trigger, gc->ngpio))

		if (bitmap_empty(trigger, gc->ngpio) &&

		    bitmap_empty(int_stat, gc->ngpio))

			return false;

	}
@@ -972,6 +994,7 @@ static bool pca953x_irq_pending(struct pca953x_chip *chip, unsigned long *pendin 
	bitmap_and(old_stat, chip->irq_trig_raise, new_stat, gc->ngpio);

	bitmap_or(edges, old_stat, cur_stat, gc->ngpio);

	bitmap_and(pending, edges, trigger, gc->ngpio);

	bitmap_or(pending, pending, int_stat, gc->ngpio);



	bitmap_and(cur_stat, new_stat, chip->irq_trig_level_high, gc->ngpio);

	bitmap_and(cur_stat, cur_stat, chip->irq_mask, gc->ngpio);

drivers/gpio/gpio-rockchip.c

+1 −0
Original line number Diff line number Diff line
@@ -593,6 +593,7 @@ static int rockchip_gpiolib_register(struct rockchip_pin_bank *bank) 
	gc->ngpio = bank->nr_pins;

	gc->label = bank->name;

	gc->parent = bank->dev;

	gc->can_sleep = true;



	ret = gpiochip_add_data(gc, bank);

	if (ret) {

drivers/gpio/gpiolib-shared.c

+179 −70
Original line number Diff line number Diff line
@@ -38,8 +38,10 @@ struct gpio_shared_ref { 
	int dev_id;

	/* Protects the auxiliary device struct and the lookup table. */

	struct mutex lock;

	struct lock_class_key lock_key;

	struct auxiliary_device adev;

	struct gpiod_lookup_table *lookup;

	bool is_reset_gpio;

};



/* Represents a single GPIO pin. */
@@ -76,6 +78,60 @@ gpio_shared_find_entry(struct fwnode_handle *controller_node, 
	return NULL;

}



static struct gpio_shared_ref *gpio_shared_make_ref(struct fwnode_handle *fwnode,

						    const char *con_id,

						    enum gpiod_flags flags)

{

	char *con_id_cpy __free(kfree) = NULL;



	struct gpio_shared_ref *ref __free(kfree) = kzalloc(sizeof(*ref), GFP_KERNEL);

	if (!ref)

		return NULL;



	if (con_id) {

		con_id_cpy = kstrdup(con_id, GFP_KERNEL);

		if (!con_id_cpy)

			return NULL;

	}



	ref->dev_id = ida_alloc(&gpio_shared_ida, GFP_KERNEL);

	if (ref->dev_id < 0)

		return NULL;



	ref->flags = flags;

	ref->con_id = no_free_ptr(con_id_cpy);

	ref->fwnode = fwnode;

	lockdep_register_key(&ref->lock_key);

	mutex_init_with_key(&ref->lock, &ref->lock_key);



	return no_free_ptr(ref);

}



static int gpio_shared_setup_reset_proxy(struct gpio_shared_entry *entry,

					 enum gpiod_flags flags)

{

	struct gpio_shared_ref *ref;



	list_for_each_entry(ref, &entry->refs, list) {

		if (ref->is_reset_gpio)

			/* Already set-up. */

			return 0;

	}



	ref = gpio_shared_make_ref(NULL, "reset", flags);

	if (!ref)

		return -ENOMEM;



	ref->is_reset_gpio = true;



	list_add_tail(&ref->list, &entry->refs);



	pr_debug("Created a secondary shared GPIO reference for potential reset-gpio device for GPIO %u at %s\n",

		 entry->offset, fwnode_get_name(entry->fwnode));



	return 0;

}



/* Handle all special nodes that we should ignore. */

static bool gpio_shared_of_node_ignore(struct device_node *node)

{
@@ -106,6 +162,7 @@ static int gpio_shared_of_traverse(struct device_node *curr) 
	size_t con_id_len, suffix_len;

	struct fwnode_handle *fwnode;

	struct of_phandle_args args;

	struct gpio_shared_ref *ref;

	struct property *prop;

	unsigned int offset;

	const char *suffix;
@@ -138,6 +195,7 @@ static int gpio_shared_of_traverse(struct device_node *curr) 


		for (i = 0; i < count; i++) {

			struct device_node *np __free(device_node) = NULL;

			char *con_id __free(kfree) = NULL;



			ret = of_parse_phandle_with_args(curr, prop->name,

							 "#gpio-cells", i,
@@ -182,15 +240,6 @@ static int gpio_shared_of_traverse(struct device_node *curr) 
				list_add_tail(&entry->list, &gpio_shared_list);

			}



			struct gpio_shared_ref *ref __free(kfree) =

					kzalloc(sizeof(*ref), GFP_KERNEL);

			if (!ref)

				return -ENOMEM;



			ref->fwnode = fwnode_handle_get(of_fwnode_handle(curr));

			ref->flags = args.args[1];

			mutex_init(&ref->lock);



			if (strends(prop->name, "gpios"))

				suffix = "-gpios";

			else if (strends(prop->name, "gpio"))
@@ -202,27 +251,32 @@ static int gpio_shared_of_traverse(struct device_node *curr) 


			/* We only set con_id if there's actually one. */

			if (strcmp(prop->name, "gpios") && strcmp(prop->name, "gpio")) {

				ref->con_id = kstrdup(prop->name, GFP_KERNEL);

				if (!ref->con_id)

				con_id = kstrdup(prop->name, GFP_KERNEL);

				if (!con_id)

					return -ENOMEM;



				con_id_len = strlen(ref->con_id);

				con_id_len = strlen(con_id);

				suffix_len = strlen(suffix);



				ref->con_id[con_id_len - suffix_len] = '\0';

				con_id[con_id_len - suffix_len] = '\0';

			}



			ref->dev_id = ida_alloc(&gpio_shared_ida, GFP_KERNEL);

			if (ref->dev_id < 0) {

				kfree(ref->con_id);

			ref = gpio_shared_make_ref(fwnode_handle_get(of_fwnode_handle(curr)),

						   con_id, args.args[1]);

			if (!ref)

				return -ENOMEM;

			}



			if (!list_empty(&entry->refs))

				pr_debug("GPIO %u at %s is shared by multiple firmware nodes\n",

					 entry->offset, fwnode_get_name(entry->fwnode));



			list_add_tail(&no_free_ptr(ref)->list, &entry->refs);

			list_add_tail(&ref->list, &entry->refs);



			if (strcmp(prop->name, "reset-gpios") == 0) {

				ret = gpio_shared_setup_reset_proxy(entry, args.args[1]);

				if (ret)

					return ret;

			}

		}

	}
@@ -306,20 +360,16 @@ static bool gpio_shared_dev_is_reset_gpio(struct device *consumer, 
	struct fwnode_handle *reset_fwnode = dev_fwnode(consumer);

	struct fwnode_reference_args ref_args, aux_args;

	struct device *parent = consumer->parent;

	struct gpio_shared_ref *real_ref;

	bool match;

	int ret;



	lockdep_assert_held(&ref->lock);



	/* The reset-gpio device must have a parent AND a firmware node. */

	if (!parent || !reset_fwnode)

		return false;



	/*

	 * FIXME: use device_is_compatible() once the reset-gpio drivers gains

	 * a compatible string which it currently does not have.

	 */

	if (!strstarts(dev_name(consumer), "reset.gpio."))

		return false;



	/*

	 * Parent of the reset-gpio auxiliary device is the GPIO chip whose

	 * fwnode we stored in the entry structure.
@@ -328,24 +378,40 @@ static bool gpio_shared_dev_is_reset_gpio(struct device *consumer, 
		return false;



	/*

	 * The device associated with the shared reference's firmware node is

	 * the consumer of the reset control exposed by the reset-gpio device.

	 * It must have a "reset-gpios" property that's referencing the entry's

	 * firmware node.

	 * Now we need to find the actual pin we want to assign to this

	 * reset-gpio device. To that end: iterate over the list of references

	 * of this entry and see if there's one, whose reset-gpios property's

	 * arguments match the ones from this consumer's node.

	 */

	list_for_each_entry(real_ref, &entry->refs, list) {

		if (real_ref == ref)

			continue;



		guard(mutex)(&real_ref->lock);



		if (!real_ref->fwnode)

			continue;



		/*

		 * The device associated with the shared reference's firmware

		 * node is the consumer of the reset control exposed by the

		 * reset-gpio device. It must have a "reset-gpios" property

		 * that's referencing the entry's firmware node.

		 *

	 * The reference args must agree between the real consumer and the

	 * auxiliary reset-gpio device.

		 * The reference args must agree between the real consumer and

		 * the auxiliary reset-gpio device.

		 */

	ret = fwnode_property_get_reference_args(ref->fwnode, "reset-gpios",

		ret = fwnode_property_get_reference_args(real_ref->fwnode,

							 "reset-gpios",

							 NULL, 2, 0, &ref_args);

		if (ret)

		return false;

			continue;



		ret = fwnode_property_get_reference_args(reset_fwnode, "reset-gpios",

							 NULL, 2, 0, &aux_args);

		if (ret) {

			fwnode_handle_put(ref_args.fwnode);

		return false;

			continue;

		}



		match = ((ref_args.fwnode == entry->fwnode) &&
@@ -354,7 +420,19 @@ static bool gpio_shared_dev_is_reset_gpio(struct device *consumer, 


		fwnode_handle_put(ref_args.fwnode);

		fwnode_handle_put(aux_args.fwnode);

	return match;



		if (!match)

			continue;



		/*

		 * Reuse the fwnode of the real device, next time we'll use it

		 * in the normal path.

		 */

		ref->fwnode = fwnode_handle_get(reset_fwnode);

		return true;

	}



	return false;

}

#else

static bool gpio_shared_dev_is_reset_gpio(struct device *consumer,
@@ -365,24 +443,33 @@ static bool gpio_shared_dev_is_reset_gpio(struct device *consumer, 
}

#endif /* CONFIG_RESET_GPIO */



int gpio_shared_add_proxy_lookup(struct device *consumer, unsigned long lflags)

int gpio_shared_add_proxy_lookup(struct device *consumer, const char *con_id,

				 unsigned long lflags)

{

	const char *dev_id = dev_name(consumer);

	struct gpiod_lookup_table *lookup;

	struct gpio_shared_entry *entry;

	struct gpio_shared_ref *ref;



	struct gpiod_lookup_table *lookup __free(kfree) =

			kzalloc(struct_size(lookup, table, 2), GFP_KERNEL);

	if (!lookup)

		return -ENOMEM;



	list_for_each_entry(entry, &gpio_shared_list, list) {

		list_for_each_entry(ref, &entry->refs, list) {

			if (!device_match_fwnode(consumer, ref->fwnode) &&

			    !gpio_shared_dev_is_reset_gpio(consumer, entry, ref))

			guard(mutex)(&ref->lock);



			/*

			 * FIXME: use device_is_compatible() once the reset-gpio

			 * drivers gains a compatible string which it currently

			 * does not have.

			 */

			if (!ref->fwnode && strstarts(dev_name(consumer), "reset.gpio.")) {

				if (!gpio_shared_dev_is_reset_gpio(consumer, entry, ref))

					continue;

			} else if (!device_match_fwnode(consumer, ref->fwnode)) {

				continue;

			}



			guard(mutex)(&ref->lock);

			if ((!con_id && ref->con_id) || (con_id && !ref->con_id) ||

			    (con_id && ref->con_id && strcmp(con_id, ref->con_id) != 0))

				continue;



			/* We've already done that on a previous request. */

			if (ref->lookup)
@@ -395,6 +482,10 @@ int gpio_shared_add_proxy_lookup(struct device *consumer, unsigned long lflags) 
			if (!key)

				return -ENOMEM;



			lookup = kzalloc(struct_size(lookup, table, 2), GFP_KERNEL);

			if (!lookup)

				return -ENOMEM;



			pr_debug("Adding machine lookup entry for a shared GPIO for consumer %s, with key '%s' and con_id '%s'\n",

				 dev_id, key, ref->con_id ?: "none");
@@ -402,7 +493,7 @@ int gpio_shared_add_proxy_lookup(struct device *consumer, unsigned long lflags) 
			lookup->table[0] = GPIO_LOOKUP(no_free_ptr(key), 0,

						       ref->con_id, lflags);



			ref->lookup = no_free_ptr(lookup);

			ref->lookup = lookup;

			gpiod_add_lookup_table(ref->lookup);



			return 0;
@@ -466,8 +557,9 @@ int gpio_device_setup_shared(struct gpio_device *gdev) 
			 entry->offset, gpio_device_get_label(gdev));



		list_for_each_entry(ref, &entry->refs, list) {

			pr_debug("Setting up a shared GPIO entry for %s\n",

				 fwnode_get_name(ref->fwnode));

			pr_debug("Setting up a shared GPIO entry for %s (con_id: '%s')\n",

				 fwnode_get_name(ref->fwnode) ?: "(no fwnode)",

				 ref->con_id ?: "(none)");



			ret = gpio_shared_make_adev(gdev, entry, ref);

			if (ret)
@@ -487,15 +579,6 @@ void gpio_device_teardown_shared(struct gpio_device *gdev) 
		if (!device_match_fwnode(&gdev->dev, entry->fwnode))

			continue;



		/*

		 * For some reason if we call synchronize_srcu() in GPIO core,

		 * descent here and take this mutex and then recursively call

		 * synchronize_srcu() again from gpiochip_remove() (which is

		 * totally fine) called after gpio_shared_remove_adev(),

		 * lockdep prints a false positive deadlock splat. Disable

		 * lockdep here.

		 */

		lockdep_off();

		list_for_each_entry(ref, &entry->refs, list) {

			guard(mutex)(&ref->lock);
@@ -508,7 +591,6 @@ void gpio_device_teardown_shared(struct gpio_device *gdev) 


			gpio_shared_remove_adev(&ref->adev);

		}

		lockdep_on();

	}

}
@@ -604,6 +686,7 @@ static void gpio_shared_drop_ref(struct gpio_shared_ref *ref) 
{

	list_del(&ref->list);

	mutex_destroy(&ref->lock);

	lockdep_unregister_key(&ref->lock_key);

	kfree(ref->con_id);

	ida_free(&gpio_shared_ida, ref->dev_id);

	fwnode_handle_put(ref->fwnode);
@@ -635,12 +718,38 @@ static void __init gpio_shared_teardown(void) 
	}

}



static bool gpio_shared_entry_is_really_shared(struct gpio_shared_entry *entry)

{

	size_t num_nodes = list_count_nodes(&entry->refs);

	struct gpio_shared_ref *ref;



	if (num_nodes <= 1)

		return false;



	if (num_nodes > 2)

		return true;



	/* Exactly two references: */

	list_for_each_entry(ref, &entry->refs, list) {

		/*

		 * Corner-case: the second reference comes from the potential

		 * reset-gpio instance. However, this pin is not really shared

		 * as it would have three references in this case. Avoid

		 * creating unnecessary proxies.

		 */

		if (ref->is_reset_gpio)

			return false;

	}



	return true;

}



static void gpio_shared_free_exclusive(void)

{

	struct gpio_shared_entry *entry, *epos;



	list_for_each_entry_safe(entry, epos, &gpio_shared_list, list) {

		if (list_count_nodes(&entry->refs) > 1)

		if (gpio_shared_entry_is_really_shared(entry))

			continue;



		gpio_shared_drop_ref(list_first_entry(&entry->refs,