[PATCH net-next v1 1/3] net: phy: micrel: Extend KSZ886X PHY Special Ctrl/Status Reg definitions

[PATCH net-next v1 1/3] net: phy: micrel: Extend KSZ886X PHY Special Ctrl/Status Reg definitions

[Oleksij Rempel]

Extend 'micrel_phy.h' with additional definitions for KSZ886X PHY
Special Control/Status Register (Reg 31), for upcoming usage in
subsequent patches.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 include/linux/micrel_phy.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/linux/micrel_phy.h b/include/linux/micrel_phy.h
index 4e27ca7c49de..591bf5b5e8dc 100644
--- a/include/linux/micrel_phy.h
+++ b/include/linux/micrel_phy.h
@@ -64,6 +64,10 @@
 #define KSZ886X_BMCR_DISABLE_TRANSMIT		BIT(1)
 #define KSZ886X_BMCR_DISABLE_LED		BIT(0)
 
+/* PHY Special Control/Status Register (Reg 31) */
 #define KSZ886X_CTRL_MDIX_STAT			BIT(4)
+#define KSZ886X_CTRL_FORCE_LINK			BIT(3)
+#define KSZ886X_CTRL_PWRSAVE			BIT(2)
+#define KSZ886X_CTRL_REMOTE_LOOPBACK		BIT(1)
 
 #endif /* _MICREL_PHY_H */
-- 
2.39.2

[PATCH net-next v1 2/3] net: dsa: microchip: ksz8: Enable MIIM PHY Control reg access

[Oleksij Rempel]

Provide access to MIIM PHY Control register (Reg. 31) through
ksz8_r_phy_ctrl() and ksz8_w_phy_ctrl() functions. Necessary for
upcoming micrel.c patch to address forced link mode configuration.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/net/dsa/microchip/ksz8795.c | 81 +++++++++++++++++++++++++++--
 1 file changed, 78 insertions(+), 3 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c
index 91aba470fb2f..11cb054cb54f 100644
--- a/drivers/net/dsa/microchip/ksz8795.c
+++ b/drivers/net/dsa/microchip/ksz8795.c
@@ -632,6 +632,47 @@ static void ksz8_w_vlan_table(struct ksz_device *dev, u16 vid, u16 vlan)
 	ksz8_w_table(dev, TABLE_VLAN, addr, buf);
 }
 
+/**
+ * ksz8_r_phy_ctrl - Translates and reads from the SMI interface to a MIIM PHY
+ *		     Control register (Reg. 31).
+ * @dev: The KSZ device instance.
+ * @port: The port number to be read.
+ *
+ * This function reads the SMI interface and translates the hardware register
+ * bit values into their corresponding control settings for a MIIM PHY Control
+ * register.
+ */
+static int ksz8_r_phy_ctrl(struct ksz_device *dev, int port, u16 *val)
+{
+	const u16 *regs = dev->info->regs;
+	u8 reg_val;
+	int ret;
+
+	*val = 0;
+
+	ret = ksz_pread8(dev, port, regs[P_LINK_STATUS], &reg_val);
+	if (ret < 0)
+		return ret;
+
+	if (reg_val & PORT_MDIX_STATUS)
+		*val |= KSZ886X_CTRL_MDIX_STAT;
+
+	ret = ksz_pread8(dev, port, REG_PORT_LINK_MD_CTRL, &reg_val);
+	if (ret < 0)
+		return ret;
+
+	if (reg_val & PORT_FORCE_LINK)
+		*val |= KSZ886X_CTRL_FORCE_LINK;
+
+	if (reg_val & PORT_POWER_SAVING)
+		*val |= KSZ886X_CTRL_PWRSAVE;
+
+	if (reg_val & PORT_PHY_REMOTE_LOOPBACK)
+		*val |= KSZ886X_CTRL_REMOTE_LOOPBACK;
+
+	return 0;
+}
+
 int ksz8_r_phy(struct ksz_device *dev, u16 phy, u16 reg, u16 *val)
 {
 	u8 restart, speed, ctrl, link;
@@ -769,12 +810,10 @@ int ksz8_r_phy(struct ksz_device *dev, u16 phy, u16 reg, u16 *val)
 				FIELD_GET(PORT_CABLE_FAULT_COUNTER_L, val2));
 		break;
 	case PHY_REG_PHY_CTRL:
-		ret = ksz_pread8(dev, p, regs[P_LINK_STATUS], &link);
+		ret = ksz8_r_phy_ctrl(dev, p, &data);
 		if (ret)
 			return ret;
 
-		if (link & PORT_MDIX_STATUS)
-			data |= KSZ886X_CTRL_MDIX_STAT;
 		break;
 	default:
 		processed = false;
