Re: [PATCH net-next v9 2/4] net: rnpgbe: Add basic TX packet transmission support

[Simon Horman <horms@kernel.org>]

On Wed, Aug 19, 2026 at 04:49:04PM +0800, Yibo Dong wrote:

...

> > > @@ -633,13 +758,144 @@ static void rnpgbe_napi_disable_all(struct mucse *mucse)
> > > +static void rnpgbe_stop_tx_ring(struct mucse_ring *tx_ring)
> > > +{
> > > +	if (!tx_ring->tx_buffer_info)
> > > +		return;
> > > +
> > > +	/* Stop hw. No new descriptors are fetched after TX_START=0.
> > > +	 * DMA for descriptors fetched before the stop may still be in flight.
> > > +	 */
> > > +	mucse_ring_wr32(tx_ring, RNPGBE_TX_START, 0);
> > > +	/* Flush posted write to ensure hardware sees TX_START=0 */
> > > +	(void)mucse_ring_rd32(tx_ring, RNPGBE_TX_START);
> > > +}
> > > +
> > > +static void rnpgbe_wait_tx_dma_idle(struct mucse *mucse)
> > > +{
> > > +	struct mucse_hw *hw = &mucse->hw;
> > > +	u32 dma_status;
> > > +	int err;
> > > +
> > > +	err = readl_poll_timeout(hw->hw_addr + RNPGBE_DMA_STATUS,
> > > +				 dma_status,
> > > +				 (dma_status & RNPGBE_DMA_TX_STATUS) ==
> > > +				 RNPGBE_DMA_TX_STATUS,
> > > +				 10, 100000);
> > > +	if (err)
> > > +		dev_warn(&mucse->pdev->dev,
> > > +			 "Timed out waiting for TX DMA to quiesce, status %#x\n",
> > > +			 dma_status);
> > > +}
> > 
> > [Severity: Medium]
> > [Finding: gpt-5-6-sol-4-6]
> > [Sources: gpt-5-6-sol, opus-5]
> > Should the timeout here be propagated to the callers?
> > 
> > rnpgbe_wait_tx_dma_idle() returns void, so a timeout is only logged and
> > both callers continue as if the engine had confirmed idle. On the ifdown
> > side that means rnpgbe_clean_all_tx_rings() clears TX_AXI_RW_EN and then
> > rnpgbe_clean_tx_ring() calls dma_unmap_single()/dma_unmap_page() on every
> > in-flight TX buffer, and rnpgbe_close() -> rnpgbe_free_all_tx_resources()
> > -> rnpgbe_free_tx_resources() then dma_free_coherent()s the descriptor
> > ring.
> > 
> > The comment in rnpgbe_stop_tx_ring() states that DMA for descriptors
> > fetched before the stop may still be in flight, which is what this poll
> > is guarding. If the poll times out, can the device still be reading the
> > mapped buffers or writing back into the descriptor ring after they are
> > released?
> > 
> > The same unchecked wait sits at the head of rnpgbe_configure_tx(), where
> > RNPGBE_TX_BASE_ADDR_LO/HI, RNPGBE_TX_LEN and TAIL are reprogrammed and
> > TX_START is set to 1 again. Is a device reset or an error return needed
> > when the engine never reports idle?
> > 
> After TX_START has been cleared for all TX rings, normal outstanding
> DMA transactions reach idle well within 100 ms. If DMA_STATUS still does
> not report idle after that timeout, the hardware has entered an
> unrecoverable AXI fault state. In this state, the EP will not issue any
> further DMA accesses to RC memory, so teardown can safely release the DMA
> mappings. Recovery requires a chip-level reset.
> I will make the helper return an error. On this error, the driver will
> report it with dev_err() and will not enable or reprogram TX DMA again;
> the configuration/open path will fail instead. Does this approach sound
> reasonable?

Yes, sounds good.
Thanks.