lkml.org 
[lkml]   [2023]   [Mar]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v3 02/28] usb: xhci: Add XHCI APIs to support USB offloading
From
Hi Greg,

On 3/8/2023 10:38 PM, Greg KH wrote:
> On Wed, Mar 08, 2023 at 03:57:25PM -0800, Wesley Cheng wrote:
>> Some use cases, such as USB audio offloading, will allow for a DSP to take
>> over issuing USB transfers to the host controller. In order for the DSP to
>> submit transfers for a particular endpoint, and to handle its events, the
>> client driver will need to query for some parameters allocated by XHCI.
>>
>> - XHCI secondary interrupter event ring address
>> - XHCI transfer ring address (for a particular EP)
>> - Stop endpoint command API
>>
>> Once the resources are handed off to the DSP, the offload begins, and the
>> main processor can enter idle. When stopped, since there are no URBs
>> submitted from the main processor, the client will just issue a stop
>> endpoint command to halt any pending transfers.
>>
>> Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com>
>> ---
>> drivers/usb/host/xhci.c | 130 ++++++++++++++++++++++++++++++++++
>> include/linux/usb/xhci-intr.h | 8 +++
>> 2 files changed, 138 insertions(+)
>
> Please use checkpatch.pl on your patches before sending them out :(
>
> Some other minor comments:
>

Thanks for taking the time to review these!

Hmm, I did run checkpatch, and cleaned up the warnings it did give.
However, I think something changed with regards to the tools on my host
env. Will address those and make sure it runs properly next time.

Will fix the minor changes you mentioned, and focus on the general
questions you had.

>> diff --git a/include/linux/usb/xhci-intr.h b/include/linux/usb/xhci-intr.h
>> index 738b0f0481a6..d42cc9a1e698 100644
>> --- a/include/linux/usb/xhci-intr.h
>> +++ b/include/linux/usb/xhci-intr.h
>> @@ -80,7 +80,15 @@ struct xhci_interrupter {
>> u64 s3_erst_dequeue;
>> };
>>
>> +/* Secondary interrupter */
>> 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);
>> +
>> +/* Offload */
>> +int xhci_stop_endpoint(struct usb_device *udev,
>> + struct usb_host_endpoint *ep);
>> +phys_addr_t xhci_get_xfer_resource(struct usb_device *udev,
>> + struct usb_host_endpoint *ep, dma_addr_t *dma);
>> +phys_addr_t xhci_get_ir_resource(struct usb_device *udev, struct xhci_interrupter *ir);
>
> Why are these functions unique to offload?
>

Wanted to separate the set of APIs used for creating a secondary
interrupter versus offload related ones. In general, the APIs under the
secondary interrupter portion can be used for other things other than
offloading. As Mathias pointed out, they had a use case where they
wanted to utilize the secondary interrupter to actually route and
receive interrupts on the secondary ring, not to suppress them. (which
is opposite of what the offload concept is doing)

Now for the offload section, those are specific to that feature, because
we need to pass certain memory information about what was allocated by
XHCI to the entity that we are offloading the IO operations to. Hence,
why they are APIs which fetch the transfer ring and event ring
addresses. In addition, we do have the stop EP as well, since in the
offload case, since the main processor doesn't submit TDs (transfer
descriptors) then it isn't aware there are transfers in progress. When
the endpoint is released, then the offload driver needs to be the one
that halts the EP.

As you mentioned, I will add documentation to better describe these.

Thanks
Wesley Cheng

\
 
 \ /
  Last update: 2023-03-27 00:52    [W:0.319 / U:0.720 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site