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

Changhuang Liang changhuang.liang at starfivetech.com
Fri May 15 01:53:04 PDT 2026


Hi, Conor

Thank for the comments

> On Fri, May 15, 2026 at 06:10:28AM +0000, Changhuang Liang wrote:
> > Hi, Conor
> >
> > Thanks for the review.
> >
> > > 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 at 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-pin
> > > > ctrl
> > > > .yaml  create mode 100644
> > > > include/dt-bindings/pinctrl/starfive,jhb100-pinctrl.h
> > > >
> > > > diff --git
> > > > a/Documentation/devicetree/bindings/pinctrl/starfive,jhb100-sys0-p
> > > > inct
> > > > rl.yaml
> > > > b/Documentation/devicetree/bindings/pinctrl/starfive,jhb100-sys0-p
> > > > inct
> > > > rl.yaml
> > > > new file mode 100644
> > > > index 000000000000..21d3693587fd
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/pinctrl/starfive,jhb100-sy
> > > > +++ s0-p
> > > > +++ inctrl.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-pinctr
> > > > +l.ya
> > > > +ml#
> > > > +$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 at starfivetech.com>
> > >
> > > Why is Alex the maintainer when you are the sole author?

I will change it to myself. Alex has already resigned.

> > >
> > > > +
> > > > +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 at 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 at 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?
> >
> > However, the current series of drivers will use some of these definitions.
> >
> > Copied from patch 12:
> > +static const struct pinvref_desc pinvref_desc_sys2[] = {
> > +	{
> > +		.name = "gpiow0",
> > +		.pin_grp = {
> > +			PADNUM_SYS2_GPIO_A36,
> > +			PADNUM_SYS2_GPIO_A37,
> > +			PADNUM_SYS2_GPIO_A38,
> > +			PADNUM_SYS2_GPIO_A39
> > +		},
> > +		.num_pins = 4,
> > +		.range = BIT(JHB100_PINVREF_1_8V) | BIT(JHB100_PINVREF_3_3V)
> 
> Can you explain why you need something like this when you're using pins and
> functions? These look a lot like your own home-rolled groups, that exist just to
> set the range of permitted values for vref?

This part of the voltage configuration only has one register, but it applies to A36/A37/A38/A39.
The range field is used to restrict the valid voltages that can be configured.

> As far as I can tell, the only controller that supports something other than 1.8v
> and 3.3v is "per2", so you should be able to apply the restrictions here entirely
> in the dt-binding?

Okay, I will provide more details about the power source and set limits on the supported voltage values.

Best Regards,
Changhuang

> I notice you have
> > > > +          power-source:
> > > > +             enum: [ 0, 1, 2 ]
> in all bindings, but no explanation of what the values are. That needs to
> change, and you should not permit 1 in anything other than "per2", since that
> appears to be the only user.
> 
> Also, you've not responded to my other comments, one of which was a
> question. Why not?
> 
> Thanks,
> Conor.
> 
> > +	},
> > +	{
> > +		.name = "gpiow-inner",
> > +		.pin_grp = {
> > +			PADNUM_SYS2_GPIO_A40,
> > +			PADNUM_SYS2_GPIO_A41,
> > +			PADNUM_SYS2_GPIO_A42,
> > +			PADNUM_SYS2_GPIO_A43
> > +		},
> > +		.num_pins = 4,
> > +		.range = BIT(JHB100_PINVREF_1_8V) | BIT(JHB100_PINVREF_3_3V)
> > +	},
> > +	{ NULL },
> > +};
> >
> > Therefore, I have uniformly created
> > include/dt-bindings/pinctrl/starfive,jhb100-pinctrl.h
> >
> > Best Regards,
> > Changhuang
> >



More information about the linux-riscv mailing list