[PATCH] Input: zinitix - support the 6-byte/count touch report used by bt532

[PATCH] Input: zinitix - support the 6-byte/count touch report used by bt532

[Kavan Smith]

The driver assumes an 8-byte per-contact report stride with a per-finger
bitmask in report byte[2]. bt532 firmware (Samsung Galaxy Tab A 8.0,
msm8916-samsung-gt58) instead packs contacts at a 6-byte stride and reports
a finger COUNT in byte[2]. Under the 8-byte/bitmask assumption fingers 1+
are read misaligned, and a two-finger report (count 0x02) makes
for_each_set_bit() visit only slot 1, collapsing multitouch to a single
contact.

Add a per-compatible struct zinitix_chip_data that selects the report stride
and how byte[2] is interpreted, and attach it to the bt532 compatible (the
only in-tree user of it). All other compatibles keep the existing 8-byte,
bitmask behaviour unchanged. Contacts are parsed at the chip's stride, and
for count-style firmware every slot the chip marks with SUB_BIT_EXIST is
reported.

Signed-off-by: Kavan Smith <kavansmith82@gmail.com>
---
 drivers/input/touchscreen/zinitix.c | 71 +++++++++++++++++++++++++++--
 1 file changed, 58 insertions(+), 13 deletions(-)

diff --git a/drivers/input/touchscreen/zinitix.c b/drivers/input/touchscreen/zinitix.c
--- a/drivers/input/touchscreen/zinitix.c
+++ b/drivers/input/touchscreen/zinitix.c
@@ -135,26 +135,47 @@
 #define CHIP_ON_DELAY				15 // ms
 #define FIRMWARE_ON_DELAY			40 // ms
 
+/*
+ * Two Zinitix touch-report layouts exist in the wild. The default is an 8-byte
+ * per-contact stride with a per-finger bitmask in report byte[2]. Some firmware
+ * (e.g. bt532 on the Samsung Galaxy Tab A 8.0) packs contacts at a 6-byte stride
+ * and reports a finger COUNT in byte[2] instead of a bitmask. Selected per
+ * compatible via the of_device_id .data below.
+ */
+struct zinitix_chip_data {
+	unsigned int coord_stride;	/* per-contact byte stride in the report */
+	bool finger_count;		/* byte[2] is a finger count, not a bitmask */
+};
+
+static const struct zinitix_chip_data zinitix_bt532_data = {
+	.coord_stride = 6,
+	.finger_count = true,
+};
+
 struct point_coord {
 	__le16	x;
 	__le16	y;
 	u8	width;
 	u8	sub_status;
-	// currently unused, but needed as padding:
-	u8	minor_width;
-	u8	angle;
+	/*
+	 * 8-byte-stride firmware appends two unused bytes here (minor_width,
+	 * angle); the report is parsed at the chip's coord_stride, so on that
+	 * firmware they are simply skipped rather than being part of this struct.
+	 */
 };
 
 struct touch_event {
 	__le16	status;
 	u8	finger_mask;
 	u8	time_stamp;
-	struct point_coord point_coord[MAX_SUPPORTED_FINGER_NUM];
+	/* raw per-contact bytes, parsed at the chip's coord_stride (max 8) */
+	u8	coords[MAX_SUPPORTED_FINGER_NUM * 8];
 };
 
 struct bt541_ts_data {
 	struct i2c_client *client;
 	struct input_dev *input_dev;
+	const struct zinitix_chip_data *chip;
 	struct touchscreen_properties prop;
 	struct regulator_bulk_data supplies[2];
 	u32 zinitix_mode;
@@ -444,6 +465,8 @@
 {
 	struct bt541_ts_data *bt541 = bt541_handler;
 	struct i2c_client *client = bt541->client;
+	unsigned int stride = bt541->chip ? bt541->chip->coord_stride : 8;
+	bool finger_count = bt541->chip ? bt541->chip->finger_count : false;
 	struct touch_event touch_event;
 	unsigned long finger_mask;
 	__le16 icon_events;
@@ -452,8 +475,11 @@
 
 	memset(&touch_event, 0, sizeof(struct touch_event));
 
+	/* header (status, finger_mask, time_stamp) + one stride per contact */
 	error = zinitix_read_data(bt541->client, ZINITIX_POINT_STATUS_REG,
-				  &touch_event, sizeof(struct touch_event));
+				  &touch_event,
+				  offsetof(struct touch_event, coords) +
+					  MAX_SUPPORTED_FINGER_NUM * stride);
 	if (error) {
 		dev_err(&client->dev, "Failed to read in touchpoint struct\n");
 		goto out;
@@ -470,13 +496,31 @@
 		zinitix_report_keys(bt541, le16_to_cpu(icon_events));
 	}
 
-	finger_mask = touch_event.finger_mask;
-	for_each_set_bit(i, &finger_mask, MAX_SUPPORTED_FINGER_NUM) {
-		const struct point_coord *p = &touch_event.point_coord[i];
-
-		/* Only process contacts that are actually reported */
-		if (p->sub_status & SUB_BIT_EXIST)
-			zinitix_report_finger(bt541, i, p);
+	if (finger_count) {
+		/*
+		 * On this firmware byte[2] is a finger COUNT, not a bitmask: a
+		 * 2-finger report carries 0x02, so for_each_set_bit() would look
+		 * only at slot 1 and silently drop slot 0. The chip marks every
+		 * populated contact with SUB_BIT_EXIST, so iterate all slots and
+		 * report the present ones.
+		 */
+		for (i = 0; i < MAX_SUPPORTED_FINGER_NUM; i++) {
+			const struct point_coord *p =
+				(const struct point_coord *)&touch_event.coords[i * stride];
+
+			if (p->sub_status & SUB_BIT_EXIST)
+				zinitix_report_finger(bt541, i, p);
+		}
+	} else {
+		/* default: byte[2] is a per-finger bitmask */
+		finger_mask = touch_event.finger_mask;
+		for_each_set_bit(i, &finger_mask, MAX_SUPPORTED_FINGER_NUM) {
+			const struct point_coord *p =
+				(const struct point_coord *)&touch_event.coords[i * stride];
+
+			if (p->sub_status & SUB_BIT_EXIST)
+				zinitix_report_finger(bt541, i, p);
+		}
 	}
 
 	input_mt_sync_frame(bt541->input_dev);
@@ -627,6 +671,7 @@
 		return -ENOMEM;
 
 	bt541->client = client;
+	bt541->chip = device_get_match_data(&client->dev);
 	i2c_set_clientdata(client, bt541);
 
 	error = zinitix_init_regulators(bt541);
@@ -741,7 +786,7 @@
 	{ .compatible = "zinitix,bt431" },
 	{ .compatible = "zinitix,bt432" },
 	{ .compatible = "zinitix,bt531" },
-	{ .compatible = "zinitix,bt532" },
+	{ .compatible = "zinitix,bt532", .data = &zinitix_bt532_data },
 	{ .compatible = "zinitix,bt538" },
 	{ .compatible = "zinitix,bt541" },
 	{ .compatible = "zinitix,bt548" },

Re: [PATCH] Input: zinitix - support the 6-byte/count touch report used by bt532

[sashiko-bot]

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

Pre-existing issues:
- [High] The `zinitix,mode` property is read and validated after `input_register_device()` is called. This creates a race condition where userspace can open and use the input device before its private state is fully configured, violating input subsystem initialization rules.
- [High] I2C transactions use stack-allocated buffers, which are unsafe for DMA.
--

commit 25626131785e9a530425fa65d2a03db62641f88e
Author: Kavan Smith <kavansmith82@gmail.com>

Input: zinitix - support the 6-byte/count touch report used by bt532

This commit adds a per-compatible struct zinitix_chip_data to select
the report stride and how byte[2] is interpreted. It provides support
for firmware packing contacts at a 6-byte stride and reporting a finger
count, fixing multitouch behavior on devices like the Samsung Galaxy Tab
A 8.0.

> diff --git a/drivers/input/touchscreen/zinitix.c b/drivers/input/touchscreen/zinitix.c
> index 0c36765bd79f8..04ee4d804b511 100644
> --- a/drivers/input/touchscreen/zinitix.c
> +++ b/drivers/input/touchscreen/zinitix.c

[ ... ]

> @@ -452,8 +475,11 @@ static irqreturn_t zinitix_ts_irq_handler(int irq, void *bt541_handler)
>  
>  	memset(&touch_event, 0, sizeof(struct touch_event));
>  
> +	/* header (status, finger_mask, time_stamp) + one stride per contact */
>  	error = zinitix_read_data(bt541->client, ZINITIX_POINT_STATUS_REG,
> -				  &touch_event, sizeof(struct touch_event));
> +				  &touch_event,
> +				  offsetof(struct touch_event, coords) +
> +					  MAX_SUPPORTED_FINGER_NUM * stride);

[Severity: High]
This is a pre-existing issue, but does zinitix_read_data() perform safe I2C
transactions here?

The &touch_event pointer points to stack memory and is passed to
i2c_master_recv() inside zinitix_read_data(). Passing stack-allocated buffers
to I2C transfer functions can violate DMA safety rules. If the I2C host
controller driver maps these buffers for DMA, it can corrupt adjacent stack
variables on non-cache-coherent architectures.

This same issue seems to exist with &reg_le in zinitix_read_data() and the
packet array in zinitix_write_u16(). Would it be possible to allocate these
buffers dynamically to avoid potential DMA stack corruption?

[ ... ]

> @@ -627,6 +671,7 @@ static int zinitix_ts_probe(struct i2c_client *client)
>  		return -ENOMEM;
>  
>  	bt541->client = client;
> +	bt541->chip = device_get_match_data(&client->dev);
>  	i2c_set_clientdata(client, bt541);
>  
>  	error = zinitix_init_regulators(bt541);

[Severity: High]
This isn't a bug introduced by this patch, but looking further down in
zinitix_ts_probe(), zinitix_init_input_dev() is called to register the input
device before reading the zinitix,mode property:

	error = zinitix_init_input_dev(bt541);
	if (error) {
		dev_err(&client->dev,
			"Failed to initialize input device: %d\n", error);
		return error;
	}

	error = device_property_read_u32(&client->dev, "zinitix,mode",
					 &bt541->zinitix_mode);

Does this create a race condition where userspace could open and use the
input device before its private state is fully configured?

If userspace opens the device in this window, zinitix_input_open() might use
bt541->zinitix_mode while it is still 0 (from kzalloc). This could configure
the hardware in an invalid state instead of the device-tree specified mode.
Should the property be read and validated before calling
input_register_device()?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260706180826.408989-1-kavansmith82@gmail.com?part=1

Re: [PATCH] Input: zinitix - support the 6-byte/count touch report used by bt532

[Nikita Travkin]

Kavan Smith писал(а) 06.07.2026 23:08:
> The driver assumes an 8-byte per-contact report stride with a per-finger
> bitmask in report byte[2]. bt532 firmware (Samsung Galaxy Tab A 8.0,
> msm8916-samsung-gt58) instead packs contacts at a 6-byte stride and reports
> a finger COUNT in byte[2]. Under the 8-byte/bitmask assumption fingers 1+
> are read misaligned, and a two-finger report (count 0x02) makes
> for_each_set_bit() visit only slot 1, collapsing multitouch to a single
> contact.
> 
> Add a per-compatible struct zinitix_chip_data that selects the report stride
> and how byte[2] is interpreted, and attach it to the bt532 compatible (the
> only in-tree user of it). All other compatibles keep the existing 8-byte,
> bitmask behaviour unchanged. Contacts are parsed at the chip's stride, and
> for count-style firmware every slot the chip marks with SUB_BIT_EXIST is
> reported.
> 

Hi!

I don't think this approach is correct here. Since the layout is most
likely fw defined, I don't think it should be attached to the chip
compatible but rather to each specific device - oem driver has a
dedicated fw update just for gt58 for example.

Notably, the driver already has facilities to define which touch mode a
device uses, just that only one mode was implemented so far.

Please refer to the OEM driver [1], perhaps you will be able to notice
something simpler there that'd make this firmware accept mode=2...

See few more minor comments below

Nikita

[1] https://github.com/msm8916-mainline/linux-downstream/blob/SM-T355Y/drivers/input/touchscreen/zinitix_bt532/zinitix_bt532_ts.c

> Signed-off-by: Kavan Smith <kavansmith82@gmail.com>
> ---
>  drivers/input/touchscreen/zinitix.c | 71 +++++++++++++++++++++++++++--
>  1 file changed, 58 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/input/touchscreen/zinitix.c b/drivers/input/touchscreen/zinitix.c
> --- a/drivers/input/touchscreen/zinitix.c
> +++ b/drivers/input/touchscreen/zinitix.c
> @@ -135,26 +135,47 @@
>  #define CHIP_ON_DELAY				15 // ms
>  #define FIRMWARE_ON_DELAY			40 // ms
>  
> +/*
> + * Two Zinitix touch-report layouts exist in the wild. The default is an 8-byte
> + * per-contact stride with a per-finger bitmask in report byte[2]. Some firmware
> + * (e.g. bt532 on the Samsung Galaxy Tab A 8.0) packs contacts at a 6-byte stride
> + * and reports a finger COUNT in byte[2] instead of a bitmask. Selected per
> + * compatible via the of_device_id .data below.
> + */
> +struct zinitix_chip_data {
> +	unsigned int coord_stride;	/* per-contact byte stride in the report */
> +	bool finger_count;		/* byte[2] is a finger count, not a bitmask */
> +};
> +
> +static const struct zinitix_chip_data zinitix_bt532_data = {
> +	.coord_stride = 6,
> +	.finger_count = true,
> +};
> +
>  struct point_coord {
>  	__le16	x;
>  	__le16	y;
>  	u8	width;
>  	u8	sub_status;
> -	// currently unused, but needed as padding:
> -	u8	minor_width;
> -	u8	angle;
> +	/*
> +	 * 8-byte-stride firmware appends two unused bytes here (minor_width,
> +	 * angle); the report is parsed at the chip's coord_stride, so on that
> +	 * firmware they are simply skipped rather than being part of this struct.
> +	 */
>  };
>  
>  struct touch_event {
>  	__le16	status;
>  	u8	finger_mask;
>  	u8	time_stamp;
> -	struct point_coord point_coord[MAX_SUPPORTED_FINGER_NUM];
> +	/* raw per-contact bytes, parsed at the chip's coord_stride (max 8) */
> +	u8	coords[MAX_SUPPORTED_FINGER_NUM * 8];
>  };
>  
>  struct bt541_ts_data {
>  	struct i2c_client *client;
>  	struct input_dev *input_dev;
> +	const struct zinitix_chip_data *chip;
>  	struct touchscreen_properties prop;
>  	struct regulator_bulk_data supplies[2];
>  	u32 zinitix_mode;
> @@ -444,6 +465,8 @@
>  {
>  	struct bt541_ts_data *bt541 = bt541_handler;
>  	struct i2c_client *client = bt541->client;
> +	unsigned int stride = bt541->chip ? bt541->chip->coord_stride : 8;
> +	bool finger_count = bt541->chip ? bt541->chip->finger_count : false;
>  	struct touch_event touch_event;
>  	unsigned long finger_mask;
>  	__le16 icon_events;
> @@ -452,8 +475,11 @@
>  
>  	memset(&touch_event, 0, sizeof(struct touch_event));
>  
> +	/* header (status, finger_mask, time_stamp) + one stride per contact */
>  	error = zinitix_read_data(bt541->client, ZINITIX_POINT_STATUS_REG,
> -				  &touch_event, sizeof(struct touch_event));
> +				  &touch_event,
> +				  offsetof(struct touch_event, coords) +
> +					  MAX_SUPPORTED_FINGER_NUM * stride);
>  	if (error) {
>  		dev_err(&client->dev, "Failed to read in touchpoint struct\n");
>  		goto out;
> @@ -470,13 +496,31 @@
>  		zinitix_report_keys(bt541, le16_to_cpu(icon_events));
>  	}
>  
> -	finger_mask = touch_event.finger_mask;
> -	for_each_set_bit(i, &finger_mask, MAX_SUPPORTED_FINGER_NUM) {
> -		const struct point_coord *p = &touch_event.point_coord[i];
> -
> -		/* Only process contacts that are actually reported */
> -		if (p->sub_status & SUB_BIT_EXIST)
> -			zinitix_report_finger(bt541, i, p);
> +	if (finger_count) {
> +		/*
> +		 * On this firmware byte[2] is a finger COUNT, not a bitmask: a

Which firmware?

> +		 * 2-finger report carries 0x02, so for_each_set_bit() would look
> +		 * only at slot 1 and silently drop slot 0. The chip marks every
> +		 * populated contact with SUB_BIT_EXIST, so iterate all slots and
> +		 * report the present ones.
> +		 */

Maybe worth checking how OEM driver works for different devices
that use zinitix chips.

> +		for (i = 0; i < MAX_SUPPORTED_FINGER_NUM; i++) {
> +			const struct point_coord *p =
> +				(const struct point_coord *)&touch_event.coords[i * stride];
> +
> +			if (p->sub_status & SUB_BIT_EXIST)
> +				zinitix_report_finger(bt541, i, p);
> +		}
> +	} else {
> +		/* default: byte[2] is a per-finger bitmask */
> +		finger_mask = touch_event.finger_mask;
> +		for_each_set_bit(i, &finger_mask, MAX_SUPPORTED_FINGER_NUM) {
> +			const struct point_coord *p =
> +				(const struct point_coord *)&touch_event.coords[i * stride];
> +
> +			if (p->sub_status & SUB_BIT_EXIST)
> +				zinitix_report_finger(bt541, i, p);
> +		}
>  	}
>  
>  	input_mt_sync_frame(bt541->input_dev);
> @@ -627,6 +671,7 @@
>  		return -ENOMEM;
>  
>  	bt541->client = client;
> +	bt541->chip = device_get_match_data(&client->dev);
>  	i2c_set_clientdata(client, bt541);
>  
>  	error = zinitix_init_regulators(bt541);
> @@ -741,7 +786,7 @@
>  	{ .compatible = "zinitix,bt431" },
>  	{ .compatible = "zinitix,bt432" },
>  	{ .compatible = "zinitix,bt531" },
> -	{ .compatible = "zinitix,bt532" },
> +	{ .compatible = "zinitix,bt532", .data = &zinitix_bt532_data },
>  	{ .compatible = "zinitix,bt538" },
>  	{ .compatible = "zinitix,bt541" },
>  	{ .compatible = "zinitix,bt548" },