Re: [RFC PATCH 1/3] xhci: Add polling support for USB keyboards

On 25/06/2020 17:10, Jason Wessel wrote:

The xhci driver was causing intermittent 5 second delays from the USB keyboard polling hook. Executing something like a "sleep 1" for example would sleep for 5 seconds, unless an event occurred on the USB bus to shorten the delay.

Modeled after the code in the DWC2 driver, a nonblock state was added to quickly return instead of blocking for up to 5 seconds waiting for an event before timing out.

Signed-off-by: Jason Wessel jason.wessel@windriver.com

drivers/usb/host/xhci-ring.c | 24 +++++++++++++++--------- drivers/usb/host/xhci.c | 10 +++++----- include/usb/xhci.h | 5 +++-- 3 files changed, 23 insertions(+), 16 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 86aeaab412..1f7b3a8e0b 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -434,7 +434,8 @@ static int event_ready(struct xhci_ctrl *ctrl)

• @param expected TRB type expected from Event TRB
• @return pointer to event trb

*/ -union xhci_trb *xhci_wait_for_event(struct xhci_ctrl *ctrl, trb_type expected) +union xhci_trb *xhci_wait_for_event(struct xhci_ctrl *ctrl, trb_type expected,

• 		    bool nonblock)
  

{ trb_type type; unsigned long ts = get_timer(0); @@ -442,8 +443,11 @@ union xhci_trb *xhci_wait_for_event(struct xhci_ctrl *ctrl, trb_type expected) do { union xhci_trb *event = ctrl->event_ring->dequeue;

• if (!event_ready(ctrl))
  

• if (!event_ready(ctrl)) {
  

• 	if (nonblock)
  

• 		return NULL;
  continue;
  

• }
  
  

  type = TRB_FIELD_TO_TYPE(le32_to_cpu(event->event_cmd.flags)); if (type == expected)

@@ -493,7 +497,7 @@ static void abort_td(struct usb_device *udev, int ep_index)

xhci_queue_command(ctrl, NULL, udev->slot_id, ep_index, TRB_STOP_RING);

• event = xhci_wait_for_event(ctrl, TRB_TRANSFER);

• event = xhci_wait_for_event(ctrl, TRB_TRANSFER, false); field = le32_to_cpu(event->trans_event.flags); BUG_ON(TRB_TO_SLOT_ID(field) != udev->slot_id); BUG_ON(TRB_TO_EP_INDEX(field) != ep_index);

@@ -501,7 +505,7 @@ static void abort_td(struct usb_device *udev, int ep_index) != COMP_STOP))); xhci_acknowledge_event(ctrl);

• event = xhci_wait_for_event(ctrl, TRB_COMPLETION);

• event = xhci_wait_for_event(ctrl, TRB_COMPLETION, false); BUG_ON(TRB_TO_SLOT_ID(le32_to_cpu(event->event_cmd.flags)) != udev->slot_id || GET_COMP_CODE(le32_to_cpu( event->event_cmd.status)) != COMP_SUCCESS);

@@ -509,7 +513,7 @@ static void abort_td(struct usb_device *udev, int ep_index)

xhci_queue_command(ctrl, (void *)((uintptr_t)ring->enqueue | ring->cycle_state), udev->slot_id, ep_index, TRB_SET_DEQ);

• event = xhci_wait_for_event(ctrl, TRB_COMPLETION);

• event = xhci_wait_for_event(ctrl, TRB_COMPLETION, false); BUG_ON(TRB_TO_SLOT_ID(le32_to_cpu(event->event_cmd.flags)) != udev->slot_id || GET_COMP_CODE(le32_to_cpu( event->event_cmd.status)) != COMP_SUCCESS);

@@ -555,7 +559,7 @@ static void record_transfer_result(struct usb_device *udev,

• @return returns 0 if successful else -1 on failure

*/ int xhci_bulk_tx(struct usb_device *udev, unsigned long pipe,

• 	int length, void *buffer)
  

• 	int length, void *buffer, bool nonblock)
  

