Re: [PATCH v6 00/21] Wangxun Fixes

[Stephen Hemminger <stephen@networkplumber.org>]

On Tue, 16 Jun 2026 20:20:08 +0800
Zaiyu Wang <zaiyuwang@trustnetic.com> wrote:

> This series fixes several issues found on Wangxun Emerald, Sapphire and
> Amber-lite NICs, with a focus on link-related problems.
> ---
> v6:
> - Fixed more issues identified by AI review
> ---
> v5:
> - Fixed issues identified by AI review
> ---
> v4:
> - Fixed issues identified by devtools scripts
> ---
> v3:
> - Addressed Stephen's comments
> ---
> v2:
> - Fixed compilation error and code style issues
> ---
> 
> Zaiyu Wang (21):
>   net/txgbe: remove duplicate xstats counters
>   net/ngbe: remove duplicate xstats counters
>   net/ngbe: add missing CDR config for YT PHY
>   net/ngbe: fix VF promiscuous and allmulticast
>   net/txgbe: fix inaccuracy in Tx rate limiting
>   net/txgbe: fix link status check condition
>   net/txgbe: fix Tx desc free logic
>   net/txgbe: fix link flow control registers for Amber-Lite
>   net/txgbe: fix link flow control config for Sapphire
>   net/txgbe: fix a mass of unknown interrupts
>   net/txgbe: fix traffic class priority configuration
>   net/txgbe: fix link stability for 25G NIC
>   net/txgbe: fix link stability for 40G NIC
>   net/txgbe: fix link stability for Amber-Lite backplane mode
>   net/txgbe: fix FEC mode configuration on 25G NIC
>   net/txgbe: fix SFP module identification
>   net/txgbe: fix get module info operation
>   net/txgbe: fix get EEPROM operation
>   net/txgbe: fix to reset Tx write-back pointer
>   net/txgbe: fix to enable Tx desc check
>   net/txgbe: fix temperature track for AML NIC
> 
>  drivers/net/ngbe/base/ngbe_phy_yt.c       |    3 +
>  drivers/net/ngbe/ngbe_ethdev.c            |    5 -
>  drivers/net/ngbe/ngbe_ethdev_vf.c         |   11 +-
>  drivers/net/txgbe/base/meson.build        |    2 +
>  drivers/net/txgbe/base/txgbe.h            |    2 +
>  drivers/net/txgbe/base/txgbe_aml.c        |  185 +-
>  drivers/net/txgbe/base/txgbe_aml.h        |    6 +-
>  drivers/net/txgbe/base/txgbe_aml40.c      |  114 +-
>  drivers/net/txgbe/base/txgbe_aml40.h      |    6 +-
>  drivers/net/txgbe/base/txgbe_dcb_hw.c     |    2 +-
>  drivers/net/txgbe/base/txgbe_e56.c        | 3773 +++++++++++++++++++++
>  drivers/net/txgbe/base/txgbe_e56.h        | 1753 ++++++++++
>  drivers/net/txgbe/base/txgbe_e56_bp.c     | 2597 ++++++++++++++
>  drivers/net/txgbe/base/txgbe_e56_bp.h     |  282 ++
>  drivers/net/txgbe/base/txgbe_hw.c         |   54 +-
>  drivers/net/txgbe/base/txgbe_hw.h         |    4 +-
>  drivers/net/txgbe/base/txgbe_osdep.h      |    4 +
>  drivers/net/txgbe/base/txgbe_phy.c        |  362 +-
>  drivers/net/txgbe/base/txgbe_phy.h        |   46 +-
>  drivers/net/txgbe/base/txgbe_regs.h       |   13 +-
>  drivers/net/txgbe/base/txgbe_type.h       |   43 +-
>  drivers/net/txgbe/txgbe_ethdev.c          |  472 ++-
>  drivers/net/txgbe/txgbe_ethdev.h          |    7 +-
>  drivers/net/txgbe/txgbe_rxtx.c            |  109 +-
>  drivers/net/txgbe/txgbe_rxtx.h            |   36 +
>  drivers/net/txgbe/txgbe_rxtx_vec_common.h |   17 +-
>  26 files changed, 9464 insertions(+), 444 deletions(-)
>  create mode 100644 drivers/net/txgbe/base/txgbe_e56.c
>  create mode 100644 drivers/net/txgbe/base/txgbe_e56.h
>  create mode 100644 drivers/net/txgbe/base/txgbe_e56_bp.c
>  create mode 100644 drivers/net/txgbe/base/txgbe_e56_bp.h
> 

AI review was mostly clean, do you want to change this one thing?

Review of [PATCH v6 00/21] net/{txgbe,ngbe} fixes from Zaiyu Wang

All findings from the v5 review are addressed:

  - 07/21: Tx desc free now loads the 32-bit headwb_mem atomically and
    casts to uint16_t at the use site, removing the type pun.
  - 14/21: kr_read_poll() switches from usleep() to usec_delay() to
    match the rest of the txgbe base layer.
  - 16/21: stray tab+space indent in txgbe_read_i2c_sff8636() is gone.
  - 17/21: stray tab+space indent in txgbe_get_module_info() is gone.
    TXGBE_SFF_DDM_IMPLEMENTED is now indented as a bit mask under the
    8472_SWAP register definition, which reads correctly.
  - 18/21: page is reset to 0 inside each iteration of the for loop so
    it no longer accumulates across iterations.  The flat-memory check
    is now driven by an explicit read of module byte 2 via
    read_i2c_sff8636() into a local is_flat_mem flag, instead of
    reading data[0x2] out of the caller's output buffer.  databyte is
    cleared to 0 at the top of each loop body so the skipped-read
    branch can no longer leak the previous iteration's value.
  - 20/21: RTE_LIBRTE_SECURITY is corrected to RTE_LIB_SECURITY so the
    !txq->using_ipsec guard is actually preprocessed in.

One small remaining comment on 18/21, not blocking.

Patch 18/21 (net/txgbe: fix get EEPROM operation)

Info: is_flat_mem has inverted semantics relative to its name, which
is going to confuse future maintainers.  SFF-8636 byte 2 bit 2
("Flat_mem") is 1 for flat-memory modules and 0 for paged-memory
modules (this is the same convention ethtool uses).  The new code
treats the bit the opposite way:

  +	bool is_flat_mem = true;
  ...
  +		if (rdata & 0x4)
  +			is_flat_mem = false;
  ...
  +			if (page == 0 || is_flat_mem) {
  +				status = hw->phy.read_i2c_sff8636(hw, page, ...);

So when the module reports flat (bit set), is_flat_mem is set to
false; when paged (bit clear), is_flat_mem stays true.  The condition
'page == 0 || is_flat_mem' reads only page 0 in the flat case and any
page in the paged case, which is the right behavior, but the variable
ends up meaning "paged memory is OK to read" rather than "module is
flat".  Suggest either:

	bool is_flat_mem = false;
	if (rdata & 0x4)
		is_flat_mem = true;
	...
	if (page == 0 || !is_flat_mem) {
		...
	}

or renaming to allow_paged_reads (or similar) and keeping the existing
default and condition.