[01/10] dt-bindings: input: touchscreen: stmpe: add touch window configuration
diff mbox series

Message ID 20190527160736.30569-2-leif.middelschulte@klsmartin.com
State Changes Requested
Headers show
Series
  • input: touchscreen: stmpe: ext. features
Related show

Checks

Context Check Description
robh/checkpatch success

Commit Message

Middelschulte, Leif May 27, 2019, 4:07 p.m. UTC
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.

Signed-off-by: Leif Middelschulte <leif.middelschulte@klsmartin.com>
---
 .../devicetree/bindings/input/touchscreen/stmpe.txt      | 9 +++++++++
 1 file changed, 9 insertions(+)

Comments

Rob Herring June 14, 2019, 7:48 p.m. UTC | #1
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 which I think 
we already have properties for, wouldn't this be something the user 
may want to adjust/move at runtime?

> 
> Signed-off-by: Leif Middelschulte <leif.middelschulte@klsmartin.com>
> ---
>  .../devicetree/bindings/input/touchscreen/stmpe.txt      | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/stmpe.txt b/Documentation/devicetree/bindings/input/touchscreen/stmpe.txt
> index c549924603d2..04962e2e1ad0 100644
> --- a/Documentation/devicetree/bindings/input/touchscreen/stmpe.txt
> +++ b/Documentation/devicetree/bindings/input/touchscreen/stmpe.txt
> @@ -33,6 +33,13 @@ Optional properties:
>  - st,i-drive		: current limit value of the touchscreen drivers
>  				0 -> 20 mA (typical 35mA max)
>  				1 -> 50 mA (typical 80 mA max)
> +- st,window-tracking	: An u12 array that allows to pre-set a sub-window in the touchscreen.

u16?

> +				Any touch position that is outside the sub-window is discarded
> +				(top-right x, top-right y, bottom-left x, bottom-left y).
> +				The given window also affects the value range (limits) set
> +				within the input subsystem which, in turn, affects
> +				the input resolution calculation on both axes.
> +
>  
>  Optional properties common with MFD (deprecated):
>   - st,sample-time	: ADC conversion time in number of clock.
> @@ -101,6 +108,8 @@ stmpe811@41 {
>  		 * current limit value
>  		 */
>  		st,i-drive = <1>;
> +		/* enable entire window */
> +		st,window-tracking = /bits/ 16 <4095 4095 0 0>;
>  	};
>  	stmpe_adc {
>  		compatible = "st,stmpe-adc";
> -- 
> 2.21.0
>
Middelschulte, Leif June 19, 2019, 11:55 a.m. UTC | #2
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?

> wouldn't this be something the user may want to adjust/move at runtime?
It is adjustable at runtime via the hwdb or calling ioctl on the evdev device directly.


[0] https://www.kernel.org/doc/Documentation/devicetree/bindings/input/touchscreen/touchscreen.txt
[1] https://who-t.blogspot.com/2018/08/how-60-evdevhwdb-works.html
> 
> > Signed-off-by: Leif Middelschulte <
> > leif.middelschulte@klsmartin.com
> > >
> > ---
> >  .../devicetree/bindings/input/touchscreen/stmpe.txt      | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/input/touchscreen/stmpe.txt b/Documentation/devicetree/bindings/input/touchscreen/stmpe.txt
> > index c549924603d2..04962e2e1ad0 100644
> > --- a/Documentation/devicetree/bindings/input/touchscreen/stmpe.txt
> > +++ b/Documentation/devicetree/bindings/input/touchscreen/stmpe.txt
> > @@ -33,6 +33,13 @@ Optional properties:
> >  - st,i-drive		: current limit value of the touchscreen drivers
> >  				0 -> 20 mA (typical 35mA max)
> >  				1 -> 50 mA (typical 80 mA max)
> > +- st,window-tracking	: An u12 array that allows to pre-set a sub-window in the touchscreen.
> 
> u16?
> 
> > +				Any touch position that is outside the sub-window is discarded
> > +				(top-right x, top-right y, bottom-left x, bottom-left y).
> > +				The given window also affects the value range (limits) set
> > +				within the input subsystem which, in turn, affects
> > +				the input resolution calculation on both axes.
> > +
> >  
> >  Optional properties common with MFD (deprecated):
> >   - st,sample-time	: ADC conversion time in number of clock.
> > @@ -101,6 +108,8 @@ stmpe811@41 {
> >  		 * current limit value
> >  		 */
> >  		st,i-drive = <1>;
> > +		/* enable entire window */
> > +		st,window-tracking = /bits/ 16 <4095 4095 0 0>;
> >  	};
> >  	stmpe_adc {
> >  		compatible = "st,stmpe-adc";
> > -- 
> > 2.21.0
> >
Rob Herring June 19, 2019, 1:51 p.m. UTC | #3
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.

Rob
Middelschulte, Leif June 24, 2019, 10:33 a.m. UTC | #4
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
>

Patch
diff mbox series

diff --git a/Documentation/devicetree/bindings/input/touchscreen/stmpe.txt b/Documentation/devicetree/bindings/input/touchscreen/stmpe.txt
index c549924603d2..04962e2e1ad0 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/stmpe.txt
+++ b/Documentation/devicetree/bindings/input/touchscreen/stmpe.txt
@@ -33,6 +33,13 @@  Optional properties:
 - st,i-drive		: current limit value of the touchscreen drivers
 				0 -> 20 mA (typical 35mA max)
 				1 -> 50 mA (typical 80 mA max)
+- st,window-tracking	: An u12 array that allows to pre-set a sub-window in the touchscreen.
+				Any touch position that is outside the sub-window is discarded
+				(top-right x, top-right y, bottom-left x, bottom-left y).
+				The given window also affects the value range (limits) set
+				within the input subsystem which, in turn, affects
+				the input resolution calculation on both axes.
+
 
 Optional properties common with MFD (deprecated):
  - st,sample-time	: ADC conversion time in number of clock.
@@ -101,6 +108,8 @@  stmpe811@41 {
 		 * current limit value
 		 */
 		st,i-drive = <1>;
+		/* enable entire window */
+		st,window-tracking = /bits/ 16 <4095 4095 0 0>;
 	};
 	stmpe_adc {
 		compatible = "st,stmpe-adc";