From 452939456bf79e68bef92becbe7b145b43764a9f Mon Sep 17 00:00:00 2001 From: "Bjoern A. Zeeb" Date: Tue, 14 Jun 2022 10:07:15 +0000 Subject: [PATCH] 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. --- sys/conf/files.arm64 | 1 + sys/dev/dpaa2/dpaa2_ni.c | 137 ++++++++++++++++++++------------ sys/dev/dpaa2/memac_mdio_acpi.c | 51 +++++++----- sys/dev/dpaa2/memac_mdio_if.m | 37 +++++++++ sys/modules/dpaa2/Makefile | 3 +- 5 files changed, 158 insertions(+), 71 deletions(-) create mode 100644 sys/dev/dpaa2/memac_mdio_if.m diff --git a/sys/conf/files.arm64 b/sys/conf/files.arm64 index f1750744798b97..d470e17756e6f4 100644 --- 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 index c90a120c4abcfb..49a2fbc61ecf72 100644 --- 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,7 +2187,7 @@ 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 || @@ -2201,11 +2195,11 @@ dpaa2_ni_media_status(struct ifnet *ifp, struct ifmediareq *ifmr) /* 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 index db79b0228292c5..63dd52f8c5c245 100644 --- 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 #include +#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 new file mode 100644 index 00000000000000..9186940aa2c2af --- /dev/null +++ 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 + +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 index 7f5fe93f8bc74c..57cc2cfee83b55 100644 --- 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