Re: [PATCH v2] usbcore: add check on usb device's state before trying to disable lpm

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, Sep 01, 2013 at 09:34:13PM -0400, Alan Stern wrote:
> On Mon, 2 Sep 2013, Xenia Ragiadakou wrote:
> 
> > This patch adds a check to ensure that the device's state is not NOTATTACHED,
> > ATTACHED, POWERED or RECONNECTING before trying to disable lpm, because if
> > the device is in one of those states the control transfer to disable
> > device-initiated LPM will fail (as well as any transfer, since usb_submit_urb()
> > will fail).
> > 
> > Signed-off-by: Xenia Ragiadakou <burzalodowa@xxxxxxxxx>
> > Reported-by: Martin MOKREJS <mmokrejs@xxxxxxxxx>
> > ---
> >  drivers/usb/core/hub.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> > index fe8d95d..a6c10f0 100644
> > --- a/drivers/usb/core/hub.c
> > +++ b/drivers/usb/core/hub.c
> > @@ -3716,7 +3716,7 @@ int usb_disable_lpm(struct usb_device *udev)
> >  {
> >  	struct usb_hcd *hcd;
> >  
> > -	if (!udev || !udev->parent ||
> > +	if (!udev || udev->state < USB_STATE_UNAUTHENTICATED || !udev->parent ||
> >  			udev->speed != USB_SPEED_SUPER ||
> >  			!udev->lpm_capable)
> >  		return 0;
> 
> You should ask Sarah to check this out, because link power management
> is used only with USB-3 devices.

Unfortunately, this isn't the right fix.  You need to push down the
check to just before the LPM functions attempt to send control transfers
to the device, but you need to allow the code to send control transfers
to the parent hub.

That's because the USB core needs to reset the U1/U2 timeout values for
the port on the parent hub.  Otherwise the next device that connects to
that port will have LPM enabled.  For example, the xHCI driver needs to
know that the U1/U2 timeouts are disabled, so it can modify the
roothub's U1/U2 timeouts and device data structures.  Therefore,
hcd->driver->disable_usb3_lpm_timeout needs to be called as well.

So, you need to push down the check against the device state to avoid
the control transfers to the device in:
 - usb_req_set_sel
 - usb_set_device_initiated_lpm

usb_set_device_initiated_lpm() already checks whether the device is in
the configured state, but usb_req_set_sel needs to as well.  I would
explicitly check for the device to be in the USB_STATE_CONFIGURED state
instead of saying < USB_STATE_UNAUTHENTICATED, in case the USB core ever
adds a new state.

Sarah Sharp
--
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




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux