diff mbox series

[v7,01/33] xhci: add support to allocate several interrupters

Message ID 20230921214843.18450-2-quic_wcheng@quicinc.com
State New
Headers show
Series Introduce QC USB SND audio offloading support | expand

Commit Message

Wesley Cheng Sept. 21, 2023, 9:48 p.m. UTC 
From: Mathias Nyman <mathias.nyman@linux.intel.com>

Modify the XHCI drivers to accommodate for handling multiple event rings in
case there are multiple interrupters.  Add the required APIs so clients are
able to allocate/request for an interrupter ring, and pass this information
back to the client driver.  This allows for users to handle the resource
accordingly, such as passing the event ring base address to an audio DSP.
There is no actual support for multiple MSI/MSI-X vectors.

Factoring out XHCI interrupter APIs and structures done by Wesley Cheng, in
order to allow for USB class drivers to utilze them.

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/xhci-debugfs.c |  2 +-
 drivers/usb/host/xhci-mem.c     | 93 ++++++++++++++++++++++++++++++---
 drivers/usb/host/xhci-ring.c    |  2 +-
 drivers/usb/host/xhci.c         | 49 ++++++++++-------
 drivers/usb/host/xhci.h         | 77 +--------------------------
 include/linux/usb/xhci-intr.h   | 86 ++++++++++++++++++++++++++++++
 6 files changed, 207 insertions(+), 102 deletions(-)
 create mode 100644 include/linux/usb/xhci-intr.h

Comments

