From: Hemant Kumar <hemantk@xxxxxxxxxxxxxx> USB control transfers can contain an optional IN data stage, in which case the xHCI driver would queue an additional TRB. The interrupt on short packet (ISP) bit is set so that the host controller driver can update the URB's actual_length field if packet is received has length less than the queued buffer. A zero-length packet (ZLP) received during the data stage is a special case of a short packet but is currently not handled properly since during the subsequent status stage the driver ends up overwriting the actual_length field with transfer_buffer_length, which was the original buffer length. A function driver will then incorrectly interpret the completed URB as being of full length. Fix this by setting a flag when a ZLP is received in the data stage so that the urb->actual_length remains 0 and does not get overwritten in the status stage. Signed-off-by: Hemant Kumar <hemantk@xxxxxxxxxxxxxx> Signed-off-by: Jack Pham <jackp@xxxxxxxxxxxxxx> --- Hi Sarah, Here's an RFC I'm hoping will revive this thread: http://marc.info/?l=linux-usb&m=138128415812207&w=2 The discussion was left hanging after Alan asked: > } else { > td->urb->actual_length = > td->urb->transfer_buffer_length; > } > > What's the purpose of this clause? > > It looks like the driver is confusing "actual_length == 0" with > "actual_length was never set". What if actual_length _was_ previously > set, but it was set to 0? This patch attemps to address this very issue by "catching" the ZLP in the data stage handler and marking a flag so that actual_length doesn't get overwritten before giving back the URB. Can you please have a look? Thanks, Jack drivers/usb/host/xhci-ring.c | 6 ++++++ drivers/usb/host/xhci.h | 3 +++ 2 files changed, 9 insertions(+), 0 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 1e2f3f4..a896474 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -2162,6 +2162,8 @@ static int process_ctrl_td(struct xhci_hcd *xhci, struct xhci_td *td, /* Did we already see a short data * stage? */ *status = -EREMOTEIO; + } else if (td->zlp_data) { + td->zlp_data = false; } else { td->urb->actual_length = td->urb->transfer_buffer_length; @@ -2171,6 +2173,10 @@ static int process_ctrl_td(struct xhci_hcd *xhci, struct xhci_td *td, td->urb->actual_length = td->urb->transfer_buffer_length - EVENT_TRB_LEN(le32_to_cpu(event->transfer_len)); + + if (td->urb->actual_length == 0) + td->zlp_data = true; + xhci_dbg(xhci, "Waiting for status " "stage event\n"); return 0; diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h index 03c74b7..cbd1497 100644 --- a/drivers/usb/host/xhci.h +++ b/drivers/usb/host/xhci.h @@ -1283,6 +1283,9 @@ struct xhci_td { struct xhci_segment *start_seg; union xhci_trb *first_trb; union xhci_trb *last_trb; + + /* ZLP received in data stage of a control transfer */ + bool zlp_data; }; /* xHCI command default timeout value */ -- The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
