Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/sys/dev/usb adjust axe(4) similar to recent axen(4)/cdce(4)/...



details:   https://anonhg.NetBSD.org/src/rev/d9b542a60fac
branches:  trunk
changeset: 457726:d9b542a60fac
user:      mrg <mrg%NetBSD.org@localhost>
date:      Mon Jul 15 06:40:21 2019 +0000

description:
adjust axe(4) similar to recent axen(4)/cdce(4)/ure(4) updates
(which were in turn partly based upon smsc(4) changes):
- mark network interface MPSAFE, and use MPSAFE calls
- convert to local tick task doing watchdog timeout
- add global, tx and rx locks
- add ratelimited tx error message
- split many functions into locked/unlocked version
- use more const
- fix some comments
- remove spl
- don't bother with OACTIVE and do it all internally (axe_tx_cnt)

additional changes here:
- use axe_stop() to abort pipes in detach

fixes a crash potential i only saw when almost finished debugging
these changes..

diffstat:

 sys/dev/usb/if_axe.c |  451 ++++++++++++++++++++++++++++++++++----------------
 1 files changed, 307 insertions(+), 144 deletions(-)

diffs (truncated from 963 to 300 lines):

diff -r f7f220951e3f -r d9b542a60fac sys/dev/usb/if_axe.c
--- a/sys/dev/usb/if_axe.c      Mon Jul 15 05:41:16 2019 +0000
+++ b/sys/dev/usb/if_axe.c      Mon Jul 15 06:40:21 2019 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: if_axe.c,v 1.99 2019/07/14 21:37:09 mrg Exp $  */
+/*     $NetBSD: if_axe.c,v 1.100 2019/07/15 06:40:21 mrg Exp $ */
 /*     $OpenBSD: if_axe.c,v 1.137 2016/04/13 11:03:37 mpi Exp $ */
 
 /*
@@ -87,7 +87,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_axe.c,v 1.99 2019/07/14 21:37:09 mrg Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_axe.c,v 1.100 2019/07/15 06:40:21 mrg Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_inet.h"
@@ -161,6 +161,7 @@
 
        uint16_t                axe_vendor;
        uint16_t                axe_product;
+       uint16_t                axe_timer;
        uint32_t                axe_flags;      /* copied from axe_type */
 #define AX178          __BIT(0)        /* AX88178 */
 #define AX772          __BIT(1)        /* AX88772 */
@@ -174,17 +175,22 @@
        int                     axe_if_flags;
        int                     axe_phyno;
        struct axe_cdata        axe_cdata;
-       struct callout axe_stat_ch;
+       struct callout          axe_stat_ch;
 
        uint8_t                 axe_enaddr[ETHER_ADDR_LEN];
 
        int                     axe_refcnt;
        bool                    axe_dying;
+       bool                    axe_stopping;
        bool                    axe_attached;
 
        struct usb_task         axe_tick_task;
 
+       kmutex_t                axe_lock;
        kmutex_t                axe_mii_lock;
+       kmutex_t                axe_rxlock;
+       kmutex_t                axe_txlock;
+       kcondvar_t              axe_detachcv;
 
        int                     axe_link;
 
@@ -194,6 +200,7 @@
        uint16_t                sc_lenmask;
 
        struct timeval          axe_rx_notice;
+       struct timeval          axe_tx_notice;
        int                     axe_bufsz;
 
 #define sc_if  axe_ec.ec_if
@@ -342,19 +349,23 @@
 static void    axe_tick(void *);
 static void    axe_tick_task(void *);
 static void    axe_start(struct ifnet *);
+static void    axe_start_locked(struct ifnet *);
 static int     axe_ioctl(struct ifnet *, u_long, void *);
 static int     axe_init(struct ifnet *);
+static int     axe_init_locked(struct ifnet *);
 static void    axe_stop(struct ifnet *, int);
+static void    axe_stop_locked(struct ifnet *, int);
 static void    axe_watchdog(struct ifnet *);
+static int     axe_miibus_readreg(device_t, int, int, uint16_t *);
 static int     axe_miibus_readreg_locked(device_t, int, int, uint16_t *);
-static int     axe_miibus_readreg(device_t, int, int, uint16_t *);
+static int     axe_miibus_writereg(device_t, int, int, uint16_t);
 static int     axe_miibus_writereg_locked(device_t, int, int, uint16_t);