Mathias Nyman Sept. 28, 2023, 10:31 a.m. UTC | #1 
On 22.9.2023 0.48, Wesley Cheng wrote:
> From: Mathias Nyman <mathias.nyman@linux.intel.com>
> 
> Modify the XHCI drivers to accommodate for handling multiple event rings in
> case there are multiple interrupters.  Add the required APIs so clients are
> able to allocate/request for an interrupter ring, and pass this information
> back to the client driver.  This allows for users to handle the resource
> accordingly, such as passing the event ring base address to an audio DSP.
> There is no actual support for multiple MSI/MSI-X vectors.
> 
> Factoring out XHCI interrupter APIs and structures done by Wesley Cheng, in
> order to allow for USB class drivers to utilze them.
> 
> 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/xhci-debugfs.c |  2 +-
>   drivers/usb/host/xhci-mem.c     | 93 ++++++++++++++++++++++++++++++---
>   drivers/usb/host/xhci-ring.c    |  2 +-
>   drivers/usb/host/xhci.c         | 49 ++++++++++-------
>   drivers/usb/host/xhci.h         | 77 +--------------------------
>   include/linux/usb/xhci-intr.h   | 86 ++++++++++++++++++++++++++++++
>   6 files changed, 207 insertions(+), 102 deletions(-)
>   create mode 100644 include/linux/usb/xhci-intr.h
> 
> diff --git a/drivers/usb/host/xhci-debugfs.c b/drivers/usb/host/xhci-debugfs.c
> index 99baa60ef50f..15a8402ee8a1 100644
> --- a/drivers/usb/host/xhci-debugfs.c
> +++ b/drivers/usb/host/xhci-debugfs.c
> @@ -693,7 +693,7 @@ void xhci_debugfs_init(struct xhci_hcd *xhci)
>   				     "command-ring",
>   				     xhci->debugfs_root);
>   
> -	xhci_debugfs_create_ring_dir(xhci, &xhci->interrupter->event_ring,
> +	xhci_debugfs_create_ring_dir(xhci, &xhci->interrupters[0]->event_ring,
>   				     "event-ring",
>   				     xhci->debugfs_root);
>   
> diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
> index 8714ab5bf04d..2f9228d7d22d 100644
> --- a/drivers/usb/host/xhci-mem.c
> +++ b/drivers/usb/host/xhci-mem.c
> @@ -1837,6 +1837,26 @@ xhci_free_interrupter(struct xhci_hcd *xhci, struct xhci_interrupter *ir)
>   	kfree(ir);
>   }
>   
> +void xhci_remove_secondary_interrupter(struct usb_hcd *hcd, struct xhci_interrupter *ir)
> +{
> +	struct xhci_hcd *xhci = hcd_to_xhci(hcd);
> +	unsigned int intr_num;
> +
> +	/* interrupter 0 is primary interrupter, don't touch it */
> +	if (!ir || !ir->intr_num || ir->intr_num >= xhci->max_interrupters) {
> +		xhci_dbg(xhci, "Invalid secondary interrupter, can't remove\n");
> +		return;
> +	}
> +
> +	/* fixme, should we check xhci->interrupter[intr_num] == ir */
> +	spin_lock(&xhci->lock);

Needs to be spin_lock_irq() ir spin_lock_irqsave() as xhci->lock is used in interrupt handler.


> +	intr_num = ir->intr_num;
> +	xhci_free_interrupter(xhci, ir);
> +	xhci->interrupters[intr_num] = NULL;
> +	spin_unlock(&xhci->lock);

likewise

> +}
> +EXPORT_SYMBOL_GPL(xhci_remove_secondary_interrupter);
> +
>   void xhci_mem_cleanup(struct xhci_hcd *xhci)
>   {
>   	struct device	*dev = xhci_to_hcd(xhci)->self.sysdev;
> @@ -1844,9 +1864,13 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci)
>   
>   	cancel_delayed_work_sync(&xhci->cmd_timer);
>   
> -	xhci_free_interrupter(xhci, xhci->interrupter);
> -	xhci->interrupter = NULL;
> -	xhci_dbg_trace(xhci, trace_xhci_dbg_init, "Freed primary event ring");
> +	for (i = 0; i < xhci->max_interrupters; i++) {
> +		if (xhci->interrupters[i]) {
> +			xhci_free_interrupter(xhci, xhci->interrupters[i]);
> +			xhci->interrupters[i] = NULL;
> +		}
> +	}
> +	xhci_dbg_trace(xhci, trace_xhci_dbg_init, "Freed interrupters");
>   
>   	if (xhci->cmd_ring)
>   		xhci_ring_free(xhci, xhci->cmd_ring);
> @@ -1916,6 +1940,7 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci)
>   	for (i = 0; i < xhci->num_port_caps; i++)
>   		kfree(xhci->port_caps[i].psi);
>   	kfree(xhci->port_caps);
> +	kfree(xhci->interrupters);
>   	xhci->num_port_caps = 0;
>   
>   	xhci->usb2_rhub.ports = NULL;
> @@ -1924,6 +1949,7 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci)
>   	xhci->rh_bw = NULL;
>   	xhci->ext_caps = NULL;
>   	xhci->port_caps = NULL;
> +	xhci->interrupters = NULL;
>   
>   	xhci->page_size = 0;
>   	xhci->page_shift = 0;
> @@ -2276,6 +2302,13 @@ xhci_add_interrupter(struct xhci_hcd *xhci, struct xhci_interrupter *ir,
>   		return -EINVAL;
>   	}
>   
> +	if (xhci->interrupters[intr_num]) {
> +		xhci_warn(xhci, "Interrupter %d\n already set up", intr_num);
> +		return -EINVAL;
> +	}
> +
> +	xhci->interrupters[intr_num] = ir;
> +	ir->intr_num = intr_num;
>   	ir->ir_set = &xhci->run_regs->ir_set[intr_num];
>   
>   	/* set ERST count with the number of entries in the segment table */
> @@ -2295,10 +2328,53 @@ xhci_add_interrupter(struct xhci_hcd *xhci, struct xhci_interrupter *ir,
>   	return 0;
>   }
>   
> +struct xhci_interrupter *
> +xhci_create_secondary_interrupter(struct usb_hcd *hcd)
> +{
> +	struct xhci_hcd *xhci = hcd_to_xhci(hcd);
> +	struct xhci_interrupter *ir;
> +	unsigned int i;
> +	int err = -ENOSPC;
> +
> +	if (!xhci->interrupters)
> +		return NULL;
> +
> +	ir = xhci_alloc_interrupter(xhci, GFP_KERNEL);
> +	if (!ir)
> +		return NULL;
> +
> +	spin_lock_irq(&xhci->lock);
> +
> +	/* Find available secondary interrupter, interrupter 0 is reserverd for primary */

reserved

> +	for (i = 1; i < xhci->max_interrupters; i++) {
> +		if (xhci->interrupters[i] == NULL) {
> +			err = xhci_add_interrupter(xhci, ir, i);
> +			break;
> +		}
> +	}
> +
> +	spin_unlock_irq(&xhci->lock);
> +	if (err) {
> +		xhci_warn(xhci, "Failed to add secondary interrupter, max interrupters %d\n",
> +			xhci->max_interrupters);
> +		xhci_free_interrupter(xhci, ir);
> +		ir = NULL;
> +		goto out;
> +	}
> +
> +	xhci_dbg(xhci, "Add secondary interrupter %d, max interrupters %d\n",
> +		 i, xhci->max_interrupters);
> +
> +out:
> +	return ir;
> +}
> +EXPORT_SYMBOL_GPL(xhci_create_secondary_interrupter);
> +
>   int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
>   {
> -	dma_addr_t	dma;
> +	struct xhci_interrupter *ir;
>   	struct device	*dev = xhci_to_hcd(xhci)->self.sysdev;
> +	dma_addr_t	dma;
>   	unsigned int	val, val2;
>   	u64		val_64;
>   	u32		page_size, temp;
> @@ -2422,11 +2498,14 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
>   	/* Allocate and set up primary interrupter 0 with an event ring. */
>   	xhci_dbg_trace(xhci, trace_xhci_dbg_init,
>   		       "Allocating primary event ring");
> -	xhci->interrupter = xhci_alloc_interrupter(xhci, flags);
> -	if (!xhci->interrupter)
> +	xhci->interrupters = kcalloc_node(xhci->max_interrupters, sizeof(*xhci->interrupters),
> +					  flags, dev_to_node(dev));
> +
> +	ir = xhci_alloc_interrupter(xhci, flags);
> +	if (!ir)
>   		goto fail;
>   
> -	if (xhci_add_interrupter(xhci, xhci->interrupter, 0))
> +	if (xhci_add_interrupter(xhci, ir, 0))
>   		goto fail;
>   
>   	xhci->isoc_bei_interval = AVOID_BEI_INTERVAL_MAX;
> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> index 1dde53f6eb31..93233cf5ff21 100644
> --- a/drivers/usb/host/xhci-ring.c
> +++ b/drivers/usb/host/xhci-ring.c
> @@ -3074,7 +3074,7 @@ irqreturn_t xhci_irq(struct usb_hcd *hcd)
>   	writel(status, &xhci->op_regs->status);
>   
>   	/* This is the handler of the primary interrupter */
> -	ir = xhci->interrupter;
> +	ir = xhci->interrupters[0];
>   	if (!hcd->msi_enabled) {
>   		u32 irq_pending;
>   		irq_pending = readl(&ir->ir_set->irq_pending);
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index e1b1b64a0723..3fd2b58ee1d3 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -456,7 +456,7 @@ static int xhci_init(struct usb_hcd *hcd)
>   
>   static int xhci_run_finished(struct xhci_hcd *xhci)
>   {
> -	struct xhci_interrupter *ir = xhci->interrupter;
> +	struct xhci_interrupter *ir = xhci->interrupters[0];
>   	unsigned long	flags;
>   	u32		temp;
>   
> @@ -508,7 +508,7 @@ int xhci_run(struct usb_hcd *hcd)
>   	u64 temp_64;
>   	int ret;
>   	struct xhci_hcd *xhci = hcd_to_xhci(hcd);
> -	struct xhci_interrupter *ir = xhci->interrupter;
> +	struct xhci_interrupter *ir = xhci->interrupters[0];
>   	/* Start the xHCI host controller running only after the USB 2.0 roothub
>   	 * is setup.
>   	 */
> @@ -572,7 +572,7 @@ void xhci_stop(struct usb_hcd *hcd)
>   {
>   	u32 temp;
>   	struct xhci_hcd *xhci = hcd_to_xhci(hcd);
> -	struct xhci_interrupter *ir = xhci->interrupter;
> +	struct xhci_interrupter *ir = xhci->interrupters[0];
>   
>   	mutex_lock(&xhci->mutex);
>   
> @@ -668,36 +668,49 @@ EXPORT_SYMBOL_GPL(xhci_shutdown);
>   #ifdef CONFIG_PM
>   static void xhci_save_registers(struct xhci_hcd *xhci)
>   {
> -	struct xhci_interrupter *ir = xhci->interrupter;
> +	struct xhci_interrupter *ir;
> +	unsigned int i;
>   
>   	xhci->s3.command = readl(&xhci->op_regs->command);
>   	xhci->s3.dev_nt = readl(&xhci->op_regs->dev_notification);
>   	xhci->s3.dcbaa_ptr = xhci_read_64(xhci, &xhci->op_regs->dcbaa_ptr);
>   	xhci->s3.config_reg = readl(&xhci->op_regs->config_reg);
>   
> -	if (!ir)
> -		return;
> +	/* save both primary and all secondary interrupters */
> +	for (i = 0; i < xhci->max_interrupters; i++) {
> +		ir = xhci->interrupters[i];
> +		if (!ir)
> +			continue;
>   
> -	ir->s3_erst_size = readl(&ir->ir_set->erst_size);
> -	ir->s3_erst_base = xhci_read_64(xhci, &ir->ir_set->erst_base);
> -	ir->s3_erst_dequeue = xhci_read_64(xhci, &ir->ir_set->erst_dequeue);
> -	ir->s3_irq_pending = readl(&ir->ir_set->irq_pending);
> -	ir->s3_irq_control = readl(&ir->ir_set->irq_control);
> +		ir->s3_erst_size = readl(&ir->ir_set->erst_size);
> +		ir->s3_erst_base = xhci_read_64(xhci, &ir->ir_set->erst_base);
> +		ir->s3_erst_dequeue = xhci_read_64(xhci, &ir->ir_set->erst_dequeue);
> +		ir->s3_irq_pending = readl(&ir->ir_set->irq_pending);
> +		ir->s3_irq_control = readl(&ir->ir_set->irq_control);
> +	}
>   }
>   
>   static void xhci_restore_registers(struct xhci_hcd *xhci)
>   {
> -	struct xhci_interrupter *ir = xhci->interrupter;
> +	struct xhci_interrupter *ir;
> +	unsigned int i;
>   
>   	writel(xhci->s3.command, &xhci->op_regs->command);
>   	writel(xhci->s3.dev_nt, &xhci->op_regs->dev_notification);
>   	xhci_write_64(xhci, xhci->s3.dcbaa_ptr, &xhci->op_regs->dcbaa_ptr);
>   	writel(xhci->s3.config_reg, &xhci->op_regs->config_reg);
> -	writel(ir->s3_erst_size, &ir->ir_set->erst_size);
> -	xhci_write_64(xhci, ir->s3_erst_base, &ir->ir_set->erst_base);
> -	xhci_write_64(xhci, ir->s3_erst_dequeue, &ir->ir_set->erst_dequeue);
> -	writel(ir->s3_irq_pending, &ir->ir_set->irq_pending);
> -	writel(ir->s3_irq_control, &ir->ir_set->irq_control);
> +
> +	for (i = 0; i < xhci->max_interrupters; i++) {
> +		ir = xhci->interrupters[i];
> +		if (!ir)
> +			continue;
> +
> +		writel(ir->s3_erst_size, &ir->ir_set->erst_size);
> +		xhci_write_64(xhci, ir->s3_erst_base, &ir->ir_set->erst_base);
> +		xhci_write_64(xhci, ir->s3_erst_dequeue, &ir->ir_set->erst_dequeue);
> +		writel(ir->s3_irq_pending, &ir->ir_set->irq_pending);
> +		writel(ir->s3_irq_control, &ir->ir_set->irq_control);
> +	}
>   }
>   
>   static void xhci_set_cmd_ring_deq(struct xhci_hcd *xhci)
> @@ -1059,7 +1072,7 @@ int xhci_resume(struct xhci_hcd *xhci, pm_message_t msg)
>   		xhci_dbg(xhci, "// Disabling event ring interrupts\n");
>   		temp = readl(&xhci->op_regs->status);
>   		writel((temp & ~0x1fff) | STS_EINT, &xhci->op_regs->status);
> -		xhci_disable_interrupter(xhci->interrupter);
> +		xhci_disable_interrupter(xhci->interrupters[0]);
>   
>   		xhci_dbg(xhci, "cleaning up memory\n");
>   		xhci_mem_cleanup(xhci);

All code above looks like it should be its own patch.

The header shuffling below part of somethine else.

> diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
> index 7e282b4522c0..d706a27ec0a3 100644
> --- a/drivers/usb/host/xhci.h
> +++ b/drivers/usb/host/xhci.h
> @@ -17,6 +17,7 @@
>   #include <linux/kernel.h>
>   #include <linux/usb/hcd.h>
>   #include <linux/io-64-nonatomic-lo-hi.h>
> +#include <linux/usb/xhci-intr.h>
>   
>   /* Code sharing between pci-quirks and xhci hcd */
>   #include	"xhci-ext-caps.h"
> @@ -1541,18 +1542,6 @@ static inline const char *xhci_trb_type_string(u8 type)
>   #define AVOID_BEI_INTERVAL_MIN	8
>   #define AVOID_BEI_INTERVAL_MAX	32
>   
> -struct xhci_segment {
> -	union xhci_trb		*trbs;
> -	/* private to HCD */
> -	struct xhci_segment	*next;
> -	dma_addr_t		dma;
> -	/* Max packet sized bounce buffer for td-fragmant alignment */
> -	dma_addr_t		bounce_dma;
> -	void			*bounce_buf;
> -	unsigned int		bounce_offs;
> -	unsigned int		bounce_len;
> -};
> -
>   enum xhci_cancelled_td_status {
>   	TD_DIRTY = 0,
>   	TD_HALTED,
> @@ -1585,16 +1574,6 @@ struct xhci_cd {
>   	union xhci_trb		*cmd_trb;
>   };
>   
> -enum xhci_ring_type {
> -	TYPE_CTRL = 0,
> -	TYPE_ISOC,
> -	TYPE_BULK,
> -	TYPE_INTR,
> -	TYPE_STREAM,
> -	TYPE_COMMAND,
> -	TYPE_EVENT,
> -};
> -
>   static inline const char *xhci_ring_type_string(enum xhci_ring_type type)
>   {
>   	switch (type) {
> @@ -1615,46 +1594,6 @@ static inline const char *xhci_ring_type_string(enum xhci_ring_type type)
>   	}
>   
>   	return "UNKNOWN";
> -}
> -
> -struct xhci_ring {
> -	struct xhci_segment	*first_seg;
> -	struct xhci_segment	*last_seg;
> -	union  xhci_trb		*enqueue;
> -	struct xhci_segment	*enq_seg;
> -	union  xhci_trb		*dequeue;
> -	struct xhci_segment	*deq_seg;
> -	struct list_head	td_list;
> -	/*
> -	 * Write the cycle state into the TRB cycle field to give ownership of
> -	 * the TRB to the host controller (if we are the producer), or to check
> -	 * if we own the TRB (if we are the consumer).  See section 4.9.1.
> -	 */
> -	u32			cycle_state;
> -	unsigned int		stream_id;
> -	unsigned int		num_segs;
> -	unsigned int		num_trbs_free; /* used only by xhci DbC */
> -	unsigned int		bounce_buf_len;
> -	enum xhci_ring_type	type;
> -	bool			last_td_was_short;
> -	struct radix_tree_root	*trb_address_map;
> -};
> -
> -struct xhci_erst_entry {
> -	/* 64-bit event ring segment address */
> -	__le64	seg_addr;
> -	__le32	seg_size;
> -	/* Set to zero */
> -	__le32	rsvd;
> -};
> -
> -struct xhci_erst {
> -	struct xhci_erst_entry	*entries;
> -	unsigned int		num_entries;
> -	/* xhci->event_ring keeps track of segment dma addresses */
> -	dma_addr_t		erst_dma_addr;
> -	/* Num entries the ERST can contain */
> -	unsigned int		erst_size;
>   };
>   
>   struct xhci_scratchpad {
> @@ -1707,18 +1646,6 @@ struct xhci_bus_state {
>   	unsigned long		resuming_ports;
>   };
>   
> -struct xhci_interrupter {
> -	struct xhci_ring	*event_ring;
> -	struct xhci_erst	erst;
> -	struct xhci_intr_reg __iomem *ir_set;
> -	unsigned int		intr_num;
> -	/* For interrupter registers save and restore over suspend/resume */
> -	u32	s3_irq_pending;
> -	u32	s3_irq_control;
> -	u32	s3_erst_size;
> -	u64	s3_erst_base;
> -	u64	s3_erst_dequeue;
> -};
>   /*
>    * It can take up to 20 ms to transition from RExit to U0 on the
>    * Intel Lynx Point LP xHCI host.
> @@ -1799,7 +1726,7 @@ struct xhci_hcd {
>   	struct reset_control *reset;
>   	/* data structures */
>   	struct xhci_device_context_array *dcbaa;
> -	struct xhci_interrupter *interrupter;
> +	struct xhci_interrupter **interrupters;
>   	struct xhci_ring	*cmd_ring;
>   	unsigned int            cmd_ring_state;
>   #define CMD_RING_STATE_RUNNING         (1 << 0)
> diff --git a/include/linux/usb/xhci-intr.h b/include/linux/usb/xhci-intr.h
> new file mode 100644
> index 000000000000..e0091ee2c73a
> --- /dev/null
> +++ b/include/linux/usb/xhci-intr.h
> @@ -0,0 +1,86 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef __LINUX_XHCI_INTR_H
> +#define __LINUX_XHCI_INTR_H
> +
> +#include <linux/kernel.h>
> +
> +struct xhci_erst_entry {
> +	/* 64-bit event ring segment address */
> +	__le64	seg_addr;
> +	__le32	seg_size;
> +	/* Set to zero */
> +	__le32	rsvd;
> +};
> +
> +enum xhci_ring_type {
> +	TYPE_CTRL = 0,
> +	TYPE_ISOC,
> +	TYPE_BULK,
> +	TYPE_INTR,
> +	TYPE_STREAM,
> +	TYPE_COMMAND,
> +	TYPE_EVENT,
> +};
> +
> +struct xhci_erst {
> +	struct xhci_erst_entry	*entries;
> +	unsigned int		num_entries;
> +	/* xhci->event_ring keeps track of segment dma addresses */
> +	dma_addr_t		erst_dma_addr;
> +	/* Num entries the ERST can contain */
> +	unsigned int		erst_size;
> +};
> +
> +struct xhci_segment {
> +	union xhci_trb		*trbs;
> +	/* private to HCD */
> +	struct xhci_segment	*next;
> +	dma_addr_t		dma;
> +	/* Max packet sized bounce buffer for td-fragmant alignment */
> +	dma_addr_t		bounce_dma;
> +	void			*bounce_buf;
> +	unsigned int		bounce_offs;
> +	unsigned int		bounce_len;
> +};
> +
> +struct xhci_ring {
> +	struct xhci_segment	*first_seg;
> +	struct xhci_segment	*last_seg;
> +	union  xhci_trb		*enqueue;
> +	struct xhci_segment	*enq_seg;
> +	union  xhci_trb		*dequeue;
> +	struct xhci_segment	*deq_seg;
> +	struct list_head	td_list;
> +	/*
> +	 * Write the cycle state into the TRB cycle field to give ownership of
> +	 * the TRB to the host controller (if we are the producer), or to check
> +	 * if we own the TRB (if we are the consumer).  See section 4.9.1.
> +	 */
> +	u32			cycle_state;
> +	unsigned int		stream_id;
> +	unsigned int		num_segs;
> +	unsigned int		num_trbs_free;
> +	unsigned int		num_trbs_free_temp;
> +	unsigned int		bounce_buf_len;
> +	enum xhci_ring_type	type;
> +	bool			last_td_was_short;
> +	struct radix_tree_root	*trb_address_map;
> +};
> +
> +struct xhci_interrupter {
> +	struct xhci_ring	*event_ring;
> +	struct xhci_erst	erst;
> +	struct xhci_intr_reg __iomem *ir_set;
> +	unsigned int		intr_num;
> +	/* For interrupter registers save and restore over suspend/resume */
> +	u32	s3_irq_pending;
> +	u32	s3_irq_control;
> +	u32	s3_erst_size;
> +	u64	s3_erst_base;
> +	u64	s3_erst_dequeue;
> +};
> +
> +struct xhci_interrupter *
> +xhci_create_secondary_interrupter(struct usb_hcd *hcd);
> +void xhci_remove_secondary_interrupter(struct usb_hcd *hcd, struct xhci_interrupter *ir);
> +#endif
> 

Not convinced we want to share all these xhci private structures in a separate
header outside of the xhci code.

As much as possible should be abstracted and added to the xhci sideband
API in patch 3/33 instead of sharing these.
  
Thanks
Mathias
Wesley Cheng Oct. 2, 2023, 8:07 p.m. UTC | #2 
Hi Mathias,

On 9/28/2023 3:31 AM, Mathias Nyman wrote:
> On 22.9.2023 0.48, Wesley Cheng wrote:
>> From: Mathias Nyman <mathias.nyman@linux.intel.com>
>>
>> Modify the XHCI drivers to accommodate for handling multiple event 
>> rings in
>> case there are multiple interrupters.  Add the required APIs so 
>> clients are
>> able to allocate/request for an interrupter ring, and pass this 
>> information
>> back to the client driver.  This allows for users to handle the resource
>> accordingly, such as passing the event ring base address to an audio DSP.
>> There is no actual support for multiple MSI/MSI-X vectors.
>>
>> Factoring out XHCI interrupter APIs and structures done by Wesley 
>> Cheng, in
>> order to allow for USB class drivers to utilze them.
>>
>> 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/xhci-debugfs.c |  2 +-
>>   drivers/usb/host/xhci-mem.c     | 93 ++++++++++++++++++++++++++++++---
>>   drivers/usb/host/xhci-ring.c    |  2 +-
>>   drivers/usb/host/xhci.c         | 49 ++++++++++-------
>>   drivers/usb/host/xhci.h         | 77 +--------------------------
>>   include/linux/usb/xhci-intr.h   | 86 ++++++++++++++++++++++++++++++
>>   6 files changed, 207 insertions(+), 102 deletions(-)
>>   create mode 100644 include/linux/usb/xhci-intr.h
>>
>> diff --git a/drivers/usb/host/xhci-debugfs.c 
>> b/drivers/usb/host/xhci-debugfs.c
>> index 99baa60ef50f..15a8402ee8a1 100644
>> --- a/drivers/usb/host/xhci-debugfs.c
>> +++ b/drivers/usb/host/xhci-debugfs.c
>> @@ -693,7 +693,7 @@ void xhci_debugfs_init(struct xhci_hcd *xhci)
>>                        "command-ring",
>>                        xhci->debugfs_root);
>> -    xhci_debugfs_create_ring_dir(xhci, &xhci->interrupter->event_ring,
>> +    xhci_debugfs_create_ring_dir(xhci, 
>> &xhci->interrupters[0]->event_ring,
>>                        "event-ring",
>>                        xhci->debugfs_root);
>> diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
>> index 8714ab5bf04d..2f9228d7d22d 100644
>> --- a/drivers/usb/host/xhci-mem.c
>> +++ b/drivers/usb/host/xhci-mem.c
>> @@ -1837,6 +1837,26 @@ xhci_free_interrupter(struct xhci_hcd *xhci, 
>> struct xhci_interrupter *ir)
>>       kfree(ir);
>>   }
>> +void xhci_remove_secondary_interrupter(struct usb_hcd *hcd, struct 
>> xhci_interrupter *ir)
>> +{
>> +    struct xhci_hcd *xhci = hcd_to_xhci(hcd);
>> +    unsigned int intr_num;
>> +
>> +    /* interrupter 0 is primary interrupter, don't touchit */
>> +    if (!ir || !ir->intr_num || ir->intr_num >= 
>> xhci->max_interrupters) {
>> +        xhci_dbg(xhci, "Invalid secondary interrupter, can't remove\n");
>> +        return;
>> +    }
>> +
>> +    /* fixme, should we check xhci->interrupter[intr_num] == ir */
>> +    spin_lock(&xhci->lock);
> 
> Needs to be spin_lock_irq() ir spin_lock_irqsave() as xhci->lock is used 
> in interrupt handler.
> 
> 
>> +    intr_num = ir->intr_num;
>> +    xhci_free_interrupter(xhci, ir);
>> +    xhci->interrupters[intr_num] = NULL;
>> +    spin_unlock(&xhci->lock);
> 
> likewise
> 

Let me check these again.  In general, I think I will use both the 
xhci->mutex and xhci->lock where needed, because I believe we'd run into 
sleep while atomic issues while freeing the DMA memory.  Will rework 
this and submit in the next rev.

>> +}
>> +EXPORT_SYMBOL_GPL(xhci_remove_secondary_interrupter);
>> +
>>   void xhci_mem_cleanup(struct xhci_hcd *xhci)
>>   {
>>       struct device    *dev = xhci_to_hcd(xhci)->self.sysdev;
>> @@ -1844,9 +1864,13 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci)
>>       cancel_delayed_work_sync(&xhci->cmd_timer);
>> -    xhci_free_interrupter(xhci, xhci->interrupter);
>> -    xhci->interrupter = NULL;
>> -    xhci_dbg_trace(xhci, trace_xhci_dbg_init, "Freed primary event 
>> ring");
>> +    for (i = 0; i < xhci->max_interrupters; i++) {
>> +        if (xhci->interrupters[i]) {
>> +            xhci_free_interrupter(xhci, xhci->interrupters[i]);
>> +            xhci->interrupters[i] = NULL;
>> +        }
>> +    }
>> +    xhci_dbg_trace(xhci, trace_xhci_dbg_init, "Freed interrupters");
>>       if (xhci->cmd_ring)
>>           xhci_ring_free(xhci, xhci->cmd_ring);
>> @@ -1916,6 +1940,7 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci)
>>       for (i = 0; i < xhci->num_port_caps; i++)
>>           kfree(xhci->port_caps[i].psi);
>>       kfree(xhci->port_caps);
>> +    kfree(xhci->interrupters);
>>       xhci->num_port_caps = 0;
>>       xhci->usb2_rhub.ports = NULL;
>> @@ -1924,6 +1949,7 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci)
>>       xhci->rh_bw = NULL;
>>       xhci->ext_caps = NULL;
>>       xhci->port_caps = NULL;
>> +    xhci->interrupters = NULL;
>>       xhci->page_size = 0;
>>       xhci->page_shift = 0;
>> @@ -2276,6 +2302,13 @@ xhci_add_interrupter(struct xhci_hcd *xhci, 
>> struct xhci_interrupter *ir,
>>           return -EINVAL;
>>       }
>> +    if (xhci->interrupters[intr_num]) {
>> +        xhci_warn(xhci, "Interrupter%d\n already set up", intr_num);
>> +        return -EINVAL;
>> +    }
>> +
>> +    xhci->interrupters[intr_num] = ir;
>> +    ir->intr_num = intr_num;
>>       ir->ir_set = &xhci->run_regs->ir_set[intr_num];
>>       /* set ERST count with the number of entries in the segment 
>> table */
>> @@ -2295,10 +2328,53 @@ xhci_add_interrupter(struct xhci_hcd *xhci, 
>> struct xhci_interrupter *ir,
>>       return 0;
>>   }
>> +struct xhci_interrupter *
>> +xhci_create_secondary_interrupter(struct usb_hcd *hcd)
>> +{
>> +    struct xhci_hcd *xhci = hcd_to_xhci(hcd);
>> +    struct xhci_interrupter *ir;
>> +    unsigned int i;
>> +    int err = -ENOSPC;
>> +
>> +    if (!xhci->interrupters)
>> +        return NULL;
>> +
>> +    ir = xhci_alloc_interrupter(xhci, GFP_KERNEL);
>> +    if (!ir)
>> +        return NULL;
>> +
>> +    spin_lock_irq(&xhci->lock);
>> +
>> +    /* Find available secondary interrupter, interrupter0 is 
>> reserverd for primary */
> 
> reserved
> 
>> +    for (i = 1; i < xhci->max_interrupters; i++) {
>> +        if (xhci->interrupters[i] == NULL) {
>> +            err = xhci_add_interrupter(xhci, ir, i);
>> +            break;
>> +        }
>> +    }
>> +
>> +    spin_unlock_irq(&xhci->lock);
>> +    if (err) {
>> +        xhci_warn(xhci, "Failed to add secondary interrupter, max 
>> interrupters %d\n",
>> +            xhci->max_interrupters);
>> +        xhci_free_interrupter(xhci, ir);
>> +        ir = NULL;
>> +        goto out;
>> +    }
>> +
>> +    xhci_dbg(xhci, "Add secondary interrupter %d, max interrupters 
>> %d\n",
>> +         i, xhci->max_interrupters);
>> +
>> +out:
>> +    return ir;
>> +}
>> +EXPORT_SYMBOL_GPL(xhci_create_secondary_interrupter);
>> +
>>   int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
>>   {
>> -    dma_addr_t    dma;
>> +    struct xhci_interrupter *ir;
>>       struct device    *dev = xhci_to_hcd(xhci)->self.sysdev;
>> +    dma_addr_t    dma;
>>       unsigned int    val, val2;
>>       u64        val_64;
>>       u32        page_size, temp;
>> @@ -2422,11 +2498,14 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t 
>> flags)
>>       /* Allocate and set up primary interrupter 0 with an event ring. */
>>       xhci_dbg_trace(xhci, trace_xhci_dbg_init,
>>                  "Allocating primary event ring");
>> -    xhci->interrupter = xhci_alloc_interrupter(xhci, flags);
>> -    if (!xhci->interrupter)
>> +    xhci->interrupters = kcalloc_node(xhci->max_interrupters, 
>> sizeof(*xhci->interrupters),
>> +                      flags, dev_to_node(dev));
>> +
>> +    ir = xhci_alloc_interrupter(xhci, flags);
>> +    if (!ir)
>>           goto fail;
>> -    if (xhci_add_interrupter(xhci, xhci->interrupter, 0))
>> +    if (xhci_add_interrupter(xhci, ir, 0))
>>           goto fail;
>>       xhci->isoc_bei_interval = AVOID_BEI_INTERVAL_MAX;
>> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
>> index 1dde53f6eb31..93233cf5ff21 100644
>> --- a/drivers/usb/host/xhci-ring.c
>> +++ b/drivers/usb/host/xhci-ring.c
>> @@ -3074,7 +3074,7 @@ irqreturn_t xhci_irq(struct usb_hcd *hcd)
>>       writel(status, &xhci->op_regs->status);
>>       /* This is the handler of the primary interrupter */
>> -    ir = xhci->interrupter;
>> +    ir = xhci->interrupters[0];
>>       if (!hcd->msi_enabled) {
>>           u32 irq_pending;
>>           irq_pending = readl(&ir->ir_set->irq_pending);
>> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
>> index e1b1b64a0723..3fd2b58ee1d3 100644
>> --- a/drivers/usb/host/xhci.c
>> +++ b/drivers/usb/host/xhci.c
>> @@ -456,7 +456,7 @@ static int xhci_init(struct usb_hcd *hcd)
>>   static int xhci_run_finished(struct xhci_hcd *xhci)
>>   {
>> -    struct xhci_interrupter *ir = xhci->interrupter;
>> +    struct xhci_interrupter *ir = xhci->interrupters[0];
>>       unsigned long    flags;
>>       u32        temp;
>> @@ -508,7 +508,7 @@ int xhci_run(struct usb_hcd *hcd)
>>       u64 temp_64;
>>       int ret;
>>       struct xhci_hcd *xhci = hcd_to_xhci(hcd);
>> -    struct xhci_interrupter *ir = xhci->interrupter;
>> +    struct xhci_interrupter *ir = xhci->interrupters[0];
>>       /* Start the xHCI host controller runningonly after the USB 2.0 
>> roothub
>>        * is setup.
>>        */
>> @@ -572,7 +572,7 @@ void xhci_stop(struct usb_hcd *hcd)
>>   {
>>       u32 temp;
>>       struct xhci_hcd *xhci = hcd_to_xhci(hcd);
>> -    struct xhci_interrupter *ir = xhci->interrupter;
>> +    struct xhci_interrupter *ir = xhci->interrupters[0];
>>       mutex_lock(&xhci->mutex);
>> @@ -668,36 +668,49 @@ EXPORT_SYMBOL_GPL(xhci_shutdown);
>>   #ifdef CONFIG_PM
>>   static void xhci_save_registers(struct xhci_hcd *xhci)
>>   {
>> -    struct xhci_interrupter *ir = xhci->interrupter;
>> +    struct xhci_interrupter *ir;
>> +    unsigned int i;
>>       xhci->s3.command = readl(&xhci->op_regs->command);
>>       xhci->s3.dev_nt = readl(&xhci->op_regs->dev_notification);
>>       xhci->s3.dcbaa_ptr = xhci_read_64(xhci,&xhci->op_regs->dcbaa_ptr);
>>       xhci->s3.config_reg = readl(&xhci->op_regs->config_reg);
>> -    if (!ir)
>> -        return;
>> +    /* save both primary and all secondary interrupters */
>> +    for (i = 0; i < xhci->max_interrupters; i++) {
>> +        ir = xhci->interrupters[i];
>> +        if (!ir)
>> +            continue;
>> -    ir->s3_erst_size = readl(&ir->ir_set->erst_size);
>> -    ir->s3_erst_base = xhci_read_64(xhci, &ir->ir_set->erst_base);
>> -    ir->s3_erst_dequeue = xhci_read_64(xhci, &ir->ir_set->erst_dequeue);
>> -    ir->s3_irq_pending = readl(&ir->ir_set->irq_pending);
>> -    ir->s3_irq_control = readl(&ir->ir_set->irq_control);
>> +        ir->s3_erst_size = readl(&ir->ir_set->erst_size);
>> +        ir->s3_erst_base = xhci_read_64(xhci, &ir->ir_set->erst_base);
>> +        ir->s3_erst_dequeue = xhci_read_64(xhci, 
>> &ir->ir_set->erst_dequeue);
>> +        ir->s3_irq_pending = readl(&ir->ir_set->irq_pending);
>> +        ir->s3_irq_control = readl(&ir->ir_set->irq_control);
>> +    }
>>   }
>>   static void xhci_restore_registers(struct xhci_hcd *xhci)
>>   {
>> -    struct xhci_interrupter *ir = xhci->interrupter;
>> +    struct xhci_interrupter *ir;
>> +    unsigned int i;
>>       writel(xhci->s3.command, &xhci->op_regs->command);
>>       writel(xhci->s3.dev_nt, &xhci->op_regs->dev_notification);
>>       xhci_write_64(xhci, xhci->s3.dcbaa_ptr, &xhci->op_regs->dcbaa_ptr);
>>       writel(xhci->s3.config_reg, &xhci->op_regs->config_reg);
>> -    writel(ir->s3_erst_size, &ir->ir_set->erst_size);
>> -    xhci_write_64(xhci, ir->s3_erst_base, &ir->ir_set->erst_base);
>> -    xhci_write_64(xhci, ir->s3_erst_dequeue, &ir->ir_set->erst_dequeue);
>> -    writel(ir->s3_irq_pending, &ir->ir_set->irq_pending);
>> -    writel(ir->s3_irq_control, &ir->ir_set->irq_control);
>> +
>> +    for (i = 0; i < xhci->max_interrupters; i++) {
>> +        ir = xhci->interrupters[i];
>> +        if (!ir)
>> +            continue;
>> +
>> +        writel(ir->s3_erst_size, &ir->ir_set->erst_size);
>> +        xhci_write_64(xhci, ir->s3_erst_base, &ir->ir_set->erst_base);
>> +        xhci_write_64(xhci, ir->s3_erst_dequeue, 
>> &ir->ir_set->erst_dequeue);
>> +        writel(ir->s3_irq_pending, &ir->ir_set->irq_pending);
>> +        writel(ir->s3_irq_control, &ir->ir_set->irq_control);
>> +    }
>>   }
>>   static void xhci_set_cmd_ring_deq(struct xhci_hcd *xhci)
>> @@ -1059,7 +1072,7 @@ int xhci_resume(struct xhci_hcd *xhci, 
>> pm_message_t msg)
>>           xhci_dbg(xhci, "// Disabling event ring interrupts\n");
>>           temp = readl(&xhci->op_regs->status);
>>           writel((temp & ~0x1fff) | STS_EINT, &xhci->op_regs->status);
>> -        xhci_disable_interrupter(xhci->interrupter);
>> +        xhci_disable_interrupter(xhci->interrupters[0]);
>>           xhci_dbg(xhci, "cleaning up memory\n");
>>           xhci_mem_cleanup(xhci);
> 
> All code above looks like it should be its own patch.
> 
> The header shuffling below part of somethine else.
>  >> diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
>> index 7e282b4522c0..d706a27ec0a3 100644
>> --- a/drivers/usb/host/xhci.h
>> +++ b/drivers/usb/host/xhci.h
>> @@ -17,6 +17,7 @@
>>   #include <linux/kernel.h>
>>   #include <linux/usb/hcd.h>
>>   #include <linux/io-64-nonatomic-lo-hi.h>
>> +#include <linux/usb/xhci-intr.h>
>>   /* Code sharing between pci-quirks and xhci hcd */
>>   #include    "xhci-ext-caps.h"
>> @@ -1541,18 +1542,6 @@ static inline const char 
>> *xhci_trb_type_string(u8 type)
>>   #define AVOID_BEI_INTERVAL_MIN    8
>>   #define AVOID_BEI_INTERVAL_MAX    32
>> -struct xhci_segment {
>> -    union xhci_trb        *trbs;
>> -    /* private to HCD */
>> -    struct xhci_segment    *next;
>> -    dma_addr_t       dma;
>> -    /* Max packet sized bounce buffer for td-fragmant alignment */
>> -    dma_addr_t       bounce_dma;
>> -    void            *bounce_buf;
>> -    unsigned int        bounce_offs;
>> -    unsigned int        bounce_len;
>> -};
>> -
>>   enum xhci_cancelled_td_status {
>>       TD_DIRTY = 0,
>>       TD_HALTED,
>> @@ -1585,16 +1574,6 @@ struct xhci_cd {
>>       union xhci_trb        *cmd_trb;
>>   };
>> -enum xhci_ring_type {
>> -    TYPE_CTRL = 0,
>> -    TYPE_ISOC,
>> -    TYPE_BULK,
>> -    TYPE_INTR,
>> -    TYPE_STREAM,
>> -    TYPE_COMMAND,
>> -    TYPE_EVENT,
>> -};
>> -
>>   static inline const char *xhci_ring_type_string(enum xhci_ring_type 
>> type)
>>   {
>>       switch (type) {
>> @@ -1615,46 +1594,6 @@ static inline const char 
>> *xhci_ring_type_string(enum xhci_ring_type type)
>>       }
>>       return "UNKNOWN";
>> -}
>> -
>> -struct xhci_ring {
>> -    struct xhci_segment    *first_seg;
>> -    struct xhci_segment    *last_seg;
>> -    union  xhci_trb        *enqueue;
>> -    struct xhci_segment    *enq_seg;
>> -    union  xhci_trb        *dequeue;
>> -    struct xhci_segment    *deq_seg;
>> -    struct list_head    td_list;
>> -    /*
>> -     * Write the cycle state into the TRB cycle field to give 
>> ownership of
>> -     * the TRB to the host controller (if we are the producer), or to 
>> check
>> -     * if we own the TRB (if we are the consumer).  See section 4.9.1.
>> -     */
>> -    u32            cycle_state;
>> -    unsigned int        stream_id;
>> -    unsigned int        num_segs;
>> -    unsigned int        num_trbs_free; /* used only by xhci DbC */
>> -    unsigned int        bounce_buf_len;
>> -    enum xhci_ring_type    type;
>> -    bool            last_td_was_short;
>> -    struct radix_tree_root    *trb_address_map;
>> -};
>> -
>> -struct xhci_erst_entry {
>> -    /* 64-bit event ring segment address */
>> -    __le64    seg_addr;
>> -    __le32    seg_size;
>> -    /* Set to zero */
>> -    __le32    rsvd;
>> -};
>> -
>> -struct xhci_erst {
>> -    struct xhci_erst_entry    *entries;
>> -    unsigned int        num_entries;
>> -    /* xhci->event_ring keeps track of segment dma addresses */
>> -    dma_addr_t       erst_dma_addr;
>> -    /* Num entries the ERST can contain */
>> -    unsigned int        erst_size;
>>   };
>>   struct xhci_scratchpad {
>> @@ -1707,18 +1646,6 @@ struct xhci_bus_state {
>>       unsigned long        resuming_ports;
>>   };
>> -struct xhci_interrupter {
>> -    struct xhci_ring    *event_ring;
>> -    struct xhci_erst    erst;
>> -    struct xhci_intr_reg __iomem *ir_set;
>> -    unsigned int        intr_num;
>> -    /* For interrupter registers save and restore over suspend/resume */
>> -    u32    s3_irq_pending;
>> -    u32    s3_irq_control;
>> -    u32    s3_erst_size;
>> -    u64    s3_erst_base;
>> -    u64    s3_erst_dequeue;
>> -};
>>   /*
>>    * It can take up to 20 ms to transition from RExit to U0 onthe
>>    * Intel Lynx Point LP xHCI host.
>> @@ -1799,7 +1726,7 @@ struct xhci_hcd {
>>       struct reset_control *reset;
>>       /* data structures */
>>       struct xhci_device_context_array *dcbaa;
>> -    struct xhci_interrupter *interrupter;
>> +    struct xhci_interrupter **interrupters;
>>       struct xhci_ring    *cmd_ring;
>>       unsigned int            cmd_ring_state;
>>   #define CMD_RING_STATE_RUNNING         (1 << 0)
>> diff --git a/include/linux/usb/xhci-intr.h 
>> b/include/linux/usb/xhci-intr.h
>> new file mode 100644
>> index 000000000000..e0091ee2c73a
>> --- /dev/null
>> +++ b/include/linux/usb/xhci-intr.h
>> @@ -0,0 +1,86 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +#ifndef __LINUX_XHCI_INTR_H
>> +#define __LINUX_XHCI_INTR_H
>> +
>> +#include <linux/kernel.h>
>> +
>> +struct xhci_erst_entry {
>> +    /* 64-bit event ring segment address */
>> +    __le64    seg_addr;
>> +    __le32    seg_size;
>> +    /* Set to zero */
>> +    __le32    rsvd;
>> +};
>> +
>> +enum xhci_ring_type {
>> +    TYPE_CTRL = 0,
>> +    TYPE_ISOC,
>> +    TYPE_BULK,
>> +    TYPE_INTR,
>> +    TYPE_STREAM,
>> +    TYPE_COMMAND,
>> +    TYPE_EVENT,
>> +};
>> +
>> +struct xhci_erst {
>> +    struct xhci_erst_entry    *entries;
>> +    unsigned int        num_entries;
>> +    /* xhci->event_ring keeps track of segment dma addresses */
>> +    dma_addr_t       erst_dma_addr;
>> +    /* Num entries the ERST can contain */
>> +    unsigned int        erst_size;
>> +};
>> +
>> +struct xhci_segment {
>> +    union xhci_trb        *trbs;
>> +    /* private to HCD */
>> +    struct xhci_segment    *next;
>> +    dma_addr_t       dma;
>> +    /* Max packet sized bounce buffer for td-fragmant alignment */
>> +    dma_addr_t       bounce_dma;
>> +    void            *bounce_buf;
>> +    unsigned int        bounce_offs;
>> +    unsigned int        bounce_len;
>> +};
>> +
>> +struct xhci_ring {
>> +    struct xhci_segment    *first_seg;
>> +    struct xhci_segment    *last_seg;
>> +    union  xhci_trb        *enqueue;
>> +    struct xhci_segment    *enq_seg;
>> +    union  xhci_trb        *dequeue;
>> +    struct xhci_segment    *deq_seg;
>> +    struct list_head    td_list;
>> +    /*
>> +     * Write the cycle state into the TRB cycle field to give 
>> ownership of
>> +     * the TRB to the host controller (if we are the producer), or to 
>> check
>> +     * if we own the TRB (if we are the consumer).  See section 4.9.1.
>> +     */
>> +    u32            cycle_state;
>> +    unsigned int        stream_id;
>> +    unsigned int        num_segs;
>> +    unsigned int        num_trbs_free;
>> +    unsigned int        num_trbs_free_temp;
>> +    unsigned int        bounce_buf_len;
>> +    enum xhci_ring_type    type;
>> +    bool            last_td_was_short;
>> +    struct radix_tree_root    *trb_address_map;
>> +};
>> +
>> +struct xhci_interrupter {
>> +    struct xhci_ring    *event_ring;
>> +    struct xhci_erst    erst;
>> +    struct xhci_intr_reg __iomem *ir_set;
>> +    unsigned int        intr_num;
>> +    /* For interrupter registers save and restore over suspend/resume */
>> +    u32    s3_irq_pending;
>> +    u32    s3_irq_control;
>> +    u32    s3_erst_size;
>> +    u64    s3_erst_base;
>> +    u64    s3_erst_dequeue;
>> +};
>> +
>> +struct xhci_interrupter *
>> +xhci_create_secondary_interrupter(struct usb_hcd *hcd);
>> +void xhci_remove_secondary_interrupter(struct usb_hcd *hcd, struct 
>> xhci_interrupter *ir);
>> +#endif
>>
> 
> Not convinced we want to share all these xhci private structures in a 
> separate
> header outside of the xhci code.
> 
> As much as possible should be abstracted and added to the xhci sideband
> API in patch 3/33 instead of sharing these.

It gets a bit difficult because xhci_create_secondary_interrupter() will 
return struct xhci_interrupter, so that the class (offload) driver can 
fetch information about the event ring.  So part of that is that the 
class driver has to reference struct xhci_ring as well.

Instead of exposing all these into a header file, what do you think 
about adding the drivers/xhci path as an include directory in the class 
driver make arguments in the makefile?

Thanks
Wesley Cheng
Mathias Nyman Oct. 4, 2023, 2:02 p.m. UTC | #3 
On 2.10.2023 23.07, Wesley Cheng wrote:
> Hi Mathias,
> 
> On 9/28/2023 3:31 AM, Mathias Nyman wrote:
>> On 22.9.2023 0.48, Wesley Cheng wrote:
>>> From: Mathias Nyman <mathias.nyman@linux.intel.com>
>>>
>>> Modify the XHCI drivers to accommodate for handling multiple event rings in
>>> case there are multiple interrupters.  Add the required APIs so clients are
>>> able to allocate/request for an interrupter ring, and pass this information
>>> back to the client driver.  This allows for users to handle the resource
>>> accordingly, such as passing the event ring base address to an audio DSP.
>>> There is no actual support for multiple MSI/MSI-X vectors.
>>>
>>> Factoring out XHCI interrupter APIs and structures done by Wesley Cheng, in
>>> order to allow for USB class drivers to utilze them.
>>>
>>>   }
>>> +void xhci_remove_secondary_interrupter(struct usb_hcd *hcd, struct xhci_interrupter *ir)
>>> +{
>>> +    struct xhci_hcd *xhci = hcd_to_xhci(hcd);
>>> +    unsigned int intr_num;
>>> +
>>> +    /* interrupter 0 is primary interrupter, don't touchit */
>>> +    if (!ir || !ir->intr_num || ir->intr_num >= xhci->max_interrupters) {
>>> +        xhci_dbg(xhci, "Invalid secondary interrupter, can't remove\n");
>>> +        return;
>>> +    }
>>> +
>>> +    /* fixme, should we check xhci->interrupter[intr_num] == ir */
>>> +    spin_lock(&xhci->lock);
>>
>> Needs to be spin_lock_irq() ir spin_lock_irqsave() as xhci->lock is used in interrupt handler.
>>
>>
>>> +    intr_num = ir->intr_num;
>>> +    xhci_free_interrupter(xhci, ir);
>>> +    xhci->interrupters[intr_num] = NULL;
>>> +    spin_unlock(&xhci->lock);
>>
>> likewise
>>
> 
> Let me check these again.  In general, I think I will use both the xhci->mutex and 
> xhci->lock where needed, because I believe we'd run into sleep while atomic issues
> while freeing the DMA memory.  Will rework this and submit in the next rev.
> 

Maybe we need to split xhci_free_interrupter() into separate remove and free functions

Did some work on this, and on the sideband api in general.

Code still has a lot of FIXMEs, and it's completely untested, but to avoid us
from doing duplicate work I pushed it to my feature_interrupters branch anyway

git://git.kernel.org/pub/scm/linux/kernel/git/mnyman/xhci.git feature_interrupters
https://git.kernel.org/pub/scm/linux/kernel/git/mnyman/xhci.git/log/?h=feature_interrupters

Thanks
-Mathias
Wesley Cheng Oct. 4, 2023, 6:35 p.m. UTC | #4 
Hi Mathias,

On 10/4/2023 7:02 AM, Mathias Nyman wrote:
> On 2.10.2023 23.07, Wesley Cheng wrote:
>> Hi Mathias,
>>
>> On 9/28/2023 3:31 AM, Mathias Nyman wrote:
>>> On 22.9.2023 0.48, Wesley Cheng wrote:
>>>> From: Mathias Nyman <mathias.nyman@linux.intel.com>
>>>>
>>>> Modify the XHCI drivers to accommodate for handling multiple event 
>>>> rings in
>>>> case there are multiple interrupters.  Add the required APIs so 
>>>> clients are
>>>> able to allocate/request for an interrupter ring, and pass this 
>>>> information
>>>> back to the client driver.  This allows for users to handle the 
>>>> resource
>>>> accordingly, such as passing the event ring base address to an audio 
>>>> DSP.
>>>> There is no actual support for multiple MSI/MSI-X vectors.
>>>>
>>>> Factoring out XHCI interrupter APIs and structures done by Wesley 
>>>> Cheng, in
>>>> order to allow for USB class drivers to utilze them.
>>>>
>>>>   }
>>>> +void xhci_remove_secondary_interrupter(struct usb_hcd *hcd, struct 
>>>> xhci_interrupter *ir)
>>>> +{
>>>> +    struct xhci_hcd *xhci = hcd_to_xhci(hcd);
>>>> +    unsigned int intr_num;
>>>> +
>>>> +    /* interrupter 0 is primary interrupter, don't touchit */
>>>> +    if (!ir || !ir->intr_num || ir->intr_num >= 
>>>> xhci->max_interrupters) {
>>>> +        xhci_dbg(xhci, "Invalid secondary interrupter, can't 
>>>> remove\n");
>>>> +        return;
>>>> +    }
>>>> +
>>>> +    /* fixme, should we check xhci->interrupter[intr_num] == ir */
>>>> +    spin_lock(&xhci->lock);
>>>
>>> Needs to be spin_lock_irq() ir spin_lock_irqsave() as xhci->lock is 
>>> used in interrupt handler.
>>>
>>>
>>>> +    intr_num = ir->intr_num;
>>>> +    xhci_free_interrupter(xhci, ir);
>>>> +    xhci->interrupters[intr_num] = NULL;
>>>> +    spin_unlock(&xhci->lock);
>>>
>>> likewise
>>>
>>
>> Let me check these again.  In general, I think I will use both the 
>> xhci->mutex and xhci->lock where needed, because I believe we'd run 
>> into sleep while atomic issues
>> while freeing the DMA memory.  Will rework this and submit in the next 
>> rev.
>>
> 
> Maybe we need to split xhci_free_interrupter() into separate remove and 
> free functions
> 

Thanks for sharing the work you've been doing.  Yes, I did something 
similar as well on my end, but will refactor in your code and re-test.

> Did some work on this, and on the sideband api in general.
> 
> Code still has a lot of FIXMEs, and it's completely untested, but to 
> avoid us
> from doing duplicate work I pushed it to my feature_interrupters branch 
> anyway
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/mnyman/xhci.git 
> feature_interrupters
> https://git.kernel.org/pub/scm/linux/kernel/git/mnyman/xhci.git/log/?h=feature_interrupters 
> 

Ok.  Initial look at it seems like it will be fine, but will integrate 
and make changes where needed.

Thanks
Wesley Cheng
Wesley Cheng Oct. 4, 2023, 11:42 p.m. UTC | #5 
Hi Mathias,

On 10/4/2023 11:35 AM, Wesley Cheng wrote:
> Hi Mathias,
> 
> On 10/4/2023 7:02 AM, Mathias Nyman wrote:
>> On 2.10.2023 23.07, Wesley Cheng wrote:
>>> Hi Mathias,
>>>
>>> On 9/28/2023 3:31 AM, Mathias Nyman wrote:
>>>> On 22.9.2023 0.48, Wesley Cheng wrote:
>>>>> From: Mathias Nyman <mathias.nyman@linux.intel.com>
>>>>>
>>>>> Modify the XHCI drivers to accommodate for handling multiple event 
>>>>> rings in
>>>>> case there are multiple interrupters.  Add the required APIs so 
>>>>> clients are
>>>>> able to allocate/request for an interrupter ring, and pass this 
>>>>> information
>>>>> back to the client driver.  This allows for users to handle the 
>>>>> resource
>>>>> accordingly, such as passing the event ring base address to an 
>>>>> audio DSP.
>>>>> There is no actual support for multiple MSI/MSI-X vectors.
>>>>>
>>>>> Factoring out XHCI interrupter APIs and structures done by Wesley 
>>>>> Cheng, in
>>>>> order to allow for USB class drivers to utilze them.
>>>>>
>>>>>   }
>>>>> +void xhci_remove_secondary_interrupter(struct usb_hcd *hcd, struct 
>>>>> xhci_interrupter *ir)
>>>>> +{
>>>>> +    struct xhci_hcd *xhci = hcd_to_xhci(hcd);
>>>>> +    unsigned int intr_num;
>>>>> +
>>>>> +    /* interrupter 0 is primary interrupter, don't touchit */
>>>>> +    if (!ir || !ir->intr_num || ir->intr_num >= 
>>>>> xhci->max_interrupters) {
>>>>> +        xhci_dbg(xhci, "Invalid secondary interrupter, can't 
>>>>> remove\n");
>>>>> +        return;
>>>>> +    }
>>>>> +
>>>>> +    /* fixme, should we check xhci->interrupter[intr_num] == ir */
>>>>> +    spin_lock(&xhci->lock);
>>>>
>>>> Needs to be spin_lock_irq() ir spin_lock_irqsave() as xhci->lock is 
>>>> used in interrupt handler.
>>>>
>>>>
>>>>> +    intr_num = ir->intr_num;
>>>>> +    xhci_free_interrupter(xhci, ir);
>>>>> +    xhci->interrupters[intr_num] = NULL;
>>>>> +    spin_unlock(&xhci->lock);
>>>>
>>>> likewise
>>>>
>>>
>>> Let me check these again.  In general, I think I will use both the 
>>> xhci->mutex and xhci->lock where needed, because I believe we'd run 
>>> into sleep while atomic issues
>>> while freeing the DMA memory.  Will rework this and submit in the 
>>> next rev.
>>>
>>
>> Maybe we need to split xhci_free_interrupter() into separate remove 
>> and free functions
>>
> 
> Thanks for sharing the work you've been doing.  Yes, I did something 
> similar as well on my end, but will refactor in your code and re-test.
> 
>> Did some work on this, and on the sideband api in general.
>>
>> Code still has a lot of FIXMEs, and it's completely untested, but to 
>> avoid us
>> from doing duplicate work I pushed it to my feature_interrupters 
>> branch anyway
>>
>> git://git.kernel.org/pub/scm/linux/kernel/git/mnyman/xhci.git 
>> feature_interrupters
>> https://git.kernel.org/pub/scm/linux/kernel/git/mnyman/xhci.git/log/?h=feature_interrupters 
>>
> 
> Ok.  Initial look at it seems like it will be fine, but will integrate 
> and make changes where needed.
> 

Had to make some minor tweaks here and there, but nothing major.  Was 
able to validate the changes on my end, and they look good.  Will test a 
bit more, and include these in my next submission.  Will try to address 
your FIXME tags as well.

Thanks
Wesley Cheng
diff mbox series

Patch

diff --git a/drivers/usb/host/xhci-debugfs.c b/drivers/usb/host/xhci-debugfs.c
index 99baa60ef50f..15a8402ee8a1 100644
--- a/drivers/usb/host/xhci-debugfs.c
+++ b/drivers/usb/host/xhci-debugfs.c
@@ -693,7 +693,7 @@  void xhci_debugfs_init(struct xhci_hcd *xhci)
 				     "command-ring",
 				     xhci->debugfs_root);
 
-	xhci_debugfs_create_ring_dir(xhci, &xhci->interrupter->event_ring,
+	xhci_debugfs_create_ring_dir(xhci, &xhci->interrupters[0]->event_ring,
 				     "event-ring",
 				     xhci->debugfs_root);
 
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 8714ab5bf04d..2f9228d7d22d 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -1837,6 +1837,26 @@  xhci_free_interrupter(struct xhci_hcd *xhci, struct xhci_interrupter *ir)
 	kfree(ir);
 }
 
+void xhci_remove_secondary_interrupter(struct usb_hcd *hcd, struct xhci_interrupter *ir)
+{
+	struct xhci_hcd *xhci = hcd_to_xhci(hcd);
+	unsigned int intr_num;
+
+	/* interrupter 0 is primary interrupter, don't touch it */
+	if (!ir || !ir->intr_num || ir->intr_num >= xhci->max_interrupters) {
+		xhci_dbg(xhci, "Invalid secondary interrupter, can't remove\n");
+		return;
+	}
+
+	/* fixme, should we check xhci->interrupter[intr_num] == ir */
+	spin_lock(&xhci->lock);
+	intr_num = ir->intr_num;
+	xhci_free_interrupter(xhci, ir);
+	xhci->interrupters[intr_num] = NULL;
+	spin_unlock(&xhci->lock);
+}
+EXPORT_SYMBOL_GPL(xhci_remove_secondary_interrupter);
+
 void xhci_mem_cleanup(struct xhci_hcd *xhci)
 {
 	struct device	*dev = xhci_to_hcd(xhci)->self.sysdev;
@@ -1844,9 +1864,13 @@  void xhci_mem_cleanup(struct xhci_hcd *xhci)
 
 	cancel_delayed_work_sync(&xhci->cmd_timer);
 
-	xhci_free_interrupter(xhci, xhci->interrupter);
-	xhci->interrupter = NULL;
-	xhci_dbg_trace(xhci, trace_xhci_dbg_init, "Freed primary event ring");
+	for (i = 0; i < xhci->max_interrupters; i++) {
+		if (xhci->interrupters[i]) {
+			xhci_free_interrupter(xhci, xhci->interrupters[i]);
+			xhci->interrupters[i] = NULL;
+		}
+	}
+	xhci_dbg_trace(xhci, trace_xhci_dbg_init, "Freed interrupters");
 
 	if (xhci->cmd_ring)
 		xhci_ring_free(xhci, xhci->cmd_ring);
@@ -1916,6 +1940,7 @@  void xhci_mem_cleanup(struct xhci_hcd *xhci)
 	for (i = 0; i < xhci->num_port_caps; i++)
 		kfree(xhci->port_caps[i].psi);
 	kfree(xhci->port_caps);
+	kfree(xhci->interrupters);
 	xhci->num_port_caps = 0;
 
 	xhci->usb2_rhub.ports = NULL;
@@ -1924,6 +1949,7 @@  void xhci_mem_cleanup(struct xhci_hcd *xhci)
 	xhci->rh_bw = NULL;
 	xhci->ext_caps = NULL;
 	xhci->port_caps = NULL;
+	xhci->interrupters = NULL;
 
 	xhci->page_size = 0;
 	xhci->page_shift = 0;
@@ -2276,6 +2302,13 @@  xhci_add_interrupter(struct xhci_hcd *xhci, struct xhci_interrupter *ir,
 		return -EINVAL;
 	}
 
+	if (xhci->interrupters[intr_num]) {
+		xhci_warn(xhci, "Interrupter %d\n already set up", intr_num);
+		return -EINVAL;
+	}
+
+	xhci->interrupters[intr_num] = ir;
+	ir->intr_num = intr_num;
 	ir->ir_set = &xhci->run_regs->ir_set[intr_num];
 
 	/* set ERST count with the number of entries in the segment table */
@@ -2295,10 +2328,53 @@  xhci_add_interrupter(struct xhci_hcd *xhci, struct xhci_interrupter *ir,
 	return 0;
 }
 
+struct xhci_interrupter *
+xhci_create_secondary_interrupter(struct usb_hcd *hcd)
+{
+	struct xhci_hcd *xhci = hcd_to_xhci(hcd);
+	struct xhci_interrupter *ir;
+	unsigned int i;
+	int err = -ENOSPC;
+
+	if (!xhci->interrupters)
+		return NULL;
+
+	ir = xhci_alloc_interrupter(xhci, GFP_KERNEL);
+	if (!ir)
+		return NULL;
+
+	spin_lock_irq(&xhci->lock);
+
+	/* Find available secondary interrupter, interrupter 0 is reserverd for primary */
+	for (i = 1; i < xhci->max_interrupters; i++) {
+		if (xhci->interrupters[i] == NULL) {
+			err = xhci_add_interrupter(xhci, ir, i);
+			break;
+		}
+	}
+
+	spin_unlock_irq(&xhci->lock);
+	if (err) {
+		xhci_warn(xhci, "Failed to add secondary interrupter, max interrupters %d\n",
+			xhci->max_interrupters);
+		xhci_free_interrupter(xhci, ir);
+		ir = NULL;
+		goto out;
+	}
+
+	xhci_dbg(xhci, "Add secondary interrupter %d, max interrupters %d\n",
+		 i, xhci->max_interrupters);
+
+out:
+	return ir;
+}
+EXPORT_SYMBOL_GPL(xhci_create_secondary_interrupter);
+
 int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
 {
-	dma_addr_t	dma;
+	struct xhci_interrupter *ir;
 	struct device	*dev = xhci_to_hcd(xhci)->self.sysdev;
+	dma_addr_t	dma;
 	unsigned int	val, val2;
 	u64		val_64;
 	u32		page_size, temp;
@@ -2422,11 +2498,14 @@  int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
 	/* Allocate and set up primary interrupter 0 with an event ring. */
 	xhci_dbg_trace(xhci, trace_xhci_dbg_init,
 		       "Allocating primary event ring");
-	xhci->interrupter = xhci_alloc_interrupter(xhci, flags);
-	if (!xhci->interrupter)
+	xhci->interrupters = kcalloc_node(xhci->max_interrupters, sizeof(*xhci->interrupters),
+					  flags, dev_to_node(dev));
+
+	ir = xhci_alloc_interrupter(xhci, flags);
+	if (!ir)
 		goto fail;
 
-	if (xhci_add_interrupter(xhci, xhci->interrupter, 0))
+	if (xhci_add_interrupter(xhci, ir, 0))
 		goto fail;
 
 	xhci->isoc_bei_interval = AVOID_BEI_INTERVAL_MAX;
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 1dde53f6eb31..93233cf5ff21 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -3074,7 +3074,7 @@  irqreturn_t xhci_irq(struct usb_hcd *hcd)
 	writel(status, &xhci->op_regs->status);
 
 	/* This is the handler of the primary interrupter */
-	ir = xhci->interrupter;
+	ir = xhci->interrupters[0];
 	if (!hcd->msi_enabled) {
 		u32 irq_pending;
 		irq_pending = readl(&ir->ir_set->irq_pending);
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index e1b1b64a0723..3fd2b58ee1d3 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -456,7 +456,7 @@  static int xhci_init(struct usb_hcd *hcd)
 
 static int xhci_run_finished(struct xhci_hcd *xhci)
 {
-	struct xhci_interrupter *ir = xhci->interrupter;
+	struct xhci_interrupter *ir = xhci->interrupters[0];
 	unsigned long	flags;
 	u32		temp;
 
@@ -508,7 +508,7 @@  int xhci_run(struct usb_hcd *hcd)
 	u64 temp_64;
 	int ret;
 	struct xhci_hcd *xhci = hcd_to_xhci(hcd);
-	struct xhci_interrupter *ir = xhci->interrupter;
+	struct xhci_interrupter *ir = xhci->interrupters[0];
 	/* Start the xHCI host controller running only after the USB 2.0 roothub
 	 * is setup.
 	 */
@@ -572,7 +572,7 @@  void xhci_stop(struct usb_hcd *hcd)
 {
 	u32 temp;
 	struct xhci_hcd *xhci = hcd_to_xhci(hcd);
-	struct xhci_interrupter *ir = xhci->interrupter;
+	struct xhci_interrupter *ir = xhci->interrupters[0];
 
 	mutex_lock(&xhci->mutex);
 
@@ -668,36 +668,49 @@  EXPORT_SYMBOL_GPL(xhci_shutdown);
 #ifdef CONFIG_PM
 static void xhci_save_registers(struct xhci_hcd *xhci)
 {
-	struct xhci_interrupter *ir = xhci->interrupter;
+	struct xhci_interrupter *ir;
+	unsigned int i;
 
 	xhci->s3.command = readl(&xhci->op_regs->command);
 	xhci->s3.dev_nt = readl(&xhci->op_regs->dev_notification);
 	xhci->s3.dcbaa_ptr = xhci_read_64(xhci, &xhci->op_regs->dcbaa_ptr);
 	xhci->s3.config_reg = readl(&xhci->op_regs->config_reg);
 
-	if (!ir)
-		return;
+	/* save both primary and all secondary interrupters */
+	for (i = 0; i < xhci->max_interrupters; i++) {
+		ir = xhci->interrupters[i];
+		if (!ir)
+			continue;
 
-	ir->s3_erst_size = readl(&ir->ir_set->erst_size);
-	ir->s3_erst_base = xhci_read_64(xhci, &ir->ir_set->erst_base);
-	ir->s3_erst_dequeue = xhci_read_64(xhci, &ir->ir_set->erst_dequeue);
-	ir->s3_irq_pending = readl(&ir->ir_set->irq_pending);
-	ir->s3_irq_control = readl(&ir->ir_set->irq_control);
+		ir->s3_erst_size = readl(&ir->ir_set->erst_size);
+		ir->s3_erst_base = xhci_read_64(xhci, &ir->ir_set->erst_base);
+		ir->s3_erst_dequeue = xhci_read_64(xhci, &ir->ir_set->erst_dequeue);
+		ir->s3_irq_pending = readl(&ir->ir_set->irq_pending);
+		ir->s3_irq_control = readl(&ir->ir_set->irq_control);
+	}
 }
 
 static void xhci_restore_registers(struct xhci_hcd *xhci)
 {
-	struct xhci_interrupter *ir = xhci->interrupter;
+	struct xhci_interrupter *ir;
+	unsigned int i;
 
 	writel(xhci->s3.command, &xhci->op_regs->command);
 	writel(xhci->s3.dev_nt, &xhci->op_regs->dev_notification);
 	xhci_write_64(xhci, xhci->s3.dcbaa_ptr, &xhci->op_regs->dcbaa_ptr);
 	writel(xhci->s3.config_reg, &xhci->op_regs->config_reg);
-	writel(ir->s3_erst_size, &ir->ir_set->erst_size);
-	xhci_write_64(xhci, ir->s3_erst_base, &ir->ir_set->erst_base);
-	xhci_write_64(xhci, ir->s3_erst_dequeue, &ir->ir_set->erst_dequeue);
-	writel(ir->s3_irq_pending, &ir->ir_set->irq_pending);
-	writel(ir->s3_irq_control, &ir->ir_set->irq_control);
+
+	for (i = 0; i < xhci->max_interrupters; i++) {
+		ir = xhci->interrupters[i];
+		if (!ir)
+			continue;
+
+		writel(ir->s3_erst_size, &ir->ir_set->erst_size);
+		xhci_write_64(xhci, ir->s3_erst_base, &ir->ir_set->erst_base);
+		xhci_write_64(xhci, ir->s3_erst_dequeue, &ir->ir_set->erst_dequeue);
+		writel(ir->s3_irq_pending, &ir->ir_set->irq_pending);
+		writel(ir->s3_irq_control, &ir->ir_set->irq_control);
+	}
 }
 
 static void xhci_set_cmd_ring_deq(struct xhci_hcd *xhci)
@@ -1059,7 +1072,7 @@  int xhci_resume(struct xhci_hcd *xhci, pm_message_t msg)
 		xhci_dbg(xhci, "// Disabling event ring interrupts\n");
 		temp = readl(&xhci->op_regs->status);
 		writel((temp & ~0x1fff) | STS_EINT, &xhci->op_regs->status);
-		xhci_disable_interrupter(xhci->interrupter);
+		xhci_disable_interrupter(xhci->interrupters[0]);
 
 		xhci_dbg(xhci, "cleaning up memory\n");
 		xhci_mem_cleanup(xhci);
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 7e282b4522c0..d706a27ec0a3 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -17,6 +17,7 @@ 
 #include <linux/kernel.h>
 #include <linux/usb/hcd.h>
 #include <linux/io-64-nonatomic-lo-hi.h>
+#include <linux/usb/xhci-intr.h>
 
 /* Code sharing between pci-quirks and xhci hcd */
 #include	"xhci-ext-caps.h"
@@ -1541,18 +1542,6 @@  static inline const char *xhci_trb_type_string(u8 type)
 #define AVOID_BEI_INTERVAL_MIN	8
 #define AVOID_BEI_INTERVAL_MAX	32
 
-struct xhci_segment {
-	union xhci_trb		*trbs;
-	/* private to HCD */
-	struct xhci_segment	*next;
-	dma_addr_t		dma;
-	/* Max packet sized bounce buffer for td-fragmant alignment */
-	dma_addr_t		bounce_dma;
-	void			*bounce_buf;
-	unsigned int		bounce_offs;
-	unsigned int		bounce_len;
-};
-
 enum xhci_cancelled_td_status {
 	TD_DIRTY = 0,
 	TD_HALTED,
@@ -1585,16 +1574,6 @@  struct xhci_cd {
 	union xhci_trb		*cmd_trb;
 };
 
-enum xhci_ring_type {
-	TYPE_CTRL = 0,
-	TYPE_ISOC,
-	TYPE_BULK,
-	TYPE_INTR,
-	TYPE_STREAM,
-	TYPE_COMMAND,
-	TYPE_EVENT,
-};
-
 static inline const char *xhci_ring_type_string(enum xhci_ring_type type)
 {
 	switch (type) {
@@ -1615,46 +1594,6 @@  static inline const char *xhci_ring_type_string(enum xhci_ring_type type)
 	}
 
 	return "UNKNOWN";
-}
-
-struct xhci_ring {
-	struct xhci_segment	*first_seg;
-	struct xhci_segment	*last_seg;
-	union  xhci_trb		*enqueue;
-	struct xhci_segment	*enq_seg;
-	union  xhci_trb		*dequeue;
-	struct xhci_segment	*deq_seg;
-	struct list_head	td_list;
-	/*
-	 * Write the cycle state into the TRB cycle field to give ownership of
-	 * the TRB to the host controller (if we are the producer), or to check
-	 * if we own the TRB (if we are the consumer).  See section 4.9.1.
-	 */
-	u32			cycle_state;
-	unsigned int		stream_id;
-	unsigned int		num_segs;
-	unsigned int		num_trbs_free; /* used only by xhci DbC */
-	unsigned int		bounce_buf_len;
-	enum xhci_ring_type	type;
-	bool			last_td_was_short;
-	struct radix_tree_root	*trb_address_map;
-};
-
-struct xhci_erst_entry {
-	/* 64-bit event ring segment address */
-	__le64	seg_addr;
-	__le32	seg_size;
-	/* Set to zero */
-	__le32	rsvd;
-};
-
-struct xhci_erst {
-	struct xhci_erst_entry	*entries;
-	unsigned int		num_entries;
-	/* xhci->event_ring keeps track of segment dma addresses */
-	dma_addr_t		erst_dma_addr;
-	/* Num entries the ERST can contain */
-	unsigned int		erst_size;
 };
 
 struct xhci_scratchpad {
@@ -1707,18 +1646,6 @@  struct xhci_bus_state {
 	unsigned long		resuming_ports;
 };
 
-struct xhci_interrupter {
-	struct xhci_ring	*event_ring;
-	struct xhci_erst	erst;
-	struct xhci_intr_reg __iomem *ir_set;
-	unsigned int		intr_num;
-	/* For interrupter registers save and restore over suspend/resume */
-	u32	s3_irq_pending;
-	u32	s3_irq_control;
-	u32	s3_erst_size;
-	u64	s3_erst_base;
-	u64	s3_erst_dequeue;
-};
 /*
  * It can take up to 20 ms to transition from RExit to U0 on the
  * Intel Lynx Point LP xHCI host.
@@ -1799,7 +1726,7 @@  struct xhci_hcd {
 	struct reset_control *reset;
 	/* data structures */
 	struct xhci_device_context_array *dcbaa;
-	struct xhci_interrupter *interrupter;
+	struct xhci_interrupter **interrupters;
 	struct xhci_ring	*cmd_ring;
 	unsigned int            cmd_ring_state;
 #define CMD_RING_STATE_RUNNING         (1 << 0)
diff --git a/include/linux/usb/xhci-intr.h b/include/linux/usb/xhci-intr.h
new file mode 100644
index 000000000000..e0091ee2c73a
--- /dev/null
+++ b/include/linux/usb/xhci-intr.h
@@ -0,0 +1,86 @@ 
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __LINUX_XHCI_INTR_H
+#define __LINUX_XHCI_INTR_H
+
+#include <linux/kernel.h>
+
+struct xhci_erst_entry {
+	/* 64-bit event ring segment address */
+	__le64	seg_addr;
+	__le32	seg_size;
+	/* Set to zero */
+	__le32	rsvd;
+};
+
+enum xhci_ring_type {
+	TYPE_CTRL = 0,
+	TYPE_ISOC,
+	TYPE_BULK,
+	TYPE_INTR,
+	TYPE_STREAM,
+	TYPE_COMMAND,
+	TYPE_EVENT,
+};
+
+struct xhci_erst {
+	struct xhci_erst_entry	*entries;
+	unsigned int		num_entries;
+	/* xhci->event_ring keeps track of segment dma addresses */
+	dma_addr_t		erst_dma_addr;
+	/* Num entries the ERST can contain */
+	unsigned int		erst_size;
+};
+
+struct xhci_segment {
+	union xhci_trb		*trbs;
+	/* private to HCD */
+	struct xhci_segment	*next;
+	dma_addr_t		dma;
+	/* Max packet sized bounce buffer for td-fragmant alignment */
+	dma_addr_t		bounce_dma;
+	void			*bounce_buf;
+	unsigned int		bounce_offs;
+	unsigned int		bounce_len;
+};
+
+struct xhci_ring {
+	struct xhci_segment	*first_seg;
+	struct xhci_segment	*last_seg;
+	union  xhci_trb		*enqueue;
+	struct xhci_segment	*enq_seg;
+	union  xhci_trb		*dequeue;
+	struct xhci_segment	*deq_seg;
+	struct list_head	td_list;
+	/*
+	 * Write the cycle state into the TRB cycle field to give ownership of
+	 * the TRB to the host controller (if we are the producer), or to check
+	 * if we own the TRB (if we are the consumer).  See section 4.9.1.
+	 */
+	u32			cycle_state;
+	unsigned int		stream_id;
+	unsigned int		num_segs;
+	unsigned int		num_trbs_free;
+	unsigned int		num_trbs_free_temp;
+	unsigned int		bounce_buf_len;
+	enum xhci_ring_type	type;
+	bool			last_td_was_short;
+	struct radix_tree_root	*trb_address_map;
+};
+
+struct xhci_interrupter {
+	struct xhci_ring	*event_ring;
+	struct xhci_erst	erst;
+	struct xhci_intr_reg __iomem *ir_set;
+	unsigned int		intr_num;
+	/* For interrupter registers save and restore over suspend/resume */
+	u32	s3_irq_pending;
+	u32	s3_irq_control;
+	u32	s3_erst_size;
+	u64	s3_erst_base;
+	u64	s3_erst_dequeue;
+};
+
+struct xhci_interrupter *
+xhci_create_secondary_interrupter(struct usb_hcd *hcd);
+void xhci_remove_secondary_interrupter(struct usb_hcd *hcd, struct xhci_interrupter *ir);
+#endif