drivers: touchscreen: tsc2007 and ads7846/tsc2046 improvements (use common touchscreen bindings, pre-calibration, spi fix and provide iio raw values)

commit b98abe52fa8e ("Input: add common DT binding for touchscreens") introduced common DT bindings for touchscreens [1] and a helper function to parse the DT.

commit ed7c9870c9bc ("Input: of_touchscreen - add support for inverted / swapped axes") added another helper for parsing axis inversion and swapping
and applying them to x and y coordinates.

Both helpers have been integrated to accommodate any orientation of the touch panel in relation to the LCD.

A new feature is to introduce scaling the min/max ADC values to the screen size.

This makes it possible to pre-calibrate the touch so that is (almost) exactly matches the LCD pixel coordinates it is glued onto. This allows to well enough operate the touch before a user space calibration step can improve the precision.

Please note that the old ti,fuzz properties have been removed since they are replaced by the common bindings touchscreen-fuzz-x/y/z.

Finally, calculate_pressure has been renamed to calculate_resistance because that is what it is doing.

[1]: Documentation/devicetree/bindings/input/touchscreen/touchscreen.txt

Signed-off-by H. Nikolaus Schaller <[email protected]>

---
 .../bindings/input/touchscreen/tsc2007.txt         |  20 ++--

 drivers/input/touchscreen/tsc2007.c                | 120 +++++++++++++++++----
 include/linux/i2c/tsc2007.h                        |   8 ++
 3 files changed, 118 insertions(+), 30 deletions(-)

commit b98abe52fa8e ("Input: add common DT binding for touchscreens") introduced common DT bindings for touchscreens [1] and a helper function to parse the DT.

commit ed7c9870c9bc ("Input: of_touchscreen - add support for inverted / swapped axes") added another helper for parsing axis inversion and swapping
and applying them to x and y coordinates.

Both helpers have been integrated to accommodate any orientation of the touch panel in relation to the LCD.

A new feature is to introduce scaling the min/max ADC values to the screen size.

This makes it possible to pre-calibrate the touch so that is (almost) exactly matches the LCD pixel coordinates it is glued onto. This allows to well enough operate the touch before a user space calibration step can improve the precision.

Please note that the old ti,fuzz properties have been removed since they are replaced by the common bindings touchscreen-fuzz-x/y/z.

Finally, calculate_pressure has been renamed to calculate_resistance because that is what it is doing.

Am 28.01.2017 um 20:33 schrieb Dmitry Torokhov <[email protected]>:

Hi Nikolaus,

Quoted 14 lines · H. Nikolaus Schaller

commit b98abe52fa8e ("Input: add common DT binding for touchscreens") introduced common DT bindings for touchscreens [1] and a helper function to parse the DT.

commit ed7c9870c9bc ("Input: of_touchscreen - add support for inverted / swapped axes") added another helper for parsing axis inversion and swapping
and applying them to x and y coordinates.

Both helpers have been integrated to accommodate any orientation of the touch panel in relation to the LCD.

A new feature is to introduce scaling the min/max ADC values to the screen size.

Quoted 4 lines

This makes it possible to pre-calibrate the touch so that is (almost) exactly matches the LCD pixel coordinates it is glued onto. This allows to well enough operate the touch before a user space calibration step can improve the precision.

I question whether doing scaling in kernel is really right solution.

Why do you want this?

If your touch resolution is lower than your screen
then it might be useful, but if it is lower then you are losing data that can be very helpful for gesture recognition, and I hope you design your userspace so it can handle not only "bad" hardware, but "good" as well. And even with "bad" there are a lot of tricks that can be done to get "better" touch position in userspace.

Quoted 8 lines

This makes it possible to pre-calibrate the touch so that is (almost)

Please note that the old ti,fuzz properties have been removed since they are replaced by the common bindings touchscreen-fuzz-x/y/z.

Quoted 2 lines

Finally, calculate_pressure has been renamed to calculate_resistance because that is what it is doing.

That is not what your patch does though. In the presence of "ti,report-resistance" parameter you start reporting resistance through ABS_PRESSURE

without any indication to the userspace that meaning of event changed. This is no better if than reporting it through ABS_X. You should not override meaning of input events.

Hi Dmitry,

Quoted 19 lines

Am 28.01.2017 um 20:33 schrieb Dmitry Torokhov <[email protected]>:

Hi Nikolaus,

Quoted 14 lines · H. Nikolaus Schaller

commit b98abe52fa8e ("Input: add common DT binding for touchscreens") introduced common DT bindings for touchscreens [1] and a helper function to parse the DT.

commit ed7c9870c9bc ("Input: of_touchscreen - add support for inverted / swapped axes") added another helper for parsing axis inversion and swapping
and applying them to x and y coordinates.

Both helpers have been integrated to accommodate any orientation of the touch panel in relation to the LCD.

A new feature is to introduce scaling the min/max ADC values to the screen size.

Quoted 6 lines

Quoted 4 lines

This makes it possible to pre-calibrate the touch so that is (almost) exactly matches the LCD pixel coordinates it is glued onto. This allows to well enough operate the touch before a user space calibration step can improve the precision.

I question whether doing scaling in kernel is really right solution.

Since lower left corner does not exactly report [0 0] and upper right corner does not report [4095 4095] from the ADC we need offset and steepness correction of the ADC values.

This steepness is the scaling that must happen in kernel and I don't understand why you want to propagate this ADC errors to user-space by avoiding scaling.

Let me iterate what we want to achieve:
* use new common bindings
* offset and steepness calibration of the ADC (called pre-calibration). This makes a real device much more reliable to operate with factory installed scaling factors.
* flipping and rotation

(note that touch pixel to LCD pixel scaling is not explicitly on this list!)

Now to achieve the ADC pre-calibration we must calculate

	x' = (x - ti,min-x) / (ti,max-x-ti,min-x) giving a rante from 0.0 ... 1.0

This is scaled up to what is defined by touchscreen-size-x, i.e.

	x' = (touchscreen-size-x * (x - ti,min-x)) / (ti,max-x-ti,min-x)

How do you want to avoid this scaling to take place? It happens automatically. It is not even an additional line of code. And is necessary for compensating ADC offsets and steepness.

So the only way to avoid the scaling option is to eliminate the precalibration/ADC compensation which is essential for a device which has no means to properly calibrate before operating the device through touch.

The other option would be to avoid common bindings and set

touchscreen-size-x = (ti,max-x-ti,min-x)

This is heavily dependent on specific ADC offsets forwarded to user-space. IMHO the worst we can do (and the current tsc2007 driver does it that way!).

Quoted 2 lines

Why do you want this?

It seems that you assume that we want to enforce 1:1 scaling between touch pixels and LCD pixels and have designed code to achieve exactly that.

This is not the case. It is just a byproduct that you can do it.

And since it is easier to understand we have made the examples this way.

Quoted 6 lines

If your touch resolution is lower than your screen
then it might be useful, but if it is lower then you are losing data that can be very helpful for gesture recognition, and I hope you design your userspace so it can handle not only "bad" hardware, but "good" as well. And even with "bad" there are a lot of tricks that can be done to get "better" touch position in userspace.

You can just *choose* DT parameters touchscreen-size-x to match the LCD size. This of course reduces the touch precision to full LCD pixels. For finger- touch operated devices, subpixel precision is rarely needed.

Also, some user-spaces (e.g. older Replicant for GTA04) assume that there is such an 1:1 mapping and they will be perfectly happy about this.

But, if you can modify your user-space easily, you can also choose a different factor.

You can for example define touchscreen-size-x=<4096> and then you get almost the highest precision of the ADC and won't loose any bits. Or even define a bigger range and get steps >1 bit.

LCD driver (e.g. X11 calibration matrix) can scale it down again to LCD pixels.

So effectively we get for LCD pixels when the touch sets a mouse pointer:

	x'' = user-space-scale * <<input-event<< ((touchscreen-size-x * (x - ti,min-x)) / (ti,max-x-ti,min-x))

The most simple setup would then be but others are possible:

user-space-scale = 1
touchscreen-size-x = LCD-size-x

Let me cite myself:

Quoted 1 line

Quoted 1 line

This makes it possible to pre-calibrate the touch so that is (almost)

Quoted 6 lines

Quoted 6 lines

Please note that the old ti,fuzz properties have been removed since they are replaced by the common bindings touchscreen-fuzz-x/y/z.

Quoted 6 lines

Quoted 2 lines

Finally, calculate_pressure has been renamed to calculate_resistance because that is what it is doing.

That is not what your patch does though. In the presence of "ti,report-resistance" parameter you start reporting resistance through ABS_PRESSURE

Well, there is some historic confusion wether this driver reports resistance or pressure.

The unpatched tsc2007 driver does it wrong (please test!) and we fix it on the fly (because a separate patch is much more complex than doing it right immediately).

This ti,report-resistance property is a means to get the old (wrong) meaning back in case someone urgently needs it and can't fix the user-space workaround which he must be using.

AFAIK there is no mainline board using the DT except ours (and the upcoming OMAP5-Pyra), so we shouldn't care too much. If you prefer, you can remove this compatibility property. We don't need it for our devices.

That the function name is wrong is a second issue and this double negation might confuse a litte.

Please test on a real device if the patched driver reports pressure now (unless ti,report-resistance is specified).

Quoted 3 lines

AFAIK there is no mainline board using the DT except ours (and the upcoming OMAP5-Pyra), so we shouldn't care too much. If you prefer, you can remove this compatibility property. We don't need it for our devices.

You seem to be treating DT data as something very fluid, which is wrong. You need to treat it as a firmware, unlikely to change once device is shipped. Unlike legacy platform data, the fact that DTS files are not present in mainline does not mean that we can ignore such users and change behavior at will.

That said, if driver behavior is out of line from the subsystem expectations, we need to fix it.

Quoted 5 lines

That the function name is wrong is a second issue and this double negation might confuse a litte.

Please test on a real device if the patched driver reports pressure now (unless ti,report-resistance is specified).

I unfortunately can not test this driver as I do not have the hardware. So all my observations are from code and data sheets.

That said, what is the values emitted as ABS_PRESSURE when finger is not touching the device, barely touching the device, or pressing firmly? It seems that between TSC2007, TSC2004, TSC2005, and ADS7846, we have confusion as to what is being reported.

I am adding a few more folks to the CC so we can try and soft this out. Sebastian, Pali, Pavel, any input here?

Am 18.02.2017 um 04:22 schrieb Sebastian Reichel <[email protected]>:

Hi,

Quoted 3 lines · Dmitry Torokhov

Quoted 3 lines

AFAIK there is no mainline board using the DT except ours (and the upcoming OMAP5-Pyra), so we shouldn't care too much. If you prefer, you can remove this compatibility property. We don't need it for our devices.

$ cd linux.git/arch
$ git grep -l tsc2004
arm/boot/dts/imx6qdl-nit6xlite.dtsi
arm/boot/dts/imx7d-nitrogen7.dts
arm/boot/dts/logicpd-torpedo-37xx-devkit.dts
arm/boot/dts/omap4-var-som-om44.dtsi
$ git grep -l tsc2005
arm/boot/dts/omap3-n900.dts

$ git grep -l tsc2007
arm/boot/dts/imx28-tx28.dts
arm/boot/dts/imx35-eukrea-cpuimx35.dtsi
arm/boot/dts/imx51-eukrea-cpuimx51.dtsi
arm/boot/dts/imx53-tx53-x03x.dts
arm/boot/dts/imx6qdl-tx6.dtsi
arm/boot/dts/imx6ul-tx6ul.dtsi
arm/boot/dts/omap3-gta04.dtsi
sh/boards/mach-ecovec24/setup.c

Quoted 20 lines

You seem to be treating DT data as something very fluid, which is wrong. You need to treat it as a firmware, unlikely to change once device is shipped. Unlike legacy platform data, the fact that DTS files are not present in mainline does not mean that we can ignore such users and change behavior at will.

That said, if driver behavior is out of line from the subsystem expectations, we need to fix it.

Quoted 5 lines

That the function name is wrong is a second issue and this double negation might confuse a litte.

Please test on a real device if the patched driver reports pressure now (unless ti,report-resistance is specified).

I unfortunately can not test this driver as I do not have the hardware. So all my observations are from code and data sheets.

Quoted 4 lines

That said, what is the values emitted as ABS_PRESSURE when finger is not touching the device, barely touching the device, or pressing firmly? It seems that between TSC2007, TSC2004, TSC2005, and ADS7846, we have confusion as to what is being reported.

As far as I can see all calculate Rtouch and ADS7846 reports (Rmax - Rtouch), which looks sensible.

Quoted 3 lines

I am adding a few more folks to the CC so we can try and soft this out. Sebastian, Pali, Pavel, any input here?

I think tsc200x works, since usually userspace is Xorg and I think it only cares for x/y coordinates + boolean pressure. Since no-pressure is correctly reported as 0, everything works as expected.

I currently don't have X running on my N900 due some omapdrm bug, so I can't test this, sorry.

I suggest to put the resistance vs pressure thing in its own patch, that also fixes tsc200x-core and merge it to linux-next after the merge window.

-- Sebastian

Hi Sebastian,

Quoted 9 lines

Am 18.02.2017 um 04:22 schrieb Sebastian Reichel <[email protected]>:

Hi,

Quoted 3 lines · Dmitry Torokhov

Quoted 3 lines

AFAIK there is no mainline board using the DT except ours (and the upcoming OMAP5-Pyra), so we shouldn't care too much. If you prefer, you can remove this compatibility property. We don't need it for our devices.

Quoted 8 lines

$ cd linux.git/arch
$ git grep -l tsc2004
arm/boot/dts/imx6qdl-nit6xlite.dtsi
arm/boot/dts/imx7d-nitrogen7.dts
arm/boot/dts/logicpd-torpedo-37xx-devkit.dts
arm/boot/dts/omap4-var-som-om44.dtsi
$ git grep -l tsc2005
arm/boot/dts/omap3-n900.dts

Those are not relevant since tsc2004/5 and tsc2007 are independent drivers and don't share code.

Hence the N900 is not influenced by this patch series. If it has a similar issue, it should be fixed of course.

Quoted 9 lines

$ git grep -l tsc2007
arm/boot/dts/imx28-tx28.dts
arm/boot/dts/imx35-eukrea-cpuimx35.dtsi
arm/boot/dts/imx51-eukrea-cpuimx51.dtsi
arm/boot/dts/imx53-tx53-x03x.dts
arm/boot/dts/imx6qdl-tx6.dtsi
arm/boot/dts/imx6ul-tx6ul.dtsi
arm/boot/dts/omap3-gta04.dtsi
sh/boards/mach-ecovec24/setup.c

Sorry, I was a little imprecise here, because I looked for the min/max properties. Of course, the imx devices use the tsc2007 as well.

Maybe we should edit all these DTS and set the "ti,report-resistance" property by default. Then, no user should notice a difference.

Is any user/maintainer of these devices following this discussion and can comment?

Quoted 25 lines

Quoted 24 lines

You seem to be treating DT data as something very fluid, which is wrong. You need to treat it as a firmware, unlikely to change once device is shipped. Unlike legacy platform data, the fact that DTS files are not present in mainline does not mean that we can ignore such users and change behavior at will.

That said, if driver behavior is out of line from the subsystem expectations, we need to fix it.

Quoted 5 lines

That the function name is wrong is a second issue and this double negation might confuse a litte.

Please test on a real device if the patched driver reports pressure now (unless ti,report-resistance is specified).

I unfortunately can not test this driver as I do not have the hardware. So all my observations are from code and data sheets.

That said, what is the values emitted as ABS_PRESSURE when finger is not touching the device, barely touching the device, or pressing firmly? It seems that between TSC2007, TSC2004, TSC2005, and ADS7846, we have confusion as to what is being reported.

Quoted 2 lines

As far as I can see all calculate Rtouch and ADS7846 reports (Rmax - Rtouch), which looks sensible.

I don't see where this subtraction from Rmax takes place for the tsc2007:

http://lxr.free-electrons.com/source/drivers/input/touchscreen/tsc2007.c#L131

Quoted 7 lines

Quoted 2 lines

I am adding a few more folks to the CC so we can try and soft this out. Sebastian, Pali, Pavel, any input here?

I think tsc200x works, since usually userspace is Xorg and I think it only cares for x/y coordinates + boolean pressure. Since no-pressure is correctly reported as 0, everything works as expected.

No pressure is usually treated as a special case in these drivers, so reduction to "boolean" in user-space works well by accident and might still hide a bug.

Quoted 2 lines

I currently don't have X running on my N900 due some omapdrm bug, so I can't test this, sorry.

I usually look with evtest if ABS_PRESSURE is monotonic.

Quoted 3 lines

I suggest to put the resistance vs pressure thing in its own patch, that also fixes tsc200x-core and merge it to linux-next after the merge window.

Am 19.02.2017 um 00:44 schrieb Sebastian Reichel <[email protected]>:

Hi,

Quoted 20 lines · H. Nikolaus Schaller

Hi Sebastian,

Quoted 9 lines

Am 18.02.2017 um 04:22 schrieb Sebastian Reichel <[email protected]>:

Hi,

Quoted 3 lines · Dmitry Torokhov

Quoted 3 lines

AFAIK there is no mainline board using the DT except ours (and the upcoming OMAP5-Pyra), so we shouldn't care too much. If you prefer, you can remove this compatibility property. We don't need it for our devices.

Quoted 8 lines

$ cd linux.git/arch
$ git grep -l tsc2004
arm/boot/dts/imx6qdl-nit6xlite.dtsi
arm/boot/dts/imx7d-nitrogen7.dts
arm/boot/dts/logicpd-torpedo-37xx-devkit.dts
arm/boot/dts/omap4-var-som-om44.dtsi
$ git grep -l tsc2005
arm/boot/dts/omap3-n900.dts

Quoted 2 lines

Those are not relevant since tsc2004/5 and tsc2007 are independent drivers and don't share code.

Yes, I'm aware.

Quoted 2 lines

Hence the N900 is not influenced by this patch series. If it has a similar issue, it should be fixed of course.

Right. I added them to see every board affect by the patch suggested by me in my last paragraph.

Quoted 14 lines

Quoted 10 lines

$ git grep -l tsc2007
arm/boot/dts/imx28-tx28.dts
arm/boot/dts/imx35-eukrea-cpuimx35.dtsi
arm/boot/dts/imx51-eukrea-cpuimx51.dtsi
arm/boot/dts/imx53-tx53-x03x.dts
arm/boot/dts/imx6qdl-tx6.dtsi
arm/boot/dts/imx6ul-tx6ul.dtsi
arm/boot/dts/omap3-gta04.dtsi
sh/boards/mach-ecovec24/setup.c

Sorry, I was a little imprecise here, because I looked for the min/max properties. Of course, the imx devices use the tsc2007 as well.

Quoted 2 lines

Maybe we should edit all these DTS and set the "ti,report-resistance" property by default. Then, no user should notice a difference.

I suggest to create a patch without the report-reistance stuff and add it early after the merge window and see what happens. If no users notices anything the change is not an ABI break from kernel's PoV.

Quoted 33 lines

Is any user/maintainer of these devices following this discussion and can comment?

Quoted 20 lines

Quoted 20 lines

You seem to be treating DT data as something very fluid, which is wrong. You need to treat it as a firmware, unlikely to change once device is shipped. Unlike legacy platform data, the fact that DTS files are not present in mainline does not mean that we can ignore such users and change behavior at will.

That said, if driver behavior is out of line from the subsystem expectations, we need to fix it.

Quoted 5 lines

That the function name is wrong is a second issue and this double negation might confuse a litte.

Please test on a real device if the patched driver reports pressure now (unless ti,report-resistance is specified).

I unfortunately can not test this driver as I do not have the hardware. So all my observations are from code and data sheets.

Quoted 7 lines

Quoted 4 lines

