Re: [PATCH v6 1/4] dt-bindings: touchscreen: add touch-overlay property

[Jeff LaBundy]

Hi Javier,

This is excellent, just one small comment below.

On Wed, Dec 20, 2023 at 09:39:43AM +0100, Javier Carrasco wrote:
> The touch-overlay encompasses a number of touch areas that define a
> clipped touchscreen area and/or buttons with a specific functionality.
> 
> A clipped touchscreen area avoids getting events from regions that are
> physically hidden by overlay frames.
> 
> For touchscreens with printed overlay buttons, sub-nodes with a suitable
> key code can be defined to report key events instead of the original
> touch events.
> 
> Signed-off-by: Javier Carrasco <javier.carrasco@xxxxxxxxxxxxxx>
> ---
>  .../bindings/input/touchscreen/touchscreen.yaml    | 119 +++++++++++++++++++++
>  1 file changed, 119 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml b/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml
> index 431c13335c40..d5ac90667bef 100644
> --- a/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml
> +++ b/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml
> @@ -87,6 +87,125 @@ properties:
>    touchscreen-y-plate-ohms:
>      description: Resistance of the Y-plate in Ohms
>  
> +  touch-overlay:
> +    description: list of nodes defining segments (touch areas) on the
> +      touchscreen.
> +
> +      This object can be used to describe a series of segments to
> +      restrict the region within touch events are reported or buttons
> +      with a specific functionality.
> +
> +      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. Printed buttons on that overlay are also a typical
> +      use case.
> +
> +      A new touchscreen area is defined as a sub-node without a key code. If a
> +      key code is defined in the sub-node, it will be interpreted as a button.
> +
> +      The x-origin and y-origin properties of a touchscreen area 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 touchscreen effective area.
> +
> +      The following example shows a new touchscreen area with the new origin
> +      (0',0') for the touch events generated by the device.
> +
> +                   Touchscreen (full area)
> +         ┌────────────────────────────────────────┐
> +         │    ┌───────────────────────────────┐   │
> +         │    │                               │   │
> +         │    ├ y-size                        │   │
> +         │    │                               │   │
> +         │    │       touchscreen area        │   │
> +         │    │         (no key code)         │   │
> +         │    │                               │   │
> +         │    │            x-size             │   │
> +         │   ┌└──────────────┴────────────────┘   │
> +         │(0',0')                                 │
> +        ┌└────────────────────────────────────────┘
> +      (0,0)
> +
> +     where (0',0') = (0+x-origin,0+y-origin)
> +
> +      Sub-nodes with key codes report the touch events on their surface as key
> +      events instead.
> +
> +      The following example shows a touchscreen with a single button on it.
> +
> +              Touchscreen (full area)
> +        ┌───────────────────────────────────┐
> +        │                                   │
> +        │                                   │
> +        │   ┌─────────┐                     │
> +        │   │button 0 │                     │
> +        │   │KEY_POWER│                     │
> +        │   └─────────┘                     │
> +        │                                   │
> +        │                                   │
> +       ┌└───────────────────────────────────┘
> +     (0,0)
> +
> +      Segments defining buttons and clipped toushcreen areas can be combined
> +      as shown in the following example.
> +      In that case only the events within the touchscreen area are reported
> +      as touch events. Events within the button areas report their associated
> +      key code. Any events outside the defined areas are ignored.
> +
> +                  Touchscreen (full area)
> +        ┌─────────┬──────────────────────────────┐
> +        │         │                              │
> +        │         │    ┌───────────────────────┐ │
> +        │ button 0│    │                       │ │
> +        │KEY_POWER│    │                       │ │
> +        │         │    │                       │ │
> +        ├─────────┤    │   touchscreen area    │ │
> +        │         │    │     (no key code)     │ │
> +        │         │    │                       │ │
> +        │ button 1│    │                       │ │
> +        │ KEY_INFO│   ┌└───────────────────────┘ │
> +        │         │(0',0')                       │
> +       ┌└─────────┴──────────────────────────────┘
> +     (0,0)
> +
> +    type: object
> +
> +    patternProperties:
> +      '^segment-':
> +        type: object
> +        description:
> +          Each segment is represented as a sub-node.
> +        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
> +
> +          label:
> +            $ref: /schemas/types.yaml#/definitions/string
> +            description: descriptive name of the button

Please consider replacing the word "button" with "segment"; as we see in
patch [3/4], the label can describe a touch surface and not just a button.

> +
> +          linux,code: true
> +
> +        required:
> +          - x-origin
> +          - y-origin
> +          - x-size
> +          - y-size
> +
> +        unevaluatedProperties: false
> +
>  dependencies:
>    touchscreen-size-x: [ touchscreen-size-y ]
>    touchscreen-size-y: [ touchscreen-size-x ]
> 
> -- 
> 2.39.2
> 
> 

With that changed, feel free to add: 

Reviewed-by: Jeff LaBundy <jeff@xxxxxxxxxxx>

Obviously we need Dmitry and the binding maintainers to review as well; I am
merely expressing my own agreement as a future customer of this function.

Kind regards,
Jeff LaBundy