[PATCH v2 0/3] Input: add initial support for Goodix GTX8 touchscreen ICs

[PATCH v2 0/3] Input: add initial support for Goodix GTX8 touchscreen ICs

[Aelin Reidel]

These ICs support SPI and I2C interfaces, up to 10 finger touch, stylus
and gesture events.

This driver is derived from the Goodix gtx8_driver_linux available at
[1] and only supports the GT9886 and GT9896 ICs present in the Xiaomi
Mi 9T and Xiaomi Redmi Note 10 Pro smartphones.

The current implementation only supports Normandy and Yellowstone type
ICs, aka only GT9886 and GT9896. It is also limited to I2C only, since I
don't have a device with GTX8 over SPI at hand. Adding support for SPI
should be fairly easy in the future, since the code uses a regmap.

Support for advanced features like:
- Firmware updates
- Stylus events
- Gesture events
- Nanjing IC support
is not included in current version.

The current support requires a previously flashed firmware to be
present.

As I did not have access to datasheets for these ICs, I extracted the
addresses from a couple of config files using a small tool [2]. The
addresses are identical for the same IC families in all configs I
observed, however not all of them make sense and I stubbed out firmware
request support due to this.

I've taken a lot of inspiration from the goodix_berlin driver, but the 
Berlin and GTX8 series of touchscreen ICs differ quite a bit. The driver 
architecture is the same overall, i.e. the power-up sequence and general 
concepts are the mostly same, but it is very clear that they are 
different generations when looking at it in more detail.

Some of the differences:
- There is no equivalent to the bootoption reg that I can find in the 
public GTX8 drivers
- Firmware version struct layout is different yet again
- GTX8 does not expose IC information at runtime as far as I can tell
- The checksum method differs yet again
- The vendor driver reads only 1 touch upfront rather than 2
- Register addresses are 16-bit on GTX8 and 32-bit on Berlin
- Firmware requests don't appear to really exist on GTX8

From what I can tell, the evolution seems to be:
Normandy -> Yellowstone -> Berlin
since Normandy and Yellowstone are already quite different (especially 
with the way checksums work) and Yellowstone has a couple of things 
(checksum, fw_version) that appear similar to Berlin series ICs.

I've tried to make the Berlin driver work for GTX8 ICs before, but 
they're so different (and I lack documentation for registers to perhaps 
make some parts work on GTX8) that I'd rather support these ICs in a new 
and tiny driver. I hope that makes sense. I took heavy inspiration from 
the Berlin driver, but the only parts that are really common between 
them are very trivial things like e.g. the input dev config or power on, 
which I don't think are worth putting in a separate header.

[1] https://github.com/goodix/gtx8_driver_linux
[2] https://github.com/sm7150-mainline/goodix-cfg-bin

Signed-off-by: Aelin Reidel <aelin@mainlining.org>
---
Changes in v2:
- Fix compilation issues found by Intel's kernel test robot
- Add Alexander's T-b to the driver patch
- Link to v1: https://lore.kernel.org/r/20260218-gtx8-v1-0-0d575b3dedc5@mainlining.org

Changes in v1 (post-RFC):
- Drop RFC prefix, the series has been tested enough and works well
  as-is
- Update my name and email address
- Add some reasoning for a new driver to the cover letter
- Add Rob's R-b on the dt-bindings patch
- Add Piyush's T-b to the driver patch
- Link to RFC: https://lore.kernel.org/r/20250918-gtx8-v1-0-cba879c84775@mainlining.org

---
Aelin Reidel (3):
      dt-bindings: input: document Goodix GTX8 Touchscreen ICs
      Input: add support for Goodix GTX8 Touchscreen ICs
      MAINTAINERS: add an entry for Goodix GTX8 Touchscreen driver

 .../bindings/input/touchscreen/goodix,gt9886.yaml  |  71 +++
 MAINTAINERS                                        |   7 +
 drivers/input/touchscreen/Kconfig                  |  15 +
 drivers/input/touchscreen/Makefile                 |   1 +
 drivers/input/touchscreen/goodix_gtx8.c            | 563 +++++++++++++++++++++
 drivers/input/touchscreen/goodix_gtx8.h            | 141 ++++++
 6 files changed, 798 insertions(+)
---
base-commit: 3fa5e5702a82d259897bd7e209469bc06368bf31
change-id: 20250918-gtx8-59a50ccd78a5

Best regards,
-- 
Aelin Reidel <aelin@mainlining.org>

[PATCH v2 1/3] dt-bindings: input: document Goodix GTX8 Touchscreen ICs

[Aelin Reidel]

Document the Goodix GT9886 and GT9896 which are part of the GTX8 series
of Touchscreen controller ICs from Goodix.

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
Signed-off-by: Aelin Reidel <aelin@mainlining.org>
---
 .../bindings/input/touchscreen/goodix,gt9886.yaml  | 71 ++++++++++++++++++++++
 1 file changed, 71 insertions(+)

diff --git a/Documentation/devicetree/bindings/input/touchscreen/goodix,gt9886.yaml b/Documentation/devicetree/bindings/input/touchscreen/goodix,gt9886.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..6307495c2746313cfc32cdbb701455d1596be435
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/goodix,gt9886.yaml
@@ -0,0 +1,71 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/touchscreen/goodix,gt9886.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Goodix GTX8 series touchscreen controller
+
+maintainers:
+  - Aelin Reidel <aelin@mainlining.org>
+
+allOf:
+  - $ref: touchscreen.yaml#
+
+properties:
+  compatible:
+    enum:
+      - goodix,gt9886
+      - goodix,gt9896
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  reset-gpios:
+    maxItems: 1
+
+  avdd-supply:
+    description: Analog power supply regulator on AVDD pin
+
+  vddio-supply:
+    description: power supply regulator on VDDIO pin
+
+  touchscreen-inverted-x: true
+  touchscreen-inverted-y: true
+  touchscreen-size-x: true
+  touchscreen-size-y: true
+  touchscreen-swapped-x-y: true
+
+additionalProperties: false
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - avdd-supply
+  - touchscreen-size-x
+  - touchscreen-size-y
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/gpio/gpio.h>
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+      touchscreen@5d {
+        compatible = "goodix,gt9886";
+        reg = <0x5d>;
+        interrupt-parent = <&gpio>;
+        interrupts = <9 IRQ_TYPE_LEVEL_LOW>;
+        reset-gpios = <&gpio1 8 GPIO_ACTIVE_LOW>;
+        avdd-supply = <&ts_avdd>;
+        touchscreen-size-x = <1080>;
+        touchscreen-size-y = <2340>;
+      };
+    };
+
+...

-- 
2.53.0

[PATCH v2 2/3] Input: add support for Goodix GTX8 Touchscreen ICs

[Aelin Reidel]

Add initial support for the Goodix GTX8 touchscreen ICs.

These ICs support SPI and I2C interfaces, up to 10 finger touch, stylus
and gesture events.

This driver is derived from the Goodix gtx8_driver_linux available at
[1] and only supports the GT9886 and GT9896 ICs present in the Xiaomi
Mi 9T and Xiaomi Redmi Note 10 Pro smartphones.

The current implementation only supports Normandy and Yellowstone type
ICs, aka only GT9886 and GT9896. It is also limited to I2C only, since I
don't have a device with GTX8 over SPI at hand. Adding support for SPI
should be fairly easy in the future, since the code uses a regmap.

Support for advanced features like:
- Firmware updates
- Stylus events
- Gesture events
- Nanjing IC support
is not included in current version.

The current support requires a previously flashed firmware to be
present.

As I did not have access to datasheets for these ICs, I extracted the
addresses from a couple of config files using a small tool [2]. The
addresses are identical for the same IC families in all configs I
observed, however not all of them make sense and I stubbed out firmware
request support due to this.

