On 01/26/2012 07:36 AM, Sarah Sharp wrote:
> xHCI roothubs go through slightly different port state machines when
> either a device initiates a remote wakeup and signals resume, or when
> the host initiates a resume.
>
> According to section 4.19.1.2.13 of the xHCI 1.0 spec, on host-initiated
> resume, the xHC port state machine automatically goes through the U3Exit
> state into the U0 state, setting the port link state change (PLC) bit in
> the process.
>
> When a device initiates resume, the xHCI port state machine goes into
> the "Resume" state and sets the PLC bit. Then the xHCI driver writes U0
> into the port link state register to transition the port to U0 from the
> Resume state.
>
> We can't be sure the device is actually in the U0 state until we receive
> the next port status change event with the PLC bit set. We really don't
> want khubd to be polling the roothub port status bits until the device
> is really in U0.
>
> Andiry, this is a pretty unlikely race condition to hit, but do you
> think it should be queued for stable? khubd could see the device in a
> resuming state, and maybe even start issuing transfers before the device
> fully wakes up.
>
Sorry for the late reply. Return from a long vacation and drown in mails...
Yes, I think the patch is reasonable and compliant with 4.15.2.1. You
can queue it to stable.
Thanks,
Andiry
> Signed-off-by: Sarah Sharp <sarah.a.sharp@xxxxxxxxxxxxxxx>
> Cc: Andiry Xu <andiry.xu@xxxxxxx>
> ---
> drivers/usb/host/xhci-ring.c | 29 +++++++++++++++++------------
> 1 files changed, 17 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> index 5a818cb..67149bb 100644
> --- a/drivers/usb/host/xhci-ring.c
> +++ b/drivers/usb/host/xhci-ring.c
> @@ -1321,20 +1321,16 @@ static void handle_port_status(struct xhci_hcd *xhci,
> }
>
> if (DEV_SUPERSPEED(temp)) {
> - xhci_dbg(xhci, "resume SS port %d\n", port_id);
> + xhci_dbg(xhci, "remote wake SS port %d\n", port_id);
> + xhci_test_and_clear_bit(xhci, port_array,
> + faked_port_index, PORT_PLC);
> xhci_set_link_state(xhci, port_array, faked_port_index,
> XDEV_U0);
> - slot_id = xhci_find_slot_id_by_port(hcd, xhci,
> - faked_port_index + 1);
> - if (!slot_id) {
> - xhci_dbg(xhci, "slot_id is zero\n");
> - goto cleanup;
> - }
> - xhci_ring_device(xhci, slot_id);
> - xhci_dbg(xhci, "resume SS port %d finished\n", port_id);
> - /* Clear PORT_PLC */
> - xhci_test_and_clear_bit(xhci, port_array,
> - faked_port_index, PORT_PLC);
> + /* Need to wait until the next link state change
> + * indicates the device is actually in U0.
> + */
> + bogus_port_status = true;
> + goto cleanup;
> } else {
> xhci_dbg(xhci, "resume HS port %d\n", port_id);
> bus_state->resume_done[faked_port_index] = jiffies +
> @@ -1345,6 +1341,15 @@ static void handle_port_status(struct xhci_hcd *xhci,
> }
> }
>
> + if ((temp & PORT_PLC) && (temp & PORT_PLS_MASK) == XDEV_U0 &&
> + DEV_SUPERSPEED(temp)) {
> + xhci_dbg(xhci, "resume SS port %d finished\n", port_id);
> + slot_id = xhci_find_slot_id_by_port(hcd, xhci,
> + faked_port_index + 1);
> + if (slot_id && xhci->devs[slot_id])
> + xhci_ring_device(xhci, slot_id);
> + }
> +
> if (hcd->speed != HCD_USB3)
> xhci_test_and_clear_bit(xhci, port_array, faked_port_index,
> PORT_PLC);
--
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
