[PATCH 4/4] Input: edt-ft5x06 - poll while a contact is down to recover dropped releases

Alexandre Hamamdjian <[email protected]>
Newsgroups org.kernel.feeds.b4-sent,org.kernel.vger.linux-devicetree,org.kernel.vger.linux-input,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
The driver is purely interrupt driven: a touch-up is only reported when
the controller raises an edge for the release frame and that frame is
read successfully. On a marginal i2c bus a read can fail or be dropped,
and if the read that carried the release is the one lost there is no
further edge to re-read it, so the contact stays held down forever - the
pointer sticks mid-drag. This is readily reproducible on the AYANEO
Pocket DS, whose FT5426 hangs off an unreliable Qualcomm GENI bus.

Stop trusting a single edge to deliver the release. Track which slots are
held down in a mask and, while any contact is down, re-read the frame on
a short timer. A contact missing from the frame is released only after a
few consecutive misses so a lone glitchy read cannot cut a still-present
tap or drag; conversely, once the finger is really gone the polled reads
stop listing it and it is released regardless of whether an explicit
touch-up frame ever arrives. The timer stops as soon as the last contact
is released, so an idle panel is still fully interrupt driven.

The read path also gains a bounded retry over the transient bus errors
and, when reads keep failing for over a second, a reset-line pulse to
recover a wedged controller, dropping any held contacts afterwards since
the post-reset finger state is unknown. The poll worker is cancelled on
suspend and, via a devm action registered before the IRQ, on removal, so
it can never touch i2c after the device is powered down or the IRQ freed.

Signed-off-by: Alexandre Hamamdjian <[email protected]>
---
 drivers/input/touchscreen/edt-ft5x06.c | 173 ++++++++++++++++++++++++++++++---
 1 file changed, 157 insertions(+), 16 deletions(-)

diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
index ac61ac44fd64..794c0650f5cb 100644
--- a/drivers/input/touchscreen/edt-ft5x06.c
+++ b/drivers/input/touchscreen/edt-ft5x06.c
@@ -19,6 +19,7 @@
 #include <linux/gpio/consumer.h>
 #include <linux/i2c.h>
 #include <linux/interrupt.h>
+#include <linux/workqueue.h>
 #include <linux/input.h>
 #include <linux/input/mt.h>
 #include <linux/input/touchscreen.h>
@@ -147,8 +148,19 @@ struct edt_ft5x06_ts_data {
 	unsigned int crc_errors;
 	unsigned int header_errors;
 	bool no_regmap_bulk_read;
+	unsigned long last_reset;
+	unsigned long last_success;
+	struct delayed_work poll_work;
+	/* io_lock serialises the frame read between the IRQ and the poll work */
+	struct mutex io_lock;
+	u16 down_mask;
+	u8 miss[16];
 };
 
+/* poll cadence and release debounce for the poll-while-touched recovery */
+#define EDT_POLL_INTERVAL_MS	15
+#define EDT_RELEASE_MISSES	3
+
 struct edt_i2c_chip_data {
 	int  max_support_points;
 };
@@ -321,26 +333,83 @@ static int edt_ft5x06_bulk_read(struct regmap *map, unsigned int start,
 	return 0;
 }
 
