Re: [PATCH 1/2] dt-bindings: pinctrl: renesas: Add DT bindings for RZ/V2M pinctrl

From: Geert Uytterhoeven <geert@linux-m68k.org>
To: Phil Edworthy <phil.edworthy@renesas.com>
Cc: Linus Walleij <linus.walleij@linaro.org>,
	Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>,
	Linux-Renesas <linux-renesas-soc@vger.kernel.org>,
	"open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>,
	"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
	<devicetree@vger.kernel.org>
Subject: Re: [PATCH 1/2] dt-bindings: pinctrl: renesas: Add DT bindings for RZ/V2M pinctrl
Date: Tue, 14 Jun 2022 14:10:22 +0200	[thread overview]
Message-ID: <CAMuHMdVyoCG7PNtk9b32JAnzq4R+EeLCxM-8owuphhD=KFS=Ow@mail.gmail.com> (raw)
In-Reply-To: <20220520154051.29088-2-phil.edworthy@renesas.com>

Hi Phil,

On Fri, May 20, 2022 at 5:41 PM Phil Edworthy <phil.edworthy@renesas.com> wrote:
> Add device tree binding documentation and header file for Renesas
> RZ/V2M pinctrl.
>
> Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Thanks for your patch!

> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pinctrl/renesas,rzv2m-pinctrl.yaml
> @@ -0,0 +1,174 @@

> +additionalProperties:
> +  anyOf:
> +    - type: object
> +      allOf:
> +        - $ref: pincfg-node.yaml#
> +        - $ref: pinmux-node.yaml#
> +
> +      description:
> +        Pin controller client devices use pin configuration subnodes (children
> +        and grandchildren) for desired pin configuration.
> +        Client device subnodes use below standard properties.
> +
> +      properties:
> +        phandle: true
> +        pinmux:
> +          description:
> +            Values are constructed from GPIO port number, pin number, and
> +            alternate function configuration number using the RZV2M_PORT_PINMUX()
> +            helper macro in <dt-bindings/pinctrl/rzv2m-pinctrl.h>.
> +        pins: true
> +        bias-disable: true
> +        bias-pull-down: true
> +        bias-pull-up: true
> +        drive-strength-microamp:
> +          # Superset of supported values
> +          enum: [ 1600, 1800, 2000, 3200, 3800, 4000, 6400, 7800, 8000,
> +                  9000, 9600, 11000, 12000, 13000, 18000 ]
> +
> +        power-source:
> +          description: I/O voltage in millivolt.
> +          enum: [ 1800, 3300 ]

Is power-source actually supported?
While the documentation shows there are some 1.8/3.3V pin groups,
I didn't find how to switch voltage?

> +        slew-rate: true

What are valid values?
Looking at the code, 0 = slow, 1 = fast?

> +        gpio-hog: true
> +        gpios: true
> +        input-enable: true

Missing output-enable?

> +        output-high: true
> +        output-low: true
> +        line-name: true

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds