| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <44877f61-8f23-4334-90ae-35545c76c1a0@linaro.org>
Date: Thu, 29 Feb 2024 18:41:14 +0100
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: Balakrishnan Sambath <balakrishnan.s@...rochip.com>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Conor Dooley <conor+dt@...nel.org>,
Nicolas Ferre <nicolas.ferre@...rochip.com>,
Alexandre Belloni <alexandre.belloni@...tlin.com>,
Claudiu Beznea <claudiu.beznea@...on.dev>,
Linus Walleij <linus.walleij@...aro.org>
Cc: devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-kernel@...r.kernel.org, linux-gpio@...r.kernel.org
Subject: Re: [PATCH 3/3] dt-bindings: pinctrl: at91-pio4: convert Atmel's PIO4
bindings to json-schema
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@...rochip.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@...rochip.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@...38000 {
> + 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
Powered by blists - more mailing lists