| 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
| ||
|
Date: Fri, 30 Sep 2022 13:05:45 +0200
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: Hal Feng <hal.feng@...ux.starfivetech.com>,
linux-riscv@...ts.infradead.org, devicetree@...r.kernel.org,
linux-clk@...r.kernel.org, linux-gpio@...r.kernel.org
Cc: Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Paul Walmsley <paul.walmsley@...ive.com>,
Palmer Dabbelt <palmer@...belt.com>,
Albert Ou <aou@...s.berkeley.edu>,
Daniel Lezcano <daniel.lezcano@...aro.org>,
Thomas Gleixner <tglx@...utronix.de>,
Marc Zyngier <maz@...nel.org>,
Philipp Zabel <p.zabel@...gutronix.de>,
Stephen Boyd <sboyd@...nel.org>,
Michael Turquette <mturquette@...libre.com>,
Linus Walleij <linus.walleij@...aro.org>,
Emil Renner Berthing <kernel@...il.dk>,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 25/30] dt-bindings: pinctrl: Add StarFive JH7110
pinctrl bindings
On 30/09/2022 09:38, Hal Feng wrote:
> From: Jianlong Huang <jianlong.huang@...rfivetech.com>
>
> Add pinctrl bindings for StarFive JH7110 SoC.
>
> Signed-off-by: Jianlong Huang <jianlong.huang@...rfivetech.com>
> Signed-off-by: Hal Feng <hal.feng@...ux.starfivetech.com>
> ---
> .../pinctrl/starfive,jh7110-pinctrl.yaml | 202 ++++++++++++++++++
> 1 file changed, 202 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/pinctrl/starfive,jh7110-pinctrl.yaml
>
> diff --git a/Documentation/devicetree/bindings/pinctrl/starfive,jh7110-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/starfive,jh7110-pinctrl.yaml
> new file mode 100644
> index 000000000000..482012ad8a14
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pinctrl/starfive,jh7110-pinctrl.yaml
> @@ -0,0 +1,202 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pinctrl/starfive,jh7110-pinctrl.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: StarFive JH7110 Pin Controller Device Tree Bindings
> +
> +description: |
> + Bindings for the JH7110 RISC-V SoC from StarFive Technology Ltd.
> +
> +maintainers:
> + - Jianlong Huang <jianlong.huang@...rfivetech.com>
> +
> +properties:
> + compatible:
> + enum:
> + - starfive,jh7110-sys-pinctrl
> + - starfive,jh7110-aon-pinctrl
Wrong indentation.
Does not look like you tested the bindings. Please run `make
dt_binding_check` (see
Documentation/devicetree/bindings/writing-schema.rst for instructions).
> +
> + reg:
> + minItems: 2
No need.
> + maxItems: 2
> +
> + reg-names:
> + items:
> + - const: control
This does not match reg at all. Again - not tested.
> +
> + clocks:
> + maxItems: 1
> +
> + resets:
> + maxItems: 1
> +
> + "#gpio-cells":
> + const: 2
> +
> + interrupts:
> + maxItems: 1
> + description: The GPIO parent interrupt.
> +
> + interrupt-controller: true
> +
> + "#interrupt-cells":
> + const: 2
> +
> + ngpios:
> + enum:
> + - 64
> + - 4
Wrong indentation. Increasing order.
> +
> +required:
> + - compatible
> + - reg
> + - reg-names
> + - clocks
> + - "#gpio-cells"
> + - interrupts
> + - interrupt-controller
> + - "#interrupt-cells"
> +
> +patternProperties:
This goes before required block
> + '-[0-9]+$':
Too loose pattern. Need some more specific pattern. What do you exactly
match here?
Missing description.
> + type: object
> + patternProperties:
> + '-pins$':
> + type: object
> + description: |
> + A pinctrl node should contain at least one subnode representing the
> + pinctrl groups available on the machine. Each subnode will list the
> + pins it needs, and how they should be configured, with regard to
> + muxer configuration, system signal configuration, pin groups for
> + vin/vout module, pin voltage, mux functions for output, mux functions
> + for output enable, mux functions for input.
> +
> + properties:
> + starfive,pins:
No, use generic pinctrl bindings.
> + description: |
> + The list of pin identifiers that properties in the node apply to.
> + This should be set using the PAD_GPIOX macros.
> + This has to be specified.
> + $ref: /schemas/types.yaml#/definitions/uint32
> + minimum: 0
> + maximum: 63
> +
> + starfive,pinmux:
No, use generic pinctrl bindings.
> + description: |
> + The list of GPIOs and their mux functions that properties in the
> + node apply to. This should be set using the PAD_GPIOX_FUNC_SEL
> + macro with its value.
> + This is optional for some pins.
> + The value of PAD_GPIOX_FUNC_SEL macro can selects:
> + 0: GPIOX mux function 0,
> + 1: GPIOX mux function 1,
> + 2: GPIOX mux function 2.
> +
> + starfive,pin-ioconfig:
> + description: |
> + This is used to configure the core settings of system signals.
> + The combination of GPIO_IE or GPIO_DS or GPIO_PU or GPIO_PD or
> + GPIO_SLEW or GPIO_SMT or GPIO_POS.
> + $ref: /schemas/types.yaml#/definitions/uint32
> +
> + starfive,padmux:
> + description: |
> + The padmux is for vin/vout module to select pin groups.
> + 0: vout will be set at pins from PAD_GPIO7 to PAD_GPIO34,
> + when PAD_GPIOX_FUNC_SEL is set as 1.
> + vin will be set at pins from PAD_GPIO6 to PAD_GPIO20.
> + when PAD_GPIOX_FUNC_SEL is set as 2.
> + 1: vout will be set at pins from PAD_GPIO36 to PAD_GPIO63,
> + when PAD_GPIOX_FUNC_SEL is set as 1.
> + vin will be set at pins from PAD_GPIO21 to PAD_GPIO35.
> + when PAD_GPIOX_FUNC_SEL is set as 2.
> + 2: vin will be set at pins from PAD_GPIO36 to PAD_GPIO50,
> + when PAD_GPIOX_FUNC_SEL is set as 2
> + $ref: /schemas/types.yaml#/definitions/uint32
> + enum: [0, 1, 2]
> +
> + starfive,pin-syscon:
> + description: |
> + This is used to set pin voltage,
> + 0: 3.3V, 1: 2.5V, 2: 1.8V.
> + $ref: /schemas/types.yaml#/definitions/uint32
> + enum: [0, 1, 2]
> +
> + starfive,pin-gpio-dout:
No, use generic pinctrl bindings.
> + description: |
> + This is used to set their mux functions for output.
> + This should be set using the GPO_XXX macro,
> + such as GPO_LOW, GPO_UART0_SOUT.
> + $ref: /schemas/types.yaml#/definitions/uint32
> + minimum: 0
> + maximum: 107
> +
> + starfive,pin-gpio-doen:
No, use generic pinctrl bindings.
> + description: |
> + This is used to set their mux functions for output enable.
> + This should be set using the OEN_XXX macro,
> + such as OEN_LOW, OEN_I2C0_IC_CLK_OE.
> + $ref: /schemas/types.yaml#/definitions/uint32
> + minimum: 0
> + maximum: 49
> +
> + starfive,pin-gpio-din:
No, use generic pinctrl bindings.
> + description: |
> + This is used to set their mux functions for input.
> + This should be set using the GPI_XXX macro,
> + such as GPI_CAN0_CTRL_RXD, GPI_I2C0_IC_CLK_IN_A.
> + $ref: /schemas/types.yaml#/definitions/uint32
> + minimum: 0
> + maximum: 90
> +
> + additionalProperties: false
> +
> + additionalProperties: false
> +
> +additionalProperties: false
Missing allof to generic pinctrl bindings.
> +
> +examples:
> + - |
> + #include <dt-bindings/clock/starfive-jh7110-sys.h>
> + #include <dt-bindings/reset/starfive-jh7110.h>
> + #include <dt-bindings/pinctrl/pinctrl-starfive-jh7110.h>
> +
> + gpio: gpio@...40000 {
> + compatible = "starfive,jh7110-sys-pinctrl";
> + reg = <0x0 0x13040000 0x0 0x10000>;
> + reg-names = "control";
> + clocks = <&clkgen JH7110_SYS_IOMUX_PCLK>;
> + resets = <&rstgen RSTN_U0_SYS_IOMUX_PRESETN>;
> + interrupts = <86>;
> + interrupt-controller;
> + #gpio-cells = <2>;
> + ngpios = <64>;
> + status = "okay";
> +
> + uart0_pins: uart0-pins {
Explain me please how it can possible match your pattern: '-[0-9]+$':
You really wrote something which was not tested and cannot work.
> + uart0-pins-tx {
> + starfive,pins = <PAD_GPIO5>;
> + starfive,pin-ioconfig = <IO(GPIO_IE(1) | GPIO_DS(3))>;
> + starfive,pin-gpio-dout = <GPO_UART0_SOUT>;
> + starfive,pin-gpio-doen = <OEN_LOW>;
> + };
> +
> + uart0-pins-rx {
> + starfive,pins = <PAD_GPIO6>;
> + starfive,pinmux = <PAD_GPIO6_FUNC_SEL 0>;
> + starfive,pin-ioconfig = <IO(GPIO_IE(1) | GPIO_PU(1))>;
> + starfive,pin-gpio-doen = <OEN_HIGH>;
> + starfive,pin-gpio-din = <GPI_UART0_SIN>;
> + };
> + };
> + };
> +
> + &uart0 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&uart0_pins>;
> + status = "okay";
Drop, obvious.
> + };
> +
> +...
Best regards,
Krzysztof
Powered by blists - more mailing lists