On 7/9/20 3:11 AM, etienne.du...@gmail.com wrote:
> From: Etienne Dublé <etienne.du...@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);
>
>
