On Thu, 2019-12-19 at 14:06 +0200, Mathias Nyman wrote:
> commit 057d476fff778f1d3b9f861fdb5437ea1a3cfc99 upstream
>
> Backport for 4.9 and 4.4 stable kernels
This seems to be needed for 3.16 as well, so I've added this to the
queue with a minor adjustment.
Ben.
> A race in xhci USB3 remote wake handling may force device back to suspend
> after it initiated resume siganaling, causing a missed resume event or warm
> reset of device.
>
> When a USB3 link completes resume signaling and goes to enabled (UO)
> state a interrupt is issued and the interrupt handler will clear the
> bus_state->port_remote_wakeup resume flag, allowing bus suspend.
>
> If the USB3 roothub thread just finished reading port status before
> the interrupt, finding ports still in suspended (U3) state, but hasn't
> yet started suspending the hub, then the xhci interrupt handler will clear
> the flag that prevented roothub suspend and allow bus to suspend, forcing
> all port links back to suspended (U3) state.
>
> Example case:
> usb_runtime_suspend() # because all ports still show suspended U3
> usb_suspend_both()
> hub_suspend(); # successful as hub->wakeup_bits not set yet
> ==> INTERRUPT
> xhci_irq()
> handle_port_status()
> clear bus_state->port_remote_wakeup
> usb_wakeup_notification()
> sets hub->wakeup_bits;
> kick_hub_wq()
> <== END INTERRUPT
> hcd_bus_suspend()
> xhci_bus_suspend() # success as port_remote_wakeup bits cleared
>
> Fix this by increasing roothub usage count during port resume to prevent
> roothub autosuspend, and by making sure bus_state->port_remote_wakeup
> flag is only cleared after resume completion is visible, i.e.
> after xhci roothub returned U0 or other non-U3 link state link on a
> get port status request.
>
> Issue rootcaused by Chiasheng Lee
>
> Cc: Lee Hou-hsun <hou-hsun.lee@xxxxxxxxx>
> Cc: Lee Chiasheng <chiasheng.lee@xxxxxxxxx>
> Reported-by: Lee Chiasheng <chiasheng.lee@xxxxxxxxx>
> Signed-off-by: Mathias Nyman <mathias.nyman@xxxxxxxxxxxxxxx>
> ---
> drivers/usb/host/xhci-hub.c | 8 ++++++++
> drivers/usb/host/xhci-ring.c | 6 +-----
> drivers/usb/host/xhci.h | 1 +
> 3 files changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
> index 39e2d3271035..1d9cb29400f3 100644
> --- a/drivers/usb/host/xhci-hub.c
> +++ b/drivers/usb/host/xhci-hub.c
> @@ -760,6 +760,14 @@ static u32 xhci_get_port_status(struct usb_hcd *hcd,
> status |= USB_PORT_STAT_C_BH_RESET << 16;
> if ((raw_port_status & PORT_CEC))
> status |= USB_PORT_STAT_C_CONFIG_ERROR << 16;
> +
> + /* USB3 remote wake resume signaling completed */
> + if (bus_state->port_remote_wakeup & (1 << wIndex) &&
> + (raw_port_status & PORT_PLS_MASK) != XDEV_RESUME &&
> + (raw_port_status & PORT_PLS_MASK) != XDEV_RECOVERY) {
> + bus_state->port_remote_wakeup &= ~(1 << wIndex);
> + usb_hcd_end_port_resume(&hcd->self, wIndex);
> + }
> }
>
> if (hcd->speed < HCD_USB3) {
> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> index 69ad9817076a..b426c83ecb9b 100644
> --- a/drivers/usb/host/xhci-ring.c
> +++ b/drivers/usb/host/xhci-ring.c
> @@ -1609,9 +1609,6 @@ static void handle_port_status(struct xhci_hcd *xhci,
> usb_hcd_resume_root_hub(hcd);
> }
>
> - if (hcd->speed >= HCD_USB3 && (temp & PORT_PLS_MASK) == XDEV_INACTIVE)
> - bus_state->port_remote_wakeup &= ~(1 << faked_port_index);
> -
> if ((temp & PORT_PLC) && (temp & PORT_PLS_MASK) == XDEV_RESUME) {
> xhci_dbg(xhci, "port resume event for port %d\n", port_id);
>
> @@ -1630,6 +1627,7 @@ static void handle_port_status(struct xhci_hcd *xhci,
> bus_state->port_remote_wakeup |= 1 << faked_port_index;
> xhci_test_and_clear_bit(xhci, port_array,
> faked_port_index, PORT_PLC);
> + usb_hcd_start_port_resume(&hcd->self, faked_port_index);
> xhci_set_link_state(xhci, port_array, faked_port_index,
> XDEV_U0);
> /* Need to wait until the next link state change
> @@ -1667,8 +1665,6 @@ static void handle_port_status(struct xhci_hcd *xhci,
> if (slot_id && xhci->devs[slot_id])
> xhci_ring_device(xhci, slot_id);
> if (bus_state->port_remote_wakeup & (1 << faked_port_index)) {
> - bus_state->port_remote_wakeup &=
> - ~(1 << faked_port_index);
> xhci_test_and_clear_bit(xhci, port_array,
> faked_port_index, PORT_PLC);
> usb_wakeup_notification(hcd->self.root_hub,
> diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
> index de4771ce0df6..424c07d1ac0e 100644
> --- a/drivers/usb/host/xhci.h
> +++ b/drivers/usb/host/xhci.h
> @@ -316,6 +316,7 @@ struct xhci_op_regs {
> #define XDEV_U3 (0x3 << 5)
> #define XDEV_INACTIVE (0x6 << 5)
> #define XDEV_POLLING (0x7 << 5)
> +#define XDEV_RECOVERY (0x8 << 5)
> #define XDEV_COMP_MODE (0xa << 5)
> #define XDEV_RESUME (0xf << 5)
> /* true: port has power (see HCC_PPC) */
--
Ben Hutchings
Larkinson's Law: All laws are basically false.
Attachment:
signature.asc
Description: This is a digitally signed message part
