lkml.org 
[lkml]   [2023]   [Oct]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v7 01/33] xhci: add support to allocate several interrupters
From
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

\
 
 \ /
  Last update: 2023-10-05 01:44    [W:0.084 / U:0.984 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site