Hi Sebastian,

Am 19.02.2017 um 00:44 schrieb Sebastian Reichel [off-list ref]:

Hi,

On Sat, Feb 18, 2017 at 12:33:34PM +0100, H. Nikolaus Schaller wrote:

quoted

Hi Sebastian,

quoted

Am 18.02.2017 um 04:22 schrieb Sebastian Reichel [off-list ref]:

Hi,

On Fri, Feb 17, 2017 at 12:40:41PM -0800, Dmitry Torokhov wrote:

quoted

quoted

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

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

Yes, I'm aware.

quoted

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.

Ok!

quoted

quoted

$ 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.

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.

That looks like a good strategy.

quoted

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

quoted

quoted

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

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.

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

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

agreed.

quoted

quoted

quoted

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.

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)

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. So it was included
but we never thought about it being something as important as the pre-calibration
which is really a benefit for setup and immediate useability of the whole system.
Hence we buried it a little in this pre-calibration, flipping and rotation patch.

quoted

quoted

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).

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

quoted

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.

BR,
Nikolaus