diff mbox series

[1/2] dt-bindings: input: iqs7222: Add properties for Azoteq IQS7222D

Message ID ZHanv+8fOYhpyMEC@nixie71
State New
Headers show
Series [1/2] dt-bindings: input: iqs7222: Add properties for Azoteq IQS7222D | expand

Commit Message

Jeff LaBundy May 31, 2023, 1:49 a.m. UTC
Extend the common binding to include a new variant of the silicon.

Signed-off-by: Jeff LaBundy <jeff@labundy.com>
---
 .../bindings/input/azoteq,iqs7222.yaml        | 202 +++++++++++++++++-
 1 file changed, 192 insertions(+), 10 deletions(-)

Comments

Jeff LaBundy June 12, 2023, 12:29 a.m. UTC | #1
Hi Krzysztof,

On Fri, Jun 02, 2023 at 09:21:00AM +0200, Krzysztof Kozlowski wrote:
> On 01/06/2023 03:23, Jeff LaBundy wrote:
> >>>  
> >>>    reg:
> >>>      maxItems: 1
> >>> @@ -173,6 +174,148 @@ properties:
> >>>      maximum: 3000
> >>>      description: Specifies the report rate (in ms) during ultra-low-power mode.
> >>>  
> >>> +  touchscreen-size-x: true
> >>> +  touchscreen-size-y: true
> >>> +  touchscreen-inverted-x: true
> >>> +  touchscreen-inverted-y: true
> >>> +  touchscreen-swapped-x-y: true
> >>
> >> Why? Aren't they coming from common schema?
> > 
> > Yes, but because additionalProperties is set to false here, we must explicitly
> > include the subset of properties from the common schema that are allowed for
> > this particular instance. I counted over a dozen other bindings doing the same.
> > 
> > In case I have misunderstood, please let me know.
> 
> If you are listing now most of touchscreen properties, it is a sign you
> should use just unevaluatedProperties: false (instead
> additionalProperties) and then no need for any of these here.

At present, touchscreen.yaml offers 17 properties. I am only replicating 5
of them, so this binding may not be such a case for this idea. That said, I
tried your suggestion locally.

In the present implementation (additionalProperties: false), the tooling
rightfully presents an error if the example includes a property that is not
explicitly offered (e.g. 'touchscreen-x-plate-ohms').

If I change the binding to use "unevaluatedProperties: false", the tooling
accepts an example which includes this same property that is not supported
by this particular hardware. This seems like a regression.

In my humble opinion, the present implementation is optimal. It is for that
reason we see multiple bindings with this same block of inherited properties.
I will leave this unchanged; please let me know in case I have misunderstood.

