| Message ID | ZHanv+8fOYhpyMEC@nixie71 (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] dt-bindings: input: iqs7222: Add properties for Azoteq IQS7222D | expand |
On 31/05/2023 03:49, Jeff LaBundy wrote: > Extend the common binding to include a new variant of the silicon. > > Signed-off-by: Jeff LaBundy <jeff@labundy.com> Please use scripts/get_maintainers.pl to get a list of necessary people and lists to CC. It might happen, that command when run on an older kernel, gives you outdated entries. Therefore please be sure you base your patches on recent Linux kernel. It's not the first time - you keep ignoring maintainers output all the time. > --- > .../bindings/input/azoteq,iqs7222.yaml | 202 +++++++++++++++++- > 1 file changed, 192 insertions(+), 10 deletions(-) > > 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. Don't split words. > > 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 Why? Aren't they coming from common schema? > + > + 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? > + > + 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? > + > + 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. > + 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? > + > + "^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? > > 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 The binding got quite complicated. Are you sure these are similar devices? Best regards, Krzysztof
Hi Krzysztof, On Wed, May 31, 2023 at 07:22:31PM +0200, Krzysztof Kozlowski wrote: > On 31/05/2023 03:49, Jeff LaBundy wrote: > > Extend the common binding to include a new variant of the silicon. > > > > Signed-off-by: Jeff LaBundy <jeff@labundy.com> > > Please use scripts/get_maintainers.pl to get a list of necessary people > and lists to CC. It might happen, that command when run on an older > kernel, gives you outdated entries. Therefore please be sure you base > your patches on recent Linux kernel. > > It's not the first time - you keep ignoring maintainers output all the time. You'll have to forgive me; I'm submitting using some homemade tooling that is not yet stitched to get_maintainers, and I neglected to see that we're fortunate to have two more great maintainers. I'll fix that going forward. > > > --- > > .../bindings/input/azoteq,iqs7222.yaml | 202 +++++++++++++++++- > > 1 file changed, 192 insertions(+), 10 deletions(-) > > > > 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. > > Don't split words. ACK. > > > > > 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 > > 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. > > > + > > + 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. > > > + > > + 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. > > > + > > + "^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. > > > > 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 > > The binding got quite complicated. Are you sure these are similar devices? Yes, I'm quite sure. The silicon is largely the same, the primary difference being the way two access are reported (2x 1-D vs. 1x 2-D). The binding hasn't necessarily grown functionally; rather, most properties from the slider-0/1 node are simply re-used or extended to two dimensions for the new trackpad node. My long-term goal is to defer most of these properties to a common vendor schema as per my note to Rob in [1], but it does not seem possible yet. If I'm mistaken, or there is a more efficient means of re-use under the current tooling, I'll happily follow suit. [1] https://patchwork.kernel.org/patch/13259035/ > > Best regards, > Krzysztof > Kind regards, Jeff LaBundy
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. > >> >>> + >>> + 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. > >> >>> + >>> + 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
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 --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:
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(-)