[PATCH v4] usb: xhci: fix lack of short packet event trb handling

[Ran Wang]

Hi Marek, Bin,


On Wednesday, November 18, 2020 3:49 PM, Ran Wang wrote:
> 
> For bulk IN transfer, the codes will set ISP flag to request event TRB being
> generated by xHC for the case of short packet. So when encountering
> buffer-cross-64K-boundary (which we will divide payload and enqueuqe more
> than 1 transfer TRB), and the first TRB ends up with a short packet condition it
> will trigger an short packet code transfer event per that flag and cause more
> than 1 event TRB generated for this transfer.
> 
> However, current codes will only handle the first transfer event TRB then mark
> current transfer completed, causing next transfer failure due to event TRB
> mis-match.
> 
> Such issue has been observed on some Layerscape platforms (LS1028A,
> LS1088A, etc) with USB ethernet device.
> 
> This patch adds a loop to make sure the event TRB for last transfer TRB has
> been handled in time.
> 
> Signed-off-by: Ran Wang <ran.wang_1 at nxp.com>
> Reviewed-by: Bin Meng <bmeng.cn at gmail.com>
> ---
> Change in v4:
>  - Update commit message: 'for case of short packet' => 'for the case of short
> packet'

   Has this v4 patch been accepted?

Thanks & Regards,
Ran

> Change in v3:
>  - Update commit message according to v2 feedback .
>  - Replace (void *) with (uintptr_t) to fix below armv7 compile warning:
>  drivers/usb/host/xhci-ring.c: In function 'xhci_bulk_tx':
>  drivers/usb/host/xhci-ring.c:726:6: warning: cast to pointer from integer of
> different size [-Wint-to-pointer-cast]
>    726 |  if ((void *)le64_to_cpu(event->trans_event.buffer) !=
> last_transfer_trb_addr) {
>        |       ^
> 
> Change in v2:
>  - Re-write commit message to describe context more clearly.
>  - Add prefix 'le64_to_cpu' for 'event->trans_event.buffer'.
> 
>  drivers/usb/host/xhci-ring.c | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index
> 13065d7..d708fc9 100644
> --- a/drivers/usb/host/xhci-ring.c
> +++ b/drivers/usb/host/xhci-ring.c
> @@ -580,10 +580,13 @@ int xhci_bulk_tx(struct usb_device *udev, unsigned
> long pipe,
>  	int ret;
>  	u32 trb_fields[4];
>  	u64 val_64 = virt_to_phys(buffer);
> +	void *last_transfer_trb_addr;
> +	int available_length;
> 
>  	debug("dev=%p, pipe=%lx, buffer=%p, length=%d\n",
>  		udev, pipe, buffer, length);
> 
> +	available_length = length;
>  	ep_index = usb_pipe_ep_index(pipe);
>  	virt_dev = ctrl->devs[slot_id];
> 
> @@ -697,7 +700,7 @@ int xhci_bulk_tx(struct usb_device *udev, unsigned
> long pipe,
>  		trb_fields[2] = length_field;
>  		trb_fields[3] = field | TRB_TYPE(TRB_NORMAL);
> 
> -		queue_trb(ctrl, ring, (num_trbs > 1), trb_fields);
> +		last_transfer_trb_addr = queue_trb(ctrl, ring, (num_trbs > 1),
> +trb_fields);
> 
>  		--num_trbs;
> 
> @@ -710,6 +713,7 @@ int xhci_bulk_tx(struct usb_device *udev, unsigned
> long pipe,
> 
>  	giveback_first_trb(udev, ep_index, start_cycle, start_trb);
> 
> +again:
>  	event = xhci_wait_for_event(ctrl, TRB_TRANSFER);
>  	if (!event) {
>  		debug("XHCI bulk transfer timed out, aborting...\n"); @@ -718,12
> +722,20 @@ int xhci_bulk_tx(struct usb_device *udev, unsigned long pipe,
>  		udev->act_len = 0;
>  		return -ETIMEDOUT;
>  	}
> -	field = le32_to_cpu(event->trans_event.flags);
> 
> +	if ((uintptr_t)(le64_to_cpu(event->trans_event.buffer))
> +			!= (uintptr_t)last_transfer_trb_addr) {
> +		available_length -=
> +
> 	(int)EVENT_TRB_LEN(le32_to_cpu(event->trans_event.transfer_len));
> +		xhci_acknowledge_event(ctrl);
> +		goto again;
> +	}
> +
> +	field = le32_to_cpu(event->trans_event.flags);
>  	BUG_ON(TRB_TO_SLOT_ID(field) != slot_id);
>  	BUG_ON(TRB_TO_EP_INDEX(field) != ep_index);
> 
> -	record_transfer_result(udev, event, length);
> +	record_transfer_result(udev, event, available_length);
>  	xhci_acknowledge_event(ctrl);
>  	xhci_inval_cache((uintptr_t)buffer, length);
> 
> --
> 2.7.4