From mboxrd@z Thu Jan 1 00:00:00 1970 From: Bin Meng Date: Thu, 20 Aug 2020 16:26:28 +0800 Subject: [PATCH 5/9] xhci-ring.c: Add the poll_pend state to properly abort transactions In-Reply-To: <20200724215049.163379-6-jason.wessel@windriver.com> References: <20200724215049.163379-1-jason.wessel@windriver.com> <20200724215049.163379-6-jason.wessel@windriver.com> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Jason, On Sat, Jul 25, 2020 at 5:51 AM Jason Wessel wrote: > > xhci_trl_tx and xhchi_bulk_tx can be called synchronously by other > drivers such as the usb storage or network, while the keyboard driver > exclusively uses the polling mode. > Could you provide more details as to when this will fail? > And pending polling transactions must be aborted before switching > modes to avoid corrupting the state of the controller. > > Signed-off-by: Jason Wessel > --- > drivers/usb/host/xhci-ring.c | 86 +++++++++++++++++++++++++++++------- > 1 file changed, 70 insertions(+), 16 deletions(-) > > diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c > index b7b2e16410..d6339f4f0a 100644 > --- a/drivers/usb/host/xhci-ring.c > +++ b/drivers/usb/host/xhci-ring.c > @@ -24,6 +24,12 @@ > > #include > > +static void *last_bulk_tx_buf; > +static struct usb_device *poll_last_udev; > +int poll_last_ep_index; > +static unsigned long bulk_tx_poll_ts; > +static bool poll_pend; Should these variables go into struct xhci_ctrl? What happens if 2 controllers are sending data? > + > /** > * Is this TRB a link TRB or was the last TRB the last TRB in this event ring > * segment? I.e. would the updated event TRB pointer step off the end of the > @@ -549,19 +555,8 @@ static void record_transfer_result(struct usb_device *udev, > } > } > > -/**** Bulk and Control transfer methods ****/ > -/** > - * Queues up the BULK Request > - * > - * @param udev pointer to the USB device structure > - * @param pipe contains the DIR_IN or OUT , devnum > - * @param length length of the buffer > - * @param buffer buffer to be read/written based on the request > - * @param nonblock when true do not block waiting for response > - * @return returns 0 if successful else -1 on failure > - */ > -int xhci_bulk_tx(struct usb_device *udev, unsigned long pipe, > - int length, void *buffer, bool nonblock) > +static int _xhci_bulk_tx_queue(struct usb_device *udev, unsigned long pipe, > + int length, void *buffer) > { > int num_trbs = 0; > struct xhci_generic_trb *start_trb; > @@ -575,7 +570,6 @@ int xhci_bulk_tx(struct usb_device *udev, unsigned long pipe, > struct xhci_virt_device *virt_dev; > struct xhci_ep_ctx *ep_ctx; > struct xhci_ring *ring; /* EP transfer ring */ > - union xhci_trb *event; > > int running_total, trb_buff_len; > unsigned int total_packet_count; > @@ -719,20 +713,73 @@ int xhci_bulk_tx(struct usb_device *udev, unsigned long pipe, > } while (running_total < length); > > giveback_first_trb(udev, ep_index, start_cycle, start_trb); > + return 0; > +} > > +/**** Bulk and Control transfer methods ****/ > +/** > + * Queues up the BULK Request > + * > + * @param udev pointer to the USB device structure > + * @param pipe contains the DIR_IN or OUT , devnum > + * @param length length of the buffer > + * @param buffer buffer to be read/written based on the request > + * @param nonblock when true do not block waiting for response > + * @return returns 0 if successful else -1 on failure > + */ > +int xhci_bulk_tx(struct usb_device *udev, unsigned long pipe, > + int length, void *buffer, bool nonblock) > +{ > + u32 field; > + int ret; > + union xhci_trb *event; > + struct xhci_ctrl *ctrl = xhci_get_ctrl(udev); > + int ep_index = usb_pipe_ep_index(pipe); > + > + if (poll_pend) { > + /* > + * Abort a pending poll operation if it should have > + * timed out, or if this is a different buffer from a > + * separate request > + */ > + if (get_timer(bulk_tx_poll_ts) > XHCI_TIMEOUT || > + last_bulk_tx_buf != buffer || poll_last_udev != udev || > + ep_index != poll_last_ep_index) { > + abort_td(poll_last_udev, poll_last_ep_index); > + poll_last_udev->status = USB_ST_NAK_REC; /* closest thing to a timeout */ > + poll_last_udev->act_len = 0; > + poll_pend = false; > + } > + } /* No else here because poll_pend might have changed above */ > + if (!poll_pend) { > + last_bulk_tx_buf = buffer; > + ret = _xhci_bulk_tx_queue(udev, pipe, length, buffer); > + if (ret) > + return ret; > + } > event = xhci_wait_for_event(ctrl, TRB_TRANSFER, nonblock); > if (!event) { > - if (nonblock) > + if (nonblock) { > + if (!poll_pend) { > + /* Start the timer */ > + bulk_tx_poll_ts = get_timer(0); > + poll_last_udev = udev; > + poll_last_ep_index = ep_index; > + poll_pend = true; > + } > 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 */ > udev->act_len = 0; > + poll_pend = false; > return -ETIMEDOUT; > } > + poll_pend = false; > field = le32_to_cpu(event->trans_event.flags); > > - BUG_ON(TRB_TO_SLOT_ID(field) != slot_id); > + BUG_ON(TRB_TO_SLOT_ID(field) != udev->slot_id); > BUG_ON(TRB_TO_EP_INDEX(field) != ep_index); > BUG_ON(*(void **)(uintptr_t)le64_to_cpu(event->trans_event.buffer) - > buffer > (size_t)length); > @@ -779,6 +826,13 @@ int xhci_ctrl_tx(struct usb_device *udev, unsigned long pipe, > le16_to_cpu(req->value), le16_to_cpu(req->value), > le16_to_cpu(req->index)); > > + if (poll_pend) { > + abort_td(poll_last_udev, poll_last_ep_index); > + poll_last_udev->status = USB_ST_NAK_REC; /* closest thing to a timeout */ > + poll_last_udev->act_len = 0; > + poll_pend = false; > + } > + > ep_index = usb_pipe_ep_index(pipe); > > ep_ring = virt_dev->eps[ep_index].ring; > -- Regards, Bin