Thread (94 messages) 94 messages, 11 authors, 2017-02-23

Re: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding names, pre-calibration, flipping and rotation

From: H. Nikolaus Schaller <hidden>
Date: 2017-02-18 11:33:54
Also in: linux-iio, linux-input, linux-omap, lkml

Hi Sebastian,
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. Hence the N900 is not influenced by this patch series.

If it has a similar issue, it should be fixed of course.
$ 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
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
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.
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.
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
BR and thanks,
Nikolaus

Attachments

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help