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

[Divya.Koppera]

> -----Original Message-----
> From: Oleksij Rempel <o.rempel@xxxxxxxxxxxxxx>
> Sent: Thursday, October 19, 2023 4:45 PM
> To: Woojung Huh - C21699 <Woojung.Huh@xxxxxxxxxxxxx>;
> UNGLinuxDriver <UNGLinuxDriver@xxxxxxxxxxxxx>; Andrew Lunn
> <andrew@xxxxxxx>; Florian Fainelli <f.fainelli@xxxxxxxxx>; Vladimir Oltean
> <olteanv@xxxxxxxxx>; David S. Miller <davem@xxxxxxxxxxxxx>; Eric Dumazet
> <edumazet@xxxxxxxxxx>; Jakub Kicinski <kuba@xxxxxxxxxx>; Paolo Abeni
> <pabeni@xxxxxxxxxx>; Heiner Kallweit <hkallweit1@xxxxxxxxx>
> Cc: Oleksij Rempel <o.rempel@xxxxxxxxxxxxxx>; kernel@xxxxxxxxxxxxxx;
> linux-kernel@xxxxxxxxxxxxxxx; Russell King <linux@xxxxxxxxxxxxxxx>;
> netdev@xxxxxxxxxxxxxxx; Alexander Stein <alexander.stein@xxxxxxxxxxxxxxx>
> 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@xxxxxxxxxxxxxx>
> ---
>  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@xxxxxxxxxxxxx>