Re: [PATCH v5 4/4] Input: Add TouchNetix aXiom I2C Touchscreen support
From: Andrew Thomas
Date: Thu Feb 26 2026 - 06:08:48 EST
On Wed, Feb 25, 2026 at 09:50:11PM +0100, Marco Felsch wrote:
Hi Andrew,
thanks for the your reply, please see below.
On 26-02-25, Andrew Thomas wrote:
On Sun, Jan 11, 2026 at 04:05:47PM +0100, Marco Felsch wrote:
...
> +struct axiom_u33_rev3 {
> + __le32 runtime_crc;
> + __le32 runtime_nvm_crc;
> + __le32 bootloader_crc;
> + __le32 nvltlusageconfig_crc;
> + __le32 vltusageconfig_crc;
> + __le32 u22_sequencedata_crc;
> + __le32 u43_hotspots_crc;
> + __le32 u77_dod_data_crc;
> + __le32 u93_profiles_crc;
> + __le32 u94_deltascalemap_crc;
> + __le32 runtimehash_crc;
> +};
> +
I think revision handling should be kept in unpacking where possible.
Currently there are 10 revisions of u33, so adding support for many
revisions and usages would add alot of code.
This isn't very complex nor very large code. Each u33 rev will add
44-byte of code, so in the end there will be 440-byte. I've also seen
that some revisions reduce the size because some fields aren't required.
E.g. u93_crc is at offset-5.
Furthermore describing the complete layout of u33 allows us using it to
query the u33 in one i2c-bulk-transfer.
Fair enough.
> +#define AXIOM_U34 0x34
> +#define AXIOM_U34_REV1_OVERFLOW_MASK BIT(7)
> +#define AXIOM_U34_REV1_REPORTLENGTH_MASK GENMASK(6, 0)
> +#define AXIOM_U34_REV1_PREAMBLE_BYTES 2
> +#define AXIOM_U34_REV1_POSTAMBLE_BYTES 4
...
> +enum axiom_runmode {
> + AXIOM_DISCOVERY_MODE,
> + AXIOM_TCP_MODE,
> + AXIOM_TCP_CFG_UPDATE_MODE,
> + AXIOM_BLP_PRE_MODE,
> + AXIOM_BLP_MODE,
> +};
There are only two actual axiom states, bootloader and runtime (TCP).
This is more of a driver state rather than an axiom state.
Could you label it as such?
Yes I know and the AXIOM_BLP_PRE_MODE will be dropped with the next
version, which I'm going to send this week!. Not sure why this would be
required.
> +struct axiom_data {
> + struct input_dev *input;
> + struct device *dev;
> +
> + struct gpio_desc *reset_gpio;
> + struct regulator_bulk_data supplies[2];
> + unsigned int num_supplies;
> +
> + struct regmap *regmap;
> + struct touchscreen_properties prop;
> + bool irq_setup_done;
> + u32 poll_interval;
> +
> + struct drm_panel_follower panel_follower;
> + bool is_panel_follower;
> +
> + enum axiom_runmode mode;
> + /*
> + * Two completion types to support firmware updates
> + * in irq and poll mode.
> + */
> + struct axiom_completion {
> + struct completion completion;
> + bool poll_done;
> + } nvm_write, boot_complete;
> +
> + /* Lock to protect both firmware interfaces */
> + struct mutex fwupdate_lock;
> + struct axiom_firmware {
> + /* Lock to protect cancel */
> + struct mutex lock;
> + bool cancel;
> + struct fw_upload *fwl;
> + } fw[AXIOM_FW_NUM];
> +
> + unsigned int fw_major;
> + unsigned int fw_minor;
> + unsigned int fw_rc;
> + unsigned int fw_status;
> + unsigned int fw_variant;
> + u16 device_id;
> + u16 jedec_id;
> + u8 silicon_rev;
> +
> + /* CRCs we need to check during a config update */
> + struct axiom_crc {
> + u32 runtime;
> + u32 vltusageconfig;
> + u32 nvltlusageconfig;
> + u32 u22_sequencedata;
> + u32 u43_hotspots;
> + u32 u77_dod_data;
> + u32 u93_profiles;
> + u32 u94_deltascalemap;
> + } crc[AXIOM_CRC_NUM];
I think this structure should hold all possible u33 CRCs and then
invalid ones can be ignored for the given u33 revision.
Why should be the bootloader CRC interessting? The bootloader can't be
updated/flashed, at least not according my documentation. Therefore I
didn't listed the bootloader CRC here.
That is true, there are newer CDUs I did not consider are not available.
I shall hopefully soon update the python so that most usage revisions
can be seen more easily.
However this change could be added later.
> + bool cds_enabled;
> + unsigned long enabled_slots;
> + unsigned int num_slots;
> +
> + unsigned int max_report_byte_len;
> + struct axiom_usage_table_entry {
> + bool populated;
> + unsigned int baseaddr;
> + unsigned int size_bytes;
> + const struct axiom_usage_info *info;
> + } usage_table[AXIOM_MAX_USAGES];
> +};
....
> +static int axiom_u02_swreset(struct axiom_data *ts)
> +{
> + struct axiom_u02_rev1_system_manager_msg msg = { };
> + int ret;
> +
> + if (!axiom_driver_supports_usage(ts, AXIOM_U02))
> + return -EINVAL;
> +
> + msg.command = cpu_to_le16(AXIOM_U02_REV1_CMD_SOFTRESET);
> + ret = axiom_u02_send_msg(ts, &msg, false);
> + if (ret)
> + return ret;
> +
> + /*
> + * Downstream http://axcfg.py waits for 1sec without checking U01 hello. Tests
> + * showed that waiting for the hello message isn't enough therefore we
> + * need both to make it robuster.
> + */
> + ret = axiom_wait_for_completion_timeout(ts, &ts->boot_complete,
> + msecs_to_jiffies(1 * MSEC_PER_SEC));
Boot can take up to 2s with all selftests enabled.
Thanks for this information :) I will add it.
> + if (!ret)
> + dev_err(ts->dev, "Error swreset timedout\n");
> +
> + fsleep(USEC_PER_SEC);
> +
> + return ret ? 0 : -ETIMEDOUT;
> +}
...
> +static int axiom_u02_enter_bootloader(struct axiom_data *ts)
> +{
> + struct axiom_u02_rev1_system_manager_msg msg = { };
> + struct device *dev = ts->dev;
> + unsigned int val;
> + int ret;
> +
> + if (!axiom_driver_supports_usage(ts, AXIOM_U02))
> + return -EINVAL;
> +
> + /*
> + * Enter the bootloader mode requires 3 consecutive messages so we can't
> + * check for the response.
> + */
> + msg.command = cpu_to_le16(AXIOM_U02_REV1_CMD_ENTERBOOTLOADER);
> + msg.parameters[0] = cpu_to_le16(AXIOM_U02_REV1_PARAM0_ENTERBOOLOADER_KEY1);
> + ret = axiom_u02_send_msg(ts, &msg, false);
> + if (ret) {
> + dev_err(dev, "Failed to send bootloader-key1: %d\n", ret);
> + return ret;
> + }
A delay is required between commands. 10ms is fine.
Can I make use of the axiom_u02_wait_idle() logic which checks the
AXIOM_U02_REV1_RESP_SUCCESS? Arbitrary delays are always a source of
trouble.
Yes, I tested with axiom_u02_wait_idle() which is OK.
I am slightly worried about too short a delay to axiom causing instability,
however this works fine.
It can unfortunately be an unstable device..
> + msg.parameters[0] = cpu_to_le16(AXIOM_U02_REV1_PARAM0_ENTERBOOLOADER_KEY2);
> + ret = axiom_u02_send_msg(ts, &msg, false);
> + if (ret) {
> + dev_err(dev, "Failed to send bootloader-key2: %d\n", ret);
> + return ret;
> + }
And here.
> +
> + msg.parameters[0] = cpu_to_le16(AXIOM_U02_REV1_PARAM0_ENTERBOOLOADER_KEY3);
> + ret = axiom_u02_send_msg(ts, &msg, false);
> + if (ret) {
> + dev_err(dev, "Failed to send bootloader-key3: %d\n", ret);
> + return ret;
> + }
> +
> + /* Sleep before the first read to give the device time */
> + fsleep(250 * USEC_PER_MSEC);
> +
> + /* Wait till the device reports it is in bootloader mode */
> + return regmap_read_poll_timeout(ts->regmap,
> + AXIOM_U31_REV1_DEVICE_ID_HIGH_REG, val,
> + FIELD_GET(AXIOM_U31_REV1_MODE_MASK, val) ==
> + AXIOM_U31_REV1_MODE_BLP, 250 * USEC_PER_MSEC,
> + USEC_PER_SEC);
> +}
Just to note if we cannot enter bootloader with u02 due to a corrupted firmware,
you can enter bootloader if the nRESET line is toggled 5 times without comms.
This could be added later on by $dev (maybe you :)) since I can't test
this. Our system has the reset line not connected :/
Sounds good.
...
> +static int axiom_u33_read(struct axiom_data *ts, struct axiom_crc *crc)
> +{
> + struct device *dev = ts->dev;
> + unsigned int reg;
> + int ret;
> +
> + if (!axiom_driver_supports_usage(ts, AXIOM_U33))
> + return -EINVAL;
> +
> + if (axiom_usage_rev(ts, AXIOM_U33) == 2) {
> + struct axiom_u33_rev2 val;
> +
> + reg = axiom_usage_baseaddr(ts, AXIOM_U33);
> + ret = regmap_raw_read(ts->regmap, reg, &val, sizeof(val));
Could we read into a raw buffer to save having to define a little endian
version of the CRCs?
I don't see the benefit.
OK.
...
> +/* Custom regmap read/write handling is required due to the aXiom protocol */
> +static int axiom_regmap_read(void *context, const void *reg_buf, size_t reg_size,
> + void *val_buf, size_t val_size)
> +{
> + struct device *dev = context;
> + struct i2c_client *i2c = to_i2c_client(dev);
> + struct axiom_data *ts = i2c_get_clientdata(i2c);
> + struct axiom_cmd_header hdr;
> + u16 xferlen, addr, baseaddr;
> + struct i2c_msg xfer[2];
> + int ret;
> +
> + if (val_size > AXIOM_MAX_XFERLEN) {
> + dev_err(ts->dev, "Exceed max xferlen: %zu > %u\n",
> + val_size, AXIOM_MAX_XFERLEN);
> + return -EINVAL;
> + }
> +
> + addr = *((u16 *)reg_buf);
> + hdr.target_address = cpu_to_le16(addr);
> + xferlen = FIELD_PREP(AXIOM_CMD_HDR_DIR_MASK, AXIOM_CMD_HDR_READ) |
> + FIELD_PREP(AXIOM_CMD_HDR_LEN_MASK, val_size);
> + hdr.xferlen = cpu_to_le16(xferlen);
> +
> + /* Verify that usage including the usage rev is supported */
> + baseaddr = addr & AXIOM_USAGE_BASEADDR_MASK;
> + if (!axiom_usage_supported(ts, baseaddr))
> + return -EINVAL;
> +
> + xfer[0].addr = i2c->addr;
> + xfer[0].flags = 0;
> + xfer[0].len = sizeof(hdr);
> + xfer[0].buf = (u8 *)&hdr;
> +
> + xfer[1].addr = i2c->addr;
> + xfer[1].flags = I2C_M_RD;
> + xfer[1].len = val_size;
> + xfer[1].buf = val_buf;
> +
> + ret = i2c_transfer(i2c->adapter, xfer, 2);
> + if (ret == 2)
> + return 0;
> + else if (ret < 0)
> + return ret;
> + else
> + return -EIO;
> +}
There needs to be atleast 40us holdoff between axiom bus transfers.
I am not sure that has been considered here.
Is this written somewhere within the datasheet/programming-guide?
In aXiom Comms Protocol in v4.8.9 if you have access to the webportal
it says to use 40us holdoff for report reading. Although this may apply
to all transactions.
Doing comms while axiom is changing the DMA causes issues (NAKs), for the
driver I posted atleast, the holdoff was required otherwise I would receive
0-length reports frequently.
It looks to be fine currently, however if there is unstability for users
we should consider adding this or something similar.
...
> +static enum fw_upload_err
> +axiom_cfg_fw_prepare(struct fw_upload *fw_upload, const u8 *data, u32 size)
> +{
...
> + cur_runtime_crc = ts->crc[AXIOM_CRC_CUR].runtime;
> + fw_runtime_crc = ts->crc[AXIOM_CRC_NEW].runtime;
> + if (cur_runtime_crc != fw_runtime_crc) {
> + dev_err(dev, "TH2CFG and device runtime CRC doesn't match: %#x != %#x\n",
> + fw_runtime_crc, cur_runtime_crc);
> + ret = FW_UPLOAD_ERR_FW_INVALID;
> + goto out;
> + }
The firmware CRCs dont need to match for a config load, only the usage revision/length.
What difference does it make? The firmware CRC implicit includes the
usage revision and the length (register layout). So we can ensure that
the configuration was made for the correct register layout without
checking each register and revision.
Different firmware revisions/CRCs can have compatible usages.
Aslong as the usage revisions match to u31 the usages will be compatible.
For us atleast we have different CRCs for small firmware changes, therefore
if testing such firmware here we would always have to uncomment this section.
In the updated python I changed the check to the following:
# Compare the firmware runtime CRC from the file with the CRC from the device.
# Only proceed if the CRCs match.
if not force:
if u33_from_file.fld_runtime_crc != ax.u33.fld_runtime_crc:
logging.error("Cannot load config file as it was saved from a different revision of firmware:")
logging.error("Firmware info from device : 0x{0:08X}, {1}".format(ax.u33.fld_runtime_crc, ax.u31.get_device_info_short()))
logging.error("Firmware info from config file : 0x{0:08X}, {1}".format(u33_from_file.fld_runtime_crc, u31_from_file.get_device_info_short()))
return ERROR_CFG_FILE_NOT_COMPATIBLE
else:
if u33_from_file.fld_runtime_crc != ax.u33.fld_runtime_crc:
logging.warning("The config file was saved from a different revision of firmware therefore it may not be compatible:")
logging.warning("Firmware info from device : 0x{0:08X}, {1}".format(ax.u33.fld_runtime_crc, ax.u31.get_device_info_short()))
logging.warning("Firmware info from config file : 0x{0:08X}, {1}".format(u33_from_file.fld_runtime_crc, u31_from_file.get_device_info_short()))
# Now ensure the config is compatible with the device
valid_cfg = False
file_usage_table = u31_from_file.get_usage_table()
for usage in usages.keys():
if usage not in ax.u31.get_usages():
logging.error(f"Usage u{usage:02x} unsupported on this device.")
break
usage_entry = ax.u31.get_usage_entry(usage)
if usage_entry.start_page != file_usage_table[usage].start_page:
logging.error(f"Incompatible config address for u{usage:02x}:")
logging.error(f" Device: 0x{usage_entry.start_page:02x}00 File: 0x{file_usage_table[usage].start_page:02x}00")
break
if ax.get_usage_length(usage) != len(usages[usage][2]):
logging.error(f"Incompatible config length for u{usage:02x}:")
logging.error(f" Device: {ax.get_usage_length(usage)} File: {len(usages[usage][2])}")
break
else:
valid_cfg = True
if not valid_cfg:
logging.error("Cannot load config as the usages are incompatible with the device.")
return ERROR_CFG_FILE_NOT_COMPATIBLE
Possibly we could add a force parameter to the sysfs like above?
...
> +static enum fw_upload_err
> +axiom_cfg_fw_write(struct fw_upload *fw_upload, const u8 *data, u32 offset,
> + u32 size, u32 *written)
> +{
....
> + /* Ensure that the chunks are written correctly */
> + ret = axiom_verify_volatile_mem(ts);
> + if (ret) {
> + dev_err(dev, "Failed to verify written config, abort\n");
> + goto err_swreset;
> + }
> +
> + ret = axiom_u02_save_config(ts);
> + if (ret)
> + goto err_swreset;
> +
> + /*
> + * TODO: Check if u02 start would be sufficient to load the new config
> + * values
> + */
It is not necessarily needed.
What do you mean by this? Do we need the axiom_u02_swreset() or can we
just start the system via u02 (without swreset)?
There is no need to do a reset after a config load you can just start the AE
with CMD_START, but we can keep it as is since it does the same thing.
> + ret = axiom_u02_swreset(ts);
> + if (ret) {
> + dev_err(dev, "Soft reset failed\n");
> + goto err_unlock;
> + }
....
> +static ssize_t fw_variant_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct i2c_client *i2c = to_i2c_client(dev);
> + struct axiom_data *ts = i2c_get_clientdata(i2c);
> + const char *val;
> +
> + switch (ts->fw_variant) {
> + case 0:
> + val = "3d";
> + break;
> + case 1:
> + val = "2d";
> + break;
> + case 3:
> + val = "force";
> + break;
> + default:
> + val = "unknown";
> + break;
> + }
The following are all the variants we currently support in order:
FW_VARIANTS = ["3D", "2D", "FORCE", "0D", "XL"]
Means:
0 == 3d
1 == 2d
3 == force
4 == 0d
5 == xl
?
This is also something I can test on my site. Patches are welcome once
this is mainline of course :)
It is like so:
#define DEVICE_BUILD_VARIANT_3D (0U)
#define DEVICE_BUILD_VARIANT_2D (1U)
#define DEVICE_BUILD_VARIANT_FORCE (2U)
#define DEVICE_BUILD_VARIANT_0D (3U)
#define DEVICE_BUILD_VARIANT_XL (4U)
Regards,
Marco
--
#gernperDu
#CallMeByMyFirstName
Pengutronix e.K. | |
Steuerwalder Str. 21 | https://www.pengutronix.de |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-9 |
I shall try to give a more prompt review once you have the new version up.
Many Thanks,
Andrew