On Thu, Aug 29, 2013 at 11:18:21AM -0700, Sarah Sharp wrote:
> On Thu, Aug 29, 2013 at 10:06:16AM -0700, Greg Kroah-Hartman wrote:
> > Hi Sarah,
> >
> > I'm getting the following warnings from the 3.10.9 kernel all the time
> > when I unplug a USB 3 storage device from my laptop:
> > [203282.987687] usb 4-1: USB disconnect, device number 21
> > [203282.992904] usb 4-1: Set SEL for device-initiated U1 failed.
> > [203282.992909] usb 4-1: Set SEL for device-initiated U2 failed.
> >
> > What can a "normal" user do with these "failed" messages? If nothing,
> > shouldn't we just turn them into debug messages instead?
>
> Yes, those messages should probably be toned down to debug level instead
> of warning level. If a device doesn't respond to the Set SEL request
> when USB 3.0 LPM is enabled, the user has a buggy device. Of course, I
> doubt anyone is going to return a drive based on those messages.
Which device is "buggy" the USB 3 device (in this case, a USB flash
drive), or the USB host controller in the laptop?
> That error message happens because the USB core is attempting to disable
> LPM for a disconnected device. The control transfer to set SEL fails,
> resulting in those messages. The xHCI driver still needs to disable the
> U1 and U2 timeouts for the port, so the core still needs to call into
> usb_set_lpm_timeout. However, we could skip the control transfer to the
> device.
>
> The problem is that the USB core doesn't mark the device as DISCONNECTED
> until after it attempts to disable LPM. The device is still marked as
> being in the configured state, because we don't return early in this
> function:
>
> static int usb_set_device_initiated_lpm(struct usb_device *udev,
> enum usb3_link_state state, bool enable)
> {
> int ret;
> int feature;
>
> switch (state) {
> case USB3_LPM_U1:
> feature = USB_DEVICE_U1_ENABLE;
> break;
> case USB3_LPM_U2:
> feature = USB_DEVICE_U2_ENABLE;
> break;
> default:
> dev_warn(&udev->dev, "%s: Can't %s non-U1 or U2 state.\n",
> __func__, enable ? "enable" : "disable");
> return -EINVAL;
> }
>
> if (udev->state != USB_STATE_CONFIGURED) {
> dev_dbg(&udev->dev, "%s: Can't %s %s state "
> "for unconfigured device.\n",
> __func__, enable ? "enable" : "disable",
> usb3_lpm_names[state]);
> return 0;
> }
>
> if (enable) {
> /*
> * Now send the control transfer to enable device-initiated LPM
> * for either U1 or U2.
> */
> ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
> USB_REQ_SET_FEATURE,
> USB_RECIP_DEVICE,
> feature,
> 0, NULL, 0,
> USB_CTRL_SET_TIMEOUT);
> } else {
> ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
> USB_REQ_CLEAR_FEATURE,
> USB_RECIP_DEVICE,
> feature,
> 0, NULL, 0,
> USB_CTRL_SET_TIMEOUT);
> }
> if (ret < 0) {
> dev_warn(&udev->dev, "%s of device-initiated %s failed.\n",
> enable ? "Enable" : "Disable",
> usb3_lpm_names[state]);
> return -EBUSY;
> }
> return 0;
> }
>
> So I don't know how the LPM code can know the device is disconnected, and thus
> it should skip the control transfer. Do we get an -ENODEV in that case?
We should, but as I don't see that error message, is that what is really
happening here?
Shouldn't everyone with a USB3 device be seing these messages if it's a
disconnect issue?
> The least we could do is change that dev_warn to dev_dbg instead.
Ok, I can do that :)
But, given that this should happen for all USB3 devices that are
removed, I think the root cause should be fixed, otherwise it will just
be annoying debug messages that we can't do anything about, right?
thanks,
greg k-h
--
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
