diff mbox series

[v3,01/28] xhci: Add support to allocate several interrupters

Message ID 20230308235751.495-2-quic_wcheng@quicinc.com (mailing list archive)
State Not Applicable
Headers show
Series Introduce QC USB SND audio offloading support | expand

Commit Message

Wesley Cheng March 8, 2023, 11:57 p.m. UTC
From: Mathias Nyman <mathias.nyman@linux.intel.com>

Introduce xHCI APIs to allow for clients to allocate and free
interrupters.  This allocates an array of interrupters, which is based on
the max_interrupters parameter.  The primary interrupter is set as the
first entry in the array, and secondary interrupters following after.

Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com>
---
 drivers/usb/host/xhci-debugfs.c |  2 +-
 drivers/usb/host/xhci-mem.c     | 97 +++++++++++++++++++++++++++++++--
 drivers/usb/host/xhci-ring.c    |  2 +-
 drivers/usb/host/xhci.c         | 51 +++++++++++------
 drivers/usb/host/xhci.h         |  2 +-
 include/linux/usb/xhci-intr.h   | 86 +++++++++++++++++++++++++++++
 6 files changed, 214 insertions(+), 26 deletions(-)
 create mode 100644 include/linux/usb/xhci-intr.h

Comments

Oliver Neukum March 9, 2023, 10:34 a.m. UTC | #1
On 09.03.23 00:57, Wesley Cheng wrote:

> @@ -1865,8 +1889,15 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci)
>   
>   	cancel_delayed_work_sync(&xhci->cmd_timer);
>   
> -	xhci_free_interrupter(xhci, xhci->interrupter);
> -	xhci->interrupter = NULL;
> +	for (i = 1; i < xhci->max_interrupters; i++) {
> +		if (xhci->interrupters[i])
> +			xhci_remove_secondary_interrupter(xhci_to_hcd(xhci),
> +							  xhci->interrupters[i]);
> +	}
> +
> +	/* free the primary interrupter, interrupter number 0 */
> +	xhci_free_interrupter(xhci, xhci->interrupters[0]);
> +	xhci->interrupters[0] = NULL;
>   	xhci_dbg_trace(xhci, trace_xhci_dbg_init, "Freed primary event ring");

Any reason you are not just counting down to zero?

> +	if (idx > 0) {
> +		ir = xhci_alloc_interrupter(xhci, idx, GFP_KERNEL);
> +		if (!ir) {
> +			spin_unlock_irqrestore(&xhci->lock, flags);

Why use _irqrestore?

	Regards
		Oliver
Takashi Iwai March 9, 2023, 10:51 a.m. UTC | #2
On Thu, 09 Mar 2023 00:57:24 +0100,
Wesley Cheng wrote:
> +struct xhci_interrupter *
> +xhci_create_secondary_interrupter(struct usb_hcd *hcd, int intr_num)
> +{
> +	struct xhci_hcd *xhci = hcd_to_xhci(hcd);
> +	struct xhci_interrupter *ir;
> +	unsigned int i;
> +	unsigned int idx = 0;
> +	unsigned long flags;
> +
> +	if (!xhci->interrupters || intr_num > xhci->max_interrupters)
> +		return NULL;
> +
> +	spin_lock_irqsave(&xhci->lock, flags);
....
> +	if (idx > 0) {
> +		ir = xhci_alloc_interrupter(xhci, idx, GFP_KERNEL);
> +		if (!ir) {
> +			spin_unlock_irqrestore(&xhci->lock, flags);
> +			return NULL;
> +		}
> +		ir->intr_num = idx;
> +		xhci->interrupters[idx] = ir;
> +		spin_unlock_irqrestore(&xhci->lock, flags);

You can't use GFP_KERNEL allocation inside the spinlock.


Takashi
Mathias Nyman March 10, 2023, 3:07 p.m. UTC | #3
On 9.3.2023 1.57, Wesley Cheng wrote:
> From: Mathias Nyman <mathias.nyman@linux.intel.com>
> 
> Introduce xHCI APIs to allow for clients to allocate and free
> interrupters.  This allocates an array of interrupters, which is based on
> the max_interrupters parameter.  The primary interrupter is set as the
> first entry in the array, and secondary interrupters following after.
> 

I'm thinking about changing this offloading xHCI API
xhci should be aware and keep track of which devices and endpoints that
are offloaded to avoid device getting offloaded twice, avoid xhci driver
from queuing anything itself for these, and act properly if the offloaded
device or entire host is removed.

So first thing audio side would need to do do is register/create an
offload entry for the device using the API:

struct xhci_sideband *xhci_sideband_register(struct usb_device *udev)

(xHCI specs calls offload sideband)
Then endpoints and interrupters can be added and removed from this
offload entry

I have some early thoughts written as non-compiling code in:

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

Let me know what you think about this.

> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
> Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com>

My Signed-off-by tag is being misused here.

I wrote a chunk of the code in this patch as PoC that I shared in a separate topic branch.
It was incomplete and not intended for upstream yet. (lacked locking, several fixme parts, etc..)
The rest of the code in this patch is completely new to me.

Thanks
-Mathias
Wesley Cheng March 13, 2023, 8:08 p.m. UTC | #4
Hi Mathias,

On 3/10/2023 7:07 AM, Mathias Nyman wrote:
> On 9.3.2023 1.57, Wesley Cheng wrote:
>> From: Mathias Nyman <mathias.nyman@linux.intel.com>
>>
>> Introduce xHCI APIs to allow for clients to allocate and free
>> interrupters.  This allocates an array of interrupters, which is based on
>> the max_interrupters parameter.  The primary interrupter is set as the
>> first entry in the array, and secondary interrupters following after.
>>
> 
> I'm thinking about changing this offloading xHCI API
> xhci should be aware and keep track of which devices and endpoints that
> are offloaded to avoid device getting offloaded twice, avoid xhci driver
> from queuing anything itself for these, and act properly if the offloaded
> device or entire host is removed.
> 
> So first thing audio side would need to do do is register/create an
> offload entry for the device using the API:
> 
> struct xhci_sideband *xhci_sideband_register(struct usb_device *udev)
> 
> (xHCI specs calls offload sideband)
> Then endpoints and interrupters can be added and removed from this
> offload entry
> 
> I have some early thoughts written as non-compiling code in:
> 
> 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 
> 
> 
> Let me know what you think about this.
> 

The concept/framework you built looks good to me.  Makes sense to have 
XHCI better maintain the offloading users.  One thing I would request is 
to move xhci-sideband.h to the include directory since the class driver 
levels would need to be able to reference the structure and APIs you've 
exposed.

I have yet to try it with our implementation, but I'll work on plugging 
it in and fix any issues I see along the way.

Thanks
Wesley Cheng
Wesley Cheng March 13, 2023, 8:32 p.m. UTC | #5
Hi Mathias,

On 3/10/2023 7:07 AM, Mathias Nyman wrote:
> On 9.3.2023 1.57, Wesley Cheng wrote:
>> From: Mathias Nyman <mathias.nyman@linux.intel.com>
>>
>> Introduce xHCI APIs to allow for clients to allocate and free
>> interrupters.  This allocates an array of interrupters, which is based on
>> the max_interrupters parameter.  The primary interrupter is set as the
>> first entry in the array, and secondary interrupters following after.
>>
> 
> I'm thinking about changing this offloading xHCI API
> xhci should be aware and keep track of which devices and endpoints that
> are offloaded to avoid device getting offloaded twice, avoid xhci driver
> from queuing anything itself for these, and act properly if the offloaded
> device or entire host is removed.
> 
> So first thing audio side would need to do do is register/create an
> offload entry for the device using the API:
> 
> struct xhci_sideband *xhci_sideband_register(struct usb_device *udev)
> 
> (xHCI specs calls offload sideband)
> Then endpoints and interrupters can be added and removed from this
> offload entry
> 
> I have some early thoughts written as non-compiling code in:
> 
> 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 
> 
> 
> Let me know what you think about this.
> 
>> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
>> Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com>
> 
> My Signed-off-by tag is being misused here.
> 
> I wrote a chunk of the code in this patch as PoC that I shared in a 
> separate topic branch.
> It was incomplete and not intended for upstream yet. (lacked locking, 
> several fixme parts, etc..)
> The rest of the code in this patch is completely new to me.
> 

Sorry about this.  I cherry picked the change directly from your branch, 
so it carried your signed off tag with it.  Will make to include them 
properly next time.

Thanks
Wesley Cheng
Wesley Cheng April 25, 2023, 1:17 a.m. UTC | #6
Hi Mathias,

On 3/13/2023 1:08 PM, Wesley Cheng wrote:
> Hi Mathias,
> 
> On 3/10/2023 7:07 AM, Mathias Nyman wrote:
>> On 9.3.2023 1.57, Wesley Cheng wrote:
>>> From: Mathias Nyman <mathias.nyman@linux.intel.com>
>>>
>>> Introduce xHCI APIs to allow for clients to allocate and free
>>> interrupters.  This allocates an array of interrupters, which is 
>>> based on
>>> the max_interrupters parameter.  The primary interrupter is set as the
>>> first entry in the array, and secondary interrupters following after.
>>>
>>
>> I'm thinking about changing this offloading xHCI API
>> xhci should be aware and keep track of which devices and endpoints that
>> are offloaded to avoid device getting offloaded twice, avoid xhci driver
>> from queuing anything itself for these, and act properly if the offloaded
>> device or entire host is removed.
>>
>> So first thing audio side would need to do do is register/create an
>> offload entry for the device using the API:
>>
>> struct xhci_sideband *xhci_sideband_register(struct usb_device *udev)
>>
>> (xHCI specs calls offload sideband)
>> Then endpoints and interrupters can be added and removed from this
>> offload entry
>>
>> I have some early thoughts written as non-compiling code in:
>>
>> 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 
>>
>>
>> Let me know what you think about this.
>>
> 
> The concept/framework you built looks good to me.  Makes sense to have 
> XHCI better maintain the offloading users.  One thing I would request is 
> to move xhci-sideband.h to the include directory since the class driver 
> levels would need to be able to reference the structure and APIs you've 
> exposed.
> 
> I have yet to try it with our implementation, but I'll work on plugging 
> it in and fix any issues I see along the way.

Sorry for the late reply on some of the efforts on adding your new 
xhci-sideband driver.

I saw your comments with respect to building the SG table for rings with 
multiple segments, ie stream xfer rings.  I had tried some things to 
achieve the page links, but after reviewing some of the Linux memory 
APIs, I'm not sure we can achieve it.  This is because we're not simply 
relying on the direct DMA ops here to build the SG table.  In the IOMMU 
mapped cases, it calls in iommu_dma_get_sgtable(), which has some 
convoluted logic to build the sgt.

Instead of allocating one sgt with multiple sgls (based on # of ring 
segments), would it make sense to just build multiple sgts for each ring 
segment?  The vendor class driver could still fetch the required memory 
information to translate each sgt to a physical address and ring segment 
linking can happen within the external DSP if it supports it.

Thanks
Wesley Cheng
Wesley Cheng June 23, 2023, 10:37 p.m. UTC | #7
Hi Mathias,

On 3/13/2023 1:32 PM, Wesley Cheng wrote:
> Hi Mathias,
> 
> On 3/10/2023 7:07 AM, Mathias Nyman wrote:
>> On 9.3.2023 1.57, Wesley Cheng wrote:
>>> From: Mathias Nyman <mathias.nyman@linux.intel.com>
>>>
>>> Introduce xHCI APIs to allow for clients to allocate and free
>>> interrupters.  This allocates an array of interrupters, which is 
>>> based on
>>> the max_interrupters parameter.  The primary interrupter is set as the
>>> first entry in the array, and secondary interrupters following after.
>>>
>>
>> I'm thinking about changing this offloading xHCI API
>> xhci should be aware and keep track of which devices and endpoints that
>> are offloaded to avoid device getting offloaded twice, avoid xhci driver
>> from queuing anything itself for these, and act properly if the offloaded
>> device or entire host is removed.
>>
>> So first thing audio side would need to do do is register/create an
>> offload entry for the device using the API:
>>
>> struct xhci_sideband *xhci_sideband_register(struct usb_device *udev)
>>
>> (xHCI specs calls offload sideband)
>> Then endpoints and interrupters can be added and removed from this
>> offload entry
>>
>> I have some early thoughts written as non-compiling code in:
>>
>> 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 
>>
>>
>> Let me know what you think about this.
>>
>>> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
>>> Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com>
>>
>> My Signed-off-by tag is being misused here.
>>
>> I wrote a chunk of the code in this patch as PoC that I shared in a 
>> separate topic branch.
>> It was incomplete and not intended for upstream yet. (lacked locking, 
>> several fixme parts, etc..)
>> The rest of the code in this patch is completely new to me.
>>
> 
> Sorry about this.  I cherry picked the change directly from your branch, 
> so it carried your signed off tag with it.  Will make to include them 
> properly next time.
> 

I'm about ready to submit the next revision for this set of changes, and 
I was wondering how we should handle the changes you made on:
https://git.kernel.org/pub/scm/linux/kernel/git/mnyman/xhci.git/log/?h=feature_interrupters

I did make some modifications to some of the interrupter fixme tags you 
had, and also updated the xhci-sideband APIs with the proper logic.  I 
don't believe it is correct for me to submit a set of patches authored 
by you without your signed off tag. (checkpatch throws an error saying 
the author did not sign off on the change)

Thanks
Wesley Cheng
Mathias Nyman June 26, 2023, 1:55 p.m. UTC | #8
On 24.6.2023 1.37, Wesley Cheng wrote:
> Hi Mathias,
> 
> On 3/13/2023 1:32 PM, Wesley Cheng wrote:
>> Hi Mathias,
>>
>> On 3/10/2023 7:07 AM, Mathias Nyman wrote:
>>> On 9.3.2023 1.57, Wesley Cheng wrote:
>>>> From: Mathias Nyman <mathias.nyman@linux.intel.com>
>>>>
>>>> Introduce xHCI APIs to allow for clients to allocate and free
>>>> interrupters.  This allocates an array of interrupters, which is based on
>>>> the max_interrupters parameter.  The primary interrupter is set as the
>>>> first entry in the array, and secondary interrupters following after.
>>>>
>>>
>>> I'm thinking about changing this offloading xHCI API
>>> xhci should be aware and keep track of which devices and endpoints that
>>> are offloaded to avoid device getting offloaded twice, avoid xhci driver
>>> from queuing anything itself for these, and act properly if the offloaded
>>> device or entire host is removed.
>>>
>>> So first thing audio side would need to do do is register/create an
>>> offload entry for the device using the API:
>>>
>>> struct xhci_sideband *xhci_sideband_register(struct usb_device *udev)
>>>
>>> (xHCI specs calls offload sideband)
>>> Then endpoints and interrupters can be added and removed from this
>>> offload entry
>>>
>>> I have some early thoughts written as non-compiling code in:
>>>
>>> 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
>>>
>>> Let me know what you think about this.
>>>
>>>> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
>>>> Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com>
>>>
>>> My Signed-off-by tag is being misused here.
>>>
>>> I wrote a chunk of the code in this patch as PoC that I shared in a separate topic branch.
>>> It was incomplete and not intended for upstream yet. (lacked locking, several fixme parts, etc..)
>>> The rest of the code in this patch is completely new to me.
>>>
>>
>> Sorry about this.  I cherry picked the change directly from your branch, so it carried your signed off tag with it.  Will make to include them properly next time.
>>
> 
> I'm about ready to submit the next revision for this set of changes, and I was wondering how we should handle the changes you made on:
> https://git.kernel.org/pub/scm/linux/kernel/git/mnyman/xhci.git/log/?h=feature_interrupters
> 
> I did make some modifications to some of the interrupter fixme tags you had, and also updated the xhci-sideband APIs with the proper logic.  I don't believe it is correct for me to submit a set of patches authored by you without your signed off tag. (checkpatch throws an error saying the author did not sign off on the change)
> 

Note that the first patch "xhci: split allocate interrupter into separate alloacte and add parts"
is already in usb-next on its way to 6.5

Maybe Co-developed-by would work in this case, with a small explanation at the end of the commit message.
Something like:

Locking, DMA something and feataure x added by Wesley Cheng to
complete original concept code by Mathias

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>

Thanks
-Mathias
Wesley Cheng June 26, 2023, 3:05 p.m. UTC | #9
Hi Mathias,

On 6/26/2023 6:55 AM, Mathias Nyman wrote:
> On 24.6.2023 1.37, Wesley Cheng wrote:
>> Hi Mathias,
>>
>> On 3/13/2023 1:32 PM, Wesley Cheng wrote:
>>> Hi Mathias,
>>>
>>> On 3/10/2023 7:07 AM, Mathias Nyman wrote:
>>>> On 9.3.2023 1.57, Wesley Cheng wrote:
>>>>> From: Mathias Nyman <mathias.nyman@linux.intel.com>
>>>>>
>>>>> Introduce xHCI APIs to allow for clients to allocate and free
>>>>> interrupters.  This allocates an array of interrupters, which is 
>>>>> based on
>>>>> the max_interrupters parameter.  The primary interrupter is set as the
>>>>> first entry in the array, and secondary interrupters following after.
>>>>>
>>>>
>>>> I'm thinking about changing this offloading xHCI API
>>>> xhci should be aware and keep track of which devices and endpoints that
>>>> are offloaded to avoid device getting offloaded twice, avoid xhci 
>>>> driver
>>>> from queuing anything itself for these, and act properly if the 
>>>> offloaded
>>>> device or entire host is removed.
>>>>
>>>> So first thing audio side would need to do do is register/create an
>>>> offload entry for the device using the API:
>>>>
>>>> struct xhci_sideband *xhci_sideband_register(struct usb_device *udev)
>>>>
>>>> (xHCI specs calls offload sideband)
>>>> Then endpoints and interrupters can be added and removed from this
>>>> offload entry
>>>>
>>>> I have some early thoughts written as non-compiling code in:
>>>>
>>>> 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 
>>>>
>>>>
>>>> Let me know what you think about this.
>>>>
>>>>> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
>>>>> Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com>
>>>>
>>>> My Signed-off-by tag is being misused here.
>>>>
>>>> I wrote a chunk of the code in this patch as PoC that I shared in a 
>>>> separate topic branch.
>>>> It was incomplete and not intended for upstream yet. (lacked 
>>>> locking, several fixme parts, etc..)
>>>> The rest of the code in this patch is completely new to me.
>>>>
>>>
>>> Sorry about this.  I cherry picked the change directly from your 
>>> branch, so it carried your signed off tag with it.  Will make to 
>>> include them properly next time.
>>>
>>
>> I'm about ready to submit the next revision for this set of changes, 
>> and I was wondering how we should handle the changes you made on:
>> https://git.kernel.org/pub/scm/linux/kernel/git/mnyman/xhci.git/log/?h=feature_interrupters 
>>
>>
>> I did make some modifications to some of the interrupter fixme tags 
>> you had, and also updated the xhci-sideband APIs with the proper 
>> logic.  I don't believe it is correct for me to submit a set of 
>> patches authored by you without your signed off tag. (checkpatch 
>> throws an error saying the author did not sign off on the change)
>>
> 
> Note that the first patch "xhci: split allocate interrupter into 
> separate alloacte and add parts"
> is already in usb-next on its way to 6.5
> 
> Maybe Co-developed-by would work in this case, with a small explanation 
> at the end of the commit message.
> Something like:
> 
> Locking, DMA something and feataure x added by Wesley Cheng to
> complete original concept code by Mathias
> 
> 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>
> 

Sounds good!  Thanks for helping with a non-technical question :).  Just 
wanted to make sure I wasn't overstepping anywhere.

Thanks
Wesley Cheng
diff mbox series

Patch

diff --git a/drivers/usb/host/xhci-debugfs.c b/drivers/usb/host/xhci-debugfs.c
index 0bc7fe11f749..06a42b68446f 100644
--- a/drivers/usb/host/xhci-debugfs.c
+++ b/drivers/usb/host/xhci-debugfs.c
@@ -692,7 +692,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 d0a9467aa5fc..c303a8e1a33d 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -1858,6 +1858,30 @@  xhci_free_interrupter(struct xhci_hcd *xhci, struct xhci_interrupter *ir)
 	kfree(ir);
 }
 
+/*
+ * Free a secondary interrupter slot.  This will allow for other users to request for
+ * the secondary interrupter in the future.
+ */
+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;
+	unsigned long flags;
+
+	if (!ir || !ir->intr_num || ir->intr_num > xhci->max_interrupters) {
+		xhci_dbg(xhci, "Invalid secondary interrupter, can't remove\n");
+		return;
+	}
+
+	/* fixme, shuld we check xhci->interrupter[intr_num] == ir */
+	intr_num = ir->intr_num;
+	xhci_free_interrupter(xhci, ir);
+	spin_lock_irqsave(&xhci->lock, flags);
+	xhci->interrupters[intr_num] = NULL;
+	spin_unlock_irqrestore(&xhci->lock, flags);
+}
+EXPORT_SYMBOL_GPL(xhci_remove_secondary_interrupter);
+
 void xhci_mem_cleanup(struct xhci_hcd *xhci)
 {
 	struct device	*dev = xhci_to_hcd(xhci)->self.sysdev;
@@ -1865,8 +1889,15 @@  void xhci_mem_cleanup(struct xhci_hcd *xhci)
 
 	cancel_delayed_work_sync(&xhci->cmd_timer);
 
-	xhci_free_interrupter(xhci, xhci->interrupter);
-	xhci->interrupter = NULL;
+	for (i = 1; i < xhci->max_interrupters; i++) {
+		if (xhci->interrupters[i])
+			xhci_remove_secondary_interrupter(xhci_to_hcd(xhci),
+							  xhci->interrupters[i]);
+	}
+
+	/* free the primary interrupter, interrupter number 0 */
+	xhci_free_interrupter(xhci, xhci->interrupters[0]);
+	xhci->interrupters[0] = NULL;
 	xhci_dbg_trace(xhci, trace_xhci_dbg_init, "Freed primary event ring");
 
 	if (xhci->cmd_ring)
@@ -1937,6 +1968,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;
@@ -1945,6 +1977,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;
@@ -2258,7 +2291,7 @@  xhci_alloc_interrupter(struct xhci_hcd *xhci, unsigned int intr_num, gfp_t flags
 		return NULL;
 	}
 
-	if (xhci->interrupter) {
+	if (xhci->interrupters[intr_num]) {
 		xhci_warn(xhci, "Can't allocate already set up interrupter %d\n", intr_num);
 		return NULL;
 	}
@@ -2305,6 +2338,56 @@  xhci_alloc_interrupter(struct xhci_hcd *xhci, unsigned int intr_num, gfp_t flags
 	return NULL;
 }
 
+/*
+ * Allocate a XHCI secondary interrupter slot.  If the user requests a specific intr
+ * number, then check if the slot is available.  Otherwise, fetch the first available
+ * entry within the interrupter array.
+ */
+struct xhci_interrupter *
+xhci_create_secondary_interrupter(struct usb_hcd *hcd, int intr_num)
+{
+	struct xhci_hcd *xhci = hcd_to_xhci(hcd);
+	struct xhci_interrupter *ir;
+	unsigned int i;
+	unsigned int idx = 0;
+	unsigned long flags;
+
+	if (!xhci->interrupters || intr_num > xhci->max_interrupters)
+		return NULL;
+
+	spin_lock_irqsave(&xhci->lock, flags);
+	/* find available secondary interrupter, interrupter 0 is reserved for primary */
+	if (intr_num > 0) {
+		idx = intr_num;
+	} else {
+		for (i = 1; i < xhci->max_interrupters; i++) {
+			if (xhci->interrupters[i] == NULL) {
+				idx = i;
+				break;
+			}
+		}
+	}
+
+	if (idx > 0) {
+		ir = xhci_alloc_interrupter(xhci, idx, GFP_KERNEL);
+		if (!ir) {
+			spin_unlock_irqrestore(&xhci->lock, flags);
+			return NULL;
+		}
+		ir->intr_num = idx;
+		xhci->interrupters[idx] = ir;
+		spin_unlock_irqrestore(&xhci->lock, flags);
+
+		return ir;
+	}
+	spin_unlock_irqrestore(&xhci->lock, flags);
+	xhci_warn(xhci, "Can't add new secondary interrupter, max interrupters %d\n",
+		  xhci->max_interrupters);
+
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(xhci_create_secondary_interrupter);
+
 int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
 {
 	dma_addr_t	dma;
@@ -2429,8 +2512,12 @@  int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
 	/* allocate and set up primary interrupter with an event ring. */
 	xhci_dbg_trace(xhci, trace_xhci_dbg_init,
 		       "Allocating primary event ring");
-	xhci->interrupter = xhci_alloc_interrupter(xhci, 0, flags);
-	if (!xhci->interrupter)
+
+	xhci->interrupters = kcalloc_node(xhci->max_interrupters, sizeof(*xhci->interrupters),
+					  flags, dev_to_node(dev));
+
+	xhci->interrupters[0] = xhci_alloc_interrupter(xhci, 0, flags);
+	if (!xhci->interrupters[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 eb788c60c1c0..445a79f36a60 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -3054,7 +3054,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 6183ce8574b1..88435b9cd66e 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -638,7 +638,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;
 
@@ -690,7 +690,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.
 	 */
@@ -758,7 +758,7 @@  static 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);
 
@@ -857,36 +857,51 @@  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 */
+	/* fixme, shold we lock  to prevent race with remove secondary interrupter? */
+	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);
+
+	/* FIXME should we lock to protect against freeing of interrupters */
+	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)
@@ -1251,7 +1266,7 @@  int xhci_resume(struct xhci_hcd *xhci, bool hibernated)
 		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 786002bb35db..43118ce83cca 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1800,7 +1800,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..738b0f0481a6
--- /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, int intr_num);
+void xhci_remove_secondary_interrupter(struct usb_hcd *hcd, struct xhci_interrupter *ir);
+#endif