Re: [PATCH 3/4] Input: edt-ft5x06 - allow reading the touch frame one register at a time
flat view
From: sashiko-bot@kernel.org
Date: 2026-07-23 11:39:58
Also in:
linux-devicetree
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 [off-list ref] 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.
quoted hunk ↗ jump to hunk
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
[ ... ]
quoted hunk ↗ jump to hunk
@@ -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.
quoted hunk ↗ jump to hunk
+ + 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.
quoted hunk ↗ jump to hunk
+ 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.
quoted hunk ↗ jump to hunk
+ } + if (ret) + return ret; + dst[off] = v; + } + + return 0; +}
[ ... ]
quoted hunk ↗ jump to hunk
@@ -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