On Thu, 1 Mar 2018, Mathias Nyman wrote:
> Disabing Latency Tolerance Messaging before port reset is unnecessary.
> LTM is automatically disabled at port reset.
>
> If host can't communicate with the device the LTM message will fail, and
> the hub driver will unnecessarily do a logical disconnect.
> Broken communication is ofter the reason for a reset in the first place.
>
> Additionally we can't guarantee device is in a configured state,
> epecially in reset-resume case when root hub lost power.
> LTM can't be modified unless device is in a configured state.
>
> Just remove LTM disabling before port reset.
That definitely sounds like the right approach.
> Details about LTM and port reset in USB 3 specification:
>
> USB 3 spec section 9.4.5
> "The LTM Enable field can be modified by the SetFeature() and
> ClearFeature() requests using the LTM_ENABLE feature selector.
> This field is reset to zero when the device is reset."
>
> USB 3 spec section 9.4.1
> "The device shall process a Clear Feature (U1_Enable or U2_Enable or
> LTM_Enable) only if the device is in the configured state."
>
> Signed-off-by: Mathias Nyman <mathias.nyman@xxxxxxxxxxxxxxx>
Acked-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
> ---
> drivers/usb/core/hub.c | 12 +++---------
> 1 file changed, 3 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index c5c1f6c..ac7bab7 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -5505,21 +5505,15 @@ static int usb_reset_and_verify_device(struct usb_device *udev)
> if (udev->usb2_hw_lpm_enabled == 1)
> usb_set_usb2_hardware_lpm(udev, 0);
>
> - /* Disable LPM and LTM while we reset the device and reinstall the alt
> - * settings. Device-initiated LPM settings, and system exit latency
> - * settings are cleared when the device is reset, so we have to set
> - * them up again.
> + /* Disable LPM while we reset the device and reinstall the alt settings.
> + * Device-initiated LPM, and system exit latency settings are cleared
> + * when the device is reset, so we have to set them up again.
> */
In theory, since you're changing this comment anyway, it could be
reformatted into the more commonly accepted form:
/*
* blah, blah, blah
* blah, blah, blah
*/
But it's not a big deal.
> ret = usb_unlocked_disable_lpm(udev);
> if (ret) {
> dev_err(&udev->dev, "%s Failed to disable LPM\n", __func__);
> goto re_enumerate_no_bos;
> }
> - ret = usb_disable_ltm(udev);
> - if (ret) {
> - dev_err(&udev->dev, "%s Failed to disable LTM\n", __func__);
> - goto re_enumerate_no_bos;
> - }
>
> bos = udev->bos;
> udev->bos = NULL;
Alan Stern
--
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
