On 18.01.2019 13:28, Jan-Marek Glogowski wrote:
Just adding some CC of commit signers from get_maintainer.pl
Am 16.01.19 um 18:07 schrieb Jan-Marek Glogowski:
On plug-in of my USB-C device, its USB_SS_PORT_LS_SS_INACTIVE
link state bit is set. Greping all the kernel for this bit shows
that the port status requests a warm-reset this way.
This just happens, if its the only device on that hub and the
hub resumes, so we don't call port_event, which would otherwise
warm-reset ports. The device works ok without this patch, if
there is already any other device connected to the hub.
I've heard about issues with bad port redrivers/retimers causing similar issues
at resume.
One reason why port_event() isn't called is that hub thread failed to report
any activity for those ports.
At bus resume we start polling the roothub, which calls
xhci_hub_status_data(). This function will check if there are any interesting
changes going on, and mark these ports.
If no activity then it asks usb core to stop roothub polling.
xhci_hub_status_data() does not check for USB_PORT_LS_SS_INACTIVE, or compliance mode,
or USB3 polling state, which all would need more attention.
Could make sense to add those.
Does the below code help your situation?
diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index e2eece6..70f9b26 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -1458,7 +1458,7 @@ int xhci_hub_status_data(struct usb_hcd *hcd, char *buf)
struct xhci_hcd *xhci = hcd_to_xhci(hcd);
int max_ports;
struct xhci_bus_state *bus_state;
- bool reset_change = false;
+ bool poll_roothub = false;
struct xhci_hub *rhub;
struct xhci_port **ports;
@@ -1491,16 +1491,23 @@ int xhci_hub_status_data(struct usb_hcd *hcd, char *buf)
trace_xhci_hub_status_data(i, temp);
if ((temp & mask) != 0 ||
- (bus_state->port_c_suspend & 1 << i) ||
- (bus_state->resume_done[i] && time_after_eq(
+ (bus_state->port_c_suspend & 1 << i) ||
+ (temp & PORT_PLS_MASK) == XDEV_COMP_MODE ||
+ (temp & PORT_PLS_MASK) == XDEV_INACTIVE ||
+ (bus_state->resume_done[i] && time_after_eq(
jiffies, bus_state->resume_done[i]))) {
buf[(i + 1) / 8] |= 1 << (i + 1) % 8;
status = 1;
}
- if ((temp & PORT_RC))
- reset_change = true;
+ /*
+ * don't stop roothub polling if port is in polling state as it
+ * can end up in compliance mode without issuing any event
+ */
+ if ((temp & PORT_RC) ||
+ (temp & PORT_PLS_MASK) == XDEV_POLLING)
+ poll_roothub = true;
}
- if (!status && !reset_change) {
+ if (!status && !poll_roothub) {
xhci_dbg(xhci, "%s: stopping port polling.\n", __func__);
clear_bit(HCD_FLAG_POLL_RH, &hcd->flags);
}
-Mathias
