From: Xuanqiang Luo <[email protected]>
The time sync interrupt queues ptp_extts0_work, which reads device
registers and reports events through pf->ptp_clock. i40e_ptp_stop()
unregisters the PHC before disabling the event source or draining the
work. The worker can therefore use the clock or PF after either has been
freed.
Move the event-source shutdown before PHC unregister. i40e_ptp_stop()
first clears I40E_FLAG_PTP_ENA under ptp_config_lock, so timestamp
configuration cannot re-enable the source after the lock is released.
Then call disable_work_sync() to reject later queue attempts and drain
work already pending or running.
INIT_WORK() is currently called from i40e_ptp_set_timestamp_mode(), which
runs on reset and whenever timestamping is reconfigured. Reinitializing
pending or running work can corrupt its workqueue state. Initialize the
work once from i40e_sw_init() instead.
Fixes: 1050713026a0 ("i40e: add support for PTP external synchronization clock")
Signed-off-by: Xuanqiang Luo <[email protected]>
---
drivers/net/ethernet/intel/i40e/i40e.h | 1 +
drivers/net/ethernet/intel/i40e/i40e_main.c | 1 +
drivers/net/ethernet/intel/i40e/i40e_ptp.c | 25 +++++++++++++++------
3 files changed, 20 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h
index 84564d747d09a..18452f9aa74e1 100644
--- a/drivers/net/ethernet/intel/i40e/i40e.h
+++ b/drivers/net/ethernet/intel/i40e/i40e.h
@@ -1318,6 +1318,7 @@ int i40e_ptp_hwtstamp_set(struct net_device *netdev,
struct netlink_ext_ack *extack);
void i40e_ptp_save_hw_time(struct i40e_pf *pf);
void i40e_ptp_restore_hw_time(struct i40e_pf *pf);
+void i40e_ptp_init_work(struct i40e_pf *pf);
void i40e_ptp_init(struct i40e_pf *pf);
void i40e_ptp_stop(struct i40e_pf *pf);
int i40e_ptp_alloc_pins(struct i40e_pf *pf);
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index c19d4c81d6532..48ba38a452d0b 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -12822,6 +12822,7 @@ static int i40e_sw_init(struct i40e_pf *pf)
}
mutex_init(&pf->switch_mutex);
mutex_init(&pf->ptp_config_lock);
+ i40e_ptp_init_work(pf);
spin_lock_init(&pf->ptp_tx_lock);
sw_init_done:
diff --git a/drivers/net/ethernet/intel/i40e/i40e_ptp.c b/drivers/net/ethernet/intel/i40e/i40e_ptp.c
index f194869947124..5f15a3d4a91ed 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_ptp.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_ptp.c
@@ -169,6 +169,17 @@ static void i40e_ptp_extts0_work(struct work_struct *work)
ptp_clock_event(pf->ptp_clock, &event);
}
+/**
+ * i40e_ptp_init_work - Initialize PTP work for a PF
+ * @pf: Board private structure
+ *
+ * Initialize work which must remain valid for the lifetime of the PF.
+ */
+void i40e_ptp_init_work(struct i40e_pf *pf)
+{
+ INIT_WORK(&pf->ptp_extts0_work, i40e_ptp_extts0_work);
+}
+
/**
* i40e_is_ptp_pin_dev - check if device supports PTP pins
* @hw: pointer to the hardware structure
@@ -1201,8 +1212,6 @@ static int i40e_ptp_set_timestamp_mode(struct i40e_pf *pf,
regval |= 1 << I40E_PRTTSYN_CTL0_EVENT_INT_ENA_SHIFT;
wr32(hw, I40E_PRTTSYN_CTL0, regval);
- INIT_WORK(&pf->ptp_extts0_work, i40e_ptp_extts0_work);
-
switch (config->tx_type) {
case HWTSTAMP_TX_OFF:
pf->ptp_tx = false;
@@ -1576,6 +1585,13 @@ void i40e_ptp_stop(struct i40e_pf *pf)
pf->ptp_rx = false;
mutex_unlock(&pf->ptp_config_lock);
+ /* Stop external timestamp events before unregistering the clock. */
+ regval = rd32(hw, I40E_PRTTSYN_CTL0);
+ regval &= ~I40E_PRTTSYN_CTL0_EVENT_INT_ENA_MASK;
+ wr32(hw, I40E_PRTTSYN_CTL0, regval);
+
+ disable_work_sync(&pf->ptp_extts0_work);
+
if (skb)
dev_kfree_skb_any(skb);
@@ -1596,10 +1612,5 @@ void i40e_ptp_stop(struct i40e_pf *pf)
regval &= ~I40E_PRTTSYN_AUX_0_PTPFLAG_MASK;
wr32(hw, I40E_PRTTSYN_AUX_0(0), regval);
- /* Disable interrupts */
- regval = rd32(hw, I40E_PRTTSYN_CTL0);
- regval &= ~I40E_PRTTSYN_CTL0_EVENT_INT_ENA_MASK;
- wr32(hw, I40E_PRTTSYN_CTL0, regval);
-
i40e_ptp_free_pins(pf);
}
--
2.43.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not
affiliated with the servers or forums shown here and is not responsible for
the content of articles, which is written by their respective authors.