Re: [PATCH v3 02/28] usb: xhci: Add XHCI APIs to support USB offloading

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxx>
---
  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



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux