[PATCH 0/2] Input: ilitek_ts: add stylus support for 0x0c reports

[PATCH 0/2] Input: ilitek_ts: add stylus support for 0x0c reports

[Kristian Mide]

Hi,

I bought a CHUWI Hi10 Max and was annoyed to find that the built-in
stylus was not usable with the upstream ilitek_ts driver.

This small series adds support for the pen report format emitted by the
device, exposing the stylus as a separate input device with hover,
pressure, and side-button support. The pen device is created lazily on
first pen report so touchscreen-only hardware does not advertise stylus
capabilities up front.

The packet format was reverse engineered on this device. In particular,
report ID 0x0c appears to carry pen events, pressure comes from
buf[6..7] shifted right by one, and the side buttons map to two bits in
buf[1].

The second patch is unrelated to the pen support itself, but keeps the
existing wrong-report logging from flooding dmesg when unexpected packets
repeat quickly.

On the tested CHUWI Hi10 Max this makes the pen usable in GNOME/Krita,
including pressure and side buttons.

Thanks,
Kristian

Kristian Mide (2):
  Input: ilitek_ts: add stylus input support
  Input: ilitek_ts: ratelimit unexpected report logging

 drivers/input/touchscreen/ilitek_ts_i2c.c | 134 +++++++++++++++++++++-
 1 file changed, 133 insertions(+), 1 deletion(-)

-- 
2.54.0

[PATCH 1/2] Input: ilitek_ts: add stylus input support

[Kristian Mide]

Add a separate stylus input device for report ID 0x0c packets,
with pressure, hover, and side-button support.

The pen device is created lazily on first pen report so
touchscreen-only hardware does not expose stylus capabilities up
front.

The packet format is reverse engineered from a tested CHUWI
Hi10 Max. Pressure is reported from buf[6..7] shifted right by
one, matching the observed 1024 pressure levels on the tested
device.

Pen coordinates are reported through touchscreen_report_pos()
so the same axis inversion and swapping properties used by the
touch path apply to the stylus as well.
---
 drivers/input/touchscreen/ilitek_ts_i2c.c | 130 ++++++++++++++++++++++
 1 file changed, 130 insertions(+)

diff --git a/drivers/input/touchscreen/ilitek_ts_i2c.c b/drivers/input/touchscreen/ilitek_ts_i2c.c
index 3de0fbf8d..f0721af02 100644
--- a/drivers/input/touchscreen/ilitek_ts_i2c.c
+++ b/drivers/input/touchscreen/ilitek_ts_i2c.c
@@ -18,6 +18,7 @@
 #include <linux/gpio/consumer.h>
 #include <linux/errno.h>
 #include <linux/acpi.h>
+#include <linux/device/devres.h>
 #include <linux/input/touchscreen.h>
 #include <linux/unaligned.h>
 
@@ -37,6 +38,18 @@
 #define ILITEK_TP_CMD_GET_IC_MODE			0xC0
 
 #define ILITEK_TP_I2C_REPORT_ID				0x48
+/* Reverse engineered stylus report on a tested CHUWI Hi10 Max device. */
+#define ILITEK_PEN_I2C_REPORT_ID			0x0C
+#define ILITEK_PEN_PRESSURE_MAX			1023
+#define ILITEK_PEN_DISTANCE_MAX			2
+/* Userspace expects tablet axis resolution; with INPUT_PROP_DIRECT this is */
+/* mostly descriptive and does not materially affect event coordinates. */
+#define ILITEK_PEN_RESOLUTION			68
+
+#define ILITEK_PEN_FLAG_TOUCH			0x01
+#define ILITEK_PEN_FLAG_STYLUS2			0x02
+#define ILITEK_PEN_FLAG_STYLUS			0x08
+#define ILITEK_PEN_FLAG_PROX			0x10
 
 #define REPORT_COUNT_ADDRESS				61
 #define ILITEK_SUPPORT_MAX_POINT			40
@@ -50,6 +63,7 @@ struct ilitek_ts_data {
 	struct i2c_client		*client;
 	struct gpio_desc		*reset_gpio;
 	struct input_dev		*input_dev;
+	struct input_dev		*pen_input_dev;
 	struct touchscreen_properties	prop;
 
 	const struct ilitek_protocol_map *ptl_cb_func;
@@ -89,6 +103,9 @@ enum ilitek_cmds {
 	MAX_CMD_CNT
 };
 
+static int ilitek_pen_input_dev_init(struct device *dev,
+				      struct ilitek_ts_data *ts);
+
 /* ILITEK I2C R/W APIs */
 static int ilitek_i2c_write_and_read(struct ilitek_ts_data *ts,
 				     u8 *cmd, int write_len, int delay,
@@ -146,6 +163,62 @@ static void ilitek_touch_down(struct ilitek_ts_data *ts, unsigned int id,
 	touchscreen_report_pos(input, &ts->prop, x, y, true);
 }
 
+static void ilitek_unregister_pen_input(void *data)
+{
+	struct ilitek_ts_data *ts = data;
+
+	input_unregister_device(ts->pen_input_dev);
+	ts->pen_input_dev = NULL;
+}
+
+/*
+ * buf[1] carries prox/touch/side-button state and buf[6..7] carries
+ * pressure. A right shift by one matches the observed 1024 pressure levels.
+ */
+static int ilitek_process_pen_report(struct ilitek_ts_data *ts, u8 *buf)
+{
+	struct device *dev = &ts->client->dev;
+	struct input_dev *input = ts->pen_input_dev;
+	unsigned int x, y, z, distance;
+	bool prox, touch, stylus, stylus2;
+	int error;
+
+	if (!input) {
+		error = ilitek_pen_input_dev_init(dev, ts);
+		if (error) {
+			dev_err_ratelimited(dev,
+					    "failed to register pen input device: %d\n",
+					    error);
+			return 0;
+		}
+		input = ts->pen_input_dev;
+	}
+
+	x = get_unaligned_le16(buf + 2);
+	y = get_unaligned_le16(buf + 4);
+	z = get_unaligned_le16(buf + 6) >> 1;
+	prox = !!(buf[1] & ILITEK_PEN_FLAG_PROX);
+	touch = !!(buf[1] & ILITEK_PEN_FLAG_TOUCH);
+	stylus = !!(buf[1] & ILITEK_PEN_FLAG_STYLUS);
+	stylus2 = !!(buf[1] & ILITEK_PEN_FLAG_STYLUS2);
+	distance = prox ? (touch ? 0 : 1) : ILITEK_PEN_DISTANCE_MAX;
+	if (!touch)
+		z = 0;
+	else if (z > ILITEK_PEN_PRESSURE_MAX)
+		z = ILITEK_PEN_PRESSURE_MAX;
+
+	input_report_key(input, BTN_TOOL_PEN, prox || touch);
+	input_report_key(input, BTN_TOUCH, touch);
+	input_report_key(input, BTN_STYLUS, stylus);
+	input_report_key(input, BTN_STYLUS2, stylus2);
+	touchscreen_report_pos(input, &ts->prop, x, y, false);
+	input_report_abs(input, ABS_PRESSURE, z);
+	input_report_abs(input, ABS_DISTANCE, distance);
+	input_sync(input);
+
+	return 0;
+}
+
 static int ilitek_process_and_report_v6(struct ilitek_ts_data *ts)
 {
 	int error = 0;
@@ -164,6 +237,9 @@ static int ilitek_process_and_report_v6(struct ilitek_ts_data *ts)
 		return error;
 	}
 
+	if (buf[0] == ILITEK_PEN_I2C_REPORT_ID)
+		return ilitek_process_pen_report(ts, buf);
+
 	if (buf[0] != ILITEK_TP_I2C_REPORT_ID) {
 		dev_err(dev, "get touch info failed. Wrong id: 0x%02X\n", buf[0]);
 		return -EINVAL;
@@ -459,6 +535,60 @@ static int ilitek_read_tp_info(struct ilitek_ts_data *ts, bool boot)
 	return 0;
 }
 
+static int ilitek_pen_input_dev_init(struct device *dev, struct ilitek_ts_data *ts)
+{
+	struct input_dev *pen_input;
+	int error;
+
+	if (ts->pen_input_dev)
+		return 0;
+
+	/* No explicit pen capability probe is known; create on first pen report. */
+
+	pen_input = input_allocate_device();
+	if (!pen_input)
+		return -ENOMEM;
+
+	ts->pen_input_dev = pen_input;
+	pen_input->dev.parent = dev;
+	pen_input->name = "ilitek_ts_pen";
+	pen_input->id.bustype = BUS_I2C;
+
+	__set_bit(INPUT_PROP_DIRECT, pen_input->propbit);
+	__set_bit(EV_KEY, pen_input->evbit);
+	__set_bit(EV_ABS, pen_input->evbit);
+	__set_bit(BTN_TOUCH, pen_input->keybit);
+	__set_bit(BTN_TOOL_PEN, pen_input->keybit);
+	__set_bit(BTN_STYLUS, pen_input->keybit);
+	__set_bit(BTN_STYLUS2, pen_input->keybit);
+
+	input_set_abs_params(pen_input, ABS_X,
+			     ts->screen_min_x, ts->screen_max_x, 0, 0);
+	input_set_abs_params(pen_input, ABS_Y,
+			     ts->screen_min_y, ts->screen_max_y, 0, 0);
+	input_set_abs_params(pen_input, ABS_PRESSURE, 0,
+			     ILITEK_PEN_PRESSURE_MAX, 0, 0);
+	input_set_abs_params(pen_input, ABS_DISTANCE, 0,
+			     ILITEK_PEN_DISTANCE_MAX, 0, 0);
+	input_abs_set_res(pen_input, ABS_X, ILITEK_PEN_RESOLUTION);
+	input_abs_set_res(pen_input, ABS_Y, ILITEK_PEN_RESOLUTION);
+
+	error = input_register_device(pen_input);
+	if (error)
+		goto err_free_pen_input;
+
+	error = devm_add_action_or_reset(dev, ilitek_unregister_pen_input, ts);
+	if (error)
+		return error;
+
+	return 0;
+
+err_free_pen_input:
+	ts->pen_input_dev = NULL;
+	input_free_device(pen_input);
+	return error;
+}
+
 static int ilitek_input_dev_init(struct device *dev, struct ilitek_ts_data *ts)
 {
 	int error;
-- 
2.54.0

[PATCH 2/2] Input: ilitek_ts: ratelimit unexpected report logging

[Kristian Mide]

Unexpected report IDs can repeat quickly when the controller
emits unknown packets.

Use dev_err_ratelimited() and include the first 16 bytes of the
raw packet to keep dmesg usable while preserving enough context
for debugging.
---
 drivers/input/touchscreen/ilitek_ts_i2c.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/input/touchscreen/ilitek_ts_i2c.c b/drivers/input/touchscreen/ilitek_ts_i2c.c
index f0721af02..8886bd39b 100644
--- a/drivers/input/touchscreen/ilitek_ts_i2c.c
+++ b/drivers/input/touchscreen/ilitek_ts_i2c.c
@@ -241,7 +241,9 @@ static int ilitek_process_and_report_v6(struct ilitek_ts_data *ts)
 		return ilitek_process_pen_report(ts, buf);
 
 	if (buf[0] != ILITEK_TP_I2C_REPORT_ID) {
-		dev_err(dev, "get touch info failed. Wrong id: 0x%02X\n", buf[0]);
+		dev_err_ratelimited(dev,
+				    "get touch info failed. Wrong id: 0x%02X raw[0:16]=%*ph\n",
+				    buf[0], 16, buf);
 		return -EINVAL;
 	}
 
-- 
2.54.0

Re: [PATCH 2/2] Input: ilitek_ts: ratelimit unexpected report logging

[sashiko-bot]

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

New issues:
- [High] The I2C read/write wrapper does not properly check for short transfers, and this patch leaks the resulting uninitialized stack memory to dmesg.

Pre-existing issues:
- [High] The patch fails to prevent dmesg flooding and causes the interrupt handler to incorrectly signal a spurious interrupt, which can permanently disable the touchscreen.
- [High] Dynamically adding a devres action in a threaded IRQ handler creates an inverted teardown order during device unbind, leading to a memory leak and Use-After-Free.
- [High] If the I2C sleep command fails during suspend, the function returns an error without re-enabling the IRQ, rendering the touchscreen permanently unresponsive.
- [High] The driver passes stack-allocated arrays to i2c_transfer(), violating DMA mapping rules and risking memory corruption.
--

commit ec332d9cd1c7923c04e4eec0684513387a92348d
Author: Kristian Mide <kristian@mide.dk>

Input: ilitek_ts: ratelimit unexpected report logging

Unexpected report IDs can repeat quickly when the controller emits unknown
packets. This commit uses dev_err_ratelimited() and includes the first 16
bytes of the raw packet for context.

> diff --git a/drivers/input/touchscreen/ilitek_ts_i2c.c b/drivers/input/touchscreen/ilitek_ts_i2c.c
> index f0721af02adf2..8886bd39bc0b6 100644
> --- a/drivers/input/touchscreen/ilitek_ts_i2c.c
> +++ b/drivers/input/touchscreen/ilitek_ts_i2c.c
> @@ -241,7 +241,9 @@ static int ilitek_process_and_report_v6(struct ilitek_ts_data *ts)

[Severity: High]
This is a pre-existing issue, but does ilitek_process_and_report_v6() pass a
stack-allocated array to i2c_transfer()?

drivers/input/touchscreen/ilitek_ts_i2c.c:ilitek_process_and_report_v6() {
    ...
    u8 buf[512];
    ...
    error = ilitek_i2c_write_and_read(ts, NULL, 0, 0, buf, 64);

If the underlying I2C host controller driver maps the buffer for DMA, it might
attempt to map a stack address. On modern kernels where VMAP_STACK is enabled,
can this cause an immediate panic because stack memory is not physically
contiguous, or lead to cacheline sharing corruption otherwise?

>  		return ilitek_process_pen_report(ts, buf);

[Severity: High]
This is a pre-existing issue, but does dynamically adding a devres action
in the threaded IRQ handler create an inverted teardown order?

The IRQ is registered via devm_request_threaded_irq() in probe. Later, the
ISR dynamically calls devm_add_action_or_reset() to register the pen cleanup:

drivers/input/touchscreen/ilitek_ts_i2c.c:ilitek_process_pen_report() {
    ...
    if (!input) {
        error = ilitek_pen_input_dev_init(dev, ts);

which calls devm_add_action_or_reset(dev, ilitek_unregister_pen_input, ts).

Because devm is a LIFO stack, the pen cleanup is placed above the IRQ release
action. During device unbind, devres_release_all() will run the pen cleanup
first, freeing the device, while the IRQ remains active. If an interrupt fires
before the IRQ is finally freed, could the ISR access the freed device and
cause a use-after-free, or attempt to append a new devres action while the
stack is being torn down?

>  
>  	if (buf[0] != ILITEK_TP_I2C_REPORT_ID) {
> -		dev_err(dev, "get touch info failed. Wrong id: 0x%02X\n", buf[0]);
> +		dev_err_ratelimited(dev,
> +				    "get touch info failed. Wrong id: 0x%02X raw[0:16]=%*ph\n",
> +				    buf[0], 16, buf);

[Severity: High]
Does this new logging leak uninitialized kernel stack memory to dmesg?

Looking at ilitek_i2c_write_and_read(), it checks if error < 0, but
i2c_transfer() returns the number of successfully transferred messages:

drivers/input/touchscreen/ilitek_ts_i2c.c:ilitek_i2c_write_and_read() {
    ...
    error = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
    if (error < 0)
        return error;

If a short transfer occurs, this evaluates to false, so the wrapper returns 0
(success) despite the read buffer not being populated. When that happens,
won't this new log message directly print 16 bytes of uninitialized kernel
stack memory to the dmesg buffer?

>  		return -EINVAL;

[Severity: High]
This is a pre-existing issue, but does returning -EINVAL here cause the
interrupt handler to incorrectly signal a spurious interrupt?

When an unknown packet is received, ilitek_process_and_report_v6() returns
-EINVAL to the ISR, which then handles the error like this:

drivers/input/touchscreen/ilitek_ts_i2c.c:ilitek_i2c_isr() {
    ...
    error = ilitek_process_and_report_v6(ts);
    if (error < 0) {
        dev_err(&ts->client->dev, "[%s] err:%d\n", __func__, error);
        return IRQ_NONE;
    }

First, the ISR unconditionally logs the error, which defeats the ratelimiting
added in this patch. Second, could repeatedly returning IRQ_NONE when the
hardware interrupt was actually serviced (cleared by the I2C read) cause the
kernel's spurious interrupt detection to eventually disable the IRQ line
permanently?

>  	}
>  

[ ... ]

[Severity: High]
This is a pre-existing issue, but does ilitek_suspend() return an error
without re-enabling the previously disabled IRQ?

drivers/input/touchscreen/ilitek_ts_i2c.c:ilitek_suspend() {
    ...
    disable_irq(client->irq);

    if (!device_may_wakeup(dev)) {
        error = api_protocol_set_cmd(ts, SET_IC_SLEEP, NULL, NULL);
        if (error)
            return error;
    }

If the I2C sleep command fails during suspend, the function returns an error
while the device IRQ is disabled. The PM core will abort the suspend
transition and resume previously suspended devices, but it will not call the
resume callback for the device that returned the error. Could this render the
touchscreen permanently unresponsive until reboot or module reload?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260626214248.5563-1-kristian@mide.dk?part=2

Re: [PATCH 1/2] Input: ilitek_ts: add stylus input support

[sashiko-bot]

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

New issues:
- [High] Lazy initialization of the pen input device from within the IRQ handler causes a devres LIFO ordering inversion, leading to a memory leak and Use-After-Free during device unbind.
- [High] The pen input device axis limits are not synced with Device Tree properties, leading to mismatched axis boundaries and massive coordinate underflow values.
- [Low] Direct bit manipulation is used instead of the preferred `input_set_capability()` helper.

Pre-existing issues:
- [High] I2C transfers are performed using buffers allocated on the stack, which is unsafe for DMA.
--

commit 0912736a7a818d72cc6868aba6ad50c6f06afd78
Author: Kristian Mide <kristian@mide.dk>

Input: ilitek_ts: add stylus input support

Add a separate stylus input device for report ID 0x0c packets,
with pressure, hover, and side-button support. The pen device is
created lazily on first pen report so touchscreen-only hardware does
not expose stylus capabilities up front.

> diff --git a/drivers/input/touchscreen/ilitek_ts_i2c.c b/drivers/input/touchscreen/ilitek_ts_i2c.c
> index 3de0fbf8da386..f0721af02adf2 100644
> --- a/drivers/input/touchscreen/ilitek_ts_i2c.c
> +++ b/drivers/input/touchscreen/ilitek_ts_i2c.c

[ ... ]

> @@ -164,6 +237,9 @@ static int ilitek_process_and_report_v6(struct ilitek_ts_data *ts)
>  		return error;
>  	}
>  
> +	if (buf[0] == ILITEK_PEN_I2C_REPORT_ID)
> +		return ilitek_process_pen_report(ts, buf);

[Severity: High]
This is a pre-existing issue and was not introduced by the patch under
review, but since buf is passed here, is it safe to allocate buf as a
512-byte array on the stack in ilitek_process_and_report_v6() and pass it to
I2C transfer functions?

When CONFIG_VMAP_STACK is enabled, stack addresses cannot be safely mapped
for DMA. If the underlying I2C host controller uses DMA, could attempting
to map this stack buffer lead to memory corruption or kernel panics?

> +
>  	if (buf[0] != ILITEK_TP_I2C_REPORT_ID) {
>  		dev_err(dev, "get touch info failed. Wrong id: 0x%02X\n", buf[0]);
>  		return -EINVAL;

[ ... ]

> @@ -459,6 +535,60 @@ static int ilitek_read_tp_info(struct ilitek_ts_data *ts, bool boot)
>  	return 0;
>  }
>  
> +static int ilitek_pen_input_dev_init(struct device *dev, struct ilitek_ts_data *ts)
> +{
> +	struct input_dev *pen_input;
> +	int error;
> +
> +	if (ts->pen_input_dev)
> +		return 0;
> +
> +	/* No explicit pen capability probe is known; create on first pen report. */
> +
> +	pen_input = input_allocate_device();
> +	if (!pen_input)
> +		return -ENOMEM;
> +
> +	ts->pen_input_dev = pen_input;
> +	pen_input->dev.parent = dev;
> +	pen_input->name = "ilitek_ts_pen";
> +	pen_input->id.bustype = BUS_I2C;
> +
> +	__set_bit(INPUT_PROP_DIRECT, pen_input->propbit);
> +	__set_bit(EV_KEY, pen_input->evbit);
> +	__set_bit(EV_ABS, pen_input->evbit);
> +	__set_bit(BTN_TOUCH, pen_input->keybit);
> +	__set_bit(BTN_TOOL_PEN, pen_input->keybit);
> +	__set_bit(BTN_STYLUS, pen_input->keybit);
> +	__set_bit(BTN_STYLUS2, pen_input->keybit);

[Severity: Low]
This isn't a bug, but could we use the input_set_capability() helper here
instead of direct bit manipulation?

> +
> +	input_set_abs_params(pen_input, ABS_X,
> +			     ts->screen_min_x, ts->screen_max_x, 0, 0);
> +	input_set_abs_params(pen_input, ABS_Y,
> +			     ts->screen_min_y, ts->screen_max_y, 0, 0);

[Severity: High]
Should the touchscreen properties (DT properties) be applied to these
absolute parameters?

The touchscreen_report_pos() function called during event processing will
apply inversion and swapping based on Device Tree properties to the
coordinates.

If the limits advertised here are strictly from the raw hardware bounds and
are unaware of those DT properties, could this cause the device to report
out-of-bounds coordinates or result in unsigned underflows when an axis is
inverted?

> +	input_set_abs_params(pen_input, ABS_PRESSURE, 0,
> +			     ILITEK_PEN_PRESSURE_MAX, 0, 0);
> +	input_set_abs_params(pen_input, ABS_DISTANCE, 0,
> +			     ILITEK_PEN_DISTANCE_MAX, 0, 0);
> +	input_abs_set_res(pen_input, ABS_X, ILITEK_PEN_RESOLUTION);
> +	input_abs_set_res(pen_input, ABS_Y, ILITEK_PEN_RESOLUTION);
> +
> +	error = input_register_device(pen_input);
> +	if (error)
> +		goto err_free_pen_input;
> +
> +	error = devm_add_action_or_reset(dev, ilitek_unregister_pen_input, ts);

[Severity: High]
Since this function is called lazily from the threaded IRQ handler after
probe has completed, does adding a devm action here break the LIFO teardown
ordering?

During device unbind, devres will release resources in LIFO order. This
action would run first and set ts->pen_input_dev = NULL, but the IRQ
handler would still be active.

If a hardware interrupt fires during unbind after this has been
unregistered, would the IRQ handler observe the NULL pointer, allocate a
new pen device, and add a new devres action that devres_release_all()
will never process, resulting in a memory leak and a potential
use-after-free?

> +	if (error)
> +		return error;
> +
> +	return 0;
> +
> +err_free_pen_input:
> +	ts->pen_input_dev = NULL;
> +	input_free_device(pen_input);
> +	return error;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260626214248.5563-1-kristian@mide.dk?part=1