[PATCH v2 3/6] dt-bindings: gpio: gpio-cdns: convert to YAML

Harshit Shah posted 6 patches 3 months, 3 weeks ago
There is a newer version of this series
[PATCH v2 3/6] dt-bindings: gpio: gpio-cdns: convert to YAML
Posted by Harshit Shah 3 months, 3 weeks ago
Convert Cadence family GPIO controller bindings to DT schema.

Changes during conversion:
   - update the naming as per the other files.
   - add gpio maintainers

Signed-off-by: Harshit Shah <hshah@axiado.com>
---
 .../devicetree/bindings/gpio/cdns,gpio.txt         | 43 ------------
 .../devicetree/bindings/gpio/gpio-cdns.yaml        | 81 ++++++++++++++++++++++
 2 files changed, 81 insertions(+), 43 deletions(-)

diff --git a/Documentation/devicetree/bindings/gpio/cdns,gpio.txt b/Documentation/devicetree/bindings/gpio/cdns,gpio.txt
deleted file mode 100644
index 706ef00f5c64951bb29c79a5541db4397e8b2733..0000000000000000000000000000000000000000
--- a/Documentation/devicetree/bindings/gpio/cdns,gpio.txt
+++ /dev/null
@@ -1,43 +0,0 @@
-Cadence GPIO controller bindings
-
-Required properties:
-- compatible: should be "cdns,gpio-r1p02".
-- reg: the register base address and size.
-- #gpio-cells: should be 2.
-	* first cell is the GPIO number.
-	* second cell specifies the GPIO flags, as defined in
-		<dt-bindings/gpio/gpio.h>. Only the GPIO_ACTIVE_HIGH
-		and GPIO_ACTIVE_LOW flags are supported.
-- gpio-controller: marks the device as a GPIO controller.
-- clocks: should contain one entry referencing the peripheral clock driving
-	the GPIO controller.
-
-Optional properties:
-- ngpios: integer number of gpio lines supported by this controller, up to 32.
-- interrupts: interrupt specifier for the controllers interrupt.
-- interrupt-controller: marks the device as an interrupt controller. When
-	defined, interrupts, interrupt-parent and #interrupt-cells
-	are required.
-- interrupt-cells: should be 2.
-	* first cell is the GPIO number you want to use as an IRQ source.
-	* second cell specifies the IRQ type, as defined in
-		<dt-bindings/interrupt-controller/irq.h>.
-		Currently only level sensitive IRQs are supported.
-
-
-Example:
-	gpio0: gpio-controller@fd060000 {
-		compatible = "cdns,gpio-r1p02";
-		reg =<0xfd060000 0x1000>;
-
-		clocks = <&gpio_clk>;
-
-		interrupt-parent = <&gic>;
-		interrupts = <0 5 IRQ_TYPE_LEVEL_HIGH>;
-
-		gpio-controller;
-		#gpio-cells = <2>;
-
-		interrupt-controller;
-		#interrupt-cells = <2>;
-	};
diff --git a/Documentation/devicetree/bindings/gpio/gpio-cdns.yaml b/Documentation/devicetree/bindings/gpio/gpio-cdns.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..e71f0137912f88e69fb3fa20f096e1572211591c
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-cdns.yaml
@@ -0,0 +1,81 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/gpio/gpio-cdns.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Cadence GPIO Controller
+
+maintainers:
+  - Linus Walleij <linus.walleij@linaro.org>
+  - Bartosz Golaszewski <brgl@bgdev.pl>
+
+properties:
+  compatible:
+    const: cdns,gpio-r1p02
+
+  reg:
+    minItems: 1
+
+  clocks:
+    maxItems: 1
+
+  ngpios:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: Number of GPIO lines supported, maximum 32.
+
+  gpio-controller: true
+
+  "#gpio-cells":
+    const: 2
+    description: |
+      - First cell is the GPIO line number.
+      - Second cell is flags as defined in <dt-bindings/gpio/gpio.h>,
+        only GPIO_ACTIVE_HIGH and GPIO_ACTIVE_LOW supported.
+
+  interrupt-controller: true
+
+  "#interrupt-cells":
+    const: 2
+    description: |
+      - First cell is the GPIO line number used as IRQ.
+      - Second cell is the trigger type, as defined in
+        <dt-bindings/interrupt-controller/irq.h>.
+
+  interrupts:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - gpio-controller
+  - "#gpio-cells"
+
+if:
+  required: [interrupt-controller]
+then:
+  required:
+    - interrupts
+    - "#interrupt-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+    gpio0: gpio-controller@fd060000 {
+        compatible = "cdns,gpio-r1p02";
+        reg = <0xfd060000 0x1000>;
+        clocks = <&gpio_clk>;
+
+        interrupt-parent = <&gic>;
+        interrupts = <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>;
+
+        gpio-controller;
+        #gpio-cells = <2>;
+
+        interrupt-controller;
+        #interrupt-cells = <2>;
+    };

-- 
2.25.1
Re: [PATCH v2 3/6] dt-bindings: gpio: gpio-cdns: convert to YAML
Posted by Krzysztof Kozlowski 3 months, 3 weeks ago
On 16/06/2025 06:31, Harshit Shah wrote:
> Convert Cadence family GPIO controller bindings to DT schema.
> 
> Changes during conversion:
>    - update the naming as per the other files.

You made it entirely different than every other file and review.

>    - add gpio maintainers

Not really, you need to find someone interested in that hardware, not
subsystem maintainers.

> 
> Signed-off-by: Harshit Shah <hshah@axiado.com>
> ---
>  .../devicetree/bindings/gpio/cdns,gpio.txt         | 43 ------------
>  .../devicetree/bindings/gpio/gpio-cdns.yaml        | 81 ++++++++++++++++++++++

Previous filename was correct.

>  2 files changed, 81 insertions(+), 43 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/gpio/cdns,gpio.txt b/Documentation/devicetree/bindings/gpio/cdns,gpio.txt
> deleted file mode 100644
> index 706ef00f5c64951bb29c79a5541db4397e8b2733..0000000000000000000000000000000000000000
> --- a/Documentation/devicetree/bindings/gpio/cdns,gpio.txt
> +++ /dev/null
> @@ -1,43 +0,0 @@
> -Cadence GPIO controller bindings
> -


...

> diff --git a/Documentation/devicetree/bindings/gpio/gpio-cdns.yaml b/Documentation/devicetree/bindings/gpio/gpio-cdns.yaml
> new file mode 100644
> index 0000000000000000000000000000000000000000..e71f0137912f88e69fb3fa20f096e1572211591c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/gpio-cdns.yaml
> @@ -0,0 +1,81 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/gpio/gpio-cdns.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Cadence GPIO Controller
> +
> +maintainers:
> +  - Linus Walleij <linus.walleij@linaro.org>
> +  - Bartosz Golaszewski <brgl@bgdev.pl>
> +
> +properties:
> +  compatible:
> +    const: cdns,gpio-r1p02
> +
> +  reg:
> +    minItems: 1

Why this is min and unconstrained...

> +
> +  clocks:
> +    maxItems: 1

But this is max?

maxItems: 1 in both cases (so clocks are correct, but why writing
similar things entirely different?).

> +
> +  ngpios:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: Number of GPIO lines supported, maximum 32.
> +
> +  gpio-controller: true
> +
> +  "#gpio-cells":
> +    const: 2
> +    description: |
> +      - First cell is the GPIO line number.
> +      - Second cell is flags as defined in <dt-bindings/gpio/gpio.h>,
> +        only GPIO_ACTIVE_HIGH and GPIO_ACTIVE_LOW supported.
> +
> +  interrupt-controller: true
> +
> +  "#interrupt-cells":
> +    const: 2
> +    description: |
> +      - First cell is the GPIO line number used as IRQ.
> +      - Second cell is the trigger type, as defined in
> +        <dt-bindings/interrupt-controller/irq.h>.
> +
> +  interrupts:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - gpio-controller
> +  - "#gpio-cells"
> +
> +if:
> +  required: [interrupt-controller]
> +then:
> +  required:
> +    - interrupts
> +    - "#interrupt-cells"

