Drivers using mii_check_media (via-rhine in particular) and also
forcing link parameters with ethtool can reach a state where the link
goes down and never comes back up. This is because mii_check_media
short-circuits early if mii->force_media != 0. This was discussed
in a couple of past threads, one of which is available here:
http://www.ussg.iu.edu/hypermail/linux/kernel/0508.3/0670.html
The patch moves the force_media check to below the carrier status
check. This allows the link state to show correctly, while avoiding
the check of link parameters.
Signed-off-by: John W. Linville <[EMAIL PROTECTED]>
---
This is a repost from 24 January 2006, which immediately drew Jeff
Garzik's reply:
> The entire point of force_media is that you don't check carrier status,
> its assumed to always be up...
>
> As I noted in a couple threads, if force_media is set, the driver should
> call netif_carrier_on() and then never call a function that causes the
> carrier state to change. The existing mii.c logic follows this.
FWIW, one of the noted threads is here:
http://lkml.org/lkml/2005/10/4/128
I'm reposting, because I don't think we should associate forcing the
link's _parameters_ (i.e. speed and duplex) with forcing the link's
_state_ (i.e up or down). The LAN switch projects in my previous life
certainly did not do that. The presence or absence of carrier on the
link is distinctly different from whether or not auto-negotiation will
be used on that link.
Points to ponder:
Interestingly, mii_ethtool_sset does not call netif_carrier_on, as
one might expect it to do when auto-negotiation is disabled. So if
a link was down before ethtool was invoked, it would never be up?
(I didn't experiment here, this is just from looking at the code...)
Also, mii_check_link does not reference force_media at all. So,
those drivers using mii_check_link (currently e100 and eepro100)
necessarily treat state as separate from parameters.
Finally, as best as I can tell almost all current references to force_media
(outside of mii_check_media) relate exclusively to link parameters,
and not to the link state (*). Can you refer me to a driver that checks
force_media and calls netif_carrier_on?
Clearly, I think it makes the most sense to move the force_media check
below the link state portion of mii_check_media. This allows the
link state to be properly reflected at all times, while preserving the
ability to force the link parameters as desired. If there is hardware
that cannot separate these items (is there any?), then those drivers
will have to override the ethtool set_settings method appropriately.
(*) The smc91x driver is partially an exception (only for certain
PHYs). I'll cook-up a patch to correct it's behaviour if we get some
acceptance here.
drivers/net/mii.c | 11 +++++++----
1 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/net/mii.c b/drivers/net/mii.c
index e42aa79..f919eb1 100644
--- a/drivers/net/mii.c
+++ b/drivers/net/mii.c
@@ -266,10 +266,6 @@ unsigned int mii_check_media (struct mii
int advertise, lpa, media, duplex;
int lpa2 = 0;
- /* if forced media, go no further */
- if (mii->force_media)
- return 0; /* duplex did not change */
-
/* check current and old link status */
old_carrier = netif_carrier_ok(mii->dev) ? 1 : 0;
new_carrier = (unsigned int) mii_link_ok(mii);
@@ -293,6 +289,13 @@ unsigned int mii_check_media (struct mii
*/
netif_carrier_on(mii->dev);
+ /* if forced media, go no further */
+ if (mii->force_media) {
+ if (ok_to_print)
+ printk(KERN_INFO "%s: link up\n", mii->dev->name);
+ return 0; /* duplex did not change */
+ }
+
/* get MII advertise and LPA values */
if ((!init_media) && (mii->advertising))
advertise = mii->advertising;
--
John W. Linville
[EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
