Author: yongari
Date: Fri Jan  8 21:15:09 2010
New Revision: 201823
URL: http://svn.freebsd.org/changeset/base/201823

Log:
  MFC r198987,199414,199543,200422
  
  Partial merge r198987:
    Use device_printf() and if_printf() instead of printf() with an explicit
    unit number and remove 'unit' members from softc.
  
  Partial merge r199414:
    Use the bus_*() routines rather than bus_space_*() for register operations.
  
  r199543:
    Several fixes to this driver:
    - Overhaul the locking to avoid recursion and add missing locking in a few
      places.
    - Don't schedule a task to call vge_start() from contexts that are safe to
      call vge_start() directly.  Just invoke the routine directly instead
      (this is what all of the other NIC drivers I am familiar with do).  Note
      that vge(4) does not use an interrupt filter handler which is the primary
      reason some other drivers use tasks.
    - Add a new private timer to drive the watchdog timer instead of using
      if_watchdog and if_timer.
    - Fixup detach by calling ether_ifdetach() before stopping the interface.
  
  r200422:
    Remove driver lock assertion in MII register access. This change
    was made in r199543 to remove MTX_RECURSE. These routines can be
    called in device attach phase(e.g. mii_phy_probe()) so checking
    assertion here is not right as caller does not hold a driver lock.

Modified:
  stable/8/sys/dev/vge/if_vge.c
  stable/8/sys/dev/vge/if_vgevar.h
Directory Properties:
  stable/8/sys/   (props changed)
  stable/8/sys/amd64/include/xen/   (props changed)
  stable/8/sys/cddl/contrib/opensolaris/   (props changed)
  stable/8/sys/contrib/dev/acpica/   (props changed)
  stable/8/sys/contrib/pf/   (props changed)
  stable/8/sys/dev/xen/xenpci/   (props changed)

Modified: stable/8/sys/dev/vge/if_vge.c
==============================================================================
--- stable/8/sys/dev/vge/if_vge.c       Fri Jan  8 21:02:12 2010        
(r201822)
+++ stable/8/sys/dev/vge/if_vge.c       Fri Jan  8 21:15:09 2010        
(r201823)
@@ -93,7 +93,6 @@ __FBSDID("$FreeBSD$");
 #include <sys/module.h>
 #include <sys/kernel.h>
 #include <sys/socket.h>
-#include <sys/taskqueue.h>
 
 #include <net/if.h>
 #include <net/if_arp.h>
@@ -160,12 +159,13 @@ static int vge_rxeof              (struct vge_softc 
 static void vge_txeof          (struct vge_softc *);
 static void vge_intr           (void *);
 static void vge_tick           (void *);
-static void vge_tx_task                (void *, int);
 static void vge_start          (struct ifnet *);
+static void vge_start_locked   (struct ifnet *);
 static int vge_ioctl           (struct ifnet *, u_long, caddr_t);
 static void vge_init           (void *);
+static void vge_init_locked    (struct vge_softc *);
 static void vge_stop           (struct vge_softc *);
-static void vge_watchdog       (struct ifnet *);
+static void vge_watchdog       (void *);
 static int vge_suspend         (device_t);
 static int vge_resume          (device_t);
 static int vge_shutdown                (device_t);
@@ -378,7 +378,6 @@ vge_miibus_readreg(dev, phy, reg)
        if (phy != (CSR_READ_1(sc, VGE_MIICFG) & 0x1F))
                return(0);
 
-       VGE_LOCK(sc);
        vge_miipoll_stop(sc);
 
        /* Specify the register we want to read. */
@@ -400,7 +399,6 @@ vge_miibus_readreg(dev, phy, reg)
                rval = CSR_READ_2(sc, VGE_MIIDATA);
 
        vge_miipoll_start(sc);
-       VGE_UNLOCK(sc);
 
        return (rval);
 }
@@ -418,7 +416,6 @@ vge_miibus_writereg(dev, phy, reg, data)
        if (phy != (CSR_READ_1(sc, VGE_MIICFG) & 0x1F))
                return(0);
 
