[PATCH 0/4] Input: edt-ft5x06: robustness fixes for a FocalTech FT5426 on a marginal bus

[PATCH 0/4] Input: edt-ft5x06: robustness fixes for a FocalTech FT5426 on a marginal bus

[Alexandre Hamamdjian via B4 Relay]

The AYANEO Pocket DS drives its FocalTech FT5426 touch controller on a
Qualcomm GENI i2c controller that has no bus recovery and, on this
board, clock-stretches close to the controller timeout. This series
collects the robustness work that helps there, smallest and most
generic first:

  1. ignore contacts reported with an out-of-range slot id -- a generic
     correctness fix, independent of the board;
  2. document, and
  3. implement an optional no-regmap-bulk-read mode that reads the touch
     frame one register at a time, for controllers whose bus aborts long
     transfers;
  4. poll while a contact is down to recover a touch release dropped by
     a missed edge interrupt.

Patch 1 stands on its own; the rest are opt-in (DT property / chip
behaviour) and inert on healthy hardware. Feedback on whether the bus
mitigations belong here vs. in the i2c controller is welcome.

Signed-off-by: Alexandre Hamamdjian <azkali.limited@gmail.com>
---
Alexandre Hamamdjian (3):
      Input: edt-ft5x06 - ignore contacts with an out-of-range slot id
      dt-bindings: input: edt-ft5x06 - add no-regmap-bulk-read property
      Input: edt-ft5x06 - poll while a contact is down to recover dropped releases

Teguh Sobirin (1):
      Input: edt-ft5x06 - allow reading the touch frame one register at a time

 .../bindings/input/touchscreen/edt-ft5x06.yaml     |   8 +
 drivers/input/touchscreen/edt-ft5x06.c             | 200 +++++++++++++++++++--
 2 files changed, 196 insertions(+), 12 deletions(-)
---
base-commit: e98d21c170b01ddef366f023bbfcf6b31509fa83
change-id: 20260723-b4-ft5426-393d8514e4bf

Best regards,
--  
Alexandre Hamamdjian <azkali.limited@gmail.com>

[PATCH 1/4] Input: edt-ft5x06 - ignore contacts with an out-of-range slot id

[Alexandre Hamamdjian via B4 Relay]

From: Alexandre Hamamdjian <azkali.limited@gmail.com>

The per-contact slot id is taken from the top nibble of the third
report byte, so it can be any value from 0 to 15. The driver only
allocates max_support_points MT slots (2 to 10 depending on the
variant), so a report that carries an id at or above that count - be it
a genuinely higher-numbered contact or a corrupted byte - is outside the
range the input core was told about.

input_mt_slot() silently ignores an ABS_MT_SLOT beyond num_slots and
leaves the current slot unchanged, so the following
input_mt_report_slot_state()/touchscreen_report_pos() pair is applied to
whichever slot happened to be selected last, reporting the contact at the
wrong position. Skip such entries instead.

Signed-off-by: Alexandre Hamamdjian <azkali.limited@gmail.com>
---
 drivers/input/touchscreen/edt-ft5x06.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
index d3b1177185a3..d6c3d033b83d 100644
--- a/drivers/input/touchscreen/edt-ft5x06.c
+++ b/drivers/input/touchscreen/edt-ft5x06.c
@@ -331,6 +331,8 @@ static irqreturn_t edt_ft5x06_ts_isr(int irq, void *dev_id)
 			swap(x, y);
 
 		id = (buf[2] >> 4) & 0x0f;
