Re: [PATCH v2 04/22] dt-bindings: pinctrl: Add starfive,jhb100-sys0-pinctrl

[Conor Dooley <conor@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 7765 bytes --]

On Thu, May 14, 2026 at 04:12:00AM -0700, Changhuang Liang wrote:
> Add pinctrl bindings for StarFive JHB100 SoC System-0(sys0) pinctrl
> controller.
> 
> Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
> ---
>  .../pinctrl/starfive,jhb100-sys0-pinctrl.yaml | 175 ++++++++++++++++++
>  .../pinctrl/starfive,jhb100-pinctrl.h         |  17 ++
>  2 files changed, 192 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pinctrl/starfive,jhb100-sys0-pinctrl.yaml
>  create mode 100644 include/dt-bindings/pinctrl/starfive,jhb100-pinctrl.h
> 
> diff --git a/Documentation/devicetree/bindings/pinctrl/starfive,jhb100-sys0-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/starfive,jhb100-sys0-pinctrl.yaml
> new file mode 100644
> index 000000000000..21d3693587fd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pinctrl/starfive,jhb100-sys0-pinctrl.yaml
> @@ -0,0 +1,175 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pinctrl/starfive,jhb100-sys0-pinctrl.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: StarFive JHB100 System-0 Pin Controller
> +
> +description: |
> +  Pinctrl bindings for JHB100 RISC-V SoC from StarFive Technology Ltd.
> +
> +  The JHB100 SoC has 13 pinctrl domains - sys0, sys0h, sys1, sys2, per0, per1,
> +  per2, per2pok, per3, adc0, adc1, emmc, and vga.
> +  This document provides an overview of the "sys0" pinctrl domain.
> +
> +  The "sys0" domain has a pin controller which provides
> +  - function selection for GPIO pads.
> +  - GPIO pad configuration.
> +  - GPIO interrupt handling.
> +
> +  In the SYS0 Pin Controller, there are 4 multi-function GPIO_PADs. Each of
> +  them can be multiplexed to different hardware blocks through function
> +  selection and each iopad has a maximum of up to 2 functions - 0 and 1.
> +  Function 0 is the default function which is generally the GPIO function
> +  (or occasionally, it can be a peripheral signal).
> +  Function 1 is the alternate function or peripheral signal that can be
> +  routed to the iopad. The function selection is carried out by writing
> +  the function number to the iopad function select register.
> +
> +  Each iopad is configurable with parameters such as input-enable, internal
> +  pull-up/pull-down bias, drive strength, schmitt trigger, slew rate,  input
> +  debounce nanoseconds, power source and drive type  (open-drain or push-pull).
> +
> +maintainers:
> +  - Alex Soo <yuklin.soo@starfivetech.com>

Why is Alex the maintainer when you are the sole author?

> +
> +properties:
> +  compatible:
> +    items:
> +      - const: starfive,jhb100-sys0-pinctrl
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +  resets:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  interrupt-controller: true
> +
> +  '#interrupt-cells':
> +    const: 3
> +
> +  gpio-controller: true
> +
> +  '#gpio-cells':
> +    const: 3
> +
> +  gpio-ranges: true
> +
> +  gpio-line-names: true
> +
> +patternProperties:
> +  '-grp$':
> +    type: object
> +    additionalProperties: false
> +    patternProperties:
> +      '-pins$':
> +        type: object
> +        description: |
> +          A pinctrl node should contain at least one subnode representing the
> +          pinctrl groups available in the domain. Each subnode will list the
> +          pins it needs, and how they should be configured, with regard to
> +          function selection, bias, input enable/disable, input schmitt
> +          trigger enable/disable, slew-rate, input debounce nanoseconds,
> +          drive-open-drain, drive-push-pull, power-source and drive-strength.
> +        allOf:
> +          - $ref: /schemas/pinctrl/pincfg-node.yaml
> +          - $ref: /schemas/pinctrl/pinmux-node.yaml
> +        unevaluatedProperties: false

