On Mon, 26 Oct 2026 20:21:46 +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).
>
> ---
> v11:
> - P12: fall back to the default value when an FFE tap does not fit the
> register field, instead of only logging it.
> - P13: change the patch from a new feature to a fix:
> take the transmitter down on AML40 link down. Clearing GPIO bit 1,
> which holds the module in reset, does not stop the transmitter for
> every DAC cable and QSFP module. The laser_off devarg and its
> documentation are dropped; link setup runs again on link-up to
> restore what link-down disabled.
> - P17: fix warnings in check-git-log, and add the Cc: stable tag that
> belongs with the Fixes line.
Better but still has issues.
Subject: Re: [PATCH v11 00/17] net/txgbe: Amber-Lite link fixes
Series applies to main and every commit builds with -Dwerror=true.
Dropping laser_off and the FFE range fallback address the v9 comments.
The 13/16 locking problem from v9 is still there.
Errors
------
Patch 13/17 (net/txgbe: fix Tx laser left on at AML40 link down)
enable_tx_laser() still takes the SWFW semaphore and does an I2C write
on AML40:
hw->mac.acquire_swfw_sync(hw, TXGBE_MNGSEM_SWPHY) == 0) {
hw->phy.write_i2c_eeprom(hw,
TXGBE_SFF_8636_TX_DISABLE, 0x0);
and txgbe_set_link_to_amlite() still calls enable_tx_laser() on the
TXGBE_ERR_PHY_INIT_NOT_DONE path, under the phy_lock spinlock taken in
txgbe_setup_phy_link_aml40().
It is worse than a long hold. txgbe_dev_setup_link_alarm_handler_aml()
acquires hw->phy.phy_semaphore_mask before calling setup_link(), and
txgbe_read_phy_if() sets that mask to TXGBE_MNGSEM_SWPHY. The nested
acquire therefore cannot succeed. It spins 200 x 5 ms holding
phy_lock, then the timeout path in txgbe_acquire_swfw_sync() clears
the SW bit the outer caller still holds, so the rest of that handler
runs without the semaphore and its final release drops a lock it no
longer owns.
Keep enable/disable_tx_laser() GPIO and PMD_CFG0 only, and do the
SFF-8636 write from the ethdev ops, which hold neither lock:
static void
txgbe_qsfp_tx_disable(struct txgbe_hw *hw, bool off)
{
if (hw->mac.type != txgbe_mac_aml40 || txgbe_is_dac_cable(hw) ||
hw->phy.sfp_type == txgbe_sfp_type_unknown ||
hw->phy.sfp_type == txgbe_sfp_type_not_present)
return;
if (hw->mac.acquire_swfw_sync(hw, TXGBE_MNGSEM_SWPHY) != 0)
return;
hw->phy.write_i2c_eeprom(hw, TXGBE_SFF_8636_TX_DISABLE,
off ? TXGBE_SFF_8636_TX_DISABLE_ALL_LANES : 0);
hw->mac.release_swfw_sync(hw, TXGBE_MNGSEM_SWPHY);
}
Call it from txgbe_dev_start/stop() and txgbe_dev_set_link_up/down().
Warnings
--------
Patch 16/17 (net/txgbe: respect manual link-down on Amber-Lite)
Same as v9. hw->dev_start is checked once at the top of
txgbe_dev_setup_link_alarm_handler_aml() and the AN73 watchdog, then
setup_link() runs for seconds on the alarm or txgbe-link thread.
txgbe_dev_set_link_down() does not cancel the alarms or wait for the
link thread, so a link-down that lands in that window is undone when
setup_link() returns.
Do what txgbe_dev_stop() does, before touching the laser:
hw->dev_start = false;
if (hw->mac.type == txgbe_mac_aml ||
hw->mac.type == txgbe_mac_aml40) {
rte_eal_alarm_cancel(txgbe_dev_e56_check_bp_event, dev);
rte_eal_alarm_cancel(txgbe_dev_setup_link_alarm_handler_aml,
hw);
txgbe_dev_wait_setup_link_complete(dev, 0);
rte_eal_alarm_cancel(txgbe_dev_setup_link_alarm_handler_aml,
hw);
}
hw->mac.disable_tx_laser(hw);
and re-arm txgbe_dev_e56_check_bp_event in txgbe_dev_set_link_up().
Info
----
Patch 5/17: need_restart is still always true past the early return.
Pass true and drop the variable.
Patch 12/17: the *_dflt variables are initialized to 0 and then
unconditionally overwritten. Declare them where they are assigned.
Patch 14/17: still busy-waits up to 200 ms (page exchange) and 400 ms
(CL72) with usec_delay() on the EAL alarm thread.
Patch 15/17: the txgbe_is_40g_fiber_qsfp() branch is still identical
to the final else. Drop the helper.
Patch 17/17: a field rename is not a fix, and 234ce0d1fa9d is not in
25.11, so there is nothing to backport. Drop Fixes: and Cc: stable.
Review-Result: ERROR
