linux-kernel - Re: [PATCH 1/1] Adding support for aXiom touchscreen controller

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <LOYP123MB27019D7C9A3EEA198CAEB08C9793A@LOYP123MB2701.GBRP123.PROD.OUTLOOK.COM>
Date: Mon, 26 Jan 2026 16:27:53 +0000
From: Andrew Thomas <andrew.thomas@...chnetix.com>
To: Marco Felsch <m.felsch@...gutronix.de>
CC: "dmitry.torokhov@...il.com" <dmitry.torokhov@...il.com>,
	"linux-input@...r.kernel.org" <linux-input@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, Mark
 Satterthwaite <mark.satterthwaite@...chnetix.com>,
	"kamel.bouhara@...tlin.com" <kamel.bouhara@...tlin.com>,
	"kernel@...gutronix.de" <kernel@...gutronix.de>
Subject: Re: [PATCH 1/1] Adding support for aXiom touchscreen controller

Hi Marco,
Thank you very much for your quick review.
I have addressed many of your comments below.

> > +config TOUCHSCREEN_AXIOM_CORE
> > + tristate "TouchNetix Axiom touchscreen"
>
> Nack, the spi or i2c should select the core.
>

Changed.

> > +#define U41_COORD_SIZE (4)
> > +#define U41_Z_OFFSET (42)
>
> Align your defines.
>

Changed.

> > + input_set_abs_params(input_dev, ABS_MT_TOOL_TYPE, 0, MT_TOOL_MAX, 0, 0);
> > + input_set_abs_params(input_dev, ABS_MT_DISTANCE, 0, 127, 0, 0);
> > + input_set_abs_params(input_dev, ABS_MT_PRESSURE, 0, 127, 0, 0);
>
> Max values are coming from the firmware.
>

I am trying to keep the initial patch as simple as possible.
This is a fixed value describedin the usage definition.

> > +
> > + input_mt_init_slots(input_dev, U41_MAX_TARGETS, INPUT_MT_DIRECT);
>
> num-targets can be configured via firmware config IIRC, so nack.
>

That is correct, all u41 can be configured separately.
As above, I wish the keep this patch as simple as possible so we can
add support for this in the future.

> > + err = ax->bus_ops->read(ax->dev, 0x0, SIZE_U31_DEVICE_INFO,
> > + ax->read_buf);
>
> No need for custom accessors, use regmap API.
>

From my understanding the the interface is the same.
Manual I2C/SPI bus handling is still required.

> > + ax->bus_ops = bus_ops;
> > + ax->irq = irq;
>
> You allocate absolute no ext. resources like regulators or reset-gpios.
>

If you think this would be needed in order for you to test this patch
we can add it.
As above, I wish the patch to a minimal support of I2C and SPI and furthur
features can be added ontop.

>
> > + switch (hdr.report_usage) {
> > + case AX_2DCTS_REPORT_ID:
> > + err = axiom_process_u41_report(ax,
> > + &report[AX_U34_PAYLOAD_BUFFER]);
>
> May I ask why you guys write in your programming manual, that the host
> needs to check the usage-revision and you completely ignore this?

As above.
all usage revisions will handle this identically.


Regards,
Andrew