netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 2.6.24] tg3: fix ethtool autonegotiate flags
@ 2007-10-02 20:16 Andy Gospodarek
  2007-10-02 22:02 ` Michael Chan
  0 siblings, 1 reply; 4+ messages in thread
From: Andy Gospodarek @ 2007-10-02 20:16 UTC (permalink / raw)
  To: mchan, davem, jeff, netdev


I recently noticed that when calling:

# ethtool -s eth0 autoneg on

on a 5722 (though I'm sure it's not specific to that card) that
subsequent checks of the cards status looked like this:

# ethtool eth0
Settings for eth0:
        Supported ports: [ MII ]
        Supported link modes:   10baseT/Half 10baseT/Full
                                100baseT/Half 100baseT/Full
                                1000baseT/Half 1000baseT/Full
        Supports auto-negotiation: Yes
        Advertised link modes:  10baseT/Half 10baseT/Full
                                100baseT/Half 100baseT/Full
                                1000baseT/Half 1000baseT/Full
        Advertised auto-negotiation: No        <---- This seems odd?!?
        Speed: 1000Mb/s
        Duplex: Full
        Port: Twisted Pair
        PHYAD: 1
        Transceiver: internal
        Auto-negotiation: on
        Supports Wake-on: g
        Wake-on: d
        Current message level: 0x000000ff (255)
        Link detected: yes

I noticed that the following commit:

commit 3600d918d870456ea8e7bb9d47f327de5c20f3d6
Author: Michael Chan <mchan@broadcom.com>
Date:   Thu Dec 7 00:21:48 2006 -0800

    [TG3]: Allow partial speed advertisement.

    Honor the advertisement bitmask from ethtool.  We used to always
    advertise the full capability when autoneg was set to on.

changed things around so that ethtool speed settings were strictly
followed.  Unfortunately ethtool doesn't seem to set ADVERTISED_Autoneg
in the advertising field (and maybe it shouldn't have to).  I'd vote
that it should be fixed there, but it should also be added here just in
case someone using ethtool ioctls in their own application gets what
they want.

Adding that flag in tg3_set_settings seemed like the most logical place
since the driver works fine on boot.  This is just an issue when
re-enabling autonegotiation, so we should probably nip it there.

Signed-off-by: Andy Gospodarek <andy@greyhouse.net>
---

 tg3.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
index d4ac6e9..0a414be 100644
--- a/drivers/net/tg3.c
+++ b/drivers/net/tg3.c
@@ -8070,7 +8070,8 @@ static int tg3_set_settings(struct net_device *dev, struct ethtool_cmd *cmd)
 
 	tp->link_config.autoneg = cmd->autoneg;
 	if (cmd->autoneg == AUTONEG_ENABLE) {
-		tp->link_config.advertising = cmd->advertising;
+		tp->link_config.advertising = (cmd->advertising |
+					      ADVERTISED_Autoneg);
 		tp->link_config.speed = SPEED_INVALID;
 		tp->link_config.duplex = DUPLEX_INVALID;
 	} else {

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH 2.6.24] tg3: fix ethtool autonegotiate flags
  2007-10-02 22:02 ` Michael Chan
@ 2007-10-02 21:16   ` Andy Gospodarek
  2007-10-08  8:09   ` David Miller
  1 sibling, 0 replies; 4+ messages in thread
From: Andy Gospodarek @ 2007-10-02 21:16 UTC (permalink / raw)
  To: Michael Chan; +Cc: David Miller, jeff, netdev

On Tue, Oct 02, 2007 at 03:02:56PM -0700, Michael Chan wrote:
> On Tue, 2007-10-02 at 16:16 -0400, Andy Gospodarek wrote:
> > Adding that flag in tg3_set_settings seemed like the most logical
> > place
> > since the driver works fine on boot.  This is just an issue when
> > re-enabling autonegotiation, so we should probably nip it there.
> > 
> > Signed-off-by: Andy Gospodarek <andy@greyhouse.net>
> 
> We also noticed this issue recently, but didn't pay too much attention
> to it since it was more of a "cosmetic" issue.  The driver behaves the
> same since we rely on cmd->autoneg to decide whether to enable autoneg
> or not.  Your fix seems reasonable to me.  Thanks.
> 
> Acked-by: Michael Chan <mchan@broadcom.com>
> 

I completely agree that it's cosmetic, it just seems like something
decent to toss in there since it's the kind of thing others will start
complaining about. 



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 2.6.24] tg3: fix ethtool autonegotiate flags
  2007-10-02 20:16 [PATCH 2.6.24] tg3: fix ethtool autonegotiate flags Andy Gospodarek
@ 2007-10-02 22:02 ` Michael Chan
  2007-10-02 21:16   ` Andy Gospodarek
  2007-10-08  8:09   ` David Miller
  0 siblings, 2 replies; 4+ messages in thread
From: Michael Chan @ 2007-10-02 22:02 UTC (permalink / raw)
  To: Andy Gospodarek; +Cc: David Miller, jeff, netdev

On Tue, 2007-10-02 at 16:16 -0400, Andy Gospodarek wrote:
> Adding that flag in tg3_set_settings seemed like the most logical
> place
> since the driver works fine on boot.  This is just an issue when
> re-enabling autonegotiation, so we should probably nip it there.
> 
> Signed-off-by: Andy Gospodarek <andy@greyhouse.net>

We also noticed this issue recently, but didn't pay too much attention
to it since it was more of a "cosmetic" issue.  The driver behaves the
same since we rely on cmd->autoneg to decide whether to enable autoneg
or not.  Your fix seems reasonable to me.  Thanks.

Acked-by: Michael Chan <mchan@broadcom.com>


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 2.6.24] tg3: fix ethtool autonegotiate flags
  2007-10-02 22:02 ` Michael Chan
  2007-10-02 21:16   ` Andy Gospodarek
@ 2007-10-08  8:09   ` David Miller
  1 sibling, 0 replies; 4+ messages in thread
From: David Miller @ 2007-10-08  8:09 UTC (permalink / raw)
  To: mchan; +Cc: andy, jeff, netdev

From: "Michael Chan" <mchan@broadcom.com>
Date: Tue, 02 Oct 2007 15:02:56 -0700

> On Tue, 2007-10-02 at 16:16 -0400, Andy Gospodarek wrote:
> > Adding that flag in tg3_set_settings seemed like the most logical
> > place
> > since the driver works fine on boot.  This is just an issue when
> > re-enabling autonegotiation, so we should probably nip it there.
> > 
> > Signed-off-by: Andy Gospodarek <andy@greyhouse.net>
> 
> We also noticed this issue recently, but didn't pay too much attention
> to it since it was more of a "cosmetic" issue.  The driver behaves the
> same since we rely on cmd->autoneg to decide whether to enable autoneg
> or not.  Your fix seems reasonable to me.  Thanks.
> 
> Acked-by: Michael Chan <mchan@broadcom.com>

Applied, thanks everyone!

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2007-10-08  8:09 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-10-02 20:16 [PATCH 2.6.24] tg3: fix ethtool autonegotiate flags Andy Gospodarek
2007-10-02 22:02 ` Michael Chan
2007-10-02 21:16   ` Andy Gospodarek
2007-10-08  8:09   ` David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).