Re: Re: [PATCH v3 06/13] arm64: dts: mediatek: mt7988: add basic ethernet-nodes

Andrew Lunn
2025-06-09T12:13:19Z

> > > + gmac0: mac@0 {
> > > + compatible = "mediatek,eth-mac";
> > > + reg = <0>;
> > > + phy-mode = "internal";
> > > +
> > > + fixed-link {
> > > + speed = <10000>;
> > > + full-duplex;
> > > + pause;
> > > + };
> >
> > Maybe i've asked this before? What is on the other end of this link?
> > phy-mode internal and fixed link seems an odd combination. It might
> > just need some comments, if this is internally connected to a switch.
>
> yes you've asked in v1 and i responded :)
>
> https://patchwork.kernel.org/project/linux-mediatek/patch/20250511141942.10284-9-linux@fw-web.de/
>
> connected to internal (mt7530) switch. Which kind of comment do you want here? Only "connected to internal switch"
> or some more details?

"Connected to internal switch" will do. The word switch explains the
fixed-link, and internal the phy-mode.

It is not the case here, but i've seen DT misused like this because
the MAC is connected to a PHY and there is no PHY driver yet, so a
fixed link is used instead.

> > > + mdio_bus: mdio-bus {
> > > + #address-cells = <1>;
> > > + #size-cells = <0>;
> > > +
> > > + /* internal 2.5G PHY */
> > > + int_2p5g_phy: ethernet-phy@f {
> > > + reg = <15>;
> >
> > It is a bit odd mixing hex and decimal.
>
> do you prefer hex or decimal for both? for r3mini i used decimal for both, so i would change unit-address
> to 15.

I suspect decimal is more common, but i don't care.

>
> > > + compatible = "ethernet-phy-ieee802.3-c45";
> >
> > I _think_ the coding standard say the compatible should be first.
>
> i can move this up of course
>
> > > + phy-mode = "internal";
> >
> > A phy should not have a phy-mode.
>
> not sure if this is needed for mt7988 internal 2.5g phy driver, but seems not when i look at the driver
> (drivers/net/phy/mediatek/mtk-2p5ge.c). The switch phys also use this and also here i do not see any
> access in the driver (drivers/net/dsa/mt7530-mmio.c + mt7530.c) on a quick look.
> Afaik binding required the property and should be read by phylink (to be not unknown, but looks like
> handled the same way).

Which binding requires this? This is a PHY node, but i don't see
anything about it in ethernet-phy.yaml.

Andrew