-static int     axe_miibus_writereg(device_t, int, int, uint16_t);
 static void    axe_miibus_statchg(struct ifnet *);
 static int     axe_cmd(struct axe_softc *, int, int, int, void *);
 static void    axe_reset(struct axe_softc *);
 
 static void    axe_setmulti(struct axe_softc *);
+static void    axe_setmulti_locked(struct axe_softc *);
 static void    axe_lock_mii(struct axe_softc *);
 static void    axe_unlock_mii(struct axe_softc *);
 
@@ -368,6 +379,18 @@
 axe_lock_mii(struct axe_softc *sc)
 {
 
+       mutex_enter(&sc->axe_lock);
+       sc->axe_refcnt++;
+       mutex_exit(&sc->axe_lock);
+
+       mutex_enter(&sc->axe_mii_lock);
+}
+
+static void
+axe_lock_mii_sc_locked(struct axe_softc *sc)
+{
+       KASSERT(mutex_owned(&sc->axe_lock));
+
        sc->axe_refcnt++;
        mutex_enter(&sc->axe_mii_lock);
 }
@@ -377,8 +400,20 @@
 {
 
        mutex_exit(&sc->axe_mii_lock);
+       mutex_enter(&sc->axe_lock);
        if (--sc->axe_refcnt < 0)
-               usb_detach_wakeupold((sc->axe_dev));
+               cv_broadcast(&sc->axe_detachcv);
+       mutex_exit(&sc->axe_lock);
+}
+
+static void
+axe_unlock_mii_sc_locked(struct axe_softc *sc)
+{
+       KASSERT(mutex_owned(&sc->axe_lock));
+
+       mutex_exit(&sc->axe_mii_lock);
+       if (--sc->axe_refcnt < 0)
+               cv_broadcast(&sc->axe_detachcv);
 }
 
 static int
@@ -421,12 +456,20 @@
        usbd_status err;
        uint16_t data;
 
+       mutex_enter(&sc->axe_lock);
+       if (sc->axe_dying || sc->axe_phyno != phy) {
+               mutex_exit(&sc->axe_lock);
+               return -1;
+       }
+       mutex_exit(&sc->axe_lock);
+
        DPRINTFN(30, "phy 0x%jx reg 0x%jx\n", phy, reg, 0, 0);
 
        axe_cmd(sc, AXE_CMD_MII_OPMODE_SW, 0, 0, NULL);
 
        err = axe_cmd(sc, AXE_CMD_MII_READ_REG, reg, phy, &data);
        axe_cmd(sc, AXE_CMD_MII_OPMODE_HW, 0, 0, NULL);
+
        if (err) {
                aprint_error_dev(sc->axe_dev, "read PHY failed\n");
                return err;
@@ -454,11 +497,12 @@
        struct axe_softc *sc = device_private(dev);
        int rv;
 
-       if (sc->axe_dying)
+       mutex_enter(&sc->axe_lock);
+       if (sc->axe_dying || sc->axe_phyno != phy) {
+               mutex_exit(&sc->axe_lock);
                return -1;
-
-       if (sc->axe_phyno != phy)
-               return -1;
+       }
+       mutex_exit(&sc->axe_lock);
 
        axe_lock_mii(sc);
        rv = axe_miibus_readreg_locked(dev, phy, reg, val);
@@ -494,11 +538,12 @@
        struct axe_softc *sc = device_private(dev);
        int rv;
 
-       if (sc->axe_dying)
+       mutex_enter(&sc->axe_lock);
+       if (sc->axe_dying || sc->axe_phyno != phy) {
+               mutex_exit(&sc->axe_lock);
                return -1;
-
-       if (sc->axe_phyno != phy)
-               return -1;
+       }
+       mutex_exit(&sc->axe_lock);
 
        axe_lock_mii(sc);
        rv = axe_miibus_writereg_locked(dev, phy, reg, aval);
@@ -512,10 +557,13 @@
 {
        AXEHIST_FUNC(); AXEHIST_CALLED();
 
-       struct axe_softc *sc = ifp->if_softc;
+       struct axe_softc * const sc = ifp->if_softc;
        struct mii_data *mii = &sc->axe_mii;
        int val, err;
 
+       if (sc->axe_dying)
+               return;
+
        val = 0;
        if ((IFM_OPTIONS(mii->mii_media_active) & IFM_FDX) != 0) {
                val |= AXE_MEDIA_FULL_DUPLEX;
@@ -556,7 +604,7 @@
 }
 
 static void
-axe_setmulti(struct axe_softc *sc)
+axe_setmulti_locked(struct axe_softc *sc)
 {
        AXEHIST_FUNC(); AXEHIST_CALLED();
        struct ethercom *ec = &sc->axe_ec;
@@ -567,12 +615,12 @@
        uint16_t rxmode;
        uint8_t hashtbl[8] = { 0, 0, 0, 0, 0, 0, 0, 0 };
 
+       KASSERT(mutex_owned(&sc->axe_mii_lock));
+
        if (sc->axe_dying)
                return;
 
-       axe_lock_mii(sc);
        if (axe_cmd(sc, AXE_CMD_RXCTL_READ, 0, 0, &rxmode)) {
-               axe_unlock_mii(sc);
                aprint_error_dev(sc->axe_dev, "can't read rxmode");
                return;
        }
@@ -611,13 +659,19 @@
 
        axe_cmd(sc, AXE_CMD_WRITE_MCAST, 0, 0, hashtbl);
        axe_cmd(sc, AXE_CMD_RXCTL_WRITE, 0, rxmode, NULL);
-       axe_unlock_mii(sc);
        return;
 
  allmulti:
        ifp->if_flags |= IFF_ALLMULTI;
        rxmode |= AXE_RXCMD_ALLMULTI;
        axe_cmd(sc, AXE_CMD_RXCTL_WRITE, 0, rxmode, NULL);
+}
+
+static void
+axe_setmulti(struct axe_softc *sc)
+{
+       axe_lock_mii(sc);
+       axe_setmulti_locked(sc);
        axe_unlock_mii(sc);
 }
 
@@ -655,7 +709,7 @@
 
        /*
         * softnet_lock can be taken when NET_MPAFE is not defined when calling
-        * if_addr_init -> if_init.  This doesn't mixe well with the
+        * if_addr_init -> if_init.  This doesn't mix well with the
         * usbd_delay_ms calls in the init routines as things like nd6_slowtimo
         * can fire during the wait and attempt to take softnet_lock and then
         * block the softclk thread meaing the wait never ends.
@@ -997,7 +1051,7 @@
        char *devinfop;
        const char *devname = device_xname(self);
        struct ifnet *ifp;
-       int i, s;
+       int i;
 
        aprint_naive("\n");
        aprint_normal("\n");
@@ -1018,8 +1072,7 @@
 
        sc->axe_flags = axe_lookup(uaa->uaa_vendor, uaa->uaa_product)->axe_flags;
 
-       mutex_init(&sc->axe_mii_lock, MUTEX_DEFAULT, IPL_NONE);
-       usb_init_task(&sc->axe_tick_task, axe_tick_task, sc, 0);
+       usb_init_task(&sc->axe_tick_task, axe_tick_task, sc, USB_TASKQ_MPSAFE);
 
        err = usbd_device2interface_handle(dev, AXE_IFACE_IDX, &sc->axe_iface);
        if (err) {
@@ -1064,12 +1117,24 @@
                }
        }
 
-       s = splnet();
+       /* Set these up now for axe_cmd().  */
+       mutex_init(&sc->axe_mii_lock, MUTEX_DEFAULT, IPL_NONE);
+       mutex_init(&sc->axe_txlock, MUTEX_DEFAULT, IPL_SOFTUSB);
+       mutex_init(&sc->axe_rxlock, MUTEX_DEFAULT, IPL_SOFTUSB);
+       mutex_init(&sc->axe_lock, MUTEX_DEFAULT, IPL_NONE);
+       cv_init(&sc->axe_detachcv, "axedet");
 
        /* We need the PHYID for init dance in some cases */
        axe_lock_mii(sc);
        if (axe_cmd(sc, AXE_CMD_READ_PHYID, 0, 0, &sc->axe_phyaddrs)) {
                aprint_error_dev(self, "failed to read phyaddrs\n");
+
+               cv_destroy(&sc->axe_detachcv);
+               mutex_destroy(&sc->axe_lock);
+               mutex_destroy(&sc->axe_rxlock);
+               mutex_destroy(&sc->axe_txlock);
+               mutex_destroy(&sc->axe_mii_lock);
+
                return;
        }
 
@@ -1099,6 +1164,13 @@
        } else {
                if (axe_cmd(sc, AXE_CMD_READ_IPG012, 0, 0, sc->axe_ipgs)) {
                        aprint_error_dev(self, "failed to read ipg\n");
+
+                       cv_destroy(&sc->axe_detachcv);



Home | Main Index | Thread Index | Old Index