Commit 7ed849b9 authored by Alan Stern's avatar Alan Stern Committed by Willy Tarreau

HID: usbhid: fix inconsistent reset/resume/reset-resume behavior

commit 972e6a99 upstream.

The usbhid driver has inconsistently duplicated code in its post-reset,
resume, and reset-resume pathways.

	reset-resume doesn't check HID_STARTED before trying to
	restart the I/O queues.

	resume fails to clear the HID_SUSPENDED flag if HID_STARTED
	isn't set.

	resume calls usbhid_restart_queues() with usbhid->lock held
	and the others call it without holding the lock.

The first item in particular causes a problem following a reset-resume
if the driver hasn't started up its I/O.  URB submission fails because
usbhid->urbin is NULL, and this triggers an unending reset-retry loop.

This patch fixes the problem by creating a new subroutine,
hid_restart_io(), to carry out all the common activities.  It also
adds some checks that were missing in the original code:

	After a reset, there's no need to clear any halted endpoints.

	After a resume, if a reset is pending there's no need to
	restart any I/O until the reset is finished.

	After a resume, if the interrupt-IN endpoint is halted there's
	no need to submit the input URB until the halt has been
	cleared.
Signed-off-by: default avatarAlan Stern <stern@rowland.harvard.edu>
Reported-by: default avatarDaniel Fraga <fragabr@gmail.com>
Tested-by: default avatarDaniel Fraga <fragabr@gmail.com>
CC: <stable@vger.kernel.org>
Signed-off-by: default avatarJiri Kosina <jkosina@suse.cz>
Signed-off-by: default avatarWilly Tarreau <w@1wt.eu>
parent 6a2ba9c0
...@@ -988,14 +988,6 @@ static int usbhid_output_raw_report(struct hid_device *hid, __u8 *buf, size_t co ...@@ -988,14 +988,6 @@ static int usbhid_output_raw_report(struct hid_device *hid, __u8 *buf, size_t co
return ret; return ret;
} }
static void usbhid_restart_queues(struct usbhid_device *usbhid)
{
if (usbhid->urbout && !test_bit(HID_OUT_RUNNING, &usbhid->iofl))
usbhid_restart_out_queue(usbhid);
if (!test_bit(HID_CTRL_RUNNING, &usbhid->iofl))
usbhid_restart_ctrl_queue(usbhid);
}
static void hid_free_buffers(struct usb_device *dev, struct hid_device *hid) static void hid_free_buffers(struct usb_device *dev, struct hid_device *hid)
{ {
struct usbhid_device *usbhid = hid->driver_data; struct usbhid_device *usbhid = hid->driver_data;
...@@ -1412,6 +1404,37 @@ static void hid_cease_io(struct usbhid_device *usbhid) ...@@ -1412,6 +1404,37 @@ static void hid_cease_io(struct usbhid_device *usbhid)
usb_kill_urb(usbhid->urbout); usb_kill_urb(usbhid->urbout);
} }
static void hid_restart_io(struct hid_device *hid)
{
struct usbhid_device *usbhid = hid->driver_data;
int clear_halt = test_bit(HID_CLEAR_HALT, &usbhid->iofl);
int reset_pending = test_bit(HID_RESET_PENDING, &usbhid->iofl);
spin_lock_irq(&usbhid->lock);
clear_bit(HID_SUSPENDED, &usbhid->iofl);
usbhid_mark_busy(usbhid);
if (clear_halt || reset_pending)
schedule_work(&usbhid->reset_work);
usbhid->retry_delay = 0;
spin_unlock_irq(&usbhid->lock);
if (reset_pending || !test_bit(HID_STARTED, &usbhid->iofl))
return;
if (!clear_halt) {
if (hid_start_in(hid) < 0)
hid_io_error(hid);
}
spin_lock_irq(&usbhid->lock);
if (usbhid->urbout && !test_bit(HID_OUT_RUNNING, &usbhid->iofl))
usbhid_restart_out_queue(usbhid);
if (!test_bit(HID_CTRL_RUNNING, &usbhid->iofl))
usbhid_restart_ctrl_queue(usbhid);
spin_unlock_irq(&usbhid->lock);
}
/* Treat USB reset pretty much the same as suspend/resume */ /* Treat USB reset pretty much the same as suspend/resume */
static int hid_pre_reset(struct usb_interface *intf) static int hid_pre_reset(struct usb_interface *intf)
{ {
...@@ -1461,14 +1484,14 @@ static int hid_post_reset(struct usb_interface *intf) ...@@ -1461,14 +1484,14 @@ static int hid_post_reset(struct usb_interface *intf)
return 1; return 1;
} }
/* No need to do another reset or clear a halted endpoint */
spin_lock_irq(&usbhid->lock); spin_lock_irq(&usbhid->lock);
clear_bit(HID_RESET_PENDING, &usbhid->iofl); clear_bit(HID_RESET_PENDING, &usbhid->iofl);
clear_bit(HID_CLEAR_HALT, &usbhid->iofl);
spin_unlock_irq(&usbhid->lock); spin_unlock_irq(&usbhid->lock);
hid_set_idle(dev, intf->cur_altsetting->desc.bInterfaceNumber, 0, 0); hid_set_idle(dev, intf->cur_altsetting->desc.bInterfaceNumber, 0, 0);
status = hid_start_in(hid);
if (status < 0) hid_restart_io(hid);
hid_io_error(hid);
usbhid_restart_queues(usbhid);
return 0; return 0;
} }
...@@ -1491,25 +1514,9 @@ void usbhid_put_power(struct hid_device *hid) ...@@ -1491,25 +1514,9 @@ void usbhid_put_power(struct hid_device *hid)
#ifdef CONFIG_PM #ifdef CONFIG_PM
static int hid_resume_common(struct hid_device *hid, bool driver_suspended) static int hid_resume_common(struct hid_device *hid, bool driver_suspended)
{ {
struct usbhid_device *usbhid = hid->driver_data; int status = 0;
int status;
spin_lock_irq(&usbhid->lock);
clear_bit(HID_SUSPENDED, &usbhid->iofl);
usbhid_mark_busy(usbhid);
if (test_bit(HID_CLEAR_HALT, &usbhid->iofl) ||
test_bit(HID_RESET_PENDING, &usbhid->iofl))
schedule_work(&usbhid->reset_work);
usbhid->retry_delay = 0;
usbhid_restart_queues(usbhid);
spin_unlock_irq(&usbhid->lock);
status = hid_start_in(hid);
if (status < 0)
hid_io_error(hid);
hid_restart_io(hid);
if (driver_suspended && hid->driver && hid->driver->resume) if (driver_suspended && hid->driver && hid->driver->resume)
status = hid->driver->resume(hid); status = hid->driver->resume(hid);
return status; return status;
...@@ -1576,12 +1583,8 @@ static int hid_suspend(struct usb_interface *intf, pm_message_t message) ...@@ -1576,12 +1583,8 @@ static int hid_suspend(struct usb_interface *intf, pm_message_t message)
static int hid_resume(struct usb_interface *intf) static int hid_resume(struct usb_interface *intf)
{ {
struct hid_device *hid = usb_get_intfdata (intf); struct hid_device *hid = usb_get_intfdata (intf);
struct usbhid_device *usbhid = hid->driver_data;
int status; int status;
if (!test_bit(HID_STARTED, &usbhid->iofl))
return 0;
status = hid_resume_common(hid, true); status = hid_resume_common(hid, true);
dev_dbg(&intf->dev, "resume status %d\n", status); dev_dbg(&intf->dev, "resume status %d\n", status);
return 0; return 0;
...@@ -1590,10 +1593,8 @@ static int hid_resume(struct usb_interface *intf) ...@@ -1590,10 +1593,8 @@ static int hid_resume(struct usb_interface *intf)
static int hid_reset_resume(struct usb_interface *intf) static int hid_reset_resume(struct usb_interface *intf)
{ {
struct hid_device *hid = usb_get_intfdata(intf); struct hid_device *hid = usb_get_intfdata(intf);
struct usbhid_device *usbhid = hid->driver_data;
int status; int status;
clear_bit(HID_SUSPENDED, &usbhid->iofl);
status = hid_post_reset(intf); status = hid_post_reset(intf);
if (status >= 0 && hid->driver && hid->driver->reset_resume) { if (status >= 0 && hid->driver && hid->driver->reset_resume) {
int ret = hid->driver->reset_resume(hid); int ret = hid->driver->reset_resume(hid);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment