Linux USB
 help / color / mirror / Atom feed
From: Igor Skalkin <igor.skalkin@oss.qualcomm.com>
To: "Michael S . Tsirkin" <mst@redhat.com>,
	Jason Wang <jasowangio@gmail.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: virtualization@lists.linux.dev, linux-usb@vger.kernel.org,
	Vasilii Ianikeev <vasilii.ianikeev@oss.qualcomm.com>,
	Aiswarya Cyriac <aiswarya.cyriac@oss.qualcomm.com>,
	Anton Yakovlev <anton.yakovlev@oss.qualcomm.com>,
	Trilok Soni <trilok.soni@oss.qualcomm.com>,
	Igor Skalkin <igor.skalkin@oss.qualcomm.com>
Subject: [PATCH 4/8] virtio-usb: add OTG role query support
Date: Thu, 24 Sep 2026 18:09:03 +0200	[thread overview]
Message-ID: <20260924160907.145405-5-igor.skalkin@oss.qualcomm.com> (raw)
In-Reply-To: <20260924160907.145405-1-igor.skalkin@oss.qualcomm.com>

With both host_role and device_role negotiated, a port's own role is
ambiguous (host|device) - determining it requires more than reading
the two feature bits. Introduce a minimal OTG command/event queue
pair (otg_vq_base, otg_vqueues[], otg_init()/otg_deinit()) and
otg_get_role(), a synchronous VIRTIO_USB_CMD_OTG_GET_ROLE request/
response exchange, and query every port's role individually during
probe instead of the alternative of overloading the config space
ports field with role bits.

This replaces the previous commit's DEVICE-only placeholder for the
both-roles-negotiated case with the real per-port answer.

Signed-off-by: Igor Skalkin <igor.skalkin@oss.qualcomm.com>
---
 drivers/usb/virtio_usb/Makefile     |    3 
 drivers/usb/virtio_usb/controller.c |   87 +++++++++++++++---
 drivers/usb/virtio_usb/controller.h |   11 ++
 drivers/usb/virtio_usb/otg.c        |  168 ++++++++++++++++++++++++++++++++++++
 drivers/usb/virtio_usb/otg.h        |   26 +++++
 5 files changed, 277 insertions(+), 18 deletions(-)
 create mode 100644 drivers/usb/virtio_usb/otg.c
 create mode 100644 drivers/usb/virtio_usb/otg.h

diff --git a/drivers/usb/virtio_usb/controller.c b/drivers/usb/virtio_usb/controller.c
index 0646807..59af5cc 100644
--- a/drivers/usb/virtio_usb/controller.c
+++ b/drivers/usb/virtio_usb/controller.c
@@ -12,6 +12,7 @@
 #include "controller.h"
 #include "host.h"
 #include "device.h"
+#include "otg.h"
 #include "vq_common.h"
 
 u32 virtio_usb_cmd_timeout_ms = MSEC_PER_SEC;
@@ -130,12 +131,12 @@ static int virtio_usb_probe(struct virtio_device *vdev)
 
 	/* Only allocate/negotiate the virtqueue triplets this instance
 	 * actually needs: HOST_* only exists when host_role is negotiated,
-	 * DEV_* only when device_role is negotiated. A pure single-role
-	 * instance therefore has exactly VIRTIO_USB_VQ_HOST_MAX (3) or
-	 * VIRTIO_USB_VQ_DEV_MAX (3) virtqueues, not a fixed layout - queues
-	 * that don't exist on the wire must not be created, since a peer
-	 * with no host-role VP has no host command/event/data queues to
-	 * negotiate at all.
+	 * DEV_* only when device_role is negotiated, OTG_* only when both
+	 * roles are negotiated. A pure single-role instance therefore has
+	 * exactly VIRTIO_USB_VQ_HOST_MAX (3) or VIRTIO_USB_VQ_DEV_MAX (3)
+	 * virtqueues, not a fixed layout - queues that don't exist on the
+	 * wire must not be created, since a peer with no host-role VP has
+	 * no host command/event/data queues to negotiate at all.
 	 */
 	vusb->host_vq_base = -1;
 	vusb->dev_vq_base = -1;
@@ -148,17 +149,15 @@ static int virtio_usb_probe(struct virtio_device *vdev)
 		vusb->dev_vq_base = nvqs;
 		nvqs += VIRTIO_USB_VQ_DEV_MAX;
 	}
