Date: Tue, 7 Nov 2006 01:33:47 GMT From: Sam Leffler <sam@FreeBSD.org> To: Perforce Change Reviews <perforce@freebsd.org> Subject: PERFORCE change 109409 for review Message-ID: <200611070133.kA71XlK7090336@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=109409 Change 109409 by sam@sam_ebb on 2006/11/07 01:33:10 lots of cleanups: o single-thread access to the mailbox registers o eliminate spinning waiting for a message reply; instead enable interrupts and do sleep/wakeup to get the msg response from the NPE o remove ixpnpe_sendmsg and ipxnpe_recvmsg; they are now dangerous to use and are otherwise we really need clients to use the send+recv api o be pedantic and cleanup resources on detach Affected files ... .. //depot/projects/arm/src/sys/arm/xscale/ixp425/ixp425_npe.c#3 edit .. //depot/projects/arm/src/sys/arm/xscale/ixp425/ixp425_npevar.h#3 edit Differences ... ==== //depot/projects/arm/src/sys/arm/xscale/ixp425/ixp425_npe.c#3 (text+ko) ==== @@ -69,6 +69,9 @@ * for loading microcode images and the associated NPE CPU * manipulations (start, stop, reset). * + * The code here basically replaces the npeDl and npeMh classes + * in the Intel Access Library (IAL). + * * NB: Microcode images are loaded with firmware(9). To * include microcode in a static kernel include the * ixpnpe_fw device. Otherwise the firmware will be @@ -103,8 +106,12 @@ device_t sc_dev; bus_space_tag_t sc_iot; bus_space_handle_t sc_ioh; + bus_size_t sc_size; /* size of mapped register window */ struct resource *sc_irq; /* IRQ resource */ void *sc_ih; /* interrupt handler */ + struct mtx sc_mtx; /* mailbox lock */ + uint32_t sc_msg[2]; /* reply msg collected in ixpnpe_intr */ + int sc_msgwaiting; /* sc_msg holds valid data */ int validImage; /* valid ucode image loaded */ int started; /* NPE is started */ @@ -221,9 +228,7 @@ static int npe_ctx_reg_write(struct ixpnpe_softc *, uint32_t ctxtNum, uint32_t ctxtReg, uint32_t ctxtRegVal, int verify); -#if 0 static void ixpnpe_intr(void *arg); -#endif static uint32_t npe_reg_read(struct ixpnpe_softc *sc, bus_size_t off) @@ -246,16 +251,17 @@ struct ixp425_softc *sa = device_get_softc(device_get_parent(dev)); struct ixpnpe_softc *sc; bus_addr_t base; - bus_size_t size; int rid, irq; /* XXX M_BUS */ sc = malloc(sizeof(struct ixpnpe_softc), M_TEMP, M_WAITOK | M_ZERO); sc->sc_dev = dev; sc->sc_iot = sa->sc_iot; + mtx_init(&sc->sc_mtx, device_get_nameunit(dev), MTX_NETWORK_LOCK, MTX_DEF); + if (device_get_unit(dev) == 0) { base = IXP425_NPE_B_HWBASE; - size = IXP425_NPE_B_SIZE; + sc->sc_size = IXP425_NPE_B_SIZE; irq = IXP425_INT_NPE_B; /* size of instruction memory */ @@ -264,7 +270,7 @@ sc->dataMemSize = IX_NPEDL_DATA_MEMSIZE_WORDS_NPEB; } else { base = IXP425_NPE_C_HWBASE; - size = IXP425_NPE_C_SIZE; + sc->sc_size = IXP425_NPE_C_SIZE; irq = IXP425_INT_NPE_C; /* size of instruction memory */ @@ -272,7 +278,7 @@ /* size of data memory */ sc->dataMemSize = IX_NPEDL_DATA_MEMSIZE_WORDS_NPEC; } - if (bus_space_map(sc->sc_iot, base, size, 0, &sc->sc_ioh)) + if (bus_space_map(sc->sc_iot, base, sc->sc_size, 0, &sc->sc_ioh)) panic("%s: Cannot map registers", device_get_name(dev)); /* @@ -284,10 +290,11 @@ if (!sc->sc_irq) panic("%s: Unable to allocate irq %u", device_get_name(dev), irq); /* XXX could be a source of entropy */ -#if 0 bus_setup_intr(dev, sc->sc_irq, INTR_TYPE_NET | INTR_MPSAFE, ixpnpe_intr, sc, &sc->sc_ih); -#endif + /* enable output fifo interrupts (NB: must also set OFIFO Write Enable) */ + npe_reg_write(sc, IX_NPECTL, + npe_reg_read(sc, IX_NPECTL) | (IX_NPECTL_OFE | IX_NPECTL_OFWE)); return sc; } @@ -295,8 +302,13 @@ void ixpnpe_detach(struct ixpnpe_softc *sc) { - /* XXX release irq */ - /* XXX unmap memory */ + /* disable output fifo interrupts */ + npe_reg_write(sc, IX_NPECTL, + npe_reg_read(sc, IX_NPECTL) &~ (IX_NPECTL_OFE | IX_NPECTL_OFWE)); + + bus_teardown_intr(sc->sc_dev, sc->sc_irq, sc->sc_ih); + bus_space_unmap(sc->sc_iot, sc->sc_ioh, sc->sc_size); + mtx_destroy(&sc->sc_mtx); free(sc, M_TEMP); } @@ -305,11 +317,14 @@ { int error; - error = npe_cpu_stop(sc); /* stop NPE */ + mtx_lock(&sc->sc_mtx); + error = npe_cpu_stop(sc); /* stop NPE */ if (error == 0) error = npe_cpu_reset(sc); /* reset it */ if (error == 0) - sc->started = 0; /* mark stopped */ + sc->started = 0; /* mark stopped */ + mtx_unlock(&sc->sc_mtx); + DPRINTF(sc->sc_dev, "%s: error %d\n", __func__, error); return error; } @@ -319,12 +334,15 @@ { int error; + mtx_lock(&sc->sc_mtx); if (!sc->started) { error = npe_cpu_start(sc); if (error == 0) sc->started = 1; } else error = 0; + mtx_unlock(&sc->sc_mtx); + DPRINTF(sc->sc_dev, "%s: error %d\n", __func__, error); return error; } @@ -334,9 +352,12 @@ { int error; + mtx_lock(&sc->sc_mtx); error = npe_cpu_stop(sc); if (error == 0) sc->started = 0; + mtx_unlock(&sc->sc_mtx); + DPRINTF(sc->sc_dev, "%s: error %d\n", __func__, error); return error; } @@ -425,6 +446,7 @@ * currently loaded images. If a critical error occured * during download, record that the NPE has an invalid image */ + mtx_lock(&sc->sc_mtx); error = npe_load_image(sc, imageCodePtr, 1 /*VERIFY*/); if (error == 0) { sc->validImage = 1; @@ -433,6 +455,7 @@ sc->validImage = 0; } sc->functionalityId = IX_NPEDL_FUNCTIONID_FROM_IMAGEID_GET(imageId); + mtx_unlock(&sc->sc_mtx); done: firmware_put(fw, FIRMWARE_UNLOAD); DPRINTF(sc->sc_dev, "%s: error %d\n", __func__, error); @@ -808,7 +831,7 @@ #undef N } -int +static int npe_cpu_start(struct ixpnpe_softc *sc) { uint32_t ecsRegVal; @@ -1214,27 +1237,18 @@ } } -#if 0 -static void -ixpnpe_intr(void *arg) -{ - struct ixpnpe_softc *sc = arg; - uint32_t status; - - status = npe_reg_read(sc, IX_NPESTAT); - device_printf(sc->sc_dev, "%s: status 0x%x\n", __func__, status); -} -#endif - +/* + * NPE Mailbox support. + */ #define IX_NPEMH_MAXTRIES 100000 static int -ixpnpe_ififo_wait(struct ixpnpe_softc *sc) +ixpnpe_ofifo_wait(struct ixpnpe_softc *sc) { int i; for (i = 0; i < IX_NPEMH_MAXTRIES; i++) { - if (npe_reg_read(sc, IX_NPESTAT) & IX_NPESTAT_IFNF) + if (npe_reg_read(sc, IX_NPESTAT) & IX_NPESTAT_OFNE) return 1; DELAY(10); } @@ -1243,54 +1257,102 @@ return 0; } -int -ixpnpe_sendmsg(struct ixpnpe_softc *sc, const uint32_t msg[2]) +static void +ixpnpe_intr(void *arg) { + struct ixpnpe_softc *sc = arg; + uint32_t status; - if (!ixpnpe_ififo_wait(sc)) - return EIO; - npe_reg_write(sc, IX_NPEFIFO, msg[0]); - if (!ixpnpe_ififo_wait(sc)) - return EIO; - npe_reg_write(sc, IX_NPEFIFO, msg[1]); - return 0; + status = npe_reg_read(sc, IX_NPESTAT); + if ((status & IX_NPESTAT_OFINT) == 0) { + /* NB: should not happen */ + device_printf(sc->sc_dev, "%s: status 0x%x\n", __func__, status); + /* XXX must silence interrupt? */ + return; + } + /* + * A message is waiting in the output FIFO, copy it so + * the interrupt will be silenced; then signal anyone + * waiting to collect the result. + */ + sc->sc_msgwaiting = -1; /* NB: error indicator */ + if (ixpnpe_ofifo_wait(sc)) { + sc->sc_msg[0] = npe_reg_read(sc, IX_NPEFIFO); + if (ixpnpe_ofifo_wait(sc)) { + sc->sc_msg[1] = npe_reg_read(sc, IX_NPEFIFO); + sc->sc_msgwaiting = 1; /* successful fetch */ + } + } + wakeup_one(sc); } static int -ixpnpe_ofifo_wait(struct ixpnpe_softc *sc) +ixpnpe_ififo_wait(struct ixpnpe_softc *sc) { int i; for (i = 0; i < IX_NPEMH_MAXTRIES; i++) { - if (npe_reg_read(sc, IX_NPESTAT) & IX_NPESTAT_OFNE) + if (npe_reg_read(sc, IX_NPESTAT) & IX_NPESTAT_IFNF) return 1; DELAY(10); } - device_printf(sc->sc_dev, "%s: timeout, last status 0x%x\n", - __func__, npe_reg_read(sc, IX_NPESTAT)); return 0; } -int -ixpnpe_recvmsg(struct ixpnpe_softc *sc, uint32_t msg[2]) +static int +ixpnpe_sendmsg_locked(struct ixpnpe_softc *sc, const uint32_t msg[2]) +{ + int error = 0; + + mtx_assert(&sc->sc_mtx, MA_OWNED); + + sc->sc_msgwaiting = 0; + if (ixpnpe_ififo_wait(sc)) { + npe_reg_write(sc, IX_NPEFIFO, msg[0]); + if (ixpnpe_ififo_wait(sc)) + npe_reg_write(sc, IX_NPEFIFO, msg[1]); + else + error = EIO; + } else + error = EIO; + + if (error) + device_printf(sc->sc_dev, "input FIFO timeout, msg [0x%x,0x%x]\n", + msg[0], msg[1]); + return error; +} + +static int +ixpnpe_recvmsg_locked(struct ixpnpe_softc *sc, uint32_t msg[2]) { - if (!ixpnpe_ofifo_wait(sc)) - return EIO; - msg[0] = npe_reg_read(sc, IX_NPEFIFO); - if (!ixpnpe_ofifo_wait(sc)) - return EIO; - msg[1] = npe_reg_read(sc, IX_NPEFIFO); - return 0; + mtx_assert(&sc->sc_mtx, MA_OWNED); + + if (!sc->sc_msgwaiting) + msleep(sc, &sc->sc_mtx, 0, "npemh", 0); + bcopy(sc->sc_msg, msg, sizeof(sc->sc_msg)); + /* NB: sc_msgwaiting != 1 means the ack fetch failed */ + return sc->sc_msgwaiting != 1 ? EIO : 0; } +/* + * Send a msg to the NPE and wait for a reply. We use the + * private mutex and sleep until an interrupt is received + * signalling the availability of data in the output FIFO + * so the caller cannot be holding a mutex. May be better + * piggyback on the caller's mutex instead but that would + * make other locking confusing. + */ int ixpnpe_sendandrecvmsg(struct ixpnpe_softc *sc, const uint32_t send[2], uint32_t recv[2]) { - int status; + int error; + + mtx_lock(&sc->sc_mtx); + error = ixpnpe_sendmsg_locked(sc, send); + if (error == 0) + error = ixpnpe_recvmsg_locked(sc, recv); + mtx_unlock(&sc->sc_mtx); - status = ixpnpe_sendmsg(sc, send); - if (status == 0) - status = ixpnpe_recvmsg(sc, recv); - return status; + return error; } ==== //depot/projects/arm/src/sys/arm/xscale/ixp425/ixp425_npevar.h#3 (text+ko) ==== @@ -87,8 +87,6 @@ const char *imageName, uint32_t imageId); int ixpnpe_getfunctionality(struct ixpnpe_softc *sc); -int ixpnpe_sendmsg(struct ixpnpe_softc *, const uint32_t msg[2]); -int ixpnpe_recvmsg(struct ixpnpe_softc *, uint32_t msg[2]); int ixpnpe_sendandrecvmsg(struct ixpnpe_softc *, const uint32_t send[2], uint32_t recv[2]); #endif /* _IXP425_NPEVAR_H_ */
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200611070133.kA71XlK7090336>