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@xxxxxxxxxx> 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
