From: Greg KH <gregkh@linuxfoundation.org>
To: Coiby Xu <coiby.xu@gmail.com>
Cc: linux-input@vger.kernel.org,
"Helmut Stult" <helmut.stult@schinfo.de>,
"Barnabás Pőcze" <pobrn@protonmail.com>,
"Baq Domalaq" <domalak@gmail.com>,
"Pedro Ribeiro" <pedrib@gmail.com>,
stable@vger.kernel.org, "Jiri Kosina" <jikos@kernel.org>,
"Benjamin Tissoires" <benjamin.tissoires@redhat.com>,
"open list" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v4] HID: i2c-hid: add polling mode based on connected GPIO chip's pin status
Date: Wed, 25 Nov 2020 16:07:27 +0100 [thread overview]
Message-ID: <X75zL12q+FF6KBHi@kroah.com> (raw)
In-Reply-To: <20201125141022.321643-1-coiby.xu@gmail.com>
On Wed, Nov 25, 2020 at 10:10:22PM +0800, Coiby Xu wrote:
> 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: <stable@vger.kernel.org>
> Cc: Barnabás Pőcze <pobrn@protonmail.com>
> BugLink: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1887190
> Signed-off-by: Coiby Xu <coiby.xu@gmail.com>
> ---
> 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
next prev parent reply other threads:[~2020-11-25 15:07 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-25 14:10 [PATCH v4] HID: i2c-hid: add polling mode based on connected GPIO chip's pin status Coiby Xu
2020-11-25 15:07 ` Greg KH [this message]
2020-12-08 21:59 ` Barnabás Pőcze
2020-12-09 7:00 ` Greg KH
2020-12-09 15:38 ` Barnabás Pőcze
2020-12-09 15:44 ` Greg KH
2020-12-26 4:29 ` Coiby Xu
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=X75zL12q+FF6KBHi@kroah.com \
--to=gregkh@linuxfoundation.org \
--cc=benjamin.tissoires@redhat.com \
--cc=coiby.xu@gmail.com \
--cc=domalak@gmail.com \
--cc=helmut.stult@schinfo.de \
--cc=jikos@kernel.org \
--cc=linux-input@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=pedrib@gmail.com \
--cc=pobrn@protonmail.com \
--cc=stable@vger.kernel.org \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).