Re: [PATCH v6 5/8] ARM: dts: r9a06g032: Add GPIO controllers

[Geert Uytterhoeven]

Hi Hervé,

Thanks for your patch!

On Mon, 27 Oct 2025 at 13:36, Herve Codina (Schneider Electric)
<herve.codina@xxxxxxxxxxx> wrote:
> Add GPIO controllers (Synosys DesignWare IPs) available in the

Synopsys

> r9a06g032 (RZ/N1D) SoC.
>
> Signed-off-by: Herve Codina (Schneider Electric) <herve.codina@xxxxxxxxxxx>
> Reviewed-by: Wolfram Sang <wsa+renesas@xxxxxxxxxxxxxxxxxxxx>
> Tested-by: Wolfram Sang <wsa+renesas@xxxxxxxxxxxxxxxxxxxx>
> Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@xxxxxxxxxx>

> --- a/arch/arm/boot/dts/renesas/r9a06g032.dtsi
> +++ b/arch/arm/boot/dts/renesas/r9a06g032.dtsi
> @@ -499,6 +499,127 @@ gic: interrupt-controller@44101000 {
>                                 <GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(2) | IRQ_TYPE_LEVEL_HIGH)>;
>                 };
>
> +               /*
> +                * The GPIO mapping to the corresponding pins is not obvious.
> +                * See the hardware documentation for details.
> +                */
> +               gpio0: gpio@5000b000 {
> +                       compatible = "snps,dw-apb-gpio";

Don't we want an SoC-specific compatible value, too?

> +                       reg = <0x5000b000 0x80>;
> +                       #address-cells = <1>;
> +                       #size-cells = <0>;
> +                       clocks = <&sysctrl R9A06G032_HCLK_GPIO0>;
> +                       clock-names = "bus";
> +                       status = "disabled";

Usually we keep all GPIO controllers enabled by default.

> +
> +                       /* GPIO0a[0]      connected to pin  GPIO0      */
> +                       /* GPIO0a[1..2]   connected to pins GPIO3..4   */
> +                       /* GPIO0a[3..4]   connected to pins GPIO9..10  */
> +                       /* GPIO0a[5]      connected to pin  GPIO12     */
> +                       /* GPIO0a[6..7]   connected to pins GPIO15..16 */
> +                       /* GPIO0a[8..9]   connected to pins GPIO21..22 */
> +                       /* GPIO0a[10]     connected to pin  GPIO24     */
> +                       /* GPIO0a[11..12] connected to pins GPIO27..28 */
> +                       /* GPIO0a[13..14] connected to pins GPIO33..34 */
> +                       /* GPIO0a[15]     connected to pin  GPIO36     */
> +                       /* GPIO0a[16..17] connected to pins GPIO39..40 */
> +                       /* GPIO0a[18..19] connected to pins GPIO45..46 */
> +                       /* GPIO0a[20]     connected to pin  GPIO48     */
> +                       /* GPIO0a[21..22] connected to pins GPIO51..52 */
> +                       /* GPIO0a[23..24] connected to pins GPIO57..58 */
> +                       /* GPIO0a[25..31] connected to pins GPIO62..68 */
> +                       gpio0a: gpio-port@0 {
> +                               compatible = "snps,dw-apb-gpio-port";
> +                               gpio-controller;
> +                               #gpio-cells = <2>;
> +                               snps,nr-gpios = <32>;
> +                               reg = <0>;

Please move "reg" just below "compatible", as per DT coding style.

The rest LGTM, so
Reviewed-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

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