[v4] net: macb: various fixes

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

Posted by Théo Lebrun 1 month, 2 weeks ago

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.

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>
---
 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

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

Posted by Nicolas Ferre 1 month, 1 week ago

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
>

[PATCH net v4 1/5] dt-bindings: net: cdns,macb: allow tsu_clk without tx_clk
[PATCH net v4 2/5] net: macb: remove illusion about TBQPH/RBQPH being per-queue
[PATCH net v4 3/5] net: macb: move ring size computation to functions
[PATCH net v4 4/5] net: macb: single dma_alloc_coherent() for DMA descriptors
[PATCH net v4 5/5] net: macb: avoid double endianness swap in macb_set_hwaddr()