Userspace might want to implement a policy to temporarily disregard input
from certain devices.
An example use case is a convertible laptop, whose keyboard can be folded
under the screen to create tablet-like experience. The user then must hold
the laptop in such a way that it is difficult to avoid pressing the keyboard
keys. It is therefore desirable to temporarily disregard input from the
keyboard, until it is folded back. This obviously is a policy which should
be kept out of the kernel, but the kernel must provide suitable means to
implement such a policy.
Due to interactions with suspend/resume, a helper has been added for drivers
to decide if the device is being used or not (PATCH 1/7) and it has been
applied to relevant drivers (PATCH 2,4,5,6/7).
PATCH 7/7 adds support for inhibiting input devices.
This work is inspired by:
https://chromium.googlesource.com/chromiumos/third_party/kernel/+/45c2d7bb398f74adfae0017e20b224152fde3822
and
https://chromium.googlesource.com/chromiumos/third_party/kernel/+/4ce0e8a3697edb8fd071110b3af65014512061c7
In this respin the elan_i2c patch is dropped and converting it will be
addressed later.
v2..v3:
- ignored autorepeat events in input_get_disposition() if a key is not
pressed (Hans)
- dropped inhibit()/uninhibit() driver callbacks (Hans)
- split ACPI button patch into taking the lock and using the helper (Rafael)
- dropped the elan_i2c conversion
- fixed typos in exynos adc
v1..v2:
- added input_device_enabled() helper and used it in drivers (Dmitry)
- the fact of open() and close() being called in inhibit/uninhibit paths has
been emphasized in the commit message of PATCH 6/7 (Dmitry)
Andrzej Pietrasiewicz (6):
Input: add input_device_enabled()
Input: use input_device_enabled()
ACPI: button: Access input device's users under appropriate mutex
ACPI: button: Use input_device_enabled() helper
iio: adc: exynos: Use input_device_enabled()
platform/x86: thinkpad_acpi: Use input_device_enabled()
Patrik Fimml (1):
Input: Add "inhibited" property
drivers/acpi/button.c | 7 +-
drivers/iio/adc/exynos_adc.c | 11 +-
drivers/input/input.c | 121 +++++++++++++++++++-
drivers/input/joystick/xpad.c | 4 +-
drivers/input/keyboard/ep93xx_keypad.c | 2 +-
drivers/input/keyboard/gpio_keys.c | 4 +-
drivers/input/keyboard/imx_keypad.c | 4 +-
drivers/input/keyboard/ipaq-micro-keys.c | 2 +-
drivers/input/keyboard/lpc32xx-keys.c | 4 +-
drivers/input/keyboard/pmic8xxx-keypad.c | 4 +-
drivers/input/keyboard/pxa27x_keypad.c | 2 +-
drivers/input/keyboard/samsung-keypad.c | 4 +-
drivers/input/keyboard/spear-keyboard.c | 8 +-
drivers/input/keyboard/st-keyscan.c | 4 +-
drivers/input/keyboard/tegra-kbc.c | 4 +-
drivers/input/misc/drv260x.c | 4 +-
drivers/input/misc/drv2665.c | 4 +-
drivers/input/misc/drv2667.c | 4 +-
drivers/input/misc/gp2ap002a00f.c | 4 +-
drivers/input/misc/kxtj9.c | 4 +-
drivers/input/misc/sirfsoc-onkey.c | 2 +-
drivers/input/mouse/navpoint.c | 4 +-
drivers/input/touchscreen/ad7879.c | 6 +-
drivers/input/touchscreen/atmel_mxt_ts.c | 4 +-
drivers/input/touchscreen/auo-pixcir-ts.c | 8 +-
drivers/input/touchscreen/bu21029_ts.c | 4 +-
drivers/input/touchscreen/chipone_icn8318.c | 4 +-
drivers/input/touchscreen/cyttsp_core.c | 4 +-
drivers/input/touchscreen/eeti_ts.c | 4 +-
drivers/input/touchscreen/ektf2127.c | 4 +-
drivers/input/touchscreen/imx6ul_tsc.c | 4 +-
drivers/input/touchscreen/ipaq-micro-ts.c | 2 +-
drivers/input/touchscreen/iqs5xx.c | 4 +-
drivers/input/touchscreen/lpc32xx_ts.c | 4 +-
drivers/input/touchscreen/melfas_mip4.c | 4 +-
drivers/input/touchscreen/mms114.c | 6 +-
drivers/input/touchscreen/pixcir_i2c_ts.c | 8 +-
drivers/input/touchscreen/ucb1400_ts.c | 4 +-
drivers/input/touchscreen/wm97xx-core.c | 14 ++-
drivers/input/touchscreen/zforce_ts.c | 8 +-
drivers/platform/x86/thinkpad_acpi.c | 4 +-
include/linux/input.h | 14 ++-
42 files changed, 230 insertions(+), 95 deletions(-)
base-commit: 3d77e6a8804abcc0504c904bd6e5cdf3a5cf8162
--
2.17.1
A new helper is available, so use it.
Signed-off-by: Andrzej Pietrasiewicz <[email protected]>
---
drivers/acpi/button.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/acpi/button.c b/drivers/acpi/button.c
index ff7ab291f678..4deb2b48d03c 100644
--- a/drivers/acpi/button.c
+++ b/drivers/acpi/button.c
@@ -411,7 +411,7 @@ static void acpi_button_notify(struct acpi_device *device, u32 event)
input = button->input;
if (button->type == ACPI_BUTTON_TYPE_LID) {
mutex_lock(&button->input->mutex);
- users = button->input->users;
+ users = input_device_enabled(button->input);
mutex_unlock(&button->input->mutex);
if (users)
acpi_lid_update_state(device, true);
@@ -460,7 +460,7 @@ static int acpi_button_resume(struct device *dev)
button->suspended = false;
mutex_lock(&input->mutex);
- if (button->type == ACPI_BUTTON_TYPE_LID && input->users) {
+ if (button->type == ACPI_BUTTON_TYPE_LID && input_device_enabled(input)) {
button->last_state = !!acpi_lid_evaluate_state(device);
button->last_time = ktime_get();
acpi_lid_initialize_state(device);
--
2.17.1
Use the new helper. Inspecting input device's 'users' member needs to be
done under device's mutex, so add appropriate invocations.
Signed-off-by: Andrzej Pietrasiewicz <[email protected]>
Acked-by: Henrique de Moraes Holschuh <[email protected]>
---
drivers/platform/x86/thinkpad_acpi.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index 0f704484ae1d..8ae11b8c3ebb 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -2671,9 +2671,10 @@ static void hotkey_poll_setup(const bool may_warn)
const u32 poll_driver_mask = hotkey_driver_mask & hotkey_source_mask;
const u32 poll_user_mask = hotkey_user_mask & hotkey_source_mask;
+ mutex_lock(&tpacpi_inputdev->mutex);
if (hotkey_poll_freq > 0 &&
(poll_driver_mask ||
- (poll_user_mask && tpacpi_inputdev->users > 0))) {
+ (poll_user_mask && input_device_enabled(tpacpi_inputdev)))) {
if (!tpacpi_hotkey_task) {
tpacpi_hotkey_task = kthread_run(hotkey_kthread,
NULL, TPACPI_NVRAM_KTHREAD_NAME);
@@ -2690,6 +2691,7 @@ static void hotkey_poll_setup(const bool may_warn)
poll_user_mask, poll_driver_mask);
}
}
+ mutex_unlock(&tpacpi_inputdev->mutex);
}
static void hotkey_poll_setup_safe(const bool may_warn)
--
2.17.1
Inspecting input device's 'users' member should be done under device's
mutex, so add appropriate invocations.
Signed-off-by: Andrzej Pietrasiewicz <[email protected]>
---
drivers/acpi/button.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/acpi/button.c b/drivers/acpi/button.c
index 78cfc70cb320..ff7ab291f678 100644
--- a/drivers/acpi/button.c
+++ b/drivers/acpi/button.c
@@ -456,13 +456,16 @@ static int acpi_button_resume(struct device *dev)
{
struct acpi_device *device = to_acpi_device(dev);
struct acpi_button *button = acpi_driver_data(device);
+ struct input_dev *input = button->input;
button->suspended = false;
- if (button->type == ACPI_BUTTON_TYPE_LID && button->input->users) {
+ mutex_lock(&input->mutex);
+ if (button->type == ACPI_BUTTON_TYPE_LID && input->users) {
button->last_state = !!acpi_lid_evaluate_state(device);
button->last_time = ktime_get();
acpi_lid_initialize_state(device);
}
+ mutex_unlock(&input->mutex);
return 0;
}
#endif
--
2.17.1
A new helper is available, so use it. Inspecting 'users' member of
input_dev requires taking device's mutex.
Signed-off-by: Andrzej Pietrasiewicz <[email protected]>
---
drivers/iio/adc/exynos_adc.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/iio/adc/exynos_adc.c b/drivers/iio/adc/exynos_adc.c
index 22131a677445..294715bafe25 100644
--- a/drivers/iio/adc/exynos_adc.c
+++ b/drivers/iio/adc/exynos_adc.c
@@ -630,10 +630,13 @@ static irqreturn_t exynos_ts_isr(int irq, void *dev_id)
struct exynos_adc *info = dev_id;
struct iio_dev *dev = dev_get_drvdata(info->dev);
u32 x, y;
- bool pressed;
+ bool pressed, cont;
int ret;
- while (info->input->users) {
+ mutex_lock(&info->input->mutex);
+ cont = input_device_enabled(info->input);
+ mutex_unlock(&info->input->mutex);
+ while (cont) {
ret = exynos_read_s3c64xx_ts(dev, &x, &y);
if (ret == -ETIMEDOUT)
break;
@@ -651,6 +654,10 @@ static irqreturn_t exynos_ts_isr(int irq, void *dev_id)
input_sync(info->input);
usleep_range(1000, 1100);
+
+ mutex_lock(&info->input->mutex);
+ cont = input_device_enabled(info->input);
+ mutex_unlock(&info->input->mutex);
}
writel(0, ADC_V1_CLRINTPNDNUP(info->regs));
--
2.17.1
On Fri, Jun 05, 2020 at 07:33:33PM +0200, Andrzej Pietrasiewicz wrote:
> A new helper is available, so use it. Inspecting 'users' member of
> input_dev requires taking device's mutex.
>
> Signed-off-by: Andrzej Pietrasiewicz <[email protected]>
> ---
> drivers/iio/adc/exynos_adc.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iio/adc/exynos_adc.c b/drivers/iio/adc/exynos_adc.c
> index 22131a677445..294715bafe25 100644
> --- a/drivers/iio/adc/exynos_adc.c
> +++ b/drivers/iio/adc/exynos_adc.c
> @@ -630,10 +630,13 @@ static irqreturn_t exynos_ts_isr(int irq, void *dev_id)
> struct exynos_adc *info = dev_id;
> struct iio_dev *dev = dev_get_drvdata(info->dev);
> u32 x, y;
> - bool pressed;
> + bool pressed, cont;
> int ret;
>
> - while (info->input->users) {
> + mutex_lock(&info->input->mutex);
> + cont = input_device_enabled(info->input);
> + mutex_unlock(&info->input->mutex);
> + while (cont) {
> ret = exynos_read_s3c64xx_ts(dev, &x, &y);
> if (ret == -ETIMEDOUT)
> break;
> @@ -651,6 +654,10 @@ static irqreturn_t exynos_ts_isr(int irq, void *dev_id)
> input_sync(info->input);
>
> usleep_range(1000, 1100);
> +
> + mutex_lock(&info->input->mutex);
> + cont = input_device_enabled(info->input);
> + mutex_unlock(&info->input->mutex);
> }
The mutex doesn't really protect anything here, but I would nevertheless
suggest this sequence instead:
lock()
while (test) {
unlock()
...
lock()
}
unlock()
Best Regards,
Micha? Miros?aw
On Fri 2020-06-05 19:33:28, Andrzej Pietrasiewicz wrote:
> Userspace might want to implement a policy to temporarily disregard input
> from certain devices.
Wow, you certainly cc a lot of lists.
> An example use case is a convertible laptop, whose keyboard can be folded
> under the screen to create tablet-like experience. The user then must hold
> the laptop in such a way that it is difficult to avoid pressing the keyboard
> keys. It is therefore desirable to temporarily disregard input from the
> keyboard, until it is folded back. This obviously is a policy which should
> be kept out of the kernel, but the kernel must provide suitable means to
> implement such a policy.
>
> Due to interactions with suspend/resume, a helper has been added for drivers
> to decide if the device is being used or not (PATCH 1/7) and it has been
> applied to relevant drivers (PATCH 2,4,5,6/7).
But is that a right way to implement it?
We want this for cellphones, too -- touchscreen should be disabled
while the device is locked in the pocket -- but we really want the
touchscreen hardware to be powered down in that case (because it keeps
SoC busy and eats a _lot_ of electricity).
But simplistic "receive an event and then drop it if device is
inhibited" does not allow that...
Best regards,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
On Sun, Jun 07, 2020 at 10:24:14PM +0200, Pavel Machek wrote:
> On Fri 2020-06-05 19:33:28, Andrzej Pietrasiewicz wrote:
> > Userspace might want to implement a policy to temporarily disregard input
> > from certain devices.
>
> Wow, you certainly cc a lot of lists.
>
> > An example use case is a convertible laptop, whose keyboard can be folded
> > under the screen to create tablet-like experience. The user then must hold
> > the laptop in such a way that it is difficult to avoid pressing the keyboard
> > keys. It is therefore desirable to temporarily disregard input from the
> > keyboard, until it is folded back. This obviously is a policy which should
> > be kept out of the kernel, but the kernel must provide suitable means to
> > implement such a policy.
> >
> > Due to interactions with suspend/resume, a helper has been added for drivers
> > to decide if the device is being used or not (PATCH 1/7) and it has been
> > applied to relevant drivers (PATCH 2,4,5,6/7).
>
> But is that a right way to implement it?
>
> We want this for cellphones, too -- touchscreen should be disabled
> while the device is locked in the pocket -- but we really want the
> touchscreen hardware to be powered down in that case (because it keeps
> SoC busy and eats a _lot_ of electricity).
>
> But simplistic "receive an event and then drop it if device is
> inhibited" does not allow that...
I do not think you read the entirety of this patch series...
Thanks.
--
Dmitry
Hi Pavel,
W dniu 08.06.2020 o 07:37, Dmitry Torokhov pisze:
> On Sun, Jun 07, 2020 at 10:24:14PM +0200, Pavel Machek wrote:
>> On Fri 2020-06-05 19:33:28, Andrzej Pietrasiewicz wrote:
>>> Userspace might want to implement a policy to temporarily disregard input
>>> from certain devices.
>>
>> Wow, you certainly cc a lot of lists.
>>
>>> An example use case is a convertible laptop, whose keyboard can be folded
>>> under the screen to create tablet-like experience. The user then must hold
>>> the laptop in such a way that it is difficult to avoid pressing the keyboard
>>> keys. It is therefore desirable to temporarily disregard input from the
>>> keyboard, until it is folded back. This obviously is a policy which should
>>> be kept out of the kernel, but the kernel must provide suitable means to
>>> implement such a policy.
>>>
>>> Due to interactions with suspend/resume, a helper has been added for drivers
>>> to decide if the device is being used or not (PATCH 1/7) and it has been
>>> applied to relevant drivers (PATCH 2,4,5,6/7).
>>
>> But is that a right way to implement it?
>>
>> We want this for cellphones, too -- touchscreen should be disabled
>> while the device is locked in the pocket -- but we really want the
>> touchscreen hardware to be powered down in that case (because it keeps
>> SoC busy and eats a _lot_ of electricity).
>>
>> But simplistic "receive an event and then drop it if device is
>> inhibited" does not allow that...
>
> I do not think you read the entirety of this patch series...
>
Yeah, kindly read the whole thread. Long story short: Inhibiting _is_ about
ignoring events from inhibited devices. Obviously we can do better than
just that. Indeed, the open() and close() callbacks (which are called at
uninhibiting/inhibiting) mean "start providing events" and "stop providing
events", respectively. How that translates into driver operation is highly
driver-specific and cannot be handled at the input subsystem level, but it
is the place where power savings can be realized: whenever the driver knows
that nobody wants events from it it can do whatever it considers appropriate,
including transitioning the device into low power mode, for example using
PM runtime.
Regards,
Andrzej