Date: Wed, 21 Jan 2009 20:48:43 -1000 (HST) From: Jeff Roberson <jroberson@jroberson.net> To: John Baldwin <jhb@FreeBSD.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r187576 - in head/sys/dev: ppbus ppc Message-ID: <20090121204615.H983@desktop> In-Reply-To: <200901212310.n0LNA6cM093944@svn.freebsd.org> References: <200901212310.n0LNA6cM093944@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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 *** >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20090121204615.H983>