| Message ID | 1311007609-28210-1-git-send-email-jarod@redhat.com (mailing list archive) |
|---|---|
| State | Rejected |
| Headers | show |
On 19/07/11 02:46, Jarod Wilson wrote: > The interface 0 urb callback was being wired up before the rc_dev device > was allocated, meaning the callback could be called with a null rc_dev, > leading to an oops. This likely only ever happens on the older 0xffdc > SoundGraph devices, which continually trigger interrupts even when they > have no valid keydata, and the window in which it could happen is small, > but its actually happening regularly for at least one user, and its an > obvious fix. Compile and sanity-tested with one of my own imon devices. As the "at least one user" I can confirm that the patch has indeed corrected the problem on my 2.6.38-gentoo-r6, 2.6.39.3 vanilla, and 3.0.0-rc7 kernels. This is what loading the module with the "debug=1" option outputs: input: iMON Panel, Knob and Mouse(15c2:ffdc) as /devices/pci0000:00/0000:00:10.2/usb4/4-2/4-2:1.0/input/input7 imon 4-2:1.0: Unknown 0xffdc device, defaulting to VFD and iMON IR (id 0x00) Registered IR keymap rc-imon-pad input: iMON Remote (15c2:ffdc) as /devices/pci0000:00/0000:00:10.2/usb4/4-2/4-2:1.0/rc/rc2/input8 rc2: iMON Remote (15c2:ffdc) as /devices/pci0000:00/0000:00:10.2/usb4/4-2/4-2:1.0/rc/rc2 imon 4-2:1.0: iMON device (15c2:ffdc, intf0) on usb<4:3> initialized usbcore: registered new interface driver imon intf0 decoded packet: 00 00 00 00 00 00 24 01 intf0 decoded packet: 00 00 00 00 00 00 24 01 intf0 decoded packet: 00 00 00 00 00 00 24 01 intf0 decoded packet: 00 00 00 00 00 00 24 01 ... The decoded packet lines are fast and furious with no deliberate IR input (the VFD is in use), which might explain how this device managed to break the code in the small window available. Thank you Jarod and Andy for taking the time to track this problem down to give it a drubbing. Regards, Chris
On Jul 18, 2011, at 6:29 PM, Chris W <lkml@psychogeeks.com> wrote: > On 19/07/11 02:46, Jarod Wilson wrote: >> The interface 0 urb callback was being wired up before the rc_dev device >> was allocated, meaning the callback could be called with a null rc_dev, >> leading to an oops. This likely only ever happens on the older 0xffdc >> SoundGraph devices, which continually trigger interrupts even when they >> have no valid keydata, and the window in which it could happen is small, >> but its actually happening regularly for at least one user, and its an >> obvious fix. Compile and sanity-tested with one of my own imon devices. > > As the "at least one user" I can confirm that the patch has indeed > corrected the problem on my 2.6.38-gentoo-r6, 2.6.39.3 vanilla, and > 3.0.0-rc7 kernels. > > This is what loading the module with the "debug=1" option outputs: > > input: iMON Panel, Knob and Mouse(15c2:ffdc) as > /devices/pci0000:00/0000:00:10.2/usb4/4-2/4-2:1.0/input/input7 > imon 4-2:1.0: Unknown 0xffdc device, defaulting to VFD and iMON IR (id 0x00) Ugh, damn, that change broke the ffdc device auto-detection... Better than a crash, but lemme see if I can alter things slightly so we can have our cake and eat it too... > Registered IR keymap rc-imon-pad > input: iMON Remote (15c2:ffdc) as > /devices/pci0000:00/0000:00:10.2/usb4/4-2/4-2:1.0/rc/rc2/input8 > rc2: iMON Remote (15c2:ffdc) as > /devices/pci0000:00/0000:00:10.2/usb4/4-2/4-2:1.0/rc/rc2 > imon 4-2:1.0: iMON device (15c2:ffdc, intf0) on usb<4:3> initialized > usbcore: registered new interface driver imon > intf0 decoded packet: 00 00 00 00 00 00 24 01 > intf0 decoded packet: 00 00 00 00 00 00 24 01 > intf0 decoded packet: 00 00 00 00 00 00 24 01 > intf0 decoded packet: 00 00 00 00 00 00 24 01 > ... > > The decoded packet lines are fast and furious with no deliberate IR > input (the VFD is in use), which might explain how this device managed > to break the code in the small window available. Yep. I hate hate hate hate the ffdc imon hardware, for this and multiple other reasons (including the nasty hack used for ffdc device type auto-detection)... My ffdc devices do similar constant spewing, but never triggered the oops, so maybe yours is even worse, or your system is faster, or a kernel config change made a difference here... > Thank you Jarod and Andy for taking the time to track this problem down > to give it a drubbing. Thanks for the testing and patience, hopefully just one more patch to test out before we can say case closed here... --jarod -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Jarod Wilson wrote: > The interface 0 urb callback was being wired up before the rc_dev device > was allocated, meaning the callback could be called with a null rc_dev, > leading to an oops. This likely only ever happens on the older 0xffdc > SoundGraph devices, which continually trigger interrupts even when they > have no valid keydata, and the window in which it could happen is small, > but its actually happening regularly for at least one user, and its an > obvious fix. Compile and sanity-tested with one of my own imon devices. Explicit self-nak on this one, just to crystal-clear, since this is handled without breaking ffdc device detection by a later patch.
diff --git a/drivers/media/rc/imon.c b/drivers/media/rc/imon.c index caa3e3a..26238f5 100644 --- a/drivers/media/rc/imon.c +++ b/drivers/media/rc/imon.c @@ -2132,6 +2132,18 @@ static struct imon_context *imon_init_intf0(struct usb_interface *intf) goto find_endpoint_failed; } + ictx->idev = imon_init_idev(ictx); + if (!ictx->idev) { + dev_err(dev, "%s: input device setup failed\n", __func__); + goto idev_setup_failed; + } + + ictx->rdev = imon_init_rdev(ictx); + if (!ictx->rdev) { + dev_err(dev, "%s: rc device setup failed\n", __func__); + goto rdev_setup_failed; + } + usb_fill_int_urb(ictx->rx_urb_intf0, ictx->usbdev_intf0, usb_rcvintpipe(ictx->usbdev_intf0, ictx->rx_endpoint_intf0->bEndpointAddress), @@ -2145,26 +2157,14 @@ static struct imon_context *imon_init_intf0(struct usb_interface *intf) goto urb_submit_failed; } - ictx->idev = imon_init_idev(ictx); - if (!ictx->idev) { - dev_err(dev, "%s: input device setup failed\n", __func__); - goto idev_setup_failed; - } - - ictx->rdev = imon_init_rdev(ictx); - if (!ictx->rdev) { - dev_err(dev, "%s: rc device setup failed\n", __func__); - goto rdev_setup_failed; - } - mutex_unlock(&ictx->lock); return ictx; +urb_submit_failed: + rc_unregister_device(ictx->rdev); rdev_setup_failed: input_unregister_device(ictx->idev); idev_setup_failed: - usb_kill_urb(ictx->rx_urb_intf0); -urb_submit_failed: find_endpoint_failed: mutex_unlock(&ictx->lock); usb_free_urb(tx_urb);
The interface 0 urb callback was being wired up before the rc_dev device was allocated, meaning the callback could be called with a null rc_dev, leading to an oops. This likely only ever happens on the older 0xffdc SoundGraph devices, which continually trigger interrupts even when they have no valid keydata, and the window in which it could happen is small, but its actually happening regularly for at least one user, and its an obvious fix. Compile and sanity-tested with one of my own imon devices. CC: Andy Walls <awalls@md.metrocast.net> CC: Chris W <lkml@psychogeeks.com> CC: Randy Dunlap <rdunlap@xenotime.net> CC: linux-kernel@vger.kernel.org Reported-by: Chris W <lkml@psychogeeks.com> Signed-off-by: Jarod Wilson <jarod@redhat.com> --- drivers/media/rc/imon.c | 28 ++++++++++++++-------------- 1 files changed, 14 insertions(+), 14 deletions(-)