Re: [PATCH v2 2/3] dt-bindings: input: Add bindings for TouchNetix axiom touchscreen

Hello Krzysztof,

Le 2023-10-09 17:05, Krzysztof Kozlowski a écrit :

On 09/10/2023 15:44, Kamel Bouhara wrote:

quoted

Add the TouchNetix axiom I2C touchscreen device tree bindings
documentation.

A nit, subject: drop second/last, redundant "bindings for". The
"dt-bindings" prefix is already stating that these are bindings.

OK.

quoted

Signed-off-by: Kamel Bouhara <kamel.bouhara@bootlin.com>
---
 .../touchscreen/touchnetix,axiom-ax54a.yaml   | 51 +++++++++++++++++++
 MAINTAINERS                                   |  6 +++
 2 files changed, 57 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/touchscreen/touchnetix,axiom-ax54a.yaml

diff --git a/Documentation/devicetree/bindings/input/touchscreen/touchnetix,axiom-ax54a.yaml b/Documentation/devicetree/bindings/input/touchscreen/touchnetix,axiom-ax54a.yaml
new file mode 100644
index 000000000000..41201d7112a6
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/touchnetix,axiom-ax54a.yaml

@@ -0,0 +1,51 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/touchscreen/touchnetix,axiom-ax54a.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: TouchNetix Axiom series touchscreen controller
+
+maintainers:
+  - Kamel Bouhara <kamel.bouhara@bootlin.com>
+
+properties:
+  compatible:
+    const: touchnetix,axiom-ax54a
+
+  reg:
+    const: 0x66
+
+  interrupts:
+    maxItems: 1
+
+  irq-gpios:
+    maxItems: 1

Why these are GPIOs? Interrupts are usually just interrupts... You need
to clearly describe this.

I've been using this for some specific acpi stuff hence it need to be removed.

quoted

+
+  reset-gpios:
+    maxItems: 1
+
+additionalProperties: false

This goes after required: block.

quoted

+
+required:
+  - compatible
+  - reg
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+      axiom@66 {

Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation

Ack.

quoted

+        compatible = "touchnetix,axiom-ax54a";
+        reg = <0x66>;
+        interrupt-parent = <&gpio2>;
+        interrupts = <2 IRQ_TYPE_EDGE_FALLING>;
+        irq-gpios = <&gpio2 0 GPIO_ACTIVE_LOW>;

Eh? This looks really wrong.

OK let's clean that as well.

Thanks

Best regards,
Krzysztof