Re: [PATCH net-next v3 05/15] net: macb: enforce reverse christmas tree (RCT) convention

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

On 1.7.2026 17:59, Théo Lebrun wrote:
> Enforce the reverse christmas tree convention in those functions:
> 
>    macb_tx_error_task()
>    gem_rx_refill()
>    gem_rx()
>    macb_rx_frame()
>    macb_init_rx_ring()
>    macb_rx()
>    macb_rx_pending()
>    macb_start_xmit()
> 
> The goal is to minimise unrelated diff in future patches.
> 
> In macb_tx_error_task(), we fold the assignment into the declaration
> statement.
> 
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> ---
>  drivers/net/ethernet/cadence/macb_main.c | 61 
> ++++++++++++++++----------------
>  1 file changed, 30 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/net/ethernet/cadence/macb_main.c 
> b/drivers/net/ethernet/cadence/macb_main.c
> index 3b75797381b6..8b52122bc134 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -1254,20 +1254,19 @@ static dma_addr_t macb_get_addr(struct macb 
> *bp, struct macb_dma_desc *desc)
> 
>  static void macb_tx_error_task(struct work_struct *work)
>  {
> -	struct macb_queue	*queue = container_of(work, struct macb_queue,
> -						      tx_error_task);
> -	bool			halt_timeout = false;
> -	struct macb		*bp = queue->bp;
> -	unsigned int		q;
> -	u32			packets = 0;
> -	u32			bytes = 0;
> -	struct macb_tx_skb	*tx_skb;
> -	struct macb_dma_desc	*desc;
> -	struct sk_buff		*skb;
> -	unsigned int		tail;
> -	unsigned long		flags;
> +	struct macb_queue *queue = container_of(work, struct macb_queue,
> +						tx_error_task);
> +	unsigned int q = queue - queue->bp->queues;
> +	struct macb *bp = queue->bp;
> +	struct macb_tx_skb *tx_skb;
> +	struct macb_dma_desc *desc;
> +	bool halt_timeout = false;
> +	struct sk_buff *skb;
> +	unsigned long flags;
> +	unsigned int tail;
> +	u32 packets = 0;
> +	u32 bytes = 0;
> 
> -	q = queue - bp->queues;
>  	netdev_vdbg(bp->netdev, "macb_tx_error_task: q = %u, t = %u, h = 
> %u\n",
>  		    q, queue->tx_tail, queue->tx_head);
> 
> @@ -1487,11 +1486,11 @@ static int macb_tx_complete(struct macb_queue 
> *queue, int budget)
> 
>  static void gem_rx_refill(struct macb_queue *queue)
>  {
> -	unsigned int		entry;
> -	struct sk_buff		*skb;
> -	dma_addr_t		paddr;
>  	struct macb *bp = queue->bp;
>  	struct macb_dma_desc *desc;
> +	struct sk_buff *skb;
> +	unsigned int entry;
> +	dma_addr_t paddr;
> 
>  	while (CIRC_SPACE(queue->rx_prepared_head, queue->rx_tail,
>  			bp->rx_ring_size) > 0) {
> @@ -1584,11 +1583,11 @@ static int gem_rx(struct macb_queue *queue, 
> struct napi_struct *napi,
>  		  int budget)
>  {
>  	struct macb *bp = queue->bp;
> -	unsigned int		len;
> -	unsigned int		entry;
> -	struct sk_buff		*skb;
> -	struct macb_dma_desc	*desc;
> -	int			count = 0;
> +	struct macb_dma_desc *desc;
> +	struct sk_buff *skb;
> +	unsigned int entry;
> +	unsigned int len;
> +	int count = 0;
> 
>  	while (count < budget) {
>  		u32 ctrl;
> @@ -1674,12 +1673,12 @@ static int gem_rx(struct macb_queue *queue, 
> struct napi_struct *napi,
>  static int macb_rx_frame(struct macb_queue *queue, struct napi_struct 
> *napi,
>  			 unsigned int first_frag, unsigned int last_frag)
>  {
> -	unsigned int len;
> -	unsigned int frag;
> +	struct macb *bp = queue->bp;
> +	struct macb_dma_desc *desc;
>  	unsigned int offset;
>  	struct sk_buff *skb;
> -	struct macb_dma_desc *desc;
> -	struct macb *bp = queue->bp;
> +	unsigned int frag;
> +	unsigned int len;
> 
>  	desc = macb_rx_desc(queue, last_frag);
>  	len = desc->ctrl & bp->rx_frm_len_mask;
> @@ -1755,9 +1754,9 @@ static int macb_rx_frame(struct macb_queue 
> *queue, struct napi_struct *napi,
> 
>  static inline void macb_init_rx_ring(struct macb_queue *queue)
>  {
> +	struct macb_dma_desc *desc = NULL;
>  	struct macb *bp = queue->bp;
>  	dma_addr_t addr;
> -	struct macb_dma_desc *desc = NULL;
>  	int i;
> 
>  	addr = queue->rx_buffers_dma;
> @@ -1776,9 +1775,9 @@ static int macb_rx(struct macb_queue *queue, 
> struct napi_struct *napi,
>  {
>  	struct macb *bp = queue->bp;
>  	bool reset_rx_queue = false;
> -	int received = 0;
> -	unsigned int tail;
>  	int first_frag = -1;
> +	unsigned int tail;
> +	int received = 0;
> 
>  	for (tail = queue->rx_tail; budget > 0; tail++) {
>  		struct macb_dma_desc *desc = macb_rx_desc(queue, tail);
> @@ -1853,8 +1852,8 @@ static int macb_rx(struct macb_queue *queue, 
> struct napi_struct *napi,
>  static bool macb_rx_pending(struct macb_queue *queue)
>  {
>  	struct macb *bp = queue->bp;
> -	unsigned int		entry;
> -	struct macb_dma_desc	*desc;
> +	struct macb_dma_desc *desc;
> +	unsigned int entry;
> 
>  	entry = macb_rx_ring_wrap(bp, queue->rx_tail);
>  	desc = macb_rx_desc(queue, entry);
> @@ -2474,10 +2473,10 @@ static netdev_tx_t macb_start_xmit(struct 
> sk_buff *skb,
>  	unsigned int q = skb_get_queue_mapping(skb);
>  	unsigned int desc_cnt, nr_frags, frag_size, f;
>  	struct macb_queue *queue = &bp->queues[q];
> +	netdev_tx_t ret = NETDEV_TX_OK;
>  	unsigned int hdrlen;
>  	unsigned long flags;
>  	bool is_lso;
> -	netdev_tx_t ret = NETDEV_TX_OK;
> 
>  	if (macb_clear_csum(skb)) {
>  		dev_kfree_skb_any(skb);

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

Thanks
Nicolai