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

From: Ansuel Smith
Date: Sat Oct 09 2021 - 18:23:44 EST


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