[net-next PATCH 0/1] Sgmii fix for qca8k qca8327 switch

[net-next PATCH 0/1] Sgmii fix for qca8k qca8327 switch

[Ansuel Smith]

Some background for this. As stated in other patch, we are testing
qca8327 switch in various device. We tought this was a specific
configuration of the switch and needed special binding to disable pll on
sgmii port. With a better analysis of the original qca driver it was
discovered that pll was only enabled on switch revision 1.

By testing the same configuration with a qca8337 switch that have a
revision 2, using the secondary cpu port (set as sgmii) as primary
(removed the first cpu port from dts and updated the driver to use the
secondary cpu port port6 for everything) confirmed that pll enabled is
not actually needed and all works correctly.
Different case for another router with a qca8327 switch that with the
extra option enabled doesn't work at all and no traffic can be detected.

Also in the original driver the signal detection (SD bit 4) is never
enabled. Having that enabled doesn't seems to give any problem so i
won't disable that, but i'm just pointing this out to think about it.

Don't know if that should be disabled or not but the pll changes is
absolutely needed as in some case it cause the malfunction of the entire
connection.

Ansuel Smith (1):
  drivers: net: dsa: qca8k: fix sgmii with some specific switch revision

 drivers/net/dsa/qca8k.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

-- 
2.32.0

[net-next PATCH 1/1] drivers: net: dsa: qca8k: fix sgmii with some specific switch revision

[Ansuel Smith]

Enable sgmii pll, tx driver and rx chain only on switch revision 1. This
is not needed on later revision and with qca8327 cause the sgmii
connection to not work at all. This is a case with some router that use
the qca8327 switch and have the cpu port 0 using a sgmii connection.
Without this, routers with this specific configuration won't work as the
ports won't be able to communicate with the cpu port with the result of
no traffic.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 drivers/net/dsa/qca8k.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
index bda5a9bf4f52..efeed8094865 100644
--- a/drivers/net/dsa/qca8k.c
+++ b/drivers/net/dsa/qca8k.c
@@ -1227,8 +1227,14 @@ qca8k_phylink_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
 		if (ret)
 			return;
 
-		val |= QCA8K_SGMII_EN_PLL | QCA8K_SGMII_EN_RX |
-			QCA8K_SGMII_EN_TX | QCA8K_SGMII_EN_SD;
+		/* SGMII PLL, TX driver and RX chain is only needed in
+		 * switch revision 1, later revision doesn't need this.
+		 */
+		if (priv->switch_revision == 1)
+			val |= QCA8K_SGMII_EN_PLL | QCA8K_SGMII_EN_RX |
+			       QCA8K_SGMII_EN_TX | QCA8K_SGMII_EN_SD;
+		else
+			val |= QCA8K_SGMII_EN_SD;
 
 		if (dsa_is_cpu_port(ds, port)) {
 			/* CPU port, we're talking to the CPU MAC, be a PHY */
-- 
2.32.0

Re: [net-next PATCH 1/1] drivers: net: dsa: qca8k: fix sgmii with some specific switch revision

[Andrey Jr. Melnikov]

In gmane.linux.network Ansuel Smith <ansuelsmth@gmail.com> wrote:
> Enable sgmii pll, tx driver and rx chain only on switch revision 1. This
> is not needed on later revision and with qca8327 cause the sgmii
> connection to not work at all. This is a case with some router that use
> the qca8327 switch and have the cpu port 0 using a sgmii connection.
> Without this, routers with this specific configuration won't work as the
> ports won't be able to communicate with the cpu port with the result of
> no traffic.

> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> ---
>  drivers/net/dsa/qca8k.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)

> diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
> index bda5a9bf4f52..efeed8094865 100644
> --- a/drivers/net/dsa/qca8k.c
> +++ b/drivers/net/dsa/qca8k.c
> @@ -1227,8 +1227,14 @@ qca8k_phylink_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
>                 if (ret)
>                         return;
>  
> -               val |= QCA8K_SGMII_EN_PLL | QCA8K_SGMII_EN_RX |
> -                       QCA8K_SGMII_EN_TX | QCA8K_SGMII_EN_SD;
> +               /* SGMII PLL, TX driver and RX chain is only needed in
> +                * switch revision 1, later revision doesn't need this.
> +                */
> +               if (priv->switch_revision == 1)
> +                       val |= QCA8K_SGMII_EN_PLL | QCA8K_SGMII_EN_RX |
> +                              QCA8K_SGMII_EN_TX | QCA8K_SGMII_EN_SD;
> +               else
> +                       val |= QCA8K_SGMII_EN_SD;
maybe better

+               val |= QCA8K_SGMII_EN_SD;
+               /* SGMII PLL, TX driver and RX chain is only needed in
+                * switch revision 1, later revision doesn't need this.
+                */
+               if (priv->switch_revision == 1)
+                       val |= QCA8K_SGMII_EN_PLL | QCA8K_SGMII_EN_RX |
+                              QCA8K_SGMII_EN_TX;

without else branch ?

>                 if (dsa_is_cpu_port(ds, port)) {
>                         /* CPU port, we're talking to the CPU MAC, be a PHY */