+		if (id >= tsdata->max_support_points)
+			continue;
 
 		input_mt_slot(tsdata->input, id);
 		if (input_mt_report_slot_state(tsdata->input, MT_TOOL_FINGER,

-- 
2.55.0

[PATCH 2/4] dt-bindings: input: edt-ft5x06 - add no-regmap-bulk-read property

[Alexandre Hamamdjian via B4 Relay]

From: Alexandre Hamamdjian <azkali.limited@gmail.com>

Document an optional boolean property for boards whose i2c controller
cannot reliably sustain the multi-byte block read of a full touch frame.
When set, the driver reads the frame one register at a time instead of in
a single bulk transfer.

Signed-off-by: Alexandre Hamamdjian <azkali.limited@gmail.com>
---
 .../devicetree/bindings/input/touchscreen/edt-ft5x06.yaml         | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.yaml b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.yaml
index 68b2f1601654..0d8310a290ea 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.yaml
+++ b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.yaml
@@ -111,6 +111,14 @@ properties:
 
   interrupt-controller: true
 
+  no-regmap-bulk-read:
+    type: boolean
+    description:
+      Set this when the i2c controller the touch controller is attached to
+      cannot reliably sustain the multi-byte block read of a full touch
+      frame and aborts long transfers. The driver then reads the touch
+      frame one register at a time instead of in a single bulk transfer.
+
 unevaluatedProperties: false
 
 required:

-- 
2.55.0

[PATCH 3/4] Input: edt-ft5x06 - allow reading the touch frame one register at a time

[Alexandre Hamamdjian via B4 Relay]

From: Teguh Sobirin <teguh@sobir.in>

Every touch interrupt reads the whole touch frame in a single i2c block
read via regmap_bulk_read(). On some boards the i2c controller the panel
is wired to cannot sustain that multi-byte transfer: on the AYANEO Pocket
DS the FocalTech FT5426 sits on a marginal Qualcomm GENI bus that
intermittently aborts a long read with -EAGAIN or -ETIMEDOUT, and the
GENI controller has no bus recovery, so the block read fails on nearly
every interrupt and the panel is unusable.

Honour the "no-regmap-bulk-read" property. When set, the driver reads the
frame one register at a time with a short retry on the transient bus
errors, keeping each transfer small enough to complete. Boards on a
healthy bus keep using the single bulk transfer and are unaffected.

Signed-off-by: Teguh Sobirin <teguh@sobir.in>
Co-developed-by: Alexandre Hamamdjian <azkali.limited@gmail.com>
Signed-off-by: Alexandre Hamamdjian <azkali.limited@gmail.com>
---
 drivers/input/touchscreen/edt-ft5x06.c | 37 ++++++++++++++++++++++++++++++++--
 1 file changed, 35 insertions(+), 2 deletions(-)

diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
index d6c3d033b83d..ac61ac44fd64 100644
--- a/drivers/input/touchscreen/edt-ft5x06.c
+++ b/drivers/input/touchscreen/edt-ft5x06.c
@@ -146,6 +146,7 @@ struct edt_ft5x06_ts_data {
 	enum edt_ver version;
 	unsigned int crc_errors;
 	unsigned int header_errors;
+	bool no_regmap_bulk_read;
 };
 
 struct edt_i2c_chip_data {
@@ -295,6 +296,31 @@ static const struct regmap_config edt_M06_i2c_regmap_config = {
 	.write = edt_M06_i2c_write,
 };
 
+static int edt_ft5x06_bulk_read(struct regmap *map, unsigned int start,
+				void *val, size_t len)
+{
+	u8 *dst = val;
+	size_t off;
+
+	for (off = 0; off < len; off++) {
+		unsigned int v;
+		int ret, tries;
+
+		for (tries = 0; tries < 3; tries++) {
+			ret = regmap_read(map, start + off, &v);
+			if (!ret)
+				break;
+			if (ret == -ETIMEDOUT || ret == -EAGAIN)
+				usleep_range(2000, 4000);
+		}
+		if (ret)
+			return ret;
+		dst[off] = v;
+	}
+
+	return 0;
+}
+
 static irqreturn_t edt_ft5x06_ts_isr(int irq, void *dev_id)
 {
 	struct edt_ft5x06_ts_data *tsdata = dev_id;
@@ -304,8 +330,12 @@ static irqreturn_t edt_ft5x06_ts_isr(int irq, void *dev_id)
 	int error;
 
 	memset(rdbuf, 0, sizeof(rdbuf));
-	error = regmap_bulk_read(tsdata->regmap, tsdata->tdata_cmd, rdbuf,
-				 tsdata->tdata_len);
+	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) {
 		dev_err_ratelimited(dev, "Unable to fetch data, error: %d\n",
 				    error);
@@ -1212,6 +1242,9 @@ static int edt_ft5x06_ts_probe(struct i2c_client *client)
 		return error;
 	}
 
+	tsdata->no_regmap_bulk_read =
+		device_property_read_bool(&client->dev, "no-regmap-bulk-read");
+
 	/*
 	 * Check which sleep modes we can support. Power-off requires the
 	 * reset-pin to ensure correct power-down/power-up behaviour. Start with

-- 
2.55.0

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

[Alexandre Hamamdjian via B4 Relay]

From: Alexandre Hamamdjian <azkali.limited@gmail.com>

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 <azkali.limited@gmail.com>
---
 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

Re: [PATCH 2/4] dt-bindings: input: edt-ft5x06 - add no-regmap-bulk-read property

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The `no-regmap-bulk-read` property violates OS-agnostic Device Tree design principles by embedding Linux-specific software terminology and misplacing parent bus limitations into a child node.
--

--- Patch [2]: [PATCH 2/4] dt-bindings: input: edt-ft5x06 - add no-regmap-bulk-read property ---
commit adbbdf0f113c3cd899e582907d5e795b5a3a7cc7
Author: Alexandre Hamamdjian <azkali.limited@gmail.com>

dt-bindings: input: edt-ft5x06 - add no-regmap-bulk-read property

This commit documents an optional boolean property, no-regmap-bulk-read, for
boards where the I2C controller cannot reliably sustain multi-byte block reads.
It instructs the driver to read the touch frame one register at a time instead
of using a bulk transfer.

> diff --git a/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.yaml b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.yaml
> index 68b2f16016543..0d8310a290ea5 100644
> --- a/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.yaml
> +++ b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.yaml
> @@ -111,6 +111,14 @@ properties:
>  
>    interrupt-controller: true
>  
> +  no-regmap-bulk-read:
> +    type: boolean
> +    description:
> +      Set this when the i2c controller the touch controller is attached to
> +      cannot reliably sustain the multi-byte block read of a full touch
> +      frame and aborts long transfers. The driver then reads the touch
> +      frame one register at a time instead of in a single bulk transfer.

[Severity: Medium]
Does this property violate OS-agnostic Device Tree design principles? The
property name uses Linux-specific internal software terminology ("regmap")
and the description explicitly dictates driver implementation behavior rather
than describing the hardware layout.

Additionally, is it appropriate to model a limitation of the parent I2C
controller inside the child touch controller's device node? This appears to
misplace parent bus limitations into a child node. Should this quirk perhaps
be handled at the I2C controller level or expressed in a more hardware-centric,
OS-agnostic way?

> +
>  unevaluatedProperties: false
>  
>  required:

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260723-b4-ft5426-v1-0-d4b4e32be042@gmail.com?part=2

Re: [PATCH 3/4] Input: edt-ft5x06 - allow reading the touch frame one register at a time

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] The no-regmap-bulk-read fallback causes an out-of-bounds memory read and functional breakage on EDT_M06 devices.
- [Medium] Immediate redundant retries on permanent bus errors in edt_ft5x06_bulk_read.
- [Medium] Incomplete implementation of no-regmap-bulk-read leaves other bulk reads unpatched.
- [Low] Subsystem guide violation: variable `ret` used for error codes instead of `error` or `err`.
--

--- Patch [3]: [PATCH 3/4] Input: edt-ft5x06 - allow reading the touch frame one register at a time ---
commit 5f06c007cc93b45a3ff2f9ecba4f1f085c956d6f
Author: Teguh Sobirin <teguh@sobir.in>

Input: edt-ft5x06 - allow reading the touch frame one register at a time

This commit introduces a "no-regmap-bulk-read" property to handle I2C
controllers that cannot sustain multi-byte transfers. It implements a fallback
that reads the touch frame one register at a time.

> diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
> index d6c3d033b83df..ac61ac44fd649 100644
> --- a/drivers/input/touchscreen/edt-ft5x06.c
> +++ b/drivers/input/touchscreen/edt-ft5x06.c
[ ... ]
> @@ -295,6 +296,31 @@ static const struct regmap_config edt_M06_i2c_regmap_config = {
>  	.write = edt_M06_i2c_write,
>  };
>  
> +static int edt_ft5x06_bulk_read(struct regmap *map, unsigned int start,
> +				void *val, size_t len)
> +{
> +	u8 *dst = val;
> +	size_t off;
> +
> +	for (off = 0; off < len; off++) {
> +		unsigned int v;
> +		int ret, tries;

[Severity: Low]
This isn't a bug, but does using 'ret' here violate the input subsystem
guidelines? 

The maintainer prefers using 'error' or 'err' for variables that only hold
negative error codes or 0 for success.

> +
> +		for (tries = 0; tries < 3; tries++) {
> +			ret = regmap_read(map, start + off, &v);

[Severity: High]
Does this 1-byte read cause an out-of-bounds access and functional breakage
on EDT_M06 devices?

If the board uses an M06 device with the no-regmap-bulk-read property, this
call will split the frame read into 1-byte chunks. However, the custom
edt_M06_i2c_read() function expects a larger buffer to validate the header:

    u8 *buf = (u8 *)val_buf;
    if (buf[0] != 0xaa || buf[1] != 0xaa ||
        buf[2] != val_size) {

Since regmap only provides a 1-byte buffer for a 1-byte read, this will read
out of bounds and deterministically fail the header validation. 

Additionally, subsequent bytes will hit the default case, sending invalid
factory commands instead of reading touch data.

> +			if (!ret)
> +				break;
> +			if (ret == -ETIMEDOUT || ret == -EAGAIN)
> +				usleep_range(2000, 4000);

[Severity: Medium]
Will this loop immediately issue redundant retries if the error is permanent?

If regmap_read() returns a permanent error like -EIO (e.g., due to a NACK), it
correctly skips the usleep_range() here. However, since there is no 'else break'
for unhandled errors, it will immediately loop and issue the failed transaction
again up to 3 times back-to-back.

> +		}
> +		if (ret)
> +			return ret;
> +		dst[off] = v;
> +	}
> +
> +	return 0;
> +}
[ ... ]
> @@ -1212,6 +1242,9 @@ static int edt_ft5x06_ts_probe(struct i2c_client *client)
>  		return error;
>  	}
>  
> +	tsdata->no_regmap_bulk_read =
> +		device_property_read_bool(&client->dev, "no-regmap-bulk-read");
> +

[Severity: Medium]
Is the no-regmap-bulk-read workaround incomplete?

While the ISR now uses the fallback, other parts of the driver still use
unconditional bulk reads. For example, in edt_ft5x06_ts_identify() called
during probe:

    error = regmap_bulk_read(tsdata->regmap, 0xBB, rdbuf, EDT_NAME_LEN - 1);

If the bus truly cannot sustain multi-byte transfers, won't the probe and
debugfs features still fail despite this property being set?

>  	/*
>  	 * Check which sleep modes we can support. Power-off requires the
>  	 * reset-pin to ensure correct power-down/power-up behaviour. Start with

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260723-b4-ft5426-v1-0-d4b4e32be042@gmail.com?part=3

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The polling worker can be erroneously re-armed during device suspend, leading to invalid I2C accesses while the device is powered off.
- [Medium] A single transient I2C read failure after an idle period immediately triggers a spurious controller reset.
- [Medium] The controller reset recovery logic breaks permanently after ~24.8 days of uptime on 32-bit systems due to jiffies wrap-around.
--

--- Patch [4]: [PATCH 4/4] Input: edt-ft5x06 - poll while a contact is down to recover dropped releases ---
commit 601140d3fc50fbfeaa9e78699c5e29c00098c932
Author: Alexandre Hamamdjian <azkali.limited@gmail.com>

Input: edt-ft5x06 - poll while a contact is down to recover dropped releases

This commit adds a polling mechanism to recover from dropped release events
on unreliable I2C buses. It tracks active contacts and polls the panel while
any are down, synthesizing releases if contacts disappear. It also adds a
bounded retry for transient bus errors and a controller reset mechanism if
reads fail for over a second.

> diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
> index ac61ac44fd649..794c0650f5cb3 100644
> --- a/drivers/input/touchscreen/edt-ft5x06.c
> +++ b/drivers/input/touchscreen/edt-ft5x06.c
[ ... ]
> @@ -321,26 +333,83 @@ static int edt_ft5x06_bulk_read(struct regmap *map, unsigned int start,
[ ... ]
> +static void edt_ft5x06_fetch_and_report(struct edt_ft5x06_ts_data *tsdata)
>  {
[ ... ]
> +	for (tries = 0; tries < 4; tries++) {
[ ... ]
> +	}
>  	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)) {

[Severity: Medium]
Will a single transient I2C read failure after an idle period immediately
trigger a spurious controller reset?

If the device sits idle for > 1 second, tsdata->last_success becomes stale.
When a new touch arrives, if the very first I2C read fails due to a transient
bus error, the time_after(jiffies, tsdata->last_success + HZ) check evaluates
to true.

This bypasses the intended 1-second delay of continuous failures, and
immediately pulses the reset line, causing a 300ms stall and dropping the
initial touch.

[Severity: Medium]
Does the controller reset recovery logic break permanently after ~24.8 days of
uptime on 32-bit systems due to jiffies wrap-around?

If the system runs for more than 24.8 days (exceeding MAX_JIFFY_OFFSET)
without the touchscreen wedging, tsdata->last_reset remains at its
initialization value.

A subsequent I2C bus error triggering the recovery path would cause the
time_after(jiffies, tsdata->last_reset + 2 * HZ) check to evaluate to false
due to signed integer overflow, bypassing the reset entirely until jiffies
wraps around the rest of the 32-bit space.

> +			tsdata->last_reset = jiffies;
> +			gpiod_set_value_cansleep(tsdata->reset_gpio, 1);
[ ... ]
> +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;
>  }
[ ... ]
> @@ -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);
> +

[Severity: High]
Can the polling worker be erroneously re-armed during device suspend?

In edt_ft5x06_ts_suspend(), cancel_delayed_work_sync() is called to stop
the poll worker, but the device's interrupt is not disabled until several
operations later (or never disabled in HIBERNATE mode).

If an interrupt occurs in this window, the ISR edt_ft5x06_ts_isr() runs,
observes a non-zero down_mask, and unconditionally calls mod_delayed_work().

This could cause the worker to execute after the suspend function has disabled
regulators and suspended the I2C adapter, potentially leading to I2C access
timeouts or hardware hangs.

>  	if (device_may_wakeup(dev))
>  		return 0;
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260723-b4-ft5426-v1-0-d4b4e32be042@gmail.com?part=4

Re: [PATCH 1/4] Input: edt-ft5x06 - ignore contacts with an out-of-range slot id

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Unbalanced `disable_irq()` calls when suspending and resuming in factory mode lead to a permanently disabled touchscreen.
- [High] Stack-allocated buffers are passed to `regmap_bulk_read()` and `i2c_transfer()`, which can cause memory corruption or panics if the underlying I2C host controller driver uses DMA.
--

--- Patch [1]: [PATCH 1/4] Input: edt-ft5x06 - ignore contacts with an out-of-range slot id ---
commit 9dab47af56724a9e66ce05f01c0e4d726933073c
Author: Alexandre Hamamdjian <azkali.limited@gmail.com>

Input: edt-ft5x06 - ignore contacts with an out-of-range slot id

The per-contact slot id is taken from the top nibble of the third
report byte, so it can be any value from 0 to 15. The driver only
allocates max_support_points MT slots (2 to 10 depending on the
variant), so a report that carries an id at or above that count - be it
a genuinely higher-numbered contact or a corrupted byte - is outside the
range the input core was told about.

input_mt_slot() silently ignores an ABS_MT_SLOT beyond num_slots and
leaves the current slot unchanged, so the following
input_mt_report_slot_state()/touchscreen_report_pos() pair is applied to
whichever slot happened to be selected last, reporting the contact at the
wrong position. Skip such entries instead.

> diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
> index d3b1177185a3b..d6c3d033b83df 100644
> --- a/drivers/input/touchscreen/edt-ft5x06.c
> +++ b/drivers/input/touchscreen/edt-ft5x06.c
> @@ -331,6 +331,8 @@ static irqreturn_t edt_ft5x06_ts_isr(int irq, void *dev_id)
>  			swap(x, y);
>  
>  		id = (buf[2] >> 4) & 0x0f;
> +		if (id >= tsdata->max_support_points)
> +			continue;
>  
>  		input_mt_slot(tsdata->input, id);
>  		if (input_mt_report_slot_state(tsdata->input, MT_TOOL_FINGER,

[Severity: High]
This isn't a bug introduced by this patch, but I noticed that stack-allocated
buffers are being passed to regmap_bulk_read() in this same function:

drivers/input/touchscreen/edt-ft5x06.c:edt_ft5x06_ts_isr() {
    ...
	u8 rdbuf[63];
    ...
	error = regmap_bulk_read(tsdata->regmap, tsdata->tdata_cmd, rdbuf,
				 tsdata->tdata_len);
    ...
}

Can this lead to memory corruption or kernel panics if the underlying I2C host
controller uses DMA without bounce buffering and CONFIG_VMAP_STACK is enabled?

I see similar stack allocations passed to i2c_transfer() elsewhere in the
driver:

drivers/input/touchscreen/edt-ft5x06.c:edt_M06_i2c_read() {
    ...
	u8 wbuf[4], rbuf[3];
    ...
	xfer[1].buf = reg_read ? rbuf : val_buf;
	ret = i2c_transfer(i2c->adapter, xfer, 2);
    ...
}

drivers/input/touchscreen/edt-ft5x06.c:edt_M06_i2c_write() {
    ...
	u8 wbuf[4];
    ...
	xfer.buf = wbuf;
	ret = i2c_transfer(i2c->adapter, &xfer, 1);
    ...
}

Should these buffers be heap-allocated to avoid violating the kernel DMA API?

[Severity: High]
This is a pre-existing issue, but there appears to be a regression with
unbalanced disable_irq() calls that could leave the touchscreen permanently
disabled.

When entering factory mode, disable_irq() increments the disable depth:

drivers/input/touchscreen/edt-ft5x06.c:edt_ft5x06_factory_mode() {
    ...
	disable_irq(client->irq);
    ...
}

If the system then suspends, edt_ft5x06_ts_suspend() unconditionally disables
the IRQ again, incrementing the depth a second time:

drivers/input/touchscreen/edt-ft5x06.c:edt_ft5x06_ts_suspend() {
    ...
	disable_irq(tsdata->client->irq);
    ...
}

During resume, edt_ft5x06_ts_resume() calls enable_irq() once, but then
detects tsdata->factory_mode is true and calls edt_ft5x06_factory_mode()
again, which disables the IRQ a second time:

drivers/input/touchscreen/edt-ft5x06.c:edt_ft5x06_ts_resume() {
    ...
		enable_irq(tsdata->client->irq);

		if (tsdata->factory_mode)
			ret = edt_ft5x06_factory_mode(tsdata);
    ...
}

When the user later switches back to work mode, edt_ft5x06_work_mode() only
calls enable_irq() once. Does this leave the IRQ disable depth at 1, keeping
the touchscreen completely dead until the module is reloaded or the system is
rebooted?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260723-b4-ft5426-v1-0-d4b4e32be042@gmail.com?part=1

Re: [PATCH 2/4] dt-bindings: input: edt-ft5x06 - add no-regmap-bulk-read property

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2192 bytes --]

On Thu, Jul 23, 2026 at 06:27:09PM +0700, Alexandre Hamamdjian via B4 Relay wrote:
> From: Alexandre Hamamdjian <azkali.limited@gmail.com>
> 
> Document an optional boolean property for boards whose i2c controller
> cannot reliably sustain the multi-byte block read of a full touch frame.
> When set, the driver reads the frame one register at a time instead of in
> a single bulk transfer.
> 
> Signed-off-by: Alexandre Hamamdjian <azkali.limited@gmail.com>
> ---
>  .../devicetree/bindings/input/touchscreen/edt-ft5x06.yaml         | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.yaml b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.yaml
> index 68b2f1601654..0d8310a290ea 100644
> --- a/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.yaml
> +++ b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.yaml
> @@ -111,6 +111,14 @@ properties:
>  
>    interrupt-controller: true
>  
> +  no-regmap-bulk-read:

"regmap" is a linux software construct, nothing to do with this
touchscreen or hardware at all for that matter - it's i2c bulk reads you
cannot do.

> +    type: boolean
> +    description:
> +      Set this when the i2c controller the touch controller is attached to

Problems with your controller should be dealt with at the controller
level, not in the device node because the device doesn't have a problem,
the controller does. I think "i2c_adapter_quirks" is the knob used for
this.
This kind of property seems to have come up the other day in a similar context:
https://lore.kernel.org/all/20260722-regmap-single-read-v1-0-aaaf31591669@linux.dev/
The regmap maintainer there rejected a device level property in favour
of discovering this from the controller.

pwbot: rejected

Cheers,
Conor.

> +      cannot reliably sustain the multi-byte block read of a full touch
> +      frame and aborts long transfers. The driver then reads the touch
> +      frame one register at a time instead of in a single bulk transfer.
> +
>  unevaluatedProperties: false
>  
>  required:
> 
> -- 
> 2.55.0
> 
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 1/4] Input: edt-ft5x06 - ignore contacts with an out-of-range slot id

[Dmitry Torokhov]

On Thu, Jul 23, 2026 at 11:40:59AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] Unbalanced `disable_irq()` calls when suspending and resuming in factory mode lead to a permanently disabled touchscreen.

This is obviously not caused by your series but might be real, need to
check...

> - [High] Stack-allocated buffers are passed to `regmap_bulk_read()` and `i2c_transfer()`, which can cause memory corruption or panics if the underlying I2C host controller driver uses DMA.

Sashiko hallucinates here. I added I2C prompt to teach it that I2C is
generally safe wrt on-stack buffers, but I am not sure if it will trace
it to regmap.

Thanks.

-- 
Dmitry