Re: [PATCH net-next v4 4/5] net: wangxun: introduce soft quiesce callbacks for AER recovery
From: Larysa Zaremba <hidden>
Date: 2026-06-02 11:16:34
On Mon, Jun 01, 2026 at 03:22:20PM +0800, Jiawen Wu wrote:
quoted hunk ↗ jump to hunk
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.
+ + 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.
+ + clear_bit(WX_FLAG_NEED_PF_RESET, wx->flags);
Same here, what if a caller needs a full reset?
+ 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.
quoted hunk ↗ jump to hunk
+ + 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