Message ID | 1509014002-29547-1-git-send-email-Ashish.Kumar@nxp.com |
---|---|
State | Accepted |
Commit | 5e9445da288c5121546235e8201768abd087b281 |
Delegated to: | York Sun |
Headers | show |
Series | [U-Boot] drivers: net: ldpaa_eth: Correct error handler for qbman_swp_acquire() | expand |
On 10/26/2017 03:33 AM, Ashish Kumar wrote: > Correcting error handing for qbman_swp_acquire. The return value is zero is > an error condition since number of buffer copied is zero meaning > there are no free buffers for allocation. > > Signed-off-by: Ashish Kumar <Ashish.Kumar@nxp.com> > Signed-off-by: Kushwaha Prabhakar <prabhakar@freescale.com> > --- > drivers/net/ldpaa_eth/ldpaa_eth.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/ldpaa_eth/ldpaa_eth.c b/drivers/net/ldpaa_eth/ldpaa_eth.c > index f235b62..21be79a 100644 > --- a/drivers/net/ldpaa_eth/ldpaa_eth.c > +++ b/drivers/net/ldpaa_eth/ldpaa_eth.c > @@ -334,7 +334,7 @@ static int ldpaa_eth_tx(struct eth_device *net_dev, void *buf, int len) > &buffer_start, 1); > } while (err == -EBUSY); > > - if (err < 0) { > + if (err <= 0) { > printf("qbman_swp_acquire() failed\n"); > return -ENOMEM; > } > Since you already check for errors in qbman_swp_acquire(), would it be better to check if "num" is zero there? York
Hello York, The definition of qbman_swp_acquire(), is not owned by u-boot, it is part of qbman drivers which is owned by Roy. u-boot gets this definition from flib code and same is used in u-boot as it is. So, moving this error handler in qbman_swp_acquire for num == 0, may result in inconsistency in flib code that was provided at the time of integration. Please suggest. Regards Ashish -----Original Message----- From: York Sun Sent: Thursday, October 26, 2017 8:49 PM To: Ashish Kumar <ashish.kumar@nxp.com>; u-boot@lists.denx.de Cc: joe.hershberger@gmail.com; Kushwaha Prabhakar <prabhakar@freescale.com> Subject: Re: [PATCH] drivers: net: ldpaa_eth: Correct error handler for qbman_swp_acquire() On 10/26/2017 03:33 AM, Ashish Kumar wrote: > Correcting error handing for qbman_swp_acquire. The return value is > zero is an error condition since number of buffer copied is zero > meaning there are no free buffers for allocation. > > Signed-off-by: Ashish Kumar <Ashish.Kumar@nxp.com> > Signed-off-by: Kushwaha Prabhakar <prabhakar@freescale.com> > --- > drivers/net/ldpaa_eth/ldpaa_eth.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/ldpaa_eth/ldpaa_eth.c > b/drivers/net/ldpaa_eth/ldpaa_eth.c > index f235b62..21be79a 100644 > --- a/drivers/net/ldpaa_eth/ldpaa_eth.c > +++ b/drivers/net/ldpaa_eth/ldpaa_eth.c > @@ -334,7 +334,7 @@ static int ldpaa_eth_tx(struct eth_device *net_dev, void *buf, int len) > &buffer_start, 1); > } while (err == -EBUSY); > > - if (err < 0) { > + if (err <= 0) { > printf("qbman_swp_acquire() failed\n"); > return -ENOMEM; > } > Since you already check for errors in qbman_swp_acquire(), would it be better to check if "num" is zero there? York
On 10/29/2017 10:23 PM, Ashish Kumar wrote: > Hello York, > > The definition of qbman_swp_acquire(), is not owned by u-boot, it is part of qbman drivers which is owned by Roy. > u-boot gets this definition from flib code and same is used in u-boot as it is. > > So, moving this error handler in qbman_swp_acquire for num == 0, may result in inconsistency in flib code that was provided at the time of integration. > OK. Let's keep it that way. BTW, please do not top post if you can avoid it. Please always use common quotation style when you reply. York
Please see inline. -----Original Message----- From: York Sun Sent: Tuesday, October 31, 2017 12:32 AM To: Ashish Kumar <ashish.kumar@nxp.com>; Roy Pledge <roy.pledge@nxp.com> Cc: joe.hershberger@gmail.com; Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com>; u-boot@lists.denx.de Subject: Re: [PATCH] drivers: net: ldpaa_eth: Correct error handler for qbman_swp_acquire() On 10/29/2017 10:23 PM, Ashish Kumar wrote: > Hello York, > > The definition of qbman_swp_acquire(), is not owned by u-boot, it is part of qbman drivers which is owned by Roy. > u-boot gets this definition from flib code and same is used in u-boot as it is. > > So, moving this error handler in qbman_swp_acquire for num == 0, may result in inconsistency in flib code that was provided at the time of integration. > OK. Let's keep it that way. BTW, please do not top post if you can avoid it. Please always use common quotation style when you reply. Sure, I will take care. York
On 10/26/2017 03:33 AM, Ashish Kumar wrote: > Correcting error handing for qbman_swp_acquire. The return value is zero is > an error condition since number of buffer copied is zero meaning > there are no free buffers for allocation. > > Signed-off-by: Ashish Kumar <Ashish.Kumar@nxp.com> > Signed-off-by: Kushwaha Prabhakar <prabhakar@freescale.com> > --- Applied to fsl-qoriq master. Thanks. York
diff --git a/drivers/net/ldpaa_eth/ldpaa_eth.c b/drivers/net/ldpaa_eth/ldpaa_eth.c index f235b62..21be79a 100644 --- a/drivers/net/ldpaa_eth/ldpaa_eth.c +++ b/drivers/net/ldpaa_eth/ldpaa_eth.c @@ -334,7 +334,7 @@ static int ldpaa_eth_tx(struct eth_device *net_dev, void *buf, int len) &buffer_start, 1); } while (err == -EBUSY); - if (err < 0) { + if (err <= 0) { printf("qbman_swp_acquire() failed\n"); return -ENOMEM; }