Re: [PATCH v2] b44: fix misalignment and wasted space in rx handling

[Michael Buesch <mb@bu3sch.de>]

On Friday 09 January 2009 13:39:57 Felix Fietkau wrote:
> --- a/drivers/net/b44.c
> +++ b/drivers/net/b44.c
> @@ -73,8 +73,8 @@
>  	  (BP)->tx_cons - (BP)->tx_prod - TX_RING_GAP(BP))
>  #define NEXT_TX(N)		(((N) + 1) & (B44_TX_RING_SIZE - 1))
>  
> -#define RX_PKT_OFFSET		30
> -#define RX_PKT_BUF_SZ		(1536 + RX_PKT_OFFSET + 64)
> +#define RX_PKT_OFFSET		(RX_HEADER_LEN + 2)
> +#define RX_PKT_BUF_SZ		(1536 + RX_PKT_OFFSET)
>  
>  /* minimum number of free TX descriptors required to wake up TX process */
>  #define B44_TX_WAKEUP_THRESH		(B44_TX_RING_SIZE / 4)
> @@ -682,7 +682,6 @@ static int b44_alloc_rx_skb(struct b44 *
>  	}
>  
>  	rh = (struct rx_header *) skb->data;
> -	skb_reserve(skb, RX_PKT_OFFSET);

Looks correct.

>  	rh->len = 0;
>  	rh->flags = 0;
> @@ -693,13 +692,13 @@ static int b44_alloc_rx_skb(struct b44 *
>  	if (src_map != NULL)
>  		src_map->skb = NULL;
>  
> -	ctrl  = (DESC_CTRL_LEN & (RX_PKT_BUF_SZ - RX_PKT_OFFSET));
> +	ctrl = (DESC_CTRL_LEN & RX_PKT_BUF_SZ);

Are you sure this is right? b43 has the same DMA engine, and we
subtract the offset from the buffer size there. So we end up with
the descriptor control field telling the space available for the frame payload.

So I think the correct code would be
	ctrl = (DESC_CTRL_LEN & (RX_PKT_BUF_SZ - RX_PKT_OFFSET));

>  	if (dest_idx == (B44_RX_RING_SIZE - 1))
>  		ctrl |= DESC_CTRL_EOT;
>  
>  	dp = &bp->rx_ring[dest_idx];
>  	dp->ctrl = cpu_to_le32(ctrl);
> -	dp->addr = cpu_to_le32((u32) mapping + RX_PKT_OFFSET + bp->dma_offset);
> +	dp->addr = cpu_to_le32((u32) mapping + bp->dma_offset);

This looks correct.

-- 
Greetings, Michael.