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