Re: Re: [PATCH v3 2/2] drivers: net: ethernet: 3com: fix return value

[Thomas Preisner]

On Tue, 2016-12-27 at 22:17:35 +0100, David Dillow wrote:
>On Sun, 2016-12-25 at 01:30 +0100, Thomas Preisner wrote:
>> In some cases the return value of a failing function is not being used
>> and the function typhoon_init_one() returns another negative error
>> code instead.
>
>I'm not sure these changes are especially valuable, since we'll need to
>look at the dmesg log anyways to figure out what went wrong, but again I
>don't feel strongly.
>
>Fix up the subject issues and I'm happy to ack them.

As requested, here are the patchsets with the fixed subjects.
The subjects aswell as the subject prefixes are more precise now.
Hopefully that's ok.

Patch 1:
Makes the function typhoon_init_one() return a negative error code instead of 0.

Patch 2 [Optional]:
Makes the function typhoon_init_one() return the return value of the
corresponding failing function calls instead of a "fixed" negative error code.

With Regards,
Milan and Thomas

Re: [PATCH v3 2/2] drivers: net: ethernet: 3com: fix return value

[David Dillow]

On Sun, 2016-12-25 at 01:30 +0100, Thomas Preisner wrote:
> In some cases the return value of a failing function is not being used
> and the function typhoon_init_one() returns another negative error
> code instead.

I'm not sure these changes are especially valuable, since we'll need to
look at the dmesg log anyways to figure out what went wrong, but again I
don't feel strongly.

Fix up the subject issues and I'm happy to ack them.

Re: [PATCH v3 2/2] drivers: net: ethernet: 3com: fix return value

[Sergei Shtylyov]

Hello!

On 12/25/2016 3:30 AM, Thomas Preisner wrote:


In some cases the return value of a failing function is not being used
and the function typhoon_init_one() returns another negative error
code instead.

Signed-off-by: Thomas Preisner 
Signed-off-by: Milan Stephan 
---
 drivers/net/ethernet/3com/typhoon.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)


   In addition to what DaveM said, your choise of the subject prefixes is too 
wide -- it would seem that you're fixing all 3com drivers, while you're only 
fixing typhoon. That "typhoon:" alone would have been an appropriate prefix.


MBR, Sergei

[PATCH v3 2/2] drivers: net: ethernet: 3com: fix return value

[Thomas Preisner]

In some cases the return value of a failing function is not being used
and the function typhoon_init_one() returns another negative error
code instead.

Signed-off-by: Thomas Preisner 
Signed-off-by: Milan Stephan 
---
 drivers/net/ethernet/3com/typhoon.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/3com/typhoon.c 
b/drivers/net/ethernet/3com/typhoon.c
index c88b88a..8821a24 100644
--- a/drivers/net/ethernet/3com/typhoon.c
+++ b/drivers/net/ethernet/3com/typhoon.c
@@ -2370,9 +2370,9 @@ typhoon_init_one(struct pci_dev *pdev, const struct 
pci_device_id *ent)
 * 4) Get the hardware address.
 * 5) Put the card to sleep.
 */
-   if (typhoon_reset(ioaddr, WaitSleep) < 0) {
+   err = typhoon_reset(ioaddr, WaitSleep);
+   if (err < 0) {
err_msg = "could not reset 3XP";
-   err = -EIO;
goto error_out_dma;
}
 
@@ -2386,16 +2386,16 @@ typhoon_init_one(struct pci_dev *pdev, const struct 
pci_device_id *ent)
typhoon_init_interface(tp);
typhoon_init_rings(tp);
 
-   if(typhoon_boot_3XP(tp, TYPHOON_STATUS_WAITING_FOR_HOST) < 0) {
+   err = typhoon_boot_3XP(tp, TYPHOON_STATUS_WAITING_FOR_HOST);
+   if (err < 0) {
err_msg = "cannot boot 3XP sleep image";
-   err = -EIO;
goto error_out_reset;
}
 
INIT_COMMAND_WITH_RESPONSE(_cmd, TYPHOON_CMD_READ_MAC_ADDRESS);
-   if(typhoon_issue_command(tp, 1, _cmd, 1, xp_resp) < 0) {
+   err = typhoon_issue_command(tp, 1, _cmd, 1, xp_resp);
+   if (err < 0) {
err_msg = "cannot read MAC address";
-   err = -EIO;
goto error_out_reset;
}
 
@@ -2430,9 +2430,9 @@ typhoon_init_one(struct pci_dev *pdev, const struct 
pci_device_id *ent)
if(xp_resp[0].numDesc != 0)
tp->capabilities |= TYPHOON_WAKEUP_NEEDS_RESET;
 
-   if(typhoon_sleep(tp, PCI_D3hot, 0) < 0) {
+   err = typhoon_sleep(tp, PCI_D3hot, 0);
+   if (err < 0) {
err_msg = "cannot put adapter to sleep";
-   err = -EIO;
goto error_out_reset;
}
 
-- 
2.7.4