linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Coiby Xu <coiby.xu@gmail.com>
To: linux-input@vger.kernel.org
Cc: "Helmut Stult" <helmut.stult@schinfo.de>,
	"Barnabás Pőcze" <pobrn@protonmail.com>,
	stable@vger.kernel.org, "Jiri Kosina" <jikos@kernel.org>,
	"Benjamin Tissoires" <benjamin.tissoires@redhat.com>,
	linux-kernel@vger.kernel.org (open list)
Subject: [PATCH v3] HID: i2c-hid: add polling mode based on connected GPIO chip's pin status
Date: Wed, 21 Oct 2020 21:49:31 +0800	[thread overview]
Message-ID: <20201021134931.462560-1-coiby.xu@gmail.com> (raw)

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.

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}.

Cc: <stable@vger.kernel.org>
Link: 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 | 145 +++++++++++++++++++++++++++--
 1 file changed, 135 insertions(+), 10 deletions(-)

diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
index 786e3e9af1c9..e44237562068 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 - 0 disabled; 1 based on GPIO pin's status");
+
+static unsigned int polling_interval_active_us = I2C_HID_POLLING_INTERVAL_ACTIVE_US;
+module_param(polling_interval_active_us, uint, 0644);
+MODULE_PARM_DESC(polling_interval_active_us,
+		 "Poll every {polling_interval_active_us} us when the touchpad is active. Default to 4000 us");
+
+static unsigned int polling_interval_idle_ms = I2C_HID_POLLING_INTERVAL_IDLE_MS;
+module_param(polling_interval_idle_ms, uint, 0644);
+MODULE_PARM_DESC(polling_interval_idle_ms,
+		 "Poll every {polling_interval_idle_ms} ms when the touchpad is idle. Default to 10 ms");
 /* debug option */
 static bool debug;
 module_param(debug, bool, 0444);
@@ -158,6 +179,8 @@ struct i2c_hid {

 	struct i2c_hid_platform_data pdata;

+	struct task_struct *polling_thread;
+
 	bool			irq_wake_enabled;
 	struct mutex		reset_lock;
 };
@@ -772,7 +795,9 @@ static int i2c_hid_start(struct hid_device *hid)
 		i2c_hid_free_buffers(ihid);

 		ret = i2c_hid_alloc_buffers(ihid, bufsize);
-		enable_irq(client->irq);
+
+		if (polling_mode == I2C_HID_POLLING_DISABLED)
+			enable_irq(client->irq);

 		if (ret)
 			return ret;
@@ -814,6 +839,91 @@ struct hid_ll_driver i2c_hid_ll_driver = {
 };
 EXPORT_SYMBOL_GPL(i2c_hid_ll_driver);

