Re: [PATCH v4 1/3] dt-bindings: pinctrl: renesas: document RZ/T2H and RZ/N2H SoCs

[Lad, Prabhakar]

Hi Geert,

Thank you for the review.

On Fri, Aug 8, 2025 at 8:51 PM Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote:
>
> Hi Prabhakar,
>
> On Fri, 1 Aug 2025 at 17:46, Prabhakar <prabhakar.csengg@xxxxxxxxx> wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
> >
> > Document the pin and GPIO controller IP for the Renesas RZ/T2H
> > (R9A09G077) and RZ/N2H (R9A09G087) SoCs, and add the shared DTSI
> > header file used by both the bindings and the driver.
> >
> > The RZ/T2H SoC supports 729 pins, while the RZ/N2H supports 576 pins.
> > Both share the same controller architecture; separate compatible
> > strings are added for each SoC to distinguish them.
> >
> > Co-developed-by: Thierry Bultel <thierry.bultel.yh@xxxxxxxxxxxxxx>
> > Signed-off-by: Thierry Bultel <thierry.bultel.yh@xxxxxxxxxxxxxx>
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
> > ---
> > v3->v4:
> > - Used patternProperties for pin configuration nodes
> > - Expanded example nodes
>
> Thanks for the update!
>
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pinctrl/renesas,rzt2h-pinctrl.yaml
> > @@ -0,0 +1,177 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/pinctrl/renesas,rzt2h-pinctrl.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Renesas RZ/T2H Pin and GPIO controller
> > +
> > +maintainers:
> > +  - Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
> > +
> > +description:
> > +  The Renesas RZ/T2H SoC features a combined Pin and GPIO controller.
> > +  Pin multiplexing and GPIO configuration is performed on a per-pin basis.
> > +  Each port features up to 8 pins, each of them configurable for GPIO function
> > +  (port mode) or in alternate function mode.
> > +  Up to 8 different alternate function modes exist for each single pin.
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - renesas,r9a09g077-pinctrl # RZ/T2H
> > +      - renesas,r9a09g087-pinctrl # RZ/N2H
> > +
> > +  reg:
> > +    minItems: 1
> > +    items:
> > +      - description: Non-safety I/O Port base
> > +      - description: Safety I/O Port safety region base
> > +      - description: Safety I/O Port Non-safety region base
> > +
> > +  reg-names:
> > +    minItems: 1
> > +    items:
> > +      - const: nsr
> > +      - const: srs
> > +      - const: srn
> > +
> > +  gpio-controller: true
> > +
> > +  '#gpio-cells':
> > +    const: 2
> > +    description:
> > +      The first cell contains the global GPIO port index, constructed using the
> > +      RZT2H_GPIO() helper macro from <dt-bindings/pinctrl/renesas,r9a09g077-pinctrl.h>
> > +      (e.g. "RZT2H_GPIO(3, 0)" for P03_0). The second cell represents the consumer
> > +      flag. Use the macros defined in include/dt-bindings/gpio/gpio.h.
> > +
> > +  gpio-ranges:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    maxItems: 1
> > +
> > +  power-domains:
> > +    maxItems: 1
> > +
> > +definitions:
> > +  renesas-rzt2h-n2h-pins-node:
> > +    type: object
> > +    allOf:
> > +      - $ref: pincfg-node.yaml#
> > +      - $ref: pinmux-node.yaml#
> > +    properties:
> > +      pinmux:
> > +        description:
> > +          Values are constructed from I/O port number, pin number, and
> > +          alternate function configuration number using the RZT2H_PORT_PINMUX()
> > +          helper macro from <dt-bindings/pinctrl/renesas,r9a09g077-pinctrl.h>.
> > +      pins: true
> > +      phandle: true
> > +      input: true
> > +      input-enable: true
> > +      output-enable: true
> > +    oneOf:
> > +      - required: [pinmux]
> > +      - required: [pins]
> > +    additionalProperties: false
> > +
> > +patternProperties:
> > +  # Grouping nodes: allow multiple "-pins" subnodes within a "-group"
> > +  '.*-group$':
> > +    type: object
> > +    description:
> > +      Pin controller client devices can organize pin configuration entries into
> > +      grouping nodes ending in "-group". These group nodes may contain multiple
> > +      child nodes each ending in "-pins" to configure distinct sets of pins.
> > +    additionalProperties: false
> > +    patternProperties:
> > +      '-pins$':
> > +        $ref: '#/definitions/renesas-rzt2h-n2h-pins-node'
> > +
> > +  # Standalone "-pins" nodes under client devices or groups
> > +  '-pins$':
> > +    $ref: '#/definitions/renesas-rzt2h-n2h-pins-node'
> > +
> > +  '-hog$':
> > +    type: object
> > +    description: GPIO hog node
> > +    properties:
> > +      gpio-hog: true
> > +      gpios: true
> > +      input: true
> > +      output-high: true
> > +      output-low: true
> > +      line-name: true
> > +    required:
> > +      - gpio-hog
> > +      - gpios
> > +    additionalProperties: false
> > +
> > +allOf:
> > +  - $ref: pinctrl.yaml#
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - reg-names
> > +  - gpio-controller
> > +  - '#gpio-cells'
> > +  - gpio-ranges
> > +  - clocks
> > +  - power-domains
> > +
> > +unevaluatedProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/clock/renesas,r9a09g077-cpg-mssr.h>
> > +    #include <dt-bindings/pinctrl/renesas,r9a09g077-pinctrl.h>
> > +
> > +    pinctrl@802c0000 {
> > +        compatible = "renesas,r9a09g077-pinctrl";
> > +        reg = <0x802c0000 0x2000>,
> > +              <0x812c0000 0x2000>,
> > +              <0x802b0000 0x2000>;
> > +        reg-names = "nsr", "srs", "srn";
> > +        clocks = <&cpg CPG_CORE R9A09G077_CLK_PCLKM>;
> > +        gpio-controller;
> > +        #gpio-cells = <2>;
> > +        gpio-ranges = <&pinctrl 0 0 288>;
> > +        power-domains = <&cpg>;
> > +
> > +        serial0-pins {
> > +            pinmux = <RZT2H_PORT_PINMUX(38, 0, 1)>, /* Tx */
> > +                     <RZT2H_PORT_PINMUX(38, 1, 1)>; /* Rx */
> > +        };
> > +
> > +        sd1-pwr-en-hog {
> > +            gpio-hog;
> > +            gpios = <RZT2H_GPIO(39, 2) 0>;
> > +            output-high;
> > +            line-name = "sd1_pwr_en";
> > +        };
> > +
> > +        i2c0-pins {
> > +            pins = "RIIC0_SDA", "RIIC0_SCL";
> > +            input-enable;
> > +        };
> > +
> > +        sdhi0_sd_pins: sd0-sd-group {
>
> No need for unused labels in examples.
>
Agreed, I will drop it.
> > +            sd0-sd-ctrl-pins {
>
> Drop the "sd0-sd-" prefix?
>
ditto.

> > +                pinmux = <RZT2H_PORT_PINMUX(12, 0, 0x29)>, /* SD0_CLK */
> > +                         <RZT2H_PORT_PINMUX(12, 1, 0x29)>; /* SD0_CMD */
> > +            };
> > +
> > +            sd0-sd-data-pins {
>
> Likewise.
>
ditto.

> > +                pinmux = <RZT2H_PORT_PINMUX(12, 0, 0x29)>, /* SD0_CLK */
> > +                         <RZT2H_PORT_PINMUX(12, 1, 0x29)>; /* SD0_CMD */
> > +            };
> > +
> > +            sd0-sd-tmp-pins {
> > +                pins = "RIIC0_SDA", "RIIC0_SCL";
> > +                input-enable;
> > +            };
>
> Please drop this subnode? It totally confuses me ;-)
>
I did drop this in the v5 series [0].

https://lore.kernel.org/all/20250808133017.2053637-2-prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx/

> > +        };
> > +    };
>
> The rest LGTM, so with the above fixed:
> Reviewed-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>
>

Cheers,
Prabhakar