On 07.11.2017 02:05, Curt Meyers wrote:
On 11/06/2017 06:57 AM, Mathias Nyman wrote:
On 03.11.2017 23:37, Curt Meyers wrote:
On 09/19/2017 06:00 AM, Mathias Nyman wrote:
On 08.09.2017 20:35, Curt Meyers wrote:
On 09/05/2017 02:56 PM, Curt Meyers wrote:
On 09/04/2017 04:13 AM, Mathias Nyman wrote:
On 04.09.2017 13:46, Felipe Balbi wrote:
Hi,
Mathias Nyman <mathias.nyman@xxxxxxxxxxxxxxx> writes:
Unfortunately config endpoint command doesn't log endpoint context in this log,
it should call trace_xhci_handle_cmd_config_ep(ep_ctx), I don't know why it's missing.
That's called conditionally:
case TRB_CONFIG_EP:
if (!cmd->completion)
xhci_handle_cmd_config_ep(xhci, slot_id, event,
cmd_comp_code);
Yep, need to change the tracing so we get it for every config endpoint command
But later on at Set TR Dequeue Pointer Command it logs the endpiont context:
259.147237: xhci_handle_cmd_set_deq: RS 00000 super-speed Ctx Entries 7 MEL 512 us Port# 19/0 [TT Slot 0 Port# 0 TTT 0 Intr 0] Addr 1 State configured
259.147238: xhci_handle_cmd_set_deq_ep: State stopped mult 1 max P. Streams 0 interval 125 us max ESIT payload 201326592 CErr 0 Type Isoc IN burst 2 maxp 1024 deq 00000003f9fd6510 avg trb len 3072
This looks odd, 201326592 bytes per ESIT, way too much. So much that I suspect tracing decodes it wrong
try this:
modified drivers/usb/host/xhci.h
@@ -2540,9 +2540,7 @@ static inline const char *xhci_decode_ep_context(u32 info, u32 info2, u64 deq,
u8 lsa;
u8 hid;
- esit = EP_MAX_ESIT_PAYLOAD_HI(info) << 16 |
- EP_MAX_ESIT_PAYLOAD_LO(tx_info);
-
+ esit = CTX_TO_MAX_ESIT_PAYLOAD(info);
ep_state = info & EP_STATE_MASK;
max_pstr = info & EP_MAXPSTREAMS_MASK;
interval = CTX_TO_EP_INTERVAL(info);
Yes, I noticed the same, and there's also a high ESIT bit field for new hosts,
I pushed a fix to my for-usb-linus branch for that
+#define CTX_TO_MAX_ESIT_PAYLOAD_HI(p) (((p) >> 24) & 0xff)
...
- esit = EP_MAX_ESIT_PAYLOAD_HI(info) << 16 |
- EP_MAX_ESIT_PAYLOAD_LO(tx_info);
+ esit = CTX_TO_MAX_ESIT_PAYLOAD_HI(info) << 16 |
+ CTX_TO_MAX_ESIT_PAYLOAD(tx_info);
-Mathias
Hi Mathias,
Hi Sorry about the delay
I have made the above suggested changed and produced a new trace file. I hope this helps out.
Curt
Would it help if I sent you a couple of these USB camera devices to speed up debugging?
Not really, It's just my TODO list is already quite long, and then there's always some urgent matter
that requires attention.
The new log however still shows the wrong ESIT payload values,
xhci_handle_cmd_set_deq_ep: State stopped mult 1 max P. Streams 0 interval 125 us max ESIT payload 201326592
-Mathias
--
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
Hi Mathias,
The manufacture hired a Linux developer to solve the problem with this isochronous camera device and it turns out to be related to LPM that only shows up in newer Intel hardware, according to the developer. He says that there is something wrong with the MEL (Max Exit Latency) and skipping it in the call to xhci_configure_endpoint() fixes the problem, or works around it.
Curt
Ok, good to know
If you are in contact with this developer please tell him that we are
interested in getting more details about the LPM issue, and getting it
fixed upstream.
Thanks
Mathias
--
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
This patch works on the current 4.10 kernel tree in Ubuntu. This fixes the problem we where seeing on an Intel Sunrise chipset but I also see the same failure on the Intel controller with PCI device ID 0xa2af after generating this patch and testing it. I do not understand what the common thread is between all the various Intel controllers so perhaps you can help out in making this more broad.
If you want the patch applied to a specific branch let me know and I can try it again but let me know specifically which tag or branch to apply this to.
Signed-off-by: Curt Meyers <cmeyers@xxxxxx>
---
diff -Naur host.ori/xhci.c host/xhci.c
--- host.ori/xhci.c 2017-11-02 01:50:04.544937000 +0800
+++ host/xhci.c 2017-11-03 00:07:04.548216000 +0800
@@ -3986,6 +3986,26 @@
unsigned long flags;
int ret;
+ bool Period_EP_Found = false;
+ if ((xhci->quirks & XHCI_SET_MEL_STUCK) && (USB_SPEED_SUPER == udev->speed)) {
+ int i = 0;
+ for (i = 1; i < 16; ++i) {
+ int ep_in_type = (udev && udev->ep_in[i]) ?(usb_endpoint_type(&udev->ep_in[i]->desc)) :(-1);
+ int ep_out_type = (udev && udev->ep_out[i]) ?(usb_endpoint_type(&udev->ep_out[i]->desc)) :(-1);
+
+ if ((USB_ENDPOINT_XFER_ISOC == ep_in_type) || (USB_ENDPOINT_XFER_INT == ep_in_type)) {
+ Period_EP_Found = true;
+ break;
+ }
+
+ if ((USB_ENDPOINT_XFER_ISOC == ep_out_type) || (USB_ENDPOINT_XFER_INT == ep_out_type)) {
+ Period_EP_Found = true;
+ break;
+ }
+ }
+ }
+
+
spin_lock_irqsave(&xhci->lock, flags);
virt_dev = xhci->devs[udev->slot_id];
@@ -4026,8 +4046,9 @@
xhci_dbg_ctx(xhci, command->in_ctx, 0);
/* Issue and wait for the evaluate context command. */
- ret = xhci_configure_endpoint(xhci, udev, command,
- true, true);
+ ret = (!Period_EP_Found)
+ ? xhci_configure_endpoint(xhci, udev, command, true, true)
+ : xhci_configure_endpoint(xhci, udev, command, false, true);
xhci_dbg(xhci, "Slot %u Output Context:\n", udev->slot_id);
Ok, so basically this code forces a 'configure endpoint' command instead of a 'evaluate context'
command if there are periodic endpoints in use.
I'm not sure about the reasoning behind this.
I do know that configure endpoint command will release the bandwidth kept by
dropped endpoints, but setting MEL values are usually done using the
'evaluate context' command.
-Mathias
--
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