-static irqreturn_t edt_ft5x06_ts_isr(int irq, void *dev_id)
+static void edt_ft5x06_release_all(struct edt_ft5x06_ts_data *tsdata)
+{
+	int id;
+
+	if (!tsdata->down_mask)
+		return;
+
+	for (id = 0; id < tsdata->max_support_points; id++) {
+		if (!(tsdata->down_mask & BIT(id)))
+			continue;
+		input_mt_slot(tsdata->input, id);
+		input_mt_report_slot_state(tsdata->input, MT_TOOL_FINGER, false);
+	}
+	tsdata->down_mask = 0;
+	memset(tsdata->miss, 0, sizeof(tsdata->miss));
+	input_mt_report_pointer_emulation(tsdata->input, true);
+	input_sync(tsdata->input);
+}
+
+static void edt_ft5x06_fetch_and_report(struct edt_ft5x06_ts_data *tsdata)
 {
-	struct edt_ft5x06_ts_data *tsdata = dev_id;
 	struct device *dev = &tsdata->client->dev;
+	u16 new_mask = 0, released = 0;
 	u8 rdbuf[63];
 	int i, type, x, y, id;
-	int error;
+	int error, tries;
 
 	memset(rdbuf, 0, sizeof(rdbuf));
-	if (tsdata->no_regmap_bulk_read)
-		error = edt_ft5x06_bulk_read(tsdata->regmap, tsdata->tdata_cmd,
-					     rdbuf, tsdata->tdata_len);
-	else
-		error = regmap_bulk_read(tsdata->regmap, tsdata->tdata_cmd,
-					 rdbuf, tsdata->tdata_len);
+	for (tries = 0; tries < 4; tries++) {
+		if (tsdata->no_regmap_bulk_read)
+			error = edt_ft5x06_bulk_read(tsdata->regmap,
+						     tsdata->tdata_cmd, rdbuf,
+						     tsdata->tdata_len);
+		else
+			error = regmap_bulk_read(tsdata->regmap,
+						 tsdata->tdata_cmd, rdbuf,
+						 tsdata->tdata_len);
+		if (!error)
+			break;
+		if (error != -EAGAIN && error != -ETIMEDOUT &&
+		    error != -EIO && error != -ENXIO)
+			break;
+		usleep_range(min(1000U << tries, 4000U),
+			     min(2000U << tries, 8000U));
+	}
 	if (error) {
 		dev_err_ratelimited(dev, "Unable to fetch data, error: %d\n",
 				    error);
-		goto out;
+		/*
+		 * A run of failed reads with no success for over a second means
+		 * the controller is wedged rather than just glitching; pulse the
+		 * reset line to recover it and drop any held contacts, since the
+		 * post-reset finger state is unknown.
+		 */
+		if (tsdata->reset_gpio &&
+		    time_after(jiffies, tsdata->last_success + HZ) &&
+		    time_after(jiffies, tsdata->last_reset + 2 * HZ)) {
+			tsdata->last_reset = jiffies;
+			gpiod_set_value_cansleep(tsdata->reset_gpio, 1);
+			usleep_range(5000, 6000);
+			gpiod_set_value_cansleep(tsdata->reset_gpio, 0);
+			msleep(300);
+			tsdata->last_success = jiffies;
+			dev_warn_ratelimited(dev, "reset to recover controller\n");
+			edt_ft5x06_release_all(tsdata);
+		}
+		return;
 	}
+	tsdata->last_success = jiffies;
+
+	/*
+	 * TD_STATUS holds the active-contact count; a value above the panel
+	 * maximum means the frame is corrupt, so keep the previous state.
+	 */
+	if (tsdata->version != EDT_M06 &&
+	    (rdbuf[2] & 0x0f) > tsdata->max_support_points)
+		return;
 
 	for (i = 0; i < tsdata->max_support_points; i++) {
 		u8 *buf = &rdbuf[i * tsdata->point_len + tsdata->tdata_offset];
@@ -349,10 +418,12 @@ static irqreturn_t edt_ft5x06_ts_isr(int irq, void *dev_id)
 		/* ignore Reserved events */
 		if (type == TOUCH_EVENT_RESERVED)
 			continue;
-
 		/* M06 sometimes sends bogus coordinates in TOUCH_DOWN */
 		if (tsdata->version == EDT_M06 && type == TOUCH_EVENT_DOWN)
 			continue;
+		/* releases are derived from the down-mask diff below */
+		if (type == TOUCH_EVENT_UP)
+			continue;
 
 		x = get_unaligned_be16(buf) & 0x0fff;
 		y = get_unaligned_be16(buf + 2) & 0x0fff;
@@ -363,21 +434,75 @@ static irqreturn_t edt_ft5x06_ts_isr(int irq, void *dev_id)
 		id = (buf[2] >> 4) & 0x0f;
 		if (id >= tsdata->max_support_points)
 			continue;
+		if (tsdata->prop.max_x &&
+		    (x > tsdata->prop.max_x || y > tsdata->prop.max_y))
+			continue;
 
 		input_mt_slot(tsdata->input, id);
-		if (input_mt_report_slot_state(tsdata->input, MT_TOOL_FINGER,
-					       type != TOUCH_EVENT_UP))
-			touchscreen_report_pos(tsdata->input, &tsdata->prop,
-					       x, y, true);
+		input_mt_report_slot_state(tsdata->input, MT_TOOL_FINGER, true);
+		touchscreen_report_pos(tsdata->input, &tsdata->prop, x, y, true);
+		new_mask |= BIT(id);
 	}
 
+	/*
+	 * Reconcile held contacts with this frame. A contact absent from the
+	 * frame is released only after EDT_RELEASE_MISSES consecutive misses so
+	 * a single glitchy read cannot cut a still-present tap or drag.
+	 */
+	for (id = 0; id < tsdata->max_support_points; id++) {
+		if (new_mask & BIT(id)) {
+			tsdata->miss[id] = 0;
+			continue;
+		}
+		if (!(tsdata->down_mask & BIT(id)))
+			continue;
+		if (++tsdata->miss[id] >= EDT_RELEASE_MISSES) {
+			input_mt_slot(tsdata->input, id);
+			input_mt_report_slot_state(tsdata->input,
+						   MT_TOOL_FINGER, false);
+			tsdata->miss[id] = 0;
+			released |= BIT(id);
+		}
+	}
+	tsdata->down_mask = (tsdata->down_mask | new_mask) & ~released;
+
 	input_mt_report_pointer_emulation(tsdata->input, true);
 	input_sync(tsdata->input);
+}
+
+static irqreturn_t edt_ft5x06_ts_isr(int irq, void *dev_id)
+{
+	struct edt_ft5x06_ts_data *tsdata = dev_id;
+
+	guard(mutex)(&tsdata->io_lock);
+	edt_ft5x06_fetch_and_report(tsdata);
+	if (tsdata->down_mask)
+		mod_delayed_work(system_wq, &tsdata->poll_work,
+				 msecs_to_jiffies(EDT_POLL_INTERVAL_MS));
 
-out:
 	return IRQ_HANDLED;
 }
 
