From: Mathias Nyman <mathias.nyman@linux.intel.com>
Introduce XHCI sec intr, 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
sec intr 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.
Driver renaming, multiple ring segment page linking, proper endpoint clean
up, and allowing module compilation added by Wesley Cheng to complete
original concept code by Mathias Nyman.
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Co-developed-by: Wesley Cheng <quic_wcheng@quicinc.com>
Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com>
---
drivers/usb/host/Kconfig | 11 +
drivers/usb/host/Makefile | 2 +
drivers/usb/host/xhci-sec-intr.c | 438 ++++++++++++++++++++++++++++++
drivers/usb/host/xhci.h | 4 +
include/linux/usb/xhci-sec-intr.h | 70 +++++
5 files changed, 525 insertions(+)
create mode 100644 drivers/usb/host/xhci-sec-intr.c
create mode 100644 include/linux/usb/xhci-sec-intr.h
diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index d011d6c753ed..a2d549e3e076 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -104,6 +104,17 @@ config USB_XHCI_RZV2M
Say 'Y' to enable the support for the xHCI host controller
found in Renesas RZ/V2M SoC.
+config USB_XHCI_SEC_INTR
+ tristate "xHCI support for secondary interrupter management"
+ help
+ Say 'Y' to enable the support for the xHCI secondary management.
+ Provide a mechanism for a sideband datapath for payload associated
+ with audio class endpoints. This allows for an audio DSP to use
+ xHCI USB endpoints directly, allowing CPU to sleep while playing
+ audio. This is not the same feature as the audio sideband
+ capability mentioned within the xHCI specification, and continues
+ to utilize main system memory for data transfers.
+
config USB_XHCI_TEGRA
tristate "xHCI support for NVIDIA Tegra SoCs"
depends on PHY_TEGRA_XUSB
diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
index be4e5245c52f..d4b127f48cf9 100644
--- a/drivers/usb/host/Makefile
+++ b/drivers/usb/host/Makefile
@@ -32,6 +32,8 @@ endif
xhci-rcar-hcd-y += xhci-rcar.o
xhci-rcar-hcd-$(CONFIG_USB_XHCI_RZV2M) += xhci-rzv2m.o
+obj-$(CONFIG_USB_XHCI_SEC_INTR) += xhci-sec-intr.o
+
obj-$(CONFIG_USB_PCI) += pci-quirks.o
obj-$(CONFIG_USB_EHCI_HCD) += ehci-hcd.o
diff --git a/drivers/usb/host/xhci-sec-intr.c b/drivers/usb/host/xhci-sec-intr.c
new file mode 100644
index 000000000000..b112c3388368
--- /dev/null
+++ b/drivers/usb/host/xhci-sec-intr.c
@@ -0,0 +1,438 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * xHCI host controller secondary interrupter management
+ *
+ * Provides logic for client drivers that support utilizing xHCI secondary
+ * interrupters.
+ *
+ * Copyright (c) 2023-2024, Intel Corporation.
+ *
+ * Author: Mathias Nyman
+ */
+
+#include <linux/usb/xhci-sec-intr.h>
+#include <linux/dma-direct.h>
+
+#include "xhci.h"
+
+/* internal helpers */
+static struct sg_table *
+xhci_ring_to_sgtable(struct xhci_sec_intr *si, struct xhci_ring *ring)
+{
+ struct xhci_segment *seg;
+ struct sg_table *sgt;
+ unsigned int n_pages;
+ struct page **pages;
+ struct device *dev;
+ size_t sz;
+ int i;
+
+ dev = xhci_to_hcd(si->xhci)->self.sysdev;
+ sz = ring->num_segs * TRB_SEGMENT_SIZE;
+ n_pages = PAGE_ALIGN(sz) >> PAGE_SHIFT;
+ pages = kvmalloc_array(n_pages, sizeof(struct page *), GFP_KERNEL);
+ if (!pages)
+ return NULL;
+
+ sgt = kzalloc(sizeof(*sgt), GFP_KERNEL);
+ if (!sgt) {
+ kvfree(pages);
+ return NULL;
+ }
+
+ seg = ring->first_seg;
+ if (!seg)
+ goto err;
+ /*
+ * Rings can potentially have multiple segments, create an array that
+ * carries page references to allocated segments. Utilize the
+ * sg_alloc_table_from_pages() to create the sg table, and to ensure
+ * that page links are created.
+ */
+ for (i = 0; i < ring->num_segs; i++) {
+ dma_get_sgtable(dev, sgt, seg->trbs, seg->dma,
+ TRB_SEGMENT_SIZE);
+ pages[i] = sg_page(sgt->sgl);
+ sg_free_table(sgt);
+ seg = seg->next;
+ }
+
+ if (sg_alloc_table_from_pages(sgt, pages, n_pages, 0, sz, GFP_KERNEL))
+ goto err;
+
+ /*
+ * Save first segment dma address to sg dma_address field for the sideband
+ * client to have access to the IOVA of the ring.
+ */
+ sg_dma_address(sgt->sgl) = ring->first_seg->dma;
+
+ return sgt;
+
+err:
+ kvfree(pages);
+ kfree(sgt);
+
+ return NULL;
+}
+
+static void
+__xhci_sec_intr_remove_endpoint(struct xhci_sec_intr *si, struct xhci_virt_ep *ep)
+{
+ /*
+ * Issue a stop endpoint command when an endpoint is removed.
+ * The stop ep cmd handler will handle the ring cleanup.
+ */
+ xhci_stop_endpoint_sync(si->xhci, ep, 0, GFP_KERNEL);
+
+ ep->sec = NULL;
+ si->eps[ep->ep_index] = NULL;
+}
+
+/* endpoint api functions */
+
+/**
+ * xhci_sec_intr_add_endpoint - add endpoint to access list
+ * @si: secondary interrupter instance for this usb device
+ * @host_ep: usb host endpoint
+ *
+ * Adds an endpoint to the list of endpoints utilizing secondary interrupters
+ * for this usb device.
+ * After an endpoint is added the client can get the endpoint transfer ring
+ * buffer by calling xhci_sec_intr_get_endpoint_buffer()
+ *
+ * Return: 0 on success, negative error otherwise.
+ */
+int
+xhci_sec_intr_add_endpoint(struct xhci_sec_intr *si,
+ struct usb_host_endpoint *host_ep)
+{
+ struct xhci_virt_ep *ep;
+ unsigned int ep_index;
+
+ mutex_lock(&si->mutex);
+ ep_index = xhci_get_endpoint_index(&host_ep->desc);
+ ep = &si->vdev->eps[ep_index];
+
+ if (ep->ep_state & EP_HAS_STREAMS) {
+ mutex_unlock(&si->mutex);
+ return -EINVAL;
+ }
+
+ /*
+ * Note, we don't know the DMA mask of the audio DSP device, if its
+ * smaller than for xhci it won't be able to access the endpoint ring
+ * buffer. This could be solved by not allowing the audio class driver
+ * to add the endpoint the normal way, but instead offload it immediately,
+ * and let this function add the endpoint and allocate the ring buffer
+ * with the smallest common DMA mask
+ */
+ if (si->eps[ep_index] || ep->sec) {
+ mutex_unlock(&si->mutex);
+ return -EBUSY;
+ }
+
+ ep->sec = si;
+ si->eps[ep_index] = ep;
+ mutex_unlock(&si->mutex);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(xhci_sec_intr_add_endpoint);
+
+/**
+ * xhci_sec_intr_remove_endpoint - remove endpoint from access list
+ * @si: secondary interrupter instance for this usb device
+ * @host_ep: usb host endpoint
+ *
+ * Removes an endpoint from the list of endpoints utilizing secondary
+ * interrupters for this usb device.
+ * Clients should no longer touch the endpoint transfer buffer after
+ * calling this.
+ *
+ * Return: 0 on success, negative error otherwise.
+ */
+int
+xhci_sec_intr_remove_endpoint(struct xhci_sec_intr *si,
+ struct usb_host_endpoint *host_ep)
+{
+ struct xhci_virt_ep *ep;
+ unsigned int ep_index;
+
+ mutex_lock(&si->mutex);
+ ep_index = xhci_get_endpoint_index(&host_ep->desc);
+ ep = si->eps[ep_index];
+
+ if (!ep || !ep->sec || ep->sec != si) {
+ mutex_unlock(&si->mutex);
+ return -ENODEV;
+ }
+
+ __xhci_sec_intr_remove_endpoint(si, ep);
+ xhci_initialize_ring_info(ep->ring, 1);
+ mutex_unlock(&si->mutex);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(xhci_sec_intr_remove_endpoint);
+
+/**
+ * xhci_sec_intr_stop_endpoint - stops a client managed endpoint
+ * @si: secondary interrupter instance for this usb device
+ * @host_ep: usb host endpoint
+ *
+ * Issue a synchronous stop endpoint command to halt any data transfers
+ * occurring. This also is required before the xHCI controller enters
+ * suspend.
+ *
+ * Return: 0 on success, and negative on failure.
+ */
+int
+xhci_sec_intr_stop_endpoint(struct xhci_sec_intr *si,
+ 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 = si->eps[ep_index];
+
+ if (!ep || !ep->sec || ep->sec != si)
+ return -EINVAL;
+
+ return xhci_stop_endpoint_sync(si->xhci, ep, 0, GFP_KERNEL);
+}
+EXPORT_SYMBOL_GPL(xhci_sec_intr_stop_endpoint);
+
+/**
+ * xhci_sec_intr_get_endpoint_buffer - gets the endpoint transfer buffer address
+ * @si: secondary interrupter 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
+ * 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_sec_intr_get_endpoint_buffer(struct xhci_sec_intr *si,
+ 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 = si->eps[ep_index];
+
+ if (!ep || !ep->sec || ep->sec != si)
+ return NULL;
+
+ return xhci_ring_to_sgtable(si, ep->ring);
+}
+EXPORT_SYMBOL_GPL(xhci_sec_intr_get_endpoint_buffer);
+
+/**
+ * xhci_sec_intr_get_event_buffer - return the event buffer for this device
+ * @si: secondary interrupter instance for this usb device
+ *
+ * If a secondary xhci interupter is set up for this usb device then this
+ * function returns the address of the event buffer where xHC writes
+ * the transfer completion events.
+ *
+ * Caller needs to free the returned sg_table
+ *
+ * Return: struct sg_table * if successful. NULL otherwise.
+ */
+struct sg_table *
+xhci_sec_intr_get_event_buffer(struct xhci_sec_intr *si)
+{
+ if (!si || !si->ir)
+ return NULL;
+
+ return xhci_ring_to_sgtable(si, si->ir->event_ring);
+}
+EXPORT_SYMBOL_GPL(xhci_sec_intr_get_event_buffer);
+
+/**
+ * xhci_sec_intr_create_interrupter - creates a new interrupter
+ * @si: secondary interrupter instance for this usb device
+ * @num_seg: number of event ring segments to allocate
+ * @ip_autoclear: IP autoclearing support such as MSI implemented
+ *
+ * Sets up a xhci interrupter that can be used for this sideband accessed usb
+ * device. Transfer events for this device can be routed to this interrupters
+ * event ring by setting the 'Interrupter Target' field correctly when queueing
+ * the transfer TRBs.
+ * Once this interrupter is created the interrupter target ID can be obtained
+ * by calling xhci_sec_intr_interrupter_id()
+ *
+ * Returns 0 on success, negative error otherwise
+ */
+int
+xhci_sec_intr_create_interrupter(struct xhci_sec_intr *si, int num_seg,
+ bool ip_autoclear, u32 imod_interval)
+{
+ int ret = 0;
+
+ if (!si || !si->xhci)
+ return -ENODEV;
+
+ mutex_lock(&si->mutex);
+ if (si->ir) {
+ ret = -EBUSY;
+ goto out;
+ }
+
+ si->ir = xhci_create_secondary_interrupter(xhci_to_hcd(si->xhci),
+ num_seg, imod_interval);
+ if (!si->ir) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ si->ir->ip_autoclear = ip_autoclear;
+ /* skip events for secondary interrupters by default */
+ si->ir->skip_events = true;
+
+out:
+ mutex_unlock(&si->mutex);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(xhci_sec_intr_create_interrupter);
+
+/**
+ * xhci_sec_intr_remove_interrupter - remove the secondary interrupter for a device
+ * @si: secondary interrupter instance for this usb device
+ *
+ * Removes a registered interrupter for a usb device. This would allow for other
+ * users to utilize this interrupter.
+ */
+void
+xhci_sec_intr_remove_interrupter(struct xhci_sec_intr *si)
+{
+ if (!si || !si->ir)
+ return;
+
+ mutex_lock(&si->mutex);
+ xhci_remove_secondary_interrupter(xhci_to_hcd(si->xhci), si->ir);
+
+ si->ir = NULL;
+ mutex_unlock(&si->mutex);
+}
+EXPORT_SYMBOL_GPL(xhci_sec_intr_remove_interrupter);
+
+/**
+ * xhci_sec_intr_interrupter_id - return the interrupter target id
+ * @si: secondary interrupter instance for this usb device
+ *
+ * If a secondary xhci interrupter is set up for this usb device then this
+ * function returns the ID used by the interrupter. The client needs
+ * to write this ID to the 'Interrupter Target' field of the transfer TRBs
+ * it queues on the endpoints transfer ring to ensure transfer completion event
+ * are written by xHC to the correct interrupter event ring.
+ *
+ * Returns interrupter id on success, negative error otherwise
+ */
+int
+xhci_sec_intr_interrupter_id(struct xhci_sec_intr *si)
+{
+ if (!si || !si->ir)
+ return -ENODEV;
+
+ return si->ir->intr_num;
+}
+EXPORT_SYMBOL_GPL(xhci_sec_intr_interrupter_id);
+
+/**
+ * xhci_sec_intr_register - register a secondary interrupter for a usb device
+ * @udev: usb device to be accessed via sideband
+ *
+ * Allows for clients to utilize XHCI interrupters and fetch transfer and event
+ * ring parameters for executing data transfers.
+ *
+ * Return: pointer to a new xhci_sec_intr instance if successful. NULL otherwise.
+ */
+struct xhci_sec_intr *
+xhci_sec_intr_register(struct usb_device *udev)
+{
+ struct usb_hcd *hcd = bus_to_hcd(udev->bus);
+ struct xhci_hcd *xhci = hcd_to_xhci(hcd);
+ struct xhci_virt_device *vdev;
+ struct xhci_sec_intr *si;
+
+ /* make sure the usb device is connected to a xhci controller */
+ if (!udev->slot_id)
+ return NULL;
+
+ si = kzalloc_node(sizeof(*si), GFP_KERNEL, dev_to_node(hcd->self.sysdev));
+ if (!si)
+ return NULL;
+
+ mutex_init(&si->mutex);
+
+ /* check this device isn't already controlled via sideband */
+ spin_lock_irq(&xhci->lock);
+
+ vdev = xhci->devs[udev->slot_id];
+
+ if (!vdev || vdev->sec) {
+ xhci_warn(xhci, "XHCI sideband for slot %d already in use\n",
+ udev->slot_id);
+ spin_unlock_irq(&xhci->lock);
+ kfree(si);
+ return NULL;
+ }
+
+ si->xhci = xhci;
+ si->vdev = vdev;
+ vdev->sec = si;
+
+ spin_unlock_irq(&xhci->lock);
+
+ return si;
+}
+EXPORT_SYMBOL_GPL(xhci_sec_intr_register);
+
+/**
+ * xhci_sec_intr_unregister - unregister secondary interrupter for a usb device
+ * @si: secondary interrupter instance to be unregistered
+ *
+ * Unregisters sideband access to a usb device and frees the secondary interrupter
+ * instance.
+ * After this the endpoint and interrupter event buffers should no longer
+ * be accessed via sideband. The xhci driver can now take over handling
+ * the buffers.
+ */
+void
+xhci_sec_intr_unregister(struct xhci_sec_intr *si)
+{
+ struct xhci_hcd *xhci;
+ int i;
+
+ if (!si)
+ return;
+
+ xhci = si->xhci;
+
+ mutex_lock(&si->mutex);
+ for (i = 0; i < EP_CTX_PER_DEV; i++)
+ if (si->eps[i])
+ __xhci_sec_intr_remove_endpoint(si, si->eps[i]);
+ mutex_unlock(&si->mutex);
+
+ xhci_sec_intr_remove_interrupter(si);
+
+ spin_lock_irq(&xhci->lock);
+ si->xhci = NULL;
+ si->vdev->sec = NULL;
+ spin_unlock_irq(&xhci->lock);
+
+ kfree(si);
+}
+EXPORT_SYMBOL_GPL(xhci_sec_intr_unregister);
+MODULE_DESCRIPTION("xHCI sideband driver for secondary interrupter management");
+MODULE_LICENSE("GPL");
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 92cde885acd3..4a6ca048a14c 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -694,6 +694,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_sec_intr *sec;
};
enum xhci_overhead_type {
@@ -756,6 +758,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_sec_intr *sec;
};
/*
diff --git a/include/linux/usb/xhci-sec-intr.h b/include/linux/usb/xhci-sec-intr.h
new file mode 100644
index 000000000000..f6edb27b1d99
--- /dev/null
+++ b/include/linux/usb/xhci-sec-intr.h
@@ -0,0 +1,70 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * xHCI host controller sideband support
+ *
+ * Copyright (c) 2023, Intel Corporation.
+ *
+ * Author: Mathias Nyman <mathias.nyman@linux.intel.com>
+ */
+
+#ifndef __LINUX_XHCI_SEC_INTR_H
+#define __LINUX_XHCI_SEC_INTR_H
+
+#include <linux/scatterlist.h>
+#include <linux/usb.h>
+
+#define EP_CTX_PER_DEV 31 /* FIXME defined twice, from xhci.h */
+
+struct xhci_sec_intr;
+
+/**
+ * struct xhci_sec_intr - representation of a sideband accessed usb device.
+ * @xhci: The xhci host controller the usb device is connected to
+ * @vdev: the usb device accessed via sideband
+ * @eps: array of endpoints controlled via sideband
+ * @ir: event handling and buffer for sideband accessed device
+ * @mutex: mutex for sideband operations
+ *
+ * FIXME usb device accessed via sideband Keeping track of sideband accessed usb devices.
+ */
+
+struct xhci_sec_intr {
+ struct xhci_hcd *xhci;
+ struct xhci_virt_device *vdev;
+ struct xhci_virt_ep *eps[EP_CTX_PER_DEV];
+ struct xhci_interrupter *ir;
+
+ /* Synchronizing xHCI sec intr operations with client drivers operations */
+ struct mutex mutex;
+};
+
+struct xhci_sec_intr *
+xhci_sec_intr_register(struct usb_device *udev);
+void
+xhci_sec_intr_unregister(struct xhci_sec_intr *si);
+int
+xhci_sec_intr_add_endpoint(struct xhci_sec_intr *si,
+ struct usb_host_endpoint *host_ep);
+int
+xhci_sec_intr_remove_endpoint(struct xhci_sec_intr *si,
+ struct usb_host_endpoint *host_ep);
+int
+xhci_sec_intr_stop_endpoint(struct xhci_sec_intr *si,
+ struct usb_host_endpoint *host_ep);
+struct sg_table *
+xhci_sec_intr_get_endpoint_buffer(struct xhci_sec_intr *si,
+ struct usb_host_endpoint *host_ep);
+struct sg_table *
+xhci_sec_intr_get_event_buffer(struct xhci_sec_intr *si);
+
+int
+xhci_sec_intr_create_interrupter(struct xhci_sec_intr *si, int num_seg,
+ bool ip_autoclear, u32 imod_interval);
+
+void
+xhci_sec_intr_remove_interrupter(struct xhci_sec_intr *si);
+
+int
+xhci_sec_intr_interrupter_id(struct xhci_sec_intr *si);
+
+#endif /* __LINUX_XHCI_SIDEBAND_H */
On 6.11.2024 21.33, Wesley Cheng wrote:
> From: Mathias Nyman <mathias.nyman@linux.intel.com>
>
> Introduce XHCI sec intr, 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
> sec intr 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.
>
> Driver renaming, multiple ring segment page linking, proper endpoint clean
> up, and allowing module compilation added by Wesley Cheng to complete
> original concept code by Mathias Nyman.
>
> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
> Co-developed-by: Wesley Cheng <quic_wcheng@quicinc.com>
> Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com>
> ---
> drivers/usb/host/Kconfig | 11 +
> drivers/usb/host/Makefile | 2 +
> drivers/usb/host/xhci-sec-intr.c | 438 ++++++++++++++++++++++++++++++
> drivers/usb/host/xhci.h | 4 +
> include/linux/usb/xhci-sec-intr.h | 70 +++++
> 5 files changed, 525 insertions(+)
> create mode 100644 drivers/usb/host/xhci-sec-intr.c
> create mode 100644 include/linux/usb/xhci-sec-intr.h
>
> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
> index d011d6c753ed..a2d549e3e076 100644
> --- a/drivers/usb/host/Kconfig
> +++ b/drivers/usb/host/Kconfig
> @@ -104,6 +104,17 @@ config USB_XHCI_RZV2M
> Say 'Y' to enable the support for the xHCI host controller
> found in Renesas RZ/V2M SoC.
>
> +config USB_XHCI_SEC_INTR
> + tristate "xHCI support for secondary interrupter management"
> + help
> + Say 'Y' to enable the support for the xHCI secondary management.
> + Provide a mechanism for a sideband datapath for payload associated
> + with audio class endpoints. This allows for an audio DSP to use
> + xHCI USB endpoints directly, allowing CPU to sleep while playing
> + audio. This is not the same feature as the audio sideband
> + capability mentioned within the xHCI specification, and continues
> + to utilize main system memory for data transfers.
This same API should be used for the hardware xHCI sideband capability.
We should add a function that checks which types of xHC sideband capability xHC
hardware can support, and pick and pass a type to xhci xhci_sec_intr_register()
when registering a sideband/sec_intr
> +
> config USB_XHCI_TEGRA
> tristate "xHCI support for NVIDIA Tegra SoCs"
> depends on PHY_TEGRA_XUSB
> diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
> index be4e5245c52f..d4b127f48cf9 100644
> --- a/drivers/usb/host/Makefile
> +++ b/drivers/usb/host/Makefile
> @@ -32,6 +32,8 @@ endif
> xhci-rcar-hcd-y += xhci-rcar.o
> xhci-rcar-hcd-$(CONFIG_USB_XHCI_RZV2M) += xhci-rzv2m.o
>
> +obj-$(CONFIG_USB_XHCI_SEC_INTR) += xhci-sec-intr.o
> +
> obj-$(CONFIG_USB_PCI) += pci-quirks.o
>
> obj-$(CONFIG_USB_EHCI_HCD) += ehci-hcd.o
> diff --git a/drivers/usb/host/xhci-sec-intr.c b/drivers/usb/host/xhci-sec-intr.c
> new file mode 100644
> index 000000000000..b112c3388368
> --- /dev/null
> +++ b/drivers/usb/host/xhci-sec-intr.c
> @@ -0,0 +1,438 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/*
> + * xHCI host controller secondary interrupter management
> + *
> + * Provides logic for client drivers that support utilizing xHCI secondary
> + * interrupters.
> + *
> + * Copyright (c) 2023-2024, Intel Corporation.
> + *
> + * Author: Mathias Nyman
> + */
> +
> +#include <linux/usb/xhci-sec-intr.h>
> +#include <linux/dma-direct.h>
> +
> +#include "xhci.h"
> +
> +/* internal helpers */
> +static struct sg_table *
> +xhci_ring_to_sgtable(struct xhci_sec_intr *si, struct xhci_ring *ring)
> +{
> + struct xhci_segment *seg;
> + struct sg_table *sgt;
> + unsigned int n_pages;
> + struct page **pages;
> + struct device *dev;
> + size_t sz;
> + int i;
> +
> + dev = xhci_to_hcd(si->xhci)->self.sysdev;
> + sz = ring->num_segs * TRB_SEGMENT_SIZE;
> + n_pages = PAGE_ALIGN(sz) >> PAGE_SHIFT;
> + pages = kvmalloc_array(n_pages, sizeof(struct page *), GFP_KERNEL);
> + if (!pages)
> + return NULL;
> +
> + sgt = kzalloc(sizeof(*sgt), GFP_KERNEL);
> + if (!sgt) {
> + kvfree(pages);
> + return NULL;
> + }
> +
> + seg = ring->first_seg;
> + if (!seg)
> + goto err;
> + /*
> + * Rings can potentially have multiple segments, create an array that
> + * carries page references to allocated segments. Utilize the
> + * sg_alloc_table_from_pages() to create the sg table, and to ensure
> + * that page links are created.
> + */
> + for (i = 0; i < ring->num_segs; i++) {
> + dma_get_sgtable(dev, sgt, seg->trbs, seg->dma,
> + TRB_SEGMENT_SIZE);
> + pages[i] = sg_page(sgt->sgl);
> + sg_free_table(sgt);
> + seg = seg->next;
> + }
> +
> + if (sg_alloc_table_from_pages(sgt, pages, n_pages, 0, sz, GFP_KERNEL))
> + goto err;
> +
> + /*
> + * Save first segment dma address to sg dma_address field for the sideband
> + * client to have access to the IOVA of the ring.
> + */
> + sg_dma_address(sgt->sgl) = ring->first_seg->dma;
> +
> + return sgt;
> +
> +err:
> + kvfree(pages);
> + kfree(sgt);
> +
> + return NULL;
> +}
> +
> +static void
> +__xhci_sec_intr_remove_endpoint(struct xhci_sec_intr *si, struct xhci_virt_ep *ep)
> +{
> + /*
> + * Issue a stop endpoint command when an endpoint is removed.
> + * The stop ep cmd handler will handle the ring cleanup.
> + */
> + xhci_stop_endpoint_sync(si->xhci, ep, 0, GFP_KERNEL);
> +
> + ep->sec = NULL;
> + si->eps[ep->ep_index] = NULL;
> +}
> +
> +/* endpoint api functions */
> +
> +/**
> + * xhci_sec_intr_add_endpoint - add endpoint to access list
> + * @si: secondary interrupter instance for this usb device
> + * @host_ep: usb host endpoint
> + *
> + * Adds an endpoint to the list of endpoints utilizing secondary interrupters
> + * for this usb device.
> + * After an endpoint is added the client can get the endpoint transfer ring
> + * buffer by calling xhci_sec_intr_get_endpoint_buffer()
> + *
> + * Return: 0 on success, negative error otherwise.
> + */
> +int
> +xhci_sec_intr_add_endpoint(struct xhci_sec_intr *si,
> + struct usb_host_endpoint *host_ep)
> +{
> + struct xhci_virt_ep *ep;
> + unsigned int ep_index;
> +
> + mutex_lock(&si->mutex);
> + ep_index = xhci_get_endpoint_index(&host_ep->desc);
> + ep = &si->vdev->eps[ep_index];
> +
> + if (ep->ep_state & EP_HAS_STREAMS) {
> + mutex_unlock(&si->mutex);
> + return -EINVAL;
> + }
> +
> + /*
> + * Note, we don't know the DMA mask of the audio DSP device, if its
> + * smaller than for xhci it won't be able to access the endpoint ring
> + * buffer. This could be solved by not allowing the audio class driver
> + * to add the endpoint the normal way, but instead offload it immediately,
> + * and let this function add the endpoint and allocate the ring buffer
> + * with the smallest common DMA mask
> + */
> + if (si->eps[ep_index] || ep->sec) {
> + mutex_unlock(&si->mutex);
> + return -EBUSY;
> + }
> +
> + ep->sec = si;
> + si->eps[ep_index] = ep;
> + mutex_unlock(&si->mutex);
We should probably check in xhci-mem.c if ep->sec is set before freeing the
endpoint ring.
We don't want the sideband client driver to touch freed rings.
Maybe we even need a way for xhci driver to notify this sideband/sec_intr client
in case a offloaded device or endpoint is being freed.
I guess usb core in most cases ensures class drivers are properly removed,
and thus this sideband/sec_interrupt should be unregistered before xhci starts
freeing endpoints, but I'm not sure sure this is true in all corner cases.
This is the first time we share endpoint ring addresses.
Thanks
Mathias
Hi Mathias,
On 11/20/2024 6:36 AM, Mathias Nyman wrote:
> On 6.11.2024 21.33, Wesley Cheng wrote:
>> From: Mathias Nyman <mathias.nyman@linux.intel.com>
>>
>> Introduce XHCI sec intr, 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
>> sec intr 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.
>>
>> Driver renaming, multiple ring segment page linking, proper endpoint clean
>> up, and allowing module compilation added by Wesley Cheng to complete
>> original concept code by Mathias Nyman.
>>
>> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
>> Co-developed-by: Wesley Cheng <quic_wcheng@quicinc.com>
>> Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com>
>> ---
>> drivers/usb/host/Kconfig | 11 +
>> drivers/usb/host/Makefile | 2 +
>> drivers/usb/host/xhci-sec-intr.c | 438 ++++++++++++++++++++++++++++++
>> drivers/usb/host/xhci.h | 4 +
>> include/linux/usb/xhci-sec-intr.h | 70 +++++
>> 5 files changed, 525 insertions(+)
>> create mode 100644 drivers/usb/host/xhci-sec-intr.c
>> create mode 100644 include/linux/usb/xhci-sec-intr.h
>>
>> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
>> index d011d6c753ed..a2d549e3e076 100644
>> --- a/drivers/usb/host/Kconfig
>> +++ b/drivers/usb/host/Kconfig
>> @@ -104,6 +104,17 @@ config USB_XHCI_RZV2M
>> Say 'Y' to enable the support for the xHCI host controller
>> found in Renesas RZ/V2M SoC.
>> +config USB_XHCI_SEC_INTR
>> + tristate "xHCI support for secondary interrupter management"
>> + help
>> + Say 'Y' to enable the support for the xHCI secondary management.
>> + Provide a mechanism for a sideband datapath for payload associated
>> + with audio class endpoints. This allows for an audio DSP to use
>> + xHCI USB endpoints directly, allowing CPU to sleep while playing
>> + audio. This is not the same feature as the audio sideband
>> + capability mentioned within the xHCI specification, and continues
>> + to utilize main system memory for data transfers.
>
> This same API should be used for the hardware xHCI sideband capability.
> We should add a function that checks which types of xHC sideband capability xHC
> hardware can support, and pick and pass a type to xhci xhci_sec_intr_register()
> when registering a sideband/sec_intr
Just to make sure we're on the same page, when you mention the term sideband capability, are you referring to section 7.9 xHCI Audio Sideband Capability in the xHCI spec? If so, I'm not entirely sure if that capability relies much on secondary interrupters. From reading the material, it just seems like its a way to map audio endpoints directly to another USB device connected to the controller? (I might be wrong, couldn't find much about potential use cases)
>
>> +
>> config USB_XHCI_TEGRA
>> tristate "xHCI support for NVIDIA Tegra SoCs"
>> depends on PHY_TEGRA_XUSB
>> diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
>> index be4e5245c52f..d4b127f48cf9 100644
>> --- a/drivers/usb/host/Makefile
>> +++ b/drivers/usb/host/Makefile
>> @@ -32,6 +32,8 @@ endif
>> xhci-rcar-hcd-y += xhci-rcar.o
>> xhci-rcar-hcd-$(CONFIG_USB_XHCI_RZV2M) += xhci-rzv2m.o
>> +obj-$(CONFIG_USB_XHCI_SEC_INTR) += xhci-sec-intr.o
>> +
>> obj-$(CONFIG_USB_PCI) += pci-quirks.o
>> obj-$(CONFIG_USB_EHCI_HCD) += ehci-hcd.o
>> diff --git a/drivers/usb/host/xhci-sec-intr.c b/drivers/usb/host/xhci-sec-intr.c
>> new file mode 100644
>> index 000000000000..b112c3388368
>> --- /dev/null
>> +++ b/drivers/usb/host/xhci-sec-intr.c
>> @@ -0,0 +1,438 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +/*
>> + * xHCI host controller secondary interrupter management
>> + *
>> + * Provides logic for client drivers that support utilizing xHCI secondary
>> + * interrupters.
>> + *
>> + * Copyright (c) 2023-2024, Intel Corporation.
>> + *
>> + * Author: Mathias Nyman
>> + */
>> +
>> +#include <linux/usb/xhci-sec-intr.h>
>> +#include <linux/dma-direct.h>
>> +
>> +#include "xhci.h"
>> +
>> +/* internal helpers */
>> +static struct sg_table *
>> +xhci_ring_to_sgtable(struct xhci_sec_intr *si, struct xhci_ring *ring)
>> +{
>> + struct xhci_segment *seg;
>> + struct sg_table *sgt;
>> + unsigned int n_pages;
>> + struct page **pages;
>> + struct device *dev;
>> + size_t sz;
>> + int i;
>> +
>> + dev = xhci_to_hcd(si->xhci)->self.sysdev;
>> + sz = ring->num_segs * TRB_SEGMENT_SIZE;
>> + n_pages = PAGE_ALIGN(sz) >> PAGE_SHIFT;
>> + pages = kvmalloc_array(n_pages, sizeof(struct page *), GFP_KERNEL);
>> + if (!pages)
>> + return NULL;
>> +
>> + sgt = kzalloc(sizeof(*sgt), GFP_KERNEL);
>> + if (!sgt) {
>> + kvfree(pages);
>> + return NULL;
>> + }
>> +
>> + seg = ring->first_seg;
>> + if (!seg)
>> + goto err;
>> + /*
>> + * Rings can potentially have multiple segments, create an array that
>> + * carries page references to allocated segments. Utilize the
>> + * sg_alloc_table_from_pages() to create the sg table, and to ensure
>> + * that page links are created.
>> + */
>> + for (i = 0; i < ring->num_segs; i++) {
>> + dma_get_sgtable(dev, sgt, seg->trbs, seg->dma,
>> + TRB_SEGMENT_SIZE);
>> + pages[i] = sg_page(sgt->sgl);
>> + sg_free_table(sgt);
>> + seg = seg->next;
>> + }
>> +
>> + if (sg_alloc_table_from_pages(sgt, pages, n_pages, 0, sz, GFP_KERNEL))
>> + goto err;
>> +
>> + /*
>> + * Save first segment dma address to sg dma_address field for the sideband
>> + * client to have access to the IOVA of the ring.
>> + */
>> + sg_dma_address(sgt->sgl) = ring->first_seg->dma;
>> +
>> + return sgt;
>> +
>> +err:
>> + kvfree(pages);
>> + kfree(sgt);
>> +
>> + return NULL;
>> +}
>> +
>> +static void
>> +__xhci_sec_intr_remove_endpoint(struct xhci_sec_intr *si, struct xhci_virt_ep *ep)
>> +{
>> + /*
>> + * Issue a stop endpoint command when an endpoint is removed.
>> + * The stop ep cmd handler will handle the ring cleanup.
>> + */
>> + xhci_stop_endpoint_sync(si->xhci, ep, 0, GFP_KERNEL);
>> +
>> + ep->sec = NULL;
>> + si->eps[ep->ep_index] = NULL;
>> +}
>> +
>> +/* endpoint api functions */
>> +
>> +/**
>> + * xhci_sec_intr_add_endpoint - add endpoint to access list
>> + * @si: secondary interrupter instance for this usb device
>> + * @host_ep: usb host endpoint
>> + *
>> + * Adds an endpoint to the list of endpoints utilizing secondary interrupters
>> + * for this usb device.
>> + * After an endpoint is added the client can get the endpoint transfer ring
>> + * buffer by calling xhci_sec_intr_get_endpoint_buffer()
>> + *
>> + * Return: 0 on success, negative error otherwise.
>> + */
>> +int
>> +xhci_sec_intr_add_endpoint(struct xhci_sec_intr *si,
>> + struct usb_host_endpoint *host_ep)
>> +{
>> + struct xhci_virt_ep *ep;
>> + unsigned int ep_index;
>> +
>> + mutex_lock(&si->mutex);
>> + ep_index = xhci_get_endpoint_index(&host_ep->desc);
>> + ep = &si->vdev->eps[ep_index];
>> +
>> + if (ep->ep_state & EP_HAS_STREAMS) {
>> + mutex_unlock(&si->mutex);
>> + return -EINVAL;
>> + }
>> +
>> + /*
>> + * Note, we don't know the DMA mask of the audio DSP device, if its
>> + * smaller than for xhci it won't be able to access the endpoint ring
>> + * buffer. This could be solved by not allowing the audio class driver
>> + * to add the endpoint the normal way, but instead offload it immediately,
>> + * and let this function add the endpoint and allocate the ring buffer
>> + * with the smallest common DMA mask
>> + */
>> + if (si->eps[ep_index] || ep->sec) {
>> + mutex_unlock(&si->mutex);
>> + return -EBUSY;
>> + }
>> +
>> + ep->sec = si;
>> + si->eps[ep_index] = ep;
>> + mutex_unlock(&si->mutex);
>
> We should probably check in xhci-mem.c if ep->sec is set before freeing the
> endpoint ring.
> We don't want the sideband client driver to touch freed rings.
> Maybe we even need a way for xhci driver to notify this sideband/sec_intr client
> in case a offloaded device or endpoint is being freed.
>
Coincidentally, we did see a corner case where there was a situation where the hub driver utilized the xhci_discover_or_reset_device() path, which the class driver is not notified on. This is why I added some extra NULL checks on some of the XHCI sec intr API when fetching the endpoint ring address. However, I do agree that it might not fully cover all the scenarios, because we need to ensure the audio DSP stops all transfers before the transfer ring is freed, in case the audio DSP is busy executing audio transfers.
I think we'd need a way to notify the client (either through some registered callback or notifier block), so that the offload path can be stopped before the ring is freed. I'll write up some changes to do so and submit on the next revision.
Thanks
Wesley Cheng
> I guess usb core in most cases ensures class drivers are properly removed,
> and thus this sideband/sec_interrupt should be unregistered before xhci starts
> freeing endpoints, but I'm not sure sure this is true in all corner cases.
> This is the first time we share endpoint ring addresses.
>
> Thanks
> Mathias
>
On 21.11.2024 3.34, Wesley Cheng wrote: > Hi Mathias, > > On 11/20/2024 6:36 AM, Mathias Nyman wrote: >> On 6.11.2024 21.33, Wesley Cheng wrote: >>> From: Mathias Nyman <mathias.nyman@linux.intel.com> >>> >>> Introduce XHCI sec intr, 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 >>> sec intr 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. >>> >>> Driver renaming, multiple ring segment page linking, proper endpoint clean >>> up, and allowing module compilation added by Wesley Cheng to complete >>> original concept code by Mathias Nyman. >>> >>> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com> >>> Co-developed-by: Wesley Cheng <quic_wcheng@quicinc.com> >>> Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com> >>> --- >>> drivers/usb/host/Kconfig | 11 + >>> drivers/usb/host/Makefile | 2 + >>> drivers/usb/host/xhci-sec-intr.c | 438 ++++++++++++++++++++++++++++++ >>> drivers/usb/host/xhci.h | 4 + >>> include/linux/usb/xhci-sec-intr.h | 70 +++++ >>> 5 files changed, 525 insertions(+) >>> create mode 100644 drivers/usb/host/xhci-sec-intr.c >>> create mode 100644 include/linux/usb/xhci-sec-intr.h >>> >>> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig >>> index d011d6c753ed..a2d549e3e076 100644 >>> --- a/drivers/usb/host/Kconfig >>> +++ b/drivers/usb/host/Kconfig >>> @@ -104,6 +104,17 @@ config USB_XHCI_RZV2M >>> Say 'Y' to enable the support for the xHCI host controller >>> found in Renesas RZ/V2M SoC. >>> +config USB_XHCI_SEC_INTR >>> + tristate "xHCI support for secondary interrupter management" >>> + help >>> + Say 'Y' to enable the support for the xHCI secondary management. >>> + Provide a mechanism for a sideband datapath for payload associated >>> + with audio class endpoints. This allows for an audio DSP to use >>> + xHCI USB endpoints directly, allowing CPU to sleep while playing >>> + audio. This is not the same feature as the audio sideband >>> + capability mentioned within the xHCI specification, and continues >>> + to utilize main system memory for data transfers. >> >> This same API should be used for the hardware xHCI sideband capability. >> We should add a function that checks which types of xHC sideband capability xHC >> hardware can support, and pick and pass a type to xhci xhci_sec_intr_register() >> when registering a sideband/sec_intr > > Just to make sure we're on the same page, when you mention the term sideband capability, are you referring to section 7.9 xHCI Audio Sideband Capability in the xHCI spec? If so, I'm not entirely sure if that capability relies much on secondary interrupters. From reading the material, it just seems like its a way to map audio endpoints directly to another USB device connected to the controller? (I might be wrong, couldn't find much about potential use cases) Yes, that is the one, 7.9 xHCI Audio Sideband Capability. I had that in mind when I started writing the sideband API. This is why registering a sideband and requesting a secondary interrupter are done in separate functions. The concept if still similar even if '7.9 Audio Sideband Capability' doesn't need a secondary interrupter, we want to tell xhci driver/xHC hardware that one connected usb device/endpoint handling is offloaded somewhere else. I don't think we should write another API for that one just because more is done by firmware than by xhci driver. The only change for now would be to add some "sideband_type" parameter to xhci_sec_intr_register(struct usb_device *udev, enum sideband_type), fail the registration if isn't "software", and save the type in struct xhci_sec_intr I'll add hardware sideband support (7.9 Audio Sideband) later, but it would be nice to not change the API then. The name change from sideband to sec-intr is a bit unfortunate with this in mind. Was there some reason for it? Thanks Mathias
Hi Mathias, On 11/21/2024 11:15 AM, Mathias Nyman wrote: > On 21.11.2024 3.34, Wesley Cheng wrote: >> Hi Mathias, >> >> On 11/20/2024 6:36 AM, Mathias Nyman wrote: >>> On 6.11.2024 21.33, Wesley Cheng wrote: >>>> From: Mathias Nyman <mathias.nyman@linux.intel.com> >>>> >>>> Introduce XHCI sec intr, 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 >>>> sec intr 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. >>>> >>>> Driver renaming, multiple ring segment page linking, proper endpoint clean >>>> up, and allowing module compilation added by Wesley Cheng to complete >>>> original concept code by Mathias Nyman. >>>> >>>> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com> >>>> Co-developed-by: Wesley Cheng <quic_wcheng@quicinc.com> >>>> Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com> >>>> --- >>>> drivers/usb/host/Kconfig | 11 + >>>> drivers/usb/host/Makefile | 2 + >>>> drivers/usb/host/xhci-sec-intr.c | 438 ++++++++++++++++++++++++++++++ >>>> drivers/usb/host/xhci.h | 4 + >>>> include/linux/usb/xhci-sec-intr.h | 70 +++++ >>>> 5 files changed, 525 insertions(+) >>>> create mode 100644 drivers/usb/host/xhci-sec-intr.c >>>> create mode 100644 include/linux/usb/xhci-sec-intr.h >>>> >>>> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig >>>> index d011d6c753ed..a2d549e3e076 100644 >>>> --- a/drivers/usb/host/Kconfig >>>> +++ b/drivers/usb/host/Kconfig >>>> @@ -104,6 +104,17 @@ config USB_XHCI_RZV2M >>>> Say 'Y' to enable the support for the xHCI host controller >>>> found in Renesas RZ/V2M SoC. >>>> +config USB_XHCI_SEC_INTR >>>> + tristate "xHCI support for secondary interrupter management" >>>> + help >>>> + Say 'Y' to enable the support for the xHCI secondary management. >>>> + Provide a mechanism for a sideband datapath for payload associated >>>> + with audio class endpoints. This allows for an audio DSP to use >>>> + xHCI USB endpoints directly, allowing CPU to sleep while playing >>>> + audio. This is not the same feature as the audio sideband >>>> + capability mentioned within the xHCI specification, and continues >>>> + to utilize main system memory for data transfers. >>> >>> This same API should be used for the hardware xHCI sideband capability. >>> We should add a function that checks which types of xHC sideband capability xHC >>> hardware can support, and pick and pass a type to xhci xhci_sec_intr_register() >>> when registering a sideband/sec_intr >> >> Just to make sure we're on the same page, when you mention the term sideband capability, are you referring to section 7.9 xHCI Audio Sideband Capability in the xHCI spec? If so, I'm not entirely sure if that capability relies much on secondary interrupters. From reading the material, it just seems like its a way to map audio endpoints directly to another USB device connected to the controller? (I might be wrong, couldn't find much about potential use cases) > > Yes, that is the one, 7.9 xHCI Audio Sideband Capability. > > I had that in mind when I started writing the sideband API. > This is why registering a sideband and requesting a secondary interrupter > are done in separate functions. > The concept if still similar even if '7.9 Audio Sideband Capability' doesn't > need a secondary interrupter, we want to tell xhci driver/xHC hardware that > one connected usb device/endpoint handling is offloaded somewhere else. > Ah, ok...now I understand a bit more on what you were trying to do. When you do eventually introduce the audio sideband capability, you'd need to modify the endpoint APIs to also issue the 'Set Resource Assignment' command with the proper device/endpoint being offloaded. Initially, when Thinh brought up this section in the xHCI spec, I couldn't find any use cases that utilized that capability, so it was a bit unclear to me what it was meant for. Now you've explained it a bit more, I think I can get the gist of it. > I don't think we should write another API for that one just because more is > done by firmware than by xhci driver. > > The only change for now would be to add some "sideband_type" parameter to > xhci_sec_intr_register(struct usb_device *udev, enum sideband_type), fail the > registration if isn't "software", and save the type in struct xhci_sec_intr > > I'll add hardware sideband support (7.9 Audio Sideband) later, but it would be > nice to not change the API then. > > The name change from sideband to sec-intr is a bit unfortunate with this in > mind. Was there some reason for it? > This was because I wasn't too sure how the audio sideband and this driver was related, but I get what you are trying to do now. I can change it back to the original xhci-sideband, as the name change was something that happened on this revision. I will fix the corner case I mentioned WRT the xhci_discover_or_reset_device() scenario, add the basic sideband type handling and rename this back to xhci-sideband in the next rev. Thanks Wesley Cheng
© 2016 - 2026 Red Hat, Inc.