Re: [PATCH v3 1/2] net: macb: fix dma_alloc for rx_buffer

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

On 29.11.23 12:33, Steffen Trumtrar wrote:
> @@ -181,14 +189,15 @@ static int gem_recv(struct eth_device *edev)
>  		barrier();
>  		status = macb->rx_ring[macb->rx_tail].ctrl;
>  		length = MACB_BFEXT(RX_FRMLEN, status);
> -		buffer = macb->rx_buffer + macb->rx_buffer_size * macb->rx_tail;
> -		dma_sync_single_for_cpu(macb->dev, (unsigned long)buffer, length,
> -					DMA_FROM_DEVICE);
> -		net_receive(edev, buffer, length);
> -		dma_sync_single_for_device(macb->dev, (unsigned long)buffer, length,
> -					   DMA_FROM_DEVICE);
>  		macb->rx_ring[macb->rx_tail].addr &= ~MACB_BIT(RX_USED);
>  		barrier();

Is moving above two lines around intentional?

> +		dma_sync_single_for_cpu(macb->dev,
> +					macb->rx_buffer_phys + macb->rx_buffer_size * macb->rx_tail,
> +					length,	DMA_FROM_DEVICE);
> +		net_receive(edev, macb->rx_buffer + macb->rx_buffer_size * macb->rx_tail, length);
> +		dma_sync_single_for_device(macb->dev,
> +					   macb->rx_buffer_phys + macb->rx_buffer_size * macb->rx_tail,
> +					   length, DMA_FROM_DEVICE);
>  
>  		macb->rx_tail++;
>  		if (macb->rx_tail >= macb->rx_ring_size)
> @@ -202,7 +211,6 @@ static int macb_recv(struct eth_device *edev)
>  {
>  	struct macb_device *macb = edev->priv;
>  	unsigned int rx_tail = macb->rx_tail;
> -	void *buffer;
>  	int length;
>  	int wrapped = 0;
>  	u32 status;
> @@ -221,7 +229,6 @@ static int macb_recv(struct eth_device *edev)
>  		}
>  
>  		if (status & MACB_BIT(RX_EOF)) {
> -			buffer = macb->rx_buffer + macb->rx_buffer_size * macb->rx_tail;

Nitpick: reworking this line would lead to less verbose changes below.


Cheers,
Ahmad

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |