[PATCH] smsc: RFC: Workaround for problems with lan8710 phy auto MDI-X

[PATCH] smsc: RFC: Workaround for problems with lan8710 phy auto MDI-X

[Peter Turczak]

Hi all,

while debugging network outages on a customers hardware I found, that the MDI-X function of the lan8710 phy seemed to cause trouble.
When connecting to almost any kind of 100/1000MBit switch, the link would seem to come up and data where sent out to the network. But all incoming packets got lost somehow. This is quite bad, as the system runs from nfsroot while booting up during development.

When I disabled the auto MDI-X function of the phy the problem went away.

Signed-off-by: Peter Turczak <pt@netconsequence.de>
---
 drivers/net/phy/Kconfig |   10 ++++++++++
 drivers/net/phy/smsc.c  |   15 +++++++++++++++
 include/linux/smscphy.h |    5 +++++
 3 files changed, 30 insertions(+), 0 deletions(-)

diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 961f0b2..341f5aa 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -60,6 +60,16 @@ config SMSC_PHY
 	---help---
 	  Currently supports the LAN83C185, LAN8187 and LAN8700 PHYs
 
+config SMSC_PHY_DISABLE_AUTOX
+	bool "Disable MDI-X upon start"
+	depends on SMSC_PHY
+	---help---
+	  When you experience problems estabishing a stable connection
+	  to a network and you have e.g. a LAN8710 ethernet phy
+	  this option might help you out.
+
+	  In doubt, say N
+
 config BROADCOM_PHY
 	tristate "Drivers for Broadcom PHYs"
 	---help---
diff --git a/drivers/net/phy/smsc.c b/drivers/net/phy/smsc.c
index 88e3991..651f71e 100644
--- a/drivers/net/phy/smsc.c
+++ b/drivers/net/phy/smsc.c
@@ -24,6 +24,8 @@
 #include <linux/netdevice.h>
 #include <linux/smscphy.h>
 
+static struct phy_driver lan8710_driver;
+
 static int smsc_phy_config_intr(struct phy_device *phydev)
 {
 	int rc = phy_write (phydev, MII_LAN83C185_IM,
@@ -53,6 +55,19 @@ static int smsc_phy_config_init(struct phy_device *phydev)
 	if (rc < 0)
 		return rc;
 
+#ifdef CONFIG_SMSC_PHY_DISABLE_AUTOX
+	if (phydev->drv == &lan8710_driver) {
+		rc = phy_read(phydev, MII_LAN8710_SCSI);
+		if (rc < 0)
+			return rc;
+		rc = phy_write(phydev, MII_LAN8710_SCSI,
+			rc | MII_LAN8710_SCSI_AMDIXCTRL);
+
+		if (rc < 0)
+			return rc;
+	}
+#endif
+
 	return smsc_phy_ack_interrupt (phydev);
 }
 
diff --git a/include/linux/smscphy.h b/include/linux/smscphy.h
index ce718cb..4084b64 100644
--- a/include/linux/smscphy.h
+++ b/include/linux/smscphy.h
@@ -22,4 +22,9 @@
 #define MII_LAN83C185_EDPWRDOWN (1 << 13) /* EDPWRDOWN */
 #define MII_LAN83C185_ENERGYON  (1 << 1)  /* ENERGYON */
 
+#define MII_LAN8710_SCSI  27 /* Special Control/Status register */
+
+#define MII_LAN8710_SCSI_AMDIXCTRL (1<<15) /* Flag to disable Auto-MDIX */
+
+
 #endif /* __LINUX_SMSCPHY_H__ */
-- 
1.7.0.4

Re: [PATCH] smsc: RFC: Workaround for problems with lan8710 phy auto MDI-X

[Jiri Kosina]

On Fri, 30 Nov 2012, Peter Turczak wrote:

> while debugging network outages on a customers hardware I found, that 
> the MDI-X function of the lan8710 phy seemed to cause trouble. When 
> connecting to almost any kind of 100/1000MBit switch, the link would 
> seem to come up and data where sent out to the network. But all incoming 
> packets got lost somehow. This is quite bad, as the system runs from 
> nfsroot while booting up during development.
> 
> When I disabled the auto MDI-X function of the phy the problem went away.
> 
> Signed-off-by: Peter Turczak <pt@netconsequence.de>
> ---
>  drivers/net/phy/Kconfig |   10 ++++++++++
>  drivers/net/phy/smsc.c  |   15 +++++++++++++++
>  include/linux/smscphy.h |    5 +++++
>  3 files changed, 30 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
> index 961f0b2..341f5aa 100644
> --- a/drivers/net/phy/Kconfig
> +++ b/drivers/net/phy/Kconfig
> @@ -60,6 +60,16 @@ config SMSC_PHY
>  	---help---
>  	  Currently supports the LAN83C185, LAN8187 and LAN8700 PHYs
>  
> +config SMSC_PHY_DISABLE_AUTOX
> +	bool "Disable MDI-X upon start"
> +	depends on SMSC_PHY
> +	---help---
> +	  When you experience problems estabishing a stable connection
> +	  to a network and you have e.g. a LAN8710 ethernet phy
> +	  this option might help you out.
> +
> +	  In doubt, say N
> +

I am not sure whether compile-time option for something like this is 
appropriate. Kernel module parameter, perhaps?

Of course it'd be far better if faulty hardware can be autodetected in 
runtime.

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH] smsc: RFC: Workaround for problems with lan8710 phy auto MDI-X

