[PATCH 0/2] net: dsa: qca8k: support internal-PHY-to-PHY CPU link

[PATCH 0/2] net: dsa: qca8k: support internal-PHY-to-PHY CPU link

[Ziyang Huang]

internal-PHY-to-PHY CPU link is a common/demo design in IPQ50xx platform,
since it only has a SGMII/SGMII+ link and a MDI link.

For DSA, CPU tag is the only requirement. Fortunately, qca8337 can enable
it on any port. So it's ok to trust a internal-PHY-to-PHY link as a CPU
link.

Ziyang Huang (2):
  net: dsa: qca8k: support internal-PHY-to-PHY CPU link
  dt-bindings: net: dsa: qca8k: add internal-PHY-to-PHY CPU link example

 Documentation/devicetree/bindings/net/dsa/qca8k.yaml |  7 +++++--
 drivers/net/dsa/qca/qca8k-8xxx.c                     | 12 +++++++-----
 2 files changed, 12 insertions(+), 7 deletions(-)

-- 
2.40.1

[PATCH 1/2] net: dsa: qca8k: support internal-PHY-to-PHY CPU link

[Ziyang Huang]

internal-PHY-to-PHY CPU link is a common/demo design in IPQ50xx platform,
since it only has a SGMII/SGMII+ link and a MDI link.

For DSA, CPU tag is the only requirement. Fortunately, qca8337 can enable
it on any port. So it's ok to trust a internal-PHY-to-PHY link as a CPU
link.

Signed-off-by: Ziyang Huang <hzyitc@outlook.com>
---
 drivers/net/dsa/qca/qca8k-8xxx.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/net/dsa/qca/qca8k-8xxx.c b/drivers/net/dsa/qca/qca8k-8xxx.c
index e8cb4da15dbe..be9a8170c048 100644
--- a/drivers/net/dsa/qca/qca8k-8xxx.c
+++ b/drivers/net/dsa/qca/qca8k-8xxx.c
@@ -1014,7 +1014,7 @@ qca8k_setup_mdio_bus(struct qca8k_priv *priv)
 			return ret;
 		}
 
-		if (!dsa_is_user_port(priv->ds, reg))
+		if (reg == 0 || reg == 6)
 			continue;
 
 		of_get_phy_mode(port, &mode);
@@ -1089,17 +1089,19 @@ qca8k_setup_mac_pwr_sel(struct qca8k_priv *priv)
 
 static int qca8k_find_cpu_port(struct dsa_switch *ds)
 {
-	struct qca8k_priv *priv = ds->priv;
+	int i;
 
-	/* Find the connected cpu port. Valid port are 0 or 6 */
 	if (dsa_is_cpu_port(ds, 0))
 		return 0;
 
-	dev_dbg(priv->dev, "port 0 is not the CPU port. Checking port 6");
-
 	if (dsa_is_cpu_port(ds, 6))
 		return 6;
 
+	/* PHY-to-PHY link */
+	for (i = 1; i <= 5; i++)
+		if (dsa_is_cpu_port(ds, i))
+			return i;
+
 	return -EINVAL;
 }
 
-- 
2.40.1

[PATCH 2/2] dt-bindings: net: dsa: qca8k: add internal-PHY-to-PHY CPU link example

[Ziyang Huang]

Current example use external PHY. With the previous patch, internal PHY
is also supported.

Signed-off-by: Ziyang Huang <hzyitc@outlook.com>
---
 Documentation/devicetree/bindings/net/dsa/qca8k.yaml | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/dsa/qca8k.yaml b/Documentation/devicetree/bindings/net/dsa/qca8k.yaml
index 167398ab253a..a71dc38d6bab 100644
--- a/Documentation/devicetree/bindings/net/dsa/qca8k.yaml
+++ b/Documentation/devicetree/bindings/net/dsa/qca8k.yaml
@@ -185,8 +185,10 @@ examples:
                 };
 
                 port@4 {
+                    /* PHY-to-PHY CPU link */
                     reg = <4>;
-                    label = "lan4";
+                    ethernet = <&gmac2>;
+                    phy-mode = "gmii";
                     phy-handle = <&external_phy_port4>;
                 };
 
