netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Marek Behún" <kabel@kernel.org>
To: netdev@vger.kernel.org
Cc: pavana.sharma@digi.com, vivien.didelot@gmail.com,
	f.fainelli@gmail.com, kuba@kernel.org, lkp@intel.com,
	davem@davemloft.net, ashkan.boldaji@digi.com, andrew@lunn.ch,
	"Chris Packham" <chris.packham@alliedtelesis.co.nz>,
	olteanv@gmail.com,
	"Russell King - ARM Linux admin" <linux@armlinux.org.uk>,
	"Marek Behún" <kabel@kernel.org>,
	"Russell King" <rmk+kernel@armlinux.org.uk>
Subject: [PATCH net] net: dsa: mv88e6xxx: do not allow inband AN for 2500base-x mode
Date: Thu, 14 Jan 2021 03:40:55 +0100	[thread overview]
Message-ID: <20210114024055.17602-1-kabel@kernel.org> (raw)

Commit a5a6858b793ff ("net: dsa: mv88e6xxx: extend phylink to Serdes
PHYs") introduced method mv88e6390_serdes_pcs_config(), which is called
(indirectly) by phylink to configure the PCS.

This method enables inband AN if requested by phylink.

It seems though that for 2500base-x mode some of these switches (at
least Amethyst) do not support inband AN on Serdes.

Moreover the above mentioned commit causes a regression when the Serdes
of the switch is connected to a Marvell 88X3310 PHY in 2500base-x mode,
because this PHY does not enable inband AN when it self-switches to
2500base-x (nor does it seem to work if manually enabling AN by writing
the register), and it seems that 88E6390 devices won't link on
2500base-x mode if AN is enabled on the switch and disabled on the peer.

Since it seems that 2500base-x mode has no definitive documentation of
what exactly it is, and that Marvell devices disable inband AN when
switching to this mode, this patch disables inband AN when configuring
for 2500base-x in the mv88e6390_serdes_pcs_config() function and prints
a warning if such mode is requested. mv88e6xxx_serdes_pcs_get_state is
edited accordingly.

We may need to refactor phylink code to not request inband AN for
2500base-x at all, but until then we need at least to fix this
regression.

Signed-off-by: Marek Behún <kabel@kernel.org>
Fixes: a5a6858b793ff ("net: dsa: mv88e6xxx: extend phylink to Serdes PHYs")
Cc: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/net/dsa/mv88e6xxx/serdes.c | 30 ++++++++++++++++++------------
 1 file changed, 18 insertions(+), 12 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/serdes.c b/drivers/net/dsa/mv88e6xxx/serdes.c
index 3195936dc5be..b8241820679e 100644
--- a/drivers/net/dsa/mv88e6xxx/serdes.c
+++ b/drivers/net/dsa/mv88e6xxx/serdes.c
@@ -55,9 +55,20 @@ static int mv88e6xxx_serdes_pcs_get_state(struct mv88e6xxx_chip *chip,
 {
 	if (status & MV88E6390_SGMII_PHY_STATUS_SPD_DPL_VALID) {
 		state->link = !!(status & MV88E6390_SGMII_PHY_STATUS_LINK);
+
+		if (state->interface == PHY_INTERFACE_MODE_2500BASEX) {
+			if (state->link) {
+				state->speed = SPEED_2500;
+				state->duplex = DUPLEX_FULL;
+			}
+
+			return 0;
+		}
+
+		state->an_complete = 1;
 		state->duplex = status &
 				MV88E6390_SGMII_PHY_STATUS_DUPLEX_FULL ?
-			                         DUPLEX_FULL : DUPLEX_HALF;
+						DUPLEX_FULL : DUPLEX_HALF;
 
 		if (status & MV88E6390_SGMII_PHY_STATUS_TX_PAUSE)
 			state->pause |= MLO_PAUSE_TX;
@@ -66,10 +77,7 @@ static int mv88e6xxx_serdes_pcs_get_state(struct mv88e6xxx_chip *chip,
 
 		switch (status & MV88E6390_SGMII_PHY_STATUS_SPEED_MASK) {
 		case MV88E6390_SGMII_PHY_STATUS_SPEED_1000:
-			if (state->interface == PHY_INTERFACE_MODE_2500BASEX)
-				state->speed = SPEED_2500;
-			else
-				state->speed = SPEED_1000;
+			state->speed = SPEED_1000;
 			break;
 		case MV88E6390_SGMII_PHY_STATUS_SPEED_100:
 			state->speed = SPEED_100;
@@ -85,10 +93,7 @@ static int mv88e6xxx_serdes_pcs_get_state(struct mv88e6xxx_chip *chip,
 		state->link = false;
 	}
 
-	if (state->interface == PHY_INTERFACE_MODE_2500BASEX)
-		mii_lpa_mod_linkmode_x(state->lp_advertising, lpa,
-				       ETHTOOL_LINK_MODE_2500baseX_Full_BIT);
-	else if (state->interface == PHY_INTERFACE_MODE_1000BASEX)
+	if (state->interface == PHY_INTERFACE_MODE_1000BASEX)
 		mii_lpa_mod_linkmode_x(state->lp_advertising, lpa,
 				       ETHTOOL_LINK_MODE_1000baseX_Full_BIT);
 
@@ -820,9 +825,10 @@ int mv88e6390_serdes_pcs_config(struct mv88e6xxx_chip *chip, int port,
 		break;
 
 	case PHY_INTERFACE_MODE_2500BASEX:
-		adv = linkmode_adv_to_mii_adv_x(advertise,
-					ETHTOOL_LINK_MODE_2500baseX_Full_BIT);
-		break;
+		if (phylink_autoneg_inband(mode))
+			dev_warn(chip->dev,
+				 "inband AN unsupported on 2500base-x mode\n");
+		return 0;
 
 	default:
 		return 0;

base-commit: f50e2f9f791647aa4e5b19d0064f5cabf630bf6e
-- 
2.26.2


             reply	other threads:[~2021-01-14  2:42 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-14  2:40 Marek Behún [this message]
2021-01-14 23:00 ` [PATCH net] net: dsa: mv88e6xxx: do not allow inband AN for 2500base-x mode Andrew Lunn
2021-01-14 23:55   ` Marek Behún
2021-01-15 14:16     ` Andrew Lunn

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210114024055.17602-1-kabel@kernel.org \
    --to=kabel@kernel.org \
    --cc=andrew@lunn.ch \
    --cc=ashkan.boldaji@digi.com \
    --cc=chris.packham@alliedtelesis.co.nz \
    --cc=davem@davemloft.net \
    --cc=f.fainelli@gmail.com \
    --cc=kuba@kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=lkp@intel.com \
    --cc=netdev@vger.kernel.org \
    --cc=olteanv@gmail.com \
    --cc=pavana.sharma@digi.com \
    --cc=rmk+kernel@armlinux.org.uk \
    --cc=vivien.didelot@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).