Re: e1000 speed/duplex error

[Auke Kok <auke-jan.h.kok@intel.com>]

a1 wrote:
> JK> I agree.  Although ethtool does not have that functionality as of yet.
> JK>  Feel free to provide a patch to the ethtool maintainer (Jeff Garzik)
> JK> if you would like.  I will put it on my plate of things to do, but I
> JK> will admit that it is near the bottom of the list of items to get done
> JK> for me.  Feel free to ping me once in awhile to remind me.
> 
> Ethtool already have support for that, but e1000 driver doesn't treat
> all values passed from ethtool correctly.
> 
> For example, if I run ethtool with the following parameters:
> ethtool -s eth0 speed 100 duplex full autoneg on
> 
> parameters filled by ethtool looks like:
> 
> ecmd->autoneg = AUTONEG_ENABLE;
> ecmd->advertising = ADVERTISED_100baseT_Full;
> 
> but then they passed to the driver, driver fills the structure passed
> to the hw layer with all possible advertise values.

>         if (ecmd->autoneg == AUTONEG_ENABLE) {
>                 hw->autoneg = 1;
>                 if (hw->media_type == e1000_media_type_fiber)
>                         hw->autoneg_advertised = ADVERTISED_1000baseT_Full |
>                                      ADVERTISED_FIBRE |
>                                      ADVERTISED_Autoneg;
>                 else
> --->                    hw->autoneg_advertised = ADVERTISED_10baseT_Half |
>                                                   ADVERTISED_10baseT_Full |
>                                                   ADVERTISED_100baseT_Half |
>                                                   ADVERTISED_100baseT_Full |
>                                                   ADVERTISED_1000baseT_Full|
>                                                   ADVERTISED_Autoneg |
>                                                   ADVERTISED_TP;
>                 ecmd->advertising = hw->autoneg_advertised;
>         } else

> If you change it that way everything works like I thought
> 
> --- e1000_ethtool.c.orig        Mon Jun 26 14:13:26 2006
> +++ e1000_ethtool.c     Wed Aug 02 12:35:36 2006
> @@ -225,13 +225,7 @@
>                                      ADVERTISED_FIBRE |
>                                      ADVERTISED_Autoneg;
>                 else
> -                       hw->autoneg_advertised = ADVERTISED_10baseT_Half |
> -                                                 ADVERTISED_10baseT_Full |
> -                                                 ADVERTISED_100baseT_Half |
> -                                                 ADVERTISED_100baseT_Full |
> -                                                 ADVERTISED_1000baseT_Full|
> -                                                 ADVERTISED_Autoneg |
> -                                                 ADVERTISED_TP;
> +                       hw->autoneg_advertised = ecmd->advertising;

Don't you mean this? :

+                       hw->autoneg_advertised = ecmd->advertising |
+                                                ADVERTISED_Autoneg |
+                                                ADVERTISED_TP;

and we'd also have to do this for fibre...

 >
>                 ecmd->advertising = hw->autoneg_advertised;
>         } else
>                 if (e1000_set_spd_dplx(adapter, ecmd->speed + ecmd->duplex))


but that's not really what you want: the way ethtool works currently only 
allows you to pass *one* speed/duplex tuple and autonegotiate with that, or 
all (by omitting any speed/duplex tuple).

ethtool needs some code that allows you to specify "autonegotiate 10_half or 
100_full or 1000_full" (3 tuples, but not implying 100_half or 10_full). This 
is something mii-tool was able to do but this functionality never made it into 
ethtool AFAIK :)

This is the most useful case for everyone, you can omit advertising gig link 
if you only have 100mbit switches and speed up link times that way etc.

Auke