Patch for link detection for R8169

Patch for link detection for R8169

[Miroslaw Mieszczak]

[-- Attachment #1: Type: text/plain, Size: 179 bytes --]

There is a patch to driver of RLT8169 network card. This match make 
possible detection of the link status even if network interface is down.
This is usefull for laptop users.




[-- Attachment #2: r8169.diff --]
[-- Type: text/plain, Size: 1442 bytes --]

--- r8169.c	2005-09-02 15:34:52.000000000 +0200
+++ linux/drivers/net/r8169.c	2005-09-05 21:11:15.000000000 +0200
@@ -538,14 +538,27 @@
 
 static unsigned int rtl8169_tbi_link_ok(void __iomem *ioaddr)
 {
-	return RTL_R32(TBICSR) & TBILinkOk;
+	return (RTL_R32(TBICSR) & TBILinkOk) == TBILinkOk ? 1:0;
 }
 
 static unsigned int rtl8169_xmii_link_ok(void __iomem *ioaddr)
 {
-	return RTL_R8(PHYstatus) & LinkStatus;
+	return (RTL_R8(PHYstatus) & LinkStatus) == LinkStatus ? 1:0;
 }
 
+static u32 rtl8169_get_link(struct net_device *dev)
+{
+	struct rtl8169_private *np = netdev_priv(dev);
+	unsigned int result;
+	unsigned long flags;
+  
+	spin_lock_irqsave(&np->lock, flags);
+	result = np->link_ok(np->mmio_addr);
+	spin_unlock_irqrestore(&np->lock, flags);
+	return result;
+}
+
+
 static void rtl8169_tbi_reset_enable(void __iomem *ioaddr)
 {
 	RTL_W32(TBICSR, RTL_R32(TBICSR) | TBIReset);
@@ -577,6 +590,8 @@
 	spin_unlock_irqrestore(&tp->lock, flags);
 }
 
+
+
 static void rtl8169_link_option(int idx, u8 *autoneg, u16 *speed, u8 *duplex)
 {
 	struct {
@@ -1010,7 +1025,7 @@
 static struct ethtool_ops rtl8169_ethtool_ops = {
 	.get_drvinfo		= rtl8169_get_drvinfo,
 	.get_regs_len		= rtl8169_get_regs_len,
-	.get_link		= ethtool_op_get_link,
+	.get_link		= rtl8169_get_link,/*              ethtool_op_get_link,*/
 	.get_settings		= rtl8169_get_settings,
 	.set_settings		= rtl8169_set_settings,
 	.get_msglevel		= rtl8169_get_msglevel,

Re: Patch for link detection for R8169

[Matthieu CASTET]

Le Tue, 06 Sep 2005 16:32:39 +0200, Miroslaw Mieszczak a écrit :

> There is a patch to driver of RLT8169 network card. This match make 
> possible detection of the link status even if network interface is down.
> This is usefull for laptop users.
> 
> 
> 
> --- r8169.c	2005-09-02 15:34:52.000000000 +0200
> +++ linux/drivers/net/r8169.c	2005-09-05 21:11:15.000000000 +0200
> @@ -538,14 +538,27 @@
>  
>  static unsigned int rtl8169_tbi_link_ok(void __iomem *ioaddr)
>  {
> -	return RTL_R32(TBICSR) & TBILinkOk;
> +	return (RTL_R32(TBICSR) & TBILinkOk) == TBILinkOk ? 1:0;
>  }
>  
>  static unsigned int rtl8169_xmii_link_ok(void __iomem *ioaddr)
>  {
> -	return RTL_R8(PHYstatus) & LinkStatus;
> +	return (RTL_R8(PHYstatus) & LinkStatus) == LinkStatus ? 1:0;
>  }
>  
(a==b)?1:0 is stupid just use (a==b) ...
And in this case I am sure we care only if it is null or non-null,
so there no need to change that...

Re: Patch for link detection for R8169

[Francois Romieu]

Miroslaw Mieszczak <mieszcz@zabrze.zigzag.pl> :
> There is a patch to driver of RLT8169 network card. This match make 
> possible detection of the link status even if network interface is down.
> This is usefull for laptop users.

(side note: there is maintainer entry for the r8169 and network related
patches are welcome on netdev@vger.kernel.org)

Can you elaborate why it is usefull for laptop users ?

I am sceptical: tg3/bn2x/skge do not seem to allow it either.

Jeff, is it a requirement ?

--
Ueimor

Re: Patch for link detection for R8169

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 1141 bytes --]

On Tue, 06 Sep 2005 21:46:02 +0200, Francois Romieu said:
> Miroslaw Mieszczak <mieszcz@zabrze.zigzag.pl> :
> > There is a patch to driver of RLT8169 network card. This match make 
> > possible detection of the link status even if network interface is down.
> > This is usefull for laptop users.
> 
> (side note: there is maintainer entry for the r8169 and network related
> patches are welcome on netdev@vger.kernel.org)
> 
> Can you elaborate why it is usefull for laptop users ?

Desktops and rack mounts you tend to leave that RJ45 plugged into the back
all the time.  As a result, "no link" is a rare error condition.

On the other hand, laptops are often sitting around with no RJ45 in sight.
Being able to detect "card present but no cable plugged in" can be useful in
startup scripts and the like, so you can do something like

if [ link-is-up eth3 ];
then
	ifup eth3;
fi

and avoid throwing nasty error messages (and even worse, timeouts) trying to
bring up the card if there's nothing plugged in.  There's no sense in trying to
do the whole DHCP thing (or whatever you need to do) if you can tell beforehand
that it will fail....


[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]

Re: Patch for link detection for R8169

[Francois Romieu]

Valdis.Kletnieks@vt.edu <Valdis.Kletnieks@vt.edu> :
[...]

Ok, thanks for the hint.

Currently one can do 'ifconfig ethX up', check the link status, then try
to DHCP or whatever. Apparently a few drivers do not support tne detection
of link as presented above. So is it anything like a vendor requirement/a
standard (or should it be the new right way (TM)) or does the userspace
needs fixing wrt its expectation ?

The lack of irq means that netif_carrier_on/off can not be reliable until
the device is up. It is a bit worrying.

--
Ueimor

Re: Patch for link detection for R8169

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 943 bytes --]

On Tue, 06 Sep 2005 22:42:21 +0200, Francois Romieu said:

> Currently one can do 'ifconfig ethX up', check the link status, then try
> to DHCP or whatever. Apparently a few drivers do not support tne detection
> of link as presented above. So is it anything like a vendor requirement/a
> standard (or should it be the new right way (TM)) or does the userspace
> needs fixing wrt its expectation ?

The "ifconfig up then check link status" method is probably usable for the
vast majority of cases.  Are there any driver/card combos that *can't* be fixed
to support that?  (A somewhat hidden side effect is that if you're doing this,
the driver also needs to be able to support additional ifconfig calls later
to set encapsulation, address/netmask, and the like.  I've run across dain-bramaged
older hardware/software (not Linux-based) that would require an 'ifconfig down'
followed by 'ifconfig foo <value> bar <value> up' to change stuff....

[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]

Re: Patch for link detection for R8169

[Francois Romieu]

Valdis.Kletnieks@vt.edu <Valdis.Kletnieks@vt.edu> :
> On Tue, 06 Sep 2005 22:42:21 +0200, Francois Romieu said:
> 
> > Currently one can do 'ifconfig ethX up', check the link status, then try
> > to DHCP or whatever. Apparently a few drivers do not support tne detection
> > of link as presented above. So is it anything like a vendor requirement/a
> > standard (or should it be the new right way (TM)) or does the userspace
> > needs fixing wrt its expectation ?
> 
> The "ifconfig up then check link status" method is probably usable for the
> vast majority of cases.  Are there any driver/card combos that *can't* be
> fixed to support that?  (A somewhat hidden side effect is that if you're

I meant "as presented above" == "check the link while the network device
is down".

--
Ueimor

Re: Patch for link detection for R8169

[Miroslaw Mieszczak]

Francois Romieu napisał(a):

>Valdis.Kletnieks@vt.edu <Valdis.Kletnieks@vt.edu> :
>[...]
>
>Ok, thanks for the hint.
>
>Currently one can do 'ifconfig ethX up', check the link status, then try
>to DHCP or whatever. Apparently a few drivers do not support tne detection
>of link as presented above. So is it anything like a vendor requirement/a
>standard (or should it be the new right way (TM)) or does the userspace
>needs fixing wrt its expectation ?
>  
>
The main problem with this driver is, that if I do like this, then every 
10 seconds I receive new message from the network card in kernel log.
There is following message:

Sep  4 16:31:43 laptop_mirka eth0: PHY reset until link up
Sep  4 16:31:53 laptop_mirka eth0: PHY reset until link up

Do you think, that this is correct way t do the things? I
In my opinion, the solution, that link status can be checked (if 
hardware allow this) when interface is down is more usefull.

Re: Patch for link detection for R8169

[Francois Romieu]

Miroslaw Mieszczak <mieszcz@zabrze.zigzag.pl> :
[...]
> The main problem with this driver is, that if I do like this, then every 
> 10 seconds I receive new message from the network card in kernel log.
> There is following message:
> 
> Sep  4 16:31:43 laptop_mirka eth0: PHY reset until link up
> Sep  4 16:31:53 laptop_mirka eth0: PHY reset until link up

You can silence this message in 2.6.13 by using the 'msglvl'
option of the ethtool command.

> Do you think, that this is correct way t do the things?

You would not believe how arrogant I can be :o)

More seriously, does there remain any issue for you wrt the
usability of the r8169 driver ?

--
Ueimor

Re: Patch for link detection for R8169

[Francois Romieu]

Miroslaw Mieszczak <mieszcz@zabrze.zigzag.pl> :
> Francois Romieu napisał(a):
> 
> >You can silence this message in 2.6.13 by using the 'msglvl'
> >option of the ethtool command.
>
> It would be disabled only this message, or all warning messages from the 
> net driver?

One issues 'ethtool msglvl xyz' where xyz is the bitwise OR of the messages
which should be kept. The r8169 driver allows the same mask via the "debug"
option of the module.

The meaning of the bitflags is driver dependent. A summary of the messages
for the r8169 driver is available at:
http://www.zoreil.com/~romieu/r8169/doc/msglvl.txt

--
Ueimor

Re: Patch for link detection for R8169

[Jeff Garzik]

Francois Romieu wrote:
> Miroslaw Mieszczak <mieszcz@zabrze.zigzag.pl> :
> 
>>There is a patch to driver of RLT8169 network card. This match make 
>>possible detection of the link status even if network interface is down.
>>This is usefull for laptop users.
> 
> 
> (side note: there is maintainer entry for the r8169 and network related
> patches are welcome on netdev@vger.kernel.org)
> 
> Can you elaborate why it is usefull for laptop users ?
> 
> I am sceptical: tg3/bn2x/skge do not seem to allow it either.
> 
> Jeff, is it a requirement ?

Generally most drivers power down hardware, MAC at least, when the 
interface is down.  As such, many drivers do not (cannot), as written, 
report any useful link information.

IF the phy is not powered down, when the interface goes down, and IF 
hardware permits, it would certainly be nice to report link state when 
interface is down.  This is a hardware-dependent, driver-dependent choice.

	Jeff