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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <trinity-ccdae81d-0e10-4e78-a29d-917b623095e8-1749548543301@trinity-msg-rest-gmx-gmx-live-5d9b465786-6h7cc>
Date: Tue, 10 Jun 2025 09:42:23 +0000
From: Frank Wunderlich <frank-w@...lic-files.de>
To: andrew@...n.ch
Cc: linux@...web.de, daniel@...rotopia.org, myungjoo.ham@...sung.com,
 kyungmin.park@...sung.com, cw00.choi@...sung.com, djakov@...nel.org,
 robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org,
 olteanv@...il.com, davem@...emloft.net, edumazet@...gle.com,
 kuba@...nel.org, pabeni@...hat.com, matthias.bgg@...il.com,
 angelogioacchino.delregno@...labora.com, johnson.wang@...iatek.com,
 arinc.unal@...nc9.com, Landen.Chao@...iatek.com, dqfext@...il.com,
 sean.wang@...iatek.com, lorenzo@...nel.org, nbd@....name,
 linux-pm@...r.kernel.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org, netdev@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org, linux-mediatek@...ts.infradead.org
Subject: Aw: Re: Re: [PATCH v3 06/13] arm64: dts: mediatek: mt7988: add
 basic ethernet-nodes

Hi


> Gesendet: Montag, 9. Juni 2025 um 14:12
> Von: "Andrew Lunn" <andrew@...n.ch>
> An: "Frank Wunderlich" <frank-w@...lic-files.de>
> Betreff: Re: Re: [PATCH v3 06/13] arm64: dts: mediatek: mt7988: add basic ethernet-nodes
>
> > > > +			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.

seems like only the cpu-port on switch requires the phy-mode for binding...

  DTC [C] arch/arm64/boot/dts/mediatek/mt7988a-bananapi-bpi-r4.dtb
arch/arm64/boot/dts/mediatek/mt7988a-bananapi-bpi-r4.dtb: switch@...20000: ports:port@6: 'phy-mode' is a required property
	from schema $id: http://devicetree.org/schemas/net/dsa/mediatek,mt7530.yaml#

tested on hardware with the others disabled:

- phy-mode on gsw-ports are needed else i get this:
[    1.386586] mt7530-mmio 15020000.switch wan (uninitialized): validation of gmii with support 0000000,00000000,00000000,000062ef and advertisement 0000000,00000000,00000000,000062ef failed: -EINVAL
[    1.408209] mt7530-mmio 15020000.switch wan (uninitialized): failed to connect to PHY: -EINVAL
[    1.421308] mt7530-mmio 15020000.switch wan (uninitialized): error -22 setting up PHY for tree 0, switch 0, port 0
- phy-mode on gmac0 is needed alse ethernet-controller (and switch) does not get up

the phy-modes (gsw_phyX) can be dropped, the 2g5 phy-mode is currently set twice (mt7988a.dtsi and in 2g5 dts), so i would leave this in mt7988a.dtsi

> 
> 	Andrew

regards Frank