diff mbox

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

Message ID 1434013388-26605-14-git-send-email-prabhakar@freescale.com
State Superseded
Delegated to: York Sun
Headers show

Commit Message

Prabhakar Kushwaha June 11, 2015, 9:03 a.m. UTC
Change infinite loop mechanism to timer based polling.

Signed-off-by: Prabhakar Kushwaha <prabhakar@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(-)

Comments

Joe Hershberger June 25, 2015, 11:56 p.m. UTC | #1
Hi Prabhakar,

On Thu, Jun 11, 2015 at 4:03 AM, Prabhakar Kushwaha
<prabhakar@freescale.com> wrote:
> Change infinite loop mechanism to timer based polling.
>
> Signed-off-by: Prabhakar Kushwaha <prabhakar@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@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
diff mbox

Patch

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)
+		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);
 
+	printf("TX data: QBMAN buffer release fails\n");
 	return err;
 }