Re: [PATCH v7 4/4] Input: Add TouchNetix aXiom I2C Touchscreen support

Hi Marco,

On Fri, Jul 03, 2026 at 12:32:25AM +0200, Marco Felsch wrote:
>This adds the initial support for the TouchNetix AX54A touchcontroller
>which is part of TouchNetix's aXiom touchscreen controller family.
>
>The TouchNetix aXiom family provides two physical interfaces: SPI and
>I2C. This patch covers only the I2C interface.
>
>Apart the input event handling the driver supports firmware updates too.
>One firmware interface handles the touchcontroller firmware (AXFW)
>update the other handles the touchcontroller configuration (TH2CFGBIN)
>update.
>
>Co-developed-by: Mamta Shukla <[email protected]>
>Signed-off-by: Mamta Shukla <[email protected]>
>Signed-off-by: Marco Felsch <[email protected]>
>---
> .../testing/sysfs-driver-input-touchnetix-axiom    |   80 +
> drivers/input/touchscreen/Kconfig                  |   17 +
> drivers/input/touchscreen/Makefile                 |    1 +
> drivers/input/touchscreen/touchnetix_axiom.c       | 3141 ++++++++++++++++++++
> 4 files changed, 3239 insertions(+)
>
>diff --git a/Documentation/ABI/testing/sysfs-driver-input-touchnetix-axiom b/Documentation/ABI/testing/sysfs-driver-input-touchnetix-axiom
>new file mode 100644
>index 0000000000000000000000000000000000000000..8262673630557bf1e595a97ec23e66c1c5370f71
>--- /dev/null
>+++ b/Documentation/ABI/testing/sysfs-driver-input-touchnetix-axiom
>@@ -0,0 +1,80 @@
>+What:		/sys/bus/i2c/devices/xxx/fw_major
>+Date:		Jan 2026
>+Contact:	[email protected]

...

>+
>+What:		/sys/bus/i2c/devices/xxx/fw_rc
>+Date:		Jan 2026
>+Contact:	[email protected]
>+Description:
>+		Reports the firmware release canidate version provided by the touchscreen.
>+
>+		Access: Read
>+
>+		Valid values: Represented as string
>+
>+What:		/sys/bus/i2c/devices/xxx/fw_status
>+Date:		Jan 2026
>+Contact:	[email protected]
>+Description:
>+		Reports the firmware status provided by the touchscreen. It may
>+		be either "release" or "engineering".

This is right, it is release. However in the driver you refer to "production",
which does not match.

>+
>+		Access: Read
>+

...

>+ * The devices have a 16bit ADC but Touchnetix used the lower two bits for other
>+ * information.
>+ */
>+#define AXIOM_MAX_XY				(65535 - 3)

This assumes ENABLECOORDINATESCALING is set in u41.

>+#define AXIOM_DEFAULT_POLL_INTERVAL_MS		10
>+#define AXIOM_PAGE_BYTE_LEN			256
>+#define AXIOM_MAX_XFERLEN			0x7fff
>+#define AXIOM_MAX_TOUCHSLOTS			10
>+#define AXIOM_MAX_TOUCHSLOTS_MASK		GENMASK(9, 0)
>+

...

>+ * same usage to handle different usage revisions.
>+ *
>+ * Note:
>+ * During a th2cfgbin update the driver may use usages not listed here.
>+ * Therefore the th2cfgbin update compares the current running FW again the
>+ * th2cfgbin targets FW.
>+ */
>+static const struct axiom_usage_info driver_required_usages[] = {

Is this too strict?
Could you split it into "update" and "required"?
So a probe succeeds with "update" usages, so you can atleast get touch
reports, but a download is blocked without them.
Also should u64 be optional?

>+	AXIOM_REPORT_USAGE(AXIOM_U01, 1, axiom_u01_rev1_process_report),
>+	AXIOM_REPORT_USAGE(AXIOM_U01, 3, axiom_u01_rev1_process_report),
>+	AXIOM_USAGE(AXIOM_U02, 1),
>+	AXIOM_USAGE(AXIOM_U02, 2),
>+	AXIOM_USAGE(AXIOM_U04, 1),
>+	AXIOM_RO_USAGE(AXIOM_U31, 1),
>+	AXIOM_RO_USAGE(AXIOM_U33, 2),
>+	AXIOM_RO_USAGE(AXIOM_U33, 3),
>+	AXIOM_RO_USAGE(AXIOM_U33, 6),

...

>+
>+	return entry->info->rev_num;
>+}
>+
>+static bool axiom_driver_supports_usage(struct axiom_data *ts,
>+					unsigned char usage_num)