[David Miller]

From: Jiri Kosina <jkosina@suse.cz>
Date: Fri, 30 Nov 2012 16:23:30 +0100 (CET)

> Of course it'd be far better if faulty hardware can be autodetected in 
> runtime.

That is how this must be handled.

Re: [PATCH] smsc: RFC: Workaround for problems with lan8710 phy auto MDI-X

[Ben Hutchings]

On Fri, 2012-11-30 at 12:23 -0500, David Miller wrote:
> From: Jiri Kosina <jkosina@suse.cz>
> Date: Fri, 30 Nov 2012 16:23:30 +0100 (CET)
> 
> > Of course it'd be far better if faulty hardware can be autodetected in 
> > runtime.
> 
> That is how this must be handled.

Well, we also have the recent extension to ETHTOOL_SSET for forcing
MDI-X on or off.  If it's not possible to detect the faulty hardware
then the driver can implement this.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: [PATCH] smsc: RFC: Workaround for problems with lan8710 phy auto MDI-X

[David Miller]

From: Ben Hutchings <bhutchings@solarflare.com>
Date: Fri, 30 Nov 2012 19:14:40 +0000

> On Fri, 2012-11-30 at 12:23 -0500, David Miller wrote:
>> From: Jiri Kosina <jkosina@suse.cz>
>> Date: Fri, 30 Nov 2012 16:23:30 +0100 (CET)
>> 
>> > Of course it'd be far better if faulty hardware can be autodetected in 
>> > runtime.
>> 
>> That is how this must be handled.
> 
> Well, we also have the recent extension to ETHTOOL_SSET for forcing
> MDI-X on or off.  If it's not possible to detect the faulty hardware
> then the driver can implement this.

Agreed.

Re: [PATCH] smsc: RFC: Workaround for problems with lan8710 phy auto MDI-X

[Peter Turczak]

On Nov 30, 2012, at 4:23 PM, Jiri Kosina <jkosina@suse.cz> wrote:

> I am not sure whether compile-time option for something like this is 
> appropriate. Kernel module parameter, perhaps?
> 
> Of course it'd be far better if faulty hardware can be autodetected in 
> runtime.
Thanks for the input. Currently only the symbol error counter seems to give 
a good indication that there might be a problem. So I suggest monitoring
the symbol error counter. When a certain amount of symbol errors per
poll interval is exceeded the auto MDI-X will be disabled.

Signed-off-by: Peter Turczak <pt@netconsequence.de>
---
 drivers/net/phy/smsc.c  |   57 ++++++++++++++++++++++++++++++++++++++++++++++-
 include/linux/smscphy.h |    7 +++++
 2 files changed, 64 insertions(+), 1 deletions(-)

diff --git a/drivers/net/phy/smsc.c b/drivers/net/phy/smsc.c
index 88e3991..0748266 100644
--- a/drivers/net/phy/smsc.c
+++ b/drivers/net/phy/smsc.c
@@ -24,6 +24,16 @@
 #include <linux/netdevice.h>
 #include <linux/smscphy.h>
 