-
-	/* Resolve every port's role. With only one role negotiated, every
-	 * port unambiguously has that role. With both negotiated, a port's
-	 * own role is ambiguous until a later commit adds an OTG-based
-	 * per-port query - default to DEVICE for now as a placeholder.
-	 */
-	for (i = 0; i < vusb->nports; i++) {
-		if (vusb->host_role && !vusb->device_role)
-			vusb->vports[i].role = VIRTIO_USB_ROLE_HOST;
-		else if (vusb->device_role)
-			vusb->vports[i].role = VIRTIO_USB_ROLE_DEVICE;
+	if (vusb->host_role && vusb->device_role) {
+		/* The OTG command/event queue pair is needed whenever both
+		 * roles are negotiated - it is how the driver asks each
+		 * port for its actual role via otg_get_role() below, since
+		 * a port's own role is otherwise ambiguous (host|device)
+		 * until then.
+		 */
+		vusb->otg_vq_base = nvqs;
+		nvqs += VIRTIO_USB_VQ_OTG_MAX;
 	}
 
 	vusb->vqueues = devm_kcalloc(&vdev->dev, nvqs, sizeof(*vusb->vqueues),
@@ -192,6 +191,18 @@ static int virtio_usb_probe(struct virtio_device *vdev)
 				dev_vqueues[i].stop;
 		}
 
+	if (vusb->host_role && vusb->device_role)
+		for (i = 0; i < VIRTIO_USB_VQ_OTG_MAX; i++) {
+			vusb->vqueues[vusb->otg_vq_base + i].name =
+				otg_vqueues[i].name;
+			vusb->vqueues[vusb->otg_vq_base + i].callback =
+				otg_vqueues[i].callback;
+			vusb->vqueues[vusb->otg_vq_base + i].process =
+				otg_vqueues[i].process;
+			vusb->vqueues[vusb->otg_vq_base + i].stop =
+				otg_vqueues[i].stop;
+		}
+
 	rc = virtio_usb_find_vqs(vusb);
 	if (rc) {
 		dev_err(&vdev->dev, "%s virtio_usb_find_vqs() error(%d)\n",
@@ -199,6 +210,46 @@ static int virtio_usb_probe(struct virtio_device *vdev)
 		goto on_error;
 	}
 
+	if (vusb->host_role && vusb->device_role) {
+		rc = otg_init(vusb);
+		if (rc) {
+			dev_err(&vdev->dev, "%s otg_init() error(%d)\n",
+				__func__, rc);
+			goto on_error;
+		}
+	}
+
+	/* Resolve every port's role. With only one role negotiated, every
+	 * port unambiguously has that role. With both negotiated, query
+	 * each port's actual role individually via otg_get_role(), since
+	 * it is otherwise ambiguous (host|device).
+	 */
+	for (i = 0; i < vusb->nports; i++) {
+		if (vusb->host_role && !vusb->device_role) {
+			vusb->vports[i].role = VIRTIO_USB_ROLE_HOST;
+		} else if (vusb->device_role && !vusb->host_role) {
+			vusb->vports[i].role = VIRTIO_USB_ROLE_DEVICE;
+		} else {
+			u32 status, role;
+
+			status = otg_get_role(vusb, i, &role);
+			if (status != VIRTIO_USB_S_OK) {
+				dev_err(&vdev->dev, "%s status(%d)\n", __func__,
+					status);
+				rc = -EIO;
+				goto on_error;
+			}
+			if (role != VIRTIO_USB_ROLE_HOST &&
+			    role != VIRTIO_USB_ROLE_DEVICE) {
+				dev_err(&vdev->dev, "%s port%d wrong role %d\n",
+					__func__, i, role);
+				rc = -EIO;
+				goto on_error;
+			}
+			vusb->vports[i].role = role;
+		}
+	}
+
 	if (vusb->host_role) {
 		INIT_WORK(&vusb->vq_host_data_rx_work, virtio_usb_hc_rx_work);
 		INIT_WORK(&vusb->vq_host_evt_work, virtio_usb_hc_evt_work);
@@ -286,6 +337,8 @@ static void virtio_usb_remove(struct virtio_device *vdev)
 	virtio_reset_device(vdev);
 
 	vdev->config->del_vqs(vdev);
+
+	otg_deinit(vusb);
 }
 
 static const unsigned int virtio_usb_features[] = {
diff --git a/drivers/usb/virtio_usb/controller.h b/drivers/usb/virtio_usb/controller.h
index ec59922..4d9e0e2 100644
--- a/drivers/usb/virtio_usb/controller.h
+++ b/drivers/usb/virtio_usb/controller.h
@@ -18,6 +18,8 @@
 struct virtio_usb_hc_vp;
 /* Forward declaration - full definition in device.h */
 struct virtio_usb_dc;
+/* Forward declaration - full definition in otg.h */
+struct virtio_usb_otg;
 
 #define VIRTIO_USB_VQ_COMMAND_IDX 0
 #define VIRTIO_USB_VQ_EVENT_IDX 1
@@ -25,6 +27,7 @@ struct virtio_usb_dc;
 
 #define VIRTIO_USB_VQ_HOST_MAX 3
 #define VIRTIO_USB_VQ_DEV_MAX 3
+#define VIRTIO_USB_VQ_OTG_MAX 2
 
 /**
  * struct virtio_usb_port - Per-virtual-port state.
@@ -54,6 +57,12 @@ struct virtio_usb_port {
  * @dev_vq_base: index into vqueues[] where the DEV_COMMAND/EVENT/DATA
  *               triplet starts, or -1 if this instance has no
  *               device-role VP.
+ * @otg_vq_base: index into vqueues[] where the OTG_COMMAND/EVENT pair
+ *               starts, or -1 if this instance has neither a host-role
+ *               nor a device-role VP. Used to query each port's role via
+ *               otg_get_role() below, since with both host_role and
+ *               device_role negotiated a port's own role is otherwise
+ *               ambiguous.
  * @vq_host_data_rx_work: Kernel work draining the host data queue, shared
  *                        across every host-role VP.
  * @vq_host_evt_work: Kernel work draining the host event queue, shared
@@ -73,10 +82,12 @@ struct virtio_usb {
 	bool device_role;
 	int host_vq_base;
 	int dev_vq_base;
+	int otg_vq_base;
 	struct work_struct vq_host_data_rx_work;
 	struct work_struct vq_host_evt_work;
 	struct work_struct vq_dev_data_rx_work;
 	struct work_struct vq_dev_event_work;
+	struct virtio_usb_otg *otg;
 };
 
 /**
diff --git a/drivers/usb/virtio_usb/Makefile b/drivers/usb/virtio_usb/Makefile
index 2222222..b7ee9e8 100644
--- a/drivers/usb/virtio_usb/Makefile
+++ b/drivers/usb/virtio_usb/Makefile
@@ -1,8 +1,9 @@
 # SPDX-License-Identifier: GPL-2.0-or-later
 
 virtio-usb-y := controller.o \
 	vq_common.o \
 	host.o \
-	device.o
+	device.o \
+	otg.o
 
 obj-$(CONFIG_USB_VIRTIO) += virtio-usb.o
diff --git a/drivers/usb/virtio_usb/otg.c b/drivers/usb/virtio_usb/otg.c
new file mode 100644
index 0000000..557dfae
--- /dev/null
+++ b/drivers/usb/virtio_usb/otg.c
@@ -0,0 +1,168 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * virtio_usb: VirtIO USB device
+ *
+ * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
+ */
+
+#include <linux/mutex.h>
+#include "controller.h"
+#include "otg.h"
+#include "vq_common.h"
+
+int otg_init(struct virtio_usb *vusb)
+{
+	struct virtio_usb_otg *otg =
+		devm_kzalloc(&vusb->vdev->dev, sizeof(*otg), GFP_KERNEL);
+	unsigned int i;
+
+	if (!otg)
+		return -ENOMEM;
+
+	otg->vusb = vusb;
+	vusb->otg = otg;
+	for (i = 0; i < VIRTIO_USB_VQ_OTG_MAX; i++)
+		otg->oqs[i] = &vusb->vqueues[vusb->otg_vq_base + i];
+
+	mutex_init(&otg->lock);
+	init_completion(&otg->completion);
+
+	return 0;
+}
+
+void otg_deinit(struct virtio_usb *vusb)
+{
+	struct virtio_usb_otg *otg = vusb->otg;
+
+	if (!otg)
+		return;
+
+	/* Wake potential OTG command waiters before releasing OTG objects. */
+	complete_all(&otg->completion);
+
+	vusb->otg = NULL;
+}
+
+/* Send an OTG command and get a response.
+ *
+ * The function is implemented as synchronous. Design pattern is
+ * virtio_can.c/virtio_can_send_ctrl_msg()
+ */
+u32 otg_get_role(struct virtio_usb *vusb, int port_id, u32 *role)
+{
+	struct scatterlist sg_out, sg_in, *sgs[2] = { &sg_out, &sg_in };
+	struct virtqueue *vq =
+		vusb->otg->oqs[VIRTIO_USB_VQ_COMMAND_IDX]->vqueue;
+	unsigned int len;
+	u32 status = VIRTIO_USB_S_ERR_INTERNAL;
+
+	struct otg_get_role {
+		struct virtio_usb_otg_cmd_hdr cmd_hdr;
+		struct virtio_usb_otg_cmd_role cmd_role;
+	} *msg = kzalloc(sizeof(struct otg_get_role), GFP_KERNEL);
+
+	if (!msg)
+		return status;
+
+	msg->cmd_hdr.code = cpu_to_le32(VIRTIO_USB_CMD_OTG_GET_ROLE);
+	msg->cmd_hdr.port = cpu_to_le32(port_id);
+	sg_init_one(&sg_out, &msg->cmd_hdr, sizeof(msg->cmd_hdr));
+	sg_init_one(&sg_in, &msg->cmd_role, sizeof(msg->cmd_role));
+
+	mutex_lock(&vusb->otg->lock);
+
+	if (virtqueue_add_sgs(vq, sgs, 1u, 1u, msg, GFP_ATOMIC)) {
+		pr_err("%s virtqueue_add_sgs error\n", __func__);
+		goto exit;
+	}
+
+	if (!virtqueue_kick(vq)) {
+		pr_err("%s virtqueue_kick error\n", __func__);
+		goto exit;
+	}
+
+	while (!virtqueue_get_buf(vq, &len) && !virtqueue_is_broken(vq))
+		wait_for_completion(&vusb->otg->completion);
+
+	status = le32_to_cpu(msg->cmd_role.status.code);
+	*role = le32_to_cpu(msg->cmd_role.role);
+
+	if (*role != VIRTIO_USB_ROLE_HOST && *role != VIRTIO_USB_ROLE_DEVICE)
+		pr_err("%s - wrong role (%d)\n", __func__, *role);
+	else {
+		pr_info("%s otg_role %s\n", __func__,
+			*role == VIRTIO_USB_ROLE_HOST ?
+				"VIRTIO_USB_ROLE_HOST" :
+				"VIRTIO_USB_ROLE_DEVICE");
+	}
+
+exit:
+	kfree(msg);
+	mutex_unlock(&vusb->otg->lock);
+	return status;
+}
+
+static void virtio_usb_otg_cmd_notify_cb(struct virtqueue *vqueue)
+{
+	struct virtio_usb *vusb = vqueue->vdev->priv;
+
+	if (!vusb->otg)
+		return;
+
+	complete(&vusb->otg->completion);
+}
+
+static void virtio_usb_otg_cmdq_stop_cb(struct virtio_usb *vusb,
+					struct virtio_usb_queue *vq)
+{
+	unsigned long flags;
+
+	if (!vusb->otg || !vq->vqueue)
+		return;
+
+	/*
+	 * Wake sleepers in OTG synchronous command paths so they can
+	 * observe started=false and exit.
+	 */
+	complete_all(&vusb->otg->completion);
+
+	spin_lock_irqsave(&vq->lock, flags);
+	virtqueue_disable_cb(vq->vqueue);
+	spin_unlock_irqrestore(&vq->lock, flags);
+}
+
+static void virtio_usb_otg_evtq_stop_cb(struct virtio_usb *vusb,
+					struct virtio_usb_queue *vq)
+{
+	unsigned long flags;
+	u32 length;
+	void *buf;
+
+	if (!vq->vqueue)
+		return;
+
+	/* The OTG event queue is not populated yet at this stage (no
+	 * VIRTIO_USB_F_SWITCH_ROLE negotiation, no CHANGE_ROLE events),
+	 * so this only has to make sure del_vqs() finds the ring empty.
+	 */
+	spin_lock_irqsave(&vq->lock, flags);
+	virtqueue_disable_cb(vq->vqueue);
+	while ((buf = virtqueue_get_buf(vq->vqueue, &length)))
+		;
+	spin_unlock_irqrestore(&vq->lock, flags);
+}
+
+const struct virtio_usb_vq_desc otg_vqueues[VIRTIO_USB_VQ_OTG_MAX] = {
+	[VIRTIO_USB_VQ_COMMAND_IDX] = {
+		.callback = virtio_usb_otg_cmd_notify_cb,
+		.name = "virtusb-otg-cmd",
+		.process = NULL,
+		.stop = virtio_usb_otg_cmdq_stop_cb,
+	},
+	[VIRTIO_USB_VQ_EVENT_IDX] = {
+		.callback = NULL,
+		.name = "virtusb-otg-evt",
+		.process = NULL,
+		.stop = virtio_usb_otg_evtq_stop_cb,
+	},
+};
diff --git a/drivers/usb/virtio_usb/otg.h b/drivers/usb/virtio_usb/otg.h
new file mode 100644
index 0000000..a34317c
--- /dev/null
+++ b/drivers/usb/virtio_usb/otg.h
@@ -0,0 +1,26 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * virtio_usb: VirtIO USB device
+ *
+ * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
+ */
+
+#ifndef VIRTIO_USB_OTG_H
+#define VIRTIO_USB_OTG_H
+
+#include "controller.h"
+
+extern int otg_init(struct virtio_usb *vusb);
+extern void otg_deinit(struct virtio_usb *vusb);
+extern u32 otg_get_role(struct virtio_usb *vusb, int port_id, u32 *role);
+
+struct virtio_usb_otg {
+	struct virtio_usb *vusb;
+	struct mutex lock;
+	struct completion completion;
+	struct virtio_usb_queue *oqs[VIRTIO_USB_VQ_OTG_MAX];
+};
+
+extern const struct virtio_usb_vq_desc otg_vqueues[VIRTIO_USB_VQ_OTG_MAX];
+
+#endif /* VIRTIO_USB_OTG_H */

  parent reply	other threads:[~2026-09-24 16:09 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-24 16:08 [PATCH 0/8] virtio-usb: add dual-role virtio USB driver Igor Skalkin
2026-09-24 16:09 ` [PATCH 1/8] virtio-usb: add protocol header and skeleton dual-role driver Igor Skalkin
2026-09-25  5:14   ` Greg Kroah-Hartman
2026-09-28 14:19     ` Igor Skalkin
2026-09-25  5:21   ` Greg Kroah-Hartman
2026-09-28 14:14     ` Igor Skalkin
2026-09-24 16:09 ` [PATCH 2/8] virtio-usb: add host role (USB Host Controller) support Igor Skalkin
2026-09-25  5:18   ` Greg Kroah-Hartman
2026-09-28 14:01     ` Igor Skalkin
2026-09-24 16:09 ` [PATCH 3/8] virtio-usb: add device role (USB Device " Igor Skalkin
2026-09-24 16:09 ` Igor Skalkin [this message]
2026-09-24 16:09 ` [PATCH 5/8] virtio-usb: add USB On-The-Go role-switching support Igor Skalkin
2026-09-24 16:09 ` [PATCH 6/8] virtio-usb: rework endpoint lifecycle to an async split-phase state machine Igor Skalkin
2026-09-24 16:09 ` [PATCH 7/8] virtio-usb: add SuperSpeed device-role support Igor Skalkin
2026-09-24 16:09 ` [PATCH 8/8] virtio-usb: support a guest UDC name prefix from the bind event Igor Skalkin
2026-09-25  5:17 ` [PATCH 0/8] virtio-usb: add dual-role virtio USB driver Greg Kroah-Hartman
2026-09-28 13:55   ` Igor Skalkin
2026-09-28 14:44     ` Greg Kroah-Hartman
2026-09-28 15:56       ` Igor Skalkin
2026-09-28 16:10         ` Greg Kroah-Hartman
2026-09-29  9:47     ` Michael S. Tsirkin
2026-09-29 16:01       ` Greg Kroah-Hartman
2026-09-29 19:02         ` Vasilii Ianikeev
2026-09-29 19:58         ` Vasilii Ianikeev

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260924160907.145405-5-igor.skalkin@oss.qualcomm.com \
    --to=igor.skalkin@oss.qualcomm.com \
    --cc=aiswarya.cyriac@oss.qualcomm.com \
    --cc=anton.yakovlev@oss.qualcomm.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jasowangio@gmail.com \
    --cc=linux-usb@vger.kernel.org \
    --cc=mst@redhat.com \
    --cc=trilok.soni@oss.qualcomm.com \
    --cc=vasilii.ianikeev@oss.qualcomm.com \
    --cc=virtualization@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox