On Wed, 21 Jan 2009, John Baldwin wrote:

Author: jhb
Date: Wed Jan 21 23:10:06 2009
New Revision: 187576
URL: http://svn.freebsd.org/changeset/base/187576

Log:
 Add locking to ppc and ppbus and mark the whole lot MPSAFE:

Looks like there might be some kinks still:

ppc0: <Parallel port> port 0x378-0x37f,0x778-0x77f irq 7 drq 3 on acpi0
ppc0: SMC-like chipset (ECP/EPP/PS2/NIBBLE) in COMPATIBLE mode
ppc0: FIFO with 16/16/9 bytes threshold
ppc0: [ITHREAD]
ppbus0: <Parallel port bus> on ppc0
panic: mutex ppc0 not owned at ../../../dev/ppc/ppc.c:1983
cpuid = 0
KDB: enter: panic
[thread pid 0 tid 100000 ]
Stopped at      kdb_enter+0x3d: movq    $0,0x652ea8(%rip)
_mtx_assert() at _mtx_assert+0xdc
ppc_write_ivar() at ppc_write_ivar+0x6e
ppbus_attach() at ppbus_attach+0x14b

Thanks,
Jeff

 - To avoid having a bunch of locks that end up always getting acquired as
   a group, give each ppc(4) device a mutex which it shares with all the
   child devices including ppbus(4), lpt(4), plip(4), etc.  This mutex
   is then used for all the locking.
 - Rework the interrupt handling stuff yet again.  Now ppbus drivers setup
   their interrupt handler during attach and tear it down during detach
   like most other drivers.  ppbus(4) only invokes the interrupt handler
   of the device that currently owns the bus (if any) when an interrupt
   occurs, however.  Also, interrupt handlers in general now accept their
   softc pointers as their argument rather than the device_t.  Another
   feature of the ppbus interrupt handlers is that they are called with
   the parent ppc device's lock already held.  This minimizes the number
   of lock operations during an interrupt.
 - Mark plip(4), lpt(4), pcfclock(4), ppi(4), vpo(4) MPSAFE.
 - lpbb(4) uses the ppc lock instead of Giant.
 - Other plip(4) changes:
   - Add a mutex to protect the global tables in plip(4) and free them on
     module unload.
   - Add a detach routine.
   - Split out the init/stop code from the ioctl routine into separate
     functions.
 - Other lpt(4) changes:
   - Use device_printf().
   - Use a dedicated callout for the lptout timer.
   - Allocate the I/O buffers at attach and detach rather than during
     open and close as this simplifies the locking at the cost of
     1024+32 bytes when the driver is attached.
 - Other ppi(4) changes:
   - Use an sx lock to serialize open and close.
   - Remove unused HADBUS flag.
   - Add a detach routine.
   - Use a malloc'd buffer for each read and write to avoid races with
     concurrent read/write.
 - Other pps(4) changes:
   - Use a callout rather than a callout handle with timeout().
   - Conform to the new ppbus requirements (regular mutex, non-filter
     interrupt handler).  pps(4) is probably going to have to become a
     standalone driver that doesn't use ppbus(4) to satisfy it's
     requirements for low latency as a result.
   - Use an sx lock to serialize open and close.
 - Other vpo(4) changes:
   - Use the parent ppc device's lock to create the CAM sim instead of
     Giant.
 - Other ppc(4) changes:
   - Fix ppc_isa's detach method to detach instead of calling attach.

 Tested by:       no one :-(

Modified:
 head/sys/dev/ppbus/if_plip.c
 head/sys/dev/ppbus/immio.c
 head/sys/dev/ppbus/lpbb.c
 head/sys/dev/ppbus/lpt.c
 head/sys/dev/ppbus/pcfclock.c
 head/sys/dev/ppbus/ppb_1284.c
 head/sys/dev/ppbus/ppb_base.c
 head/sys/dev/ppbus/ppb_msq.c
 head/sys/dev/ppbus/ppbconf.c
 head/sys/dev/ppbus/ppbconf.h
 head/sys/dev/ppbus/ppi.c
 head/sys/dev/ppbus/pps.c
 head/sys/dev/ppbus/vpo.c
 head/sys/dev/ppbus/vpoio.c
 head/sys/dev/ppc/ppc.c
 head/sys/dev/ppc/ppc_acpi.c
 head/sys/dev/ppc/ppc_isa.c
 head/sys/dev/ppc/ppc_pci.c
 head/sys/dev/ppc/ppc_puc.c
 head/sys/dev/ppc/ppcreg.h
 head/sys/dev/ppc/ppcvar.h

Modified: head/sys/dev/ppbus/if_plip.c
==============================================================================
--- head/sys/dev/ppbus/if_plip.c        Wed Jan 21 21:48:46 2009        
(r187575)
+++ head/sys/dev/ppbus/if_plip.c        Wed Jan 21 23:10:06 2009        
(r187576)
@@ -152,8 +152,12 @@ struct lp_data {
        int             sc_iferrs;

        struct resource *res_irq;
+       void            *sc_intr_cookie;
};

+static struct mtx lp_tables_lock;
+MTX_SYSINIT(lp_tables, &lp_tables_lock, "plip tables", MTX_DEF);
+
/* Tables for the lp# interface */
static u_char *txmith;
#define txmitl (txmith + (1 * LPIPTBLSIZE))
@@ -170,13 +174,41 @@ static int lpinittables(void);
static int lpioctl(struct ifnet *, u_long, caddr_t);
static int lpoutput(struct ifnet *, struct mbuf *, struct sockaddr *,
        struct rtentry *);
+static void lpstop(struct lp_data *);
static void lp_intr(void *);
+static int lp_module_handler(module_t, int, void *);

#define DEVTOSOFTC(dev) \
        ((struct lp_data *)device_get_softc(dev))

static devclass_t lp_devclass;

+static int
+lp_module_handler(module_t mod, int what, void *arg)
+{
+
+       switch (what) {
+       case MOD_UNLOAD:
+               mtx_lock(&lp_tables_lock);
+               if (txmith != NULL) {
+                       free(txmith, M_DEVBUF);
+                       txmith = NULL;
+               }
+               if (ctxmith != NULL) {
+                       free(ctxmith, M_DEVBUF);
+                       ctxmith = NULL;
+               }
+               mtx_unlock(&lp_tables_lock);
+               break;
+       case MOD_LOAD:
+       case MOD_QUIESCE:
+               break;
+       default:
+               return (EOPNOTSUPP);
+       }
+       return (0);
+}
+
static void
lp_identify(driver_t *driver, device_t parent)
{
@@ -201,7 +233,7 @@ lp_attach(device_t dev)
{
        struct lp_data *lp = DEVTOSOFTC(dev);
        struct ifnet *ifp;
-       int rid = 0;
+       int error, rid = 0;

        lp->sc_dev = dev;

@@ -224,8 +256,7 @@ lp_attach(device_t dev)
        ifp->if_softc = lp;
        if_initname(ifp, device_get_name(dev), device_get_unit(dev));
        ifp->if_mtu = LPMTU;
-       ifp->if_flags = IFF_SIMPLEX | IFF_POINTOPOINT | IFF_MULTICAST |
-           IFF_NEEDSGIANT;
+       ifp->if_flags = IFF_SIMPLEX | IFF_POINTOPOINT | IFF_MULTICAST;
        ifp->if_ioctl = lpioctl;
        ifp->if_output = lpoutput;
        ifp->if_hdrlen = 0;
@@ -235,8 +266,39 @@ lp_attach(device_t dev)

        bpfattach(ifp, DLT_NULL, sizeof(u_int32_t));

+       /*
+        * Attach our interrupt handler.  It is only called while we
+        * own the ppbus.
+        */
+       error = bus_setup_intr(dev, lp->res_irq, INTR_TYPE_NET | INTR_MPSAFE,
+           NULL, lp_intr, lp, &lp->sc_intr_cookie);
+       if (error) {
+               bpfdetach(ifp);
+               if_detach(ifp);
+               bus_release_resource(dev, SYS_RES_IRQ, 0, lp->res_irq);
+               device_printf(dev, "Unable to register interrupt handler\n");
+               return (error);
+       }
+
        return (0);
}
+
+static int
+lp_detach(device_t dev)
+{
+       struct lp_data *sc = device_get_softc(dev);
+       device_t ppbus = device_get_parent(dev);
+
+       ppb_lock(ppbus);
+       lpstop(sc);
+       ppb_unlock(ppbus);
+       bpfdetach(sc->sc_ifp);
+       if_detach(sc->sc_ifp);
+       bus_teardown_intr(dev, sc->res_irq, sc->sc_intr_cookie);
+       bus_release_resource(dev, SYS_RES_IRQ, 0, sc->res_irq);
+       return (0);
+}
+
/*
 * Build the translation tables for the LPIP (BSD unix) protocol.
 * We don't want to calculate these nasties in our tight loop, so we
@@ -247,17 +309,22 @@ lpinittables(void)
{
        int i;

+       mtx_lock(&lp_tables_lock);
        if (txmith == NULL)
                txmith = malloc(4 * LPIPTBLSIZE, M_DEVBUF, M_NOWAIT);

-       if (txmith == NULL)
+       if (txmith == NULL) {
+               mtx_unlock(&lp_tables_lock);
                return (1);
+       }

        if (ctxmith == NULL)
                ctxmith = malloc(4 * LPIPTBLSIZE, M_DEVBUF, M_NOWAIT);

-       if (ctxmith == NULL)
+       if (ctxmith == NULL) {
+               mtx_unlock(&lp_tables_lock);
                return (1);
+       }

        for (i = 0; i < LPIPTBLSIZE; i++) {
                ctxmith[i] = (i & 0xF0) >> 4;
@@ -272,10 +339,61 @@ lpinittables(void)
                trecvh[i] = ((~i) & 0x80) | ((i & 0x38) << 1);
                trecvl[i] = (((~i) & 0x80) >> 4) | ((i & 0x38) >> 3);
        }
+       mtx_unlock(&lp_tables_lock);

        return (0);
}

+static void
+lpstop(struct lp_data *sc)
+{
+       device_t ppbus = device_get_parent(sc->sc_dev);
+
+       ppb_assert_locked(ppbus);
+       ppb_wctr(ppbus, 0x00);
+       sc->sc_ifp->if_drv_flags &= ~(IFF_DRV_RUNNING | IFF_DRV_OACTIVE);
+       free(sc->sc_ifbuf, M_DEVBUF);
+       sc->sc_ifbuf = NULL;
+
+       /* IFF_UP is not set, try to release the bus anyway */
+       ppb_release_bus(ppbus, sc->sc_dev);
+}
+
+static int
+lpinit_locked(struct ifnet *ifp)
+{
+       struct lp_data *sc = ifp->if_softc;
+       device_t dev = sc->sc_dev;
+       device_t ppbus = device_get_parent(dev);
+       int error;
+
+       ppb_assert_locked(ppbus);
+       error = ppb_request_bus(ppbus, dev, PPB_DONTWAIT);
+       if (error)
+               return (error);
+
+       /* Now IFF_UP means that we own the bus */
+       ppb_set_mode(ppbus, PPB_COMPATIBLE);
+
+       if (lpinittables()) {
+               ppb_release_bus(ppbus, dev);
+               return (ENOBUFS);
+       }
+
+       sc->sc_ifbuf = malloc(sc->sc_ifp->if_mtu + MLPIPHDRLEN,
+           M_DEVBUF, M_NOWAIT);
+       if (sc->sc_ifbuf == NULL) {
+               ppb_release_bus(ppbus, dev);
+               return (ENOBUFS);
+       }
+
+       ppb_wctr(ppbus, IRQENABLE);
+
+       ifp->if_drv_flags |= IFF_DRV_RUNNING;
+       ifp->if_drv_flags &= ~IFF_DRV_OACTIVE;
+       return (0);
+}
+
/*
 * Process an ioctl request.
 */
@@ -288,7 +406,6 @@ lpioctl(struct ifnet *ifp, u_long cmd, c
        struct ifaddr *ifa = (struct ifaddr *)data;
        struct ifreq *ifr = (struct ifreq *)data;
        u_char *ptr;
-       void *ih;
        int error;

        switch (cmd) {
@@ -301,67 +418,32 @@ lpioctl(struct ifnet *ifp, u_long cmd, c
                ifp->if_flags |= IFF_UP;
                /* FALLTHROUGH */
        case SIOCSIFFLAGS:
+               error = 0;
+               ppb_lock(ppbus);
                if ((!(ifp->if_flags & IFF_UP)) &&
-                   (ifp->if_drv_flags & IFF_DRV_RUNNING)) {
-
-                       ppb_wctr(ppbus, 0x00);
-                       ifp->if_drv_flags &= ~IFF_DRV_RUNNING;
-
-                       /* IFF_UP is not set, try to release the bus anyway */
-                       ppb_release_bus(ppbus, dev);
-                       break;
-               }
-               if (((ifp->if_flags & IFF_UP)) &&
-                   (!(ifp->if_drv_flags & IFF_DRV_RUNNING))) {
-
-                       /* XXX
-                        * Should the request be interruptible?
-                        */
-                       if ((error = ppb_request_bus(ppbus, dev, PPB_WAIT |
-                           PPB_INTR)))
-                               return (error);
+                   (ifp->if_drv_flags & IFF_DRV_RUNNING))
+                       lpstop(sc);
+               else if (((ifp->if_flags & IFF_UP)) &&
+                   (!(ifp->if_drv_flags & IFF_DRV_RUNNING)))
+                       error = lpinit_locked(ifp);
+               ppb_unlock(ppbus);
+               return (error);

-                       /* Now IFF_UP means that we own the bus */
-                       ppb_set_mode(ppbus, PPB_COMPATIBLE);
-
-                       if (lpinittables()) {
-                               ppb_release_bus(ppbus, dev);
-                               return (ENOBUFS);
-                       }
-
-                       sc->sc_ifbuf = malloc(sc->sc_ifp->if_mtu + MLPIPHDRLEN,
-                           M_DEVBUF, M_WAITOK);
-                       if (sc->sc_ifbuf == NULL) {
-                               ppb_release_bus(ppbus, dev);
+       case SIOCSIFMTU:
+               ppb_lock(ppbus);
+               if (ifp->if_drv_flags & IFF_DRV_RUNNING) {
+                       ptr = malloc(ifr->ifr_mtu + MLPIPHDRLEN, M_DEVBUF,
+                           M_NOWAIT);
+                       if (ptr == NULL) {
+                               ppb_unlock(ppbus);
                                return (ENOBUFS);
                        }
-
-                       /*
-                        * Attach our interrupt handler.  It is
-                        * detached later when the bus is released.
-                        */
-                       if ((error = bus_setup_intr(dev, sc->res_irq,
-                           INTR_TYPE_NET, NULL, lp_intr, dev, &ih))) {
-                               ppb_release_bus(ppbus, dev);
-                               return (error);
-                       }
-
-                       ppb_wctr(ppbus, IRQENABLE);
-                       ifp->if_drv_flags |= IFF_DRV_RUNNING;
-               }
-               break;
-
-       case SIOCSIFMTU:
-               ptr = sc->sc_ifbuf;
-               sc->sc_ifbuf = malloc(ifr->ifr_mtu + MLPIPHDRLEN, M_DEVBUF,
-                   M_NOWAIT);
-               if (sc->sc_ifbuf == NULL) {
+                       if (sc->sc_ifbuf)
+                               free(sc->sc_ifbuf, M_DEVBUF);
                        sc->sc_ifbuf = ptr;
-                       return (ENOBUFS);
                }
-               if (ptr)
-                       free(ptr, M_DEVBUF);
                sc->sc_ifp->if_mtu = ifr->ifr_mtu;
+               ppb_unlock(ppbus);
                break;

        case SIOCGIFMTU:
@@ -417,14 +499,14 @@ clpinbyte(int spin, device_t ppbus)
{
        u_char c, cl;

-       while((ppb_rstr(ppbus) & CLPIP_SHAKE))
+       while ((ppb_rstr(ppbus) & CLPIP_SHAKE))
                if (!--spin) {
                        return (-1);
                }
        cl = ppb_rstr(ppbus);
        ppb_wdtr(ppbus, 0x10);

-       while(!(ppb_rstr(ppbus) & CLPIP_SHAKE))
+       while (!(ppb_rstr(ppbus) & CLPIP_SHAKE))
                if (!--spin) {
                        return (-1);
                }
@@ -445,16 +527,14 @@ lptap(struct ifnet *ifp, struct mbuf *m)
static void
lp_intr(void *arg)
{
-       device_t dev = (device_t)arg;
-       device_t ppbus = device_get_parent(dev);
-       struct lp_data *sc = DEVTOSOFTC(dev);
-       int len, s, j;
+       struct lp_data *sc = arg;
+       device_t ppbus = device_get_parent(sc->sc_dev);
+       int len, j;
        u_char *bp;
        u_char c, cl;
        struct mbuf *top;

-       s = splhigh();
-
+       ppb_assert_locked(ppbus);
        if (sc->sc_ifp->if_flags & IFF_LINK0) {

                /* Ack. the request */
@@ -500,13 +580,15 @@ lp_intr(void *arg)
                top = m_devget(sc->sc_ifbuf + CLPIPHDRLEN, len, 0, sc->sc_ifp,
                    0);
                if (top) {
+                       ppb_unlock(ppbus);
                        if (bpf_peers_present(sc->sc_ifp->if_bpf))
                                lptap(sc->sc_ifp, top);

                        /* mbuf is free'd on failure. */
                        netisr_queue(NETISR_IP, top);
+                       ppb_lock(ppbus);
                }
-               goto done;
+               return;
        }
        while ((ppb_rstr(ppbus) & LPIP_SHAKE)) {
                len = sc->sc_ifp->if_mtu + LPIPHDRLEN;
@@ -517,7 +599,7 @@ lp_intr(void *arg)
                        ppb_wdtr(ppbus, 8);

                        j = LPMAXSPIN2;
-                       while((ppb_rstr(ppbus) & LPIP_SHAKE))
+                       while ((ppb_rstr(ppbus) & LPIP_SHAKE))
                                if (!--j)
                                        goto err;

@@ -550,14 +632,16 @@ lp_intr(void *arg)
                top = m_devget(sc->sc_ifbuf + LPIPHDRLEN, len, 0, sc->sc_ifp,
                    0);
                if (top) {
+                       ppb_unlock(ppbus);
                        if (bpf_peers_present(sc->sc_ifp->if_bpf))
                                lptap(sc->sc_ifp, top);

                        /* mbuf is free'd on failure. */
                        netisr_queue(NETISR_IP, top);
+                       ppb_lock(ppbus);
                }
        }
-       goto done;
+       return;

err:
        ppb_wdtr(ppbus, 0);
@@ -575,9 +659,6 @@ err:
                sc->sc_ifp->if_drv_flags &= ~IFF_DRV_RUNNING;
                sc->sc_iferrs = 0;
        }
-
-done:
-       splx(s);
}

static __inline int
@@ -602,7 +683,7 @@ lpoutput(struct ifnet *ifp, struct mbuf
        struct lp_data *sc = ifp->if_softc;
        device_t dev = sc->sc_dev;
        device_t ppbus = device_get_parent(dev);
-       int s, err;
+       int err;
        struct mbuf *mm;
        u_char *cp = "\0\0";
        u_char chksum = 0;
@@ -611,19 +692,18 @@ lpoutput(struct ifnet *ifp, struct mbuf

        /* We need a sensible value if we abort */
        cp++;
-       ifp->if_drv_flags |= IFF_DRV_RUNNING;
+       ppb_lock(ppbus);
+       ifp->if_drv_flags |= IFF_DRV_OACTIVE;

        err = 1;                /* assume we're aborting because of an error */

-       s = splhigh();
-
        /* Suspend (on laptops) or receive-errors might have taken us offline */
        ppb_wctr(ppbus, IRQENABLE);

        if (ifp->if_flags & IFF_LINK0) {
                if (!(ppb_rstr(ppbus) & CLPIP_SHAKE)) {
                        lprintf("&");
-                       lp_intr(dev);
+                       lp_intr(sc);
                }

                /* Alert other end to pending packet */
@@ -681,6 +761,7 @@ lpoutput(struct ifnet *ifp, struct mbuf
                err = 0;                        /* No errors */

        nend:
+               ifp->if_drv_flags &= ~IFF_DRV_OACTIVE;
                if (err)  {                     /* if we didn't timeout... */
                        ifp->if_oerrors++;
                        lprintf("X");
@@ -695,15 +776,15 @@ lpoutput(struct ifnet *ifp, struct mbuf

                if (!(ppb_rstr(ppbus) & CLPIP_SHAKE)) {
                        lprintf("^");
-                       lp_intr(dev);
+                       lp_intr(sc);
                }
-               (void) splx(s);
+               ppb_unlock(ppbus);
                return (0);
        }

        if (ppb_rstr(ppbus) & LPIP_SHAKE) {
                lprintf("&");
-               lp_intr(dev);
+               lp_intr(sc);
        }

        if (lpoutbyte(0x08, LPMAXSPIN1, ppbus))
@@ -726,6 +807,7 @@ end:
        --cp;
        ppb_wdtr(ppbus, txmitl[*cp] ^ 0x17);

+       ifp->if_drv_flags &= ~IFF_DRV_OACTIVE;
        if (err)  {                     /* if we didn't timeout... */
                ifp->if_oerrors++;
                lprintf("X");
@@ -740,10 +822,10 @@ end:

        if (ppb_rstr(ppbus) & LPIP_SHAKE) {
                lprintf("^");
-               lp_intr(dev);
+               lp_intr(sc);
        }

-       (void) splx(s);
+       ppb_unlock(ppbus);
        return (0);
}

@@ -752,6 +834,7 @@ static device_method_t lp_methods[] = {
        DEVMETHOD(device_identify,      lp_identify),
        DEVMETHOD(device_probe,         lp_probe),
        DEVMETHOD(device_attach,        lp_attach),
+       DEVMETHOD(device_detach,        lp_detach),

        { 0, 0 }
};
@@ -762,5 +845,5 @@ static driver_t lp_driver = {
        sizeof(struct lp_data),
};

-DRIVER_MODULE(plip, ppbus, lp_driver, lp_devclass, 0, 0);
+DRIVER_MODULE(plip, ppbus, lp_driver, lp_devclass, lp_module_handler, 0);
MODULE_DEPEND(plip, ppbus, 1, 1, 1);

Modified: head/sys/dev/ppbus/immio.c
==============================================================================
--- head/sys/dev/ppbus/immio.c  Wed Jan 21 21:48:46 2009        (r187575)
+++ head/sys/dev/ppbus/immio.c  Wed Jan 21 23:10:06 2009        (r187576)
@@ -606,6 +606,7 @@ imm_attach(struct vpoio_data *vpo)
        /*
         * Initialize mode dependent in/out microsequences
         */
+       ppb_lock(ppbus);
        if ((error = ppb_request_bus(ppbus, vpo->vpo_dev, PPB_WAIT)))
            goto error;

@@ -632,6 +633,7 @@ imm_attach(struct vpoio_data *vpo)

        ppb_release_bus(ppbus, vpo->vpo_dev);
 error:
+       ppb_unlock(ppbus);
        return (error);
}


Modified: head/sys/dev/ppbus/lpbb.c
==============================================================================
--- head/sys/dev/ppbus/lpbb.c   Wed Jan 21 21:48:46 2009        (r187575)
+++ head/sys/dev/ppbus/lpbb.c   Wed Jan 21 23:10:06 2009        (r187576)
@@ -103,16 +103,16 @@ lpbb_callback(device_t dev, int index, c
        case IIC_REQUEST_BUS:
                /* request the ppbus */
                how = *(int *)data;
-               mtx_lock(&Giant);
+               ppb_lock(ppbus);
                error = ppb_request_bus(ppbus, dev, how);
-               mtx_unlock(&Giant);
+               ppb_unlock(ppbus);
                break;

        case IIC_RELEASE_BUS:
                /* release the ppbus */
-               mtx_lock(&Giant);
+               ppb_lock(ppbus);
                error = ppb_release_bus(ppbus, dev);
-               mtx_unlock(&Giant);
+               ppb_unlock(ppbus);
                break;

        default:
@@ -129,25 +129,38 @@ lpbb_callback(device_t dev, int index, c
#define ALIM    0x20
#define I2CKEY  0x50

+/* Reset bus by setting SDA first and then SCL. */
+static void
+lpbb_reset_bus(device_t dev)
+{
+       device_t ppbus = device_get_parent(dev);
+
+       ppb_assert_locked(ppbus);
+       ppb_wdtr(ppbus, (u_char)~SDA_out);
+       ppb_wctr(ppbus, (u_char)(ppb_rctr(ppbus) | SCL_out));
+}
+
static int
lpbb_getscl(device_t dev)
{
+       device_t ppbus = device_get_parent(dev);
        int rval;

-       mtx_lock(&Giant);
-       rval = ((ppb_rstr(device_get_parent(dev)) & SCL_in) == SCL_in);
-       mtx_unlock(&Giant);
+       ppb_lock(ppbus);
+       rval = ((ppb_rstr(ppbus) & SCL_in) == SCL_in);
+       ppb_unlock(ppbus);
        return (rval);
}

static int
lpbb_getsda(device_t dev)
{
+       device_t ppbus = device_get_parent(dev);
        int rval;

-       mtx_lock(&Giant);
-       rval = ((ppb_rstr(device_get_parent(dev)) & SDA_in) == SDA_in);
-       mtx_unlock(&Giant);
+       ppb_lock(ppbus);
+       rval = ((ppb_rstr(ppbus) & SDA_in) == SDA_in);
+       ppb_unlock(ppbus);
        return (rval);
}

@@ -156,12 +169,12 @@ lpbb_setsda(device_t dev, char val)
{
        device_t ppbus = device_get_parent(dev);

-       mtx_lock(&Giant);
+       ppb_lock(ppbus);
        if (val == 0)
                ppb_wdtr(ppbus, (u_char)SDA_out);
        else
                ppb_wdtr(ppbus, (u_char)~SDA_out);
-       mtx_unlock(&Giant);
+       ppb_unlock(ppbus);
}

static void
@@ -169,12 +182,12 @@ lpbb_setscl(device_t dev, unsigned char
{
        device_t ppbus = device_get_parent(dev);

-       mtx_lock(&Giant);
+       ppb_lock(ppbus);
        if (val == 0)
                ppb_wctr(ppbus, (u_char)(ppb_rctr(ppbus) & ~SCL_out));
        else
                ppb_wctr(ppbus, (u_char)(ppb_rctr(ppbus) | SCL_out));
-       mtx_unlock(&Giant);
+       ppb_unlock(ppbus);
}

static int
@@ -182,23 +195,24 @@ lpbb_detect(device_t dev)
{
        device_t ppbus = device_get_parent(dev);

+       ppb_lock(ppbus);
        if (ppb_request_bus(ppbus, dev, PPB_DONTWAIT)) {
+               ppb_unlock(ppbus);
                device_printf(dev, "can't allocate ppbus\n");
                return (0);
        }

-       /* reset bus */
-       lpbb_setsda(dev, 1);
-       lpbb_setscl(dev, 1);
+       lpbb_reset_bus(dev);

        if ((ppb_rstr(ppbus) & I2CKEY) ||
                ((ppb_rstr(ppbus) & ALIM) != ALIM)) {
-
                ppb_release_bus(ppbus, dev);
+               ppb_unlock(ppbus);
                return (0);
        }

        ppb_release_bus(ppbus, dev);
+       ppb_unlock(ppbus);

        return (1);
}
@@ -208,18 +222,17 @@ lpbb_reset(device_t dev, u_char speed, u
{
        device_t ppbus = device_get_parent(dev);

-       mtx_lock(&Giant);
+       ppb_lock(ppbus);
        if (ppb_request_bus(ppbus, dev, PPB_DONTWAIT)) {
+               ppb_unlock(ppbus);
                device_printf(dev, "can't allocate ppbus\n");
                return (0);
        }

-       /* reset bus */
-       lpbb_setsda(dev, 1);
-       lpbb_setscl(dev, 1);
+       lpbb_reset_bus(dev);

        ppb_release_bus(ppbus, dev);
-       mtx_unlock(&Giant);
+       ppb_unlock(ppbus);

        return (IIC_ENOADDR);
}

Modified: head/sys/dev/ppbus/lpt.c
==============================================================================
--- head/sys/dev/ppbus/lpt.c    Wed Jan 21 21:48:46 2009        (r187575)
+++ head/sys/dev/ppbus/lpt.c    Wed Jan 21 23:10:06 2009        (r187576)
@@ -105,9 +105,9 @@ static int volatile lptflag = 1;
#define BUFSTATSIZE     32

struct lpt_data {
-       device_t dev;
-       struct cdev *cdev;
-       struct cdev *cdev_bypass;
+       device_t sc_dev;
+       struct cdev *sc_cdev;
+       struct cdev *sc_cdev_bypass;
        short   sc_state;
        /* default case: negative prime, negative ack, handshake strobe,
           prime once */
@@ -130,9 +130,10 @@ struct lpt_data {
#define LP_ENABLE_IRQ   0x04    /* enable IRQ on open */
#define LP_ENABLE_EXT   0x10    /* we shall use advanced mode when possible */
        u_char  sc_backoff ;    /* time to call lptout() again */
+       struct callout sc_timer;

-       struct resource *intr_resource; /* interrupt resource */
-       void *intr_cookie;              /* interrupt registration cookie */
+       struct resource *sc_intr_resource;      /* interrupt resource */
+       void    *sc_intr_cookie;                /* interrupt cookie */
};

#define LPT_NAME        "lpt"         /* our official name */
@@ -144,8 +145,7 @@ static int  lpt_detect(device_t dev);
#define DEVTOSOFTC(dev) \
        ((struct lpt_data *)device_get_softc(dev))

-static void lptintr(device_t dev);
-static void lpt_intr(void *arg);       /* without spls */
+static void lptintr(void *arg);

static devclass_t lpt_devclass;

@@ -183,7 +183,6 @@ static      d_ioctl_t       lptioctl;

static struct cdevsw lpt_cdevsw = {
        .d_version =    D_VERSION,
-       .d_flags =      D_NEEDGIANT,
        .d_open =       lptopen,
        .d_close =      lptclose,
        .d_read =       lptread,
@@ -199,13 +198,17 @@ lpt_request_ppbus(device_t dev, int how)
        struct lpt_data *sc = DEVTOSOFTC(dev);
        int error;

+       /*
+        * We might already have the bus for a write(2) after an interrupted
+        * write(2) call.
+        */
+       ppb_assert_locked(ppbus);
        if (sc->sc_state & HAVEBUS)
                return (0);

-       /* we have the bus only if the request succeded */
-       if ((error = ppb_request_bus(ppbus, dev, how)) == 0)
+       error = ppb_request_bus(ppbus, dev, how);
+       if (error == 0)
                sc->sc_state |= HAVEBUS;
-
        return (error);
}

@@ -216,9 +219,12 @@ lpt_release_ppbus(device_t dev)
        struct lpt_data *sc = DEVTOSOFTC(dev);
        int error = 0;

-       if ((error = ppb_release_bus(ppbus, dev)) == 0)
-               sc->sc_state &= ~HAVEBUS;
-
+       ppb_assert_locked(ppbus);
+       if (sc->sc_state & HAVEBUS) {
+               error = ppb_release_bus(ppbus, dev);
+               if (error == 0)
+                       sc->sc_state &= ~HAVEBUS;
+       }
        return (error);
}

@@ -306,24 +312,25 @@ lpt_detect(device_t dev)

        status = 1;                             /* assume success */

+       ppb_lock(ppbus);
        if ((error = lpt_request_ppbus(dev, PPB_DONTWAIT))) {
-               printf(LPT_NAME ": cannot alloc ppbus (%d)!\n", error);
-               status = 0;
-               goto end_probe;
+               ppb_unlock(ppbus);
+               device_printf(dev, "cannot alloc ppbus (%d)!\n", error);
+               return (0);
        }

        for (i = 0; i < 18 && status; i++)
                if (!lpt_port_test(ppbus, testbyte[i], 0xff)) {
                        status = 0;
-                       goto end_probe;
+                       break;
                }

-end_probe:
        /* write 0's to control and data ports */
        ppb_wdtr(ppbus, 0);
        ppb_wctr(ppbus, 0);

        lpt_release_ppbus(dev);
+       ppb_unlock(ppbus);

        return (status);
}
@@ -363,21 +370,33 @@ lpt_attach(device_t dev)
        int error;

        sc->sc_primed = 0;   /* not primed yet */
+       ppb_init_callout(ppbus, &sc->sc_timer, 0);

+       ppb_lock(ppbus);
        if ((error = lpt_request_ppbus(dev, PPB_DONTWAIT))) {
-               printf(LPT_NAME ": cannot alloc ppbus (%d)!\n", error);
+               ppb_unlock(ppbus);
+               device_printf(dev, "cannot alloc ppbus (%d)!\n", error);
                return (0);
        }

        ppb_wctr(ppbus, LPC_NINIT);
-
-       /* check if we can use interrupt, should be done by ppc stuff */
-       lprintf(("oldirq %x\n", sc->sc_irq));
+       ppb_unlock(ppbus);
+       lpt_release_ppbus(dev);

        /* declare our interrupt handler */
-       sc->intr_resource = bus_alloc_resource_any(dev, SYS_RES_IRQ, &rid,
+       sc->sc_intr_resource = bus_alloc_resource_any(dev, SYS_RES_IRQ, &rid,
            RF_SHAREABLE);
-       if (sc->intr_resource) {
+       if (sc->sc_intr_resource) {
+               error = bus_setup_intr(dev, sc->sc_intr_resource,
+                   INTR_TYPE_TTY | INTR_MPSAFE, NULL, lptintr, sc,
+                   &sc->sc_intr_cookie);
+               if (error) {
+                       bus_release_resource(dev, SYS_RES_IRQ, rid,
+                           sc->sc_intr_resource);
+                       device_printf(dev,
+                           "Unable to register interrupt handler\n");
+                       return (error);
+               }
                sc->sc_irq = LP_HAS_IRQ | LP_USE_IRQ | LP_ENABLE_IRQ;
                device_printf(dev, "Interrupt-driven port\n");
        } else {
@@ -386,17 +405,17 @@ lpt_attach(device_t dev)
        }
        lprintf(("irq %x\n", sc->sc_irq));

-       lpt_release_ppbus(dev);
-
-       sc->dev = dev;
-       sc->cdev = make_dev(&lpt_cdevsw, unit,
+       sc->sc_inbuf = malloc(BUFSIZE, M_DEVBUF, M_WAITOK);
+       sc->sc_statbuf = malloc(BUFSTATSIZE, M_DEVBUF, M_WAITOK);
+       sc->sc_dev = dev;
+       sc->sc_cdev = make_dev(&lpt_cdevsw, unit,
            UID_ROOT, GID_WHEEL, 0600, LPT_NAME "%d", unit);
-       sc->cdev->si_drv1 = sc;
-       sc->cdev->si_drv2 = 0;
-       sc->cdev_bypass = make_dev(&lpt_cdevsw, unit,
+       sc->sc_cdev->si_drv1 = sc;
+       sc->sc_cdev->si_drv2 = 0;
+       sc->sc_cdev_bypass = make_dev(&lpt_cdevsw, unit,
            UID_ROOT, GID_WHEEL, 0600, LPT_NAME "%d.ctl", unit);
-       sc->cdev_bypass->si_drv1 = sc;
-       sc->cdev_bypass->si_drv2 = (void *)LP_BYPASS;
+       sc->sc_cdev_bypass->si_drv1 = sc;
+       sc->sc_cdev_bypass->si_drv2 = (void *)LP_BYPASS;
        return (0);
}

@@ -404,15 +423,21 @@ static int
lpt_detach(device_t dev)
{
        struct lpt_data *sc = DEVTOSOFTC(dev);
+       device_t ppbus = device_get_parent(dev);

-       destroy_dev(sc->cdev);
-       destroy_dev(sc->cdev_bypass);
+       destroy_dev(sc->sc_cdev);
+       destroy_dev(sc->sc_cdev_bypass);
+       ppb_lock(ppbus);
        lpt_release_ppbus(dev);
-       if (sc->intr_resource != 0) {
-               BUS_TEARDOWN_INTR(device_get_parent(dev), dev,
-                   sc->intr_resource, sc->intr_cookie);
-               bus_release_resource(dev, SYS_RES_IRQ, 0, sc->intr_resource);
+       ppb_unlock(ppbus);
+       callout_drain(&sc->sc_timer);
+       if (sc->sc_intr_resource != NULL) {
+               bus_teardown_intr(dev, sc->sc_intr_resource,
+                   sc->sc_intr_cookie);
+               bus_release_resource(dev, SYS_RES_IRQ, 0, sc->sc_intr_resource);
        }
+       free(sc->sc_inbuf, M_DEVBUF);
+       free(sc->sc_statbuf, M_DEVBUF);

        return (0);
}
@@ -420,18 +445,17 @@ lpt_detach(device_t dev)
static void
lptout(void *arg)
{
-       device_t dev = (device_t)arg;
-       struct lpt_data *sc = DEVTOSOFTC(dev);
-#ifdef LPT_DEBUG
+       struct lpt_data *sc = arg;
+       device_t dev = sc->sc_dev;
        device_t ppbus = device_get_parent(dev);
-#endif

+       ppb_assert_locked(ppbus);
        lprintf(("T %x ", ppb_rstr(ppbus)));
        if (sc->sc_state & OPEN) {
                sc->sc_backoff++;
                if (sc->sc_backoff > hz/LPTOUTMAX)
                        sc->sc_backoff = sc->sc_backoff > hz/LPTOUTMAX;
-               timeout(lptout, (caddr_t)dev, sc->sc_backoff);
+               callout_reset(&sc->sc_timer, sc->sc_backoff, lptout, sc);
        } else
                sc->sc_state &= ~TOUT;

@@ -442,7 +466,7 @@ lptout(void *arg)
         * Avoid possible hangs due to missed interrupts
         */
        if (sc->sc_xfercnt) {
-               lptintr(dev);
+               lptintr(sc);
        } else {
                sc->sc_state &= ~OBUSY;
                wakeup(dev);
@@ -458,17 +482,19 @@ lptout(void *arg)
static  int
lptopen(struct cdev *dev, int flags, int fmt, struct thread *td)
{
-       int s;
        int trys, err;
        struct lpt_data *sc = dev->si_drv1;
-       device_t lptdev = sc->dev;
+       device_t lptdev = sc->sc_dev;
        device_t ppbus = device_get_parent(lptdev);

        if (!sc)
                return (ENXIO);

+       ppb_lock(ppbus);
        if (sc->sc_state) {
-               lprintf((LPT_NAME ": still open %x\n", sc->sc_state));
+               lprintf(("%s: still open %x\n", device_get_nameunit(lptdev),
+                   sc->sc_state));
+               ppb_unlock(ppbus);
                return(EBUSY);
        } else
                sc->sc_state |= LPTINIT;
@@ -478,6 +504,7 @@ lptopen(struct cdev *dev, int flags, int
        /* Check for open with BYPASS flag set. */
        if (sc->sc_flags & LP_BYPASS) {
                sc->sc_state = OPEN;
+               ppb_unlock(ppbus);
                return(0);
        }

@@ -485,11 +512,12 @@ lptopen(struct cdev *dev, int flags, int
        if ((err = lpt_request_ppbus(lptdev, PPB_WAIT|PPB_INTR)) != 0) {
                /* give it a chance to try later */
                sc->sc_state = 0;
+               ppb_unlock(ppbus);
                return (err);
        }

-       s = spltty();
-       lprintf((LPT_NAME " flags 0x%x\n", sc->sc_flags));
+       lprintf(("%s flags 0x%x\n", device_get_nameunit(lptdev),
+           sc->sc_flags));

        /* set IRQ status according to ENABLE_IRQ flag
         */
@@ -514,21 +542,21 @@ lptopen(struct cdev *dev, int flags, int
        do {
                /* ran out of waiting for the printer */
                if (trys++ >= LPINITRDY*4) {
-                       splx(s);
                        sc->sc_state = 0;
                        lprintf(("status %x\n", ppb_rstr(ppbus)));

                        lpt_release_ppbus(lptdev);
+                       ppb_unlock(ppbus);
                        return (EBUSY);
                }

                /* wait 1/4 second, give up if we get a signal */
-               if (tsleep(lptdev, LPPRI|PCATCH, "lptinit", hz/4) !=
-                   EWOULDBLOCK) {
+               if (ppb_sleep(ppbus, lptdev, LPPRI | PCATCH, "lptinit",
+                   hz / 4) != EWOULDBLOCK) {
                        sc->sc_state = 0;
-                       splx(s);

                        lpt_release_ppbus(lptdev);
+                       ppb_unlock(ppbus);
                        return (EBUSY);
                }

@@ -548,22 +576,20 @@ lptopen(struct cdev *dev, int flags, int
        ppb_wctr(ppbus, sc->sc_control);

        sc->sc_state = OPEN;
-       sc->sc_inbuf = malloc(BUFSIZE, M_DEVBUF, M_WAITOK);
-       sc->sc_statbuf = malloc(BUFSTATSIZE, M_DEVBUF, M_WAITOK);
        sc->sc_xfercnt = 0;
-       splx(s);
-
-       /* release the ppbus */
-       lpt_release_ppbus(lptdev);

        /* only use timeout if using interrupt */
        lprintf(("irq %x\n", sc->sc_irq));
        if (sc->sc_irq & LP_USE_IRQ) {
                sc->sc_state |= TOUT;
-               timeout(lptout, (caddr_t)lptdev,
-                        (sc->sc_backoff = hz/LPTOUTINITIAL));
+               sc->sc_backoff = hz / LPTOUTINITIAL;
+               callout_reset(&sc->sc_timer, sc->sc_backoff, lptout, sc);
        }

+       /* release the ppbus */
+       lpt_release_ppbus(lptdev);
+       ppb_unlock(ppbus);
+
        lprintf(("opened.\n"));
        return(0);
}
@@ -578,17 +604,21 @@ static    int
lptclose(struct cdev *dev, int flags, int fmt, struct thread *td)
{
        struct lpt_data *sc = dev->si_drv1;
-       device_t lptdev = sc->dev;

*** DIFF OUTPUT TRUNCATED AT 1000 LINES ***

_______________________________________________
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