DPAA2: properly set link state on startup

Factor out the link-state change tracking from media_change into a new function dpaa2_ni_miibus_statchg(). There use mii information for the link state as ifp->if_linkstate only gets updated after other mii functions are called and thus lacks behind. Add a new bus function to memac_mdio so we can set the dpni interface and call MIIBUS_STATCHG on the dpni and pass that through to dpaa2_ni_miibus_statchg(). It is a bit of a weird setup with two parallel device tress off ACPI as ideally memac_mdio0 would be a child of dpaa2_ni0. We will see how this will work with FDT at some point but at least the current way of doing is flexible enough. Currently we need to make sure MC0 can discover the memac_mdio via the ACPI reference upon attach and have a way for DPNI to query that. The new bus function now allows us to set a back-pointer in the other direction from dpaa2_ni_setup(). nexus0 acpi0 dpaa2_mc0 dpaa2_rc0 dpaa2_ni0 memac_mdio0 memacphy0 miibus1 atphy0 memac_mdio1 Most importantly and not to be missed at the end of dpaa2_ni_init() make sure we initialize the link state as otherwise we will not see any interrupts. That can be observed by a hanging NFS Root mount for example, whereas with the previous code ifconfig would trigger a SIOCGIFMEDIA call (from multi-user at some point) which then indirectly and more by accident set the the MAC LINK STATE via DPAA2_CMD_MAC_SET_LINK_STATE() and got the interface started.
mcusim · Jun 15, 2022 · 4529394 · 4529394
1 parent 1e28980
commit 4529394
Show file tree

Hide file tree

Showing 5 changed files with 158 additions and 71 deletions.
diff --git a/sys/conf/files.arm64 b/sys/conf/files.arm64
@@ -337,6 +337,7 @@ dev/dpaa2/dpaa2_mac.c				optional SOC_NXP_LS dpaa2
 dev/dpaa2/dpaa2_con.c				optional SOC_NXP_LS dpaa2
 dev/dpaa2/dpaa2_mc_acpi.c			optional SOC_NXP_LS dpaa2 acpi
 dev/dpaa2/dpaa2_mc_fdt.c			optional SOC_NXP_LS dpaa2 fdt
+dev/dpaa2/memac_mdio_if.m			optional SOC_NXP_LS dpaa2 acpi | SOC_NXP_LS dpaa2 fdt
 dev/dpaa2/memac_mdio_acpi.c			optional SOC_NXP_LS dpaa2 acpi
 
 ##

diff --git a/sys/dev/dpaa2/dpaa2_ni.c b/sys/dev/dpaa2/dpaa2_ni.c
@@ -80,6 +80,7 @@ __FBSDID("$FreeBSD$");
 #include "pci_if.h"
 #include "miibus_if.h"
 #include "mdio_if.h"
+#include "memac_mdio_if.h"
 
 #include "dpaa2_mc.h"
 #include "dpaa2_mc_if.h"
@@ -451,6 +452,7 @@ static u_int dpaa2_ni_add_maddr(void *, struct sockaddr_dl *, u_int);
 static void dpaa2_ni_intr(void *);
 
 /* MII handlers */
+static void dpaa2_ni_miibus_statchg(device_t);
 static int  dpaa2_ni_media_change(struct ifnet *);
 static void dpaa2_ni_media_status(struct ifnet *, struct ifmediareq *);
 static void dpaa2_ni_media_tick(void *);
@@ -862,21 +864,31 @@ dpaa2_ni_setup(device_t dev)
 				    "mode\n");
 				error = DPAA2_MC_GET_PHY_DEV(dev,
 				    &sc->mac.phy_dev, sc->mac.dpmac_id);
+				if (error == 0) {
+					error = MEMAC_MDIO_SET_NI_DEV(
+					    sc->mac.phy_dev, dev);
+					if (error != 0)
+						device_printf(dev, "%s: failed "
+						    "to set dpni dev on memac "
+						    "mdio dev %s: error=%d\n",
+						    __func__,
+						    device_get_nameunit(
+						    sc->mac.phy_dev), error);
+				}
 				if (error == 0) {
 					error = mii_attach(sc->mac.phy_dev,
 					    &sc->miibus, sc->ifp,
 					    dpaa2_ni_media_change,
 					    dpaa2_ni_media_status,
 					    BMSR_DEFCAPMASK, MII_PHY_ANY, 0, 0);
+					if (error != 0)
+						device_printf(dev, "%s: failed "
+						    "to attach to miibus: "
+						    "error=%d\n",
+						    __func__, error);
 				}
