On Tue, Jun 2, 2026 7:16 PM, Larysa Zaremba wrote:

On Mon, Jun 01, 2026 at 03:22:20PM +0800, Jiawen Wu wrote:

quoted

Introduce device-specific soft quiesce callbacks for ngbe and txgbe to
provide a lightweight shutdown path during PCI error recovery. It avoids
MMIO-dependent operations in PCI error status, for the later
implementation of PCIe error callback function in libwx.

Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>
---
 drivers/net/ethernet/wangxun/libwx/wx_type.h  |  1 +
 drivers/net/ethernet/wangxun/ngbe/ngbe_main.c | 23 +++++++++++++++++++
 .../net/ethernet/wangxun/txgbe/txgbe_main.c   | 22 ++++++++++++++++++
 3 files changed, 46 insertions(+)

diff --git a/drivers/net/ethernet/wangxun/libwx/wx_type.h b/drivers/net/ethernet/wangxun/libwx/wx_type.h
index a8b4e84787f4..1b25a52188f7 100644
--- a/drivers/net/ethernet/wangxun/libwx/wx_type.h
+++ b/drivers/net/ethernet/wangxun/libwx/wx_type.h

@@ -1409,6 +1409,7 @@ struct wx {
 	void (*configure_fdir)(struct wx *wx);
 	int (*setup_tc)(struct net_device *netdev, u8 tc);
 	void (*do_reset)(struct net_device *netdev, bool reinit);
+	void (*soft_quiesce)(struct wx *wx);
 	int (*ptp_setup_sdp)(struct wx *wx);
 	void (*set_num_queues)(struct wx *wx);

diff --git a/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c b/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c
index 3dc8342dd3a7..2484d3177034 100644
--- a/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c
+++ b/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c

@@ -47,6 +47,28 @@ static const struct pci_device_id ngbe_pci_tbl[] = {
 	{ }
 };

+static void ngbe_soft_quiesce(struct wx *wx)
+{
+	if (test_and_set_bit(WX_STATE_DOWN, wx->state))
+		return;

Careful about reusing the same flags for different reinit processes.

I think WX_STATE_DOWN should be used here.
The soft_quiesce calls back only in pcie error handler when netif is running.
It is desired to implement all software quiesce on the device close path.

quoted

+
+	wx_ptp_stop(wx);
+	phylink_stop(wx->phylink);
+	pci_clear_master(wx->pdev);
+	wx_napi_disable_all(wx);

Sashiko says:

 "If NAPI is not disabled yet, the NAPI poll routine wx_clean_tx_irq() can run
  concurrently. If WX_STATE_PTP_TX_IN_PROGRESS is set, wx_clean_tx_irq() might
  call ptp_schedule_worker(), leading to a dereference of the ptp_clock
  object just as it's being freed."

I am not sure, if such race actually happens, but stopping napi before any kind
of deinit is not a terrible idea.

WX_STATE_PTP_TX_IN_PROGRESS is cleared in wx_ptp_stop().
But I'll change wx_ptp_stop() to wx_ptp_quiesce() that drop all MMIO operations.

quoted

+
+	clear_bit(WX_FLAG_NEED_PF_RESET, wx->flags);

Same here, what if a caller needs a full reset?

PCIe error handler has a higher priority and it will achieve a more complete
reset. In fact, these two resets are not expected to be executed at the same
time.

quoted

+	timer_delete_sync(&wx->service_timer);

Sashiko says:

 "The timer is stopped, but the corresponding cancel_work_sync(&wx->service_task)
  is missing."

And it does seem to me that this is the case here. I see that you have explained
this in another thread, but you proposal does not address the case, when
ngbe_service_task() starts, interface is not down, and after that we have a PCI
error, and its handling interferes with service task. While this is unlikely,
synchronization should be more robust.

I plan to check WX_STATE_DOWN and WX_STATE_RESETTING at the entry of every work
item. Looks like a rough block, but is there a better way? :(

quoted

+
+	wx_clean_all_tx_rings(wx);
+	wx_clean_all_rx_rings(wx);
+
+	wx_free_irq(wx);
+	wx_free_isb_resources(wx);
+	wx_free_resources(wx);
+	phylink_disconnect_phy(wx->phylink);
+}
+
 /**
  *  ngbe_init_type_code - Initialize the shared code
  *  @wx: pointer to hardware structure

@@ -135,6 +157,7 @@ static int ngbe_sw_init(struct wx *wx)
 	wx->mbx.size = WX_VXMAILBOX_SIZE;
 	wx->setup_tc = ngbe_setup_tc;
 	wx->do_reset = ngbe_do_reset;
+	wx->soft_quiesce = ngbe_soft_quiesce;
 	set_bit(0, &wx->fwd_bitmask);

 	return 0;

diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c b/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c
index b37c9ed57cf7..816aba4a9099 100644
--- a/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c
+++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c

@@ -296,6 +296,27 @@ void txgbe_up(struct wx *wx)
 	txgbe_up_complete(wx);
 }

+static void txgbe_soft_quiesce(struct wx *wx)
+{
+	if (test_and_set_bit(WX_STATE_DOWN, wx->state))
+		return;
+
+	wx_ptp_stop(wx);
+	phylink_stop(wx->phylink);
+	pci_clear_master(wx->pdev);
+	wx_napi_disable_all(wx);
+
+	clear_bit(WX_FLAG_NEED_PF_RESET, wx->flags);
+	timer_delete_sync(&wx->service_timer);
+
+	wx_clean_all_tx_rings(wx);
+	wx_clean_all_rx_rings(wx);
+
+	wx_free_irq(wx);
+	txgbe_free_misc_irq(wx->priv);
+	wx_free_resources(wx);
+}
+
 /**
  *  txgbe_init_type_code - Initialize the shared code
  *  @wx: pointer to hardware structure

@@ -412,6 +433,7 @@ static int txgbe_sw_init(struct wx *wx)

 	wx->setup_tc = txgbe_setup_tc;
 	wx->do_reset = txgbe_do_reset;
+	wx->soft_quiesce = txgbe_soft_quiesce;
 	set_bit(0, &wx->fwd_bitmask);

 	switch (wx->mac.type) {
--

2.51.0