Re: CVS commit: src/sys/dev/i2o

[Manuel Bouyer <bouyer%antioche.eu.org@localhost>]

On Sun, Feb 21, 2010 at 11:52:13PM +0900, Izumi Tsutsui wrote:
> > Modified Files:
> >     src/sys/dev/i2o: iop.c
> > 
> > Log Message:
> > Fix bus_dmamap_sync(): it should be a BUS_DMASYNC_POSTWRITE after
> > writing to DMA memory.
> > Found by code inspection.
> 
> >     *sw = htole32(0);
> >     bus_dmamap_sync(sc->sc_dmat, sc->sc_scr_dmamap, 0, sizeof(*sw),
> > -       BUS_DMASYNC_PREWRITE|BUS_DMASYNC_PREREAD);
> > +       BUS_DMASYNC_POSTWRITE);
> 
> This looks wrong. POSTWRITE should be used after
> xfer from host to device is complete.
> 
> It looks bus_dmamap_sync() before *sw = htole32(0) statement
> should be POSTWRITE (for the previous xfer) instead.

Ops, you're right. I got it in the wrong order.
There's another place where it seems to be in the wrong order as well.

> 
> (POLL() macro with bus_dmamap_sync() seems also wrong..)

Hum, it looks correct to me. We are after the device wrote something
to RAM, so it's a POSTREAD operation, isn't it ?

Or do you mean it should be POSTREAD | POSTWRITE ? this would seems more
correct to me but it probably doesn't matters ...

Does the attached patch looks correct ?

-- 
Manuel Bouyer <bouyer%antioche.eu.org@localhost>
     NetBSD: 26 ans d'experience feront toujours la difference
--

Index: iop.c
===================================================================
RCS file: /cvsroot/src/sys/dev/i2o/iop.c,v
retrieving revision 1.79
diff -u -p -u -r1.79 iop.c
--- iop.c       21 Feb 2010 14:16:47 -0000      1.79
+++ iop.c       21 Feb 2010 16:21:47 -0000
@@ -990,17 +990,17 @@ iop_ofifo_init(struct iop_softc *sc)
        mb[0] += 2 << 16;
 
        bus_dmamap_sync(sc->sc_dmat, sc->sc_scr_dmamap, 0, sizeof(*sw),
-           BUS_DMASYNC_PREWRITE);
+           BUS_DMASYNC_POSTWRITE);
        *sw = 0;
        bus_dmamap_sync(sc->sc_dmat, sc->sc_scr_dmamap, 0, sizeof(*sw),
-           BUS_DMASYNC_POSTWRITE);
+           BUS_DMASYNC_PREWRITE | BUS_DMASYNC_PREREAD);
 
        if ((rv = iop_post(sc, mb)) != 0)
                return (rv);
 
        POLL(5000,
            (bus_dmamap_sync(sc->sc_dmat, sc->sc_scr_dmamap, 0, sizeof(*sw),
-           BUS_DMASYNC_POSTREAD),
+           BUS_DMASYNC_POSTWRITE | BUS_DMASYNC_POSTREAD),
            *sw == htole32(I2O_EXEC_OUTBOUND_INIT_COMPLETE)));
 
        if (*sw != htole32(I2O_EXEC_OUTBOUND_INIT_COMPLETE)) {
@@ -1535,17 +1535,17 @@ iop_reset(struct iop_softc *sc)
        mf.statushigh = (u_int32_t)((u_int64_t)pa >> 32);
 
        bus_dmamap_sync(sc->sc_dmat, sc->sc_scr_dmamap, 0, sizeof(*sw),
-           BUS_DMASYNC_PREWRITE);
+           BUS_DMASYNC_POSTWRITE);
        *sw = htole32(0);
        bus_dmamap_sync(sc->sc_dmat, sc->sc_scr_dmamap, 0, sizeof(*sw),
-           BUS_DMASYNC_POSTWRITE);
+           BUS_DMASYNC_PREWRITE|BUS_DMASYNC_PREREAD);
 
        if ((rv = iop_post(sc, (u_int32_t *)&mf)))
                return (rv);
 
        POLL(2500,
            (bus_dmamap_sync(sc->sc_dmat, sc->sc_scr_dmamap, 0, sizeof(*sw),
-           BUS_DMASYNC_POSTREAD), *sw != 0));
+           BUS_DMASYNC_POSTWRITE | BUS_DMASYNC_POSTREAD), *sw != 0));
        if (*sw != htole32(I2O_RESET_IN_PROGRESS)) {
                aprint_error_dev(&sc->sc_dv, "reset rejected, status 0x%x\n",
                    le32toh(*sw));