[PATCH v1] [media] mceusb: fix out of bounds read in MCE receiver buffer
Fix multiple cases of out of bounds (OOB) read associated with
MCE device receive/input data handling.
In reference for the OOB cases below, the incoming/read (byte) data
format when the MCE device responds to a command is:
{ cmd_prefix, subcmd, data0, data1, ... }
where cmd_prefix are:
MCE_CMD_PORT_SYS
MCE_CMD_PORT_IR
and subcmd examples are:
MCE_RSP_GETPORTSTATUS
MCE_RSP_EQIRNUMPORTS
...
Response size dynamically depends on cmd_prefix and subcmd.
So data0, data1, ... may or may not be present on input.
Multiple responses may return in a single receiver buffer.
The trigger condition for OOB read is typically random or
corrupt input data that fills the mceusb receiver buffer.
Case 1:
mceusb_handle_command() reads data0 (var hi) and data1 (var lo)
regardless of whether the response includes such data.
If { cmd_prefix, subcmd } is at the end of the receiver buffer,
read past end of buffer occurs.
This case was reported by
KASAN: slab-out-of-bounds Read in mceusb_dev_recv
https://syzkaller.appspot.com/bug?extid=c7fdb6cb36e65f2fe8c9
Fix: In mceusb_handle_command(), change variable hi and lo to
pointers, and dereference only when required.
Case 2:
If response with data is truncated at end of buffer after
{ cmd_prefix, subcmd }, mceusb_handle_command() reads past
end of buffer for data0, data1, ...
Fix: In mceusb_process_ir_data(), check response size with
remaining buffer size before invoking mceusb_handle_command().
+if (i + ir->rem < buf_len)
mceusb_handle_command(ir, &ir->buf_in[i - 1]);
Case 3:
mceusb_handle_command() handles invalid/bad response such as
{ 0x??, MCE_RSP_GETPORTSTATUS } of length 2 as a response
{ MCE_CMD_PORT_SYS, MCE_RSP_GETPORTSTATUS, data0, ... }
of length 7. Read OOB occurs for non-existent data0, data1, ...
Cause is mceusb_handle_command() does not check cmd_prefix value.
Fix: mceusb_handle_command() must test both cmd_prefix and subcmd.
Case 4:
mceusb_process_ir_data() receiver parser state SUBCMD is
possible at start (i=0) of receiver buffer resulting in buffer
offset=-1 passed to mceusb_dev_printdata().
Bad offset results in OOB read before start of buffer.
[1214218.580308] mceusb 1-1.3:1.0: rx data[0]: 00 80 (length=2)
[1214218.580323] mceusb 1-1.3:1.0: Unknown command 0x00 0x80
...
[1214218.580406] mceusb 1-1.3:1.0: rx data[14]: 7f 7f (length=2)
[1214218.679311] mceusb 1-1.3:1.0: rx data[-1]: 80 90 (length=2)
[1214218.679325] mceusb 1-1.3:1.0: End of raw IR data
[1214218.679340] mceusb 1-1.3:1.0: rx data[1]: 7f 7f (length=2)
Fix: If parser_state is SUBCMD after processing receiver buffer,
reset parser_state to CMD_HEADER.
In effect, discard cmd_prefix at end of receiver buffer.
In mceusb_dev_printdata(), abort if buffer offset is out of bounds.
Case 5:
If response with data is truncated at end of buffer after
{ cmd_prefix, subcmd }, mceusb_dev_printdata() reads past
end of buffer for data0, data1, ...
while decoding the response to print out.
Fix: In mceusb_dev_printdata(), remove unneeded buffer offset
adjustments (var start and var skip) associated with MCE gen1 header.
Test for truncated MCE cmd response (compare offset+len with buf_len)
and skip decoding of incomplete response.
Move IR data tracing to execute before the truncation test.
Signed-off-by: A Sun
---
drivers/media/rc/mceusb.c | 141 --
1 file changed, 98 insertions(+), 43 deletions(-)
diff --git a/drivers/media/rc/mceusb.c b/drivers/media/rc/mceusb.c
index f9ed855e2..9c1cf53b0 100644
--- a/drivers/media/rc/mceusb.c
+++ b/drivers/media/rc/mceusb.c
@@ -559,7 +559,7 @@ static int mceusb_cmd_datasize(u8 cmd, u8 subcmd)
datasize = 4;
break;
case MCE_CMD_G_REVISION:
- datasize = 2;
+ datasize = 4;
break;
case MCE_RSP_EQWAKESUPPORT:
case MCE_RSP_GETWAKESOURCE:
@@ -595,14 +595,9 @@ static void mceusb_dev_printdata(struct mceusb_dev *ir, u8
*buf, int buf_len,
char *inout;
u8 cmd, subcmd, *data;
struct device *dev = ir->dev;
- int start, skip = 0;
u32 carrier, period;
- /* skip meaningless 0xb1 0x60 header bytes on orig receiver */
- if (ir->flags.microsoft_gen1 && !out && !offset)
- skip = 2;
-
- if (len <= skip)
+ if (offset < 0 || offset >= buf_len)
return;
dev_dbg(dev, "%cx data[%d]: %*ph (len=%d sz=%d)",
@@ -611,11 +606,32 @@ static void mceusb_dev_printdata(struct mceusb_dev *ir,
u8 *buf, int buf_len,
inout = out ? "Request" : "Got";
- start = offset + skip;
- cmd= buf[start] & 0xff;
- subcmd = buf[start + 1] & 0xff;
- data = buf + start + 2;
+ cmd= buf[offset];
+ subcmd = (offset + 1 < buf_len) ? buf[of
[PATCH v1] [media] mceusb: fix (eliminate) TX IR signal length limit
length = 128, urb->status = 0) [1549306.614861] mceusb 1-1.3:1.0: tx data[0]: 9e 01 81 01 81 01 81 01 81 01 81 01 81 01 81 01 81 01 81 01 81 01 81 01 81 01 81 01 81 01 81 9e 01 81 01 81 01 81 01 81 01 81 01 81 01 81 01 81 01 81 01 81 01 81 01 81 01 81 01 81 01 81 9e 01 (len=128 sz=128) [1549306.614869] mceusb 1-1.3:1.0: Raw IR data, 30 pulse/space samples [1549306.620199] mceusb 1-1.3:1.0: tx done status = 128 (wait = 100, expire = 100 (1000ms), urb->actual_length = 128, urb->status = 0) [1549306.620212] mceusb 1-1.3:1.0: tx data[0]: 89 81 01 81 01 81 01 81 01 81 80 (len=11 sz=11) [1549306.620221] mceusb 1-1.3:1.0: Raw IR data, 9 pulse/space samples [1549306.633294] mceusb 1-1.3:1.0: tx done status = 11 (wait = 98, expire = 100 (1000ms), urb->actual_length = 11, urb->status = 0) Hex dumps limited to 64 bytes. 0x81 is MCE minimum time pulse, 0x01 is MCE minimum time space. TX IR part 3 sz=11 shows 20msec I/O blocking delay (100expire - 98wait = 2jiffies) Signed-off-by: A Sun --- drivers/media/rc/mceusb.c | 334 +++--- 1 file changed, 196 insertions(+), 138 deletions(-) diff --git a/drivers/media/rc/mceusb.c b/drivers/media/rc/mceusb.c index 8ba1e2b7e..f9ed855e2 100644 --- a/drivers/media/rc/mceusb.c +++ b/drivers/media/rc/mceusb.c @@ -31,21 +31,22 @@ #include #include -#define DRIVER_VERSION "1.94" +#define DRIVER_VERSION "1.95" #define DRIVER_AUTHOR "Jarod Wilson " #define DRIVER_DESC"Windows Media Center Ed. eHome Infrared Transceiver " \ "device driver" #define DRIVER_NAME"mceusb" +#define USB_TX_TIMEOUT 1000 /* in milliseconds */ #define USB_CTRL_MSG_SZ2 /* Size of usb ctrl msg on gen1 hw */ #define MCE_G1_INIT_MSGS 40 /* Init messages on gen1 hw to throw out */ /* MCE constants */ -#define MCE_CMDBUF_SIZE384 /* MCE Command buffer length */ +#define MCE_IRBUF_SIZE 128 /* TX IR buffer length */ #define MCE_TIME_UNIT 50 /* Approx 50us resolution */ -#define MCE_CODE_LENGTH5/* Normal length of packet (with header) */ -#define MCE_PACKET_SIZE4/* Normal length of packet (without header) */ -#define MCE_IRDATA_HEADER 0x84 /* Actual header format is 0x80 + num_bytes */ +#define MCE_PACKET_SIZE31 /* Max length of packet (with header) */ +#define MCE_IRDATA_HEADER (0x80 + MCE_PACKET_SIZE - 1) +/* Actual format is 0x80 + num_bytes */ #define MCE_IRDATA_TRAILER 0x80 /* End of IR data */ #define MCE_MAX_CHANNELS 2/* Two transmitters, hardware dependent? */ #define MCE_DEFAULT_TX_MASK0x03 /* Vals: TX1=0x01, TX2=0x02, ALL=0x03 */ @@ -604,9 +605,9 @@ static void mceusb_dev_printdata(struct mceusb_dev *ir, u8 *buf, int buf_len, if (len <= skip) return; - dev_dbg(dev, "%cx data: %*ph (length=%d)", - (out ? 't' : 'r'), - min(len, buf_len - offset), buf + offset, len); + dev_dbg(dev, "%cx data[%d]: %*ph (len=%d sz=%d)", + (out ? 't' : 'r'), offset, + min(len, buf_len - offset), buf + offset, len, buf_len); inout = out ? "Request" : "Got"; @@ -728,6 +729,9 @@ static void mceusb_dev_printdata(struct mceusb_dev *ir, u8 *buf, int buf_len, case MCE_RSP_CMD_ILLEGAL: dev_dbg(dev, "Illegal PORT_IR command"); break; + case MCE_RSP_TX_TIMEOUT: + dev_dbg(dev, "IR TX timeout (TX buffer underrun)"); + break; default: dev_dbg(dev, "Unknown command 0x%02x 0x%02x", cmd, subcmd); @@ -742,13 +746,14 @@ static void mceusb_dev_printdata(struct mceusb_dev *ir, u8 *buf, int buf_len, dev_dbg(dev, "End of raw IR data"); else if ((cmd != MCE_CMD_PORT_IR) && ((cmd & MCE_PORT_MASK) == MCE_COMMAND_IRDATA)) - dev_dbg(dev, "Raw IR data, %d pulse/space samples", ir->rem); + dev_dbg(dev, "Raw IR data, %d pulse/space samples", + cmd & MCE_PACKET_LENGTH_MASK); #endif } /* * Schedule work that can't be done in interrupt handlers - * (mceusb_dev_recv() and mce_async_callback()) nor tasklets. + * (mceusb_dev_recv() and mce_write_callback()) nor tasklets. * Invokes mceusb_deferred_kevent() for recovering from * error events specified by the kevent bit field. */ @@ -768,23 +773,80 @@ static void mceusb_defer_kevent(struct mceusb_dev *ir, int kevent) dev_dbg(ir->dev, "kevent %d scheduled", kevent); }
Re: [PATCH v2 2/3] [media] mceusb: Reword messages referring to "urb"
Hi again,
On 7/15/2019 8:28 AM, Sean Young wrote:
> Hi,
>
> On Tue, Jun 25, 2019 at 05:29:02PM -0400, A Sun wrote:
>> In a quick look though other USB drivers, here are some other possibilities:
>>
>> Endpoint-in / Endpoint-out (not concise)
>> ep-in / ep-out (abbrev may be too obscure)
>> in / out ("Error: in urb status.." confusing,
>> "in" is noun, adj, or verb?)
>> read / write (confusing, "read" is noun, adj, verb?)
>> RD / WR
>> RX / TX
>>
>> I suggest RX/TX. However, I'll have to leave it up to you to make a choice.
>
> TBH I've been mulling this over. I think you're right that the terms should
> be usb centric. TX seems confusing, there are mceusb devices with no IR
> transmit ports, so it would be surprising to see errors about TX. Your
> first option is usb centric and can be wordy in errors messages and concise
> in code (option #2).
>
> However as you're writing patches I'll leave it up to you. It would be nice
> if the usage is consistent in the driver code.
>
ok, I'll leave this patch proposal as is, where RX/TX is the favored
terminology.
rx/tx was already in use elsewhere outside the message text of this patch,
and is prevalent in mceusb_dev_printdata() output.
I also found that my later patch submission:
[PATCH v1] [media] mceusb: USB reset device following USB clear halt error
has an unintended dependency on this [PATCH v2 2/3].
FYI, I'm in progress on another mceusb patch to fix, and eliminate, the driver's
TX IR length limits. Limit causes -EINVAL errors for > ~300 pulse/space samples
and
I've seen reports (and patches for) of appliances with IR over 400 pulse/spaces.
The future patch rewrites:
mceusb_tx_ir()
And revises "write/tx" async I/O to sync I/O to do unlimited multipart TX IR.
These functions will need rewrite and rename:
mce_async_callback() -> mce_tx_callback()
mce_request_packet() -> mce_tx()
The present mce_async_out() name will become misleading. mce_command_out()
or mce_request_out() (which calls mce_tx()), are probably better names.
I'm still mulling over whether the more generic "read/write" term
(e.g. mce_write() and mce_write_callback()) may be a better migration path,
for future work.
Thanks, ..A Sun
Re: [PATCH v2 3/3] [media] mceusb: Show USB halt/stall error recovery
This patch is superseded by [PATCH v1] [media] mceusb: USB reset device following USB clear halt error
[PATCH v1] [media] mceusb: USB reset device following USB clear halt error
This patch supersedes
[PATCH v2 3/3][media] mceusb: Show USB halt/stall error recovery
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
---
drivers/media/rc/mceusb.c | 67 ++-
1 file changed, 55 insertions(+), 12 deletions(-)
diff --git a/drivers/media/rc/mceusb.c b/drivers/media/rc/mceusb.c
index efffb1795..2e86876f2 100644
--- a/drivers/media/rc/mceusb.c
+++ b/drivers/media/rc/mceusb.c
@@ -467,6 +467,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;
@@ -523,6 +524,7 @@ struct mceusb_dev {
unsigned long kevent_flags;
# define EVENT_TX_HALT0
# define EVENT_RX_HALT1
+# define EVENT_RST_PEND 31
};
/* MCE Device Command Strings, generally a port and command pair */
@@ -764,8 +766,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);
}
@@ -1404,28 +1413,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->d
Re: [PATCH v2 2/3] [media] mceusb: Reword messages referring to "urb"
Hi again,
On 6/25/2019 12:12 PM, Sean Young wrote:
> Hello,
>
> On Tue, Jun 25, 2019 at 11:01:32AM -0400, A Sun wrote:
>> On 6/25/2019 6:51 AM, Sean Young wrote:
>>> On Wed, Jun 19, 2019 at 03:54:21AM -0400, A Sun wrote:
>>>>
>>
>>>> - dev_err(ir->dev, "Error: request urb status = %d (TX HALT)",
>>>> + dev_err(ir->dev, "Error: tx urb status = %d (TX HALT)",
>>>
>>> I am not sure this makes things clearer. Some of the code still refers
>>> to request, e.g. mce_request_packet.
>>>
>>> Since this is an IR device, there is IR TX and RX; however this unrelated
>>> to that.
>>>
>>> There is one urb/endpoint on which commands are sent; on the other, we
>>> receiver responses and IR data. Calling those TX and RX is not a good
>>> idea I think.
>>
>> The tx urb described is also for IR data TX.
>> IR TX is one primary function/purpose of the device.
>>
>> It happens that the urb/endpoint for IR TX is the same urb/endpoint for
>> mceusb device commands.
>
> The outgoing urb is for command, e.g. get version, set IR Rx timeout,
> set IR Rx carrier report, set IR wakeup protocol/scancode, resume
> device after suspend. Not everything is IR Tx.
>
Right. Not everything is command either.
The subject USB interface endpoint-out (and -in) is intended for data
(interrupt/bulk) transfer. The only true data here is IR data.
It's a quirk of mceusb devices to use endpoint-out as a command channel too,
where commands masquerade as data.
A quick sampling of other USB devices, I'm seeing they customarily use
USB default endpoint 0 and control transfers (rather than data transfers)
for device management commands.
>>> The existing code refers to request and response, and also TX and RX in
>>> places. The microsoft documentation refers to "command and response" which
>>> would be consistent with the #define's we have (MCE_CMD_* and MCE_RSP_*).
>>>
>>
>> My intent is to try and better word several of the USB layer messages,
>> and avoid confusing Microsoft MCE terms in those messages.
>>
>> The original "(request) urb" phrases expand to "request USB Request Block"
>> and " USB Request Block" which are particularly confusing, and
>> non-specific, respectively.
>
> How about command and response?
>
Still seem confusing and awkward to me, as in "response USB Request Block",
or "command USB Request Block" that might be IR data. "Command" also suggests
to me USB interface endpoint 0, which isn't the case with mceusb.
Again, my intent is to avoid Microsoft MCE specific terms in messages pertaining
to USB.
In a quick look though other USB drivers, here are some other possibilities:
Endpoint-in / Endpoint-out (not concise)
ep-in / ep-out (abbrev may be too obscure)
in / out("Error: in urb status.." confusing,
"in" is noun, adj, or verb?)
read / write(confusing, "read" is noun, adj, verb?)
RD / WR
RX / TX
I suggest RX/TX. However, I'll have to leave it up to you to make a choice.
Thanks, ..A Sun
Re: [PATCH v2 2/3] [media] mceusb: Reword messages referring to "urb"
Hi, On 6/25/2019 6:51 AM, Sean Young wrote: > On Wed, Jun 19, 2019 at 03:54:21AM -0400, A Sun wrote: >> >> -dev_err(ir->dev, "Error: request urb status = %d (TX HALT)", >> +dev_err(ir->dev, "Error: tx urb status = %d (TX HALT)", > > I am not sure this makes things clearer. Some of the code still refers > to request, e.g. mce_request_packet. > > Since this is an IR device, there is IR TX and RX; however this unrelated > to that. > > There is one urb/endpoint on which commands are sent; on the other, we > receiver responses and IR data. Calling those TX and RX is not a good > idea I think. The tx urb described is also for IR data TX. IR TX is one primary function/purpose of the device. It happens that the urb/endpoint for IR TX is the same urb/endpoint for mceusb device commands. The same is found for IR data RX. The urb/endpoint for IR RX is the same urb/endpoint for mceusb device responses (to commands). > > The existing code refers to request and response, and also TX and RX in > places. The microsoft documentation refers to "command and response" which > would be consistent with the #define's we have (MCE_CMD_* and MCE_RSP_*). > My intent is to try and better word several of the USB layer messages, and avoid confusing Microsoft MCE terms in those messages. The original "(request) urb" phrases expand to "request USB Request Block" and " USB Request Block" which are particularly confusing, and non-specific, respectively. Thanks, ..A Sun
[PATCH v2 0/3] [media] mceusb: Error message text revisions
patch v2 revisions: . limit patch to message changes only in part 3/3 Several error message revisions for mceusb.c proposed patches applicable to mceusb.c version https://github.com/torvalds/linux/blob/master/drivers/media/rc/mceusb.c Mar 1, 2019 commit 04ad30112aec61004f994d8f51461ec06e208e54
[PATCH v2 3/3] [media] mceusb: Show USB halt/stall error recovery
patch v2 revisions:
. Limit patch to message changes only.
Update dev_err() messages to report status (including success) for each
step of USB RX HALT and TX HALT error recovery. If error recovery fails,
show the message:
stuck RX HALT state requires USB Reset Device to clear
or
stuck TX HALT state requires USB Reset Device to clear
This patch only affects RX and TX HALT error reporting.
The capability for mceusb to invoke USB reset device for itself is
deferred to a future patch. It's unsafe to perform usb_reset_device()
when and where the driver posts the "... requires USB Reset ..."
message.
Users can fix their mceusb device manually by issuing
ioctl(fd, USBDEVFS_RESET, 0) to the mceusb device, as in:
$ sudo ./usbreset /dev/bus/usb/001/010
---
As seen on very rare occasions with mceusb device 2304:0225
[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 fails with HALT/STALL.
Cause and problem replication conditions are still unknown.
Further troubleshooting revealed that usb_reset_device() successfully
restores mceusb device operation.
Of note is "modprobe -r mceusb" with "modprobe mceusb"
usb_reset_endpoint(), and usb_reset_configuration(), were all
unsuccessful.
Signed-off-by: A Sun
---
drivers/media/rc/mceusb.c | 27 +--
1 file changed, 21 insertions(+), 6 deletions(-)
diff --git a/drivers/media/rc/mceusb.c b/drivers/media/rc/mceusb.c
index efffb1795..fffd826c6 100644
--- a/drivers/media/rc/mceusb.c
+++ b/drivers/media/rc/mceusb.c
@@ -765,7 +765,7 @@ static void mceusb_defer_kevent(struct mceusb_dev *ir, int
kevent)
{
set_bit(kevent, &ir->kevent_flags);
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);
}
@@ -1404,19 +1404,27 @@ 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_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");
}
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",
+ "rx unhalt submit urb error = %d",
status);
}
}
@@ -1424,8 +1432,15 @@ static void mceusb_deferred_kevent(struct work_struct
*work)
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");
+ }
clear_bit(EVENT_TX_HALT, &ir->kevent_flags);
}
}
--
2.11.0
[PATCH v2 1/3] [media] mceusb: Disable "nonsensical irdata" messages
mceusb device 2304:0225, and likely others, produces numerous
warnings:
[ 4231.111310] mceusb 1-1.1.2:1.0: nonsensical irdata 80 with duration 0
[ 4381.493597] mceusb 1-1.1.2:1.0: nonsensical irdata 80 with duration 0
[ 4410.247568] mceusb 1-1.1.2:1.0: nonsensical irdata 80 with duration 0
...
[60153.264064] mceusb 1-1.1.2:1.0: nonsensical irdata 00 with duration 0
...
due to reception of ambient IR noise.
Change these warning messages to debug messages.
Signed-off-by: A Sun
---
drivers/media/rc/mceusb.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/media/rc/mceusb.c b/drivers/media/rc/mceusb.c
index 82d4261c9..0cd8f6f57 100644
--- a/drivers/media/rc/mceusb.c
+++ b/drivers/media/rc/mceusb.c
@@ -1182,8 +1182,8 @@ static void mceusb_process_ir_data(struct mceusb_dev *ir,
int buf_len)
rawir.pulse = ((ir->buf_in[i] & MCE_PULSE_BIT) != 0);
rawir.duration = (ir->buf_in[i] & MCE_PULSE_MASK);
if (unlikely(!rawir.duration)) {
- dev_warn(ir->dev, "nonsensical irdata %02x with
duration 0",
-ir->buf_in[i]);
+ dev_dbg(ir->dev, "nonsensical irdata %02x with
duration 0",
+ ir->buf_in[i]);
break;
}
if (rawir.pulse) {
--
2.11.0
[PATCH v2 2/3] [media] mceusb: Reword messages referring to "urb"
Clarify messages referencing "request urb" to mean "tx urb"
(host transmit/send (to mceusb device)).
Qualify messages referencing plain "urb" to mean "rx urb"
(host receive (from mceusb device)).
Add missing "couldn't allocate rx urb" error message.
Clean extraneous "\n" in dev_dbg messages.
Signed-off-by: A Sun
---
drivers/media/rc/mceusb.c | 24 +---
1 file changed, 13 insertions(+), 11 deletions(-)
diff --git a/drivers/media/rc/mceusb.c b/drivers/media/rc/mceusb.c
index 0cd8f6f57..efffb1795 100644
--- a/drivers/media/rc/mceusb.c
+++ b/drivers/media/rc/mceusb.c
@@ -796,13 +796,13 @@ static void mce_async_callback(struct urb *urb)
break;
case -EPIPE:
- dev_err(ir->dev, "Error: request urb status = %d (TX HALT)",
+ dev_err(ir->dev, "Error: tx urb status = %d (TX HALT)",
urb->status);
mceusb_defer_kevent(ir, EVENT_TX_HALT);
break;
default:
- dev_err(ir->dev, "Error: request urb status = %d", urb->status);
+ dev_err(ir->dev, "Error: tx urb status = %d", urb->status);
break;
}
@@ -822,7 +822,7 @@ static void mce_request_packet(struct mceusb_dev *ir,
unsigned char *data,
async_urb = usb_alloc_urb(0, GFP_KERNEL);
if (unlikely(!async_urb)) {
- dev_err(dev, "Error, couldn't allocate urb!");
+ dev_err(dev, "Error: couldn't allocate tx urb!");
return;
}
@@ -1268,13 +1268,13 @@ static void mceusb_dev_recv(struct urb *urb)
return;
case -EPIPE:
- dev_err(ir->dev, "Error: urb status = %d (RX HALT)",
+ dev_err(ir->dev, "Error: rx urb status = %d (RX HALT)",
urb->status);
mceusb_defer_kevent(ir, EVENT_RX_HALT);
return;
default:
- dev_err(ir->dev, "Error: urb status = %d", urb->status);
+ dev_err(ir->dev, "Error: rx urb status = %d", urb->status);
break;
}
@@ -1544,27 +1544,27 @@ static int mceusb_dev_probe(struct usb_interface *intf,
if (ep_in == NULL) {
if (usb_endpoint_is_bulk_in(ep)) {
ep_in = ep;
- dev_dbg(&intf->dev, "acceptable bulk inbound
endpoint found\n");
+ dev_dbg(&intf->dev, "acceptable bulk inbound
endpoint found");
} else if (usb_endpoint_is_int_in(ep)) {
ep_in = ep;
ep_in->bInterval = 1;
- dev_dbg(&intf->dev, "acceptable interrupt
inbound endpoint found\n");
+ dev_dbg(&intf->dev, "acceptable interrupt
inbound endpoint found");
}
}
if (ep_out == NULL) {
if (usb_endpoint_is_bulk_out(ep)) {
ep_out = ep;
- dev_dbg(&intf->dev, "acceptable bulk outbound
endpoint found\n");
+ dev_dbg(&intf->dev, "acceptable bulk outbound
endpoint found");
} else if (usb_endpoint_is_int_out(ep)) {
ep_out = ep;
ep_out->bInterval = 1;
- dev_dbg(&intf->dev, "acceptable interrupt
outbound endpoint found\n");
+ dev_dbg(&intf->dev, "acceptable interrupt
outbound endpoint found");
}
}
}
if (!ep_in || !ep_out) {
- dev_dbg(&intf->dev, "required endpoints not found\n");
+ dev_dbg(&intf->dev, "required endpoints not found");
return -ENODEV;
}
@@ -1584,8 +1584,10 @@ static int mceusb_dev_probe(struct usb_interface *intf,
goto buf_in_alloc_fail;
ir->urb_in = usb_alloc_urb(0, GFP_KERNEL);
- if (!ir->urb_in)
+ if (!ir->urb_in) {
+ dev_err(&intf->dev, "Error: couldn't allocate rx urb!");
goto urb_in_alloc_fail;
+ }
ir->usbdev = usb_get_dev(dev);
ir->dev = &intf->dev;
--
2.11.0
Re: [PATCH v1 3/3] [media] mceusb: Show USB halt/stall error recovery
On 6/8/2019 4:37 AM, Sean Young wrote: > Hi, > >>> I think you can call usb_lock_device_for_reset() to prevent that problem. >> >> Deadlock still occurs in test: >> mceusb_deferred_kevent() >> ->usb_reset_device() >> ->mceusb_dev_disconnect() >> ->cancel_work_sync() >> where cancel_work_sync() blocks because mceusb_deferred_kevent() is active. > > I understand. The usb_reset_device() does not need to happen synchronously > in mceusb_deferred_kevent(). Possibly another delayed workqueue. > > Actually there is also a function usb_queue_reset_device() which might do > what you want here. > Thanks! Usb_queue_reset_device() appears to be exactly what I'm looking for. With the help of your tip, I found Alan Stern ran into a virtually identical problem (clear halt error and reset deadlock) with drivers/hid/usbhid/hid-core.c https://linux-usb.vger.kernel.narkive.com/fc7Lng6q/patch-usbhid-improve-handling-of-clear-halt-and-reset So patch for mceusb may be able to use the same techniques. >> >>> It would be nice to see this implemented too. >>> >> >> Any additional tips to implement would help! >> How to validate and test a rare condition with a larger audience? > > This is hard. Do you know the model of the mceusb and host hardware? > Host is Raspberry Pi 3B+ raspbian 4.14.xx version of mid year 2018. mceusb Pinnacle Systems PCTV Remote USB with mce emulator interface version 1 device 2304:0225 produced the sequence: [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 RX permanently stops after the error. All the work I'm doing for mceusb is with the above. The other case (https://ubuntuforums.org/showthread.php?t=2406882) I heard about is: Host ubuntu, mceusb unknown, possibly SMK eHome Infrared Transceiver with mce emulator interface version 1 Flooding of: mceusb 2-1.4:1.0: Error: urb status = -32 (RX HALT) >> Hence, the moved line. > > That's in a future patch. Please only change error strings in this patch. > ok, I'll post a v2 version of this patch with only the error strings changes. The instructions the error strings say (USB reset device manually) will be my patch resolution for the mceusb stuck halt bug for some time. In the meantime, I'll try and test (harder part) use of usb_queue_reset_device for a future patch to automate the USB reset procedure. Thanks again, ..A Sun
Re: [PATCH v1 3/3] [media] mceusb: Show USB halt/stall error recovery
Hi Sean, Thanks again for reviewing my patch submission. This patch updates mceusb RX and TX HALT error handling and recovery reporting, and provides placeholder for a later patch. The possible later patch would have mceusb call usb_reset_device() in place of the new "... requires USB Reset ..." message. I say "possible" because I'm running into multiple issues invoking usb_reset_device() from within mceusb. I'll also mention the difficulty obtaining even a single fault replication (~ 1/month) and test cycle. Additional comments below... ..A Sun On 6/6/2019 5:53 AM, Sean Young wrote: > On Sat, Jun 01, 2019 at 07:35:09PM -0400, A Sun wrote: >> Update dev_err() messages to report status (including success) for each >> step of USB RX HALT and TX HALT error recovery. If error recovery fails, >> show the message: >> stuck RX HALT state requires USB Reset Device to clear >> or >> stuck TX HALT state requires USB Reset Device to clear >> >> >> This patch just affects RX and TX HALT error handling and recovery >> reporting. >> >> A possible future patch may enable the mceusb driver to attempt >> itself usb_reset_device() when necessary. >> >> --- >> >> As seen on very rare occasions with mceusb device 2304:0225 >> [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 fails with HALT/STALL. >> Cause and problem replication conditions are still unknown. >> >> As seen in https://ubuntuforums.org/showthread.php?t=2406882 >> ... >> mceusb 2-1.4:1.0: Error: urb status = -32 (RX HALT) >> ... >> for an unknown mceusb device, usb_clear_halt() apparently can return >> false success. After which, RX HALT immediately reoccurs resulting in > > I'm having trouble understanding this. usb_clear_halt() returns an int. > Do you mean usb_clear_halt() returns 0? > >> a RX halt and clear halt infinite loop and message flooding. Again, >> cause and problem replication conditions remain unknown. > > So usb_clear_halt() returns 0 yet after the next usb_submit_urb() > the completion function gets -EPIPE again? Yes, for the infinite loop with the unknown mceusb device, RX completion gets -EPIPE, usb_clear_halt() probably returned 0 (success), but after the usb_submit_urb(), the RX completion again gets -EPIPE. > Are you trying to address this problem in this patch or just the error > reporting? If you're trying to change the error reporting then there is > no reason to move code around like you have. This patch is mainly just error reporting. A future patch would replace the "stuck HALT state" placeholder with code. The move code would be for the placeholder. The infinite loop case isn't resolved. But a false usb_clear_halt() status 0 (success) return would be printed/shown by this patch to confirm the infinite loop theory above. > >> >> After observing a rx clear halt fault with mceusb 2304:0225, further >> troubleshooting reveals usb_reset_device() is able to restore device >> functionality. >> $ lsusb >> Bus 001 Device 010: ID 2304:0225 Pinnacle Systems, Inc. Remote Kit >> Infrared Transceiver >> ... >> $ sudo ./usbreset /dev/bus/usb/001/010 # ioctl(fd, USBDEVFS_RESET, >> 0); >> Resetting USB device /dev/bus/usb/001/010 >> Reset successful >> $ dmesg >> ... >> [272392.540679] usb 1-1.2.7: reset full-speed USB device number 10 >> using dwc_otg >> [272392.676616] Registered IR keymap rc-rc6-mce >> ... >> [272392.678313] rc rc0: lirc_dev: driver ir-lirc-codec (mceusb) >> registered at minor = 0 > > Note the ir-lirc-codec. This is a pre-4.16 kernel. > >> [272392.678616] mceusb 1-1.2.7:1.0: long-range (0x1) receiver active >> [272392.940671] mceusb 1-1.2.7:1.0: Registered Pinnacle Systems PCTV >> Remote USB with mce emulator interface version 1 >> [272392.940687] mceusb 1-1.2.7:1.0: 2 tx ports (0x3 cabled) and 2 rx >> sensors (0x1 active) > > >> ... >> >> Additional findings are "modprobe -r mceusb" and "modprobe mceusb" >> are unable to clear stuck RX or TX HALT state. >> Attempts to call usb_reset_endpoint() and usb_reset_configuration() >> from within the mceusb dr
[PATCH v1 0/3] [media] mceusb: Error message text and reporting revisions
Several error message and error reporting revisions for mceusb.c proposed patches applicable to mceusb.c version https://github.com/torvalds/linux/blob/master/drivers/media/rc/mceusb.c Mar 1, 2019 commit 04ad30112aec61004f994d8f51461ec06e208e54 Test platform: Raspberry Pi 3B+ Linux raspberrypi 4.14.98-v7+ #1200 SMP Tue Feb 12 20:27:48 GMT 2019 armv7l GNU/Linux mceusb 1-1.2.7:1.0: long-range (0x1) receiver active mceusb 1-1.2.7:1.0: Registered Pinnacle Systems PCTV Remote USB with mce emulator interface version 1 mceusb 1-1.2.7:1.0: 2 tx ports (0x3 cabled) and 2 rx sensors (0x1 active)
[PATCH v1 3/3] [media] mceusb: Show USB halt/stall error recovery
Update dev_err() messages to report status (including success) for each
step of USB RX HALT and TX HALT error recovery. If error recovery fails,
show the message:
stuck RX HALT state requires USB Reset Device to clear
or
stuck TX HALT state requires USB Reset Device to clear
This and related message revisions pertain to functions:
mceusb_defer_kevent() error handler dispatcher
mceusb_deferred_kevent()error handler itself
This patch just affects RX and TX HALT error handling and recovery
reporting.
A possible future patch may enable the mceusb driver to attempt
itself usb_reset_device() when necessary.
---
As seen on very rare occasions with mceusb device 2304:0225
[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 fails with HALT/STALL.
Cause and problem replication conditions are still unknown.
As seen in https://ubuntuforums.org/showthread.php?t=2406882
...
mceusb 2-1.4:1.0: Error: urb status = -32 (RX HALT)
...
for an unknown mceusb device, usb_clear_halt() apparently can return
false success. After which, RX HALT immediately reoccurs resulting in
a RX halt and clear halt infinite loop and message flooding. Again,
cause and problem replication conditions remain unknown.
After observing a rx clear halt fault with mceusb 2304:0225, further
troubleshooting reveals usb_reset_device() is able to restore device
functionality.
$ lsusb
Bus 001 Device 010: ID 2304:0225 Pinnacle Systems, Inc. Remote Kit
Infrared Transceiver
...
$ sudo ./usbreset /dev/bus/usb/001/010 # ioctl(fd, USBDEVFS_RESET, 0);
Resetting USB device /dev/bus/usb/001/010
Reset successful
$ dmesg
...
[272392.540679] usb 1-1.2.7: reset full-speed USB device number 10
using dwc_otg
[272392.676616] Registered IR keymap rc-rc6-mce
...
[272392.678313] rc rc0: lirc_dev: driver ir-lirc-codec (mceusb)
registered at minor = 0
[272392.678616] mceusb 1-1.2.7:1.0: long-range (0x1) receiver active
[272392.940671] mceusb 1-1.2.7:1.0: Registered Pinnacle Systems PCTV
Remote USB with mce emulator interface version 1
[272392.940687] mceusb 1-1.2.7:1.0: 2 tx ports (0x3 cabled) and 2 rx
sensors (0x1 active)
...
Additional findings are "modprobe -r mceusb" and "modprobe mceusb"
are unable to clear stuck RX or TX HALT state.
Attempts to call usb_reset_endpoint() and usb_reset_configuration()
from within the mceusb driver also did not clear stuck HALTs.
A possible future patch could have the mceusb call usb_reset_device()
on itself, when necessary. Limiting the number of HALT error recovery
attempts may also be necessary to prevent halt/clear-halt loops.
Unresolved for now, deadlock complications occur if mceusb's worker
thread, mceusb_deferred_kevent(), calls usb_reset_device(),
which calls mceusb_dev_disconnect(), which calls cancel_work_sync(),
which waits on the still active worker thread.
Signed-off-by: A Sun
---
drivers/media/rc/mceusb.c | 35 +--
1 file changed, 25 insertions(+), 10 deletions(-)
diff --git a/drivers/media/rc/mceusb.c b/drivers/media/rc/mceusb.c
index efffb1795..5d81ccafc 100644
--- a/drivers/media/rc/mceusb.c
+++ b/drivers/media/rc/mceusb.c
@@ -765,7 +765,7 @@ static void mceusb_defer_kevent(struct mceusb_dev *ir, int
kevent)
{
set_bit(kevent, &ir->kevent_flags);
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);
}
@@ -1404,19 +1404,26 @@ static void mceusb_deferred_kevent(struct work_struct
*work)
container_of(work, struct mceusb_dev, kevent);
int status;
+ dev_info(ir->dev, "kevent handler called (flags 0x%lx)",
+ir->kevent_flags);
+
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);
- }
- clear_bit(EVENT_RX_HALT, &ir->kevent_flags);
- if (status == 0) {
+ /*
+* Unable to clear R
[PATCH v1 2/3] [media] mceusb: Reword messages referring to "urb"
Clarify messages referencing "request urb" to mean "tx urb"
(host transmit/send (to mceusb device)).
Qualify messages referencing plain "urb" to mean "rx urb"
(host receive (from mceusb device)).
Add missing "couldn't allocate rx urb" error message.
Clean extraneous "\n" in dev_dbg messages.
Signed-off-by: A Sun
---
drivers/media/rc/mceusb.c | 24 +---
1 file changed, 13 insertions(+), 11 deletions(-)
diff --git a/drivers/media/rc/mceusb.c b/drivers/media/rc/mceusb.c
index 0cd8f6f57..efffb1795 100644
--- a/drivers/media/rc/mceusb.c
+++ b/drivers/media/rc/mceusb.c
@@ -796,13 +796,13 @@ static void mce_async_callback(struct urb *urb)
break;
case -EPIPE:
- dev_err(ir->dev, "Error: request urb status = %d (TX HALT)",
+ dev_err(ir->dev, "Error: tx urb status = %d (TX HALT)",
urb->status);
mceusb_defer_kevent(ir, EVENT_TX_HALT);
break;
default:
- dev_err(ir->dev, "Error: request urb status = %d", urb->status);
+ dev_err(ir->dev, "Error: tx urb status = %d", urb->status);
break;
}
@@ -822,7 +822,7 @@ static void mce_request_packet(struct mceusb_dev *ir,
unsigned char *data,
async_urb = usb_alloc_urb(0, GFP_KERNEL);
if (unlikely(!async_urb)) {
- dev_err(dev, "Error, couldn't allocate urb!");
+ dev_err(dev, "Error: couldn't allocate tx urb!");
return;
}
@@ -1268,13 +1268,13 @@ static void mceusb_dev_recv(struct urb *urb)
return;
case -EPIPE:
- dev_err(ir->dev, "Error: urb status = %d (RX HALT)",
+ dev_err(ir->dev, "Error: rx urb status = %d (RX HALT)",
urb->status);
mceusb_defer_kevent(ir, EVENT_RX_HALT);
return;
default:
- dev_err(ir->dev, "Error: urb status = %d", urb->status);
+ dev_err(ir->dev, "Error: rx urb status = %d", urb->status);
break;
}
@@ -1544,27 +1544,27 @@ static int mceusb_dev_probe(struct usb_interface *intf,
if (ep_in == NULL) {
if (usb_endpoint_is_bulk_in(ep)) {
ep_in = ep;
- dev_dbg(&intf->dev, "acceptable bulk inbound
endpoint found\n");
+ dev_dbg(&intf->dev, "acceptable bulk inbound
endpoint found");
} else if (usb_endpoint_is_int_in(ep)) {
ep_in = ep;
ep_in->bInterval = 1;
- dev_dbg(&intf->dev, "acceptable interrupt
inbound endpoint found\n");
+ dev_dbg(&intf->dev, "acceptable interrupt
inbound endpoint found");
}
}
if (ep_out == NULL) {
if (usb_endpoint_is_bulk_out(ep)) {
ep_out = ep;
- dev_dbg(&intf->dev, "acceptable bulk outbound
endpoint found\n");
+ dev_dbg(&intf->dev, "acceptable bulk outbound
endpoint found");
} else if (usb_endpoint_is_int_out(ep)) {
ep_out = ep;
ep_out->bInterval = 1;
- dev_dbg(&intf->dev, "acceptable interrupt
outbound endpoint found\n");
+ dev_dbg(&intf->dev, "acceptable interrupt
outbound endpoint found");
}
}
}
if (!ep_in || !ep_out) {
- dev_dbg(&intf->dev, "required endpoints not found\n");
+ dev_dbg(&intf->dev, "required endpoints not found");
return -ENODEV;
}
@@ -1584,8 +1584,10 @@ static int mceusb_dev_probe(struct usb_interface *intf,
goto buf_in_alloc_fail;
ir->urb_in = usb_alloc_urb(0, GFP_KERNEL);
- if (!ir->urb_in)
+ if (!ir->urb_in) {
+ dev_err(&intf->dev, "Error: couldn't allocate rx urb!");
goto urb_in_alloc_fail;
+ }
ir->usbdev = usb_get_dev(dev);
ir->dev = &intf->dev;
--
2.11.0
[PATCH v1 1/3] [media] mceusb: Disable "nonsensical irdata" messages
mceusb device 2304:0225, and likely others, produces numerous
warnings:
[ 4231.111310] mceusb 1-1.1.2:1.0: nonsensical irdata 80 with duration 0
[ 4381.493597] mceusb 1-1.1.2:1.0: nonsensical irdata 80 with duration 0
[ 4410.247568] mceusb 1-1.1.2:1.0: nonsensical irdata 80 with duration 0
...
[60153.264064] mceusb 1-1.1.2:1.0: nonsensical irdata 00 with duration 0
...
due to reception of ambient IR noise.
Change these warning messages to debug messages.
Signed-off-by: A Sun
---
drivers/media/rc/mceusb.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/media/rc/mceusb.c b/drivers/media/rc/mceusb.c
index 82d4261c9..0cd8f6f57 100644
--- a/drivers/media/rc/mceusb.c
+++ b/drivers/media/rc/mceusb.c
@@ -1182,8 +1182,8 @@ static void mceusb_process_ir_data(struct mceusb_dev *ir,
int buf_len)
rawir.pulse = ((ir->buf_in[i] & MCE_PULSE_BIT) != 0);
rawir.duration = (ir->buf_in[i] & MCE_PULSE_MASK);
if (unlikely(!rawir.duration)) {
- dev_warn(ir->dev, "nonsensical irdata %02x with
duration 0",
-ir->buf_in[i]);
+ dev_dbg(ir->dev, "nonsensical irdata %02x with
duration 0",
+ ir->buf_in[i]);
break;
}
if (rawir.pulse) {
--
2.11.0
[PATCH v3 0/1] media: mceusb: add IR learning support features (IR carrier frequency measurement and wide-band/short-range receiver)
Hi Sean, Thanks again for your review and notes. I'm forwarding PATCH v3 after this note. Please also see my notes below. ..A Sun On 3/15/2018 8:46 AM, Sean Young wrote: > On Wed, Mar 14, 2018 at 03:32:02PM -0400, A Sun wrote: >> patch v2 revisions: >> . Carrier frequency measurement results were consistently low in patch v1. >>Improve measurement accuracy by adjusting IR carrier cycle count >>assuming 1 missed count per IR "on" pulse. >>Adjustments may need to be hardware specific, so future refinements >>may be necessary. > > I've retested my four mceusb devices. Three of them, including the original > microsoft mce ir receiver, need this fix. However, one with pid 0471 and > vid 2093 produces correct results before this change, and starts giving > carrier with numbers which are too high after this. > Well, it appears the fix will need to be applied on a device specific basis. In v3, I've added parameter "rx2" to mceusb_model to accomplish this. I'm not sure how to proceed further with this. Many of the mceusb supported devices would need to be checked to determine what "rx2" setting works best for the device. I can only check two other mceusb devices I have access to. Maybe later, after we complete this patch. Can you identify your other mceusb devices by v/p-id? That would help too. >> . Remove unneeded argument "enable" validation in >>mceusb_set_rx_wideband() and mceusb_set_rx_carrier_report(). >> . In mceusb_set_rx_carrier_report(), when enabling RX carrier report >> feature, >>also implicitly enable RX wide-band (short-range) receiver. >>Maintains consistency with winbond-cir, redrat3, ene-cir, IR receivers. > > So if carrier reports are enabled, and then disabled again, in your code > the wideband receiver remains enabled. Please can it be disabled again > when carrier reports are turned off again (while learning mode is off). > I've added code in V3 to revert to normal receiver after disabling carrier reports, but only if learning mode (wideband receiver) was not explicitly turned on. file mceusb-rx2-setting.txt: Mceusb confirmed device dependent setting for rx2 which provides the best results for learning mode IR receiver carrier frequency measurement. IR receiver only devices (no IR emitters (no_tx = 1)) are unlikely, and not required by specification, to support IR learning mode. So those devices may not need to be checked. * Original Microsoft MCE IR Transceiver (often HP-branded) * USB_DEVICE(VENDOR_MICROSOFT, 0x006d) rx2 = 2 * Philips IR transceiver (Dell branded) * USB_DEVICE(VENDOR_PHILIPS, 0x2093) rx2 = 1 * Pinnacle Remote Kit * USB_DEVICE(VENDOR_PINNACLE, 0x0225) rx2 = 2
[PATCH v3 1/1] media: mceusb: add IR learning support features (IR carrier frequency measurement and wide-band/short-range receiver)
patch v3 revisions:
. Add rx2 parameter to mceusb_model. Enables device specific
adjustments for carrier cycle counting during IR carrier frequency
measurement.
. When disabling RX carrier report, revert device to normal
non-learning mode IR receiver if learning mode (wide-band receiver)
was not explicitly enabled.
patch v2 revisions:
. Carrier frequency measurement results were consistently low in patch v1.
Improve measurement accuracy by adjusting IR carrier cycle count
assuming 1 missed count per IR "on" pulse.
Adjustments may need to be hardware specific, so future refinements
may be necessary.
. Remove unneeded argument "enable" validation in
mceusb_set_rx_wideband() and mceusb_set_rx_carrier_report().
. In mceusb_set_rx_carrier_report(), when enabling RX carrier report feature,
also implicitly enable RX wide-band (short-range) receiver.
Maintains consistency with winbond-cir, redrat3, ene-cir, IR receivers.
. Comment revisions and style corrections.
---
Windows Media Center IR transceivers include two IR receivers;
wide-band/short-range and narrow-band/long-range. The short-range
(5cm distance) receiver is for IR learning and has IR carrier
frequency measuring ability.
Add mceusb driver support to select the short range IR receiver
and enable pass through of its IR carrier frequency measurements.
RC and LIRC already support these mceusb driver additions.
Test platform:
Linux raspberrypi 4.9.59-v7+ #1047 SMP Sun Oct 29 12:19:23 GMT 2017 armv7l
GNU/Linux
mceusb 1-1.2:1.0: Registered Pinnacle Systems PCTV Remote USB with mce emulator
interface version 1
mceusb 1-1.2:1.0: 2 tx ports (0x0 cabled) and 2 rx sensors (0x1 active)
Sony TV remote control
ir-ctl from v4l-utils
pi@raspberrypi:~ $ ir-ctl -V
IR raw version 1.12.3
pi@raspberrypi:~ $ ir-ctl -m -r
...
pulse 650
space 550
pulse 650
space 600
pulse 600
space 600
pulse 1200
space 600
pulse 650
space 550
pulse 650
space 600
pulse 600
space 600
pulse 550
carrier 40004
space 16777215
^C
pi@raspberrypi:~ $ exit
Signed-off-by: A Sun
---
drivers/media/rc/mceusb.c | 152 ++
1 file changed, 140 insertions(+), 12 deletions(-)
diff --git a/drivers/media/rc/mceusb.c b/drivers/media/rc/mceusb.c
index a9187b0b4..f8c23d577 100644
--- a/drivers/media/rc/mceusb.c
+++ b/drivers/media/rc/mceusb.c
@@ -42,7 +42,7 @@
#include
#include
-#define DRIVER_VERSION "1.93"
+#define DRIVER_VERSION "1.94"
#define DRIVER_AUTHOR "Jarod Wilson "
#define DRIVER_DESC"Windows Media Center Ed. eHome Infrared Transceiver " \
"device driver"
@@ -198,6 +198,13 @@ struct mceusb_model {
u32 mce_gen3:1;
u32 tx_mask_normal:1;
u32 no_tx:1;
+ /*
+* 2nd IR receiver (short-range, wideband) for learning mode:
+* 0, absent 2nd receiver (rx2)
+* 1, rx2 present
+* 2, rx2 which under counts IR carrier cycles
+*/
+ u32 rx2;
int ir_intfnum;
@@ -209,9 +216,11 @@ static const struct mceusb_model mceusb_model[] = {
[MCE_GEN1] = {
.mce_gen1 = 1,
.tx_mask_normal = 1,
+ .rx2 = 2,
},
[MCE_GEN2] = {
.mce_gen2 = 1,
+ .rx2 = 2,
},
[MCE_GEN2_NO_TX] = {
.mce_gen2 = 1,
@@ -220,10 +229,12 @@ static const struct mceusb_model mceusb_model[] = {
[MCE_GEN2_TX_INV] = {
.mce_gen2 = 1,
.tx_mask_normal = 1,
+ .rx2 = 1,
},
[MCE_GEN3] = {
.mce_gen3 = 1,
.tx_mask_normal = 1,
+ .rx2 = 2,
},
[POLARIS_EVK] = {
/*
@@ -232,6 +243,7 @@ static const struct mceusb_model mceusb_model[] = {
* to allow testing it
*/
.name = "Conexant Hybrid TV (cx231xx) MCE IR",
+ .rx2 = 2,
},
[CX_HYBRID_TV] = {
.no_tx = 1, /* tx isn't wired up at all */
@@ -244,10 +256,12 @@ static const struct mceusb_model mceusb_model[] = {
[MULTIFUNCTION] = {
.mce_gen2 = 1,
.ir_intfnum = 2,
+ .rx2 = 2,
},
[TIVO_KIT] = {
.mce_gen2 = 1,
.rc_map = RC_MAP_TIVO,
+ .rx2 = 2,
},
[EVROMEDIA_FULL_HYBRID_FULLHD] = {
.name = "Evromedia USB Full Hybrid Full HD",
@@ -427,7 +441,8 @@ struct mceusb_dev {
struct rc_dev *rc;
/* optional features we can enable */
- bool learning_enabled;
+ bool carrier_report_enabled;
+ bool wideband_rx_enabled; /* aka learning mode, short-range rx */
/* core device bits */
struct device *dev;
@@ -458,6 +473,7 @@ struct mceusb_dev {
[PATCH v2 1/1] media: mceusb: add IR learning support features (IR carrier frequency measurement and wide-band/short-range receiver)
patch v2 revisions:
. Carrier frequency measurement results were consistently low in patch v1.
Improve measurement accuracy by adjusting IR carrier cycle count
assuming 1 missed count per IR "on" pulse.
Adjustments may need to be hardware specific, so future refinements
may be necessary.
. Remove unneeded argument "enable" validation in
mceusb_set_rx_wideband() and mceusb_set_rx_carrier_report().
. In mceusb_set_rx_carrier_report(), when enabling RX carrier report feature,
also implicitly enable RX wide-band (short-range) receiver.
Maintains consistency with winbond-cir, redrat3, ene-cir, IR receivers.
. Comment revisions and style corrections.
---
Windows Media Center IR transceivers include two IR receivers;
wide-band/short-range and narrow-band/long-range. The short-range
(5cm distance) receiver is for IR learning and has IR carrier
frequency measuring ability.
Add mceusb driver support to select the short range IR receiver
and enable pass through of its IR carrier frequency measurements.
RC and LIRC already support these mceusb driver additions.
Test platform:
Linux raspberrypi 4.9.59-v7+ #1047 SMP Sun Oct 29 12:19:23 GMT 2017 armv7l
GNU/Linux
mceusb 1-1.2:1.0: Registered Pinnacle Systems PCTV Remote USB with mce emulator
interface version 1
mceusb 1-1.2:1.0: 2 tx ports (0x0 cabled) and 2 rx sensors (0x1 active)
Sony TV remote control
ir-ctl from v4l-utils
pi@raspberrypi:~ $ ir-ctl -V
IR raw version 1.12.3
pi@raspberrypi:~ $ ir-ctl -m -r
...
pulse 650
space 550
pulse 650
space 600
pulse 600
space 600
pulse 1200
space 600
pulse 650
space 550
pulse 650
space 600
pulse 600
space 600
pulse 550
carrier 40004
space 16777215
^C
pi@raspberrypi:~ $ exit
Signed-off-by: A Sun
---
drivers/media/rc/mceusb.c | 109 ++
1 file changed, 101 insertions(+), 8 deletions(-)
diff --git a/drivers/media/rc/mceusb.c b/drivers/media/rc/mceusb.c
index a9187b0b4..392d26226 100644
--- a/drivers/media/rc/mceusb.c
+++ b/drivers/media/rc/mceusb.c
@@ -42,7 +42,7 @@
#include
#include
-#define DRIVER_VERSION "1.93"
+#define DRIVER_VERSION "1.94"
#define DRIVER_AUTHOR "Jarod Wilson "
#define DRIVER_DESC"Windows Media Center Ed. eHome Infrared Transceiver " \
"device driver"
@@ -427,6 +427,7 @@ struct mceusb_dev {
struct rc_dev *rc;
/* optional features we can enable */
+ bool carrier_report_enabled;
bool learning_enabled;
/* core device bits */
@@ -475,6 +476,10 @@ struct mceusb_dev {
u8 txports_cabled; /* bitmask of transmitters with cable */
u8 rxports_active; /* bitmask of active receive sensors */
+ /* receiver carrier frequency detection support */
+ u32 pulse_tunit;/* IR pulse "on" cumulative time units */
+ u32 pulse_count;/* pulse "on" count in measurement interval */
+
/*
* support for async error handler mceusb_deferred_kevent()
* where usb_clear_halt(), usb_reset_configuration(),
@@ -956,14 +961,62 @@ static int mceusb_set_tx_carrier(struct rc_dev *dev, u32
carrier)
}
/*
+ * Select or deselect the 2nd receiver port.
+ * Second receiver is learning mode, wide-band, short-range receiver.
+ * Only one receiver (long or short range) may be active at a time.
+ */
+static int mceusb_set_rx_wideband(struct rc_dev *dev, int enable)
+{
+ struct mceusb_dev *ir = dev->priv;
+ unsigned char cmdbuf[3] = { MCE_CMD_PORT_IR,
+ MCE_CMD_SETIRRXPORTEN, 0x00 };
+
+ dev_dbg(ir->dev, "select %s-range receive sensor",
+ enable ? "short" : "long");
+ /*
+* cmdbuf[2] is receiver port number
+* port 1 is long range receiver
+* port 2 is short range receiver
+*/
+ cmdbuf[2] = enable ? 2 : 1;
+ mce_async_out(ir, cmdbuf, sizeof(cmdbuf));
+
+ return 0;
+}
+
+/*
+ * Enable/disable receiver carrier frequency pass through reporting.
+ * Only the short-range receiver has carrier frequency measuring capability.
+ * Implicitly select this receiver when enabling carrier frequency reporting.
+ */
+static int mceusb_set_rx_carrier_report(struct rc_dev *dev, int enable)
+{
+ struct mceusb_dev *ir = dev->priv;
+
+ dev_dbg(ir->dev, "%s short-range receiver carrier reporting",
+ enable ? "enable" : "disable");
+ if (enable) {
+ if (!ir->learning_enabled)
+ mceusb_set_rx_wideband(dev, 1);
+ ir->carrier_report_enabled = true;
+ } else {
+ ir->carrier_report_enabled = false;
+ }
+
+ return 0;
+}
+
+/*
* We don't do anything but print debug spew for many of the command bits
* we receive f
[PATCH v2 0/1] media: mceusb: add IR learning support features (IR carrier frequency measurement and wide-band/short-range receiver)
Hi Sean,
Thanks again for your review and notes. I'm forwarding PATCH v2 after this
note. Please also see my notes below. ..A Sun
On 3/13/2018 6:38 AM, Sean Young wrote:
> Hi,
>
> On Sun, Mar 11, 2018 at 05:40:28AM -0400, A Sun wrote:
>>
>>
>> Add mceusb driver support to select the short range IR receiver
>> and enable pass through of its IR carrier frequency measurements.
>>
>> RC and LIRC already support these mceusb driver additions.
>
> That's great, this feature has been missing for a long time.
>
> I've tested it with my four mceusb devices, and I get carrier reports with
> all of them. Please see the notes below.
>
>> Test platform:
>> ...
>> pulse 600
>> space 600
>> pulse 1250
>> space 550
>> pulse 650
>> space 600
>> pulse 550
>> space 600
>> pulse 600
>> space 600
>> pulse 650
>> carrier 38803
>
> Sony protocol remotes have a 4Hz carrier, and I am getting lower values
> for the carrier with other carrier frequencies as well. Any idea why?
>
I'm also seeing consistently low Hz results myself. My guess is the mceusb
hardware may be under counting carrier cycles during IR pulse "on" it sees. I
see the following data from the receiver for a spurious IR:
[ 329.602445] mceusb 1-1.2:1.0: RX carrier frequency 0 Hz (pulse count = 1,
duration = 2, cycles = 0)
To see a pulse, there must exist a carrier, so the IR carrier cycle count
should be > 0.
In patch v2, I'm assuming the mceusb hardware misses 1 carrier cycle count for
every IR on pulse it sees and adjusted the carrier freq calculations
accordingly. The adjustments needed may turn out to be hardware specific, so
further refinements may be needed.
Did you see low Hz values for all your four mceusb devices?
>> +/*
>> + * cmdbuf[2] is receiver port number
>> + * port 1 is long range receiver
>> + * port 2 is short range receiver
>> + */
>> +cmdbuf[2] = enable + 1;
>
> You could do enable ? 2 : 1 here and do away with the check above. Enable
> always is 0 or 1 anyway.
>
>> +dev_dbg(ir->dev, "select %s-range receive sensor",
>> +enable ? "short" : "long");
>> +mce_async_out(ir, cmdbuf, sizeof(cmdbuf));
>> +
>> +return 0;
>> +}
>> +
>> +/*
>> + * Enable/disable receiver carrier frequency pass through reporting.
>> + * Frequency measurement only works with the short-range receiver.
>> + * The long-range receiver always reports no carrier frequency
>> + * (MCE_RSP_EQIRRXCFCNT, 0, 0) so we always ignore its report.
>> + */
>> +static int mceusb_set_rx_carrier_report(struct rc_dev *dev, int enable)
>> +{
>> +struct mceusb_dev *ir = dev->priv;
>> +
>> +if (enable != 0 && enable != 1)
>> +return -EINVAL;
>
> This is only called from lirc_dev.c, where the expression is:
>
> ret = dev->s_carrier_report(dev, !!val);
>
> There is no need to check for enable being not 0 or 1.
>
>> +
>> +dev_dbg(ir->dev, "%s short-range receiver carrier reporting",
>> +enable ? "enable" : "disable");
>> +ir->carrier_report_enabled = (enable == 1);
>
> Since enable is 0 or 1, there is no need for the enable == 1 expression.
>
> Note that the other drivers that support carrier reports (winbond-cir,
> redrat3, ene-cir) all enable the wideband receiver when carrier reports
> are turned on. You won't get carrier reports with the narrowband
> receiver, so if the user does:
>
> ir-ctl -r -m
>
> Then they will get nothing. It should really be consistent with the other
> drivers and enable wideband implicitly.
>
I've added to patch v2 to implicitly enable the wide-band receiver when
enabling carrier reporting. Plus additional revisions from your other comments.
[PATCH] [media] mceusb: add IR learning support features (IR carrier frequency measurement and wide-band/short range receiver)
Windows Media Center IR transceivers include two IR receivers;
wide-band/short-range and narrow-band/long-range. The short-range
(5cm distance) receiver is for IR learning and has IR carrier
frequency measuring ability.
Add mceusb driver support to select the short range IR receiver
and enable pass through of its IR carrier frequency measurements.
RC and LIRC already support these mceusb driver additions.
Test platform:
Linux raspberrypi 4.9.59-v7+ #1047 SMP Sun Oct 29 12:19:23 GMT 2017 armv7l
GNU/Linux
mceusb 1-1.2:1.0: Registered Pinnacle Systems PCTV Remote USB with mce emulator
interface version 1
mceusb 1-1.2:1.0: 2 tx ports (0x0 cabled) and 2 rx sensors (0x1 active)
Sony TV remote control
ir-ctl from v4l-utils
pi@raspberrypi:~ $ ir-ctl -V
IR raw version 1.12.3
pi@raspberrypi:~ $ ir-ctl -w -m -d /dev/lirc0 -r
...
pulse 600
space 600
pulse 1250
space 550
pulse 650
space 600
pulse 550
space 600
pulse 600
space 600
pulse 650
carrier 38803
space 16777215
^C
pi@raspberrypi:~ $ exit
Signed-off-by: A Sun
---
drivers/media/rc/mceusb.c | 90 ++-
1 file changed, 82 insertions(+), 8 deletions(-)
diff --git a/drivers/media/rc/mceusb.c b/drivers/media/rc/mceusb.c
index a9187b0b4..8bbb0f2da 100644
--- a/drivers/media/rc/mceusb.c
+++ b/drivers/media/rc/mceusb.c
@@ -42,7 +42,7 @@
#include
#include
-#define DRIVER_VERSION "1.93"
+#define DRIVER_VERSION "1.94"
#define DRIVER_AUTHOR "Jarod Wilson "
#define DRIVER_DESC"Windows Media Center Ed. eHome Infrared Transceiver " \
"device driver"
@@ -427,6 +427,7 @@ struct mceusb_dev {
struct rc_dev *rc;
/* optional features we can enable */
+ bool carrier_report_enabled;
bool learning_enabled;
/* core device bits */
@@ -475,6 +476,9 @@ struct mceusb_dev {
u8 txports_cabled; /* bitmask of transmitters with cable */
u8 rxports_active; /* bitmask of active receive sensors */
+ /* receiver carrier frequency detection support */
+ u32 pulse_tunit;/* IR pulse "on" cumulative time units */
+
/*
* support for async error handler mceusb_deferred_kevent()
* where usb_clear_halt(), usb_reset_configuration(),
@@ -956,12 +960,60 @@ static int mceusb_set_tx_carrier(struct rc_dev *dev, u32
carrier)
}
/*
+ * Select or deselect the 2nd receiver port.
+ * Second receiver is learning mode, wide-band, short-range receiver.
+ * Only one receiver (long or short range) may be active at a time.
+ */
+static int mceusb_set_rx_wideband(struct rc_dev *dev, int enable)
+{
+ struct mceusb_dev *ir = dev->priv;
+ unsigned char cmdbuf[3] = { MCE_CMD_PORT_IR,
+ MCE_CMD_SETIRRXPORTEN, 0x00 };
+
+ if (enable != 0 && enable != 1)
+ return -EINVAL;
+
+ /*
+* cmdbuf[2] is receiver port number
+* port 1 is long range receiver
+* port 2 is short range receiver
+*/
+ cmdbuf[2] = enable + 1;
+ dev_dbg(ir->dev, "select %s-range receive sensor",
+ enable ? "short" : "long");
+ mce_async_out(ir, cmdbuf, sizeof(cmdbuf));
+
+ return 0;
+}
+
+/*
+ * Enable/disable receiver carrier frequency pass through reporting.
+ * Frequency measurement only works with the short-range receiver.
+ * The long-range receiver always reports no carrier frequency
+ * (MCE_RSP_EQIRRXCFCNT, 0, 0) so we always ignore its report.
+ */
+static int mceusb_set_rx_carrier_report(struct rc_dev *dev, int enable)
+{
+ struct mceusb_dev *ir = dev->priv;
+
+ if (enable != 0 && enable != 1)
+ return -EINVAL;
+
+ dev_dbg(ir->dev, "%s short-range receiver carrier reporting",
+ enable ? "enable" : "disable");
+ ir->carrier_report_enabled = (enable == 1);
+
+ return 0;
+}
+
+/*
* We don't do anything but print debug spew for many of the command bits
* we receive from the hardware, but some of them are useful information
* we want to store so that we can use them.
*/
static void mceusb_handle_command(struct mceusb_dev *ir, int index)
{
+ DEFINE_IR_RAW_EVENT(rawir);
u8 hi = ir->buf_in[index + 1] & 0xff;
u8 lo = ir->buf_in[index + 2] & 0xff;
@@ -980,6 +1032,18 @@ static void mceusb_handle_command(struct mceusb_dev *ir,
int index)
ir->num_txports = hi;
ir->num_rxports = lo;
break;
+ case MCE_RSP_EQIRRXCFCNT:
+ if (ir->carrier_report_enabled && ir->learning_enabled
+ && ir->pulse_tunit > 0) {
+ init_ir_raw_event(&rawir);
+ rawir.carrier_report = 1;
+ r
[PATCH 1/1] [media] mceusb: coding style & comments update for -EPIPE error patches
Cosmetic updates to recent code revisions for better compliance with
https://www.kernel.org/doc/html/latest/process/coding-style.html
This patch depends on an earlier (marked accepted) patch:
[PATCH v2] [media] mceusb: TX -EPIPE (urb status = -32) lockup fix
Signed-off-by: A Sun
---
drivers/media/rc/mceusb.c | 39 ---
1 file changed, 20 insertions(+), 19 deletions(-)
diff --git a/drivers/media/rc/mceusb.c b/drivers/media/rc/mceusb.c
index af46860..66d0be5 100644
--- a/drivers/media/rc/mceusb.c
+++ b/drivers/media/rc/mceusb.c
@@ -457,8 +457,11 @@ struct mceusb_dev {
u8 txports_cabled; /* bitmask of transmitters with cable */
u8 rxports_active; /* bitmask of active receive sensors */
- /* async error handler mceusb_deferred_kevent() support
-* via workqueue kworker (previously keventd) threads */
+ /*
+* support for async error handler mceusb_deferred_kevent()
+* where usb_clear_halt(), usb_reset_configuration(),
+* usb_reset_device(), etc. must be done in process context
+*/
struct work_struct kevent;
unsigned long kevent_flags;
# define EVENT_TX_HALT0
@@ -705,11 +708,10 @@ static void mceusb_dev_printdata(struct mceusb_dev *ir,
char *buf,
static void mceusb_defer_kevent(struct mceusb_dev *ir, int kevent)
{
set_bit(kevent, &ir->kevent_flags);
- if (!schedule_work(&ir->kevent)) {
+ if (!schedule_work(&ir->kevent))
dev_err(ir->dev, "kevent %d may have been dropped", kevent);
- } else {
+ else
dev_dbg(ir->dev, "kevent %d scheduled", kevent);
- }
}
static void mce_async_callback(struct urb *urb)
@@ -775,14 +777,13 @@ static void mce_request_packet(struct mceusb_dev *ir,
unsigned char *data,
}
/* outbound data */
- if (usb_endpoint_xfer_int(ir->usb_ep_out)) {
+ if (usb_endpoint_xfer_int(ir->usb_ep_out))
usb_fill_int_urb(async_urb, ir->usbdev, ir->pipe_out,
async_buf, size, mce_async_callback, ir,
ir->usb_ep_out->bInterval);
- } else {
+ else
usb_fill_bulk_urb(async_urb, ir->usbdev, ir->pipe_out,
async_buf, size, mce_async_callback, ir);
- }
memcpy(async_buf, data, size);
dev_dbg(dev, "send request called (size=%#x)", size);
@@ -1225,13 +1226,12 @@ static void mceusb_deferred_kevent(struct work_struct
*work)
}
clear_bit(EVENT_RX_HALT, &ir->kevent_flags);
status = usb_submit_urb(ir->urb_in, GFP_KERNEL);
- if (status < 0) {
+ if (status < 0)
dev_err(ir->dev, "rx unhalt submit urb error %d",
status);
- goto done_rx_halt;
- }
}
done_rx_halt:
+
if (test_bit(EVENT_TX_HALT, &ir->kevent_flags)) {
status = usb_clear_halt(ir->usbdev, ir->pipe_out);
if (status < 0) {
@@ -1242,6 +1242,7 @@ static void mceusb_deferred_kevent(struct work_struct
*work)
clear_bit(EVENT_TX_HALT, &ir->kevent_flags);
}
done_tx_halt:
+
return;
}
@@ -1397,13 +1398,12 @@ static int mceusb_dev_probe(struct usb_interface *intf,
/* Saving usb interface data for use by the transmitter routine */
ir->usb_ep_out = ep_out;
- if (usb_endpoint_xfer_int(ir->usb_ep_out)) {
+ if (usb_endpoint_xfer_int(ir->usb_ep_out))
ir->pipe_out = usb_sndintpipe(ir->usbdev,
ir->usb_ep_out->bEndpointAddress);
- } else {
+ else
ir->pipe_out = usb_sndbulkpipe(ir->usbdev,
ir->usb_ep_out->bEndpointAddress);
- }
if (dev->descriptor.iManufacturer
&& usb_string(dev, dev->descriptor.iManufacturer,
@@ -1415,8 +1415,10 @@ static int mceusb_dev_probe(struct usb_interface *intf,
snprintf(name + strlen(name), sizeof(name) - strlen(name),
" %s", buf);
- /* initialize async USB error handler before registering
-* or activating any mceusb RX and TX functions */
+ /*
+* async USB error handler must initialize before registering
+* or activating any mceusb RX or TX functions
+*/
INIT_WORK(&ir->kevent, mceusb_deferred_kevent);
ir->rc = mceusb_init_rc_dev(ir);
@@ -1424,13 +1426,12 @@ static int mceusb_dev_probe(struct usb_interface *intf,
goto rc_dev_fail;
/* wire up inbound data handler */
- if (usb_endpoint_xfer
[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.
On 4/27/2017 4:54 PM, Sean Young wrote:
> On Thu, Apr 13, 2017 at 04:06:47AM -0400, A Sun wrote:
>> @@ -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.
>
>> }
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
[PATCH v2] [media] mceusb: TX -EPIPE (urb status = -32) lockup fix
point, regardless of its halt/stall state, TX functionality silently ceases.
mce_async_callback() invocations cease, and there are no other error
indications from usb_submit_urb() or anywhere else.
An escalating USB reset was necessary to restore this device to working state.
Certain mceusb devices may require device specific recovery procedure for TX
halt conditions, which this patch does not address.
Signed-off-by: A Sun
---
drivers/media/rc/mceusb.c | 89 +--
1 file changed, 56 insertions(+), 33 deletions(-)
diff --git a/drivers/media/rc/mceusb.c b/drivers/media/rc/mceusb.c
index a9a9a85..af46860 100644
--- a/drivers/media/rc/mceusb.c
+++ b/drivers/media/rc/mceusb.c
@@ -419,6 +419,7 @@ struct mceusb_dev {
struct urb *urb_in;
unsigned int pipe_in;
struct usb_endpoint_descriptor *usb_ep_out;
+ unsigned int pipe_out;
/* buffers and dma */
unsigned char *buf_in;
@@ -456,7 +457,8 @@ struct mceusb_dev {
u8 txports_cabled; /* bitmask of transmitters with cable */
u8 rxports_active; /* bitmask of active receive sensors */
- /* kevent support */
+ /* async error handler mceusb_deferred_kevent() support
+* via workqueue kworker (previously keventd) threads */
struct work_struct kevent;
unsigned long kevent_flags;
# define EVENT_TX_HALT0
@@ -694,6 +696,22 @@ static void mceusb_dev_printdata(struct mceusb_dev *ir,
char *buf,
#endif
}
+/*
+ * Schedule work that can't be done in interrupt handlers
+ * (mceusb_dev_recv() and mce_async_callback()) nor tasklets.
+ * Invokes mceusb_deferred_kevent() for recovering from
+ * error events specified by the kevent bit field.
+ */
+static void mceusb_defer_kevent(struct mceusb_dev *ir, int kevent)
+{
+ set_bit(kevent, &ir->kevent_flags);
+ if (!schedule_work(&ir->kevent)) {
+ dev_err(ir->dev, "kevent %d may have been dropped", kevent);
+ } else {
+ dev_dbg(ir->dev, "kevent %d scheduled", kevent);
+ }
+}
+
static void mce_async_callback(struct urb *urb)
{
struct mceusb_dev *ir;
@@ -720,6 +738,11 @@ static void mce_async_callback(struct urb *urb)
break;
case -EPIPE:
+ dev_err(ir->dev, "Error: request urb status = %d (TX HALT)",
+ urb->status);
+ mceusb_defer_kevent(ir, EVENT_TX_HALT);
+ break;
+
default:
dev_err(ir->dev, "Error: request urb status = %d", urb->status);
break;
@@ -734,7 +757,7 @@ static void mce_async_callback(struct urb *urb)
static void mce_request_packet(struct mceusb_dev *ir, unsigned char *data,
int size)
{
- int res, pipe;
+ int res;
struct urb *async_urb;
struct device *dev = ir->dev;
unsigned char *async_buf;
@@ -753,17 +776,12 @@ static void mce_request_packet(struct mceusb_dev *ir,
unsigned char *data,
/* outbound data */
if (usb_endpoint_xfer_int(ir->usb_ep_out)) {
- pipe = usb_sndintpipe(ir->usbdev,
-ir->usb_ep_out->bEndpointAddress);
- usb_fill_int_urb(async_urb, ir->usbdev, pipe, async_buf,
-size, mce_async_callback, ir,
+ usb_fill_int_urb(async_urb, ir->usbdev, ir->pipe_out,
+async_buf, size, mce_async_callback, ir,
ir->usb_ep_out->bInterval);
} else {
- pipe = usb_sndbulkpipe(ir->usbdev,
-ir->usb_ep_out->bEndpointAddress);
- usb_fill_bulk_urb(async_urb, ir->usbdev, pipe,
-async_buf, size, mce_async_callback,
-ir);
+ usb_fill_bulk_urb(async_urb, ir->usbdev, ir->pipe_out,
+async_buf, size, mce_async_callback, ir);
}
memcpy(async_buf, data, size);
@@ -1034,23 +1052,6 @@ static void mceusb_process_ir_data(struct mceusb_dev
*ir, int buf_len)
}
}
-/*
- * Workqueue task dispatcher
- * for work that can't be done in interrupt handlers
- * (mceusb_dev_recv() and mce_async_callback()) nor tasklets.
- * Invokes mceusb_deferred_kevent() for recovering from
- * error events specified by the kevent bit field.
- */
-static void mceusb_defer_kevent(struct mceusb_dev *ir, int kevent)
-{
- set_bit(kevent, &ir->kevent_flags);
- if (!schedule_work(&ir->kevent)) {
- dev_err(ir->dev, "kevent %d may have been dropped", kevent);
- } else {
- dev_dbg(ir->dev, "kevent %d scheduled", keven
[PATCH] [media] mceusb: TX -EPIPE (urb status = -32) lockup fix
are no other error
indications from usb_submit_urb() or anywhere else.
An escalating USB reset was necessary to restore this device to working state.
Certain mceusb devices may require device specific recovery procedure for TX
halt conditions, which this patch does not address.
Signed-off-by: A Sun
---
drivers/media/rc/mceusb.c | 89 +--
1 file changed, 56 insertions(+), 33 deletions(-)
diff --git a/drivers/media/rc/mceusb.c b/drivers/media/rc/mceusb.c
index a9a9a85..d1d7246 100644
--- a/drivers/media/rc/mceusb.c
+++ b/drivers/media/rc/mceusb.c
@@ -419,6 +419,7 @@ struct mceusb_dev {
struct urb *urb_in;
unsigned int pipe_in;
struct usb_endpoint_descriptor *usb_ep_out;
+ unsigned int pipe_out;
/* buffers and dma */
unsigned char *buf_in;
@@ -456,7 +457,8 @@ struct mceusb_dev {
u8 txports_cabled; /* bitmask of transmitters with cable */
u8 rxports_active; /* bitmask of active receive sensors */
- /* kevent support */
+ /* async error handler mceusb_deferred_kevent() support
+* via workqueue kworker (previously keventd) */
struct work_struct kevent;
unsigned long kevent_flags;
# define EVENT_TX_HALT0
@@ -694,6 +696,22 @@ static void mceusb_dev_printdata(struct mceusb_dev *ir,
char *buf,
#endif
}
+/*
+ * Schedule work that can't be done in interrupt handlers
+ * (mceusb_dev_recv() and mce_async_callback()) nor tasklets.
+ * Invokes mceusb_deferred_kevent() for recovering from
+ * error events specified by the kevent bit field.
+ */
+static void mceusb_defer_kevent(struct mceusb_dev *ir, int kevent)
+{
+ set_bit(kevent, &ir->kevent_flags);
+ if (!schedule_work(&ir->kevent)) {
+ dev_err(ir->dev, "kevent %d may have been dropped", kevent);
+ } else {
+ dev_dbg(ir->dev, "kevent %d scheduled", kevent);
+ }
+}
+
static void mce_async_callback(struct urb *urb)
{
struct mceusb_dev *ir;
@@ -720,6 +738,11 @@ static void mce_async_callback(struct urb *urb)
break;
case -EPIPE:
+ dev_err(ir->dev, "Error: request urb status = %d (TX HALT)",
+ urb->status);
+ mceusb_defer_kevent(ir, EVENT_TX_HALT);
+ break;
+
default:
dev_err(ir->dev, "Error: request urb status = %d", urb->status);
break;
@@ -734,7 +757,7 @@ static void mce_async_callback(struct urb *urb)
static void mce_request_packet(struct mceusb_dev *ir, unsigned char *data,
int size)
{
- int res, pipe;
+ int res;
struct urb *async_urb;
struct device *dev = ir->dev;
unsigned char *async_buf;
@@ -753,17 +776,12 @@ static void mce_request_packet(struct mceusb_dev *ir,
unsigned char *data,
/* outbound data */
if (usb_endpoint_xfer_int(ir->usb_ep_out)) {
- pipe = usb_sndintpipe(ir->usbdev,
-ir->usb_ep_out->bEndpointAddress);
- usb_fill_int_urb(async_urb, ir->usbdev, pipe, async_buf,
-size, mce_async_callback, ir,
+ usb_fill_int_urb(async_urb, ir->usbdev, ir->pipe_out,
+async_buf, size, mce_async_callback, ir,
ir->usb_ep_out->bInterval);
} else {
- pipe = usb_sndbulkpipe(ir->usbdev,
-ir->usb_ep_out->bEndpointAddress);
- usb_fill_bulk_urb(async_urb, ir->usbdev, pipe,
-async_buf, size, mce_async_callback,
-ir);
+ usb_fill_bulk_urb(async_urb, ir->usbdev, ir->pipe_out,
+async_buf, size, mce_async_callback, ir);
}
memcpy(async_buf, data, size);
@@ -1034,23 +1052,6 @@ static void mceusb_process_ir_data(struct mceusb_dev
*ir, int buf_len)
}
}
-/*
- * Workqueue task dispatcher
- * for work that can't be done in interrupt handlers
- * (mceusb_dev_recv() and mce_async_callback()) nor tasklets.
- * Invokes mceusb_deferred_kevent() for recovering from
- * error events specified by the kevent bit field.
- */
-static void mceusb_defer_kevent(struct mceusb_dev *ir, int kevent)
-{
- set_bit(kevent, &ir->kevent_flags);
- if (!schedule_work(&ir->kevent)) {
- dev_err(ir->dev, "kevent %d may have been dropped", kevent);
- } else {
- dev_dbg(ir->dev, "kevent %d scheduled", kevent);
- }
-}
-
static void mceusb_dev_recv(struct urb *urb)
{
struct mceusb_dev *ir;
@@ -1220,16 +1221,28 @@
Re: [PATCH 1/3] [media] mceusb: RX -EPIPE (urb status = -32) lockup failure fix
On 3/30/2017 3:12 AM, Sean Young wrote:
> On Wed, Mar 29, 2017 at 06:04:58PM -0400, A Sun wrote:
>> On 3/29/2017 5:06 PM, Sean Young wrote:
>>
>>>
>>> Anyway, you're right and this patch looks ok. It would be nice to have the
>>> tx case handled too though.
>>>
>>> Thanks
>>> Sean
>>>
>>
>> Thanks; I'm looking at handling the tx case. If I can figure out the
>> details, I'll post a new patch proposal separate, and likely dependent, on
>> this one.
>>
>> My main obstacle at the moment, is I'm looking for a way to get mceusb
>> device to respond with a USB TX error halt/stall (rather than the typical
>> ACK and NAK) on a TX endpoint, in order to test halt/stall error detection
>> and recovery for TX. ..A Sun
>
> If you send IR, the drivers send a usb packet. However, the kernel will
> sleep for however long the IR is in ir_lirc_transmit_ir, so your other option
> is to set the transmit carrier repeatedly instead. You'd have to set the
> carrier to a different value every time.
>
>
> {
> int fd, carrier;
>
> fd = open("/dev/lirc0", O_RDWR);
> carrier = 38000;
> for (;;) {
> ioctl(fd, LIRC_SET_SEND_CARRIER, &carrier);
> if (++carrier >= 4)
> carrier = 38000;
> }
> }
>
>
> Sean
>
Thanks, this is good to know, for testing where multiple TX I/Os are pending
prior to fault.
I found a way to set up the TX -EPIPE fault administratively:
retval = usb_control_msg(ir->usbdev, usb_sndctrlpipe(ir->usbdev, 0),
USB_REQ_SET_FEATURE, USB_RECIP_ENDPOINT,
USB_ENDPOINT_HALT, usb_pipeendpoint(ir->pipe_out),
NULL, 0, USB_CTRL_SET_TIMEOUT);
dev_dbg(ir->dev, "set halt retval, %d", retval);
and have replications now for TX error and lock -out. Error occurs for every
TX. No message flooding otherwise, as we expect. The RX side remains working.
...
[ 249.986174] mceusb 1-1.2:1.0: requesting 38000 HZ carrier
[ 249.986210] mceusb 1-1.2:1.0: send request called (size=0x4)
[ 249.986256] mceusb 1-1.2:1.0: send request complete (res=0)
[ 249.986403] mceusb 1-1.2:1.0: Error: request urb status = -32 (TX HALT)
[ 249.999885] mceusb 1-1.2:1.0: send request called (size=0x3)
[ 249.29] mceusb 1-1.2:1.0: send request complete (res=0)
[ 250.13] mceusb 1-1.2:1.0: Error: request urb status = -32 (TX HALT)
[ 250.019830] mceusb 1-1.2:1.0: send request called (size=0x21)
[ 250.019868] mceusb 1-1.2:1.0: send request complete (res=0)
[ 250.020007] mceusb 1-1.2:1.0: Error: request urb status = -32 (TX HALT)
...
Re: [PATCH 1/3] [media] mceusb: RX -EPIPE (urb status = -32) lockup failure fix
On 3/29/2017 5:06 PM, Sean Young wrote: > > Anyway, you're right and this patch looks ok. It would be nice to have the > tx case handled too though. > > Thanks > Sean > Thanks; I'm looking at handling the tx case. If I can figure out the details, I'll post a new patch proposal separate, and likely dependent, on this one. My main obstacle at the moment, is I'm looking for a way to get mceusb device to respond with a USB TX error halt/stall (rather than the typical ACK and NAK) on a TX endpoint, in order to test halt/stall error detection and recovery for TX. ..A Sun
Re: [PATCH 1/3] [media] mceusb: RX -EPIPE (urb status = -32) lockup failure fix
space with duration 635 Mar 22 12:16:35 raspberrypi kernel: [ 4863.489207] mceusb 1-1.2:1.0: Storing space with duration 635 Mar 22 12:16:35 raspberrypi kernel: [ 4863.489211] mceusb 1-1.2:1.0: Storing space with duration 635 Mar 22 12:16:35 raspberrypi kernel: [ 4863.489216] mceusb 1-1.2:1.0: Storing space with duration 635 Mar 22 12:16:35 raspberrypi kernel: [ 4863.489220] mceusb 1-1.2:1.0: Storing space with duration 635 Mar 22 12:16:35 raspberrypi kernel: [ 4863.489224] mceusb 1-1.2:1.0: Storing space with duration 635 Mar 22 12:16:35 raspberrypi kernel: [ 4863.489228] mceusb 1-1.2:1.0: Storing space with duration 635 Mar 22 12:16:35 raspberrypi kernel: [ 4863.489232] mceusb 1-1.2:1.0: Storing space with duration 635 Mar 22 12:16:35 raspberrypi kernel: [ 4863.489236] mceusb 1-1.2:1.0: Storing space with duration 635 Mar 22 12:16:35 raspberrypi kernel: [ 4863.489240] mceusb 1-1.2:1.0: Storing space with duration 635 Mar 22 12:16:35 raspberrypi kernel: [ 4863.489246] mceusb 1-1.2:1.0: Storing space with duration 635 Mar 22 12:16:35 raspberrypi kernel: [ 4863.489250] mceusb 1-1.2:1.0: Storing space with duration 635 Mar 22 12:16:35 raspberrypi kernel: [ 4863.489254] mceusb 1-1.2:1.0: Storing space with duration 635 Mar 22 12:16:35 raspberrypi kernel: [ 4863.489257] mceusb 1-1.2:1.0: processed IR data Mar 22 12:16:35 raspberrypi kernel: [ 4863.589429] mceusb 1-1.2:1.0: Error: urb status = -32 (RX HALT) Mar 22 12:16:35 raspberrypi kernel: [ 4863.589452] mceusb 1-1.2:1.0: kevent 1 scheduled Mar 22 12:16:35 raspberrypi kernel: [ 4863.590203] mceusb 1-1.2:1.0: unhalt usb_submit_urb, status 0 Mar 22 12:16:36 raspberrypi kernel: [ 4864.614325] mceusb 1-1.2:1.0: rx data: 90 0b 98 0c 8c 0c 8c 0c 98 0c 98 0c 98 0c 8c 0c 8c (length=17) Mar 22 12:16:36 raspberrypi kernel: [ 4864.614338] mceusb 1-1.2:1.0: Raw IR data, 16 pulse/space samples Mar 22 12:16:36 raspberrypi kernel: [ 4864.614346] mceusb 1-1.2:1.0: Storing space with duration 55 Mar 22 12:16:36 raspberrypi kernel: [ 4864.614355] mceusb 1-1.2:1.0: Storing pulse with duration 120 Mar 22 12:16:36 raspberrypi kernel: [ 4864.614363] mceusb 1-1.2:1.0: Storing space with duration 60 Mar 22 12:16:36 raspberrypi kernel: [ 4864.614370] mceusb 1-1.2:1.0: Storing pulse with duration 60 Mar 22 12:16:36 raspberrypi kernel: [ 4864.614377] mceusb 1-1.2:1.0: Storing space with duration 60 Mar 22 12:16:36 raspberrypi kernel: [ 4864.614384] mceusb 1-1.2:1.0: Storing pulse with duration 60 Mar 22 12:16:36 raspberrypi kernel: [ 4864.614391] mceusb 1-1.2:1.0: Storing space with duration 60 Mar 22 12:16:36 raspberrypi kernel: [ 4864.614399] mceusb 1-1.2:1.0: Storing pulse with duration 120 Mar 22 12:16:36 raspberrypi kernel: [ 4864.614405] mceusb 1-1.2:1.0: Storing space with duration 60 Mar 22 12:16:36 raspberrypi kernel: [ 4864.614414] mceusb 1-1.2:1.0: Storing pulse with duration 120 Mar 22 12:16:36 raspberrypi kernel: [ 4864.614421] mceusb 1-1.2:1.0: Storing space with duration 60 Mar 22 12:16:36 raspberrypi kernel: [ 4864.614428] mceusb 1-1.2:1.0: Storing pulse with duration 120 Mar 22 12:16:36 raspberrypi kernel: [ 4864.614435] mceusb 1-1.2:1.0: Storing space with duration 60 Mar 22 12:16:36 raspberrypi kernel: [ 4864.614443] mceusb 1-1.2:1.0: Storing pulse with duration 60 Mar 22 12:16:36 raspberrypi kernel: [ 4864.614450] mceusb 1-1.2:1.0: Storing space with duration 60 Mar 22 12:16:36 raspberrypi kernel: [ 4864.614458] mceusb 1-1.2:1.0: Storing pulse with duration 60 Mar 22 12:16:36 raspberrypi kernel: [ 4864.614463] mceusb 1-1.2:1.0: processed IR data Mar 22 12:16:36 raspberrypi kernel: [ 4864.648331] mceusb 1-1.2:1.0: rx data: 90 0b 8d 0b 8d 7f 7f 7f 72 b0 0c 98 0c 8c 0c 98 0c (length=17) ... Thanks, ..A Sun
Re: [PATCH 1/3] [media] mceusb: RX -EPIPE (urb status = -32) lockup failure fix
On 3/26/2017 4:31 PM, Sean Young wrote:
> On Sun, Mar 26, 2017 at 02:28:08PM -0400, A Sun wrote:
>> commit
>> https://github.com/asunxx/linux/commit/17fe3b51f4ad5202a876ea4c92b5d99d4e166823
>> Author: A Sun
>> Date: Sun, 26 Mar 2017 13:24:18 -0400
>
> Please don't include this.
>
>>
...
>> mceusb 1-1.2:1.0: 2 tx ports (0x1 cabled) and 2 rx sensors (0x1 active)
>
> It would be nice to have this tested against a mainline kernel. I thought
> that was entirely possible on raspberry pis nowadays.
...
>> +/* kevent support */
>> +struct work_struct kevent;
>
> kevent is not a descriptive name. How about something like clear_halt?
>
>> +unsigned long kevent_flags;
>> +# define EVENT_TX_HALT0
>> +# define EVENT_RX_HALT1
>
> EVENT_TX_HALT is never used, so kevent_flags is only ever set to 1. The
> entire field can be dropped.
>
...
>> +if (!schedule_work(&ir->kevent)) {
>> +dev_err(ir->dev, "kevent %d may have been dropped", kevent);
>> +} else {
>> +dev_dbg(ir->dev, "kevent %d scheduled", kevent);
>> +}
>> +}
>
> Again name is not very descriptive.
>
...
>> +dev_err(ir->dev, "Error: urb status = %d (RX HALT)",
>> +urb->status);
>> +mceusb_defer_kevent(ir, EVENT_RX_HALT);
>
> Here you could simply call schedule_work(). Note that EPIPE might also
> be returned for device disconnect for some host controllers.
>
>> +return;
...
>> +int status;
>> +
>> +if (test_bit(EVENT_RX_HALT, &ir->kevent_flags)) {
>
> If condition can go.
>
>> +usb_unlink_urb(ir->urb_in);
>> +status = usb_clear_halt(ir->usbdev, ir->pipe_in);
Hi Sean,
Thanks again for looking at this. This patch is based on similar error and
recovery, with the USB ethernet driver usbnet (usbnet.c, usbnet.h).
In usbnet, they call "kevent" (kernel device event?) any kind of hardware state
change or event in interrupt context that requires invoking non-interrupt code
to handle. I'm not sure what else I should name it. Possible kevent-s are not
limited to situations needing usb_clear_halt(). From usbnet:
69 # define EVENT_TX_HALT0
70 # define EVENT_RX_HALT1
71 # define EVENT_RX_MEMORY 2
72 # define EVENT_STS_SPLIT 3
73 # define EVENT_LINK_RESET 4
74 # define EVENT_RX_PAUSED 5
75 # define EVENT_DEV_ASLEEP 6
76 # define EVENT_DEV_OPEN 7
77 # define EVENT_DEVICE_REPORT_IDLE 8
78 # define EVENT_NO_RUNTIME_PM 9
79 # define EVENT_RX_KILL10
80 # define EVENT_LINK_CHANGE11
81 # define EVENT_SET_RX_MODE12
So far, the first two are appearing applicable for mceusb.
The unused EVENT_TX_HALT and the apparently extra _kevent functions and
kevent_flags are necessary for a later:
[PATCH] [media] mceusb: TX -EPIPE lockup fix
...not yet written, transmit side equivalent bug. I respectfully recommend
keeping these hooks in place.
For now, I think the transmit side EPIPE bug fix is less critical, since the TX
bug avoids hanging the host/kernel, but would still cause lockup of the device.
In case of RX EPIPE on disconnect, the fix is still safe. Recovery attempt
should fail (in usb_clear_halt() or usb_submit_urb()) and abort without further
retry, and the recovery handler itself gets shutdown in mceusb_dev_disconnect().
Please let me know how to proceed. Thanks. ..A Sun
[PATCH 3/3] [media] mceusb: fix inaccurate debug buffer dumps,and misleading debug messages
commit
https://github.com/asunxx/linux/commit/14cae79824739ae03caa3b1c4f66cbdf73654bde
Author: A Sun
Date: Sun, 26 Mar 2017 14:55:31 -0400
Bug:
Some dev_dbg messages are misleading.
Some dev_dbg messages have inconsistent formatting.
mceusb_dev_printdata() prints incorrect range of bytes (0 to len) in buffer
which the driver will actually process next.
Fix:
Add size of received data argument to mceusb_dev_printdata().
Revise buffer print range to (offset to offset+len).
Remove static USB_BUFLEN = 32, which is variable depending on device
(buffer size is 64 for Pinnacle IR transceiver).
Revise bound test to prevent reporting data beyond end of read or end of buffer.
Drop "\n" use from dev_dbg().
References to "receive request" should read "send request".
Tested with:
Linux raspberrypi 4.4.50-v7+ #970 SMP Mon Feb 20 19:18:29 GMT 2017 armv7l
GNU/Linux
mceusb 1-1.2:1.0: Registered Pinnacle Systems PCTV Remote USB with mce emulator
interface version 1
mceusb 1-1.2:1.0: 2 tx ports (0x1 cabled) and 2 rx sensors (0x1 active)
Signed-off-by: A Sun
---
drivers/media/rc/mceusb.c | 29 +++--
1 file changed, 15 insertions(+), 14 deletions(-)
diff --git a/drivers/media/rc/mceusb.c b/drivers/media/rc/mceusb.c
index 720df6f..a9a9a85 100644
--- a/drivers/media/rc/mceusb.c
+++ b/drivers/media/rc/mceusb.c
@@ -48,7 +48,6 @@
"device driver"
#define DRIVER_NAME"mceusb"
-#define USB_BUFLEN 32 /* USB reception buffer length */
#define USB_CTRL_MSG_SZ2 /* Size of usb ctrl msg on gen1 hw */
#define MCE_G1_INIT_MSGS 40 /* Init messages on gen1 hw to throw out */
@@ -535,7 +534,7 @@ static int mceusb_cmd_datasize(u8 cmd, u8 subcmd)
}
static void mceusb_dev_printdata(struct mceusb_dev *ir, char *buf,
-int offset, int len, bool out)
+int buf_len, int offset, int len, bool out)
{
#if defined(DEBUG) || defined(CONFIG_DYNAMIC_DEBUG)
char *inout;
@@ -552,7 +551,8 @@ static void mceusb_dev_printdata(struct mceusb_dev *ir,
char *buf,
return;
dev_dbg(dev, "%cx data: %*ph (length=%d)",
- (out ? 't' : 'r'), min(len, USB_BUFLEN), buf, len);
+ (out ? 't' : 'r'),
+ min(len, buf_len - offset), buf + offset, len);
inout = out ? "Request" : "Got";
@@ -709,7 +709,8 @@ static void mce_async_callback(struct urb *urb)
case 0:
len = urb->actual_length;
- mceusb_dev_printdata(ir, urb->transfer_buffer, 0, len, true);
+ mceusb_dev_printdata(ir, urb->transfer_buffer, len,
+0, len, true);
break;
case -ECONNRESET:
@@ -729,7 +730,7 @@ static void mce_async_callback(struct urb *urb)
usb_free_urb(urb);
}
-/* request incoming or send outgoing usb packet - used to initialize remote */
+/* request outgoing (send) usb packet - used to initialize remote */
static void mce_request_packet(struct mceusb_dev *ir, unsigned char *data,
int size)
{
@@ -740,7 +741,7 @@ static void mce_request_packet(struct mceusb_dev *ir,
unsigned char *data,
async_urb = usb_alloc_urb(0, GFP_KERNEL);
if (unlikely(!async_urb)) {
- dev_err(dev, "Error, couldn't allocate urb!\n");
+ dev_err(dev, "Error, couldn't allocate urb!");
return;
}
@@ -766,17 +767,17 @@ static void mce_request_packet(struct mceusb_dev *ir,
unsigned char *data,
}
memcpy(async_buf, data, size);
- dev_dbg(dev, "receive request called (size=%#x)", size);
+ dev_dbg(dev, "send request called (size=%#x)", size);
async_urb->transfer_buffer_length = size;
async_urb->dev = ir->usbdev;
res = usb_submit_urb(async_urb, GFP_ATOMIC);
if (res) {
- dev_err(dev, "receive request FAILED! (res=%d)", res);
+ dev_err(dev, "send request FAILED! (res=%d)", res);
return;
}
- dev_dbg(dev, "receive request complete (res=%d)", res);
+ dev_dbg(dev, "send request complete (res=%d)", res);
}
static void mce_async_out(struct mceusb_dev *ir, unsigned char *data, int size)
@@ -982,7 +983,7 @@ static void mceusb_process_ir_data(struct mceusb_dev *ir,
int buf_len)
switch (ir->parser_state) {
case SUBCMD:
ir->rem = mceusb_cmd_datasize(ir->cmd, ir->buf_in[i]);
- mceusb_dev_printdata(ir, ir->buf_in, i - 1,
+ mceusb_dev_printdata(ir, ir->buf_in, buf_len, i - 1,
[PATCH 2/3] [media] mceusb: sporadic RX truncation corruption fix
commit
https://github.com/asunxx/linux/commit/d71c609e99cabc0a92a4d0b225486cbc09bbd8bd
Author: A Sun
Date: Sun, 26 Mar 2017 14:11:49 -0400
Bug:
Intermittent RX truncation and loss of IR received data.
This resulted in receive stream synchronization errors where driver attempted
to incorrectly parse IR data (eg 0x90 below) as command response.
Mar 22 12:01:40 raspberrypi kernel: [ 3969.139898] mceusb 1-1.2:1.0: processed
IR data
Mar 22 12:01:40 raspberrypi kernel: [ 3969.151315] mceusb 1-1.2:1.0: rx data:
00 90 (length=2)
Mar 22 12:01:40 raspberrypi kernel: [ 3969.151321] mceusb 1-1.2:1.0: Unknown
command 0x00 0x90
Mar 22 12:01:40 raspberrypi kernel: [ 3969.151336] mceusb 1-1.2:1.0: rx data:
98 0a 8d 0a 8e 0a 8e 0a 8e 0a 8e 0a 9a 0a 8e 0a 0b 3a 8e 00 80 41 59 00 00
(length=25)
Mar 22 12:01:40 raspberrypi kernel: [ 3969.151341] mceusb 1-1.2:1.0: Raw IR
data, 24 pulse/space samples
Mar 22 12:01:40 raspberrypi kernel: [ 3969.151348] mceusb 1-1.2:1.0: Storing
space with duration 50
Bug trigger appears to be normal, but heavy, IR receiver use.
Fix:
Cause may be receiver with ep_in bulk endpoint incorrectly bound to
usb_fill_int_urb() urb for interrupt endpoint.
This may also have been the root cause for "RX -EPIPE (urb status = -32)"
lockups.
In mceusb_dev_probe(), test ep_in endpoint for int versus bulk and use
usb_fill_bulk_urb() as appropriate.
Tested with:
Linux raspberrypi 4.4.50-v7+ #970 SMP Mon Feb 20 19:18:29 GMT 2017 armv7l
GNU/Linux
mceusb 1-1.2:1.0: Registered Pinnacle Systems PCTV Remote USB with mce emulator
interface version 1
mceusb 1-1.2:1.0: 2 tx ports (0x1 cabled) and 2 rx sensors (0x1 active)
Signed-off-by: A Sun
---
drivers/media/rc/mceusb.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/media/rc/mceusb.c b/drivers/media/rc/mceusb.c
index 7b6f9e5..720df6f 100644
--- a/drivers/media/rc/mceusb.c
+++ b/drivers/media/rc/mceusb.c
@@ -1401,8 +1401,13 @@ static int mceusb_dev_probe(struct usb_interface *intf,
INIT_WORK(&ir->kevent, mceusb_deferred_kevent);
/* wire up inbound data handler */
- usb_fill_int_urb(ir->urb_in, dev, pipe, ir->buf_in, maxp,
+ if (usb_endpoint_xfer_int(ep_in)) {
+ usb_fill_int_urb(ir->urb_in, dev, pipe, ir->buf_in, maxp,
mceusb_dev_recv, ir, ep_in->bInterval);
+ } else {
+ usb_fill_bulk_urb(ir->urb_in, dev, pipe, ir->buf_in, maxp,
+ mceusb_dev_recv, ir);
+ }
ir->urb_in->transfer_dma = ir->dma_in;
ir->urb_in->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
--
2.1.4
[PATCH 1/3] [media] mceusb: RX -EPIPE (urb status = -32) lockup failure fix
commit
https://github.com/asunxx/linux/commit/17fe3b51f4ad5202a876ea4c92b5d99d4e166823
Author: A Sun
Date: Sun, 26 Mar 2017 13:24:18 -0400
Bug:
RX -EPIPE failure with infinite loop and flooding of
Mar 20 22:24:56 raspberrypi kernel: [ 2851.966506] mceusb 1-1.2:1.0: Error: urb
status = -32
log message at 8000 messages per second.
Bug trigger appears to be normal, but heavy, IR receiver use.
Driver and Linux host become unusable after error.
Also seen at https://sourceforge.net/p/lirc/mailman/message/34886165/
Fix:
Message reports RX usb halt (stall) condition requiring usb_clear_halt() call
in non-interrupt context to recover.
Add driver workqueue call to perform this recovery based on method in use for
the usbnet device driver.
Tested with:
Linux raspberrypi 4.4.50-v7+ #970 SMP Mon Feb 20 19:18:29 GMT 2017 armv7l
GNU/Linux
mceusb 1-1.2:1.0: Registered Pinnacle Systems PCTV Remote USB with mce emulator
interface version 1
mceusb 1-1.2:1.0: 2 tx ports (0x1 cabled) and 2 rx sensors (0x1 active)
Signed-off-by: A Sun
---
drivers/media/rc/mceusb.c | 67 ++-
1 file changed, 66 insertions(+), 1 deletion(-)
diff --git a/drivers/media/rc/mceusb.c b/drivers/media/rc/mceusb.c
index 238d8ea..7b6f9e5 100644
--- a/drivers/media/rc/mceusb.c
+++ b/drivers/media/rc/mceusb.c
@@ -36,12 +36,13 @@
#include
#include
#include
+#include
#include
#include
#include
#include
-#define DRIVER_VERSION "1.92"
+#define DRIVER_VERSION "1.93"
#define DRIVER_AUTHOR "Jarod Wilson "
#define DRIVER_DESC"Windows Media Center Ed. eHome Infrared Transceiver " \
"device driver"
@@ -417,6 +418,7 @@ struct mceusb_dev {
/* usb */
struct usb_device *usbdev;
struct urb *urb_in;
+ unsigned int pipe_in;
struct usb_endpoint_descriptor *usb_ep_out;
/* buffers and dma */
@@ -454,6 +456,12 @@ struct mceusb_dev {
u8 num_rxports; /* number of receive sensors */
u8 txports_cabled; /* bitmask of transmitters with cable */
u8 rxports_active; /* bitmask of active receive sensors */
+
+ /* kevent support */
+ struct work_struct kevent;
+ unsigned long kevent_flags;
+# define EVENT_TX_HALT0
+# define EVENT_RX_HALT1
};
/* MCE Device Command Strings, generally a port and command pair */
@@ -1025,6 +1033,23 @@ static void mceusb_process_ir_data(struct mceusb_dev
*ir, int buf_len)
}
}
+/*
+ * Workqueue task dispatcher
+ * for work that can't be done in interrupt handlers
+ * (mceusb_dev_recv() and mce_async_callback()) nor tasklets.
+ * Invokes mceusb_deferred_kevent() for recovering from
+ * error events specified by the kevent bit field.
+ */
+static void mceusb_defer_kevent(struct mceusb_dev *ir, int kevent)
+{
+ set_bit(kevent, &ir->kevent_flags);
+ if (!schedule_work(&ir->kevent)) {
+ dev_err(ir->dev, "kevent %d may have been dropped", kevent);
+ } else {
+ dev_dbg(ir->dev, "kevent %d scheduled", kevent);
+ }
+}
+
static void mceusb_dev_recv(struct urb *urb)
{
struct mceusb_dev *ir;
@@ -1052,6 +1077,11 @@ static void mceusb_dev_recv(struct urb *urb)
return;
case -EPIPE:
+ dev_err(ir->dev, "Error: urb status = %d (RX HALT)",
+ urb->status);
+ mceusb_defer_kevent(ir, EVENT_RX_HALT);
+ return;
+
default:
dev_err(ir->dev, "Error: urb status = %d", urb->status);
break;
@@ -1170,6 +1200,37 @@ static void mceusb_flash_led(struct mceusb_dev *ir)
mce_async_out(ir, FLASH_LED, sizeof(FLASH_LED));
}
+/*
+ * Workqueue function
+ * for resetting or recovering device after occurrence of error events
+ * specified in ir->kevent bit field.
+ * Function runs (via schedule_work()) in non-interrupt context, for
+ * calls here (such as usb_clear_halt()) requiring non-interrupt context.
+ */
+static void mceusb_deferred_kevent(struct work_struct *work)
+{
+ struct mceusb_dev *ir =
+ container_of(work, struct mceusb_dev, kevent);
+ int status;
+
+ if (test_bit(EVENT_RX_HALT, &ir->kevent_flags)) {
+ usb_unlink_urb(ir->urb_in);
+ status = usb_clear_halt(ir->usbdev, ir->pipe_in);
+ if (status < 0) {
+ dev_err(ir->dev, "rx clear halt error %d",
+ status);
+ return;
+ }
+ clear_bit(EVENT_RX_HALT, &ir->kevent_flags);
+ status = usb_submit_urb(ir->urb_in, GFP_KERNEL);
+ if (status < 0) {
+ dev_err(ir->dev, "rx unhalt submit u
Re: [PATCH 0/3] [media] mceusb: RX -EPIPE lockup fault and more
On 3/26/2017 6:27 AM, Sean Young wrote: ... >> +status = usb_submit_urb(ir->urb_in, GFP_ATOMIC); > > This can be GFP_KERNEL. > ... >> +rc_free_device(ir->rc); > > That change is wrong and will cause a double free. > >> usb_kill_urb(ir->urb_in); >> usb_free_urb(ir->urb_in); >> usb_free_coherent(dev, ir->len_in, ir->buf_in, ir->dma_in); > > Would you be able to split this into multiple commits please? > > Thanks, > Sean > Hi Sean, Thank you for the quick reply, review, corrections, and suggestions. Please bear with me since this is my first contribution for Linux. The patch production submission/review process is entirely new to me at this time. I'll perform the corrections in the forthcoming replies containing the split patches: [PATCH 1/3] [media] mceusb: RX -EPIPE (urb status = -32) lockup failure fix [PATCH 2/3] [media] mceusb: sporadic RX truncation corruption fix [PATCH 3/3] [media] mceusb: fix inaccurate debug buffer dumps and misleading debug messages Thanks again. ..A Sun
[PATCH] mceusb: RX -EPIPE lockup fault and more
commit
https://github.com/asunxx/linux/commit/e557c1d737462961f2aadfbfb0836ffa3c778518
Author: A Sun
Date: Sat Mar 25 02:42:03 2017 -0400
[PATCH] mceusb RX -EPIPE fault and more
Patch for mceusb driver tested with
[8.627769] mceusb 1-1.2:1.0: Registered Pinnacle Systems PCTV Remote USB
with mce emulator interface version 1
[8.627797] mceusb 1-1.2:1.0: 2 tx ports (0x1 cabled) and 2 rx sensors (0x1
active)
running on
pi@raspberrypi:~ $ uname -a
Linux raspberrypi 4.4.50-v7+ #970 SMP Mon Feb 20 19:18:29 GMT 2017 armv7l
GNU/Linux
pi@raspberrypi:~ $
This patch is bug fix and debug messages fix (used for troubleshooting) for
Bug:
RX -EPIPE failure with infinite loop and flooding of
Mar 20 22:24:56 raspberrypi kernel: [ 2851.966506] mceusb 1-1.2:1.0: Error: urb
status = -32
log message at 8000 messages per second.
Bug trigger appears to be normal, but heavy, IR receiver use.
Driver and Linux host become unusable after error.
Also seen at https://sourceforge.net/p/lirc/mailman/message/34886165/
Fix:
Message reports RX usb halt (stall) condition requiring usb_clear_halt() call
in non-interrupt context to recover.
Add driver workqueue call to perform this recovery based on method in use for
the usbnet device driver.
Bug:
Intermittent RX truncation and loss of IR received data. Results in receive
data stream synchronization errors where driver attempts to incorrectly parse
IR data as command responses.
Mar 22 12:01:40 raspberrypi kernel: [ 3969.139898] mceusb 1-1.2:1.0: processed
IR data
Mar 22 12:01:40 raspberrypi kernel: [ 3969.151315] mceusb 1-1.2:1.0: rx data:
00 90 (length=2)
Mar 22 12:01:40 raspberrypi kernel: [ 3969.151321] mceusb 1-1.2:1.0: Unknown
command 0x00 0x90
Mar 22 12:01:40 raspberrypi kernel: [ 3969.151336] mceusb 1-1.2:1.0: rx data:
98 0a 8d 0a 8e 0a 8e 0a 8e 0a 8e 0a 9a 0a 8e 0a 0b 3a 8e 00 80 41 59 00 00
(length=25)
Mar 22 12:01:40 raspberrypi kernel: [ 3969.151341] mceusb 1-1.2:1.0: Raw IR
data, 24 pulse/space samples
Mar 22 12:01:40 raspberrypi kernel: [ 3969.151348] mceusb 1-1.2:1.0: Storing
space with duration 50
Bug trigger appears to be normal, but heavy, IR receiver use.
Fix:
Cause may be receiver with ep_in bulk endpoint incorrectly bound to
usb_fill_int_urb() urb for interrupt endpoint.
In mceusb_dev_probe(), test ep_in endpoint for int versus bulk and use
usb_fill_bulk_urb() as appropriate.
Bug:
Memory leak on disconnect for rc = rc_allocate_device().
Fix:
Add call rc_free_device() to mceusb_dev_disconnect()
Bug:
mceusb_dev_printdata() prints incorrect window of bytes (0 to len) that are of
interest and will be processed next.
Fix:
Add size of received data argument to mceusb_dev_printdata().
Revise buffer print window (offset to offset+len).
Remove static USB_BUFLEN = 32, which is variable depending on device
(my receiver buffer size is 64 for Pinnacle IR transceiver).
Revise bound test to prevent reporting data beyond end of read or end of buffer.
Bug:
Debug messages; some misleading.
Fix:
Drop "\n" use from dev_dbg().
References to "receive request" should read "send request".
Various syntax and other corrections.
Signed-off-by: A Sun
diff --git a/drivers/media/rc/mceusb.c b/drivers/media/rc/mceusb.c
index 238d8ea..48e942e 100644
--- a/drivers/media/rc/mceusb.c
+++ b/drivers/media/rc/mceusb.c
@@ -36,18 +36,18 @@
#include
#include
#include
+#include
#include
#include
#include
#include
-#define DRIVER_VERSION "1.92"
+#define DRIVER_VERSION "1.93"
#define DRIVER_AUTHOR "Jarod Wilson "
#define DRIVER_DESC"Windows Media Center Ed. eHome Infrared Transceiver " \
"device driver"
#define DRIVER_NAME"mceusb"
-#define USB_BUFLEN 32 /* USB reception buffer length */
#define USB_CTRL_MSG_SZ2 /* Size of usb ctrl msg on gen1 hw */
#define MCE_G1_INIT_MSGS 40 /* Init messages on gen1 hw to throw out */
@@ -417,6 +417,7 @@ struct mceusb_dev {
/* usb */
struct usb_device *usbdev;
struct urb *urb_in;
+ unsigned int pipe_in;
struct usb_endpoint_descriptor *usb_ep_out;
/* buffers and dma */
@@ -454,6 +455,12 @@ struct mceusb_dev {
u8 num_rxports; /* number of receive sensors */
u8 txports_cabled; /* bitmask of transmitters with cable */
u8 rxports_active; /* bitmask of active receive sensors */
+
+ /* kevent support */
+ struct work_struct kevent;
+ unsigned long kevent_flags;
+# define EVENT_TX_HALT0
+# define EVENT_RX_HALT1
};
/* MCE Device Command Strings, generally a port and command pair */
@@ -527,7 +534,7 @@ static int mceusb_cmd_datasize(u8 cmd, u8 subcmd)
}
static void mceusb_dev_printdata(struct mceusb_dev *ir, char *buf,
-int offset, int len, bool out)
+
