[U-Boot] [PATCH] net: rtl8169: Fix return value for rtl_send_common

[U-Boot] [PATCH] net: rtl8169: Fix return value for rtl_send_common

[Oleksandr Tymoshenko]

Return value of rtl_send_common propogates unmodified all the way
up to eth_send and further to API consumer if CONFIG_API is enabled.
Previously rtl_send_common returned number of bytes sent on success
which was erroneouly detected as error condition by API consumers
that checked for operation success by comparing return value with 0.

Switch rtl_send_common to use common convention: return 0 on success
and negative value for failure.

Cc: Stephen Warren <swarren@nvidia.com>
Cc: Joe Hershberger <joe.hershberger@ni.com>
Signed-off-by: Oleksandr Tymoshenko <gonzo@bluezbox.com>
---
 drivers/net/rtl8169.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/rtl8169.c b/drivers/net/rtl8169.c
index 843b083..1cc0b40 100644
--- a/drivers/net/rtl8169.c
+++ b/drivers/net/rtl8169.c
@@ -666,12 +666,12 @@ static int rtl_send_common(pci_dev_t dev, unsigned long dev_iobase,
 		puts("tx timeout/error\n");
 		printf("%s elapsed time : %lu\n", __func__, currticks()-stime);
 #endif
-		ret = 0;
+		ret = -ETIMEDOUT;
 	} else {
 #ifdef DEBUG_RTL8169_TX
 		puts("tx done\n");
 #endif
-		ret = length;
+		ret = 0;
 	}
 	/* Delay to make net console (nc) work properly */
 	udelay(20);
-- 
2.7.4

[U-Boot] [PATCH] net: rtl8169: Fix return value for rtl_send_common

[Simon Glass]

On 1 July 2016 at 14:22, Oleksandr Tymoshenko <gonzo@bluezbox.com> wrote:
> Return value of rtl_send_common propogates unmodified all the way
> up to eth_send and further to API consumer if CONFIG_API is enabled.
> Previously rtl_send_common returned number of bytes sent on success
> which was erroneouly detected as error condition by API consumers
> that checked for operation success by comparing return value with 0.
>
> Switch rtl_send_common to use common convention: return 0 on success
> and negative value for failure.
>
> Cc: Stephen Warren <swarren@nvidia.com>
> Cc: Joe Hershberger <joe.hershberger@ni.com>
> Signed-off-by: Oleksandr Tymoshenko <gonzo@bluezbox.com>
> ---
>  drivers/net/rtl8169.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

But I'd like to see a function comment on rtl_send_common(), and on
the driver-model send() method in struct eth_ops, for that matter.

- Simon

[U-Boot] [PATCH] net: rtl8169: Fix return value for rtl_send_common

[Oleksandr Tymoshenko]

> On Jul 4, 2016, at 8:40 AM, Simon Glass <sjg@chromium.org> wrote:
> 
> On 1 July 2016 at 14:22, Oleksandr Tymoshenko <gonzo@bluezbox.com> wrote:
>> Return value of rtl_send_common propogates unmodified all the way
>> up to eth_send and further to API consumer if CONFIG_API is enabled.
>> Previously rtl_send_common returned number of bytes sent on success
>> which was erroneouly detected as error condition by API consumers
>> that checked for operation success by comparing return value with 0.
>> 
>> Switch rtl_send_common to use common convention: return 0 on success
>> and negative value for failure.
>> 
>> Cc: Stephen Warren <swarren@nvidia.com>
>> Cc: Joe Hershberger <joe.hershberger@ni.com>
>> Signed-off-by: Oleksandr Tymoshenko <gonzo@bluezbox.com>
>> ---
>> drivers/net/rtl8169.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>
> 
> But I'd like to see a function comment on rtl_send_common(), and on
> the driver-model send() method in struct eth_ops, for that matter.

Hi Simon,

Should I submit new version or is it OK to submit new patch with comments
added to these two functions?

Thank you

[U-Boot] [PATCH] net: rtl8169: Fix return value for rtl_send_common

[Simon Glass]

Hi,

On 4 July 2016 at 11:10, Oleksandr Tymoshenko <gonzo@bluezbox.com> wrote:
>
>> On Jul 4, 2016, at 8:40 AM, Simon Glass <sjg@chromium.org> wrote:
>>
>> On 1 July 2016 at 14:22, Oleksandr Tymoshenko <gonzo@bluezbox.com> wrote:
>>> Return value of rtl_send_common propogates unmodified all the way
>>> up to eth_send and further to API consumer if CONFIG_API is enabled.
>>> Previously rtl_send_common returned number of bytes sent on success
>>> which was erroneouly detected as error condition by API consumers
>>> that checked for operation success by comparing return value with 0.
>>>
>>> Switch rtl_send_common to use common convention: return 0 on success
>>> and negative value for failure.
>>>
>>> Cc: Stephen Warren <swarren@nvidia.com>
>>> Cc: Joe Hershberger <joe.hershberger@ni.com>
>>> Signed-off-by: Oleksandr Tymoshenko <gonzo@bluezbox.com>
>>> ---
>>> drivers/net/rtl8169.c | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> Reviewed-by: Simon Glass <sjg@chromium.org>
>>
>> But I'd like to see a function comment on rtl_send_common(), and on
>> the driver-model send() method in struct eth_ops, for that matter.
>
> Hi Simon,
>
> Should I submit new version or is it OK to submit new patch with comments
> added to these two functions?

A new patch is fine, thanks.

- Simon

[U-Boot] [PATCH] net: rtl8169: Fix return value for rtl_send_common

[Joe Hershberger]

On Fri, Jul 1, 2016 at 3:22 PM, Oleksandr Tymoshenko <gonzo@bluezbox.com> wrote:
> Return value of rtl_send_common propogates unmodified all the way
> up to eth_send and further to API consumer if CONFIG_API is enabled.
> Previously rtl_send_common returned number of bytes sent on success
> which was erroneouly detected as error condition by API consumers
> that checked for operation success by comparing return value with 0.
>
> Switch rtl_send_common to use common convention: return 0 on success
> and negative value for failure.
>
> Cc: Stephen Warren <swarren@nvidia.com>
> Cc: Joe Hershberger <joe.hershberger@ni.com>
> Signed-off-by: Oleksandr Tymoshenko <gonzo@bluezbox.com>

Acked-by: Joe Hershberger <joe.hershberger@ni.com>

[U-Boot] net: rtl8169: Fix return value for rtl_send_common

[Joe Hershberger]

Hi Oleksandr,

https://patchwork.ozlabs.org/patch/643462/ was applied to u-boot-net.git.

Thanks!
-Joe