The branch main has been updated by bz: URL: https://cgit.FreeBSD.org/src/commit/?id=964b3408fa872178aacf58f2d84dc43564ec0aa7
commit 964b3408fa872178aacf58f2d84dc43564ec0aa7 Author: Bjoern A. Zeeb <[email protected]> AuthorDate: 2023-11-17 00:47:11 +0000 Commit: Bjoern A. Zeeb <[email protected]> CommitDate: 2023-11-17 12:20:03 +0000 dpaa2: defer link_state updates until we are up dpaa2_ni_media_change() was called in early setup stages, before we were fully setup. That lead to internal driver state being all synched and fine but hardware state was lost/never setup corrently. Introduce dpaa2_ni_media_change_locked() so we can avoid reccursive locking and call "dpaa2_ni_media_change()" instead of mii_mediachg() as the latter does not setup our state there either. In order for this all to work, call if_setdrvflagbits() just before rather than after the above. Also remove an unecessary direct call to dpaa2_ni_miibus_statchg() which mii_mediachg() will trigger anyway. This all fixes a problem [1] that one had to lose the link (either unplugging/replugging the cable or using ifconfig media none; ifconfig media auto) to re-trigger the all updates and get the full state programmed when hardware expected. MFC after: 3 days GH-Issue: https://github.com/mcusim/freebsd-src/issues/21 [1] Reviewed by: dsl, dch Differential Revision: https://reviews.freebsd.org/D42643 --- sys/dev/dpaa2/dpaa2_ni.c | 46 ++++++++++++++++++++++++++++++++++++---------- 1 file changed, 36 insertions(+), 10 deletions(-) diff --git a/sys/dev/dpaa2/dpaa2_ni.c b/sys/dev/dpaa2/dpaa2_ni.c index b274d5670d5f..a9e6aa120549 100644 --- a/sys/dev/dpaa2/dpaa2_ni.c +++ b/sys/dev/dpaa2/dpaa2_ni.c @@ -116,6 +116,9 @@ mtx_assert(&(__sc)->lock, MA_OWNED); \ mtx_unlock(&(__sc)->lock); \ } while (0) +#define DPNI_LOCK_ASSERT(__sc) do { \ + mtx_assert(&(__sc)->lock, MA_OWNED); \ +} while (0) #define DPAA2_TX_RING(sc, chan, tc) \ (&(sc)->channels[(chan)]->txc_queue.tx_rings[(tc)]) @@ -2269,6 +2272,16 @@ dpaa2_ni_miibus_statchg(device_t dev) if (sc->fixed_link || sc->mii == NULL) { return; } + if ((if_getdrvflags(sc->ifp) & IFF_DRV_RUNNING) == 0) { + /* + * We will receive calls and adjust the changes but + * not have setup everything (called before dpaa2_ni_init() + * really). This will then setup the link and internal + * sc->link_state and not trigger the update once needed, + * so basically dpmac never knows about it. + */ + return; + } /* * Note: ifp link state will only be changed AFTER we are called so we @@ -2344,23 +2357,33 @@ err_exit: * @brief Callback function to process media change request. */ static int -dpaa2_ni_media_change(if_t ifp) +dpaa2_ni_media_change_locked(struct dpaa2_ni_softc *sc) { - struct dpaa2_ni_softc *sc = if_getsoftc(ifp); - DPNI_LOCK(sc); + DPNI_LOCK_ASSERT(sc); if (sc->mii) { mii_mediachg(sc->mii); sc->media_status = sc->mii->mii_media.ifm_media; } else if (sc->fixed_link) { - if_printf(ifp, "%s: can't change media in fixed mode\n", + if_printf(sc->ifp, "%s: can't change media in fixed mode\n", __func__); } - DPNI_UNLOCK(sc); return (0); } +static int +dpaa2_ni_media_change(if_t ifp) +{ + struct dpaa2_ni_softc *sc = if_getsoftc(ifp); + int error; + + DPNI_LOCK(sc); + error = dpaa2_ni_media_change_locked(sc); + DPNI_UNLOCK(sc); + return (error); +} + /** * @brief Callback function to process media status request. */ @@ -2443,17 +2466,20 @@ dpaa2_ni_init(void *arg) } DPNI_LOCK(sc); + /* Announce we are up and running and can queue packets. */ + if_setdrvflagbits(ifp, IFF_DRV_RUNNING, IFF_DRV_OACTIVE); + if (sc->mii) { - mii_mediachg(sc->mii); + /* + * mii_mediachg() will trigger a call into + * dpaa2_ni_miibus_statchg() to setup link state. + */ + dpaa2_ni_media_change_locked(sc); } callout_reset(&sc->mii_callout, hz, dpaa2_ni_media_tick, sc); - if_setdrvflagbits(ifp, IFF_DRV_RUNNING, IFF_DRV_OACTIVE); DPNI_UNLOCK(sc); - /* Force link-state update to initilize things. */ - dpaa2_ni_miibus_statchg(dev); - (void)DPAA2_CMD_NI_CLOSE(dev, child, DPAA2_CMD_TK(&cmd, ni_token)); (void)DPAA2_CMD_RC_CLOSE(dev, child, DPAA2_CMD_TK(&cmd, rc_token)); return;
