[PATCH net-next v3] net: wangxun: use cached NCSI and WOL capability flags

[PATCH net-next v3] net: wangxun: use cached NCSI and WOL capability flags

[Jiawen Wu]

wx_disable_rx() and txgbe_disable_device() open-code the same subsystem
device ID tests to decide whether the MAC receiver and transmitter have
to stay enabled for out-of-band management:

	(subsystem_device_id & WX_NCSI_MASK) == WX_NCSI_SUP ||
	(subsystem_device_id & WX_WOL_MASK) == WX_WOL_SUP

ngbe_init_type_code() already decodes the subsystem device ID into
wx->ncsi_enabled and wx->wol_hw_supported. Do the same in
txgbe_init_type_code() and let both call sites test the cached flags, so
that each driver decides the capability once while probing instead of
having shared code re-derive it from raw IDs.

This is not equivalent for ngbe, and that is intentional.
wx->ncsi_enabled has never been read since it was added by commit
02338c484ab6 ("net: ngbe: Initialize sw info and register netdev"), and
it is wider than the inline test:

	wx->ncsi_enabled = (ncsi_mask == WX_NCSI_SUP ||
			   type_mask == NGBE_SUBID_OCP_CARD) ? 1 : 0;

OCP mezzanine cards are NCSI capable by design, so the NCSI semantics do
apply to them, and they need the MAC receiver to keep running for
out-of-band management. The inline test does not cover the OCP card
type, so on such a card that does not have the NCSI bit set the receiver
was turned off on every path reaching wx_disable_rx(), i.e.
ngbe_disable_device() on ifdown, wx_stop_adapter() from ngbe_reset_hw()
and wx_configure_rx(), and management traffic was dropped. Reading
ncsi_enabled keeps the receiver enabled on these cards.

No Fixes: tag is added on purpose. The broken behaviour goes back to the
initial ngbe support in v6.2, and wx_disable_rx() has since been moved
into libwx and reworked several times, so this does not cherry-pick into
stable kernels in any useful form.

For txgbe both flags are newly initialized, but wol_hw_supported is only
consumed by txgbe_disable_device() there - txgbe does not implement the
WoL ethtool ops - so txgbe behaviour is unchanged.

Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>
---
v3:
- Rewritting commit message.

v2: https://lore.kernel.org/all/F76F75E3FFF42A39+20260908025416.42250-1-jiawenwu@trustnetic.com
- Remove single-use locals.
- Describe the behavior change on ngbe OCP cards.

v1: https://lore.kernel.org/all/95D34449BA183C54+20260901070238.78509-1-jiawenwu@trustnetic.com
---
 drivers/net/ethernet/wangxun/libwx/wx_hw.c      | 3 +--
 drivers/net/ethernet/wangxun/txgbe/txgbe_main.c | 6 ++++--
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/wangxun/libwx/wx_hw.c b/drivers/net/ethernet/wangxun/libwx/wx_hw.c
index 111aadf79208..2490c4dd548f 100644
--- a/drivers/net/ethernet/wangxun/libwx/wx_hw.c
+++ b/drivers/net/ethernet/wangxun/libwx/wx_hw.c
@@ -1313,8 +1313,7 @@ void wx_disable_rx(struct wx *wx)
 		rxctrl &= ~WX_RDB_PB_CTL_RXEN;
 		wr32(wx, WX_RDB_PB_CTL, rxctrl);
 
