Hi,
Following patchset fixes several issues with imon driver and reverts one
(previously applied) buggy patch.
Mauro, could you please apply this on top of your tree?
Best regards,
Kevin Baradon (4):
Revert "media/rc/imon.c: make send_packet() delay larger for 15c2:0036"
media/rc/imon.c: make send_packet() delay larger for 15c2:0036 [v2]
media/rc/imon.c: do not try to register 2nd intf if 1st intf failed
media/rc/imon.c: kill urb when send_packet() is interrupted
drivers/media/rc/imon.c | 26 ++++++++++++++++++--------
1 file changed, 18 insertions(+), 8 deletions(-)
--
1.7.10.4
This reverts commit d92f150f9cb80b4df56331d1f42442da78e351f0.
It seems send_packet() is used during initialization, before send_packet_delay is set.
This will be fixed by another patch.
Signed-off-by: Kevin Baradon <[email protected]>
---
drivers/media/rc/imon.c | 17 ++++-------------
1 file changed, 4 insertions(+), 13 deletions(-)
diff --git a/drivers/media/rc/imon.c b/drivers/media/rc/imon.c
index b8f9f85..e5d1c0d 100644
--- a/drivers/media/rc/imon.c
+++ b/drivers/media/rc/imon.c
@@ -112,7 +112,6 @@ struct imon_context {
bool tx_control;
unsigned char usb_rx_buf[8];
unsigned char usb_tx_buf[8];
- unsigned int send_packet_delay;
struct tx_t {
unsigned char data_buf[35]; /* user data buffer */
@@ -186,10 +185,6 @@ enum {
IMON_KEY_PANEL = 2,
};
-enum {
- IMON_NEED_20MS_PKT_DELAY = 1
-};
-
/*
* USB Device ID for iMON USB Control Boards
*
@@ -220,7 +215,7 @@ static struct usb_device_id imon_usb_id_table[] = {
/* SoundGraph iMON OEM Touch LCD (IR & 4.3" VGA LCD) */
{ USB_DEVICE(0x15c2, 0x0035) },
/* SoundGraph iMON OEM VFD (IR & VFD) */
- { USB_DEVICE(0x15c2, 0x0036), .driver_info = IMON_NEED_20MS_PKT_DELAY },
+ { USB_DEVICE(0x15c2, 0x0036) },
/* device specifics unknown */
{ USB_DEVICE(0x15c2, 0x0037) },
/* SoundGraph iMON OEM LCD (IR & LCD) */
@@ -540,12 +535,12 @@ static int send_packet(struct imon_context *ictx)
kfree(control_req);
/*
- * Induce a mandatory delay before returning, as otherwise,
+ * Induce a mandatory 5ms delay before returning, as otherwise,
* send_packet can get called so rapidly as to overwhelm the device,
* particularly on faster systems and/or those with quirky usb.
*/
- timeout = msecs_to_jiffies(ictx->send_packet_delay);
- set_current_state(TASK_INTERRUPTIBLE);
+ timeout = msecs_to_jiffies(5);
+ set_current_state(TASK_UNINTERRUPTIBLE);
schedule_timeout(timeout);
return retval;
@@ -2329,10 +2324,6 @@ static int imon_probe(struct usb_interface *interface,
}
- /* default send_packet delay is 5ms but some devices need more */
- ictx->send_packet_delay = id->driver_info & IMON_NEED_20MS_PKT_DELAY ?
- 20 : 5;
-
usb_set_intfdata(interface, ictx);
if (ifnum == 0) {
--
1.7.10.4
This bug could be triggered if 1st interface configuration fails:
Apr 8 18:20:30 homeserver kernel: usb 5-1: new low-speed USB device number 2 using ohci_hcd
Apr 8 18:20:30 homeserver kernel: input: iMON Panel, Knob and Mouse(15c2:0036) as /devices/pci0000:00/0000:00:13.0/usb5/5-1/5-1:1.0/input/input2
Apr 8 18:20:30 homeserver kernel: Registered IR keymap rc-imon-pad
Apr 8 18:20:30 homeserver kernel: input: iMON Remote (15c2:0036) as /devices/pci0000:00/0000:00:13.0/usb5/5-1/5-1:1.0/rc/rc0/input3
Apr 8 18:20:30 homeserver kernel: rc0: iMON Remote (15c2:0036) as /devices/pci0000:00/0000:00:13.0/usb5/5-1/5-1:1.0/rc/rc0
Apr 8 18:20:30 homeserver kernel: imon:send_packet: packet tx failed (-32)
Apr 8 18:20:30 homeserver kernel: imon 5-1:1.0: remote input dev register failed
Apr 8 18:20:30 homeserver kernel: imon 5-1:1.0: imon_init_intf0: rc device setup failed
Apr 8 18:20:30 homeserver kernel: imon 5-1:1.0: unable to initialize intf0, err 0
Apr 8 18:20:30 homeserver kernel: imon:imon_probe: failed to initialize context!
Apr 8 18:20:30 homeserver kernel: imon 5-1:1.0: unable to register, err -19
Apr 8 18:20:30 homeserver kernel: BUG: unable to handle kernel NULL pointer dereference at 00000014
Apr 8 18:20:30 homeserver kernel: IP: [<c05c4e4c>] mutex_lock+0xc/0x30
Apr 8 18:20:30 homeserver kernel: *pde = 00000000
Apr 8 18:20:30 homeserver kernel: Oops: 0002 [#1] PREEMPT SMP
Apr 8 18:20:30 homeserver kernel: Modules linked in:
Apr 8 18:20:30 homeserver kernel: Pid: 367, comm: khubd Not tainted 3.8.3-htpc-00002-g79b1403 #23 Unknow Unknow/RS780-SB700
Apr 8 18:20:30 homeserver kernel: EIP: 0060:[<c05c4e4c>] EFLAGS: 00010296 CPU: 1
Apr 8 18:20:30 homeserver kernel: EIP is at mutex_lock+0xc/0x30
Apr 8 18:20:30 homeserver kernel: EAX: 00000014 EBX: 00000014 ECX: 00000000 EDX: f590e480
Apr 8 18:20:30 homeserver kernel: ESI: f5deac00 EDI: f590e480 EBP: f5f3ee00 ESP: f6577c28
Apr 8 18:20:30 homeserver kernel: DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068
Apr 8 18:20:30 homeserver kernel: CR0: 8005003b CR2: 00000014 CR3: 0081b000 CR4: 000007d0
Apr 8 18:20:30 homeserver kernel: DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000
Apr 8 18:20:30 homeserver kernel: DR6: ffff0ff0 DR7: 00000400
Apr 8 18:20:30 homeserver kernel: Process khubd (pid: 367, ti=f6576000 task=f649ea00 task.ti=f6576000)
Apr 8 18:20:30 homeserver kernel: Stack:
Apr 8 18:20:30 homeserver kernel: 00000000 f5deac00 c0448de4 f59714c0 f5deac64 c03b8ad2 f6577c90 00000004
Apr 8 18:20:30 homeserver kernel: f649ea00 c0205142 f6779820 a1ff7f08 f5deac00 00000001 f5f3ee1c 00000014
Apr 8 18:20:30 homeserver kernel: 00000004 00000202 15c20036 c07a03e8 fffee0ca f6795c00 f5f3ee1c f5deac00
Apr 8 18:20:30 homeserver kernel: Call Trace:
Apr 8 18:20:30 homeserver kernel: [<c0448de4>] ? imon_probe+0x494/0xde0
Apr 8 18:20:30 homeserver kernel: [<c03b8ad2>] ? rpm_resume+0xb2/0x4f0
Apr 8 18:20:30 homeserver kernel: [<c0205142>] ? sysfs_addrm_finish+0x12/0x90
Apr 8 18:20:30 homeserver kernel: [<c04170e9>] ? usb_probe_interface+0x169/0x240
Apr 8 18:20:30 homeserver kernel: [<c03b0ca0>] ? __driver_attach+0x80/0x80
Apr 8 18:20:30 homeserver kernel: [<c03b0ca0>] ? __driver_attach+0x80/0x80
Apr 8 18:20:30 homeserver kernel: [<c03b0a94>] ? driver_probe_device+0x54/0x1e0
Apr 8 18:20:30 homeserver kernel: [<c0416abe>] ? usb_device_match+0x4e/0x80
Apr 8 18:20:30 homeserver kernel: [<c03af314>] ? bus_for_each_drv+0x34/0x70
Apr 8 18:20:30 homeserver kernel: [<c03b0a0b>] ? device_attach+0x7b/0x90
Apr 8 18:20:30 homeserver kernel: [<c03b0ca0>] ? __driver_attach+0x80/0x80
Apr 8 18:20:30 homeserver kernel: [<c03b00ff>] ? bus_probe_device+0x5f/0x80
Apr 8 18:20:30 homeserver kernel: [<c03aeab7>] ? device_add+0x567/0x610
Apr 8 18:20:30 homeserver kernel: [<c041a7bc>] ? usb_create_ep_devs+0x7c/0xd0
Apr 8 18:20:30 homeserver kernel: [<c0413837>] ? create_intf_ep_devs+0x47/0x70
Apr 8 18:20:30 homeserver kernel: [<c04156c4>] ? usb_set_configuration+0x454/0x750
Apr 8 18:20:30 homeserver kernel: [<c03b0ca0>] ? __driver_attach+0x80/0x80
Apr 8 18:20:30 homeserver kernel: [<c041de8a>] ? generic_probe+0x2a/0x80
Apr 8 18:20:30 homeserver kernel: [<c03b0ca0>] ? __driver_attach+0x80/0x80
Apr 8 18:20:30 homeserver kernel: [<c0205aff>] ? sysfs_create_link+0xf/0x20
Apr 8 18:20:30 homeserver kernel: [<c04171db>] ? usb_probe_device+0x1b/0x40
Apr 8 18:20:30 homeserver kernel: [<c03b0a94>] ? driver_probe_device+0x54/0x1e0
Apr 8 18:20:30 homeserver kernel: [<c03af314>] ? bus_for_each_drv+0x34/0x70
Apr 8 18:20:30 homeserver kernel: [<c03b0a0b>] ? device_attach+0x7b/0x90
Apr 8 18:20:30 homeserver kernel: [<c03b0ca0>] ? __driver_attach+0x80/0x80
Apr 8 18:20:30 homeserver kernel: [<c03b00ff>] ? bus_probe_device+0x5f/0x80
Apr 8 18:20:30 homeserver kernel: [<c03aeab7>] ? device_add+0x567/0x610
Apr 8 18:20:30 homeserver kernel: [<c040e6df>] ? usb_new_device+0x12f/0x1e0
Apr 8 18:20:30 homeserver kernel: [<c040f4d8>] ? hub_thread+0x458/0x1230
Apr 8 18:20:30 homeserver kernel: [<c015554f>] ? dequeue_task_fair+0x9f/0xc0
Apr 8 18:20:30 homeserver kernel: [<c0131312>] ? release_task+0x1d2/0x330
Apr 8 18:20:30 homeserver kernel: [<c01477b0>] ? abort_exclusive_wait+0x90/0x90
Apr 8 18:20:30 homeserver kernel: [<c040f080>] ? usb_remote_wakeup+0x40/0x40
Apr 8 18:20:30 homeserver kernel: [<c0146ed2>] ? kthread+0x92/0xa0
Apr 8 18:20:30 homeserver kernel: [<c05c7877>] ? ret_from_kernel_thread+0x1b/0x28
Apr 8 18:20:30 homeserver kernel: [<c0146e40>] ? kthread_freezable_should_stop+0x50/0x50
Apr 8 18:20:30 homeserver kernel: Code: 89 04 24 89 f0 e8 05 ff ff ff 8b 5c 24 24 8b 74 24 28 8b 7c 24 2c 8b 6c 24 30 83 c4 34 c3 00 83 ec 08 89 1c 24 89 74 24 04 89 c3 <f0> ff 08 79 05 e8 ca 03 00 00 64 a1 70 d6 80 c0 8b 74 24 04 89
Apr 8 18:20:30 homeserver kernel: EIP: [<c05c4e4c>] mutex_lock+0xc/0x30 SS:ESP 0068:f6577c28
Apr 8 18:20:30 homeserver kernel: CR2: 0000000000000014
Apr 8 18:20:30 homeserver kernel: ---[ end trace df134132c967205c ]---
Signed-off-by: Kevin Baradon <[email protected]>
---
drivers/media/rc/imon.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/media/rc/imon.c b/drivers/media/rc/imon.c
index 624fd33..3af7bb6 100644
--- a/drivers/media/rc/imon.c
+++ b/drivers/media/rc/imon.c
@@ -2324,7 +2324,14 @@ static int imon_probe(struct usb_interface *interface,
}
} else {
- /* this is the secondary interface on the device */
+ /* this is the secondary interface on the device */
+
+ /* fail early if first intf failed to register */
+ if (!first_if_ctx) {
+ ret = -ENODEV;
+ goto fail;
+ }
+
ictx = imon_init_intf1(interface, first_if_ctx);
if (!ictx) {
pr_err("failed to attach to context!\n");
--
1.7.10.4
Imon device 15c2:0036 need a higher delay between send_packet() calls.
Also use interruptible wait to avoid load average going too high (and let caller handle signals).
Signed-off-by: Kevin Baradon <[email protected]>
---
drivers/media/rc/imon.c | 22 ++++++++++++++++------
1 file changed, 16 insertions(+), 6 deletions(-)
diff --git a/drivers/media/rc/imon.c b/drivers/media/rc/imon.c
index e5d1c0d..624fd33 100644
--- a/drivers/media/rc/imon.c
+++ b/drivers/media/rc/imon.c
@@ -112,6 +112,7 @@ struct imon_context {
bool tx_control;
unsigned char usb_rx_buf[8];
unsigned char usb_tx_buf[8];
+ unsigned int send_packet_delay;
struct tx_t {
unsigned char data_buf[35]; /* user data buffer */
@@ -185,6 +186,10 @@ enum {
IMON_KEY_PANEL = 2,
};
+enum {
+ IMON_NEED_20MS_PKT_DELAY = 1
+};
+
/*
* USB Device ID for iMON USB Control Boards
*
@@ -215,7 +220,7 @@ static struct usb_device_id imon_usb_id_table[] = {
/* SoundGraph iMON OEM Touch LCD (IR & 4.3" VGA LCD) */
{ USB_DEVICE(0x15c2, 0x0035) },
/* SoundGraph iMON OEM VFD (IR & VFD) */
- { USB_DEVICE(0x15c2, 0x0036) },
+ { USB_DEVICE(0x15c2, 0x0036), .driver_info = IMON_NEED_20MS_PKT_DELAY },
/* device specifics unknown */
{ USB_DEVICE(0x15c2, 0x0037) },
/* SoundGraph iMON OEM LCD (IR & LCD) */
@@ -535,12 +540,12 @@ static int send_packet(struct imon_context *ictx)
kfree(control_req);
/*
- * Induce a mandatory 5ms delay before returning, as otherwise,
+ * Induce a mandatory delay before returning, as otherwise,
* send_packet can get called so rapidly as to overwhelm the device,
* particularly on faster systems and/or those with quirky usb.
*/
- timeout = msecs_to_jiffies(5);
- set_current_state(TASK_UNINTERRUPTIBLE);
+ timeout = msecs_to_jiffies(ictx->send_packet_delay);
+ set_current_state(TASK_INTERRUPTIBLE);
schedule_timeout(timeout);
return retval;
@@ -2088,7 +2093,8 @@ static bool imon_find_endpoints(struct imon_context *ictx,
}
-static struct imon_context *imon_init_intf0(struct usb_interface *intf)
+static struct imon_context *imon_init_intf0(struct usb_interface *intf,
+ const struct usb_device_id *id)
{
struct imon_context *ictx;
struct urb *rx_urb;
@@ -2128,6 +2134,10 @@ static struct imon_context *imon_init_intf0(struct usb_interface *intf)
ictx->vendor = le16_to_cpu(ictx->usbdev_intf0->descriptor.idVendor);
ictx->product = le16_to_cpu(ictx->usbdev_intf0->descriptor.idProduct);
+ /* default send_packet delay is 5ms but some devices need more */
+ ictx->send_packet_delay = id->driver_info & IMON_NEED_20MS_PKT_DELAY ?
+ 20 : 5;
+
ret = -ENODEV;
iface_desc = intf->cur_altsetting;
if (!imon_find_endpoints(ictx, iface_desc)) {
@@ -2306,7 +2316,7 @@ static int imon_probe(struct usb_interface *interface,
first_if_ctx = usb_get_intfdata(first_if);
if (ifnum == 0) {
- ictx = imon_init_intf0(interface);
+ ictx = imon_init_intf0(interface, id);
if (!ictx) {
pr_err("failed to initialize context!\n");
ret = -ENODEV;
--
1.7.10.4
This avoids:
Apr 12 23:52:16 homeserver kernel: imon:send_packet: task interrupted
Apr 12 23:52:16 homeserver kernel: ------------[ cut here ]------------
Apr 12 23:52:16 homeserver kernel: WARNING: at drivers/usb/core/urb.c:327 usb_submit_urb+0x353/0x370()
Apr 12 23:52:16 homeserver kernel: Hardware name: Unknow
Apr 12 23:52:16 homeserver kernel: URB f64b6f00 submitted while active
Apr 12 23:52:16 homeserver kernel: Modules linked in:
Apr 12 23:52:16 homeserver kernel: Pid: 3154, comm: LCDd Not tainted 3.8.6-htpc-00005-g9e6fc5e #26
Apr 12 23:52:16 homeserver kernel: Call Trace:
Apr 12 23:52:16 homeserver kernel: [<c012d778>] ? warn_slowpath_common+0x78/0xb0
Apr 12 23:52:16 homeserver kernel: [<c04136c3>] ? usb_submit_urb+0x353/0x370
Apr 12 23:52:16 homeserver kernel: [<c04136c3>] ? usb_submit_urb+0x353/0x370
Apr 12 23:52:16 homeserver kernel: [<c0447010>] ? imon_ir_change_protocol+0x150/0x150
Apr 12 23:52:16 homeserver kernel: [<c012d843>] ? warn_slowpath_fmt+0x33/0x40
Apr 12 23:52:16 homeserver kernel: [<c04136c3>] ? usb_submit_urb+0x353/0x370
Apr 12 23:52:16 homeserver kernel: [<c0446c67>] ? send_packet+0x97/0x270
Apr 12 23:52:16 homeserver kernel: [<c0446cfe>] ? send_packet+0x12e/0x270
Apr 12 23:52:16 homeserver kernel: [<c05c5743>] ? do_nanosleep+0xa3/0xd0
Apr 12 23:52:16 homeserver kernel: [<c044760e>] ? vfd_write+0xae/0x250
Apr 12 23:52:16 homeserver kernel: [<c0447560>] ? lcd_write+0x180/0x180
Apr 12 23:52:16 homeserver kernel: [<c01b2b19>] ? vfs_write+0x89/0x140
Apr 12 23:52:16 homeserver kernel: [<c01b2dda>] ? sys_write+0x4a/0x90
Apr 12 23:52:16 homeserver kernel: [<c05c7c45>] ? sysenter_do_call+0x12/0x26
Apr 12 23:52:16 homeserver kernel: ---[ end trace a0b6f0fcfd2f9a1d ]---
Apr 12 23:52:16 homeserver kernel: imon:send_packet: error submitting urb(-16)
Apr 12 23:52:16 homeserver kernel: imon:vfd_write: send packet #3 failed
Apr 12 23:52:16 homeserver kernel: imon:send_packet: error submitting urb(-16)
Apr 12 23:52:16 homeserver kernel: imon:vfd_write: send packet #0 failed
Signed-off-by: Kevin Baradon <[email protected]>
---
drivers/media/rc/imon.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/media/rc/imon.c b/drivers/media/rc/imon.c
index 3af7bb6..72e3fa6 100644
--- a/drivers/media/rc/imon.c
+++ b/drivers/media/rc/imon.c
@@ -528,8 +528,10 @@ static int send_packet(struct imon_context *ictx)
mutex_unlock(&ictx->lock);
retval = wait_for_completion_interruptible(
&ictx->tx.finished);
- if (retval)
+ if (retval) {
+ usb_kill_urb(ictx->tx_urb);
pr_err_ratelimited("task interrupted\n");
+ }
mutex_lock(&ictx->lock);
retval = ictx->tx.status;
--
1.7.10.4
Hi Kevin,
Em 22-04-2013 17:09, Kevin Baradon escreveu:
> This reverts commit d92f150f9cb80b4df56331d1f42442da78e351f0.
> It seems send_packet() is used during initialization, before send_packet_delay is set.
>
> This will be fixed by another patch.
Reverting patches is a resource that we generally use only when
there's something deadly wrong, as it makes the git history
dirtier, hides the reasons why a change is needed, and might be bad
for git bisecting.
In this specific case, by applying both the revert patch and your newer
one, it is clear that your intent is to move the logic that changes
the send packet delay, because it needs to happen earlier.
So, instead of applying both patches, I'll fold them into one,
as enclosed.
-
From: Kevin Baradon <[email protected]>
[media] imon: Use large delays earlier
send_packet() is used during initialization, before send_packet_delay
is set. So, move ictx->send_packet_delay to happen earlier.
[[email protected]: fold two patches into one to make git history clearer]
Signed-off-by: Kevin Baradon <[email protected]>
diff --git a/drivers/media/rc/imon.c b/drivers/media/rc/imon.c
index b8f9f85..624fd33 100644
--- a/drivers/media/rc/imon.c
+++ b/drivers/media/rc/imon.c
@@ -2093,7 +2093,8 @@ static bool imon_find_endpoints(struct imon_context *ictx,
}
-static struct imon_context *imon_init_intf0(struct usb_interface *intf)
+static struct imon_context *imon_init_intf0(struct usb_interface *intf,
+ const struct usb_device_id *id)
{
struct imon_context *ictx;
struct urb *rx_urb;
@@ -2133,6 +2134,10 @@ static struct imon_context *imon_init_intf0(struct usb_interface *intf)
ictx->vendor = le16_to_cpu(ictx->usbdev_intf0->descriptor.idVendor);
ictx->product = le16_to_cpu(ictx->usbdev_intf0->descriptor.idProduct);
+ /* default send_packet delay is 5ms but some devices need more */
+ ictx->send_packet_delay = id->driver_info & IMON_NEED_20MS_PKT_DELAY ?
+ 20 : 5;
+
ret = -ENODEV;
iface_desc = intf->cur_altsetting;
if (!imon_find_endpoints(ictx, iface_desc)) {
@@ -2311,7 +2316,7 @@ static int imon_probe(struct usb_interface *interface,
first_if_ctx = usb_get_intfdata(first_if);
if (ifnum == 0) {
- ictx = imon_init_intf0(interface);
+ ictx = imon_init_intf0(interface, id);
if (!ictx) {
pr_err("failed to initialize context!\n");
ret = -ENODEV;
@@ -2329,10 +2334,6 @@ static int imon_probe(struct usb_interface *interface,
}
- /* default send_packet delay is 5ms but some devices need more */
- ictx->send_packet_delay = id->driver_info & IMON_NEED_20MS_PKT_DELAY ?
- 20 : 5;
-
usb_set_intfdata(interface, ictx);
if (ifnum == 0) {