Re: [rfd] saving old mice -- button glitching/debouncing
Hi Pavel, I love your patch! Perhaps something to improve: [auto build test WARNING on input/next] [also build test WARNING on v4.20-rc7 next-20181218] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Pavel-Machek/saving-old-mice-button-glitching-debouncing/20181216-025214 base: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next reproduce: make htmldocs All warnings (new ones prefixed by >>): WARNING: convert(1) not found, for SVG to PDF conversion install ImageMagick (https://www.imagemagick.org) include/net/cfg80211.h:4216: warning: Function parameter or member 'wext.ibss' not described in 'wireless_dev' include/net/cfg80211.h:4216: warning: Function parameter or member 'wext.connect' not described in 'wireless_dev' include/net/cfg80211.h:4216: warning: Function parameter or member 'wext.keys' not described in 'wireless_dev' include/net/cfg80211.h:4216: warning: Function parameter or member 'wext.ie' not described in 'wireless_dev' include/net/cfg80211.h:4216: warning: Function parameter or member 'wext.ie_len' not described in 'wireless_dev' include/net/cfg80211.h:4216: warning: Function parameter or member 'wext.bssid' not described in 'wireless_dev' include/net/cfg80211.h:4216: warning: Function parameter or member 'wext.ssid' not described in 'wireless_dev' include/net/cfg80211.h:4216: warning: Function parameter or member 'wext.default_key' not described in 'wireless_dev' include/net/cfg80211.h:4216: warning: Function parameter or member 'wext.default_mgmt_key' not described in 'wireless_dev' include/net/cfg80211.h:4216: warning: Function parameter or member 'wext.prev_bssid_valid' not described in 'wireless_dev' include/net/mac80211.h:2282: warning: Function parameter or member 'radiotap_timestamp.units_pos' not described in 'ieee80211_hw' include/net/mac80211.h:2282: warning: Function parameter or member 'radiotap_timestamp.accuracy' not described in 'ieee80211_hw' include/net/mac80211.h:955: warning: Function parameter or member 'control.rates' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'control.rts_cts_rate_idx' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'control.use_rts' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'control.use_cts_prot' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'control.short_preamble' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'control.skip_table' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'control.jiffies' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'control.vif' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'control.hw_key' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'control.flags' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'control.enqueue_time' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'ack' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'ack.cookie' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'status.rates' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'status.ack_signal' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'status.ampdu_ack_len' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'status.ampdu_len' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'status.antenna' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'status.tx_time' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'status.is_valid_ack_signal' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'status.status_driver_data' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'driver_rates' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'pad' not described in 'ieee80211_tx_info' include/net/mac80211.h:95
Re: [rfd] saving old mice -- button glitching/debouncing
On Sat 2018-12-15 11:12:21, Vojtech Pavlik wrote: > On Sat, Dec 15, 2018 at 10:47:22AM +0100, Pavel Machek wrote: > > > > > b) would it be acceptable if done properly? (cmd line option to > > > > enable, avoiding duplicate/wrong events?) > > > > > > Well, for one, you shouldn't be using a timer, all the debouncing can be > > > done by math on the event timestamps. > > > > Not... really, right? You need to send an release some time after > > button indicates release if bounce did not happen. It is similar to > > autorepeat needing a timer. > > You can send the first release and ignore all presses and releases in a > time window after that. I could, and it would work for glitches on release, but that would do very bad things if switch glitched on press, right? " XX " -> " " ... ok "XX X..." -> "XX ..." ... bad idea. [Maybe could be detected with "XX " being unusually short?] But I believe I see "XX XXX" on X220 device, too. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: Digital signature
Re: [rfd] saving old mice -- button glitching/debouncing
On Sat, Dec 15, 2018 at 10:47:22AM +0100, Pavel Machek wrote: > > > b) would it be acceptable if done properly? (cmd line option to > > > enable, avoiding duplicate/wrong events?) > > > > Well, for one, you shouldn't be using a timer, all the debouncing can be > > done by math on the event timestamps. > > Not... really, right? You need to send an release some time after > button indicates release if bounce did not happen. It is similar to > autorepeat needing a timer. You can send the first release and ignore all presses and releases in a time window after that. > Let me gain some experience with the patch. I don't think hardware > does as heavy debouncing as you describe. Microswitches vibrate significantly when clicked. Without hardware debouncing, you'd have many clicks instead of one even on a new mouse. -- Vojtech Pavlik
Re: [rfd] saving old mice -- button glitching/debouncing
Hi! > > Patch is obviously not ready; but: > > > > a) would it be useful to people > > Probably not. > > Mice already do internal software/hardware debouncing on switches. If you > see duplicate clicks making it all the way to the kernel driver, something > is very wrong with the switch, to the point where it'll soon fail > completely. It seems mice normally survive 2 years under my use. This one has left button repaired and middle button failing... If debouncing gives it one more year, it will be success... > > b) would it be acceptable if done properly? (cmd line option to > > enable, avoiding duplicate/wrong events?) > > Well, for one, you shouldn't be using a timer, all the debouncing can be > done by math on the event timestamps. Not... really, right? You need to send an release some time after button indicates release if bounce did not happen. It is similar to autorepeat needing a timer. Let me gain some experience with the patch. I don't think hardware does as heavy debouncing as you describe. Best regards, Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: Digital signature
Re: [rfd] saving old mice -- button glitching/debouncing
On Sat, Dec 15, 2018 at 12:24:37AM +0100, Pavel Machek wrote: > Patch is obviously not ready; but: > > a) would it be useful to people Probably not. Mice already do internal software/hardware debouncing on switches. If you see duplicate clicks making it all the way to the kernel driver, something is very wrong with the switch, to the point where it'll soon fail completely. Hence the value of doing extra processing in the kernel is very limited. > b) would it be acceptable if done properly? (cmd line option to > enable, avoiding duplicate/wrong events?) Well, for one, you shouldn't be using a timer, all the debouncing can be done by math on the event timestamps. You'd also have a hard time distinguishing between doubleclicks and bounces. Since the mice already filter the quick bounces and there is significant delay (100ms on USB, similar on PS/2) between updates from the hardware, a real doubleclick can look absolutely the same as what you observe as a bounce. As such, it couldn't be enabled by default. If you really want to go this way, a userspace solution is the better choice. -- Vojtech Pavlik
Re: [rfd] saving old mice -- button glitching/debouncing
Hi Pavel, On Fri, Dec 14, 2018 at 3:24 PM Pavel Machek wrote: > > > I believe I have hardware problem, but I'm kind of hoping software > could help me...? > > Mouse wheel on my machine started glitching on my machine, generating > double-clicks when I click it once. Which unfortunately is quite > annoying: texts are pasted twice, two tabs are closed instead of one, > > > Event: time 1544733054.903129, type 4 (EV_MSC), code 4 (MSC_SCAN), value 90003 > Event: time 1544733054.903129, type 1 (EV_KEY), code 274 (BTN_MIDDLE), value 1 > Event: time 1544733054.903129, -- EV_SYN > 1544733054.967251, type 4 (EV_MSC), code 4 (MSC_SCAN), value 90003 > Event: time 1544733054.967251, type 1 (EV_KEY), code 274 (BTN_MIDDLE), value 0 > Event: time 1544733054.967251, -- EV_SYN > Event: time 1544733054.975144, type 4 (EV_MSC), code 4 (MSC_SCAN), value 90003 > Event: time 1544733054.975144, type 1 (EV_KEY), code 274 (BTN_MIDDLE), value 1 > Event: time 1544733054.975144, -- EV_SYN > : time 1544733065.127190, type 4 (EV_MSC), code 4 (MSC_SCAN), value 90003 > Event: time 1544733065.127190, type 1 (EV_KEY), code 274 (BTN_MIDDLE), value 0 > Event: time 1544733065.127190, -- EV_SYN > > Now, I could just buy a new mouse, but it seems that most optical mice > die like this... so maybe it would be nice to have an option to > debounce the buttons, so that the useful life of mice is extended a > bit? > > (So... I have two mice with that fault -- cheap to replace, but button > in thinkpad X220 started doing that, too. That one will not be so > cheap to fix :-( ). > > It is possible that some X versions already do something like this. > > Patch is obviously not ready; but: > > a) would it be useful to people > > b) would it be acceptable if done properly? (cmd line option to > enable, avoiding duplicate/wrong events?) I'd say if you are attached to failing hardware, solve it in userspace. Have a utility/daemon that you run (from udev?) that: - "grabs" input device with EVIOCGRAB - does the debouncing/filtering/adjusting for the dirty sensor - reinject events back into kernel with /dev/uinput It will add some latency, but should be workable. Thanks. -- Dmitry
[rfd] saving old mice -- button glitching/debouncing
I believe I have hardware problem, but I'm kind of hoping software
could help me...?
Mouse wheel on my machine started glitching on my machine, generating
double-clicks when I click it once. Which unfortunately is quite
annoying: texts are pasted twice, two tabs are closed instead of one,
Event: time 1544733054.903129, type 4 (EV_MSC), code 4 (MSC_SCAN), value 90003
Event: time 1544733054.903129, type 1 (EV_KEY), code 274 (BTN_MIDDLE), value 1
Event: time 1544733054.903129, -- EV_SYN
1544733054.967251, type 4 (EV_MSC), code 4 (MSC_SCAN), value 90003
Event: time 1544733054.967251, type 1 (EV_KEY), code 274 (BTN_MIDDLE), value 0
Event: time 1544733054.967251, -- EV_SYN
Event: time 1544733054.975144, type 4 (EV_MSC), code 4 (MSC_SCAN), value 90003
Event: time 1544733054.975144, type 1 (EV_KEY), code 274 (BTN_MIDDLE), value 1
Event: time 1544733054.975144, -- EV_SYN
: time 1544733065.127190, type 4 (EV_MSC), code 4 (MSC_SCAN), value 90003
Event: time 1544733065.127190, type 1 (EV_KEY), code 274 (BTN_MIDDLE), value 0
Event: time 1544733065.127190, -- EV_SYN
Now, I could just buy a new mouse, but it seems that most optical mice
die like this... so maybe it would be nice to have an option to
debounce the buttons, so that the useful life of mice is extended a
bit?
(So... I have two mice with that fault -- cheap to replace, but button
in thinkpad X220 started doing that, too. That one will not be so
cheap to fix :-( ).
It is possible that some X versions already do something like this.
Patch is obviously not ready; but:
a) would it be useful to people
b) would it be acceptable if done properly? (cmd line option to
enable, avoiding duplicate/wrong events?)
Thanks,
Pavel
Signed-off-by: Pavel Machek
diff --git a/drivers/input/input.c b/drivers/input/input.c
index 3304aaa..ce0d081 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -40,6 +40,11 @@ static DEFINE_IDA(input_ida);
static LIST_HEAD(input_dev_list);
static LIST_HEAD(input_handler_list);
+static void input_repeat_key(struct timer_list *t);
+static void input_debounce_key(struct timer_list *t);
+
+static int debounce = 20;
+
/*
* input_mutex protects access to both input_dev_list and input_handler_list.
* This also causes input_[un]register_device and input_[un]register_handler
@@ -77,6 +82,7 @@ static void input_start_autorepeat(struct input_dev *dev, int
code)
if (test_bit(EV_REP, dev->evbit) &&
dev->rep[REP_PERIOD] && dev->rep[REP_DELAY] &&
dev->timer.function) {
+ dev->timer.function = input_repeat_key;
dev->repeat_key = code;
mod_timer(&dev->timer,
jiffies + msecs_to_jiffies(dev->rep[REP_DELAY]));
@@ -88,18 +94,42 @@ static void input_stop_autorepeat(struct input_dev *dev)
del_timer(&dev->timer);
}
+static void input_start_debounce(struct input_dev *dev, int code)
+{
+ dev->timer.function = input_debounce_key;
+ dev->debounce_key = code;
+ mod_timer(&dev->timer,
+ jiffies + msecs_to_jiffies(debounce));
+}
+
+static void input_stop_debounce(struct input_dev *dev)
+{
+ del_timer(&dev->timer);
+ dev->debounce_key = -1;
+}
+
/*
* Pass event first through all filters and then, if event has not been
* filtered out, through all open handles. This function is called with
* dev->event_lock held and interrupts disabled.
*/
-static unsigned int input_to_handler(struct input_handle *handle,
+static unsigned int input_to_handler(struct input_dev *dev, struct
input_handle *handle,
struct input_value *vals, unsigned int count)
{
struct input_handler *handler = handle->handler;
struct input_value *end = vals;
struct input_value *v;
+ if (!test_bit(EV_REP, dev->evbit) && test_bit(EV_KEY, dev->evbit) &&
debounce)
+ for (v = vals; v != vals + count; v++) {
+ if (v->type == EV_KEY && v->value == 0 &&
dev->debounce_key == -1) {
+ input_start_debounce(dev, v->code);
+ v->code = -2;
+ }
+ if (v->type == EV_KEY && v->value == 1 &&
dev->debounce_key == v->code)
+ input_stop_debounce(dev);
+ }
+
if (handler->filter) {
for (v = vals; v != vals + count; v++) {
if (handler->filter(handle, v->type, v->code, v->value))
@@ -117,8 +147,9 @@ static unsigned int input_to_handler(struct input_handle
*handle,
if (handler->events)
handler->events(handle, vals, count);
else if (handler->event)
- for (v = vals; v != vals + count; v++)
+ for (v = vals; v != vals +
