Hi, On Sat, Feb 18, 2017 at 12:33:34PM +0100, H. Nikolaus Schaller wrote: > Hi Sebastian, > > > Am 18.02.2017 um 04:22 schrieb Sebastian Reichel <sre@xxxxxxxxxx>: > > > > Hi, > > > > On Fri, Feb 17, 2017 at 12:40:41PM -0800, Dmitry Torokhov wrote: > >>> 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. > 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. > > $ 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. > Is any user/maintainer of these devices following this discussion and can comment? > > > > >> 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. > >> > >> > >>> 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 > >> 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 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). > > 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
Attachment:
signature.asc
Description: PGP signature
