netdev - Re: [PATCH net-next 2/2] ARM: dts: Fix up wrv54g device tree

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 for Android: free password hash cracker in your pocket

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

Message-ID: <08531445-a27d-413f-96de-81087d6f61e1@lunn.ch>
Date: Tue, 24 Jun 2025 10:16:21 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Linus Walleij <linus.walleij@...aro.org>
Cc: Vladimir Oltean <olteanv@...il.com>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>, Imre Kaloz <kaloz@...nwrt.org>,
	Frederic Lambert <frdrc66@...il.com>,
	Gabor Juhos <juhosg@...nwrt.org>,
	Philipp Zabel <p.zabel@...gutronix.de>, netdev@...r.kernel.org,
	devicetree@...r.kernel.org
Subject: Re: [PATCH net-next 2/2] ARM: dts: Fix up wrv54g device tree

On Tue, Jun 24, 2025 at 09:41:12AM +0200, Linus Walleij wrote:
> Fix up the KS8995 switch and PHYs the way that is most likely:
> 
> - Phy 1-4 is certainly the PHYs of the KS8995 (mask 0x1e in
>   the outoftree code masks PHYs 1,2,3,4).
> - Phy 5 is likely the separate WAN phy directly connected
>   to ethc.
> - The ethb is probably connected as CPU interface to
>   the KS8995.
> 
> There are some confused comments in the old board file
> replicated into the device tree like ethc being "connected
> to port 5 of the ks8995" but this makes no sense as it
> is certainly connected to a phy.
> 
> Properly integrate the KS8995 switch using the new bindings.
> 
> Signed-off-by: Linus Walleij <linus.walleij@...aro.org>
> ---
>  .../dts/intel/ixp/intel-ixp42x-linksys-wrv54g.dts  | 75 +++++++++++++++++-----
>  1 file changed, 59 insertions(+), 16 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/intel/ixp/intel-ixp42x-linksys-wrv54g.dts b/arch/arm/boot/dts/intel/ixp/intel-ixp42x-linksys-wrv54g.dts
> index 98275a363c57cde22ef57c3885bc4469677ef790..14b766083e3a870a1154a93be74af6e6738fe137 100644
> --- a/arch/arm/boot/dts/intel/ixp/intel-ixp42x-linksys-wrv54g.dts
> +++ b/arch/arm/boot/dts/intel/ixp/intel-ixp42x-linksys-wrv54g.dts
> @@ -72,10 +72,50 @@ spi {
>  		cs-gpios = <&gpio0 5 GPIO_ACTIVE_LOW>;
>  		num-chipselects = <1>;
>  
> -		switch@0 {
> +		ethernet-switch@0 {
>  			compatible = "micrel,ks8995";
>  			reg = <0>;
>  			spi-max-frequency = <50000000>;
> +
> +			ethernet-ports {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +
> +				ethernet-port@0 {
> +					reg = <0>;
> +					label = "1";
> +					phy-mode = "rgmii";

If this is an internal PHY, it would be better to use 'internal'. I
would like to avoid all the issues around 'rgmii' vs 'rgmii-id'.

> +				ethernet-port@4 {
> +					reg = <4>;
> +					ethernet = <&ethb>;
> +					phy-mode = "rgmii-id";
> +					fixed-link {
> +						speed = <100>;
> +						full-duplex;
> +					};

That is a bit odd, rgmii-id, yet speed limited to 100. It would be
good to add a comment about this.

> @@ -134,41 +174,44 @@ pci@...00000 {
>  			<0x0800 0 0 2 &gpio0 10 IRQ_TYPE_LEVEL_LOW>; /* INT B on slot 1 is irq 10 */
>  		};
>  
> -		/*
> -		 * EthB - connected to the KS8995 switch ports 1-4
> -		 * FIXME: the boardfile defines .phy_mask = 0x1e for this port to enable output to
> -		 * all four switch ports, also using an out of tree multiphy patch.
> -		 * Do we need a new binding and property for this?
> -		 */
> -		ethernet@...09000 {
> +		ethb: ethernet@...09000 {
>  			status = "okay";
>  			queue-rx = <&qmgr 3>;
>  			queue-txready = <&qmgr 20>;
> -			phy-mode = "rgmii";
> -			phy-handle = <&phy4>;
> +			phy-mode = "rgmii-id";
> +			fixed-link {
> +				speed = <100>;
> +				full-duplex;
> +			};

This is all confusing. Do you have the board, or a schematic for it? 

Looking at the old DT, this ethernet interface has its own MDIO bus,
with PHYs at address 4 and 5. The phy-handle above means this MAC is
connected to the PHY at address 4. The PHY at address 5 is connected
to the second MAC instance of this SoC. This implies it is:

SOC:MAC-PHY-PHY-MAC:SWITCH 

Rather than the more usual back to back MAC. There are boards with
back to back PHY, so it is not out of the question.

However, it could also be this old DT description is completely
broken, and the PHYs on this bus are the external PHYs for the
switches? There should not be a phy-handle in the MAC nodes.

>  
>  			mdio {
>  				#address-cells = <1>;
>  				#size-cells = <0>;
>  
> -				/* Should be ports 1-4 on the KS8995 switch */
> +				/* Should be LAN ports 1-4 on the KS8995 switch */
> +				phy1: ethernet-phy@1 {
> +					reg = <1>;
> +				};
> +				phy2: ethernet-phy@2 {
> +					reg = <2>;
> +				};
> +				phy3: ethernet-phy@3 {
> +					reg = <3>;
> +				};
>  				phy4: ethernet-phy@4 {
>  					reg = <4>;
>  				};

This node is the SoC interface MDIO bus. Why would the internal PHYs
of switch bus on the SoC MDIO bus? I would expect the switch to have
its own MDIO bus and place its PHYs there.

	Andrew