Re: [PATCH net v4 3/5] net: macb: move ring size computation to functions

[Nicolas Ferre <nicolas.ferre@microchip.com>]

On 20/08/2025 at 16:55, Théo Lebrun wrote:
> The tx/rx ring size calculation is somewhat complex and partially hidden
> behind a macro. Move that out of the {RX,TX}_RING_BYTES() macros and
> macb_{alloc,free}_consistent() functions into neat separate functions.

I agree.

> In macb_free_consistent(), we drop the size variable and directly call
> the size helpers in the arguments list. In macb_alloc_consistent(), we
> keep the size variable that is used by netdev_dbg() calls.
> 
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>

Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>

> ---
>   drivers/net/ethernet/cadence/macb_main.c | 27 ++++++++++++++++-----------
>   1 file changed, 16 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index 69325665c766927797ca2e1eb1384105bcde3cb5..d413e8bd4977187fd73f7cc48268baf933aab051 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -51,14 +51,10 @@ struct sifive_fu540_macb_mgmt {
>   #define DEFAULT_RX_RING_SIZE   512 /* must be power of 2 */
>   #define MIN_RX_RING_SIZE       64
>   #define MAX_RX_RING_SIZE       8192
> -#define RX_RING_BYTES(bp)      (macb_dma_desc_get_size(bp)     \
> -                                * (bp)->rx_ring_size)
> 
>   #define DEFAULT_TX_RING_SIZE   512 /* must be power of 2 */
>   #define MIN_TX_RING_SIZE       64
>   #define MAX_TX_RING_SIZE       4096
> -#define TX_RING_BYTES(bp)      (macb_dma_desc_get_size(bp)     \
> -                                * (bp)->tx_ring_size)
> 
>   /* level of occupied TX descriptors under which we wake up TX process */
>   #define MACB_TX_WAKEUP_THRESH(bp)      (3 * (bp)->tx_ring_size / 4)
> @@ -2466,11 +2462,20 @@ static void macb_free_rx_buffers(struct macb *bp)
>          }
>   }
> 
> +static unsigned int macb_tx_ring_size_per_queue(struct macb *bp)
> +{
> +       return macb_dma_desc_get_size(bp) * bp->tx_ring_size + bp->tx_bd_rd_prefetch;
> +}
> +
> +static unsigned int macb_rx_ring_size_per_queue(struct macb *bp)
> +{
> +       return macb_dma_desc_get_size(bp) * bp->rx_ring_size + bp->rx_bd_rd_prefetch;
> +}
> +
>   static void macb_free_consistent(struct macb *bp)
>   {
>          struct macb_queue *queue;
>          unsigned int q;
> -       int size;
> 
>          if (bp->rx_ring_tieoff) {
>                  dma_free_coherent(&bp->pdev->dev, macb_dma_desc_get_size(bp),
> @@ -2484,14 +2489,14 @@ static void macb_free_consistent(struct macb *bp)
>                  kfree(queue->tx_skb);
>                  queue->tx_skb = NULL;
>                  if (queue->tx_ring) {
> -                       size = TX_RING_BYTES(bp) + bp->tx_bd_rd_prefetch;
> -                       dma_free_coherent(&bp->pdev->dev, size,
> +                       dma_free_coherent(&bp->pdev->dev,
> +                                         macb_tx_ring_size_per_queue(bp),
>                                            queue->tx_ring, queue->tx_ring_dma);
>                          queue->tx_ring = NULL;
>                  }
>                  if (queue->rx_ring) {
> -                       size = RX_RING_BYTES(bp) + bp->rx_bd_rd_prefetch;
> -                       dma_free_coherent(&bp->pdev->dev, size,
> +                       dma_free_coherent(&bp->pdev->dev,
> +                                         macb_rx_ring_size_per_queue(bp),
>                                            queue->rx_ring, queue->rx_ring_dma);
>                          queue->rx_ring = NULL;
>                  }
> @@ -2542,7 +2547,7 @@ static int macb_alloc_consistent(struct macb *bp)
>          int size;
> 
>          for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
> -               size = TX_RING_BYTES(bp) + bp->tx_bd_rd_prefetch;
> +               size = macb_tx_ring_size_per_queue(bp);
>                  queue->tx_ring = dma_alloc_coherent(&bp->pdev->dev, size,
>                                                      &queue->tx_ring_dma,
>                                                      GFP_KERNEL);
> @@ -2560,7 +2565,7 @@ static int macb_alloc_consistent(struct macb *bp)
>                  if (!queue->tx_skb)
>                          goto out_err;
> 
> -               size = RX_RING_BYTES(bp) + bp->rx_bd_rd_prefetch;
> +               size = macb_rx_ring_size_per_queue(bp);
>                  queue->rx_ring = dma_alloc_coherent(&bp->pdev->dev, size,
>                                                      &queue->rx_ring_dma,
>                                                      GFP_KERNEL);
> 
> --
> 2.50.1
>