diff mbox series

[RFC,2/9] mfd: Add driver for Multifunction USB Device

Message ID 20200216172117.49832-3-noralf@tronnes.org (mailing list archive)
State New, archived
Headers show
Series Regmap over USB for Multifunction USB Device (gpio, display, ...) | expand

Commit Message

Noralf Trønnes Feb. 16, 2020, 5:21 p.m. UTC 
A Multifunction USB Device is a device that supports functions like gpio
and display or any other function that can be represented as a USB regmap.
Interrupts over USB is also supported if such an endpoint is present.

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/mfd/Kconfig     |   8 +
 drivers/mfd/Makefile    |   1 +
 drivers/mfd/mud.c       | 580 ++++++++++++++++++++++++++++++++++++++++
 include/linux/mfd/mud.h |  16 ++
 4 files changed, 605 insertions(+)
 create mode 100644 drivers/mfd/mud.c
 create mode 100644 include/linux/mfd/mud.h

Comments

Lee Jones Feb. 27, 2020, 9:09 a.m. UTC | #1 
I'd really like someone from USB to have a look through this too.

I'll do a quick first pass and provide some general comments though.

On Sun, 16 Feb 2020, Noralf Trønnes wrote:
> A Multifunction USB Device is a device that supports functions like gpio
> and display or any other function that can be represented as a USB regmap.
> Interrupts over USB is also supported if such an endpoint is present.

Do you have a datasheet?

> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> ---
>  drivers/mfd/Kconfig     |   8 +
>  drivers/mfd/Makefile    |   1 +
>  drivers/mfd/mud.c       | 580 ++++++++++++++++++++++++++++++++++++++++

As interesting as a "mud driver" sounds.  Something more forthcoming
might be better, "multi_usb" as a very quick example, but perhaps
something more imaginative/distinguishing would be better.