-
-				if (error != 0) {
-					device_printf(dev, "%s: failed to "
-					    "attach miibus: error=%d\n",
-					    __func__, error);
-				} else {
+				if (error == 0)
 					sc->mii = device_get_softc(sc->miibus);
-				}
 			} else {
 				device_printf(dev, "%s: DPMAC link type is not "
 				    "supported\n", __func__);
@@ -2142,47 +2154,29 @@ dpaa2_ni_set_mac_addr(device_t dev, struct dpaa2_cmd *cmd, uint16_t rc_token,
 	return (0);
 }
 
-/**
- * @brief Callback function to process media change request.
- */
-static int
-dpaa2_ni_media_change(struct ifnet *ifp)
-{
-	struct dpaa2_ni_softc *sc = ifp->if_softc;
-
-	DPNI_LOCK(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",
-		    __func__);
-	}
-	DPNI_UNLOCK(sc);
-
-	return (0);
-}
-
-/**
- * @brief Callback function to process media status request.
- */
 static void
-dpaa2_ni_media_status(struct ifnet *ifp, struct ifmediareq *ifmr)
+dpaa2_ni_miibus_statchg(device_t dev)
 {
-	struct dpaa2_ni_softc *sc = ifp->if_softc;
-	struct dpaa2_mac_link_state mac_link = {0};
-	device_t child = sc->dev;
+	struct dpaa2_ni_softc *sc;
+	device_t child;
+	struct dpaa2_mac_link_state mac_link = { 0 };
 	uint16_t mac_token;
-	int link_state = ifp->if_link_state;
-	int error;
+	int error, link_state;
 
-	DPNI_LOCK(sc);
-	if (sc->mii) {
-		mii_pollstat(sc->mii);
-		ifmr->ifm_active = sc->mii->mii_media_active;
-		ifmr->ifm_status = sc->mii->mii_media_status;
-	}
-	DPNI_UNLOCK(sc);
+	sc = device_get_softc(dev);
+	child = sc->dev;
+
+	/*
+	 * Note: ifp link state will only be changed AFTER we are called so we
+	 * cannot rely on ifp->if_linkstate here.
+	 */
+	if (sc->mii->mii_media_status & IFM_AVALID) {
+		if (sc->mii->mii_media_status & IFM_ACTIVE)
+			link_state = LINK_STATE_UP;
+		else
+			link_state = LINK_STATE_DOWN;
+	} else
+		link_state = LINK_STATE_UNKNOWN;
 
 	if (link_state != sc->link_state) {
 		sc->link_state = link_state;
@@ -2193,19 +2187,19 @@ dpaa2_ni_media_status(struct ifnet *ifp, struct ifmediareq *ifmr)
 			device_printf(sc->dev, "%s: failed to open DPMAC: "
 			    "id=%d, error=%d\n", __func__, sc->mac.dpmac_id,
 			    error);
-			goto err_exit;
+			return;
 		}
 
 		if (link_state == LINK_STATE_UP ||
 		    link_state == LINK_STATE_DOWN) {
 			/* Update DPMAC link state. */
 			mac_link.supported = sc->mii->mii_media.ifm_media;
 			mac_link.advert = sc->mii->mii_media.ifm_media;
-			mac_link.rate = 1000; /* TODO: Where to get from? */
+			mac_link.rate = 1000; /* TODO: Where to get from? */	/* ifmedia_baudrate? */
 			mac_link.options =
 			    DPAA2_MAC_LINK_OPT_AUTONEG |
 			    DPAA2_MAC_LINK_OPT_PAUSE;
-			mac_link.up = link_state == LINK_STATE_UP ? true : false;
+			mac_link.up = (link_state == LINK_STATE_UP) ? true : false;
 			mac_link.state_valid = true;
 
 			/* Inform DPMAC about link state. */
@@ -2223,10 +2217,46 @@ dpaa2_ni_media_status(struct ifnet *ifp, struct ifmediareq *ifmr)
 	}
 	return;
 
- err_close_mac:
+err_close_mac:
 	DPAA2_CMD_MAC_CLOSE(sc->dev, child, dpaa2_mcp_tk(sc->cmd, mac_token));
- err_exit:
-	return;
+}
+
+/**
+ * @brief Callback function to process media change request.
+ */
+static int
+dpaa2_ni_media_change(struct ifnet *ifp)
+{
+	struct dpaa2_ni_softc *sc = ifp->if_softc;
+
+	DPNI_LOCK(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",
+		    __func__);
+	}
+	DPNI_UNLOCK(sc);
+
+	return (0);
+}
+
+/**
+ * @brief Callback function to process media status request.
+ */
+static void
+dpaa2_ni_media_status(struct ifnet *ifp, struct ifmediareq *ifmr)
+{
+	struct dpaa2_ni_softc *sc = ifp->if_softc;
+
+	DPNI_LOCK(sc);
+	if (sc->mii) {
+		mii_pollstat(sc->mii);
+		ifmr->ifm_active = sc->mii->mii_media_active;
+		ifmr->ifm_status = sc->mii->mii_media_status;
+	}
+	DPNI_UNLOCK(sc);
 }
 
 /**
@@ -2282,6 +2312,9 @@ dpaa2_ni_init(void *arg)
 	ifp->if_drv_flags &= ~IFF_DRV_OACTIVE;
 	DPNI_UNLOCK(sc);
 
+	/* Force link-state update to initilize things. */
+	dpaa2_ni_miibus_statchg(dev);
+
 	return;
 }
 