That said, what is the values emitted as ABS_PRESSURE when finger is not touching the device, barely touching the device, or pressing firmly? It seems that between TSC2007, TSC2004, TSC2005, and ADS7846, we have confusion as to what is being reported.

As far as I can see all calculate Rtouch and ADS7846 reports (Rmax - Rtouch), which looks sensible.

I don't see where this subtraction from Rmax takes place for the tsc2007:

Quoted 1 line

http://lxr.free-electrons.com/source/drivers/input/touchscreen/tsc2007.c#L131

sorry if I wrote this ambiguous, let me split my sentence

1. tsc200x & ads7846 calculate Rtouch
2. ads7846 reports Rmax - Rtouch
(3. tsc200x does not, it reports Rtouch instead)
4. ads7846 behaviour looks sensible to me

Quoted 4 lines

Quoted 4 lines

Quoted 3 lines

I am adding a few more folks to the CC so we can try and soft this out. Sebastian, Pali, Pavel, any input here?

Quoted 8 lines

Quoted 4 lines

I think tsc200x works, since usually userspace is Xorg and I think it only cares for x/y coordinates + boolean pressure. Since no-pressure is correctly reported as 0, everything works as expected.

No pressure is usually treated as a special case in these drivers, so reduction to "boolean" in user-space works well by accident and might still hide a bug.

That's what I assumed.

Btw. how did you notice that tsc2007 sends "inverted" pressure values? Just in evtest or in some non-development application? (Just asking because the behavour obviously changes at least for that usecase)

Quoted 5 lines

Quoted 3 lines

I currently don't have X running on my N900 due some omapdrm bug, so I can't test this, sorry.

I usually look with evtest if ABS_PRESSURE is monotonic.

That would not have helped to check if X handles the touchscreen in a boolean way. I can provide some N900 evtest data, though (tomorrow, I don't have my dev N900 with me at the moment).

Quoted 4 lines

Quoted 4 lines

I suggest to put the resistance vs pressure thing in its own patch, that also fixes tsc200x-core and merge it to linux-next after the merge window.

I don't really remember when we noticed it first. Maybe it was back in tslib times some years ago where setting the sensitivity threshold made problems. We then carried along our patch for a long time in our local repo (and modified it several times) and only started upstreaming some months ago. [...]

AFAIK, GIMP and for example https://sourceforge.net/projects/xournal/ appear to be able to handle X pressure, but I haven't running and tested either one on our devices. Pressure is used in such drawing tools to simulate that some physical pens make wider strokes on higher pressure.

This seems to indicate that X can handle pressure in a non-boolean way, but rarely does. Especially I think the usual menu, click, drag, scroll gestures are only based on BTN_TOUCH status and not on ABS_PRESSURE. So it is rarely noticed to make a difference.

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

I suggest to put the resistance vs pressure thing in its own patch, that also fixes tsc200x-core and merge it to linux-next after the merge window.

Ok. I will propose a patch.

Am 19.02.2017 um 21:15 schrieb Sebastian Reichel <[email protected]>:

Hi,

Quoted 11 lines · H. Nikolaus Schaller

I don't really remember when we noticed it first. Maybe it was back in tslib times some years ago where setting the sensitivity threshold made problems. We then carried along our patch for a long time in our local repo (and modified it several times) and only started upstreaming some months ago. [...]

AFAIK, GIMP and for example https://sourceforge.net/projects/xournal/ appear to be able to handle X pressure, but I haven't running and tested either one on our devices. Pressure is used in such drawing tools to simulate that some physical pens make wider strokes on higher pressure.

Quoted 4 lines

This seems to indicate that X can handle pressure in a non-boolean way, but rarely does. Especially I think the usual menu, click, drag, scroll gestures are only based on BTN_TOUCH status and not on ABS_PRESSURE. So it is rarely noticed to make a difference.

ok.

Quoted 5 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

I suggest to put the resistance vs pressure thing in its own patch, that also fixes tsc200x-core and merge it to linux-next after the merge window.

Ok. I will propose a patch.

Thanks. I suggest to add this in the patch description:

While this patch changes the values reported to userspace, ABS_PRESSURE is used rarely by userspace. Most software only relies on BTN_TOUCH (boolean), which is not affected by this patch. Some graphics software makes use of the interface and does not work correctly with the currently used inverted behaviour.

Quoted 4 lines

Let me cite myself:

Quoted 2 lines

Quoted 2 lines

This makes it possible to pre-calibrate the touch so that is (almost)

Yes, it allows you to pre-calibrate, I get it. But you still do properly calibrate later, right? So you are doing double work here, once in kernel, and second time in userspace.

Quoted 7 lines

Well, there is some historic confusion wether this driver reports resistance or pressure.

The unpatched tsc2007 driver does it wrong (please test!) and we fix it on the fly (because a separate patch is much more complex than doing it right immediately).

Quoted 8 lines

This ti,report-resistance property is a means to get the old (wrong) meaning back in case someone urgently needs it and can't fix the user-space workaround which he must be using.

AFAIK there is no mainline board using the DT except ours (and the upcoming OMAP5-Pyra), so we shouldn't care too much. If you prefer, you can remove this compatibility property. We don't need it for our devices.

Quoted 5 lines

That the function name is wrong is a second issue and this double negation might confuse a litte.

Please test on a real device if the patched driver reports pressure now (unless ti,report-resistance is specified).

I unfortunately can not test this driver as I do not have the hardware. So all my observations are from code and data sheets.

That said, what is the values emitted as ABS_PRESSURE when finger is not touching the device, barely touching the device, or pressing firmly? It seems that between TSC2007, TSC2004, TSC2005, and ADS7846, we have confusion as to what is being reported.

I am adding a few more folks to the CC so we can try and soft this out. Sebastian, Pali, Pavel, any input here?

Am 18.02.2017 um 10:15 schrieb Pavel Machek <[email protected]>:

Hi!

Quoted 12 lines

Quoted 12 lines

Well, there is some historic confusion wether this driver reports resistance or pressure.

The unpatched tsc2007 driver does it wrong (please test!) and we fix it on the fly (because a separate patch is much more complex than doing it right immediately).

This ti,report-resistance property is a means to get the old (wrong) meaning back in case someone urgently needs it and can't fix the user-space workaround which he must be using.

Quoted 3 lines

Quoted 3 lines

AFAIK there is no mainline board using the DT except ours (and the upcoming OMAP5-Pyra), so we shouldn't care too much. If you prefer, you can remove this compatibility property. We don't need it for our devices.

N900 is mainline and uses DT.

Quoted 15 lines

Quoted 6 lines

That the function name is wrong is a second issue and this double negation might confuse a litte.

Please test on a real device if the patched driver reports pressure now (unless ti,report-resistance is specified).

I unfortunately can not test this driver as I do not have the hardware. So all my observations are from code and data sheets.

That said, what is the values emitted as ABS_PRESSURE when finger is not touching the device, barely touching the device, or pressing firmly? It seems that between TSC2007, TSC2004, TSC2005, and ADS7846, we have confusion as to what is being reported.

Quoted 2 lines

I am adding a few more folks to the CC so we can try and soft this out. Sebastian, Pali, Pavel, any input here?

X work ok on N900. Nikolaus wrote rather long email, but I'm not what the meaning is and what is supposed to be broken there.

I do this on X startup:

xinput --set-prop --type=float "TSC200X touchscreen" "Coordinate Transformation Matrix" 1.10 0.00 -0.05 0.00 1.18 -0.10 0.00 0.00 1.00 xinput --set-prop --type=int "TSC200X touchscreen" "Evdev Axis Inversion" 0 1
xinput --set-prop --type=float "TSC2005 touchscreen" "Coordinate Transformation Matrix" 1\
.10 0.00 -0.05 0.00 1.18 -0.10 0.00 0.00 1.00
xinput --set-prop --type=int "TSC2005 touchscreen" "Evdev Axis Inversion" 0 1

And I agree that kernel should _not_ attempt rescaling itself, as it would lose precision.

Providing default calibration info is ok.

Quoted 2 lines

And I agree that kernel should _not_ attempt rescaling itself, as it would lose precision.

With an almost 1:1 mapping you won't loose precision.

Am 18.02.2017 um 19:08 schrieb Pavel Machek <[email protected]>:

Quoted 4 lines

Quoted 2 lines

And I agree that kernel should _not_ attempt rescaling itself, as it would lose precision.

With an almost 1:1 mapping you won't loose precision.

How do you propose to do that?

Quoted 8 lines

Am 18.02.2017 um 19:08 schrieb Pavel Machek <[email protected]>:

Quoted 4 lines

Quoted 2 lines

And I agree that kernel should _not_ attempt rescaling itself, as it would lose precision.

With an almost 1:1 mapping you won't loose precision.

Quoted 1 line

How do you propose to do that?

something like

xinput --set-prop --type=float "TSC200X touchscreen" "Coordinate Transformation Matrix" 1.00 0.00 0.00 0.00 1.00 0.00 0.00 0.00 1.00

but I think it is the default of X11 if you use no coordinate transformation at all. And having the kernel to properly scale from ADC values to screen coordinates.

Am 18.02.2017 um 23:54 schrieb Pavel Machek <[email protected]>:

Quoted 12 lines · H. Nikolaus Schaller

Quoted 9 lines

Am 18.02.2017 um 19:08 schrieb Pavel Machek <[email protected]>:

Quoted 4 lines

Quoted 2 lines

And I agree that kernel should _not_ attempt rescaling itself, as it would lose precision.

With an almost 1:1 mapping you won't loose precision.

How do you propose to do that?

something like

Quoted 5 lines

xinput --set-prop --type=float "TSC200X touchscreen" "Coordinate Transformation Matrix" 1.00 0.00 0.00 0.00 1.00 0.00 0.00 0.00 1.00

but I think it is the default of X11 if you use no coordinate transformation at all. And having the kernel to properly scale from ADC values to screen coordinates.

No. How do you propose doing rescaling in the kernel without loosing precision?

About loosing precision: there is already noise (jitter) in real-world devices so that you can't achieve subpixel precision anyways (unless your panel has a very low resolution). Please see my answer to Dmitry some mails ago.

Am 19.02.2017 um 15:17 schrieb Pavel Machek <[email protected]>:

Hi!

Quoted 4 lines

About loosing precision: there is already noise (jitter) in real-world devices so that you can't achieve subpixel precision anyways (unless your panel has a very low resolution). Please see my answer to Dmitry some mails ago.

Maybe you can achieve better precision with averaging.

Anyway "input is already noisy" does not mean "so it is okay to degrade it more".

Solve it properly. That means passing calibration
data from kernel to userland.

Quoted 3 lines

Solve it properly. That means passing calibration
data from kernel to userland.

As written before, the really proper solution would be to provide floating or fixed point subpixel input events. Not arbitrarily scaling up in kernel and leaving downscaling to user space (where everybody can make it worse).

But I don't think it is worth implementing subpixel touch events for real world devices due to the jitter I mentioned.

Am 19.02.2017 um 18:15 schrieb Pavel Machek <[email protected]>:

Quoted 3 lines

Quoted 2 lines

Solve it properly. That means passing calibration
data from kernel to userland.

Quoted 4 lines

As written before, the really proper solution would be to provide floating or fixed point subpixel input events. Not arbitrarily scaling up in kernel and leaving downscaling to user space (where everybody can make it worse).

That has no advantages,

and floating point in kernel is hard. Also
you'd either have to invent new interface, or you'd break touchscreen for people that already have their touchscreens calibrated.

Just
pass calibration data to userland.

Quoted 3 lines

But I don't think it is worth implementing subpixel touch events for real world devices due to the jitter I mentioned.

Yes, that's not really proper solution, that just overengineered. Not worth implementing. Pass calibration data to userland.

Hi Pavel,
I love discussions with you :)

Quoted 11 lines

Am 19.02.2017 um 18:15 schrieb Pavel Machek <[email protected]>:

Quoted 7 lines

Quoted 2 lines

Solve it properly. That means passing calibration
data from kernel to userland.

As written before, the really proper solution would be to provide floating or fixed point subpixel input events. Not arbitrarily scaling up in kernel and leaving downscaling to user space (where everybody can make it worse).

Quoted 1 line

That has no advantages,

It has the advantage of providing you with the full precision of raw data (but properly scaled) so that you don't loose any bit of information. This is what you just asked for - one or two mails before.

Quoted 3 lines

and floating point in kernel is hard. Also
you'd either have to invent new interface, or you'd break touchscreen for people that already have their touchscreens calibrated.

No, I don't break calibration for people using a different chip.

Quoted 2 lines

Yes, that's not really proper solution, that just overengineered. Not worth implementing. Pass calibration data to userland.

You seem to repeat yourself and just say which solution you prefer, but I am missing the arguments why your solution (Pass calibration data to userland) is right and the best one.
Which problems does it solve?

Which one does it solve better than others?

How can you implement it in
a stable and portable way?

How can you make sure that all user-space GUI systems can and will make use of this calibration data?

Am 19.02.2017 um 20:05 schrieb Pavel Machek <[email protected]>:

Hi!

Quoted 2 lines

Hi Pavel,
I love discussions with you :)

Unfortunately I can't say the same.

Quoted 11 lines

Quoted 11 lines

Am 19.02.2017 um 18:15 schrieb Pavel Machek <[email protected]>:

Quoted 7 lines

Quoted 2 lines

Solve it properly. That means passing calibration
data from kernel to userland.

As written before, the really proper solution would be to provide floating or fixed point subpixel input events. Not arbitrarily scaling up in kernel and leaving downscaling to user space (where everybody can make it worse).

Quoted 5 lines

Quoted 1 line

That has no advantages,

It has the advantage of providing you with the full precision of raw data (but properly scaled) so that you don't loose any bit of information. This is what you just asked for - one or two mails before.

Not really, right? No matter what kind of fixed point you introduce, you'll still loose precision.

Quoted 6 lines

Quoted 4 lines

and floating point in kernel is hard. Also
you'd either have to invent new interface, or you'd break touchscreen for people that already have their touchscreens calibrated.

No, I don't break calibration for people using a different chip.

So you propose your touchscreen to behave differently from all other touchscreens in tree?

That's just no-go.

Quoted 8 lines

Quoted 3 lines

Yes, that's not really proper solution, that just overengineered. Not worth implementing. Pass calibration data to userland.

You seem to repeat yourself and just say which solution you prefer, but I am missing the arguments why your solution (Pass calibration data to userland) is right and the best one.
Which problems does it solve?

All you described.

Quoted 2 lines

Which one does it solve better than others?

It is not terminally ugly.

Quoted 3 lines

How can you implement it in
a stable and portable way?

Easily.

Quoted 3 lines

How can you make sure that all user-space GUI systems can and will make use of this calibration data?

You can't,

and you don't need to.

But as said I don't think we need float or fixed point for practical systems at all.

Quoted 4 lines

Quoted 4 lines

Quoted 3 lines

and floating point in kernel is hard. Also
you'd either have to invent new interface, or you'd break touchscreen for people that already have their touchscreens calibrated.

Quoted 4 lines

Quoted 1 line

No, I don't break calibration for people using a different chip.

So you propose your touchscreen to behave differently from all other touchscreens in tree?

No. I only propose that my touch screen behaves properly and in the best way it can. If others are worse, they should also be improved at some time.

And note that I am not making things different from others in tree, I am making the tsc2007 right (incl. following the touchscreen bindings which define the touchscreen size in "Pixels").

Quoted 1 line

That's just no-go.

In other words: you want to block any improvements unless your favourite touchscreen is giving directions...

Quoted 6 lines

Quoted 4 lines

You seem to repeat yourself and just say which solution you prefer, but I am missing the arguments why your solution (Pass calibration data to userland) is right and the best one.
Which problems does it solve?

All you described.

I think you are missing one problem: providing already properly scaled touch values to user space.

Quoted 4 lines

Quoted 2 lines

How can you implement it in
a stable and portable way?

Easily.

Please go ahead and show code.

Am 19.02.2017 um 21:57 schrieb Pavel Machek <[email protected]>:

Hi!

Quoted 2 lines

But as said I don't think we need float or fixed point for practical systems at all.

So you are going to loose precision. And if userspace decides to calibrate it slightly differently from kernel, lost precision will matter.

Quoted 7 lines

Quoted 7 lines

Quoted 6 lines

Quoted 4 lines

and floating point in kernel is hard. Also
you'd either have to invent new interface, or you'd break touchscreen for people that already have their touchscreens calibrated.

No, I don't break calibration for people using a different chip.

Quoted 6 lines

Quoted 2 lines

So you propose your touchscreen to behave differently from all other touchscreens in tree?

No. I only propose that my touch screen behaves properly and in the best way it can. If others are worse, they should also be improved at some time.

Different from all other drivers. Read: broken.

No. You have to design interface such that they _can_ be improved, and what you propose does not work that way.

Quoted 4 lines

And note that I am not making things different from others in tree, I am making the tsc2007 right (incl. following the touchscreen bindings which define the touchscreen size in "Pixels").

Your touch screen is not in any way special, so it has to behave in the same way others do.

Quoted 5 lines

Quoted 2 lines

That's just no-go.

In other words: you want to block any improvements unless your favourite touchscreen is giving directions...

Yes. I want to prevent you from pushing crap into the kernel.

If you want to improve it in reasonable way, you know what to do.

Quoted 5 lines

Quoted 5 lines

Quoted 4 lines

You seem to repeat yourself and just say which solution you prefer, but I am missing the arguments why your solution (Pass calibration data to userland) is right and the best one.
Which problems does it solve?

Quoted 4 lines

Quoted 1 line

All you described.

I think you are missing one problem: providing already properly scaled touch values to user space.

I'm not.

Userspace has to know how to do the calibration _anyway_ (for other hardware),

so giving scaled values to userspace is useless.

Quoted 7 lines

Quoted 5 lines

Quoted 3 lines

How can you implement it in
a stable and portable way?

Easily.

Please go ahead and show code.

You don't get to tell me what to do, unless you pay me.

You want to break kernel, you do coding. Or pay someone else, preferably someone who knows how to design kernel code.

Quoted 6 lines

Quoted 2 lines

But as said I don't think we need float or fixed point for practical systems at all.

So you are going to loose precision. And if userspace decides to calibrate it slightly differently from kernel, lost precision will matter.

Really?

Example:

ADC values go 100 .. 3995 (i.e. touch margin is 100 steps in pre-calibration)

This is scaled to let's say 0..640.

Quoted 2 lines

No. You have to design interface such that they _can_ be improved, and what you propose does not work that way.

It works. Please do real world tests...

Quoted 1 line

Yes. I want to prevent you from pushing crap into the kernel.

Crap? Well, we have discussed this driver for months here on the list and after a lot of improvements we came up to v9.

And you still think it is crap and none of the other reviewers has noticed?

Quoted 2 lines

Userspace has to know how to do the calibration _anyway_ (for other hardware),

What for? I do not understand which other hardware you are talking about.

On our devices there is only one touch glued to the panel and that one has to be calibrated. Ideally before the user gets the device into his hands => precalibration...

If you connect a digitizer, then that one has to be calibrated of course, but it is not glued onto the display panel. Hence it is a different issue.

Am 19.02.2017 um 23:19 schrieb Pavel Machek <[email protected]>:

hi!

Quoted 8 lines

Quoted 6 lines

Quoted 2 lines

But as said I don't think we need float or fixed point for practical systems at all.

So you are going to loose precision. And if userspace decides to calibrate it slightly differently from kernel, lost precision will matter.

Really?

Really.

Quoted 5 lines

Example:

ADC values go 100 .. 3995 (i.e. touch margin is 100 steps in pre-calibration)

This is scaled to let's say 0..640.

Ok. Now userspace realizes that kernel alignemnt is off, and it would want to scale it to 1..642.

That will mean that single pixel will be
inaccessible, right?

Quoted 5 lines

Quoted 3 lines

