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

[Frank Wunderlich]

Hi Andrew

> Gesendet: Sonntag, 8. Juni 2025 um 23:23
> Von: "Andrew Lunn" <andrew at lunn.ch>
> An: "Frank Wunderlich" <linux at fw-web.de>
> Betreff: Re: [PATCH v3 06/13] arm64: dts: mediatek: mt7988: add basic ethernet-nodes
>
> > +			gmac0: mac at 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?

> > +			mdio_bus: mdio-bus {
> > +				#address-cells = <1>;
> > +				#size-cells = <0>;
> > +
> > +				/* internal 2.5G PHY */
> > +				int_2p5g_phy: ethernet-phy at 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.

> > +					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).

Maybe daniel can describe a bit deeper.

> 	Andrew

regards Frank