-       VGE_LOCK(sc);
        vge_miipoll_stop(sc);
 
        /* Specify the register we want to write. */
@@ -443,7 +440,6 @@ vge_miibus_writereg(dev, phy, reg, data)
        }
 
        vge_miipoll_start(sc);
-       VGE_UNLOCK(sc);
 
        return (rval);
 }
@@ -929,7 +925,9 @@ vge_attach(dev)
        sc->vge_dev = dev;
 
        mtx_init(&sc->vge_mtx, device_get_nameunit(dev), MTX_NETWORK_LOCK,
-           MTX_DEF | MTX_RECURSE);
+           MTX_DEF);
+       callout_init_mtx(&sc->vge_watchdog, &sc->vge_mtx, 0);
+
        /*
         * Map control/status registers.
         */
@@ -945,9 +943,6 @@ vge_attach(dev)
                goto fail;
        }
 
-       sc->vge_btag = rman_get_bustag(sc->vge_res);
-       sc->vge_bhandle = rman_get_bushandle(sc->vge_res);
-
        /* Allocate interrupt */
        rid = 0;
        sc->vge_irq = bus_alloc_resource(dev, SYS_RES_IRQ, &rid,
@@ -967,8 +962,6 @@ vge_attach(dev)
         */
        vge_read_eeprom(sc, (caddr_t)eaddr, VGE_EE_EADDR, 3, 0);
 
-       sc->vge_unit = unit;
-
        /*
         * Allocate the parent bus DMA tag appropriate for PCI.
         */
@@ -993,7 +986,7 @@ vge_attach(dev)
 
        ifp = sc->vge_ifp = if_alloc(IFT_ETHER);
        if (ifp == NULL) {
-               printf("vge%d: can not if_alloc()\n", sc->vge_unit);
+               device_printf(dev, "can not if_alloc()\n");
                error = ENOSPC;
                goto fail;
        }
@@ -1001,7 +994,7 @@ vge_attach(dev)
        /* Do MII setup */
        if (mii_phy_probe(dev, &sc->vge_miibus,
            vge_ifmedia_upd, vge_ifmedia_sts)) {
-               printf("vge%d: MII without any phy!\n", sc->vge_unit);
+               device_printf(dev, "MII without any phy!\n");
                error = ENXIO;
                goto fail;
        }
@@ -1019,14 +1012,11 @@ vge_attach(dev)
 #ifdef DEVICE_POLLING
        ifp->if_capabilities |= IFCAP_POLLING;
 #endif
-       ifp->if_watchdog = vge_watchdog;
        ifp->if_init = vge_init;
        IFQ_SET_MAXLEN(&ifp->if_snd, VGE_IFQ_MAXLEN);
        ifp->if_snd.ifq_drv_maxlen = VGE_IFQ_MAXLEN;
        IFQ_SET_READY(&ifp->if_snd);
 
-       TASK_INIT(&sc->vge_txtask, 0, vge_tx_task, ifp);
-
        /*
         * Call MI attach routine.
         */
@@ -1075,21 +1065,11 @@ vge_detach(dev)
 
        /* These should only be active if attach succeeded */
        if (device_is_attached(dev)) {
-               vge_stop(sc);
-               /*
-                * Force off the IFF_UP flag here, in case someone
-                * still had a BPF descriptor attached to this
-                * interface. If they do, ether_ifattach() will cause
-                * the BPF code to try and clear the promisc mode
-                * flag, which will bubble down to vge_ioctl(),
-                * which will try to call vge_init() again. This will
-                * turn the NIC back on and restart the MII ticker,
-                * which will panic the system when the kernel tries
-                * to invoke the vge_tick() function that isn't there
-                * anymore.
-                */
-               ifp->if_flags &= ~IFF_UP;
                ether_ifdetach(ifp);
+               VGE_LOCK(sc);
+               vge_stop(sc);
+               VGE_UNLOCK(sc);
+               callout_drain(&sc->vge_watchdog);
        }
        if (sc->vge_miibus)
                device_delete_child(dev, sc->vge_miibus);
@@ -1528,7 +1508,7 @@ vge_txeof(sc)
        if (idx != sc->vge_ldata.vge_tx_considx) {
                sc->vge_ldata.vge_tx_considx = idx;
                ifp->if_drv_flags &= ~IFF_DRV_OACTIVE;
-               ifp->if_timer = 0;
+               sc->vge_timer = 0;
        }
 
        /*
@@ -1554,7 +1534,7 @@ vge_tick(xsc)
 
        sc = xsc;
        ifp = sc->vge_ifp;
-       VGE_LOCK(sc);
+       VGE_LOCK_ASSERT(sc);
        mii = device_get_softc(sc->vge_miibus);
 
        mii_tick(mii);
@@ -1571,13 +1551,10 @@ vge_tick(xsc)
                        if_link_state_change(sc->vge_ifp,
                            LINK_STATE_UP);
                        if (!IFQ_DRV_IS_EMPTY(&ifp->if_snd))
-                               taskqueue_enqueue(taskqueue_swi,
-                                   &sc->vge_txtask);
+                               vge_start_locked(ifp);
                }
        }
 
-       VGE_UNLOCK(sc);
-
        return;
 }
 
@@ -1597,7 +1574,7 @@ vge_poll (struct ifnet *ifp, enum poll_c
        vge_txeof(sc);
 
        if (!IFQ_DRV_IS_EMPTY(&ifp->if_snd))
-               taskqueue_enqueue(taskqueue_swi, &sc->vge_txtask);
+               vge_start_locked(ifp);
 
        if (cmd == POLL_AND_CHECK_STATUS) { /* also check status register */
                u_int32_t       status;
@@ -1613,7 +1590,7 @@ vge_poll (struct ifnet *ifp, enum poll_c
 
                if (status & VGE_ISR_TXDMA_STALL ||
                    status & VGE_ISR_RXDMA_STALL)
-                       vge_init(sc);
+                       vge_init_locked(sc);
 
                if (status & (VGE_ISR_RXOFLOW|VGE_ISR_RXNODESC)) {
                        vge_rxeof(sc);
@@ -1686,7 +1663,7 @@ vge_intr(arg)
                        vge_txeof(sc);
 
                if (status & (VGE_ISR_TXDMA_STALL|VGE_ISR_RXDMA_STALL))
-                       vge_init(sc);
+                       vge_init_locked(sc);
 
                if (status & VGE_ISR_LINKSTS)
                        vge_tick(sc);
@@ -1695,10 +1672,10 @@ vge_intr(arg)
        /* Re-enable interrupts */
        CSR_WRITE_1(sc, VGE_CRS3, VGE_CR3_INT_GMSK);
 
-       VGE_UNLOCK(sc);
-
        if (!IFQ_DRV_IS_EMPTY(&ifp->if_snd))
-               taskqueue_enqueue(taskqueue_swi, &sc->vge_txtask);
+               vge_start_locked(ifp);
+
+       VGE_UNLOCK(sc);
 
        return;
 }
@@ -1736,8 +1713,7 @@ vge_encap(sc, m_head, idx)
            m_head, vge_dma_map_tx_desc, &arg, BUS_DMA_NOWAIT);
 
        if (error && error != EFBIG) {
-               printf("vge%d: can't map mbuf (error %d)\n",
-                   sc->vge_unit, error);
+               if_printf(sc->vge_ifp, "can't map mbuf (error %d)\n", error);
                return (ENOBUFS);
        }
 
@@ -1758,8 +1734,8 @@ vge_encap(sc, m_head, idx)
                error = bus_dmamap_load_mbuf(sc->vge_ldata.vge_mtag, map,
                    m_head, vge_dma_map_tx_desc, &arg, BUS_DMA_NOWAIT);
                if (error) {
-                       printf("vge%d: can't map mbuf (error %d)\n",
-                           sc->vge_unit, error);
+                       if_printf(sc->vge_ifp, "can't map mbuf (error %d)\n",
+                           error);
                        return (EFBIG);
                }
        }
@@ -1780,19 +1756,6 @@ vge_encap(sc, m_head, idx)
        return (0);
 }
 
-static void
-vge_tx_task(arg, npending)
-       void                    *arg;
-       int                     npending;
-{
-       struct ifnet            *ifp;
-
-       ifp = arg;
-       vge_start(ifp);
-
-       return;
-}
-
 /*
  * Main transmit routine.
  */
@@ -1802,21 +1765,29 @@ vge_start(ifp)
        struct ifnet            *ifp;
 {
        struct vge_softc        *sc;
+
+       sc = ifp->if_softc;
+       VGE_LOCK(sc);
+       vge_start_locked(ifp);
+       VGE_UNLOCK(sc);
+}
+
+static void
+vge_start_locked(ifp)
+       struct ifnet            *ifp;
+{
+       struct vge_softc        *sc;
        struct mbuf             *m_head = NULL;
        int                     idx, pidx = 0;
 
        sc = ifp->if_softc;
-       VGE_LOCK(sc);
+       VGE_LOCK_ASSERT(sc);
 
-       if (!sc->vge_link || ifp->if_drv_flags & IFF_DRV_OACTIVE) {
-               VGE_UNLOCK(sc);
+       if (!sc->vge_link || ifp->if_drv_flags & IFF_DRV_OACTIVE)
                return;
-       }
 
-       if (IFQ_DRV_IS_EMPTY(&ifp->if_snd)) {
-               VGE_UNLOCK(sc);
+       if (IFQ_DRV_IS_EMPTY(&ifp->if_snd))
                return;
-       }
 
        idx = sc->vge_ldata.vge_tx_prodidx;
 
@@ -1849,10 +1820,8 @@ vge_start(ifp)
                ETHER_BPF_MTAP(ifp, m_head);
        }
 