[1] https://github.com/goodix/gtx8_driver_linux
[2] https://github.com/sm7150-mainline/goodix-cfg-bin

Tested-by: Piyush Raj Chouhan <pc1598@mainlining.org>
Tested-by: Alexander Koskovich <AKoskovich@pm.me>
Signed-off-by: Aelin Reidel <aelin@mainlining.org>
---
 drivers/input/touchscreen/Kconfig       |  15 +
 drivers/input/touchscreen/Makefile      |   1 +
 drivers/input/touchscreen/goodix_gtx8.c | 563 ++++++++++++++++++++++++++++++++
 drivers/input/touchscreen/goodix_gtx8.h | 141 ++++++++
 4 files changed, 720 insertions(+)

diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index 7d5b72ee07fa1313da39a625b5129a0459720865..099ccd3679383dcf037bc7c6e6a3dbf0741722b4 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -429,6 +429,21 @@ config TOUCHSCREEN_GOODIX_BERLIN_SPI
 	  To compile this driver as a module, choose M here: the
 	  module will be called goodix_berlin_spi.
 
+config TOUCHSCREEN_GOODIX_GTX8
+	tristate "Goodix GTX8 touchscreen"
+	depends on I2C
+	select REGMAP_I2C
+	help
+	  Say Y here if you have a Goodix GTX8 IC connected to
+	  your system via I2C. This driver supports Normandy and
+	  Yellowstone ICs like the GT9886 and GT9896.
+	  They are commonly found in mobile phones.
+
+	  if unsure, say N.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called goodix_gtx8.
+
 config TOUCHSCREEN_HIDEEP
 	tristate "HiDeep Touch IC"
 	depends on I2C
diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
index ab9abd151078831a4b22d6998e00ef74fe01c356..9bcb8f01ea785dcbe2a22bd3293601dd4259ba1d 100644
--- a/drivers/input/touchscreen/Makefile
+++ b/drivers/input/touchscreen/Makefile
@@ -48,6 +48,7 @@ obj-$(CONFIG_TOUCHSCREEN_GOODIX)	+= goodix_ts.o
 obj-$(CONFIG_TOUCHSCREEN_GOODIX_BERLIN_CORE)	+= goodix_berlin_core.o
 obj-$(CONFIG_TOUCHSCREEN_GOODIX_BERLIN_I2C)	+= goodix_berlin_i2c.o
 obj-$(CONFIG_TOUCHSCREEN_GOODIX_BERLIN_SPI)	+= goodix_berlin_spi.o
+obj-$(CONFIG_TOUCHSCREEN_GOODIX_GTX8)	+= goodix_gtx8.o
 obj-$(CONFIG_TOUCHSCREEN_HIDEEP)	+= hideep.o
 obj-$(CONFIG_TOUCHSCREEN_HIMAX_HX852X)	+= himax_hx852x.o
 obj-$(CONFIG_TOUCHSCREEN_HYNITRON_CSTXXX)	+= hynitron_cstxxx.o
