On 2019年05月02日 18:56, Mathias Nyman wrote:
On 30.4.2019 6.06, Jim Lin wrote:
USB 2.0 specification chapter 11.17.5 says "as part of endpoint halt
processing for full-/low-speed endpoints connected via a TT, the host
software must use the Clear_TT_Buffer request to the TT to ensure
that the buffer is not in the busy state".
Good point, xhci isn't making sure TT buffers get cleared when they
should.
In our case, a full-speed speaker (ConferenceCam) is behind a high-
speed hub (ConferenceCam Connect), sometimes once we get STALL on a
request we may continue to get STALL with the folllowing requests,
like Set_Interface.
Here we add Clear_TT_Buffer for the following Set_Interface requests
to get ACK successfully.
Signed-off-by: Jim Lin <jilin@xxxxxxxxxx>
---
v2: xhci_clear_tt_buffer_complete: add static, shorter indentation
, remove its claiming in xhci.h
v3: Add description for clearing_tt (xhci.h)
drivers/usb/host/xhci-ring.c | 28 ++++++++++++++++++++++++++++
drivers/usb/host/xhci.c | 7 +++++++
drivers/usb/host/xhci.h | 2 ++
3 files changed, 37 insertions(+)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 9215a28dad40..02b1ebad81e7 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1786,6 +1786,33 @@ struct xhci_segment *trb_in_td(struct xhci_hcd
*xhci,
return NULL;
}
+static void xhci_clear_hub_tt_buffer(struct xhci_hcd *xhci,
+ unsigned int slot_id, struct xhci_td *td)
+{
+ struct xhci_virt_device *dev;
+ struct xhci_slot_ctx *slot_ctx;
+ int saved_devnum;
+
+ /*
+ * As part of low/full-speed endpoint-halt processing
+ * we must clear the TT buffer (USB 2.0 specification 11.17.5).
+ */
+ if (td->urb->dev->tt && !usb_pipeint(td->urb->pipe) &&
+ (td->urb->dev->tt->hub != xhci_to_hcd(xhci)->self.root_hub) &&
+ !xhci->clearing_tt) {
+ xhci->clearing_tt = 1;
one xhci->clearing_tt under is not enough, there might be several HS
hubs, or
multi TT hubs with halted endpoints at the same time that need TT
clearing.
How about a flag per endpoint?
For example Aadding a EP_CLEARING_TT flag for ep_state in struct
xhci_virt_ep?
just like EP_STOP_CMD_PENDING, or EP_HALTED
+ dev = xhci->devs[slot_id];
+ slot_ctx = xhci_get_slot_ctx(xhci, dev->out_ctx);
+ /* Update devnum temporarily for usb_hub_clear_tt_buffer */
+ saved_devnum = td->urb->dev->devnum;
+ td->urb->dev->devnum = (int) le32_to_cpu(slot_ctx->dev_state) &
+ DEV_ADDR_MASK;
Changing the struct usb_device devnum on the fly seems like a bit of a
hack, and probably
causes issues elsewhere. Devnum is tied to uevents, usbfs, sysfs etc.
We need another solution, some options:
- Let usb_hub_clear_tt_buffer() figure out address and not just use
devnum if host == xhci.
- Add address to struct usb_device, (would have both address and
devnum), use it when needed.
- Provide address as parameter to usb_clear_tt_buffer() (api change,
changes other host drivers)
- Force devnum to be same as address, usb core can't choose address
for xhci devices.
-Mathias
Thanks for review. Will try.
--nvpublic
