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

[sashiko-bot@kernel.org]

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