On 2013-05-24 23:37, Mike Belopuhov wrote:
<...>
I also noticed if_bge.c line 2293:
BGE_SETBIT(sc, BGE_MI_MODE, BGE_MIMODE_AUTOPOLL|10<<16);
Does any one have a clue why the (10<<16) is there? it changes the
MI clock. But why? and why OR 0xA with the default 0xC, instead of
0x2? FreeBSD and Linux use the default of 0xC for the MI Clock.
I think it's a remnant of the original code and it should be
BGE_MIMODE_BASE (as in "the other frequency").
But since BGE_MIMODE_BASE is the default value after reset and
OR'ing the value wouldn't be correct, I suggest to just remove
the '|(10<<16)'.
I'm not OR'ing it in, I'm overwriting the value.
Sorry, i meant in the old code.
<...>
This also changes the "Use Short Preamble" flag and "PHY Address"
which are not 0 by default.
I've followed FreeBSD here. If you inspect the Linux driver you'll
find the same:
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/net/ethernet/broadcom/tg3.c#n16149
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/net/ethernet/broadcom/tg3.c#n1479
I guess we need to think about it a bit more.
Tests will help though.
Ok, I tested this with a Broadcom BCM5721. Auto-polling doesn't work
anymore when the "PHY Address" is set to 0. So I added setting of the
correct phy address in the patch below. Clearing "Use Short Preamble"
doesn't seem to matter, and is also the default for older chips.
Apart from that I think the patch is ok.
Kind regards,
David
Index: sys/dev/pci/if_bge.c
===================================================================
RCS file: /cvs/src/sys/dev/pci/if_bge.c,v
retrieving revision 1.328
diff -u -p -r1.328 if_bge.c
--- sys/dev/pci/if_bge.c 22 May 2013 16:02:31 -0000 1.328
+++ sys/dev/pci/if_bge.c 27 May 2013 13:01:24 -0000
@@ -1057,6 +1057,18 @@ bge_miibus_statchg(struct device *dev)
mii->mii_media_active &= ~IFM_ETH_FMASK;
}
+ if (!BGE_STS_BIT(sc, BGE_STS_LINK) &&
+ mii->mii_media_status & IFM_ACTIVE &&
+ IFM_SUBTYPE(mii->mii_media_active) != IFM_NONE)
+ BGE_STS_SETBIT(sc, BGE_STS_LINK);
+ else if (BGE_STS_BIT(sc, BGE_STS_LINK) &&
+ (!(mii->mii_media_status & IFM_ACTIVE) ||
+ IFM_SUBTYPE(mii->mii_media_active) == IFM_NONE))
+ BGE_STS_CLRBIT(sc, BGE_STS_LINK);
+
+ if (!BGE_STS_BIT(sc, BGE_STS_LINK))
+ return;
+
/* Set the port mode (MII/GMII) to match the link speed. */
mac_mode = CSR_READ_4(sc, BGE_MAC_MODE) &
~(BGE_MACMODE_PORTMODE | BGE_MACMODE_HALF_DUPLEX);
@@ -1775,7 +1787,7 @@ bge_blockinit(struct bge_softc *sc)
volatile struct bge_rcb *rcb;
vaddr_t rcb_addr;
bge_hostaddr taddr;
- u_int32_t dmactl, val;
+ u_int32_t dmactl, mimode, val;
int i, limit;
/*
@@ -2371,9 +2383,19 @@ bge_blockinit(struct bge_softc *sc)
if (sc->bge_flags & BGE_PHY_FIBER_TBI) {
CSR_WRITE_4(sc, BGE_MI_STS, BGE_MISTS_LINK);
} else {
- BGE_STS_SETBIT(sc, BGE_STS_AUTOPOLL);
- BGE_SETBIT(sc, BGE_MI_MODE, BGE_MIMODE_AUTOPOLL|10<<16);
- if (BGE_ASICREV(sc->bge_chipid) == BGE_ASICREV_BCM5700)
+ if ((sc->bge_flags & BGE_CPMU_PRESENT) != 0)
+ mimode = BGE_MIMODE_500KHZ_CONST;
+ else
+ mimode = BGE_MIMODE_BASE;
+ mimode |= ((sc->bge_phy_addr & 0x1f) << 5);
+ if (BGE_IS_5700_FAMILY(sc) ||
+ BGE_ASICREV(sc->bge_chipid) == BGE_ASICREV_BCM5705) {
+ mimode |= BGE_MIMODE_AUTOPOLL;
+ BGE_STS_SETBIT(sc, BGE_STS_AUTOPOLL);
+ }
+ CSR_WRITE_4(sc, BGE_MI_MODE, mimode);
+ if (BGE_ASICREV(sc->bge_chipid) == BGE_ASICREV_BCM5700 &&
+ sc->bge_chipid != BGE_CHIPID_BCM5700_B2)
CSR_WRITE_4(sc, BGE_MAC_EVT_ENB,
BGE_EVTENB_MI_INTERRUPT);
}
@@ -2721,9 +2743,6 @@ bge_attach(struct device *parent, struct
BGE_ASICREV(sc->bge_chipid) == BGE_ASICREV_BCM57780)
sc->bge_flags |= BGE_CPMU_PRESENT;
- if ((sc->bge_flags & BGE_CPMU_PRESENT) != 0)
- BGE_SETBIT(sc, BGE_MI_MODE, BGE_MIMODE_500KHZ_CONST);
-
/* Try to reset the chip. */
DPRINTFN(5, ("bge_reset\n"));
bge_sig_pre_reset(sc, BGE_RESET_START);
@@ -4491,11 +4510,6 @@ bge_link_upd(struct bge_softc *sc)
if_link_state_change(ifp);
ifp->if_baudrate = 0;
}
- /*
- * Discard link events for MII/GMII cards if MI auto-polling
disabled.
- * This should not happen since mii callouts are locked now, but
- * we keep this check for debug.
- */
} else if (BGE_STS_BIT(sc, BGE_STS_AUTOPOLL)) {
/*
* Some broken BCM chips have BGE_STATFLAG_LINKSTATE_CHANGED bit
@@ -4517,6 +4531,13 @@ bge_link_upd(struct bge_softc *sc)
IFM_SUBTYPE(mii->mii_media_active) == IFM_NONE))
BGE_STS_CLRBIT(sc, BGE_STS_LINK);
}
+ } else {
+ /*
+ * For controllers that call mii_tick, we have to poll
+ * link status.
+ */
+ mii_pollstat(mii);
+ bge_miibus_statchg(&sc->bge_dev);
}
/* Clear the attention */