+/* Maximum number of symbol errors between two lan8720_read_status calls. */
+#define LAN8720_MAX_SYM_ERR_CNT 100
+
+static struct phy_driver lan8710_driver;
+
+struct smsc_phy_private {
+	/* Keeps track of the number of the broken received packets */
+	int sym_err_count;
+};
+
 static int smsc_phy_config_intr(struct phy_device *phydev)
 {
 	int rc = phy_write (phydev, MII_LAN83C185_IM,
@@ -58,6 +68,7 @@ static int smsc_phy_config_init(struct phy_device *phydev)
 
 static int lan87xx_config_init(struct phy_device *phydev)
 {
+	struct smsc_phy_private *privdata;
 	/*
 	 * Make sure the EDPWRDOWN bit is NOT set. Setting this bit on
 	 * LAN8710/LAN8720 PHY causes the PHY to misbehave, likely due
@@ -79,6 +90,12 @@ static int lan87xx_config_init(struct phy_device *phydev)
 	if (rc < 0)
 		return rc;
 
+	privdata = kzalloc(sizeof(*privdata), GFP_KERNEL);
+	if (!privdata)
+		return -ENOMEM;
+
+	phydev->priv = privdata;
+
 	return smsc_phy_ack_interrupt(phydev);
 }
 
@@ -87,6 +104,44 @@ static int lan911x_config_init(struct phy_device *phydev)
 	return smsc_phy_ack_interrupt(phydev);
 }
 
+int lan8720_read_status(struct phy_device *phydev)
+{
+	int err_count, err_since_last, rc;
+	struct smsc_phy_private *priv = phydev->priv;
+
+	if (priv != NULL) {
+
+		err_count = phy_read(phydev, MII_LAN8710_SYM_ERR_CNT);
+		err_since_last = err_count - priv->sym_err_count;
+
+		if (err_since_last < 0)
+				err_since_last += 65535;
+
+		priv->sym_err_count = err_count;
+		if (err_since_last > LAN8720_MAX_SYM_ERR_CNT) {
+			rc = phy_read(phydev, MII_LAN8710_SCSI);
+
+			if (rc < 0)
+				return rc;
+
+			if (!(rc & MII_LAN8710_SCSI_AMDIXCTRL)) {
+
+				pr_warn("%s: Too may RX errors.",
+					phydev->bus->name);
+				pr_warn("Disabling MDI-X\n");
+
+				rc = phy_write(phydev, MII_LAN8710_SCSI,
+					rc | MII_LAN8710_SCSI_AMDIXCTRL);
+				}
+
+			if (rc < 0)
+				return rc;
+			}
+	}
+
+	return genphy_read_status(phydev);
+}
+
 static struct phy_driver smsc_phy_driver[] = {
 {
 	.phy_id		= 0x0007c0a0, /* OUI=0x00800f, Model#=0x0a */
@@ -187,7 +242,7 @@ static struct phy_driver smsc_phy_driver[] = {
 
 	/* basic functions */
 	.config_aneg	= genphy_config_aneg,
-	.read_status	= genphy_read_status,
+	.read_status	= lan8720_read_status,
 	.config_init	= lan87xx_config_init,
 
 	/* IRQ related */
diff --git a/include/linux/smscphy.h b/include/linux/smscphy.h
index ce718cb..a0d3893 100644
--- a/include/linux/smscphy.h
+++ b/include/linux/smscphy.h
@@ -22,4 +22,11 @@
 #define MII_LAN83C185_EDPWRDOWN (1 << 13) /* EDPWRDOWN */
 #define MII_LAN83C185_ENERGYON  (1 << 1)  /* ENERGYON */
 
+#define MII_LAN8710_SCSI  27 /* Special Control/Status register */
+
+#define MII_LAN8710_SCSI_AMDIXCTRL (1<<15) /* Flag to disable Auto-MDIX */
+
+#define MII_LAN8710_SYM_ERR_CNT	26 /* Amount of invalid code symbols received */
+
+
 #endif /* __LINUX_SMSCPHY_H__ */
-- 
1.7.0.4

Re: [PATCHv2] smsc: RFC: Workaround for problems with lan8710 phy auto MDI-X

[Peter Turczak]

On Nov 30, 2012, at 4:23 PM, Jiri Kosina <jkosina@suse.cz> wrote:

> I am not sure whether compile-time option for something like this is 
> appropriate. Kernel module parameter, perhaps?
> 
> Of course it'd be far better if faulty hardware can be autodetected in 
> runtime.
Thanks for the input. Currently only the symbol error counter seems to give 
a good indication that there might be a problem. So I suggest monitoring
the symbol error counter. When a certain amount of symbol errors per
poll interval is exceeded the auto MDI-X will be disabled.

BTW: is it okay to hook into read_status or should I use work queues?

Signed-off-by: Peter Turczak <pt@netconsequence.de>
---
drivers/net/phy/smsc.c  |   57 ++++++++++++++++++++++++++++++++++++++++++++++-
include/linux/smscphy.h |    7 +++++
2 files changed, 64 insertions(+), 1 deletions(-)

diff --git a/drivers/net/phy/smsc.c b/drivers/net/phy/smsc.c
index 88e3991..0748266 100644
--- a/drivers/net/phy/smsc.c
+++ b/drivers/net/phy/smsc.c
@@ -24,6 +24,16 @@
#include <linux/netdevice.h>
#include <linux/smscphy.h>

+/* Maximum number of symbol errors between two lan8720_read_status calls. */
+#define LAN8720_MAX_SYM_ERR_CNT 100
+
+static struct phy_driver lan8710_driver;
+
+struct smsc_phy_private {
+	/* Keeps track of the number of the broken received packets */
+	int sym_err_count;
+};
+
static int smsc_phy_config_intr(struct phy_device *phydev)
{
	int rc = phy_write (phydev, MII_LAN83C185_IM,
@@ -58,6 +68,7 @@ static int smsc_phy_config_init(struct phy_device *phydev)

static int lan87xx_config_init(struct phy_device *phydev)
{
+	struct smsc_phy_private *privdata;
	/*
	 * Make sure the EDPWRDOWN bit is NOT set. Setting this bit on
	 * LAN8710/LAN8720 PHY causes the PHY to misbehave, likely due
@@ -79,6 +90,12 @@ static int lan87xx_config_init(struct phy_device *phydev)
	if (rc < 0)
		return rc;

+	privdata = kzalloc(sizeof(*privdata), GFP_KERNEL);
+	if (!privdata)
+		return -ENOMEM;
+
+	phydev->priv = privdata;
+
	return smsc_phy_ack_interrupt(phydev);
}

@@ -87,6 +104,44 @@ static int lan911x_config_init(struct phy_device *phydev)
	return smsc_phy_ack_interrupt(phydev);
}

+int lan8720_read_status(struct phy_device *phydev)
+{
+	int err_count, err_since_last, rc;
+	struct smsc_phy_private *priv = phydev->priv;
+
+	if (priv != NULL) {
+
+		err_count = phy_read(phydev, MII_LAN8710_SYM_ERR_CNT);
+		err_since_last = err_count - priv->sym_err_count;
+
+		if (err_since_last < 0)
+				err_since_last += 65535;
+
+		priv->sym_err_count = err_count;
+		if (err_since_last > LAN8720_MAX_SYM_ERR_CNT) {
+			rc = phy_read(phydev, MII_LAN8710_SCSI);
+
+			if (rc < 0)
+				return rc;
+
+			if (!(rc & MII_LAN8710_SCSI_AMDIXCTRL)) {
+
+				pr_warn("%s: Too may RX errors.",
+					phydev->bus->name);
+				pr_warn("Disabling MDI-X\n");
+
+				rc = phy_write(phydev, MII_LAN8710_SCSI,
+					rc | MII_LAN8710_SCSI_AMDIXCTRL);
+				}
+
+			if (rc < 0)
+				return rc;
+			}
+	}
+
+	return genphy_read_status(phydev);
+}
+
static struct phy_driver smsc_phy_driver[] = {
{
	.phy_id		= 0x0007c0a0, /* OUI=0x00800f, Model#=0x0a */
@@ -187,7 +242,7 @@ static struct phy_driver smsc_phy_driver[] = {

	/* basic functions */
	.config_aneg	= genphy_config_aneg,
-	.read_status	= genphy_read_status,
+	.read_status	= lan8720_read_status,
	.config_init	= lan87xx_config_init,

	/* IRQ related */
diff --git a/include/linux/smscphy.h b/include/linux/smscphy.h
index ce718cb..a0d3893 100644
--- a/include/linux/smscphy.h
+++ b/include/linux/smscphy.h
@@ -22,4 +22,11 @@
#define MII_LAN83C185_EDPWRDOWN (1 << 13) /* EDPWRDOWN */
#define MII_LAN83C185_ENERGYON  (1 << 1)  /* ENERGYON */

+#define MII_LAN8710_SCSI  27 /* Special Control/Status register */
+
+#define MII_LAN8710_SCSI_AMDIXCTRL (1<<15) /* Flag to disable Auto-MDIX */
+
+#define MII_LAN8710_SYM_ERR_CNT	26 /* Amount of invalid code symbols received */
+
+
#endif /* __LINUX_SMSCPHY_H__ */
-- 
1.7.0.4