Re: [PATCH 2/2] net: macb: convert to volatile accesses

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

Hello Steffen,

On 28.11.23 17:29, Steffen Trumtrar wrote:
> +	writel(ctrl, &macb->tx_ring[tx_head].ctrl);
> +	writel((ulong)packet, &macb->tx_ring[tx_head].addr);
>  	dma_sync_single_for_device(macb->dev, (unsigned long)packet, length, DMA_TO_DEVICE);

For this buffer dma_map_single is missing. I just sent out a series to implement
CONFIG_DMA_API_DEBUG by the way that should catch this (as well as the original
issue in the Rx path).

> -	macb->gem_q1_descs[0].addr = 0;
> -	macb->gem_q1_descs[0].ctrl = MACB_BIT(TX_WRAP) |
> -		MACB_BIT(TX_LAST) | MACB_BIT(TX_USED);
> +	writel(0, &macb->gem_q1_descs[0].addr);
> +	setbits_le32(&macb->gem_q1_descs[0].ctrl,
> +		     MACB_BIT(TX_WRAP) | MACB_BIT(TX_LAST) | MACB_BIT(TX_USED));

Should be writel to maintain previous semantics.

> -	macb->tx_ring[TX_RING_SIZE - 1].addr |= MACB_BIT(TX_WRAP);
> +	writel(MACB_BIT(TX_WRAP), &macb->tx_ring[TX_RING_SIZE - 1].addr);

Should be a setbits_le32 to maintain previous semantics.

>  		/* Disable the second priority rx queue */
> -		macb->gem_q1_descs[1].addr = MACB_BIT(RX_USED) |
> -				MACB_BIT(RX_WRAP);
> -		macb->gem_q1_descs[1].ctrl = 0;
> +		setbits_le32(&macb->gem_q1_descs[1].addr,
> +			     MACB_BIT(RX_USED) | MACB_BIT(RX_WRAP));

Should be a writel to maintain previous semantics.

> +		writel(0, &macb->gem_q1_descs[1].ctrl);
>  
>  		gem_writel(macb, RQ1, (ulong)&macb->gem_q1_descs[1]);
>  	}
> 

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 |