Attachment 144650 Details for Bug 191786 – Check for proper phy messages, never check phy status in bce_tick()

[patch] Check for proper phy messages, never check phy status in bce_tick()

bce-phy-intr.diff (text/plain), 8.92 KB, created by Kajetan Staszkiewicz on 2014-07-14 12:01:40 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Kajetan Staszkiewicz

Created: 2014-07-14 12:01:40 UTC

Size: 8.92 KB

patch

obsolete

>diff --git a/sys/dev/bce/if_bce.c b/sys/dev/bce/if_bce.c
>index 8367fb9..d84d620 100644
>--- a/sys/dev/bce/if_bce.c
>+++ b/sys/dev/bce/if_bce.c
>@@ -5309,7 +5309,7 @@ bce_blockinit(struct bce_softc *sc)
> 	    BCE_DRV_MSG_CODE_RESET);
> 
> 	/* Enable link state change interrupt generation. */
>-	REG_WR(sc, BCE_HC_ATTN_BITS_ENABLE, STATUS_ATTN_BITS_LINK_STATE);
>+	REG_WR(sc, BCE_HC_ATTN_BITS_ENABLE, STATUS_ATTN_EVENTS);
> 
> 	/* Enable the RXP. */
> 	bce_start_rxp_cpu(sc);
>@@ -6317,7 +6317,9 @@ bce_ifmedia_sts_rphy(struct bce_softc *sc, struct ifmediareq *ifmr)
> 	ifmr->ifm_status = IFM_AVALID;
> 	ifmr->ifm_active = IFM_ETHER;
> 	link = bce_shmem_rd(sc, BCE_LINK_STATUS);
>-	/* XXX Handle heart beat status? */
>+	if (link & BCE_LINK_STATUS_HEART_BEAT_EXPIRED)
>+		bce_pulse(sc);
>+	link &= ~BCE_LINK_STATUS_HEART_BEAT_EXPIRED;
> 	if ((link & BCE_LINK_STATUS_LINK_UP) != 0)
> 		ifmr->ifm_status |= IFM_ACTIVE;
> 	else {
>@@ -6429,56 +6431,129 @@ bce_ifmedia_sts(struct ifnet *ifp, struct ifmediareq *ifmr)
> static void
> bce_phy_intr(struct bce_softc *sc)
> {
>+	int was_changed = 0;
> 	u32 new_link_state, old_link_state;
>+	u32 event_code;
>+	struct mii_data *mii;
>+	struct ifmediareq ifmr;
>+	struct ifnet *ifp;
>+
>+	ifp = sc->bce_ifp;
> 
> 	DBENTER(BCE_VERBOSE_PHY | BCE_VERBOSE_INTR);
> 
> 	DBRUN(sc->phy_interrupts++);
> 
>+	/* Acknowledge link state event, if it was sent. */
> 	new_link_state = sc->status_block->status_attn_bits &
> 	    STATUS_ATTN_BITS_LINK_STATE;
> 	old_link_state = sc->status_block->status_attn_bits_ack &
> 	    STATUS_ATTN_BITS_LINK_STATE;
>-
>-	/* Handle any changes if the link state has changed. */
> 	if (new_link_state != old_link_state) {
>-
>-		/* Update the status_attn_bits_ack field. */
> 		if (new_link_state) {
> 			REG_WR(sc, BCE_PCICFG_STATUS_BIT_SET_CMD,
> 			    STATUS_ATTN_BITS_LINK_STATE);
>-			DBPRINT(sc, BCE_INFO_PHY, "%s(): Link is now UP.\n",
>-			    __FUNCTION__);
> 		} else {
> 			REG_WR(sc, BCE_PCICFG_STATUS_BIT_CLEAR_CMD,
> 			    STATUS_ATTN_BITS_LINK_STATE);
>-			DBPRINT(sc, BCE_INFO_PHY, "%s(): Link is now DOWN.\n",
>-			    __FUNCTION__);
> 		}
>+		was_changed = 1;
>+	}
>+
>+	/* Acknowledge timeout event, if it was sent. */
>+	new_link_state = sc->status_block->status_attn_bits &
>+	    STATUS_ATTN_BITS_TIMER_ABORT;
>+	old_link_state = sc->status_block->status_attn_bits_ack &
>+	    STATUS_ATTN_BITS_TIMER_ABORT;
>+	if (new_link_state != old_link_state) {
>+		if (new_link_state) {
>+			REG_WR(sc, BCE_PCICFG_STATUS_BIT_SET_CMD,
>+			    STATUS_ATTN_BITS_TIMER_ABORT);
>+		} else {
>+			REG_WR(sc, BCE_PCICFG_STATUS_BIT_CLEAR_CMD,
>+			    STATUS_ATTN_BITS_TIMER_ABORT);
>+		}
>+		was_changed = 1;
>+	}
>+
> 
>+	/*
>+	 * Check what was really sent. Remote PHY link changes are sent as
>+	 * timeout events but can be distinguised.
>+	 */
>+	event_code = bce_shmem_rd(sc, BCE_FW_EVT_CODE_MB);
>+
>+	if (event_code == BCE_FW_EVT_CODE_LINK_EVENT) {
>+
>+		/* Remember old link state. */
>+		old_link_state = sc->bce_link_up;
>+
>+		/* Pretend that link is down. */
>+		sc->bce_link_up = FALSE;
>+
>+		/* Get current PHY status. */
> 		if ((sc->bce_phy_flags & BCE_PHY_REMOTE_CAP_FLAG) != 0) {
>-			if (new_link_state) {
>-				if (bootverbose)
>-					if_printf(sc->bce_ifp, "link UP\n");
>-				if_link_state_change(sc->bce_ifp,
>-				    LINK_STATE_UP);
>-			} else {
>-				if (bootverbose)
>-					if_printf(sc->bce_ifp, "link DOWN\n");
>-				if_link_state_change(sc->bce_ifp,
>-				    LINK_STATE_DOWN);
>+			bzero(&ifmr, sizeof(ifmr));
>+			bce_ifmedia_sts_rphy(sc, &ifmr);
>+			if ((ifmr.ifm_status & (IFM_ACTIVE | IFM_AVALID)) ==
>+			    (IFM_ACTIVE | IFM_AVALID)) {
>+				sc->bce_link_up = TRUE;
>+				bce_miibus_statchg(sc->bce_dev);
>+			}
>+		} else {
>+			mii = device_get_softc(sc->bce_miibus);
>+			mii_tick(mii);
>+			/* Check if the link has come up. */
>+			if ((mii->mii_media_status & IFM_ACTIVE) &&
>+			    (IFM_SUBTYPE(mii->mii_media_active) != IFM_NONE)) {
>+				DBPRINT(sc, BCE_VERBOSE_MISC, "%s(): Link up!\n",
>+				    __FUNCTION__);
>+				sc->bce_link_up = TRUE;
>+				if ((IFM_SUBTYPE(mii->mii_media_active) == IFM_1000_T ||
>+				    IFM_SUBTYPE(mii->mii_media_active) == IFM_1000_SX ||
>+				    IFM_SUBTYPE(mii->mii_media_active) == IFM_2500_SX) &&
>+				    (bce_verbose || bootverbose))
>+					BCE_PRINTF("Gigabit link up!\n");
>+			}
>+	
>+		}
>+		if (sc->bce_link_up == TRUE) {
>+			/* Now that link is up, handle any outstanding TX traffic. */
>+			if (!IFQ_DRV_IS_EMPTY(&ifp->if_snd)) {
>+				DBPRINT(sc, BCE_VERBOSE_MISC, "%s(): Found "
>+				    "pending TX traffic.\n", __FUNCTION__);
>+				bce_start_locked(ifp);
> 			}
> 		}
>+	
> 		/*
>-		 * Assume link is down and allow
>-		 * tick routine to update the state
>-		 * based on the actual media state.
>+		 * Now that the real link status is read,
>+		 * handle its change, if there is any.
> 		 */
>-		sc->bce_link_up = FALSE;
>-		callout_stop(&sc->bce_tick_callout);
>-		bce_tick(sc);
>+		if (old_link_state != sc->bce_link_up) {
>+			/* Handle remote PHY state change */
>+			if ((sc->bce_phy_flags & BCE_PHY_REMOTE_CAP_FLAG) != 0
>+			    && was_changed) {
>+				if (sc->bce_link_up) {
>+					if (bce_verbose || bootverbose)
>+						if_printf(sc->bce_ifp, "link UP, ifm_active: %08x\n",
>+						    ifmr.ifm_active);
>+					if_link_state_change(sc->bce_ifp,
>+					    LINK_STATE_UP);
>+				} else {
>+					if (bce_verbose || bootverbose)
>+						if_printf(sc->bce_ifp, "link DOWN, ifm_active: %08x\n",
>+						    ifmr.ifm_active);
>+					if_link_state_change(sc->bce_ifp,
>+				    	    LINK_STATE_DOWN);
>+				}
>+			}
>+		}
>+	} else if (event_code == BCE_FW_EVT_CODE_SW_TIMER_EXPIRE_EVENT) {
>+		bce_pulse(sc);
> 	}
> 
>+
> 	/* Acknowledge the link change interrupt. */
> 	REG_WR(sc, BCE_EMAC_STATUS, BCE_EMAC_STATUS_LINK_CHANGE);
> 
>@@ -7961,10 +8036,14 @@ bce_intr(void *xsc)
> 		    status_attn_bits = status_attn_bits |
> 		    STATUS_ATTN_BITS_PARITY_ERROR);
> 
>-		/* Was it a link change interrupt? */
>-		if ((status_attn_bits & STATUS_ATTN_BITS_LINK_STATE) !=
>+		/*
>+		  * We may be notified about link state change or request
>+		  * for pulse to be sent.
>+		  */
>+		if ((status_attn_bits & STATUS_ATTN_EVENTS) !=
> 		    (sc->status_block->status_attn_bits_ack &
>-		     STATUS_ATTN_BITS_LINK_STATE)) {
>+		     STATUS_ATTN_EVENTS)) {
>+
> 			bce_phy_intr(sc);
> 
> 			/* Clear transient updates during link state change. */
>@@ -7974,9 +8053,9 @@ bce_intr(void *xsc)
> 		}
> 
> 		/* If any other attention is asserted, the chip is toast. */
>-		if (((status_attn_bits & ~STATUS_ATTN_BITS_LINK_STATE) !=
>+		if (((status_attn_bits & ~STATUS_ATTN_EVENTS) !=
> 		    (sc->status_block->status_attn_bits_ack &
>-		    ~STATUS_ATTN_BITS_LINK_STATE))) {
>+		    ~STATUS_ATTN_EVENTS))) {
> 
> 			sc->unexpected_attention_count++;
> 
>@@ -8440,11 +8519,6 @@ static void
> bce_tick(void *xsc)
> {
> 	struct bce_softc *sc = xsc;
>-	struct mii_data *mii;
>-	struct ifnet *ifp;
>-	struct ifmediareq ifmr;
>-
>-	ifp = sc->bce_ifp;
> 
> 	DBENTER(BCE_EXTREME_MISC);
> 
>@@ -8464,46 +8538,6 @@ bce_tick(void *xsc)
> 	/* Check that chip hasn't hung. */
> 	bce_watchdog(sc);
> 
>-	/* If link is up already up then we're done. */
>-	if (sc->bce_link_up == TRUE)
>-		goto bce_tick_exit;
>-
>-	/* Link is down.  Check what the PHY's doing. */
>-	if ((sc->bce_phy_flags & BCE_PHY_REMOTE_CAP_FLAG) != 0) {
>-		bzero(&ifmr, sizeof(ifmr));
>-		bce_ifmedia_sts_rphy(sc, &ifmr);
>-		if ((ifmr.ifm_status & (IFM_ACTIVE | IFM_AVALID)) ==
>-		    (IFM_ACTIVE | IFM_AVALID)) {
>-			sc->bce_link_up = TRUE;
>-			bce_miibus_statchg(sc->bce_dev);
>-		}
>-	} else {
>-		mii = device_get_softc(sc->bce_miibus);
>-		mii_tick(mii);
>-		/* Check if the link has come up. */
>-		if ((mii->mii_media_status & IFM_ACTIVE) &&
>-		    (IFM_SUBTYPE(mii->mii_media_active) != IFM_NONE)) {
>-			DBPRINT(sc, BCE_VERBOSE_MISC, "%s(): Link up!\n",
>-			    __FUNCTION__);
>-			sc->bce_link_up = TRUE;
>-			if ((IFM_SUBTYPE(mii->mii_media_active) == IFM_1000_T ||
>-			    IFM_SUBTYPE(mii->mii_media_active) == IFM_1000_SX ||
>-			    IFM_SUBTYPE(mii->mii_media_active) == IFM_2500_SX) &&
>-			    (bce_verbose || bootverbose))
>-				BCE_PRINTF("Gigabit link up!\n");
>-		}
>-
>-	}
>-	if (sc->bce_link_up == TRUE) {
>-		/* Now that link is up, handle any outstanding TX traffic. */
>-		if (!IFQ_DRV_IS_EMPTY(&ifp->if_snd)) {
>-			DBPRINT(sc, BCE_VERBOSE_MISC, "%s(): Found "
>-			    "pending TX traffic.\n", __FUNCTION__);
>-			bce_start_locked(ifp);
>-		}
>-	}
>-
>-bce_tick_exit:
> 	DBEXIT(BCE_EXTREME_MISC);
> }
> 
>diff --git a/sys/dev/bce/if_bcereg.h b/sys/dev/bce/if_bcereg.h
>index b2b5928..8c0698b 100644
>--- a/sys/dev/bce/if_bcereg.h
>+++ b/sys/dev/bce/if_bcereg.h
>@@ -801,6 +801,7 @@ struct flash_spec {
> #define BCE_LINK_STATUS_SERDES_LINK		 (1<<20)
> #define BCE_LINK_STATUS_PARTNER_AD_2500FULL	 (1<<21)
> #define BCE_LINK_STATUS_PARTNER_AD_2500HALF	 (1<<22)
>+#define BCE_LINK_STATUS_HEART_BEAT_EXPIRED	 (1<<31)
> 
> #define BCE_DRV_PULSE_MB			0x00000010
> #define BCE_DRV_PULSE_SEQ_MASK			 0x00007fff
>@@ -1248,6 +1249,9 @@ struct status_block {
> 		#define STATUS_ATTN_BITS_GRC_ABORT		(1L<<27)
> 		#define STATUS_ATTN_BITS_PARITY_ERROR		(1L<<31)
> 
>+		#define STATUS_ATTN_EVENTS (STATUS_ATTN_BITS_LINK_STATE | \
>+			STATUS_ATTN_BITS_TIMER_ABORT)
>+
> 	u32 status_attn_bits_ack;
> #if defined(__BIG_ENDIAN)
> 	u16 status_tx_quick_consumer_index0;

Actions: View | Diff

Attachments on bug 191786: 144650