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 <as1033x@comcast.net>
Signed-off-by: Sean Young <sean@mess.org>
Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>

diff --git a/drivers/media/rc/mceusb.c b/drivers/media/rc/mceusb.c
index 4d5351ebb9408b02a2f13fbacedb47f4c98c51cc..bc74c09ddeb6a49b320c6d8dec172cdfc4242721 100644 (file)
--- 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)