[U-Boot] [PATCH 14/14][v2] driver/ldpaa_eth:Avoid infinite loop in QBMAN buf release

[Joe Hershberger]

Hi Prabhakar,

On Thu, Jun 11, 2015 at 4:03 AM, Prabhakar Kushwaha
<prabhakar at freescale.com> wrote:
> Change infinite loop mechanism to timer based polling.
>
> Signed-off-by: Prabhakar Kushwaha <prabhakar at freescale.com>
> ---
> Changes for v2: Sending as it is for patchset
>
>  drivers/net/ldpaa_eth/ldpaa_eth.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ldpaa_eth/ldpaa_eth.c b/drivers/net/ldpaa_eth/ldpaa_eth.c
> index 839e78a..fe8f189 100644
> --- a/drivers/net/ldpaa_eth/ldpaa_eth.c
> +++ b/drivers/net/ldpaa_eth/ldpaa_eth.c
> @@ -31,6 +31,8 @@ static void ldpaa_eth_rx(struct ldpaa_eth_priv *priv,
>         uint32_t fd_length;
>         struct ldpaa_fas *fas;
>         uint32_t status, err;
> +       u32 timeo = (CONFIG_SYS_HZ * 2) / 1000;
> +       u32 time_start;
>         struct qbman_release_desc releasedesc;
>         struct qbman_swp *swp = dflt_dpio->sw_portal;
>
> @@ -65,10 +67,15 @@ error:
>         flush_dcache_range(fd_addr, fd_addr + LDPAA_ETH_RX_BUFFER_SIZE);
>         qbman_release_desc_clear(&releasedesc);
>         qbman_release_desc_set_bpid(&releasedesc, dflt_dpbp->dpbp_attr.bpid);
> +       time_start = get_timer(0);
>         do {
>                 /* Release buffer into the QBMAN */
>                 err = qbman_swp_release(swp, &releasedesc, &fd_addr, 1);
> -       } while (err == -EBUSY);
> +       } while (get_timer(time_start) < timeo && err == -EBUSY);
> +
> +       if (get_timer(time_start) >= timeo)

Can't you just check the value of err here instead of calling this
function again? That would also eliminate the race condition.

> +               printf("Rx frame: QBMAN buffer release fails\n");
> +
>         return;
>  }
>
> @@ -195,11 +202,13 @@ static int ldpaa_eth_tx(struct eth_device *net_dev, void *buf, int len)
>  error:
>         qbman_release_desc_clear(&releasedesc);
>         qbman_release_desc_set_bpid(&releasedesc, dflt_dpbp->dpbp_attr.bpid);
> +       time_start = get_timer(0);
>         do {
>                 /* Release buffer into the QBMAN */
>                 err = qbman_swp_release(swp, &releasedesc, &buffer_start, 1);
> -       } while (err == -EBUSY);
> +       } while (get_timer(time_start) < timeo && err == -EBUSY);

It looks like you are addressing the infinite loop you added in a
previous patch. Instead of making it tag along with this patch, you
should squash the _tx side into that original patch.

>
> +       printf("TX data: QBMAN buffer release fails\n");

This message seems wrong. The release didn't necessarily fail. Only
the qbman_swp_dqrr_next() definitely failed here.

>         return err;
>  }
>
> --
> 1.9.1
>
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot