Messages in this thread | | | Date | Wed, 4 Oct 2023 16:42:39 -0700 | Subject | Re: [PATCH v7 01/33] xhci: add support to allocate several interrupters | From | Wesley Cheng <> |
| |
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
| |