|SUCCESS| net/txgbe: fix AML mailbox lock acquisition

[dpdklab at iol.unh.edu]

Test-Label: ai-code-review
Test-Status: SUCCESS
https://dpdk.org/patch/164259

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-05-22

I will analyze this patch against the DPDK coding standards and contribution guidelines.

git sha: 6a139ade82e7 (referenced in Fixes tag)
Author: Stephen Hemminger <stephen at networkplumber.org>
Subject: [PATCH] net/txgbe: fix AML mailbox lock acquisition

This patch fixes an inverted lock acquisition condition in the txgbe
driver's host interface command function. The rte_atomic32_test_and_set()
return value check was backwards, causing immediate exit when the lock
was busy and unnecessary delay when free.

> diff --git a/drivers/net/txgbe/base/txgbe_mng.c b/drivers/net/txgbe/base/txgbe_mng.c
> index a1974820b6..dcd8f58a68 100644
> --- a/drivers/net/txgbe/base/txgbe_mng.c
> +++ b/drivers/net/txgbe/base/txgbe_mng.c
> @@ -185,7 +185,7 @@ txgbe_host_interface_command_aml(struct txgbe_hw *hw, u32 *buffer,
>  	}
>  
>  	/* try to get lock */
> -	while (rte_atomic32_test_and_set(&hw->swfw_busy)) {
> +	while (rte_atomic32_test_and_set(&hw->swfw_busy) == 0) {

The explicit comparison to zero is a C coding style violation. According to
the DPDK coding style guide, "Do not explicitly compare to true, false, 0
or NULL."

Can this code use the preferred implicit form? For example:

    while (!rte_atomic32_test_and_set(&hw->swfw_busy)) {

>  		timeout--;
>  		if (!timeout)
>  			return TXGBE_ERR_TIMEOUT;

Does this function properly decrement timeout before checking? In
txgbe_host_interface_command_aml(), what is the initial value of timeout?
If the function starts with timeout set to some value N and immediately
enters the loop, does the first decrement happen before the lock is ever
successfully acquired? Can this cause premature timeout when the lock is
legitimately busy for exactly N-1 iterations?