linux-kernel - Re: [net-next PATCH v2 08/15] dt-bindings: net: dsa: qca8k: Add MAC swap and clock phase properties

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]

Date:   Sun, 10 Oct 2021 00:23:27 +0200
From:   Ansuel Smith <ansuelsmth@...il.com>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     Vivien Didelot <vivien.didelot@...il.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        Vladimir Oltean <olteanv@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Rob Herring <robh+dt@...nel.org>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Russell King <linux@...linux.org.uk>, netdev@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        Matthew Hagan <mnhagan88@...il.com>
Subject: Re: [net-next PATCH v2 08/15] dt-bindings: net: dsa: qca8k: Add MAC
 swap and clock phase properties

On Sat, Oct 09, 2021 at 11:37:10PM +0200, Andrew Lunn wrote:
> > Here is 2 configuration one from an Netgear r7800 qca8337:
> > 
> > 	switch@10 {
> > 		compatible = "qca,qca8337";
> > 		#address-cells = <1>;
> > 		#size-cells = <0>;
> > 		reg = <0x10>;
> > 
> > 		qca8k,rgmii0_1_8v;
> > 		qca8k,rgmii56_1_8v;
> > 
> > 		ports {
> > 			#address-cells = <1>;
> > 			#size-cells = <0>;
> > 
> > 			port@0 {
> > 				reg = <0>;
> > 				label = "cpu";
> > 				ethernet = <&gmac1>;
> > 				phy-mode = "rgmii-id";
> > 
> > 				fixed-link {
> > 					speed = <1000>;
> > 					full-duplex;
> > 				};
> > 			};
> > 
> > 			port@1 {
> > 				reg = <1>;
> > 				label = "lan1";
> > 				phy-mode = "internal";
> > 				phy-handle = <&phy_port1>;
> > 			};
> > 
> > 			port@2 {
> > 				reg = <2>;
> > 				label = "lan2";
> > 				phy-mode = "internal";
> > 				phy-handle = <&phy_port2>;
> > 			};
> > 
> > 			port@3 {
> > 				reg = <3>;
> > 				label = "lan3";
> > 				phy-mode = "internal";
> > 				phy-handle = <&phy_port3>;
> > 			};
> > 
> > 			port@4 {
> > 				reg = <4>;
> > 				label = "lan4";
> > 				phy-mode = "internal";
> > 				phy-handle = <&phy_port4>;
> > 			};
> > 
> > 			port@5 {
> > 				reg = <5>;
> > 				label = "wan";
> > 				phy-mode = "internal";
> > 				phy-handle = <&phy_port5>;
> > 			};
> > 
> > 			port@6 {
> > 				reg = <6>;
> > 				label = "cpu";
> > 				ethernet = <&gmac2>;
> > 				phy-mode = "sgmii";
> > 
> > 				fixed-link {
> > 					speed = <1000>;
> > 					full-duplex;
> > 				};
> 
> So here, it is a second CPU port.  But some other board could connect
> an SGMII PHY, and call the port lan5. Or it could be connected to an
> SFP cage, and used that way. Or are you forced to use it as a CPU
> port, or not use it at all?
>

We have a bit to set the mode. So yes it can be used to different modes.
(base-x, phy and mac)

> > And here is one with mac swap Tp-Link Archer c7 v4 qca8327 
> > 
> > 	switch0@10 {
> > 		compatible = "qca,qca8337";
> > 		#address-cells = <1>;
> > 		#size-cells = <0>;
> > 
> > 		reg = <0>;
> > 		qca,sgmii-rxclk-falling-edge;
> > 		qca,mac6-exchange;
> > 
> > 		ports {
> > 			#address-cells = <1>;
> > 			#size-cells = <0>;
> > 
> > 			port@0 {
> > 				reg = <0>;
> > 				label = "cpu";
> > 				ethernet = <&eth0>;
> > 				phy-mode = "sgmii";
> > 
> > 				fixed-link {
> > 					speed = <1000>;
> > 					full-duplex;
> > 				};
> 
> So when looking for SGMI properties, you need to look here. Where as
> in the previous example, you would look in port 6. And the reverse is
> true for RGMII delays.
> 
> > 			};
> > 
> > 			port@1 {
> > 				reg = <1>;
> > 				label = "wan";
> > 				phy-mode = "internal";
> > 				phy-handle = <&phy_port1>;
> > 			};
> > 
> > 			port@2 {
> > 				reg = <2>;
> > 				label = "lan1";
> > 				phy-mode = "internal";
> > 				phy-handle = <&phy_port2>;
> > 			};
> > 
> > 			port@3 {
> > 				reg = <3>;
> > 				label = "lan2";
> > 				phy-mode = "internal";
> > 				phy-handle = <&phy_port3>;
> > 			};
> > 
> > 			port@4 {
> > 				reg = <4>;
> > 				label = "lan3";
> > 				phy-mode = "internal";
> > 				phy-handle = <&phy_port4>;
> > 			};
> > 
> > 			port@5 {
> > 				reg = <5>;
> > 				label = "lan4";
> > 				phy-mode = "internal";
> > 				phy-handle = <&phy_port5>;
> > 			};
> > 		};
> 
> So here, port '6' is not used. But it could be connected to an RGMII
> PHY and called lan5. Would the naming work out? What does devlink
> think of it, etc. What about phy-handle? Is there an external MDIO
> bus? What address would be used if there is no phy-handle?
> 
>       Andrew

In this case port6 is not used as it's not connected at all in hardware.
>From the configuration list yes, it can be used as lan5 in phy mode and
it would have address 5 (internally the address are with an offset of
-1). Anyway I honestly think that we are putting too much effort in
something that can and should be handled differently. I agree that all
this mac exchange is bs and doesn't make much sense. I tried to
implement this as we currently qca8k is hardcoded to expect
the cpu port 0 for everything and doesn't actually found a valid cpu
port (aka it doesn't expect a configuration with cpu6)
I think the driver was writtent with the concept of mac exchange from
the start. That's why it's hardoced to port0.
I actually tied for fun running the switch using only the port6 cpu
port and it worked just right. So I think I will just drop the mac
exchange and fix the code to make it dynamic.

-- 
	Ansuel