[PATCH] bcmgenet: fix DMA buffer management

[Jason Wessel]

On 7/9/20 3:11 AM, etienne.duble at gmail.com wrote:
> From: Etienne Dublé <etienne.duble at imag.fr>
> 
> This commit fixes a serious issue occuring when several network
> commands are run on a raspberry pi 4 board: for instance a "dhcp"
> command and then one or several "tftp" commands. In this case,
> packet recv callbacks were called several times on the same packets,
> and send function was failing most of the time.
> 
> note: if the boot procedure is made of a single network
> command, the issue is not visible.
> 
> The issue is related to management of the packet ring buffers
> (producer / consumer) and DMA.
> Each time a packet is received, the ethernet device stores it
> in the buffer and increments an index called RDMA_PROD_INDEX.
> Each time the driver outputs a received packet, it increments
> another index called RDMA_CONS_INDEX.
> 
> Between each pair of network commands, as part of the driver
> 'start' function, previous code tried to reset both RDMA_CONS_INDEX
> and RDMA_PROD_INDEX to 0. But RDMA_PROD_INDEX cannot be written from
> driver side, thus its value was actually not updated, and only
> RDMA_CONS_INDEX was reset to 0. This was resulting in a major
> synchronization issue between the driver and the device. Most
> visible bahavior was that the driver seemed to receive again the
> packets from the previous commands (e.g. DHCP response packets
> "received" again when performing the first TFTP command).
> 
> This fix consists in setting RDMA_CONS_INDEX to the same
> value as RDMA_PROD_INDEX, when resetting the driver.
> 
> The same kind of fix was needed on the TX side, and a few variables
> had to be reset accordingly (c_index, tx_index, rx_index).


While there is some kind of problem with the driver, because I too
have observed a problem with multiple requests timing out or failing,
this patch makes the problem much worse.  I was only able to complete
a single tftp request. 

In my case I am using a static IP address and serverip. 

Also your patch was missing the sign-off line.  Please consider
running your patches through scripts/checkpatch.pl.

Cheers,
Jason.

> ---
>  drivers/net/bcmgenet.c | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/bcmgenet.c b/drivers/net/bcmgenet.c
> index 11b6148ab6..a4facfd63f 100644
> --- a/drivers/net/bcmgenet.c
> +++ b/drivers/net/bcmgenet.c
> @@ -378,8 +378,6 @@ static void rx_descs_init(struct bcmgenet_eth_priv *priv)
>  	u32 len_stat, i;
>  	void *desc_base = priv->rx_desc_base;
>  
> -	priv->c_index = 0;
> -
>  	len_stat = (RX_BUF_LENGTH << DMA_BUFLENGTH_SHIFT) | DMA_OWN;
>  
>  	for (i = 0; i < RX_DESCS; i++) {
> @@ -403,8 +401,10 @@ static void rx_ring_init(struct bcmgenet_eth_priv *priv)
>  	writel(RX_DESCS * DMA_DESC_SIZE / 4 - 1,
>  	       priv->mac_reg + RDMA_RING_REG_BASE + DMA_END_ADDR);
>  
> -	writel(0x0, priv->mac_reg + RDMA_PROD_INDEX);
> -	writel(0x0, priv->mac_reg + RDMA_CONS_INDEX);
> +	/* cannot init RDMA_PROD_INDEX to 0, so align RDMA_CONS_INDEX on it instead */
> +	priv->c_index = readl(priv->mac_reg + RDMA_PROD_INDEX);
> +	writel(priv->c_index, priv->mac_reg + RDMA_CONS_INDEX);
> +	priv->rx_index = priv->c_index;
>  	writel((RX_DESCS << DMA_RING_SIZE_SHIFT) | RX_BUF_LENGTH,
>  	       priv->mac_reg + RDMA_RING_REG_BASE + DMA_RING_BUF_SIZE);
>  	writel(DMA_FC_THRESH_VALUE, priv->mac_reg + RDMA_XON_XOFF_THRESH);
> @@ -421,8 +421,9 @@ static void tx_ring_init(struct bcmgenet_eth_priv *priv)
>  	writel(0x0, priv->mac_reg + TDMA_WRITE_PTR);
>  	writel(TX_DESCS * DMA_DESC_SIZE / 4 - 1,
>  	       priv->mac_reg + TDMA_RING_REG_BASE + DMA_END_ADDR);
> -	writel(0x0, priv->mac_reg + TDMA_PROD_INDEX);
> -	writel(0x0, priv->mac_reg + TDMA_CONS_INDEX);
> +	/* cannot init TDMA_CONS_INDEX to 0, so align TDMA_PROD_INDEX on it instead */
> +	priv->tx_index = readl(priv->mac_reg + TDMA_CONS_INDEX);
> +	writel(priv->tx_index, priv->mac_reg + TDMA_PROD_INDEX);
>  	writel(0x1, priv->mac_reg + TDMA_RING_REG_BASE + DMA_MBUF_DONE_THRESH);
>  	writel(0x0, priv->mac_reg + TDMA_FLOW_PERIOD);
>  	writel((TX_DESCS << DMA_RING_SIZE_SHIFT) | RX_BUF_LENGTH,
> @@ -469,8 +470,6 @@ static int bcmgenet_gmac_eth_start(struct udevice *dev)
>  
>  	priv->tx_desc_base = priv->mac_reg + GENET_TX_OFF;
>  	priv->rx_desc_base = priv->mac_reg + GENET_RX_OFF;
> -	priv->tx_index = 0x0;
> -	priv->rx_index = 0x0;
>  
>  	bcmgenet_umac_reset(priv);
>  
>