Re: [PATCH net v2] net: atlantic: fix overwritten return value in Aquantia driver

[Simon Horman <horms@kernel.org>]

On Tue, Jul 29, 2025 at 03:39:01AM +0200, Andrew Lunn wrote:
> > This patch uses `err |=` instead of assignment, so that any earlier error
> > is preserved even if later calls succeed. This is safe because all involved
> > functions return standard negative error codes (e.g. -EIO, -ETIMEDOUT, etc).
> > OR-ing such values does not convert them into success, and preserves the
> > indication that an error occurred.
> 
> 22 | 110 = 132 = ERFKILL
> 
> How easy will it be for somebody to debug that?

Hi Andrew and Tian,

I agree that this does not seem to be a good approach.

Looking over the code, it seems that we can just bail
out if an error occurs. Returning error codes as they arise.

Does this seem reasonable to you?

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 7e88d7234b14..6c490f61ce5c 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
@@ -492,6 +492,9 @@ static int hw_atl_utils_init_ucp(struct aq_hw_s *self,
 					self, self->mbox_addr,
 					self->mbox_addr != 0U,
 					1000U, 10000U);
+	if (err)
+		return err;
+
 	err = readx_poll_timeout_atomic(aq_fw1x_rpc_get, self,
 					self->rpc_addr,
 					self->rpc_addr != 0U,
diff --git a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils_fw2x.c b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils_fw2x.c
index 4d4cfbc91e19..48f7c863c04b 100644
--- a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils_fw2x.c
+++ b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils_fw2x.c
@@ -101,11 +101,15 @@ static int aq_fw2x_init(struct aq_hw_s *self)
 					self, self->mbox_addr,
 					self->mbox_addr != 0U,
 					1000U, 10000U);
+	if (err)
+		return err;
 
 	err = readx_poll_timeout_atomic(aq_fw2x_rpc_get,
 					self, self->rpc_addr,
 					self->rpc_addr != 0U,
 					1000U, 100000U);
+	if (err)
+		return err;
 
 	err = aq_fw2x_settings_get(self, &self->settings_addr);