-		if (!(((wx->subsystem_device_id & WX_NCSI_MASK) == WX_NCSI_SUP) ||
-		      ((wx->subsystem_device_id & WX_WOL_MASK) == WX_WOL_SUP))) {
+		if (!(wx->ncsi_enabled || wx->wol_hw_supported)) {
 			/* disable mac receiver */
 			wr32m(wx, WX_MAC_RX_CFG,
 			      WX_MAC_RX_CFG_RE, 0);
diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c b/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c
index eb91c4f28ecd..0ecbd1c0e553 100644
--- a/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c
+++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c
@@ -267,8 +267,7 @@ static void txgbe_disable_device(struct wx *wx)
 		wx_set_all_vfs(wx);
 	}
 
-	if (!(((wx->subsystem_device_id & WX_NCSI_MASK) == WX_NCSI_SUP) ||
-	      ((wx->subsystem_device_id & WX_WOL_MASK) == WX_WOL_SUP))) {
+	if (!(wx->ncsi_enabled || wx->wol_hw_supported)) {
 		/* disable mac transmiter */
 		wr32m(wx, WX_MAC_TX_CFG, WX_MAC_TX_CFG_TE, 0);
 	}
@@ -381,6 +380,9 @@ static void txgbe_init_type_code(struct wx *wx)
 		wx->media_type = wx_media_unknown;
 		break;
 	}
+
+	wx->wol_hw_supported = ((wx->subsystem_device_id & WX_WOL_MASK) == WX_WOL_SUP);
+	wx->ncsi_enabled = ((wx->subsystem_device_id & WX_NCSI_MASK) == WX_NCSI_SUP);
 }
 
 /**
-- 
2.51.0

RE: [PATCH net-next v3] net: wangxun: use cached NCSI and WOL capability flags

[Jiawen Wu]

On Wed, Sep 16, 2026 2:33 PM, Jiawen Wu wrote:
> wx_disable_rx() and txgbe_disable_device() open-code the same subsystem
> device ID tests to decide whether the MAC receiver and transmitter have
> to stay enabled for out-of-band management:
> 
> 	(subsystem_device_id & WX_NCSI_MASK) == WX_NCSI_SUP ||
> 	(subsystem_device_id & WX_WOL_MASK) == WX_WOL_SUP
> 
> ngbe_init_type_code() already decodes the subsystem device ID into
> wx->ncsi_enabled and wx->wol_hw_supported. Do the same in
> txgbe_init_type_code() and let both call sites test the cached flags, so
> that each driver decides the capability once while probing instead of
> having shared code re-derive it from raw IDs.
> 
> This is not equivalent for ngbe, and that is intentional.
> wx->ncsi_enabled has never been read since it was added by commit
> 02338c484ab6 ("net: ngbe: Initialize sw info and register netdev"), and
> it is wider than the inline test:
> 
> 	wx->ncsi_enabled = (ncsi_mask == WX_NCSI_SUP ||
> 			   type_mask == NGBE_SUBID_OCP_CARD) ? 1 : 0;
> 
> OCP mezzanine cards are NCSI capable by design, so the NCSI semantics do
> apply to them, and they need the MAC receiver to keep running for
> out-of-band management. The inline test does not cover the OCP card
> type, so on such a card that does not have the NCSI bit set the receiver
> was turned off on every path reaching wx_disable_rx(), i.e.
> ngbe_disable_device() on ifdown, wx_stop_adapter() from ngbe_reset_hw()
> and wx_configure_rx(), and management traffic was dropped. Reading
> ncsi_enabled keeps the receiver enabled on these cards.
> 
> No Fixes: tag is added on purpose. The broken behaviour goes back to the
> initial ngbe support in v6.2, and wx_disable_rx() has since been moved
> into libwx and reworked several times, so this does not cherry-pick into
> stable kernels in any useful form.
> 
> For txgbe both flags are newly initialized, but wol_hw_supported is only
> consumed by txgbe_disable_device() there - txgbe does not implement the
> WoL ethtool ops - so txgbe behaviour is unchanged.
> 
> Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>

This patch should be deferred.
Another conflicting fix needs to be committed first.

> ---
> v3:
> - Rewritting commit message.
> 
> v2: https://lore.kernel.org/all/F76F75E3FFF42A39+20260908025416.42250-1-jiawenwu@trustnetic.com
> - Remove single-use locals.
> - Describe the behavior change on ngbe OCP cards.
> 
> v1: https://lore.kernel.org/all/95D34449BA183C54+20260901070238.78509-1-jiawenwu@trustnetic.com
> ---
>  drivers/net/ethernet/wangxun/libwx/wx_hw.c      | 3 +--
>  drivers/net/ethernet/wangxun/txgbe/txgbe_main.c | 6 ++++--
>  2 files changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/wangxun/libwx/wx_hw.c b/drivers/net/ethernet/wangxun/libwx/wx_hw.c
> index 111aadf79208..2490c4dd548f 100644
> --- a/drivers/net/ethernet/wangxun/libwx/wx_hw.c
> +++ b/drivers/net/ethernet/wangxun/libwx/wx_hw.c
> @@ -1313,8 +1313,7 @@ void wx_disable_rx(struct wx *wx)
>  		rxctrl &= ~WX_RDB_PB_CTL_RXEN;
>  		wr32(wx, WX_RDB_PB_CTL, rxctrl);
> 
> -		if (!(((wx->subsystem_device_id & WX_NCSI_MASK) == WX_NCSI_SUP) ||
> -		      ((wx->subsystem_device_id & WX_WOL_MASK) == WX_WOL_SUP))) {
> +		if (!(wx->ncsi_enabled || wx->wol_hw_supported)) {
>  			/* disable mac receiver */
>  			wr32m(wx, WX_MAC_RX_CFG,
>  			      WX_MAC_RX_CFG_RE, 0);
> diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c b/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c
> index eb91c4f28ecd..0ecbd1c0e553 100644
> --- a/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c
> +++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c
> @@ -267,8 +267,7 @@ static void txgbe_disable_device(struct wx *wx)
>  		wx_set_all_vfs(wx);
>  	}
> 
> -	if (!(((wx->subsystem_device_id & WX_NCSI_MASK) == WX_NCSI_SUP) ||
> -	      ((wx->subsystem_device_id & WX_WOL_MASK) == WX_WOL_SUP))) {
> +	if (!(wx->ncsi_enabled || wx->wol_hw_supported)) {
>  		/* disable mac transmiter */
>  		wr32m(wx, WX_MAC_TX_CFG, WX_MAC_TX_CFG_TE, 0);
>  	}
> @@ -381,6 +380,9 @@ static void txgbe_init_type_code(struct wx *wx)
>  		wx->media_type = wx_media_unknown;
>  		break;
>  	}
> +
> +	wx->wol_hw_supported = ((wx->subsystem_device_id & WX_WOL_MASK) == WX_WOL_SUP);
> +	wx->ncsi_enabled = ((wx->subsystem_device_id & WX_NCSI_MASK) == WX_NCSI_SUP);
>  }
> 
>  /**
> --
> 2.51.0
> 

Re: [PATCH net-next v3] net: wangxun: use cached NCSI and WOL capability flags

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 0 (1)

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The patch is submitted as a mechanical cleanup ("use cached
  NCSI and WOL capability flags") but carries a real, user-visible…

Pre-existing issues:
- [Low] Semantic overload of wx->wol_hw_supported: two libwx consumers
  now read the same field with different meanings.

--

net: wangxun: use cached NCSI and WOL capability flags

This replaces the open-coded subsystem device ID tests in wx_disable_rx()
and txgbe_disable_device() with the cached wx->ncsi_enabled and
wx->wol_hw_supported flags, and initializes both flags in
txgbe_init_type_code().  For ngbe this changes behaviour, because
ngbe_init_type_code() also sets ncsi_enabled for OCP cards.

> diff --git a/drivers/net/ethernet/wangxun/libwx/wx_hw.c b/drivers/net/ethernet/wangxun/libwx/wx_hw.c
> index 59cdd7d71e446..6fbfbb77646c0 100644
> --- a/drivers/net/ethernet/wangxun/libwx/wx_hw.c
> +++ b/drivers/net/ethernet/wangxun/libwx/wx_hw.c
> @@ -1313,8 +1313,7 @@ void wx_disable_rx(struct wx *wx)
>  		rxctrl &= ~WX_RDB_PB_CTL_RXEN;
>  		wr32(wx, WX_RDB_PB_CTL, rxctrl);
>  
> -		if (!(((wx->subsystem_device_id & WX_NCSI_MASK) == WX_NCSI_SUP) ||
> -		      ((wx->subsystem_device_id & WX_WOL_MASK) == WX_WOL_SUP))) {
> +		if (!(wx->ncsi_enabled || wx->wol_hw_supported)) {
>  			/* disable mac receiver */
>  			wr32m(wx, WX_MAC_RX_CFG,
>  			      WX_MAC_RX_CFG_RE, 0);

[Severity: Medium]
Should the ngbe behaviour change here be a separate patch with a Fixes:
tag, rather than part of a patch whose subject describes a cleanup?

For txgbe the conversion looks bit-for-bit equivalent: WX_NCSI_SUP ==
WX_NCSI_MASK == 0x8000 and WX_WOL_SUP == WX_WOL_MASK == 0x4000 in
libwx/wx_type.h, and txgbe_init_type_code() runs from txgbe_sw_init()
before the first txgbe_reset_hw() and before any
txgbe_disable_device()/wx_disable_rx() call.

The whole functional delta comes from ngbe, where ncsi_enabled covers an
extra card type:

drivers/net/ethernet/wangxun/ngbe/ngbe_main.c:ngbe_init_type_code() {
	...
	wx->ncsi_enabled = (ncsi_mask == WX_NCSI_SUP ||
			   type_mask == NGBE_SUBID_OCP_CARD) ? 1 : 0;
	...
}

So on an OCP card with the 0x8000 NCSI bit clear, WX_MAC_RX_CFG_RE is no
longer cleared on the paths reaching wx_disable_rx():

  ngbe_close() -> ngbe_down() -> ngbe_disable_device() -> wx_disable_rx()
  ngbe_reset_hw() -> wx_stop_adapter() -> wx_disable_rx()
  wx_configure_rx() -> wx_disable_rx()

