Skip to content

Commit

Permalink
stm32h7-eth: Properly maintain Tx tail pointer
Browse files Browse the repository at this point in the history
Analagous to #1493, we should maintain the Tx tail pointer
properly, as well.  Note that since the device won't examine a
Tx descriptor until the tail pointer is set appropriately, we
can relax stores and rely on the barrier in `tx_notify` to flush
everything to RAM properly before writing the tail pointer and
kicking off a DMA.
  • Loading branch information
dancrossnyc committed Aug 17, 2023
1 parent f04c427 commit 55af02b
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 18 deletions.
14 changes: 8 additions & 6 deletions drv/stm32h7-eth/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) });

Expand Down Expand Up @@ -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)
});
}

Expand Down
22 changes: 10 additions & 12 deletions drv/stm32h7-eth/src/ring.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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())
}
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand All @@ -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
Expand Down

0 comments on commit 55af02b

Please sign in to comment.