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

[Claudiu.Beznea]

On 09.03.2023 01:57, Wesley Cheng wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> 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(+)
> 
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index 88435b9cd66e..5c6b3d8f834c 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -1603,6 +1603,136 @@ static int xhci_check_args(struct usb_hcd *hcd, struct usb_device *udev,
>         return 1;
>  }
> 
> +int xhci_stop_endpoint(struct usb_device *udev,
> +                       struct usb_host_endpoint *ep)
> +{
> +       struct usb_hcd *hcd = bus_to_hcd(udev->bus);
> +       struct xhci_hcd *xhci = hcd_to_xhci(hcd);
> +       unsigned int ep_index;
> +       struct xhci_virt_device *virt_dev;
> +       struct xhci_command *cmd;
> +       unsigned long flags;
> +       int ret = 0;

No need to initialize it.

> +
> +       ret = xhci_check_args(hcd, udev, ep, 1, true, __func__);
> +       if (ret <= 0)
> +               return ret;
> +
> +       cmd = xhci_alloc_command(xhci, true, GFP_NOIO);
> +       if (!cmd)
> +               return -ENOMEM;
> +
> +       spin_lock_irqsave(&xhci->lock, flags);
> +       virt_dev = xhci->devs[udev->slot_id];
> +       if (!virt_dev) {
> +               ret = -ENODEV;
> +               goto err;
> +       }
> +
> +       ep_index = xhci_get_endpoint_index(&ep->desc);
> +       if (virt_dev->eps[ep_index].ring &&
> +                       virt_dev->eps[ep_index].ring->dequeue) {
> +               ret = xhci_queue_stop_endpoint(xhci, cmd, udev->slot_id,
> +                               ep_index, 0);
> +               if (ret)
> +                       goto err;
> +
> +               xhci_ring_cmd_db(xhci);
> +               spin_unlock_irqrestore(&xhci->lock, flags);
> +
> +               /* Wait for stop endpoint command to finish */
> +               wait_for_completion(cmd->completion);
> +
> +               if (cmd->status == COMP_COMMAND_ABORTED ||
> +                               cmd->status == COMP_STOPPED) {

                      ^ usually here go 2nd condition

> +                       xhci_warn(xhci,
> +                               "stop endpoint command timeout for ep%d%s\n",
> +                               usb_endpoint_num(&ep->desc),
> +                               usb_endpoint_dir_in(&ep->desc) ? "in" : "out");
> +                       ret = -ETIME;
> +                               }

} should be aligned with if

[ ... ]