diff mbox

[media] imon: don't parse scancodes until intf configured

Message ID 1311091967-2791-1-git-send-email-jarod@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jarod Wilson July 19, 2011, 4:12 p.m. UTC
The imon devices have either 1 or 2 usb interfaces on them, each wired
up to its own urb callback. The interface 0 urb callback is wired up
before the imon context's rc_dev pointer is filled in, which is
necessary for imon 0xffdc device auto-detection to work properly, but we
need to make sure we don't actually run the callback routines until
we've entirely filled in the necessary bits for each given interface,
lest we wind up oopsing. Technically, any imon device could have hit
this, but the issue is exacerbated on the 0xffdc devices, which send a
constant stream of interrupts, even when they have no valid key data.

CC: Andy Walls <awalls@md.metrocast.net>
CC: Chris W <lkml@psychogeeks.com>
Reported-by: Chris W <lkml@psychogeeks.com>
Signed-off-by: Jarod Wilson <jarod@redhat.com>
---
 drivers/media/rc/imon.c |   10 ++++++----
 1 files changed, 6 insertions(+), 4 deletions(-)

Comments

Chris W July 19, 2011, 10:05 p.m. UTC | #1
On 20/07/11 02:12, Jarod Wilson wrote:
> The imon devices have either 1 or 2 usb interfaces on them, each wired
> up to its own urb callback. The interface 0 urb callback is wired up
> before the imon context's rc_dev pointer is filled in, which is
> necessary for imon 0xffdc device auto-detection to work properly, but
> we need to make sure we don't actually run the callback routines until
> we've entirely filled in the necessary bits for each given interface,
> lest we wind up oopsing. Technically, any imon device could have hit
> this, but the issue is exacerbated on the 0xffdc devices, which send a
> constant stream of interrupts, even when they have no valid key data.



OK.  The patch applies and everything continues to work.   There is no
obvious difference in the dmesg output on module load, with my device
remaining unidentified.  I don't know if that is indicative of anything.

input: iMON Panel, Knob and Mouse(15c2:ffdc) as
/devices/pci0000:00/0000:00:10.2/usb4/4-2/4-2:1.0/input/input9
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/rc3/input10
rc3: iMON Remote (15c2:ffdc) as
/devices/pci0000:00/0000:00:10.2/usb4/4-2/4-2:1.0/rc/rc3
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


Regards,
Chris
Jarod Wilson July 20, 2011, 1:18 p.m. UTC | #2
On Wed, Jul 20, 2011 at 08:05:43AM +1000, Chris W wrote:
> On 20/07/11 02:12, Jarod Wilson wrote:
> > The imon devices have either 1 or 2 usb interfaces on them, each wired
> > up to its own urb callback. The interface 0 urb callback is wired up
> > before the imon context's rc_dev pointer is filled in, which is
> > necessary for imon 0xffdc device auto-detection to work properly, but
> > we need to make sure we don't actually run the callback routines until
> > we've entirely filled in the necessary bits for each given interface,
> > lest we wind up oopsing. Technically, any imon device could have hit
> > this, but the issue is exacerbated on the 0xffdc devices, which send a
> > constant stream of interrupts, even when they have no valid key data.
> 
> 
> 
> OK.  The patch applies and everything continues to work.   There is no
> obvious difference in the dmesg output on module load, with my device
> remaining unidentified.  I don't know if that is indicative of anything.

Did you apply this patch on top of the earlier patch, or instead of it?

> input: iMON Panel, Knob and Mouse(15c2:ffdc) as
> /devices/pci0000:00/0000:00:10.2/usb4/4-2/4-2:1.0/input/input9
> imon 4-2:1.0: Unknown 0xffdc device, defaulting to VFD and iMON IR (id 0x00)

The 'id 0x00' part should read 'id 0x24', as the ongoing spew below has in
index 6, which makes me think you still have the earlier patch applied.
You actually want only the newer patch applied. If you only have the newer
one applied, then I'll have to take another look to see what's up.

> 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

One other amusing tidbit: you get continuous spew like the above, because
to date, I thought all the ffdc devices had "nothing to report" spew that
started with 0xffffff, which we filter out. Sigh. I hate imon hardware...
Chris W July 20, 2011, 10:56 p.m. UTC | #3
On 20/07/11 23:18, Jarod Wilson wrote:
> On Wed, Jul 20, 2011 at 08:05:43AM +1000, Chris W wrote:
>> On 20/07/11 02:12, Jarod Wilson wrote:
>>> The imon devices have either 1 or 2 usb interfaces on them, each wired
>>> up to its own urb callback. The interface 0 urb callback is wired up
>>> before the imon context's rc_dev pointer is filled in, which is
>>> necessary for imon 0xffdc device auto-detection to work properly, but
>>> we need to make sure we don't actually run the callback routines until
>>> we've entirely filled in the necessary bits for each given interface,
>>> lest we wind up oopsing. Technically, any imon device could have hit
>>> this, but the issue is exacerbated on the 0xffdc devices, which send a
>>> constant stream of interrupts, even when they have no valid key data.
>>
>>
>>
>> OK.  The patch applies and everything continues to work.   There is no
>> obvious difference in the dmesg output on module load, with my device
>> remaining unidentified.  I don't know if that is indicative of anything.
> 
> Did you apply this patch on top of the earlier patch, or instead of it?

On top of it.   I've reversed the patches and installed just the last
one with this result on loading the module:

