[PATCH net-next] net: libwx: rename wx_pf_flags to wx_flags

[PATCH net-next] net: libwx: rename wx_pf_flags to wx_flags

[Jiawen Wu]

The flags defined in `enum wx_pf_flags` and its terminator
`WX_PF_FLAGS_NBITS` were originally named with a "PF" (Physical Function)
prefix. However, these flags are not exclusive to PFs and are also
utilized by Virtual Functions (VFs).

Rename the enumeration to `enum wx_flags` and the bit count to
`WX_FLAGS_NBITS` to accurately reflect their shared usage across both PF
and VF drivers, and to prevent potential confusion.

Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>
---
 drivers/net/ethernet/wangxun/libwx/wx_hw.c   | 2 +-
 drivers/net/ethernet/wangxun/libwx/wx_ptp.c  | 2 +-
 drivers/net/ethernet/wangxun/libwx/wx_type.h | 6 +++---
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/wangxun/libwx/wx_hw.c b/drivers/net/ethernet/wangxun/libwx/wx_hw.c
index 122c4952d203..59cdd7d71e44 100644
--- a/drivers/net/ethernet/wangxun/libwx/wx_hw.c
+++ b/drivers/net/ethernet/wangxun/libwx/wx_hw.c
@@ -2520,7 +2520,7 @@ int wx_sw_init(struct wx *wx)
 	spin_lock_init(&wx->hw_stats_lock);
 	mutex_init(&wx->reset_lock);
 	bitmap_zero(wx->state, WX_STATE_NBITS);
-	bitmap_zero(wx->flags, WX_PF_FLAGS_NBITS);
+	bitmap_zero(wx->flags, WX_FLAGS_NBITS);
 	set_bit(WX_STATE_DOWN, wx->state);
 	wx->misc_irq_domain = false;
 