Drop last one, core schema requires it.


Best regards,
Krzysztof
Re: [PATCH v2 3/6] dt-bindings: gpio: gpio-cdns: convert to YAML
Posted by Harshit Shah 3 months, 3 weeks ago
On 6/15/2025 11:05 PM, Krzysztof Kozlowski wrote:
> On 16/06/2025 06:31, Harshit Shah wrote:
>> Convert Cadence family GPIO controller bindings to DT schema.
>>
>> Changes during conversion:
>>     - update the naming as per the other files.
> You made it entirely different than every other file and review.
Got it. I will update it to "cdns,gpio.yaml"
>
>>     - add gpio maintainers
> Not really, you need to find someone interested in that hardware, not
> subsystem maintainers.
Understood. I have connected with the original author and I will update 
the same.
>
>> Signed-off-by: Harshit Shah <hshah@axiado.com>
>> ---
>>   .../devicetree/bindings/gpio/cdns,gpio.txt         | 43 ------------
>>   .../devicetree/bindings/gpio/gpio-cdns.yaml        | 81 ++++++++++++++++++++++
> Previous filename was correct.
Got it. I will update it to "cdns,gpio.yaml"
>
>>   2 files changed, 81 insertions(+), 43 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/gpio/cdns,gpio.txt b/Documentation/devicetree/bindings/gpio/cdns,gpio.txt
>> deleted file mode 100644
>> index 706ef00f5c64951bb29c79a5541db4397e8b2733..0000000000000000000000000000000000000000
>> --- a/Documentation/devicetree/bindings/gpio/cdns,gpio.txt
>> +++ /dev/null
>> @@ -1,43 +0,0 @@
>> -Cadence GPIO controller bindings
>> -
>>
>> +properties:
>> +  compatible:
>> +    const: cdns,gpio-r1p02
>> +
>> +  reg:
>> +    minItems: 1
> Why this is min and unconstrained...
Make sense. I will update it to maxItems: 1
>
>> +
>> +  clocks:
>> +    maxItems: 1
> But this is max?
>
> maxItems: 1 in both cases (so clocks are correct, but why writing
> similar things entirely different?).
Yes will keep it to maxItems: 1
>
>> +
>> +  ngpios:
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    description: Number of GPIO lines supported, maximum 32.
>> +
>> +  gpio-controller: true
>> +
>> +  "#gpio-cells":
>> +    const: 2
>> +    description: |
>> +      - First cell is the GPIO line number.
>> +      - Second cell is flags as defined in <dt-bindings/gpio/gpio.h>,
>> +        only GPIO_ACTIVE_HIGH and GPIO_ACTIVE_LOW supported.
>> +
>> +  interrupt-controller: true
>> +
>> +  "#interrupt-cells":
>> +    const: 2
>> +    description: |
>> +      - First cell is the GPIO line number used as IRQ.
>> +      - Second cell is the trigger type, as defined in
>> +        <dt-bindings/interrupt-controller/irq.h>.
>> +
>> +  interrupts:
>> +    maxItems: 1
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - clocks
>> +  - gpio-controller
>> +  - "#gpio-cells"
>> +
>> +if:
>> +  required: [interrupt-controller]
>> +then:
>> +  required:
>> +    - interrupts
>> +    - "#interrupt-cells"
> Drop last one, core schema requires it.
Got it. I will remove the "#interrupt-cells".
>
>
> Best regards,
> Krzysztof

Thank you for the review, I will update as per the above comments.

I have verified that the dt_binding_check is passing without errors.
However, check_patch.pl is giving warning about the DT binding docs and includes.

checkpatch.pl: dev/null:10: WARNING: DT binding docs and includes should be a separate patch. See: Documentation/devicetree/bindings/submitting-patches.rst
checkpatch.pl: Documentation/devicetree/bindings/gpio/gpio-cdns.yaml:-1: WARNING: DT binding docs and includes should be a separate patch. See: Documentation/devicetree/bindings/submitting-patches.rst

I am unsure about how to split files as suggested, could you please advise?