No. You have to design interface such that they _can_ be improved, and what you propose does not work that way.

It works. Please do real world tests...

You do a real world test on N900, and propose upgrade path.

Quoted 7 lines

Quoted 2 lines

Yes. I want to prevent you from pushing crap into the kernel.

Crap? Well, we have discussed this driver for months here on the list and after a lot of improvements we came up to v9.

And you still think it is crap and none of the other reviewers has noticed?

I'm pretty sure you will not be able to push calibration into kernel.

Quoted 9 lines

Quoted 2 lines

Userspace has to know how to do the calibration _anyway_ (for other hardware),

What for? I do not understand which other hardware you are talking about.

On our devices there is only one touch glued to the panel and that one has to be calibrated. Ideally before the user gets the device into his hands => precalibration...

Quoted 3 lines

If you connect a digitizer, then that one has to be calibrated of course, but it is not glued onto the display panel. Hence it is a different issue.

It is actually same issue. One kernel interface should work for all the touchscreens.

As long as it is small (I expect <1.01 = 1% error in scale) it is barely noticeable.

Therefore, I asked before: how big in pixels is your finger or stylus? Does this effect matter?

A resistive touch is a man-machine-interface where people press buttons of at least 12x12 pixels size (or they are no longer visually recognizable).

So the discussion boils down to "what gives the better usability?":
a) getting rid of the nasty user-space calibration step (and plethora of different tools)
b) getting highest theoretical precision which has a low practical relevance

I am in favor of a). Like most users we ask. A minority is in favor of b). Since we don't exclude b) users from reconfiguring their system to get it done as they like. I think this is the best we can achieve.

Quoted 6 lines

Quoted 4 lines

Quoted 2 lines

No. You have to design interface such that they _can_ be improved, and what you propose does not work that way.

It works. Please do real world tests...

You do a real world test on N900, and propose upgrade path.

I have no N900 running. But since it uses a tsc2004/5 controller which seems to be quite similar, you can likely copy&paste some code or add the algorithm:

ABS_X = (touchscreen-size-x * (adc_x - adc_min_x)) / (adc_max_x - adc_min_x)

Thats it.

If you set touchscreen-size-x = (adc_max_x - adc_min_x) you get maximum precision you can achieve with integer arithmetic. And if you set adc_min_x = 0 your user-space gets what it would have got before adding such a formula and then you can and must do calibration there.

Taking this as the defaults if none of the new properties is specified, makes the scaling feature completely disappear. And I don't care about 2 additional subtractions, one multiplication and one division per axis.

So the upgrade path is:
1. introduce new optional properties, parse and store them in the struct
2. set defaults for the optional properties as described above
3. add the formula to the code (1 line for each axis)
4. deploy - nobody will notice

5. update the DT and remove user-space calibration - people will be happy that they do not have to calibrate first any more

Am 20.02.2017 um 20:29 schrieb Pavel Machek <[email protected]>:

Hi!

Quoted 5 lines

As long as it is small (I expect <1.01 = 1% error in scale) it is barely noticeable.

Therefore, I asked before: how big in pixels is your finger or stylus? Does this effect matter?

If I draw a line in gimp, I don't expect it to have steps because of "barely noticeable" errors.

Quoted 3 lines

A resistive touch is a man-machine-interface where people press buttons of at least 12x12 pixels size (or they are no longer visually recognizable).

Resistive touch is used for drawing, too.

Quoted 5 lines

So the discussion boils down to "what gives the better usability?":
a) getting rid of the nasty user-space calibration step (and plethora of different tools)
b) getting highest theoretical precision which has a low practical relevance

Quoted 3 lines

I am in favor of a). Like most users we ask. A minority is in favor of b). Since we don't exclude b) users from reconfiguring their system to get it done as they like. I think this is the best we can achieve.

Do you even read what I wrote?

Because I presented way to have both a) _and_ b).

Quoted 24 lines

Quoted 7 lines

Quoted 5 lines

Quoted 3 lines

No. You have to design interface such that they _can_ be improved, and what you propose does not work that way.

It works. Please do real world tests...

You do a real world test on N900, and propose upgrade path.

I have no N900 running. But since it uses a tsc2004/5 controller which seems to be quite similar, you can likely copy&paste some code or add the algorithm:

ABS_X = (touchscreen-size-x * (adc_x - adc_min_x)) / (adc_max_x - adc_min_x)

Thats it.

If you set touchscreen-size-x = (adc_max_x - adc_min_x) you get maximum precision you can achieve with integer arithmetic. And if you set adc_min_x = 0 your user-space gets what it would have got before adding such a formula and then you can and must do calibration there.

Taking this as the defaults if none of the new properties is specified, makes the scaling feature completely disappear. And I don't care about 2 additional subtractions, one multiplication and one division per axis.

Quoted 5 lines

So the upgrade path is:
1. introduce new optional properties, parse and store them in the struct
2. set defaults for the optional properties as described above
3. add the formula to the code (1 line for each axis)
4. deploy - nobody will notice

Good so far.

Quoted 3 lines

5. update the DT and remove user-space calibration - people will be happy that they do not have to calibrate first any more

You can't do this. And this is fatal problem with your proposal.

If I update the DT in the kernel, my users will be very unhappy, because their screens will now be miscalibrated.

New kernel must not
force users to update their userland at the same time.

Hi Pavel,

Quoted 15 lines

Am 19.02.2017 um 23:19 schrieb Pavel Machek <[email protected]>:

hi!

Quoted 8 lines

Quoted 6 lines

Quoted 2 lines

But as said I don't think we need float or fixed point for practical systems at all.

So you are going to loose precision. And if userspace decides to calibrate it slightly differently from kernel, lost precision will matter.

Really?

Really.

Quoted 8 lines

Quoted 5 lines

Example:

ADC values go 100 .. 3995 (i.e. touch margin is 100 steps in pre-calibration)

This is scaled to let's say 0..640.

Ok. Now userspace realizes that kernel alignemnt is off, and it would want to scale it to 1..642.

Screen coordinates are still 0..639.

Quoted 2 lines

That will mean that single pixel will be
inaccessible, right?

Yes, that can happen if the additional user-space scale is > 1.0.

As long as it is small (I expect <1.01 = 1% error in scale) it is barely noticeable.

Therefore, I asked before: how big in pixels is your finger or stylus? Does this effect matter?

A resistive touch is a man-machine-interface where people press buttons of at least 12x12 pixels size (or they are no longer visually recognizable).

Am 20.02.2017 um 23:26 schrieb Petr Cvek <petr.cvek-qphu/[email protected]>:

Hi

Dne 20.2.2017 v 17:50 H. Nikolaus Schaller napsal(a):

Quoted 36 lines

Hi Pavel,

Quoted 15 lines

Am 19.02.2017 um 23:19 schrieb Pavel Machek <[email protected]>:

hi!

Quoted 8 lines

Quoted 6 lines

Quoted 2 lines

But as said I don't think we need float or fixed point for practical systems at all.

So you are going to loose precision. And if userspace decides to calibrate it slightly differently from kernel, lost precision will matter.

Really?

Really.

Quoted 8 lines

Quoted 5 lines

Example:

ADC values go 100 .. 3995 (i.e. touch margin is 100 steps in pre-calibration)

This is scaled to let's say 0..640.

Ok. Now userspace realizes that kernel alignemnt is off, and it would want to scale it to 1..642.

Screen coordinates are still 0..639.

Quoted 2 lines

That will mean that single pixel will be
inaccessible, right?

Yes, that can happen if the additional user-space scale is > 1.0.

As long as it is small (I expect <1.01 = 1% error in scale) it is barely noticeable.

Quoted 5 lines

Therefore, I asked before: how big in pixels is your finger or stylus? Does this effect matter?

A resistive touch is a man-machine-interface where people press buttons of at least 12x12 pixels size (or they are no longer visually recognizable).

Smallest kernel font is 4x6 (i think) and I'm regularly using 8x8. I would like to be able to select a single letter in the console.

Hi!

Quoted 2 lines

But as said I don't think we need float or fixed point for practical systems at all.

So you are going to loose precision. And if userspace decides to calibrate it slightly differently from kernel, lost precision will matter.

Quoted 10 lines

Quoted 9 lines

Quoted 6 lines

Quoted 4 lines

and floating point in kernel is hard. Also
you'd either have to invent new interface, or you'd break touchscreen for people that already have their touchscreens calibrated.

No, I don't break calibration for people using a different chip.

So you propose your touchscreen to behave differently from all other touchscreens in tree?

Quoted 3 lines

No. I only propose that my touch screen behaves properly and in the best way it can. If others are worse, they should also be improved at some time.

Different from all other drivers. Read: broken.

No. You have to design interface such that they _can_ be improved, and what you propose does not work that way.

Quoted 3 lines

And note that I am not making things different from others in tree, I am making the tsc2007 right (incl. following the touchscreen bindings which define the touchscreen size in "Pixels").

Your touch screen is not in any way special, so it has to behave in the same way others do.

Quoted 7 lines

Quoted 3 lines

And note that I am not making things different from others in tree, I am making the tsc2007 right (incl. following the touchscreen bindings which define the touchscreen size in "Pixels").

Your touch screen is not in any way special, so it has to behave in the same way others do.

I agree, the tsc2007 (=what the interface provides to userspace) should not behave special, for example it should behave like the virtual touchscreen (=what the interface provides to userspace) virtualbox gives. No need to be calibrated. Well, the internals are different. But that is what the kernel is good for, abstract such things.
Conclusion: It cannot be totally wrong behavior to have pixel values there.

Am 19.02.2017 um 23:39 schrieb Pavel Machek <[email protected]>:

Hi!

Quoted 8 lines

Quoted 7 lines

Quoted 3 lines

And note that I am not making things different from others in tree, I am making the tsc2007 right (incl. following the touchscreen bindings which define the touchscreen size in "Pixels").

Your touch screen is not in any way special, so it has to behave in the same way others do.

Quoted 7 lines

I agree, the tsc2007 (=what the interface provides to userspace) should not behave special, for example it should behave like the virtual touchscreen (=what the interface provides to userspace) virtualbox gives. No need to be calibrated. Well, the internals are different. But that is what the kernel is good for, abstract such things.
Conclusion: It cannot be totally wrong behavior to have pixel values there.

It is not "totally wrong". But it is useless code that should not be in kernel. Calibration certainly does not belong to single _driver_.

Feel free to submit driver

but keep the calibration code out
of tree...

But if you have userspace that depends on touchscreen to be calibrated... that _is_ wrong.

Quoted 2 lines

But if you have userspace that depends on touchscreen to be calibrated... that _is_ wrong.

User-space people and real users have the opposite opinion. They prefer if a touch is plug&play. I.e. without need for calibration.

Am 20.02.2017 um 20:32 schrieb Pavel Machek <[email protected]>:

Hi!

Quoted 3 lines

Quoted 2 lines

But if you have userspace that depends on touchscreen to be calibrated... that _is_ wrong.

Quoted 2 lines

User-space people and real users have the opposite opinion. They prefer if a touch is plug&play. I.e. without need for calibration.

Some people prefer to calibrate their touchscreens; because of per-device variations, that gives better precision.

You can pass default calibration data from device tree to kernel to userland. But you can not force people to use that calibration.

Am 20.02.2017 um 21:22 schrieb H. Nikolaus Schaller <[email protected]>:

Hi,

Quoted 10 lines

Am 20.02.2017 um 20:32 schrieb Pavel Machek <[email protected]>:

Hi!

Quoted 5 lines

Quoted 2 lines

But if you have userspace that depends on touchscreen to be calibrated... that _is_ wrong.

User-space people and real users have the opposite opinion. They prefer if a touch is plug&play. I.e. without need for calibration.

Quoted 5 lines

Some people prefer to calibrate their touchscreens; because of per-device variations, that gives better precision.

You can pass default calibration data from device tree to kernel to userland. But you can not force people to use that calibration.

Yes, you can do that if you want. I hope it became clear by the formula in my last mail.

I just want to be able to do it the other way for those users who don't care about this per-device variation.

BR,
Nikolaus

_______________________________________________
http://projects.goldelico.com/p/gta04-kernel/
Letux-kernel mailing list [email protected]
http://lists.goldelico.com/mailman/listinfo.cgi/letux-kernel

Am 17.02.2017 um 21:40 schrieb Dmitry Torokhov <[email protected]>:

Hi Nikolaus,

Quoted 42 lines · H. Nikolaus Schaller

Hi Dmitry,

Quoted 19 lines

Am 28.01.2017 um 20:33 schrieb Dmitry Torokhov <[email protected]>:

Hi Nikolaus,

Quoted 14 lines · H. Nikolaus Schaller

commit b98abe52fa8e ("Input: add common DT binding for touchscreens") introduced common DT bindings for touchscreens [1] and a helper function to parse the DT.

commit ed7c9870c9bc ("Input: of_touchscreen - add support for inverted / swapped axes") added another helper for parsing axis inversion and swapping
and applying them to x and y coordinates.

Both helpers have been integrated to accommodate any orientation of the touch panel in relation to the LCD.

A new feature is to introduce scaling the min/max ADC values to the screen size.

Quoted 6 lines

Quoted 4 lines

This makes it possible to pre-calibrate the touch so that is (almost) exactly matches the LCD pixel coordinates it is glued onto. This allows to well enough operate the touch before a user space calibration step can improve the precision.

I question whether doing scaling in kernel is really right solution.

Since lower left corner does not exactly report [0 0] and upper right corner does not report [4095 4095] from the ADC we need offset and steepness correction of the ADC values.

This steepness is the scaling that must happen in kernel and I don't understand why you want to propagate this ADC errors to user-space by avoiding scaling.

Let me iterate what we want to achieve:
* use new common bindings
* offset and steepness calibration of the ADC (called pre-calibration). This makes a real device much more reliable to operate with factory installed scaling factors.
* flipping and rotation

Quoted 1 line

(note that touch pixel to LCD pixel scaling is not explicitly on this list!)

That was explicitly called out in the patch:

"A new feature is to introduce scaling the min/max ADC values to the screen size."

Quoted 67 lines

Now to achieve the ADC pre-calibration we must calculate

	x' = (x - ti,min-x) / (ti,max-x-ti,min-x) giving a rante from 0.0 ... 1.0

This is scaled up to what is defined by touchscreen-size-x, i.e.

	x' = (touchscreen-size-x * (x - ti,min-x)) / (ti,max-x-ti,min-x)

How do you want to avoid this scaling to take place? It happens automatically. It is not even an additional line of code. And is necessary for compensating ADC offsets and steepness.

So the only way to avoid the scaling option is to eliminate the precalibration/ADC compensation which is essential for a device which has no means to properly calibrate before operating the device through touch.

The other option would be to avoid common bindings and set

touchscreen-size-x = (ti,max-x-ti,min-x)

This is heavily dependent on specific ADC offsets forwarded to user-space. IMHO the worst we can do (and the current tsc2007 driver does it that way!).

Quoted 2 lines

Why do you want this?

It seems that you assume that we want to enforce 1:1 scaling between touch pixels and LCD pixels and have designed code to achieve exactly that.

This is not the case. It is just a byproduct that you can do it.

And since it is easier to understand we have made the examples this way.

Quoted 6 lines

If your touch resolution is lower than your screen
then it might be useful, but if it is lower then you are losing data that can be very helpful for gesture recognition, and I hope you design your userspace so it can handle not only "bad" hardware, but "good" as well. And even with "bad" there are a lot of tricks that can be done to get "better" touch position in userspace.

You can just *choose* DT parameters touchscreen-size-x to match the LCD size. This of course reduces the touch precision to full LCD pixels. For finger- touch operated devices, subpixel precision is rarely needed.

Also, some user-spaces (e.g. older Replicant for GTA04) assume that there is such an 1:1 mapping and they will be perfectly happy about this.

But, if you can modify your user-space easily, you can also choose a different factor.

You can for example define touchscreen-size-x=<4096> and then you get almost the highest precision of the ADC and won't loose any bits. Or even define a bigger range and get steps >1 bit.

LCD driver (e.g. X11 calibration matrix) can scale it down again to LCD pixels.

So effectively we get for LCD pixels when the touch sets a mouse pointer:

	x'' = user-space-scale * <<input-event<< ((touchscreen-size-x * (x - ti,min-x)) / (ti,max-x-ti,min-x))

The most simple setup would then be but others are possible:

user-space-scale = 1
touchscreen-size-x = LCD-size-x

Quoted 3 lines

Let me cite myself:

Quoted 1 line

Quoted 1 line

This makes it possible to pre-calibrate the touch so that is (almost)

Yes, it allows you to pre-calibrate, I get it. But you still do properly calibrate later, right?

So you are doing double work here, once in
kernel, and second time in userspace.

I'd be more open to allowing setting the "min-axis" values to allow reporting typical range for given device, and let userspace scale as it sees fit.

Quoted 26 lines

Quoted 7 lines

Quoted 7 lines

Please note that the old ti,fuzz properties have been removed since they are replaced by the common bindings touchscreen-fuzz-x/y/z.

Quoted 6 lines

Quoted 2 lines

Finally, calculate_pressure has been renamed to calculate_resistance because that is what it is doing.

That is not what your patch does though. In the presence of "ti,report-resistance" parameter you start reporting resistance through ABS_PRESSURE

Well, there is some historic confusion wether this driver reports resistance or pressure.

The unpatched tsc2007 driver does it wrong (please test!) and we fix it on the fly (because a separate patch is much more complex than doing it right immediately).

This ti,report-resistance property is a means to get the old (wrong) meaning back in case someone urgently needs it and can't fix the user-space workaround which he must be using.

Quoted 4 lines

AFAIK there is no mainline board using the DT except ours (and the upcoming OMAP5-Pyra), so we shouldn't care too much. If you prefer, you can remove this compatibility property. We don't need it for our devices.

You seem to be treating DT data as something very fluid, which is wrong.

You need to treat it as a firmware, unlikely to change once device is shipped. Unlike legacy platform data, the fact that DTS files are not present in mainline does not mean that we can ignore such users and change behavior at will.

That said, if driver behavior is out of line from the subsystem expectations, we need to fix it.

Quoted 5 lines

That the function name is wrong is a second issue and this double negation might confuse a litte.

Please test on a real device if the patched driver reports pressure now (unless ti,report-resistance is specified).

I unfortunately can not test this driver as I do not have the hardware.

So all my observations are from code and data sheets.

That said, what is the values emitted as ABS_PRESSURE when finger is not touching the device, barely touching the device, or pressing firmly?

It seems that between TSC2007, TSC2004, TSC2005, and ADS7846, we have confusion as to what is being reported.

I am adding a few more folks to the CC so we can try and soft this out. Sebastian, Pali, Pavel, any input here?

Hi Dmitry,

Quoted 47 lines

Am 17.02.2017 um 21:40 schrieb Dmitry Torokhov <[email protected]>:

Hi Nikolaus,

Quoted 42 lines · H. Nikolaus Schaller

Hi Dmitry,

Quoted 19 lines

Am 28.01.2017 um 20:33 schrieb Dmitry Torokhov <[email protected]>:

Hi Nikolaus,

Quoted 14 lines · H. Nikolaus Schaller

commit b98abe52fa8e ("Input: add common DT binding for touchscreens") introduced common DT bindings for touchscreens [1] and a helper function to parse the DT.

commit ed7c9870c9bc ("Input: of_touchscreen - add support for inverted / swapped axes") added another helper for parsing axis inversion and swapping
and applying them to x and y coordinates.

Both helpers have been integrated to accommodate any orientation of the touch panel in relation to the LCD.

A new feature is to introduce scaling the min/max ADC values to the screen size.

Quoted 6 lines

Quoted 4 lines

This makes it possible to pre-calibrate the touch so that is (almost) exactly matches the LCD pixel coordinates it is glued onto. This allows to well enough operate the touch before a user space calibration step can improve the precision.

