Re: [PATCH net v4 4/5] net: macb: single dma_alloc_coherent() for DMA descriptors

Hello Nicolas,

On Tue Aug 26, 2025 at 5:23 PM CEST, Nicolas Ferre wrote:

On 20/08/2025 at 16:55, Théo Lebrun wrote:

quoted

Move from 2*NUM_QUEUES dma_alloc_coherent() for DMA descriptor rings to
2 calls overall.

Issue is with how all queues share the same register for configuring the
upper 32-bits of Tx/Rx descriptor rings. Taking Tx, notice how TBQPH
does *not* depend on the queue index:

         #define GEM_TBQP(hw_q)          (0x0440 + ((hw_q) << 2))
         #define GEM_TBQPH(hw_q)         (0x04C8)

         queue_writel(queue, TBQP, lower_32_bits(queue->tx_ring_dma));
         #ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
         if (bp->hw_dma_cap & HW_DMA_CAP_64B)
                 queue_writel(queue, TBQPH, upper_32_bits(queue->tx_ring_dma));
         #endif

To maximise our chances of getting valid DMA addresses, we do a single
dma_alloc_coherent() across queues. This improves the odds because
alloc_pages() guarantees natural alignment. Other codepaths (IOMMU or
dev/arch dma_map_ops) don't give high enough guarantees
(even page-aligned isn't enough).

Two consideration:

  - dma_alloc_coherent() gives us page alignment. Here we remove this
    constraint meaning each queue's ring won't be page-aligned anymore.

However... We must guarantee alignement depending on the controller's 
bus width (32 or 64 bits)... but being page aligned and having 
descriptors multiple of 64 bits anyway, we're good for each descriptor 
(might be worth explicitly adding).

Sorry, your comment was unclear to me.

 - I don't see how we can guarantee bus alignment using
   dma_alloc_coherent() which doesn't ask for desired alignment. In
   what case can the DMA APIs return something with less than the
   tolerated bus alignment?

 - What does "having descriptors multiple of 64 bits anyway" mean?

Thanks for your review and acks! V5 got published here:
https://lore.kernel.org/lkml/20250910-macb-fixes-v5-0-f413a3601ce4@bootlin.com/ (local)

Regards,

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com