Jarod Wilson (the imon driver maintainer) asked my to open a report here so that the issues are not forgotten. - There is no fallback timer if a release code is missed (i.e. remote pointed away from receiver or some other anomaly), causing a key stuck on autorepeat. The driver should inject a release code itself if there is no release/repeat code in 500ms after initial press or in 120ms after a repeat code. - No release code is sent for 0x02XXXXXX keys if pressing any other button before release, examples: example 1: hold '5', then press 'Play' once, then release '5' The 'Play' codes are relayed properly, but the release code for '5' (the 'all 0x02XXXXXX keys released' (i.e. empty HID input report) which the hardware does send properly) is wrongly interpreted as a release code for 'Play'. The driver should either release '5' when the empty report is received, or, as this is just a remote control, just inject a release code for '5' when 'Play' is pressed. example 2: hold '5', then hold '4', then release '5', then release '4' As the 0x02XXXXXX range is not completely released until after '4' is released, the zeroed bitfield is not sent until after '4', and the driver doesn't release '5' at this point anymore. The driver should've injected a release code for '5' when '4' was pressed. - imon_mce_timeout() timer function seems to access ictx->last_keycode without locking - imon_parse_press_type() tests for ictx->release_code which is in an undefined state if ktype isn't IMON_KEY_IMON - when the dpad is in keyboard mode and you hold it down to one direction, instead of autorepeat there is a constant stream of release/press events
Additional bits from ml thread: BTW, I wonder if we should also disable kernel autorepeat and use the remote control's own repeat signals instead? Then extra repeat codes would not not emitted if the release signal is missed.
Some additional issues pointed out by Dmitry Torokhov: Random notes about irmon: imon_init_idev(): memcpy(&ir->dev, ictx->dev, sizeof(struct device)); This is... scary. Devices are refcounted and if you copy them around all hell may break loose. On an unrelated note you do not need memcpy to copy a structire, *it->dev = *ictx->dev will do. imon_init_idev(), imon_init_touch(): - consizer returning proper error codes via ERR_PTR() and check wit IS_ERR().
Created attachment 29852 [details] Patch to address a number of issues raised here I think this patch handles the bulk of the issues reported here. This is still a bit of a work-in-progress though, as I'm also looking to incorporate some changes from David Hardeman to split the mouse/panel keys from the remote keys in preparation for some further ir-core interface abstraction work he's doing.
David's patch actually converts the imon driver over to using more native ir-core functionality, which actually addresses the key release issues better, in addition to fixing the repeat issue when the dpad is in keyboard mode. I've got a follow-up patch that adds some locking around access to ictx->kc and ictx->last_keycode, and with that, I think we've got just about everything here licked... Will be posted to linux-media list shortly.
fixed in .37-rc1: commit 693508df9824ecc2bf308a35b58159aa2fecf91f Author: Jarod Wilson <jarod@redhat.com> Date: Wed Sep 15 15:56:03 2010 -0300 V4L/DVB: IR/imon: protect ictx's kc and last_keycode w/spinlock