On Wed, May 21, 2014 at 1:12 PM, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote:
> On Tue, 20 May 2014, Dan Williams wrote:
>
>> Resuming a powered down port sometimes results in the port state being
>> stuck in the training sequence.
>>
>> hub 3-0:1.0: debounce: port 1: total 2000ms stable 0ms status 0x2e0
>> port1: can't get reconnection after setting port power on, status -110
>> hub 3-0:1.0: port 1 status 0000.02e0 after resume, -19
>> usb 3-1: can't resume, status -19
>> hub 3-0:1.0: logical disconnect on port 1
>>
>> In the case above we wait for the port re-connect timeout of 2 seconds
>> and observe that the port status is USB_SS_PORT_LS_POLLING (although it
>> is likely toggling between this state and USB_SS_PORT_LS_RX_DETECT).
>> This is indicative of a case where the device is failing to progress the
>> link training state machine.
>>
>> It is resolved by issuing a warm reset to get the hub and device link
>> state machines back in sync.
>>
>> hub 3-0:1.0: debounce: port 1: total 2000ms stable 0ms status 0x2e0
>> usb usb3: port1 usb_port_runtime_resume requires warm reset
>> hub 3-0:1.0: port 1 not warm reset yet, waiting 50ms
>> usb 3-1: reset SuperSpeed USB device number 2 using xhci_hcd
>>
>> After a reconnect timeout when we expect the device to be present, force
>> a warm reset of the device. Note that we can not simply look at the
>> link status to determine if a warm reset is required as any of the
>> training states USB_SS_PORT_LS_POLLING, USB_SS_PORT_LS_RX_DETECT, or
>> USB_SS_PORT_LS_COMP_MOD are valid states that do not indicate the need
>> for warm reset by themselves.
>
>
>> --- a/drivers/usb/core/hub.c
>> +++ b/drivers/usb/core/hub.c
>> @@ -2570,10 +2570,12 @@ static int hub_port_reset(struct usb_hub *hub, int port1,
>> /* Is a USB 3.0 port in the Inactive or Compliance Mode state?
>> * Port worm reset is required to recover
>> */
>> -static bool hub_port_warm_reset_required(struct usb_hub *hub, u16 portstatus)
>> +static bool hub_port_warm_reset_required(struct usb_hub *hub, int port1,
>> + u16 portstatus)
>> {
>> return hub_is_superspeed(hub->hdev) &&
>> - (((portstatus & USB_PORT_STAT_LINK_STATE) ==
>> + (test_bit(port1, hub->warm_reset_bits) ||
>> + ((portstatus & USB_PORT_STAT_LINK_STATE) ==
>> USB_SS_PORT_LS_SS_INACTIVE) ||
>> ((portstatus & USB_PORT_STAT_LINK_STATE) ==
>> USB_SS_PORT_LS_COMP_MOD)) ;
>
> This might be a little more clear if you separate out the tests and
> remove the excess parens:
>
> unsigned ls;
>
> if (!hub_is_superspeed(hub->hdev))
> return 0;
> if (test_bit(port1, hub->warm_reset_bits))
> return 1;
> ls = portstatus & USB_PORT_STAT_LINK_STATE;
> return ls == USB_SS_PORT_LS_SS_INACTIVE ||
> ls == USB_SS_PORT_LS_COMP_MOD;
>
Fixed.
>> @@ -2839,8 +2843,13 @@ static int check_port_resume_type(struct usb_device *udev,
>> {
>> struct usb_port *port_dev = hub->ports[port1 - 1];
>>
>> + /* Is a warm reset needed to recover the connection? */
>> + if (udev->reset_resume
>> + && hub_port_warm_reset_required(hub, port1, portstatus)) {
>> + /* pass */;
>
> You mustn't call hub_port_warm_reset_required when status < 0, because
> then the portstatus value is undefined.
Good catch.
> Have you tested this after turning off the device's persist_enabled
> flag? Without a reset-resume, this will go through an awkward sequence
> involving disabling the port. I think you still end up performing the
> warm reset, but it's worth checking.
Fixed.
The warm reset is still performed, however the "force warm reset" flag
was not being cleared correctly.
--
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