I question whether doing scaling in kernel is really right solution.

Since lower left corner does not exactly report [0 0] and upper right corner does not report [4095 4095] from the ADC we need offset and steepness correction of the ADC values.

This steepness is the scaling that must happen in kernel and I don't understand why you want to propagate this ADC errors to user-space by avoiding scaling.

Let me iterate what we want to achieve:
* use new common bindings
* offset and steepness calibration of the ADC (called pre-calibration). This makes a real device much more reliable to operate with factory installed scaling factors.
* flipping and rotation

Quoted 6 lines

Quoted 1 line

(note that touch pixel to LCD pixel scaling is not explicitly on this list!)

That was explicitly called out in the patch:

"A new feature is to introduce scaling the min/max ADC values to the screen size."

Because it is a feature that was not planned nor required, but is there. So it came into the description of what can be done. If this is the key problem I am happy with removing it from the commit messages.

Anyways, scaling to screen coordinates is not my invention. It is based on

http://lxr.free-electrons.com/source/Documentation/devicetree/bindings/input/touchscreen/touchscreen.txt

which defines the size to be in pixels. Well, a resistive touch screen does not have pixels. It might have a resolution/precision given by the ADC conversion steps but I assume this is not meant here.

So this scaling to screen size was also stimulated by this DT bindings.

Quoted 67 lines

Quoted 67 lines

Now to achieve the ADC pre-calibration we must calculate

	x' = (x - ti,min-x) / (ti,max-x-ti,min-x) giving a rante from 0.0 ... 1.0

This is scaled up to what is defined by touchscreen-size-x, i.e.

	x' = (touchscreen-size-x * (x - ti,min-x)) / (ti,max-x-ti,min-x)

How do you want to avoid this scaling to take place? It happens automatically. It is not even an additional line of code. And is necessary for compensating ADC offsets and steepness.

So the only way to avoid the scaling option is to eliminate the precalibration/ADC compensation which is essential for a device which has no means to properly calibrate before operating the device through touch.

The other option would be to avoid common bindings and set

touchscreen-size-x = (ti,max-x-ti,min-x)

This is heavily dependent on specific ADC offsets forwarded to user-space. IMHO the worst we can do (and the current tsc2007 driver does it that way!).

Quoted 2 lines

Why do you want this?

It seems that you assume that we want to enforce 1:1 scaling between touch pixels and LCD pixels and have designed code to achieve exactly that.

This is not the case. It is just a byproduct that you can do it.

And since it is easier to understand we have made the examples this way.

Quoted 6 lines

If your touch resolution is lower than your screen
then it might be useful, but if it is lower then you are losing data that can be very helpful for gesture recognition, and I hope you design your userspace so it can handle not only "bad" hardware, but "good" as well. And even with "bad" there are a lot of tricks that can be done to get "better" touch position in userspace.

You can just *choose* DT parameters touchscreen-size-x to match the LCD size. This of course reduces the touch precision to full LCD pixels. For finger- touch operated devices, subpixel precision is rarely needed.

Also, some user-spaces (e.g. older Replicant for GTA04) assume that there is such an 1:1 mapping and they will be perfectly happy about this.

But, if you can modify your user-space easily, you can also choose a different factor.

You can for example define touchscreen-size-x=<4096> and then you get almost the highest precision of the ADC and won't loose any bits. Or even define a bigger range and get steps >1 bit.

LCD driver (e.g. X11 calibration matrix) can scale it down again to LCD pixels.

So effectively we get for LCD pixels when the touch sets a mouse pointer:

	x'' = user-space-scale * <<input-event<< ((touchscreen-size-x * (x - ti,min-x)) / (ti,max-x-ti,min-x))

The most simple setup would then be but others are possible:

user-space-scale = 1
touchscreen-size-x = LCD-size-x

Quoted 6 lines

Quoted 3 lines

Let me cite myself:

Quoted 1 line

Quoted 1 line

This makes it possible to pre-calibrate the touch so that is (almost)

Yes, it allows you to pre-calibrate, I get it. But you still do properly calibrate later, right?

No. We want to get rid of this step unless we need to improve the precision by some tiny %.

Why do we want to get rid of it?

This needs a longer explanation.

First of all we are working on the Letux distribution which aims at supporting multiple devices in the same way and with a single SD card image that contains one or multiple rootfs (Debian X11, QtMoko, Replicant etc.) the kernel uImage and DTB files for each supported device.

One of the ideas is to allow users to swap the SD card from one device to the other. This means that the touch screen calibration must magically be carried along - or we enforce to recalibrate every time the sd card is swapped back or forth.

The same is true for prebuilt SD-images. To have a single (no longer device specific) download requires to have a well defined default calibration. Especially on devices where you have no keyboard where you can ask the user to manually fine-tune something.

Our challenge is to make it work well without asking for explicit calibration. This is where pre-calibration comes into the game.

It turns out to be related to the touch screen properties and how it is glued onto the specific LCD panel, i.e. hardware and its description.

So the natural location of defining this relation is the DTB for each device and not the user-space. And it happens that we already have one per device (touch screen, lcd, tsc variant) on the SD card.

Quoted 2 lines

So you are doing double work here, once in
kernel, and second time in userspace.

This is no longer required, as long as we can use the default 1:1 coordinate mapping of e.g. framebuffer or X11 based GUI toolkits.

If we don't have the 1:1 mapping of touch screen input event coordinates to LCD pixels we have a mess of different scaling factors back in user-space again. And the problem is setting them up properly in an installable .tbz or .dd image.

Recently, you have raised another topic I had indeed not thought abut, which is the potential subpixel precision of a resistive touch. You get approx. 12 bit from the ADC but the x coordinate of the 480 pixel wide screen is just 9 bit. So scaling the input event coordinates to pixels throws away 3 bits.

But there are two things to consider.

One is that we could set the pre-calibration to keep as much resolution as possible, ca. 12 pixels and make the user-space scale down to screen coordinates.

Unfortunately that differs between devices and hence we need not only different DTBs (which we already have) but different scale factors in user-space.

Next, one could argue that subpixel coordinates should not be handled by scaling input event coordinates up and then down again in user-space. The correct solution would be that input events could report fixed or floating point coordinates, but this is probably beyond what we all want to do.

The other thing is noise. No resistive touch screen I have seen in the wild with the tsc2007 or tsc2046 controller has the precision to really report subpixels. Rather we have to set the fuzz between 3 or 10 or so. Which means that those bits we cut off by scaling to screen coordinates are already lost in noise.

Quoted 5 lines

I'd be more open to allowing setting the "min-axis" values to allow reporting typical range for given device, and let userspace scale as it sees fit.

The problem is: what is "typical range"? It turns out to be a completely arbitrary definition.

I see a choice between:
a) full theoretical ADC range
b) millimeters
c) pixels of the LCD the touch is glued to

For me, the most natural and practical "typical range" still seems to be the pixels of the lcd panel the touch is glued onto.

The reason why I see it as superior to the others is that only this leads to a stable 1:1 mapping in user-space avoiding to adjust any factors there.

And this 1:1 saves a lot of configuration hassle if you swap GUI systems (X, Wayland, Android, DirectFB, fb based Qt, ...).

Quoted 27 lines

Quoted 27 lines

Quoted 8 lines

Quoted 8 lines

Please note that the old ti,fuzz properties have been removed since they are replaced by the common bindings touchscreen-fuzz-x/y/z.

Quoted 6 lines

Quoted 2 lines

Finally, calculate_pressure has been renamed to calculate_resistance because that is what it is doing.

That is not what your patch does though. In the presence of "ti,report-resistance" parameter you start reporting resistance through ABS_PRESSURE

Well, there is some historic confusion wether this driver reports resistance or pressure.

The unpatched tsc2007 driver does it wrong (please test!) and we fix it on the fly (because a separate patch is much more complex than doing it right immediately).

This ti,report-resistance property is a means to get the old (wrong) meaning back in case someone urgently needs it and can't fix the user-space workaround which he must be using.

Quoted 6 lines

Quoted 4 lines

AFAIK there is no mainline board using the DT except ours (and the upcoming OMAP5-Pyra), so we shouldn't care too much. If you prefer, you can remove this compatibility property. We don't need it for our devices.

You seem to be treating DT data as something very fluid, which is wrong.

No,only sometimes.

Of course it should be done right once. Of course there is long discussion about what is right...

Quoted 4 lines

You need to treat it as a firmware, unlikely to change once device is shipped. Unlike legacy platform data, the fact that DTS files are not present in mainline does not mean that we can ignore such users and change behavior at will.

Well, it happens to me as maintainer of a 99.5% mainline device every now and then. Of course this should not be an argument to do it equally bad here.

One more thought: for some devices it is easier to modify the DTB "firmware" than the user space (e.g. if you have no access to it because it is maintained by somebody else).

Quoted 6 lines

That said, if driver behavior is out of line from the subsystem expectations, we need to fix it.

Quoted 7 lines

Quoted 5 lines

That the function name is wrong is a second issue and this double negation might confuse a litte.

Please test on a real device if the patched driver reports pressure now (unless ti,report-resistance is specified).

I unfortunately can not test this driver as I do not have the hardware.

Ah, ok. This is indeed a road block.

Maybe someone else with a real tsc2007 hardware is reading this discussion and can comment?

Quoted 4 lines

So all my observations are from code and data sheets.

That said, what is the values emitted as ABS_PRESSURE when finger is not touching the device, barely touching the device, or pressing firmly?

Quoted 2 lines

It seems that between TSC2007, TSC2004, TSC2005, and ADS7846, we have confusion as to what is being reported.

Indeed. The original tsc2007 jumps from 0 to a maximum value and goes down when pressing more firmly. This is not "pressure". The other drivers (I have tested the OpenPandora with tsc2046) are doing it in a monotonic curve.

Am 20.02.2017 um 02:07 schrieb Dmitry Torokhov <[email protected]>:

Hi Nikolaus,

Quoted 70 lines · H. Nikolaus Schaller

Hi Dmitry,

Quoted 47 lines

Am 17.02.2017 um 21:40 schrieb Dmitry Torokhov <[email protected]>:

Hi Nikolaus,

Quoted 42 lines · H. Nikolaus Schaller

Hi Dmitry,

Quoted 19 lines

Am 28.01.2017 um 20:33 schrieb Dmitry Torokhov <[email protected]>:

Hi Nikolaus,

Quoted 14 lines · H. Nikolaus Schaller

commit b98abe52fa8e ("Input: add common DT binding for touchscreens") introduced common DT bindings for touchscreens [1] and a helper function to parse the DT.

commit ed7c9870c9bc ("Input: of_touchscreen - add support for inverted / swapped axes") added another helper for parsing axis inversion and swapping
and applying them to x and y coordinates.

Both helpers have been integrated to accommodate any orientation of the touch panel in relation to the LCD.

A new feature is to introduce scaling the min/max ADC values to the screen size.

Quoted 6 lines

Quoted 4 lines

This makes it possible to pre-calibrate the touch so that is (almost) exactly matches the LCD pixel coordinates it is glued onto. This allows to well enough operate the touch before a user space calibration step can improve the precision.

I question whether doing scaling in kernel is really right solution.

Since lower left corner does not exactly report [0 0] and upper right corner does not report [4095 4095] from the ADC we need offset and steepness correction of the ADC values.

This steepness is the scaling that must happen in kernel and I don't understand why you want to propagate this ADC errors to user-space by avoiding scaling.

Let me iterate what we want to achieve:
* use new common bindings
* offset and steepness calibration of the ADC (called pre-calibration). This makes a real device much more reliable to operate with factory installed scaling factors.
* flipping and rotation

Quoted 6 lines

Quoted 1 line

(note that touch pixel to LCD pixel scaling is not explicitly on this list!)

That was explicitly called out in the patch:

"A new feature is to introduce scaling the min/max ADC values to the screen size."

Because it is a feature that was not planned nor required, but is there. So it came into the description of what can be done. If this is the key problem I am happy with removing it from the commit messages.

Anyways, scaling to screen coordinates is not my invention. It is based on

http://lxr.free-electrons.com/source/Documentation/devicetree/bindings/input/touchscreen/touchscreen.txt

which defines the size to be in pixels. Well, a resistive touch screen does not have pixels. It might have a resolution/precision given by the ADC conversion steps but I assume this is not meant here.

Quoted 3 lines

So this scaling to screen size was also stimulated by this DT bindings.

OK, now I see where you are coming from. You assume that pixels mentioned in the DT binding are LCD pixels,

however this is incorrect.

They are "pixels" or the touch controller, i.e. native unites in which device reports coordinates, as opposed to points per inch, or millimeters, or whatever. These "pixels" do not have to have 1:1 relation to the LCD pixels; in fact they rarely do.

Input driver may set resolution for given axis in units per mm (or units per radian for rotational axis ABS_RX, ABS_RY, ABS_RZ), and if you check the binding, you can use "touchscreen-x-mm" and "touchscreen-y-mm" to specify the size of entire touch surface and set resolution from it so that userspace can calculate the proper scaling factor.

Quoted 113 lines

Quoted 68 lines

Quoted 68 lines

Now to achieve the ADC pre-calibration we must calculate

	x' = (x - ti,min-x) / (ti,max-x-ti,min-x) giving a rante from 0.0 ... 1.0

This is scaled up to what is defined by touchscreen-size-x, i.e.

	x' = (touchscreen-size-x * (x - ti,min-x)) / (ti,max-x-ti,min-x)

How do you want to avoid this scaling to take place? It happens automatically. It is not even an additional line of code. And is necessary for compensating ADC offsets and steepness.

So the only way to avoid the scaling option is to eliminate the precalibration/ADC compensation which is essential for a device which has no means to properly calibrate before operating the device through touch.

The other option would be to avoid common bindings and set

touchscreen-size-x = (ti,max-x-ti,min-x)

This is heavily dependent on specific ADC offsets forwarded to user-space. IMHO the worst we can do (and the current tsc2007 driver does it that way!).

Quoted 2 lines

Why do you want this?

It seems that you assume that we want to enforce 1:1 scaling between touch pixels and LCD pixels and have designed code to achieve exactly that.

This is not the case. It is just a byproduct that you can do it.

And since it is easier to understand we have made the examples this way.

Quoted 6 lines

If your touch resolution is lower than your screen
then it might be useful, but if it is lower then you are losing data that can be very helpful for gesture recognition, and I hope you design your userspace so it can handle not only "bad" hardware, but "good" as well. And even with "bad" there are a lot of tricks that can be done to get "better" touch position in userspace.

You can just *choose* DT parameters touchscreen-size-x to match the LCD size. This of course reduces the touch precision to full LCD pixels. For finger- touch operated devices, subpixel precision is rarely needed.

Also, some user-spaces (e.g. older Replicant for GTA04) assume that there is such an 1:1 mapping and they will be perfectly happy about this.

But, if you can modify your user-space easily, you can also choose a different factor.

You can for example define touchscreen-size-x=<4096> and then you get almost the highest precision of the ADC and won't loose any bits. Or even define a bigger range and get steps >1 bit.

LCD driver (e.g. X11 calibration matrix) can scale it down again to LCD pixels.

So effectively we get for LCD pixels when the touch sets a mouse pointer:

	x'' = user-space-scale * <<input-event<< ((touchscreen-size-x * (x - ti,min-x)) / (ti,max-x-ti,min-x))

The most simple setup would then be but others are possible:

user-space-scale = 1
touchscreen-size-x = LCD-size-x

Quoted 6 lines

Quoted 3 lines

Let me cite myself:

Quoted 1 line

Quoted 1 line

This makes it possible to pre-calibrate the touch so that is (almost)

Yes, it allows you to pre-calibrate, I get it. But you still do properly calibrate later, right?

No. We want to get rid of this step unless we need to improve the precision by some tiny %.

Why do we want to get rid of it?

This needs a longer explanation.

First of all we are working on the Letux distribution which aims at supporting multiple devices in the same way and with a single SD card image that contains one or multiple rootfs (Debian X11, QtMoko, Replicant etc.) the kernel uImage and DTB files for each supported device.

One of the ideas is to allow users to swap the SD card from one device to the other. This means that the touch screen calibration must magically be carried along - or we enforce to recalibrate every time the sd card is swapped back or forth.

The same is true for prebuilt SD-images. To have a single (no longer device specific) download requires to have a well defined default calibration. Especially on devices where you have no keyboard where you can ask the user to manually fine-tune something.

Our challenge is to make it work well without asking for explicit calibration. This is where pre-calibration comes into the game.

It turns out to be related to the touch screen properties and how it is glued onto the specific LCD panel, i.e. hardware and its description.

So the natural location of defining this relation is the DTB for each device and not the user-space. And it happens that we already have one per device (touch screen, lcd, tsc variant) on the SD card.

Quoted 2 lines

So you are doing double work here, once in
kernel, and second time in userspace.

This is no longer required, as long as we can use the default 1:1 coordinate mapping of e.g. framebuffer or X11 based GUI toolkits.

Quoted 4 lines

If we don't have the 1:1 mapping of touch screen input event coordinates to LCD pixels we have a mess of different scaling factors back in user-space again. And the problem is setting them up properly in an installable .tbz or .dd image.

Why? Nothing stops you from querying the device and figure out scaling.

We do that, for example, on Chrome OS, where we do not have 1:1 matching between LCDs and touch controllers, and still we do not have static transformation matrices on the file system either.

Quoted 20 lines

Recently, you have raised another topic I had indeed not thought abut, which is the potential subpixel precision of a resistive touch. You get approx. 12 bit from the ADC but the x coordinate of the 480 pixel wide screen is just 9 bit. So scaling the input event coordinates to pixels throws away 3 bits.

But there are two things to consider.

One is that we could set the pre-calibration to keep as much resolution as possible, ca. 12 pixels and make the user-space scale down to screen coordinates.

Unfortunately that differs between devices and hence we need not only different DTBs (which we already have) but different scale factors in user-space.

Next, one could argue that subpixel coordinates should not be handled by scaling input event coordinates up and then down again in user-space. The correct solution would be that input events could report fixed or floating point coordinates, but this is probably beyond what we all want to do.

Quoted 4 lines

The other thing is noise. No resistive touch screen I have seen in the wild with the tsc2007 or tsc2046 controller has the precision to really report subpixels. Rather we have to set the fuzz between 3 or 10 or so. Which means that those bits we cut off by scaling to screen coordinates are already lost in noise.

I really do not care what exactly tsc2007 or any other particular touchscreen controller does. Please design your software so that it can use wide range of devices, from noisy to noiseless. If there is noise, filter it out.

It there is a nice touch controller with a lot of
precision - use it,

do not dumb it down to the lowest common
denominator.

Quoted 9 lines

Quoted 5 lines

I'd be more open to allowing setting the "min-axis" values to allow reporting typical range for given device, and let userspace scale as it sees fit.

The problem is: what is "typical range"? It turns out to be a completely arbitrary definition.

Quoted 4 lines

I see a choice between:
a) full theoretical ADC range
b) millimeters
c) pixels of the LCD the touch is glued to

d) Native reporting units for the touch controller. You will have the touch controller output range, you will have your LCD screen size, and you scale one into another.

Quoted 5 lines

For me, the most natural and practical "typical range" still seems to be the pixels of the lcd panel the touch is glued onto.

Quoted 5 lines

The reason why I see it as superior to the others is that only this leads to a stable 1:1 mapping in user-space avoiding to adjust any factors there.

And this 1:1 saves a lot of configuration hassle if you swap GUI systems (X, Wayland, Android, DirectFB, fb based Qt, ...).

Yes, they will need to do that, if they want to use "nice" hardware and not lose precision.

And since they need to do that anyway, there is no
point in doing it in kernel, not with per driver code, and not even in input core.

You are getting this feedback from several people,

please consider topic
of adding scaling to the drivers closed.

The only transformations that I
think make sense is to "normalize" output with axis swaps and inversions, so that kernel uses the same notion of coordinates for all devices, regardless of how the device was mounted.