@@ -266,8 +268,9 @@ examples:
                 };
 
                 port@4 {
+                    /* PHY-to-PHY CPU link */
                     reg = <4>;
-                    label = "lan4";
+                    ethernet = <&gmac2>;
                     phy-mode = "internal";
                     phy-handle = <&internal_phy_port4>;
                 };
-- 
2.40.1

Re: [PATCH 2/2] dt-bindings: net: dsa: qca8k: add internal-PHY-to-PHY CPU link example

[Andrew Lunn]

On Mon, Mar 03, 2025 at 11:24:35PM +0800, Ziyang Huang wrote:
> Current example use external PHY. With the previous patch, internal PHY
> is also supported.
> 
> Signed-off-by: Ziyang Huang <hzyitc@outlook.com>
> ---
>  Documentation/devicetree/bindings/net/dsa/qca8k.yaml | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/dsa/qca8k.yaml b/Documentation/devicetree/bindings/net/dsa/qca8k.yaml
> index 167398ab253a..a71dc38d6bab 100644
> --- a/Documentation/devicetree/bindings/net/dsa/qca8k.yaml
> +++ b/Documentation/devicetree/bindings/net/dsa/qca8k.yaml
> @@ -185,8 +185,10 @@ examples:
>                  };
>  
>                  port@4 {
> +                    /* PHY-to-PHY CPU link */
>                      reg = <4>;
> -                    label = "lan4";
> +                    ethernet = <&gmac2>;
> +                    phy-mode = "gmii";
>                      phy-handle = <&external_phy_port4>;
>                  };
>  
> @@ -266,8 +268,9 @@ examples:
>                  };
>  
>                  port@4 {
> +                    /* PHY-to-PHY CPU link */
>                      reg = <4>;
> -                    label = "lan4";
> +                    ethernet = <&gmac2>;
>                      phy-mode = "internal";
>                      phy-handle = <&internal_phy_port4>;
>                  };

Adding some more context:

                port@4 {
                    reg = <4>;
                    label = "lan4";
                    phy-mode = "internal";
                    phy-handle = <&internal_phy_port4>;
                };

                port@5 {
                    reg = <5>;
                    label = "wan";
                    phy-mode = "internal";
                    phy-handle = <&internal_phy_port5>;
                };

                port@6 {
                    reg = <0>;
                    ethernet = <&gmac1>;
                    phy-mode = "sgmii";

                    qca,sgmii-rxclk-falling-edge;

                    fixed-link {
                        speed = <1000>;
                        full-duplex;
                    };
                };
            };

The previous patch still causes it to look at port 0 and then port 6
first. Only if they are not CPU ports will it look at other ports. So
this example does not work, port 6 will be the CPU port, even with the
properties you added.

When you fix this, i also think it would be good to extend:

> +                    /* PHY-to-PHY CPU link */

with the work internal.

This also seems an odd architecture to me. If this is SoC internal,
why not do a MAC to MAC link? What benefit do you get from having the
PHYs?


    Andrew

---
pw-bot: cr

Re: [PATCH 2/2] dt-bindings: net: dsa: qca8k: add internal-PHY-to-PHY CPU link example

[Ziyang Huang]

在 2025/3/4 0:15, Andrew Lunn 写道:
> ...
> 
> The previous patch still causes it to look at port 0 and then port 6
> first. Only if they are not CPU ports will it look at other ports. So
> this example does not work, port 6 will be the CPU port, even with the
> properties you added.

Sorry, I forget that the following patch is still penging:
https://lore.kernel.org/all/20230620063747.19175-1-ansuelsmth@gmail.com/

With this path, we can have multi CPU link.

> When you fix this, i also think it would be good to extend:
> 
>> +                    /* PHY-to-PHY CPU link */
> 
> with the work internal.
> 
> This also seems an odd architecture to me. If this is SoC internal,
> why not do a MAC to MAC link? What benefit do you get from having the
> PHYs?

This patches are for IPQ50xx platform which has only one a SGMII/SGMII+ 
link and a MDI link.

It has 2 common designs:
  1. SGMII+ is used to connect a 2.5G PHY, which make qca8337 only be 
able to be connected through the MDI link.
  2. Both SGMII and MDI links are used to connect the qca8337, so we can 
get 2G link which is beneficial in NAT mode (total 2G bidirectional).

> 
> 
>      Andrew
> 
> ---
> pw-bot: cr

Re: [PATCH 2/2] dt-bindings: net: dsa: qca8k: add internal-PHY-to-PHY CPU link example

