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

[Andrew Thomas]

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