From 19d41a2899dcca7862af453d62a8e61933f3252d Mon Sep 17 00:00:00 2001 From: A Sun Date: Sun, 14 Jul 2019 22:51:26 -0400 Subject: media: mceusb: USB reset device following USB clear halt error This patch schedules a USB reset device call following a USB clear halt error. The issues solved, and patch implementation, are similar to those found in drivers/hid/usbhid/hid-core.c. As seen on very rare occasions approximately one time per month (mceusb device 2304:0225 in this sample) Jul 27 2018 15:09:39 [59388.696941] mceusb 1-1.1.2:1.0: Error: urb status = -32 (RX HALT) [59388.698838] mceusb 1-1.1.2:1.0: rx clear halt error -32 the device can get into RX or TX HALT state where usb_clear_halt() also fails and also returns -EPIPE (HALT/STALL). After which, all further mceusb device control and data I/O always fail with HALT/STALL. Subsequently, the entire mceusb device no longer functions. Cause and problem replication conditions remain unknown. Further troubleshooting reveals usb_reset_device() restores mceusb device operation. Patch test 1: Hot unplugging the mceusb device triggers USB RX HALT and USB clear halt errors. A mceusb_dev_disconnect() call follows unplug. This patch's reset device call invokes an extra mceusb_dev_probe() mceusb_dev_disconnect() cycle, before the mceusb driver detaches. The additional probe/disconnect verifies the patch's device reset code executed. But note this patch is for USB clear halt error cases not caused by unplugging the mceusb device. Patch test 2: Simulate a RX HALT and a clear halt error with instrumented code in the driver. Jul 12 2019 19:41:18 [522745.263104] mceusb 1-1.3:1.0: set rx halt retval, 0 [522745.263943] mceusb 1-1.3:1.0: Error: rx urb status = -32 (RX HALT) [522745.263970] mceusb 1-1.3:1.0: kevent 1 scheduled [522745.264016] mceusb 1-1.3:1.0: kevent handler called (flags 0x2) [522745.272883] mceusb 1-1.3:1.0: rx clear halt status = 0 [522745.272917] mceusb 1-1.3:1.0: stuck RX HALT state requires USB Reset Device to clear [522745.273005] mceusb 1-1.3:1.0: mceusb_dev_disconnect called [522745.702815] usb 1-1.3: reset full-speed USB device number 14 using dwc_otg [522745.836812] mceusb 1-1.3:1.0: mceusb_dev_probe called [522745.836823] mceusb 1-1.3:1.0: acceptable bulk inbound endpoint found [522745.836832] mceusb 1-1.3:1.0: acceptable bulk outbound endpoint found ... The result matches what is expected when the device gets into a real rx clear halt error case by itself. This is the same sequence of messages when manually invoking the ./usbreset command line utility with an unpatched mceusb driver. Signed-off-by: A Sun Signed-off-by: Sean Young Signed-off-by: Mauro Carvalho Chehab --- drivers/media/rc/mceusb.c | 67 ++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 55 insertions(+), 12 deletions(-) (limited to 'drivers/media/rc') diff --git a/drivers/media/rc/mceusb.c b/drivers/media/rc/mceusb.c index 4d5351ebb940..bc74c09ddeb6 100644 --- a/drivers/media/rc/mceusb.c +++ b/drivers/media/rc/mceusb.c @@ -461,6 +461,7 @@ struct mceusb_dev { /* usb */ struct usb_device *usbdev; + struct usb_interface *usbintf; struct urb *urb_in; unsigned int pipe_in; struct usb_endpoint_descriptor *usb_ep_out; @@ -517,6 +518,7 @@ struct mceusb_dev { unsigned long kevent_flags; # define EVENT_TX_HALT 0 # define EVENT_RX_HALT 1 +# define EVENT_RST_PEND 31 }; /* MCE Device Command Strings, generally a port and command pair */ @@ -758,8 +760,15 @@ static void mceusb_dev_printdata(struct mceusb_dev *ir, u8 *buf, int buf_len, static void mceusb_defer_kevent(struct mceusb_dev *ir, int kevent) { set_bit(kevent, &ir->kevent_flags); + + if (test_bit(EVENT_RST_PEND, &ir->kevent_flags)) { + dev_dbg(ir->dev, "kevent %d dropped pending USB Reset Device", + kevent); + return; + } + if (!schedule_work(&ir->kevent)) - dev_err(ir->dev, "kevent %d may have been dropped", kevent); + dev_dbg(ir->dev, "kevent %d already scheduled", kevent); else dev_dbg(ir->dev, "kevent %d scheduled", kevent); } @@ -1398,28 +1407,59 @@ static void mceusb_deferred_kevent(struct work_struct *work) container_of(work, struct mceusb_dev, kevent); int status; + dev_err(ir->dev, "kevent handler called (flags 0x%lx)", + ir->kevent_flags); + + if (test_bit(EVENT_RST_PEND, &ir->kevent_flags)) { + dev_err(ir->dev, "kevent handler canceled pending USB Reset Device"); + return; + } + if (test_bit(EVENT_RX_HALT, &ir->kevent_flags)) { usb_unlink_urb(ir->urb_in); status = usb_clear_halt(ir->usbdev, ir->pipe_in); + dev_err(ir->dev, "rx clear halt status = %d", status); if (status < 0) { - dev_err(ir->dev, "rx clear halt error %d", - status); + /* + * Unable to clear RX halt/stall. + * Will need to call usb_reset_device(). + */ + dev_err(ir->dev, + "stuck RX HALT state requires USB Reset Device to clear"); + usb_queue_reset_device(ir->usbintf); + set_bit(EVENT_RST_PEND, &ir->kevent_flags); + clear_bit(EVENT_RX_HALT, &ir->kevent_flags); + + /* Cancel all other error events and handlers */ + clear_bit(EVENT_TX_HALT, &ir->kevent_flags); + return; } clear_bit(EVENT_RX_HALT, &ir->kevent_flags); - if (status == 0) { - status = usb_submit_urb(ir->urb_in, GFP_KERNEL); - if (status < 0) { - dev_err(ir->dev, - "rx unhalt submit urb error %d", - status); - } + status = usb_submit_urb(ir->urb_in, GFP_KERNEL); + if (status < 0) { + dev_err(ir->dev, "rx unhalt submit urb error = %d", + status); } } if (test_bit(EVENT_TX_HALT, &ir->kevent_flags)) { status = usb_clear_halt(ir->usbdev, ir->pipe_out); - if (status < 0) - dev_err(ir->dev, "tx clear halt error %d", status); + dev_err(ir->dev, "tx clear halt status = %d", status); + if (status < 0) { + /* + * Unable to clear TX halt/stall. + * Will need to call usb_reset_device(). + */ + dev_err(ir->dev, + "stuck TX HALT state requires USB Reset Device to clear"); + usb_queue_reset_device(ir->usbintf); + set_bit(EVENT_RST_PEND, &ir->kevent_flags); + clear_bit(EVENT_TX_HALT, &ir->kevent_flags); + + /* Cancel all other error events and handlers */ + clear_bit(EVENT_RX_HALT, &ir->kevent_flags); + return; + } clear_bit(EVENT_TX_HALT, &ir->kevent_flags); } } @@ -1581,6 +1621,7 @@ static int mceusb_dev_probe(struct usb_interface *intf, if (!ir->urb_in) goto urb_in_alloc_fail; + ir->usbintf = intf; ir->usbdev = usb_get_dev(dev); ir->dev = &intf->dev; ir->len_in = maxp; @@ -1688,6 +1729,8 @@ static void mceusb_dev_disconnect(struct usb_interface *intf) struct usb_device *dev = interface_to_usbdev(intf); struct mceusb_dev *ir = usb_get_intfdata(intf); + dev_dbg(&intf->dev, "%s called", __func__); + usb_set_intfdata(intf, NULL); if (!ir) -- cgit v1.2.3