usb/gpio/i2c: Add Intel USBIO USB IO-expander drivers

include/linux/usb/usbio.h new file +6 -0

+};
+
+#define USBIO_GPIO_PINMOD_MASK 0x3
+#define USBIO_GPIO_SET_PINMOD(pin) (pin & USBIO_GPIO_PINMOD_MASK)
+
+/*************************

include/linux/usb/usbio.h new file +14 -0

+};
+
+/************************
+ * USBIO I2C Controller *
+ ************************/
+
+#define USBIO_MAX_I2CBUSES 5
+
+#define USBIO_I2C_BUS_ADDR_CAP_10B	BIT(3) /* 10bit address support */
+#define USBIO_I2C_BUS_MODE_CAP_MASK	0x3
+#define USBIO_I2C_BUS_MODE_CAP_SM	0 /* Standard Mode */
+#define USBIO_I2C_BUS_MODE_CAP_FM	1 /* Fast Mode */
+#define USBIO_I2C_BUS_MODE_CAP_FMP	2 /* Fast Mode+ */
+#define USBIO_I2C_BUS_MODE_CAP_HSM	3 /* High-Speed Mode */

drivers/usb/misc/usbio.c line 45

Quoted 2 lines · Hans de Goede

drivers/usb/misc/usbio.c new file +2 -0

+struct usbio_protver {
+	uint8_t ver;

Nit, but you do this everywhere. Kernel types are "u8", not "uint8_t", that's a userspace C type. Please use the correct ones when writing kernel code.

drivers/usb/misc/usbio.c line 52

Quoted 7 lines

drivers/usb/misc/usbio.c new file +7 -0

+} __packed;
+
+struct usbio_fwver {
+	uint8_t major;
+	uint8_t minor;
+	uint16_t patch;
+	uint16_t build;

What is the endian of these u16 variables?

include/linux/usb/usbio.h line 36

Quoted 5 lines

include/linux/usb/usbio.h new file +5 -0

+/* USBIO Packet Header */
+struct usbio_packet_header {
+	uint8_t type;
+	uint8_t cmd;
+	uint8_t flags;

Are these crossing the user/kernel boundry? I think so (same with above), and so shouldn't they use the proper types (__u8)?

From: Israel Cepeda <[email protected]>

Add a driver for the Intel USBIO USB IO-expander used by the MIPI cameras on various new (Meteor Lake and later) Intel laptops.

This is an USB bridge driver which adds auxbus child devices for the GPIO, I2C and SPI functions of the USBIO chip and which exports IO-functions for the drivers for the auxbus child devices to communicate with the USBIO device's firmware.

Co-developed-by Hans de Goede <[email protected]>

Signed-off-by Hans de Goede <[email protected]>

Signed-off-by Israel Cepeda <[email protected]>

---
 MAINTAINERS               |   7 +
 drivers/usb/misc/Kconfig  |  14 +
 drivers/usb/misc/Makefile |   1 +
 drivers/usb/misc/usbio.c  | 693 ++++++++++++++++++++++++++++++++++++++
 include/linux/usb/usbio.h | 168 +++++++++
 5 files changed, 883 insertions(+)
 create mode 100644 drivers/usb/misc/usbio.c
 create mode 100644 include/linux/usb/usbio.h

MAINTAINERS +0 -0

index 0618351510ad..1208efe41f9f 100644

MAINTAINERS +4 -0

+S:	Maintained
+F:	drivers/usb/misc/usbio.c
+F:	include/linux/usb/usbio.h
+
 INTEL VENDOR SPECIFIC EXTENDED CAPABILITIES DRIVER
 M:	David E. Box <[email protected]>
 S:	Supported

drivers/usb/misc/Kconfig +14 -0

index 6497c4e81e95..bfe08e15d051 100644
@@ -178,6 +178,20 @@ config USB_LJCA
 	  This driver can also be built as a module. If so, the module
 	  will be called usb-ljca.
 
+config USB_USBIO
+	tristate "Intel USBIO Bridge support"
+	depends on USB && ACPI
+	select AUXILIARY_BUS
+	help
+	  This adds support for Intel USBIO drivers.
+	  This enables the USBIO bridge driver module in charge to talk
+	  to the USB device. Additional drivers such as GPIO_USBIO and
+	  I2C_USBIO must be enabled in order to use the device's full
+	  functionality.
+
+	  This driver can also be built as a module. If so, the module
+	  will be called usbio.
+
 source "drivers/usb/misc/sisusbvga/Kconfig"
 
 config USB_LD

drivers/usb/misc/Makefile +1 -0

index 0cd5bc8f52fe..494ab0377f35 100644
@@ -12,6 +12,7 @@ obj-$(CONFIG_USB_EMI62)			+= emi62.o
 obj-$(CONFIG_USB_EZUSB_FX2)		+= ezusb.o
 obj-$(CONFIG_APPLE_MFI_FASTCHARGE)	+= apple-mfi-fastcharge.o
 obj-$(CONFIG_USB_LJCA)			+= usb-ljca.o
+obj-$(CONFIG_USB_USBIO)			+= usbio.o
 obj-$(CONFIG_USB_IDMOUSE)		+= idmouse.o
 obj-$(CONFIG_USB_IOWARRIOR)		+= iowarrior.o
 obj-$(CONFIG_USB_ISIGHTFW)		+= isight_firmware.o

drivers/usb/misc/usbio.c new file +138 -0

new file mode 100644
index 000000000000..88197092f39a
@@ -0,0 +1,693 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Intel USBIO Bridge driver
+ *
+ * Copyright (c) 2025 Intel Corporation.
+ * Copyright (c) 2025 Red Hat, Inc.
+ */
+
+#include <linux/acpi.h>
+#include <linux/auxiliary_bus.h>
+#include <linux/completion.h>
+#include <linux/dev_printk.h>
+#include <linux/device.h>
+#include <linux/lockdep.h>
+#include <linux/mutex.h>
+#include <linux/string.h>
+#include <linux/types.h>
+#include <linux/usb.h>
+#include <linux/usb/usbio.h>
+
+/*************************************
+ * USBIO Bridge Protocol Definitions *
+ *************************************/
+
+/* USBIO Control Commands */
+#define USBIO_CTRLCMD_PROTVER	0
+#define USBIO_CTRLCMD_FWVER	1
+#define USBIO_CTRLCMD_HS	2
+#define USBIO_CTRLCMD_ENUMGPIO	16
+#define USBIO_CTRLCMD_ENUMI2C	17
+#define USBIO_CTRLCMD_ENUMSPI	18
+
+/* USBIO Packet Flags */
+#define USBIO_PKTFLAG_ACK	BIT(0)
+#define USBIO_PKTFLAG_RSP	BIT(1)
+#define USBIO_PKTFLAG_CMP	BIT(2)
+#define USBIO_PKTFLAG_ERR	BIT(3)
+
+#define USBIO_PKTFLAGS_REQRESP	(USBIO_PKTFLAG_CMP | USBIO_PKTFLAG_ACK)
+
+#define USBIO_CTRLXFER_TIMEOUT 0
+#define USBIO_BULKXFER_TIMEOUT 100
+
+struct usbio_protver {
+	uint8_t ver;
+} __packed;
+
+struct usbio_fwver {
+	uint8_t major;
+	uint8_t minor;
+	uint16_t patch;
+	uint16_t build;
+} __packed;
+
+#define USBIO_SPI_BUS_WIRE_CAP_3W	BIT(3) /* 3 wires support */
+#define USBIO_SPI_BUS_ADDR_CAP_MASK	0x3
+#define USBIO_SPI_BUS_ADDR_CAP_CS0	0 /* CS0 */
+#define USBIO_SPI_BUS_ADDR_CAP_CS1	1 /* CS0/1 */
+#define USBIO_SPI_BUS_ADDR_CAP_CS2	2 /* CS0/1/2 */
+#define USBIO_SPI_BUS_ADDR_CAP_CS3	3 /* CS0/1/2/3 */
+
+struct usbio_spi_bus_desc {
+	uint8_t id;
+	uint8_t caps;
+} __packed;
+
+/***********************************
+ * USBIO Bridge Device Definitions *
+ ***********************************/
+struct usbio_dev_info {
+	struct usbio_protver protver;
+	struct usbio_fwver fwver;
+};
+
+#define MAX_SPIBUSES	5
+
+/**
+ * struct usbio_device - the usb device exposing IOs
+ *
+ * @dev: the device in the usb interface
+ * @udev: the detected usb device
+ * @intf: the usb interface
+ * @ctrl_pipe: the control transfer pipe
+ * @ctrlbuf_len: the size of the control transfer pipe
+ * @ctrlbuf: the buffer used for control transfers
+ * @tx_pipe: the bulk out pipe
+ * @txbuf_len: the size of the bulk out pipe
+ * @txbuf: the buffer used for bulk out transfers
+ * @urb: the urb to read bulk pipe
+ * @rx_pipe: the bulk in pipe
+ * @rxbuf_len: the size of the bulk in pipe
+ * @rxdat_len: the data length at rx buffer
+ * @rxbuf: the buffer used for bulk in transfers
+ * @info: the device's protocol and firmware information
+ * @done: completion object as request is done
+ * @mutex: protection against access concurrency
+ * @cli_list: device's client list
+ */
+struct usbio_device {
+	struct device *dev;
+	struct usb_device *udev;
+	struct usb_interface *intf;
+
+	unsigned int ctrl_pipe;
+	u16 ctrlbuf_len;
+	void *ctrlbuf;
+
+	unsigned int tx_pipe;
+	u16 txbuf_len;
+	void *txbuf;
+
+	struct urb *urb;
+	unsigned int rx_pipe;
+	u16 rxbuf_len;
+	u16 rxdat_len;
+	void *rxbuf;
+
+	struct usbio_dev_info info;
+
+	struct completion done;
+	struct mutex mutex;
+
+	struct list_head cli_list;
+
+	unsigned int nr_gpio_banks;
+	struct usbio_gpio_bank_desc gpios[USBIO_MAX_GPIOBANKS];
+
+	unsigned int nr_i2c_buses;
+	struct usbio_i2c_bus_desc i2cs[USBIO_MAX_I2CBUSES];
+
+	unsigned int nr_spi_buses;
+	struct usbio_spi_bus_desc spis[MAX_SPIBUSES];
+};
+
+/**
+ * struct usbio_client - represents a usbio client
+ *
+ * @adev: auxiliary device object

drivers/usb/misc/usbio.c new file +4 -0

+
+static int usbio_ctrl_msg(struct usbio_device *usbio, u8 type, u8 cmd,
+			  const void *obuf, u16 obuf_len, void *ibuf, u16 ibuf_len)
+{

drivers/usb/misc/usbio.c new file +11 -0

+		return -EMSGSIZE;
+
+	/* Prepare Control Packet Header */
+	cpkt = usbio->ctrlbuf;
+	cpkt->header.type = type;
+	cpkt->header.cmd = cmd;
+	if (type == USBIO_PKTTYPE_CTRL || ibuf_len)
+		cpkt->header.flags = USBIO_PKTFLAGS_REQRESP;
+	else
+		cpkt->header.flags = USBIO_PKTFLAG_CMP;
+	cpkt->len = obuf_len;

drivers/usb/misc/usbio.c new file +5 -0

+	}
+
+	if (!(cpkt->header.flags & USBIO_PKTFLAG_ACK))
+		return 0;
+

drivers/usb/misc/usbio.c new file +6 -0

+	}
+
+	if (cpkt->header.type != type || cpkt->header.cmd != cmd ||
+	    !(cpkt->header.flags & USBIO_PKTFLAG_RSP)) {
+		dev_err(usbio->dev, "Unexpected reply type: %u, cmd: %u, flags: %u\n",
+			cpkt->header.type, cpkt->header.cmd, cpkt->header.flags);

drivers/usb/misc/usbio.c new file +89 -0

+	return cpkt->len;
+}
+
+int usbio_control_msg(struct auxiliary_device *adev, u8 type, u8 cmd,
+		      const void *obuf, u16 obuf_len, void *ibuf, u16 ibuf_len)
+{
+	struct usbio_client *client = adev_to_client(adev);
+	int ret;
+
+	ret = usbio_acquire(adev);
+	if (ret)
+		return ret;
+
+	ret = usbio_ctrl_msg(client->bridge, type, cmd, obuf, obuf_len, ibuf, ibuf_len);
+
+	usbio_release(adev);
+
+	return ret;
+}
+EXPORT_SYMBOL_NS_GPL(usbio_control_msg, "USBIO");
+
+static void usbio_bulk_recv(struct urb *urb)
+{
+	struct usbio_bulk_packet *bpkt = urb->transfer_buffer;
+	struct usbio_device *usbio = urb->context;
+
+	if (!urb->status) {
+		if (bpkt->header.flags & USBIO_PKTFLAG_RSP) {
+			usbio->rxdat_len = urb->actual_length;
+			complete(&usbio->done);
+		}
+	} else if (urb->status != -ENOENT) {
+		dev_err(usbio->dev, "Bulk in error %d\n", urb->status);
+	}
+
+	usb_submit_urb(usbio->urb, GFP_ATOMIC);
+}
+
+int usbio_bulk_msg(struct auxiliary_device *adev, u8 type, u8 cmd, bool last,
+		   const void *obuf, u16 obuf_len, void *ibuf, u16 ibuf_len)
+{
+	struct usbio_device *usbio = adev_to_client(adev)->bridge;
+	struct usbio_bulk_packet *bpkt;
+	int ret, act;
+	u16 bpkt_len;
+
+	lockdep_assert_held(&usbio->mutex);
+
+	if ((obuf_len > (usbio->txbuf_len - sizeof(*bpkt))) ||
+	    (ibuf_len > (usbio->txbuf_len - sizeof(*bpkt))))
+		return -EMSGSIZE;
+
+	if (ibuf_len)
+		reinit_completion(&usbio->done);
+
+	/* If no data to send, skip to read */
+	if (!obuf_len)
+		goto read;
+
+	/* Prepare Bulk Packet Header */
+	bpkt = usbio->txbuf;
+	bpkt->header.type = type;
+	bpkt->header.cmd = cmd;
+	if (!last)
+		bpkt->header.flags = 0;
+	else if (ibuf_len)
+		bpkt->header.flags = USBIO_PKTFLAGS_REQRESP;
+	else
+		bpkt->header.flags = USBIO_PKTFLAG_CMP;
+	bpkt->len = obuf_len;
+
+	/* Copy the data */
+	memcpy(bpkt->data, obuf, obuf_len);
+
+	bpkt_len = sizeof(*bpkt) + obuf_len;
+	ret = usb_bulk_msg(usbio->udev, usbio->tx_pipe, bpkt, bpkt_len, &act,
+			   USBIO_BULKXFER_TIMEOUT);
+	dev_dbg(usbio->dev, "bulk out %d hdr %*phN data %*phN\n", act,
+		(int)sizeof(*bpkt), bpkt, (int)bpkt->len, bpkt->data);
+
+	if (ret || act != bpkt_len) {
+		dev_err(usbio->dev, "Bulk out failed: %d\n", ret);
+		return ret ?: -EPROTO;
+	}
+
+	if (!(bpkt->header.flags & USBIO_PKTFLAG_ACK))
+		return obuf_len;
+
+read:

drivers/usb/misc/usbio.c new file +19 -0

+
+	/*
+	 * Unsupported bulk commands get only an usbio_packet_header with
+	 * the error flag set as reply. Return -EPIPE for this case.
+	 */
+	if (act == sizeof(struct usbio_packet_header) &&
+	    (bpkt->header.flags & USBIO_PKTFLAG_ERR))
+		return -EPIPE;
+
+	if (act < sizeof(*bpkt)) {
+		dev_err(usbio->dev, "Bulk in short read: %d\n", act);
+		return -EPROTO;
+	}
+
+	if (bpkt->header.type != type || bpkt->header.cmd != cmd ||
+	    !(bpkt->header.flags & USBIO_PKTFLAG_RSP)) {
+		dev_err(usbio->dev,
+			"Unexpected bulk in type 0x%02x cmd 0x%02x flags 0x%02x\n",
+			bpkt->header.type, bpkt->header.cmd, bpkt->header.flags);

drivers/usb/misc/usbio.c new file +64 -0

+	return bpkt->len;
+}
+EXPORT_SYMBOL_NS_GPL(usbio_bulk_msg, "USBIO");
+
+static int usbio_get_and_lock(struct usbio_device *usbio)
+{
+	int ret;
+
+	ret = usb_autopm_get_interface(usbio->intf);
+	if (ret)
+		return ret;
+
+	mutex_lock(&usbio->mutex);
+	return 0;
+}
+
+static void usbio_unlock_and_put(struct usbio_device *usbio)
+{
+	mutex_unlock(&usbio->mutex);
+	usb_autopm_put_interface(usbio->intf);
+}
+
+int usbio_acquire(struct auxiliary_device *adev)
+{
+	return usbio_get_and_lock(adev_to_client(adev)->bridge);
+}
+EXPORT_SYMBOL_NS_GPL(usbio_acquire, "USBIO");
+
+void usbio_release(struct auxiliary_device *adev)
+{
+	usbio_unlock_and_put(adev_to_client(adev)->bridge);
+}
+EXPORT_SYMBOL_NS_GPL(usbio_release, "USBIO");
+
+void usbio_get_txrxbuf_len(struct auxiliary_device *adev, u16 *txbuf_len, u16 *rxbuf_len)
+{
+	struct usbio_device *usbio = adev_to_client(adev)->bridge;
+
+	*txbuf_len = usbio->txbuf_len;
+	*rxbuf_len = usbio->rxbuf_len;
+}
+EXPORT_SYMBOL_NS_GPL(usbio_get_txrxbuf_len, "USBIO");
+
+static void usbio_auxdev_release(struct device *dev)
+{
+	struct auxiliary_device *adev = to_auxiliary_dev(dev);
+	struct usbio_client *client = adev_to_client(adev);
+
+	kfree(client);
+}
+
+static int usbio_add_client(struct usbio_device *usbio, char *name, u8 id, void *data)
+{
+	struct usbio_client *client;
+	struct auxiliary_device *adev;
+	int ret;
+
+	client = kzalloc(sizeof(*client), GFP_KERNEL);
+	if (!client)
+		return -ENOMEM;
+
+	client->bridge = usbio;
+	adev = &client->adev;
+	adev->name = name;

drivers/usb/misc/usbio.c new file +5 -0

+
+	ret = usbio_ctrl_msg(usbio, USBIO_PKTTYPE_CTRL, USBIO_CTRLCMD_ENUMGPIO, NULL, 0,
+			     gpio, sizeof(usbio->gpios));
+	if (ret < 0 || ret % sizeof(*gpio)) {
+		dev_err(usbio->dev, "CTRL ENUMGPIO failed: %d\n", ret);

drivers/usb/misc/usbio.c new file +12 -0

+
+	ret = usbio_ctrl_msg(usbio, USBIO_PKTTYPE_CTRL, USBIO_CTRLCMD_ENUMI2C, NULL, 0,
+			     i2c, sizeof(usbio->i2cs));
+	if (ret < 0 || ret % sizeof(*i2c)) {
+		dev_err(usbio->dev, "CTRL ENUMI2C failed: %d\n", ret);
+		return ret;
+	}
+
+	usbio->nr_i2c_buses = ret / sizeof(*i2c);
+	dev_dbg(usbio->dev, "I2C Busses: %d\n", usbio->nr_i2c_buses);
+	for (i = 0; i < usbio->nr_i2c_buses; i++) {
+		dev_dbg(usbio->dev, "\tBus%d caps: %#02x\n", i2c[i].id, i2c[i].caps);

drivers/usb/misc/usbio.c new file +35 -0

+
+	ret = usbio_ctrl_msg(usbio, USBIO_PKTTYPE_CTRL, USBIO_CTRLCMD_ENUMSPI, NULL, 0,
+			     spi, sizeof(usbio->spis));
+	if (ret < 0 || ret % sizeof(*spi)) {
+		dev_err(usbio->dev, "CTRL ENUMSPI failed: %d\n", ret);
+		return ret;
+	}
+
+	usbio->nr_spi_buses = ret / sizeof(*spi);
+	dev_dbg(usbio->dev, "SPI Busses: %d\n", usbio->nr_spi_buses);
+	for (i = 0; i < usbio->nr_spi_buses; i++)
+		dev_dbg(usbio->dev, "\tBus%d caps: %#02x\n", spi[i].id, spi[i].caps);
+
+	return 0;
+}
+
+static int usbio_suspend(struct usb_interface *intf, pm_message_t msg)
+{
+	struct usbio_device *usbio = usb_get_intfdata(intf);
+
+	usb_kill_urb(usbio->urb);
+
+	return 0;
+}
+
+static int usbio_resume(struct usb_interface *intf)
+{
+	struct usbio_device *usbio = usb_get_intfdata(intf);
+
+	return usb_submit_urb(usbio->urb, GFP_KERNEL);
+}
+
+static void usbio_disconnect(struct usb_interface *intf)
+{
+	struct usbio_device *usbio = usb_get_intfdata(intf);

drivers/usb/misc/usbio.c new file +67 -0

+	if (!usbio)
+		return -ENOMEM;
+
+	ret = devm_mutex_init(dev, &usbio->mutex);
+	if (ret)
+		return ret;
+
+	usbio->dev = dev;
+	usbio->udev = udev;
+	usbio->intf = intf;
+	init_completion(&usbio->done);
+	INIT_LIST_HEAD(&usbio->cli_list);
+	usb_set_intfdata(intf, usbio);
+
+	usbio->ctrl_pipe = usb_endpoint_num(&udev->ep0.desc);
+	usbio->ctrlbuf_len = usb_maxpacket(udev, usbio->ctrl_pipe);
+	usbio->ctrlbuf = devm_kzalloc(dev, usbio->ctrlbuf_len, GFP_KERNEL);
+	if (!usbio->ctrlbuf)
+		return -ENOMEM;
+
+	/* Find the first bulk-in and bulk-out endpoints */
+	ret = usb_find_common_endpoints(intf->cur_altsetting, &ep_in, &ep_out,
+					NULL, NULL);
+	if (ret) {
+		dev_err(dev, "Cannot find bulk endpoints: %d\n", ret);
+		return ret;
+	}
+
+	usbio->tx_pipe = usb_sndbulkpipe(udev, usb_endpoint_num(ep_out));
+	usbio->txbuf_len = usb_endpoint_maxp(ep_out);
+	usbio->txbuf = devm_kzalloc(dev, usbio->txbuf_len, GFP_KERNEL);
+	if (!usbio->txbuf)
+		return -ENOMEM;
+
+	usbio->rx_pipe = usb_rcvbulkpipe(udev, usb_endpoint_num(ep_in));
+	usbio->rxbuf_len = usb_endpoint_maxp(ep_in);
+	usbio->rxbuf = devm_kzalloc(dev, usbio->rxbuf_len, GFP_KERNEL);
+	if (!usbio->rxbuf)
+		return -ENOMEM;
+
+	usbio->urb = usb_alloc_urb(0, GFP_KERNEL);
+	if (!usbio->urb)
+		return -ENOMEM;
+
+	usb_fill_bulk_urb(usbio->urb, udev, usbio->rx_pipe, usbio->rxbuf,
+			  usbio->rxbuf_len, usbio_bulk_recv, usbio);
+	ret = usb_submit_urb(usbio->urb, GFP_KERNEL);
+	if (ret)
+		return dev_err_probe(dev, ret, "Submitting usb urb\n");
+
+	ret = usbio_get_and_lock(usbio);
+	if (ret)
+		return ret;
+
+	ret = usbio_ctrl_msg(usbio, USBIO_PKTTYPE_CTRL, USBIO_CTRLCMD_HS, NULL, 0, NULL, 0);
+	if (ret < 0)
+		goto error;
+
+	ret = usbio_ctrl_msg(usbio, USBIO_PKTTYPE_CTRL, USBIO_CTRLCMD_PROTVER, NULL, 0,
+			     &usbio->info.protver, sizeof(struct usbio_protver));
+	if (ret < 0)
+		goto error;
+
+	ret = usbio_ctrl_msg(usbio, USBIO_PKTTYPE_CTRL, USBIO_CTRLCMD_FWVER, NULL, 0,
+			     &usbio->info.fwver, sizeof(struct usbio_fwver));
+	if (ret < 0)
+		goto error;

drivers/usb/misc/usbio.c new file +6 -0

+};
+module_usb_driver(usbio_driver);
+
+struct usbio_match_ids_walk_data {
+	struct acpi_device *adev;
+	const struct acpi_device_id *hids;

drivers/usb/misc/usbio.c new file +36 -0

+				if (!kstrtouint(&uid[i], 10, &id))
+					break;
+
+		if (!uid || (uid && wd->id == (u8)id)) {
+			wd->adev = adev;
+			return 1;
+		}
+	}
+
+	return 0;
+}
+
+void usbio_acpi_bind(struct auxiliary_device *adev, const struct acpi_device_id *hids)
+{
+	struct device *dev = &adev->dev;
+	struct acpi_device *parent;
+	struct usbio_match_ids_walk_data wd = {
+		.adev = NULL,
+		.hids = hids,
+		.id = adev->id,
+	};
+
+	parent = ACPI_COMPANION(dev->parent);
+	if (!parent)
+		return;
+
+	acpi_dev_for_each_child(parent, usbio_match_device_ids, &wd);
+	if (wd.adev)
+		ACPI_COMPANION_SET(dev, wd.adev);
+}
+EXPORT_SYMBOL_NS_GPL(usbio_acpi_bind, "USBIO");
+
+MODULE_DESCRIPTION("Intel USBIO Bridge driver");
+MODULE_AUTHOR("Israel Cepeda <[email protected]>");
+MODULE_AUTHOR("Hans de Goede <[email protected]>");
+MODULE_LICENSE("GPL");

include/linux/usb/usbio.h new file +168 -0

new file mode 100644
index 000000000000..9c34b157664f
@@ -0,0 +1,168 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2025 Intel Corporation.
+ *
+ */
+
+#ifndef _LINUX_USBIO_H_
+#define _LINUX_USBIO_H_
+
+#include <linux/auxiliary_bus.h>
+#include <linux/list.h>
+#include <linux/types.h>
+
+/***********************
+ * USBIO Clients Names *
+ ***********************/
+#define USBIO_GPIO_CLIENT	"usbio-gpio"
+#define USBIO_I2C_CLIENT	"usbio-i2c"
+#define USBIO_SPI_CLIENT	"usbio-spi"
+
+/**************************
+ * USBIO Type Definitions *
+ **************************/
+
+/* USBIO Packet Type */
+#define USBIO_PKTTYPE_CTRL	1
+#define USBIO_PKTTYPE_DBG	2
+#define USBIO_PKTTYPE_GPIO	3
+#define USBIO_PKTTYPE_I2C	4
+#define USBIO_PKTTYPE_SPI	5
+
+/* USBIO Packet Header */
+struct usbio_packet_header {
+	uint8_t type;
+	uint8_t cmd;
+	uint8_t flags;
+} __packed;
+
+/* USBIO Control Transfer Packet */
+struct usbio_ctrl_packet {
+	struct usbio_packet_header header;
+	uint8_t len;
+	uint8_t data[] __counted_by(len);
+} __packed;
+
+/* USBIO Bulk Transfer Packet */
+struct usbio_bulk_packet {
+	struct usbio_packet_header header;
+	uint16_t len;
+	uint8_t data[] __counted_by(len);
+} __packed;
+
+/* USBIO GPIO commands */
+enum usbio_gpio_cmd {
+	USBIO_GPIOCMD_DEINIT,
+	USBIO_GPIOCMD_INIT,
+	USBIO_GPIOCMD_READ,
+	USBIO_GPIOCMD_WRITE,
+	USBIO_GPIOCMD_END
+};
+
+/* USBIO GPIO config */
+enum usbio_gpio_pincfg {
+	USBIO_GPIO_PINCFG_DEFAULT,
+	USBIO_GPIO_PINCFG_PULLUP,
+	USBIO_GPIO_PINCFG_PULLDOWN,
+	USBIO_GPIO_PINCFG_PUSHPULL
+};
+
+#define USBIO_GPIO_PINCFG_SHIFT 2
+#define USBIO_GPIO_PINCFG_MASK (0x3 << USBIO_GPIO_PINCFG_SHIFT)
+#define USBIO_GPIO_SET_PINCFG(pincfg) \
+	(((pincfg) << USBIO_GPIO_PINCFG_SHIFT) & USBIO_GPIO_PINCFG_MASK)
+
+enum usbio_gpio_pinmode {
+	USBIO_GPIO_PINMOD_INVAL,
+	USBIO_GPIO_PINMOD_INPUT,
+	USBIO_GPIO_PINMOD_OUTPUT,
+	USBIO_GPIO_PINMOD_MAXVAL
+};
+
+#define USBIO_GPIO_PINMOD_MASK 0x3
+#define USBIO_GPIO_SET_PINMOD(pin) (pin & USBIO_GPIO_PINMOD_MASK)
+
+/*************************
+ * USBIO GPIO Controller *
+ *************************/
+
+#define USBIO_MAX_GPIOBANKS	5
+#define USBIO_GPIOSPERBANK	32
+
+struct usbio_gpio_bank_desc {
+	uint8_t id;
+	uint8_t pins;
+	uint32_t bmap;
+} __packed;
+
+struct usbio_gpio_init {
+	u8 bankid;
+	u8 config;
+	u8 pincount;
+	u8 pin;
+} __packed;
+
+struct usbio_gpio_rw {
+	u8 bankid;
+	u8 pincount;
+	u8 pin;
+	u32 value;
+} __packed;
+
+/* USBIO I2C commands */
+enum usbio_i2c_cmd {
+	USBIO_I2CCMD_UNINIT,
+	USBIO_I2CCMD_INIT,
+	USBIO_I2CCMD_READ,
+	USBIO_I2CCMD_WRITE,
+	USBIO_I2CCMD_END
+};
+
+/************************
+ * USBIO I2C Controller *
+ ************************/
+
+#define USBIO_MAX_I2CBUSES 5
+
+#define USBIO_I2C_BUS_ADDR_CAP_10B	BIT(3) /* 10bit address support */
+#define USBIO_I2C_BUS_MODE_CAP_MASK	0x3
+#define USBIO_I2C_BUS_MODE_CAP_SM	0 /* Standard Mode */
+#define USBIO_I2C_BUS_MODE_CAP_FM	1 /* Fast Mode */
+#define USBIO_I2C_BUS_MODE_CAP_FMP	2 /* Fast Mode+ */
+#define USBIO_I2C_BUS_MODE_CAP_HSM	3 /* High-Speed Mode */
+
+struct usbio_i2c_bus_desc {
+	uint8_t id;
+	uint8_t caps;
+} __packed;
+
+struct usbio_i2c_uninit {
+	u8 busid;
+	u16 config;
+} __packed;
+
+struct usbio_i2c_init {
+	u8 busid;
+	u16 config;
+	u32 speed;
+} __packed;
+
+struct usbio_i2c_rw {
+	u8 busid;
+	u16 config;
+	u16 size;
+	u8 data[] __counted_by(size);
+} __packed;
+
+int usbio_control_msg(struct auxiliary_device *adev, u8 type, u8 cmd,
+		      const void *obuf, u16 obuf_len, void *ibuf, u16 ibuf_len);
+
+int usbio_bulk_msg(struct auxiliary_device *adev, u8 type, u8 cmd, bool last,
+		   const void *obuf, u16 obuf_len, void *ibuf, u16 ibuf_len);
+
+int usbio_acquire(struct auxiliary_device *adev);
+void usbio_release(struct auxiliary_device *adev);
+void usbio_get_txrxbuf_len(struct auxiliary_device *adev, u16 *txbuf_len, u16 *rxbuf_len);
+void usbio_acpi_bind(struct auxiliary_device *adev, const struct acpi_device_id *hids);
+
+#endif

Quoted 9 lines

drivers/usb/misc/usbio.c new file +9 -0

+/**
+ * struct usbio_client - represents a usbio client
+ *
+ * @adev: auxiliary device object
+ * @bridge: usbio bridge who service the client
+ * @link: usbio bridge clients list member
+ */
+struct usbio_client {
+	struct auxiliary_device adev;

drivers/usb/misc/usbio.c line 148

Quoted 5 lines

drivers/usb/misc/usbio.c new file +5 -0

+	struct usbio_device *bridge;
+	struct list_head link;
+};
+
+#define adev_to_client(adev) container_of(adev, struct usbio_client, adev)

Please use a different name than "adev" for the argument, which is also the struct field of interest.

Quoted 7 lines

drivers/usb/misc/usbio.c new file +7 -0

+static int usbio_ctrl_msg(struct usbio_device *usbio, u8 type, u8 cmd,
+			  const void *obuf, u16 obuf_len, void *ibuf, u16 ibuf_len)
+{
+	u8 request = USB_TYPE_VENDOR | USB_RECIP_DEVICE;
+	struct usbio_ctrl_packet *cpkt;
+	unsigned int pipe;
+	u16 cpkt_len;

drivers/usb/misc/usbio.c line 162

Quoted 6 lines

drivers/usb/misc/usbio.c new file +6 -0

+	int ret;
+
+	lockdep_assert_held(&usbio->mutex);
+
+	if ((obuf_len > (usbio->ctrlbuf_len - sizeof(*cpkt))) ||
+	    (ibuf_len > (usbio->ctrlbuf_len - sizeof(*cpkt))))

You can (and should) remove all parentheses except the outer ones here.

Quoted 11 lines

drivers/usb/misc/usbio.c new file +11 -0

+		return -EMSGSIZE;
+
+	/* Prepare Control Packet Header */
+	cpkt = usbio->ctrlbuf;
+	cpkt->header.type = type;
+	cpkt->header.cmd = cmd;
+	if (type == USBIO_PKTTYPE_CTRL || ibuf_len)
+		cpkt->header.flags = USBIO_PKTFLAGS_REQRESP;
+	else
+		cpkt->header.flags = USBIO_PKTFLAG_CMP;
+	cpkt->len = obuf_len;

drivers/usb/misc/usbio.c line 183

Quoted 10 lines

drivers/usb/misc/usbio.c new file +10 -0

+
+	/* Copy the data */
+	memcpy(cpkt->data, obuf, obuf_len);
+
+	pipe = usb_sndctrlpipe(usbio->udev, usbio->ctrl_pipe);
+	cpkt_len = sizeof(*cpkt) + obuf_len;
+	ret = usb_control_msg(usbio->udev, pipe, 0, request | USB_DIR_OUT, 0, 0,
+			      cpkt, cpkt_len, USBIO_CTRLXFER_TIMEOUT);
+	dev_dbg(usbio->dev, "control out %d hdr %*phN data %*phN\n", ret,
+		(int)sizeof(*cpkt), cpkt, (int)cpkt->len, cpkt->data);

Instead of casting, how about using %zu for printing a size_t?

drivers/usb/misc/usbio.c line 187

Quoted 4 lines

drivers/usb/misc/usbio.c new file +4 -0

+
+	if (ret != cpkt_len) {
+		dev_err(usbio->dev, "USB control out failed: %d\n", ret);
+		return (ret < 0) ? ret : -EPROTO;

Redundant parentheses.

drivers/usb/misc/usbio.c line 436

Quoted 4 lines

drivers/usb/misc/usbio.c new file +4 -0

+static int usbio_ctrl_enumgpios(struct usbio_device *usbio)
+{
+	struct usbio_gpio_bank_desc *gpio = usbio->gpios;
+	int ret, i;

unsigned int i, please.

drivers/usb/misc/usbio.c line 458

Quoted 4 lines

drivers/usb/misc/usbio.c new file +4 -0

+static int usbio_ctrl_enumi2cs(struct usbio_device *usbio)
+{
+	struct usbio_i2c_bus_desc *i2c = usbio->i2cs;
+	int ret, i;

unsigned int i, please.

drivers/usb/misc/usbio.c line 480

Quoted 4 lines

drivers/usb/misc/usbio.c new file +4 -0

+static int usbio_ctrl_enumspis(struct usbio_device *usbio)
+{
+	struct usbio_spi_bus_desc *spi = usbio->spis;
+	int ret, i;

Ditto.

drivers/usb/misc/usbio.c line 525

Quoted 13 lines

drivers/usb/misc/usbio.c new file +13 -0

+static void usbio_disconnect(struct usb_interface *intf)
+{
+	struct usbio_device *usbio = usb_get_intfdata(intf);
+	struct usbio_client *client, *prev;
+
+	list_for_each_entry_safe_reverse(client, prev, &usbio->cli_list, link) {
+		auxiliary_device_delete(&client->adev);
+		list_del_init(&client->link);
+		auxiliary_device_uninit(&client->adev);
+	}
+
+	usb_kill_urb(usbio->urb);
+	usb_free_urb(usbio->urb);

What will happen on client drivers if they're working with the bridge while disconnect happens?

One easy solution to this could be to use an rw_semaphore where client acquire it for readingin conjunction (in a helper that also checks the interface status) and disconnect callback for writing.

Quoted 7 lines

drivers/usb/misc/usbio.c new file +7 -0

+}
+
+static int usbio_probe(struct usb_interface *intf, const struct usb_device_id *id)
+{
+	struct usb_device *udev = interface_to_usbdev(intf);
+	struct usb_endpoint_descriptor *ep_in, *ep_out;
+	struct device *dev = &intf->dev;

drivers/usb/misc/usbio.c line 536

Quoted 4 lines

drivers/usb/misc/usbio.c new file +4 -0

+	struct usbio_device *usbio;
+	int ret;
+
+	usbio = devm_kzalloc(dev, sizeof(*usbio), GFP_KERNEL);

usbio will be released at the exit from usbio_disconnect(). I think you'll need to use a release callback in struct device to release usbio once all clients are gone.

drivers/usb/misc/usbio.c line 657

Quoted 3 lines

drivers/usb/misc/usbio.c new file +3 -0

+		uid = acpi_device_uid(adev);
+		if (uid)
+			for (int i = 0; i < strlen(uid); i++)

size_t i?

Quoted 9 lines · Sakari Ailus

Quoted 9 lines

drivers/usb/misc/usbio.c new file +9 -0

+/**
+ * struct usbio_client - represents a usbio client
+ *
+ * @adev: auxiliary device object
+ * @bridge: usbio bridge who service the client
+ * @link: usbio bridge clients list member
+ */
+struct usbio_client {
+	struct auxiliary_device adev;

Quoted 8 lines

drivers/usb/misc/usbio.c line 148

Quoted 5 lines

drivers/usb/misc/usbio.c new file +5 -0

+	struct usbio_device *bridge;
+	struct list_head link;
+};
+
+#define adev_to_client(adev) container_of(adev, struct usbio_client, adev)

Please use a different name than "adev" for the argument, which is also the struct field of interest.

Why? That's a very common way of doing this. My only complaint is that it really should be "container_of_const()" instead of just "container_of()"

Quoted 17 lines

Quoted 5 lines

drivers/usb/misc/usbio.c new file +3 -0

+static int usbio_ctrl_msg(struct usbio_device *usbio, u8 type, u8 cmd,
+			  const void *obuf, u16 obuf_len, void *ibuf, u16 ibuf_len)
+{

drivers/usb/misc/usbio.c line 162

Quoted 10 lines

drivers/usb/misc/usbio.c new file +10 -0

+	u8 request = USB_TYPE_VENDOR | USB_RECIP_DEVICE;
+	struct usbio_ctrl_packet *cpkt;
+	unsigned int pipe;
+	u16 cpkt_len;
+	int ret;
+
+	lockdep_assert_held(&usbio->mutex);
+
+	if ((obuf_len > (usbio->ctrlbuf_len - sizeof(*cpkt))) ||
+	    (ibuf_len > (usbio->ctrlbuf_len - sizeof(*cpkt))))

You can (and should) remove all parentheses except the outer ones here.

No, don't do that. If you do that you will have to manually go and try to remember the order of operations every time you read this code.

Remember, we write code for people first, compilers second. Make it totally obvious what is going on here as you want PEOPLE to catch your issues.

The statement is fine as-is.

Quoted 24 lines

Quoted 11 lines

drivers/usb/misc/usbio.c new file +11 -0

+		return -EMSGSIZE;
+
+	/* Prepare Control Packet Header */
+	cpkt = usbio->ctrlbuf;
+	cpkt->header.type = type;
+	cpkt->header.cmd = cmd;
+	if (type == USBIO_PKTTYPE_CTRL || ibuf_len)
+		cpkt->header.flags = USBIO_PKTFLAGS_REQRESP;
+	else
+		cpkt->header.flags = USBIO_PKTFLAG_CMP;
+	cpkt->len = obuf_len;

drivers/usb/misc/usbio.c line 183

Quoted 10 lines

drivers/usb/misc/usbio.c new file +10 -0

+
+	/* Copy the data */
+	memcpy(cpkt->data, obuf, obuf_len);
+
+	pipe = usb_sndctrlpipe(usbio->udev, usbio->ctrl_pipe);
+	cpkt_len = sizeof(*cpkt) + obuf_len;
+	ret = usb_control_msg(usbio->udev, pipe, 0, request | USB_DIR_OUT, 0, 0,
+			      cpkt, cpkt_len, USBIO_CTRLXFER_TIMEOUT);
+	dev_dbg(usbio->dev, "control out %d hdr %*phN data %*phN\n", ret,
+		(int)sizeof(*cpkt), cpkt, (int)cpkt->len, cpkt->data);

Instead of casting, how about using %zu for printing a size_t?

Quoted 6 lines

drivers/usb/misc/usbio.c line 187

Quoted 4 lines

drivers/usb/misc/usbio.c new file +4 -0

+
+	if (ret != cpkt_len) {
+		dev_err(usbio->dev, "USB control out failed: %d\n", ret);
+		return (ret < 0) ? ret : -EPROTO;

Redundant parentheses.

Nope! Again, please mat it obvioue.

Actually, I hate ? : lines, this should be:

	if (ret < 0)
		return ret
	else
		return -EPROTO

by spelling it all out.

Quoted 6 lines

drivers/usb/misc/usbio.c line 436

Quoted 4 lines

drivers/usb/misc/usbio.c new file +4 -0

+static int usbio_ctrl_enumgpios(struct usbio_device *usbio)
+{
+	struct usbio_gpio_bank_desc *gpio = usbio->gpios;
+	int ret, i;

unsigned int i, please.

Nope, 'int' is just fine. Please see Dan's many rants about why this is acceptable for loops.

Quoted 6 lines

drivers/usb/misc/usbio.c line 458

Quoted 4 lines

drivers/usb/misc/usbio.c new file +4 -0

+static int usbio_ctrl_enumi2cs(struct usbio_device *usbio)
+{
+	struct usbio_i2c_bus_desc *i2c = usbio->i2cs;
+	int ret, i;

unsigned int i, please.

Nope, 'int' is fine.

Quoted 6 lines

drivers/usb/misc/usbio.c line 480

Quoted 4 lines

drivers/usb/misc/usbio.c new file +4 -0

+static int usbio_ctrl_enumspis(struct usbio_device *usbio)
+{
+	struct usbio_spi_bus_desc *spi = usbio->spis;
+	int ret, i;

Ditto.

Nope :)

Quoted 17 lines

drivers/usb/misc/usbio.c line 525

Quoted 13 lines

drivers/usb/misc/usbio.c new file +13 -0

+static void usbio_disconnect(struct usb_interface *intf)
+{
+	struct usbio_device *usbio = usb_get_intfdata(intf);
+	struct usbio_client *client, *prev;
+
+	list_for_each_entry_safe_reverse(client, prev, &usbio->cli_list, link) {
+		auxiliary_device_delete(&client->adev);
+		list_del_init(&client->link);
+		auxiliary_device_uninit(&client->adev);
+	}
+
+	usb_kill_urb(usbio->urb);
+	usb_free_urb(usbio->urb);

What will happen on client drivers if they're working with the bridge while disconnect happens?

Quoted 3 lines

One easy solution to this could be to use an rw_semaphore where client acquire it for readingin conjunction (in a helper that also checks the interface status) and disconnect callback for writing.

How is that going to change anything? And how can a disconnect happen? Isn't this an onboard device?

Quoted 16 lines

drivers/usb/misc/usbio.c line 536

Quoted 12 lines

drivers/usb/misc/usbio.c new file +11 -0

+}
+
+static int usbio_probe(struct usb_interface *intf, const struct usb_device_id *id)
+{
+	struct usb_device *udev = interface_to_usbdev(intf);
+	struct usb_endpoint_descriptor *ep_in, *ep_out;
+	struct device *dev = &intf->dev;
+	struct usbio_device *usbio;
+	int ret;
+
+	usbio = devm_kzalloc(dev, sizeof(*usbio), GFP_KERNEL);

usbio will be released at the exit from usbio_disconnect(). I think you'll need to use a release callback in struct device to release usbio once all clients are gone.

Which clients? The disconnect will cause that to happen, it can not happen at the same time probe is called.

Quoted 6 lines

drivers/usb/misc/usbio.c line 657

Quoted 4 lines

drivers/usb/misc/usbio.c new file +3 -0

+		uid = acpi_device_uid(adev);
+		if (uid)
+			for (int i = 0; i < strlen(uid); i++)

size_t i?

Ick, no, "int" is fine.

Hi Greg,

Quoted 19 lines · Greg Kroah-Hartman

Quoted 17 lines · Sakari Ailus

Quoted 4 lines

drivers/usb/misc/usbio.c new file +4 -0

+/**
+ * struct usbio_client - represents a usbio client
+ *
+ * @adev: auxiliary device object

drivers/usb/misc/usbio.c line 148

Quoted 10 lines

drivers/usb/misc/usbio.c new file +10 -0

+ * @bridge: usbio bridge who service the client
+ * @link: usbio bridge clients list member
+ */
+struct usbio_client {
+	struct auxiliary_device adev;
+	struct usbio_device *bridge;
+	struct list_head link;
+};
+
+#define adev_to_client(adev) container_of(adev, struct usbio_client, adev)

Please use a different name than "adev" for the argument, which is also the struct field of interest.

Quoted 3 lines

Why? That's a very common way of doing this. My only complaint is that it really should be "container_of_const()" instead of just "container_of()"

Because the struct field has the same name. The macro isn't intended for obtaining the container struct based on any field in the struct, only the field called "adev".

I'll post a patch to add the container_of() check to checkpatch.pl.

Quoted 16 lines

Quoted 15 lines

drivers/usb/misc/usbio.c line 162

Quoted 13 lines

drivers/usb/misc/usbio.c new file +13 -0

+static int usbio_ctrl_msg(struct usbio_device *usbio, u8 type, u8 cmd,
+			  const void *obuf, u16 obuf_len, void *ibuf, u16 ibuf_len)
+{
+	u8 request = USB_TYPE_VENDOR | USB_RECIP_DEVICE;
+	struct usbio_ctrl_packet *cpkt;
+	unsigned int pipe;
+	u16 cpkt_len;
+	int ret;
+
+	lockdep_assert_held(&usbio->mutex);
+
+	if ((obuf_len > (usbio->ctrlbuf_len - sizeof(*cpkt))) ||
+	    (ibuf_len > (usbio->ctrlbuf_len - sizeof(*cpkt))))

You can (and should) remove all parentheses except the outer ones here.

Quoted 2 lines

No, don't do that. If you do that you will have to manually go and try to remember the order of operations every time you read this code.

I presume kernel developers in general do.

But if in doubt: <URL:https://users.ece.utexas.edu/~adnan/c-refcard.pdf>.

Quoted 17 lines

Quoted 17 lines

drivers/usb/misc/usbio.c line 525

Quoted 13 lines

drivers/usb/misc/usbio.c new file +13 -0

+static void usbio_disconnect(struct usb_interface *intf)
+{
+	struct usbio_device *usbio = usb_get_intfdata(intf);
+	struct usbio_client *client, *prev;
+
+	list_for_each_entry_safe_reverse(client, prev, &usbio->cli_list, link) {
+		auxiliary_device_delete(&client->adev);
+		list_del_init(&client->link);
+		auxiliary_device_uninit(&client->adev);
+	}
+
+	usb_kill_urb(usbio->urb);
+	usb_free_urb(usbio->urb);

What will happen on client drivers if they're working with the bridge while disconnect happens?

Quoted 6 lines

Quoted 3 lines

One easy solution to this could be to use an rw_semaphore where client acquire it for readingin conjunction (in a helper that also checks the interface status) and disconnect callback for writing.

How is that going to change anything? And how can a disconnect happen? Isn't this an onboard device?

It is, but the device firmware is known to crash occasionally.

The documantation says you can't access USB interfaces once disconnect has returned. I'm not sure if there are checks to safeguard against ongoing or additional accesses in the USB stack but on many other buses this may simply lead to a system crash.

Quoted 22 lines · Sakari Ailus

Hi Greg,

Quoted 19 lines · Greg Kroah-Hartman

Quoted 17 lines · Sakari Ailus

Quoted 4 lines

drivers/usb/misc/usbio.c new file +4 -0

+/**
+ * struct usbio_client - represents a usbio client
+ *
+ * @adev: auxiliary device object

drivers/usb/misc/usbio.c line 148

Quoted 10 lines

drivers/usb/misc/usbio.c new file +10 -0

+ * @bridge: usbio bridge who service the client
+ * @link: usbio bridge clients list member
+ */
+struct usbio_client {
+	struct auxiliary_device adev;
+	struct usbio_device *bridge;
+	struct list_head link;
+};
+
+#define adev_to_client(adev) container_of(adev, struct usbio_client, adev)

Please use a different name than "adev" for the argument, which is also the struct field of interest.

Quoted 7 lines

Quoted 3 lines

Why? That's a very common way of doing this. My only complaint is that it really should be "container_of_const()" instead of just "container_of()"

Because the struct field has the same name. The macro isn't intended for obtaining the container struct based on any field in the struct, only the field called "adev".

And that's fine, the macro works like this, so all should be ok.

Quoted 1 line

I'll post a patch to add the container_of() check to checkpatch.pl.

Patch to add it to do what?

Quoted 17 lines

Quoted 17 lines

Quoted 16 lines

Quoted 4 lines

drivers/usb/misc/usbio.c new file +3 -0

+static int usbio_ctrl_msg(struct usbio_device *usbio, u8 type, u8 cmd,
+			  const void *obuf, u16 obuf_len, void *ibuf, u16 ibuf_len)
+{

drivers/usb/misc/usbio.c line 162

Quoted 10 lines

drivers/usb/misc/usbio.c new file +10 -0

+	u8 request = USB_TYPE_VENDOR | USB_RECIP_DEVICE;
+	struct usbio_ctrl_packet *cpkt;
+	unsigned int pipe;
+	u16 cpkt_len;
+	int ret;
+
+	lockdep_assert_held(&usbio->mutex);
+
+	if ((obuf_len > (usbio->ctrlbuf_len - sizeof(*cpkt))) ||
+	    (ibuf_len > (usbio->ctrlbuf_len - sizeof(*cpkt))))

You can (and should) remove all parentheses except the outer ones here.

Quoted 6 lines

Quoted 2 lines

No, don't do that. If you do that you will have to manually go and try to remember the order of operations every time you read this code.

I presume kernel developers in general do.

But if in doubt: <URL:https://users.ece.utexas.edu/~adnan/c-refcard.pdf>.

Don't force me to look it up all the time, use () to make it obvious please. That's the biggest thing I hate about that checkpatch "rule", please do not follow it for any code that I am a maintainer for.

Quoted 21 lines

Quoted 21 lines

Quoted 20 lines

drivers/usb/misc/usbio.c line 525

Quoted 13 lines

drivers/usb/misc/usbio.c new file +13 -0

+static void usbio_disconnect(struct usb_interface *intf)
+{
+	struct usbio_device *usbio = usb_get_intfdata(intf);
+	struct usbio_client *client, *prev;
+
+	list_for_each_entry_safe_reverse(client, prev, &usbio->cli_list, link) {
+		auxiliary_device_delete(&client->adev);
+		list_del_init(&client->link);
+		auxiliary_device_uninit(&client->adev);
+	}
+
+	usb_kill_urb(usbio->urb);
+	usb_free_urb(usbio->urb);

What will happen on client drivers if they're working with the bridge while disconnect happens?

One easy solution to this could be to use an rw_semaphore where client acquire it for readingin conjunction (in a helper that also checks the interface status) and disconnect callback for writing.

Quoted 4 lines

Quoted 2 lines

How is that going to change anything? And how can a disconnect happen? Isn't this an onboard device?

It is, but the device firmware is known to crash occasionally.

Then fix the firmware :)

Quoted 5 lines

The documantation says you can't access USB interfaces once disconnect has returned. I'm not sure if there are checks to safeguard against ongoing or additional accesses in the USB stack but on many other buses this may simply lead to a system crash.

How can you access the USB interface after disconnect has returned on these codepaths? The child devices should all be cleaned up properly after disconnect happens so there should not be a pointer to even use anymore.

Quoted 9 lines

Quoted 4 lines

The documantation says you can't access USB interfaces once disconnect has returned. I'm not sure if there are checks to safeguard against ongoing or additional accesses in the USB stack but on many other buses this may simply lead to a system crash.

How can you access the USB interface after disconnect has returned on these codepaths? The child devices should all be cleaned up properly after disconnect happens so there should not be a pointer to even use anymore.

See functions exported by the main USBIO driver.

Hi Hans,

Thanks for posting this. Some comments below...

Quoted 31 lines · Hans de Goede

From: Israel Cepeda <[email protected]>

Add a driver for the Intel USBIO USB IO-expander used by the MIPI cameras on various new (Meteor Lake and later) Intel laptops.

This is an USB bridge driver which adds auxbus child devices for the GPIO, I2C and SPI functions of the USBIO chip and which exports IO-functions for the drivers for the auxbus child devices to communicate with the USBIO device's firmware.

Co-developed-by Hans de Goede <[email protected]>

Signed-off-by Hans de Goede <[email protected]>

Signed-off-by Israel Cepeda <[email protected]>

---
 MAINTAINERS               |   7 +
 drivers/usb/misc/Kconfig  |  14 +
 drivers/usb/misc/Makefile |   1 +
 drivers/usb/misc/usbio.c  | 693 ++++++++++++++++++++++++++++++++++++++
 include/linux/usb/usbio.h | 168 +++++++++
 5 files changed, 883 insertions(+)
 create mode 100644 drivers/usb/misc/usbio.c
 create mode 100644 include/linux/usb/usbio.h

MAINTAINERS +0 -0

index 0618351510ad..1208efe41f9f 100644
@@ -12518,6 +12518,13 @@ S:	Maintained
 F:	Documentation/admin-guide/pm/intel_uncore_frequency_scaling.rst
 F:	drivers/platform/x86/intel/uncore-frequency/
 

MAINTAINERS S: Maintained · line 12523

Quoted 3 lines

MAINTAINERS +3 -0

+INTEL USBIO USB I/O EXPANDER DRIVERS
+M:	Israel Cepeda <[email protected]>
+M:	Hans de Goede <[email protected]>

Could you add:

R: Sakari Ailus <[email protected]>

?

Quoted 7 lines

MAINTAINERS +4 -0

+S:	Maintained
+F:	drivers/usb/misc/usbio.c
+F:	include/linux/usb/usbio.h
+
 INTEL VENDOR SPECIFIC EXTENDED CAPABILITIES DRIVER
 M:	David E. Box <[email protected]>
 S:	Supported

Quoted 5 lines

drivers/usb/misc/usbio.c new file +0 -0

new file mode 100644
index 000000000000..88197092f39a

Quoted 9 lines

drivers/usb/misc/usbio.c new file +9 -0

+/**
+ * struct usbio_client - represents a usbio client
+ *
+ * @adev: auxiliary device object
+ * @bridge: usbio bridge who service the client
+ * @link: usbio bridge clients list member
+ */
+struct usbio_client {
+	struct auxiliary_device adev;

drivers/usb/misc/usbio.c line 148

Quoted 5 lines

drivers/usb/misc/usbio.c new file +5 -0

+	struct usbio_device *bridge;
+	struct list_head link;
+};
+
+#define adev_to_client(adev) container_of(adev, struct usbio_client, adev)

Please use a different name than "adev" for the argument, which is also the struct field of interest.

Quoted 8 lines

drivers/usb/misc/usbio.c new file +8 -0

+
+static int usbio_ctrl_msg(struct usbio_device *usbio, u8 type, u8 cmd,
+			  const void *obuf, u16 obuf_len, void *ibuf, u16 ibuf_len)
+{
+	u8 request = USB_TYPE_VENDOR | USB_RECIP_DEVICE;
+	struct usbio_ctrl_packet *cpkt;
+	unsigned int pipe;
+	u16 cpkt_len;

drivers/usb/misc/usbio.c line 162

Quoted 6 lines

drivers/usb/misc/usbio.c new file +6 -0

+	int ret;
+
+	lockdep_assert_held(&usbio->mutex);
+
+	if ((obuf_len > (usbio->ctrlbuf_len - sizeof(*cpkt))) ||
+	    (ibuf_len > (usbio->ctrlbuf_len - sizeof(*cpkt))))

You can (and should) remove all parentheses except the outer ones here.

Quoted 16 lines

drivers/usb/misc/usbio.c new file +15 -0

+		return -EMSGSIZE;
+
+	/* Prepare Control Packet Header */
+	cpkt = usbio->ctrlbuf;
+	cpkt->header.type = type;
+	cpkt->header.cmd = cmd;
+	if (type == USBIO_PKTTYPE_CTRL || ibuf_len)
+		cpkt->header.flags = USBIO_PKTFLAGS_REQRESP;
+	else
+		cpkt->header.flags = USBIO_PKTFLAG_CMP;
+	cpkt->len = obuf_len;
+
+	/* Copy the data */
+	memcpy(cpkt->data, obuf, obuf_len);
+

drivers/usb/misc/usbio.c line 183

Quoted 6 lines

drivers/usb/misc/usbio.c new file +6 -0

+	pipe = usb_sndctrlpipe(usbio->udev, usbio->ctrl_pipe);
+	cpkt_len = sizeof(*cpkt) + obuf_len;
+	ret = usb_control_msg(usbio->udev, pipe, 0, request | USB_DIR_OUT, 0, 0,
+			      cpkt, cpkt_len, USBIO_CTRLXFER_TIMEOUT);
+	dev_dbg(usbio->dev, "control out %d hdr %*phN data %*phN\n", ret,
+		(int)sizeof(*cpkt), cpkt, (int)cpkt->len, cpkt->data);

Instead of casting, how about using %zu for printing a size_t?

drivers/usb/misc/usbio.c line 187

Quoted 4 lines

drivers/usb/misc/usbio.c new file +4 -0

+
+	if (ret != cpkt_len) {
+		dev_err(usbio->dev, "USB control out failed: %d\n", ret);
+		return (ret < 0) ? ret : -EPROTO;

Redundant parentheses.

Quoted 6 lines

drivers/usb/misc/usbio.c new file +5 -0

+	}
+
+	if (!(cpkt->header.flags & USBIO_PKTFLAG_ACK))
+		return 0;
+

drivers/usb/misc/usbio.c line 202

Quoted 10 lines

drivers/usb/misc/usbio.c new file +10 -0

+	pipe = usb_rcvctrlpipe(usbio->udev, usbio->ctrl_pipe);
+	cpkt_len = sizeof(*cpkt) + ibuf_len;
+	ret = usb_control_msg(usbio->udev, pipe, 0, request | USB_DIR_IN, 0, 0,
+			      cpkt, cpkt_len, USBIO_CTRLXFER_TIMEOUT);
+	dev_dbg(usbio->dev, "control in %d hdr %*phN data %*phN\n", ret,
+		(int)sizeof(*cpkt), cpkt, (int)cpkt->len, cpkt->data);
+
+	if (ret < sizeof(*cpkt)) {
+		dev_err(usbio->dev, "USB control in failed: %d\n", ret);
+		return (ret < 0) ? ret : -EPROTO;

Remove parentheses? There's no need to try to comply with MISRA C, is there? :-) I'm not commenting anymore on further instances of this.

Quoted 10 lines

drivers/usb/misc/usbio.c new file +10 -0

+	}
+
+	if (cpkt->header.type != type || cpkt->header.cmd != cmd ||
+	    !(cpkt->header.flags & USBIO_PKTFLAG_RSP)) {
+		dev_err(usbio->dev, "Unexpected reply type: %u, cmd: %u, flags: %u\n",
+			cpkt->header.type, cpkt->header.cmd, cpkt->header.flags);
+		return -EPROTO;
+	}
+
+	if (cpkt->header.flags & USBIO_PKTFLAG_ERR)

drivers/usb/misc/usbio.c line 218

Quoted 6 lines

drivers/usb/misc/usbio.c new file +6 -0

+		return -EREMOTEIO;
+
+	if (ibuf_len < cpkt->len)
+		return -ENOSPC;
+
+	memcpy(ibuf, cpkt->data, cpkt->len);

It'd be nice to have one more newline here.

Quoted 93 lines

drivers/usb/misc/usbio.c new file +93 -0

+	return cpkt->len;
+}
+
+int usbio_control_msg(struct auxiliary_device *adev, u8 type, u8 cmd,
+		      const void *obuf, u16 obuf_len, void *ibuf, u16 ibuf_len)
+{
+	struct usbio_client *client = adev_to_client(adev);
+	int ret;
+
+	ret = usbio_acquire(adev);
+	if (ret)
+		return ret;
+
+	ret = usbio_ctrl_msg(client->bridge, type, cmd, obuf, obuf_len, ibuf, ibuf_len);
+
+	usbio_release(adev);
+
+	return ret;
+}
+EXPORT_SYMBOL_NS_GPL(usbio_control_msg, "USBIO");
+
+static void usbio_bulk_recv(struct urb *urb)
+{
+	struct usbio_bulk_packet *bpkt = urb->transfer_buffer;
+	struct usbio_device *usbio = urb->context;
+
+	if (!urb->status) {
+		if (bpkt->header.flags & USBIO_PKTFLAG_RSP) {
+			usbio->rxdat_len = urb->actual_length;
+			complete(&usbio->done);
+		}
+	} else if (urb->status != -ENOENT) {
+		dev_err(usbio->dev, "Bulk in error %d\n", urb->status);
+	}
+
+	usb_submit_urb(usbio->urb, GFP_ATOMIC);
+}
+
+int usbio_bulk_msg(struct auxiliary_device *adev, u8 type, u8 cmd, bool last,
+		   const void *obuf, u16 obuf_len, void *ibuf, u16 ibuf_len)
+{
+	struct usbio_device *usbio = adev_to_client(adev)->bridge;
+	struct usbio_bulk_packet *bpkt;
+	int ret, act;
+	u16 bpkt_len;
+
+	lockdep_assert_held(&usbio->mutex);
+
+	if ((obuf_len > (usbio->txbuf_len - sizeof(*bpkt))) ||
+	    (ibuf_len > (usbio->txbuf_len - sizeof(*bpkt))))
+		return -EMSGSIZE;
+
+	if (ibuf_len)
+		reinit_completion(&usbio->done);
+
+	/* If no data to send, skip to read */
+	if (!obuf_len)
+		goto read;
+
+	/* Prepare Bulk Packet Header */
+	bpkt = usbio->txbuf;
+	bpkt->header.type = type;
+	bpkt->header.cmd = cmd;
+	if (!last)
+		bpkt->header.flags = 0;
+	else if (ibuf_len)
+		bpkt->header.flags = USBIO_PKTFLAGS_REQRESP;
+	else
+		bpkt->header.flags = USBIO_PKTFLAG_CMP;
+	bpkt->len = obuf_len;
+
+	/* Copy the data */
+	memcpy(bpkt->data, obuf, obuf_len);
+
+	bpkt_len = sizeof(*bpkt) + obuf_len;
+	ret = usb_bulk_msg(usbio->udev, usbio->tx_pipe, bpkt, bpkt_len, &act,
+			   USBIO_BULKXFER_TIMEOUT);
+	dev_dbg(usbio->dev, "bulk out %d hdr %*phN data %*phN\n", act,
+		(int)sizeof(*bpkt), bpkt, (int)bpkt->len, bpkt->data);
+
+	if (ret || act != bpkt_len) {
+		dev_err(usbio->dev, "Bulk out failed: %d\n", ret);
+		return ret ?: -EPROTO;
+	}
+
+	if (!(bpkt->header.flags & USBIO_PKTFLAG_ACK))
+		return obuf_len;
+
+read:
+	ret = wait_for_completion_timeout(&usbio->done, USBIO_BULKXFER_TIMEOUT);
+	if (ret <= 0) {
+		dev_err(usbio->dev, "Bulk in wait failed: %d\n", ret);
+		return ret ?: -ETIMEDOUT;

drivers/usb/misc/usbio.c line 317

Quoted 6 lines

drivers/usb/misc/usbio.c new file +6 -0

+	}
+
+	act = usbio->rxdat_len;
+	bpkt = usbio->rxbuf;
+	dev_dbg(usbio->dev, "bulk in %d hdr %*phN data %*phN\n", act,
+		(int)sizeof(*bpkt), bpkt, (int)bpkt->len, bpkt->data);

%zu, please.

Quoted 23 lines

drivers/usb/misc/usbio.c new file +23 -0

+
+	/*
+	 * Unsupported bulk commands get only an usbio_packet_header with
+	 * the error flag set as reply. Return -EPIPE for this case.
+	 */
+	if (act == sizeof(struct usbio_packet_header) &&
+	    (bpkt->header.flags & USBIO_PKTFLAG_ERR))
+		return -EPIPE;
+
+	if (act < sizeof(*bpkt)) {
+		dev_err(usbio->dev, "Bulk in short read: %d\n", act);
+		return -EPROTO;
+	}
+
+	if (bpkt->header.type != type || bpkt->header.cmd != cmd ||
+	    !(bpkt->header.flags & USBIO_PKTFLAG_RSP)) {
+		dev_err(usbio->dev,
+			"Unexpected bulk in type 0x%02x cmd 0x%02x flags 0x%02x\n",
+			bpkt->header.type, bpkt->header.cmd, bpkt->header.flags);
+		return -EPROTO;
+	}
+
+	if (bpkt->header.flags & USBIO_PKTFLAG_ERR)

drivers/usb/misc/usbio.c line 346

Quoted 6 lines

drivers/usb/misc/usbio.c new file +6 -0

+		return -EREMOTEIO;
+
+	if (ibuf_len < bpkt->len)
+		return -ENOSPC;
+
+	memcpy(ibuf, bpkt->data, bpkt->len);

One more newline, perhaps?

Quoted 68 lines

drivers/usb/misc/usbio.c new file +68 -0

+	return bpkt->len;
+}
+EXPORT_SYMBOL_NS_GPL(usbio_bulk_msg, "USBIO");
+
+static int usbio_get_and_lock(struct usbio_device *usbio)
+{
+	int ret;
+
+	ret = usb_autopm_get_interface(usbio->intf);
+	if (ret)
+		return ret;
+
+	mutex_lock(&usbio->mutex);
+	return 0;
+}
+
+static void usbio_unlock_and_put(struct usbio_device *usbio)
+{
+	mutex_unlock(&usbio->mutex);
+	usb_autopm_put_interface(usbio->intf);
+}
+
+int usbio_acquire(struct auxiliary_device *adev)
+{
+	return usbio_get_and_lock(adev_to_client(adev)->bridge);
+}
+EXPORT_SYMBOL_NS_GPL(usbio_acquire, "USBIO");
+
+void usbio_release(struct auxiliary_device *adev)
+{
+	usbio_unlock_and_put(adev_to_client(adev)->bridge);
+}
+EXPORT_SYMBOL_NS_GPL(usbio_release, "USBIO");
+
+void usbio_get_txrxbuf_len(struct auxiliary_device *adev, u16 *txbuf_len, u16 *rxbuf_len)
+{
+	struct usbio_device *usbio = adev_to_client(adev)->bridge;
+
+	*txbuf_len = usbio->txbuf_len;
+	*rxbuf_len = usbio->rxbuf_len;
+}
+EXPORT_SYMBOL_NS_GPL(usbio_get_txrxbuf_len, "USBIO");
+
+static void usbio_auxdev_release(struct device *dev)
+{
+	struct auxiliary_device *adev = to_auxiliary_dev(dev);
+	struct usbio_client *client = adev_to_client(adev);
+
+	kfree(client);
+}
+
+static int usbio_add_client(struct usbio_device *usbio, char *name, u8 id, void *data)
+{
+	struct usbio_client *client;
+	struct auxiliary_device *adev;
+	int ret;
+
+	client = kzalloc(sizeof(*client), GFP_KERNEL);
+	if (!client)
+		return -ENOMEM;
+
+	client->bridge = usbio;
+	adev = &client->adev;
+	adev->name = name;
+	adev->id = id;
+
+	adev->dev.parent = usbio->dev;
+	adev->dev.platform_data = data;

drivers/usb/misc/usbio.c line 420

Quoted 6 lines

drivers/usb/misc/usbio.c new file +6 -0

+	adev->dev.release = usbio_auxdev_release;
+
+	ret = auxiliary_device_init(adev);
+	if (!ret) {
+		if (auxiliary_device_add(adev))
+			auxiliary_device_uninit(adev);

This isn't enough for error handling if auxiliary_device_add() fails.

drivers/usb/misc/usbio.c line 429

Quoted 10 lines

drivers/usb/misc/usbio.c new file +9 -0

+	}
+
+	if (ret) {
+		dev_err(usbio->dev, "Client %s.%d add failed: %d\n", name, id, ret);
+		kfree(client);
+		return ret;
+	}
+
+	list_add_tail(&client->link, &usbio->cli_list);

A newline, please?

drivers/usb/misc/usbio.c line 436

Quoted 7 lines

drivers/usb/misc/usbio.c new file +7 -0

+	return 0;
+}
+
+static int usbio_ctrl_enumgpios(struct usbio_device *usbio)
+{
+	struct usbio_gpio_bank_desc *gpio = usbio->gpios;
+	int ret, i;

unsigned int i, please.

Quoted 9 lines

drivers/usb/misc/usbio.c new file +9 -0

+
+	ret = usbio_ctrl_msg(usbio, USBIO_PKTTYPE_CTRL, USBIO_CTRLCMD_ENUMGPIO, NULL, 0,
+			     gpio, sizeof(usbio->gpios));
+	if (ret < 0 || ret % sizeof(*gpio)) {
+		dev_err(usbio->dev, "CTRL ENUMGPIO failed: %d\n", ret);
+		return ret;
+	}
+
+	usbio->nr_gpio_banks = ret / sizeof(*gpio);

drivers/usb/misc/usbio.c line 451

Quoted 6 lines

drivers/usb/misc/usbio.c new file +6 -0

+	dev_dbg(usbio->dev, "GPIO Banks: %d\n", usbio->nr_gpio_banks);
+	for (i = 0; i < usbio->nr_gpio_banks; i++)
+		dev_dbg(usbio->dev, "\tBank%d[%d] map: %#08x\n",
+			gpio[i].id, gpio[i].pins, gpio[i].bmap);
+
+	usbio_add_client(usbio, USBIO_GPIO_CLIENT, 0, gpio);

One more newline?

drivers/usb/misc/usbio.c line 458

Quoted 7 lines

drivers/usb/misc/usbio.c new file +7 -0

+	return 0;
+}
+
+static int usbio_ctrl_enumi2cs(struct usbio_device *usbio)
+{
+	struct usbio_i2c_bus_desc *i2c = usbio->i2cs;
+	int ret, i;

unsigned int i, please.

Quoted 12 lines

drivers/usb/misc/usbio.c new file +12 -0

+
+	ret = usbio_ctrl_msg(usbio, USBIO_PKTTYPE_CTRL, USBIO_CTRLCMD_ENUMI2C, NULL, 0,
+			     i2c, sizeof(usbio->i2cs));
+	if (ret < 0 || ret % sizeof(*i2c)) {
+		dev_err(usbio->dev, "CTRL ENUMI2C failed: %d\n", ret);
+		return ret;
+	}
+
+	usbio->nr_i2c_buses = ret / sizeof(*i2c);
+	dev_dbg(usbio->dev, "I2C Busses: %d\n", usbio->nr_i2c_buses);
+	for (i = 0; i < usbio->nr_i2c_buses; i++) {
+		dev_dbg(usbio->dev, "\tBus%d caps: %#02x\n", i2c[i].id, i2c[i].caps);

drivers/usb/misc/usbio.c line 480

Quoted 10 lines

drivers/usb/misc/usbio.c new file +10 -0

+		usbio_add_client(usbio, USBIO_I2C_CLIENT, i, &i2c[i]);
+	}
+
+	return 0;
+}
+
+static int usbio_ctrl_enumspis(struct usbio_device *usbio)
+{
+	struct usbio_spi_bus_desc *spi = usbio->spis;
+	int ret, i;

Ditto.

Quoted 40 lines

drivers/usb/misc/usbio.c new file +40 -0

+
+	ret = usbio_ctrl_msg(usbio, USBIO_PKTTYPE_CTRL, USBIO_CTRLCMD_ENUMSPI, NULL, 0,
+			     spi, sizeof(usbio->spis));
+	if (ret < 0 || ret % sizeof(*spi)) {
+		dev_err(usbio->dev, "CTRL ENUMSPI failed: %d\n", ret);
+		return ret;
+	}
+
+	usbio->nr_spi_buses = ret / sizeof(*spi);
+	dev_dbg(usbio->dev, "SPI Busses: %d\n", usbio->nr_spi_buses);
+	for (i = 0; i < usbio->nr_spi_buses; i++)
+		dev_dbg(usbio->dev, "\tBus%d caps: %#02x\n", spi[i].id, spi[i].caps);
+
+	return 0;
+}
+
+static int usbio_suspend(struct usb_interface *intf, pm_message_t msg)
+{
+	struct usbio_device *usbio = usb_get_intfdata(intf);
+
+	usb_kill_urb(usbio->urb);
+
+	return 0;
+}
+
+static int usbio_resume(struct usb_interface *intf)
+{
+	struct usbio_device *usbio = usb_get_intfdata(intf);
+
+	return usb_submit_urb(usbio->urb, GFP_KERNEL);
+}
+
+static void usbio_disconnect(struct usb_interface *intf)
+{
+	struct usbio_device *usbio = usb_get_intfdata(intf);
+	struct usbio_client *client, *prev;
+
+	list_for_each_entry_safe_reverse(client, prev, &usbio->cli_list, link) {
+		auxiliary_device_delete(&client->adev);
+		list_del_init(&client->link);

drivers/usb/misc/usbio.c line 525

Quoted 5 lines

drivers/usb/misc/usbio.c new file +5 -0

+		auxiliary_device_uninit(&client->adev);
+	}
+
+	usb_kill_urb(usbio->urb);
+	usb_free_urb(usbio->urb);

What will happen on client drivers if they're working with the bridge while disconnect happens?

One easy solution to this could be to use an rw_semaphore where client acquire it for readingin conjunction (in a helper that also checks the interface status) and disconnect callback for writing.

drivers/usb/misc/usbio.c line 536

Quoted 12 lines

drivers/usb/misc/usbio.c new file +11 -0

+}
+
+static int usbio_probe(struct usb_interface *intf, const struct usb_device_id *id)
+{
+	struct usb_device *udev = interface_to_usbdev(intf);
+	struct usb_endpoint_descriptor *ep_in, *ep_out;
+	struct device *dev = &intf->dev;
+	struct usbio_device *usbio;
+	int ret;
+
+	usbio = devm_kzalloc(dev, sizeof(*usbio), GFP_KERNEL);

usbio will be released at the exit from usbio_disconnect(). I think you'll need to use a release callback in struct device to release usbio once all clients are gone.

Quoted 72 lines

drivers/usb/misc/usbio.c new file +71 -0

+	if (!usbio)
+		return -ENOMEM;
+
+	ret = devm_mutex_init(dev, &usbio->mutex);
+	if (ret)
+		return ret;
+
+	usbio->dev = dev;
+	usbio->udev = udev;
+	usbio->intf = intf;
+	init_completion(&usbio->done);
+	INIT_LIST_HEAD(&usbio->cli_list);
+	usb_set_intfdata(intf, usbio);
+
+	usbio->ctrl_pipe = usb_endpoint_num(&udev->ep0.desc);
+	usbio->ctrlbuf_len = usb_maxpacket(udev, usbio->ctrl_pipe);
+	usbio->ctrlbuf = devm_kzalloc(dev, usbio->ctrlbuf_len, GFP_KERNEL);
+	if (!usbio->ctrlbuf)
+		return -ENOMEM;
+
+	/* Find the first bulk-in and bulk-out endpoints */
+	ret = usb_find_common_endpoints(intf->cur_altsetting, &ep_in, &ep_out,
+					NULL, NULL);
+	if (ret) {
+		dev_err(dev, "Cannot find bulk endpoints: %d\n", ret);
+		return ret;
+	}
+
+	usbio->tx_pipe = usb_sndbulkpipe(udev, usb_endpoint_num(ep_out));
+	usbio->txbuf_len = usb_endpoint_maxp(ep_out);
+	usbio->txbuf = devm_kzalloc(dev, usbio->txbuf_len, GFP_KERNEL);
+	if (!usbio->txbuf)
+		return -ENOMEM;
+
+	usbio->rx_pipe = usb_rcvbulkpipe(udev, usb_endpoint_num(ep_in));
+	usbio->rxbuf_len = usb_endpoint_maxp(ep_in);
+	usbio->rxbuf = devm_kzalloc(dev, usbio->rxbuf_len, GFP_KERNEL);
+	if (!usbio->rxbuf)
+		return -ENOMEM;
+
+	usbio->urb = usb_alloc_urb(0, GFP_KERNEL);
+	if (!usbio->urb)
+		return -ENOMEM;
+
+	usb_fill_bulk_urb(usbio->urb, udev, usbio->rx_pipe, usbio->rxbuf,
+			  usbio->rxbuf_len, usbio_bulk_recv, usbio);
+	ret = usb_submit_urb(usbio->urb, GFP_KERNEL);
+	if (ret)
+		return dev_err_probe(dev, ret, "Submitting usb urb\n");
+
+	ret = usbio_get_and_lock(usbio);
+	if (ret)
+		return ret;
+
+	ret = usbio_ctrl_msg(usbio, USBIO_PKTTYPE_CTRL, USBIO_CTRLCMD_HS, NULL, 0, NULL, 0);
+	if (ret < 0)
+		goto error;
+
+	ret = usbio_ctrl_msg(usbio, USBIO_PKTTYPE_CTRL, USBIO_CTRLCMD_PROTVER, NULL, 0,
+			     &usbio->info.protver, sizeof(struct usbio_protver));
+	if (ret < 0)
+		goto error;
+
+	ret = usbio_ctrl_msg(usbio, USBIO_PKTTYPE_CTRL, USBIO_CTRLCMD_FWVER, NULL, 0,
+			     &usbio->info.fwver, sizeof(struct usbio_fwver));
+	if (ret < 0)
+		goto error;
+
+	dev_dbg(dev, "ProtVer(BCD): %02x FwVer: %d.%d.%d.%d\n",
+		usbio->info.protver.ver, usbio->info.fwver.major,
+		usbio->info.fwver.minor, usbio->info.fwver.patch,

drivers/usb/misc/usbio.c line 613

Quoted 6 lines

drivers/usb/misc/usbio.c new file +6 -0

+		usbio->info.fwver.build);
+
+	usbio_ctrl_enumgpios(usbio);
+	usbio_ctrl_enumi2cs(usbio);
+	usbio_ctrl_enumspis(usbio);
+	usbio_unlock_and_put(usbio);

A newline here?

drivers/usb/misc/usbio.c line 618

Quoted 6 lines

drivers/usb/misc/usbio.c new file +5 -0

+	return 0;
+
+error:
+	usbio_unlock_and_put(usbio);
+	usbio_disconnect(intf);

Ditto.

drivers/usb/misc/usbio.c line 626

Quoted 9 lines

drivers/usb/misc/usbio.c new file +8 -0

+	return ret;
+}
+
+static const struct usb_device_id usbio_table[] = {
+	{ USB_DEVICE(0x2AC1, 0x20C1) }, /* Lattice NX40 */
+	{ USB_DEVICE(0x2AC1, 0x20C9) }, /* Lattice NX33 */
+	{ USB_DEVICE(0x2AC1, 0x20CB) }, /* Lattice NX33U */
+	{ USB_DEVICE(0x06CB, 0x0701) }, /* Synaptics Sabre */

Lower case hexadecimals would be nice.

drivers/usb/misc/usbio.c line 638

Quoted 13 lines

drivers/usb/misc/usbio.c new file +12 -0

+	{ }
+};
+MODULE_DEVICE_TABLE(usb, usbio_table);
+
+static struct usb_driver usbio_driver = {
+	.name = "usbio-bridge",
+	.probe = usbio_probe,
+	.disconnect = usbio_disconnect,
+	.suspend = usbio_suspend,
+	.resume = usbio_resume,
+	.id_table = usbio_table,
+	.supports_autosuspend = 1

Add the leading comma perhaps?

Quoted 12 lines

drivers/usb/misc/usbio.c new file +11 -0

+};
+module_usb_driver(usbio_driver);
+
+struct usbio_match_ids_walk_data {
+	struct acpi_device *adev;
+	const struct acpi_device_id *hids;
+	const u8 id;
+};
+
+static int usbio_match_device_ids(struct acpi_device *adev, void *data)
+{

drivers/usb/misc/usbio.c line 654

Quoted 5 lines

drivers/usb/misc/usbio.c new file +5 -0

+	struct usbio_match_ids_walk_data *wd = data;
+	unsigned int id = 0;
+	char *uid;
+
+	if (!acpi_match_device_ids(adev, wd->hids)) {

You could return here if acpi_match_device_ids() returns non-zero and unindent the rest.

drivers/usb/misc/usbio.c line 657

Quoted 4 lines

drivers/usb/misc/usbio.c new file +3 -0

+		uid = acpi_device_uid(adev);
+		if (uid)
+			for (int i = 0; i < strlen(uid); i++)

size_t i?

Quoted 4 lines

drivers/usb/misc/usbio.c new file +4 -0

+				if (!kstrtouint(&uid[i], 10, &id))
+					break;
+
+		if (!uid || (uid && wd->id == (u8)id)) {

drivers/usb/misc/usbio.c line 669

Quoted 8 lines

drivers/usb/misc/usbio.c new file +8 -0

+			wd->adev = adev;
+			return 1;
+		}
+	}
+
+	return 0;
+}
+

Quoted 4 lines

Quoted 1 line

+#define adev_to_client(adev) container_of(adev, struct usbio_client, adev)

Please use a different name than "adev" for the argument, which is also the struct field of interest.

As gkh mentioned doing things this way is quite normal.

Quoted 8 lines

Quoted 8 lines

drivers/usb/misc/usbio.c new file +8 -0

+static void usbio_disconnect(struct usb_interface *intf)
+{
+	struct usbio_device *usbio = usb_get_intfdata(intf);
+	struct usbio_client *client, *prev;
+
+	list_for_each_entry_safe_reverse(client, prev, &usbio->cli_list, link) {
+		auxiliary_device_delete(&client->adev);
+		list_del_init(&client->link);

Quoted 8 lines

drivers/usb/misc/usbio.c line 525

Quoted 5 lines

drivers/usb/misc/usbio.c new file +5 -0

+		auxiliary_device_uninit(&client->adev);
+	}
+
+	usb_kill_urb(usbio->urb);
+	usb_free_urb(usbio->urb);

What will happen on client drivers if they're working with the bridge while disconnect happens?

All clients will have their remove() function called. If clients somehow need to have some userspace-api related bits stick around longer they will need to handle that themselves. Clients are no longer allowed to call an usbio_*() functions after having their remove() function called.

Quoted 3 lines

One easy solution to this could be to use an rw_semaphore where client acquire it for readingin conjunction (in a helper that also checks the interface status) and disconnect callback for writing.

That is not necessary.

One thing which should be done here is set a disconnected flag to make new usbio_*() calls fail immediately and signal the "done" completion to wakeup any waiters, something like this:

static void usbio_disconnect(struct usb_interface *intf)

{
	struct usbio_device *usbio = usb_get_intfdata(intf);
	struct usbio_client *client, *prev;

	list_for_each_entry(client, &usbio->cli_list, link)
		client->disconnected = true;

	complete(&usbio->done);

	/* Ensure all clients see the disconnect flag */
	mutex_lock(&usbio->mutex);
	mutex_unlock(&usbio->mutex);

	/* From here on clients will no longer touch struct usbio_device */
	usb_kill_urb(usbio->urb);
	usb_free_urb(usbio->urb);

	list_for_each_entry_safe_reverse(client, prev, &usbio->cli_list, link) {
		auxiliary_device_delete(&client->adev);
		list_del_init(&client->link);
		auxiliary_device_uninit(&client->adev);
	}
}

Combined with checking client->disconnected in entry points and after waiting for the completion.

This will even allow the client to keep the struct auxdev around by keeping a reference and it will even be safe for the client to make usbio_*() calls on the auxdev since as long as the reference is there the memory of the usbio_client which embeds the auxdev will stick around and the disconnected flag will ensure this is safe. Note client drivers really should not do that though!

Quoted 8 lines

Quoted 15 lines

drivers/usb/misc/usbio.c line 536

Quoted 11 lines

drivers/usb/misc/usbio.c new file +11 -0

+}
+
+static int usbio_probe(struct usb_interface *intf, const struct usb_device_id *id)
+{
+	struct usb_device *udev = interface_to_usbdev(intf);
+	struct usb_endpoint_descriptor *ep_in, *ep_out;
+	struct device *dev = &intf->dev;
+	struct usbio_device *usbio;
+	int ret;
+
+	usbio = devm_kzalloc(dev, sizeof(*usbio), GFP_KERNEL);

usbio will be released at the exit from usbio_disconnect(). I think you'll need to use a release callback in struct device to release usbio once all clients are gone.

This is fine, see above it is up to the client drivers to no longer call any usbio_*() also for v2 I'll make all the public usbio_*() functions test client->disconnected and then they will never deref client->bridge.

Hi Hans,

Thanks for posting this. Some comments below...

Quoted 13 lines · Hans de Goede

From: Israel Cepeda <[email protected]>

Add a driver for the Intel USBIO USB IO-expander used by the MIPI cameras on various new (Meteor Lake and later) Intel laptops.

This is an USB bridge driver which adds auxbus child devices for the GPIO, I2C and SPI functions of the USBIO chip and which exports IO-functions for the drivers for the auxbus child devices to communicate with the USBIO device's firmware.

Co-developed-by Hans de Goede <[email protected]>

Signed-off-by Hans de Goede <[email protected]>

Signed-off-by Israel Cepeda <[email protected]>

Quoted 6 lines

drivers/usb/misc/usbio.c new file +0 -0

new file mode 100644
index 000000000000..88197092f39a
@@ -0,0 +1,693 @@

Quoted 1 line

+#define adev_to_client(adev) container_of(adev, struct usbio_client, adev)

Please use a different name than "adev" for the argument, which is also the struct field of interest.

Quoted 8 lines

drivers/usb/misc/usbio.c new file +8 -0

+
+static int usbio_ctrl_msg(struct usbio_device *usbio, u8 type, u8 cmd,
+			  const void *obuf, u16 obuf_len, void *ibuf, u16 ibuf_len)
+{
+	u8 request = USB_TYPE_VENDOR | USB_RECIP_DEVICE;
+	struct usbio_ctrl_packet *cpkt;
+	unsigned int pipe;
+	u16 cpkt_len;

drivers/usb/misc/usbio.c line 162

Quoted 6 lines

drivers/usb/misc/usbio.c new file +6 -0

+	int ret;
+
+	lockdep_assert_held(&usbio->mutex);
+
+	if ((obuf_len > (usbio->ctrlbuf_len - sizeof(*cpkt))) ||
+	    (ibuf_len > (usbio->ctrlbuf_len - sizeof(*cpkt))))

You can (and should) remove all parentheses except the outer ones here.

drivers/usb/misc/usbio.c line 183

Quoted 6 lines

drivers/usb/misc/usbio.c new file +6 -0

+	pipe = usb_sndctrlpipe(usbio->udev, usbio->ctrl_pipe);
+	cpkt_len = sizeof(*cpkt) + obuf_len;
+	ret = usb_control_msg(usbio->udev, pipe, 0, request | USB_DIR_OUT, 0, 0,
+			      cpkt, cpkt_len, USBIO_CTRLXFER_TIMEOUT);
+	dev_dbg(usbio->dev, "control out %d hdr %*phN data %*phN\n", ret,
+		(int)sizeof(*cpkt), cpkt, (int)cpkt->len, cpkt->data);

Instead of casting, how about using %zu for printing a size_t?

drivers/usb/misc/usbio.c line 218

Quoted 4 lines

drivers/usb/misc/usbio.c new file +4 -0

+	if (ibuf_len < cpkt->len)
+		return -ENOSPC;
+
+	memcpy(ibuf, cpkt->data, cpkt->len);

It'd be nice to have one more newline here.

drivers/usb/misc/usbio.c

Quoted 3 lines

drivers/usb/misc/usbio.c new file +2 -0

+	return cpkt->len;
+}

From: Israel Cepeda <[email protected]>

Add a a driver for the GPIO auxbus child device of the Intel USBIO USB IO-expander used by the MIPI cameras on various new (Meteor Lake and later) Intel laptops.

Co-developed-by Hans de Goede <[email protected]>

Signed-off-by Hans de Goede <[email protected]>

Signed-off-by Israel Cepeda <[email protected]>

---
 MAINTAINERS               |   1 +
 drivers/gpio/Kconfig      |  11 ++
 drivers/gpio/Makefile     |   1 +
 drivers/gpio/gpio-usbio.c | 258 ++++++++++++++++++++++++++++++++++++++
 4 files changed, 271 insertions(+)
 create mode 100644 drivers/gpio/gpio-usbio.c

MAINTAINERS +1 -0

index 1208efe41f9f..81db1457e9d1 100644
@@ -12522,6 +12522,7 @@ INTEL USBIO USB I/O EXPANDER DRIVERS
 M:	Israel Cepeda <[email protected]>
 M:	Hans de Goede <[email protected]>
 S:	Maintained
+F:	drivers/gpio/gpio-usbio.c
 F:	drivers/usb/misc/usbio.c
 F:	include/linux/usb/usbio.h
 

drivers/gpio/Kconfig +11 -0

index 44f922e10db2..f3f7b3b782a3 100644
@@ -1436,6 +1436,17 @@ config GPIO_LJCA
 	  This driver can also be built as a module. If so, the module
 	  will be called gpio-ljca.
 
+config GPIO_USBIO
+	tristate "Intel USBIO GPIO support"
+	depends on USB_USBIO
+	default USB_USBIO
+	help
+	  Select this option to enable GPIO driver for the INTEL
+	  USBIO driver stack.
+
+	  This driver can also be built as a module. If so, the module
+	  will be called gpio_usbio.
+
 config GPIO_LP3943
 	tristate "TI/National Semiconductor LP3943 GPIO expander"
 	depends on MFD_LP3943

drivers/gpio/Makefile +1 -0

index 88dedd298256..a2c054ea2d4c 100644
@@ -90,6 +90,7 @@ obj-$(CONFIG_GPIO_JANZ_TTL)		+= gpio-janz-ttl.o
 obj-$(CONFIG_GPIO_KEMPLD)		+= gpio-kempld.o
 obj-$(CONFIG_GPIO_LATCH)		+= gpio-latch.o
 obj-$(CONFIG_GPIO_LJCA) 		+= gpio-ljca.o
+obj-$(CONFIG_GPIO_USBIO) 		+= gpio-usbio.o
 obj-$(CONFIG_GPIO_LOGICVC)		+= gpio-logicvc.o
 obj-$(CONFIG_GPIO_LOONGSON1)		+= gpio-loongson1.o
 obj-$(CONFIG_GPIO_LOONGSON)		+= gpio-loongson.o

drivers/gpio/gpio-usbio.c new file +30 -0

new file mode 100644
index 000000000000..08a1219153f4
@@ -0,0 +1,258 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2025 Intel Corporation.
+ * Copyright (c) 2025 Red Hat, Inc.
+ */
+
+#include <linux/acpi.h>
+#include <linux/auxiliary_bus.h>
+#include <linux/dev_printk.h>
+#include <linux/device.h>
+#include <linux/gpio/driver.h>
+#include <linux/types.h>
+#include <linux/usb/usbio.h>
+
+struct usbio_gpio_bank {
+	u8 config[USBIO_GPIOSPERBANK];
+	u32 bitmap;
+};
+
+struct usbio_gpio {
+	struct usbio_gpio_bank banks[USBIO_MAX_GPIOBANKS];
+	struct gpio_chip gc;
+	struct auxiliary_device *adev;
+};
+
+static const struct acpi_device_id usbio_gpio_acpi_hids[] = {
+	{ "INTC1007" }, /* MTL */
+	{ "INTC10B2" }, /* ARL */
+	{ "INTC10B5" }, /* LNL */
+	{ "INTC10E2" }, /* PTL */

drivers/gpio/gpio-usbio.c new file +20 -0

+
+	if (offset >= gc->ngpio)
+		return false;
+
+	bank = &gpio->banks[offset / USBIO_GPIOSPERBANK];
+	pin = offset % USBIO_GPIOSPERBANK;
+	if (~bank->bitmap & BIT(pin)) {
+		/* The FW bitmap sometimes is invalid, warn and continue */
+		dev_warn_once(dev, FW_BUG "GPIO %u is not in FW pins bitmap\n", offset);
+	}
+
+	*bank_ret = bank;
+	*pin_ret = pin;
+	return true;
+}
+
+static int usbio_gpio_get_direction(struct gpio_chip *gc, unsigned int offset)
+{
+	struct usbio_gpio_bank *bank;
+	int pin;