| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <ZTp72LUXxr+Z9gn8@nixie71>
Date: Thu, 26 Oct 2023 09:46:48 -0500
From: Jeff LaBundy <jeff@...undy.com>
To: Javier Carrasco <javier.carrasco@...fvision.net>
Cc: Dmitry Torokhov <dmitry.torokhov@...il.com>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Conor Dooley <conor+dt@...nel.org>,
Henrik Rydberg <rydberg@...math.org>,
Bastian Hecht <hechtb@...il.com>,
Michael Riesch <michael.riesch@...fvision.net>,
linux-kernel@...r.kernel.org, linux-input@...r.kernel.org,
devicetree@...r.kernel.org
Subject: Re: [PATCH v5 2/4] dt-bindings: touchscreen: add overlay-touchscreen
and overlay-buttons properties
Hi Javier,
Thank you for continuing to drive this high-quality work.
On Tue, Oct 17, 2023 at 01:00:08PM +0200, Javier Carrasco wrote:
> The overlay-touchscreen object defines an area within the touchscreen
> where touch events are reported and their coordinates get converted to
> the overlay origin. This object avoids getting events from areas that
> are physically hidden by overlay frames.
>
> For touchscreens where overlay buttons on the touchscreen surface are
> provided, the overlay-buttons object contains a node for every button
> and the key event that should be reported when pressed.
>
> Signed-off-by: Javier Carrasco <javier.carrasco@...fvision.net>
> ---
> .../bindings/input/touchscreen/touchscreen.yaml | 143 +++++++++++++++++++++
> 1 file changed, 143 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml b/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml
> index 431c13335c40..5c58eb79ee9a 100644
> --- a/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml
> +++ b/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml
> @@ -87,6 +87,129 @@ properties:
> touchscreen-y-plate-ohms:
> description: Resistance of the Y-plate in Ohms
>
> + overlay-touchscreen:
> + description: Clipped touchscreen area
> +
> + This object can be used to describe a frame that restricts the area
> + within touch events are reported, ignoring the events that occur outside
> + this area. This is of special interest if the touchscreen is shipped
> + with a physical overlay on top of it with a frame that hides some part
> + of the original touchscreen area.
> +
> + The x-origin and y-origin properties of this object define the offset of
> + a new origin from where the touchscreen events are referenced.
> + This offset is applied to the events accordingly. The x-size and y-size
> + properties define the size of the overlay-touchscreen (effective area).
> +
> + The following example shows the new touchscreen area and the new origin
> + (0',0') for the touch events generated by the device.
> +
> + Touchscreen (full area)
> + ┌────────────────────────────────────────┐
> + │ ┌───────────────────────────────┐ │
> + │ │ │ │
> + │ ├ y-size │ │
> + │ │ │ │
> + │ │ overlay-touchscreen │ │
> + │ │ │ │
> + │ │ │ │
> + │ │ x-size │ │
> + │ ┌└──────────────┴────────────────┘ │
> + │(0',0') │
> + ┌└────────────────────────────────────────┘
> + (0,0)
> +
> + where (0',0') = (0+x-origin,0+y-origin)
> +
> + type: object
> + $ref: '#/$defs/overlay-node'
> + unevaluatedProperties: false
> +
> + required:
> + - x-origin
> + - y-origin
> + - x-size
> + - y-size
> +
> + overlay-buttons:
> + description: list of nodes defining the buttons on the touchscreen
> +
> + This object can be used to describe buttons on the touchscreen area,
> + reporting the touch events on their surface as key events instead of
> + the original touch events.
> +
> + This is of special interest if the touchscreen is shipped with a
> + physical overlay on top of it where a number of buttons with some
> + predefined functionality are printed. In that case a specific behavior
> + is expected from those buttons instead of raw touch events.
> +
> + The overlay-buttons properties define a per-button area as well as an
> + origin relative to the real touchscreen origin. Touch events within the
> + button area are reported as the key event defined in the linux,code
> + property. Given that the key events do not provide coordinates, the
> + button origin is only used to place the button area on the touchscreen
> + surface. Any event outside the overlay-buttons object is reported as a
> + touch event with no coordinate transformation.
> +
> + The following example shows a touchscreen with a single button on it
> +
> + Touchscreen (full area)
> + ┌───────────────────────────────────┐
> + │ │
> + │ │
> + │ ┌─────────┐ │
> + │ │button 0 │ │
> + │ │KEY_POWER│ │
> + │ └─────────┘ │
> + │ │
> + │ │
> + ┌└───────────────────────────────────┘
> + (0,0)
> +
> + The overlay-buttons object can be combined with the overlay-touchscreen
> + object as shown in the following example. In that case only the events
> + within the overlay-touchscreen object are reported as touch events.
> +
> + Touchscreen (full area)
> + ┌─────────┬──────────────────────────────┐
> + │ │ │
> + │ │ ┌───────────────────────┐ │
> + │ button 0│ │ │ │
> + │KEY_POWER│ │ │ │
> + │ │ │ │ │
> + ├─────────┤ │ overlay-touchscreen │ │
> + │ │ │ │ │
> + │ │ │ │ │
> + │ button 1│ │ │ │
> + │ KEY_INFO│ ┌└───────────────────────┘ │
> + │ │(0',0') │
> + ┌└─────────┴──────────────────────────────┘
> + (0,0)
> +
> + type: object
I am still confused why the buttons need to live under an 'overlay-buttons'
parent node, which seems like an imaginary boundary. In my view, the touch
surface comprises the following types of rectangular areas:
1. A touchscreen, wherein granular coordinates and pressure are reported.
2. A momentary button, wherein pressure is quantized into a binary value
(press or release), and coordinates are ignored.
Any contact that falls outside of (1) and (2) is presumed to be part of a
border or matting, and is hence ignored.
Areas (1) and (2) exist in the same "plane", so why can they not reside
under the same parent node? The following seems much more representative
of the actual hardware we intend to describe in the device tree:
touchscreen {
compatible = "...";
reg = <...>;
/* raw coordinates reported here */
touch-area-1 {
x-origin = <...>;
y-origin = <...>;
x-size = <...>;
y-size = <...>;
};
/* a button */
touch-area-2a {
x-origin = <...>;
y-origin = <...>;
x-size = <...>;
y-size = <...>;
linux,code = <KEY_POWER>;
};
/* another button */
touch-area-2b {
x-origin = <...>;
y-origin = <...>;
x-size = <...>;
y-size = <...>;
linux,code = <KEY_INFO>;
};
};
With this method, the driver merely stores a list head. The parsing code
then walks the client device node; for each touch* child encountered, it
allocates memory for a structure of five members, and adds it to the list.
The event handling code then simply iterates through the list and checks
if the coordinates reported by the hardware fall within each rectangle. If
so, and the keycode in the list element is equal to KEY_RESERVED (zero),
we assume the rectangle is of type (1); the coordinates are passed to
touchscreen_report_pos() and the pressure is reported as well.
If the keycode is not equal to KEY_RESERVED (e.g. KEY_POWER), we assume
the rectangle is of type (2); input_report_key() is called instead and
the coordinates are ignored altogether.
Instead, the existing implementation has two separate structures, one of
which inherits the other. Its corresponding properties are then parsed in
a separate function to account for the fact that the two structures exist
at different layers in the heirarchy.
My argument is that all nodes can be parsed in the same way, without having
to understand whether they represent case (1) or (2). The existing method
also has to count the nodes; this would not be necessary with a linked list.
Can you help me understand why the buttons must be "wrapped" in their own
node? As I mention in v2, this isn't a deal breaker necessarily, but I'd
like to understand the reasoning behind what I interpret as additional
complexity, and a degree of separation from a natural description of the
touch surface.
The only reason I can think to insert the 'overlay-buttons' parent is in
case we want to specify a property within this node that applies to all
buttons, but this does not exist in the current implementation, nor do I
see a compelling reason to do so in the future.
> +
> + patternProperties:
> + '^button-':
> + type: object
> + description:
> + Each button (key) is represented as a sub-node.
> + $ref: '#/$defs/overlay-node'
> + unevaluatedProperties: false
> +
> + properties:
> + label:
> + $ref: /schemas/types.yaml#/definitions/string
> + description: descriptive name of the button
> +
> + linux,code: true
> +
> + required:
> + - linux,code
> + - x-origin
> + - y-origin
> + - x-size
> + - y-size
> +
> dependencies:
> touchscreen-size-x: [ touchscreen-size-y ]
> touchscreen-size-y: [ touchscreen-size-x ]
> @@ -94,3 +217,23 @@ dependencies:
> touchscreen-y-mm: [ touchscreen-x-mm ]
>
> additionalProperties: true
> +
> +$defs:
> + overlay-node:
> + type: object
> + properties:
> + x-origin:
> + description: horizontal origin of the node area
> + $ref: /schemas/types.yaml#/definitions/uint32
> +
> + y-origin:
> + description: vertical origin of the node area
> + $ref: /schemas/types.yaml#/definitions/uint32
> +
> + x-size:
> + description: horizontal resolution of the node area
> + $ref: /schemas/types.yaml#/definitions/uint32
> +
> + y-size:
> + description: vertical resolution of the node area
> + $ref: /schemas/types.yaml#/definitions/uint32
>
> --
> 2.39.2
>
Kind regards,
Jeff LaBundy
Powered by blists - more mailing lists