diff mbox series

[net-next,v3,2/2] net: phy: micrel: Fix forced link mode for KSZ886X switches

Message ID 20231019111459.1000218-3-o.rempel@pengutronix.de (mailing list archive)
State Accepted
Delegated to: Netdev Maintainers
Headers show
Series fix forced link mode for KSZ886X switches | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 1363 this patch: 1363
netdev/cc_maintainers success CCed 8 of 8 maintainers
netdev/build_clang success Errors and warnings before: 1388 this patch: 1388
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 1388 this patch: 1388
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 28 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Oleksij Rempel Oct. 19, 2023, 11:14 a.m. UTC
Address a link speed detection issue in KSZ886X PHY driver when in
forced link mode. Previously, link partners like "ASIX AX88772B"
with KSZ8873 could fall back to 10Mbit instead of configured 100Mbit.

The issue arises as KSZ886X PHY continues sending Fast Link Pulses (FLPs)
even with autonegotiation off, misleading link partners in autoneg mode,
leading to incorrect link speed detection.

Now, when autonegotiation is disabled, the driver sets the link state
forcefully using KSZ886X_CTRL_FORCE_LINK bit. This action, beyond just
disabling autonegotiation, makes the PHY state more reliably detected by
link partners using parallel detection, thus fixing the link speed
misconfiguration.

With autonegotiation enabled, link state is not forced, allowing proper
autonegotiation process participation.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/net/phy/micrel.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

Comments

Vladimir Oltean Oct. 19, 2023, 4:32 p.m. UTC | #1
On Thu, Oct 19, 2023 at 01:14:59PM +0200, Oleksij Rempel wrote:
> Address a link speed detection issue in KSZ886X PHY driver when in
> forced link mode. Previously, link partners like "ASIX AX88772B"
> with KSZ8873 could fall back to 10Mbit instead of configured 100Mbit.
> 
> The issue arises as KSZ886X PHY continues sending Fast Link Pulses (FLPs)
> even with autonegotiation off, misleading link partners in autoneg mode,
> leading to incorrect link speed detection.
> 
> Now, when autonegotiation is disabled, the driver sets the link state
> forcefully using KSZ886X_CTRL_FORCE_LINK bit. This action, beyond just
> disabling autonegotiation, makes the PHY state more reliably detected by
> link partners using parallel detection, thus fixing the link speed
> misconfiguration.
> 
> With autonegotiation enabled, link state is not forced, allowing proper
> autonegotiation process participation.
> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---

Reviewed-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Divya Koppera Oct. 20, 2023, 8:01 a.m. UTC | #2
> -----Original Message-----
> From: Oleksij Rempel <o.rempel@pengutronix.de>
> Sent: Thursday, October 19, 2023 4:45 PM
> To: Woojung Huh - C21699 <Woojung.Huh@microchip.com>;
> UNGLinuxDriver <UNGLinuxDriver@microchip.com>; Andrew Lunn
> <andrew@lunn.ch>; Florian Fainelli <f.fainelli@gmail.com>; Vladimir Oltean
> <olteanv@gmail.com>; David S. Miller <davem@davemloft.net>; Eric Dumazet
> <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo Abeni
> <pabeni@redhat.com>; Heiner Kallweit <hkallweit1@gmail.com>
> Cc: Oleksij Rempel <o.rempel@pengutronix.de>; kernel@pengutronix.de;
> linux-kernel@vger.kernel.org; Russell King <linux@armlinux.org.uk>;
> netdev@vger.kernel.org; Alexander Stein <alexander.stein@ew.tq-group.com>
> Subject: [PATCH net-next v3 2/2] net: phy: micrel: Fix forced link mode for
> KSZ886X switches
> 
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
> 
> Address a link speed detection issue in KSZ886X PHY driver when in forced link
> mode. Previously, link partners like "ASIX AX88772B"
> with KSZ8873 could fall back to 10Mbit instead of configured 100Mbit.
> 
> The issue arises as KSZ886X PHY continues sending Fast Link Pulses (FLPs)
> even with autonegotiation off, misleading link partners in autoneg mode,
> leading to incorrect link speed detection.
> 
> Now, when autonegotiation is disabled, the driver sets the link state forcefully
> using KSZ886X_CTRL_FORCE_LINK bit. This action, beyond just disabling
> autonegotiation, makes the PHY state more reliably detected by link partners
> using parallel detection, thus fixing the link speed misconfiguration.
> 
> With autonegotiation enabled, link state is not forced, allowing proper
> autonegotiation process participation.
> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
>  drivers/net/phy/micrel.c | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c index
> 927d3d54658e..08e3915001c3 100644
> --- a/drivers/net/phy/micrel.c
> +++ b/drivers/net/phy/micrel.c
> @@ -1733,6 +1733,28 @@ static int ksz886x_config_aneg(struct phy_device
> *phydev)
>         if (ret)
>                 return ret;
> 
> +       if (phydev->autoneg != AUTONEG_ENABLE) {
> +               /* When autonegotation is disabled, we need to manually force
> +                * the link state. If we don't do this, the PHY will keep
> +                * sending Fast Link Pulses (FLPs) which are part of the
> +                * autonegotiation process. This is not desired when
> +                * autonegotiation is off.
> +                */
> +               ret = phy_set_bits(phydev, MII_KSZPHY_CTRL,
> +                                  KSZ886X_CTRL_FORCE_LINK);
> +               if (ret)
> +                       return ret;
> +       } else {
> +               /* If we had previously forced the link state, we need to
> +                * clear KSZ886X_CTRL_FORCE_LINK bit now. Otherwise, the PHY
> +                * will not perform autonegotiation.
> +                */
> +               ret = phy_clear_bits(phydev, MII_KSZPHY_CTRL,
> +                                    KSZ886X_CTRL_FORCE_LINK);
> +               if (ret)
> +                       return ret;
> +       }
> +
>         /* The MDI-X configuration is automatically changed by the PHY after
>          * switching from autoneg off to on. So, take MDI-X configuration under
>          * own control and set it after autoneg configuration was done.
> --
> 2.39.2

Reviewed-by: Divya Koppera <divya.koppera@microchip.com>
diff mbox series

Patch

diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 927d3d54658e..08e3915001c3 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -1733,6 +1733,28 @@  static int ksz886x_config_aneg(struct phy_device *phydev)
 	if (ret)
 		return ret;
 
+	if (phydev->autoneg != AUTONEG_ENABLE) {
+		/* When autonegotation is disabled, we need to manually force
+		 * the link state. If we don't do this, the PHY will keep
+		 * sending Fast Link Pulses (FLPs) which are part of the
+		 * autonegotiation process. This is not desired when
+		 * autonegotiation is off.
+		 */
+		ret = phy_set_bits(phydev, MII_KSZPHY_CTRL,
+				   KSZ886X_CTRL_FORCE_LINK);
+		if (ret)
+			return ret;
+	} else {
+		/* If we had previously forced the link state, we need to
+		 * clear KSZ886X_CTRL_FORCE_LINK bit now. Otherwise, the PHY
+		 * will not perform autonegotiation.
+		 */
+		ret = phy_clear_bits(phydev, MII_KSZPHY_CTRL,
+				     KSZ886X_CTRL_FORCE_LINK);
+		if (ret)
+			return ret;
+	}
+
 	/* The MDI-X configuration is automatically changed by the PHY after
 	 * switching from autoneg off to on. So, take MDI-X configuration under
 	 * own control and set it after autoneg configuration was done.