Quoted 49 lines

Quoted 29 lines

Quoted 29 lines

Quoted 10 lines

Quoted 10 lines

Please note that the old ti,fuzz properties have been removed since they are replaced by the common bindings touchscreen-fuzz-x/y/z.

Quoted 6 lines

Quoted 2 lines

Finally, calculate_pressure has been renamed to calculate_resistance because that is what it is doing.

That is not what your patch does though. In the presence of "ti,report-resistance" parameter you start reporting resistance through ABS_PRESSURE

Well, there is some historic confusion wether this driver reports resistance or pressure.

The unpatched tsc2007 driver does it wrong (please test!) and we fix it on the fly (because a separate patch is much more complex than doing it right immediately).

This ti,report-resistance property is a means to get the old (wrong) meaning back in case someone urgently needs it and can't fix the user-space workaround which he must be using.

Quoted 6 lines

Quoted 4 lines

AFAIK there is no mainline board using the DT except ours (and the upcoming OMAP5-Pyra), so we shouldn't care too much. If you prefer, you can remove this compatibility property. We don't need it for our devices.

You seem to be treating DT data as something very fluid, which is wrong.

No,only sometimes.

Of course it should be done right once. Of course there is long discussion about what is right...

Quoted 4 lines

You need to treat it as a firmware, unlikely to change once device is shipped. Unlike legacy platform data, the fact that DTS files are not present in mainline does not mean that we can ignore such users and change behavior at will.

Well, it happens to me as maintainer of a 99.5% mainline device every now and then. Of course this should not be an argument to do it equally bad here.

Quoted 3 lines

One more thought: for some devices it is easier to modify the DTB "firmware" than the user space (e.g. if you have no access to it because it is maintained by somebody else).

And still, according to DT folks, device tree forms an ABI and thus we are not to change it, even if it is easy.

Quoted 26 lines

Quoted 6 lines

That said, if driver behavior is out of line from the subsystem expectations, we need to fix it.

Quoted 7 lines

Quoted 5 lines

That the function name is wrong is a second issue and this double negation might confuse a litte.

Please test on a real device if the patched driver reports pressure now (unless ti,report-resistance is specified).

I unfortunately can not test this driver as I do not have the hardware.

Ah, ok. This is indeed a road block.

Maybe someone else with a real tsc2007 hardware is reading this discussion and can comment?

Quoted 7 lines

So all my observations are from code and data sheets.

That said, what is the values emitted as ABS_PRESSURE when finger is not touching the device, barely touching the device, or pressing firmly?

It seems that between TSC2007, TSC2004, TSC2005, and ADS7846, we have confusion as to what is being reported.

Quoted 3 lines

Indeed. The original tsc2007 jumps from 0 to a maximum value and goes down when pressing more firmly. This is not "pressure". The other drivers (I have tested the OpenPandora with tsc2046) are doing it in a monotonic curve.

OK, then this is clearly wrong and we need to fix it (without any funky flags).

Hi Dmitry,

Quoted 6 lines

Input driver may set resolution for given axis in units per mm (or units per radian for rotational axis ABS_RX, ABS_RY, ABS_RZ), and if you check the binding, you can use "touchscreen-x-mm" and "touchscreen-y-mm" to specify the size of entire touch surface and set resolution from it so that userspace can calculate the proper scaling factor.

How is this information exposed by the kernel to user-space? By scanning the DT file or tree?

Am 20.02.2017 um 20:42 schrieb Pali Rohár <[email protected]>:

Hi Nikolaus!

Quoted 9 lines · On Monday 20 February 2017 17:50:04 H. Nikolaus Schaller wrote:

Hi Dmitry,

Quoted 6 lines

Input driver may set resolution for given axis in units per mm (or units per radian for rotational axis ABS_RX, ABS_RY, ABS_RZ), and if you check the binding, you can use "touchscreen-x-mm" and "touchscreen-y-mm" to specify the size of entire touch surface and set resolution from it so that userspace can calculate the proper scaling factor.

Quoted 2 lines

How is this information exposed by the kernel to user-space? By scanning the DT file or tree?

Set input_abs_set_res() from kernel. And in userspace call EVIOCGABS ioctl() on input device. Look at struct input_absinfo, you should have all needed information here. This is generic input interface, no DT is needed.

I hope that XServer is already using it for evdev devices...

For whole implementation look at evtest program. That should be good starting point for your userspace implementation.

While I'm watching this discussion... in my opinion kernel should just invert input axes (when needed) and should not do any other normalization or integer/floating-point re-calibration/re-calculation. If it correctly exports minimum value, maximum value and resolution then userspace can correctly re-scale input events to units which userspace needs (e.g. mapping into LCD screen pixels or whatever is needed).

Hi Pali,

Quoted 17 lines

Am 20.02.2017 um 20:42 schrieb Pali Rohár <[email protected]>:

Hi Nikolaus!

Quoted 11 lines · On Monday 20 February 2017 17:50:04 H. Nikolaus Schaller wrote:

Hi Dmitry,

Quoted 6 lines

Input driver may set resolution for given axis in units per mm (or units per radian for rotational axis ABS_RX, ABS_RY, ABS_RZ), and if you check the binding, you can use "touchscreen-x-mm" and "touchscreen-y-mm" to specify the size of entire touch surface and set resolution from it so that userspace can calculate the proper scaling factor.

How is this information exposed by the kernel to user-space? By scanning the DT file or tree?

Quoted 4 lines

Set input_abs_set_res() from kernel. And in userspace call EVIOCGABS ioctl() on input device. Look at struct input_absinfo, you should have all needed information here. This is generic input interface, no DT is needed.

This assumes that I can and want to write a graphics system myself.

Quoted 1 line

I hope that XServer is already using it for evdev devices...

No idea if it does. It is a black box for me out of our control.

Quoted 2 lines

For whole implementation look at evtest program. That should be good starting point for your userspace implementation.

The problem I have is that *I* have no userspace implementation and the GTA04 project does not want to enforce one. We have several different ones: X11 based (LXDE and others), Qt (fb based), Replicant to name some.

All have the same problem to be solved once. The common denominator for a solution are 2 lines of code in the kernel plus some DT properties you need anyways if calibration should be automated in userland.

Quoted 7 lines

While I'm watching this discussion... in my opinion kernel should just invert input axes (when needed) and should not do any other normalization or integer/floating-point re-calibration/re-calculation. If it correctly exports minimum value, maximum value and resolution then userspace can correctly re-scale input events to units which userspace needs (e.g. mapping into LCD screen pixels or whatever is needed).

It can, but afaik it does not yet.

And if it does, it does it in a
plethora of different implementation states. That is the reason why we want to solve it once for all userlands in the kernel and not rely on user-space help.

Surely, userland can do a lot of things. It could also do the whole file system stuff (FUSE).

A more input device related example comes to my mind: userland could do keyboard mapping completely. It would suffice if the kernel presents some x/y coordinates or gpio-numbers for buttons and user-space could map. Still there is a (pre-)mapping to Key-Codes. And yes, they are mapped a second time in userland if needed, but it works sufficiently well if not done.

BR and thanks,
Nikolaus

Am 20.02.2017 um 22:07 schrieb Pali Rohár <[email protected]>:

Quoted 24 lines · On Monday 20 February 2017 21:35:18 H. Nikolaus Schaller wrote:

Hi Pali,

Quoted 21 lines

Am 20.02.2017 um 20:42 schrieb Pali Rohár <[email protected]>:

Hi Nikolaus!

Quoted 11 lines · On Monday 20 February 2017 17:50:04 H. Nikolaus Schaller wrote:

Hi Dmitry,

Quoted 6 lines

Input driver may set resolution for given axis in units per mm (or units per radian for rotational axis ABS_RX, ABS_RY, ABS_RZ), and if you check the binding, you can use "touchscreen-x-mm" and "touchscreen-y-mm" to specify the size of entire touch surface and set resolution from it so that userspace can calculate the proper scaling factor.

How is this information exposed by the kernel to user-space? By scanning the DT file or tree?

Set input_abs_set_res() from kernel. And in userspace call EVIOCGABS ioctl() on input device. Look at struct input_absinfo, you should have all needed information here. This is generic input interface, no DT is needed.

Quoted 1 line

This assumes that I can and want to write a graphics system myself.

Not only. There are already existing graphics systems. And you need to provide needed information from kernel, so they can start using it.

So input_abs_set_res() is needed to use in your kernel driver.

Quoted 4 lines

Quoted 2 lines

I hope that XServer is already using it for evdev devices...

No idea if it does. It is a black box for me out of our control.

https://cgit.freedesktop.org/xorg/driver/xf86-input-evdev/tree/src/evdev.c#n1479

So yes, it does.

Quoted 8 lines

Quoted 2 lines

For whole implementation look at evtest program. That should be good starting point for your userspace implementation.

The problem I have is that *I* have no userspace implementation and the GTA04 project does not want to enforce one. We have several different ones: X11 based (LXDE and others), Qt (fb based), Replicant to name some.

Quoted 4 lines

All have the same problem to be solved once. The common denominator for a solution are 2 lines of code in the kernel plus some DT properties you need anyways if calibration should be automated in userland.

As I wrote above part of linux input API is resolution value. And from all information I understood that having current value, minimal value, maximal value and resolution is enough for correct calculation of pixel coordinates in userspace.

And Xserver evdev driver is using it.

If other non-X11 application (which you want/need to use) use resolution information incorrectly (or calculate positions incorrectly), then this is bug that application. Not in Linux kernel, that is important.

And I would rather see fixes of such bugs in that (broken) application as doing workarounds in kernel, just because of bugs in application.

More important, are those applications really broken?

From my point of view: Reporting size of input device is already part of stable kernel <--> userspace API/ABI and it should be used instead of inventing new way...

Quoted 9 lines

Quoted 7 lines

While I'm watching this discussion... in my opinion kernel should just invert input axes (when needed) and should not do any other normalization or integer/floating-point re-calibration/re-calculation. If it correctly exports minimum value, maximum value and resolution then userspace can correctly re-scale input events to units which userspace needs (e.g. mapping into LCD screen pixels or whatever is needed).

It can, but afaik it does not yet.

I did not tested it, but code is in xf86-input-evdev already there.

So please try to implement input_abs_set_res() in kernel driver and test userspace.

Quoted 4 lines

And if it does, it does it in a
plethora of different implementation states. That is the reason why we want to solve it once for all userlands in the kernel and not rely on user-space help.

For me this looks like "we are going to fix userspace bugs in kernel".

Really! Not a good idea. Plus I still see this as abusing kernel API/ABI as resolution should be handled differently as you are proposing.

Quoted 11 lines

Surely, userland can do a lot of things. It could also do the whole file system stuff (FUSE).

A more input device related example comes to my mind: userland could do keyboard mapping completely. It would suffice if the kernel presents some x/y coordinates or gpio-numbers for buttons and user-space could map. Still there is a (pre-)mapping to Key-Codes. And yes, they are mapped a second time in userland if needed, but it works sufficiently well if not done.

Quoted 2 lines

BR and thanks,
Nikolaus

Hi Pali,

Quoted 30 lines

Am 20.02.2017 um 22:07 schrieb Pali Rohár <[email protected]>:

Quoted 26 lines · On Monday 20 February 2017 21:35:18 H. Nikolaus Schaller wrote:

Hi Pali,

Quoted 14 lines

Am 20.02.2017 um 20:42 schrieb Pali Rohár <[email protected]>:

Hi Nikolaus!

Quoted 9 lines · On Monday 20 February 2017 17:50:04 H. Nikolaus Schaller wrote:

Hi Dmitry,

Quoted 6 lines

Input driver may set resolution for given axis in units per mm (or units per radian for rotational axis ABS_RX, ABS_RY, ABS_RZ), and if you check the binding, you can use "touchscreen-x-mm" and "touchscreen-y-mm" to specify the size of entire touch surface and set resolution from it so that userspace can calculate the proper scaling factor.

Quoted 7 lines

Quoted 2 lines

How is this information exposed by the kernel to user-space? By scanning the DT file or tree?

Set input_abs_set_res() from kernel. And in userspace call EVIOCGABS ioctl() on input device. Look at struct input_absinfo, you should have all needed information here. This is generic input interface, no DT is needed.

This assumes that I can and want to write a graphics system myself.

Quoted 5 lines

Not only. There are already existing graphics systems. And you need to provide needed information from kernel, so they can start using it.

So input_abs_set_res() is needed to use in your kernel driver.

I didn't know about this feature and obviously nobody else has implemented it in the tsc2007 driver.

Quoted 7 lines

Quoted 4 lines

And if it does, it does it in a
plethora of different implementation states. That is the reason why we want to solve it once for all userlands in the kernel and not rely on user-space help.

For me this looks like "we are going to fix userspace bugs in kernel".

Such things are system bugs and it is neither necessarily a userspace or kernel bug.

Quoted 3 lines

Really! Not a good idea. Plus I still see this as abusing kernel API/ABI as resolution should be handled differently as you are proposing.

I don't understand what you say here. Where are we abusing kernel API/ABI?

Am 20.02.2017 um 22:54 schrieb Pali Rohár <[email protected]>:

Quoted 32 lines · On Monday 20 February 2017 22:24:31 H. Nikolaus Schaller wrote:

Hi Pali,

Quoted 30 lines

Am 20.02.2017 um 22:07 schrieb Pali Rohár <[email protected]>:

Quoted 26 lines · On Monday 20 February 2017 21:35:18 H. Nikolaus Schaller wrote:

Hi Pali,

Quoted 14 lines

Am 20.02.2017 um 20:42 schrieb Pali Rohár <[email protected]>:

Hi Nikolaus!

Quoted 9 lines · On Monday 20 February 2017 17:50:04 H. Nikolaus Schaller wrote:

Hi Dmitry,

Quoted 6 lines

Input driver may set resolution for given axis in units per mm (or units per radian for rotational axis ABS_RX, ABS_RY, ABS_RZ), and if you check the binding, you can use "touchscreen-x-mm" and "touchscreen-y-mm" to specify the size of entire touch surface and set resolution from it so that userspace can calculate the proper scaling factor.

Quoted 7 lines

Quoted 2 lines

How is this information exposed by the kernel to user-space? By scanning the DT file or tree?

Set input_abs_set_res() from kernel. And in userspace call EVIOCGABS ioctl() on input device. Look at struct input_absinfo, you should have all needed information here. This is generic input interface, no DT is needed.

This assumes that I can and want to write a graphics system myself.

Quoted 8 lines

Quoted 5 lines

Not only. There are already existing graphics systems. And you need to provide needed information from kernel, so they can start using it.

So input_abs_set_res() is needed to use in your kernel driver.

I didn't know about this feature and obviously nobody else has implemented it in the tsc2007 driver.

So... before doing other things, can you deeply look at it and check if it really fixes this problem? Because I think that yes.

You can probably set it from DT and in your DT file you can have stored screen size (or resolution factor).

Also for testing, you can set it even via userspace (ioctl which I wrote in previous email).

Quoted 9 lines

Quoted 8 lines

Quoted 5 lines

And if it does, it does it in a
plethora of different implementation states. That is the reason why we want to solve it once for all userlands in the kernel and not rely on user-space help.

For me this looks like "we are going to fix userspace bugs in kernel".

Quoted 2 lines

Such things are system bugs and it is neither necessarily a userspace or kernel bug.

In case kernel defines stable API/ABI and correctly provides information via that API/ABI and application does not work correctly, then I would say it is bug in application. Not in kernel.

We can say that some kernel API/ABI is wrong too. And in this case it could be bug in kernel.

So is current stable kernel API/ABI for input device wrong? I do not thing.

But if you think that yes, please show us what exactly and we can start discussing how to fix such problem which you see/have. I know that no application is without bugs, but in my opinion problem which you are describing is already solved and defined by current stable kernel ABI.

Quoted 4 lines

Quoted 3 lines

Really! Not a good idea. Plus I still see this as abusing kernel API/ABI as resolution should be handled differently as you are proposing.

Quoted 2 lines

I don't understand what you say here. Where are we abusing kernel API/ABI?

I mean that we already have stable API/ABI how to export size of input screen from kernel to userspace. And you want to rescale event data directly in kernel to workaround problem of screen size. So I think this is abusing API/ABI as kernel already have API for it.

Hi Pali,

Quoted 50 lines

Am 20.02.2017 um 22:54 schrieb Pali Rohár <[email protected]>:

Quoted 32 lines · On Monday 20 February 2017 22:24:31 H. Nikolaus Schaller wrote:

Hi Pali,

Quoted 30 lines

Am 20.02.2017 um 22:07 schrieb Pali Rohár <[email protected]>:

Quoted 26 lines · On Monday 20 February 2017 21:35:18 H. Nikolaus Schaller wrote:

Hi Pali,

Quoted 14 lines

Am 20.02.2017 um 20:42 schrieb Pali Rohár <[email protected]>:

Hi Nikolaus!

Quoted 9 lines · On Monday 20 February 2017 17:50:04 H. Nikolaus Schaller wrote:

Hi Dmitry,

Quoted 6 lines

Input driver may set resolution for given axis in units per mm (or units per radian for rotational axis ABS_RX, ABS_RY, ABS_RZ), and if you check the binding, you can use "touchscreen-x-mm" and "touchscreen-y-mm" to specify the size of entire touch surface and set resolution from it so that userspace can calculate the proper scaling factor.

Quoted 7 lines

Quoted 2 lines

How is this information exposed by the kernel to user-space? By scanning the DT file or tree?

Set input_abs_set_res() from kernel. And in userspace call EVIOCGABS ioctl() on input device. Look at struct input_absinfo, you should have all needed information here. This is generic input interface, no DT is needed.

This assumes that I can and want to write a graphics system myself.

Quoted 8 lines

Quoted 5 lines

Not only. There are already existing graphics systems. And you need to provide needed information from kernel, so they can start using it.

So input_abs_set_res() is needed to use in your kernel driver.

I didn't know about this feature and obviously nobody else has implemented it in the tsc2007 driver.

So... before doing other things, can you deeply look at it and check if it really fixes this problem? Because I think that yes.

You can probably set it from DT and in your DT file you can have stored screen size (or resolution factor).

Quoted 2 lines

Also for testing, you can set it even via userspace (ioctl which I wrote in previous email).

Interesting thing. It does not seem to be well known because nobody else brought it up during several months of lenghty discussions.

I have seen it is in use for scaling topics, e.g. https://lkml.org/lkml/2015/7/9/749

Quoted 9 lines

Quoted 9 lines

Quoted 8 lines

Quoted 5 lines

And if it does, it does it in a
plethora of different implementation states. That is the reason why we want to solve it once for all userlands in the kernel and not rely on user-space help.

For me this looks like "we are going to fix userspace bugs in kernel".

Quoted 6 lines

Quoted 2 lines

Such things are system bugs and it is neither necessarily a userspace or kernel bug.

In case kernel defines stable API/ABI and correctly provides information via that API/ABI and application does not work correctly, then I would say it is bug in application. Not in kernel.

So a kernel can simply add a new interface and declare bugs for userland?

Quoted 6 lines

We can say that some kernel API/ABI is wrong too. And in this case it could be bug in kernel.

So is current stable kernel API/ABI for input device wrong? I do not thing.

Difficult to judge because there is scarce documentation of this.

Quoted 9 lines

But if you think that yes, please show us what exactly and we can start discussing how to fix such problem which you see/have. I know that no application is without bugs, but in my opinion problem which you are describing is already solved and defined by current stable kernel ABI.

Quoted 4 lines

Quoted 3 lines

Really! Not a good idea. Plus I still see this as abusing kernel API/ABI as resolution should be handled differently as you are proposing.

Quoted 7 lines

Quoted 2 lines

I don't understand what you say here. Where are we abusing kernel API/ABI?

