On 30/09/2022 09:38, Hal Feng wrote: > From: Jianlong Huang <jianlong.huang@xxxxxxxxxxxxxxxx> > > Add pinctrl bindings for StarFive JH7110 SoC. > > Signed-off-by: Jianlong Huang <jianlong.huang@xxxxxxxxxxxxxxxx> > Signed-off-by: Hal Feng <hal.feng@xxxxxxxxxxxxxxxxxxxxxx> > --- > .../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@xxxxxxxxxxxxxxxx> > + > +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@13040000 { > + 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