input: iMON Panel, Knob and Mouse(15c2:ffdc) as
/devices/pci0000:00/0000:00:10.2/usb4/4-2/4-2:1.0/input/input8
imon 4-2:1.0: 0xffdc iMON VFD, iMON IR (id 0x24)
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/rc3/input9
rc3: iMON Remote (15c2:ffdc) as
/devices/pci0000:00/0000:00:10.2/usb4/4-2/4-2:1.0/rc/rc3
imon 4-2:1.0: iMON device (15c2:ffdc, intf0) on usb<4:3> initialized
usbcore: registered new interface driver imon

Much better.

>> 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
> 
> One other amusing tidbit: you get continuous spew like the above, because
> to date, I thought all the ffdc devices had "nothing to report" spew that
> started with 0xffffff, which we filter out. Sigh. I hate imon hardware...

I am beginning to understand why. That output was only printed with the
"debug=1" option and is not printed with the patched module.

Regards,
Chris
Jarod Wilson July 26, 2011, 5:57 p.m. UTC | #4
Chris W wrote:
> On 20/07/11 23:18, Jarod Wilson wrote:
>> On Wed, Jul 20, 2011 at 08:05:43AM +1000, Chris W wrote:
>>> On 20/07/11 02:12, Jarod Wilson wrote:
>>>> The imon devices have either 1 or 2 usb interfaces on them, each wired
>>>> up to its own urb callback. The interface 0 urb callback is wired up
>>>> before the imon context's rc_dev pointer is filled in, which is
>>>> necessary for imon 0xffdc device auto-detection to work properly, but
>>>> we need to make sure we don't actually run the callback routines until
>>>> we've entirely filled in the necessary bits for each given interface,
>>>> lest we wind up oopsing. Technically, any imon device could have hit
>>>> this, but the issue is exacerbated on the 0xffdc devices, which send a
>>>> constant stream of interrupts, even when they have no valid key data.
>>>
>>>
>>> OK.  The patch applies and everything continues to work.   There is no
>>> obvious difference in the dmesg output on module load, with my device
>>> remaining unidentified.  I don't know if that is indicative of anything.
>> Did you apply this patch on top of the earlier patch, or instead of it?
>
> On top of it.   I've reversed the patches and installed just the last
> one with this result on loading the module:
>
> input: iMON Panel, Knob and Mouse(15c2:ffdc) as
> /devices/pci0000:00/0000:00:10.2/usb4/4-2/4-2:1.0/input/input8
> imon 4-2:1.0: 0xffdc iMON VFD, iMON IR (id 0x24)
> 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/rc3/input9
> rc3: iMON Remote (15c2:ffdc) as
> /devices/pci0000:00/0000:00:10.2/usb4/4-2/4-2:1.0/rc/rc3
> imon 4-2:1.0: iMON device (15c2:ffdc, intf0) on usb<4:3>  initialized
> usbcore: registered new interface driver imon
>
> Much better.

Yeah, that looks sane now. We missed 3.0, but I'll try to flag this one 
to go into the various stable trees when it gets merged for 3.1.


>>> 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
>> One other amusing tidbit: you get continuous spew like the above, because
>> to date, I thought all the ffdc devices had "nothing to report" spew that
>> started with 0xffffff, which we filter out. Sigh. I hate imon hardware...
>
> I am beginning to understand why. That output was only printed with the
> "debug=1" option and is not printed with the patched module.

Yup. The additional filtering was added because my own ffdc imon devices 
were so noisy, it was next to impossible to see what was going on when 
trying to debug anything.
diff mbox

Patch

diff --git a/drivers/media/rc/imon.c b/drivers/media/rc/imon.c
index caa3e3a..6ed9646 100644
--- a/drivers/media/rc/imon.c
+++ b/drivers/media/rc/imon.c
@@ -1658,7 +1658,7 @@  static void usb_rx_callback_intf0(struct urb *urb)
 		return;
 
 	ictx = (struct imon_context *)urb->context;
-	if (!ictx)
+	if (!ictx || !ictx->dev_present_intf0)
 		return;
 
 	switch (urb->status) {
@@ -1690,7 +1690,7 @@  static void usb_rx_callback_intf1(struct urb *urb)
 		return;
 
 	ictx = (struct imon_context *)urb->context;
-	if (!ictx)
+	if (!ictx || !ictx->dev_present_intf1)
 		return;
 
 	switch (urb->status) {
@@ -2118,7 +2118,6 @@  static struct imon_context *imon_init_intf0(struct usb_interface *intf)
 
 	ictx->dev = dev;
 	ictx->usbdev_intf0 = usb_get_dev(interface_to_usbdev(intf));
-	ictx->dev_present_intf0 = true;
 	ictx->rx_urb_intf0 = rx_urb;
 	ictx->tx_urb = tx_urb;
 	ictx->rf_device = false;
@@ -2157,6 +2156,8 @@  static struct imon_context *imon_init_intf0(struct usb_interface *intf)
 		goto rdev_setup_failed;
 	}
 
+	ictx->dev_present_intf0 = true;
+
 	mutex_unlock(&ictx->lock);
 	return ictx;
 
@@ -2200,7 +2201,6 @@  static struct imon_context *imon_init_intf1(struct usb_interface *intf,
 	}
 
 	ictx->usbdev_intf1 = usb_get_dev(interface_to_usbdev(intf));
-	ictx->dev_present_intf1 = true;
 	ictx->rx_urb_intf1 = rx_urb;
 
 	ret = -ENODEV;
@@ -2229,6 +2229,8 @@  static struct imon_context *imon_init_intf1(struct usb_interface *intf,
 		goto urb_submit_failed;
 	}
 
+	ictx->dev_present_intf1 = true;
+
 	mutex_unlock(&ictx->lock);
 	return ictx;