> 
> Le 05/05/2026 à 10:14, Holger Brunck a écrit :
> >>
> >> Le 04/05/2026 à 17:56, Holger Brunck a écrit :
> >>> If dma_rmb is used it has to be done after reading bd_status and
> >>> checking if R_E_S is zero. Therefore we need to move it into the while 
> >>> loop.
> >>
> >> Can you give more details ? Why does dma_rmb() has to be done after
> >> reading bd_status and checking if R_E_S is zero ?
> >>
> >
> > when R_E_S is zero in the status of the buffer descriptor it means the
> > buffer is filled with data from the device.  Now the CPU owns the
> > descriptor. Now we should execute the dma_rmb to be sure that we read the
> data correctly.
> > And this we need to redo for each buffer descriptor which is filled
> > with data, that’s why it must be done within the for loop and not before and
> after.
> 
> We enter hdlc_rx_done() after an interrupt which triggers scheduling of
> ucc_hdlc_poll(). I think dma_rmb() is needed _before_ reading the first 
> status,
> otherwise it might read an erroneous status.
> 

we end up in hdlc_rx_done also if a TX interrupt was triggered due to the NAPI 
layer.
If RX bd_status is still 1 we would read a pending RX packet next time.  So the 
dma_rmb()
before is not necessary. But if we have read a zero in bd_status we need to be 
sure that
the data written from the QE to RAM is fully available. That is why I think 
doing it after
reading the status is correct here.
 
> Once we are here the interrupt has been cleared so any new buffer will 
> trigger a
> new interrupt and call again this function. Therefore I don't think it is 
> worth the
> cost of a dma_rmb() inside the loop.
> 

Yes, but while we are in the for loop the QE might have written the next packet 
to
the memory in background and sets the bd_status to zero. And if this is the
case we need again make sure that the memory we read is correct.

For example the following driver does the same AFAIU.
drivers/net/ethernet/intel/e1000e/netdev.c  
from line 1524 ff

It first reads the status for the RX packet then does dma_rmb() and then reads
the data itself. And it is done within the while loop as any further available 
packet
may have arrived just now and we need to be sure that the data is valid. 

Best regards
Holger

Reply via email to