r8169 - it's dead jim

r8169 - it's dead jim

[James Feeney]

Hey

Going from 3.12.0-1-ARCH to 3.12.1-1-ARCH, the r8169 module is having the "link
detect" problem again.

Initially, "ethtool -s <blah> autoneg off" gives "invalid argument".

"sudo mii-tool -R enp22s0" - Reset the MII to its default configuration - will
allow a 10Mb/s half duplex connection, with "autoneg off".

"ethtool -s <blah> autoneg off" then will work, after the "mii-tool -R <blah>".

After some time, Auto-negotiation will spontaneously go on, and a 100Mb/s full
duplex connection will come up, on a 1Gb/s interface.  Subsequent unplugging and
plugging the cable gives a 1Gb/s full duplex connection.

This problem has been recurring for so many years, I hope the solution is
something obvious, perhaps a recent change to the kernel?


Thanks
James

Re: r8169 - it's dead jim

[Holger Hoffstaette]

On Sat, 23 Nov 2013 12:46:00 -0700, James Feeney wrote:

> Going from 3.12.0-1-ARCH to 3.12.1-1-ARCH, the r8169 module is having the
> "link detect" problem again.

3.12.1 didn't contain any r8169 changes, and my cards in 2 different
machines work just fine. Maybe a micro-fractured connector or cable kink?

-h

Re: r8169 - it's dead jim

[Francois Romieu]

James Feeney <james@nurealm.net> :
[...]
> Going from 3.12.0-1-ARCH to 3.12.1-1-ARCH, the r8169 module is having the
> "link detect" problem again.

$ grep james@nurealm.net ~/Mail/linux/netdev/201[123]?? | grep -v 201311

Which one exactly ?

> Initially, "ethtool -s <blah> autoneg off" gives "invalid argument".

blah ?

[...]
> After some time, Auto-negotiation will spontaneously go on, and a 100Mb/s full
> duplex connection will come up, on a 1Gb/s interface.  Subsequent unplugging and
> plugging the cable gives a 1Gb/s full duplex connection.
> 
> This problem has been recurring for so many years, I hope the solution is
> something obvious, perhaps a recent change to the kernel?

The r8169 driver is the same in v3.12 and v3.12.1. Either some userspace
application works in your back or your device experienced a runtime
suspend / resume cycle where it previously didn't.

You can try the genuinely untested hack below if you don't like current
runtime suspend / resume behavior.

What do you want exactly ? 10 Mb/s, 100 Mb/s ? Limited / no advertising ?

diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index 7280d5d..701ca00 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -812,6 +812,13 @@ struct rtl8169_private {
 #define RTL_FIRMWARE_UNKNOWN	ERR_PTR(-EAGAIN)
 
 	u32 ocp_base;
+
+	struct {
+		u32 adv;
+		u16 speed;
+		u8 duplex;
+		u8 autoneg;
+	} rtl_settings;
 };
 
 MODULE_AUTHOR("Realtek and the Linux r8169 crew <netdev@vger.kernel.org>");
@@ -1755,6 +1762,11 @@ static int rtl8169_set_speed(struct net_device *dev,
 	if (ret < 0)
 		goto out;
 
+	tp->autoneg	= autoneg;
+	tp->speed	= speed;
+	tp->duplex	= duplex;
+	tp->adv		= advertising;
+
 	if (netif_running(dev) && (autoneg == AUTONEG_ENABLE) &&
 	    (advertising & ADVERTISED_1000baseT_Full)) {
 		mod_timer(&tp->timer, jiffies + RTL8169_PHY_TIMEOUT);
@@ -3778,6 +3790,14 @@ static void rtl8169_init_phy(struct net_device *dev, struct rtl8169_private *tp)
 		netif_info(tp, link, dev, "TBI auto-negotiating\n");
 }
 
+static void rtl_resume_phy(struct net_device *dev, struct rtl8169_private *tp)
+{
+	rtl8169_init_phy(dev, tp);
+
+	if (tp->speed)
+		tp->set_speed(dev, tp->autoneg, tp->speed, tp->duplex, tp->adv);
+}
+
 static void rtl_rar_set(struct rtl8169_private *tp, u8 *addr)
 {
 	void __iomem *ioaddr = tp->mmio_addr;
@@ -6661,7 +6681,7 @@ static int rtl8169_resume(struct device *device)
 	struct net_device *dev = pci_get_drvdata(pdev);
 	struct rtl8169_private *tp = netdev_priv(dev);
 
-	rtl8169_init_phy(dev, tp);
+	rtl_resume_phy(dev, tp);
 
 	if (netif_running(dev))
 		__rtl8169_resume(dev);
@@ -6702,7 +6722,7 @@ static int rtl8169_runtime_resume(struct device *device)
 	tp->saved_wolopts = 0;
 	rtl_unlock_work(tp);
 
-	rtl8169_init_phy(dev, tp);
+	rtl_resume_phy(dev, tp);
 
 	__rtl8169_resume(dev);
 

Re: r8169 - it's dead jim

[Francois Romieu]

Francois Romieu <romieu@fr.zoreil.com> :
[...]
> You can try the genuinely untested hack below if you don't like current
> runtime suspend / resume behavior.

The patch is badly broken.

-- 
Ueimor

Re: r8169 - it's dead jim

[James Feeney]

Hi Francois

> Which one exactly ?

Sorry - just based upon a Google search, this problem with the card not
recognizing that a cable has been plugged into the interface, seems to have come
up off and on for a while, many years.

> blah ?

"<blah>" - referring to the network interface name, in my case "enp22s0".  This
interface is a Netgear "Gigabit PC Card", model GA511, into a Thinkpad T60,
acting as a router.

> The r8169 driver is the same in v3.12 and v3.12.1. Either some userspace
> application works in your back or your device experienced a runtime
> suspend / resume cycle where it previously didn't.

That would seem to make sense.  After some more problems with the interface, I
later noticed that having the card come-up at 1000Mb/s and Full duplex was not
enough to indicate that the card was working.  When the interface was just
handling ping packets it seemed to work fine.  But then, whenever there was a
file transfer - downloading some email or a large file, for instance - the
interface seems to "choke".  Watching a repeated "ethtool enp22s0", the link
speed and duplex would change constantly, dropping down to 10Mb/s and Half
duplex, then up to 100Mb/s and Full duplex, than back down.  If the file
transfers were halted, so that nothing more than ping was going through the
interface, the speed and duplex would come back up, to 1000Mb/s and Full duplex.
 Practically, the interface was unusable, and I swapped-out the card for an
Intel Pro 100, which works fine.   So the problem would then seem to have
nothing to do with the PC Card hardware on the Thinkpad.

> What do you want exactly ? 10 Mb/s, 100 Mb/s ? Limited / no advertising ?

Well, according to "ethtool", the card advertises up to 1000Mb/s, Full duplex,
and Auto-negotiation.  It just seems that it should not constantly try to
re-configure itself.  I have not studied the code to find-out what would trigger
a re-negotiation, or trigger a suspend/resume.

> The patch is badly broken.

Sorry - I haven't tried to re-compile the driver, since it was easier to just
swap-out the card, for another with a different driver.

Maybe there is a simple way to modify the driver to "lock" the configuration?
Or ... ?

If you have another patch, I can plug the card back in and try it out.


Thanks

James

Re: r8169 - it's dead jim

[Francois Romieu]

James Feeney <james@nurealm.net> :
[...]
> > Which one exactly ?
> 
> Sorry - just based upon a Google search, this problem with the card not
> recognizing that a cable has been plugged into the interface, seems to
> have come up off and on for a while, many years.

Kind of. The r8169 driver has been evolving for more than 10 years with
quite some features / chipsets / contributors / bugs.

[...]
> That would seem to make sense.  After some more problems with the interface, I
> later noticed that having the card come-up at 1000Mb/s and Full duplex was not
> enough to indicate that the card was working.  When the interface was just
> handling ping packets it seemed to work fine.  But then, whenever there was a
> file transfer - downloading some email or a large file, for instance - the
> interface seems to "choke".  Watching a repeated "ethtool enp22s0", the link
> speed and duplex would change constantly, dropping down to 10Mb/s and Half
> duplex, then up to 100Mb/s and Full duplex, than back down.  If the file

You should increase the verbosity level through 'ethtool msglvl'. No need
to be clever, start with a gross 65535.

I'll then welcome the dmesg (ethtool -S may want to tell something too).

> transfers were halted, so that nothing more than ping was going through the
> interface, the speed and duplex would come back up, to 1000Mb/s and Full
> duplex.

Stating the obvious: your switch supports 1000Mb/s, right ?

>  Practically, the interface was unusable, and I swapped-out the card for an
> Intel Pro 100, which works fine.   So the problem would then seem to have
> nothing to do with the PC Card hardware on the Thinkpad.

Ok.

> > What do you want exactly ? 10 Mb/s, 100 Mb/s ? Limited / no advertising ?
> 
> Well, according to "ethtool", the card advertises up to 1000Mb/s, Full duplex,
> and Auto-negotiation.  It just seems that it should not constantly try to
> re-configure itself.  I have not studied the code to find-out what would
> trigger a re-negotiation, or trigger a suspend/resume.

Runtime suspend will kick in automatically if there is no activity and
it has been enabled. You can check the later through the
/sys/devices/.../power/control file (on = disabled, auto = enabled, see
Documentation/power/runtime_pm.txt).

[...]
> Maybe there is a simple way to modify the driver to "lock" the configuration?
> Or ... ?
> 
> If you have another patch, I can plug the card back in and try it out.

The patch below should restore whatever link settings you asked for if
runtime suspend / resume kicks in.

I would suggest you avoid disabling autoneg. You can reduce the set of
advertised speed though.

Subject: [PATCH] r8169: restore user link settings after resume.

Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>
---
 drivers/net/ethernet/realtek/r8169.c | 27 +++++++++++++++++++++++++--
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index b7a01c8..ad67481 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -812,6 +812,13 @@ struct rtl8169_private {
 #define RTL_FIRMWARE_UNKNOWN	ERR_PTR(-EAGAIN)
 
 	u32 ocp_base;
+
+	struct rtl_link_setting {
+		u32 adv;
+		u16 speed;
+		u8 duplex;
+		u8 autoneg;
+	} link_sets;
 };
 
 MODULE_AUTHOR("Realtek and the Linux r8169 crew <netdev@vger.kernel.org>");
@@ -1749,12 +1756,18 @@ static int rtl8169_set_speed(struct net_device *dev,
 			     u8 autoneg, u16 speed, u8 duplex, u32 advertising)
 {
 	struct rtl8169_private *tp = netdev_priv(dev);
+	struct rtl_link_setting *ls = &tp->link_sets;
 	int ret;
 
 	ret = tp->set_speed(dev, autoneg, speed, duplex, advertising);
 	if (ret < 0)
 		goto out;
 
+	ls->autoneg	= autoneg;
+	ls->speed	= speed;
+	ls->duplex	= duplex;
+	ls->adv		= advertising;
+
 	if (netif_running(dev) && (autoneg == AUTONEG_ENABLE) &&
 	    (advertising & ADVERTISED_1000baseT_Full)) {
 		mod_timer(&tp->timer, jiffies + RTL8169_PHY_TIMEOUT);
@@ -3778,6 +3791,16 @@ static void rtl8169_init_phy(struct net_device *dev, struct rtl8169_private *tp)
 		netif_info(tp, link, dev, "TBI auto-negotiating\n");
 }
 
+static void rtl_resume_phy(struct net_device *dev, struct rtl8169_private *tp)
+{
+	struct rtl_link_setting *ls = &tp->link_sets;
+
+	rtl8169_init_phy(dev, tp);
+
+	if (ls->speed)
+		tp->set_speed(dev, ls->autoneg, ls->speed, ls->duplex, ls->adv);
+}
+
 static void rtl_rar_set(struct rtl8169_private *tp, u8 *addr)
 {
 	void __iomem *ioaddr = tp->mmio_addr;
@@ -6661,7 +6684,7 @@ static int rtl8169_resume(struct device *device)
 	struct net_device *dev = pci_get_drvdata(pdev);
 	struct rtl8169_private *tp = netdev_priv(dev);
 
-	rtl8169_init_phy(dev, tp);
+	rtl_resume_phy(dev, tp);
 
 	if (netif_running(dev))
 		__rtl8169_resume(dev);
@@ -6702,7 +6725,7 @@ static int rtl8169_runtime_resume(struct device *device)
 	tp->saved_wolopts = 0;
 	rtl_unlock_work(tp);
 
-	rtl8169_init_phy(dev, tp);
+	rtl_resume_phy(dev, tp);
 
 	__rtl8169_resume(dev);
 
-- 
1.8.3.1

Re: r8169 - it's dead jim

[David Miller]

From: Francois Romieu <romieu@fr.zoreil.com>
Date: Sat, 30 Nov 2013 21:39:31 +0100

> Subject: [PATCH] r8169: restore user link settings after resume.
> 
> Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>

Francois should I apply this or are you just posting this to get
testing feedback?

Thanks.

Re: r8169 - it's dead jim

[Francois Romieu]

David Miller <davem@davemloft.net> :
[...]
> Francois should I apply this or are you just posting this to get
> testing feedback?

No, it's only debuging help and testing feedback straight from
format-patch.

If I don't send fixes as "[PATCH net x/y] r8169: ..." or features as
"[PATCH net-next 1/1] r8169: perform Tx completion before Rx processing"
during the net-next window in self-contained messages, you should
consider that nothing is expected to be applied and something is probably
going wrong.

-- 
Ueimor