[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <2eedec51-d773-4a3f-a936-4752d768702a@microchip.com>
Date: Tue, 26 Aug 2025 11:30:52 +0200
From: Nicolas Ferre <nicolas.ferre@...rochip.com>
To: Théo Lebrun <theo.lebrun@...tlin.com>, Andrew Lunn
<andrew+netdev@...n.ch>, "David S. Miller" <davem@...emloft.net>, Eric
Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni
<pabeni@...hat.com>, Rob Herring <robh@...nel.org>, Krzysztof Kozlowski
<krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, Claudiu Beznea
<claudiu.beznea@...on.dev>, Geert Uytterhoeven <geert@...ux-m68k.org>, Harini
Katakam <harini.katakam@...inx.com>, Richard Cochran
<richardcochran@...il.com>, Russell King <linux@...linux.org.uk>
CC: <netdev@...r.kernel.org>, <devicetree@...r.kernel.org>,
<linux-kernel@...r.kernel.org>, Thomas Petazzoni
<thomas.petazzoni@...tlin.com>, Tawfik Bayouk <tawfik.bayouk@...ileye.com>
Subject: Re: [PATCH net v4 3/5] net: macb: move ring size computation to
functions
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@...tlin.com>
Acked-by: Nicolas Ferre <nicolas.ferre@...rochip.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
>
Powered by blists - more mailing lists