Hi Oleh, On Thu, Sep 12, 2024 at 03:28:22PM +0200, Oleh Kuzhylnyi wrote: > Add documentation for the Hynitron CST816X touchscreen bindings. > > Reviewed-by: Conor Dooley <conor.dooley@xxxxxxxxxxxxx> > Signed-off-by: Oleh Kuzhylnyi <kuzhylol@xxxxxxxxx> > --- > > Changes in v7: > - Introduce the gestures field along with its sub-fields > - Make reset-gpio property optional > - Extend main description > - Remove "touchscreen" reference > > Changes in v6: > - Fix minor tweak adviced by Krzysztof: > - Move additionalProperties field after required > > Changes in v5: > - No code changes > > Changes in v4: > - Add Conor's Dooley "Reviewed-by" tag > > Changes in v3: > - Rename filename to hynitron,cst816s.yaml > - Update description with display details > > Changes in v2: > - Apply pin definitions and DT headers > - Use generic name for DT node > - Drop status field > > .../input/touchscreen/hynitron,cst816s.yaml | 126 ++++++++++++++++++ > 1 file changed, 126 insertions(+) > create mode 100644 Documentation/devicetree/bindings/input/touchscreen/hynitron,cst816s.yaml > > diff --git a/Documentation/devicetree/bindings/input/touchscreen/hynitron,cst816s.yaml b/Documentation/devicetree/bindings/input/touchscreen/hynitron,cst816s.yaml > new file mode 100644 > index 000000000000..99ac29da7a5a > --- /dev/null > +++ b/Documentation/devicetree/bindings/input/touchscreen/hynitron,cst816s.yaml > @@ -0,0 +1,126 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/input/touchscreen/hynitron,cst816s.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Hynitron CST816S Touchscreen controller > + > +description: > + The CST816S is a touchscreen controller from Hynitron, which supports gesture > + recognition for swipe directions, tap, and long-press actions. This binding > + document defines the necessary properties for integrating the CST816S with > + a Linux system. > + > +maintainers: > + - Oleh Kuzhylnyi <kuzhylol@xxxxxxxxx> > + > +properties: > + compatible: > + enum: > + - hynitron,cst816s > + > + reg: > + maxItems: 1 > + > + interrupts: > + maxItems: 1 > + > + reset-gpios: > + maxItems: 1 > + description: > + Optional GPIO line used to reset the touchscreen controller. > + optional: true Thank you for the spin, but I think there is some misunderstanding here. You do not need to explicitly define the property as optional; I do not think this is even a keyword. All properties are optional unless explicitly listed under the 'required' keyword, so all that needs to be done here is simply remove 'reset-gpios' from under the 'required' keyword as you have done. > + > + gestures: > + type: object > + description: > + A list of gestures supported by the CST816S touchscreen controller and > + their associated Linux input event codes. > + optional: true > + > + properties: > + "^.*$": Again, I think there is some misunderstanding here; I do not think this can compile, as evidenced by the bot and others' feedback. Even if this regex were valid, it would need to be placed under a 'patternProperties' keyword. However, this is not really an optional approach either. I think it can help to put yourself in a customer's perspective; it will not be clear to many people what is the "gesture ID". This seems like an arbitrary index that is only relevant within the driver code. In general, there are two common ways to specify key codes for a touch controller that can recognize gesture events: 1. By name. Each gesture is represented by a child node with a unique name (e.g. 'gesture-x-neg'), and your regex looks something like the following: patternProperties: "^gesture-(x|y)-(pos|neg)$": The driver then matches child nodes by name, and the customer need not have any sense of the order in which interrupt status bits appear within the register map. Each child node then specifies the 'linux,code' property, as well as any other properties relevant to the gesture. 2. A single array with length equal to the number of gestures reported by the device, defined as follows: properties: linux,keycodes: true Option (1) is useful if you must support other properties associated with each gesture such as distance threshold, timing parameters, etc. If the HW does not expose any such properties and the key code is the sole parameter associated with each gesture, then option (2) is sufficient. There are many upstream bindings that use either approach; please follow them. In either case, 'linux,code' and 'linux,keycodes' are both standard properties defined in [1], please study this binding and reference it in this one. > + type: object > + description: > + Each child node represents a gesture that the touchscreen controller > + can recognize. > + > + properties: > + cst816x,gesture: > + description: > + Numeric value representing the gesture ID recognized by the > + CST816S touchscreen controller. > + $ref: /schemas/types.yaml#/definitions/uint32 > + > + linux,code: > + description: > + Linux input event code (from linux/input-event-codes.h) that > + corresponds to the gesture. > + $ref: /schemas/types.yaml#/definitions/uint32 This property's description and type are already defined in [1]; they do not need to be redefined here so long as [1] is referenced correctly. > + > + required: > + - cst816x,gesture > + - linux,code > + > + additionalProperties: false > + > +required: > + - compatible > + - reg > + - interrupts > + > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/gpio/gpio.h> > + #include <dt-bindings/input/linux-event-codes.h> > + #include <dt-bindings/interrupt-controller/irq.h> > + > + i2c { > + #address-cells = <1>; > + #size-cells = <0>; > + > + touchscreen@15 { > + compatible = "hynitron,cst816s"; > + reg = <0x15>; > + interrupt-parent = <&gpio0>; > + interrupts = <4 IRQ_TYPE_EDGE_RISING>; > + reset-gpios = <&gpio 17 GPIO_ACTIVE_LOW>; > + > + gestures { > + swipe_up { > + cst816x,gesture = <0x1>; > + linux,code = <BTN_FORWARD>; > + }; > + > + swipe_down { > + cst816x,gesture = <0x2>; > + linux,code = <BTN_BACK>; > + }; > + > + swipe_left { > + cst816x,gesture = <0x3>; > + linux,code = <BTN_LEFT>; > + }; > + > + swipe_right { > + cst816x,gesture = <0x4>; > + linux,code = <BTN_RIGHT>; > + }; > + > + single_tap { > + cst816x,gesture = <0x5>; > + linux,code = <BTN_TOUCH>; > + }; > + > + long_press { > + cst816x,gesture = <0xC>; > + linux,code = <BTN_TOOL_TRIPLETAP>; > + }; > + }; > + }; > + }; > + > +... > -- > 2.34.1 > [1] Documentation/devicetree/bindings/input/input.yaml Kind regards, Jeff LaBundy