I mean that we already have stable API/ABI how to export size of input screen from kernel to userspace. And you want to rescale event data directly in kernel to workaround problem of screen size. So I think this is abusing API/ABI as kernel already have API for it.

BR and thanks,
Nikolaus

Hi Nikolaus!

Quoted 9 lines · On Monday 20 February 2017 17:50:04 H. Nikolaus Schaller wrote:

Hi Dmitry,

Quoted 6 lines

Input driver may set resolution for given axis in units per mm (or units per radian for rotational axis ABS_RX, ABS_RY, ABS_RZ), and if you check the binding, you can use "touchscreen-x-mm" and "touchscreen-y-mm" to specify the size of entire touch surface and set resolution from it so that userspace can calculate the proper scaling factor.

Quoted 2 lines

How is this information exposed by the kernel to user-space? By scanning the DT file or tree?

Set input_abs_set_res() from kernel. And in userspace call EVIOCGABS ioctl() on input device. Look at struct input_absinfo, you should have all needed information here. This is generic input interface, no DT is needed.

I hope that XServer is already using it for evdev devices...

For whole implementation look at evtest program. That should be good starting point for your userspace implementation.

While I'm watching this discussion... in my opinion kernel should just invert input axes (when needed) and should not do any other normalization or integer/floating-point re-calibration/re-calculation. If it correctly exports minimum value, maximum value and resolution then userspace can correctly re-scale input events to units which userspace needs (e.g. mapping into LCD screen pixels or whatever is needed).

Am 20.02.2017 um 22:08 schrieb Pali Rohár <[email protected]>:

Quoted 12 lines · On Monday 20 February 2017 20:42:15 Pali Rohár wrote:

Hi Nikolaus!

Quoted 9 lines · On Monday 20 February 2017 17:50:04 H. Nikolaus Schaller wrote:

Hi Dmitry,

Quoted 6 lines

Input driver may set resolution for given axis in units per mm (or units per radian for rotational axis ABS_RX, ABS_RY, ABS_RZ), and if you check the binding, you can use "touchscreen-x-mm" and "touchscreen-y-mm" to specify the size of entire touch surface and set resolution from it so that userspace can calculate the proper scaling factor.

Quoted 7 lines

Quoted 2 lines

How is this information exposed by the kernel to user-space? By scanning the DT file or tree?

Set input_abs_set_res() from kernel. And in userspace call EVIOCGABS ioctl() on input device. Look at struct input_absinfo, you should have all needed information here. This is generic input interface, no DT is needed.

Looking at kernel code... via EVIOCSABS ioctl() you can even set resolution from userspace for specified input device.

So this could be potentially used for calibrating input device from userspace? (In case DT data will not fully match current HW)

Quoted 7 lines

I hope that XServer is already using it for evdev devices...

For whole implementation look at evtest program. That should be good starting point for your userspace implementation.

While I'm watching this discussion... in my opinion kernel should just invert input axes (when needed)

Quoted 6 lines

and should not do any other
normalization or integer/floating-point re-calibration/re-calculation. If it correctly exports minimum value, maximum value and resolution then userspace can correctly re-scale input events to units which userspace needs (e.g. mapping into LCD screen pixels or whatever is needed).

--
Pali Rohár [email protected]

Quoted 32 lines

Am 20.02.2017 um 22:08 schrieb Pali Rohár <[email protected]>:

Quoted 12 lines · On Monday 20 February 2017 20:42:15 Pali Rohár wrote:

Hi Nikolaus!

Quoted 9 lines · On Monday 20 February 2017 17:50:04 H. Nikolaus Schaller wrote:

Hi Dmitry,

Quoted 6 lines

Input driver may set resolution for given axis in units per mm (or units per radian for rotational axis ABS_RX, ABS_RY, ABS_RZ), and if you check the binding, you can use "touchscreen-x-mm" and "touchscreen-y-mm" to specify the size of entire touch surface and set resolution from it so that userspace can calculate the proper scaling factor.

Quoted 7 lines

Quoted 2 lines

How is this information exposed by the kernel to user-space? By scanning the DT file or tree?

Set input_abs_set_res() from kernel. And in userspace call EVIOCGABS ioctl() on input device. Look at struct input_absinfo, you should have all needed information here. This is generic input interface, no DT is needed.

Looking at kernel code... via EVIOCSABS ioctl() you can even set resolution from userspace for specified input device.

So this could be potentially used for calibrating input device from userspace? (In case DT data will not fully match current HW)

Quoted 2 lines

I hope that XServer is already using it for evdev devices...

Quoted 5 lines

Quoted 5 lines

For whole implementation look at evtest program. That should be good starting point for your userspace implementation.

While I'm watching this discussion... in my opinion kernel should just invert input axes (when needed)

It is questionable why it should do that at all then.

Quoted 35 lines

Quoted 35 lines

Am 20.02.2017 um 22:08 schrieb Pali Rohár <[email protected]>:

Quoted 12 lines · On Monday 20 February 2017 20:42:15 Pali Rohár wrote:

Hi Nikolaus!

Quoted 9 lines · On Monday 20 February 2017 17:50:04 H. Nikolaus Schaller wrote:

Hi Dmitry,

Quoted 6 lines

Input driver may set resolution for given axis in units per mm (or units per radian for rotational axis ABS_RX, ABS_RY, ABS_RZ), and if you check the binding, you can use "touchscreen-x-mm" and "touchscreen-y-mm" to specify the size of entire touch surface and set resolution from it so that userspace can calculate the proper scaling factor.

Quoted 7 lines

Quoted 2 lines

How is this information exposed by the kernel to user-space? By scanning the DT file or tree?

Set input_abs_set_res() from kernel. And in userspace call EVIOCGABS ioctl() on input device. Look at struct input_absinfo, you should have all needed information here. This is generic input interface, no DT is needed.

Looking at kernel code... via EVIOCSABS ioctl() you can even set resolution from userspace for specified input device.

So this could be potentially used for calibrating input device from userspace? (In case DT data will not fully match current HW)

Quoted 5 lines

I hope that XServer is already using it for evdev devices...

For whole implementation look at evtest program. That should be good starting point for your userspace implementation.

Quoted 4 lines

Quoted 2 lines

Quoted 2 lines

While I'm watching this discussion... in my opinion kernel should just invert input axes (when needed)

It is questionable why it should do that at all then.

Because the task of the kernel is to provide unified view of the hardware. Axis swapping and inversion is needed to that "up" is always "up" and "right" is always "right".

Hi,

Dne 20.2.2017 v 22:50 Dmitry Torokhov napsal(a):

Quoted 39 lines · H. Nikolaus Schaller

Quoted 38 lines

Quoted 37 lines

Am 20.02.2017 um 22:08 schrieb Pali Rohár <[email protected]>:

Quoted 12 lines · On Monday 20 February 2017 20:42:15 Pali Rohár wrote:

Hi Nikolaus!

Quoted 9 lines · On Monday 20 February 2017 17:50:04 H. Nikolaus Schaller wrote:

Hi Dmitry,

Quoted 6 lines

Input driver may set resolution for given axis in units per mm (or units per radian for rotational axis ABS_RX, ABS_RY, ABS_RZ), and if you check the binding, you can use "touchscreen-x-mm" and "touchscreen-y-mm" to specify the size of entire touch surface and set resolution from it so that userspace can calculate the proper scaling factor.

Quoted 7 lines

Quoted 2 lines

How is this information exposed by the kernel to user-space? By scanning the DT file or tree?

Set input_abs_set_res() from kernel. And in userspace call EVIOCGABS ioctl() on input device. Look at struct input_absinfo, you should have all needed information here. This is generic input interface, no DT is needed.

Looking at kernel code... via EVIOCSABS ioctl() you can even set resolution from userspace for specified input device.

So this could be potentially used for calibrating input device from userspace? (In case DT data will not fully match current HW)

Quoted 7 lines

I hope that XServer is already using it for evdev devices...

For whole implementation look at evtest program. That should be good starting point for your userspace implementation.

While I'm watching this discussion... in my opinion kernel should just invert input axes (when needed)

Quoted 5 lines

Quoted 1 line

It is questionable why it should do that at all then.

Because the task of the kernel is to provide unified view of the hardware. Axis swapping and inversion is needed to that "up" is always "up" and "right" is always "right".

Actually my Xorg calibration 3x3 matrix is fine with both axis inverted (on TSC2046).

Am 20.02.2017 um 23:24 schrieb Dmitry Torokhov <[email protected]>:

Quoted 44 lines · Petr Cvek

Hi,

Dne 20.2.2017 v 22:50 Dmitry Torokhov napsal(a):

Quoted 41 lines · H. Nikolaus Schaller

Quoted 39 lines

Quoted 24 lines

Am 20.02.2017 um 22:08 schrieb Pali Rohár <[email protected]>:

Quoted 12 lines · On Monday 20 February 2017 20:42:15 Pali Rohár wrote:

Hi Nikolaus!

Quoted 9 lines · On Monday 20 February 2017 17:50:04 H. Nikolaus Schaller wrote:

Hi Dmitry,

Quoted 6 lines

Input driver may set resolution for given axis in units per mm (or units per radian for rotational axis ABS_RX, ABS_RY, ABS_RZ), and if you check the binding, you can use "touchscreen-x-mm" and "touchscreen-y-mm" to specify the size of entire touch surface and set resolution from it so that userspace can calculate the proper scaling factor.

Quoted 7 lines

Quoted 2 lines

How is this information exposed by the kernel to user-space? By scanning the DT file or tree?

Set input_abs_set_res() from kernel. And in userspace call EVIOCGABS ioctl() on input device. Look at struct input_absinfo, you should have all needed information here. This is generic input interface, no DT is needed.

Quoted 13 lines

Looking at kernel code... via EVIOCSABS ioctl() you can even set resolution from userspace for specified input device.

So this could be potentially used for calibrating input device from userspace? (In case DT data will not fully match current HW)

Quoted 7 lines

I hope that XServer is already using it for evdev devices...

For whole implementation look at evtest program. That should be good starting point for your userspace implementation.

While I'm watching this discussion... in my opinion kernel should just invert input axes (when needed)

It is questionable why it should do that at all then.

Quoted 5 lines

Quoted 3 lines

Because the task of the kernel is to provide unified view of the hardware. Axis swapping and inversion is needed to that "up" is always "up" and "right" is always "right".

Actually my Xorg calibration 3x3 matrix is fine with both axis inverted (on TSC2046).

Yes, you can make it work for your touchscreen as long as you know that it inverted somehow. How you gain this knowledge is the question.

Hi,

Dne 20.2.2017 v 22:50 Dmitry Torokhov napsal(a):

Quoted 40 lines

Quoted 38 lines · H. Nikolaus Schaller

Quoted 37 lines

Am 20.02.2017 um 22:08 schrieb Pali Rohár <[email protected]>:

Quoted 12 lines · On Monday 20 February 2017 20:42:15 Pali Rohár wrote:

Hi Nikolaus!

Quoted 9 lines · On Monday 20 February 2017 17:50:04 H. Nikolaus Schaller wrote:

Hi Dmitry,

Quoted 6 lines

Input driver may set resolution for given axis in units per mm (or units per radian for rotational axis ABS_RX, ABS_RY, ABS_RZ), and if you check the binding, you can use "touchscreen-x-mm" and "touchscreen-y-mm" to specify the size of entire touch surface and set resolution from it so that userspace can calculate the proper scaling factor.

Quoted 7 lines

Quoted 2 lines

How is this information exposed by the kernel to user-space? By scanning the DT file or tree?

Set input_abs_set_res() from kernel. And in userspace call EVIOCGABS ioctl() on input device. Look at struct input_absinfo, you should have all needed information here. This is generic input interface, no DT is needed.

Looking at kernel code... via EVIOCSABS ioctl() you can even set resolution from userspace for specified input device.

So this could be potentially used for calibrating input device from userspace? (In case DT data will not fully match current HW)

Quoted 7 lines

I hope that XServer is already using it for evdev devices...

For whole implementation look at evtest program. That should be good starting point for your userspace implementation.

While I'm watching this discussion... in my opinion kernel should just invert input axes (when needed)

Quoted 5 lines

Quoted 1 line

It is questionable why it should do that at all then.

Because the task of the kernel is to provide unified view of the hardware. Axis swapping and inversion is needed to that "up" is always "up" and "right" is always "right".

Actually my Xorg calibration 3x3 matrix is fine with both axis inverted (on TSC2046).

Am 20.02.2017 um 22:50 schrieb Dmitry Torokhov <[email protected]>:

Quoted 35 lines · H. Nikolaus Schaller

Quoted 35 lines

Am 20.02.2017 um 22:08 schrieb Pali Rohár <[email protected]>:

Quoted 12 lines · On Monday 20 February 2017 20:42:15 Pali Rohár wrote:

Hi Nikolaus!

Quoted 9 lines · On Monday 20 February 2017 17:50:04 H. Nikolaus Schaller wrote:

Hi Dmitry,

Quoted 6 lines

Input driver may set resolution for given axis in units per mm (or units per radian for rotational axis ABS_RX, ABS_RY, ABS_RZ), and if you check the binding, you can use "touchscreen-x-mm" and "touchscreen-y-mm" to specify the size of entire touch surface and set resolution from it so that userspace can calculate the proper scaling factor.

Quoted 7 lines

Quoted 2 lines

How is this information exposed by the kernel to user-space? By scanning the DT file or tree?

Set input_abs_set_res() from kernel. And in userspace call EVIOCGABS ioctl() on input device. Look at struct input_absinfo, you should have all needed information here. This is generic input interface, no DT is needed.

Looking at kernel code... via EVIOCSABS ioctl() you can even set resolution from userspace for specified input device.

So this could be potentially used for calibrating input device from userspace? (In case DT data will not fully match current HW)

Quoted 5 lines

I hope that XServer is already using it for evdev devices...

For whole implementation look at evtest program. That should be good starting point for your userspace implementation.

Quoted 4 lines

Quoted 2 lines

Quoted 2 lines

While I'm watching this discussion... in my opinion kernel should just invert input axes (when needed)

It is questionable why it should do that at all then.

Because the task of the kernel is to provide unified view of the hardware. Axis swapping and inversion is needed to that "up" is always "up" and "right" is always "right".

Hi,

Quoted 41 lines

Am 20.02.2017 um 22:50 schrieb Dmitry Torokhov <[email protected]>:

Quoted 38 lines · H. Nikolaus Schaller

Quoted 37 lines

Am 20.02.2017 um 22:08 schrieb Pali Rohár <[email protected]>:

Quoted 12 lines · On Monday 20 February 2017 20:42:15 Pali Rohár wrote:

Hi Nikolaus!

Quoted 9 lines · On Monday 20 February 2017 17:50:04 H. Nikolaus Schaller wrote:

Hi Dmitry,

Quoted 6 lines

Input driver may set resolution for given axis in units per mm (or units per radian for rotational axis ABS_RX, ABS_RY, ABS_RZ), and if you check the binding, you can use "touchscreen-x-mm" and "touchscreen-y-mm" to specify the size of entire touch surface and set resolution from it so that userspace can calculate the proper scaling factor.

Quoted 7 lines

Quoted 2 lines

How is this information exposed by the kernel to user-space? By scanning the DT file or tree?

Set input_abs_set_res() from kernel. And in userspace call EVIOCGABS ioctl() on input device. Look at struct input_absinfo, you should have all needed information here. This is generic input interface, no DT is needed.

Looking at kernel code... via EVIOCSABS ioctl() you can even set resolution from userspace for specified input device.

So this could be potentially used for calibrating input device from userspace? (In case DT data will not fully match current HW)

Quoted 7 lines

I hope that XServer is already using it for evdev devices...

For whole implementation look at evtest program. That should be good starting point for your userspace implementation.

While I'm watching this discussion... in my opinion kernel should just invert input axes (when needed)

Quoted 5 lines

Quoted 1 line

It is questionable why it should do that at all then.

Because the task of the kernel is to provide unified view of the hardware. Axis swapping and inversion is needed to that "up" is always "up" and "right" is always "right".

Hm. Why not touching pixel (0,0) on the touch is always pixel (0,0) on the screen and touching pixel (639,479) is always (639,479)?

I think it is time to end this discussion.

It has show me how much a mess and half-baked area this is, which I did not expect. I read contradicting messages from different people:

* don't break user space because it is carved in stone
* fix users space if you want to do it properly
* scaling by +/-1 and shifting by full range is ok
* scaling by ts-size/adc-range and shifting by adc_min is not ok
* full numeric ADC resolution is required but subpixel coordinates is not acceptable

I will monitor this to see if this becomes sorted out before submitting anything new.

BR and thanks,
Nikolaus

Quoted 2 lines

Hm. Why not touching pixel (0,0) on the touch is always pixel (0,0) on the screen and touching pixel (639,479) is always (639,479)?

Important is that there is no 1:1 mapping between input evdev device and drm/fb output device. These are two independent devices. There is no connection between screen and touch. So such presumption should not be done in kernel. You can do that in userspace.

Lets take e.g. touchpad. It acts similarly as touchscreen input device (both reports absolute positioned touch events), but touchpad is not connected with screen. And from kernel point of view these devices are both input and absolute positioned.

It looks like the whole problem is there that you wanted to do this mapping for your hardware in kernel. And this is not what is kernel doing or should do. Moreover I know people who are using integrated touchscreen on laptop as (touch) input device for external monitor. And in this configuration it does not make any sense to map touchscreen input to pixels of integrated LCD touchscreen (as external monitor could have different resolution as integrated LCD touchcreen).

Quoted 5 lines

Am 20.02.2017 um 22:08 schrieb Pali Rohár <[email protected]>:

Quoted 2 lines · On Monday 20 February 2017 20:42:15 Pali Rohár wrote:

While I'm watching this discussion... in my opinion kernel should just invert input axes (when needed)

It is questionable why it should do that at all then.

User-Space can also easily do it. Either the driver should provide raw data only or if it does pre-processing (scaling by +/-1), why exclude pre-scaling by other factors?

Quoted 6 lines

Quoted 6 lines

and should not do any other
normalization or integer/floating-point re-calibration/re-calculation. If it correctly exports minimum value, maximum value and resolution then userspace can correctly re-scale input events to units which userspace needs (e.g. mapping into LCD screen pixels or whatever is needed).

Am 20.02.2017 um 23:04 schrieb Pali Rohár <[email protected]>:

Quoted 11 lines · On Monday 20 February 2017 22:27:39 H. Nikolaus Schaller wrote:

Quoted 5 lines

Am 20.02.2017 um 22:08 schrieb Pali Rohár <[email protected]>:

Quoted 2 lines · On Monday 20 February 2017 20:42:15 Pali Rohár wrote:

While I'm watching this discussion... in my opinion kernel should just invert input axes (when needed)

It is questionable why it should do that at all then.

User-Space can also easily do it. Either the driver should provide raw data only or if it does pre-processing (scaling by +/-1), why exclude pre-scaling by other factors?

Via resolution property which is in that EVIOCSABS ioctl() you specify value which represent unit per mm. So you cannot do full rescaling like via affine transformation. Specially you cannot swap axes or invert it.

As such thing is not supported by current kernel <--> userspace API it needs to be done in kernel.

Moreover I see that this is already handled by kernel's of_touchscreen.c code via DT properties: touchscreen-inverted-* touchscreen-swapped-x-y

And... I'm not sure but I think that linux exports absolute input devices with coordinates where point (0,0) is mapped as left upper corner.

Quoted 6 lines

Quoted 6 lines

Quoted 6 lines

and should not do any other
normalization or integer/floating-point re-calibration/re-calculation. If it correctly exports minimum value, maximum value and resolution then userspace can correctly re-scale input events to units which userspace needs (e.g. mapping into LCD screen pixels or whatever is needed).

Am 20.02.2017 um 20:42 schrieb Pali Rohár <[email protected]>:

Hi Nikolaus!