diff --git a/drivers/input/touchscreen/goodix_gtx8.c b/drivers/input/touchscreen/goodix_gtx8.c
new file mode 100644
index 0000000000000000000000000000000000000000..3fbebfa8b75672866788adf89d83be16e32abf81
--- /dev/null
+++ b/drivers/input/touchscreen/goodix_gtx8.c
@@ -0,0 +1,563 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Driver for Goodix GTX8 Touchscreens
+ *
+ * Copyright (c) 2019 - 2020 Goodix, Inc.
+ * Copyright (C) 2023 Linaro Ltd.
+ * Copyright (c) 2025 Aelin Reidel <aelin@mainlining.org>
+ *
+ * Based on gtx8_driver_linux vendor driver and goodix_berlin kernel driver.
+ *
+ * The driver currently relies on the pre-flashed firmware and only supports
+ * Normandy / Yellowstone ICs.
+ * Pen support is also missing.
+ */
+#include <linux/bitfield.h>
+#include <linux/gpio/consumer.h>
+#include <linux/i2c.h>
+#include <linux/input.h>
+#include <linux/input/mt.h>
+#include <linux/input/touchscreen.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/unaligned.h>
+
+#include "goodix_gtx8.h"
+
+static const struct regmap_config goodix_gtx8_regmap_conf = {
+	.reg_bits = 16,
+	.val_bits = 8,
+	.max_raw_read = I2C_MAX_TRANSFER_SIZE,
+	.max_raw_write = I2C_MAX_TRANSFER_SIZE,
+};
+
+/* vendor & product left unassigned here, should probably be updated from fw info */
+static const struct input_id goodix_gtx8_input_id = {
+	.bustype = BUS_I2C,
+};
+
+static bool goodix_gtx8_checksum_valid_normandy(const u8 *data, int size)
+{
+	u8 cal_checksum = 0;
+	int i;
+
+	if (size < GOODIX_GTX8_CHECKSUM_SIZE)
+		return false;
+
+	for (i = 0; i < size; i++)
+		cal_checksum += data[i];
+
+	return cal_checksum == 0;
+}
+
+static bool goodix_gtx8_checksum_valid_yellowstone(const u8 *data, int size)
+{
+	u16 cal_checksum = 0;
+	u16 r_checksum;
+	int i;
+
+	if (size < GOODIX_GTX8_CHECKSUM_SIZE)
+		return false;
+
+	for (i = 0; i < size - GOODIX_GTX8_CHECKSUM_SIZE; i++)
+		cal_checksum += data[i];
+
+	r_checksum = get_unaligned_be16(&data[i]);
+
+	return cal_checksum == r_checksum;
+}
+
+static int goodix_gtx8_get_remaining_contacts(struct goodix_gtx8_core *cd,
+					      int n)
+{
+	size_t offset = cd->ic_data->header_size + GOODIX_GTX8_TOUCH_SIZE +
+			GOODIX_GTX8_CHECKSUM_SIZE;
+	u32 addr = cd->ic_data->touch_data_addr + offset;
+	int error;
+
+	error = regmap_raw_read(cd->regmap, addr, &cd->event_buffer[offset],
+				(n - 1) * GOODIX_GTX8_TOUCH_SIZE);
+	if (error) {
+		dev_err_ratelimited(cd->dev, "failed to get touch data, %d\n",
+				    error);
+		return error;
+	}
+
+	return 0;
+}
+
+static void goodix_gtx8_report_state(struct goodix_gtx8_core *cd, u8 touch_num,
+				     union goodix_gtx8_touch *touch_data)
+{
+	union goodix_gtx8_touch *t;
+	int i;
+	u8 finger_id;
+
+	for (i = 0; i < touch_num; i++) {
+		t = &touch_data[i];
+
+		if (cd->ic_data->ic_type == IC_TYPE_NORMANDY) {
+			input_mt_slot(cd->input_dev, t->normandy.finger_id);
+			input_mt_report_slot_state(cd->input_dev,
+						   MT_TOOL_FINGER, true);
+
+			touchscreen_report_pos(cd->input_dev, &cd->props,
+					       __le16_to_cpu(t->normandy.x),
+					       __le16_to_cpu(t->normandy.y),
+					       true);
+			input_report_abs(cd->input_dev, ABS_MT_TOUCH_MAJOR,
+					 t->normandy.w);
+		} else {
+			finger_id = FIELD_GET(
+				GOODIX_GTX8_FINGER_ID_MASK_YELLOWSTONE,
+				t->yellowstone.finger_id);
+			input_mt_slot(cd->input_dev, finger_id);
+			input_mt_report_slot_state(cd->input_dev,
+						   MT_TOOL_FINGER, true);
+
+			touchscreen_report_pos(cd->input_dev, &cd->props,
+					       __be16_to_cpu(t->yellowstone.x),
+					       __be16_to_cpu(t->yellowstone.y),
+					       true);
+			input_report_abs(cd->input_dev, ABS_MT_TOUCH_MAJOR,
+					 t->yellowstone.w);
+		}
+	}
+
+	input_mt_sync_frame(cd->input_dev);
+	input_sync(cd->input_dev);
+}
+
+static void goodix_gtx8_touch_handler(struct goodix_gtx8_core *cd, u8 touch_num,
+				      union goodix_gtx8_touch *touch_data)
+{
+	int error;
+
+	touch_num = FIELD_GET(GOODIX_GTX8_TOUCH_COUNT_MASK, touch_num);
+
+	if (touch_num > GOODIX_GTX8_MAX_TOUCH) {
+		dev_warn(cd->dev, "invalid touch num %d\n", touch_num);
+		return;
+	}
+
+	if (touch_num > 1) {
+		/* read additional contact data if more than 1 touch event */
+		error = goodix_gtx8_get_remaining_contacts(cd, touch_num);
+		if (error)
+			return;
+	}
+
+	if (touch_num) {
+		/*
+		 * Normandy checksum is for the entire read buffer,
+		 * Yellowstone is only for the touch data (since header
+		 * has a separate checksum)
+		 */
+		if (cd->ic_data->ic_type == IC_TYPE_NORMANDY) {
+			int len = GOODIX_GTX8_HEADER_SIZE_NORMANDY +
+				  touch_num * GOODIX_GTX8_TOUCH_SIZE +
+				  GOODIX_GTX8_CHECKSUM_SIZE;
+			if (!goodix_gtx8_checksum_valid_normandy(
+				    cd->event_buffer, len)) {
+				dev_err(cd->dev,
+					"touch data checksum error: %*ph\n",
+					len, cd->event_buffer);
+				return;
+			}
+		} else {
+			int len = touch_num * GOODIX_GTX8_TOUCH_SIZE +
+				  GOODIX_GTX8_CHECKSUM_SIZE;
+			if (!goodix_gtx8_checksum_valid_yellowstone(
+				    (u8 *)touch_data, len)) {
+				dev_err(cd->dev,
+					"touch data checksum error: %*ph\n",
+					len, (u8 *)touch_data);
+				return;
+			}
+		}
+	}
+
+	goodix_gtx8_report_state(cd, touch_num, touch_data);
+}
+
+static irqreturn_t goodix_gtx8_irq(int irq, void *data)
+{
+	struct goodix_gtx8_core *cd = data;
+	struct goodix_gtx8_event_normandy *ev_normandy;
+	struct goodix_gtx8_event_yellowstone *ev_yellowstone;
+	union goodix_gtx8_touch *touch_data;
+	int error;
+	u8 status, touch_num;
+
+	error = regmap_raw_read(
+		cd->regmap, cd->ic_data->touch_data_addr, cd->event_buffer,
+		cd->ic_data->header_size + GOODIX_GTX8_TOUCH_SIZE +
+			GOODIX_GTX8_CHECKSUM_SIZE);
+	if (error) {
+		dev_warn_ratelimited(
+			cd->dev, "failed to get event head data: %d\n", error);
+		goto out;
+	}
+
+	/*
+	 * Both IC types have the same data in the header, just at different
+	 * offsets
+	 */
+	if (cd->ic_data->ic_type == IC_TYPE_NORMANDY) {
+		ev_normandy =
+			(struct goodix_gtx8_event_normandy *)cd->event_buffer;
+		status = ev_normandy->hdr.status;
+		touch_num = ev_normandy->hdr.touch_num;
+		touch_data = (union goodix_gtx8_touch *)ev_normandy->data;
+	} else {
+		ev_yellowstone = (struct goodix_gtx8_event_yellowstone *)
+					 cd->event_buffer;
+		status = ev_yellowstone->hdr.status;
+		touch_num = ev_yellowstone->hdr.touch_num;
+		touch_data = (union goodix_gtx8_touch *)ev_yellowstone->data;
+	}
+
+	if (status == 0)
+		goto out;
+
+	/* Yellowstone ICs have a checksum for the header */
+	if (cd->ic_data->ic_type == IC_TYPE_YELLOWSTONE &&
+	    !goodix_gtx8_checksum_valid_yellowstone(
+		    cd->event_buffer, GOODIX_GTX8_HEADER_SIZE_YELLOWSTONE)) {
+		dev_warn_ratelimited(cd->dev,
+				     "touch head checksum error: %*ph\n",
+				     (int)GOODIX_GTX8_HEADER_SIZE_YELLOWSTONE,
+				     cd->event_buffer);
+		goto out_clear;
+	}
+
+	if (status & GOODIX_GTX8_TOUCH_EVENT)
+		goodix_gtx8_touch_handler(cd, touch_num, touch_data);
+
+	if (status & GOODIX_GTX8_REQUEST_EVENT) {
+		/*
+		 * All configs seen so far either set the firmware request
+		 * address to 0 (Normandy) or have it equal the touch data
+		 * address (Yellowstone). Neither seems correct, and this
+		 * is not testable. Therefore it is currently omitted.
+		 */
+		dev_dbg(cd->dev, "received request event, ignoring\n");
+	}
+
+out_clear:
+	/* Clear up status field */
+	regmap_write(cd->regmap, cd->ic_data->touch_data_addr, 0);
+
+out:
+	return IRQ_HANDLED;
+}
+
+static int goodix_gtx8_input_dev_config(struct goodix_gtx8_core *cd)
+{
+	struct input_dev *input_dev;
+	int error;
+
+	input_dev = devm_input_allocate_device(cd->dev);
+	if (!input_dev)
+		return -ENOMEM;
+
+	cd->input_dev = input_dev;
+	input_set_drvdata(input_dev, cd);
+
+	input_dev->name = "Goodix GTX8 Capacitive TouchScreen";
+	input_dev->phys = "input/ts";
+
+	input_dev->id = goodix_gtx8_input_id;
+
+	input_set_abs_params(cd->input_dev, ABS_MT_POSITION_X, 0, SZ_64K - 1, 0,
+			     0);
+	input_set_abs_params(cd->input_dev, ABS_MT_POSITION_Y, 0, SZ_64K - 1, 0,
+			     0);
+	input_set_abs_params(cd->input_dev, ABS_MT_TOUCH_MAJOR, 0, 255, 0, 0);
+
+	touchscreen_parse_properties(cd->input_dev, true, &cd->props);
+
+	error = input_mt_init_slots(cd->input_dev, GOODIX_GTX8_MAX_TOUCH,
+				    INPUT_MT_DIRECT | INPUT_MT_DROP_UNUSED);
+	if (error)
+		return error;
+
+	error = input_register_device(cd->input_dev);
+	if (error)
+		return error;
+
+	return 0;
+}
+
+static int goodix_gtx8_read_version(struct goodix_gtx8_core *cd)
+{
+	int error;
+
+	/*
+	 * The vendor driver reads a whole lot more data to calculate and
+	 * verify a checksum. Without documentation, we don't know what
+	 * most of that data is, so we only read the parts we know about
+	 * and instead ensure their values are as expected
+	 */
+	error = regmap_raw_read(cd->regmap, cd->ic_data->fw_version_addr,
+				&cd->fw_version, sizeof(cd->fw_version));
+	if (error) {
+		dev_err(cd->dev, "error reading fw version, %d\n", error);
+		return error;
+	}
+
+	/*
+	 * Since we don't verify the checksum, do a basic check that the
+	 * product ID meets expectations
+	 */
+	if (memcmp(cd->fw_version.product_id, cd->ic_data->product_id,
+		   sizeof(cd->fw_version.product_id))) {
+		dev_err(cd->dev, "unexpected product ID, got: %c%c%c%c\n",
+			cd->fw_version.product_id[0],
+			cd->fw_version.product_id[1],
+			cd->fw_version.product_id[2],
+			cd->fw_version.product_id[3]);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int goodix_gtx8_dev_confirm(struct goodix_gtx8_core *cd)
+{
+	u8 rx_buf[1];
+	int retry = 3;
+	int error;
+
+	while (retry--) {
+		/*
+		 * test_addr appears to always be the touch_data_addr for
+		 * Normandy, but it doesn't really matter since all we
+		 * need is a valid address
+		 */
+		error = regmap_raw_read(cd->regmap,
+					cd->ic_data->touch_data_addr, rx_buf,
+					sizeof(rx_buf));
+
+		if (!error)
+			return 0;
+
+		usleep_range(5000, 5100);
+	}
+
+	dev_err(cd->dev, "device confirm failed\n");
+
+	return -EINVAL;
+}
+
+static int goodix_gtx8_power_on(struct goodix_gtx8_core *cd)
+{
+	int error;
+
+	error = regulator_enable(cd->vddio);
+	if (error) {
+		dev_err(cd->dev, "Failed to enable VDDIO: %d\n", error);
+		return error;
+	}
+
+	error = regulator_enable(cd->avdd);
+	if (error) {
+		dev_err(cd->dev, "Failed to enable AVDD: %d\n", error);
+		goto err_vddio_disable;
+	}
+
+	/* Vendors usually configure the power on delay as 300ms */
+	msleep(GOODIX_GTX8_POWER_ON_DELAY_MS);
+
+	gpiod_set_value_cansleep(cd->reset_gpio, 0);
+
+	/* Vendor waits 5ms for firmware to initialize */
+	usleep_range(5000, 5100);
+
+	error = goodix_gtx8_dev_confirm(cd);
+	if (error)
+		goto err_dev_reset;
+
+	/* Vendor waits 100ms for firmware to fully boot */
+	msleep(GOODIX_GTX8_NORMAL_RESET_DELAY_MS);
+
+	return 0;
+
+err_dev_reset:
+	gpiod_set_value_cansleep(cd->reset_gpio, 1);
+	regulator_disable(cd->avdd);
+err_vddio_disable:
+	regulator_disable(cd->vddio);
+	return error;
+}
+
+static void goodix_gtx8_power_off(struct goodix_gtx8_core *cd)
+{
+	gpiod_set_value_cansleep(cd->reset_gpio, 1);
+	regulator_disable(cd->avdd);
+	regulator_disable(cd->vddio);
+}
+
+static int goodix_gtx8_suspend(struct device *dev)
+{
+	struct goodix_gtx8_core *cd = dev_get_drvdata(dev);
+
+	disable_irq(cd->irq);
+	goodix_gtx8_power_off(cd);
+
+	return 0;
+}
+
+static int goodix_gtx8_resume(struct device *dev)
+{
+	struct goodix_gtx8_core *cd = dev_get_drvdata(dev);
+	int error;
+
+	error = goodix_gtx8_power_on(cd);
+	if (error)
+		return error;
+
+	enable_irq(cd->irq);
+
+	return 0;
+}
+
+EXPORT_GPL_SIMPLE_DEV_PM_OPS(goodix_gtx8_pm_ops, goodix_gtx8_suspend,
+			     goodix_gtx8_resume);
+
+static void goodix_gtx8_power_off_act(void *data)
+{
+	struct goodix_gtx8_core *cd = data;
+
+	goodix_gtx8_power_off(cd);
+}
+
+static int goodix_gtx8_probe(struct i2c_client *client)
+{
+	struct goodix_gtx8_core *cd;
+	struct regmap *regmap;
+	int error;
+
+	cd = devm_kzalloc(&client->dev, sizeof(*cd), GFP_KERNEL);
+	if (!cd)
+		return -ENOMEM;
+
+	regmap = devm_regmap_init_i2c(client, &goodix_gtx8_regmap_conf);
+	if (IS_ERR(regmap))
+		return PTR_ERR(regmap);
+
+	cd->dev = &client->dev;
+	cd->irq = client->irq;
+	cd->regmap = regmap;
+	cd->ic_data = i2c_get_match_data(client);
+
+	cd->event_buffer =
+		devm_kzalloc(cd->dev, cd->ic_data->event_size, GFP_KERNEL);
+	if (!cd->event_buffer)
+		return -ENOMEM;
+
+	cd->reset_gpio =
+		devm_gpiod_get_optional(cd->dev, "reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(cd->reset_gpio))
+		return dev_err_probe(cd->dev, PTR_ERR(cd->reset_gpio),
+				     "Failed to request reset GPIO\n");
+
+	cd->avdd = devm_regulator_get(cd->dev, "avdd");
+	if (IS_ERR(cd->avdd))
+		return dev_err_probe(cd->dev, PTR_ERR(cd->avdd),
+				     "Failed to request AVDD regulator\n");
+
+	cd->vddio = devm_regulator_get(cd->dev, "vddio");
+	if (IS_ERR(cd->vddio))
+		return dev_err_probe(cd->dev, PTR_ERR(cd->vddio),
+				     "Failed to request VDDIO regulator\n");
+
+	error = goodix_gtx8_power_on(cd);
+	if (error) {
+		dev_err(cd->dev, "failed power on");
+		return error;
+	}
+
+	error = devm_add_action_or_reset(cd->dev, goodix_gtx8_power_off_act,
+					 cd);
+	if (error)
+		return error;
+
+	error = goodix_gtx8_read_version(cd);
+	if (error) {
+		dev_err(cd->dev, "failed to get version info");
+		return error;
+	}
+
+	error = goodix_gtx8_input_dev_config(cd);
+	if (error) {
+		dev_err(cd->dev, "failed to set input device");
+		return error;
+	}
+
+	error = devm_request_threaded_irq(cd->dev, cd->irq, NULL,
+					  goodix_gtx8_irq, IRQF_ONESHOT,
+					  "goodix-gtx8", cd);
+	if (error) {
+		dev_err(cd->dev, "request threaded IRQ failed: %d\n", error);
+		return error;
+	}
+
+	dev_set_drvdata(cd->dev, cd);
+
+	dev_dbg(cd->dev,
+		"Goodix GT%c%c%c%c Touchscreen Controller, Version %d.%d.%d.%d\n",
+		cd->fw_version.product_id[0], cd->fw_version.product_id[1],
+		cd->fw_version.product_id[2], cd->fw_version.product_id[3],
+		cd->fw_version.fw_version[0], cd->fw_version.fw_version[1],
+		cd->fw_version.fw_version[2], cd->fw_version.fw_version[3]);
+
+	return 0;
+}
+
+static const struct goodix_gtx8_ic_data gt9886_data = {
+	.event_size = GOODIX_GTX8_EVENT_SIZE_NORMANDY,
+	.fw_version_addr = GOODIX_GTX8_FW_VERSION_ADDR_NORMANDY,
+	.header_size = GOODIX_GTX8_HEADER_SIZE_NORMANDY,
+	.ic_type = IC_TYPE_NORMANDY,
+	.product_id = { '9', '8', '8', '6' },
+	.touch_data_addr = GOODIX_GTX8_TOUCH_DATA_ADDR_NORMANDY,
+};
+
+static const struct goodix_gtx8_ic_data gt9896_data = {
+	.event_size = GOODIX_GTX8_EVENT_SIZE_YELLOWSTONE,
+	.fw_version_addr = GOODIX_GTX8_FW_VERSION_ADDR_YELLOWSTONE,
+	.header_size = GOODIX_GTX8_HEADER_SIZE_YELLOWSTONE,
+	.ic_type = IC_TYPE_YELLOWSTONE,
+	.product_id = { '9', '8', '9', '6' },
+	.touch_data_addr = GOODIX_GTX8_TOUCH_DATA_ADDR_YELLOWSTONE,
+};
+
+static const struct i2c_device_id goodix_gtx8_i2c_id[] = {
+	{ .name = "gt9886", .driver_data = (long)&gt9886_data },
+	{ .name = "gt9896", .driver_data = (long)&gt9896_data },
+	{},
+};
+MODULE_DEVICE_TABLE(i2c, goodix_gtx8_i2c_id);
+
+static const struct of_device_id goodix_gtx8_of_match[] = {
+	{ .compatible = "goodix,gt9886", .data = &gt9886_data },
+	{ .compatible = "goodix,gt9896", .data = &gt9896_data },
+	{},
+};
+MODULE_DEVICE_TABLE(of, goodix_gtx8_of_match);
+
+static struct i2c_driver goodix_gtx8_driver = {
+	.probe = goodix_gtx8_probe,
+	.id_table = goodix_gtx8_i2c_id,
+	.driver = {
+		.name = "goodix-gtx8",
+		.of_match_table = of_match_ptr(goodix_gtx8_of_match),
+		.pm = pm_sleep_ptr(&goodix_gtx8_pm_ops),
+	},
+};
+module_i2c_driver(goodix_gtx8_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Goodix GTX8 Touchscreen driver");
+MODULE_AUTHOR("Aelin Reidel <aelin@mainlining.org>");
diff --git a/drivers/input/touchscreen/goodix_gtx8.h b/drivers/input/touchscreen/goodix_gtx8.h
new file mode 100644
index 0000000000000000000000000000000000000000..9b13cfce38720b35bb11f0d7f56d671b31664ade
--- /dev/null
+++ b/drivers/input/touchscreen/goodix_gtx8.h
@@ -0,0 +1,141 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef __GOODIX_GTX8_H__
+#define __GOODIX_GTX8_H__
+
+#include <linux/pm.h>
+
+#define GOODIX_GTX8_NORMAL_RESET_DELAY_MS	100
+#define GOODIX_GTX8_POWER_ON_DELAY_MS		300
+
+#define GOODIX_GTX8_TOUCH_EVENT			BIT(7)
+#define GOODIX_GTX8_REQUEST_EVENT		BIT(6)
+#define GOODIX_GTX8_TOUCH_COUNT_MASK		GENMASK(3, 0)
+#define GOODIX_GTX8_FINGER_ID_MASK_YELLOWSTONE	GENMASK(7, 4)
+
+#define GOODIX_GTX8_MAX_TOUCH			10
+#define GOODIX_GTX8_CHECKSUM_SIZE		sizeof(u16)
+
+#define GOODIX_GTX8_FW_VERSION_ADDR_NORMANDY	0x4535
+#define GOODIX_GTX8_FW_VERSION_ADDR_YELLOWSTONE	0x4022
+#define GOODIX_GTX8_TOUCH_DATA_ADDR_NORMANDY	0x4100
+#define GOODIX_GTX8_TOUCH_DATA_ADDR_YELLOWSTONE	0x4180
+
+#define I2C_MAX_TRANSFER_SIZE			256
+
+enum goodix_gtx8_ic_type {
+	IC_TYPE_NORMANDY,
+	IC_TYPE_YELLOWSTONE,
+};
+
+struct goodix_gtx8_ic_data {
+	size_t event_size;
+	/*
+	 * This is technically not the firmware version address
+	 * referenced in the vendor driver, but rather the
+	 * address of the product ID part. The meaning of the
+	 * other parts is unknown and they are therefore omitted
+	 * for now.
+	 */
+	int fw_version_addr;
+	size_t header_size;
+	enum goodix_gtx8_ic_type ic_type;
+	char product_id[4];
+	int touch_data_addr;
+};
+
+struct goodix_gtx8_header_normandy {
+	u8 status;
+	/* Only the lower 4 bits are actually used */
+	u8 touch_num;
+};
+#define GOODIX_GTX8_HEADER_SIZE_NORMANDY \
+	sizeof(struct goodix_gtx8_header_normandy)
+
+struct goodix_gtx8_header_yellowstone {
+	u8 status;
+	/* Most likely unused */
+	u8 __unknown1;
+	/* Only the lower 4 bits are actually used */
+	u8 touch_num;
+	/* Most likely unused */
+	u8 __unknown2[3];
+	__le16 checksum;
+} __packed __aligned(1);
+#define GOODIX_GTX8_HEADER_SIZE_YELLOWSTONE \
+	sizeof(struct goodix_gtx8_header_yellowstone)
+
+struct goodix_gtx8_touch_normandy {
+	u8 finger_id;
+	__le16 x;
+	__le16 y;
+	u8 w;
+	u8 __unknown[2];
+} __packed __aligned(1);
+
+struct goodix_gtx8_touch_yellowstone {
+	/*
+	 * Only the upper 4 bits are used, lower 4 bits are
+	 * probably the sensor ID.
+	 */
+	u8 finger_id;
+	u8 __unknown1;
+	__be16 x;
+	__be16 y;
+	/*
+	 * Vendor driver claims that this is a single __be16,
+	 * but testing shows that it likely isn't.
+	 */
+	u8 __unknown2;
+	u8 w;
+} __packed __aligned(1);
+
+union goodix_gtx8_touch {
+	struct goodix_gtx8_touch_normandy normandy;
+	struct goodix_gtx8_touch_yellowstone yellowstone;
+};
+#define GOODIX_GTX8_TOUCH_SIZE		sizeof(union goodix_gtx8_touch)
+
+struct goodix_gtx8_event_normandy {
+	struct goodix_gtx8_header_normandy hdr;
+	/* The data below is u16 aligned */
+	u8 data[GOODIX_GTX8_TOUCH_SIZE * GOODIX_GTX8_MAX_TOUCH +
+		GOODIX_GTX8_CHECKSUM_SIZE];
+};
+#define GOODIX_GTX8_EVENT_SIZE_NORMANDY \
+	sizeof(struct goodix_gtx8_event_normandy)
+
+struct goodix_gtx8_event_yellowstone {
+	struct goodix_gtx8_header_yellowstone hdr;
+	/* The data below is u16 aligned */
+	u8 data[GOODIX_GTX8_TOUCH_SIZE * GOODIX_GTX8_MAX_TOUCH +
+		GOODIX_GTX8_CHECKSUM_SIZE];
+};
+#define GOODIX_GTX8_EVENT_SIZE_YELLOWSTONE \
+	sizeof(struct goodix_gtx8_event_yellowstone)
+
+struct goodix_gtx8_fw_version {
+	/* 4 digits IC number */
+	char product_id[4];
+	/* Most likely unused */
+	u8 __unknown[4];
+	/* Four components version number */
+	u8 fw_version[4];
+};
+
+struct goodix_gtx8_core {
+	struct device *dev;
+	struct regmap *regmap;
+	struct regulator *avdd;
+	struct regulator *vddio;
+	struct gpio_desc *reset_gpio;
+	struct touchscreen_properties props;
+	struct goodix_gtx8_fw_version fw_version;
+	struct input_dev *input_dev;
+	int irq;
+	const struct goodix_gtx8_ic_data *ic_data;
+	u8 *event_buffer;
+};
+
+extern const struct dev_pm_ops goodix_gtx8_pm_ops;
+
+#endif

-- 
2.53.0

[PATCH v2 3/3] MAINTAINERS: add an entry for Goodix GTX8 Touchscreen driver

[Aelin Reidel]

Add MAINTAINERS entry for the Goodix GTX8 Touchscreen IC driver.

Signed-off-by: Aelin Reidel <aelin@mainlining.org>
---
 MAINTAINERS | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 14899f1de77ed2e8a583cf7b0fea25725c8534cb..c76f9fbe51f929f7eded37760cb5c83dfa337d0b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10849,6 +10849,13 @@ M:	Maud Spierings <maudspierings@gocontroll.com>
 S:	Maintained
 F:	Documentation/devicetree/bindings/connector/gocontroll,moduline-module-slot.yaml
 
+GOODIX GTX8 TOUCHSCREEN
+M:	Aelin Reidel <aelin@mainlining.org>
+L:	linux-input@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/input/touchscreen/goodix,gt9886.yaml
+F:	drivers/input/touchscreen/goodix_gtx8*
+
 GOODIX TOUCHSCREEN
 M:	Hans de Goede <hansg@kernel.org>
 L:	linux-input@vger.kernel.org

-- 
2.53.0

Re: [PATCH v2 0/3] Input: add initial support for Goodix GTX8 touchscreen ICs

[Hans de Goede]

Hi,

On 28-Feb-26 02:56, Aelin Reidel wrote:
> These ICs support SPI and I2C interfaces, up to 10 finger touch, stylus
> and gesture events.
> 
> This driver is derived from the Goodix gtx8_driver_linux available at
> [1] and only supports the GT9886 and GT9896 ICs present in the Xiaomi
> Mi 9T and Xiaomi Redmi Note 10 Pro smartphones.
> 
> The current implementation only supports Normandy and Yellowstone type
> ICs, aka only GT9886 and GT9896. It is also limited to I2C only, since I
> don't have a device with GTX8 over SPI at hand. Adding support for SPI
> should be fairly easy in the future, since the code uses a regmap.
> 
> Support for advanced features like:
> - Firmware updates
> - Stylus events
> - Gesture events
> - Nanjing IC support
> is not included in current version.
> 
> The current support requires a previously flashed firmware to be
> present.
> 
> As I did not have access to datasheets for these ICs, I extracted the
> addresses from a couple of config files using a small tool [2]. The
> addresses are identical for the same IC families in all configs I
> observed, however not all of them make sense and I stubbed out firmware
> request support due to this.
> 
> I've taken a lot of inspiration from the goodix_berlin driver, but the 
> Berlin and GTX8 series of touchscreen ICs differ quite a bit. The driver 
> architecture is the same overall, i.e. the power-up sequence and general 
> concepts are the mostly same, but it is very clear that they are 
> different generations when looking at it in more detail.

Right, this answers my main question about this driver which was:
"why another goodix driver?" (this would be the third one).

I've also compared this driver with the original goodix.c touchscreen
driver (which I know well) and the protocol is somewhat closer
to the original goodix.c driver then it is to goodix_berlin, but still
different enough that having a separate driver is the best option IMHO.

...

> From what I can tell, the evolution seems to be:
> Normandy -> Yellowstone -> Berlin
> since Normandy and Yellowstone are already quite different (especially 
> with the way checksums work) and Yellowstone has a couple of things 
> (checksum, fw_version) that appear similar to Berlin series ICs.

You forgot the original goodix.c driver, adding that it seems
the evolution is:

GTx1/GTx2/GTx6 -> Normandy -> Yellowstone -> Berlin

With GTx1/GTx2/GTx6 having no checksum at all (and 16 bit
registers) and some of the original GTx1/GTx2/GTx6 don't have
nvram for the firmware, so Linux must upload firmware every boot.

Anyways I agree that these are different enough from the existing
goodix and goodix_berlin drivers, so based on that (and only on that):

Acked-by: Hans de Goede <johannes.goede@oss.qualcomm.com>

Regards,

Hans

Re: [PATCH v2 2/3] Input: add support for Goodix GTX8 Touchscreen ICs

[Dmitry Torokhov]

Hi Aelin,

On Sat, Feb 28, 2026 at 02:56:11AM +0100, Aelin Reidel wrote:
> diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
> index 7d5b72ee07fa1313da39a625b5129a0459720865..099ccd3679383dcf037bc7c6e6a3dbf0741722b4 100644
> --- a/drivers/input/touchscreen/Kconfig
> +++ b/drivers/input/touchscreen/Kconfig
> @@ -429,6 +429,21 @@ config TOUCHSCREEN_GOODIX_BERLIN_SPI
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called goodix_berlin_spi.
>  
> +config TOUCHSCREEN_GOODIX_GTX8
> +	tristate "Goodix GTX8 touchscreen"
> +	depends on I2C
> +	select REGMAP_I2C
> +	help
> +	  Say Y here if you have a Goodix GTX8 IC connected to
> +	  your system via I2C. This driver supports Normandy and
> +	  Yellowstone ICs like the GT9886 and GT9896.
> +	  They are commonly found in mobile phones.
> +
> +	  if unsure, say N.

s/if/If/

> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called goodix_gtx8.
> +
...
> diff --git a/drivers/input/touchscreen/goodix_gtx8.c b/drivers/input/touchscreen/goodix_gtx8.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..3fbebfa8b75672866788adf89d83be16e32abf81
> --- /dev/null
> +++ b/drivers/input/touchscreen/goodix_gtx8.c
> @@ -0,0 +1,563 @@
...
> +#include <linux/bitfield.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/i2c.h>
> +#include <linux/input.h>
> +#include <linux/input/mt.h>
> +#include <linux/input/touchscreen.h>
> +#include <linux/module.h>
> +#include <linux/regmap.h>
> +#include <linux/unaligned.h>
> +
> +#include "goodix_gtx8.h"

Please include headers directly for all primitives and types used by the
driver (linux/bits.h, linux/delay.h, linux/device.h, linux/err.h,
linux/interrupt.h, linux/pm.h, linux/regulator/consumer.h,
linux/sizes.h, linux/types.h).

Also, since this is a single-module driver and SPI support does not
exist yet, please fold goodix_gtx8.h into goodix_gtx8.c. We can split it
out if and when a separate SPI transport module is added.

> +
> +static const struct regmap_config goodix_gtx8_regmap_conf = {
> +	.reg_bits = 16,
> +	.val_bits = 8,
> +	.max_raw_read = I2C_MAX_TRANSFER_SIZE,
> +	.max_raw_write = I2C_MAX_TRANSFER_SIZE,
> +};
> +
> +/* vendor & product left unassigned here, should probably be updated from fw info */
> +static const struct input_id goodix_gtx8_input_id = {
> +	.bustype = BUS_I2C,
> +};

No need for a separate static struct for this, just assign
input_dev->id.bustype = BUS_I2C in goodix_gtx8_input_dev_config().

> +static int goodix_gtx8_get_remaining_contacts(struct goodix_gtx8_core *cd,
> +					      int n)
> +{
> +	size_t offset = cd->ic_data->header_size + GOODIX_GTX8_TOUCH_SIZE +
> +			GOODIX_GTX8_CHECKSUM_SIZE;
> +	u32 addr = cd->ic_data->touch_data_addr + offset;
> +	int error;
> +
> +	error = regmap_raw_read(cd->regmap, addr, &cd->event_buffer[offset],
> +				(n - 1) * GOODIX_GTX8_TOUCH_SIZE);

Please add a brief comment here (similar to goodix_berlin_core.c)
explaining why offset includes GOODIX_GTX8_CHECKSUM_SIZE while reading
(n - 1) * GOODIX_GTX8_TOUCH_SIZE bytes (i.e. that the initial read for
the first contact plus checksum already fetched the first 2 bytes of the
second contact).

> +	if (error) {
> +		dev_err_ratelimited(cd->dev, "failed to get touch data, %d\n",
> +				    error);
> +		return error;
> +	}
> +
> +	return 0;
> +}
> +
> +static void goodix_gtx8_report_state(struct goodix_gtx8_core *cd, u8 touch_num,
> +				     union goodix_gtx8_touch *touch_data)
> +{
> +	union goodix_gtx8_touch *t;
> +	int i;
> +	u8 finger_id;
> +
> +	for (i = 0; i < touch_num; i++) {
> +		t = &touch_data[i];
> +
> +		if (cd->ic_data->ic_type == IC_TYPE_NORMANDY) {
> +			input_mt_slot(cd->input_dev, t->normandy.finger_id);
> +			input_mt_report_slot_state(cd->input_dev,
> +						   MT_TOOL_FINGER, true);
> +
> +			touchscreen_report_pos(cd->input_dev, &cd->props,
> +					       __le16_to_cpu(t->normandy.x),
> +					       __le16_to_cpu(t->normandy.y),
> +					       true);
> +			input_report_abs(cd->input_dev, ABS_MT_TOUCH_MAJOR,
> +					 t->normandy.w);
> +		} else {
> +			finger_id = FIELD_GET(
> +				GOODIX_GTX8_FINGER_ID_MASK_YELLOWSTONE,
> +				t->yellowstone.finger_id);
> +			input_mt_slot(cd->input_dev, finger_id);
> +			input_mt_report_slot_state(cd->input_dev,
> +						   MT_TOOL_FINGER, true);
> +
> +			touchscreen_report_pos(cd->input_dev, &cd->props,
> +					       __be16_to_cpu(t->yellowstone.x),
> +					       __be16_to_cpu(t->yellowstone.y),
> +					       true);
> +			input_report_abs(cd->input_dev, ABS_MT_TOUCH_MAJOR,
> +					 t->yellowstone.w);
> +		}
> +	}

Several issues here:

1. finger_id is not checked against GOODIX_GTX8_MAX_TOUCH (10). On
   Yellowstone it is a 4-bit field (0..15), and on Normandy
   t->normandy.finger_id is an unmasked u8 (where only the lower 4 bits
   are the slot ID and upper bits carry event/tool flags such as BIT(7)
   for stylus). If finger_id >= GOODIX_GTX8_MAX_TOUCH, input_mt_slot()
   ignores the slot change and leaves the previously selected slot
   active, so the subsequent calls overwrite another contact's slot.
   Also, on Yellowstone the lower 4 bits encode the contact/tool type
   (like in goodix_berlin), so non-finger contacts should be skipped.

2. Please use le16_to_cpu() and be16_to_cpu() instead of the underscored
   __le16_to_cpu() and __be16_to_cpu() variants.

3. Please extract finger_id, x, y, and w in the if/else block and call
   input_mt_slot(), input_mt_report_slot_state(),
   touchscreen_report_pos(), and input_report_abs() once below it.

> +
> +	input_mt_sync_frame(cd->input_dev);
> +	input_sync(cd->input_dev);
> +}
> +
> +static void goodix_gtx8_touch_handler(struct goodix_gtx8_core *cd, u8 touch_num,
> +				      union goodix_gtx8_touch *touch_data)
> +{
> +	int error;
> +
> +	touch_num = FIELD_GET(GOODIX_GTX8_TOUCH_COUNT_MASK, touch_num);
> +
> +	if (touch_num > GOODIX_GTX8_MAX_TOUCH) {
> +		dev_warn(cd->dev, "invalid touch num %d\n", touch_num);
> +		return;
> +	}
> +
> +	if (touch_num > 1) {
> +		/* read additional contact data if more than 1 touch event */
> +		error = goodix_gtx8_get_remaining_contacts(cd, touch_num);
> +		if (error)
> +			return;
> +	}
> +
> +	if (touch_num) {
> +		/*
> +		 * Normandy checksum is for the entire read buffer,
> +		 * Yellowstone is only for the touch data (since header
> +		 * has a separate checksum)
> +		 */
> +		if (cd->ic_data->ic_type == IC_TYPE_NORMANDY) {
> +			int len = GOODIX_GTX8_HEADER_SIZE_NORMANDY +
> +				  touch_num * GOODIX_GTX8_TOUCH_SIZE +
> +				  GOODIX_GTX8_CHECKSUM_SIZE;
> +			if (!goodix_gtx8_checksum_valid_normandy(
> +				    cd->event_buffer, len)) {
> +				dev_err(cd->dev,
> +					"touch data checksum error: %*ph\n",
> +					len, cd->event_buffer);
> +				return;
> +			}
> +		} else {
> +			int len = touch_num * GOODIX_GTX8_TOUCH_SIZE +
> +				  GOODIX_GTX8_CHECKSUM_SIZE;
> +			if (!goodix_gtx8_checksum_valid_yellowstone(
> +				    (u8 *)touch_data, len)) {
> +				dev_err(cd->dev,
> +					"touch data checksum error: %*ph\n",
> +					len, (u8 *)touch_data);
> +				return;
> +			}
> +		}
> +	}

Please use dev_warn_ratelimited() / dev_err_ratelimited() in the IRQ
path so a noisy bus or misbehaving firmware does not flood the logs.

> +static irqreturn_t goodix_gtx8_irq(int irq, void *data)
> +{
...
> +	error = regmap_raw_read(
> +		cd->regmap, cd->ic_data->touch_data_addr, cd->event_buffer,
> +		cd->ic_data->header_size + GOODIX_GTX8_TOUCH_SIZE +
> +			GOODIX_GTX8_CHECKSUM_SIZE);
> +	if (error) {
> +		dev_warn_ratelimited(
> +			cd->dev, "failed to get event head data: %d\n", error);
> +		goto out;
> +	}
...
> +	if (status == 0)
> +		goto out;
...
> +out_clear:
> +	/* Clear up status field */
> +	regmap_write(cd->regmap, cd->ic_data->touch_data_addr, 0);
> +
> +out:
> +	return IRQ_HANDLED;
> +}

If the initial read fails or status == 0, returning IRQ_HANDLED on a
level-low interrupt prevents the kernel from detecting a stuck interrupt
line. Those paths should return IRQ_NONE.

> +static int goodix_gtx8_dev_confirm(struct goodix_gtx8_core *cd)
> +{
> +	u8 rx_buf[1];
> +	int retry = 3;
> +	int error;
> +
> +	while (retry--) {
> +		/*
> +		 * test_addr appears to always be the touch_data_addr for
> +		 * Normandy, but it doesn't really matter since all we
> +		 * need is a valid address
> +		 */
> +		error = regmap_raw_read(cd->regmap,
> +					cd->ic_data->touch_data_addr, rx_buf,
> +					sizeof(rx_buf));
> +
> +		if (!error)
> +			return 0;
> +
> +		usleep_range(5000, 5100);
> +	}
> +
> +	dev_err(cd->dev, "device confirm failed\n");
> +
> +	return -EINVAL;
> +}

Please return 'error' (and log it) instead of replacing it with -EINVAL.

> +static int goodix_gtx8_suspend(struct device *dev)
> +{
> +	struct goodix_gtx8_core *cd = dev_get_drvdata(dev);
> +
> +	disable_irq(cd->irq);
> +	goodix_gtx8_power_off(cd);
> +
> +	return 0;
> +}
> +
> +static int goodix_gtx8_resume(struct device *dev)
> +{
> +	struct goodix_gtx8_core *cd = dev_get_drvdata(dev);
> +	int error;
> +
> +	error = goodix_gtx8_power_on(cd);
> +	if (error)
> +		return error;
> +
> +	enable_irq(cd->irq);
> +
> +	return 0;
> +}
> +
> +EXPORT_GPL_SIMPLE_DEV_PM_OPS(goodix_gtx8_pm_ops, goodix_gtx8_suspend,
> +			     goodix_gtx8_resume);

1. Use static DEFINE_SIMPLE_DEV_PM_OPS() instead of exporting
   goodix_gtx8_pm_ops when there are no external callers.

2. If goodix_gtx8_power_on() fails in goodix_gtx8_resume(), it leaves
   both regulators disabled and cd->irq disabled. A subsequent driver
   unbind (via goodix_gtx8_power_off_act) or the next suspend cycle will
   call goodix_gtx8_power_off() again on already-disabled regulators
   (triggering a regulator underflow WARN) and disable the IRQ a second
   time.

   Better yet, please wire up input_dev->open and input_dev->close:
   power the chip on in probe() only long enough to read the version,
   then power it off, request the IRQ with IRQF_ONESHOT | IRQF_NO_AUTOEN,
   power on + enable_irq() in open(), and disable_irq() + power off in
   close(). Then suspend() and resume() can take &cd->input_dev->mutex
   and only power down/up when input_device_enabled(cd->input_dev) is
   true, which also avoids keeping the controller powered when nothing
   uses the input device.

> +static int goodix_gtx8_probe(struct i2c_client *client)
> +{
...
> +	cd->dev = &client->dev;
> +	cd->irq = client->irq;
> +	cd->regmap = regmap;
> +	cd->ic_data = i2c_get_match_data(client);
> +
> +	cd->event_buffer =
> +		devm_kzalloc(cd->dev, cd->ic_data->event_size, GFP_KERNEL);
> +	if (!cd->event_buffer)
> +		return -ENOMEM;

Please check if (!cd->ic_data) before dereferencing it.

Also, instead of a separate devm_kzalloc() for cd->event_buffer and
casting it across the driver, you can embed a union of
goodix_gtx8_event_normandy and goodix_gtx8_event_yellowstone directly in
struct goodix_gtx8_core (like goodix_berlin_core.c does).

> +	error = goodix_gtx8_power_on(cd);
> +	if (error) {
> +		dev_err(cd->dev, "failed power on");
> +		return error;
> +	}
> +
> +	error = devm_add_action_or_reset(cd->dev, goodix_gtx8_power_off_act,
> +					 cd);
> +	if (error)
> +		return error;
> +
> +	error = goodix_gtx8_read_version(cd);
> +	if (error) {
> +		dev_err(cd->dev, "failed to get version info");
> +		return error;
> +	}
> +
> +	error = goodix_gtx8_input_dev_config(cd);
> +	if (error) {
> +		dev_err(cd->dev, "failed to set input device");
> +		return error;
> +	}
> +
> +	error = devm_request_threaded_irq(cd->dev, cd->irq, NULL,
> +					  goodix_gtx8_irq, IRQF_ONESHOT,
> +					  "goodix-gtx8", cd);
> +	if (error) {
> +		dev_err(cd->dev, "request threaded IRQ failed: %d\n", error);
> +		return error;
> +	}
> +
> +	dev_set_drvdata(cd->dev, cd);

The three dev_err() messages above are missing trailing '\n' (and
goodix_gtx8_power_on() / goodix_gtx8_read_version() already log their
own errors).

Also, please set drvdata and request the IRQ before registering the
input device in goodix_gtx8_input_dev_config().

> +static const struct i2c_device_id goodix_gtx8_i2c_id[] = {
> +	{ .name = "gt9886", .driver_data = (long)&gt9886_data },
> +	{ .name = "gt9896", .driver_data = (long)&gt9896_data },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(i2c, goodix_gtx8_i2c_id);
> +
> +static const struct of_device_id goodix_gtx8_of_match[] = {
> +	{ .compatible = "goodix,gt9886", .data = &gt9886_data },
> +	{ .compatible = "goodix,gt9896", .data = &gt9896_data },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, goodix_gtx8_of_match);
> +
> +static struct i2c_driver goodix_gtx8_driver = {
> +	.probe = goodix_gtx8_probe,
> +	.id_table = goodix_gtx8_i2c_id,
> +	.driver = {
> +		.name = "goodix-gtx8",
> +		.of_match_table = of_match_ptr(goodix_gtx8_of_match),
> +		.pm = pm_sleep_ptr(&goodix_gtx8_pm_ops),
> +	},
> +};

Use (kernel_ulong_t) for .driver_data in goodix_gtx8_i2c_id, and drop
of_match_ptr() so goodix_gtx8_of_match does not trigger an unused
variable warning when CONFIG_OF is disabled.

> diff --git a/drivers/input/touchscreen/goodix_gtx8.h b/drivers/input/touchscreen/goodix_gtx8.h
> new file mode 100644
> index 0000000000000000000000000000000000000000..9b13cfce38720b35bb11f0d7f56d671b31664ade
> --- /dev/null
> +++ b/drivers/input/touchscreen/goodix_gtx8.h
...
> +#define I2C_MAX_TRANSFER_SIZE			256

Please prefix this with GOODIX_GTX8_ so it does not collide with the I2C
subsystem namespace.

> +struct goodix_gtx8_header_yellowstone {
> +	u8 status;
> +	/* Most likely unused */
> +	u8 __unknown1;
> +	/* Only the lower 4 bits are actually used */
> +	u8 touch_num;
> +	/* Most likely unused */
> +	u8 __unknown2[3];
> +	__le16 checksum;
> +} __packed __aligned(1);

Yellowstone checksum is big-endian (read via get_unaligned_be16() in
goodix_gtx8_checksum_valid_yellowstone()), so this field should be
__be16. Also, __aligned(1) is redundant alongside __packed (here and on
the touch structs below), and the "The data below is u16 aligned"
comments on the event structs are inaccurate for Normandy where x and y
sit at odd byte offsets.

Thanks.

-- 
Dmitry

Re: [PATCH v2 1/3] dt-bindings: input: document Goodix GTX8 Touchscreen ICs

[Dmitry Torokhov]

Hi Aelin,

On Sat, Feb 28, 2026 at 02:56:10AM +0100, Aelin Reidel wrote:
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/goodix,gt9886.yaml b/Documentation/devicetree/bindings/input/touchscreen/goodix,gt9886.yaml
> new file mode 100644
> index 0000000000000000000000000000000000000000..6307495c2746313cfc32cdbb701455d1596be435
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/touchscreen/goodix,gt9886.yaml
> @@ -0,0 +1,71 @@
...
> +allOf:
> +  - $ref: touchscreen.yaml#
> +
> +properties:
> +  compatible:
> +    enum:
> +      - goodix,gt9886
> +      - goodix,gt9896
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  reset-gpios:
> +    maxItems: 1
> +
> +  avdd-supply:
> +    description: Analog power supply regulator on AVDD pin
> +
> +  vddio-supply:
> +    description: power supply regulator on VDDIO pin
> +
> +  touchscreen-inverted-x: true
> +  touchscreen-inverted-y: true
> +  touchscreen-size-x: true
> +  touchscreen-size-y: true
> +  touchscreen-swapped-x-y: true
> +
> +additionalProperties: false

Could this use unevaluatedProperties: false instead of listing a subset
of touchscreen-* properties and setting additionalProperties: false?
That way other standard touchscreen.yaml properties supported by
touchscreen_parse_properties() (such as touchscreen-x-mm,
touchscreen-y-mm, touchscreen-fuzz-x, touchscreen-fuzz-y) can be used as
well.

> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - avdd-supply
> +  - touchscreen-size-x
> +  - touchscreen-size-y

Why is avdd-supply required while vddio-supply is optional (given that
the IC requires both rails and the driver calls devm_regulator_get() for
both)? Also, are touchscreen-size-x and touchscreen-size-y truly
required if the controller firmware already knows the panel resolution?

Thanks.

-- 
Dmitry