I think this should be additionalProperties, since you're citing all the
properties you do support below.

> +
> +        properties:
> +          pins:
> +            description:
> +              The list of IOs that properties in the pincfg node apply to.
> +
> +          function:
> +            description:
> +              A string containing the name of the function to mux for these
> +              pins.
> +            enum: [ auxpwrgood, gpio, hbled, pe2rst_out ]
> +
> +          bias-disable: true
> +
> +          bias-pull-down:
> +            type: boolean
> +
> +          bias-pull-up:
> +            oneOf:
> +              - type: boolean
> +              - enum: [ 600, 900, 1200, 2000 ]
> +                description: Pull up RSEL type resistance values (in ohms)
> +            description:
> +              For normal pull up type there is no need to specify a resistance
> +              value, hence this can be specified as a boolean property.
> +              For RSEL pull up type a resistance value (in ohms) can be added.
> +
> +          drive-open-drain: true
> +
> +          drive-push-pull: true
> +
> +          drive-strength:
> +            enum: [ 2, 4, 8, 12 ]
> +
> +          drive-strength-microamp:
> +            enum: [ 2000, 4000, 8000, 12000 ]
> +
> +          input-debounce-nanoseconds:
> +            minimum: 0
> +            maximum: 4294967295
> +
> +          input-disable: true
> +
> +          input-enable: true
> +
> +          input-schmitt-enable: true
> +
> +          input-schmitt-disable: true
> +
> +          power-source:
> +             enum: [ 0, 1, 2 ]
> +
> +          slew-rate:
> +            enum: [ 0, 1 ]
> +            default: 0
> +            description: |
> +                0: slow (half frequency)
> +                1: fast
> +
> +required:
> +  - compatible
> +  - reg
> +  - resets
> +  - interrupts
> +  - interrupt-controller
> +  - '#interrupt-cells'
> +  - gpio-controller
> +  - '#gpio-cells'
> +  - gpio-ranges
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    soc {
> +        #address-cells = <2>;
> +        #size-cells = <2>;
> +
> +        pinctrl_sys0: pinctrl@13080000 {
> +            compatible = "starfive,jhb100-sys0-pinctrl";
> +            reg = <0x0 0x13080000 0x0 0x800>;
> +            resets = <&sys0crg 2>;
> +            interrupts = <56>;
> +            interrupt-controller;
> +            #interrupt-cells = <3>;
> +            gpio-controller;
> +            #gpio-cells = <3>;
> +            gpio-ranges = <&pinctrl_sys0 0 0 0 4>;
> +        };
> +    };
> diff --git a/include/dt-bindings/pinctrl/starfive,jhb100-pinctrl.h b/include/dt-bindings/pinctrl/starfive,jhb100-pinctrl.h
> new file mode 100644
> index 000000000000..6d8f5516a178
> --- /dev/null
> +++ b/include/dt-bindings/pinctrl/starfive,jhb100-pinctrl.h
> @@ -0,0 +1,17 @@
> +/* SPDX-License-Identifier: GPL-2.0 OR MIT */
> +/*
> + * Copyright (C) 2022 StarFive Technology Co., Ltd.
> + *
> + * Author: Changhuang Liang <changhuang.liang@starfivetech.com>
> + */
> +
> +#ifndef __DT_BINDINGS_PINCTRL_STARFIVE_JHB100_H__
> +#define __DT_BINDINGS_PINCTRL_STARFIVE_JHB100_H__
> +
> +/* sys0 pad numbers */
> +#define PADNUM_SYS0_GPIO_A0				0
> +#define PADNUM_SYS0_GPIO_A1				1
> +#define PADNUM_SYS0_GPIO_A2				2
> +#define PADNUM_SYS0_GPIO_A3				3

Does this provide any actual value? Across the whole series, most
numbers you put in this binding headers do not appear in any drivers
at all. Seems like these defines should appear in the dts directly?

pw-bot: changes-requested

Cheers,
Conor.

> +
> +#endif
> -- 
> 2.25.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]