@@ -3508,6 +3541,9 @@ static device_method_t dpaa2_ni_methods[] = {
 	DEVMETHOD(device_attach,	dpaa2_ni_attach),
 	DEVMETHOD(device_detach,	dpaa2_ni_detach),
 
+	/* mii via memac_mdio */
+	DEVMETHOD(miibus_statchg,	dpaa2_ni_miibus_statchg),
+
 	DEVMETHOD_END
 };
 
@@ -3521,3 +3557,4 @@ DRIVER_MODULE(miibus, dpaa2_ni, miibus_driver, 0, 0);
 DRIVER_MODULE(dpaa2_ni, dpaa2_rc, dpaa2_ni_driver, 0, 0);
 
 MODULE_DEPEND(dpaa2_ni, miibus, 1, 1, 1);
+MODULE_DEPEND(dpaa2_ni, memac_mdio, 1, 1, 1);
diff --git a/sys/dev/dpaa2/memac_mdio_acpi.c b/sys/dev/dpaa2/memac_mdio_acpi.c
@@ -1,7 +1,7 @@
 /*-
  * SPDX-License-Identifier: BSD-2-Clause
  *
- * Copyright (c) 2021 Bjoern A. Zeeb
+ * Copyright (c) 2021-2022 Bjoern A. Zeeb
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -50,15 +50,17 @@ __FBSDID("$FreeBSD$");
 #include <dev/mii/mii.h>
 #include <dev/mii/miivar.h>
 
+#include "memac_mdio_if.h"
 #include "acpi_bus_if.h"
 #include "miibus_if.h"
 
 /* -------------------------------------------------------------------------- */
 
 struct memacphy_softc {
-	int			 uid;
-	uint64_t		 phy_channel;
-	char			 compatible[64];
+	int			uid;
+	uint64_t		phy_channel;
+	char			compatible[64];
+	device_t		dpnidev;
 };
 
 static int