-       if (idx == sc->vge_ldata.vge_tx_prodidx) {
-               VGE_UNLOCK(sc);
+       if (idx == sc->vge_ldata.vge_tx_prodidx)
                return;
-       }
 
        /* Flush the TX descriptors */
 
@@ -1876,12 +1845,10 @@ vge_start(ifp)
         */
        CSR_WRITE_1(sc, VGE_CRS1, VGE_CR1_TIMER0_ENABLE);
 
-       VGE_UNLOCK(sc);
-
        /*
         * Set a timeout in case the chip goes out to lunch.
         */
-       ifp->if_timer = 5;
+       sc->vge_timer = 5;
 
        return;
 }
@@ -1891,11 +1858,20 @@ vge_init(xsc)
        void                    *xsc;
 {
        struct vge_softc        *sc = xsc;
+
+       VGE_LOCK(sc);
+       vge_init_locked(sc);
+       VGE_UNLOCK(sc);
+}
+
+static void
+vge_init_locked(struct vge_softc *sc)
+{
        struct ifnet            *ifp = sc->vge_ifp;
        struct mii_data         *mii;
        int                     i;
 
-       VGE_LOCK(sc);
+       VGE_LOCK_ASSERT(sc);
        mii = device_get_softc(sc->vge_miibus);
 
        /*
@@ -2051,12 +2027,11 @@ vge_init(xsc)
 
        ifp->if_drv_flags |= IFF_DRV_RUNNING;
        ifp->if_drv_flags &= ~IFF_DRV_OACTIVE;
+       callout_reset(&sc->vge_watchdog, hz, vge_watchdog, sc);
 
        sc->vge_if_flags = 0;
        sc->vge_link = 0;
 
-       VGE_UNLOCK(sc);
-
        return;
 }
 
@@ -2093,7 +2068,9 @@ vge_ifmedia_sts(ifp, ifmr)
        sc = ifp->if_softc;
        mii = device_get_softc(sc->vge_miibus);
 
+       VGE_LOCK(sc);
        mii_pollstat(mii);
+       VGE_UNLOCK(sc);
        ifmr->ifm_active = mii->mii_media_active;
        ifmr->ifm_status = mii->mii_media_status;
 
@@ -2168,6 +2145,7 @@ vge_ioctl(ifp, command, data)
                ifp->if_mtu = ifr->ifr_mtu;
                break;
        case SIOCSIFFLAGS:
+               VGE_LOCK(sc);
                if (ifp->if_flags & IFF_UP) {
                        if (ifp->if_drv_flags & IFF_DRV_RUNNING &&
                            ifp->if_flags & IFF_PROMISC &&
@@ -2182,16 +2160,19 @@ vge_ioctl(ifp, command, data)
                                    VGE_RXCTL_RX_PROMISC);
                                vge_setmulti(sc);
                         } else
-                               vge_init(sc);
+                               vge_init_locked(sc);
                } else {
                        if (ifp->if_drv_flags & IFF_DRV_RUNNING)
                                vge_stop(sc);
                }
                sc->vge_if_flags = ifp->if_flags;
+               VGE_UNLOCK(sc);
                break;
        case SIOCADDMULTI:
        case SIOCDELMULTI:
+               VGE_LOCK(sc);
                vge_setmulti(sc);
+               VGE_UNLOCK(sc);
                break;
        case SIOCGIFMEDIA:
        case SIOCSIFMEDIA:
@@ -2225,6 +2206,7 @@ vge_ioctl(ifp, command, data)
                        }
                }
 #endif /* DEVICE_POLLING */
+               VGE_LOCK(sc);
                if ((mask & IFCAP_TXCSUM) != 0 &&
                    (ifp->if_capabilities & IFCAP_TXCSUM) != 0) {
                        ifp->if_capenable ^= IFCAP_TXCSUM;
@@ -2236,6 +2218,7 @@ vge_ioctl(ifp, command, data)
                if ((mask & IFCAP_RXCSUM) != 0 &&
                    (ifp->if_capabilities & IFCAP_RXCSUM) != 0)
                        ifp->if_capenable ^= IFCAP_RXCSUM;
+               VGE_UNLOCK(sc);
            }
                break;
        default:
@@ -2247,22 +2230,25 @@ vge_ioctl(ifp, command, data)
 }
 
 static void
