[PATCH 21/62] qed: Make _qed_mcp_cmd_and_union() easier to analyze

[PATCH 21/62] qed: Make _qed_mcp_cmd_and_union() easier to analyze

[Bart Van Assche]

Make the implementation of this function compatible with clang's
compile-time thread-safety analysis by moving error-handling code to the
end of this function. No functionality has been changed.

Cc: Manish Chopra <manishc@marvell.com>
Cc: netdev@vger.kernel.org
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/net/ethernet/qlogic/qed/qed_mcp.c | 56 ++++++++++++-----------
 1 file changed, 30 insertions(+), 26 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qed/qed_mcp.c b/drivers/net/ethernet/qlogic/qed/qed_mcp.c
index 7e37fe631a58..462e758c5890 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_mcp.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_mcp.c
@@ -467,7 +467,7 @@ _qed_mcp_cmd_and_union(struct qed_hwfn *p_hwfn,
 	int rc = 0;
 
 	/* Wait until the mailbox is non-occupied */
-	do {
+	for (;;) {
 		/* Exit the loop if there is no pending command, or if the
 		 * pending command is completed during this iteration.
 		 * The spinlock stays locked until the command is sent.
@@ -486,18 +486,14 @@ _qed_mcp_cmd_and_union(struct qed_hwfn *p_hwfn,
 
 		spin_unlock_bh(&p_hwfn->mcp_info->cmd_lock);
 
+		if (++cnt >= QED_DRV_MB_MAX_RETRIES)
+			goto retries_exceeded_1;
+
 		if (QED_MB_FLAGS_IS_SET(p_mb_params, CAN_SLEEP))
 			usleep_range(QED_MCP_RESP_ITER_US,
 				     QED_MCP_RESP_ITER_US * 2);
 		else
 			udelay(QED_MCP_RESP_ITER_US);
-	} while (++cnt < QED_DRV_MB_MAX_RETRIES);
-
-	if (cnt >= QED_DRV_MB_MAX_RETRIES) {
-		DP_NOTICE(p_hwfn,
-			  "The MFW mailbox is occupied by an uncompleted command. Failed to send command 0x%08x [param 0x%08x].\n",
-			  p_mb_params->cmd, p_mb_params->param);
-		return -EAGAIN;
 	}
 
 	/* Send the mailbox command */
@@ -513,7 +509,7 @@ _qed_mcp_cmd_and_union(struct qed_hwfn *p_hwfn,
 	spin_unlock_bh(&p_hwfn->mcp_info->cmd_lock);
 
 	/* Wait for the MFW response */
-	do {
+	for (;;) {
 		/* Exit the loop if the command is already completed, or if the
 		 * command is completed during this iteration.
 		 * The spinlock stays locked until the list element is removed.
@@ -537,24 +533,9 @@ _qed_mcp_cmd_and_union(struct qed_hwfn *p_hwfn,
 			goto err;
 
 		spin_unlock_bh(&p_hwfn->mcp_info->cmd_lock);
-	} while (++cnt < QED_DRV_MB_MAX_RETRIES);
-
-	if (cnt >= QED_DRV_MB_MAX_RETRIES) {
-		DP_NOTICE(p_hwfn,
-			  "The MFW failed to respond to command 0x%08x [param 0x%08x].\n",
-			  p_mb_params->cmd, p_mb_params->param);
-		qed_mcp_print_cpu_info(p_hwfn, p_ptt);
-
-		spin_lock_bh(&p_hwfn->mcp_info->cmd_lock);
-		qed_mcp_cmd_del_elem(p_hwfn, p_cmd_elem);
-		spin_unlock_bh(&p_hwfn->mcp_info->cmd_lock);
 
-		if (!QED_MB_FLAGS_IS_SET(p_mb_params, AVOID_BLOCK))
-			qed_mcp_cmd_set_blocking(p_hwfn, true);
-
-		qed_hw_err_notify(p_hwfn, p_ptt,
-				  QED_HW_ERR_MFW_RESP_FAIL, NULL);
-		return -EAGAIN;
+		if (++cnt >= QED_DRV_MB_MAX_RETRIES)
+			goto retries_exceeded_2;
 	}
 
 	qed_mcp_cmd_del_elem(p_hwfn, p_cmd_elem);
@@ -576,6 +557,29 @@ _qed_mcp_cmd_and_union(struct qed_hwfn *p_hwfn,
 err:
 	spin_unlock_bh(&p_hwfn->mcp_info->cmd_lock);
 	return rc;
+
+retries_exceeded_1:
+	DP_NOTICE(p_hwfn,
+		  "The MFW mailbox is occupied by an uncompleted command. Failed to send command 0x%08x [param 0x%08x].\n",
+		  p_mb_params->cmd, p_mb_params->param);
+	return -EAGAIN;
+
+retries_exceeded_2:
+	DP_NOTICE(p_hwfn,
+		  "The MFW failed to respond to command 0x%08x [param 0x%08x].\n",
+		  p_mb_params->cmd, p_mb_params->param);
+	qed_mcp_print_cpu_info(p_hwfn, p_ptt);
+
+	spin_lock_bh(&p_hwfn->mcp_info->cmd_lock);
+	qed_mcp_cmd_del_elem(p_hwfn, p_cmd_elem);
+	spin_unlock_bh(&p_hwfn->mcp_info->cmd_lock);
+
+	if (!QED_MB_FLAGS_IS_SET(p_mb_params, AVOID_BLOCK))
+		qed_mcp_cmd_set_blocking(p_hwfn, true);
+
+	qed_hw_err_notify(p_hwfn, p_ptt,
+			  QED_HW_ERR_MFW_RESP_FAIL, NULL);
+	return -EAGAIN;
 }
 
 static int qed_mcp_cmd_and_union(struct qed_hwfn *p_hwfn,

[PATCH 17/62] bnxt_en: Make bnxt_resume() easier to analyze

[Bart Van Assche]

From: Bart Van Assche <bvanassche@acm.org>

Pass the same argument to netdev_lock() and netdev_unlock(). This patch
prepares for enabling the Clang thread-safety analysis functionality. No
functional change intended.

Cc: Shantiprasad Shettar <shantiprasad.shettar@broadcom.com>
Cc: netdev@vger.kernel.org
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index e062d5d400da..950708575268 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -17121,7 +17121,7 @@ static int bnxt_resume(struct device *device)
 	}
 
 resume_exit:
-	netdev_unlock(bp->dev);
+	netdev_unlock(dev);
 	bnxt_ulp_start(bp, rc);
 	if (!rc)
 		bnxt_reenable_sriov(bp);

[PATCH 18/62] bnxt_en: Fix bnxt_dl_reload_up()

[Bart Van Assche]

From: Bart Van Assche <bvanassche@acm.org>

bnxt_dl_reload_down() calls rtnl_lock() and netdev_lock() if it returns
0. Hence, bnxt_dl_reload_up() should invoke the corresponding unlock
calls. This issue has been detected by the clang thread-sanitizer.
Compile-tested only.

Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Shantiprasad Shettar <shantiprasad.shettar@broadcom.com>
Cc: Stanislav Fomichev <sdf@fomichev.me>
Cc: netdev@vger.kernel.org
Fixes: 004b5008016a ("eth: bnxt: remove most dependencies on RTNL")
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
index 15de802bbac4..1e9a3454bb29 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
@@ -562,6 +562,8 @@ static int bnxt_dl_reload_up(struct devlink *dl, enum devlink_reload_action acti
 		break;
 	}
 	default:
+		netdev_unlock(bp->dev);
+		rtnl_unlock();
 		return -EOPNOTSUPP;
 	}
 

[PATCH 20/62] octeontx2-pf: Fix locking in an error path

[Bart Van Assche]

From: Bart Van Assche <bvanassche@acm.org>

Only unlock pf->mbox.lock if it has been locked by otx2_do_set_vf_vlan().
This bug has been detected by the Clang thread-safety analyzer.

Cc: Naveen Mamindlapalli <naveenm@marvell.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Sunil Goutham <sgoutham@marvell.com>
Cc: Geetha sowjanya <gakula@marvell.com>
Cc: Subbaraya Sundeep <sbhatta@marvell.com>
Cc: hariprasad <hkelam@marvell.com>
Cc: Bharat Bhushan <bbhushan2@marvell.com>
Cc: netdev@vger.kernel.org
Fixes: f0c2982aaf98 ("octeontx2-pf: Add support for SR-IOV management functions")
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
index ee623476e5ff..8c9f08ed90fd 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
@@ -2588,7 +2588,7 @@ static int otx2_do_set_vf_vlan(struct otx2_nic *pf, int vf, u16 vlan, u8 qos,
 	config = &pf->vf_configs[vf];
 
 	if (!vlan && !config->vlan)
-		goto out;
+		return err;
 
 	mutex_lock(&pf->mbox.lock);
 

[PATCH 21/62] qed: Make _qed_mcp_cmd_and_union() easier to analyze

[Bart Van Assche]

From: Bart Van Assche <bvanassche@acm.org>

Make the implementation of this function compatible with clang's
compile-time thread-safety analysis by moving error-handling code to the
end of this function. No functionality has been changed.

Cc: Manish Chopra <manishc@marvell.com>
Cc: netdev@vger.kernel.org
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/net/ethernet/qlogic/qed/qed_mcp.c | 56 ++++++++++++-----------
 1 file changed, 30 insertions(+), 26 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qed/qed_mcp.c b/drivers/net/ethernet/qlogic/qed/qed_mcp.c
index 7e37fe631a58..462e758c5890 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_mcp.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_mcp.c
@@ -467,7 +467,7 @@ _qed_mcp_cmd_and_union(struct qed_hwfn *p_hwfn,
 	int rc = 0;
 
 	/* Wait until the mailbox is non-occupied */
-	do {
+	for (;;) {
 		/* Exit the loop if there is no pending command, or if the
 		 * pending command is completed during this iteration.
 		 * The spinlock stays locked until the command is sent.
@@ -486,18 +486,14 @@ _qed_mcp_cmd_and_union(struct qed_hwfn *p_hwfn,
 
 		spin_unlock_bh(&p_hwfn->mcp_info->cmd_lock);
 
+		if (++cnt >= QED_DRV_MB_MAX_RETRIES)
+			goto retries_exceeded_1;
+
 		if (QED_MB_FLAGS_IS_SET(p_mb_params, CAN_SLEEP))
 			usleep_range(QED_MCP_RESP_ITER_US,
 				     QED_MCP_RESP_ITER_US * 2);
 		else
 			udelay(QED_MCP_RESP_ITER_US);
-	} while (++cnt < QED_DRV_MB_MAX_RETRIES);
-
-	if (cnt >= QED_DRV_MB_MAX_RETRIES) {
-		DP_NOTICE(p_hwfn,
-			  "The MFW mailbox is occupied by an uncompleted command. Failed to send command 0x%08x [param 0x%08x].\n",
-			  p_mb_params->cmd, p_mb_params->param);
-		return -EAGAIN;
 	}
 
 	/* Send the mailbox command */
@@ -513,7 +509,7 @@ _qed_mcp_cmd_and_union(struct qed_hwfn *p_hwfn,
 	spin_unlock_bh(&p_hwfn->mcp_info->cmd_lock);
 
 	/* Wait for the MFW response */
-	do {
+	for (;;) {
 		/* Exit the loop if the command is already completed, or if the
 		 * command is completed during this iteration.
 		 * The spinlock stays locked until the list element is removed.
@@ -537,24 +533,9 @@ _qed_mcp_cmd_and_union(struct qed_hwfn *p_hwfn,
 			goto err;
 
 		spin_unlock_bh(&p_hwfn->mcp_info->cmd_lock);
-	} while (++cnt < QED_DRV_MB_MAX_RETRIES);
-
-	if (cnt >= QED_DRV_MB_MAX_RETRIES) {
-		DP_NOTICE(p_hwfn,
-			  "The MFW failed to respond to command 0x%08x [param 0x%08x].\n",
-			  p_mb_params->cmd, p_mb_params->param);
-		qed_mcp_print_cpu_info(p_hwfn, p_ptt);
-
-		spin_lock_bh(&p_hwfn->mcp_info->cmd_lock);
-		qed_mcp_cmd_del_elem(p_hwfn, p_cmd_elem);
-		spin_unlock_bh(&p_hwfn->mcp_info->cmd_lock);
 
-		if (!QED_MB_FLAGS_IS_SET(p_mb_params, AVOID_BLOCK))
-			qed_mcp_cmd_set_blocking(p_hwfn, true);
-
-		qed_hw_err_notify(p_hwfn, p_ptt,
-				  QED_HW_ERR_MFW_RESP_FAIL, NULL);
-		return -EAGAIN;
+		if (++cnt >= QED_DRV_MB_MAX_RETRIES)
+			goto retries_exceeded_2;
 	}
 
 	qed_mcp_cmd_del_elem(p_hwfn, p_cmd_elem);
@@ -576,6 +557,29 @@ _qed_mcp_cmd_and_union(struct qed_hwfn *p_hwfn,
 err:
 	spin_unlock_bh(&p_hwfn->mcp_info->cmd_lock);
 	return rc;
+
+retries_exceeded_1:
+	DP_NOTICE(p_hwfn,
+		  "The MFW mailbox is occupied by an uncompleted command. Failed to send command 0x%08x [param 0x%08x].\n",
+		  p_mb_params->cmd, p_mb_params->param);
+	return -EAGAIN;
+
+retries_exceeded_2:
+	DP_NOTICE(p_hwfn,
+		  "The MFW failed to respond to command 0x%08x [param 0x%08x].\n",
+		  p_mb_params->cmd, p_mb_params->param);
+	qed_mcp_print_cpu_info(p_hwfn, p_ptt);
+
+	spin_lock_bh(&p_hwfn->mcp_info->cmd_lock);
+	qed_mcp_cmd_del_elem(p_hwfn, p_cmd_elem);
+	spin_unlock_bh(&p_hwfn->mcp_info->cmd_lock);
+
+	if (!QED_MB_FLAGS_IS_SET(p_mb_params, AVOID_BLOCK))
+		qed_mcp_cmd_set_blocking(p_hwfn, true);
+
+	qed_hw_err_notify(p_hwfn, p_ptt,
+			  QED_HW_ERR_MFW_RESP_FAIL, NULL);
+	return -EAGAIN;
 }
 
 static int qed_mcp_cmd_and_union(struct qed_hwfn *p_hwfn,

[PATCH 22/62] mctp i3c: Fix locking in error paths

[Bart Van Assche]

From: Bart Van Assche <bvanassche@acm.org>

Only unlock mi->lock if it has been locked. This bug has been detected by
the Clang thread-safety analyzer.

Cc: Leo Yang <Leo-Yang@quantatw.com>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: Jeremy Kerr <jk@codeconstruct.com.au>
Cc: Matt Johnston <matt@codeconstruct.com.au>
Cc: netdev@vger.kernel.org
Fixes: 2d2d4f60ed26 ("mctp i3c: fix MCTP I3C driver multi-thread issue")
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/net/mctp/mctp-i3c.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/mctp/mctp-i3c.c b/drivers/net/mctp/mctp-i3c.c
index 6d2bbae7477b..d4a29912c7e1 100644
--- a/drivers/net/mctp/mctp-i3c.c
+++ b/drivers/net/mctp/mctp-i3c.c
@@ -112,7 +112,7 @@ static int mctp_i3c_read(struct mctp_i3c_device *mi)
 	if (!skb) {
 		stats->rx_dropped++;
 		rc = -ENOMEM;
-		goto err;
+		goto free_skb;
 	}
 
 	skb->protocol = htons(ETH_P_MCTP);
@@ -170,8 +170,11 @@ static int mctp_i3c_read(struct mctp_i3c_device *mi)
 
 	mutex_unlock(&mi->lock);
 	return 0;
+
 err:
 	mutex_unlock(&mi->lock);
+
+free_skb:
 	kfree_skb(skb);
 	return rc;
 }

[PATCH 23/62] net: phy: mxl-86110: Fix locking in an error path

[Bart Van Assche]

From: Bart Van Assche <bvanassche@acm.org>

Only call phy_unlock_mdio_bus() if phy_lock_mdio_bus() has been called
first. This has been detected by the Clang thread-safety analyzer.

Cc: Daniel Golle <daniel@makrotopia.org>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Xu Liang <lxu@maxlinear.com>
Cc: netdev@vger.kernel.org
Fixes: 3d1b3f4ffc0a ("net: phy: mxl-86110: add basic support for MxL86111 PHY")
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/net/phy/mxl-86110.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/phy/mxl-86110.c b/drivers/net/phy/mxl-86110.c
index 42a5fe3f115f..d010b023f7b9 100644
--- a/drivers/net/phy/mxl-86110.c
+++ b/drivers/net/phy/mxl-86110.c
@@ -891,7 +891,7 @@ static int mxl86111_config_inband(struct phy_device *phydev, unsigned int modes)
 			       MII_BMCR, BMCR_ANENABLE,
 			       (modes == LINK_INBAND_DISABLE) ? 0 : BMCR_ANENABLE);
 	if (ret < 0)
-		goto out;
+		return ret;
 
 	phy_lock_mdio_bus(phydev);
 

Re: [PATCH 23/62] net: phy: mxl-86110: Fix locking in an error path

[Daniel Golle]

On Mon, Feb 23, 2026 at 02:00:23PM -0800, Bart Van Assche wrote:
> From: Bart Van Assche <bvanassche@acm.org>
> 
> Only call phy_unlock_mdio_bus() if phy_lock_mdio_bus() has been called
> first. This has been detected by the Clang thread-safety analyzer.
> 
> Cc: Daniel Golle <daniel@makrotopia.org>
> Cc: Andrew Lunn <andrew@lunn.ch>
> Cc: Jakub Kicinski <kuba@kernel.org>
> Cc: Xu Liang <lxu@maxlinear.com>
> Cc: netdev@vger.kernel.org
> Fixes: 3d1b3f4ffc0a ("net: phy: mxl-86110: add basic support for MxL86111 PHY")
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>

Reviewed-by: Daniel Golle <daniel@makrotopia.org>

> ---
>  drivers/net/phy/mxl-86110.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/phy/mxl-86110.c b/drivers/net/phy/mxl-86110.c
> index 42a5fe3f115f..d010b023f7b9 100644
> --- a/drivers/net/phy/mxl-86110.c
> +++ b/drivers/net/phy/mxl-86110.c
> @@ -891,7 +891,7 @@ static int mxl86111_config_inband(struct phy_device *phydev, unsigned int modes)
>  			       MII_BMCR, BMCR_ANENABLE,
>  			       (modes == LINK_INBAND_DISABLE) ? 0 : BMCR_ANENABLE);
>  	if (ret < 0)
> -		goto out;
> +		return ret;
>  
>  	phy_lock_mdio_bus(phydev);
>  

Re: [PATCH 23/62] net: phy: mxl-86110: Fix locking in an error path

[Andrew Lunn]

On Mon, Feb 23, 2026 at 02:00:23PM -0800, Bart Van Assche wrote:
> From: Bart Van Assche <bvanassche@acm.org>
> 
> Only call phy_unlock_mdio_bus() if phy_lock_mdio_bus() has been called
> first. This has been detected by the Clang thread-safety analyzer.
> 
> Cc: Daniel Golle <daniel@makrotopia.org>
> Cc: Andrew Lunn <andrew@lunn.ch>
> Cc: Jakub Kicinski <kuba@kernel.org>
> Cc: Xu Liang <lxu@maxlinear.com>
> Cc: netdev@vger.kernel.org
> Fixes: 3d1b3f4ffc0a ("net: phy: mxl-86110: add basic support for MxL86111 PHY")
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH 22/62] mctp i3c: Fix locking in error paths

[Matt Johnston]

Hi Bart,

On Mon, 2026-02-23 at 14:00 -0800, Bart Van Assche wrote:
> From: Bart Van Assche <bvanassche@acm.org>
> 
> Only unlock mi->lock if it has been locked. This bug has been detected by
> the Clang thread-safety analyzer.

Thanks for the fix.

Acked-by: Matt Johnston <matt@codeconstruct.com.au>

Cheers,
Matt