> 
> > 
> >>
> >>> +
> >>> +  trackpad:
> >>> +    type: object
> >>> +    description: Represents all channels associated with the trackpad.
> >>> +
> >>> +    properties:
> >>> +      azoteq,channel-select:
> >>> +        $ref: /schemas/types.yaml#/definitions/uint32-array
> >>> +        minItems: 1
> >>> +        maxItems: 12
> >>> +        items:
> >>> +          minimum: 0
> >>> +          maximum: 13
> >>> +        description:
> >>> +          Specifies the order of the channels that participate in the trackpad.
> >>> +          Specify 255 to omit a given channel for the purpose of mapping a non-
> >>> +          rectangular trackpad.
> >>> +
> >>> +      azoteq,num-rows:
> >>> +        $ref: /schemas/types.yaml#/definitions/uint32
> >>> +        minimum: 1
> >>> +        maximum: 12
> >>> +        description: Specifies the number of rows that comprise the trackpad.
> >>> +
> >>> +      azoteq,num-cols:
> >>> +        $ref: /schemas/types.yaml#/definitions/uint32
> >>> +        minimum: 1
> >>> +        maximum: 12
> >>> +        description: Specifies the number of columns that comprise the trackpad.
> >>> +
> >>> +      azoteq,top-speed:
> >>> +        $ref: /schemas/types.yaml#/definitions/uint32
> >>> +        multipleOf: 4
> >>> +        minimum: 0
> >>> +        maximum: 1020
> >>> +        description:
> >>> +          Specifies the speed of movement after which coordinate filtering is
> >>> +          no longer applied.
> >>
> >> Units?
> > 
> > This is a ratiometric, i.e. unitless value that represents a hardware filter
> > coefficient. It already exists in this binding prior to this patch under the
> > slider-0/1 node and is simply re-used here.
> 
> Then mention the ratio (e.g. "speed of movement expressed as ratio
> of..."). Description said "speed" and we usually measure speed in very
> specific units.

ACK; this is a good call-out. Please note that the existing binding uses the
same language elsewhere; I will mirror the updated language to the existing
instances in a separate patch.

> 
> > 
> >>
> >>> +
> >>> +      azoteq,bottom-speed:
> >>> +        $ref: /schemas/types.yaml#/definitions/uint32
> >>> +        minimum: 0
> >>> +        maximum: 255
> >>> +        description:
> >>> +          Specifies the speed of movement after which coordinate filtering is
> >>> +          linearly reduced.
> >>
> >> Units?
> > 
> > Same here.
> > 
> >>
> >>> +
> >>> +      azoteq,use-prox:
> >>> +        type: boolean
> >>> +        description:
> >>> +          Directs the trackpad to respond to the proximity states of the se-
> >>> +          lected channels instead of their corresponding touch states. Note
> >>
> >> Don't split the words.
> > 
> > ACK.
> > 
> >>
> >>> +          the trackpad cannot report granular coordinates during a state of
> >>> +          proximity.
> >>> +
> >>> +    patternProperties:
> >>> +      "^azoteq,lower-cal-(x|y)$":
> >>> +        $ref: /schemas/types.yaml#/definitions/uint32
> >>> +        minimum: 0
> >>> +        maximum: 255
> >>> +        description: Specifies the trackpad's lower starting points.
> >>
> >> Why would you need this property? Why does this represent hardware property?
> > 
> > This property and its cousin below define the physical boundaries of the
> > touch surface. They are typically used to mask areas that cannot elicit
> > an electrical response due to manufacturing tolerances or the presence of
> > an overlay. For that reason, they descend directly from properties of the
> > hardware.
> > 
> > Similar properties already exist in this binding for the slider case; this
> > device simply extends the functionality to a second dimenstion.
> 
> OK
> 
> > 
> >>
> >>> +
> >>> +      "^azoteq,upper-cal-(x|y)$":
> >>> +        $ref: /schemas/types.yaml#/definitions/uint32
> >>> +        minimum: 0
> >>> +        maximum: 255
> >>> +        description: Specifies the trackpad's upper starting points.
> >>> +
> >>> +      "^event-(press|tap|(swipe|flick)-(x|y)-(pos|neg))$":
> >>> +        type: object
> >>> +        $ref: input.yaml#
> >>> +        description:
> >>> +          Represents a press or gesture event reported by the trackpad. Specify
> >>> +          'linux,code' under the press event to report absolute coordinates.
> >>> +
> >>> +        properties:
> >>> +          linux,code: true
> >>> +
> >>> +          azoteq,gesture-angle-tighten:
> >>> +            type: boolean
> >>> +            description:
> >>> +              Limits the tangent of the gesture angle to 0.5 (axial gestures
> >>> +              only). If specified in one direction, the effect is applied in
> >>> +              either direction.
> >>> +
> >>> +          azoteq,gesture-max-ms:
> >>> +            multipleOf: 16
> >>> +            minimum: 0
> >>> +            maximum: 4080
> >>> +            description:
> >>> +              Specifies the length of time (in ms) within which a tap, swipe
> >>> +              or flick gesture must be completed in order to be acknowledged
> >>> +              by the device. The number specified for any one swipe or flick
> >>> +              gesture applies to all other swipe or flick gestures.
> >>> +
> >>> +          azoteq,gesture-min-ms:
> >>> +            multipleOf: 16
> >>> +            minimum: 0
> >>> +            maximum: 4080
> >>> +            description:
> >>> +              Specifies the length of time (in ms) for which a tap gesture must
> >>> +              be held in order to be acknowledged by the device.
> >>> +
> >>> +          azoteq,gesture-dist:
> >>> +            $ref: /schemas/types.yaml#/definitions/uint32
> >>> +            minimum: 0
> >>> +            maximum: 65535
> >>> +            description:
> >>> +              Specifies the distance across which a tap, swipe or flick gesture
> >>> +              must travel in order to be acknowledged by the device. The number
> >>> +              specified for any one swipe or flick gesture applies to all other
> >>> +              swipe or flick gestures.
> >>> +
> >>> +          azoteq,gpio-select:
> >>> +            $ref: /schemas/types.yaml#/definitions/uint32-array
> >>> +            minItems: 1
> >>> +            maxItems: 3
> >>> +            items:
> >>> +              minimum: 0
> >>> +              maximum: 2
> >>> +            description: |
> >>> +              Specifies one or more GPIO mapped to the event as follows:
> >>> +              0: GPIO0
> >>> +              1: GPIO3
> >>> +              2: GPIO4
> >>> +
> >>> +              Note that although multiple events can be mapped to a single
> >>> +              GPIO, they must all be of the same type (proximity, touch or
> >>> +              trackpad gesture).
> >>> +
> >>> +        additionalProperties: false
> >>> +
> >>> +    required:
> >>> +      - azoteq,channel-select
> >>> +
> >>> +    additionalProperties: false
> >>> +
> >>>  patternProperties:
> >>>    "^cycle-[0-9]$":
> >>>      type: object
> >>> @@ -288,6 +431,10 @@ patternProperties:
> >>>            Activates the reference channel in response to proximity events
> >>>            instead of touch events.
> >>>  
> >>> +      azoteq,counts-filt-enable:
> >>> +        type: boolean
> >>> +        description: Applies counts filtering to the channel.
> >>> +
> >>>        azoteq,ati-band:
> >>>          $ref: /schemas/types.yaml#/definitions/uint32
> >>>          enum: [0, 1, 2, 3]
> >>> @@ -432,12 +579,12 @@ patternProperties:
> >>>              description: |
> >>>                Specifies one or more GPIO mapped to the event as follows:
> >>>                0: GPIO0
> >>> -              1: GPIO3 (IQS7222C only)
> >>> -              2: GPIO4 (IQS7222C only)
> >>> +              1: GPIO3
> >>> +              2: GPIO4
> >>
> >> Why changing this? Is it valid for IQS7222A?
> > 
> > It's not, only for 'C' and now 'D'. However, the restriction for 'A' is already
> > conveyed in an if/then schema in the original binding. So rather than updating
> > this text to say "(IQS7222C and IQS7222D only)", I opted to drop the open-coded
> > text and rely on the existing schema.
> 
> OK
> 
> > 
> 
> 
> Best regards,
> Krzysztof
> 

Kind regards,
Jeff LaBundy
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/input/azoteq,iqs7222.yaml b/Documentation/devicetree/bindings/input/azoteq,iqs7222.yaml
index 9ddba7f2e7aa..a4c251a430fa 100644
--- a/Documentation/devicetree/bindings/input/azoteq,iqs7222.yaml
+++ b/Documentation/devicetree/bindings/input/azoteq,iqs7222.yaml
@@ -4,14 +4,14 @@ 
 $id: http://devicetree.org/schemas/input/azoteq,iqs7222.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
-title: Azoteq IQS7222A/B/C Capacitive Touch Controller
+title: Azoteq IQS7222A/B/C/D Capacitive Touch Controller
 
 maintainers:
   - Jeff LaBundy <jeff@labundy.com>
 
 description: |
-  The Azoteq IQS7222A, IQS7222B and IQS7222C are multichannel capacitive touch
-  controllers that feature additional sensing capabilities.
+  The Azoteq IQS7222A, IQS7222B, IQS7222C and IQS7222D are multichannel capac-
+  itive touch controllers that feature additional sensing capabilities.
 
   Link to datasheets: https://www.azoteq.com/
 
@@ -21,6 +21,7 @@  properties:
       - azoteq,iqs7222a
       - azoteq,iqs7222b
       - azoteq,iqs7222c
+      - azoteq,iqs7222d
 
   reg:
     maxItems: 1
@@ -173,6 +174,148 @@  properties:
     maximum: 3000
     description: Specifies the report rate (in ms) during ultra-low-power mode.
 
+  touchscreen-size-x: true
+  touchscreen-size-y: true
+  touchscreen-inverted-x: true
+  touchscreen-inverted-y: true
+  touchscreen-swapped-x-y: true
+
+  trackpad:
+    type: object
+    description: Represents all channels associated with the trackpad.
+
+    properties:
+      azoteq,channel-select:
+        $ref: /schemas/types.yaml#/definitions/uint32-array
+        minItems: 1
+        maxItems: 12
+        items:
+          minimum: 0
+          maximum: 13
+        description:
+          Specifies the order of the channels that participate in the trackpad.
+          Specify 255 to omit a given channel for the purpose of mapping a non-
+          rectangular trackpad.
+
+      azoteq,num-rows:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        minimum: 1
+        maximum: 12
+        description: Specifies the number of rows that comprise the trackpad.
+
+      azoteq,num-cols:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        minimum: 1
+        maximum: 12
+        description: Specifies the number of columns that comprise the trackpad.
+
+      azoteq,top-speed:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        multipleOf: 4
+        minimum: 0
+        maximum: 1020
+        description:
+          Specifies the speed of movement after which coordinate filtering is
+          no longer applied.
+
+      azoteq,bottom-speed:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        minimum: 0
+        maximum: 255
+        description:
+          Specifies the speed of movement after which coordinate filtering is
+          linearly reduced.
+
+      azoteq,use-prox:
+        type: boolean
+        description:
+          Directs the trackpad to respond to the proximity states of the se-
+          lected channels instead of their corresponding touch states. Note
+          the trackpad cannot report granular coordinates during a state of
+          proximity.
+
+    patternProperties:
+      "^azoteq,lower-cal-(x|y)$":
+        $ref: /schemas/types.yaml#/definitions/uint32
+        minimum: 0
+        maximum: 255
+        description: Specifies the trackpad's lower starting points.
+
+      "^azoteq,upper-cal-(x|y)$":
+        $ref: /schemas/types.yaml#/definitions/uint32
+        minimum: 0
+        maximum: 255
+        description: Specifies the trackpad's upper starting points.
+
+      "^event-(press|tap|(swipe|flick)-(x|y)-(pos|neg))$":
+        type: object
+        $ref: input.yaml#
+        description:
+          Represents a press or gesture event reported by the trackpad. Specify
+          'linux,code' under the press event to report absolute coordinates.
+
+        properties:
+          linux,code: true
+
+          azoteq,gesture-angle-tighten:
+            type: boolean
+            description:
+              Limits the tangent of the gesture angle to 0.5 (axial gestures
+              only). If specified in one direction, the effect is applied in
+              either direction.
+
+          azoteq,gesture-max-ms:
+            multipleOf: 16
+            minimum: 0
+            maximum: 4080
+            description:
+              Specifies the length of time (in ms) within which a tap, swipe
+              or flick gesture must be completed in order to be acknowledged
+              by the device. The number specified for any one swipe or flick
+              gesture applies to all other swipe or flick gestures.
+
+          azoteq,gesture-min-ms:
+            multipleOf: 16
+            minimum: 0
+            maximum: 4080
+            description:
+              Specifies the length of time (in ms) for which a tap gesture must
+              be held in order to be acknowledged by the device.
+
+          azoteq,gesture-dist:
+            $ref: /schemas/types.yaml#/definitions/uint32
+            minimum: 0
+            maximum: 65535
+            description:
+              Specifies the distance across which a tap, swipe or flick gesture
+              must travel in order to be acknowledged by the device. The number
+              specified for any one swipe or flick gesture applies to all other
+              swipe or flick gestures.
+
+          azoteq,gpio-select:
+            $ref: /schemas/types.yaml#/definitions/uint32-array
+            minItems: 1
+            maxItems: 3
+            items:
+              minimum: 0
+              maximum: 2
+            description: |
+              Specifies one or more GPIO mapped to the event as follows:
+              0: GPIO0
+              1: GPIO3
+              2: GPIO4
+
+              Note that although multiple events can be mapped to a single
+              GPIO, they must all be of the same type (proximity, touch or
+              trackpad gesture).
+
+        additionalProperties: false
+
+    required:
+      - azoteq,channel-select
+
+    additionalProperties: false
+
 patternProperties:
   "^cycle-[0-9]$":
     type: object
@@ -288,6 +431,10 @@  patternProperties:
           Activates the reference channel in response to proximity events
           instead of touch events.
 
+      azoteq,counts-filt-enable:
+        type: boolean
+        description: Applies counts filtering to the channel.
+
       azoteq,ati-band:
         $ref: /schemas/types.yaml#/definitions/uint32
         enum: [0, 1, 2, 3]
@@ -432,12 +579,12 @@  patternProperties:
             description: |
               Specifies one or more GPIO mapped to the event as follows:
               0: GPIO0
-              1: GPIO3 (IQS7222C only)
-              2: GPIO4 (IQS7222C only)
+              1: GPIO3
+              2: GPIO4
 
               Note that although multiple events can be mapped to a single
               GPIO, they must all be of the same type (proximity, touch or
-              slider gesture).
+              slider/trackpad gesture).
 
           azoteq,thresh:
             $ref: /schemas/types.yaml#/definitions/uint32
@@ -610,8 +757,8 @@  patternProperties:
             description: |
               Specifies one or more GPIO mapped to the event as follows:
               0: GPIO0
-              1: GPIO3 (IQS7222C only)
-              2: GPIO4 (IQS7222C only)
+              1: GPIO3
+              2: GPIO4
 
               Note that although multiple events can be mapped to a single
               GPIO, they must all be of the same type (proximity, touch or
@@ -629,8 +776,8 @@  patternProperties:
     description: |
       Represents a GPIO mapped to one or more events as follows:
       gpio-0: GPIO0
-      gpio-1: GPIO3 (IQS7222C only)
-      gpio-2: GPIO4 (IQS7222C only)
+      gpio-1: GPIO3
+      gpio-2: GPIO4
 
     allOf:
       - $ref: ../pinctrl/pincfg-node.yaml#
@@ -641,6 +788,41 @@  patternProperties:
     additionalProperties: false
 
 allOf:
+  - $ref: touchscreen/touchscreen.yaml#
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: azoteq,iqs7222d
+
+    then:
+      patternProperties:
+        "^cycle-[0-9]$":
+          properties:
+            azoteq,iref-enable: false
+
+        "^channel-([0-9]|1[0-9])$":
+          properties:
+            azoteq,ref-select: false
+
+        "^slider-[0-1]$": false
+
+    else:
+      properties:
+        touchscreen-size-x: false
+        touchscreen-size-y: false
+        touchscreen-inverted-x: false
+        touchscreen-inverted-y: false
+        touchscreen-swapped-x-y: false
+
+        trackpad: false
+
+      patternProperties:
+        "^channel-([0-9]|1[0-9])$":
+          properties:
+            azoteq,counts-filt-enable: false
+
   - if:
       properties:
         compatible: