All of lore.kernel.org
 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: 32+ 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-24 16:23   ` sashiko-bot
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-24 16:25   ` sashiko-bot
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:28   ` sashiko-bot
2026-09-24 16:09 ` Igor Skalkin [this message]
2026-09-24 16:19   ` [PATCH 4/8] virtio-usb: add OTG role query support sashiko-bot
2026-09-24 16:09 ` [PATCH 5/8] virtio-usb: add USB On-The-Go role-switching support Igor Skalkin
2026-09-24 16:24   ` sashiko-bot
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:30   ` sashiko-bot
2026-09-24 16:09 ` [PATCH 7/8] virtio-usb: add SuperSpeed device-role support Igor Skalkin
2026-09-24 16:35   ` sashiko-bot
2026-09-24 16:09 ` [PATCH 8/8] virtio-usb: support a guest UDC name prefix from the bind event Igor Skalkin
2026-09-24 16:35   ` sashiko-bot
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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.