On Mon, 28 Nov 2022 at 02:04, Jianlong Huang <jianlong.huang@xxxxxxxxxxxxxxxx> wrote: > > On 2022/11/21 16:43, Krzysztof Kozlowski wrote: > > On 18/11/2022 02:11, Hal Feng wrote: > >> From: Jianlong Huang <jianlong.huang@xxxxxxxxxxxxxxxx> > >> > >> Add pinctrl bindings for StarFive JH7110 SoC sys pinctrl controller. > >> > >> Signed-off-by: Jianlong Huang <jianlong.huang@xxxxxxxxxxxxxxxx> > >> Signed-off-by: Hal Feng <hal.feng@xxxxxxxxxxxxxxxx> > >> --- > >> .../pinctrl/starfive,jh7110-sys-pinctrl.yaml | 165 ++++++++++++++++++ > >> 1 file changed, 165 insertions(+) > >> create mode 100644 Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml > >> > >> diff --git a/Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml > >> new file mode 100644 > >> index 000000000000..79623f884a9c > >> --- /dev/null > >> +++ b/Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml > >> @@ -0,0 +1,165 @@ > >> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > >> +%YAML 1.2 > >> +--- > >> +$id: http://devicetree.org/schemas/pinctrl/starfive,jh7110-sys-pinctrl.yaml# > >> +$schema: http://devicetree.org/meta-schemas/core.yaml# > >> + > >> +title: StarFive JH7110 Sys Pin Controller > >> + > >> +description: | > >> + Bindings for the JH7110 RISC-V SoC from StarFive Technology Ltd. > >> + > >> + Out of the SoC's many pins only the ones named PAD_GPIO0 to PAD_GPIO63 > >> + can be multiplexed and have configurable bias, drive strength, > >> + schmitt trigger etc. > >> + Some peripherals have their I/O go through the 64 "GPIOs". This also > >> + includes a number of other UARTs, I2Cs, SPIs, PWMs etc. > >> + All these peripherals are connected to all 64 GPIOs such that > >> + any GPIO can be set up to be controlled by any of the peripherals. > >> + > >> +maintainers: > >> + - Jianlong Huang <jianlong.huang@xxxxxxxxxxxxxxxx> > >> + > >> +properties: > >> + compatible: > >> + const: starfive,jh7110-sys-pinctrl > >> + > >> + reg: > >> + maxItems: 1 > >> + > >> + reg-names: > >> + items: > >> + - const: control > > > > Why reg-names for one entry? Perhaps just drop it. > > Will fix, drop it. > > > > >> + > >> + clocks: > >> + maxItems: 1 > >> + > >> + resets: > >> + maxItems: 1 > >> + > >> + gpio-controller: true > >> + > >> + "#gpio-cells": > >> + const: 2 > >> + > >> + interrupts: > >> + maxItems: 1 > >> + description: The GPIO parent interrupt. > > > > Drop description, it's obvious. > > Will fix, drop it. > > > > >> + > >> + interrupt-controller: true > >> + > >> + "#interrupt-cells": > >> + const: 2 > >> + > >> +required: > >> + - compatible > >> + - reg > >> + - reg-names > >> + - clocks > >> + - gpio-controller > >> + - "#gpio-cells" > >> + - interrupts > >> + - interrupt-controller > >> + - "#interrupt-cells" > >> + > >> +patternProperties: > >> + '-[0-9]+$': > > > > Keep consistent quotes, either ' or " > > > > How do you differentiate hogs? The pattern is a bit unspecific. > > Will fix. > Keep consisitent quotes, use ' > > > > >> + 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: > >> + pinmux: > >> + description: | > >> + The list of GPIOs and their mux settings that properties in the > >> + node apply to. This should be set using the GPIOMUX macro. > >> + $ref: "/schemas/pinctrl/pinmux-node.yaml#/properties/pinmux" > > > > Drop quotes. > > Will fix, drop quotes. > > > > >> + > >> + bias-disable: true > >> + > >> + bias-pull-up: > >> + type: boolean > >> + > >> + bias-pull-down: > >> + type: boolean > >> + > >> + drive-strength: > >> + enum: [ 2, 4, 8, 12 ] > >> + > >> + input-enable: true > >> + > >> + input-disable: true > >> + > >> + input-schmitt-enable: true > >> + > >> + input-schmitt-disable: true > >> + > >> + slew-rate: > >> + maximum: 1 > >> + > >> + additionalProperties: false > >> + > >> + additionalProperties: false > >> + > >> +additionalProperties: false > >> + > >> +examples: > >> + - | > >> + #include <dt-bindings/clock/starfive-jh7110.h> > >> + #include <dt-bindings/reset/starfive-jh7110.h> > >> + #include <dt-bindings/pinctrl/pinctrl-starfive-jh7110.h> > >> + > >> + soc { > > > > Use 4 spaces for example indentation. > > Will fix. > > > > >> + #address-cells = <2>; > >> + #size-cells = <2>; You can also drop these to lines.. > >> + > >> + gpio: gpio@13040000 { > >> + compatible = "starfive,jh7110-sys-pinctrl"; > >> + reg = <0x0 0x13040000 0x0 0x10000>; ..and then just make this reg = <0x13040000 0x10000>; > >> + reg-names = "control"; > >> + clocks = <&syscrg_clk JH7110_SYSCLK_IOMUX>; > >> + resets = <&syscrg_rst JH7110_SYSRST_IOMUX>; > >> + interrupts = <86>; > >> + interrupt-controller; > >> + #interrupt-cells = <2>; > >> + #gpio-cells = <2>; > >> + gpio-controller; > >> + status = "okay"; > > > > No status in examples. > > Will fix, drop it. > > > > >> + > >> + uart0_pins: uart0-0 { > >> + tx-pins { > >> + pinmux = <GPIOMUX(5, GPOUT_SYS_UART0_TX, GPOEN_ENABLE, GPI_NONE)>; > >> + bias-disable; > >> + drive-strength = <12>; > >> + input-disable; > >> + input-schmitt-disable; > >> + slew-rate = <0>; > >> + }; > >> + > >> + rx-pins { > >> + pinmux = <GPIOMUX(6, GPOUT_LOW, GPOEN_DISABLE, GPI_SYS_UART0_RX)>; > >> + bias-pull-up; > >> + drive-strength = <2>; > >> + input-enable; > >> + input-schmitt-enable; > >> + slew-rate = <0>; > >> + }; > >> + }; > >> + }; > >> + > >> + uart0 { > >> + pinctrl-names = "default"; > >> + pinctrl-0 = <&uart0_pins>; > >> + status = "okay"; > > > > Drop this node, useless. > > Will fix, drop this node. > > > > >> + }; > >> + }; > >> + > >> +... > > > > Best regards, > Jianlong Huang > >