@@ -100,26 +102,40 @@ memacphy_acpi_attach(device_t dev)
 static int
 memacphy_miibus_readreg(device_t dev, int phy, int reg)
 {
-#if 0
-	device_printf(dev, "phy read %d:%d\n", phy, reg);
-#endif
+
 	return (MIIBUS_READREG(device_get_parent(dev), phy, reg));
 }
 
 static int
 memacphy_miibus_writereg(device_t dev, int phy, int reg, int data)
 {
-#if 0
-	device_printf(dev, "phy write %d:%d\n", phy, reg);
-#endif
+
 	return (MIIBUS_WRITEREG(device_get_parent(dev), phy, reg, data));
 }
 
 static void
 memacphy_miibus_statchg(device_t dev)
 {
-	device_printf(dev, "statchg\n");
-	MIIBUS_STATCHG(device_get_parent(dev));
+	struct memacphy_softc *sc;
+
+	sc = device_get_softc(dev);
+
+	if (sc->dpnidev != NULL)
+		MIIBUS_STATCHG(sc->dpnidev);
+}
+
+static int
+memac_mdio_set_ni_dev(device_t dev, device_t nidev)
+{
+	struct memacphy_softc *sc;
+
+	sc = device_get_softc(dev);
+
+	if (sc->dpnidev != NULL)
+		return (EBUSY);
+
+	sc->dpnidev = nidev;
+	return (0);
 }
 
 static device_method_t memacphy_acpi_methods[] = {
@@ -133,6 +149,9 @@ static device_method_t memacphy_acpi_methods[] = {
 	DEVMETHOD(miibus_writereg,	memacphy_miibus_writereg),
 	DEVMETHOD(miibus_statchg,	memacphy_miibus_statchg),
 
+	/* memac */
+	DEVMETHOD(memac_mdio_set_ni_dev, memac_mdio_set_ni_dev),
+
 	DEVMETHOD_END
 };
 
@@ -403,12 +422,6 @@ memac_miibus_writereg(device_t dev, int phy, int reg, int data)
         return (0);
 }
 
-static void
-memac_miibus_statchg(device_t dev)
-{
-	device_printf(dev, "statchg\n");
-}
-
 static ssize_t
 memac_mdio_get_property(device_t dev, device_t child, const char *propname,
     void *propvalue, size_t size, device_property_type_t type)
@@ -424,7 +437,6 @@ memac_mdio_acpi_read_ivar(device_t dev, device_t child, int index, uintptr_t *re
 	return (acpi_read_ivar(dev, child, index, result));
 }
 
-
 static device_method_t memac_mdio_acpi_methods[] = {
 	/* Device interface */
 	DEVMETHOD(device_probe,		memac_mdio_acpi_probe),
@@ -434,7 +446,6 @@ static device_method_t memac_mdio_acpi_methods[] = {
 	/* MII interface */
 	DEVMETHOD(miibus_readreg,	memac_miibus_readreg),
 	DEVMETHOD(miibus_writereg,	memac_miibus_writereg),
-	DEVMETHOD(miibus_statchg,	memac_miibus_statchg),
 
 	/* .. */
 	DEVMETHOD(bus_add_child,	bus_generic_add_child),

diff --git a/sys/dev/dpaa2/memac_mdio_if.m b/sys/dev/dpaa2/memac_mdio_if.m
@@ -0,0 +1,37 @@
+#-
+# SPDX-License-Identifier: BSD-2-Clause
+#
+# Copyright (c) 2022, Bjoern A. Zeeb
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions
+# are met:
+# 1. Redistributions of source code must retain the above copyright
+#    notice, this list of conditions and the following disclaimer.
+# 2. Redistributions in binary form must reproduce the above copyright
+#    notice, this list of conditions and the following disclaimer in the
+#    documentation and/or other materials provided with the distribution.
+#
+# THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
+# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+# ARE DISCLAIMED.  IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
+# FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+# DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
+# OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+# HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+# LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
+# OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+# SUCH DAMAGE.
+#
+# $FreeBSD$
+#
+
+#include <machine/bus.h>
+
+INTERFACE memac_mdio;
+
+METHOD int set_ni_dev {
+	device_t		 dev;
+	device_t		 nidev;
+};
diff --git a/sys/modules/dpaa2/Makefile b/sys/modules/dpaa2/Makefile
@@ -38,6 +38,7 @@ SRCS+=	dpaa2_mc_fdt.c \
 
 MFILES=	dev/dpaa2/dpaa2_cmd_if.m \
 	dev/dpaa2/dpaa2_swp_if.m \
-	dev/dpaa2/dpaa2_mc_if.m
+	dev/dpaa2/dpaa2_mc_if.m \
+	dev/dpaa2/memac_mdio_if.m
 
 .include <bsd.kmod.mk>