From: Andrzej Pietrasiewicz <andrzej.p@collabora.com> To: Dmitry Torokhov <dmitry.torokhov@gmail.com>, Peter Hutterer <peter.hutterer@who-t.net> Cc: Hans de Goede <hdegoede@redhat.com>, linux-input@vger.kernel.org, linux-acpi@vger.kernel.org, linux-iio@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-samsung-soc@vger.kernel.org, linux-tegra@vger.kernel.org, patches@opensource.cirrus.com, ibm-acpi-devel@lists.sourceforge.net, platform-driver-x86@vger.kernel.org, "Rafael J . Wysocki" <rjw@rjwysocki.net>, Len Brown <lenb@kernel.org>, Jonathan Cameron <jic23@kernel.org>, Hartmut Knaack <knaack.h@gmx.de>, Lars-Peter Clausen <lars@metafoo.de>, Peter Meerwald-Stadler <pmeerw@pmeerw.net>, Kukjin Kim <kgene@kernel.org>, Krzysztof Kozlowski <krzk@kernel.org>, Shawn Guo <shawnguo@kernel.org>, Sascha Hauer <s.hauer@pengutronix.de>, Pengutronix Kernel Team <kernel@pengutronix.de>, Fabio Estevam <festevam@gmail.com>, NXP Linux Team <linux-imx@nxp.com>, Vladimir Zapolskiy <vz@mleia.com>, Sylvain Lemieux <slemieux.tyco@gmail.com>, Laxman Dewangan <ldewangan@nvidia.com>, Thierry Reding <thierry.reding@gmail.com>, Jonathan Hunter <jonathanh@nvidia.com>, Barry Song <baohua@kernel.org>, Michael Hennerich <michael.hennerich@analog.com>, Nick Dyer <nick@shmanahar.org>, Ferruh Yigit <fery@cypress.com>, Sangwon Jee <jeesw@melfas.com>, Henrique de Moraes Holschuh <ibm-acpi@hmh.eng.br>, kernel@collabora.com, Peter Hutterer <peter.hutterer@redhat.com>, Benjamin Tissoires <btissoir@redhat.com> Subject: Re: [PATCHv2 0/7] Support inhibiting input devices Date: Fri, 22 May 2020 17:35:56 +0200 Message-ID: <513f25c0-7125-c564-0090-052d626fe508@collabora.com> (raw) In-Reply-To: <20200518024034.GL89269@dtor-ws> Hi Hans, hi Dmitry, W dniu 18.05.2020 o 04:40, Dmitry Torokhov pisze: > Hi Hans, Peter, > > On Mon, May 18, 2020 at 08:55:10AM +1000, Peter Hutterer wrote: >> On Fri, May 15, 2020 at 08:19:10PM +0200, Hans de Goede wrote: >>> Hi Andrezj, >>> <snip> >> >>> I also noticed that you keep the device open (do not call the >>> input_device's close callback) when inhibited and just throw away >>> any events generated. This seems inefficient and may lead to >>> the internal state getting out of sync. What if a key is pressed >>> while inhibited and then the device is uninhibited while the key >>> is still pressed? Now the press event is lost and userspace >>> querying the current state will see the pressed key as being >>> released. > > This is a good point. We should look into signalling that some events > have been dropped (via EV_SYN/SYN_DROPPED) so that clients are aware of > it. > It seems to me that the situation Hans envisions is not possible, or will not be possible with a simple change. Let me explain. For a start, let's recall that the input core prevents consecutive events of the same kind (type _and_ code _and_ value) from being delivered to handlers. The decision is made in input_get_disposition(). For EV_KEY it is: if (is_event_supported(code, dev->keybit, KEY_MAX)) { /* auto-repeat bypasses state updates */ if (value == 2) { disposition = INPUT_PASS_TO_HANDLERS; break; } if (!!test_bit(code, dev->key) != !!value) { __change_bit(code, dev->key); disposition = INPUT_PASS_TO_HANDLERS; } } Let's now focus on value != 2 (events other than auto-repeat). The disposition changes from the default INPUT_IGNORE_EVENT to INPUT_PASS_TO_HANDLERS only when the event in question changes the current state: either by releasing a pressed key, or by pressing a released key. Subsequent releases of a released key or subsequent presses of a pressed key will be ignored. What Hans points out is the possibility of uninhibiting a device while its key is pressed and then releasing the key. First of all, during inhibiting input_dev_release_keys() is called, so input_dev's internal state will be cleared of all pressed keys. Then the device - after being uninhibited - all of a sudden produces a key release event. It will be ignored as per the "subsequent releases of a released key" case, so the handlers will not be passed an unmatched key release event. Assuming that passing an unmatched key release event was Hans's concern, in this case it seems impossible. Now, the value of 2 (auto-repeat) needs some attention. There are two cases to consider: the device uses input core's software repeat or it uses its own (hardware) repeat. Let's consider the first case. The timer which generates auto-repeat is only started on a key press event and only stopped on a key release event. As such, if any auto-repeat was in progress when inhibiting happened, it must have been stopped as per input_dev_release_keys(). Then the key is pressed and held after the device has been inhibited, and the device is being uninhibited. Since it uses software auto-repeat, no events will be reported by the device until the key is released, and, as explained above, the release event will be ignored. Let's consider the second case. The key is pressed and held after the device has been inhibited and the device is being uninhibited. The worst thing that can happen is unmatched key repeat events will start coming from the device. We must prevent them from reaching the handlers and ignore them instead. So I suggest something on the lines of: if (is_event_supported(code, dev->keybit, KEY_MAX)) { /* auto-repeat bypasses state updates */ - if (value == 2) { + if (value == 2 && test_bit(code, dev->key)) { disposition = INPUT_PASS_TO_HANDLERS; break; } The intended meaning is "ignore key repeat events if the key is not pressed". With this small change I believe it is not possible to have neither unmatched release nor unmatched repeat being delivered to handlers. Regards, Andrzej
next prev parent reply index Thread overview: 90+ messages / expand[flat|nested] mbox.gz Atom feed top [not found] <20200506002746.GB89269@dtor-ws> 2020-05-15 16:49 ` Andrzej Pietrasiewicz 2020-05-15 16:51 ` [PATCHv2 1/7] Input: add input_device_enabled() Andrzej Pietrasiewicz 2020-05-15 16:52 ` [PATCHv2 4/7] iio: adc: exynos: Use input_device_enabled() Andrzej Pietrasiewicz 2020-05-15 16:52 ` [PATCHv2 6/7] Input: Add "inhibited" property Andrzej Pietrasiewicz 2020-05-15 18:19 ` [PATCHv2 0/7] Support inhibiting input devices Hans de Goede 2020-05-17 22:55 ` Peter Hutterer 2020-05-18 2:40 ` Dmitry Torokhov 2020-05-18 7:36 ` Hans de Goede 2020-05-22 15:35 ` Andrzej Pietrasiewicz [this message] 2020-05-27 6:13 ` Peter Hutterer 2020-05-18 10:48 ` Andrzej Pietrasiewicz 2020-05-18 12:24 ` Hans de Goede 2020-05-18 13:49 ` Andrzej Pietrasiewicz 2020-05-18 14:23 ` Hans de Goede 2020-05-19 9:02 ` Andrzej Pietrasiewicz 2020-05-19 9:36 ` Hans de Goede 2020-05-27 6:34 ` Dmitry Torokhov 2020-06-02 16:56 ` Andrzej Pietrasiewicz 2020-06-02 17:52 ` Dmitry Torokhov 2020-06-02 18:50 ` Andrzej Pietrasiewicz 2020-06-02 20:19 ` Hans de Goede 2020-06-03 13:07 ` Andrzej Pietrasiewicz 2020-06-03 17:38 ` Hans de Goede 2020-06-03 17:54 ` Andrzej Pietrasiewicz 2020-06-03 19:37 ` Hans de Goede 2020-06-04 7:28 ` Dmitry Torokhov 2020-06-05 17:33 ` [PATCH v3 " Andrzej Pietrasiewicz 2020-06-05 17:33 ` [PATCH v3 1/7] Input: add input_device_enabled() Andrzej Pietrasiewicz 2020-06-05 17:33 ` [PATCH v3 2/7] Input: use input_device_enabled() Andrzej Pietrasiewicz 2020-06-05 17:33 ` [PATCH v3 3/7] ACPI: button: Access input device's users under appropriate mutex Andrzej Pietrasiewicz 2020-06-05 17:33 ` [PATCH v3 4/7] ACPI: button: Use input_device_enabled() helper Andrzej Pietrasiewicz 2020-06-05 17:33 ` [PATCH v3 5/7] iio: adc: exynos: Use input_device_enabled() Andrzej Pietrasiewicz 2020-06-05 19:49 ` Michał Mirosław 2020-06-05 17:33 ` [PATCH v3 6/7] platform/x86: thinkpad_acpi: " Andrzej Pietrasiewicz 2020-06-05 17:33 ` [PATCH v3 7/7] Input: Add "inhibited" property Andrzej Pietrasiewicz 2020-06-05 17:41 ` Hans de Goede 2020-06-08 11:22 ` [PATCH v4 0/7] Support inhibiting input devices Andrzej Pietrasiewicz 2020-06-08 11:22 ` [PATCH v4 1/7] Input: add input_device_enabled() Andrzej Pietrasiewicz 2020-12-03 6:25 ` Dmitry Torokhov 2020-06-08 11:22 ` [PATCH v4 2/7] Input: use input_device_enabled() Andrzej Pietrasiewicz 2020-12-03 6:26 ` Dmitry Torokhov [not found] ` <CGME20201207133237eucas1p26f8484944760a14e51dc7353ed33cd28@eucas1p2.samsung.com> 2020-12-07 13:32 ` Marek Szyprowski 2020-12-07 15:50 ` Andrzej Pietrasiewicz 2020-12-08 10:05 ` Marek Szyprowski 2020-12-09 6:37 ` Dmitry Torokhov 2020-12-11 7:09 ` [PATCH] Input: cyapa - do not call input_device_enabled from power mode handler Dmitry Torokhov 2020-12-11 8:22 ` Marek Szyprowski 2020-12-11 8:31 ` Dmitry Torokhov 2020-06-08 11:22 ` [PATCH v4 3/7] ACPI: button: Access input device's users under appropriate mutex Andrzej Pietrasiewicz 2020-06-24 15:00 ` Rafael J. Wysocki 2020-06-25 5:23 ` Dmitry Torokhov 2020-06-25 10:55 ` Rafael J. Wysocki 2020-10-05 5:08 ` Dmitry Torokhov 2020-06-08 11:22 ` [PATCH v4 4/7] ACPI: button: Use input_device_enabled() helper Andrzej Pietrasiewicz 2020-06-25 5:24 ` Dmitry Torokhov 2020-10-05 5:06 ` Dmitry Torokhov 2020-06-08 11:22 ` [PATCH v4 5/7] iio: adc: exynos: Use input_device_enabled() Andrzej Pietrasiewicz 2020-06-10 1:28 ` Michał Mirosław 2020-06-10 7:52 ` [FIXED PATCH " Andrzej Pietrasiewicz 2020-06-08 11:22 ` [PATCH v4 6/7] platform/x86: thinkpad_acpi: " Andrzej Pietrasiewicz 2020-06-08 11:22 ` [PATCH v4 7/7] Input: Add "inhibited" property Andrzej Pietrasiewicz 2020-10-05 18:10 ` Dmitry Torokhov 2020-10-06 13:04 ` Andrzej Pietrasiewicz 2020-10-07 1:11 ` Dmitry Torokhov 2020-10-07 1:12 ` Dmitry Torokhov 2020-12-03 6:26 ` Dmitry Torokhov 2020-06-10 9:49 ` [PATCH v4 0/7] Support inhibiting input devices Hans de Goede 2020-06-10 10:38 ` Rafael J. Wysocki 2020-06-10 13:12 ` Andrzej Pietrasiewicz 2020-06-10 13:21 ` Hans de Goede 2020-06-10 13:41 ` Andrzej Pietrasiewicz 2020-06-12 8:30 ` Hans de Goede 2020-06-12 8:47 ` Andrzej Pietrasiewicz 2020-06-16 17:29 ` [PATCH] Input: document inhibiting Andrzej Pietrasiewicz 2020-06-16 17:38 ` Randy Dunlap 2020-06-17 7:44 ` Hans de Goede 2020-06-17 10:18 ` [PATCH v2] " Andrzej Pietrasiewicz 2020-06-17 10:21 ` Hans de Goede 2020-06-17 16:52 ` Randy Dunlap 2020-06-23 13:35 ` Pavel Machek 2020-12-03 6:27 ` Dmitry Torokhov 2020-06-10 14:01 ` [PATCH v4 0/7] Support inhibiting input devices Rafael J. Wysocki 2020-06-10 13:52 ` Hans de Goede 2020-06-10 18:28 ` Dmitry Torokhov 2020-06-12 8:14 ` Hans de Goede 2020-06-12 8:17 ` Hans de Goede 2020-08-03 14:40 ` Andrzej Pietrasiewicz 2020-06-07 20:24 ` [PATCH v3 " Pavel Machek 2020-06-08 5:37 ` Dmitry Torokhov 2020-06-08 9:28 ` Andrzej Pietrasiewicz
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=513f25c0-7125-c564-0090-052d626fe508@collabora.com \ --to=andrzej.p@collabora.com \ --cc=baohua@kernel.org \ --cc=btissoir@redhat.com \ --cc=dmitry.torokhov@gmail.com \ --cc=fery@cypress.com \ --cc=festevam@gmail.com \ --cc=hdegoede@redhat.com \ --cc=ibm-acpi-devel@lists.sourceforge.net \ --cc=ibm-acpi@hmh.eng.br \ --cc=jeesw@melfas.com \ --cc=jic23@kernel.org \ --cc=jonathanh@nvidia.com \ --cc=kernel@collabora.com \ --cc=kernel@pengutronix.de \ --cc=kgene@kernel.org \ --cc=knaack.h@gmx.de \ --cc=krzk@kernel.org \ --cc=lars@metafoo.de \ --cc=ldewangan@nvidia.com \ --cc=lenb@kernel.org \ --cc=linux-acpi@vger.kernel.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-iio@vger.kernel.org \ --cc=linux-imx@nxp.com \ --cc=linux-input@vger.kernel.org \ --cc=linux-samsung-soc@vger.kernel.org \ --cc=linux-tegra@vger.kernel.org \ --cc=michael.hennerich@analog.com \ --cc=nick@shmanahar.org \ --cc=patches@opensource.cirrus.com \ --cc=peter.hutterer@redhat.com \ --cc=peter.hutterer@who-t.net \ --cc=platform-driver-x86@vger.kernel.org \ --cc=pmeerw@pmeerw.net \ --cc=rjw@rjwysocki.net \ --cc=s.hauer@pengutronix.de \ --cc=shawnguo@kernel.org \ --cc=slemieux.tyco@gmail.com \ --cc=thierry.reding@gmail.com \ --cc=vz@mleia.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
Linux-IIO Archive on lore.kernel.org Archives are clonable: git clone --mirror https://lore.kernel.org/linux-iio/0 linux-iio/git/0.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 linux-iio linux-iio/ https://lore.kernel.org/linux-iio \ linux-iio@vger.kernel.org public-inbox-index linux-iio Example config snippet for mirrors Newsgroup available over NNTP: nntp://nntp.lore.kernel.org/org.kernel.vger.linux-iio AGPL code for this site: git clone https://public-inbox.org/public-inbox.git