On Mon, 12 Oct 2026 18:56:17 +0800
Zaiyu Wang <[email protected]> wrote:
> This series addresses link-related issues on Wangxun Amber-lite 25G/40G NICs
> (CR/KR training, hot-plug, 10G link state).
>
> ---
> v8:
> - P05: keep a retry from reconfiguring an established AN73 link, while
> still restarting PHY configuration when the link is down or AN is
> incomplete.
> - P10: on AML40, mark unsupported and unclassifiable QSFP modules as
> unknown so the module poll does not repeatedly identify them or
> rerun module and link setup.
> - P12: place the txgbe release-note entries between the Solarflare and
> ZTE driver entries.
> - P13: rerun complete AML40 link setup on set_link_up() with laser_off
> enabled, restoring the DAC configuration without adding a separate
> state flag.
> - P14: correct the AN FSM condition described in the commit message
> and remove the stray line after the Signed-off-by trailer.
> - P15: assign the detected 10G speed for passive and active-limiting
> DACs, and remove the unreachable AML40 10G SFP capability branch.
> - P16: add a new patch to keep Amber-Lite ports administratively down
> by preventing the AN73 watchdog and setup-link retry paths from
> configuring PHY while hw->dev_start is cleared.
> ---
Still lots of issues shown from running Claude AI review from web.
Subject: Re: [PATCH v8 00/16] net/txgbe: Amber-Lite link fixes
Series applies cleanly to main and every commit builds with
-Dwerror=true. All Fixes: hashes resolve.
Two errors: patch 16 can leave the link permanently down after a
set_link_down/set_link_up cycle, and patch 13 adds a driver-specific
devarg that changes set_link_down semantics.
Errors
------
[PATCH v8 16/16] net/txgbe: respect manual link-down on Amber-Lite
The early return in txgbe_dev_setup_link_alarm_handler_aml() skips
clearing TXGBE_FLAG_NEED_LINK_CONFIG.
On 25G SFP (media_type fiber) with LSC enabled, set_link_down()
calls txgbe_dev_link_update(); link is down, so link_update_share()
sets NEED_LINK_CONFIG and spawns the link thread. The thread calls
the AML handler, which now returns because dev_start is false, and
the flag stays set. After that, every txgbe_dev_link_update_share()
returns link down at the NEED_LINK_CONFIG check before calling
check_link or starting a new thread, including the call from
set_link_up(). Nothing else clears the flag, so the port reports
link down until the application restarts the process.
Clear the flag before returning:
struct rte_eth_dev *dev = hw->dev_back;
struct txgbe_interrupt *intr = TXGBE_DEV_INTR(dev);
...
if (!hw->dev_start) {
intr->flags &= ~TXGBE_FLAG_NEED_LINK_CONFIG;
return;
}
[PATCH v8 13/16] net/txgbe: add devarg to turn off Tx laser for 40G NIC
NAK on the devarg. rte_eth_dev_set_link_down() is supposed to take
the link down, and the peer seeing loss of signal is the point of
it. Whether that happens should not depend on a per-driver knob.
Make it the unconditional behaviour on AML40 when link is set down,
and run link setup unconditionally in set_link_up().
The commit message already shows the cost of the knob: the enable
path has to stay ungated because a previous run with laser_off=1
can leave the module transmitter disabled. Without the devarg that
case goes away.
Warnings
--------
[PATCH v8 03/16] net/txgbe: fix e56 PHY configuration error
Commit message says the wrong field degrades both 40G and 10G, but
only txgbe_e56_cfg_40g() is changed. txgbe_e56_cfg_10g() still has
the same write:
addr = E56PHY_RXS_RINGO_0_ADDR;
rdata = rd32_ephy(hw, addr);
set_fields_e56(&rdata, 9, 4, 0x366);
cfg_25g and the e56_bp.c copies already use (21, 12). Either fix
cfg_10g in this patch or reword the message to say 40G only.
[PATCH v8 12/16] net/txgbe: add pre2 FFE tap and backplane capability devargs
bp_capa is not range checked. Any value other than 0..2 leaves the
40G block in txgbe_e56_set_phy_link_mode() advertising nothing for
40G. Validate it in txgbe_parse_devargs():
if (bp_capa > 2) {
PMD_INIT_LOG(WARNING, "invalid bp_capa %u, using 0",
bp_capa);
bp_capa = 0;
}
Also, why is KR4 vs CR4 a user setting at all? This is a property
of the board; derive it from the subsystem id if possible.
Info
----
[PATCH v8 02/16] net/txgbe: use the requested speed in E56 AN setup
The three ifs building autoneg_advertised reduce to:
hw->phy.autoneg_advertised = speed &
(TXGBE_LINK_SPEED_10GB_FULL |
TXGBE_LINK_SPEED_25GB_FULL |
TXGBE_LINK_SPEED_40GB_FULL);
Drop "as the base layer of the out-of-tree driver does" from the
commit message; the change stands on its own.
[PATCH v8 10/16] net/txgbe: fix DAC hot-plug on 40G NIC with auto-negotiation
With no module plugged in, every 2 s poll goes to the NOT_PRESENT
branch, logs "SFP not present." and cancels the watchdog again. Act
only on the transition:
} else if (err == TXGBE_ERR_SFP_NOT_PRESENT) {
if (hw->phy.sfp_type != txgbe_sfp_type_not_present) {
PMD_DRV_LOG(INFO, "SFP not present.");
hw->phy.sfp_type = txgbe_sfp_type_not_present;
rte_eal_alarm_cancel(txgbe_dev_e56_check_bp_event,
dev);
}
}
[PATCH v8 11/16] net/txgbe: fix 40G FFE tuning applied to first lane only
On AML40 a user ffe_* value above 0xFF is silently truncated by
S40G_TX_FFE_4LANE(). Reject or warn instead.
[PATCH v8 12/16] and [PATCH v8 13/16]
12 adds a stray blank line in release_26_11.rst and 13 removes it.
Drop the hunk from 12.
[PATCH v8 15/16] net/txgbe: align link capabilities and DAC classification
The comment "mirroring the temporary workaround in the previous
version" describes patch history, not the code. Suggest:
/* Unknown module: report 40G so setup_mac_link() does not
* fail with TXGBE_ERR_LINK_SETUP.
*/
The DEBUGOUT added at the top of txgbe_get_link_capabilities_aml40()
now runs on every watchdog tick, since txgbe_xpcs_an_enabled() calls
get_link_capabilities(). Remove it.
The bullet "Remove the AML40 10G SFP capability branch" describes a
branch that is not in the tree this patch applies to. Drop it.
