On Tue, 2007-04-24 at 20:48 +0300, [EMAIL PROTECTED] wrote: > > On Tue, Apr 24, 2007 at 12:49:12PM -0400, Jeff Garzik wrote: > >> Long term, Greg seemed OK with moving the net drivers from > >> drivers/usb/net > >> to drivers/usb/net, in line with the current policy of placing net > >> drivers > >> in drivers/net/*, bus agnostic. After that move, sending to netdev and > >> me > >> (as you did here) would be the preferred avenue. > > > > Speaking of which, do you want me to do this in the 2.6.22-rc1 > > timeframe? Usually big code moves like this are good to do right after > > rc1 comes out as the major churn is usually completed then. > > Sorry to interfere, but could you guys wait until tomorrow before applying > the patch to your respective GIT trees? I'd like to check if the code is > doing the right thing and avoid patch reversal.
Original problem was that the patch I referenced in the commit message from Jan 6 2006 switched the return value semantics from read_mii_word(). Before the patch, read_mii_word returned 1 on success, 0 on error. After the patch, it returns the generally accepted 0 on success and !0 on error. That causes set_carrier() to return immediately rather than fiddle with netif_carrier_*. When the Jan 6 2006 patch went in changing the return values, set_carrier() was not updated for the new return values. Nothing else in the code cares about read_mii_word()'s return value except set_carrier(). But when the card is brought up and no cable is plugged in, intr_callback() gets called repeatedly, which itself repeatedly calls netif_carrier_on() due to the NO_CARRIER check. The comment there about "NO_CARRIER kicks in on TX failure" seems accurate, because even with no cable plugged in, and therefore no packets getting transmitted, the NO_CARRIER check is never true on the Belkin part. Therefore, netif_carrier_on() is always called as a result of the failure of d[0] & NO_CARRIER, turning carrier back on even if there is no cable plugged in. This bulldozes over the MII carrier_check routine too. I don't think the intr_callback() code should ever turn the carrier _on_, because there's that 2*HZ MII carrier check which can certainly handle the carrier on/off stuff. LINK_STATUS appears valid on the Belkin part too, so we can add that as a reverse-quirk and use LINK_STATUS on parts where it works. If you think that the NO_CARRIER check should be in _addition_ to the LINK_STATUS check, that's fine with me, provided that the NO_CARRIER check only turns carrier off. Dan - 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
