[PATCH 0/5] Minor Imagis driver refactoring and support for IST4050

[PATCH 0/5] Minor Imagis driver refactoring and support for IST4050

[Markuss Broks via B4 Relay]

The Imagis IST4050 is a touchscreen IC of the IST40xx family. It shares
the register interface, the power-up sequence and the coordinate format
with IST3038C, but reports touches differently: instead of a per-index
contact snapshot paired with a "pressed" bitmask in the interrupt
message, the controller exposes one record per contact, 8 bytes apart,
carrying a hardware-assigned touch ID and a press/release event type in
its upper byte. The bits which hold the contact area on IST3038C hold
the event type on these chips, and no contact area is available in the
coordinate word.

Patch 1 documents the IST4050 compatible. Patch 2 stops reporting
coordinates of released contacts; besides avoiding pointless events on
slots which have just been deactivated, this is required for IST40xx,
where release records must not be parsed for the contact area. Patch 3
replaces the protocol_b flag with an enum which describes each touch
reporting format, patch 4 adds support for the IST40xx reporting format
and patch 5 wires the chip up.

The series has been tested on IST4050 (Samsung Galaxy A70 SM-A705FN).
No functional change is intended for the already-supported chips,
except that coordinates are no longer reported for contacts being released.

Signed-off-by: Markuss Broks <markuss.broks@gmail.com>
---
Markuss Broks (5):
      dt-bindings: input: touchscreen: imagis: add compatible for IST4050
      Input: imagis - do not report coordinates of released contacts
      Input: imagis - replace the protocol_b flag with a protocol enum
      Input: imagis - add support for the IST40xx touch reporting format
      Input: imagis - add support for IST4050

 .../input/touchscreen/imagis,ist3038c.yaml         |  3 +-
 drivers/input/touchscreen/Kconfig                  |  2 +-
 drivers/input/touchscreen/imagis.c                 | 83 +++++++++++++++++++---
 3 files changed, 76 insertions(+), 12 deletions(-)
---
base-commit: 415606a7be939835db9b0d6b711887586646346d
change-id: 20260820-imagis-40xx-62fd11f8d7f8

Best regards,
--  
Markuss Broks <markuss.broks@gmail.com>

[PATCH 0/5] Minor Imagis driver refactoring and support for IST4050

[Markuss Broks]

The Imagis IST4050 is a touchscreen IC of the IST40xx family. It shares
the register interface, the power-up sequence and the coordinate format
with IST3038C, but reports touches differently: instead of a per-index
contact snapshot paired with a "pressed" bitmask in the interrupt
message, the controller exposes one record per contact, 8 bytes apart,
carrying a hardware-assigned touch ID and a press/release event type in
its upper byte. The bits which hold the contact area on IST3038C hold
the event type on these chips, and no contact area is available in the
coordinate word.

Patch 1 documents the IST4050 compatible. Patch 2 stops reporting
coordinates of released contacts; besides avoiding pointless events on
slots which have just been deactivated, this is required for IST40xx,
where release records must not be parsed for the contact area. Patch 3
replaces the protocol_b flag with an enum which describes each touch
reporting format, patch 4 adds support for the IST40xx reporting format
and patch 5 wires the chip up.

The series has been tested on IST4050 (Samsung Galaxy A70 SM-A705FN).
No functional change is intended for the already-supported chips,
except that coordinates are no longer reported for contacts being released.

Signed-off-by: Markuss Broks <markuss.broks@gmail.com>
---
Markuss Broks (5):
      dt-bindings: input: touchscreen: imagis: add compatible for IST4050
      Input: imagis - do not report coordinates of released contacts
      Input: imagis - replace the protocol_b flag with a protocol enum
      Input: imagis - add support for the IST40xx touch reporting format
      Input: imagis - add support for IST4050

 .../input/touchscreen/imagis,ist3038c.yaml         |  3 +-
 drivers/input/touchscreen/Kconfig                  |  2 +-
 drivers/input/touchscreen/imagis.c                 | 83 +++++++++++++++++++---
 3 files changed, 76 insertions(+), 12 deletions(-)
---
base-commit: 415606a7be939835db9b0d6b711887586646346d
change-id: 20260820-imagis-40xx-62fd11f8d7f8

Best regards,
--  
Markuss Broks <markuss.broks@gmail.com>

[PATCH 1/5] dt-bindings: input: touchscreen: imagis: add compatible for IST4050

[Markuss Broks via B4 Relay]

From: Markuss Broks <markuss.broks@gmail.com>

The IST4050 is a touchscreen IC of the Imagis IST40xx family. It shares
the register interface with IST3038C, but uses a different, event-based
touch reporting format and does not have touch keys.

The binding no longer covers only the IST30XXC family, so generalize its
title as well.

Signed-off-by: Markuss Broks <markuss.broks@gmail.com>
---
 .../devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml         | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml b/Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml
index dfaffbc398d3..a4f3e2f0cf45 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml
+++ b/Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml
@@ -4,7 +4,7 @@
 $id: http://devicetree.org/schemas/input/touchscreen/imagis,ist3038c.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
-title: Imagis IST30XXC family touchscreen controller
+title: Imagis touchscreen controller
 
 maintainers:
   - Markuss Broks <markuss.broks@gmail.com>
@@ -20,6 +20,7 @@ properties:
       - imagis,ist3038b
       - imagis,ist3038c
       - imagis,ist3038h
+      - imagis,ist4050
 
   reg:
     maxItems: 1

-- 
2.55.0

[PATCH 1/5] dt-bindings: input: touchscreen: imagis: add compatible for IST4050

[Markuss Broks]

The IST4050 is a touchscreen IC of the Imagis IST40xx family. It shares
the register interface with IST3038C, but uses a different, event-based
touch reporting format and does not have touch keys.

The binding no longer covers only the IST30XXC family, so generalize its
title as well.

Signed-off-by: Markuss Broks <markuss.broks@gmail.com>
---
 .../devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml         | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml b/Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml
index dfaffbc398d3..a4f3e2f0cf45 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml
+++ b/Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml
@@ -4,7 +4,7 @@
 $id: http://devicetree.org/schemas/input/touchscreen/imagis,ist3038c.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
-title: Imagis IST30XXC family touchscreen controller
+title: Imagis touchscreen controller
 
 maintainers:
   - Markuss Broks <markuss.broks@gmail.com>
@@ -20,6 +20,7 @@ properties:
       - imagis,ist3038b
       - imagis,ist3038c
       - imagis,ist3038h
+      - imagis,ist4050
 
   reg:
     maxItems: 1

-- 
2.55.0

[PATCH 2/5] Input: imagis - do not report coordinates of released contacts

[Markuss Broks via B4 Relay]

From: Markuss Broks <markuss.broks@gmail.com>

The position and the contact area are currently reported even for
contacts which are being released. The slot has already been deactivated
at that point, so these events carry no information for userspace.
Report the position and the contact area only for active contacts.

This also prepares the driver for the IST40xx family: its per-contact
records carry an event type in the bits which hold the contact area on
the currently supported chips, so release records must not be parsed for
coordinates or the contact area.

Signed-off-by: Markuss Broks <markuss.broks@gmail.com>
---
 drivers/input/touchscreen/imagis.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/input/touchscreen/imagis.c b/drivers/input/touchscreen/imagis.c
index 7bbb00beec3b..7cc91f97c06e 100644
--- a/drivers/input/touchscreen/imagis.c
+++ b/drivers/input/touchscreen/imagis.c
@@ -127,6 +127,8 @@ static irqreturn_t imagis_interrupt(int irq, void *dev_id)
 	finger_pressed = FIELD_GET(IST3038C_FINGER_STATUS_MASK, intr_message);
 
 	for (i = 0; i < finger_count; i++) {
+		bool pressed;
+
 		if (ts->tdata->protocol_b)
 			error = imagis_i2c_read_reg(ts,
 						    ts->tdata->touch_coord_cmd + (i * 4),
@@ -141,9 +143,13 @@ static irqreturn_t imagis_interrupt(int irq, void *dev_id)
 			goto out;
 		}
 
+		pressed = finger_pressed & BIT(i);
+
 		input_mt_slot(ts->input_dev, i);
-		input_mt_report_slot_state(ts->input_dev, MT_TOOL_FINGER,
-					   finger_pressed & BIT(i));
+		input_mt_report_slot_state(ts->input_dev, MT_TOOL_FINGER, pressed);
+		if (!pressed)
+			continue;
+
 		touchscreen_report_pos(ts->input_dev, &ts->prop,
 				       FIELD_GET(IST3038C_X_MASK, finger_status),
 				       FIELD_GET(IST3038C_Y_MASK, finger_status),

-- 
2.55.0

[PATCH 2/5] Input: imagis - do not report coordinates of released contacts

[Markuss Broks]

The position and the contact area are currently reported even for
contacts which are being released. The slot has already been deactivated
at that point, so these events carry no information for userspace.
Report the position and the contact area only for active contacts.

This also prepares the driver for the IST40xx family: its per-contact
records carry an event type in the bits which hold the contact area on
the currently supported chips, so release records must not be parsed for
coordinates or the contact area.

Signed-off-by: Markuss Broks <markuss.broks@gmail.com>
---
 drivers/input/touchscreen/imagis.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/input/touchscreen/imagis.c b/drivers/input/touchscreen/imagis.c
index 7bbb00beec3b..7cc91f97c06e 100644
--- a/drivers/input/touchscreen/imagis.c
+++ b/drivers/input/touchscreen/imagis.c
@@ -127,6 +127,8 @@ static irqreturn_t imagis_interrupt(int irq, void *dev_id)
 	finger_pressed = FIELD_GET(IST3038C_FINGER_STATUS_MASK, intr_message);
 
 	for (i = 0; i < finger_count; i++) {
+		bool pressed;
+
 		if (ts->tdata->protocol_b)
 			error = imagis_i2c_read_reg(ts,
 						    ts->tdata->touch_coord_cmd + (i * 4),
@@ -141,9 +143,13 @@ static irqreturn_t imagis_interrupt(int irq, void *dev_id)
 			goto out;
 		}
 
+		pressed = finger_pressed & BIT(i);
+
 		input_mt_slot(ts->input_dev, i);
-		input_mt_report_slot_state(ts->input_dev, MT_TOOL_FINGER,
-					   finger_pressed & BIT(i));
+		input_mt_report_slot_state(ts->input_dev, MT_TOOL_FINGER, pressed);
+		if (!pressed)
+			continue;
+
 		touchscreen_report_pos(ts->input_dev, &ts->prop,
 				       FIELD_GET(IST3038C_X_MASK, finger_status),
 				       FIELD_GET(IST3038C_Y_MASK, finger_status),

-- 
2.55.0

Re: [PATCH 2/5] Input: imagis - do not report coordinates of released contacts

[sashiko-bot]

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

Pre-existing issues:
- [High] DMA mapping of stack variables in I2C transfer.
- [High] Missing I2C client data initialization leads to a guaranteed NULL pointer dereference during system suspend.
- [High] Unconditionally returning IRQ_HANDLED on I2C failure can cause an interrupt storm.
--

--- Patch [2]: [PATCH 2/5] Input: imagis - do not report coordinates of released contacts ---
commit e577f0168e2b3c0834df658e8cd2f5b4436f2fcb
Author: Markuss Broks <markuss.broks@gmail.com>
Subject: Input: imagis - do not report coordinates of released contacts

This commit modifies the imagis driver to only report the position and
contact area for active contacts. It also prepares the driver for the
IST40xx family, which carries an event type in the contact area bits for
release records.

> diff --git a/drivers/input/touchscreen/imagis.c b/drivers/input/touchscreen/imagis.c
> index 7bbb00beec3be..7cc91f97c06e0 100644
> --- a/drivers/input/touchscreen/imagis.c
> +++ b/drivers/input/touchscreen/imagis.c

[ ... ]

> @@ -127,6 +127,8 @@ static irqreturn_t imagis_interrupt(int irq, void *dev_id)
>  	finger_pressed = FIELD_GET(IST3038C_FINGER_STATUS_MASK, intr_message);
>  
>  	for (i = 0; i < finger_count; i++) {
> +		bool pressed;
> +
>  		if (ts->tdata->protocol_b)
>  			error = imagis_i2c_read_reg(ts,

[Severity: High]
This isn't a bug introduced by this patch, but could this I2C read cause DMA
mapping issues?

In imagis_i2c_read_reg(), the variables reg_be and ret_be are allocated
on the stack:

    __be32 ret_be;
    __be32 reg_be = cpu_to_be32(reg);
    struct i2c_msg msg[] = {
        {
            .addr = ts->client->addr,
            .flags = 0,
            .buf = (unsigned char *)&reg_be,
            .len = sizeof(reg_be),
        },
        ...

Their addresses are passed directly to i2c_transfer(). If the underlying I2C
adapter relies on DMA and CONFIG_VMAP_STACK is enabled, these stack
addresses reside in vmalloc space and are not DMA-mappable, which can lead
to dma_map_single() failures or memory corruption.

>  						    ts->tdata->touch_coord_cmd + (i * 4),
> @@ -141,9 +143,13 @@ static irqreturn_t imagis_interrupt(int irq, void *dev_id)
>  			goto out;
>  		}

[Severity: High]
This is a pre-existing issue, but does this error path bypass the kernel's
spurious interrupt detector?

If imagis_i2c_read_reg() fails, the code jumps to the out label which
unconditionally returns IRQ_HANDLED.

If the controller fails to respond, the physical IRQ line might remain
asserted because the device status registers aren't cleared. By returning
IRQ_HANDLED instead of IRQ_NONE, the IRQ core will unmask the line and it
will immediately fire again, potentially creating an interrupt storm that
locks up the CPU core.

>  
> +		pressed = finger_pressed & BIT(i);
> +
>  		input_mt_slot(ts->input_dev, i);
> -		input_mt_report_slot_state(ts->input_dev, MT_TOOL_FINGER,
> -					   finger_pressed & BIT(i));
> +		input_mt_report_slot_state(ts->input_dev, MT_TOOL_FINGER, pressed);
> +		if (!pressed)
> +			continue;
> +
>  		touchscreen_report_pos(ts->input_dev, &ts->prop,
>  				       FIELD_GET(IST3038C_X_MASK, finger_status),
>  				       FIELD_GET(IST3038C_Y_MASK, finger_status),

[Severity: High]
This is a pre-existing issue, but could system suspend trigger a NULL pointer
dereference?

Looking at imagis_suspend(), it calls i2c_get_clientdata(client) to get the
driver data, but imagis_probe() never sets it via i2c_set_clientdata() or
dev_set_drvdata().

This means i2c_get_clientdata() will return NULL, and the very next line
will dereference it:

    struct imagis_ts *ts = i2c_get_clientdata(client);
    int error;

    guard(mutex)(&ts->input_dev->mutex);

Would it be possible to initialize the client data in imagis_probe() to
prevent this?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260822-imagis-40xx-v1-0-73e6d6cdf110@gmail.com?part=2

[PATCH 3/5] Input: imagis - replace the protocol_b flag with a protocol enum

[Markuss Broks via B4 Relay]

From: Markuss Broks <markuss.broks@gmail.com>

The driver tells the two supported touch reporting formats apart with
the protocol_b flag. The naming is unfortunate: "protocol B" is the
format of the IST30XXC series, while the B-suffixed chips use the
unnamed other format. Adding support for the IST40xx family is about to
introduce a third format, at which point a collection of booleans gets
easy to misuse.

Replace the flag with an enum which describes each reporting format:
IMAGIS_PROTOCOL_SHARED_REGISTER for the single coordinate register
shared by all contacts (IST3038, IST3038B and IST3038H), and
IMAGIS_PROTOCOL_PER_CONTACT_REGISTERS for one coordinate register per
contact (IST3032C and IST3038C).

No functional change intended.

Signed-off-by: Markuss Broks <markuss.broks@gmail.com>
---
 drivers/input/touchscreen/imagis.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/input/touchscreen/imagis.c b/drivers/input/touchscreen/imagis.c
index 7cc91f97c06e..6552d97efe32 100644
--- a/drivers/input/touchscreen/imagis.c
+++ b/drivers/input/touchscreen/imagis.c
@@ -44,12 +44,19 @@
 #define IST3038C_FINGER_STATUS_MASK	GENMASK(9, 0)
 #define IST3032C_KEY_STATUS_MASK	GENMASK(20, 16)
 
+enum imagis_protocol {
+	/* one coordinate register shared by all contacts */
+	IMAGIS_PROTOCOL_SHARED_REGISTER,
+	/* one coordinate register per contact */
+	IMAGIS_PROTOCOL_PER_CONTACT_REGISTERS,
+};
+
 struct imagis_properties {
 	unsigned int interrupt_msg_cmd;
 	unsigned int touch_coord_cmd;
 	unsigned int whoami_cmd;
 	unsigned int whoami_val;
-	bool protocol_b;
+	enum imagis_protocol protocol;
 	bool touch_keys_supported;
 };
 
@@ -129,7 +136,7 @@ static irqreturn_t imagis_interrupt(int irq, void *dev_id)
 	for (i = 0; i < finger_count; i++) {
 		bool pressed;
 
-		if (ts->tdata->protocol_b)
+		if (ts->tdata->protocol == IMAGIS_PROTOCOL_PER_CONTACT_REGISTERS)
 			error = imagis_i2c_read_reg(ts,
 						    ts->tdata->touch_coord_cmd + (i * 4),
 						    &finger_status);
@@ -410,8 +417,8 @@ static const struct imagis_properties imagis_3032c_data = {
 	.touch_coord_cmd = IST3038C_REG_TOUCH_COORD,
 	.whoami_cmd = IST3038C_REG_CHIPID,
 	.whoami_val = IST3032C_WHOAMI,
+	.protocol = IMAGIS_PROTOCOL_PER_CONTACT_REGISTERS,
 	.touch_keys_supported = true,
-	.protocol_b = true,
 };
 
 static const struct imagis_properties imagis_3038_data = {
@@ -419,6 +426,7 @@ static const struct imagis_properties imagis_3038_data = {
 	.touch_coord_cmd = IST30XX_REG_STATUS,
 	.whoami_cmd = IST30XX_REG_CHIPID,
 	.whoami_val = IST3038_WHOAMI,
+	.protocol = IMAGIS_PROTOCOL_SHARED_REGISTER,
 	.touch_keys_supported = true,
 };
 
@@ -427,6 +435,7 @@ static const struct imagis_properties imagis_3038b_data = {
 	.touch_coord_cmd = IST30XX_REG_STATUS,
 	.whoami_cmd = IST3038B_REG_CHIPID,
 	.whoami_val = IST3038B_WHOAMI,
+	.protocol = IMAGIS_PROTOCOL_SHARED_REGISTER,
 };
 
 static const struct imagis_properties imagis_3038c_data = {
@@ -434,7 +443,7 @@ static const struct imagis_properties imagis_3038c_data = {
 	.touch_coord_cmd = IST3038C_REG_TOUCH_COORD,
 	.whoami_cmd = IST3038C_REG_CHIPID,
 	.whoami_val = IST3038C_WHOAMI,
-	.protocol_b = true,
+	.protocol = IMAGIS_PROTOCOL_PER_CONTACT_REGISTERS,
 };
 
 static const struct imagis_properties imagis_3038h_data = {
@@ -442,6 +451,7 @@ static const struct imagis_properties imagis_3038h_data = {
 	.touch_coord_cmd = IST3038C_REG_TOUCH_COORD,
 	.whoami_cmd = IST3038C_REG_CHIPID,
 	.whoami_val = IST3038H_WHOAMI,
+	.protocol = IMAGIS_PROTOCOL_SHARED_REGISTER,
 };
 
 static const struct of_device_id imagis_of_match[] = {

-- 
2.55.0

[PATCH 3/5] Input: imagis - replace the protocol_b flag with a protocol enum

[Markuss Broks]

The driver tells the two supported touch reporting formats apart with
the protocol_b flag. The naming is unfortunate: "protocol B" is the
format of the IST30XXC series, while the B-suffixed chips use the
unnamed other format. Adding support for the IST40xx family is about to
introduce a third format, at which point a collection of booleans gets
easy to misuse.

Replace the flag with an enum which describes each reporting format:
IMAGIS_PROTOCOL_SHARED_REGISTER for the single coordinate register
shared by all contacts (IST3038, IST3038B and IST3038H), and
IMAGIS_PROTOCOL_PER_CONTACT_REGISTERS for one coordinate register per
contact (IST3032C and IST3038C).

No functional change intended.

Signed-off-by: Markuss Broks <markuss.broks@gmail.com>
---
 drivers/input/touchscreen/imagis.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/input/touchscreen/imagis.c b/drivers/input/touchscreen/imagis.c
index 7cc91f97c06e..6552d97efe32 100644
--- a/drivers/input/touchscreen/imagis.c
+++ b/drivers/input/touchscreen/imagis.c
@@ -44,12 +44,19 @@
 #define IST3038C_FINGER_STATUS_MASK	GENMASK(9, 0)
 #define IST3032C_KEY_STATUS_MASK	GENMASK(20, 16)
 
+enum imagis_protocol {
+	/* one coordinate register shared by all contacts */
+	IMAGIS_PROTOCOL_SHARED_REGISTER,
+	/* one coordinate register per contact */
+	IMAGIS_PROTOCOL_PER_CONTACT_REGISTERS,
+};
+
 struct imagis_properties {
 	unsigned int interrupt_msg_cmd;
 	unsigned int touch_coord_cmd;
 	unsigned int whoami_cmd;
 	unsigned int whoami_val;
-	bool protocol_b;
+	enum imagis_protocol protocol;
 	bool touch_keys_supported;
 };
 
@@ -129,7 +136,7 @@ static irqreturn_t imagis_interrupt(int irq, void *dev_id)
 	for (i = 0; i < finger_count; i++) {
 		bool pressed;
 
-		if (ts->tdata->protocol_b)
+		if (ts->tdata->protocol == IMAGIS_PROTOCOL_PER_CONTACT_REGISTERS)
 			error = imagis_i2c_read_reg(ts,
 						    ts->tdata->touch_coord_cmd + (i * 4),
 						    &finger_status);
@@ -410,8 +417,8 @@ static const struct imagis_properties imagis_3032c_data = {
 	.touch_coord_cmd = IST3038C_REG_TOUCH_COORD,
 	.whoami_cmd = IST3038C_REG_CHIPID,
 	.whoami_val = IST3032C_WHOAMI,
+	.protocol = IMAGIS_PROTOCOL_PER_CONTACT_REGISTERS,
 	.touch_keys_supported = true,
-	.protocol_b = true,
 };
 
 static const struct imagis_properties imagis_3038_data = {
@@ -419,6 +426,7 @@ static const struct imagis_properties imagis_3038_data = {
 	.touch_coord_cmd = IST30XX_REG_STATUS,
 	.whoami_cmd = IST30XX_REG_CHIPID,
 	.whoami_val = IST3038_WHOAMI,
+	.protocol = IMAGIS_PROTOCOL_SHARED_REGISTER,
 	.touch_keys_supported = true,
 };
 
@@ -427,6 +435,7 @@ static const struct imagis_properties imagis_3038b_data = {
 	.touch_coord_cmd = IST30XX_REG_STATUS,
 	.whoami_cmd = IST3038B_REG_CHIPID,
 	.whoami_val = IST3038B_WHOAMI,
+	.protocol = IMAGIS_PROTOCOL_SHARED_REGISTER,
 };
 
 static const struct imagis_properties imagis_3038c_data = {
@@ -434,7 +443,7 @@ static const struct imagis_properties imagis_3038c_data = {
 	.touch_coord_cmd = IST3038C_REG_TOUCH_COORD,
 	.whoami_cmd = IST3038C_REG_CHIPID,
 	.whoami_val = IST3038C_WHOAMI,
-	.protocol_b = true,
+	.protocol = IMAGIS_PROTOCOL_PER_CONTACT_REGISTERS,
 };
 
 static const struct imagis_properties imagis_3038h_data = {
@@ -442,6 +451,7 @@ static const struct imagis_properties imagis_3038h_data = {
 	.touch_coord_cmd = IST3038C_REG_TOUCH_COORD,
 	.whoami_cmd = IST3038C_REG_CHIPID,
 	.whoami_val = IST3038H_WHOAMI,
+	.protocol = IMAGIS_PROTOCOL_SHARED_REGISTER,
 };
 
 static const struct of_device_id imagis_of_match[] = {

-- 
2.55.0

Re: [PATCH 3/5] Input: imagis - replace the protocol_b flag with a protocol enum

[sashiko-bot]

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

Pre-existing issues:
- [High] Missing i2c_set_clientdata() in probe leads to a NULL pointer dereference during system suspend and resume.
--

--- Patch [3]: [PATCH 3/5] Input: imagis - replace the protocol_b flag with a protocol enum ---
commit 83084be7bf2e640e645cd6ac0416107ab34ef58c
Author: Markuss Broks <markuss.broks@gmail.com>
Subject: Input: imagis - replace the protocol_b flag with a protocol enum

This commit replaces the protocol_b flag in the imagis driver with a protocol
enum to describe each reporting format. The change clarifies the naming
convention and prevents a confusing collection of booleans as new chip formats
are added. No functional changes are intended.

> diff --git a/drivers/input/touchscreen/imagis.c b/drivers/input/touchscreen/imagis.c
> index 7cc91f97c06e0..6552d97efe32e 100644
> --- a/drivers/input/touchscreen/imagis.c
> +++ b/drivers/input/touchscreen/imagis.c
[ ... ]
> @@ -442,6 +451,7 @@ static const struct imagis_properties imagis_3038h_data = {
>  	.touch_coord_cmd = IST3038C_REG_TOUCH_COORD,
>  	.whoami_cmd = IST3038C_REG_CHIPID,
>  	.whoami_val = IST3038H_WHOAMI,
> +	.protocol = IMAGIS_PROTOCOL_SHARED_REGISTER,
>  };
>  
>  static const struct of_device_id imagis_of_match[] = {

[Severity: High]
This isn't a bug introduced by this patch, but could a missing
i2c_set_clientdata() in probe lead to a NULL pointer dereference during system
suspend and resume?

The imagis_probe() function dynamically allocates the imagis_ts structure but
never stores it in the device's driver data via i2c_set_clientdata().

When the system later suspends or resumes, imagis_suspend() and imagis_resume()
retrieve the client data using i2c_get_clientdata(), which would return NULL.
The code then immediately dereferences this NULL pointer:

drivers/input/touchscreen/imagis.c:imagis_suspend() {
	struct i2c_client *client = to_i2c_client(dev);
	struct imagis_ts *ts = i2c_get_clientdata(client);
	int error;

	guard(mutex)(&ts->input_dev->mutex);
...

Will this cause a kernel panic during suspend and resume cycles?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260822-imagis-40xx-v1-0-73e6d6cdf110@gmail.com?part=3

[PATCH 4/5] Input: imagis - add support for the IST40xx touch reporting format

[Markuss Broks via B4 Relay]

From: Markuss Broks <markuss.broks@gmail.com>

Chips of the Imagis IST40xx family share the register interface and the
coordinate format with IST3038C, but report touches differently: the
per-contact records are 8 bytes apart, and instead of pairing the record
index with a "pressed" bitmask from the interrupt message, each record
carries a hardware-assigned touch ID (1-based, bits 31:28) and an event
type (press or release, bits 27:24). The event type occupies the bits
which hold the contact area on IST3038C, and no contact area is
available in the coordinate word.

Add an IMAGIS_PROTOCOL_TOUCH_EVENTS variant for this format: derive the
contact slot from the touch ID instead of the record index, translate
press/release events into the contact state, ignore records with an
out-of-range touch ID or an unknown event type, and do not declare the
ABS_MT_TOUCH_MAJOR axis.

Signed-off-by: Markuss Broks <markuss.broks@gmail.com>
---
 drivers/input/touchscreen/imagis.c | 49 +++++++++++++++++++++++++++++++++-----
 1 file changed, 43 insertions(+), 6 deletions(-)

diff --git a/drivers/input/touchscreen/imagis.c b/drivers/input/touchscreen/imagis.c
index 6552d97efe32..5254cccdd653 100644
--- a/drivers/input/touchscreen/imagis.c
+++ b/drivers/input/touchscreen/imagis.c
@@ -44,11 +44,18 @@
 #define IST3038C_FINGER_STATUS_MASK	GENMASK(9, 0)
 #define IST3032C_KEY_STATUS_MASK	GENMASK(20, 16)
 
+#define IST40XX_TOUCH_ID_MASK		GENMASK(31, 28)
+#define IST40XX_TOUCH_EVENT_MASK	GENMASK(27, 24)
+#define IST40XX_TOUCH_EVENT_PRESS	1
+#define IST40XX_TOUCH_EVENT_RELEASE	2
+
 enum imagis_protocol {
 	/* one coordinate register shared by all contacts */
 	IMAGIS_PROTOCOL_SHARED_REGISTER,
 	/* one coordinate register per contact */
 	IMAGIS_PROTOCOL_PER_CONTACT_REGISTERS,
+	/* one record per contact, carrying a touch ID and an event type */
+	IMAGIS_PROTOCOL_TOUCH_EVENTS,
 };
 
 struct imagis_properties {
@@ -134,9 +141,14 @@ static irqreturn_t imagis_interrupt(int irq, void *dev_id)
 	finger_pressed = FIELD_GET(IST3038C_FINGER_STATUS_MASK, intr_message);
 
 	for (i = 0; i < finger_count; i++) {
+		unsigned int slot = i;
 		bool pressed;
 
-		if (ts->tdata->protocol == IMAGIS_PROTOCOL_PER_CONTACT_REGISTERS)
+		if (ts->tdata->protocol == IMAGIS_PROTOCOL_TOUCH_EVENTS)
+			error = imagis_i2c_read_reg(ts,
+						    ts->tdata->touch_coord_cmd + (i * 8),
+						    &finger_status);
+		else if (ts->tdata->protocol == IMAGIS_PROTOCOL_PER_CONTACT_REGISTERS)
 			error = imagis_i2c_read_reg(ts,
 						    ts->tdata->touch_coord_cmd + (i * 4),
 						    &finger_status);
@@ -150,9 +162,31 @@ static irqreturn_t imagis_interrupt(int irq, void *dev_id)
 			goto out;
 		}
 
-		pressed = finger_pressed & BIT(i);
+		if (ts->tdata->protocol == IMAGIS_PROTOCOL_TOUCH_EVENTS) {
+			unsigned int id = FIELD_GET(IST40XX_TOUCH_ID_MASK,
+						    finger_status);
+
+			if (!id || id > IST3038C_MAX_FINGER_NUM)
+				continue;
+
+			slot = id - 1;
+
+			switch (FIELD_GET(IST40XX_TOUCH_EVENT_MASK,
+					  finger_status)) {
+			case IST40XX_TOUCH_EVENT_PRESS:
+				pressed = true;
+				break;
+			case IST40XX_TOUCH_EVENT_RELEASE:
+				pressed = false;
+				break;
+			default:
+				continue;
+			}
+		} else {
+			pressed = finger_pressed & BIT(i);
+		}
 
-		input_mt_slot(ts->input_dev, i);
+		input_mt_slot(ts->input_dev, slot);
 		input_mt_report_slot_state(ts->input_dev, MT_TOOL_FINGER, pressed);
 		if (!pressed)
 			continue;
@@ -161,8 +195,10 @@ static irqreturn_t imagis_interrupt(int irq, void *dev_id)
 				       FIELD_GET(IST3038C_X_MASK, finger_status),
 				       FIELD_GET(IST3038C_Y_MASK, finger_status),
 				       true);
-		input_report_abs(ts->input_dev, ABS_MT_TOUCH_MAJOR,
-				 FIELD_GET(IST3038C_AREA_MASK, finger_status));
+		if (ts->tdata->protocol != IMAGIS_PROTOCOL_TOUCH_EVENTS)
+			input_report_abs(ts->input_dev, ABS_MT_TOUCH_MAJOR,
+					 FIELD_GET(IST3038C_AREA_MASK,
+						   finger_status));
 	}
 
 	key_pressed = FIELD_GET(IST3032C_KEY_STATUS_MASK, intr_message);
@@ -255,7 +291,8 @@ static int imagis_init_input_dev(struct imagis_ts *ts)
 
 	input_set_capability(input_dev, EV_ABS, ABS_MT_POSITION_X);
 	input_set_capability(input_dev, EV_ABS, ABS_MT_POSITION_Y);
-	input_set_abs_params(input_dev, ABS_MT_TOUCH_MAJOR, 0, 16, 0, 0);
+	if (ts->tdata->protocol != IMAGIS_PROTOCOL_TOUCH_EVENTS)
+		input_set_abs_params(input_dev, ABS_MT_TOUCH_MAJOR, 0, 16, 0, 0);
 	if (ts->tdata->touch_keys_supported) {
 		ts->num_keycodes = of_property_read_variable_u32_array(
 				ts->client->dev.of_node, "linux,keycodes",

-- 
2.55.0

[PATCH 4/5] Input: imagis - add support for the IST40xx touch reporting format

[Markuss Broks]

Chips of the Imagis IST40xx family share the register interface and the
coordinate format with IST3038C, but report touches differently: the
per-contact records are 8 bytes apart, and instead of pairing the record
index with a "pressed" bitmask from the interrupt message, each record
carries a hardware-assigned touch ID (1-based, bits 31:28) and an event
type (press or release, bits 27:24). The event type occupies the bits
which hold the contact area on IST3038C, and no contact area is
available in the coordinate word.

Add an IMAGIS_PROTOCOL_TOUCH_EVENTS variant for this format: derive the
contact slot from the touch ID instead of the record index, translate
press/release events into the contact state, ignore records with an
out-of-range touch ID or an unknown event type, and do not declare the
ABS_MT_TOUCH_MAJOR axis.

Signed-off-by: Markuss Broks <markuss.broks@gmail.com>
---
 drivers/input/touchscreen/imagis.c | 49 +++++++++++++++++++++++++++++++++-----
 1 file changed, 43 insertions(+), 6 deletions(-)

diff --git a/drivers/input/touchscreen/imagis.c b/drivers/input/touchscreen/imagis.c
index 6552d97efe32..5254cccdd653 100644
--- a/drivers/input/touchscreen/imagis.c
+++ b/drivers/input/touchscreen/imagis.c
@@ -44,11 +44,18 @@
 #define IST3038C_FINGER_STATUS_MASK	GENMASK(9, 0)
 #define IST3032C_KEY_STATUS_MASK	GENMASK(20, 16)
 
+#define IST40XX_TOUCH_ID_MASK		GENMASK(31, 28)
+#define IST40XX_TOUCH_EVENT_MASK	GENMASK(27, 24)
+#define IST40XX_TOUCH_EVENT_PRESS	1
+#define IST40XX_TOUCH_EVENT_RELEASE	2
+
 enum imagis_protocol {
 	/* one coordinate register shared by all contacts */
 	IMAGIS_PROTOCOL_SHARED_REGISTER,
 	/* one coordinate register per contact */
 	IMAGIS_PROTOCOL_PER_CONTACT_REGISTERS,
+	/* one record per contact, carrying a touch ID and an event type */
+	IMAGIS_PROTOCOL_TOUCH_EVENTS,
 };
 
 struct imagis_properties {
@@ -134,9 +141,14 @@ static irqreturn_t imagis_interrupt(int irq, void *dev_id)
 	finger_pressed = FIELD_GET(IST3038C_FINGER_STATUS_MASK, intr_message);
 
 	for (i = 0; i < finger_count; i++) {
+		unsigned int slot = i;
 		bool pressed;
 
-		if (ts->tdata->protocol == IMAGIS_PROTOCOL_PER_CONTACT_REGISTERS)
+		if (ts->tdata->protocol == IMAGIS_PROTOCOL_TOUCH_EVENTS)
+			error = imagis_i2c_read_reg(ts,
+						    ts->tdata->touch_coord_cmd + (i * 8),
+						    &finger_status);
+		else if (ts->tdata->protocol == IMAGIS_PROTOCOL_PER_CONTACT_REGISTERS)
 			error = imagis_i2c_read_reg(ts,
 						    ts->tdata->touch_coord_cmd + (i * 4),
 						    &finger_status);
@@ -150,9 +162,31 @@ static irqreturn_t imagis_interrupt(int irq, void *dev_id)
 			goto out;
 		}
 
-		pressed = finger_pressed & BIT(i);
+		if (ts->tdata->protocol == IMAGIS_PROTOCOL_TOUCH_EVENTS) {
+			unsigned int id = FIELD_GET(IST40XX_TOUCH_ID_MASK,
+						    finger_status);
+
+			if (!id || id > IST3038C_MAX_FINGER_NUM)
+				continue;
+
+			slot = id - 1;
+
+			switch (FIELD_GET(IST40XX_TOUCH_EVENT_MASK,
+					  finger_status)) {
+			case IST40XX_TOUCH_EVENT_PRESS:
+				pressed = true;
+				break;
+			case IST40XX_TOUCH_EVENT_RELEASE:
+				pressed = false;
+				break;
+			default:
+				continue;
+			}
+		} else {
+			pressed = finger_pressed & BIT(i);
+		}
 
-		input_mt_slot(ts->input_dev, i);
+		input_mt_slot(ts->input_dev, slot);
 		input_mt_report_slot_state(ts->input_dev, MT_TOOL_FINGER, pressed);
 		if (!pressed)
 			continue;
@@ -161,8 +195,10 @@ static irqreturn_t imagis_interrupt(int irq, void *dev_id)
 				       FIELD_GET(IST3038C_X_MASK, finger_status),
 				       FIELD_GET(IST3038C_Y_MASK, finger_status),
 				       true);
-		input_report_abs(ts->input_dev, ABS_MT_TOUCH_MAJOR,
-				 FIELD_GET(IST3038C_AREA_MASK, finger_status));
+		if (ts->tdata->protocol != IMAGIS_PROTOCOL_TOUCH_EVENTS)
+			input_report_abs(ts->input_dev, ABS_MT_TOUCH_MAJOR,
+					 FIELD_GET(IST3038C_AREA_MASK,
+						   finger_status));
 	}
 
 	key_pressed = FIELD_GET(IST3032C_KEY_STATUS_MASK, intr_message);
@@ -255,7 +291,8 @@ static int imagis_init_input_dev(struct imagis_ts *ts)
 
 	input_set_capability(input_dev, EV_ABS, ABS_MT_POSITION_X);
 	input_set_capability(input_dev, EV_ABS, ABS_MT_POSITION_Y);
-	input_set_abs_params(input_dev, ABS_MT_TOUCH_MAJOR, 0, 16, 0, 0);
+	if (ts->tdata->protocol != IMAGIS_PROTOCOL_TOUCH_EVENTS)
+		input_set_abs_params(input_dev, ABS_MT_TOUCH_MAJOR, 0, 16, 0, 0);
 	if (ts->tdata->touch_keys_supported) {
 		ts->num_keycodes = of_property_read_variable_u32_array(
 				ts->client->dev.of_node, "linux,keycodes",

-- 
2.55.0

Re: [PATCH 4/5] Input: imagis - add support for the IST40xx touch reporting format

[sashiko-bot]

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

New issues:
- [High] Unconditional use of INPUT_MT_DROP_UNUSED breaks multitouch tracking for the new event-based protocol.

Pre-existing issues:
- [High] Missing i2c_set_clientdata() in probe leads to a NULL pointer dereference during suspend/resume.
--

--- Patch [4]: [PATCH 4/5] Input: imagis - add support for the IST40xx touch reporting format ---
commit 5561fc562e0ffc99ddfa7c21852a4e2ad13ae0e3
Author: Markuss Broks <markuss.broks@gmail.com>

Input: imagis - add support for the IST40xx touch reporting format

This commit adds support for the IST40xx family to the Imagis touchscreen
driver. It introduces the IMAGIS_PROTOCOL_TOUCH_EVENTS protocol which 
derives contact slots from hardware-assigned touch IDs and translates
press/release events into contact state.

> diff --git a/drivers/input/touchscreen/imagis.c b/drivers/input/touchscreen/imagis.c
> index 6552d97efe32e..5254cccdd653e 100644
> --- a/drivers/input/touchscreen/imagis.c
> +++ b/drivers/input/touchscreen/imagis.c
[ ... ]
> @@ -150,9 +162,31 @@ static irqreturn_t imagis_interrupt(int irq, void *dev_id)
>  			goto out;
>  		}
>  
> -		pressed = finger_pressed & BIT(i);
> +		if (ts->tdata->protocol == IMAGIS_PROTOCOL_TOUCH_EVENTS) {
> +			unsigned int id = FIELD_GET(IST40XX_TOUCH_ID_MASK,
> +						    finger_status);
> +
> +			if (!id || id > IST3038C_MAX_FINGER_NUM)
> +				continue;
> +
> +			slot = id - 1;
> +
> +			switch (FIELD_GET(IST40XX_TOUCH_EVENT_MASK,
> +					  finger_status)) {
> +			case IST40XX_TOUCH_EVENT_PRESS:
> +				pressed = true;
> +				break;
> +			case IST40XX_TOUCH_EVENT_RELEASE:
> +				pressed = false;
> +				break;
> +			default:
> +				continue;

[Severity: High]
Will this continue cause active, stationary contacts to be erroneously dropped?

When using the new IMAGIS_PROTOCOL_TOUCH_EVENTS protocol, records with an
unrecognized event type are skipped here. Additionally, if the hardware omits
stationary contacts from the event stream entirely, they will not be processed
in this loop.

Because imagis_init_input_dev() unconditionally initializes MT slots with
INPUT_MT_DROP_UNUSED:

	error = input_mt_init_slots(input_dev, IST3038C_MAX_FINGER_NUM,
				    INPUT_MT_DIRECT | INPUT_MT_DROP_UNUSED);

If a slot is not explicitly updated with input_mt_report_slot_state() during
the frame, won't input_mt_sync_frame() incorrectly release these valid,
skipped contacts and break multitouch tracking?

> +			}
> +		} else {
> +			pressed = finger_pressed & BIT(i);
> +		}
>  
> -		input_mt_slot(ts->input_dev, i);
> +		input_mt_slot(ts->input_dev, slot);
>  		input_mt_report_slot_state(ts->input_dev, MT_TOOL_FINGER, pressed);
>  		if (!pressed)
>  			continue;

[Severity: High]
This is a pre-existing issue, but does the driver fail to set the I2C client
data during probe?

Looking at imagis_probe(), the private ts structure is allocated but never
attached to the I2C client device via i2c_set_clientdata().

Later, during suspend/resume:

imagis_suspend() {
	struct imagis_ts *ts = i2c_get_clientdata(client);
	...
	guard(mutex)(&ts->input_dev->mutex);
}

Since i2c_get_clientdata() will return NULL because it was never set, won't
this cause a NULL pointer dereference panic on suspend or resume?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260822-imagis-40xx-v1-0-73e6d6cdf110@gmail.com?part=4

[PATCH 5/5] Input: imagis - add support for IST4050

[Markuss Broks via B4 Relay]

From: Markuss Broks <markuss.broks@gmail.com>

The Imagis IST4050 shares the register interface and the power-up
sequence with IST3038C, and reports touches using the IST40xx touch
reporting format. Its chip ID register reads back 0x4050.

Also mention the IST40xx family in the Kconfig help text, and refer to
the older chips as IST30xx, since the driver supports more than the C
variants.

Signed-off-by: Markuss Broks <markuss.broks@gmail.com>
---
 drivers/input/touchscreen/Kconfig  |  2 +-
 drivers/input/touchscreen/imagis.c | 10 ++++++++++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index 9b9ae8ac3f7f..96fdc4384a75 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -705,7 +705,7 @@ config TOUCHSCREEN_IMAGIS
 	tristate "Imagis touchscreen support"
 	depends on I2C
 	help
-	  Say Y here if you have an Imagis IST30xxC touchscreen.
+	  Say Y here if you have an Imagis IST30xx or IST40xx touchscreen.
 	  If unsure, say N.
 
 	  To compile this driver as a module, choose M here: the
diff --git a/drivers/input/touchscreen/imagis.c b/drivers/input/touchscreen/imagis.c
index 5254cccdd653..9bf5afd43848 100644
--- a/drivers/input/touchscreen/imagis.c
+++ b/drivers/input/touchscreen/imagis.c
@@ -23,6 +23,7 @@
 #define IST3032C_WHOAMI			0x32c
 #define IST3038C_WHOAMI			0x38c
 #define IST3038H_WHOAMI			0x38d
+#define IST4050_WHOAMI			0x4050
 
 #define IST3038B_REG_CHIPID		0x30
 #define IST3038B_WHOAMI			0x30380b
@@ -491,12 +492,21 @@ static const struct imagis_properties imagis_3038h_data = {
 	.protocol = IMAGIS_PROTOCOL_SHARED_REGISTER,
 };
 
+static const struct imagis_properties imagis_4050_data = {
+	.interrupt_msg_cmd = IST3038C_REG_INTR_MESSAGE,
+	.touch_coord_cmd = IST3038C_REG_TOUCH_COORD,
+	.whoami_cmd = IST3038C_REG_CHIPID,
+	.whoami_val = IST4050_WHOAMI,
+	.protocol = IMAGIS_PROTOCOL_TOUCH_EVENTS,
+};
+
 static const struct of_device_id imagis_of_match[] = {
 	{ .compatible = "imagis,ist3032c", .data = &imagis_3032c_data },
 	{ .compatible = "imagis,ist3038", .data = &imagis_3038_data },
 	{ .compatible = "imagis,ist3038b", .data = &imagis_3038b_data },
 	{ .compatible = "imagis,ist3038c", .data = &imagis_3038c_data },
 	{ .compatible = "imagis,ist3038h", .data = &imagis_3038h_data },
+	{ .compatible = "imagis,ist4050", .data = &imagis_4050_data },
 	{ },
 };
 MODULE_DEVICE_TABLE(of, imagis_of_match);

-- 
2.55.0

[PATCH 5/5] Input: imagis - add support for IST4050

[Markuss Broks]

The Imagis IST4050 shares the register interface and the power-up
sequence with IST3038C, and reports touches using the IST40xx touch
reporting format. Its chip ID register reads back 0x4050.

Also mention the IST40xx family in the Kconfig help text, and refer to
the older chips as IST30xx, since the driver supports more than the C
variants.

Signed-off-by: Markuss Broks <markuss.broks@gmail.com>
---
 drivers/input/touchscreen/Kconfig  |  2 +-
 drivers/input/touchscreen/imagis.c | 10 ++++++++++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index 9b9ae8ac3f7f..96fdc4384a75 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -705,7 +705,7 @@ config TOUCHSCREEN_IMAGIS
 	tristate "Imagis touchscreen support"
 	depends on I2C
 	help
-	  Say Y here if you have an Imagis IST30xxC touchscreen.
+	  Say Y here if you have an Imagis IST30xx or IST40xx touchscreen.
 	  If unsure, say N.
 
 	  To compile this driver as a module, choose M here: the
diff --git a/drivers/input/touchscreen/imagis.c b/drivers/input/touchscreen/imagis.c
index 5254cccdd653..9bf5afd43848 100644
--- a/drivers/input/touchscreen/imagis.c
+++ b/drivers/input/touchscreen/imagis.c
@@ -23,6 +23,7 @@
 #define IST3032C_WHOAMI			0x32c
 #define IST3038C_WHOAMI			0x38c
 #define IST3038H_WHOAMI			0x38d
+#define IST4050_WHOAMI			0x4050
 
 #define IST3038B_REG_CHIPID		0x30
 #define IST3038B_WHOAMI			0x30380b
@@ -491,12 +492,21 @@ static const struct imagis_properties imagis_3038h_data = {
 	.protocol = IMAGIS_PROTOCOL_SHARED_REGISTER,
 };
 
+static const struct imagis_properties imagis_4050_data = {
+	.interrupt_msg_cmd = IST3038C_REG_INTR_MESSAGE,
+	.touch_coord_cmd = IST3038C_REG_TOUCH_COORD,
+	.whoami_cmd = IST3038C_REG_CHIPID,
+	.whoami_val = IST4050_WHOAMI,
+	.protocol = IMAGIS_PROTOCOL_TOUCH_EVENTS,
+};
+
 static const struct of_device_id imagis_of_match[] = {
 	{ .compatible = "imagis,ist3032c", .data = &imagis_3032c_data },
 	{ .compatible = "imagis,ist3038", .data = &imagis_3038_data },
 	{ .compatible = "imagis,ist3038b", .data = &imagis_3038b_data },
 	{ .compatible = "imagis,ist3038c", .data = &imagis_3038c_data },
 	{ .compatible = "imagis,ist3038h", .data = &imagis_3038h_data },
+	{ .compatible = "imagis,ist4050", .data = &imagis_4050_data },
 	{ },
 };
 MODULE_DEVICE_TABLE(of, imagis_of_match);

-- 
2.55.0

Re: [PATCH 5/5] Input: imagis - add support for IST4050

[sashiko-bot]

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

New issues:
- [High] The IMAGIS_PROTOCOL_TOUCH_EVENTS protocol handling drops active contacts due to incompatibility with INPUT_MT_DROP_UNUSED.

Pre-existing issues:
- [High] Missing i2c_set_clientdata() in probe causes guaranteed NULL pointer dereference during system suspend/resume.
- [High] Stack-allocated buffers are passed to I2C transfers, violating DMA constraints and potentially failing device probe.
--

--- Patch [5]: [PATCH 5/5] Input: imagis - add support for IST4050 ---
commit cea4b151bd39f3f451e6ebe48a6b0a29508b291a
Author: Markuss Broks <markuss.broks@gmail.com>

Input: imagis - add support for IST4050

The Imagis IST4050 shares the register interface and the power-up
sequence with IST3038C, and reports touches using the IST40xx touch
reporting format. Its chip ID register reads back 0x4050.

Also mention the IST40xx family in the Kconfig help text, and refer to
the older chips as IST30xx, since the driver supports more than the C
variants.

> diff --git a/drivers/input/touchscreen/imagis.c b/drivers/input/touchscreen/imagis.c
> index 5254cccdd653e..9bf5afd438488 100644
> --- a/drivers/input/touchscreen/imagis.c
> +++ b/drivers/input/touchscreen/imagis.c
[ ... ]
> @@ -491,12 +492,21 @@ static const struct imagis_properties imagis_3038h_data = {
>  	.protocol = IMAGIS_PROTOCOL_SHARED_REGISTER,
>  };
>  
> +static const struct imagis_properties imagis_4050_data = {
> +	.interrupt_msg_cmd = IST3038C_REG_INTR_MESSAGE,
> +	.touch_coord_cmd = IST3038C_REG_TOUCH_COORD,
> +	.whoami_cmd = IST3038C_REG_CHIPID,
> +	.whoami_val = IST4050_WHOAMI,
> +	.protocol = IMAGIS_PROTOCOL_TOUCH_EVENTS,

[Severity: High]
Does the IMAGIS_PROTOCOL_TOUCH_EVENTS protocol drop active contacts?

In imagis_interrupt(), when processing the IST4050 event stream, the driver
extracts an event type. If the hardware sends an event type other than
PRESS or RELEASE (such as a MOVE event), the switch statement hits the
default case:

drivers/input/touchscreen/imagis.c:imagis_interrupt() {
    ...
			switch (FIELD_GET(IST40XX_TOUCH_EVENT_MASK,
					  finger_status)) {
			case IST40XX_TOUCH_EVENT_PRESS:
				pressed = true;
				break;
			case IST40XX_TOUCH_EVENT_RELEASE:
				pressed = false;
				break;
			default:
				continue;
			}
    ...
}

This silently skips the input_mt_slot() update. Also, if the hardware
only reports contacts with changes, unchanged stationary contacts will
not appear in the loop.

Because input_mt_init_slots() is configured with INPUT_MT_DROP_UNUSED:

drivers/input/touchscreen/imagis.c:imagis_init_input_dev() {
    ...
	error = input_mt_init_slots(input_dev,
				    IST3038C_MAX_FINGER_NUM,
				    INPUT_MT_DIRECT | INPUT_MT_DROP_UNUSED);
    ...
}

input_mt_sync_frame() will unconditionally drop any previously active
contact that is not explicitly updated in the current frame, which would
break touch tracking for moving or stationary contacts.

[Severity: High]
This is a pre-existing issue, but I noticed imagis_probe() allocates the
context but never assigns it to the device client data:

drivers/input/touchscreen/imagis.c:imagis_probe() {
    ...
	ts = devm_kzalloc(dev, sizeof(*ts), GFP_KERNEL);
	if (!ts)
		return -ENOMEM;

	ts->client = i2c;
    ...
}

When the system suspends, imagis_suspend() retrieves an uninitialized
NULL pointer and dereferences it:

drivers/input/touchscreen/imagis.c:imagis_suspend() {
    ...
	struct i2c_client *client = to_i2c_client(dev);
	struct imagis_ts *ts = i2c_get_clientdata(client);
	int error;

	guard(mutex)(&ts->input_dev->mutex);
    ...
}

Can this lead to a guaranteed kernel panic during system suspend and
resume cycles?

[Severity: High]
This is a pre-existing issue, but I noticed stack-allocated buffers are
passed to I2C transfers in imagis_i2c_read_reg():

drivers/input/touchscreen/imagis.c:imagis_i2c_read_reg() {
    ...
	__be32 ret_be;
	__be32 reg_be = cpu_to_be32(reg);
	struct i2c_msg msg[] = {
		{
			.addr = ts->client->addr,
			.flags = 0,
			.buf = (unsigned char *)&reg_be,
			.len = sizeof(reg_be),
		},
    ...
}

If the system's I2C host controller relies on DMA and CONFIG_VMAP_STACK
is enabled, the kernel stack is not physically contiguous. Could this
cause the DMA mapping to fail and the device probe to abort?

> +};
> +
>  static const struct of_device_id imagis_of_match[] = {
>  	{ .compatible = "imagis,ist3032c", .data = &imagis_3032c_data },
>  	{ .compatible = "imagis,ist3038", .data = &imagis_3038_data },

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260822-imagis-40xx-v1-0-73e6d6cdf110@gmail.com?part=5