Subject: Re: [PATCH net-next v5 11/13] ARM: dts: r9a06g032: describe GMAC2

From: Clément Léger <[email-masked]>
Date: Fri, 20 May 2022 10:13:32 +0200

Le Fri, 20 May 2022 09:18:58 +0200,
Geert Uytterhoeven <[email-masked]> a écrit :

> Hi Clément
> 
> On Thu, May 19, 2022 at 5:32 PM Clément Léger <[email-masked]> wrote:
> > RZ/N1 SoC includes two MAC named GMACx that are compatible with the
> > "snps,dwmac" driver. GMAC1 is connected directly to the MII converter
> > port 1. GMAC2 however can be used as the MAC for the switch CPU
> > management port or can be muxed to be connected directly to the MII
> > converter port 2. This commit add description for the GMAC2 which will
> > be used by the switch description.
> >
> > Signed-off-by: Clément Léger <[email-masked]>  
> 
> Thanks for your patch!
> 
> > --- a/arch/arm/boot/dts/r9a06g032.dtsi
> > +++ b/arch/arm/boot/dts/r9a06g032.dtsi
> > @@ -200,6 +200,23 @@ nand_controller: nand-controller@40102000 {
> >                         status = "disabled";
> >                 };
> >
> > +               gmac2: ethernet@44002000 {
> > +                       compatible = "snps,dwmac";  
> 
> Does this need an SoC-specific compatible value?

Indeed, it might be useful to introduce a specific SoC compatible since
in a near future, there might be some specific support for that gmac.
Here is an overview of the gmac connection on the SoC:

                                          ┌─────────┐   ┌──────────┐
                                          │         │   │          │
                                          │  GMAC2  │   │  GMAC1   │
                                          │         │   │          │
                                          └───┬─────┘   └─────┬────┘
                                              │               │
                                              │               │
                                              │               │
                                         ┌────▼──────┐        │
                                         │           │        │
            ┌────────────────────────────┤  SWITCH   │        │
            │                            │           │        │
            │          ┌─────────────────┴─┬────┬────┘        │
            │          │            ┌──────┘    │             │
            │          │            │           │             │
       ┌────▼──────────▼────────────▼───────────▼─────────────▼────┐
       │                      MII Converter                        │
       │                                                           │
       │                                                           │
       │ port 1      port 2       port 3      port 4       port 5  │
       └───────────────────────────────────────────────────────────┘

As you can see, the GMAC1 is directly connected to MIIC converter and
thus will need a "pcs-handle" property to point on the MII converter
port whereas the GMAC2 is directly connected to the switch in GMII.

Is "renesas,r9a06g032-gmac2", "renesas,rzn1-switch-gmac2" looks ok for
you for this one ?

Thanks,

Clément

> 
> > +                       reg = <0x44002000 0x2000>;
> > +                       interrupt-parent = <&gic>;
> > +                       interrupts = <GIC_SPI 37
> > IRQ_TYPE_LEVEL_HIGH>,
> > +                                    <GIC_SPI 39
> > IRQ_TYPE_LEVEL_HIGH>,
> > +                                    <GIC_SPI 38
> > IRQ_TYPE_LEVEL_HIGH>;
> > +                       interrupt-names = "macirq", "eth_wake_irq",
> > "eth_lpi";
> > +                       clock-names = "stmmaceth";
> > +                       clocks = <&sysctrl R9A06G032_HCLK_GMAC1>;  
> 
> Missing "power-domains", also in the DT bindings.
> The driver already uses Runtime PM.

Ok,I'll add that to the DT and modify the DWMAC bindings.

> 
> > +                       snps,multicast-filter-bins = <256>;
> > +                       snps,perfect-filter-entries = <128>;
> > +                       tx-fifo-depth = <2048>;
> > +                       rx-fifo-depth = <4096>;
> > +                       status = "disabled";
> > +               };
> > +
> >                 eth_miic: eth-miic@44030000 {
> >                         compatible = "renesas,r9a06g032-miic",
> > "renesas,rzn1-miic"; #address-cells = <1>;  
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 --
> [email-masked]
> 
> 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

-- 
Clément Léger,
Embedded Linux and Kernel engineer at Bootlin
https://bootlin.com