[PATCH v3 0/4] gpio: mpsse: add support for bryx brik

[PATCH v3 0/4] gpio: mpsse: add support for bryx brik

[Mary Strodl]

Hey all,

This series adds support for the Bryx Radio Interface Kit to the gpio-mpsse
driver

Here are some of the major differences compared to the sealevel device this
driver currently supports:
* Uses an FT232HL chip instead of FT2232HL (this is easy, just populates as
  only one interface rather than two)
* There are only two exposed GPIO lines, and each is hardware restricted to
  a particular direction.
* This is an external device, therefore hotpluggable. This caused me to
  discover the race condition in the polling worker teradown, which
  accounts for the bulk of the changes.

The locking changes probably should be backported even though the actual
device isn't hotpluggable. If this isn't the right avenue for introducing
those fixes and it should be sent as a separate patch first, let me know
and it can be structured that way instead.

Other than the locking changes, this series also adds a generic "quirk"
system like I have seen in similar drivers for providing device-specific
line labels and direction restrictions. This should enable easier
integration of new devices in the future.

Lastly, I changed the device label format to expose useful device
information like the device serial number, vid, and pid to userspace. If
there is a better way to get this information (perhaps through udev?), I'm
all ears.

I also noticed a little bug where gpio_mpsse_direction_input wasn't
actually propagating errors, so I have a quick fix for that too.

Changes since v2:
* No more RCU, just use a spinlock to protect the stuff that matters
* Now using _safe variants of list iteration helpers where appropriate
* Use GFP_NOWAIT kmalloc flag
* Move repeated code into new gpio_mpsse_stop_all_except method
* Move little bugfix for direction_input error propagation into its own
  patch

Changes since v1:
* Break out into separate patches
* Fix RCU/concurrency soundness mistakes I noticed (list add/del were not
  protected by a lock, so now there is a separate spin lock, which we can
  use in irq context)
* Use guards for rcu read locks

Let me know what you think!

Mary Strodl (4):
  gpio: mpsse: propagate error from direction_input
  gpio: mpsse: ensure worker is torn down
  gpio: mpsse: add quirk support
  gpio: mpsse: support bryx radio interface kit

 drivers/gpio/gpio-mpsse.c | 229 ++++++++++++++++++++++++++++++++++++--
 1 file changed, 217 insertions(+), 12 deletions(-)

-- 
2.47.0

[PATCH v3 1/4] gpio: mpsse: propagate error from direction_input

[Mary Strodl]

Not sure how I missed this, but errors encountered when setting the
direction to input weren't being propagated to the caller.

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

diff --git a/drivers/gpio/gpio-mpsse.c b/drivers/gpio/gpio-mpsse.c
index 9f42bb30b4ec..c508d9e33054 100644
--- a/drivers/gpio/gpio-mpsse.c
+++ b/drivers/gpio/gpio-mpsse.c
@@ -261,9 +261,8 @@ static int gpio_mpsse_direction_input(struct gpio_chip *chip,
 
 	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,
-- 
2.47.0

[PATCH v3 2/4] gpio: mpsse: ensure worker is torn down

[Mary Strodl]

When an IRQ worker is running, unplugging the device would cause a
crash. The sealevel hardware this driver was written for was not
hotpluggable, so I never realized it.

This change uses a spinlock to protect a list of workers, which
it tears down on disconnect.

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

diff --git a/drivers/gpio/gpio-mpsse.c b/drivers/gpio/gpio-mpsse.c
index c508d9e33054..6ec940f6b371 100644
--- a/drivers/gpio/gpio-mpsse.c
+++ b/drivers/gpio/gpio-mpsse.c
@@ -10,6 +10,7 @@
 #include <linux/cleanup.h>
 #include <linux/gpio/driver.h>
 #include <linux/mutex.h>
+#include <linux/spinlock.h>
 #include <linux/usb.h>
 
 struct mpsse_priv {
@@ -17,8 +18,10 @@ 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 */
+	struct mutex irq_race;	     /* race for polling worker teardown */
+	raw_spinlock_t irq_spin;     /* protects worker list */
 	atomic_t irq_type[16];	     /* pin -> edge detection type */
 	atomic_t irq_enabled;
 	int id;
@@ -34,6 +37,14 @@ 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 bulk_desc {
 	bool tx;	            /* direction of bulk transfer */
 	u8 *data;                   /* input (tx) or output (rx) */
@@ -283,18 +294,62 @@ static int gpio_mpsse_get_direction(struct gpio_chip *chip,
 	return ret;
 }
 
-static void gpio_mpsse_poll(struct work_struct *work)
+/*
+ * Stops all workers except `my_worker`.
+ * Safe to call only when `irq_race` is held.
+ */
+static void gpio_mpsse_stop_all_except(struct mpsse_priv *priv,
+				       struct mpsse_worker *my_worker)
+{
+	struct mpsse_worker *worker, *worker_tmp;
+	struct list_head destructors = LIST_HEAD_INIT(destructors);
+
+	scoped_guard(raw_spinlock_irqsave, &priv->irq_spin) {
+		list_for_each_entry_safe(worker, worker_tmp,
+					 &priv->workers, list) {
+			/* Don't stop ourselves */
+			if (worker == my_worker)
+				continue;
+
+			list_del(&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);
+		}
+	}
+
+	list_for_each_entry_safe(worker, worker_tmp,
+				 &destructors, destroy) {
+		list_del(&worker->destroy);
+		cancel_work_sync(&worker->work);
+		kfree(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 *my_worker = container_of(my_work, struct mpsse_worker, work);
+	struct mpsse_priv *priv = my_worker->priv;
 
 	for (offset = 0; offset < priv->gpio.ngpio; ++offset)
 		old_value[offset] = -1;
 
-	while ((irq_enabled = atomic_read(&priv->irq_enabled))) {
+	/*
+	 * We only want one worker. Workers race to acquire irq_race and tear
+	 * down all other workers. This is a cond guard so that we don't deadlock
+	 * trying to cancel a worker.
+	 */
+	scoped_cond_guard(mutex_try, return, &priv->irq_race)
+		gpio_mpsse_stop_all_except(priv, my_worker);
+
+	while ((irq_enabled = atomic_read(&priv->irq_enabled)) &&
+	       !atomic_read(&my_worker->cancelled)) {
 		usleep_range(MPSSE_POLL_INTERVAL, MPSSE_POLL_INTERVAL + 1000);
 		/* Cleanup will trigger at the end of the loop */
 		guard(mutex)(&priv->irq_mutex);
@@ -369,21 +424,45 @@ 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);
+
+	/*
+	 * Can't actually do teardown in IRQ context (it blocks).
+	 * As a result, these workers will stick around until irq is reenabled
+	 * or device gets disconnected
+	 */
+	scoped_guard(raw_spinlock_irqsave, &priv->irq_spin)
+		list_for_each_entry(worker, &priv->workers, list)
+			atomic_set(&worker->cancelled, 1);
 }
 
 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);
+		/*
+		 * Can't be devm because it uses a non-raw spinlock (illegal in
+		 * this context, where a raw spinlock is held by our caller)
+		 */
+		worker = kzalloc(sizeof(*worker), GFP_NOWAIT);
+		if (!worker)
+			return;
+
+		worker->priv = priv;
+		INIT_LIST_HEAD(&worker->list);
+		INIT_WORK(&worker->work, gpio_mpsse_poll);
+		schedule_work(&worker->work);
+
+		scoped_guard(raw_spinlock_irqsave, &priv->irq_spin)
+			list_add(&worker->list, &priv->workers);
 	}
 }
 
@@ -435,6 +514,12 @@ static int gpio_mpsse_probe(struct usb_interface *interface,
 	if (err)
 		return err;
 
+	err = devm_mutex_init(dev, &priv->irq_race);
+	if (err)
+		return err;
+
+	raw_spin_lock_init(&priv->irq_spin);
+
 	priv->gpio.label = devm_kasprintf(dev, GFP_KERNEL,
 					  "gpio-mpsse.%d.%d",
 					  priv->id, priv->intf_id);
@@ -505,6 +590,13 @@ static void gpio_mpsse_disconnect(struct usb_interface *intf)
 {
 	struct mpsse_priv *priv = usb_get_intfdata(intf);
 
+	/*
+	 * Lock prevents double-free of worker from here and the teardown
+	 * step at the beginning of gpio_mpsse_poll
+	 */
+	scoped_guard(mutex, &priv->irq_race)
+		gpio_mpsse_stop_all_except(priv, NULL);
+
 	priv->intf = NULL;
 	usb_set_intfdata(intf, NULL);
 	usb_put_dev(priv->udev);
-- 
2.47.0

[PATCH v3 3/4] gpio: mpsse: add quirk support

[Mary Strodl]

Builds out a facility for specifying compatible lines directions and
labels for MPSSE-based devices.

* dir_in/out are bitmask of lines that can go in/out. 0 means
  compatible, 1 means incompatible. This is convenient, because it means
  if the struct is zeroed out, it supports all pins.
* names is an array of line names which will be exposed to userspace.

Also changes the chip label format to include some more useful
information about the device to help identify it from userspace.

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

diff --git a/drivers/gpio/gpio-mpsse.c b/drivers/gpio/gpio-mpsse.c
index 6ec940f6b371..c2a344488a23 100644
--- a/drivers/gpio/gpio-mpsse.c
+++ b/drivers/gpio/gpio-mpsse.c
@@ -29,6 +29,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;
@@ -54,6 +57,14 @@ 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 const struct usb_device_id gpio_mpsse_table[] = {
 	{ USB_DEVICE(0x0c52, 0xa064) },   /* SeaLevel Systems, Inc. */
 	{ }                               /* Terminating entry */
@@ -171,6 +182,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 %d doesn't support %s\n",
+			(int)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)
 {
@@ -178,6 +215,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;
@@ -205,6 +246,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;
@@ -253,9 +298,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);
@@ -266,9 +317,15 @@ 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);
@@ -482,18 +539,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;
@@ -520,9 +609,15 @@ static int gpio_mpsse_probe(struct usb_interface *interface,
 
 	raw_spin_lock_init(&priv->irq_spin);
 
+	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;
 
@@ -536,10 +631,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);
@@ -578,6 +680,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)
-- 
2.47.0

Re: [PATCH v3 3/4] gpio: mpsse: add quirk support

[Dan Carpenter]

On Thu, Oct 02, 2025 at 02:11:35PM -0400, Mary Strodl wrote:
> Builds out a facility for specifying compatible lines directions and
> labels for MPSSE-based devices.
> 
> * dir_in/out are bitmask of lines that can go in/out. 0 means
>   compatible, 1 means incompatible. This is convenient, because it means
>   if the struct is zeroed out, it supports all pins.

What?  No, please make 1 be supported and 0 be not supported.  This
really weirded me out when I read the code.  If it were just 1 for
true and 0 for false a lot of comments regarding dir_in/out could be
removed because the code would be straight forward.

You can easily set everything to 1 by assigning -1 or using memset().

> * names is an array of line names which will be exposed to userspace.
> 
> Also changes the chip label format to include some more useful
> information about the device to help identify it from userspace.
> 
> Signed-off-by: Mary Strodl <mstrodl@csh.rit.edu>
> ---
>  drivers/gpio/gpio-mpsse.c | 109 ++++++++++++++++++++++++++++++++++++--
>  1 file changed, 106 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-mpsse.c b/drivers/gpio/gpio-mpsse.c
> index 6ec940f6b371..c2a344488a23 100644
> --- a/drivers/gpio/gpio-mpsse.c
> +++ b/drivers/gpio/gpio-mpsse.c
> @@ -29,6 +29,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;
> @@ -54,6 +57,14 @@ 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 const struct usb_device_id gpio_mpsse_table[] = {
>  	{ USB_DEVICE(0x0c52, 0xa064) },   /* SeaLevel Systems, Inc. */
>  	{ }                               /* Terminating entry */
> @@ -171,6 +182,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)

Just pass mask not a pointer to mask.  It would be different if
you were going to allow more than 32 bits to be set, then we would
need to pass a pointer and use set_bit() etc...

> +{
> +	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 %d doesn't support %s\n",
> +			(int)find_first_bit(&unsupported, sizeof(unsupported) * 8),

Use %lu instead of %d and get rid of the unnecessary cast.

> +			type);
> +		return -EOPNOTSUPP;
> +	}
> +
> +	return 0;
> +}

regards,
dan carpenter

Re: [PATCH v3 3/4] gpio: mpsse: add quirk support

[Mary Strodl]

Hey Dan,

On Thu, Oct 02, 2025 at 11:34:13PM +0300, Dan Carpenter wrote:
> On Thu, Oct 02, 2025 at 02:11:35PM -0400, Mary Strodl wrote:
> > * dir_in/out are bitmask of lines that can go in/out. 0 means
> >   compatible, 1 means incompatible. This is convenient, because it means
> >   if the struct is zeroed out, it supports all pins.
> 
> What?  No, please make 1 be supported and 0 be not supported.  This
> really weirded me out when I read the code.  If it were just 1 for
> true and 0 for false a lot of comments regarding dir_in/out could be
> removed because the code would be straight forward.
> 
> You can easily set everything to 1 by assigning -1 or using memset().
Okay. I was thinking leaving out the fields altogether would be most convenient.

> > +static int mpsse_ensure_supported(struct gpio_chip *chip,
> > +				  unsigned long *mask, int direction)
> 
> Just pass mask not a pointer to mask.  It would be different if
> you were going to allow more than 32 bits to be set, then we would
> need to pass a pointer and use set_bit() etc...
The reason is because of set/get_multiple. I can deref in there instead though.

> > +		dev_err(&priv->udev->dev,
> > +			"mpsse: GPIO %d doesn't support %s\n",
> > +			(int)find_first_bit(&unsupported, sizeof(unsupported) * 8),
> 
> Use %lu instead of %d and get rid of the unnecessary cast.
Good catch. This was a fix for m68k builds that I forgot to remove:
https://lore.kernel.org/linux-gpio/CAMuHMdVpdk5JaqXf6LkSWQvZ7FnfyLdMWOJX+7n0=PZ-udH-WA@mail.gmail.com/

Thanks for taking a look!

[PATCH v3 4/4] gpio: mpsse: support bryx radio interface kit

[Mary Strodl]

This device is powered by an FT232H, which is very similar to the
FT2232H this driver was written for. The key difference is it has only
one MPSSE instead of two. As a result, it presents only one USB
interface to the system, which conveniently "just works" out of the box
with this driver.

The brik exposes only two GPIO lines which are hardware limited to only
be useful in one direction. As a result, I've restricted things on the
driver side to refuse to configure any other lines.

This device, unlike the sealevel device I wrote this driver for
originally, is hotpluggable, which makes for all sorts of weird
edgecases. I've tried my best to stress-test the parts that could go
wrong, but given the new usecase, more heads taking a critical look at
the teardown and synchronization bits on the driver as a whole would be
appreciated.

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

diff --git a/drivers/gpio/gpio-mpsse.c b/drivers/gpio/gpio-mpsse.c
index c2a344488a23..3c0645374ba7 100644
--- a/drivers/gpio/gpio-mpsse.c
+++ b/drivers/gpio/gpio-mpsse.c
@@ -65,8 +65,19 @@ struct mpsse_quirk {
 	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 */
 };
 
-- 
2.47.0