-vge_watchdog(ifp)
-       struct ifnet            *ifp;
+vge_watchdog(void *arg)
 {
-       struct vge_softc                *sc;
+       struct vge_softc *sc;
+       struct ifnet *ifp;
 
-       sc = ifp->if_softc;
-       VGE_LOCK(sc);
-       printf("vge%d: watchdog timeout\n", sc->vge_unit);
+       sc = arg;
+       VGE_LOCK_ASSERT(sc);
+       callout_reset(&sc->vge_watchdog, hz, vge_watchdog, sc);
+       if (sc->vge_timer == 0 || --sc->vge_timer > 0)
+               return;
+
+       ifp = sc->vge_ifp;
+       if_printf(ifp, "watchdog timeout\n");
        ifp->if_oerrors++;
 
        vge_txeof(sc);
        vge_rxeof(sc);
 
-       vge_init(sc);
-
-       VGE_UNLOCK(sc);
+       vge_init_locked(sc);
 
        return;
 }
@@ -2278,9 +2264,10 @@ vge_stop(sc)
        register int            i;
        struct ifnet            *ifp;
 
-       VGE_LOCK(sc);
+       VGE_LOCK_ASSERT(sc);
        ifp = sc->vge_ifp;
-       ifp->if_timer = 0;
+       sc->vge_timer = 0;
+       callout_stop(&sc->vge_watchdog);
 
        ifp->if_drv_flags &= ~(IFF_DRV_RUNNING | IFF_DRV_OACTIVE);
 
@@ -2318,8 +2305,6 @@ vge_stop(sc)
                }
        }
 
-       VGE_UNLOCK(sc);
-
        return;
 }
 
@@ -2336,9 +2321,11 @@ vge_suspend(dev)
 
        sc = device_get_softc(dev);
 