That is a user-visible fix for dropped out-of-band management traffic.
Could it be split out and carry a Fixes: tag identifying the introducing
commit, with the stable exclusion expressed separately (for example
Cc: stable+noautosel) instead of by omitting the tag?  The commit message
says:

	No Fixes: tag is added on purpose. The broken behaviour goes back to
	the initial ngbe support in v6.2, and wx_disable_rx() has since been
	moved into libwx and reworked several times, so this does not
	cherry-pick into stable kernels in any useful form.

Without the tag, does the provenance of the fix become hard to recover,
and does bundling it with the refactor make it hard to revert or bisect
the two changes independently?

> diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c b/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c
> index eb91c4f28ecd4..0ecbd1c0e5532 100644
> --- a/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c
> +++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c
> @@ -267,8 +267,7 @@ static void txgbe_disable_device(struct wx *wx)
>  		wx_set_all_vfs(wx);
>  	}
>  
> -	if (!(((wx->subsystem_device_id & WX_NCSI_MASK) == WX_NCSI_SUP) ||
> -	      ((wx->subsystem_device_id & WX_WOL_MASK) == WX_WOL_SUP))) {
> +	if (!(wx->ncsi_enabled || wx->wol_hw_supported)) {
>  		/* disable mac transmiter */
>  		wr32m(wx, WX_MAC_TX_CFG, WX_MAC_TX_CFG_TE, 0);
>  	}

[Severity: Low]
This is a pre-existing issue and not introduced by this patch, but is
wol_hw_supported now being read with two different meanings in libwx?

In wx_get_wol()/wx_set_wol() (libwx/wx_ethtool.c) it is strictly a
hardware capability gate, with the armed state kept in wx->wol:

	if (!wx->wol_hw_supported)
		return -EOPNOTSUPP;

	wx->wol = 0;
	if (wol->wolopts & WAKE_MAGIC)
		wx->wol = WX_PSR_WKUP_CTL_MAG;

The new tests in wx_disable_rx() and txgbe_disable_device() read the same
capability flag as "the MAC has to stay alive", so after
"ethtool -s ethX wol d" leaves wx->wol == 0, the MAC receiver (and the
txgbe transmitter) is still kept enabled on any WoL-capable board.  This
matches the removed open-coded test exactly, since WX_WOL_SUP ==
WX_WOL_MASK == 0x4000, so nothing changes at runtime today.

Would testing wx->ncsi_enabled || wx->wol, or introducing a flag named
for the actual decision (keep the MAC alive for out-of-band management),
make the shared-code contract explicit?  Otherwise, narrowing
wol_hw_supported later, or wiring the WoL ethtool ops into txgbe, would
silently change whether the MAC stays enabled.

> @@ -381,6 +380,9 @@ static void txgbe_init_type_code(struct wx *wx)
>  		wx->media_type = wx_media_unknown;
>  		break;
>  	}
> +
> +	wx->wol_hw_supported = ((wx->subsystem_device_id & WX_WOL_MASK) == WX_WOL_SUP);
> +	wx->ncsi_enabled = ((wx->subsystem_device_id & WX_NCSI_MASK) == WX_NCSI_SUP);
>  }
>  
>  /**

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/E5215E3514E295BA%2B20260916063312.203457-1-jiawenwu%40trustnetic.com