>  include/linux/mfd/mud.h |  16 ++
>  4 files changed, 605 insertions(+)
>  create mode 100644 drivers/mfd/mud.c
>  create mode 100644 include/linux/mfd/mud.h
> 
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 52818dbcfe1f..9950794d907e 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -1968,6 +1968,14 @@ config MFD_STMFX
>  	  additional drivers must be enabled in order to use the functionality
>  	  of the device.
>  
> +config MFD_MUD
> +	tristate "Multifunction USB Device core driver"
> +	depends on USB
> +	select MFD_CORE
> +	select REGMAP_USB
> +	help
> +	  Select this to get support for the Multifunction USB Device.
> +
>  menu "Multimedia Capabilities Port drivers"
>  	depends on ARCH_SA1100
>  
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index 29e6767dd60c..0adfab9afaed 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -255,4 +255,5 @@ obj-$(CONFIG_MFD_ROHM_BD70528)	+= rohm-bd70528.o
>  obj-$(CONFIG_MFD_ROHM_BD718XX)	+= rohm-bd718x7.o
>  obj-$(CONFIG_MFD_STMFX) 	+= stmfx.o
>  obj-$(CONFIG_MFD_RPISENSE_CORE)	+= rpisense-core.o
> +obj-$(CONFIG_MFD_MUD)		+= mud.o
>  
> diff --git a/drivers/mfd/mud.c b/drivers/mfd/mud.c
> new file mode 100644
> index 000000000000..f5f31478656d
> --- /dev/null
> +++ b/drivers/mfd/mud.c
> @@ -0,0 +1,580 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright 2020 Noralf Trønnes
> + */
> +
> +#include <linux/bitmap.h>
> +#include <linux/interrupt.h>
> +#include <linux/irq.h>
> +#include <linux/irqdomain.h>
> +#include <linux/list.h>
> +#include <linux/mfd/core.h>
> +#include <linux/mfd/mud.h>
> +#include <linux/module.h>
> +#include <linux/regmap.h>
> +#include <linux/regmap_usb.h>
> +#include <linux/seq_file.h>
> +#include <linux/slab.h>
> +#include <linux/spinlock.h>
> +#include <linux/usb.h>
> +#include <linux/workqueue.h>
> +
> +/* Temporary debugging aid */
> +#undef dev_dbg
> +#define dev_dbg dev_info
> +
> +#define mdebug(fmt, ...)			\
> +do {						\
> +	if (1)					\
> +		pr_debug(fmt, ##__VA_ARGS__);	\
> +} while (0)

No thank you.

We already have quite a few debugging facilities in the kernel.

> +struct mud_irq_event {
> +	struct list_head node;
> +	DECLARE_BITMAP(status, REGMAP_USB_MAX_MAPS);
> +};
> +
> +struct mud_irq {
> +	struct irq_domain *domain;
> +	unsigned int num_irqs;
> +
> +	struct workqueue_struct	*workq;
> +	struct work_struct work;
> +	struct urb *urb;
> +
> +	spinlock_t lock; /* Protect the values below */
> +	unsigned long *mask;
> +	u16 tag;
> +	struct list_head eventlist;
> +
> +	unsigned int stats_illegal;
> +	unsigned int stats_already_seen;
> +	unsigned int stats_lost;
> +};

I think this should have a Kerneldoc header.

> +struct mud_device {

s/device/ddata/

> +	struct usb_device *usb;
> +	struct mud_irq *mirq;

> +	struct mfd_cell *cells;

Why does this need to be in here?

> +	unsigned int num_cells;

Why do these need to be stored in device data?

> +};
> +
> +static void mud_irq_work(struct work_struct *work)
> +{
> +	struct mud_irq *mirq = container_of(work, struct mud_irq, work);
> +	struct mud_irq_event *event;
> +	unsigned long n, flags;
> +	unsigned int irq;
> +
> +	mdebug("%s: IN\n", __func__);

All of these prints need to go.  They have no place in an upstreamed
driver and the whole thing reads much better without them.

> +	while (true) {
> +		spin_lock_irqsave(&mirq->lock, flags);
> +		event = list_first_entry_or_null(&mirq->eventlist, struct mud_irq_event, node);
> +		if (event) {
> +			list_del(&event->node);
> +			mdebug("    status: %*pb\n", mirq->num_irqs, event->status);
> +			bitmap_and(event->status, event->status, mirq->mask, mirq->num_irqs);
> +		}
> +		spin_unlock_irqrestore(&mirq->lock, flags);

'\n'

> +		if (!event)
> +			break;
> +
> +		for_each_set_bit(n, event->status, mirq->num_irqs) {
> +			irq = irq_find_mapping(mirq->domain, n);
> +			mdebug("        n=%lu irq=%u\n", n, irq);
> +			if (irq)
> +				handle_nested_irq(irq);
> +		}
> +
> +		kfree(event);
> +	}
> +
> +	mdebug("%s: OUT\n", __func__);
> +}
> +
> +#define BYTES_PER_LONG	(BITS_PER_LONG / BITS_PER_BYTE)
> +
> +static void mud_irq_queue(struct urb *urb)
> +{
> +	u8 *buf = urb->transfer_buffer + sizeof(u16);

Comment.

> +	struct mud_irq *mirq = urb->context;
> +	struct device *dev = &urb->dev->dev;
> +	struct mud_irq_event *event = NULL;
> +	unsigned int i, tag, diff;
> +	unsigned long flags;
> +
> +	if (urb->actual_length != urb->transfer_buffer_length) {
> +		dev_err_once(dev, "Interrupt packet wrong length: %u\n",
> +			     urb->actual_length);
> +		mirq->stats_illegal++;
> +		return;
> +	}
> +
> +	spin_lock_irqsave(&mirq->lock, flags);
> +
> +	tag = le16_to_cpup(urb->transfer_buffer);
> +	if (tag == mirq->tag) {
> +		dev_dbg(dev, "Interrupt tag=%u already seen, ignoring\n", tag);
> +		mirq->stats_already_seen++;
> +		goto unlock;
> +	}
> +
> +	if (tag > mirq->tag)
> +		diff = tag - mirq->tag;
> +	else
> +		diff = U16_MAX - mirq->tag + tag + 1;

Comment.

> +	if (diff > 1) {
> +		dev_err_once(dev, "Interrupts lost: %u\n", diff - 1);
> +		mirq->stats_lost += diff - 1;
> +	}
> +
> +	event = kzalloc(sizeof(*event), GFP_ATOMIC);
> +	if (!event) {
> +		mirq->stats_lost += 1;
> +		goto unlock;
> +	}

This hides a potential OOM issue.

Not sure what to do about that.  Maybe the USB guys have an idea.

> +	list_add_tail(&event->node, &mirq->eventlist);
> +
> +	for (i = 0; i < (urb->transfer_buffer_length - sizeof(u16)); i++) {
> +		unsigned long *val = &event->status[i / BYTES_PER_LONG];
> +		unsigned int mod = i % BYTES_PER_LONG;
> +
> +		if (!mod)
> +			*val = buf[i];
> +		else
> +			*val |= ((unsigned long)buf[i]) << (mod * BITS_PER_BYTE);
> +	}

Comment.  In fact, comments throughout please.

(I'm going to stop saying "comment" from here on).

> +	mdebug("%s: tag=%u\n", __func__, tag);
> +
> +	mirq->tag = tag;
> +unlock:
> +	spin_unlock_irqrestore(&mirq->lock, flags);
> +
> +	if (event)
> +		queue_work(mirq->workq, &mirq->work);
> +}
> +
> +static void mud_irq_urb_completion(struct urb *urb)
> +{
> +	struct device *dev = &urb->dev->dev;
> +	int ret;
> +
> +	mdebug("%s: actual_length=%u\n", __func__, urb->actual_length);
> +
> +	switch (urb->status) {
> +	case 0:
> +		mud_irq_queue(urb);
> +		break;
> +	case -EPROTO:	/* FIXME: verify: dwc2 reports this on disconnect */

What does this mean?  Why can't you fix it now?

> +	case -ECONNRESET:
> +	case -ENOENT:
> +	case -ESHUTDOWN:
> +		dev_dbg(dev, "irq urb shutting down with status: %d\n", urb->status);

s/irq/IRQ/ in all comments and prints.

Same with URB?

> +		return;
> +	default:
> +		dev_dbg(dev, "irq urb failure with status: %d\n", urb->status);
> +		break;

So it's failed, but you're going to attempt to submit it anyway?

> +	}
> +
> +	ret = usb_submit_urb(urb, GFP_ATOMIC);
> +	if (ret && ret != -ENODEV)
> +		dev_err(dev, "irq usb_submit_urb failed with result %d\n", ret);
> +}
> +
> +static void mud_irq_mask(struct irq_data *data)
> +{
> +	struct mud_irq *mirq = irq_data_get_irq_chip_data(data);
> +	unsigned long flags;
> +
> +	mdebug("%s: hwirq=%lu\n", __func__, data->hwirq);
> +
> +	spin_lock_irqsave(&mirq->lock, flags);
> +	clear_bit(data->hwirq, mirq->mask);
> +	spin_unlock_irqrestore(&mirq->lock, flags);
> +}
> +
> +static void mud_irq_unmask(struct irq_data *data)
> +{
> +	struct mud_irq *mirq = irq_data_get_irq_chip_data(data);
> +	unsigned long flags;
> +
> +	mdebug("%s: hwirq=%lu\n", __func__, data->hwirq);
> +
> +	spin_lock_irqsave(&mirq->lock, flags);
> +	set_bit(data->hwirq, mirq->mask);
> +	spin_unlock_irqrestore(&mirq->lock, flags);
> +}
> +
> +static struct irq_chip mud_irq_chip = {
> +	.name			= "mud-irq",
> +	.irq_mask		= mud_irq_mask,
> +	.irq_unmask		= mud_irq_unmask,
> +};

This tabbing seems excessive.

> +static void __maybe_unused
> +mud_irq_domain_debug_show(struct seq_file *m, struct irq_domain *d,
> +			  struct irq_data *data, int ind)

Best not to over-shorten variable names for no good reason.

'm', 'd' and 'ind' aren't good choices.

> +{
> +	struct mud_irq *mirq = d ? d->host_data : irq_data_get_irq_chip_data(data);
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&mirq->lock, flags);
> +
> +	seq_printf(m, "%*sTag:          %u\n", ind, "", mirq->tag);
> +	seq_printf(m, "%*sIllegal:      %u\n", ind, "", mirq->stats_illegal);
> +	seq_printf(m, "%*sAlready seen: %u\n", ind, "", mirq->stats_already_seen);
> +	seq_printf(m, "%*sLost:         %u\n", ind, "", mirq->stats_lost);
> +
> +	spin_unlock_irqrestore(&mirq->lock, flags);
> +}
> +
> +static int mud_irq_domain_map(struct irq_domain *d, unsigned int virq,
> +			      irq_hw_number_t hwirq)
> +{
> +	irq_set_chip_data(virq, d->host_data);
> +	irq_set_chip_and_handler(virq, &mud_irq_chip, handle_simple_irq);
> +	irq_set_nested_thread(virq, true);
> +	irq_set_noprobe(virq);
> +
> +	return 0;
> +}
> +
> +static void mud_irq_domain_unmap(struct irq_domain *d, unsigned int virq)
> +{
> +	irq_set_chip_and_handler(virq, NULL, NULL);
> +	irq_set_chip_data(virq, NULL);
> +}
> +
> +static const struct irq_domain_ops mud_irq_ops = {
> +	.map		= mud_irq_domain_map,
> +	.unmap		= mud_irq_domain_unmap,
> +#ifdef CONFIG_GENERIC_IRQ_DEBUGFS
> +	.debug_show	= mud_irq_domain_debug_show,
> +#endif
> +};
> +
> +static int mud_irq_start(struct mud_irq *mirq)
> +{
> +	if (!mirq)
> +		return 0;
> +
> +	return usb_submit_urb(mirq->urb, GFP_KERNEL);
> +}
> +
> +static void mud_irq_stop(struct mud_irq *mirq)
> +{
> +	if (!mirq)
> +		return;
> +
> +	usb_kill_urb(mirq->urb);
> +	flush_work(&mirq->work);
> +}
> +
> +static void mud_irq_release(struct mud_irq *mirq)
> +{
> +	if (!mirq)
> +		return;
> +
> +	if (mirq->workq)
> +		destroy_workqueue(mirq->workq);
> +
> +	if (mirq->domain) {
> +		irq_hw_number_t hwirq;
> +
> +		for (hwirq = 0; hwirq < mirq->num_irqs; hwirq++)
> +			irq_dispose_mapping(irq_find_mapping(mirq->domain, hwirq));
> +
> +		irq_domain_remove(mirq->domain);
> +	}
> +
> +	usb_free_coherent(mirq->urb->dev, mirq->urb->transfer_buffer_length,
> +			  mirq->urb->transfer_buffer, mirq->urb->transfer_dma);
> +	usb_free_urb(mirq->urb);
> +	bitmap_free(mirq->mask);
> +	kfree(mirq);
> +}
> +
> +static struct mud_irq *mud_irq_create(struct usb_interface *interface,
> +				      unsigned int num_irqs)
> +{
> +	struct usb_device *usb = interface_to_usbdev(interface);
> +	struct device *dev = &interface->dev;
> +	struct usb_endpoint_descriptor *ep;
> +	struct fwnode_handle *fn;
> +	struct urb *urb = NULL;
> +	struct mud_irq *mirq;
> +	void *buf = NULL;
> +	size_t buf_size;
> +	int ret;
> +
> +	mdebug("%s: dev->id=%d\n", __func__, dev->id);
> +
> +	ret = usb_find_int_in_endpoint(interface->cur_altsetting, &ep);
> +	if (ret == -ENXIO)
> +		return NULL;
> +	if (ret)
> +		return ERR_PTR(ret);
> +
> +	mirq = kzalloc(sizeof(*mirq), GFP_KERNEL);
> +	if (!mirq)
> +		return ERR_PTR(-ENOMEM);
> +
> +	mirq->mask = bitmap_zalloc(num_irqs, GFP_KERNEL);
> +	if (!mirq->mask) {
> +		ret = -ENOMEM;
> +		goto release;
> +	}
> +
> +	spin_lock_init(&mirq->lock);
> +	mirq->num_irqs = num_irqs;
> +
> +	urb = usb_alloc_urb(0, GFP_KERNEL);
> +	if (!urb) {
> +		ret = -ENOMEM;
> +		goto release;
> +	}
> +
> +	buf_size = usb_endpoint_maxp(ep);
> +	if (buf_size != (sizeof(u16) + DIV_ROUND_UP(num_irqs, BITS_PER_BYTE))) {
> +		dev_err(dev, "Interrupt endpoint wMaxPacketSize too small: %zu\n", buf_size);
> +		ret = -EINVAL;
> +		goto release;
> +	}
> +
> +	buf = usb_alloc_coherent(usb, buf_size, GFP_KERNEL, &urb->transfer_dma);
> +	if (!buf) {
> +		usb_free_urb(urb);
> +		ret = -ENOMEM;
> +		goto release;
> +	}
> +
> +	usb_fill_int_urb(urb, usb,
> +			 usb_rcvintpipe(usb, usb_endpoint_num(ep)),
> +			 buf, buf_size, mud_irq_urb_completion,
> +			 mirq, ep->bInterval);
> +	urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
> +
> +	mirq->urb = urb;
> +
> +	if (dev->of_node) {
> +		fn = of_node_to_fwnode(dev->of_node);
> +	} else {
> +		fn = irq_domain_alloc_named_fwnode("mud-irq");
> +		if (!fn) {
> +			ret = -ENOMEM;
> +			goto release;
> +		}
> +	}
> +
> +	mirq->domain = irq_domain_create_linear(fn, num_irqs, &mud_irq_ops, mirq);
> +	if (!dev->of_node)
> +		irq_domain_free_fwnode(fn);
> +	if (!mirq->domain) {
> +		ret = -ENOMEM;
> +		goto release;
> +	}
> +
> +	INIT_LIST_HEAD(&mirq->eventlist);
> +	INIT_WORK(&mirq->work, mud_irq_work);
> +
> +	mirq->workq = alloc_workqueue("mud-irq/%s", WQ_HIGHPRI, 0, dev_name(dev));
> +	if (!mirq->workq) {
> +		ret = -ENOMEM;
> +		goto release;
> +	}
> +
> +	return mirq;
> +
> +release:
> +	mud_irq_release(mirq);
> +
> +	return ERR_PTR(ret);
> +}
> +
> +static int mud_probe_regmap(struct usb_interface *interface, struct mfd_cell *cell,
> +			    unsigned int index, struct mud_irq *mirq)
> +{
> +	struct mud_cell_pdata *pdata;
> +	struct resource *res = NULL;
> +	int ret;
> +
> +	pdata = kzalloc(sizeof(*pdata), GFP_KERNEL);
> +	if (!pdata)
> +		return -ENOMEM;
> +
> +	ret = regmap_usb_get_map_descriptor(interface, index, &pdata->desc);

Can you give an example of what a desc might look like?

I'm particularly interested in pdata->desc.name.

> +	if (ret)
> +		goto error;

This will attempt to free 'res' which is currently NULL.

> +	mdebug("%s: name='%s' index=%u\n", __func__, pdata->desc.name, index);
> +	mdebug("    bRegisterValueBits=%u\n", pdata->desc.bRegisterValueBits);
> +	mdebug("    bCompression=0x%02x\n", pdata->desc.bCompression);
> +	mdebug("    bMaxTransferSizeOrder=%u (%ukB)\n",
> +	       pdata->desc.bMaxTransferSizeOrder,
> +	       (1 << pdata->desc.bMaxTransferSizeOrder) / 1024);
> +
> +	if (mirq) {
> +		res = kzalloc(sizeof(*res), GFP_KERNEL);
> +		if (!res) {
> +			ret = -ENOMEM;
> +			goto error;

This will attempt to free 'res' which is currently NULL.

> +		}
> +
> +		res->flags = IORESOURCE_IRQ;
> +		res->start = irq_create_mapping(mirq->domain, index);
> +		mdebug("    res->start=%u\n", (unsigned int)res->start);
> +		res->end = res->start;
> +
> +		cell->resources = res;
> +		cell->num_resources = 1;
> +	}
> +
> +	pdata->interface = interface;

This looks like something that should be stored in ddata.

> +	pdata->index = index;

Don't usually like indexes - what is this used for?

> +	cell->name = pdata->desc.name;
> +	cell->platform_data = pdata;
> +	cell->pdata_size = sizeof(*pdata);
> +	/*
> +	 * A Multifunction USB Device can have multiple functions of the same
> +	 * type. mfd_add_device() in its current form will only match on the
> +	 * first node in the Device Tree.
> +	 */
> +	cell->of_compatible = cell->name;
> +
> +	return 0;
> +
> +error:
> +	kfree(res);

I think you should remove this line, as it's never useful here.

> +	kfree(pdata);
> +
> +	return ret;
> +}
> +
> +static void mud_free(struct mud_device *mud)
> +{
> +	unsigned int i;
> +
> +	mud_irq_release(mud->mirq);
> +
> +	for (i = 0; i < mud->num_cells; i++) {
> +		kfree(mud->cells[i].platform_data);
> +		kfree(mud->cells[i].resources);
> +	}
> +
> +	kfree(mud->cells);
> +	kfree(mud);
> +}
> +
> +static int mud_probe(struct usb_interface *interface,
> +		     const struct usb_device_id *id)
> +{
> +	struct device *dev = &interface->dev;
> +	unsigned int i, num_regmaps;
> +	struct mud_device *mud;
> +	int ret;
> +
> +	mdebug("%s: interface->dev.of_node=%px usb->dev.of_node=%px",
> +	       __func__, interface->dev.of_node,
> +	       usb_get_dev(interface_to_usbdev(interface))->dev.of_node);
> +
> +	ret = regmap_usb_get_interface_descriptor(interface, &num_regmaps);
> +	if (ret)
> +		return ret;
> +	if (!num_regmaps)
> +		return -EINVAL;
> +
> +	mdebug("%s: num_regmaps=%u\n", __func__, num_regmaps);
> +
> +	mud = kzalloc(sizeof(*mud), GFP_KERNEL);
> +	if (!mud)
> +		return -ENOMEM;
> +
> +	mud->mirq = mud_irq_create(interface, num_regmaps);
> +	if (IS_ERR(mud->mirq)) {
> +		ret = PTR_ERR(mud->mirq);
> +		goto err_free;
> +	}
> +
> +	mud->num_cells = num_regmaps;
> +	mud->cells = kcalloc(num_regmaps, sizeof(*mud->cells), GFP_KERNEL);
> +	if (!mud->cells)
> +		goto err_free;
> +
> +	for (i = 0; i < num_regmaps; i++) {
> +		ret = mud_probe_regmap(interface, &mud->cells[i], i, mud->mirq);
> +		if (ret) {
> +			dev_err(dev, "Failed to probe regmap index %i (error %d)\n", i, ret);
> +			goto err_free;
> +		}
> +	}
> +
> +	ret = mud_irq_start(mud->mirq);
> +	if (ret) {
> +		dev_err(dev, "Failed to start irq (error %d)\n", ret);
> +		goto err_free;
> +	}
> +
> +	ret = mfd_add_hotplug_devices(dev, mud->cells, mud->num_cells);
> +	if (ret) {
> +		dev_err(dev, "Failed to add mfd devices to core.");

"MFD" is not a thing.  It's made up.

"Failed to register child devices" makes more sense.

NIT: I don't see full stops in any of your other messages.

> +		goto err_stop;
> +	}
> +
> +	mud->usb = usb_get_dev(interface_to_usbdev(interface));
> +
> +	usb_set_intfdata(interface, mud);
> +
> +	if (mud->usb->product)
> +		dev_info(dev, "%s\n", mud->usb->product);
> +
> +	return 0;
> +
> +err_stop:
> +	mud_irq_stop(mud->mirq);
> +err_free:
> +	mud_free(mud);
> +
> +	return ret;
> +}
> +
> +static void mud_disconnect(struct usb_interface *interface)
> +{
> +	struct mud_device *mud = usb_get_intfdata(interface);
> +
> +	mfd_remove_devices(&interface->dev);
> +	mud_irq_stop(mud->mirq);
> +	usb_put_dev(mud->usb);
> +	mud_free(mud);
> +
> +	dev_dbg(&interface->dev, "disconnected\n");
> +}
> +
> +static const struct usb_device_id mud_table[] = {
> +	/*
> +	 * FIXME:
> +	 * Apply for a proper pid: https://github.com/openmoko/openmoko-usb-oui
> +	 *
> +	 * Or maybe the Linux Foundation will provide one from their vendor id.
> +	 */

Probably not a good idea to take this into the upstream kernel without
a valid, registered PID.  Suggest you do this *first*.

> +	{ USB_DEVICE_INTERFACE_CLASS(0x1d50, 0x6150, USB_CLASS_VENDOR_SPEC) },
> +	{ }
> +};
> +
> +MODULE_DEVICE_TABLE(usb, mud_table);
> +
> +static struct usb_driver mud_driver = {
> +	.name		= "mud",
> +	.probe		= mud_probe,
> +	.disconnect	= mud_disconnect,
> +	.id_table	= mud_table,
> +};
> +
> +module_usb_driver(mud_driver);
> +
> +MODULE_DESCRIPTION("Generic USB Device mfd core driver");
> +MODULE_AUTHOR("Noralf Trønnes <noralf@tronnes.org>");
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/mfd/mud.h b/include/linux/mfd/mud.h
> new file mode 100644
> index 000000000000..b2059fa57429
> --- /dev/null
> +++ b/include/linux/mfd/mud.h
> @@ -0,0 +1,16 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +
> +#ifndef __LINUX_MUD_H
> +#define __LINUX_MUD_H
> +
> +#include <linux/regmap_usb.h>
> +
> +struct usb_interface;
> +
> +struct mud_cell_pdata {
> +	struct usb_interface *interface;

Shouldn't be here - more of a ddata thing.

> +	unsigned int index;

Indexes are generally not a good idea.

> +	struct regmap_usb_map_descriptor desc;

Shouldn't be here - more of a ddata thing.

> +};
> +
> +#endif
Noralf Trønnes Feb. 29, 2020, 1:26 p.m. UTC | #2 
Den 27.02.2020 10.09, skrev Lee Jones:
> I'd really like someone from USB to have a look through this too.
> 
> I'll do a quick first pass and provide some general comments though.
> 
> On Sun, 16 Feb 2020, Noralf Trønnes wrote:
>> A Multifunction USB Device is a device that supports functions like gpio
>> and display or any other function that can be represented as a USB regmap.
>> Interrupts over USB is also supported if such an endpoint is present.
> 
> Do you have a datasheet?

As mentioned in the cover letter this is about turning a Linux board
like the Raspberry Pi into a USB gadget that presents functions like
display, gpio, spi, i2c to a host over USB. Patch 3 in this series
contains the gadget side of this mfd driver.

Patch 1 has the register over USB code implemented as a regmap. After
talking to Mark Brown I realised that regmap has a limitation (no
variable register value width) so my plan is to include that code into
this mfd driver instead.

> 
>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>> ---

>> +static void mud_irq_urb_completion(struct urb *urb)
>> +{
>> +	struct device *dev = &urb->dev->dev;
>> +	int ret;
>> +
>> +	mdebug("%s: actual_length=%u\n", __func__, urb->actual_length);
>> +
>> +	switch (urb->status) {
>> +	case 0:
>> +		mud_irq_queue(urb);
>> +		break;
>> +	case -EPROTO:	/* FIXME: verify: dwc2 reports this on disconnect */
> 
> What does this mean?  Why can't you fix it now?

I don't know if this is a dwc2 driver problem or if EPROTO is a valid
disconnect error. I haven't seen it in other gadget drivers, so I need
to look more into this or even better if someone from USB can answer this.

> 
>> +	case -ECONNRESET:
>> +	case -ENOENT:
>> +	case -ESHUTDOWN:
>> +		dev_dbg(dev, "irq urb shutting down with status: %d\n", urb->status);
> 
> s/irq/IRQ/ in all comments and prints.
> 
> Same with URB?
> 
>> +		return;
>> +	default:
>> +		dev_dbg(dev, "irq urb failure with status: %d\n", urb->status);
>> +		break;
> 
> So it's failed, but you're going to attempt to submit it anyway?

Yes, I don't know the reason why it failed, it might succeed the next
time. But this is also something that someone with real life experience
with USB failures could weigh in on. Maybe I should send a reset request
so the device can reset its state machine, I don't know.

> 
>> +	}
>> +
>> +	ret = usb_submit_urb(urb, GFP_ATOMIC);
>> +	if (ret && ret != -ENODEV)
>> +		dev_err(dev, "irq usb_submit_urb failed with result %d\n", ret);
>> +}


>> +static int mud_probe_regmap(struct usb_interface *interface, struct mfd_cell *cell,
>> +			    unsigned int index, struct mud_irq *mirq)
>> +{
>> +	struct mud_cell_pdata *pdata;
>> +	struct resource *res = NULL;
>> +	int ret;
>> +
>> +	pdata = kzalloc(sizeof(*pdata), GFP_KERNEL);
>> +	if (!pdata)
>> +		return -ENOMEM;
>> +
>> +	ret = regmap_usb_get_map_descriptor(interface, index, &pdata->desc);
> 
> Can you give an example of what a desc might look like?
> 
> I'm particularly interested in pdata->desc.name.
> 

This is the definition:

/**
 * struct regmap_usb_map_descriptor - Regmap descriptor
 * @bLength: Size of descriptor in bytes
 * @bDescriptorType: DescriptorType (REGMAP_USB_DT_MAP)
 * @name: Register name (NUL terminated)
 * @bRegisterValueBits: Number of bits in the register value
 * @bCompression: Supported compression types
 * @bMaxTransferSizeOrder: Maximum transfer size the device can handle
as log2.
 */
struct regmap_usb_map_descriptor {
	__u8 bLength;
	__u8 bDescriptorType;

	__u8 name[32];
	__u8 bRegisterValueBits;
	__u8 bCompression;
#define REGMAP_USB_COMPRESSION_LZ4	BIT(0)
	__u8 bMaxTransferSizeOrder;
} __packed;


>> +	if (ret)
>> +		goto error;
> 
> This will attempt to free 'res' which is currently NULL.
> 
>> +	mdebug("%s: name='%s' index=%u\n", __func__, pdata->desc.name, index);
>> +	mdebug("    bRegisterValueBits=%u\n", pdata->desc.bRegisterValueBits);
>> +	mdebug("    bCompression=0x%02x\n", pdata->desc.bCompression);
>> +	mdebug("    bMaxTransferSizeOrder=%u (%ukB)\n",
>> +	       pdata->desc.bMaxTransferSizeOrder,
>> +	       (1 << pdata->desc.bMaxTransferSizeOrder) / 1024);
>> +
>> +	if (mirq) {
>> +		res = kzalloc(sizeof(*res), GFP_KERNEL);
>> +		if (!res) {
>> +			ret = -ENOMEM;
>> +			goto error;
> 
> This will attempt to free 'res' which is currently NULL.
> 
>> +		}
>> +
>> +		res->flags = IORESOURCE_IRQ;
>> +		res->start = irq_create_mapping(mirq->domain, index);
>> +		mdebug("    res->start=%u\n", (unsigned int)res->start);
>> +		res->end = res->start;
>> +
>> +		cell->resources = res;
>> +		cell->num_resources = 1;
>> +	}
>> +
>> +	pdata->interface = interface;
> 
> This looks like something that should be stored in ddata.
> 
>> +	pdata->index = index;
> 
> Don't usually like indexes - what is this used for?

A maximum of 255 register maps are supported on one USB interface and
this index tells which one it is. It's passed in the USB transfer header.

> 
>> +	cell->name = pdata->desc.name;
>> +	cell->platform_data = pdata;
>> +	cell->pdata_size = sizeof(*pdata);
>> +	/*
>> +	 * A Multifunction USB Device can have multiple functions of the same
>> +	 * type. mfd_add_device() in its current form will only match on the
>> +	 * first node in the Device Tree.
>> +	 */
>> +	cell->of_compatible = cell->name;
>> +
>> +	return 0;
>> +
>> +error:
>> +	kfree(res);
> 
> I think you should remove this line, as it's never useful here.
> 
>> +	kfree(pdata);
>> +
>> +	return ret;
>> +}
>> +

>> +static const struct usb_device_id mud_table[] = {
>> +	/*
>> +	 * FIXME:
>> +	 * Apply for a proper pid: https://github.com/openmoko/openmoko-usb-oui
>> +	 *
>> +	 * Or maybe the Linux Foundation will provide one from their vendor id.
>> +	 */
> 
> Probably not a good idea to take this into the upstream kernel without
> a valid, registered PID.  Suggest you do this *first*.

I didn't know if my work was fundementally flawed in some way that made
it difficult to get merged. Hence the RFC to ask for help from people
knowledgeable in this area. So I'm hoping that some USB people will have
a look on this as well.

If this multifunction idea doesn't work out, then I'll just do the USB
display part and it will only be a drm driver. So at the moment I don't
know what kind of USB device this will be: multifuntion or display.
When I know then I'll get a PID.

Noralf.

> 
>> +	{ USB_DEVICE_INTERFACE_CLASS(0x1d50, 0x6150, USB_CLASS_VENDOR_SPEC) },
>> +	{ }
>> +};
Alan Stern Feb. 29, 2020, 4:02 p.m. UTC | #3 
On Sat, 29 Feb 2020, Noralf Trønnes wrote:

> >> +static void mud_irq_urb_completion(struct urb *urb)
> >> +{
> >> +	struct device *dev = &urb->dev->dev;
> >> +	int ret;
> >> +
> >> +	mdebug("%s: actual_length=%u\n", __func__, urb->actual_length);
> >> +
> >> +	switch (urb->status) {
> >> +	case 0:
> >> +		mud_irq_queue(urb);
> >> +		break;
> >> +	case -EPROTO:	/* FIXME: verify: dwc2 reports this on disconnect */
> > 
> > What does this mean?  Why can't you fix it now?
> 
> I don't know if this is a dwc2 driver problem or if EPROTO is a valid
> disconnect error. I haven't seen it in other gadget drivers, so I need

Note: This is not a gadget driver.  You should be looking in device 
drivers.

> to look more into this or even better if someone from USB can answer this.

See Documentation/driver-api/usb/error-codes.rst.  In short, -EPROTO is
one of several status codes you may get when an URB fails because the
device was disconnected.

> >> +	case -ECONNRESET:
> >> +	case -ENOENT:
> >> +	case -ESHUTDOWN:
> >> +		dev_dbg(dev, "irq urb shutting down with status: %d\n", urb->status);
> > 
> > s/irq/IRQ/ in all comments and prints.
> > 
> > Same with URB?
> > 
> >> +		return;
> >> +	default:
> >> +		dev_dbg(dev, "irq urb failure with status: %d\n", urb->status);
> >> +		break;
> > 
> > So it's failed, but you're going to attempt to submit it anyway?
> 
> Yes, I don't know the reason why it failed, it might succeed the next
> time. But this is also something that someone with real life experience
> with USB failures could weigh in on. Maybe I should send a reset request
> so the device can reset its state machine, I don't know.

USB connections are usually pretty reliable.  Sometimes there are
transient errors, but they are relatively rare.  No one would criticize
a driver for giving up the first time it gets an error (especially if
there was an easy way to reset it) -- but people will get annoyed if a
ton of error messages shows up on the console whenever they unplug the
device.

In general, the overall design of the driver seems to be reasonable.  
I can't judge the interfaces with other subsystems or the other aspects
of their design, but the USB part is okay.  (I haven't gone through it
in detail.)

Alan Stern
diff mbox series

Patch

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 52818dbcfe1f..9950794d907e 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1968,6 +1968,14 @@  config MFD_STMFX
 	  additional drivers must be enabled in order to use the functionality
 	  of the device.
 
+config MFD_MUD
+	tristate "Multifunction USB Device core driver"
+	depends on USB
+	select MFD_CORE
+	select REGMAP_USB
+	help
+	  Select this to get support for the Multifunction USB Device.
+
 menu "Multimedia Capabilities Port drivers"
 	depends on ARCH_SA1100
 
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 29e6767dd60c..0adfab9afaed 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -255,4 +255,5 @@  obj-$(CONFIG_MFD_ROHM_BD70528)	+= rohm-bd70528.o
 obj-$(CONFIG_MFD_ROHM_BD718XX)	+= rohm-bd718x7.o
 obj-$(CONFIG_MFD_STMFX) 	+= stmfx.o
 obj-$(CONFIG_MFD_RPISENSE_CORE)	+= rpisense-core.o
+obj-$(CONFIG_MFD_MUD)		+= mud.o
 
diff --git a/drivers/mfd/mud.c b/drivers/mfd/mud.c
new file mode 100644
index 000000000000..f5f31478656d
--- /dev/null
+++ b/drivers/mfd/mud.c
@@ -0,0 +1,580 @@ 
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright 2020 Noralf Trønnes
+ */
+
+#include <linux/bitmap.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/irqdomain.h>
+#include <linux/list.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/mud.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/regmap_usb.h>
+#include <linux/seq_file.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+#include <linux/usb.h>
+#include <linux/workqueue.h>
+
+/* Temporary debugging aid */
+#undef dev_dbg
+#define dev_dbg dev_info
+
+#define mdebug(fmt, ...)			\
+do {						\
+	if (1)					\
+		pr_debug(fmt, ##__VA_ARGS__);	\
+} while (0)
+
+struct mud_irq_event {
+	struct list_head node;
+	DECLARE_BITMAP(status, REGMAP_USB_MAX_MAPS);
+};
+
+struct mud_irq {
+	struct irq_domain *domain;
+	unsigned int num_irqs;
+
+	struct workqueue_struct	*workq;
+	struct work_struct work;
+	struct urb *urb;
+
+	spinlock_t lock; /* Protect the values below */
+	unsigned long *mask;
+	u16 tag;
+	struct list_head eventlist;
+
+	unsigned int stats_illegal;
+	unsigned int stats_already_seen;
+	unsigned int stats_lost;
+};
+
+struct mud_device {
+	struct usb_device *usb;
+	struct mud_irq *mirq;
+	struct mfd_cell *cells;
+	unsigned int num_cells;
+};
+
+static void mud_irq_work(struct work_struct *work)
+{
+	struct mud_irq *mirq = container_of(work, struct mud_irq, work);
+	struct mud_irq_event *event;
+	unsigned long n, flags;
+	unsigned int irq;
+
+	mdebug("%s: IN\n", __func__);
+
+	while (true) {
+		spin_lock_irqsave(&mirq->lock, flags);
+		event = list_first_entry_or_null(&mirq->eventlist, struct mud_irq_event, node);
+		if (event) {
+			list_del(&event->node);
+			mdebug("    status: %*pb\n", mirq->num_irqs, event->status);
+			bitmap_and(event->status, event->status, mirq->mask, mirq->num_irqs);
+		}
+		spin_unlock_irqrestore(&mirq->lock, flags);
+		if (!event)
+			break;
+
+		for_each_set_bit(n, event->status, mirq->num_irqs) {
+			irq = irq_find_mapping(mirq->domain, n);
+			mdebug("        n=%lu irq=%u\n", n, irq);
+			if (irq)
+				handle_nested_irq(irq);
+		}
+
+		kfree(event);
+	}
+
+	mdebug("%s: OUT\n", __func__);
+}
+
+#define BYTES_PER_LONG	(BITS_PER_LONG / BITS_PER_BYTE)
+
+static void mud_irq_queue(struct urb *urb)
+{
+	u8 *buf = urb->transfer_buffer + sizeof(u16);
+	struct mud_irq *mirq = urb->context;
+	struct device *dev = &urb->dev->dev;
+	struct mud_irq_event *event = NULL;
+	unsigned int i, tag, diff;
+	unsigned long flags;
+
+	if (urb->actual_length != urb->transfer_buffer_length) {
+		dev_err_once(dev, "Interrupt packet wrong length: %u\n",
+			     urb->actual_length);
+		mirq->stats_illegal++;
+		return;
+	}
+
+	spin_lock_irqsave(&mirq->lock, flags);
+
+	tag = le16_to_cpup(urb->transfer_buffer);
+	if (tag == mirq->tag) {
+		dev_dbg(dev, "Interrupt tag=%u already seen, ignoring\n", tag);
+		mirq->stats_already_seen++;
+		goto unlock;
+	}
+
+	if (tag > mirq->tag)
+		diff = tag - mirq->tag;
+	else
+		diff = U16_MAX - mirq->tag + tag + 1;
+
+	if (diff > 1) {
+		dev_err_once(dev, "Interrupts lost: %u\n", diff - 1);
+		mirq->stats_lost += diff - 1;
+	}
+
+	event = kzalloc(sizeof(*event), GFP_ATOMIC);
+	if (!event) {
+		mirq->stats_lost += 1;
+		goto unlock;
+	}
+
+	list_add_tail(&event->node, &mirq->eventlist);
+
+	for (i = 0; i < (urb->transfer_buffer_length - sizeof(u16)); i++) {
+		unsigned long *val = &event->status[i / BYTES_PER_LONG];
+		unsigned int mod = i % BYTES_PER_LONG;
+
+		if (!mod)
+			*val = buf[i];
+		else
+			*val |= ((unsigned long)buf[i]) << (mod * BITS_PER_BYTE);
+	}
+
+	mdebug("%s: tag=%u\n", __func__, tag);
+
+	mirq->tag = tag;
+unlock:
+	spin_unlock_irqrestore(&mirq->lock, flags);
+
+	if (event)
+		queue_work(mirq->workq, &mirq->work);
+}
+
+static void mud_irq_urb_completion(struct urb *urb)
+{
+	struct device *dev = &urb->dev->dev;
+	int ret;
+
+	mdebug("%s: actual_length=%u\n", __func__, urb->actual_length);
+
+	switch (urb->status) {
+	case 0:
+		mud_irq_queue(urb);
+		break;
+	case -EPROTO:	/* FIXME: verify: dwc2 reports this on disconnect */
+	case -ECONNRESET:
+	case -ENOENT:
+	case -ESHUTDOWN:
+		dev_dbg(dev, "irq urb shutting down with status: %d\n", urb->status);
+		return;
+	default:
+		dev_dbg(dev, "irq urb failure with status: %d\n", urb->status);
+		break;
+	}
+
+	ret = usb_submit_urb(urb, GFP_ATOMIC);
+	if (ret && ret != -ENODEV)
+		dev_err(dev, "irq usb_submit_urb failed with result %d\n", ret);
+}
+
+static void mud_irq_mask(struct irq_data *data)
+{
+	struct mud_irq *mirq = irq_data_get_irq_chip_data(data);
+	unsigned long flags;
+
+	mdebug("%s: hwirq=%lu\n", __func__, data->hwirq);
+
+	spin_lock_irqsave(&mirq->lock, flags);
+	clear_bit(data->hwirq, mirq->mask);
+	spin_unlock_irqrestore(&mirq->lock, flags);
+}
+
+static void mud_irq_unmask(struct irq_data *data)
+{
+	struct mud_irq *mirq = irq_data_get_irq_chip_data(data);
+	unsigned long flags;
+
+	mdebug("%s: hwirq=%lu\n", __func__, data->hwirq);
+
+	spin_lock_irqsave(&mirq->lock, flags);
+	set_bit(data->hwirq, mirq->mask);
+	spin_unlock_irqrestore(&mirq->lock, flags);
+}
+
+static struct irq_chip mud_irq_chip = {
+	.name			= "mud-irq",
+	.irq_mask		= mud_irq_mask,
+	.irq_unmask		= mud_irq_unmask,
+};
+
+static void __maybe_unused
+mud_irq_domain_debug_show(struct seq_file *m, struct irq_domain *d,
+			  struct irq_data *data, int ind)
+{
+	struct mud_irq *mirq = d ? d->host_data : irq_data_get_irq_chip_data(data);
+	unsigned long flags;
+
+	spin_lock_irqsave(&mirq->lock, flags);
+
+	seq_printf(m, "%*sTag:          %u\n", ind, "", mirq->tag);
+	seq_printf(m, "%*sIllegal:      %u\n", ind, "", mirq->stats_illegal);
+	seq_printf(m, "%*sAlready seen: %u\n", ind, "", mirq->stats_already_seen);
+	seq_printf(m, "%*sLost:         %u\n", ind, "", mirq->stats_lost);
+
+	spin_unlock_irqrestore(&mirq->lock, flags);
+}
+
+static int mud_irq_domain_map(struct irq_domain *d, unsigned int virq,
+			      irq_hw_number_t hwirq)
+{
+	irq_set_chip_data(virq, d->host_data);
+	irq_set_chip_and_handler(virq, &mud_irq_chip, handle_simple_irq);
+	irq_set_nested_thread(virq, true);
+	irq_set_noprobe(virq);
+
+	return 0;
+}
+
+static void mud_irq_domain_unmap(struct irq_domain *d, unsigned int virq)
+{
+	irq_set_chip_and_handler(virq, NULL, NULL);
+	irq_set_chip_data(virq, NULL);
+}
+
+static const struct irq_domain_ops mud_irq_ops = {
+	.map		= mud_irq_domain_map,
+	.unmap		= mud_irq_domain_unmap,
+#ifdef CONFIG_GENERIC_IRQ_DEBUGFS
+	.debug_show	= mud_irq_domain_debug_show,
+#endif
+};
+
+static int mud_irq_start(struct mud_irq *mirq)
+{
+	if (!mirq)
+		return 0;
+
+	return usb_submit_urb(mirq->urb, GFP_KERNEL);
+}
+
+static void mud_irq_stop(struct mud_irq *mirq)
+{
+	if (!mirq)
+		return;
+
+	usb_kill_urb(mirq->urb);
+	flush_work(&mirq->work);
+}
+
+static void mud_irq_release(struct mud_irq *mirq)
+{
+	if (!mirq)
+		return;
+
+	if (mirq->workq)
+		destroy_workqueue(mirq->workq);
+
+	if (mirq->domain) {
+		irq_hw_number_t hwirq;
+
+		for (hwirq = 0; hwirq < mirq->num_irqs; hwirq++)
+			irq_dispose_mapping(irq_find_mapping(mirq->domain, hwirq));
+
+		irq_domain_remove(mirq->domain);
+	}
+
+	usb_free_coherent(mirq->urb->dev, mirq->urb->transfer_buffer_length,
+			  mirq->urb->transfer_buffer, mirq->urb->transfer_dma);
+	usb_free_urb(mirq->urb);
+	bitmap_free(mirq->mask);
+	kfree(mirq);
+}
+
+static struct mud_irq *mud_irq_create(struct usb_interface *interface,
+				      unsigned int num_irqs)
+{
+	struct usb_device *usb = interface_to_usbdev(interface);
+	struct device *dev = &interface->dev;
+	struct usb_endpoint_descriptor *ep;
+	struct fwnode_handle *fn;
+	struct urb *urb = NULL;
+	struct mud_irq *mirq;
+	void *buf = NULL;
+	size_t buf_size;
+	int ret;
+
+	mdebug("%s: dev->id=%d\n", __func__, dev->id);
+
+	ret = usb_find_int_in_endpoint(interface->cur_altsetting, &ep);
+	if (ret == -ENXIO)
+		return NULL;
+	if (ret)
+		return ERR_PTR(ret);
+
+	mirq = kzalloc(sizeof(*mirq), GFP_KERNEL);
+	if (!mirq)
+		return ERR_PTR(-ENOMEM);
+
+	mirq->mask = bitmap_zalloc(num_irqs, GFP_KERNEL);
+	if (!mirq->mask) {
+		ret = -ENOMEM;
+		goto release;
+	}
+
+	spin_lock_init(&mirq->lock);
+	mirq->num_irqs = num_irqs;
+
+	urb = usb_alloc_urb(0, GFP_KERNEL);
+	if (!urb) {
+		ret = -ENOMEM;
+		goto release;
+	}
+
+	buf_size = usb_endpoint_maxp(ep);
+	if (buf_size != (sizeof(u16) + DIV_ROUND_UP(num_irqs, BITS_PER_BYTE))) {
+		dev_err(dev, "Interrupt endpoint wMaxPacketSize too small: %zu\n", buf_size);
+		ret = -EINVAL;
+		goto release;
+	}
+
+	buf = usb_alloc_coherent(usb, buf_size, GFP_KERNEL, &urb->transfer_dma);
+	if (!buf) {
+		usb_free_urb(urb);
+		ret = -ENOMEM;
+		goto release;
+	}
+
+	usb_fill_int_urb(urb, usb,
+			 usb_rcvintpipe(usb, usb_endpoint_num(ep)),
+			 buf, buf_size, mud_irq_urb_completion,
+			 mirq, ep->bInterval);
+	urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
+
+	mirq->urb = urb;
+
+	if (dev->of_node) {
+		fn = of_node_to_fwnode(dev->of_node);
+	} else {
+		fn = irq_domain_alloc_named_fwnode("mud-irq");
+		if (!fn) {
+			ret = -ENOMEM;
+			goto release;
+		}
+	}
+
+	mirq->domain = irq_domain_create_linear(fn, num_irqs, &mud_irq_ops, mirq);
+	if (!dev->of_node)
+		irq_domain_free_fwnode(fn);
+	if (!mirq->domain) {
+		ret = -ENOMEM;
+		goto release;
+	}
+
+	INIT_LIST_HEAD(&mirq->eventlist);
+	INIT_WORK(&mirq->work, mud_irq_work);
+
+	mirq->workq = alloc_workqueue("mud-irq/%s", WQ_HIGHPRI, 0, dev_name(dev));
+	if (!mirq->workq) {
+		ret = -ENOMEM;
+		goto release;
+	}
+
+	return mirq;
+
+release:
+	mud_irq_release(mirq);
+
+	return ERR_PTR(ret);
+}
+
+static int mud_probe_regmap(struct usb_interface *interface, struct mfd_cell *cell,
+			    unsigned int index, struct mud_irq *mirq)
+{
+	struct mud_cell_pdata *pdata;
+	struct resource *res = NULL;
+	int ret;
+
+	pdata = kzalloc(sizeof(*pdata), GFP_KERNEL);
+	if (!pdata)
+		return -ENOMEM;
+
+	ret = regmap_usb_get_map_descriptor(interface, index, &pdata->desc);
+	if (ret)
+		goto error;
+
+	mdebug("%s: name='%s' index=%u\n", __func__, pdata->desc.name, index);
+	mdebug("    bRegisterValueBits=%u\n", pdata->desc.bRegisterValueBits);
+	mdebug("    bCompression=0x%02x\n", pdata->desc.bCompression);
+	mdebug("    bMaxTransferSizeOrder=%u (%ukB)\n",
+	       pdata->desc.bMaxTransferSizeOrder,
+	       (1 << pdata->desc.bMaxTransferSizeOrder) / 1024);
+
+	if (mirq) {
+		res = kzalloc(sizeof(*res), GFP_KERNEL);
+		if (!res) {
+			ret = -ENOMEM;
+			goto error;
+		}
+
+		res->flags = IORESOURCE_IRQ;
+		res->start = irq_create_mapping(mirq->domain, index);
+		mdebug("    res->start=%u\n", (unsigned int)res->start);
+		res->end = res->start;
+
+		cell->resources = res;
+		cell->num_resources = 1;
+	}
+
+	pdata->interface = interface;
+	pdata->index = index;
+	cell->name = pdata->desc.name;
+	cell->platform_data = pdata;
+	cell->pdata_size = sizeof(*pdata);
+	/*
+	 * A Multifunction USB Device can have multiple functions of the same
+	 * type. mfd_add_device() in its current form will only match on the
+	 * first node in the Device Tree.
+	 */
+	cell->of_compatible = cell->name;
+
+	return 0;
+
+error:
+	kfree(res);
+	kfree(pdata);
+
+	return ret;
+}
+
+static void mud_free(struct mud_device *mud)
+{
+	unsigned int i;
+
+	mud_irq_release(mud->mirq);
+
+	for (i = 0; i < mud->num_cells; i++) {
+		kfree(mud->cells[i].platform_data);
+		kfree(mud->cells[i].resources);
+	}
+
+	kfree(mud->cells);
+	kfree(mud);
+}
+
+static int mud_probe(struct usb_interface *interface,
+		     const struct usb_device_id *id)
+{
+	struct device *dev = &interface->dev;
+	unsigned int i, num_regmaps;
+	struct mud_device *mud;
+	int ret;
+
+	mdebug("%s: interface->dev.of_node=%px usb->dev.of_node=%px",
+	       __func__, interface->dev.of_node,
+	       usb_get_dev(interface_to_usbdev(interface))->dev.of_node);
+
+	ret = regmap_usb_get_interface_descriptor(interface, &num_regmaps);
+	if (ret)
+		return ret;
+	if (!num_regmaps)
+		return -EINVAL;
+
+	mdebug("%s: num_regmaps=%u\n", __func__, num_regmaps);
+
+	mud = kzalloc(sizeof(*mud), GFP_KERNEL);
+	if (!mud)
+		return -ENOMEM;
+
+	mud->mirq = mud_irq_create(interface, num_regmaps);
+	if (IS_ERR(mud->mirq)) {
+		ret = PTR_ERR(mud->mirq);
+		goto err_free;
+	}
+
+	mud->num_cells = num_regmaps;
+	mud->cells = kcalloc(num_regmaps, sizeof(*mud->cells), GFP_KERNEL);
+	if (!mud->cells)
+		goto err_free;
+
+	for (i = 0; i < num_regmaps; i++) {
+		ret = mud_probe_regmap(interface, &mud->cells[i], i, mud->mirq);
+		if (ret) {
+			dev_err(dev, "Failed to probe regmap index %i (error %d)\n", i, ret);
+			goto err_free;
+		}
+	}
+
+	ret = mud_irq_start(mud->mirq);
+	if (ret) {
+		dev_err(dev, "Failed to start irq (error %d)\n", ret);
+		goto err_free;
+	}
+
+	ret = mfd_add_hotplug_devices(dev, mud->cells, mud->num_cells);
+	if (ret) {
+		dev_err(dev, "Failed to add mfd devices to core.");
+		goto err_stop;
+	}
+
+	mud->usb = usb_get_dev(interface_to_usbdev(interface));
+
+	usb_set_intfdata(interface, mud);
+
+	if (mud->usb->product)
+		dev_info(dev, "%s\n", mud->usb->product);
+
+	return 0;
+
+err_stop:
+	mud_irq_stop(mud->mirq);
+err_free:
+	mud_free(mud);
+
+	return ret;
+}
+
+static void mud_disconnect(struct usb_interface *interface)
+{
+	struct mud_device *mud = usb_get_intfdata(interface);
+
+	mfd_remove_devices(&interface->dev);
+	mud_irq_stop(mud->mirq);
+	usb_put_dev(mud->usb);
+	mud_free(mud);
+
+	dev_dbg(&interface->dev, "disconnected\n");
+}
+
+static const struct usb_device_id mud_table[] = {
+	/*
+	 * FIXME:
+	 * Apply for a proper pid: https://github.com/openmoko/openmoko-usb-oui
+	 *
+	 * Or maybe the Linux Foundation will provide one from their vendor id.
+	 */
+	{ USB_DEVICE_INTERFACE_CLASS(0x1d50, 0x6150, USB_CLASS_VENDOR_SPEC) },
+	{ }
+};
+
+MODULE_DEVICE_TABLE(usb, mud_table);
+
+static struct usb_driver mud_driver = {
+	.name		= "mud",
+	.probe		= mud_probe,
+	.disconnect	= mud_disconnect,
+	.id_table	= mud_table,
+};
+
+module_usb_driver(mud_driver);
+
+MODULE_DESCRIPTION("Generic USB Device mfd core driver");
+MODULE_AUTHOR("Noralf Trønnes <noralf@tronnes.org>");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/mfd/mud.h b/include/linux/mfd/mud.h
new file mode 100644
index 000000000000..b2059fa57429
--- /dev/null
+++ b/include/linux/mfd/mud.h
@@ -0,0 +1,16 @@ 
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+#ifndef __LINUX_MUD_H
+#define __LINUX_MUD_H
+
+#include <linux/regmap_usb.h>
+
+struct usb_interface;
+
+struct mud_cell_pdata {
+	struct usb_interface *interface;
+	unsigned int index;
+	struct regmap_usb_map_descriptor desc;
+};
+
+#endif