{ int num_trbs = 0; struct xhci_generic_trb *start_trb; @@ -714,8 +718,10 @@ int xhci_bulk_tx(struct usb_device *udev, unsigned long pipe,

giveback_first_trb(udev, ep_index, start_cycle, start_trb);

• event = xhci_wait_for_event(ctrl, TRB_TRANSFER);

• event = xhci_wait_for_event(ctrl, TRB_TRANSFER, nonblock); if (!event) {

• if (nonblock)
  

• 	return -EINVAL;
  

  debug("XHCI bulk transfer timed out, aborting...\n"); abort_td(udev, ep_index); udev->status = USB_ST_NAK_REC; /* closest thing to a timeout */

@@ -911,7 +917,7 @@ int xhci_ctrl_tx(struct usb_device *udev, unsigned long pipe,

giveback_first_trb(udev, ep_index, start_cycle, start_trb);

• event = xhci_wait_for_event(ctrl, TRB_TRANSFER);

• event = xhci_wait_for_event(ctrl, TRB_TRANSFER, false); if (!event) goto abort; field = le32_to_cpu(event->trans_event.flags);

@@ -929,7 +935,7 @@ int xhci_ctrl_tx(struct usb_device *udev, unsigned long pipe, if (GET_COMP_CODE(le32_to_cpu(event->trans_event.transfer_len)) == COMP_SHORT_TX) { /* Short data stage, clear up additional status stage event */

• event = xhci_wait_for_event(ctrl, TRB_TRANSFER);
  

• event = xhci_wait_for_event(ctrl, TRB_TRANSFER, false);
  

  if (!event) goto abort; BUG_ON(TRB_TO_SLOT_ID(field) != slot_id);

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index ebd2954571..6f1748dee2 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -464,7 +464,7 @@ static int xhci_configure_endpoints(struct usb_device *udev, bool ctx_change) xhci_flush_cache((uintptr_t)in_ctx->bytes, in_ctx->size); xhci_queue_command(ctrl, in_ctx->bytes, udev->slot_id, 0, ctx_change ? TRB_EVAL_CONTEXT : TRB_CONFIG_EP);

• event = xhci_wait_for_event(ctrl, TRB_COMPLETION);

• event = xhci_wait_for_event(ctrl, TRB_COMPLETION, false); BUG_ON(TRB_TO_SLOT_ID(le32_to_cpu(event->event_cmd.flags)) != udev->slot_id);

@@ -660,7 +660,7 @@ static int xhci_address_device(struct usb_device *udev, int root_portnr) ctrl_ctx->drop_flags = 0;

xhci_queue_command(ctrl, (void *)ctrl_ctx, slot_id, 0, TRB_ADDR_DEV);

• event = xhci_wait_for_event(ctrl, TRB_COMPLETION);

• event = xhci_wait_for_event(ctrl, TRB_COMPLETION, false); BUG_ON(TRB_TO_SLOT_ID(le32_to_cpu(event->event_cmd.flags)) != slot_id);

  switch (GET_COMP_CODE(le32_to_cpu(event->event_cmd.status))) {

@@ -735,7 +735,7 @@ static int _xhci_alloc_device(struct usb_device *udev) }

xhci_queue_command(ctrl, NULL, 0, 0, TRB_ENABLE_SLOT);

• event = xhci_wait_for_event(ctrl, TRB_COMPLETION);

• event = xhci_wait_for_event(ctrl, TRB_COMPLETION, false); BUG_ON(GET_COMP_CODE(le32_to_cpu(event->event_cmd.status)) != COMP_SUCCESS);

@@ -1138,7 +1138,7 @@ static int _xhci_submit_int_msg(struct usb_device *udev, unsigned long pipe,

the nonblock parameter is not documented in the comment. Would you mind to fix this in the next round?

Regards, Matthias

* (at most) one TD. A TD (comprised of sg list entries) can
* take several service intervals to transmit.
*/

• return xhci_bulk_tx(udev, pipe, length, buffer);

• return xhci_bulk_tx(udev, pipe, length, buffer, nonblock);

}

/** @@ -1158,7 +1158,7 @@ static int _xhci_submit_bulk_msg(struct usb_device *udev, unsigned long pipe, return -EINVAL; }

• return xhci_bulk_tx(udev, pipe, length, buffer);

• return xhci_bulk_tx(udev, pipe, length, buffer, false);

}

/** diff --git a/include/usb/xhci.h b/include/usb/xhci.h index 20e4a21066..875ddb2cd5 100644 --- a/include/usb/xhci.h +++ b/include/usb/xhci.h @@ -1255,9 +1255,10 @@ void xhci_setup_addressable_virt_dev(struct xhci_ctrl *ctrl, void xhci_queue_command(struct xhci_ctrl *ctrl, u8 *ptr, u32 slot_id, u32 ep_index, trb_type cmd); void xhci_acknowledge_event(struct xhci_ctrl *ctrl); -union xhci_trb *xhci_wait_for_event(struct xhci_ctrl *ctrl, trb_type expected); +union xhci_trb *xhci_wait_for_event(struct xhci_ctrl *ctrl, trb_type expected,

• 		    bool nonblock);
  

int xhci_bulk_tx(struct usb_device *udev, unsigned long pipe,

•  int length, void *buffer);
  

•  int length, void *buffer, bool nonblock);
  

int xhci_ctrl_tx(struct usb_device *udev, unsigned long pipe, struct devrequest *req, int length, void *buffer); int xhci_check_maxpacket(struct usb_device *udev);