@@ -786,6 +825,36 @@ int ksz8_r_phy(struct ksz_device *dev, u16 phy, u16 reg, u16 *val)
 	return 0;
 }
 
+/**
+ * ksz8_w_phy_ctrl - Translates and writes to the SMI interface from a MIIM PHY
+ *		     Control register (Reg. 31).
+ * @dev: The KSZ device instance.
+ * @port: The port number to be configured.
+ * @val: The register value to be written.
+ *
+ * This function translates control settings from a MIIM PHY Control register
+ * into their corresponding hardware register bit values for the SMI
+ * interface.
+ */
+static int ksz8_w_phy_ctrl(struct ksz_device *dev, int port, u16 val)
+{
+	u8 reg_val = 0;
+	int ret;
+
+	if (val & KSZ886X_CTRL_FORCE_LINK)
+		reg_val |= PORT_FORCE_LINK;
+
+	if (val & KSZ886X_CTRL_PWRSAVE)
+		reg_val |= PORT_POWER_SAVING;
+
+	if (val & KSZ886X_CTRL_REMOTE_LOOPBACK)
+		reg_val |= PORT_PHY_REMOTE_LOOPBACK;
+
+	ret = ksz_prmw8(dev, port, REG_PORT_LINK_MD_CTRL, PORT_FORCE_LINK |
+			PORT_POWER_SAVING | PORT_PHY_REMOTE_LOOPBACK, reg_val);
+	return ret;
+}
+
 int ksz8_w_phy(struct ksz_device *dev, u16 phy, u16 reg, u16 val)
 {
 	u8 restart, speed, ctrl, data;
@@ -926,6 +995,12 @@ int ksz8_w_phy(struct ksz_device *dev, u16 phy, u16 reg, u16 val)
 		if (val & PHY_START_CABLE_DIAG)
 			ksz_port_cfg(dev, p, REG_PORT_LINK_MD_CTRL, PORT_START_CABLE_DIAG, true);
 		break;
+
+	case PHY_REG_PHY_CTRL:
+		ret = ksz8_w_phy_ctrl(dev, p, val);
+		if (ret)
+			return ret;
+		break;
 	default:
 		break;
 	}
-- 
2.39.2

[PATCH net-next v1 3/3] net: phy: micrel: Fix forced link mode for KSZ886X switches

[Oleksij Rempel]

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 | 32 +++++++++++++++++++++++++++++---
 1 file changed, 29 insertions(+), 3 deletions(-)

diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 927d3d54658e..12f093aed4ff 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -1729,9 +1729,35 @@ static int ksz886x_config_aneg(struct phy_device *phydev)
 {
 	int ret;
 
-	ret = genphy_config_aneg(phydev);
-	if (ret)
-		return ret;
+	if (phydev->autoneg != AUTONEG_ENABLE) {
+		ret = genphy_setup_forced(phydev);
+		if (ret)
+			return ret;
+
+		/* 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 {
+		/* Make sure, the link state is not forced.
+		 * Otherwise, the PHY we create a link by skipping the
+		 * autonegotiation process.
+		 */
+		ret = phy_clear_bits(phydev, MII_KSZPHY_CTRL,
+				     KSZ886X_CTRL_FORCE_LINK);
+		if (ret)
+			return ret;
+
+		ret = genphy_config_aneg(phydev);
+		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
-- 
2.39.2

Re: [PATCH net-next v1 3/3] net: phy: micrel: Fix forced link mode for KSZ886X switches

[Alexander Stein]

Hi Oleksij,

Am Mittwoch, 11. Oktober 2023, 14:38:56 CEST schrieb Oleksij Rempel:
> 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 | 32 +++++++++++++++++++++++++++++---
>  1 file changed, 29 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
> index 927d3d54658e..12f093aed4ff 100644
> --- a/drivers/net/phy/micrel.c
> +++ b/drivers/net/phy/micrel.c
> @@ -1729,9 +1729,35 @@ static int ksz886x_config_aneg(struct phy_device
> *phydev) {
>  	int ret;
> 
> -	ret = genphy_config_aneg(phydev);
> -	if (ret)
> -		return ret;
> +	if (phydev->autoneg != AUTONEG_ENABLE) {
> +		ret = genphy_setup_forced(phydev);
> +		if (ret)
> +			return ret;
> +
> +		/* 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 {
> +		/* Make sure, the link state is not forced.
> +		 * Otherwise, the PHY we create a link by skipping the

PHY will create?

> +		 * autonegotiation process.
> +		 */
> +		ret = phy_clear_bits(phydev, MII_KSZPHY_CTRL,
> +				     KSZ886X_CTRL_FORCE_LINK);
> +		if (ret)
> +			return ret;

Isn't this call to phy_clear_bits() a fix for autonegotiation mode? This 
should be a separate patch then.

Best regards,
Alexander

> +
> +		ret = genphy_config_aneg(phydev);
> +		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


-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/

Re: [PATCH net-next v1 3/3] net: phy: micrel: Fix forced link mode for KSZ886X switches

[Oleksij Rempel]

Hi Alexander,

thank you for review!

On Wed, Oct 11, 2023 at 03:29:49PM +0200, Alexander Stein wrote:
> Hi Oleksij,
> 
> Am Mittwoch, 11. Oktober 2023, 14:38:56 CEST schrieb Oleksij Rempel:
> > 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 | 32 +++++++++++++++++++++++++++++---
> >  1 file changed, 29 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
> > index 927d3d54658e..12f093aed4ff 100644
> > --- a/drivers/net/phy/micrel.c
> > +++ b/drivers/net/phy/micrel.c
> > @@ -1729,9 +1729,35 @@ static int ksz886x_config_aneg(struct phy_device
> > *phydev) {
> >  	int ret;
> > 
> > -	ret = genphy_config_aneg(phydev);
> > -	if (ret)
> > -		return ret;
> > +	if (phydev->autoneg != AUTONEG_ENABLE) {
> > +		ret = genphy_setup_forced(phydev);
> > +		if (ret)
> > +			return ret;
> > +
> > +		/* 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 {
> > +		/* Make sure, the link state is not forced.
> > +		 * Otherwise, the PHY we create a link by skipping the
> 
> PHY will create?
> 
> > +		 * autonegotiation process.
> > +		 */
> > +		ret = phy_clear_bits(phydev, MII_KSZPHY_CTRL,
> > +				     KSZ886X_CTRL_FORCE_LINK);
> > +		if (ret)
> > +			return ret;
> 
> Isn't this call to phy_clear_bits() a fix for autonegotiation mode? This 
> should be a separate patch then.

First time this bit is set by this patch, I assume this problem would
not exist without fixed link fix.

Regards,
Oleksij
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH net-next v1 2/3] net: dsa: microchip: ksz8: Enable MIIM PHY Control reg access

[kernel test robot]

Hi Oleksij,

kernel test robot noticed the following build warnings:

[auto build test WARNING on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Oleksij-Rempel/net-dsa-microchip-ksz8-Enable-MIIM-PHY-Control-reg-access/20231011-204502
base:   net-next/main
patch link:    https://lore.kernel.org/r/20231011123856.1443308-2-o.rempel%40pengutronix.de
patch subject: [PATCH net-next v1 2/3] net: dsa: microchip: ksz8: Enable MIIM PHY Control reg access
config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20231011/202310112224.iYgvjBUy-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231011/202310112224.iYgvjBUy-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202310112224.iYgvjBUy-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/net/dsa/microchip/ksz8795.c:646: warning: Function parameter or member 'val' not described in 'ksz8_r_phy_ctrl'


vim +646 drivers/net/dsa/microchip/ksz8795.c

   634	
   635	/**
   636	 * ksz8_r_phy_ctrl - Translates and reads from the SMI interface to a MIIM PHY
   637	 *		     Control register (Reg. 31).
   638	 * @dev: The KSZ device instance.
   639	 * @port: The port number to be read.
   640	 *
   641	 * This function reads the SMI interface and translates the hardware register
   642	 * bit values into their corresponding control settings for a MIIM PHY Control
   643	 * register.
   644	 */
   645	static int ksz8_r_phy_ctrl(struct ksz_device *dev, int port, u16 *val)
 > 646	{
   647		const u16 *regs = dev->info->regs;
   648		u8 reg_val;
   649		int ret;
   650	
   651		*val = 0;
   652	
   653		ret = ksz_pread8(dev, port, regs[P_LINK_STATUS], &reg_val);
   654		if (ret < 0)
   655			return ret;
   656	
   657		if (reg_val & PORT_MDIX_STATUS)
   658			*val |= KSZ886X_CTRL_MDIX_STAT;
   659	
   660		ret = ksz_pread8(dev, port, REG_PORT_LINK_MD_CTRL, &reg_val);
   661		if (ret < 0)
   662			return ret;
   663	
   664		if (reg_val & PORT_FORCE_LINK)
   665			*val |= KSZ886X_CTRL_FORCE_LINK;
   666	
   667		if (reg_val & PORT_POWER_SAVING)
   668			*val |= KSZ886X_CTRL_PWRSAVE;
   669	
   670		if (reg_val & PORT_PHY_REMOTE_LOOPBACK)
   671			*val |= KSZ886X_CTRL_REMOTE_LOOPBACK;
   672	
   673		return 0;
   674	}
   675	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH net-next v1 2/3] net: dsa: microchip: ksz8: Enable MIIM PHY Control reg access

[Simon Horman]

On Wed, Oct 11, 2023 at 02:38:55PM +0200, Oleksij Rempel wrote:
> Provide access to MIIM PHY Control register (Reg. 31) through
> ksz8_r_phy_ctrl() and ksz8_w_phy_ctrl() functions. Necessary for
> upcoming micrel.c patch to address forced link mode configuration.
> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
>  drivers/net/dsa/microchip/ksz8795.c | 81 +++++++++++++++++++++++++++--
>  1 file changed, 78 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c
> index 91aba470fb2f..11cb054cb54f 100644
> --- a/drivers/net/dsa/microchip/ksz8795.c
> +++ b/drivers/net/dsa/microchip/ksz8795.c
> @@ -632,6 +632,47 @@ static void ksz8_w_vlan_table(struct ksz_device *dev, u16 vid, u16 vlan)
>  	ksz8_w_table(dev, TABLE_VLAN, addr, buf);
>  }
>  
> +/**
> + * ksz8_r_phy_ctrl - Translates and reads from the SMI interface to a MIIM PHY
> + *		     Control register (Reg. 31).
> + * @dev: The KSZ device instance.
> + * @port: The port number to be read.

nit: please include an entry for @val here

> + *
> + * This function reads the SMI interface and translates the hardware register
> + * bit values into their corresponding control settings for a MIIM PHY Control
> + * register.
> + */
> +static int ksz8_r_phy_ctrl(struct ksz_device *dev, int port, u16 *val)

...

Re: [PATCH net-next v1 3/3] net: phy: micrel: Fix forced link mode for KSZ886X switches

[Simon Horman]

On Wed, Oct 11, 2023 at 02:38:56PM +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>
> ---
>  drivers/net/phy/micrel.c | 32 +++++++++++++++++++++++++++++---
>  1 file changed, 29 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
> index 927d3d54658e..12f093aed4ff 100644
> --- a/drivers/net/phy/micrel.c
> +++ b/drivers/net/phy/micrel.c
> @@ -1729,9 +1729,35 @@ static int ksz886x_config_aneg(struct phy_device *phydev)
>  {
>  	int ret;
>  
> -	ret = genphy_config_aneg(phydev);
> -	if (ret)
> -		return ret;
> +	if (phydev->autoneg != AUTONEG_ENABLE) {
> +		ret = genphy_setup_forced(phydev);
> +		if (ret)
> +			return ret;
> +
> +		/* When autonegotation is disabled, we need to manually force

nit: autonegotiation

> +		 * 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.
> +		 */

...

Re: [PATCH net-next v1 3/3] net: phy: micrel: Fix forced link mode for KSZ886X switches

[Russell King (Oracle)]

On Wed, Oct 11, 2023 at 03:29:49PM +0200, Alexander Stein wrote:
> Hi Oleksij,
> 
> Am Mittwoch, 11. Oktober 2023, 14:38:56 CEST schrieb Oleksij Rempel:
> > +	if (phydev->autoneg != AUTONEG_ENABLE) {
> > +		ret = genphy_setup_forced(phydev);
> > +		if (ret)
> > +			return ret;
> > +
...
> > +		ret = phy_set_bits(phydev, MII_KSZPHY_CTRL,
> > +				   KSZ886X_CTRL_FORCE_LINK);
> > +		if (ret)
> > +			return ret;
> > +	} else {
...
> > +		ret = phy_clear_bits(phydev, MII_KSZPHY_CTRL,
> > +				     KSZ886X_CTRL_FORCE_LINK);
> > +		if (ret)
> > +			return ret;
> 
> Isn't this call to phy_clear_bits() a fix for autonegotiation mode? This 
> should be a separate patch then.

No, I don't think that is the case. Compare the two paths above, noting
that patch 1 introduces the definition for KSZ886X_CTRL_FORCE_LINK.

If autoneg is disabled, then this bit is then set, which forces the
link. Clearly, if autoneg is then re-enabled, this bit has to be
cleared to allow the effects of the autoneg-disabled path to be undone.

So both of these, the phy_set_bits() and the phy_clear_bits() belong in
the same patch. Splitting them up, so we introduce phy_set_bits() first
will create a regression - which we don't want.

These two belong logically together.

What does concern me, however, is that the autoneg-disabled path avoids
calling genphy_setup_master_slave(), and since establishing which end
of the link is part of the fundamentals of a 1000base-T link, I wonder
whether both paths should still call genphy_config_aneg().

Apart from that, I think the patch is otherwise fine.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!