diff --git a/drivers/net/ethernet/wangxun/libwx/wx_ptp.c b/drivers/net/ethernet/wangxun/libwx/wx_ptp.c
index 3eea647c4742..e35e854652ec 100644
--- a/drivers/net/ethernet/wangxun/libwx/wx_ptp.c
+++ b/drivers/net/ethernet/wangxun/libwx/wx_ptp.c
@@ -558,7 +558,7 @@ static int wx_ptp_set_timestamp_mode(struct wx *wx,
 {
 	u32 tsync_tx_ctl = WX_TSC_1588_CTL_ENABLED;
 	u32 tsync_rx_ctl = WX_PSR_1588_CTL_ENABLED;
-	DECLARE_BITMAP(flags, WX_PF_FLAGS_NBITS);
+	DECLARE_BITMAP(flags, WX_FLAGS_NBITS);
 	u32 tsync_rx_mtrl = PTP_EV_PORT << 16;
 	bool is_l2 = false;
 	u32 regval;
diff --git a/drivers/net/ethernet/wangxun/libwx/wx_type.h b/drivers/net/ethernet/wangxun/libwx/wx_type.h
index 2eba5ab59925..39cec52c3d2f 100644
--- a/drivers/net/ethernet/wangxun/libwx/wx_type.h
+++ b/drivers/net/ethernet/wangxun/libwx/wx_type.h
@@ -1269,7 +1269,7 @@ struct wx_rss_flow_map {
 	u8 flag;
 };
 
-enum wx_pf_flags {
+enum wx_flags {
 	WX_FLAG_MULTI_64_FUNC,
 	WX_FLAG_SWFW_RING,
 	WX_FLAG_VMDQ_ENABLED,
@@ -1291,13 +1291,13 @@ enum wx_pf_flags {
 	WX_FLAG_RX_MERGE_ENABLED,
 	WX_FLAG_TXHEAD_WB_ENABLED,
 	WX_FLAG_NEED_PCIE_RECOVERY,
-	WX_PF_FLAGS_NBITS               /* must be last */
+	WX_FLAGS_NBITS               /* must be last */
 };
 
 struct wx {
 	unsigned long active_vlans[BITS_TO_LONGS(VLAN_N_VID)];
 	DECLARE_BITMAP(state, WX_STATE_NBITS);
-	DECLARE_BITMAP(flags, WX_PF_FLAGS_NBITS);
+	DECLARE_BITMAP(flags, WX_FLAGS_NBITS);
 
 	void *priv;
 	u8 __iomem *hw_addr;
-- 
2.51.0

Re: [PATCH net-next] net: libwx: rename wx_pf_flags to wx_flags

[Breno Leitao]

On Tue, Aug 11, 2026 at 02:50:38PM +0800, Jiawen Wu wrote:
> The flags defined in `enum wx_pf_flags` and its terminator
> `WX_PF_FLAGS_NBITS` were originally named with a "PF" (Physical Function)
> prefix. However, these flags are not exclusive to PFs and are also
> utilized by Virtual Functions (VFs).
> 
> Rename the enumeration to `enum wx_flags` and the bit count to
> `WX_FLAGS_NBITS` to accurately reflect their shared usage across both PF
> and VF drivers, and to prevent potential confusion.
> 
> Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>

Reviewed-by: Breno Leitao <leitao@debian.org>

Re: [PATCH net-next] net: libwx: rename wx_pf_flags to wx_flags

[Jakub Kicinski]

This is an AI-generated review of your patch. The human sending this
email says "Sashiko points out a pre-existing issue. Let's fix it
first since the rename will conflict with the fix?"
---
net: libwx: rename wx_pf_flags to wx_flags

This renames enum wx_pf_flags to enum wx_flags and its terminator
WX_PF_FLAGS_NBITS to WX_FLAGS_NBITS, since the flags are used by both the
PF and VF drivers.  The three users of the old terminator name in wx_hw.c,
wx_ptp.c and wx_type.h are updated accordingly.

> diff --git a/drivers/net/ethernet/wangxun/libwx/wx_ptp.c b/drivers/net/ethernet/wangxun/libwx/wx_ptp.c
> index 3eea647c4742c..e35e854652ece 100644
> --- a/drivers/net/ethernet/wangxun/libwx/wx_ptp.c
> +++ b/drivers/net/ethernet/wangxun/libwx/wx_ptp.c
> @@ -558,7 +558,7 @@ static int wx_ptp_set_timestamp_mode(struct wx *wx,
>  {
>  	u32 tsync_tx_ctl = WX_TSC_1588_CTL_ENABLED;
>  	u32 tsync_rx_ctl = WX_PSR_1588_CTL_ENABLED;
> -	DECLARE_BITMAP(flags, WX_PF_FLAGS_NBITS);
> +	DECLARE_BITMAP(flags, WX_FLAGS_NBITS);
>  	u32 tsync_rx_mtrl = PTP_EV_PORT << 16;
>  	bool is_l2 = false;
>  	u32 regval;

[Severity: Medium]
This isn't a bug introduced by this patch, but while this local bitmap is
being touched by the rename: can the copy-in/copy-out of wx->flags in
wx_ptp_set_timestamp_mode() drop bits that another CPU sets atomically?

The function snapshots the whole bitmap, changes only the two
WX_FLAG_RX_HWTSTAMP_* bits in the local copy, programs several registers,
and then writes the entire bitmap back:

drivers/net/ethernet/wangxun/libwx/wx_ptp.c:wx_ptp_set_timestamp_mode() {
	DECLARE_BITMAP(flags, WX_FLAGS_NBITS);
	...
	memcpy(flags, wx->flags, sizeof(wx->flags));
	...
	set_bit(WX_FLAG_RX_HWTSTAMP_ENABLED, flags);
	set_bit(WX_FLAG_RX_HWTSTAMP_IN_REGISTER, flags);
	...
	WX_WRITE_FLUSH(wx);

	/* configure adapter flags only when HW is actually configured */
	memcpy(wx->flags, flags, sizeof(wx->flags));
}

All WX_FLAG_* bits fit in one unsigned long, and every other user in the
driver updates individual bits with atomic bitops.  Some of those writers do
not take rtnl, so they are not serialized against the ndo_hwtstamp_set path:

drivers/net/ethernet/wangxun/txgbe/txgbe_aml.c:txgbe_gpio_irq_handler_aml()
	set_bit(WX_FLAG_NEED_MODULE_RESET, wx->flags);

drivers/net/ethernet/wangxun/libwx/wx_err.c:wx_tx_timeout_reset()
	set_bit(WX_FLAG_NEED_DO_RESET, wx->flags);

drivers/net/ethernet/wangxun/libwx/wx_err.c:wx_tx_timeout_recovery()
	set_bit(WX_FLAG_NEED_PCIE_RECOVERY, wx->flags);

So a request set between the two memcpy calls would be overwritten by the
write-back, and the service task would then find nothing to do:

drivers/net/ethernet/wangxun/txgbe/txgbe_main.c:txgbe_service_event_sfp() {
	if (!test_and_clear_bit(WX_FLAG_NEED_MODULE_RESET, wx->flags))
		return;
	...
}

The mirror case is a bit another CPU has just cleared being restored by the
write-back, for example the clear in wx_err.c:

	if (!test_and_clear_bit(WX_FLAG_NEED_DO_RESET, wx->flags))

which would then look like a fresh reset request.

Would applying the two bit changes directly to wx->flags with set_bit() and
clear_bit() after the registers are programmed, instead of copying the whole
bitmap in and out, work here?
-- 
pw-bot: cr