+static void edt_ft5x06_poll_work(struct work_struct *work)
+{
+	struct edt_ft5x06_ts_data *tsdata =
+		container_of(to_delayed_work(work),
+			     struct edt_ft5x06_ts_data, poll_work);
+
+	guard(mutex)(&tsdata->io_lock);
+	edt_ft5x06_fetch_and_report(tsdata);
+	if (tsdata->down_mask)
+		mod_delayed_work(system_wq, &tsdata->poll_work,
+				 msecs_to_jiffies(EDT_POLL_INTERVAL_MS));
+}
+
+static void edt_ft5x06_cancel_poll(void *data)
+{
+	struct edt_ft5x06_ts_data *tsdata = data;
+
+	cancel_delayed_work_sync(&tsdata->poll_work);
+}
+
 struct edt_ft5x06_attribute {
 	struct device_attribute dattr;
 	size_t field_offset;
@@ -1244,6 +1369,10 @@ static int edt_ft5x06_ts_probe(struct i2c_client *client)
 
 	tsdata->no_regmap_bulk_read =
 		device_property_read_bool(&client->dev, "no-regmap-bulk-read");
+	tsdata->last_success = jiffies;
+	tsdata->last_reset = jiffies;
+	mutex_init(&tsdata->io_lock);
+	INIT_DELAYED_WORK(&tsdata->poll_work, edt_ft5x06_poll_work);
 
 	/*
 	 * Check which sleep modes we can support. Power-off requires the
@@ -1341,6 +1470,15 @@ static int edt_ft5x06_ts_probe(struct i2c_client *client)
 		return error;
 	}
 
+	/*
+	 * Registered before the IRQ so it unwinds after the IRQ is freed on
+	 * removal: no edge can re-arm the poll worker once it is cancelled.
+	 */
+	error = devm_add_action_or_reset(&client->dev, edt_ft5x06_cancel_poll,
+					 tsdata);
+	if (error)
+		return error;
+
 	irq_flags = irq_get_trigger_type(client->irq);
 	if (irq_flags == IRQF_TRIGGER_NONE)
 		irq_flags = IRQF_TRIGGER_FALLING;
@@ -1383,6 +1521,9 @@ static int edt_ft5x06_ts_suspend(struct device *dev)
 	struct gpio_desc *reset_gpio = tsdata->reset_gpio;
 	int ret;
 
+	/* stop the poll worker so it cannot touch i2c after power-down */
+	cancel_delayed_work_sync(&tsdata->poll_work);
+
 	if (device_may_wakeup(dev))
 		return 0;
 

-- 
2.55.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.