diff --git a/drv/stm32h7-eth/src/lib.rs b/drv/stm32h7-eth/src/lib.rs index 79e8f1a371..e0ad1c6a31 100644 --- a/drv/stm32h7-eth/src/lib.rs +++ b/drv/stm32h7-eth/src/lib.rs @@ -149,14 +149,16 @@ impl Ethernet { dma.dmacrx_rlr .write(|w| unsafe { w.rdrl().bits(rx_ring_len) }); - // Poke both tail pointers so the hardware looks at the descriptors. We - // completely initialize the descriptor array, so the tail pointer is - // always the end. + // Poke the receive tail pointer so that the hardware looks at + // descriptors. We completely initialize the descriptor array, so the + // tail pointer is always as close to the end as we can make it. // // Doing the same drop-bottom-two-bits stuff that we had to do for DLARs // above. - dma.dmactx_dtpr - .write(|w| unsafe { w.tdt().bits(tx_ring.tail_ptr() as u32 >> 2) }); + // + // We don't set the transmit tail pointer until we enqueue a packet, + // as we don't want the hardware to race against software filling in + // descriptors. dma.dmacrx_dtpr .write(|w| unsafe { w.rdt().bits(rx_ring.tail_ptr() as u32 >> 2) }); @@ -334,7 +336,7 @@ impl Ethernet { // Poke the tail pointer so the hardware knows to recheck (dropping two // bottom bits because svd2rust) self.dma.dmactx_dtpr.write(|w| unsafe { - w.tdt().bits(self.tx_ring.tail_ptr() as u32 >> 2) + w.tdt().bits(self.tx_ring.next_tail_ptr() as u32 >> 2) }); } diff --git a/drv/stm32h7-eth/src/ring.rs b/drv/stm32h7-eth/src/ring.rs index 733e6a5fd4..d97c442223 100644 --- a/drv/stm32h7-eth/src/ring.rs +++ b/drv/stm32h7-eth/src/ring.rs @@ -144,12 +144,12 @@ impl TxRing { // ensure that they're owned by us (not the hardware). for desc in storage { #[cfg(not(feature = "vlan"))] - desc.tdes[3].store(0, Ordering::Release); + desc.tdes[3].store(0, Ordering::Relaxed); #[cfg(feature = "vlan")] { - desc.tdes[0][3].store(0, Ordering::Release); - desc.tdes[1][3].store(0, Ordering::Release); + desc.tdes[0][3].store(0, Ordering::Relaxed); + desc.tdes[1][3].store(0, Ordering::Relaxed); } } Self { @@ -165,12 +165,10 @@ impl TxRing { self.storage.as_ptr() } - /// Returns a pointer to the byte just past the end of the `TxDesc` ring. - /// This too gets loaded into the DMA controller, so that it knows what - /// section of the ring is initialized and can be read. (The answer is "all - /// of it.") - pub fn tail_ptr(&self) -> *const TxDesc { - self.storage.as_ptr_range().end + /// Returns a pointer to the "next" descriptor in the ring. We load this + /// into the device so that the DMA engine knows what descriptors are free. + pub fn next_tail_ptr(&self) -> *const TxDesc { + self.base_ptr().wrapping_add(self.next.get()) } } @@ -247,7 +245,7 @@ impl TxRing { | 1 << TDES3_LD_BIT | TDES3_CIC_CHECKSUMS_ENABLED << TDES3_CIC_BIT | len as u32; - d.tdes[3].store(tdes3, Ordering::Release); // <-- release + d.tdes[3].store(tdes3, Ordering::Relaxed); self.next.set(if self.next.get() + 1 == self.storage.len() { 0 @@ -322,7 +320,7 @@ impl TxRing { | 1 << TDES3_CTXT_BIT | 1 << TDES3_VLTV_BIT | u32::from(vid); - d.tdes[0][3].store(tdes3, Ordering::Release); // <-- release + d.tdes[0][3].store(tdes3, Ordering::Relaxed); // Program the tx descriptor to represent the packet, using the // same strategy as above for memory access ordering. @@ -335,7 +333,7 @@ impl TxRing { | 1 << TDES3_LD_BIT | TDES3_CIC_CHECKSUMS_ENABLED << TDES3_CIC_BIT | len as u32; - d.tdes[1][3].store(tdes3, Ordering::Release); // <-- release + d.tdes[1][3].store(tdes3, Ordering::Relaxed); self.next.set(if self.next.get() + 1 == self.storage.len() { 0