* [PATCH] dt-bindings: input: azoteq: Fix differing types @ 2023-01-25 22:14 Rob Herring 2023-01-26 1:50 ` Jeff LaBundy 0 siblings, 1 reply; 4+ messages in thread From: Rob Herring @ 2023-01-25 22:14 UTC (permalink / raw) To: Dmitry Torokhov, Krzysztof Kozlowski, Jeff LaBundy Cc: linux-input, devicetree, linux-kernel '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 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] dt-bindings: input: azoteq: Fix differing types 2023-01-25 22:14 [PATCH] dt-bindings: input: azoteq: Fix differing types Rob Herring @ 2023-01-26 1:50 ` Jeff LaBundy 2023-01-26 3:10 ` Rob Herring 0 siblings, 1 reply; 4+ messages in thread From: Jeff LaBundy @ 2023-01-26 1:50 UTC (permalink / raw) To: Rob Herring Cc: Dmitry Torokhov, Krzysztof Kozlowski, linux-input, devicetree, linux-kernel 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/ > --- > .../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 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] dt-bindings: input: azoteq: Fix differing types 2023-01-26 1:50 ` Jeff LaBundy @ 2023-01-26 3:10 ` Rob Herring 2023-01-27 22:37 ` Jeff LaBundy 0 siblings, 1 reply; 4+ messages in thread From: Rob Herring @ 2023-01-26 3:10 UTC (permalink / raw) To: Jeff LaBundy Cc: Dmitry Torokhov, Krzysztof Kozlowski, linux-input, devicetree, linux-kernel On Wed, Jan 25, 2023 at 7:51 PM Jeff LaBundy <jeff@labundy.com> 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 <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 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] dt-bindings: input: azoteq: Fix differing types 2023-01-26 3:10 ` Rob Herring @ 2023-01-27 22:37 ` Jeff LaBundy 0 siblings, 0 replies; 4+ messages in thread From: Jeff LaBundy @ 2023-01-27 22:37 UTC (permalink / raw) To: Rob Herring Cc: Dmitry Torokhov, Krzysztof Kozlowski, linux-input, devicetree, linux-kernel 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 <jeff@labundy.com> 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 <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 ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-01-27 22:37 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-01-25 22:14 [PATCH] dt-bindings: input: azoteq: Fix differing types Rob Herring 2023-01-26 1:50 ` Jeff LaBundy 2023-01-26 3:10 ` Rob Herring 2023-01-27 22:37 ` Jeff LaBundy
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.