So I've been looking at the ral(4) driver a bit lately, and I notice
in the function rt2661_setup_tx_desc() in sys/dev/ic/rt2661.c, that
the first word of the TX descriptor is written first.  This seems like
a bad idea, since that word has flag bits that the driver calls BUSY
and VALID in it -- by writing that word first, it seems we might get
unlucky and have the hardware start executing the descriptor before
the driver has finished setting the rest of it up.

So a diff like the one below seems like a good idea.  However I'm not
very experienced with the OpenBSD kernel and I'm wondering what the
idiomatic way is to express the fact that we need to make sure that
neither the compiler nor an out-of-order CPU reorder the TX descriptor
writes either.  Or do we just not worry about this?

Thanks,
  Roland


 rt2661.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/rt2661.c b/rt2661.c
index 9a9cc53..f059f03 100644
--- a/rt2661.c
+++ b/rt2661.c
@@ -1365,7 +1365,6 @@ rt2661_setup_tx_desc(struct rt2661_softc *sc, struct 
rt2661_tx_desc *desc,
 
        desc->flags = htole32(flags);
        desc->flags |= htole32(len << 16);
-       desc->flags |= htole32(RT2661_TX_BUSY | RT2661_TX_VALID);
 
        desc->xflags = htole16(xflags);
        desc->xflags |= htole16(nsegs << 13);
@@ -1413,6 +1412,8 @@ rt2661_setup_tx_desc(struct rt2661_softc *sc, struct 
rt2661_tx_desc *desc,
                desc->addr[i] = htole32(segs[i].ds_addr);
                desc->len [i] = htole16(segs[i].ds_len);
        }
+
+       desc->flags |= htole32(RT2661_TX_BUSY | RT2661_TX_VALID);
 }
 
 int

Reply via email to