On Thu, May 23, 2013 at 11:04 +0200, David Imhoff wrote:
> Hi,
>
> Having finally found the BCM5718 Programmers guide from
> http://www.broadcom.com/support/ethernet_nic/open_source.php I
> managed to get a bit further with this problem.
>
> The problem seems to be in the Auto polling of the mac link state.
> ifconfig shows the correct link state, but the BGE_STS_LINK bit in
> the driver is never set. Debugging the interrupt routine I found the
> driver never receives a link state changed attention, instead the
> last interrupt indicates a auto-poll error and bit 0 of register
> BGE_AUTOPOLL_STS is set.
>
> Looking at the FreeBSD and Linux drivers I found that they don't use
> auto-polling. Also the programmers guide of the BCM5718 family(page
> 204) doesn't describe the auto-polling method anymore, as was done
> in the PG of other BCM57xx.
>
I agree that this might be a better solution than what I had:
setting the BGE_STS_LINK_EVT flag back in bge_link_upd to force
the bge_intr into running it every time untill it gets the link;
and it seems to work.
> So I disabled Auto-polling for the BCM5718 family. This made all
> ports work on my Dell R320(BCM5720) and BCM5719. My BCM5719 still
> doesn't fully work because now i run into bge_watchdog reset's when
> sending big packet, the same issue reported by Robert Young on 5
> April.
>
Right, it seems like the TX completion interrupts never arrive
for large packets. Forcing txeof doesn't help. In fact I can
see some funny things going on here: for small ping packets 1
out of 1 tx buffer descriptors contain an mbuf to scratch, for
170 byte packets 1 out of 2 tx bd's contain an mbuf, for 250
byte packets 1 out of 3 tx bd's contain an mbuf, for 500 byte
packets TX completion interrupt never arrives.
I had a suspicion that this is because ASF mode is turned on
and we fail to share a port with IPMI firmware, but right now
I'm more inclined to believe that this is just another TX bug.
ChipID here is 0x5719001 as well but APE firmware NCSI 1.1.15.0.
> Attached is a patch that disables mac polling for the BCM5717, 5719
> and 5720. Tested on BCM5719, BCM5720 and BCM5750 C1.
>
> 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").
> Kind regards,
>
> David
>
> Index: sys/dev/pci/if_bge.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/pci/if_bge.c,v
> retrieving revision 1.320
> diff -u -r1.320 if_bge.c
> --- sys/dev/pci/if_bge.c 4 Mar 2013 01:33:18 -0000 1.320
> +++ sys/dev/pci/if_bge.c 21 May 2013 13:18:36 -0000
> @@ -2288,7 +2288,9 @@
> /* Enable PHY auto polling (for MII/GMII only) */
> if (sc->bge_flags & BGE_PHY_FIBER_TBI) {
> CSR_WRITE_4(sc, BGE_MI_STS, BGE_MISTS_LINK);
> - } else {
> + } else if (!(BGE_ASICREV(sc->bge_chipid) == BGE_ASICREV_BCM5717 ||
> + BGE_ASICREV(sc->bge_chipid) == BGE_ASICREV_BCM5719 ||
> + BGE_ASICREV(sc->bge_chipid) == BGE_ASICREV_BCM5720)) {
> 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)
> @@ -4386,18 +4388,13 @@
> 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)) {
> + } else {
> /*
I think you've missed some important bits here.
> * Some broken BCM chips have BGE_STATFLAG_LINKSTATE_CHANGED bit
> * in status word always set. Workaround this bug by reading
> * PHY link status directly.
> */
> - link = (CSR_READ_4(sc, BGE_MI_STS) & BGE_MISTS_LINK)?
> + link = (CSR_READ_4(sc, BGE_TX_STS) & BGE_TXSTAT_LINK_UP)?
> BGE_STS_LINK : 0;
>
> if (BGE_STS_BIT(sc, BGE_STS_LINK) != link) {
>
>
Why did you change BGE_MI_STS to BGE_TX_STS?
Please inspect my diff below; I believe it's more complete.
diff --git sys/dev/pci/if_bge.c sys/dev/pci/if_bge.c
index 1d37192..d357443 100644
--- sys/dev/pci/if_bge.c
+++ sys/dev/pci/if_bge.c
@@ -1055,10 +1055,22 @@ bge_miibus_statchg(struct device *dev)
(mii->mii_media_active & IFM_ETH_FMASK) != sc->bge_flowflags) {
sc->bge_flowflags = mii->mii_media_active & IFM_ETH_FMASK;
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);
tx_mode = CSR_READ_4(sc, BGE_TX_MODE);
rx_mode = CSR_READ_4(sc, BGE_RX_MODE);
@@ -1773,11 +1785,11 @@ int
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;
/*
* Initialize the memory window pointer register so that
* we can access the first 32K of internal NIC RAM. This will
@@ -2369,12 +2381,20 @@ bge_blockinit(struct bge_softc *sc)
/* Enable PHY auto polling (for MII/GMII only) */
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 ((sc->bge_flags & BGE_CPMU_PRESENT) != 0)
+ mimode = BGE_MIMODE_500KHZ_CONST;
+ else
+ mimode = BGE_MIMODE_BASE;
+ 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)
CSR_WRITE_4(sc, BGE_MAC_EVT_ENB,
BGE_EVTENB_MI_INTERRUPT);
}
@@ -4515,10 +4535,17 @@ bge_link_upd(struct bge_softc *sc)
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);
}
+ } 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 */
CSR_WRITE_4(sc, BGE_MAC_STS, BGE_MACSTAT_SYNC_CHANGED|
BGE_MACSTAT_CFG_CHANGED|BGE_MACSTAT_MI_COMPLETE|
