diff mbox series

[1/3] dt-bindings: gpio: add schema for NXP S32G2/S32G3 SoCs

Message ID 20240826084214.2368673-2-andrei.stefanescu@oss.nxp.com (mailing list archive)
State New
Headers show
Series gpio: siul2-s32g2: add initial GPIO driver | expand

Commit Message

Andrei Stefanescu Aug. 26, 2024, 8:42 a.m. UTC
Add the DT schema for the GPIO driver of the NXP S32G2/S32G3 SoCs.

Signed-off-by: Phu Luu An <phu.luuan@nxp.com>
Signed-off-by: Larisa Grigore <larisa.grigore@nxp.com>
Signed-off-by: Ghennadi Procopciuc <ghennadi.procopciuc@nxp.com>
Signed-off-by: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>
---
 .../bindings/gpio/nxp,gpio-siul2-s32g2.yaml   | 134 ++++++++++++++++++
 1 file changed, 134 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/nxp,gpio-siul2-s32g2.yaml

Comments

Krzysztof Kozlowski Aug. 26, 2024, 9:14 a.m. UTC | #1
On 26/08/2024 10:42, Andrei Stefanescu wrote:
> Add the DT schema for the GPIO driver of the NXP S32G2/S32G3 SoCs.
> 

A nit, subject: drop second/last, redundant "schema". The "dt-bindings"
prefix is already stating that these are bindings/schema.
See also:
https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18

> Signed-off-by: Phu Luu An <phu.luuan@nxp.com>
> Signed-off-by: Larisa Grigore <larisa.grigore@nxp.com>
> Signed-off-by: Ghennadi Procopciuc <ghennadi.procopciuc@nxp.com>
> Signed-off-by: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>
> ---
>  .../bindings/gpio/nxp,gpio-siul2-s32g2.yaml   | 134 ++++++++++++++++++
>  1 file changed, 134 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/gpio/nxp,gpio-siul2-s32g2.yaml
> 
> diff --git a/Documentation/devicetree/bindings/gpio/nxp,gpio-siul2-s32g2.yaml b/Documentation/devicetree/bindings/gpio/nxp,gpio-siul2-s32g2.yaml
> new file mode 100644
> index 000000000000..fba41a18d4c8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/nxp,gpio-siul2-s32g2.yaml

Filename matching compatible.

> @@ -0,0 +1,134 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-3-Clause
> +# Copyright 2024 NXP
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/gpio/nxp,gpio-siul2-s32g2.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: NXP S32G2 SIUL2 GPIO controller
> +
> +maintainers:
> +  - Ghennadi Procopciuc <Ghennadi.Procopciuc@nxp.com>
> +  - Larisa Grigore <larisa.grigore@nxp.com>
> +  - Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>
> +
> +description: |

Do not need '|' unless you need to preserve formatting.


> +  Support for the SIUL2 GPIOs found on the S32G2 and S32G3
> +  chips. It includes an IRQ controller for all pins which have
> +  an EIRQ associated.
> +
> +properties:
> +  compatible:
> +    items:
> +      - const: nxp,s32g2-siul2-gpio
> +
> +  reg:
> +    description: |
> +      A list of register regions for configuring interrupts,
> +      GPIO output values and reading GPIO input values.

Drop description, obvious. It cannot be anything else.

> +    items:
> +      - description: PGPDO (output value) registers for SIUL2_0
> +      - description: PGPDO (output value) registers for SIUL2_1
> +      - description: PGPDI (input value) registers for SIUL2_0
> +      - description: PGPDI (input value) registers for SIUL2_1
> +      - description: EIRQ (interrupt) configuration registers from SIUL2_1
> +      - description: EIRQ IMCR registers for interrupt muxing between pads
> +
> +  reg-names:
> +    items:
> +      - const: opads0
> +      - const: opads1
> +      - const: ipads0
> +      - const: ipads1
> +      - const: eirqs
> +      - const: eirq-imcrs
> +
> +  interrupts:
> +    description:
> +      The port interrupt shared by all 32 EIRQs.

