Skip to content

mceusb: coding style & comments update, for -EPIPE error patches

A Sun linux-media 1 version New

Get series with b4
b4 am [email protected]

Versions newest first

  1. v1 Latest1 patch 0 repliesNew

Patches in v1 No applies recorded yet

#Subject and filesSizeRepliesApplied
1/1mceusb: coding style & comments update for -EPIPE error patchesdrivers/media/rc/mceusb.c+20 -190 replies

Reviews trailers per patch

No Reviewed-by, Acked-by or Tested-by trailers for v1 yet.

Read the v1 thread

Cover letter [PATCH 0/1] [media] mceusb: coding style & comments update, for -EPIPE error patches

Hi Sean,
Thanks for the comments for:
[PATCH v2] [media] mceusb: TX -EPIPE (urb status = -32) lockup fix
I received another email indicating the patch in accepted state, so I'm following up with a subsequent patch for cosmetic (coding style and comment) changes to incorporate your review comments.

Quoted 1 line · Sean Young

On Thu, Apr 13, 2017 at 04:06:47AM -0400, A Sun wrote:

<snip>

Quoted 11 lines

Review static void mceusb_deferred_kevent(struct work_struct *work) · line 1224

Quoted 6 lines
(quoted diff) +1 -1
@@ -1220,16 +1221,28 @@ static void mceusb_deferred_kevent(struct work_struct *work)
 		if (status < 0) {
 			dev_err(ir->dev, "rx clear halt error %d",
 				status);
-			return;
+			goto done_rx_halt;

This function can easily be re-written without gotos and it will be much more readible.

Quoted 1 line
(quoted diff) +0 -0
 		}

I've left this goto for now (framework to abort error handling in deeply nested if statements), since I'm testing the following possible future enhancement:

@@ -1222,7 +1222,15 @@ static void mceusb_deferred_kevent(struc

                if (status < 0) {
                        dev_err(ir->dev, "rx clear halt error %d",
                                status);
-                       goto done_rx_halt;
+
+                       /* usb_reset_configuration() also resets tx */
+                       status = usb_reset_configuration(ir->usbdev);
+                       if (status < 0) {

+ dev_err(ir->dev, "rx usb reset configuration error %d",
+ status);
+ goto done_rx_halt;
+ }

+                       clear_bit(EVENT_TX_HALT, &ir->kevent_flags);
                }
                clear_bit(EVENT_RX_HALT, &ir->kevent_flags);
                status = usb_submit_urb(ir->urb_in, GFP_KERNEL);

I have since discovered and observed usb clear halt can fail too during normal usage while running the non-debug mceusb driver.

Apr 18 00:07:43 raspberrypi kernel: [ 11.627760] Bluetooth: BNEP socket layer initialized
Apr 18 19:49:22 raspberrypi kernel: [70890.799375] mceusb 1-1.2:1.0: Error: urb status = -32 (RX HALT)
Apr 18 19:49:22 raspberrypi kernel: [70890.800789] mceusb 1-1.2:1.0: rx clear halt error -32

This is the only time I saw this over several weeks. I don't know the cause or condition for replication. So whether 2nd stage error recovery with usb_reset_configuration() is effective is not yet known.

Thanks, ..A Sun

Read the cover letter in its thread