This device is based on the FT232H, which only has one MPSSE.
Luckily, FT232H is pretty much a drop-in for the FT2232H which I wrote
this driver for originally... It just populates with only one USB
interface rather than two!

This device is hotpluggable, and only two lines are exposed by the
hardware, with strict directionality imposed by the design. As a result,
there are some plumbing-y changes I made to support a wider range of
MPSSE-based devices in the future.

There are a few things at play here:
* Restructure things so we can limit capabilities for
  different devices. In the case of the radio interface
  kit, only two lines are connected, and each is
  unidirectional.
* This device is hotpluggable, which caused me to realize the
  synchronization stuff on the driver wasn't sound.
  I added a queue for tearing down the polling worker using an
  RCU-protected list of workers. It's possible to stop and start
  a worker, which could cause multiple workers to briefly exist
  and need to be torn down. I would love some feedback on this
  approach though!
* Lastly, we change the format of the label to expose some useful
  information about the device to userspace to help distinguish
  different devices. Maybe there's a more clever, better way to
  get this data with udev or something, but it seemed like there
  was a disconnect there. I'm not really married to doing it this
  way :)

We've been using this driver out of tree for a little while now
on our devices without issues. With that being said, we're on an
older kernel (Debian 13), so this version has seen less testing.

Please let me know what you think and if this would be better served by
a patch series or something. I originally structured it that way, but it
became kinda a pain working on two different histories in parallel, and
the synchronization stuff took several iterations to get right.

Signed-off-by: Mary Strodl <mstrodl@csh.rit.edu>
---
 drivers/gpio/gpio-mpsse.c | 319 ++++++++++++++++++++++++++++++--------
 1 file changed, 257 insertions(+), 62 deletions(-)

diff --git a/drivers/gpio/gpio-mpsse.c b/drivers/gpio/gpio-mpsse.c
index 9f42bb30b4ec..af57e7cbf969 100644
--- a/drivers/gpio/gpio-mpsse.c
+++ b/drivers/gpio/gpio-mpsse.c
@@ -17,7 +17,7 @@ struct mpsse_priv {
 	struct usb_device *udev;     /* USB device encompassing all MPSSEs */
 	struct usb_interface *intf;  /* USB interface for this MPSSE */
 	u8 intf_id;                  /* USB interface number for this MPSSE */
-	struct work_struct irq_work; /* polling work thread */
+	struct list_head workers;    /* polling work threads */
 	struct mutex irq_mutex;	     /* lock over irq_data */
 	atomic_t irq_type[16];	     /* pin -> edge detection type */
 	atomic_t irq_enabled;
@@ -26,6 +26,9 @@ struct mpsse_priv {
 	u8 gpio_outputs[2];	     /* Output states for GPIOs [L, H] */
 	u8 gpio_dir[2];		     /* Directions for GPIOs [L, H] */
 
+	unsigned long dir_in;        /* Bitmask of valid input pins  */
+	unsigned long dir_out;       /* Bitmask of valid output pins */
+
 	u8 *bulk_in_buf;	     /* Extra recv buffer to grab status bytes */
 
 	struct usb_endpoint_descriptor *bulk_in;
@@ -34,6 +37,15 @@ struct mpsse_priv {
 	struct mutex io_mutex;	    /* sync I/O with disconnect */
 };
 
+struct mpsse_worker {
+	struct mpsse_priv  *priv;
+	struct work_struct  work;
+	atomic_t       cancelled;
+	struct list_head    list;   /* linked list */
+	struct list_head destroy;   /* teardown linked list */
+	struct rcu_head     rcu;    /* synchronization */
+};
+
 struct bulk_desc {
 	bool tx;	            /* direction of bulk transfer */
 	u8 *data;                   /* input (tx) or output (rx) */
@@ -43,8 +55,27 @@ struct bulk_desc {
 	int timeout;
 };
 
+#define MPSSE_NGPIO 16
+
+struct mpsse_quirk {
+	const char   *names[MPSSE_NGPIO]; /* Pin names, if applicable     */
+	unsigned long dir_in;             /* Bitmask of valid input pins  */
+	unsigned long dir_out;            /* Bitmask of valid output pins */
+};
+
+static struct mpsse_quirk bryx_brik_quirk = {
+	.names = {
+		[3] = "Push to Talk",
+		[5] = "Channel Activity",
+	},
+	.dir_out = ~BIT(3),	/* Push to Talk     */
+	.dir_in  = ~BIT(5),	/* Channel Activity */
+};
+
 static const struct usb_device_id gpio_mpsse_table[] = {
 	{ USB_DEVICE(0x0c52, 0xa064) },   /* SeaLevel Systems, Inc. */
+	{ USB_DEVICE(0x0403, 0x6988),     /* FTDI, assigned to Bryx */
+	  .driver_info = (kernel_ulong_t)&bryx_brik_quirk},
 	{ }                               /* Terminating entry */
 };
 
@@ -160,6 +191,32 @@ static int gpio_mpsse_get_bank(struct mpsse_priv *priv, u8 bank)
 	return buf;
 }
 
+static int mpsse_ensure_supported(struct gpio_chip *chip,
+				  unsigned long *mask, int direction)
+{
+	unsigned long supported, unsupported;
+	char *type = "input";
+	struct mpsse_priv *priv = gpiochip_get_data(chip);
+
+	supported = priv->dir_in;
+	if (direction == GPIO_LINE_DIRECTION_OUT) {
+		supported = priv->dir_out;
+		type = "output";
+	}
+
+	/* An invalid bit was in the provided mask */
+	unsupported = *mask & supported;
+	if (unsupported) {
+		dev_err(&priv->udev->dev,
+			"mpsse: GPIO %ld doesn't support %s\n",
+			find_first_bit(&unsupported, sizeof(unsupported) * 8),
+			type);
+		return -EOPNOTSUPP;
+	}
+
+	return 0;
+}
+
 static int gpio_mpsse_set_multiple(struct gpio_chip *chip, unsigned long *mask,
 				   unsigned long *bits)
 {
@@ -167,6 +224,10 @@ static int gpio_mpsse_set_multiple(struct gpio_chip *chip, unsigned long *mask,
 	int ret;
 	struct mpsse_priv *priv = gpiochip_get_data(chip);
 
+	ret = mpsse_ensure_supported(chip, mask, GPIO_LINE_DIRECTION_OUT);
+	if (ret)
+		return ret;
+
 	guard(mutex)(&priv->io_mutex);
 	for_each_set_clump8(i, bank_mask, mask, chip->ngpio) {
 		bank = i / 8;
@@ -194,6 +255,10 @@ static int gpio_mpsse_get_multiple(struct gpio_chip *chip, unsigned long *mask,
 	int ret;
 	struct mpsse_priv *priv = gpiochip_get_data(chip);
 
+	ret = mpsse_ensure_supported(chip, mask, GPIO_LINE_DIRECTION_IN);
+	if (ret)
+		return ret;
+
 	guard(mutex)(&priv->io_mutex);
 	for_each_set_clump8(i, bank_mask, mask, chip->ngpio) {
 		bank = i / 8;
@@ -242,9 +307,15 @@ static int gpio_mpsse_gpio_set(struct gpio_chip *chip, unsigned int offset,
 static int gpio_mpsse_direction_output(struct gpio_chip *chip,
 				       unsigned int offset, int value)
 {
+	int ret;
 	struct mpsse_priv *priv = gpiochip_get_data(chip);
 	int bank = (offset & 8) >> 3;
 	int bank_offset = offset & 7;
+	unsigned long mask = BIT(offset);
+
+	ret = mpsse_ensure_supported(chip, &mask, GPIO_LINE_DIRECTION_OUT);
+	if (ret)
+		return ret;
 
 	scoped_guard(mutex, &priv->io_mutex)
 		priv->gpio_dir[bank] |= BIT(bank_offset);
@@ -255,15 +326,20 @@ static int gpio_mpsse_direction_output(struct gpio_chip *chip,
 static int gpio_mpsse_direction_input(struct gpio_chip *chip,
 				      unsigned int offset)
 {
+	int ret;
 	struct mpsse_priv *priv = gpiochip_get_data(chip);
 	int bank = (offset & 8) >> 3;
 	int bank_offset = offset & 7;
+	unsigned long mask = BIT(offset);
+
+	ret = mpsse_ensure_supported(chip, &mask, GPIO_LINE_DIRECTION_IN);
+	if (ret)
+		return ret;
 
 	guard(mutex)(&priv->io_mutex);
 	priv->gpio_dir[bank] &= ~BIT(bank_offset);
-	gpio_mpsse_set_bank(priv, bank);
 
-	return 0;
+	return gpio_mpsse_set_bank(priv, bank);
 }
 
 static int gpio_mpsse_get_direction(struct gpio_chip *chip,
@@ -284,76 +360,111 @@ static int gpio_mpsse_get_direction(struct gpio_chip *chip,
 	return ret;
 }
 
-static void gpio_mpsse_poll(struct work_struct *work)
+static void gpio_mpsse_stop(struct mpsse_worker *worker)
+{
+	cancel_work_sync(&worker->work);
+	devm_kfree(&worker->priv->udev->dev, worker);
+}
+
+static void gpio_mpsse_poll(struct work_struct *my_work)
 {
 	unsigned long pin_mask, pin_states, flags;
 	int irq_enabled, offset, err, value, fire_irq,
 		irq, old_value[16], irq_type[16];
-	struct mpsse_priv *priv = container_of(work, struct mpsse_priv,
-					       irq_work);
+	struct mpsse_worker *worker;
+	struct mpsse_worker *my_worker = container_of(my_work, struct mpsse_worker, work);
+	struct mpsse_priv *priv = my_worker->priv;
+	struct list_head destructors = LIST_HEAD_INIT(destructors);
 
 	for (offset = 0; offset < priv->gpio.ngpio; ++offset)
 		old_value[offset] = -1;
 
-	while ((irq_enabled = atomic_read(&priv->irq_enabled))) {
-		usleep_range(MPSSE_POLL_INTERVAL, MPSSE_POLL_INTERVAL + 1000);
-		/* Cleanup will trigger at the end of the loop */
-		guard(mutex)(&priv->irq_mutex);
-
-		pin_mask = 0;
-		pin_states = 0;
-		for (offset = 0; offset < priv->gpio.ngpio; ++offset) {
-			irq_type[offset] = atomic_read(&priv->irq_type[offset]);
-			if (irq_type[offset] != IRQ_TYPE_NONE &&
-			    irq_enabled & BIT(offset))
-				pin_mask |= BIT(offset);
-			else
-				old_value[offset] = -1;
+	scoped_guard(mutex, &priv->irq_mutex) {
+		rcu_read_lock();
+		list_for_each_entry_rcu(worker, &priv->workers, list) {
+			/* Don't stop ourselves */
+			if (worker == my_worker)
+				continue;
+			list_del_rcu(&worker->list);
+			/* Give worker a chance to terminate itself */
+			atomic_set(&worker->cancelled, 1);
+			/* Keep track of stuff to cancel */
+			INIT_LIST_HEAD(&worker->destroy);
+			list_add(&worker->destroy, &destructors);
 		}
+		rcu_read_unlock();
+		/* Make sure list consumers are finished before we tear down */
+		synchronize_rcu();
+		list_for_each_entry(worker, &destructors, destroy)
+			gpio_mpsse_stop(worker);
+	}
 
-		err = gpio_mpsse_get_multiple(&priv->gpio, &pin_mask,
-					      &pin_states);
-		if (err) {
-			dev_err_ratelimited(&priv->intf->dev,
-					    "Error polling!\n");
-			continue;
-		}
+	while ((irq_enabled = atomic_read(&priv->irq_enabled)) &&
+	       !atomic_read(&my_worker->cancelled)) {
+		usleep_range(MPSSE_POLL_INTERVAL, MPSSE_POLL_INTERVAL + 1000);
 
-		/* Check each value */
-		for (offset = 0; offset < priv->gpio.ngpio; ++offset) {
-			if (old_value[offset] == -1)
+		/* Cleanup will trigger at the end of the loop */
+		/* We can't just lock here, otherwise we'll deadlock with
+		 * the worker teardown
+		 */
+		scoped_cond_guard(mutex_try, continue, &priv->irq_mutex) {
+			pin_mask = 0;
+			pin_states = 0;
+			for (offset = 0; offset < priv->gpio.ngpio; ++offset) {
+				irq_type[offset] =
+					atomic_read(&priv->irq_type[offset]);
+				if (irq_type[offset] != IRQ_TYPE_NONE &&
+				    irq_enabled & BIT(offset))
+					pin_mask |= BIT(offset);
+				else
+					old_value[offset] = -1;
+			}
+
+			err = gpio_mpsse_get_multiple(&priv->gpio, &pin_mask,
+						      &pin_states);
+			if (err) {
+				dev_err_ratelimited(&priv->intf->dev,
+						    "Error polling!\n");
 				continue;
+			}
 
-			fire_irq = 0;
-			value = pin_states & BIT(offset);
-
-			switch (irq_type[offset]) {
-			case IRQ_TYPE_EDGE_RISING:
-				fire_irq = value > old_value[offset];
-				break;
-			case IRQ_TYPE_EDGE_FALLING:
-				fire_irq = value < old_value[offset];
-				break;
-			case IRQ_TYPE_EDGE_BOTH:
-				fire_irq = value != old_value[offset];
-				break;
+			/* Check each value */
+			for (offset = 0; offset < priv->gpio.ngpio; ++offset) {
+				if (old_value[offset] == -1)
+					continue;
+
+				fire_irq = 0;
+				value = pin_states & BIT(offset);
+
+				switch (irq_type[offset]) {
+				case IRQ_TYPE_EDGE_RISING:
+					fire_irq = value > old_value[offset];
+					break;
+				case IRQ_TYPE_EDGE_FALLING:
+					fire_irq = value < old_value[offset];
+					break;
+				case IRQ_TYPE_EDGE_BOTH:
+					fire_irq = value != old_value[offset];
+					break;
+				}
+				if (!fire_irq)
+					continue;
+
+				irq = irq_find_mapping(priv->gpio.irq.domain,
+						       offset);
+				local_irq_save(flags);
+				generic_handle_irq(irq);
+				local_irq_disable();
+				local_irq_restore(flags);
 			}
-			if (!fire_irq)
-				continue;
 
-			irq = irq_find_mapping(priv->gpio.irq.domain,
-					       offset);
-			local_irq_save(flags);
-			generic_handle_irq(irq);
-			local_irq_disable();
-			local_irq_restore(flags);
+			/* Sync back values so we can refer to them next tick */
+			for (offset = 0; offset < priv->gpio.ngpio; ++offset)
+				if (irq_type[offset] != IRQ_TYPE_NONE &&
+				    irq_enabled & BIT(offset))
+					old_value[offset] =
+						pin_states & BIT(offset);
 		}
-
-		/* Sync back values so we can refer to them next tick */
-		for (offset = 0; offset < priv->gpio.ngpio; ++offset)
-			if (irq_type[offset] != IRQ_TYPE_NONE &&
-			    irq_enabled & BIT(offset))
-				old_value[offset] = pin_states & BIT(offset);
 	}
 }
 
@@ -370,21 +481,38 @@ static int gpio_mpsse_set_irq_type(struct irq_data *irqd, unsigned int type)
 
 static void gpio_mpsse_irq_disable(struct irq_data *irqd)
 {
+	struct mpsse_worker *worker;
 	struct mpsse_priv *priv = irq_data_get_irq_chip_data(irqd);
 
 	atomic_and(~BIT(irqd->hwirq), &priv->irq_enabled);
 	gpiochip_disable_irq(&priv->gpio, irqd->hwirq);
+
+	rcu_read_lock();
+	list_for_each_entry_rcu(worker, &priv->workers, list) {
+		/* Can't actually do teardown in IRQ context (blocks...) */
+		atomic_set(&worker->cancelled, 1);
+	}
+	rcu_read_unlock();
 }
 
 static void gpio_mpsse_irq_enable(struct irq_data *irqd)
 {
+	struct mpsse_worker *worker;
 	struct mpsse_priv *priv = irq_data_get_irq_chip_data(irqd);
 
 	gpiochip_enable_irq(&priv->gpio, irqd->hwirq);
 	/* If no-one else was using the IRQ, enable it */
 	if (!atomic_fetch_or(BIT(irqd->hwirq), &priv->irq_enabled)) {
-		INIT_WORK(&priv->irq_work, gpio_mpsse_poll);
-		schedule_work(&priv->irq_work);
+		worker = devm_kmalloc(&priv->udev->dev, sizeof(*worker), GFP_KERNEL);
+		if (!worker)
+			return;
+
+		worker->priv = priv;
+		INIT_LIST_HEAD(&worker->list);
+		INIT_WORK(&worker->work, gpio_mpsse_poll);
+		schedule_work(&worker->work);
+
+		list_add_rcu(&worker->list, &priv->workers);
 	}
 }
 
@@ -404,18 +532,50 @@ static void gpio_mpsse_ida_remove(void *data)
 	ida_free(&gpio_mpsse_ida, priv->id);
 }
 
+static int mpsse_init_valid_mask(struct gpio_chip *chip,
+				 unsigned long *valid_mask,
+				 unsigned int ngpios)
+{
+	struct mpsse_priv *priv = gpiochip_get_data(chip);
+
+	if (WARN_ON(priv == NULL))
+		return -ENODEV;
+
+	/* If bit is set in both, set to 0 (NAND) */
+	*valid_mask = ~priv->dir_in | ~priv->dir_out;
+
+	return 0;
+}
+
+static void mpsse_irq_init_valid_mask(struct gpio_chip *chip,
+				      unsigned long *valid_mask,
+				      unsigned int ngpios)
+{
+	struct mpsse_priv *priv = gpiochip_get_data(chip);
+
+	if (WARN_ON(priv == NULL))
+		return;
+
+	/* Can only use IRQ on input capable pins */
+	*valid_mask = ~priv->dir_in;
+}
+
 static int gpio_mpsse_probe(struct usb_interface *interface,
 			    const struct usb_device_id *id)
 {
 	struct mpsse_priv *priv;
 	struct device *dev;
+	char *serial;
 	int err;
+	struct mpsse_quirk *quirk = (void *)id->driver_info;
 
 	dev = &interface->dev;
 	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
 		return -ENOMEM;
 
+	INIT_LIST_HEAD(&priv->workers);
+
 	priv->udev = usb_get_dev(interface_to_usbdev(interface));
 	priv->intf = interface;
 	priv->intf_id = interface->cur_altsetting->desc.bInterfaceNumber;
@@ -436,9 +596,15 @@ static int gpio_mpsse_probe(struct usb_interface *interface,
 	if (err)
 		return err;
 
+	serial = priv->udev->serial;
+	if (!serial)
+		serial = "NONE";
+
 	priv->gpio.label = devm_kasprintf(dev, GFP_KERNEL,
-					  "gpio-mpsse.%d.%d",
-					  priv->id, priv->intf_id);
+					  "MPSSE%04x:%04x.%d.%d.%s",
+					  id->idVendor, id->idProduct,
+					  priv->intf_id, priv->id,
+					  serial);
 	if (!priv->gpio.label)
 		return -ENOMEM;
 
@@ -452,10 +618,17 @@ static int gpio_mpsse_probe(struct usb_interface *interface,
 	priv->gpio.get_multiple = gpio_mpsse_get_multiple;
 	priv->gpio.set_multiple = gpio_mpsse_set_multiple;
 	priv->gpio.base = -1;
-	priv->gpio.ngpio = 16;
+	priv->gpio.ngpio = MPSSE_NGPIO;
 	priv->gpio.offset = priv->intf_id * priv->gpio.ngpio;
 	priv->gpio.can_sleep = 1;
 
+	if (quirk) {
+		priv->dir_out = quirk->dir_out;
+		priv->dir_in = quirk->dir_in;
+		priv->gpio.names = quirk->names;
+		priv->gpio.init_valid_mask = mpsse_init_valid_mask;
+	}
+
 	err = usb_find_common_endpoints(interface->cur_altsetting,
 					&priv->bulk_in, &priv->bulk_out,
 					NULL, NULL);
@@ -494,6 +667,7 @@ static int gpio_mpsse_probe(struct usb_interface *interface,
 	priv->gpio.irq.parents = NULL;
 	priv->gpio.irq.default_type = IRQ_TYPE_NONE;
 	priv->gpio.irq.handler = handle_simple_irq;
+	priv->gpio.irq.init_valid_mask = mpsse_irq_init_valid_mask;
 
 	err = devm_gpiochip_add_data(dev, &priv->gpio, priv);
 	if (err)
@@ -504,7 +678,28 @@ static int gpio_mpsse_probe(struct usb_interface *interface,
 
 static void gpio_mpsse_disconnect(struct usb_interface *intf)
 {
+	struct mpsse_worker *worker;
 	struct mpsse_priv *priv = usb_get_intfdata(intf);
+	struct list_head destructors = LIST_HEAD_INIT(destructors);
+
+	scoped_guard(mutex, &priv->irq_mutex) {
+		rcu_read_lock();
+		list_for_each_entry_rcu(worker, &priv->workers, list) {
+			list_del_rcu(&worker->list);
+			/* Give worker a chance to terminate itself */
+			atomic_set(&worker->cancelled, 1);
+			/* Keep track of stuff to cancel */
+			INIT_LIST_HEAD(&worker->destroy);
+			list_add(&worker->destroy, &destructors);
+		}
+		rcu_read_unlock();
+		/* Make sure list consumers are finished before we tear down */
+		synchronize_rcu();
+		list_for_each_entry(worker, &destructors, destroy)
+			gpio_mpsse_stop(worker);
+	}
+
+	rcu_barrier();
 
 	priv->intf = NULL;
 	usb_set_intfdata(intf, NULL);
-- 
2.47.0

Hi Mary,

kernel test robot noticed the following build warnings:

[auto build test WARNING on brgl/gpio/for-next]
[also build test WARNING on linus/master v6.17-rc5 next-20250909]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Mary-Strodl/gpio-mpsse-support-bryx-radio-interface-kit/20250909-014744
base:   https://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux.git gpio/for-next
patch link:    https://lore.kernel.org/r/20250908173804.3816149-1-mstrodl%40csh.rit.edu
patch subject: [PATCH] gpio: mpsse: support bryx radio interface kit
config: m68k-allmodconfig (https://download.01.org/0day-ci/archive/20250909/202509092305.ncd9mzaZ-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 15.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250909/202509092305.ncd9mzaZ-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202509092305.ncd9mzaZ-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from include/linux/device.h:15,
                    from include/linux/usb.h:19,
                    from drivers/gpio/gpio-mpsse.c:13:
   drivers/gpio/gpio-mpsse.c: In function 'mpsse_ensure_supported':
>> drivers/gpio/gpio-mpsse.c:211:25: warning: format '%ld' expects argument of type 'long int', but argument 3 has type 'int' [-Wformat=]
     211 |                         "mpsse: GPIO %ld doesn't support %s\n",
         |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/dev_printk.h:110:30: note: in definition of macro 'dev_printk_index_wrap'
     110 |                 _p_func(dev, fmt, ##__VA_ARGS__);                       \
         |                              ^~~
   include/linux/dev_printk.h:154:56: note: in expansion of macro 'dev_fmt'
     154 |         dev_printk_index_wrap(_dev_err, KERN_ERR, dev, dev_fmt(fmt), ##__VA_ARGS__)
         |                                                        ^~~~~~~
   drivers/gpio/gpio-mpsse.c:210:17: note: in expansion of macro 'dev_err'
     210 |                 dev_err(&priv->udev->dev,
         |                 ^~~~~~~
   drivers/gpio/gpio-mpsse.c:211:40: note: format string is defined here
     211 |                         "mpsse: GPIO %ld doesn't support %s\n",
         |                                      ~~^
         |                                        |
         |                                        long int
         |                                      %d


vim +211 drivers/gpio/gpio-mpsse.c

   193	
   194	static int mpsse_ensure_supported(struct gpio_chip *chip,
   195					  unsigned long *mask, int direction)
   196	{
   197		unsigned long supported, unsupported;
   198		char *type = "input";
   199		struct mpsse_priv *priv = gpiochip_get_data(chip);
   200	
   201		supported = priv->dir_in;
   202		if (direction == GPIO_LINE_DIRECTION_OUT) {
   203			supported = priv->dir_out;
   204			type = "output";
   205		}
   206	
   207		/* An invalid bit was in the provided mask */
   208		unsupported = *mask & supported;
   209		if (unsupported) {
   210			dev_err(&priv->udev->dev,
 > 211				"mpsse: GPIO %ld doesn't support %s\n",
   212				find_first_bit(&unsupported, sizeof(unsupported) * 8),
   213				type);
   214			return -EOPNOTSUPP;
   215		}
   216	
   217		return 0;
   218	}
   219	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

On Tue, Sep 09, 2025 at 09:32:09PM +0800, kernel test robot wrote:
>    drivers/gpio/gpio-mpsse.c: In function 'mpsse_ensure_supported':
> >> drivers/gpio/gpio-mpsse.c:211:25: warning: format '%ld' expects argument of type 'long int', but argument 3 has type 'int' [-Wformat=]
>      211 |                         "mpsse: GPIO %ld doesn't support %s\n",
>          |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Oops. Turns out this is m68k-specific weirdness. v2 will convert the
result to an int and use %d.

I'll hold off on posting it because I'm anticipating there will be some feedback
on the actual code changes that will need a revision anyways ;)

Hi Mary,

On Wed, 10 Sept 2025 at 15:24, Mary Strodl <mstrodl@csh.rit.edu> wrote:
> On Tue, Sep 09, 2025 at 09:32:09PM +0800, kernel test robot wrote:
> >    drivers/gpio/gpio-mpsse.c: In function 'mpsse_ensure_supported':
> > >> drivers/gpio/gpio-mpsse.c:211:25: warning: format '%ld' expects argument of type 'long int', but argument 3 has type 'int' [-Wformat=]
> >      211 |                         "mpsse: GPIO %ld doesn't support %s\n",
> >          |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> Oops. Turns out this is m68k-specific weirdness. v2 will convert the
> result to an int and use %d.
>
> I'll hold off on posting it because I'm anticipating there will be some feedback
> on the actual code changes that will need a revision anyways ;)

Please don't.  The m68k version is wrong.  I will send a patch to fix it.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

On Wed, Sep 10, 2025 at 04:55:05PM +0200, Geert Uytterhoeven wrote:
> Please don't.  The m68k version is wrong.  I will send a patch to fix it.
Even better. Thanks!

On Wed, Sep 10, 2025 at 2:47 PM Mary Strodl <mstrodl@csh.rit.edu> wrote:
>
> On Tue, Sep 09, 2025 at 09:32:09PM +0800, kernel test robot wrote:
> >    drivers/gpio/gpio-mpsse.c: In function 'mpsse_ensure_supported':
> > >> drivers/gpio/gpio-mpsse.c:211:25: warning: format '%ld' expects argument of type 'long int', but argument 3 has type 'int' [-Wformat=]
> >      211 |                         "mpsse: GPIO %ld doesn't support %s\n",
> >          |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> Oops. Turns out this is m68k-specific weirdness. v2 will convert the
> result to an int and use %d.
>
> I'll hold off on posting it because I'm anticipating there will be some feedback
> on the actual code changes that will need a revision anyways ;)

I cannot really give you much feedback because this patch should be
first split into smaller chunks that explain what each change is
doing. As it is: it's so complex, I simply don't understand it and
don't have enough time to try and decipher it. Please try to make it
into a series of smaller patches.

Bartosz

On Wed, Sep 10, 2025 at 03:15:46PM +0200, Bartosz Golaszewski wrote:
> I cannot really give you much feedback because this patch should be
> first split into smaller chunks that explain what each change is
> doing. As it is: it's so complex, I simply don't understand it and
> don't have enough time to try and decipher it. Please try to make it
> into a series of smaller patches.

Got it. That's why I offered to break it up.

Here's the order I did the work in:
1. Add quirk support + brik quirk
2. Label format (Only a few lines, could go into 1 or 3 if preferred)
3. RCU stuff

Is this a reasonable order for the series? This would be the easiest
way for me to do it given what I have in git.

In a perfect world, I would like to have had:

1. RCU stuff (These are effectively bugfixes)
2. Label stuff
3. Quirks
4. Brik quirk (possibly squashed into 3)

Let me know what you think... Thank you!

On Wed, Sep 10, 2025 at 3:29 PM Mary Strodl <mstrodl@csh.rit.edu> wrote:
>
> On Wed, Sep 10, 2025 at 03:15:46PM +0200, Bartosz Golaszewski wrote:
> > I cannot really give you much feedback because this patch should be
> > first split into smaller chunks that explain what each change is
> > doing. As it is: it's so complex, I simply don't understand it and
> > don't have enough time to try and decipher it. Please try to make it
> > into a series of smaller patches.
>
> Got it. That's why I offered to break it up.
>
> Here's the order I did the work in:
> 1. Add quirk support + brik quirk
> 2. Label format (Only a few lines, could go into 1 or 3 if preferred)
> 3. RCU stuff
>
> Is this a reasonable order for the series? This would be the easiest
> way for me to do it given what I have in git.
>
> In a perfect world, I would like to have had:
>
> 1. RCU stuff (These are effectively bugfixes)

If these are bugfixes that should be backported to stable then they
should indeed come first.

FYI: Maybe consider also adding lock guards for rcu read locks if you're at it?

Bart

> 2. Label stuff
> 3. Quirks
> 4. Brik quirk (possibly squashed into 3)
>
> Let me know what you think... Thank you!

On Wed, Sep 10, 2025 at 03:46:03PM +0200, Bartosz Golaszewski wrote:
> If these are bugfixes that should be backported to stable then they
> should indeed come first.

Yeah I agree... I'll try my best :)

To be fair, in practice this can never happen with the hardware that
implements this since it can't do hotplug.

...With that said, I think you can bind it to another device with
new_id so maybe it would still be good to backport? Dunno.

On Wed, Sep 10, 2025 at 03:46:03PM +0200, Bartosz Golaszewski wrote:
> FYI: Maybe consider also adding lock guards for rcu read locks if you're at it?

I'll take a look! I didn't notice there were guards for rcu when I
was working on this.

Thank you!