Missing items. Look at existing examples. There is no code like this.

> +
> +  gpio-controller:
> +    description:
> +      Marks the device node as a gpio controller

Drop description, obvious.

> +
> +  "#gpio-cells":
> +    description: |
> +      Should be two. The first cell is the pin number and
> +      the second cell is used to specify the gpio polarity
> +      0 = active high
> +      1 = active low

This binding is nowhere near what we have in kernel. Please do not
re-invent stuff. Take recent binding and customize it.

Don't repeat constraints in free form text.

> +
> +  interrupt-controller:
> +    description:
> +      Marks the device node as an interrupt controller

Really? Drop.

> +
> +  "#interrupt-cells":
> +    const: 2
> +    description:
> +      Refer to ../interrupt-controller/interrupts.txt for more details.

Drop description, useless.

> +
> +  gpio-ranges:
> +    description:
> +      Interaction with the PINCTRL subsystem
> +      Refer to gpio.txt for more details.

Oh... drop. Missing constraints.

> +
> +  gpio-reserved-ranges:
> +    description:
> +      A list of start GPIO and number of GPIO pairs which are unusable.
> +      Refer to gpio.txt for more details.

Drop. Missing constraints.

> +
> +patternProperties:
> +  "^(hog-[0-9]+|.+-hog(-[0-9]+)?)$":
> +    additionalProperties: false
> +    type: object
> +    properties:
> +      gpio-hog: true
> +      gpios: true
> +      input: true
> +      output-high: true
> +      output-low: true
> +      line-name: true
> +    required:
> +      - gpio-hog
> +      - gpios

Drop all this and use simplified form.
https://elixir.bootlin.com/linux/v6.9-rc3/source/Documentation/devicetree/bindings/pinctrl/qcom,sdm845-pinctrl.yaml#L45

This applies to all your patches in the future as well.

> +
> +required:
> +  - compatible
> +  - interrupts
> +  - reg
> +  - reg-names
> +  - gpio-controller
> +  - "#gpio-cells"
> +  - interrupt-controller
> +  - "#interrupt-cells"
> +  - gpio-ranges
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    gpio: siul2-gpio@4009d700 {
> +        compatible = "nxp,s32g2-siul2-gpio";
> +        reg = <0x4009d700 0x10>,
> +              <0x44011700 0x18>,
> +              <0x4009d740 0x10>,
> +              <0x44011740 0x18>,
> +              <0x44010010 0xb4>,
> +              <0x44011078 0x80>;
> +        reg-names = "opads0", "opads1", "ipads0",
> +                    "ipads1", "eirqs", "eirq-imcrs";
> +        gpio-controller;
> +        #gpio-cells = <2>;
> +                      /* GPIO 0-101 */
> +        gpio-ranges = <&pinctrl 0   0   102>,
> +                      /* GPIO 112-190 */
> +                      <&pinctrl 112 112 79>;
> +        gpio-reserved-ranges = <102 10>, <123 21>;
> +        interrupt-controller;
> +        #interrupt-cells = <2>;
> +        interrupts = <GIC_SPI 210 IRQ_TYPE_LEVEL_HIGH>;
> +        status = "okay";

Drop.

> +    };

Best regards,
Krzysztof
Krzysztof Kozlowski Aug. 26, 2024, 9:36 a.m. UTC | #2
On 26/08/2024 10:42, Andrei Stefanescu wrote:
> Add the DT schema for the GPIO driver of the NXP S32G2/S32G3 SoCs.
> 
> Signed-off-by: Phu Luu An <phu.luuan@nxp.com>
> Signed-off-by: Larisa Grigore <larisa.grigore@nxp.com>

