Re: [PATCH net-next v4 08/15] net: macb: avoid macb_init_rx_buffer_size() modifying state

[Nicolai Buchwitz <nb@tipi-net.de>]

Hi Théo

On 17.7.2026 21:48, Théo Lebrun wrote:
> macb_init_rx_buffer_size() takes the macb private data struct and
> overrides its bp->ctx->rx_buffer_size. To make it usable with multiple
> contexts, make it return its value.
> 
> Also, move the `bufsz` computation into it. The value is only used if
> GEM, and for historical reason it currently lives in macb_open().
> 
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> ---
>  drivers/net/ethernet/cadence/macb_main.c | 26 
> +++++++++++++-------------
>  1 file changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/net/ethernet/cadence/macb_main.c 
> b/drivers/net/ethernet/cadence/macb_main.c
> index d50d8520231c..29bf3c497f2a 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -2615,25 +2615,26 @@ static netdev_tx_t macb_start_xmit(struct 
> sk_buff *skb,
>  	return ret;
>  }
> 
> -static void macb_init_rx_buffer_size(struct macb *bp, size_t size)
> +static unsigned int macb_rx_buffer_size(struct macb *bp, unsigned int 
> mtu)
>  {
> -	if (!macb_is_gem(bp)) {
> -		bp->ctx->rx_buffer_size = MACB_RX_BUFFER_SIZE;
> -	} else {
> -		bp->ctx->rx_buffer_size = MIN(size, RX_BUFFER_MAX);
> +	unsigned int size;
> 
> -		if (bp->ctx->rx_buffer_size % RX_BUFFER_MULTIPLE) {
> +	if (!macb_is_gem(bp)) {
> +		size = MACB_RX_BUFFER_SIZE;
> +	} else {
> +		size = mtu + ETH_HLEN + ETH_FCS_LEN + NET_IP_ALIGN;
> +		size = MIN(size, RX_BUFFER_MAX);
> +
> +		if (size % RX_BUFFER_MULTIPLE) {
>  			netdev_dbg(bp->netdev,
>  				   "RX buffer must be multiple of %d bytes, expanding\n",
>  				   RX_BUFFER_MULTIPLE);
> -			bp->ctx->rx_buffer_size =
> -				roundup(bp->ctx->rx_buffer_size,
> -					RX_BUFFER_MULTIPLE);
> +			size = roundup(size, RX_BUFFER_MULTIPLE);
>  		}
>  	}
> 
> -	netdev_dbg(bp->netdev, "mtu [%u] rx_buffer_size [%u]\n",
> -		   bp->netdev->mtu, bp->ctx->rx_buffer_size);
> +	netdev_dbg(bp->netdev, "mtu [%u] rx_buffer_size [%u]\n", mtu, size);
> +	return size;
>  }
> 
>  static void gem_free_rx_buffers(struct macb *bp)
> @@ -3193,7 +3194,6 @@ static void macb_set_rx_mode(struct net_device 
> *netdev)
> 
>  static int macb_open(struct net_device *netdev)
>  {
> -	size_t bufsz = netdev->mtu + ETH_HLEN + ETH_FCS_LEN + NET_IP_ALIGN;
>  	struct macb *bp = netdev_priv(netdev);
>  	struct macb_queue *queue;
>  	unsigned int q;
> @@ -3212,7 +3212,7 @@ static int macb_open(struct net_device *netdev)
>  	}
> 
>  	/* RX buffers initialization */
> -	macb_init_rx_buffer_size(bp, bufsz);
> +	bp->ctx->rx_buffer_size = macb_rx_buffer_size(bp, netdev->mtu);
>  	bp->ctx->rx_ring_size = bp->configured_rx_ring_size;
>  	bp->ctx->tx_ring_size = bp->configured_tx_ring_size;

Reviewed-by: Nicolai Buchwitz <nb@tipi-net.de>

Thanks,
Nicolai