[Fwd: Re: bge Ierr rate increase from 5.3R -> 6.1R]

[John Polstra]

On 09-Jan-2007 Oleg Bulyzhin wrote:
> On Mon, Jan 08, 2007 at 04:33:29PM -0800, John Polstra wrote:
>> On 30-Dec-2006 Bruce Evans wrote:
>> > More debugging showed that almost any of the reads of the phy in mii
>> > cause an input error,
>> 
>> The errors appear to be caused by the code in bge_miibus_{read,write}reg
>> that clears and then restores the BGE_MIMODE_AUTOPOLL bit of the
>> BGE_MI_MODE register.  If you remove those chunks of code, the errors
>> go away even when mii_tick is called periodically, and the chip
>> performs quite well under heavy traffic load in both directions.
> 
> I've seen this kind of ierrors (LINK_LOST error flag in rx buffer descriptor)
> only on bcm5701 chip. Other chips i tried (5700 and 5721) seems are not
> affected.

The packet loss I observed was on a 5704.  I didn't check the
LINK_LOST flags.

>> I agree with you that we shouldn't need to call mii_tick periodically,
>> except perhaps for a few very old variants of the Broadcom chip.
>> 
>> John
> mii_tick periodic calls are requried -  MII layer use it for link
> autonegotiation. Though i guess it's possible to avoid phy access if we
> have link up.
> I see two different ways to solve this:
> 1) convert driver for using phy interrupts as default (like linux driver),
>    thus BGE_MI_MODE read/write can be avoided.
> 2) do not touch PHY inside bge_tick() if we have link up. Actually, this
>    check was in driver but was removed as workaround of existed link state
>    issues (due to some bugs in if_bge.c and brgphy.c link loss events were
>    detected with few seconds delay or even not detected at all).
>    Those bugs are fixed now, so we can revive this check (bge_link_upd() can
>    be simplified a bit too).

I've been using #2 today, and it's working well so far.  I
implemented it like this.  (Ignore the version numbers; I'm working
in a private repository.)

--- if_bge.c    8 Jan 2007 22:46:51 -0000       1.26
+++ if_bge.c    9 Jan 2007 22:52:43 -0000       1.27
@@ -3122,8 +3122,8 @@ bge_tick(void *xsc)
 
        if ((sc->bge_flags & BGE_FLAG_TBI) == 0) {
                mii = device_get_softc(sc->bge_miibus);
-               /* Don't mess with the PHY in IPMI/ASF mode */
-               if (!((sc->bge_asf_mode & ASF_STACKUP) && (sc->bge_link)))
+               /* Don't mess with the PHY unless link is down. */
+               if (!sc->bge_link)
                        mii_tick(mii);
        } else {
                /*


John