First, there are some SFP modules that only uses RX_LOS for link
indication. Add check that link is operational before actual read of
line-side status.
Second, it is invalid to set 10G speed without autonegotiation,
according to phy_ethtool_ksettings_set(). Implement switching between
10GBase-R and 1000Base-X/SGMII if autonegotiation can't complete but
there is signal in line.
Changelog:
v1 -> v2:
* make checking that link is operational more friendly for
trancievers without SFP cages.
* split swapping 1G/10G modes into non-functional and functional
commits for the sake of easier review.
Ivan Bornyakov (3):
net: phy: marvell-88x2222: check that link is operational
net: phy: marvell-88x2222: move read_status after config_aneg
net: phy: marvell-88x2222: swap 1G/10G modes on autoneg
drivers/net/phy/marvell-88x2222.c | 314 ++++++++++++++++++++----------
1 file changed, 209 insertions(+), 105 deletions(-)
--
2.26.3
Setting 10G without autonegotiation is invalid according to
phy_ethtool_ksettings_set(). Thus, we need to set it during
autonegotiation.
If 1G autonegotiation can't complete for quite a time, but there is
signal in line, switch line interface type to 10GBase-R, if supported,
in hope for link to be established.
And vice versa. If 10GBase-R link can't be established for quite a time,
and autonegotiation is enabled, and there is signal in line, switch line
interface type to appropriate 1G mode, i.e. 1000Base-X or SGMII, if
supported.
Signed-off-by: Ivan Bornyakov <[email protected]>
---
drivers/net/phy/marvell-88x2222.c | 117 +++++++++++++++++++++++-------
1 file changed, 89 insertions(+), 28 deletions(-)
diff --git a/drivers/net/phy/marvell-88x2222.c b/drivers/net/phy/marvell-88x2222.c
index 640b133f1371..9b9ac3ef735d 100644
--- a/drivers/net/phy/marvell-88x2222.c
+++ b/drivers/net/phy/marvell-88x2222.c
@@ -52,6 +52,8 @@
#define MV_1GBX_PHY_STAT_SPEED100 BIT(14)
#define MV_1GBX_PHY_STAT_SPEED1000 BIT(15)
+#define AUTONEG_TIMEOUT 3
+
struct mv2222_data {
phy_interface_t line_interface;
__ETHTOOL_DECLARE_LINK_MODE_MASK(supported);
@@ -173,6 +175,24 @@ static bool mv2222_is_1gbx_capable(struct phy_device *phydev)
priv->supported);
}
+static bool mv2222_is_sgmii_capable(struct phy_device *phydev)
+{
+ struct mv2222_data *priv = phydev->priv;
+
+ return (linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT,
+ priv->supported) ||
+ linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseT_Half_BIT,
+ priv->supported) ||
+ linkmode_test_bit(ETHTOOL_LINK_MODE_100baseT_Full_BIT,
+ priv->supported) ||
+ linkmode_test_bit(ETHTOOL_LINK_MODE_100baseT_Half_BIT,
+ priv->supported) ||
+ linkmode_test_bit(ETHTOOL_LINK_MODE_10baseT_Full_BIT,
+ priv->supported) ||
+ linkmode_test_bit(ETHTOOL_LINK_MODE_10baseT_Half_BIT,
+ priv->supported));
+}
+
static int mv2222_config_line(struct phy_device *phydev)
{
struct mv2222_data *priv = phydev->priv;
@@ -192,7 +212,8 @@ static int mv2222_config_line(struct phy_device *phydev)
}
}
-static int mv2222_setup_forced(struct phy_device *phydev)
+/* Switch between 1G (1000Base-X/SGMII) and 10G (10GBase-R) modes */
+static int mv2222_swap_line_type(struct phy_device *phydev)
{
struct mv2222_data *priv = phydev->priv;
bool changed = false;
@@ -200,25 +221,23 @@ static int mv2222_setup_forced(struct phy_device *phydev)
switch (priv->line_interface) {
case PHY_INTERFACE_MODE_10GBASER:
- if (phydev->speed == SPEED_1000 &&
- mv2222_is_1gbx_capable(phydev)) {
+ if (mv2222_is_1gbx_capable(phydev)) {
priv->line_interface = PHY_INTERFACE_MODE_1000BASEX;
changed = true;
}
- break;
- case PHY_INTERFACE_MODE_1000BASEX:
- if (phydev->speed == SPEED_10000 &&
- mv2222_is_10g_capable(phydev)) {
- priv->line_interface = PHY_INTERFACE_MODE_10GBASER;
+ if (mv2222_is_sgmii_capable(phydev)) {
+ priv->line_interface = PHY_INTERFACE_MODE_SGMII;
changed = true;
}
break;
+ case PHY_INTERFACE_MODE_1000BASEX:
case PHY_INTERFACE_MODE_SGMII:
- ret = mv2222_set_sgmii_speed(phydev);
- if (ret < 0)
- return ret;
+ if (mv2222_is_10g_capable(phydev)) {
+ priv->line_interface = PHY_INTERFACE_MODE_10GBASER;
+ changed = true;
+ }
break;
default:
@@ -231,6 +250,29 @@ static int mv2222_setup_forced(struct phy_device *phydev)
return ret;
}
+ return 0;
+}
+
+static int mv2222_setup_forced(struct phy_device *phydev)
+{
+ struct mv2222_data *priv = phydev->priv;
+ int ret;
+
+ if (priv->line_interface == PHY_INTERFACE_MODE_10GBASER) {
+ if (phydev->speed < SPEED_10000 &&
+ phydev->speed != SPEED_UNKNOWN) {
+ ret = mv2222_swap_line_type(phydev);
+ if (ret < 0)
+ return ret;
+ }
+ }
+
+ if (priv->line_interface == PHY_INTERFACE_MODE_SGMII) {
+ ret = mv2222_set_sgmii_speed(phydev);
+ if (ret < 0)
+ return ret;
+ }
+
return mv2222_disable_aneg(phydev);
}
@@ -244,17 +286,9 @@ static int mv2222_config_aneg(struct phy_device *phydev)
return 0;
if (phydev->autoneg == AUTONEG_DISABLE ||
- phydev->speed == SPEED_10000)
+ priv->line_interface == PHY_INTERFACE_MODE_10GBASER)
return mv2222_setup_forced(phydev);
- if (priv->line_interface == PHY_INTERFACE_MODE_10GBASER &&
- mv2222_is_1gbx_capable(phydev)) {
- priv->line_interface = PHY_INTERFACE_MODE_1000BASEX;
- ret = mv2222_config_line(phydev);
- if (ret < 0)
- return ret;
- }
-
adv = linkmode_adv_to_mii_adv_x(priv->supported,
ETHTOOL_LINK_MODE_1000baseX_Full_BIT);
@@ -291,6 +325,7 @@ static int mv2222_aneg_done(struct phy_device *phydev)
/* Returns negative on error, 0 if link is down, 1 if link is up */
static int mv2222_read_status_10g(struct phy_device *phydev)
{
+ static int timeout;
int val, link = 0;
val = phy_read_mmd(phydev, MDIO_MMD_PCS, MDIO_STAT1);
@@ -304,6 +339,20 @@ static int mv2222_read_status_10g(struct phy_device *phydev)
phydev->autoneg = AUTONEG_DISABLE;
phydev->speed = SPEED_10000;
phydev->duplex = DUPLEX_FULL;
+ } else {
+ if (phydev->autoneg == AUTONEG_ENABLE) {
+ timeout++;
+
+ if (timeout > AUTONEG_TIMEOUT) {
+ timeout = 0;
+
+ val = mv2222_swap_line_type(phydev);
+ if (val < 0)
+ return val;
+
+ return mv2222_config_aneg(phydev);
+ }
+ }
}
return link;
@@ -312,15 +361,31 @@ static int mv2222_read_status_10g(struct phy_device *phydev)
/* Returns negative on error, 0 if link is down, 1 if link is up */
static int mv2222_read_status_1g(struct phy_device *phydev)
{
+ static int timeout;
int val, link = 0;
val = phy_read_mmd(phydev, MDIO_MMD_PCS, MV_1GBX_STAT);
if (val < 0)
return val;
- if (!(val & BMSR_LSTATUS) ||
- (phydev->autoneg == AUTONEG_ENABLE &&
- !(val & BMSR_ANEGCOMPLETE)))
+ if (phydev->autoneg == AUTONEG_ENABLE &&
+ !(val & BMSR_ANEGCOMPLETE)) {
+ timeout++;
+
+ if (timeout > AUTONEG_TIMEOUT) {
+ timeout = 0;
+
+ val = mv2222_swap_line_type(phydev);
+ if (val < 0)
+ return val;
+
+ return mv2222_config_aneg(phydev);
+ }
+
+ return 0;
+ }
+
+ if (!(val & BMSR_LSTATUS))
return 0;
link = 1;
@@ -447,11 +512,7 @@ static int mv2222_sfp_insert(void *upstream, const struct sfp_eeprom_id *id)
return ret;
if (mutex_trylock(&phydev->lock)) {
- if (priv->line_interface == PHY_INTERFACE_MODE_10GBASER)
- ret = mv2222_setup_forced(phydev);
- else
- ret = mv2222_config_aneg(phydev);
-
+ ret = mv2222_config_aneg(phydev);
mutex_unlock(&phydev->lock);
}
--
2.26.3
Hello:
This series was applied to netdev/net-next.git (refs/heads/master):
On Tue, 13 Apr 2021 23:54:49 +0300 you wrote:
> First, there are some SFP modules that only uses RX_LOS for link
> indication. Add check that link is operational before actual read of
> line-side status.
>
> Second, it is invalid to set 10G speed without autonegotiation,
> according to phy_ethtool_ksettings_set(). Implement switching between
> 10GBase-R and 1000Base-X/SGMII if autonegotiation can't complete but
> there is signal in line.
>
> [...]
Here is the summary with links:
- [net-next,v2,1/3] net: phy: marvell-88x2222: check that link is operational
https://git.kernel.org/netdev/net-next/c/58581478a734
- [net-next,v2,2/3] net: phy: marvell-88x2222: move read_status after config_aneg
https://git.kernel.org/netdev/net-next/c/473960a7b443
- [net-next,v2,3/3] net: phy: marvell-88x2222: swap 1G/10G modes on autoneg
https://git.kernel.org/netdev/net-next/c/d7029f55cc46
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html