[PATCH v6 00/15] Wangxun fixes and new features
Stephen Hemminger
stephen at networkplumber.org
Tue Sep 29 18:05:56 CEST 2026
On Tue, 29 Sep 2026 21:04:31 +0800
Zaiyu Wang <zaiyuwang at trustnetic.com> wrote:
> This series addresses link-related issues on Wangxun Amber-lite 25G/40G NICs
> (CR/KR training, hot-plug, 10G link state).
>
> ---
> v6:
> - fix the apply failure.
> ---
> v5:
> - drop the UDP tunnel patch: the generic UDP tunnel flag covers any UDP
> tunnel, so the tunnel type cannot be resolved from the destination port.
> - P02: advertise 10G, 25G, and 40G from the requested speed mask instead
> of the device ID, so the 25G and 40G parts advertise 10G by
> default as well.
> - P07: keep the patch limited to decoding the 10G link speed from PORTSTAT;
> the AML40 default 10G|40G advertisement change is now in P02.
> - P09: gate the SFP detection alarm and the AN73 watchdog on a new per-port
> flag that dev_stop clears before the first cancel.
> cancel SFP detection before the watchdog.
> - P10: re-arm the 40G module poll only while the SFP/AN73 alarm flag is set.
> - P12: treat the ffe_pre2 and bp_capa devargs as a feature rather than a fix;
> drop the Fixes tags and stable Cc, and document the 25G/40G Amber-Lite
> FFE defaults.
> - P14: bound the AN page exchange, propagate its timeout to the watchdog h
> andler, and avoid register reads used only by disabled BP debug logs.
> - P15: stop clearing the auto_neg devarg when a 10G-only DAC disables AN;
> derive the effective AN73 state from the current module capabilities
> instead. Classify 40G active cables through the optical path, unify
> DAC checks on txgbe_is_dac_cable().
>
> Not handled in this revision:
>
> - P12: bp_capa is not range-checked. Devarg validation will be added in a
> separate change so all txgbe devargs can be handled consistently.
> - P14: the AN page exchange still busy-waits on the alarm thread and can
> delay handling for other ports. A later change will decouple negotiation
> from the alarm callback and run per-port negotiations in parallel.
> ---
>
>
> Zaiyu Wang (15):
> net/txgbe: fix failure to configure 10G on dual-speed DAC
> net/txgbe: use the requested speed in E56 AN setup
> net/txgbe: fix e56 PHY configuration error
> net/txgbe: fix incorrect link state in 10G forced mode
> net/txgbe: do not force reconfig on link retry
> net/txgbe: set i2c sda hold time
> net/txgbe: fix link speed display info for 10G mode
> net/txgbe: remove stale outer UDP checksum offload flag
> net/txgbe: fix SFP hot-plug when auto-negotiation is on
> net/txgbe: fix DAC hot-plug on 40G NIC with auto-negotiation
> net/txgbe: fix 40G FFE tuning applied to first lane only
> net/txgbe: add pre2 FFE tap and backplane capability devargs
> net/txgbe: add devarg to turn off Tx laser for 40G NIC
> net/txgbe: fix CR/KR link training and recovery
> net/txgbe: align link capabilities and DAC classification
>
> doc/guides/nics/txgbe.rst | 25 +++-
> doc/guides/rel_notes/release_26_11.rst | 11 ++
> drivers/net/txgbe/base/txgbe_aml.c | 4 +-
> drivers/net/txgbe/base/txgbe_aml40.c | 92 ++++++++++--
> drivers/net/txgbe/base/txgbe_e56.c | 14 +-
> drivers/net/txgbe/base/txgbe_e56.h | 6 +
> drivers/net/txgbe/base/txgbe_e56_bp.c | 196 +++++++++++++++----------
> drivers/net/txgbe/base/txgbe_e56_bp.h | 4 +-
> drivers/net/txgbe/base/txgbe_hw.c | 33 +++++
> drivers/net/txgbe/base/txgbe_osdep.h | 12 +-
> drivers/net/txgbe/base/txgbe_phy.c | 25 +++-
> drivers/net/txgbe/base/txgbe_phy.h | 7 +
> drivers/net/txgbe/base/txgbe_regs.h | 3 +
> drivers/net/txgbe/base/txgbe_type.h | 17 ++-
> drivers/net/txgbe/txgbe_ethdev.c | 178 ++++++++++++++++++++--
> drivers/net/txgbe/txgbe_ethdev.h | 2 +
> drivers/net/txgbe/txgbe_rxtx.c | 1 -
> 17 files changed, 502 insertions(+), 128 deletions(-)
Looks OK to me, AI did find a couple typos that you should fix:
Reviewed the whole v6 series applied on top of main. All 15 patches apply
cleanly, net/txgbe builds with no new warnings, and I found no correctness
bugs. Three Info-level nits below, all in 14/15.
14/15 net/txgbe: fix CR/KR link training and recovery
A typo regression in a log string the patch rewrites:
BP_LOG("KR TRAINNING CHECK = %x. pmd_ctrl:%lx-%lx-%lx-%lx\n",
"TRAINNING" should be "TRAINING". The line it replaces spelled it
correctly ("KR TRAINING CHECK = %x, %s. ..."), so this loses the old string
for anyone grepping logs. checkpatch flags it as TYPO_SPELLING.
Declarations after a statement in the new txgbe_e56_get_txffe():
if (!rte_log_can_log(RTE_LOGTYPE_TXGBE_BP, RTE_LOG_DEBUG))
return;
/* 21. read txffe to check kr training status */
u32 rdata = 0, pmd_ctrl = 0, lane_idx = 0, lane_num = 0, txffe = 0;
This compiles, but the rest of txgbe_e56_bp.c declares at the top of the
block. Hoisting the declaration above the early return would match the
file.
Stale comment on the page-exchange loop:
/* 50ms timeout */
for (count = 0; count < 50; count++) {
count is reset to 0 each time a next page arrives, so the loop is really
bounded by count2 against AN_PAGE_EXCHANGE_TIMEOUT_MS (200). The comment
describes the per-round budget, not the loop bound.
Review-Result: CLEAN
---
Notes on things that looked suspicious but check out, recorded so they do
not get re-reviewed:
- 02/15: the "readers fall back to get_link_capabilities() when the field
is zero" claim in the commit message is accurate -- both readers do
"speed = hw->phy.autoneg_advertised; if (!speed) ...get_link_capabilities()".
- 06/15: wr32m(hw, TXGBE_I2C_SDA_HOLD, ..., 0x640064) against masks
0xff0000 / 0xffff places 0x64 correctly in both the RX and TX fields.
- 08/15: no remaining RTE_MBUF_F_TX_OUTER_UDP_CKSUM reference anywhere in
the driver, so dropping it from TXGBE_TX_OFFLOAD_MASK is complete.
- 11/15: moving txgbe_parse_devargs() after txgbe_init_shared_code() is
safe -- the latter only sets the MAC type and the ops tables and reads no
devarg. The four-lane FFE value reaches the register whole through
wr32_ephy(), so widening the fields to u32 is what makes the replication
work.
- 14/15: removing the "if (status) return status;" after
txgbe_e56_cl72_training() does not swallow the error -- status is not
reassigned through the idle-detect writes and is still returned, and the
caller checks ret. The commit message says the non-abort is deliberate.
- 15/15: txgbe_xpcs_an_enabled() calling get_link_capabilities() does not
recurse (no capability function calls back into it), and the
!hw->devarg.auto_neg early return keeps the 25G path's unconditional
*autoneg = true from re-enabling AN against the devarg.
- 09/15 and 14/15 are the two patches checkpatch rejects, both on false
positives: "no space after cast" on RTE_ATOMIC(uint32_t), and
COMPLEX_MACRO on "#define ...CFG_0 1, 0", which is the bitfield-pair
convention used throughout the e56 headers.
One design point worth a second opinion rather than a fix: in 10/15,
txgbe_dev_detect_sfp() re-arms itself at the "rearm:" label, which the two
sfp_an_alarm_enabled == 0 early returns deliberately skip. That is how the
poll stops on dev_stop, and it is correct, but it does make that gate the
only thing terminating the 2-second poll that 40G hot-plug detection now
depends on.
More information about the dev
mailing list