'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 <[email protected]>
---
.../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 <[email protected]>
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/
> ---
> .../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
>
Kind regards,
Jeff LaBundy
On Wed, Jan 25, 2023 at 7:51 PM Jeff LaBundy <[email protected]> wrote:
>
> 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 <[email protected]>
>
> 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 <[email protected]> wrote:
> >
> > 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 <[email protected]>
> >
> > 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