On Wed, 29 Apr 2026 18:25:06 +0800
Zaiyu Wang <[email protected]> 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: [email protected]
>
> Signed-off-by: Zaiyu Wang <[email protected]>
> ---
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.