BTW, these emails bounce. What is the intention of having them here? How
did they contribute to the SCHEMA file? We all are going to get bounces
from replying to you...

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/gpio/nxp,gpio-siul2-s32g2.yaml b/Documentation/devicetree/bindings/gpio/nxp,gpio-siul2-s32g2.yaml
new file mode 100644
index 000000000000..fba41a18d4c8
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/nxp,gpio-siul2-s32g2.yaml
@@ -0,0 +1,134 @@ 
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-3-Clause
+# Copyright 2024 NXP
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/gpio/nxp,gpio-siul2-s32g2.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NXP S32G2 SIUL2 GPIO controller
+
+maintainers:
+  - Ghennadi Procopciuc <Ghennadi.Procopciuc@nxp.com>
+  - Larisa Grigore <larisa.grigore@nxp.com>
+  - Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>
+
+description: |
+  Support for the SIUL2 GPIOs found on the S32G2 and S32G3
+  chips. It includes an IRQ controller for all pins which have
+  an EIRQ associated.
+
+properties:
+  compatible:
+    items:
+      - const: nxp,s32g2-siul2-gpio
+
+  reg:
+    description: |
+      A list of register regions for configuring interrupts,
+      GPIO output values and reading GPIO input values.
+    items:
+      - description: PGPDO (output value) registers for SIUL2_0
+      - description: PGPDO (output value) registers for SIUL2_1
+      - description: PGPDI (input value) registers for SIUL2_0
+      - description: PGPDI (input value) registers for SIUL2_1
+      - description: EIRQ (interrupt) configuration registers from SIUL2_1
+      - description: EIRQ IMCR registers for interrupt muxing between pads
+
+  reg-names:
+    items:
+      - const: opads0
+      - const: opads1
+      - const: ipads0
+      - const: ipads1
+      - const: eirqs
+      - const: eirq-imcrs
+
+  interrupts:
+    description:
+      The port interrupt shared by all 32 EIRQs.
+
+  gpio-controller:
+    description:
+      Marks the device node as a gpio controller
+
+  "#gpio-cells":
+    description: |
+      Should be two. The first cell is the pin number and
+      the second cell is used to specify the gpio polarity
+      0 = active high
+      1 = active low
+
+  interrupt-controller:
+    description:
+      Marks the device node as an interrupt controller
+
+  "#interrupt-cells":
+    const: 2
+    description:
+      Refer to ../interrupt-controller/interrupts.txt for more details.
+
+  gpio-ranges:
+    description:
+      Interaction with the PINCTRL subsystem
+      Refer to gpio.txt for more details.
+
+  gpio-reserved-ranges:
+    description:
+      A list of start GPIO and number of GPIO pairs which are unusable.
+      Refer to gpio.txt for more details.
+
+patternProperties:
+  "^(hog-[0-9]+|.+-hog(-[0-9]+)?)$":
+    additionalProperties: false
+    type: object
+    properties:
+      gpio-hog: true
+      gpios: true
+      input: true
+      output-high: true
+      output-low: true
+      line-name: true
+    required:
+      - gpio-hog
+      - gpios
+
+required:
+  - compatible
+  - interrupts
+  - reg
+  - reg-names
+  - gpio-controller
+  - "#gpio-cells"
+  - interrupt-controller
+  - "#interrupt-cells"
+  - gpio-ranges
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    gpio: siul2-gpio@4009d700 {
+        compatible = "nxp,s32g2-siul2-gpio";
+        reg = <0x4009d700 0x10>,
+              <0x44011700 0x18>,
+              <0x4009d740 0x10>,
+              <0x44011740 0x18>,
+              <0x44010010 0xb4>,
+              <0x44011078 0x80>;
+        reg-names = "opads0", "opads1", "ipads0",
+                    "ipads1", "eirqs", "eirq-imcrs";
+        gpio-controller;
+        #gpio-cells = <2>;
+                      /* GPIO 0-101 */
+        gpio-ranges = <&pinctrl 0   0   102>,
+                      /* GPIO 112-190 */
+                      <&pinctrl 112 112 79>;
+        gpio-reserved-ranges = <102 10>, <123 21>;
+        interrupt-controller;
+        #interrupt-cells = <2>;
+        interrupts = <GIC_SPI 210 IRQ_TYPE_LEVEL_HIGH>;
+        status = "okay";
+    };