+       VGE_LOCK(sc);
        vge_stop(sc);
 
        sc->suspended = 1;
+       VGE_UNLOCK(sc);
 
        return (0);
 }
@@ -2363,10 +2350,12 @@ vge_resume(dev)
        pci_enable_io(dev, SYS_RES_MEMORY);
 
        /* reinitialize interface if necessary */
+       VGE_LOCK(sc);
        if (ifp->if_flags & IFF_UP)
-               vge_init(sc);
+               vge_init_locked(sc);
 
        sc->suspended = 0;
+       VGE_UNLOCK(sc);
 
        return (0);
 }
@@ -2383,7 +2372,9 @@ vge_shutdown(dev)
 
        sc = device_get_softc(dev);
 
+       VGE_LOCK(sc);
        vge_stop(sc);
+       VGE_UNLOCK(sc);
 
        return (0);
 }

Modified: stable/8/sys/dev/vge/if_vgevar.h
==============================================================================
--- stable/8/sys/dev/vge/if_vgevar.h    Fri Jan  8 21:02:12 2010        
(r201822)
+++ stable/8/sys/dev/vge/if_vgevar.h    Fri Jan  8 21:15:09 2010        
(r201823)
@@ -100,22 +100,20 @@ struct vge_list_data {
 struct vge_softc {
        struct ifnet            *vge_ifp;       /* interface info */
        device_t                vge_dev;
-       bus_space_handle_t      vge_bhandle;    /* bus space handle */
-       bus_space_tag_t         vge_btag;       /* bus space tag */
        struct resource         *vge_res;
        struct resource         *vge_irq;
        void                    *vge_intrhand;
        device_t                vge_miibus;
        bus_dma_tag_t           vge_parent_tag;
        bus_dma_tag_t           vge_tag;
-       u_int8_t                vge_unit;       /* interface number */
        u_int8_t                vge_type;
        int                     vge_if_flags;
        int                     vge_rx_consumed;
        int                     vge_link;
        int                     vge_camidx;
-       struct task             vge_txtask;
        struct mtx              vge_mtx;
+       struct callout          vge_watchdog;
+       int                     vge_timer;
        struct mbuf             *vge_head;
        struct mbuf             *vge_tail;
 
@@ -135,20 +133,20 @@ struct vge_softc {
  * register space access macros
  */
 #define CSR_WRITE_STREAM_4(sc, reg, val)       \
-       bus_space_write_stream_4(sc->vge_btag, sc->vge_bhandle, reg, val)
+       bus_write_stream_4(sc->vge_res, reg, val)
 #define CSR_WRITE_4(sc, reg, val)      \
-       bus_space_write_4(sc->vge_btag, sc->vge_bhandle, reg, val)
+       bus_write_4(sc->vge_res, reg, val)
 #define CSR_WRITE_2(sc, reg, val)      \
-       bus_space_write_2(sc->vge_btag, sc->vge_bhandle, reg, val)
+       bus_write_2(sc->vge_res, reg, val)
 #define CSR_WRITE_1(sc, reg, val)      \
-       bus_space_write_1(sc->vge_btag, sc->vge_bhandle, reg, val)
+       bus_write_1(sc->vge_res, reg, val)
 
 #define CSR_READ_4(sc, reg)            \
-       bus_space_read_4(sc->vge_btag, sc->vge_bhandle, reg)
+       bus_read_4(sc->vge_res, reg)
 #define CSR_READ_2(sc, reg)            \
-       bus_space_read_2(sc->vge_btag, sc->vge_bhandle, reg)
+       bus_read_2(sc->vge_res, reg)
 #define CSR_READ_1(sc, reg)            \
-       bus_space_read_1(sc->vge_btag, sc->vge_bhandle, reg)
+       bus_read_1(sc->vge_res, reg)
 
 #define CSR_SETBIT_1(sc, reg, x)       \
        CSR_WRITE_1(sc, reg, CSR_READ_1(sc, reg) | (x))
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Reply via email to