[PATCH] dt-bindings: input: azoteq: Fix differing types

'azoteq,ati-base' and 'azoteq,thresh' properties are defined in multiple
bindings, but have differing types defined. Both 'uint32' and
'uint32-array' are used. Unify these to use 'uint32-array' everywhere.

Signed-off-by: Rob Herring <robh@kernel.org>
---
 .../bindings/input/azoteq,iqs7222.yaml        | 12 ++++---
 .../devicetree/bindings/input/iqs269a.yaml    | 34 +++++++++++--------
 .../devicetree/bindings/input/iqs626a.yaml    | 12 ++++---
 3 files changed, 33 insertions(+), 25 deletions(-)

diff --git a/Documentation/devicetree/bindings/input/azoteq,iqs7222.yaml b/Documentation/devicetree/bindings/input/azoteq,iqs7222.yaml
index 9ddba7f2e7aa..f2382a56884d 100644
--- a/Documentation/devicetree/bindings/input/azoteq,iqs7222.yaml
+++ b/Documentation/devicetree/bindings/input/azoteq,iqs7222.yaml

@@ -354,10 +354,11 @@ patternProperties:
         description: Specifies the channel's ATI target.
 
       azoteq,ati-base:
-        $ref: /schemas/types.yaml#/definitions/uint32
-        multipleOf: 16
-        minimum: 0
-        maximum: 496
+        $ref: /schemas/types.yaml#/definitions/uint32-array
+        items:
+          - multipleOf: 16
+            minimum: 0
+            maximum: 496
         description: Specifies the channel's ATI base.
 
       azoteq,ati-mode:

@@ -440,7 +441,8 @@ patternProperties:
               slider gesture).
 
           azoteq,thresh:
-            $ref: /schemas/types.yaml#/definitions/uint32
+            $ref: /schemas/types.yaml#/definitions/uint32-array
+            maxItems: 1
             description:
               Specifies the threshold for the event. Valid entries range from
               0-127 and 0-255 for proximity and touch events, respectively.

diff --git a/Documentation/devicetree/bindings/input/iqs269a.yaml b/Documentation/devicetree/bindings/input/iqs269a.yaml
index 3c430d38594f..4fa20f0f6847 100644
--- a/Documentation/devicetree/bindings/input/iqs269a.yaml
+++ b/Documentation/devicetree/bindings/input/iqs269a.yaml

@@ -334,9 +334,10 @@ patternProperties:
           3: Full
 
       azoteq,ati-base:
-        $ref: /schemas/types.yaml#/definitions/uint32
-        enum: [75, 100, 150, 200]
-        default: 100
+        $ref: /schemas/types.yaml#/definitions/uint32-array
+        items:
+          - enum: [75, 100, 150, 200]
+            default: 100
         description: Specifies the channel's ATI base.
 
       azoteq,ati-target:

@@ -391,10 +392,11 @@ patternProperties:
 
         properties:
           azoteq,thresh:
-            $ref: /schemas/types.yaml#/definitions/uint32
-            minimum: 0
-            maximum: 255
-            default: 10
+            $ref: /schemas/types.yaml#/definitions/uint32-array
+            items:
+              - minimum: 0
+                maximum: 255
+                default: 10
             description: Specifies the threshold for the event.
 
           linux,code: true

@@ -408,10 +410,11 @@ patternProperties:
 
         properties:
           azoteq,thresh:
-            $ref: /schemas/types.yaml#/definitions/uint32
-            minimum: 0
-            maximum: 255
-            default: 8
+            $ref: /schemas/types.yaml#/definitions/uint32-array
+            items:
+              - minimum: 0
+                maximum: 255
+                default: 8
             description: Specifies the threshold for the event.
 
           azoteq,hyst:

@@ -432,10 +435,11 @@ patternProperties:
 
         properties:
           azoteq,thresh:
-            $ref: /schemas/types.yaml#/definitions/uint32
-            minimum: 0
-            maximum: 255
-            default: 26
+            $ref: /schemas/types.yaml#/definitions/uint32-array
+            items:
+              - minimum: 0
+                maximum: 255
+                default: 26
             description: Specifies the threshold for the event.
 
           azoteq,hyst:

diff --git a/Documentation/devicetree/bindings/input/iqs626a.yaml b/Documentation/devicetree/bindings/input/iqs626a.yaml
index 7a27502095f3..dbd63d48605c 100644
--- a/Documentation/devicetree/bindings/input/iqs626a.yaml
+++ b/Documentation/devicetree/bindings/input/iqs626a.yaml

@@ -234,8 +234,9 @@ patternProperties:
           about the available RUI options.
 
       azoteq,ati-base:
-        $ref: /schemas/types.yaml#/definitions/uint32
-        enum: [75, 100, 150, 200]
+        $ref: /schemas/types.yaml#/definitions/uint32-array
+        items:
+          - enum: [75, 100, 150, 200]
         description:
           Specifies the channel's ATI base. The default value is a function
           of the channel and the device's RUI.

@@ -475,9 +476,10 @@ patternProperties:
 
         properties:
           azoteq,thresh:
-            $ref: /schemas/types.yaml#/definitions/uint32
-            minimum: 0
-            maximum: 255
+            $ref: /schemas/types.yaml#/definitions/uint32-array
+            items:
+              - minimum: 0
+                maximum: 255
             description: Specifies the threshold for the event.
 
           azoteq,hyst:

-- 
2.39.0

Hi Rob,

On Wed, Jan 25, 2023 at 04:14:16PM -0600, Rob Herring wrote:

'azoteq,ati-base' and 'azoteq,thresh' properties are defined in multiple
bindings, but have differing types defined. Both 'uint32' and
'uint32-array' are used. Unify these to use 'uint32-array' everywhere.

Signed-off-by: Rob Herring <robh@kernel.org>

Thank you for the patch. While this is a step forward in moving toward
a common binding for this vendor like we have discussed in the past, I
do not agree with this approach and will instead propose an alternative
that accomplishes the same goal.

For all of these devices, a single sensing channel takes a base and a
threshold property. IQS626A is unique in that a fixed number of channels
form a trackpad, and I decided at the time to simply define the base and
target properties for all channels as a uint32-array.

For all other existing drivers, as well as others coming down the pipe,
base and threshold are uint32s. I find it confusing to redefine all of
those as single-element arrays, especially on account of one device.

In hindsight, a better design would have been to define a child node
for each channel under the trackpad node, with each child node accepting
a uint32 base and threshold. That would follow other devices, be more
representative of the actual hardware, and keep the definitions the same
across each binding.

So, that's what I propose to do here instead. I happen to have a fix in
review [1] that addresses a bug related to this parsing code, and would
be happy to build this solution on top assuming it can wait until the
next cycle. Does this compromise sound OK?

[1] https://patchwork.kernel.org/patch/https://patchwork.kernel.org/patch/13087768/

Kind regards,
Jeff LaBundy

On Wed, Jan 25, 2023 at 7:51 PM Jeff LaBundy [off-list ref] wrote:

Hi Rob,

On Wed, Jan 25, 2023 at 04:14:16PM -0600, Rob Herring wrote:

quoted

'azoteq,ati-base' and 'azoteq,thresh' properties are defined in multiple
bindings, but have differing types defined. Both 'uint32' and
'uint32-array' are used. Unify these to use 'uint32-array' everywhere.

Signed-off-by: Rob Herring <robh@kernel.org>

Thank you for the patch. While this is a step forward in moving toward
a common binding for this vendor like we have discussed in the past, I
do not agree with this approach and will instead propose an alternative
that accomplishes the same goal.

For all of these devices, a single sensing channel takes a base and a
threshold property. IQS626A is unique in that a fixed number of channels
form a trackpad, and I decided at the time to simply define the base and
target properties for all channels as a uint32-array.

For all other existing drivers, as well as others coming down the pipe,
base and threshold are uint32s. I find it confusing to redefine all of
those as single-element arrays, especially on account of one device.

In hindsight, a better design would have been to define a child node
for each channel under the trackpad node, with each child node accepting
a uint32 base and threshold. That would follow other devices, be more
representative of the actual hardware, and keep the definitions the same
across each binding.

So, that's what I propose to do here instead. I happen to have a fix in
review [1] that addresses a bug related to this parsing code, and would
be happy to build this solution on top assuming it can wait until the
next cycle. Does this compromise sound OK?

Shrug

How exactly are you going to change an existing property and not break
existing users? Or are there not any users?

We have the same properties with 2 definitions. At the end of the day,
I just want to fix that...

Rob

Hi Rob,

On Wed, Jan 25, 2023 at 09:10:33PM -0600, Rob Herring wrote:

On Wed, Jan 25, 2023 at 7:51 PM Jeff LaBundy [off-list ref] wrote:

quoted

Hi Rob,

On Wed, Jan 25, 2023 at 04:14:16PM -0600, Rob Herring wrote:

quoted

'azoteq,ati-base' and 'azoteq,thresh' properties are defined in multiple
bindings, but have differing types defined. Both 'uint32' and
'uint32-array' are used. Unify these to use 'uint32-array' everywhere.

Signed-off-by: Rob Herring <robh@kernel.org>

Thank you for the patch. While this is a step forward in moving toward
a common binding for this vendor like we have discussed in the past, I
do not agree with this approach and will instead propose an alternative
that accomplishes the same goal.

For all of these devices, a single sensing channel takes a base and a
threshold property. IQS626A is unique in that a fixed number of channels
form a trackpad, and I decided at the time to simply define the base and
target properties for all channels as a uint32-array.

For all other existing drivers, as well as others coming down the pipe,
base and threshold are uint32s. I find it confusing to redefine all of
those as single-element arrays, especially on account of one device.

In hindsight, a better design would have been to define a child node
for each channel under the trackpad node, with each child node accepting
a uint32 base and threshold. That would follow other devices, be more
representative of the actual hardware, and keep the definitions the same
across each binding.

So, that's what I propose to do here instead. I happen to have a fix in
review [1] that addresses a bug related to this parsing code, and would
be happy to build this solution on top assuming it can wait until the
next cycle. Does this compromise sound OK?

Shrug

How exactly are you going to change an existing property and not break
existing users? Or are there not any users?

There are no known users at this time.

We have the same properties with 2 definitions. At the end of the day,
I just want to fix that...

Agreed on all counts. I've folded my proposal into the existing fix for
this driver and sent [1] for your consideration.

[1] https://patchwork.kernel.org/patch/13119464/

Rob

Kind regards,
Jeff LaBundy