Thread (7 messages) 7 messages, 3 authors, 2020-12-26

Re: [PATCH v4] HID: i2c-hid: add polling mode based on connected GPIO chip's pin status

From: Greg KH <gregkh@linuxfoundation.org>
Date: 2020-11-25 15:07:32
Also in: lkml, stable

On Wed, Nov 25, 2020 at 10:10:22PM +0800, Coiby Xu wrote:
quoted hunk ↗ jump to hunk
For a broken touchpad, it may take several months or longer to be fixed.
Polling mode could be a fallback solution for enthusiastic Linux users
when they have a new laptop. It also acts like a debugging feature. If
polling mode works for a broken touchpad, we can almost be certain
the root cause is related to the interrupt or power setting.

This patch could fix touchpads of Lenovo AMD gaming laptops including
Legion-5 15ARH05 (R7000), Legion-5P (R7000P) and IdeaPad Gaming 3
15ARH05.

When polling mode is enabled, an I2C device can't wake up the suspended
system since enable/disable_irq_wake is invalid for polling mode.

Three module parameters are added to i2c-hid,
    - polling_mode: by default set to 0, i.e., polling is disabled
    - polling_interval_idle_ms: the polling internal when the touchpad
      is idle, default to 10ms
    - polling_interval_active_us: the polling internal when the touchpad
      is active, default to 4000us

User can change the last two runtime polling parameter by writing to
/sys/module/i2c_hid/parameters/polling_interval_{idle_ms,active_us}.

Note xf86-input-synaptics doesn't work well with this polling mode
for the Synaptics touchpad. The Synaptics touchpad would often locks
into scroll mode when using multitouch gestures [1]. One remedy is to
decrease the polling interval.

Thanks to Barnabás's thorough review of this patch and the useful
feedback!

[1] https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1887190/comments/235

Cc: <redacted>
Cc: Barnabás Pőcze <redacted>
BugLink: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1887190
Signed-off-by: Coiby Xu <redacted>
---
 drivers/hid/i2c-hid/i2c-hid-core.c | 152 +++++++++++++++++++++++++++--
 1 file changed, 142 insertions(+), 10 deletions(-)
diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
index aeff1ffb0c8b..f25503f31ccf 100644
--- a/drivers/hid/i2c-hid/i2c-hid-core.c
+++ b/drivers/hid/i2c-hid/i2c-hid-core.c
@@ -36,6 +36,8 @@
 #include <linux/hid.h>
 #include <linux/mutex.h>
 #include <linux/acpi.h>
+#include <linux/kthread.h>
+#include <linux/gpio/driver.h>
 #include <linux/of.h>
 #include <linux/regulator/consumer.h>
 
@@ -60,6 +62,25 @@
 #define I2C_HID_PWR_ON		0x00
 #define I2C_HID_PWR_SLEEP	0x01
 
+/* polling mode */
+#define I2C_HID_POLLING_DISABLED 0
+#define I2C_HID_POLLING_GPIO_PIN 1
+#define I2C_HID_POLLING_INTERVAL_ACTIVE_US 4000
+#define I2C_HID_POLLING_INTERVAL_IDLE_MS 10
+
+static u8 polling_mode;
+module_param(polling_mode, byte, 0444);
+MODULE_PARM_DESC(polling_mode, "How to poll (default=0) - 0 disabled; 1 based on GPIO pin's status");
Module parameters are for the 1990's, they are global and horrible to
try to work with.  You should provide something on a per-device basis,
as what happens if your system requires different things here for
different devices?  You set this for all devices :(

thanks,

greg k-h
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help