[PATCH net-next] net: aquantia: return 'err' if set MPI_DEINIT state fails

[PATCH net-next] net: aquantia: return 'err' if set MPI_DEINIT state fails

[YueHaibing]

Fixes gcc '-Wunused-but-set-variable' warning:

drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils.c:260:7:
 warning: variable 'err' set but not used [-Wunused-but-set-variable]

'err' should be returned while set MPI_DEINIT state fails
in hw_atl_utils_soft_reset.

Fixes: cce96d1883da ("net: aquantia: Regression on reset with 1.x firmware")
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
 drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils.c b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils.c
index 1af6606..9b74a31 100644
--- a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils.c
+++ b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils.c
@@ -263,6 +263,8 @@ int hw_atl_utils_soft_reset(struct aq_hw_s *self)
 		AQ_HW_WAIT_FOR((aq_hw_read_reg(self, HW_ATL_MPI_STATE_ADR) &
 				HW_ATL_MPI_STATE_MSK) == MPI_DEINIT,
 			       10, 1000U);
+		if (err)
+			return err;
 	}
 
 	if (self->rbl_enabled)
-- 
2.7.0

Re: [PATCH net-next] net: aquantia: return 'err' if set MPI_DEINIT state fails

[David Miller]

From: YueHaibing <yuehaibing@huawei.com>
Date: Sat, 24 Nov 2018 18:16:41 +0800

> Fixes gcc '-Wunused-but-set-variable' warning:
> 
> drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils.c:260:7:
>  warning: variable 'err' set but not used [-Wunused-but-set-variable]
> 
> 'err' should be returned while set MPI_DEINIT state fails
> in hw_atl_utils_soft_reset.
> 
> Fixes: cce96d1883da ("net: aquantia: Regression on reset with 1.x firmware")
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>

Applied.

But this AQ_HW_STATE_FOR macro _MUST_ be fixed.

No macro should have invisible references to variables like this, and
that is exactly what leads to bugs like this.  CPP macros written this
way make it impossible to properly audit code.

'err' should be an explicit argument given to this function instead of
how it works now.

Re: [PATCH net-next] net: aquantia: return 'err' if set MPI_DEINIT state fails

[Igor Russkikh]

>> drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils.c:260:7:
>>  warning: variable 'err' set but not used [-Wunused-but-set-variable]
>>
>> 'err' should be returned while set MPI_DEINIT state fails
>> in hw_atl_utils_soft_reset.
>>
>> Fixes: cce96d1883da ("net: aquantia: Regression on reset with 1.x firmware")
>> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> 
> Applied.
> 
> But this AQ_HW_STATE_FOR macro _MUST_ be fixed.
> 
> No macro should have invisible references to variables like this, and
> that is exactly what leads to bugs like this.  CPP macros written this
> way make it impossible to properly audit code.
> 
> 'err' should be an explicit argument given to this function instead of
> how it works now.

Hi David,

Agreed, I'll clean this up.

Regards,
  Igor