[Andrew Lunn]

On Tue, Mar 04, 2025 at 12:37:36AM +0800, Ziyang Huang wrote:
> 在 2025/3/4 0:15, Andrew Lunn 写道:
> > ...
> > 
> > The previous patch still causes it to look at port 0 and then port 6
> > first. Only if they are not CPU ports will it look at other ports. So
> > this example does not work, port 6 will be the CPU port, even with the
> > properties you added.
> 
> Sorry, I forget that the following patch is still penging:
> https://lore.kernel.org/all/20230620063747.19175-1-ansuelsmth@gmail.com/
> 
> With this path, we can have multi CPU link.

So you should get that merged first. Then this patch.

> > When you fix this, i also think it would be good to extend:
> > 
> > > +                    /* PHY-to-PHY CPU link */
> > 
> > with the work internal.
> > 
> > This also seems an odd architecture to me. If this is SoC internal,
> > why not do a MAC to MAC link? What benefit do you get from having the
> > PHYs?
> 
> This patches are for IPQ50xx platform which has only one a SGMII/SGMII+ link
> and a MDI link.
> 
> It has 2 common designs:
>  1. SGMII+ is used to connect a 2.5G PHY, which make qca8337 only be able to
> be connected through the MDI link.

Please do not call it SGMII+. It is not SGMII if it is running at
2.5G. It is more likely to be broken 2500BaseX, broken in that it does
not implement the inband signalling.

>  2. Both SGMII and MDI links are used to connect the qca8337, so we can get
> 2G link which is beneficial in NAT mode (total 2G bidirectional).

So is this actually internally? Or do you have a IPQ50xx SoC connected
to a qca8337 switch, with copper traces on a PCB? If so, it is not
internal.

	Andrew

Re: [PATCH 2/2] dt-bindings: net: dsa: qca8k: add internal-PHY-to-PHY CPU link example

[Ziyang Huang]

在 2025/3/4 1:15, Andrew Lunn 写道:
> On Tue, Mar 04, 2025 at 12:37:36AM +0800, Ziyang Huang wrote:
>> 在 2025/3/4 0:15, Andrew Lunn 写道:
>>> ...
>>>
>>> The previous patch still causes it to look at port 0 and then port 6
>>> first. Only if they are not CPU ports will it look at other ports. So
>>> this example does not work, port 6 will be the CPU port, even with the
>>> properties you added.
>>
>> Sorry, I forget that the following patch is still penging:
>> https://lore.kernel.org/all/20230620063747.19175-1-ansuelsmth@gmail.com/
>>
>> With this path, we can have multi CPU link.
> 
> So you should get that merged first. Then this patch.

After checking the code again, the demo 2 has already had 2 CPU link
(Port0 and Port6). Could I just keep this or should I need to add a new
case ?

>>> When you fix this, i also think it would be good to extend:
>>>
>>>> +                    /* PHY-to-PHY CPU link */
>>>
>>> with the work internal.
>>>
>>> This also seems an odd architecture to me. If this is SoC internal,
>>> why not do a MAC to MAC link? What benefit do you get from having the
>>> PHYs?
>>
>> This patches are for IPQ50xx platform which has only one a SGMII/SGMII+ link
>> and a MDI link.
>>
>> It has 2 common designs:
>>   1. SGMII+ is used to connect a 2.5G PHY, which make qca8337 only be able to
>> be connected through the MDI link.
> 
> Please do not call it SGMII+. It is not SGMII if it is running at
> 2.5G. It is more likely to be broken 2500BaseX, broken in that it does
> not implement the inband signalling.
> 
>>   2. Both SGMII and MDI links are used to connect the qca8337, so we can get
>> 2G link which is beneficial in NAT mode (total 2G bidirectional).
> 
> So is this actually internally? Or do you have a IPQ50xx SoC connected
> to a qca8337 switch, with copper traces on a PCB? If so, it is not
> internal.

I think I known which point you are confused about. Sorry for my poor
English.

The "internal" is used to describe the localcation of PHY not the link.
In current code, qca8k has supported to use a external PHY to do a
PHY-to-PHY link (Port0 and Port6). This patch make the internal PHYs
support it too (Port1-5).

The followiing topology is existed in most IPQ50xx-based router:
      _______________________         _______________________
     |        IPQ5018        |       |        QCA8337        |
     | +------+   +--------+ |       | +--------+   +------+ |
     | | MAC0 |---| GE Phy |-+--MDI--+-|  Phy4  |---| MAC5 | |
     | +------+   +--------+ |       | +--------+   +------+ |
     | +------+   +--------+ |       | +--------+   +------+ |
     | | MAC1 |---| Uniphy |-+-SGMII-+-| SerDes |---| MAC0 | |
     | +------+   +--------+ |       | +--------+   +------+ |
     |_______________________|       |_______________________|

> 	Andrew

Re: [PATCH 2/2] dt-bindings: net: dsa: qca8k: add internal-PHY-to-PHY CPU link example

[Andrew Lunn]

> (Port0 and Port6). Could I just keep this or should I need to add a new
> case ?

The existing examples are probably sufficient. Just check the text to
make sure it does not limit it to ports 0 and 6.

> > So is this actually internally? Or do you have a IPQ50xx SoC connected
> > to a qca8337 switch, with copper traces on a PCB? If so, it is not
> > internal.
> 
> The "internal" is used to describe the localcation of PHY not the link.
> In current code, qca8k has supported to use a external PHY to do a
> PHY-to-PHY link (Port0 and Port6). This patch make the internal PHYs
> support it too (Port1-5).
> 
> The followiing topology is existed in most IPQ50xx-based router:
>     _______________________         _______________________
>    |        IPQ5018        |       |        QCA8337        |
>    | +------+   +--------+ |       | +--------+   +------+ |
>    | | MAC0 |---| GE Phy |-+--MDI--+-|  Phy4  |---| MAC5 | |
>    | +------+   +--------+ |       | +--------+   +------+ |
>    | +------+   +--------+ |       | +--------+   +------+ |
>    | | MAC1 |---| Uniphy |-+-SGMII-+-| SerDes |---| MAC0 | |
>    | +------+   +--------+ |       | +--------+   +------+ |
>    |_______________________|       |_______________________|

So logically, it does not matter if the PHY is internal or
external. The patch would be the same. I've even see setups where the
SGMII link would have a PHY, then a connection to a daughter board,
and then a PHY back to SGMII before connecting to the switch. Running
Ethernet over the connector is easier than SERDES lines.

So i would probably drop the word internal from this discussion,
unless it is really relevant.

	Andrew

Re: [PATCH 2/2] dt-bindings: net: dsa: qca8k: add internal-PHY-to-PHY CPU link example

[Ziyang Huang]

在 2025/3/4 22:11, Andrew Lunn 写道:
>> (Port0 and Port6). Could I just keep this or should I need to add a new
>> case ?
> 
> The existing examples are probably sufficient. Just check the text to
> make sure it does not limit it to ports 0 and 6.
> 
>>> So is this actually internally? Or do you have a IPQ50xx SoC connected
>>> to a qca8337 switch, with copper traces on a PCB? If so, it is not
>>> internal.
>>
>> The "internal" is used to describe the localcation of PHY not the link.
>> In current code, qca8k has supported to use a external PHY to do a
>> PHY-to-PHY link (Port0 and Port6). This patch make the internal PHYs
>> support it too (Port1-5).
>>
>> The followiing topology is existed in most IPQ50xx-based router:
>>      _______________________         _______________________
>>     |        IPQ5018        |       |        QCA8337        |
>>     | +------+   +--------+ |       | +--------+   +------+ |
>>     | | MAC0 |---| GE Phy |-+--MDI--+-|  Phy4  |---| MAC5 | |
>>     | +------+   +--------+ |       | +--------+   +------+ |
>>     | +------+   +--------+ |       | +--------+   +------+ |
>>     | | MAC1 |---| Uniphy |-+-SGMII-+-| SerDes |---| MAC0 | |
>>     | +------+   +--------+ |       | +--------+   +------+ |
>>     |_______________________|       |_______________________|
> 
> So logically, it does not matter if the PHY is internal or
> external. The patch would be the same. I've even see setups where the
> SGMII link would have a PHY, then a connection to a daughter board,
> and then a PHY back to SGMII before connecting to the switch. Running
> Ethernet over the connector is easier than SERDES lines.
> 
> So i would probably drop the word internal from this discussion,
> unless it is really relevant.
> 
> 	Andrew

Ok, will remove the word in next patch