Re: [PATCH v2 12/20] net/txgbe: fix link stability for 25G NIC

[Stephen Hemminger <stephen@networkplumber.org>]

On Wed, 29 Apr 2026 18:25:06 +0800
Zaiyu Wang <zaiyuwang@trustnetic.com> wrote:

> The link was previously configured via firmware, but this approach
> resulted in unstable link behavior. To resolve the issue, re-add the
> PHY configuration flow directly into the driver.
> 
> Fixes: ead3616f630d ("net/txgbe: support PHY configuration via SW-FW mailbox")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Zaiyu Wang <zaiyuwang@trustnetic.com>
> ---

AI review feedback spotted some things:

Review of this patch is partial; the patch adds 2,489 lines in a new txgbe_e56.c file plus 1,742-line and 275-line headers, and a full per-function trace was not completed. From what I have looked at:
Info: drivers/net/txgbe/base/txgbe_e56.c defines a generic file-scope helper named compare for qsort. Even though it's static, the name is unhelpfully generic; consider txgbe_e56_int_cmp or similar.
Warning: txgbe_setup_phy_link_aml(): when txgbe_set_link_to_amlite() returns TXGBE_ERR_TIMEOUT the function falls through to the out: label, which then reads TXGBE_PORTSTAT and re-checks link_up. But in the timeout path link_up is whatever the earlier check_phy_link loop left it as, and the function clears hw->link_valid only in this branch — yet check_phy_link reads if (!hw->link_valid) *link_up = false;. After clearing link_valid, no further check_phy_link is called before the out: block reads link_up, so the logic still works, but it's fragile. Worth a comment or restructure.