Quoted 9 lines · On Monday 20 February 2017 17:50:04 H. Nikolaus Schaller wrote:

Hi Dmitry,

Quoted 6 lines

Input driver may set resolution for given axis in units per mm (or units per radian for rotational axis ABS_RX, ABS_RY, ABS_RZ), and if you check the binding, you can use "touchscreen-x-mm" and "touchscreen-y-mm" to specify the size of entire touch surface and set resolution from it so that userspace can calculate the proper scaling factor.

Quoted 2 lines

How is this information exposed by the kernel to user-space? By scanning the DT file or tree?

Set input_abs_set_res() from kernel.

And in userspace call EVIOCGABS
ioctl() on input device. Look at struct input_absinfo, you should have all needed information here. This is generic input interface, no DT is needed.

I hope that XServer is already using it for evdev devices...

For whole implementation look at evtest program. That should be good starting point for your userspace implementation.

While I'm watching this discussion... in my opinion kernel should just invert input axes (when needed) and should not do any other normalization or integer/floating-point re-calibration/re-calculation. If it correctly exports minimum value, maximum value and resolution then userspace can correctly re-scale input events to units which userspace needs (e.g. mapping into LCD screen pixels or whatever is needed).

Hi Pali,

Quoted 17 lines

Am 20.02.2017 um 20:42 schrieb Pali Rohár <[email protected]>:

Hi Nikolaus!

Quoted 11 lines · On Monday 20 February 2017 17:50:04 H. Nikolaus Schaller wrote:

Hi Dmitry,

Quoted 6 lines

Input driver may set resolution for given axis in units per mm (or units per radian for rotational axis ABS_RX, ABS_RY, ABS_RZ), and if you check the binding, you can use "touchscreen-x-mm" and "touchscreen-y-mm" to specify the size of entire touch surface and set resolution from it so that userspace can calculate the proper scaling factor.

How is this information exposed by the kernel to user-space? By scanning the DT file or tree?

Quoted 1 line

Set input_abs_set_res() from kernel.

I can't find this function defined anywhere but used in 101 LOC.
LXR doesn't know it either: http://lxr.free-electrons.com/ident?i=input_abs_set_res

What is going on here?

Quoted 4 lines

And in userspace call EVIOCGABS
ioctl() on input device. Look at struct input_absinfo, you should have all needed information here. This is generic input interface, no DT is needed.

Ok, if this is not set by a driver it is indeed a driver bug.

But we have to define it's value in DT because the tsc2007 does not know anything about the panel dimensions.

IMHO something that should be done by generic of_touchscreen.c

If of_touchscreen would simply pass the touchscreen-size parameters to input_abs_set_res() and the bindings would define it to be units/mm it might have saved us all a lot of work and discussion.

Quoted 6 lines

I hope that XServer is already using it for evdev devices...

For whole implementation look at evtest program. That should be good starting point for your userspace implementation.

Quoted 6 lines

While I'm watching this discussion... in my opinion kernel should just invert input axes (when needed) and should not do any other normalization or integer/floating-point re-calibration/re-calculation. If it correctly exports minimum value, maximum value and resolution then userspace can correctly re-scale input events to units which userspace needs (e.g. mapping into LCD screen pixels or whatever is needed).

BR and thanks,
Nikolaus

But it really make sense to set input_abs_set_res() from DT.

Quoted 4 lines

They are "pixels" or the touch controller, i.e. native unites in which device reports coordinates, as opposed to points per inch, or millimeters, or whatever. These "pixels" do not have to have 1:1 relation to the LCD pixels; in fact they rarely do.

I wouldn't call this "pixels". Rather "ADC steps" or something.

Quoted 1 line

Why? Nothing stops you from querying the device and figure out scaling.

What stops me is that I have no (and do not want to have) that level of control over user-space code.

Quoted 2 lines

And still, according to DT folks, device tree forms an ABI and thus we are not to change it, even if it is easy.

I think this needs a more differentiated view.

In my view the names of the binding properties and what they influence form indeed an ABI. It should be stable and interpreted in the same way.

But it allows to load different firmware for different requirements. Like the user application ABI is stable but you can still load different software.

1. check if chip is really present and don't succeed if it isn't.
2. if it succeeds, power down the chip until accessed

Signed-off-by H. Nikolaus Schaller <[email protected]>

---
 drivers/input/touchscreen/tsc2007.c | 8 ++++++++
 1 file changed, 8 insertions(+)

drivers/input/touchscreen/tsc2007.c +8 -0