+static int get_gpio_pin_state(struct irq_desc *irq_desc)
+{
+	struct gpio_chip *gc = irq_data_get_irq_chip_data(&irq_desc->irq_data);
+
+	return gc->get(gc, irq_desc->irq_data.hwirq);
+}
+
+static bool interrupt_line_active(struct i2c_client *client)
+{
+	unsigned long trigger_type = irq_get_trigger_type(client->irq);
+	struct irq_desc *irq_desc = irq_to_desc(client->irq);
+	ssize_t	status = get_gpio_pin_state(irq_desc);
+
+	if (status < 0) {
+		dev_warn(&client->dev,
+			 "Failed to get GPIO Interrupt line status for %s",
+			 client->name);
+		return false;
+	}
+	/*
+	 * According to Windows Precsiontion Touchpad's specs
+	 * https://docs.microsoft.com/en-us/windows-hardware/design/component-guidelines/windows-precision-touchpad-device-bus-connectivity,
+	 * GPIO Interrupt Assertion Leve could be either ActiveLow or
+	 * ActiveHigh.
+	 */
+	if (trigger_type & IRQF_TRIGGER_LOW)
+		return !status;
+
+	return status;
+}
+
+static int i2c_hid_polling_thread(void *i2c_hid)
+{
+	struct i2c_hid *ihid = i2c_hid;
+	struct i2c_client *client = ihid->client;
+	unsigned int polling_interval_idle;
+
+	while (1) {
+		if (kthread_should_stop())
+			break;
+
+		while (interrupt_line_active(client) &&
+		       !test_bit(I2C_HID_READ_PENDING, &ihid->flags) &&
+		       !kthread_should_stop()) {
+			i2c_hid_get_input(ihid);
+			usleep_range(polling_interval_active_us,
+				     polling_interval_active_us + 100);
+		}
+		/*
+		 * re-calculate polling_interval_idle
+		 * so the module parameters polling_interval_idle_ms can be
+		 * changed dynamically through sysfs as polling_interval_active_us
+		 */
+		polling_interval_idle = polling_interval_idle_ms * 1000;
+		usleep_range(polling_interval_idle,
+			     polling_interval_idle + 1000);
+	}
+
+	do_exit(0);
+	return 0;
+}
+
+static int i2c_hid_init_polling(struct i2c_hid *ihid)
+{
+	struct i2c_client *client = ihid->client;
+
+	if (!irq_get_trigger_type(client->irq)) {
+		dev_warn(&client->dev,
+			 "Failed to get GPIO Interrupt Assertion Level, could not enable polling mode for %s",
+			 client->name);
+		return -EINVAL;
+	}
+
+	ihid->polling_thread = kthread_create(i2c_hid_polling_thread, ihid,
+					      "I2C HID polling thread");
+
+	if (!IS_ERR(ihid->polling_thread)) {
+		pr_info("I2C HID polling thread created");
+		wake_up_process(ihid->polling_thread);
+		return 0;
+	}
+
+	return PTR_ERR(ihid->polling_thread);
+}
+
 static int i2c_hid_init_irq(struct i2c_client *client)
 {
 	struct i2c_hid *ihid = i2c_get_clientdata(client);
@@ -1007,6 +1117,15 @@ static void i2c_hid_fwnode_probe(struct i2c_client *client,
 		pdata->post_power_delay_ms = val;
 }

+static void free_irq_or_stop_polling(struct i2c_client *client,
+				     struct i2c_hid *ihid)
+{
+	if (polling_mode != I2C_HID_POLLING_DISABLED)
+		kthread_stop(ihid->polling_thread);
+	else
+		free_irq(client->irq, ihid);
+}
+
 static int i2c_hid_probe(struct i2c_client *client,
 			 const struct i2c_device_id *dev_id)
 {
@@ -1102,7 +1221,11 @@ static int i2c_hid_probe(struct i2c_client *client,
 	if (ret < 0)
 		goto err_regulator;

-	ret = i2c_hid_init_irq(client);
+	if (polling_mode != I2C_HID_POLLING_DISABLED)
+		ret = i2c_hid_init_polling(ihid);
+	else
+		ret = i2c_hid_init_irq(client);
+
 	if (ret < 0)
 		goto err_regulator;

@@ -1141,7 +1264,7 @@ static int i2c_hid_probe(struct i2c_client *client,
 	hid_destroy_device(hid);

 err_irq:
-	free_irq(client->irq, ihid);
+	free_irq_or_stop_polling(client, ihid);

 err_regulator:
 	regulator_bulk_disable(ARRAY_SIZE(ihid->pdata.supplies),
@@ -1158,7 +1281,7 @@ static int i2c_hid_remove(struct i2c_client *client)
 	hid = ihid->hid;
 	hid_destroy_device(hid);

-	free_irq(client->irq, ihid);
+	free_irq_or_stop_polling(client, ihid);

 	if (ihid->bufsize)
 		i2c_hid_free_buffers(ihid);
@@ -1174,7 +1297,7 @@ static void i2c_hid_shutdown(struct i2c_client *client)
 	struct i2c_hid *ihid = i2c_get_clientdata(client);

 	i2c_hid_set_power(client, I2C_HID_PWR_SLEEP);
-	free_irq(client->irq, ihid);
+	free_irq_or_stop_polling(client, ihid);
 }

 #ifdef CONFIG_PM_SLEEP
@@ -1195,15 +1318,16 @@ static int i2c_hid_suspend(struct device *dev)
 	/* Save some power */
 	i2c_hid_set_power(client, I2C_HID_PWR_SLEEP);

-	disable_irq(client->irq);
+	if (polling_mode == I2C_HID_POLLING_DISABLED)
+		disable_irq(client->irq);

-	if (device_may_wakeup(&client->dev)) {
+	if (device_may_wakeup(&client->dev) && polling_mode == I2C_HID_POLLING_DISABLED) {
 		wake_status = enable_irq_wake(client->irq);
 		if (!wake_status)
 			ihid->irq_wake_enabled = true;
 		else
 			hid_warn(hid, "Failed to enable irq wake: %d\n",
-				wake_status);
+				 wake_status);
 	} else {
 		regulator_bulk_disable(ARRAY_SIZE(ihid->pdata.supplies),
 				       ihid->pdata.supplies);
@@ -1220,7 +1344,7 @@ static int i2c_hid_resume(struct device *dev)
 	struct hid_device *hid = ihid->hid;
 	int wake_status;

-	if (!device_may_wakeup(&client->dev)) {
+	if (!device_may_wakeup(&client->dev) || polling_mode != I2C_HID_POLLING_DISABLED) {
 		ret = regulator_bulk_enable(ARRAY_SIZE(ihid->pdata.supplies),
 					    ihid->pdata.supplies);
 		if (ret)
@@ -1237,7 +1361,8 @@ static int i2c_hid_resume(struct device *dev)
 				wake_status);
 	}

-	enable_irq(client->irq);
+	if (polling_mode == I2C_HID_POLLING_DISABLED)
+		enable_irq(client->irq);

 	/* Instead of resetting device, simply powers the device on. This
 	 * solves "incomplete reports" on Raydium devices 2386:3118 and
--
2.28.0


             reply	other threads:[~2020-10-21 13:49 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-21 13:49 Coiby Xu [this message]
2020-10-22 14:22 ` [PATCH v3] HID: i2c-hid: add polling mode based on connected GPIO chip's pin status Barnabás Pőcze
2020-11-22 10:15   ` Coiby Xu
2020-11-22 13:33     ` Barnabás Pőcze
2020-11-23 14:36       ` Coiby Xu
2020-11-23 16:32         ` Barnabás Pőcze
2020-11-25 10:57           ` Coiby Xu
2020-11-25 12:39             ` Barnabás Pőcze
2020-11-25 13:18               ` 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=20201021134931.462560-1-coiby.xu@gmail.com \
    --to=coiby.xu@gmail.com \
    --cc=benjamin.tissoires@redhat.com \
    --cc=helmut.stult@schinfo.de \
    --cc=jikos@kernel.org \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --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).