diff mbox series

[3/3] dt-bindings: pinctrl: at91-pio4: convert Atmel's PIO4 bindings to json-schema

Message ID 20240229-pio4-pinctrl-yaml-v1-3-c4d8279c083f@microchip.com
State New
Headers show
Series convert Atmel's PIO4 bindings to json-schema | expand

Commit Message

Balakrishnan Sambath Feb. 29, 2024, 11:39 a.m. UTC
Convert the existing text DT bindings of Atmel's PIO4 pincontroller to
yaml based DT schema.

Signed-off-by: Balakrishnan Sambath <balakrishnan.s@microchip.com>
---
 .../bindings/pinctrl/atmel,at91-pio4-pinctrl.txt   |  98 ---------------
 .../bindings/pinctrl/atmel,sama5d2-pinctrl.yaml    | 140 +++++++++++++++++++++
 2 files changed, 140 insertions(+), 98 deletions(-)

Comments

Rob Herring (Arm) Feb. 29, 2024, 12:45 p.m. UTC | #1
On Thu, 29 Feb 2024 17:09:32 +0530, Balakrishnan Sambath wrote:
> Convert the existing text DT bindings of Atmel's PIO4 pincontroller to
> yaml based DT schema.
> 
> Signed-off-by: Balakrishnan Sambath <balakrishnan.s@microchip.com>
> ---
>  .../bindings/pinctrl/atmel,at91-pio4-pinctrl.txt   |  98 ---------------
>  .../bindings/pinctrl/atmel,sama5d2-pinctrl.yaml    | 140 +++++++++++++++++++++
>  2 files changed, 140 insertions(+), 98 deletions(-)
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/pinctrl/atmel,sama5d2-pinctrl.example.dts:27:18: fatal error: dt-bindings/pinctrl/sama5d2-pinfunc.h: No such file or directory
   27 |         #include <dt-bindings/pinctrl/sama5d2-pinfunc.h>
      |                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
make[2]: *** [scripts/Makefile.lib:419: Documentation/devicetree/bindings/pinctrl/atmel,sama5d2-pinctrl.example.dtb] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1428: dt_binding_check] Error 2
make: *** [Makefile:240: __sub-make] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240229-pio4-pinctrl-yaml-v1-3-c4d8279c083f@microchip.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
Krzysztof Kozlowski Feb. 29, 2024, 5:41 p.m. UTC | #2
On 29/02/2024 12:39, Balakrishnan Sambath wrote:
> Convert the existing text DT bindings of Atmel's PIO4 pincontroller to
> yaml based DT schema.
> 
> Signed-off-by: Balakrishnan Sambath <balakrishnan.s@microchip.com>
> ---

Dependency shall be noted here, with lore link.

>  .../bindings/pinctrl/atmel,at91-pio4-pinctrl.txt   |  98 ---------------
>  .../bindings/pinctrl/atmel,sama5d2-pinctrl.yaml    | 140 +++++++++++++++++++++
>  2 files changed, 140 insertions(+), 98 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pinctrl/atmel,at91-pio4-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/atmel,at91-pio4-pinctrl.txt
> deleted file mode 100644
> index 774c3c269c40..000000000000
> --- a/Documentation/devicetree/bindings/pinctrl/atmel,at91-pio4-pinctrl.txt
> +++ /dev/null
> @@ -1,98 +0,0 @@
> -* Atmel PIO4 Controller

...

> -...
> diff --git a/Documentation/devicetree/bindings/pinctrl/atmel,sama5d2-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/atmel,sama5d2-pinctrl.yaml
> new file mode 100644
> index 000000000000..8a2dee1d6dd3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pinctrl/atmel,sama5d2-pinctrl.yaml
> @@ -0,0 +1,140 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pinctrl/atmel,sama5d2-pinctrl.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Microchip PIO4 Controller
> +
> +maintainers:
> +  - Balakrishnan Sambath <balakrishnan.s@microchip.com>
> +
> +description:
> +  The Microchip PIO4 controller is used to select the function of a pin and to
> +  configure it.
> +
> +

One blank line only.

> +properties:
> +  compatible:
> +    enum:
> +      - microchip,sama7g5-pinctrl
> +      - atmel,sama5d2-pinctrl

Keep them alphabetically ordered.

> +
> +  reg:
> +    minItems: 1
> +    maxItems: 2

You need to describe items instead. And why is this flexible?

> +
> +  interrupts:
> +    description:
> +      Interrupt outputs from the controller, one for each bank.

maxItems

> +
> +  interrupt-controller: true
> +
> +  '#interrupt-cells':
> +    const: 2
> +
> +  gpio-controller: true
> +
> +  '#gpio-cells':
> +    const: 2
> +
> +  clocks:
> +    maxItems: 1

Missing blank line.


> +if:

Missing allOf: and then put it how it is in example schema.

> +  properties:
> +    compatible:
> +      contains:
> +        const: microchip,sama7g5-pinctrl
> +then:
> +  patternProperties:
> +    '^.*([-_]default)?$':

No, properties must be defined in top level, not in if:then:. Also
underscore should not be allowed.

> +      anyOf:
> +        - $ref: "#/$defs/mchp-pio4-pincfg-node-1"
> +        - patternProperties:
> +            '^[a-z_-][a-z_-]*$':

Both regexes are way too permissive. Look how other bindings do it.
Usually these are -pins or -group.

> +              $ref: "#/$defs/mchp-pio4-pincfg-node-1"
> +else:
> +  patternProperties:
> +    '^.*([-_]default)?$':
> +      anyOf:
> +        - $ref: "#/$defs/mchp-pio4-pincfg-node-2"
> +        - patternProperties:
> +            '^[a-z_-][a-z_-]*$':
> +              $ref: "#/$defs/mchp-pio4-pincfg-node-2"
> +
> +$defs:
> +  mchp-pio4-pincfg-node-1:
> +    $ref: pincfg-node.yaml#properties
> +    properties:
> +      pinmux:
> +        $ref: pinmux-node.yaml#/properties/pinmux
> +      atmel,drive-strength:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        enum: [0, 1, 2, 3]
> +        default: 0
> +    required:
> +      - pinmux
> +
> +  mchp-pio4-pincfg-node-2:
> +    $ref: pincfg-node.yaml#properties
> +    properties:
> +      pinmux:
> +        $ref: pinmux-node.yaml#/properties/pinmux
> +    required:
> +      - pinmux
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - interrupt-controller
> +  - '#interrupt-cells'
> +  - gpio-controller
> +  - '#gpio-cells'
> +
> +unevaluatedProperties: false

Nope, this must be additionalProperties: false. If you put here
unevaluated, it's a sign you define properties not in correct spot.

> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +    #include <dt-bindings/clock/at91.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/pinctrl/sama5d2-pinfunc.h>
> +
> +    pinctrl@fc038000 {
> +        compatible = "atmel,sama5d2-pinctrl";
> +        reg = <0xfc038000 0x600>;
> +        interrupts = <18 IRQ_TYPE_LEVEL_HIGH 7>,
> +                     <68 IRQ_TYPE_LEVEL_HIGH 7>,
> +                     <69 IRQ_TYPE_LEVEL_HIGH 7>,
> +                     <70 IRQ_TYPE_LEVEL_HIGH 7>;
> +        interrupt-controller;
> +        #interrupt-cells = <2>;
> +        gpio-controller;
> +        #gpio-cells = <2>;
> +        clocks = <&pioA_clk>;
> +
> +        pinctrl_i2c0_default: i2c0_default {

Underscores are not allowed. Please open DTS coding style. Or test your
DTS with W=2.



Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/pinctrl/atmel,at91-pio4-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/atmel,at91-pio4-pinctrl.txt
deleted file mode 100644
index 774c3c269c40..000000000000
--- a/Documentation/devicetree/bindings/pinctrl/atmel,at91-pio4-pinctrl.txt
+++ /dev/null
@@ -1,98 +0,0 @@ 
-* Atmel PIO4 Controller
-
-The Atmel PIO4 controller is used to select the function of a pin and to
-configure it.
-
-Required properties:
-- compatible:
-	"atmel,sama5d2-pinctrl"
-	"microchip,sama7g5-pinctrl"
-- reg: base address and length of the PIO controller.
-- interrupts: interrupt outputs from the controller, one for each bank.
-- interrupt-controller: mark the device node as an interrupt controller.
-- #interrupt-cells: should be two.
-- gpio-controller: mark the device node as a gpio controller.
-- #gpio-cells: should be two.
-
-Please refer to ../gpio/gpio.txt and ../interrupt-controller/interrupts.txt for
-a general description of GPIO and interrupt bindings.
-
-Please refer to pinctrl-bindings.txt in this directory for details of the
-common pinctrl bindings used by client devices.
-
-Subnode format
-Each node (or subnode) will list the pins it needs and how to configured these
-pins.
-
-	node {
-		pinmux = <PIN_NUMBER_PINMUX>;
-		GENERIC_PINCONFIG;
-	};
-
-Required properties:
-- pinmux: integer array. Each integer represents a pin number plus mux and
-ioset settings. Use the macros from boot/dts/<soc>-pinfunc.h file to get the
-right representation of the pin.
-
-Optional properties:
-- GENERIC_PINCONFIG: generic pinconfig options to use:
-	- bias-disable, bias-pull-down, bias-pull-up, drive-open-drain,
-	 drive-push-pull input-schmitt-enable, input-debounce, output-low,
-	 output-high.
-	- for microchip,sama7g5-pinctrl only:
-		- slew-rate: 0 - disabled, 1 - enabled (default)
-- atmel,drive-strength: 0 or 1 for low drive, 2 for medium drive and 3 for
-high drive. The default value is low drive.
-
-Example:
-
-#include <sama5d2-pinfunc.h>
-
-...
-{
-	pioA: pinctrl@fc038000 {
-		compatible = "atmel,sama5d2-pinctrl";
-		reg = <0xfc038000 0x600>;
-		interrupts = <18 IRQ_TYPE_LEVEL_HIGH 7>,
-			     <68 IRQ_TYPE_LEVEL_HIGH 7>,
-			     <69 IRQ_TYPE_LEVEL_HIGH 7>,
-			     <70 IRQ_TYPE_LEVEL_HIGH 7>;
-		interrupt-controller;
-		#interrupt-cells = <2>;
-		gpio-controller;
-		#gpio-cells = <2>;
-		clocks = <&pioA_clk>;
-
-		pinctrl_i2c0_default: i2c0_default {
-			pinmux = <PIN_PD21__TWD0>,
-				 <PIN_PD22__TWCK0>;
-			bias-disable;
-		};
-
-		pinctrl_led_gpio_default: led_gpio_default {
-			pinmux = <PIN_PB0>,
-				 <PIN_PB5>;
-			bias-pull-up;
-			atmel,drive-strength = <ATMEL_PIO_DRVSTR_ME>;
-		};
-
-		pinctrl_sdmmc1_default: sdmmc1_default {
-			cmd_data {
-				pinmux = <PIN_PA28__SDMMC1_CMD>,
-					 <PIN_PA18__SDMMC1_DAT0>,
-					 <PIN_PA19__SDMMC1_DAT1>,
-					 <PIN_PA20__SDMMC1_DAT2>,
-					 <PIN_PA21__SDMMC1_DAT3>;
-				bias-pull-up;
-			};
-
-			ck_cd {
-				pinmux = <PIN_PA22__SDMMC1_CK>,
-					 <PIN_PA30__SDMMC1_CD>;
-				bias-disable;
-			};
-		};
-		...
-	};
-};
-...
diff --git a/Documentation/devicetree/bindings/pinctrl/atmel,sama5d2-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/atmel,sama5d2-pinctrl.yaml
new file mode 100644
index 000000000000..8a2dee1d6dd3
--- /dev/null
+++ b/Documentation/devicetree/bindings/pinctrl/atmel,sama5d2-pinctrl.yaml
@@ -0,0 +1,140 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pinctrl/atmel,sama5d2-pinctrl.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Microchip PIO4 Controller
+
+maintainers:
+  - Balakrishnan Sambath <balakrishnan.s@microchip.com>
+
+description:
+  The Microchip PIO4 controller is used to select the function of a pin and to
+  configure it.
+
+
+properties:
+  compatible:
+    enum:
+      - microchip,sama7g5-pinctrl
+      - atmel,sama5d2-pinctrl
+
+  reg:
+    minItems: 1
+    maxItems: 2
+
+  interrupts:
+    description:
+      Interrupt outputs from the controller, one for each bank.
+
+  interrupt-controller: true
+
+  '#interrupt-cells':
+    const: 2
+
+  gpio-controller: true
+
+  '#gpio-cells':
+    const: 2
+
+  clocks:
+    maxItems: 1
+if:
+  properties:
+    compatible:
+      contains:
+        const: microchip,sama7g5-pinctrl
+then:
+  patternProperties:
+    '^.*([-_]default)?$':
+      anyOf:
+        - $ref: "#/$defs/mchp-pio4-pincfg-node-1"
+        - patternProperties:
+            '^[a-z_-][a-z_-]*$':
+              $ref: "#/$defs/mchp-pio4-pincfg-node-1"
+else:
+  patternProperties:
+    '^.*([-_]default)?$':
+      anyOf:
+        - $ref: "#/$defs/mchp-pio4-pincfg-node-2"
+        - patternProperties:
+            '^[a-z_-][a-z_-]*$':
+              $ref: "#/$defs/mchp-pio4-pincfg-node-2"
+
+$defs:
+  mchp-pio4-pincfg-node-1:
+    $ref: pincfg-node.yaml#properties
+    properties:
+      pinmux:
+        $ref: pinmux-node.yaml#/properties/pinmux
+      atmel,drive-strength:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        enum: [0, 1, 2, 3]
+        default: 0
+    required:
+      - pinmux
+
+  mchp-pio4-pincfg-node-2:
+    $ref: pincfg-node.yaml#properties
+    properties:
+      pinmux:
+        $ref: pinmux-node.yaml#/properties/pinmux
+    required:
+      - pinmux
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - interrupt-controller
+  - '#interrupt-cells'
+  - gpio-controller
+  - '#gpio-cells'
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/clock/at91.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/pinctrl/sama5d2-pinfunc.h>
+
+    pinctrl@fc038000 {
+        compatible = "atmel,sama5d2-pinctrl";
+        reg = <0xfc038000 0x600>;
+        interrupts = <18 IRQ_TYPE_LEVEL_HIGH 7>,
+                     <68 IRQ_TYPE_LEVEL_HIGH 7>,
+                     <69 IRQ_TYPE_LEVEL_HIGH 7>,
+                     <70 IRQ_TYPE_LEVEL_HIGH 7>;
+        interrupt-controller;
+        #interrupt-cells = <2>;
+        gpio-controller;
+        #gpio-cells = <2>;
+        clocks = <&pioA_clk>;
+
+        pinctrl_i2c0_default: i2c0_default {
+            pinmux = <PIN_PD21__TWD0>,
+                     <PIN_PD22__TWCK0>;
+            bias-disable;
+        };
+
+        pinctrl_sdmmc1_default: sdmmc1_default {
+            cmd_data {
+                pinmux = <PIN_PA28__SDMMC1_CMD>,
+                         <PIN_PA18__SDMMC1_DAT0>,
+                         <PIN_PA19__SDMMC1_DAT1>,
+                         <PIN_PA20__SDMMC1_DAT2>,
+                         <PIN_PA21__SDMMC1_DAT3>;
+                bias-pull-up;
+            };
+
+            ck_cd {
+                pinmux = <PIN_PA22__SDMMC1_CK>,
+                         <PIN_PA30__SDMMC1_CD>;
+                bias-disable;
+            };
+        };
+    };
+...