Re: [PATCH 01/10] dt-bindings: input: touchscreen: stmpe: add touch window configuration

From: "Middelschulte, Leif" <Leif.Middelschulte@klsmartin.com>
To: "robh@kernel.org" <robh@kernel.org>
Cc: "dmitry.torokhov@gmail.com" <dmitry.torokhov@gmail.com>,
	"linux-input@vger.kernel.org" <linux-input@vger.kernel.org>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"mark.rutland@arm.com" <mark.rutland@arm.com>
Subject: Re: [PATCH 01/10] dt-bindings: input: touchscreen: stmpe: add touch window configuration
Date: Mon, 24 Jun 2019 10:33:12 +0000	[thread overview]
Message-ID: <8b9d362f16b817e828b774c6ac6532c6f2efb821.camel@klsmartin.com> (raw)
In-Reply-To: <CAL_JsqLrMk9yNAzHWCgOFaOqnHKiwSjLOfbhU5+DzLCNBD+H_w@mail.gmail.com>

Am Mittwoch, den 19.06.2019, 07:51 -0600 schrieb Rob Herring:
> On Wed, Jun 19, 2019 at 5:56 AM Middelschulte, Leif
> <
> Leif.Middelschulte@klsmartin.com
> > wrote:
> > Am Freitag, den 14.06.2019, 13:48 -0600 schrieb Rob Herring:
> > > On Mon, May 27, 2019 at 06:07:27PM +0200, Leif Middelschulte wrote:
> > > > The STMPE811 is able to only monitor a subwindow of the
> > > > entire screen. Touches outside of this window are neglected.
> > > > This change adds the corresponding documentation.
> > > 
> > > Unless you are trying to just filter out the edge values
> > 
> > I'm trying to "just filter out the edge values" by filtering the values
> > right on the smpte microcontroller. It is a device specific feature, that is
> > why I implemented and documented the support.
> > 
> > But sure, one could leave that to a corresponding entry in the hwdb that
> > would manipulate absinfo to gain calibrated input [1].
> > 
> > > which I think we already have properties for,
> > 
> > While there [0] is "touchscreen-min-{x,y}", I could not spot a corresponding
> > "touchscreen-max-{x,y}". Could you point me to the right property?
> 
> Perhaps add them. If we have min, adding a max seems logical.
There are `touchscreen-size-x` and `touchscreen-size-y`, which are described[0] as
"horizontal resolution of touchscreen (maximum x coordinate reported + 1)"

The potentially misleading property name aside, the general
implementation in of_touchscreen.c[1] applies it as maximum value.
It is not applied as resolution as in the context of input devices[2].

So here are the options:
 A) I go with `touchscreen-min-{x,y}` for minimum values and
    `touchscreen-size-{x,y}` for maximum values.
 B) Deprecate `touchscreen-size-{x,y}` for named reasons and
    introduce `touchscreen-max-{x,y}`.

In my case, I do need to set the resolution too though.

So, another question would be how to go about handling the resolution:
 I) Introducing an explicit resolution tuple? This would be bad
    because of multiple invalid constelations.
 II) Implicitly applying a resolution if a corrsponding
     `touchscreen-{x,y}-mm` is provided too. This approach would change
     behavior though.

[0] https://www.kernel.org/doc/Documentation/devicetree/bindings/input/touchscreen/touchscreen.txt
[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/input/touchscreen/of_touchscreen.c?h=v5.2-rc6#n81
[2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/uapi/linux/input.h#n95

Leif

> 
> Rob
> 