Re: [PATCH v24 03/34] xhci: sideband: add initial api to register a sideband entity

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]



On 8/1/2024 3:16 AM, Wesley Cheng wrote:
From: Mathias Nyman <mathias.nyman@xxxxxxxxxxxxxxx>

Introduce XHCI sideband, which manages the USB endpoints being requested by
a client driver.  This is used for when client drivers are attempting to
offload USB endpoints to another entity for handling USB transfers.  XHCI
sideband will allow for drivers to fetch the required information about the
transfer ring, so the user can submit transfers independently.  Expose the
required APIs for drivers to register and request for a USB endpoint and to
manage XHCI secondary interrupters.

Multiple ring segment page linking, proper endpoint clean up, and allowing
module compliation added by Wesley Cheng to complete original concept code
by Mathias Nyman.

Signed-off-by: Mathias Nyman <mathias.nyman@xxxxxxxxxxxxxxx>
Co-developed-by: Wesley Cheng <quic_wcheng@xxxxxxxxxxx>
Signed-off-by: Wesley Cheng <quic_wcheng@xxxxxxxxxxx>
---
  drivers/usb/host/Kconfig          |   9 +
  drivers/usb/host/Makefile         |   2 +
  drivers/usb/host/xhci-sideband.c  | 419 ++++++++++++++++++++++++++++++
  drivers/usb/host/xhci.h           |   4 +
  include/linux/usb/xhci-sideband.h |  68 +++++
  5 files changed, 502 insertions(+)
  create mode 100644 drivers/usb/host/xhci-sideband.c
  create mode 100644 include/linux/usb/xhci-sideband.h

diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index 4448d0ab06f0..6135603c5dc4 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -104,6 +104,15 @@ config USB_XHCI_RZV2M
  	  Say 'Y' to enable the support for the xHCI host controller
  	  found in Renesas RZ/V2M SoC.
+config USB_XHCI_SIDEBAND
+	tristate "xHCI support for sideband"
+	help
+	  Say 'Y' to enable the support for the xHCI sideband capability.
+	  provide a mechanism for a sideband datapath for payload associated

Sentence should start from capital letter, so provide -> Provide

+	  with audio class endpoints. This allows for an audio DSP to use
+	  xHCI USB endpoints directly, allowing CPU to sleep while playing
+	  audio

Missing '.' at the end of sentence.

(...)

+/**
+ * xhci_sideband_remove_endpoint - remove endpoint from sideband access list
+ * @sb: sideband instance for this usb device
+ * @host_ep: usb host endpoint
+ *
+ * Removes an endpoint from the list of sideband accessed endpoints for this usb
+ * device.
+ * sideband client should no longer touch the endpoint transfer buffer after
+ * calling this.
+ *
+ * Return: 0 on success, negative error otherwise.
+ */
+int
+xhci_sideband_remove_endpoint(struct xhci_sideband *sb,
+			      struct usb_host_endpoint *host_ep)
+{
+	struct xhci_virt_ep *ep;
+	unsigned int ep_index;
+
+	mutex_lock(&sb->mutex);
+	ep_index = xhci_get_endpoint_index(&host_ep->desc);
+	ep = sb->eps[ep_index];
+
+	if (!ep || !ep->sideband) {
+		mutex_unlock(&sb->mutex);
+		return -ENODEV;
+	}
+
+	__xhci_sideband_remove_endpoint(sb, ep);
+	xhci_initialize_ring_info(ep->ring, 1);
+	mutex_unlock(&sb->mutex);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(xhci_sideband_remove_endpoint);
+
+int
+xhci_sideband_stop_endpoint(struct xhci_sideband *sb,
+			    struct usb_host_endpoint *host_ep)
+{
+	struct xhci_virt_ep *ep;
+	unsigned int ep_index;
+
+	ep_index = xhci_get_endpoint_index(&host_ep->desc);
+	ep = sb->eps[ep_index];
+
+	if (!ep || ep->sideband != sb)

Any reason why we check if ep->sideband != sb only on stop but not on remove above?

+		return -EINVAL;
+
+	return xhci_stop_endpoint_sync(sb->xhci, ep, 0, GFP_KERNEL);
+}
+EXPORT_SYMBOL_GPL(xhci_sideband_stop_endpoint);
+
+/**
+ * xhci_sideband_get_endpoint_buffer - gets the endpoint transfer buffer address
+ * @sb: sideband instance for this usb device
+ * @host_ep: usb host endpoint
+ *
+ * Returns the address of the endpoint buffer where xHC controller reads queued
+ * transfer TRBs from. This is the starting address of the ringbuffer where the
+ * sideband client should write TRBs to.
+ *
+ * Caller needs to free the returned sg_table
+ *
+ * Return: struct sg_table * if successful. NULL otherwise.
+ */
+struct sg_table *
+xhci_sideband_get_endpoint_buffer(struct xhci_sideband *sb,
+			      struct usb_host_endpoint *host_ep)
+{
+	struct xhci_virt_ep *ep;
+	unsigned int ep_index;
+
+	ep_index = xhci_get_endpoint_index(&host_ep->desc);
+	ep = sb->eps[ep_index];
+
+	if (!ep)

And here there is none of checks done in above 2 functions? Seems bit weird.

+		return NULL;
+
+	return xhci_ring_to_sgtable(sb, ep->ring);
+}
+EXPORT_SYMBOL_GPL(xhci_sideband_get_endpoint_buffer);
+

(...)

+MODULE_DESCRIPTION("XHCI sideband driver for secondary interrupter management");

XHCI -> xHCI

+MODULE_LICENSE("GPL");
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index efbd1f651da4..9232c53d204a 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -693,6 +693,8 @@ struct xhci_virt_ep {
  	int			next_frame_id;
  	/* Use new Isoch TRB layout needed for extended TBC support */
  	bool			use_extended_tbc;
+	/* set if this endpoint is controlled via sideband access*/
+	struct xhci_sideband			*sideband;
  };
enum xhci_overhead_type {
@@ -755,6 +757,8 @@ struct xhci_virt_device {
  	u16				current_mel;
  	/* Used for the debugfs interfaces. */
  	void				*debugfs_private;
+	/* set if this device is registered for sideband access */
+	struct xhci_sideband			*sideband;
  };
/*
diff --git a/include/linux/usb/xhci-sideband.h b/include/linux/usb/xhci-sideband.h
new file mode 100644
index 000000000000..1035dae43cee
--- /dev/null
+++ b/include/linux/usb/xhci-sideband.h
@@ -0,0 +1,68 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * xHCI host controller sideband support
+ *
+ * Copyright (c) 2023, Intel Corporation.
+ *
+ * Author: Mathias Nyman <mathias.nyman@xxxxxxxxxxxxxxx>
+ */
+
+#ifndef __LINUX_XHCI_SIDEBAND_H
+#define __LINUX_XHCI_SIDEBAND_H
+
+#include <linux/scatterlist.h>
+#include <linux/usb.h>
+
+#define	EP_CTX_PER_DEV		31	/* FIMXME defined twice, from xhci.h */

If it is left for later, FIMXME -> FIXME

(...)




[Index of Archives]     [Pulseaudio]     [Linux Audio Users]     [ALSA Devel]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]

  Powered by Linux