This is called many times before calling a given usage handler.
Could we just check in axiom_u31_device_discover() or axiom_i2c_probe()
for all the required usages?
Could the "update" usages be checked in axiom_cfg/axfw_fw_prepare()?

>+{
>+	const struct axiom_usage_info *iter = driver_required_usages;
>+	struct device *dev = ts->dev;
>+	int rev;
>+
>+	/*
>+	 * Some features depend on the current running firmware. Don't print an
>+	 * error if the usage for an optional feature is missing.
>+	 */
>+	if (!ts->usage_table[usage_num].populated) {
>+		dev_dbg(dev, "u%02X is not supported by the current firmware\n",
>+			usage_num);
>+		return false;
>+	}
>+
>+	rev = axiom_usage_rev(ts, usage_num);
>+	if (rev < 0) {
>+		dev_warn(dev, "Driver doesn't support u%02X yet\n", usage_num);
>+		return false;
>+	}
>+
>+	for (; iter; iter++) {

I assume this is supposed to be:
for (; iter->usage_num; iter++) {

>+		if (iter->usage_num != usage_num)
>+			continue;
>+
>+		if (iter->rev_num == rev)
>+			return true;
>+	}
>+
>+	dev_warn(dev, "Driver doesn't support u%02X rev.%d yet\n",
>+		 usage_num, rev);
>+
>+	return false;
>+}
>+

...

>+
>+	/*
>+	 * Downstream axcfg.py waits for 1sec without checking U01 HELLO. Tests
>+	 * showed that waiting for the HELLO message isn't enough therefore we
>+	 * need to add the additional fsleep(1sec).
>+	 * Touchnetix said that the boot can take up to 2sec if all self tests
>+	 * are enabled, so wait 2sec for the HELLO message.
>+	 */
>+	if (!axiom_wait_for_completion_timeout(ts, &ts->boot_complete,
>+					       msecs_to_jiffies(2 * MSEC_PER_SEC))) {
>+		dev_err(ts->dev, "Error swreset timedout\n");
>+		error = -ETIMEDOUT;
>+	}

There is no reinit_completion() to go after/before wait_for_completion_timeout() for any of the calls.
 From my understanding this must be called before starting the new wait for completion.
The 2s timeout may have hidden this.

>+
>+	fsleep(USEC_PER_SEC);
>+
>+	return error;
>+}
>+

...

>+	crc_report = get_unaligned_le16(&buf[len - 2]);
>+	crc_calc = crc16(0, buf, (len - 2));
>+
>+	if (crc_calc != crc_report) {
>+		dev_err_ratelimited(dev, "CRC16 mismatch!\n");
>+		return -EINVAL;
>+	}
>+
>+	report_usage = buf[1];
>+	payload = &buf[AXIOM_U34_REV1_PREAMBLE_BYTES];
>+	len -= AXIOM_U34_REV1_PREAMBLE_BYTES - AXIOM_U34_REV1_POSTAMBLE_BYTES;

Is this right?
Should it not be:
len -= AXIOM_U34_REV1_PREAMBLE_BYTES;
len -= AXIOM_U34_REV1_POSTAMBLE_BYTES;

>+
>+	switch (report_usage) {
>+	case AXIOM_U01:
>+	case AXIOM_U41:
>+		/*
>+		 * axiom_driver_supports_usage() is not required since the
>+		 * correct .process_report() hooks are assigned during the
>+		 * discovery.
>+		 */
>+		return axiom_process_report(ts, report_usage, payload, len);
>+	default:
>+		dev_dbg(dev, "Unsupported report u%02X received\n",
>+			report_usage);
>+	}
>+
>+	return 0;
>+}
>+

...

>+static int axiom_u41_rev2_process_report(struct axiom_data *ts,
>+					 const u8 *buf, size_t bufsize)
>+{
>+	struct input_dev *input = ts->input;
>+	unsigned char id;
>+	u16 targets;
>+
>+	/*
>+	 * The input registration can be postponed but the touchscreen FW is
>+	 * sending u41 reports regardless.
>+	 */
>+	if (!input)
>+		return 0;
>+
>+	targets = get_unaligned_le16(&buf[AXIOM_U41_REV2_TARGETSTATUS_REG]);
>+
>+	for_each_set_bit(id, &ts->enabled_slots, AXIOM_MAX_TOUCHSLOTS) {
>+		bool present;
>+		u16 x, y;
>+		s8 z;
>+
>+		axiom_u41_rev2_decode_target(buf, id, &x, &y, &z);
>+
>+		present = targets & BIT(id);
>+		/* Ignore possible jitters */
>+		if (z == AXIOM_PROX_LEVEL)
>+			present = false;
>+
>+		dev_dbg(ts->dev, "id:%u x:%u y:%u z:%d present:%u",
>+			id, x, y, z, present);
>+
>+		input_mt_slot(input, id);

The index in enabled_slots can be greater than num_slots since u42 does not neccesarily
therefore this can send an out of range finger.

>+		if (input_mt_report_slot_state(input, MT_TOOL_FINGER, present))
>+			touchscreen_report_pos(input, &ts->prop, x, y, true);
>+
>+		if (!present)
>+			continue;
>+
>+		input_report_abs(input, ABS_MT_DISTANCE, z < 0 ? -z : 0);
>+		if (ts->cds_enabled)
>+			input_report_abs(input, ABS_MT_PRESSURE, z >= 0 ? z : 0);
>+	}
>+
>+	input_sync(input);
>+
>+	return 0;
>+}
>+
>+static int axiom_u01_rev1_process_report(struct axiom_data *ts,
>+					 const u8 *buf, size_t bufsize)
>+{
>+	switch (buf[AXIOM_U01_REV1_REPORTTYPE_REG]) {
>+	case AXIOM_U01_REV1_REPORTTYPE_HELLO:
>+		dev_dbg(ts->dev, "u01 HELLO received\n");
>+		axiom_complete(ts, &ts->boot_complete);
>+		return 0;
>+	case AXIOM_U01_REV1_REPORTTYPE_HEARTBEAT:
>+		dev_dbg_ratelimited(ts->dev, "u01 HEARTBEAT received\n");
>+		return 0;
>+	case AXIOM_U01_REV1_REPORTTYPE_OPCOMPLETE:
>+		dev_dbg(ts->dev, "u01 OPCOMPLETE received\n");
>+		axiom_u02_handshakenvm(ts);
>+		axiom_complete(ts, &ts->nvm_write);
>+		return 0;
>+	default:
>+		return -EINVAL;
>+	}
>+}
>+

...

>+
>+	/*
>+	 * Ensure that the IRQ setup is done only once since the handler belong
>+	 * to the i2c-dev whereas the input-poller belong to the input-dev. The
>+	 * input-dev can get unregistered during a firmware update to reflect
>+	 * the new firmware state. Therefore the input-poller setup must be done
>+	 * always.
>+	 */
>+	if (client->irq) {
>+		if (!ts->irq_setup_done) {
>+			error = devm_request_threaded_irq(dev, client->irq,
>+							  NULL, axiom_irq,
>+							  IRQF_ONESHOT,
>+							  dev_name(dev), ts);
>+			if (error) {
>+				dev_err(dev, "Failed to request IRQ\n");

input_free_device() should be called.

>+				return error;
>+			}
>+			ts->irq_setup_done = true;
>+		}
>+	} else {
>+		error = input_setup_polling(input, axiom_poll);
>+		if (error) {
>+			input_free_device(input);
>+			dev_err(dev, "Setup polling mode failed\n");
>+			return error;
>+		}
>+
>+		input_set_poll_interval(input, ts->poll_interval);
>+	}
>+
>+	input_set_drvdata(input, ts);
>+	ts->input = input;
>+
>+	error = input_register_device(input);
>+	if (error) {
>+		input_free_device(input);
>+		ts->input = NULL;
>+		dev_err(dev, "Failed to register input device\n");
>+	};
>+
>+	return error;
>+}
>+
>+static int axiom_update_input_dev(struct axiom_data *ts)
>+{
>+	axiom_unregister_input_dev(ts);
>+
>+	return axiom_register_input_dev(ts, true);
>+}
>+
>+static int axiom_parse_firmware(struct axiom_data *ts)
>+{
>+	struct device *dev = ts->dev;
>+	struct gpio_desc *gpio;
>+	int error;
>+
>+	ts->supplies[0].supply = "vddi";
>+	ts->supplies[1].supply = "vdda";
>+	ts->num_supplies = ARRAY_SIZE(ts->supplies);
>+
>+	error = devm_regulator_bulk_get(dev, ts->num_supplies, ts->supplies);
>+	if (error)
>+		return dev_err_probe(dev, error,
>+				     "Failed to get power supplies\n");
>+
>+	gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
>+	if (IS_ERR(gpio))
>+		return dev_err_probe(dev, PTR_ERR(gpio),
>+				     "Failed to get reset GPIO\n");
>+	ts->reset_gpio = gpio;
>+
>+	ts->poll_interval = AXIOM_DEFAULT_POLL_INTERVAL_MS;
>+	device_property_read_u32(dev, "poll-interval", &ts->poll_interval);
>+
>+	return 0;
>+}
>+
>+static int axiom_power_up_device(struct axiom_data *ts)
>+{
>+	struct device *dev = ts->dev;
>+	int error;
>+
>+	error = regulator_bulk_enable(ts->num_supplies, ts->supplies);
>+	if (error) {
>+		dev_err(dev, "Failed to enable power supplies\n");
>+		return error;
>+	}
>+
>+	gpiod_set_value_cansleep(ts->reset_gpio, 1);
>+	fsleep(2000);
>+	gpiod_set_value_cansleep(ts->reset_gpio, 0);
>+
>+	fsleep(AXIOM_STARTUP_TIME_MS);
>+
>+	return 0;
>+}
>+

...

>+static const struct i2c_device_id axiom_i2c_id_table[] = {
>+	{ "ax54a" },
>+	{ },
>+};
>+MODULE_DEVICE_TABLE(i2c, axiom_i2c_id_table);
>+
>+static const struct of_device_id axiom_of_match[] = {
>+	{ .compatible = "touchnetix,ax54a", },
>+	{ }
>+};
>+MODULE_DEVICE_TABLE(of, axiom_of_match);

This also works for ax80a. ax54 and ax80 are the same silicon but with a
different pinout. It is up to you if you wish to add this.

>+
>+static struct i2c_driver axiom_i2c_driver = {
>+	.driver = {
>+		   .name = KBUILD_MODNAME,
>+		   .dev_groups = axiom_groups,
>+		   .pm = pm_ptr(&axiom_pm_ops),
>+		   .of_match_table = axiom_of_match,
>+	},
>+	.id_table = axiom_i2c_id_table,
>+	.probe = axiom_i2c_probe,
>+	.remove = axiom_i2c_remove,
>+};
>+module_i2c_driver(axiom_i2c_driver);
>+
>+MODULE_DESCRIPTION("TouchNetix aXiom touchscreen I2C bus driver");
>+MODULE_LICENSE("GPL");
>
>-- 
>2.47.3

Apologies for the delay in review.
I have tested with ax80a and ax198a and everything works as intended
on both of them.
This version looks to be quite reliable for the firmware/config 
download which is great!


Tested-by: Andrew Thomas <[email protected]

Regards,
Andrew