2022-11-18 01:58:55

by Hal Feng

[permalink] [raw]
Subject: [PATCH v2 2/5] dt-bindings: pinctrl: Add StarFive JH7110 sys pinctrl

From: Jianlong Huang <[email protected]>

Add pinctrl bindings for StarFive JH7110 SoC sys pinctrl controller.

Signed-off-by: Jianlong Huang <[email protected]>
Signed-off-by: Hal Feng <[email protected]>
---
.../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 <[email protected]>
+
+properties:
+ compatible:
+ const: starfive,jh7110-sys-pinctrl
+
+ reg:
+ maxItems: 1
+
+ reg-names:
+ items:
+ - const: control
+
+ clocks:
+ maxItems: 1
+
+ resets:
+ maxItems: 1
+
+ gpio-controller: true
+
+ "#gpio-cells":
+ const: 2
+
+ interrupts:
+ maxItems: 1
+ description: The GPIO parent interrupt.
+
+ 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]+$':
+ 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"
+
+ 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 {
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ gpio: gpio@13040000 {
+ compatible = "starfive,jh7110-sys-pinctrl";
+ reg = <0x0 0x13040000 0x0 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";
+
+ 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";
+ };
+ };
+
+...
--
2.38.1



2022-11-18 04:31:35

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v2 2/5] dt-bindings: pinctrl: Add StarFive JH7110 sys pinctrl


On Fri, 18 Nov 2022 09:11:05 +0800, Hal Feng wrote:
> From: Jianlong Huang <[email protected]>
>
> Add pinctrl bindings for StarFive JH7110 SoC sys pinctrl controller.
>
> Signed-off-by: Jianlong Huang <[email protected]>
> Signed-off-by: Hal Feng <[email protected]>
> ---
> .../pinctrl/starfive,jh7110-sys-pinctrl.yaml | 165 ++++++++++++++++++
> 1 file changed, 165 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml
>

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/starfive,jh7110-sys-pinctrl.example.dts:21:18: fatal error: dt-bindings/clock/starfive-jh7110.h: No such file or directory
21 | #include <dt-bindings/clock/starfive-jh7110.h>
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
make[1]: *** [scripts/Makefile.lib:406: Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.example.dtb] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1492: dt_binding_check] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/[email protected]

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

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.


2022-11-18 07:32:26

by Hal Feng

[permalink] [raw]
Subject: Re: [PATCH v2 2/5] dt-bindings: pinctrl: Add StarFive JH7110 sys pinctrl

On Thu, 17 Nov 2022 21:56:14 -0600, Rob Herring wrote:
> 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/starfive,jh7110-sys-pinctrl.example.dts:21:18: fatal error: dt-bindings/clock/starfive-jh7110.h: No such file or directory
> 21 | #include <dt-bindings/clock/starfive-jh7110.h>
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> compilation terminated.
> make[1]: *** [scripts/Makefile.lib:406: Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.example.dtb] Error 1
> make[1]: *** Waiting for unfinished jobs....
> make: *** [Makefile:1492: dt_binding_check] Error 2
>
> doc reference errors (make refcheckdocs):
>
> See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/[email protected]
>
> This check can fail if there are any dependencies. The base for a patch
> series is generally the most recent rc1.

This patch series depends on the patch series [1].

[1] https://lore.kernel.org/all/[email protected]/

Best regards,
Hal

>
> 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.


2022-11-21 08:56:10

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2 2/5] dt-bindings: pinctrl: Add StarFive JH7110 sys pinctrl

On 18/11/2022 02:11, Hal Feng wrote:
> From: Jianlong Huang <[email protected]>
>
> Add pinctrl bindings for StarFive JH7110 SoC sys pinctrl controller.
>
> Signed-off-by: Jianlong Huang <[email protected]>
> Signed-off-by: Hal Feng <[email protected]>
> ---
> .../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 <[email protected]>
> +
> +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.

> +
> + 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.

> +
> + 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.

> + 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.

> +
> + 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.

> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + gpio: gpio@13040000 {
> + compatible = "starfive,jh7110-sys-pinctrl";
> + reg = <0x0 0x13040000 0x0 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.

> +
> + 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.

> + };
> + };
> +
> +...

Best regards,
Krzysztof


2022-11-28 01:13:58

by Jianlong Huang

[permalink] [raw]
Subject: Re: [PATCH v2 2/5] dt-bindings: pinctrl: Add StarFive JH7110 sys pinctrl

On 2022/11/21 16:43, Krzysztof Kozlowski wrote:
> On 18/11/2022 02:11, Hal Feng wrote:
>> From: Jianlong Huang <[email protected]>
>>
>> Add pinctrl bindings for StarFive JH7110 SoC sys pinctrl controller.
>>
>> Signed-off-by: Jianlong Huang <[email protected]>
>> Signed-off-by: Hal Feng <[email protected]>
>> ---
>> .../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 <[email protected]>
>> +
>> +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>;
>> +
>> + gpio: gpio@13040000 {
>> + compatible = "starfive,jh7110-sys-pinctrl";
>> + reg = <0x0 0x13040000 0x0 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


2022-12-07 13:24:10

by Emil Renner Berthing

[permalink] [raw]
Subject: Re: [PATCH v2 2/5] dt-bindings: pinctrl: Add StarFive JH7110 sys pinctrl

On Mon, 28 Nov 2022 at 02:04, Jianlong Huang
<[email protected]> wrote:
>
> On 2022/11/21 16:43, Krzysztof Kozlowski wrote:
> > On 18/11/2022 02:11, Hal Feng wrote:
> >> From: Jianlong Huang <[email protected]>
> >>
> >> Add pinctrl bindings for StarFive JH7110 SoC sys pinctrl controller.
> >>
> >> Signed-off-by: Jianlong Huang <[email protected]>
> >> Signed-off-by: Hal Feng <[email protected]>
> >> ---
> >> .../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 <[email protected]>
> >> +
> >> +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
>
>

2022-12-09 03:58:44

by Icenowy Zheng

[permalink] [raw]
Subject: Re: [PATCH v2 2/5] dt-bindings: pinctrl: Add StarFive JH7110 sys pinctrl

在 2022-11-18星期五的 09:11 +0800,Hal Feng写道:
> From: Jianlong Huang <[email protected]>
>
> Add pinctrl bindings for StarFive JH7110 SoC sys pinctrl controller.
>
> Signed-off-by: Jianlong Huang <[email protected]>
> Signed-off-by: Hal Feng <[email protected]>
> ---
>  .../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 <[email protected]>
> +
> +properties:
> +  compatible:
> +    const: starfive,jh7110-sys-pinctrl
> +
> +  reg:
> +    maxItems: 1
> +
> +  reg-names:
> +    items:
> +      - const: control
> +
> +  clocks:
> +    maxItems: 1
> +
> +  resets:
> +    maxItems: 1
> +
> +  gpio-controller: true
> +
> +  "#gpio-cells":
> +    const: 2
> +
> +  interrupts:
> +    maxItems: 1
> +    description: The GPIO parent interrupt.
> +
> +  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]+$':
> +    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.

Could this handle hard wiring an internal input mux function to high or
low?

This feature is needed on the Star64 board to omit the USB overcurrent
pin.

> +
> +        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"
> +
> +          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 {
> +                #address-cells = <2>;
> +                #size-cells = <2>;
> +
> +                gpio: gpio@13040000 {
> +                        compatible = "starfive,jh7110-sys-pinctrl";
> +                        reg = <0x0 0x13040000 0x0 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";
> +
> +                        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";
> +                };
> +        };
> +
> +...