index c1d9593..76b462b 100644
@@ -529,6 +529,14 @@ static int tsc2007_probe(struct i2c_client *client,
 
 	tsc2007_stop(ts);
 
+	/* power down the chip (TSC2007_SETUP does not ACK on I2C) */
+	err = tsc2007_xfer(ts, PWRDOWN);
+	if (err < 0) {
+		dev_err(&client->dev,
+			"Failed to setup chip: %d\n", err);
+		return err;	/* usually, chip does not respond */
+	}
+
 	err = input_register_device(input_dev);
 	if (err) {
 		dev_err(&client->dev,

Signature · 1 line

2.7.3

Am 28.12.2016 um 15:53 schrieb H. Nikolaus Schaller <[email protected]>:

Signed-off-by H. Nikolaus Schaller <[email protected]>

Acked-by Rob Herring <[email protected]>

---
Documentation/devicetree/bindings/input/{ => touchscreen}/ads7846.txt | 0

1 file changed, 0 insertions(+), 0 deletions(-)

rename Documentation/devicetree/bindings/input/{ => touchscreen}/ads7846.txt (100%)

Documentation/devicetree/bindings/input/touchscreen/ads7846.txt renamed from Documentation/devicetree/bindings/input/ads7846.txt +0 -0

similarity index 100%
rename from Documentation/devicetree/bindings/input/ads7846.txt
rename to Documentation/devicetree/bindings/input/touchscreen/ads7846.txt

Signature · 1 line

2.7.3

Fix module table so that the driver is loaded if compiled as module and requested by DT.

Am 28.01.2017 um 20:35 schrieb Dmitry Torokhov <[email protected]>:

Quoted 3 lines · H. Nikolaus Schaller

Fix module table so that the driver is loaded if compiled as module and requested by DT.

I believe I already replied to a similar patch: we alreadyhave necessary aliases in this driver, we need to fix module loading to use it.

Quoted 4 lines

We really need to fix it between spi/i23c core and module utils instead of keeping adding duplicate IDs all over drivers. We already have OF module device table containing the same data, we should be able to use it.

Agreed, unfortunately until the I2C and SPI core are changed to properly report OF modaliases, we will have to keep adding these duplicated IDs.

And changing the I2C and SPI core isn't trivial since it could break a lot of drivers that rely on a platform modalias being reported (i.e: no OF device IDs present in the drivers even when are registered via DT).

Hi Dmitry,

Quoted 9 lines

Am 28.01.2017 um 20:35 schrieb Dmitry Torokhov <[email protected]>:

Quoted 3 lines · H. Nikolaus Schaller

Fix module table so that the driver is loaded if compiled as module and requested by DT.

I believe I already replied to a similar patch: we alreadyhave necessary aliases in this driver, we need to fix module loading to use it.

Yes, you did comment on [PATCH v6 7/8] (19 Nov 2016):

Quoted 4 lines

Quoted 4 lines

We really need to fix it between spi/i23c core and module utils instead of keeping adding duplicate IDs all over drivers. We already have OF module device table containing the same data, we should be able to use it.

And Javier Martinez Canillas replied (23 Nov 2016):

Quoted 6 lines

Agreed, unfortunately until the I2C and SPI core are changed to properly report OF modaliases, we will have to keep adding these duplicated IDs.

And changing the I2C and SPI core isn't trivial since it could break a lot of drivers that rely on a platform modalias being reported (i.e: no OF device IDs present in the drivers even when are registered via DT).

Therefore I didn't see a need to change it.

Am 29.01.2017 um 19:01 schrieb Dmitry Torokhov <[email protected]>:

Quoted 28 lines · H. Nikolaus Schaller

Hi Dmitry,

Quoted 9 lines

Am 28.01.2017 um 20:35 schrieb Dmitry Torokhov <[email protected]>:

Quoted 3 lines · H. Nikolaus Schaller

Fix module table so that the driver is loaded if compiled as module and requested by DT.

I believe I already replied to a similar patch: we alreadyhave necessary aliases in this driver, we need to fix module loading to use it.

Yes, you did comment on [PATCH v6 7/8] (19 Nov 2016):

Quoted 4 lines

Quoted 4 lines

We really need to fix it between spi/i23c core and module utils instead of keeping adding duplicate IDs all over drivers. We already have OF module device table containing the same data, we should be able to use it.

And Javier Martinez Canillas replied (23 Nov 2016):

Quoted 6 lines

Agreed, unfortunately until the I2C and SPI core are changed to properly report OF modaliases, we will have to keep adding these duplicated IDs.

And changing the I2C and SPI core isn't trivial since it could break a lot of drivers that rely on a platform modalias being reported (i.e: no OF device IDs present in the drivers even when are registered via DT).

Quoted 1 line

Therefore I didn't see a need to change it.

I agree that changing I2C and SPI core is not trivial, however this is no reason for piling up workarounds in all drivers. Are you seriously advocating going though *every* driver and copying OF data into I2C/SPI instead of doing the right thing and fixing the root of the issue?

Am 29.01.2017 um 19:25 schrieb H. Nikolaus Schaller <[email protected]>:

Hi Dmitry,

Quoted 33 lines

Am 29.01.2017 um 19:01 schrieb Dmitry Torokhov <[email protected]>:

Quoted 29 lines · H. Nikolaus Schaller

Hi Dmitry,

Quoted 9 lines

Am 28.01.2017 um 20:35 schrieb Dmitry Torokhov <[email protected]>:

Quoted 3 lines · H. Nikolaus Schaller

Fix module table so that the driver is loaded if compiled as module and requested by DT.

I believe I already replied to a similar patch: we alreadyhave necessary aliases in this driver, we need to fix module loading to use it.

Yes, you did comment on [PATCH v6 7/8] (19 Nov 2016):

Quoted 4 lines

Quoted 4 lines

We really need to fix it between spi/i23c core and module utils instead of keeping adding duplicate IDs all over drivers. We already have OF module device table containing the same data, we should be able to use it.

And Javier Martinez Canillas replied (23 Nov 2016):

Quoted 6 lines

Agreed, unfortunately until the I2C and SPI core are changed to properly report OF modaliases, we will have to keep adding these duplicated IDs.

And changing the I2C and SPI core isn't trivial since it could break a lot of drivers that rely on a platform modalias being reported (i.e: no OF device IDs present in the drivers even when are registered via DT).

Therefore I didn't see a need to change it.

Quoted 4 lines

I agree that changing I2C and SPI core is not trivial, however this is no reason for piling up workarounds in all drivers. Are you seriously advocating going though *every* driver and copying OF data into I2C/SPI instead of doing the right thing and fixing the root of the issue?

If you prefer to have this done (and I agree it would be a tiny improvement), please do it for us all. But please after merging this workaround.

Hi Dmitry, Javier,

Quoted 42 lines

Am 29.01.2017 um 19:25 schrieb H. Nikolaus Schaller <[email protected]>:

Hi Dmitry,

Quoted 37 lines

Am 29.01.2017 um 19:01 schrieb Dmitry Torokhov <[email protected]>:

Quoted 21 lines · H. Nikolaus Schaller

Hi Dmitry,

Quoted 9 lines

Am 28.01.2017 um 20:35 schrieb Dmitry Torokhov <[email protected]>:

Quoted 3 lines · H. Nikolaus Schaller

Fix module table so that the driver is loaded if compiled as module and requested by DT.

I believe I already replied to a similar patch: we alreadyhave necessary aliases in this driver, we need to fix module loading to use it.

Yes, you did comment on [PATCH v6 7/8] (19 Nov 2016):

Quoted 4 lines

Quoted 4 lines

We really need to fix it between spi/i23c core and module utils instead of keeping adding duplicate IDs all over drivers. We already have OF module device table containing the same data, we should be able to use it.

And Javier Martinez Canillas replied (23 Nov 2016):

Quoted 8 lines

Quoted 6 lines

Agreed, unfortunately until the I2C and SPI core are changed to properly report OF modaliases, we will have to keep adding these duplicated IDs.

And changing the I2C and SPI core isn't trivial since it could break a lot of drivers that rely on a platform modalias being reported (i.e: no OF device IDs present in the drivers even when are registered via DT).

Therefore I didn't see a need to change it.

I agree that changing I2C and SPI core is not trivial, however this is no reason for piling up workarounds in all drivers. Are you seriously advocating going though *every* driver and copying OF data into I2C/SPI instead of doing the right thing and fixing the root of the issue?

Quoted 2 lines

If you prefer to have this done (and I agree it would be a tiny improvement), please do it for us all. But please after merging this workaround.

Have we been lucky to find someone who is able and willing to work on this?

If not, I'd recommend to stay with the current level of optimality.

BR and thanks,
Nikolaus

Hello Nikolaus,

Quoted 47 lines · H. Nikolaus Schaller

Hi Dmitry, Javier,

Quoted 44 lines

Am 29.01.2017 um 19:25 schrieb H. Nikolaus Schaller <[email protected]>:

Hi Dmitry,

Quoted 31 lines

Am 29.01.2017 um 19:01 schrieb Dmitry Torokhov <[email protected]>:

Quoted 28 lines · H. Nikolaus Schaller

Hi Dmitry,

Quoted 9 lines

Am 28.01.2017 um 20:35 schrieb Dmitry Torokhov <[email protected]>:

Quoted 3 lines · H. Nikolaus Schaller

Fix module table so that the driver is loaded if compiled as module and requested by DT.

I believe I already replied to a similar patch: we alreadyhave necessary aliases in this driver, we need to fix module loading to use it.

Yes, you did comment on [PATCH v6 7/8] (19 Nov 2016):

Quoted 4 lines

Quoted 4 lines

We really need to fix it between spi/i23c core and module utils instead of keeping adding duplicate IDs all over drivers. We already have OF module device table containing the same data, we should be able to use it.

And Javier Martinez Canillas replied (23 Nov 2016):

Quoted 6 lines

Agreed, unfortunately until the I2C and SPI core are changed to properly report OF modaliases, we will have to keep adding these duplicated IDs.

And changing the I2C and SPI core isn't trivial since it could break a lot of drivers that rely on a platform modalias being reported (i.e: no OF device IDs present in the drivers even when are registered via DT).

Quoted 6 lines

Quoted 1 line

Therefore I didn't see a need to change it.

I agree that changing I2C and SPI core is not trivial, however this is no reason for piling up workarounds in all drivers. Are you seriously advocating going though *every* driver and copying OF data into I2C/SPI instead of doing the right thing and fixing the root of the issue?

If you prefer to have this done (and I agree it would be a tiny improvement), please do it for us all. But please after merging this workaround.

Quoted 2 lines

Have we been lucky to find someone who is able and willing to work on this?

As said, changing the core is trivial. A RFC patch is [0].

The problem is how to make sure that this change won't cause regressions in existing drivers.

There was particularly tricky for the spi-nor driver, it doesn't help that a lot of DT are using undocumented compatible strings (sometimes with no vendor prefix). You can see the discussion here [1].

In the same thread Mark Brown said that it wasn't that bad to have the information in the OF device ID table duplicated in a SPI device table.

Certainly isn't the best approach but IMHO is better than the module not loading.

Quoted 5 lines

As said, changing the core is trivial. A RFC patch is [0].

The problem is how to make sure that this change won't cause regressions in existing drivers.

If the concern with drivers having I2C or SPI device table, but not OF device table, then I think cocinnelle could be used to scan and find them.

Quoted 10 lines

There was particularly tricky for the spi-nor driver, it doesn't help that a lot of DT are using undocumented compatible strings (sometimes with no vendor prefix). You can see the discussion here [1].

In the same thread Mark Brown said that it wasn't that bad to have the information in the OF device ID table duplicated in a SPI device table.

Certainly isn't the best approach but IMHO is better than the module not loading.

You can always build the module into kernel or load it by hand if it is that important. Module autoloading does not work anyway if you have several compatible strings, like

	compatible = "vendor,new-device", "vendor,generic-device";

Thanks.

Am 01.02.2017 um 23:28 schrieb Dmitry Torokhov <[email protected]>:

Quoted 50 lines · Javier Martinez Canillas

Hello Nikolaus,

Quoted 47 lines · H. Nikolaus Schaller

Hi Dmitry, Javier,

Quoted 44 lines

Am 29.01.2017 um 19:25 schrieb H. Nikolaus Schaller <[email protected]>:

Hi Dmitry,

Quoted 31 lines

Am 29.01.2017 um 19:01 schrieb Dmitry Torokhov <[email protected]>:

Quoted 28 lines · H. Nikolaus Schaller

Hi Dmitry,

Quoted 9 lines

Am 28.01.2017 um 20:35 schrieb Dmitry Torokhov <[email protected]>:

Quoted 3 lines · H. Nikolaus Schaller

Fix module table so that the driver is loaded if compiled as module and requested by DT.

I believe I already replied to a similar patch: we alreadyhave necessary aliases in this driver, we need to fix module loading to use it.

Yes, you did comment on [PATCH v6 7/8] (19 Nov 2016):

Quoted 4 lines

Quoted 4 lines

We really need to fix it between spi/i23c core and module utils instead of keeping adding duplicate IDs all over drivers. We already have OF module device table containing the same data, we should be able to use it.

And Javier Martinez Canillas replied (23 Nov 2016):

Quoted 6 lines

Agreed, unfortunately until the I2C and SPI core are changed to properly report OF modaliases, we will have to keep adding these duplicated IDs.

And changing the I2C and SPI core isn't trivial since it could break a lot of drivers that rely on a platform modalias being reported (i.e: no OF device IDs present in the drivers even when are registered via DT).

Quoted 6 lines

Quoted 1 line

Therefore I didn't see a need to change it.

I agree that changing I2C and SPI core is not trivial, however this is no reason for piling up workarounds in all drivers. Are you seriously advocating going though *every* driver and copying OF data into I2C/SPI instead of doing the right thing and fixing the root of the issue?

If you prefer to have this done (and I agree it would be a tiny improvement), please do it for us all. But please after merging this workaround.

Quoted 7 lines

Quoted 2 lines

Have we been lucky to find someone who is able and willing to work on this?

As said, changing the core is trivial. A RFC patch is [0].

The problem is how to make sure that this change won't cause regressions in existing drivers.

If the concern with drivers having I2C or SPI device table, but not OF device table, then I think cocinnelle could be used to scan and find them.

Quoted 5 lines

There was particularly tricky for the spi-nor driver, it doesn't help that a lot of DT are using undocumented compatible strings (sometimes with no vendor prefix). You can see the discussion here [1].

Quoted 5 lines

In the same thread Mark Brown said that it wasn't that bad to have the information in the OF device ID table duplicated in a SPI device table.

Certainly isn't the best approach but IMHO is better than the module not loading.

You can always build the module into kernel or load it by hand if it is that important.

Module autoloading does not work anyway if you have several compatible strings, like

	compatible = "vendor,new-device", "vendor,generic-device";

The tsc2007 chip not only has a resistive touch screen controller but also an external AUX adc imput which can be used for an ambient light sensor, battery voltage monitoring or any general purpose.

Additionally it can measure the chip temperature.

This extension provides an iio interface for these adc channels.

Since it is not wasting much resources and is very straightforward, we simply provide all other adc channels as optional iio interfaces as weel. This can be used for debugging or special applications.

This patch also splits the tsc2007 driver in several source files:
tsc2007.h -- constants, structs and stubs tsc2007_core.c -- functional parts of the original driver tsc2007_iio.c -- the optional iio stuff

Makefile magic allows to conditionally link the iio stuff if CONFIG_IIO=y or =m in a way that it works with CONFIG_TOUCHSCREEN_TSC2007=m.

Signed-off-by H. Nikolaus Schaller <[email protected]>

Reviewed-by Jonathan Cameron <[email protected]>

---
 drivers/input/touchscreen/Kconfig                  |  10 ++
 drivers/input/touchscreen/Makefile                 |   2 +
 drivers/input/touchscreen/tsc2007.h                | 116 ++++++++++++++++
 .../touchscreen/{tsc2007.c => tsc2007_core.c}      |  95 +++----------
 drivers/input/touchscreen/tsc2007_iio.c            | 150 +++++++++++++++++++++
 5 files changed, 299 insertions(+), 74 deletions(-)
 create mode 100644 drivers/input/touchscreen/tsc2007.h
 rename drivers/input/touchscreen/{tsc2007.c => tsc2007_core.c} (86%)
 create mode 100644 drivers/input/touchscreen/tsc2007_iio.c

drivers/input/touchscreen/Kconfig +10 -0

index efca013..1616a8d 100644
@@ -1035,6 +1035,16 @@ config TOUCHSCREEN_TSC2007
 	  To compile this driver as a module, choose M here: the
 	  module will be called tsc2007.
 
+config TOUCHSCREEN_TSC2007_IIO
+	bool "IIO interface for external ADC input and temperature"
+	depends on TOUCHSCREEN_TSC2007
+	depends on IIO=y || IIO=TOUCHSCREEN_TSC2007
+	help
+	  Saying Y here adds an iio interface to the tsc2007 which
+	  provides values for the AUX input (used for e.g. battery
+	  or ambient light monitoring), temperature and raw input
+	  values.
+
 config TOUCHSCREEN_W90X900
 	tristate "W90P910 touchscreen driver"
 	depends on ARCH_W90X900

drivers/input/touchscreen/Makefile +2 -0

index 81b8645..05d1cc8 100644
@@ -80,6 +80,8 @@ obj-$(CONFIG_TOUCHSCREEN_TSC_SERIO)	+= tsc40.o
 obj-$(CONFIG_TOUCHSCREEN_TSC200X_CORE)	+= tsc200x-core.o
 obj-$(CONFIG_TOUCHSCREEN_TSC2004)	+= tsc2004.o
 obj-$(CONFIG_TOUCHSCREEN_TSC2005)	+= tsc2005.o
+tsc2007-y := tsc2007_core.o
+tsc2007-$(CONFIG_TOUCHSCREEN_TSC2007_IIO)	+= tsc2007_iio.o
 obj-$(CONFIG_TOUCHSCREEN_TSC2007)	+= tsc2007.o
 obj-$(CONFIG_TOUCHSCREEN_UCB1400)	+= ucb1400_ts.o
 obj-$(CONFIG_TOUCHSCREEN_WACOM_W8001)	+= wacom_w8001.o

drivers/input/touchscreen/tsc2007.h new file +116 -0

new file mode 100644
index 0000000..16efb60
@@ -0,0 +1,116 @@
+/*
+ * Copyright (c) 2008 MtekVision Co., Ltd.
+ *	Kwangwoo Lee <kwlee-ec7hoAtq5SbSUeElwK9/[email protected]>
+ *
+ * Using code from:
+ *  - ads7846.c
+ *	Copyright (c) 2005 David Brownell
+ *	Copyright (c) 2006 Nokia Corporation
+ *  - corgi_ts.c
+ *	Copyright (C) 2004-2005 Richard Purdie
+ *  - omap_ts.[hc], ads7846.h, ts_osk.c
+ *	Copyright (C) 2002 MontaVista Software
+ *	Copyright (C) 2004 Texas Instruments
+ *	Copyright (C) 2005 Dirk Behme
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ */
+
+#include <linux/input/touchscreen.h>
+
+#define TSC2007_MEASURE_TEMP0		(0x0 << 4)
+#define TSC2007_MEASURE_AUX		(0x2 << 4)
+#define TSC2007_MEASURE_TEMP1		(0x4 << 4)
+#define TSC2007_ACTIVATE_XN		(0x8 << 4)
+#define TSC2007_ACTIVATE_YN		(0x9 << 4)
+#define TSC2007_ACTIVATE_YP_XN		(0xa << 4)
+#define TSC2007_SETUP			(0xb << 4)
+#define TSC2007_MEASURE_X		(0xc << 4)
+#define TSC2007_MEASURE_Y		(0xd << 4)
+#define TSC2007_MEASURE_Z1		(0xe << 4)
+#define TSC2007_MEASURE_Z2		(0xf << 4)
+
+#define TSC2007_POWER_OFF_IRQ_EN	(0x0 << 2)
+#define TSC2007_ADC_ON_IRQ_DIS0		(0x1 << 2)
+#define TSC2007_ADC_OFF_IRQ_EN		(0x2 << 2)
+#define TSC2007_ADC_ON_IRQ_DIS1		(0x3 << 2)
+
+#define TSC2007_12BIT			(0x0 << 1)
+#define TSC2007_8BIT			(0x1 << 1)
+
+#define	MAX_12BIT			((1 << 12) - 1)
+
+#define ADC_ON_12BIT	(TSC2007_12BIT | TSC2007_ADC_ON_IRQ_DIS0)
+
+#define READ_Y		(ADC_ON_12BIT | TSC2007_MEASURE_Y)
+#define READ_Z1		(ADC_ON_12BIT | TSC2007_MEASURE_Z1)
+#define READ_Z2		(ADC_ON_12BIT | TSC2007_MEASURE_Z2)
+#define READ_X		(ADC_ON_12BIT | TSC2007_MEASURE_X)
+#define PWRDOWN		(TSC2007_12BIT | TSC2007_POWER_OFF_IRQ_EN)
+
+struct ts_event {
+	u16	x;
+	u16	y;
+	u16	z1, z2;
+};
+
+struct tsc2007 {
+	struct input_dev	*input;
+	char			phys[32];
+
+	struct i2c_client	*client;
+
+	u16			model;
+	u16			x_plate_ohms;
+
+	struct touchscreen_properties prop;
+
+	bool			report_resistance;
+	u16			min_x;
+	u16			min_y;
+	u16			max_x;
+	u16			max_y;
+	u16			max_rt;
+	unsigned long		poll_period; /* in jiffies */
+	int			fuzzx;
+	int			fuzzy;
+	int			fuzzz;
+
+	unsigned int		gpio;
+	int			irq;
+
+	wait_queue_head_t	wait;
+	bool			stopped;
+	bool			pendown;
+
+	int			(*get_pendown_state)(struct device *);
+	void			(*clear_penirq)(void);
+
+	struct mutex		mlock;
+	struct iio_dev		*iio_dev;	/* optional */
+};
+
+int tsc2007_xfer(struct tsc2007 *tsc, u8 cmd);
+u32 tsc2007_calculate_resistance(struct tsc2007 *tsc,
+					struct ts_event *tc);
+bool tsc2007_is_pen_down(struct tsc2007 *ts);
+
+#if IS_ENABLED(CONFIG_TOUCHSCREEN_TSC2007_IIO)
+
+/* defined in tsc2007_iio.c */
+int tsc2007_iio_configure(struct tsc2007 *ts);
+void tsc2007_iio_unconfigure(struct tsc2007 *ts);
+
+#else /* CONFIG_TOUCHSCREEN_TSC2007_IIO */
+
+static inline int tsc2007_iio_configure(struct tsc2007 *ts)
+{
+	return 0;
+}
+static inline void tsc2007_iio_unconfigure(struct tsc2007 *ts)
+{
+}
+
+#endif /* CONFIG_TOUCHSCREEN_TSC2007_IIO */

drivers/input/touchscreen/tsc2007_core.c renamed from drivers/input/touchscreen/tsc2007.c +21 -74

similarity index 86%
rename from drivers/input/touchscreen/tsc2007.c
rename to drivers/input/touchscreen/tsc2007_core.c
index 76b462b..812ded8 100644
@@ -27,79 +27,11 @@
 #include <linux/i2c.h>
 #include <linux/i2c/tsc2007.h>
 #include <linux/of_device.h>
-#include <linux/of.h>
 #include <linux/of_gpio.h>
-#include <linux/input/touchscreen.h>
-
-#define TSC2007_MEASURE_TEMP0		(0x0 << 4)
-#define TSC2007_MEASURE_AUX		(0x2 << 4)
-#define TSC2007_MEASURE_TEMP1		(0x4 << 4)
-#define TSC2007_ACTIVATE_XN		(0x8 << 4)
-#define TSC2007_ACTIVATE_YN		(0x9 << 4)
-#define TSC2007_ACTIVATE_YP_XN		(0xa << 4)
-#define TSC2007_SETUP			(0xb << 4)
-#define TSC2007_MEASURE_X		(0xc << 4)
-#define TSC2007_MEASURE_Y		(0xd << 4)
-#define TSC2007_MEASURE_Z1		(0xe << 4)
-#define TSC2007_MEASURE_Z2		(0xf << 4)
-
-#define TSC2007_POWER_OFF_IRQ_EN	(0x0 << 2)
-#define TSC2007_ADC_ON_IRQ_DIS0		(0x1 << 2)
-#define TSC2007_ADC_OFF_IRQ_EN		(0x2 << 2)
-#define TSC2007_ADC_ON_IRQ_DIS1		(0x3 << 2)
-
-#define TSC2007_12BIT			(0x0 << 1)
-#define TSC2007_8BIT			(0x1 << 1)
-
-#define	MAX_12BIT			((1 << 12) - 1)
-
-#define ADC_ON_12BIT	(TSC2007_12BIT | TSC2007_ADC_ON_IRQ_DIS0)
-
-#define READ_Y		(ADC_ON_12BIT | TSC2007_MEASURE_Y)
-#define READ_Z1		(ADC_ON_12BIT | TSC2007_MEASURE_Z1)
-#define READ_Z2		(ADC_ON_12BIT | TSC2007_MEASURE_Z2)
-#define READ_X		(ADC_ON_12BIT | TSC2007_MEASURE_X)
-#define PWRDOWN		(TSC2007_12BIT | TSC2007_POWER_OFF_IRQ_EN)
-
-struct ts_event {
-	u16	x;
-	u16	y;
-	u16	z1, z2;
-};
-
-struct tsc2007 {
-	struct input_dev	*input;
-	char			phys[32];
-
-	struct i2c_client	*client;
-
-	u16			model;
-	u16			x_plate_ohms;
-
-	struct touchscreen_properties prop;
-
-	bool			report_resistance;
-	u16			min_x;
-	u16			min_y;
-	u16			max_x;
-	u16			max_y;
-	u16			max_rt;
-	unsigned long		poll_period; /* in jiffies */
-	int			fuzzx;
-	int			fuzzy;
-	int			fuzzz;
-
-	unsigned		gpio;
-	int			irq;
-
-	wait_queue_head_t	wait;
-	bool			stopped;
+#include "tsc2007.h"
 
-	int			(*get_pendown_state)(struct device *);
-	void			(*clear_penirq)(void);
-};
 
-static inline int tsc2007_xfer(struct tsc2007 *tsc, u8 cmd)
+int tsc2007_xfer(struct tsc2007 *tsc, u8 cmd)
 {
 	s32 data;
 	u16 val;

@@ -137,7 +69,7 @@ static void tsc2007_read_values(struct tsc2007 *tsc, struct ts_event *tc)
 	tsc2007_xfer(tsc, PWRDOWN);
 }
 
-static u32 tsc2007_calculate_resistance(struct tsc2007 *tsc,
+u32 tsc2007_calculate_resistance(struct tsc2007 *tsc,
 					struct ts_event *tc)
 {
 	u32 rt = 0;

@@ -158,7 +90,7 @@ static u32 tsc2007_calculate_resistance(struct tsc2007 *tsc,
 	return rt;
 }
 
-static bool tsc2007_is_pen_down(struct tsc2007 *ts)
+bool tsc2007_is_pen_down(struct tsc2007 *ts)
 {
 	/*
 	 * NOTE: We can't rely on the pressure to determine the pen down

@@ -191,7 +123,10 @@ static irqreturn_t tsc2007_soft_irq(int irq, void *handle)
 	while (!ts->stopped && tsc2007_is_pen_down(ts)) {
 
 		/* pen is down, continue with the measurement */
+
+		mutex_lock(&ts->mlock);
 		tsc2007_read_values(ts, &tc);
+		mutex_unlock(&ts->mlock);
 
 		rt = tsc2007_calculate_resistance(ts, &tc);
 

@@ -441,7 +376,8 @@ static void tsc2007_call_exit_platform_hw(void *data)
 static int tsc2007_probe(struct i2c_client *client,
 			 const struct i2c_device_id *id)
 {
-	const struct tsc2007_platform_data *pdata = dev_get_platdata(&client->dev);
+	const struct tsc2007_platform_data *pdata =
+		dev_get_platdata(&client->dev);
 	struct tsc2007 *ts;
 	struct input_dev *input_dev;
 	int err;

@@ -463,7 +399,9 @@ static int tsc2007_probe(struct i2c_client *client,
 	ts->client = client;
 	ts->irq = client->irq;
 	ts->input = input_dev;
+
 	init_waitqueue_head(&ts->wait);
+	mutex_init(&ts->mlock);
 
 	snprintf(ts->phys, sizeof(ts->phys),
 		 "%s/input0", dev_name(&client->dev));

@@ -534,7 +472,7 @@ static int tsc2007_probe(struct i2c_client *client,
 	if (err < 0) {
 		dev_err(&client->dev,
 			"Failed to setup chip: %d\n", err);
-		return err;	/* usually, chip does not respond */
+		return err;	/* chip does not respond */
 	}
 
 	err = input_register_device(input_dev);

@@ -544,6 +482,14 @@ static int tsc2007_probe(struct i2c_client *client,
 		return err;
 	}
 
+	return tsc2007_iio_configure(ts);
+}
+
+static int tsc2007_remove(struct i2c_client *client)
+{
+	struct tsc2007 *ts = i2c_get_clientdata(client);
+
+	tsc2007_iio_unconfigure(ts);
 	return 0;
 }
 

@@ -569,6 +515,7 @@ static struct i2c_driver tsc2007_driver = {
 	},
 	.id_table	= tsc2007_idtable,
 	.probe		= tsc2007_probe,
+	.remove		= tsc2007_remove,
 };
 
 module_i2c_driver(tsc2007_driver);

drivers/input/touchscreen/tsc2007_iio.c new file +140 -0

new file mode 100644
index 0000000..ed79944
@@ -0,0 +1,150 @@
+/*
+ * Copyright (c) 2016 Golden Delicious Comp. GmbH&Co. KG
+ *	Nikolaus Schaller <[email protected]>
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ */
+
+#include <linux/i2c.h>
+#include <linux/iio/iio.h>
+#include "tsc2007.h"
+
+struct tsc2007_iio {
+	struct tsc2007 *ts;
+};
+
+#define TSC2007_CHAN_IIO(_chan, _name, _type, _chan_info) \
+{ \
+	.datasheet_name = _name, \
+	.type = _type, \
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |	\
+			BIT(_chan_info), \
+	.indexed = 1, \
+	.channel = _chan, \
+}
+
+static const struct iio_chan_spec tsc2007_iio_channel[] = {
+	TSC2007_CHAN_IIO(0, "x", IIO_VOLTAGE, IIO_CHAN_INFO_RAW),
+	TSC2007_CHAN_IIO(1, "y", IIO_VOLTAGE, IIO_CHAN_INFO_RAW),
+	TSC2007_CHAN_IIO(2, "z1", IIO_VOLTAGE, IIO_CHAN_INFO_RAW),
+	TSC2007_CHAN_IIO(3, "z2", IIO_VOLTAGE, IIO_CHAN_INFO_RAW),
+	TSC2007_CHAN_IIO(4, "adc", IIO_VOLTAGE, IIO_CHAN_INFO_RAW),
+	TSC2007_CHAN_IIO(5, "rt", IIO_VOLTAGE, IIO_CHAN_INFO_RAW), /* Ohms? */
+	TSC2007_CHAN_IIO(6, "pen", IIO_PRESSURE, IIO_CHAN_INFO_RAW),
+	TSC2007_CHAN_IIO(7, "temp0", IIO_TEMP, IIO_CHAN_INFO_RAW),
+	TSC2007_CHAN_IIO(8, "temp1", IIO_TEMP, IIO_CHAN_INFO_RAW),
+};
+
+static int tsc2007_read_raw(struct iio_dev *indio_dev,
+	struct iio_chan_spec const *chan, int *val, int *val2, long mask)
+{
+	struct tsc2007_iio *iio = iio_priv(indio_dev);
+	struct tsc2007 *tsc = iio->ts;
+	int adc_chan = chan->channel;
+	int ret = 0;
+
+	if (adc_chan >= ARRAY_SIZE(tsc2007_iio_channel))
+		return -EINVAL;
+
+	if (mask != IIO_CHAN_INFO_RAW)
+		return -EINVAL;
+
+	mutex_lock(&tsc->mlock);
+
+	switch (chan->channel) {
+	case 0:
+		*val = tsc2007_xfer(tsc, READ_X);
+		break;
+	case 1:
+		*val = tsc2007_xfer(tsc, READ_Y);
+		break;
+	case 2:
+		*val = tsc2007_xfer(tsc, READ_Z1);
+		break;
+	case 3:
+		*val = tsc2007_xfer(tsc, READ_Z2);
+		break;
+	case 4:
+		*val = tsc2007_xfer(tsc, (ADC_ON_12BIT | TSC2007_MEASURE_AUX));
+		break;
+	case 5: {
+		struct ts_event tc;
+
+		tc.x = tsc2007_xfer(tsc, READ_X);
+		tc.z1 = tsc2007_xfer(tsc, READ_Z1);
+		tc.z2 = tsc2007_xfer(tsc, READ_Z2);
+		*val = tsc2007_calculate_resistance(tsc, &tc);
+		break;
+	}
+	case 6:
+		*val = tsc2007_is_pen_down(tsc);
+		break;
+	case 7:
+		*val = tsc2007_xfer(tsc,
+				    (ADC_ON_12BIT | TSC2007_MEASURE_TEMP0));
+		break;
+	case 8:
+		*val = tsc2007_xfer(tsc,
+				    (ADC_ON_12BIT | TSC2007_MEASURE_TEMP1));
+		break;
+	}
+
+	/* Prepare for next touch reading - power down ADC, enable PENIRQ */
+	tsc2007_xfer(tsc, PWRDOWN);
+
+	mutex_unlock(&tsc->mlock);
+
+	ret = IIO_VAL_INT;
+
+	return ret;
+}
+
+static const struct iio_info tsc2007_iio_info = {
+	.read_raw = tsc2007_read_raw,
+	.driver_module = THIS_MODULE,
+};
+
+int tsc2007_iio_configure(struct tsc2007 *ts)
+{
+	int err;
+	struct iio_dev *indio_dev;
+	struct tsc2007_iio *iio;
+
+	indio_dev = devm_iio_device_alloc(&ts->client->dev,
+		sizeof(struct tsc2007_iio));
+	if (!indio_dev) {
+		dev_err(&ts->client->dev, "iio_device_alloc failed\n");
+		return -ENOMEM;
+	}
+
+	iio = iio_priv(indio_dev);
+	iio->ts = ts;
+	ts->iio_dev = (void *) indio_dev;
+
+	indio_dev->name = "tsc2007";
+	indio_dev->dev.parent = &ts->client->dev;
+	indio_dev->info = &tsc2007_iio_info;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->channels = tsc2007_iio_channel;
+	indio_dev->num_channels = ARRAY_SIZE(tsc2007_iio_channel);
+
+	err = iio_device_register(indio_dev);
+	if (err < 0) {
+		dev_err(&ts->client->dev, "iio_device_register() failed: %d\n",
+			err);
+		return err;
+	}
+
+	return 0;