* [PATCH net-next 0/7][pull request] ice: fix timestamping in reset process
@ 2024-01-25 21:57 Tony Nguyen
2024-01-25 21:57 ` [PATCH net-next 1/7] ice: introduce PTP state machine Tony Nguyen
` (7 more replies)
0 siblings, 8 replies; 9+ messages in thread
From: Tony Nguyen @ 2024-01-25 21:57 UTC (permalink / raw)
To: davem, kuba, pabeni, edumazet, netdev
Cc: Tony Nguyen, richardcochran, karol.kolacinski, jacob.e.keller
Karol Kolacinski says:
PTP reset process has multiple places where timestamping can end up in
an incorrect state.
This series introduces a proper state machine for PTP and refactors
a large part of the code to ensure that timestamping does not break.
The following are changes since commit 91374ba537bd60caa9ae052c9f1c0fe055b39149:
net: dsa: mt7530: support OF-based registration of switch MDIO bus
and are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/tnguy/next-queue 100GbE
Jacob Keller (7):
ice: introduce PTP state machine
ice: pass reset type to PTP reset functions
ice: rename verify_cached to has_ready_bitmap
ice: don't check has_ready_bitmap in E810 functions
ice: rename ice_ptp_tx_cfg_intr
ice: factor out ice_ptp_rebuild_owner()
ice: stop destroying and reinitalizing Tx tracker during reset
drivers/net/ethernet/intel/ice/ice.h | 1 -
drivers/net/ethernet/intel/ice/ice_ethtool.c | 2 +-
drivers/net/ethernet/intel/ice/ice_main.c | 4 +-
drivers/net/ethernet/intel/ice/ice_ptp.c | 229 +++++++++++--------
drivers/net/ethernet/intel/ice/ice_ptp.h | 34 ++-
5 files changed, 164 insertions(+), 106 deletions(-)
--
2.41.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH net-next 1/7] ice: introduce PTP state machine
2024-01-25 21:57 [PATCH net-next 0/7][pull request] ice: fix timestamping in reset process Tony Nguyen
@ 2024-01-25 21:57 ` Tony Nguyen
2024-01-25 21:57 ` [PATCH net-next 2/7] ice: pass reset type to PTP reset functions Tony Nguyen
` (6 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Tony Nguyen @ 2024-01-25 21:57 UTC (permalink / raw)
To: davem, kuba, pabeni, edumazet, netdev
Cc: Jacob Keller, anthony.l.nguyen, richardcochran, Karol Kolacinski,
Simon Horman, Pucha Himasekhar Reddy
From: Jacob Keller <jacob.e.keller@intel.com>
Add PTP state machine so that the driver can correctly identify PTP
state around resets.
When the driver got information about ungraceful reset, PTP was not
prepared for reset and it returned error. When this situation occurs,
prepare PTP before rebuilding its structures.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Co-developed-by: Karol Kolacinski <karol.kolacinski@intel.com>
Signed-off-by: Karol Kolacinski <karol.kolacinski@intel.com>
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Reviewed-by: Simon Horman <horms@kernel.org>
Tested-by: Pucha Himasekhar Reddy <himasekharx.reddy.pucha@intel.com> (A Contingent worker at Intel)
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
drivers/net/ethernet/intel/ice/ice.h | 1 -
drivers/net/ethernet/intel/ice/ice_ethtool.c | 2 +-
drivers/net/ethernet/intel/ice/ice_ptp.c | 110 +++++++++++--------
drivers/net/ethernet/intel/ice/ice_ptp.h | 10 ++
4 files changed, 74 insertions(+), 49 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice.h b/drivers/net/ethernet/intel/ice/ice.h
index 367b613d92c0..97c2a5fb5dbf 100644
--- a/drivers/net/ethernet/intel/ice/ice.h
+++ b/drivers/net/ethernet/intel/ice/ice.h
@@ -493,7 +493,6 @@ enum ice_pf_flags {
ICE_FLAG_DCB_ENA,
ICE_FLAG_FD_ENA,
ICE_FLAG_PTP_SUPPORTED, /* PTP is supported by NVM */
- ICE_FLAG_PTP, /* PTP is enabled by software */
ICE_FLAG_ADV_FEATURES,
ICE_FLAG_TC_MQPRIO, /* support for Multi queue TC */
ICE_FLAG_CLS_FLOWER,
diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool.c b/drivers/net/ethernet/intel/ice/ice_ethtool.c
index a19b06f18e40..55fcf17d503e 100644
--- a/drivers/net/ethernet/intel/ice/ice_ethtool.c
+++ b/drivers/net/ethernet/intel/ice/ice_ethtool.c
@@ -3360,7 +3360,7 @@ ice_get_ts_info(struct net_device *dev, struct ethtool_ts_info *info)
struct ice_pf *pf = ice_netdev_to_pf(dev);
/* only report timestamping if PTP is enabled */
- if (!test_bit(ICE_FLAG_PTP, pf->flags))
+ if (pf->ptp.state != ICE_PTP_READY)
return ethtool_op_get_ts_info(dev, info);
info->so_timestamping = SOF_TIMESTAMPING_TX_SOFTWARE |
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c b/drivers/net/ethernet/intel/ice/ice_ptp.c
index 3b6605c8585e..8ed4af219f9b 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp.c
+++ b/drivers/net/ethernet/intel/ice/ice_ptp.c
@@ -1430,7 +1430,7 @@ void ice_ptp_link_change(struct ice_pf *pf, u8 port, bool linkup)
struct ice_ptp_port *ptp_port;
struct ice_hw *hw = &pf->hw;
- if (!test_bit(ICE_FLAG_PTP, pf->flags))
+ if (pf->ptp.state != ICE_PTP_READY)
return;
if (WARN_ON_ONCE(port >= ICE_NUM_EXTERNAL_PORTS))
@@ -2162,7 +2162,7 @@ int ice_ptp_get_ts_config(struct ice_pf *pf, struct ifreq *ifr)
{
struct hwtstamp_config *config;
- if (!test_bit(ICE_FLAG_PTP, pf->flags))
+ if (pf->ptp.state != ICE_PTP_READY)
return -EIO;
config = &pf->ptp.tstamp_config;
@@ -2232,7 +2232,7 @@ int ice_ptp_set_ts_config(struct ice_pf *pf, struct ifreq *ifr)
struct hwtstamp_config config;
int err;
- if (!test_bit(ICE_FLAG_PTP, pf->flags))
+ if (pf->ptp.state != ICE_PTP_READY)
return -EAGAIN;
if (copy_from_user(&config, ifr->ifr_data, sizeof(config)))
@@ -2616,7 +2616,7 @@ static void ice_ptp_periodic_work(struct kthread_work *work)
struct ice_pf *pf = container_of(ptp, struct ice_pf, ptp);
int err;
- if (!test_bit(ICE_FLAG_PTP, pf->flags))
+ if (pf->ptp.state != ICE_PTP_READY)
return;
err = ice_ptp_update_cached_phctime(pf);
@@ -2628,6 +2628,42 @@ static void ice_ptp_periodic_work(struct kthread_work *work)
msecs_to_jiffies(err ? 10 : 500));
}
+/**
+ * ice_ptp_prepare_for_reset - Prepare PTP for reset
+ * @pf: Board private structure
+ */
+void ice_ptp_prepare_for_reset(struct ice_pf *pf)
+{
+ struct ice_ptp *ptp = &pf->ptp;
+ u8 src_tmr;
+
+ if (ptp->state != ICE_PTP_READY)
+ return;
+
+ ptp->state = ICE_PTP_RESETTING;
+
+ /* Disable timestamping for both Tx and Rx */
+ ice_ptp_disable_timestamp_mode(pf);
+
+ kthread_cancel_delayed_work_sync(&ptp->work);
+
+ if (test_bit(ICE_PFR_REQ, pf->state))
+ return;
+
+ ice_ptp_release_tx_tracker(pf, &pf->ptp.port.tx);
+
+ /* Disable periodic outputs */
+ ice_ptp_disable_all_clkout(pf);
+
+ src_tmr = ice_get_ptp_src_clock_index(&pf->hw);
+
+ /* Disable source clock */
+ wr32(&pf->hw, GLTSYN_ENA(src_tmr), (u32)~GLTSYN_ENA_TSYN_ENA_M);
+
+ /* Acquire PHC and system timer to restore after reset */
+ ptp->reset_time = ktime_get_real_ns();
+}
+
/**
* ice_ptp_reset - Initialize PTP hardware clock support after reset
* @pf: Board private structure
@@ -2640,6 +2676,14 @@ void ice_ptp_reset(struct ice_pf *pf)
int err, itr = 1;
u64 time_diff;
+ if (ptp->state == ICE_PTP_READY) {
+ ice_ptp_prepare_for_reset(pf);
+ } else if (ptp->state != ICE_PTP_RESETTING) {
+ err = -EINVAL;
+ dev_err(ice_pf_to_dev(pf), "PTP was not initialized\n");
+ goto err;
+ }
+
if (test_bit(ICE_PFR_REQ, pf->state) ||
!ice_pf_src_tmr_owned(pf))
goto pfr;
@@ -2700,7 +2744,7 @@ void ice_ptp_reset(struct ice_pf *pf)
if (err)
goto err;
- set_bit(ICE_FLAG_PTP, pf->flags);
+ ptp->state = ICE_PTP_READY;
/* Restart the PHY timestamping block */
if (!test_bit(ICE_PFR_REQ, pf->state) &&
@@ -2714,6 +2758,7 @@ void ice_ptp_reset(struct ice_pf *pf)
return;
err:
+ ptp->state = ICE_PTP_ERROR;
dev_err(ice_pf_to_dev(pf), "PTP reset failed %d\n", err);
}
@@ -2922,39 +2967,6 @@ int ice_ptp_clock_index(struct ice_pf *pf)
return clock ? ptp_clock_index(clock) : -1;
}
-/**
- * ice_ptp_prepare_for_reset - Prepare PTP for reset
- * @pf: Board private structure
- */
-void ice_ptp_prepare_for_reset(struct ice_pf *pf)
-{
- struct ice_ptp *ptp = &pf->ptp;
- u8 src_tmr;
-
- clear_bit(ICE_FLAG_PTP, pf->flags);
-
- /* Disable timestamping for both Tx and Rx */
- ice_ptp_disable_timestamp_mode(pf);
-
- kthread_cancel_delayed_work_sync(&ptp->work);
-
- if (test_bit(ICE_PFR_REQ, pf->state))
- return;
-
- ice_ptp_release_tx_tracker(pf, &pf->ptp.port.tx);
-
- /* Disable periodic outputs */
- ice_ptp_disable_all_clkout(pf);
-
- src_tmr = ice_get_ptp_src_clock_index(&pf->hw);
-
- /* Disable source clock */
- wr32(&pf->hw, GLTSYN_ENA(src_tmr), (u32)~GLTSYN_ENA_TSYN_ENA_M);
-
- /* Acquire PHC and system timer to restore after reset */
- ptp->reset_time = ktime_get_real_ns();
-}
-
/**
* ice_ptp_init_owner - Initialize PTP_1588_CLOCK device
* @pf: Board private structure
@@ -3195,6 +3207,8 @@ void ice_ptp_init(struct ice_pf *pf)
struct ice_hw *hw = &pf->hw;
int err;
+ ptp->state = ICE_PTP_INITIALIZING;
+
ice_ptp_init_phy_model(hw);
ice_ptp_init_tx_interrupt_mode(pf);
@@ -3219,12 +3233,13 @@ void ice_ptp_init(struct ice_pf *pf)
/* Configure initial Tx interrupt settings */
ice_ptp_cfg_tx_interrupt(pf);
- set_bit(ICE_FLAG_PTP, pf->flags);
- err = ice_ptp_init_work(pf, ptp);
+ err = ice_ptp_create_auxbus_device(pf);
if (err)
goto err;
- err = ice_ptp_create_auxbus_device(pf);
+ ptp->state = ICE_PTP_READY;
+
+ err = ice_ptp_init_work(pf, ptp);
if (err)
goto err;
@@ -3237,7 +3252,7 @@ void ice_ptp_init(struct ice_pf *pf)
ptp_clock_unregister(ptp->clock);
pf->ptp.clock = NULL;
}
- clear_bit(ICE_FLAG_PTP, pf->flags);
+ ptp->state = ICE_PTP_ERROR;
dev_err(ice_pf_to_dev(pf), "PTP failed %d\n", err);
}
@@ -3250,9 +3265,11 @@ void ice_ptp_init(struct ice_pf *pf)
*/
void ice_ptp_release(struct ice_pf *pf)
{
- if (!test_bit(ICE_FLAG_PTP, pf->flags))
+ if (pf->ptp.state != ICE_PTP_READY)
return;
+ pf->ptp.state = ICE_PTP_UNINIT;
+
/* Disable timestamping for both Tx and Rx */
ice_ptp_disable_timestamp_mode(pf);
@@ -3260,8 +3277,6 @@ void ice_ptp_release(struct ice_pf *pf)
ice_ptp_release_tx_tracker(pf, &pf->ptp.port.tx);
- clear_bit(ICE_FLAG_PTP, pf->flags);
-
kthread_cancel_delayed_work_sync(&pf->ptp.work);
ice_ptp_port_phy_stop(&pf->ptp.port);
@@ -3271,6 +3286,9 @@ void ice_ptp_release(struct ice_pf *pf)
pf->ptp.kworker = NULL;
}
+ if (ice_pf_src_tmr_owned(pf))
+ ice_ptp_unregister_auxbus_driver(pf);
+
if (!pf->ptp.clock)
return;
@@ -3280,7 +3298,5 @@ void ice_ptp_release(struct ice_pf *pf)
ptp_clock_unregister(pf->ptp.clock);
pf->ptp.clock = NULL;
- ice_ptp_unregister_auxbus_driver(pf);
-
dev_info(ice_pf_to_dev(pf), "Removed PTP clock\n");
}
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.h b/drivers/net/ethernet/intel/ice/ice_ptp.h
index 087dd32d8762..2457380142e1 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp.h
+++ b/drivers/net/ethernet/intel/ice/ice_ptp.h
@@ -203,8 +203,17 @@ struct ice_ptp_port_owner {
#define GLTSYN_TGT_H_IDX_MAX 4
+enum ice_ptp_state {
+ ICE_PTP_UNINIT = 0,
+ ICE_PTP_INITIALIZING,
+ ICE_PTP_READY,
+ ICE_PTP_RESETTING,
+ ICE_PTP_ERROR,
+};
+
/**
* struct ice_ptp - data used for integrating with CONFIG_PTP_1588_CLOCK
+ * @state: current state of PTP state machine
* @tx_interrupt_mode: the TX interrupt mode for the PTP clock
* @port: data for the PHY port initialization procedure
* @ports_owner: data for the auxiliary driver owner
@@ -227,6 +236,7 @@ struct ice_ptp_port_owner {
* @late_cached_phc_updates: number of times cached PHC update is late
*/
struct ice_ptp {
+ enum ice_ptp_state state;
enum ice_ptp_tx_interrupt tx_interrupt_mode;
struct ice_ptp_port port;
struct ice_ptp_port_owner ports_owner;
--
2.41.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH net-next 2/7] ice: pass reset type to PTP reset functions
2024-01-25 21:57 [PATCH net-next 0/7][pull request] ice: fix timestamping in reset process Tony Nguyen
2024-01-25 21:57 ` [PATCH net-next 1/7] ice: introduce PTP state machine Tony Nguyen
@ 2024-01-25 21:57 ` Tony Nguyen
2024-01-25 21:57 ` [PATCH net-next 3/7] ice: rename verify_cached to has_ready_bitmap Tony Nguyen
` (5 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Tony Nguyen @ 2024-01-25 21:57 UTC (permalink / raw)
To: davem, kuba, pabeni, edumazet, netdev
Cc: Jacob Keller, anthony.l.nguyen, richardcochran, Karol Kolacinski,
Simon Horman, Pucha Himasekhar Reddy
From: Jacob Keller <jacob.e.keller@intel.com>
The ice_ptp_prepare_for_reset() and ice_ptp_reset() functions currently
check the pf->flags ICE_FLAG_PFR_REQ bit to determine if the current
reset is a PF reset or not.
This is problematic, because it is possible that a PF reset and a higher
level reset (CORE reset, GLOBAL reset, EMP reset) are requested
simultaneously. In that case, the driver performs the highest level
reset requested. However, the ICE_FLAG_PFR_REQ flag will still be set.
The main driver reset functions take an enum ice_reset_req indicating
which reset is actually being performed. Pass this data into the PTP
functions and rely on this instead of relying on the driver flags.
This ensures that the PTP code performs the proper level of reset that
the driver is actually undergoing.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Signed-off-by: Karol Kolacinski <karol.kolacinski@intel.com>
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Reviewed-by: Simon Horman <horms@kernel.org>
Tested-by: Pucha Himasekhar Reddy <himasekharx.reddy.pucha@intel.com> (A Contingent worker at Intel)
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
drivers/net/ethernet/intel/ice/ice_main.c | 4 ++--
drivers/net/ethernet/intel/ice/ice_ptp.c | 13 +++++++------
drivers/net/ethernet/intel/ice/ice_ptp.h | 16 ++++++++++++----
3 files changed, 21 insertions(+), 12 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index dd4a9bc0dfdc..0c589524cf43 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -613,7 +613,7 @@ ice_prepare_for_reset(struct ice_pf *pf, enum ice_reset_req reset_type)
ice_pf_dis_all_vsi(pf, false);
if (test_bit(ICE_FLAG_PTP_SUPPORTED, pf->flags))
- ice_ptp_prepare_for_reset(pf);
+ ice_ptp_prepare_for_reset(pf, reset_type);
if (ice_is_feature_supported(pf, ICE_F_GNSS))
ice_gnss_exit(pf);
@@ -7548,7 +7548,7 @@ static void ice_rebuild(struct ice_pf *pf, enum ice_reset_req reset_type)
* fail.
*/
if (test_bit(ICE_FLAG_PTP_SUPPORTED, pf->flags))
- ice_ptp_reset(pf);
+ ice_ptp_reset(pf, reset_type);
if (ice_is_feature_supported(pf, ICE_F_GNSS))
ice_gnss_init(pf);
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c b/drivers/net/ethernet/intel/ice/ice_ptp.c
index 8ed4af219f9b..96b5f992f127 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp.c
+++ b/drivers/net/ethernet/intel/ice/ice_ptp.c
@@ -2631,8 +2631,9 @@ static void ice_ptp_periodic_work(struct kthread_work *work)
/**
* ice_ptp_prepare_for_reset - Prepare PTP for reset
* @pf: Board private structure
+ * @reset_type: the reset type being performed
*/
-void ice_ptp_prepare_for_reset(struct ice_pf *pf)
+void ice_ptp_prepare_for_reset(struct ice_pf *pf, enum ice_reset_req reset_type)
{
struct ice_ptp *ptp = &pf->ptp;
u8 src_tmr;
@@ -2647,7 +2648,7 @@ void ice_ptp_prepare_for_reset(struct ice_pf *pf)
kthread_cancel_delayed_work_sync(&ptp->work);
- if (test_bit(ICE_PFR_REQ, pf->state))
+ if (reset_type == ICE_RESET_PFR)
return;
ice_ptp_release_tx_tracker(pf, &pf->ptp.port.tx);
@@ -2667,8 +2668,9 @@ void ice_ptp_prepare_for_reset(struct ice_pf *pf)
/**
* ice_ptp_reset - Initialize PTP hardware clock support after reset
* @pf: Board private structure
+ * @reset_type: the reset type being performed
*/
-void ice_ptp_reset(struct ice_pf *pf)
+void ice_ptp_reset(struct ice_pf *pf, enum ice_reset_req reset_type)
{
struct ice_ptp *ptp = &pf->ptp;
struct ice_hw *hw = &pf->hw;
@@ -2677,15 +2679,14 @@ void ice_ptp_reset(struct ice_pf *pf)
u64 time_diff;
if (ptp->state == ICE_PTP_READY) {
- ice_ptp_prepare_for_reset(pf);
+ ice_ptp_prepare_for_reset(pf, reset_type);
} else if (ptp->state != ICE_PTP_RESETTING) {
err = -EINVAL;
dev_err(ice_pf_to_dev(pf), "PTP was not initialized\n");
goto err;
}
- if (test_bit(ICE_PFR_REQ, pf->state) ||
- !ice_pf_src_tmr_owned(pf))
+ if (reset_type == ICE_RESET_PFR || !ice_pf_src_tmr_owned(pf))
goto pfr;
err = ice_ptp_init_phc(hw);
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.h b/drivers/net/ethernet/intel/ice/ice_ptp.h
index 2457380142e1..afe454abe997 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp.h
+++ b/drivers/net/ethernet/intel/ice/ice_ptp.h
@@ -314,8 +314,9 @@ enum ice_tx_tstamp_work ice_ptp_process_ts(struct ice_pf *pf);
u64 ice_ptp_get_rx_hwts(const union ice_32b_rx_flex_desc *rx_desc,
const struct ice_pkt_ctx *pkt_ctx);
-void ice_ptp_reset(struct ice_pf *pf);
-void ice_ptp_prepare_for_reset(struct ice_pf *pf);
+void ice_ptp_reset(struct ice_pf *pf, enum ice_reset_req reset_type);
+void ice_ptp_prepare_for_reset(struct ice_pf *pf,
+ enum ice_reset_req reset_type);
void ice_ptp_init(struct ice_pf *pf);
void ice_ptp_release(struct ice_pf *pf);
void ice_ptp_link_change(struct ice_pf *pf, u8 port, bool linkup);
@@ -355,8 +356,15 @@ ice_ptp_get_rx_hwts(const union ice_32b_rx_flex_desc *rx_desc,
return 0;
}
-static inline void ice_ptp_reset(struct ice_pf *pf) { }
-static inline void ice_ptp_prepare_for_reset(struct ice_pf *pf) { }
+static inline void ice_ptp_reset(struct ice_pf *pf,
+ enum ice_reset_req reset_type)
+{
+}
+
+static inline void ice_ptp_prepare_for_reset(struct ice_pf *pf,
+ enum ice_reset_req reset_type)
+{
+}
static inline void ice_ptp_init(struct ice_pf *pf) { }
static inline void ice_ptp_release(struct ice_pf *pf) { }
static inline void ice_ptp_link_change(struct ice_pf *pf, u8 port, bool linkup)
--
2.41.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH net-next 3/7] ice: rename verify_cached to has_ready_bitmap
2024-01-25 21:57 [PATCH net-next 0/7][pull request] ice: fix timestamping in reset process Tony Nguyen
2024-01-25 21:57 ` [PATCH net-next 1/7] ice: introduce PTP state machine Tony Nguyen
2024-01-25 21:57 ` [PATCH net-next 2/7] ice: pass reset type to PTP reset functions Tony Nguyen
@ 2024-01-25 21:57 ` Tony Nguyen
2024-01-25 21:57 ` [PATCH net-next 4/7] ice: don't check has_ready_bitmap in E810 functions Tony Nguyen
` (4 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Tony Nguyen @ 2024-01-25 21:57 UTC (permalink / raw)
To: davem, kuba, pabeni, edumazet, netdev
Cc: Jacob Keller, anthony.l.nguyen, richardcochran, Karol Kolacinski,
Simon Horman, Pucha Himasekhar Reddy
From: Jacob Keller <jacob.e.keller@intel.com>
The tx->verify_cached flag is used to inform the Tx timestamp tracking
code whether it needs to verify the cached Tx timestamp value against
a previous captured value. This is necessary on E810 hardware which does
not have a Tx timestamp ready bitmap.
In addition, we currently rely on the fact that the
ice_get_phy_tx_tstamp_ready() function returns all 1s for E810 hardware.
Instead of introducing a brand new flag, rename and verify_cached to
has_ready_bitmap, inverting the relevant checks.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Signed-off-by: Karol Kolacinski <karol.kolacinski@intel.com>
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Reviewed-by: Simon Horman <horms@kernel.org>
Tested-by: Pucha Himasekhar Reddy <himasekharx.reddy.pucha@intel.com> (A Contingent worker at Intel)
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
drivers/net/ethernet/intel/ice/ice_ptp.c | 12 ++++++------
drivers/net/ethernet/intel/ice/ice_ptp.h | 8 +++++---
2 files changed, 11 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c b/drivers/net/ethernet/intel/ice/ice_ptp.c
index 96b5f992f127..a10e0018b2e2 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp.c
+++ b/drivers/net/ethernet/intel/ice/ice_ptp.c
@@ -606,11 +606,11 @@ void ice_ptp_complete_tx_single_tstamp(struct ice_ptp_tx *tx)
* timestamp. If it is not, skip this for now assuming it hasn't yet
* been captured by hardware.
*/
- if (!drop_ts && tx->verify_cached &&
+ if (!drop_ts && !tx->has_ready_bitmap &&
raw_tstamp == tx->tstamps[idx].cached_tstamp)
return;
- if (tx->verify_cached && raw_tstamp)
+ if (!tx->has_ready_bitmap && raw_tstamp)
tx->tstamps[idx].cached_tstamp = raw_tstamp;
clear_bit(idx, tx->in_use);
skb = tx->tstamps[idx].skb;
@@ -751,7 +751,7 @@ static void ice_ptp_process_tx_tstamp(struct ice_ptp_tx *tx)
* from the last cached timestamp. If it is not, skip this for
* now assuming it hasn't yet been captured by hardware.
*/
- if (!drop_ts && tx->verify_cached &&
+ if (!drop_ts && !tx->has_ready_bitmap &&
raw_tstamp == tx->tstamps[idx].cached_tstamp)
continue;
@@ -761,7 +761,7 @@ static void ice_ptp_process_tx_tstamp(struct ice_ptp_tx *tx)
skip_ts_read:
spin_lock_irqsave(&tx->lock, flags);
- if (tx->verify_cached && raw_tstamp)
+ if (!tx->has_ready_bitmap && raw_tstamp)
tx->tstamps[idx].cached_tstamp = raw_tstamp;
clear_bit(idx, tx->in_use);
skb = tx->tstamps[idx].skb;
@@ -1014,7 +1014,7 @@ ice_ptp_init_tx_e82x(struct ice_pf *pf, struct ice_ptp_tx *tx, u8 port)
tx->block = port / ICE_PORTS_PER_QUAD;
tx->offset = (port % ICE_PORTS_PER_QUAD) * INDEX_PER_PORT_E82X;
tx->len = INDEX_PER_PORT_E82X;
- tx->verify_cached = 0;
+ tx->has_ready_bitmap = 1;
return ice_ptp_alloc_tx_tracker(tx);
}
@@ -1037,7 +1037,7 @@ ice_ptp_init_tx_e810(struct ice_pf *pf, struct ice_ptp_tx *tx)
* verify new timestamps against cached copy of the last read
* timestamp.
*/
- tx->verify_cached = 1;
+ tx->has_ready_bitmap = 0;
return ice_ptp_alloc_tx_tracker(tx);
}
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.h b/drivers/net/ethernet/intel/ice/ice_ptp.h
index afe454abe997..aa7a5588d11d 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp.h
+++ b/drivers/net/ethernet/intel/ice/ice_ptp.h
@@ -100,7 +100,7 @@ struct ice_perout_channel {
* the last timestamp we read for a given index. If the current timestamp
* value is the same as the cached value, we assume a new timestamp hasn't
* been captured. This avoids reporting stale timestamps to the stack. This is
- * only done if the verify_cached flag is set in ice_ptp_tx structure.
+ * only done if the has_ready_bitmap flag is not set in ice_ptp_tx structure.
*/
struct ice_tx_tstamp {
struct sk_buff *skb;
@@ -130,7 +130,9 @@ enum ice_tx_tstamp_work {
* @init: if true, the tracker is initialized;
* @calibrating: if true, the PHY is calibrating the Tx offset. During this
* window, timestamps are temporarily disabled.
- * @verify_cached: if true, verify new timestamp differs from last read value
+ * @has_ready_bitmap: if true, the hardware has a valid Tx timestamp ready
+ * bitmap register. If false, fall back to verifying new
+ * timestamp values against previously cached copy.
* @last_ll_ts_idx_read: index of the last LL TS read by the FW
*/
struct ice_ptp_tx {
@@ -143,7 +145,7 @@ struct ice_ptp_tx {
u8 len;
u8 init : 1;
u8 calibrating : 1;
- u8 verify_cached : 1;
+ u8 has_ready_bitmap : 1;
s8 last_ll_ts_idx_read;
};
--
2.41.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH net-next 4/7] ice: don't check has_ready_bitmap in E810 functions
2024-01-25 21:57 [PATCH net-next 0/7][pull request] ice: fix timestamping in reset process Tony Nguyen
` (2 preceding siblings ...)
2024-01-25 21:57 ` [PATCH net-next 3/7] ice: rename verify_cached to has_ready_bitmap Tony Nguyen
@ 2024-01-25 21:57 ` Tony Nguyen
2024-01-25 21:57 ` [PATCH net-next 5/7] ice: rename ice_ptp_tx_cfg_intr Tony Nguyen
` (3 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Tony Nguyen @ 2024-01-25 21:57 UTC (permalink / raw)
To: davem, kuba, pabeni, edumazet, netdev
Cc: Jacob Keller, anthony.l.nguyen, richardcochran, Karol Kolacinski,
Simon Horman, Pucha Himasekhar Reddy
From: Jacob Keller <jacob.e.keller@intel.com>
E810 hardware does not have a Tx timestamp ready bitmap. Don't check
has_ready_bitmap in E810-specific functions.
Add has_ready_bitmap check in ice_ptp_process_tx_tstamp() to stop
relying on the fact that ice_get_phy_tx_tstamp_ready() returns all 1s.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Signed-off-by: Karol Kolacinski <karol.kolacinski@intel.com>
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Reviewed-by: Simon Horman <horms@kernel.org>
Tested-by: Pucha Himasekhar Reddy <himasekharx.reddy.pucha@intel.com> (A Contingent worker at Intel)
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
drivers/net/ethernet/intel/ice/ice_ptp.c | 23 +++++++++++------------
1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c b/drivers/net/ethernet/intel/ice/ice_ptp.c
index a10e0018b2e2..69d11dbda22c 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp.c
+++ b/drivers/net/ethernet/intel/ice/ice_ptp.c
@@ -601,17 +601,13 @@ void ice_ptp_complete_tx_single_tstamp(struct ice_ptp_tx *tx)
/* Read the low 32 bit value */
raw_tstamp |= (u64)rd32(&pf->hw, PF_SB_ATQBAH);
- /* For PHYs which don't implement a proper timestamp ready bitmap,
- * verify that the timestamp value is different from the last cached
- * timestamp. If it is not, skip this for now assuming it hasn't yet
- * been captured by hardware.
+ /* Devices using this interface always verify the timestamp differs
+ * relative to the last cached timestamp value.
*/
- if (!drop_ts && !tx->has_ready_bitmap &&
- raw_tstamp == tx->tstamps[idx].cached_tstamp)
+ if (raw_tstamp == tx->tstamps[idx].cached_tstamp)
return;
- if (!tx->has_ready_bitmap && raw_tstamp)
- tx->tstamps[idx].cached_tstamp = raw_tstamp;
+ tx->tstamps[idx].cached_tstamp = raw_tstamp;
clear_bit(idx, tx->in_use);
skb = tx->tstamps[idx].skb;
tx->tstamps[idx].skb = NULL;
@@ -701,9 +697,11 @@ static void ice_ptp_process_tx_tstamp(struct ice_ptp_tx *tx)
hw = &pf->hw;
/* Read the Tx ready status first */
- err = ice_get_phy_tx_tstamp_ready(hw, tx->block, &tstamp_ready);
- if (err)
- return;
+ if (tx->has_ready_bitmap) {
+ err = ice_get_phy_tx_tstamp_ready(hw, tx->block, &tstamp_ready);
+ if (err)
+ return;
+ }
/* Drop packets if the link went down */
link_up = ptp_port->link_up;
@@ -731,7 +729,8 @@ static void ice_ptp_process_tx_tstamp(struct ice_ptp_tx *tx)
* If we do not, the hardware logic for generating a new
* interrupt can get stuck on some devices.
*/
- if (!(tstamp_ready & BIT_ULL(phy_idx))) {
+ if (tx->has_ready_bitmap &&
+ !(tstamp_ready & BIT_ULL(phy_idx))) {
if (drop_ts)
goto skip_ts_read;
--
2.41.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH net-next 5/7] ice: rename ice_ptp_tx_cfg_intr
2024-01-25 21:57 [PATCH net-next 0/7][pull request] ice: fix timestamping in reset process Tony Nguyen
` (3 preceding siblings ...)
2024-01-25 21:57 ` [PATCH net-next 4/7] ice: don't check has_ready_bitmap in E810 functions Tony Nguyen
@ 2024-01-25 21:57 ` Tony Nguyen
2024-01-25 21:57 ` [PATCH net-next 6/7] ice: factor out ice_ptp_rebuild_owner() Tony Nguyen
` (2 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Tony Nguyen @ 2024-01-25 21:57 UTC (permalink / raw)
To: davem, kuba, pabeni, edumazet, netdev
Cc: Jacob Keller, anthony.l.nguyen, richardcochran, Karol Kolacinski,
Simon Horman, Pucha Himasekhar Reddy
From: Jacob Keller <jacob.e.keller@intel.com>
The ice_ptp_tx_cfg_intr() function sends a control queue message to
configure the PHY timestamp interrupt block. This is a very similar name
to a function which is used to configure the MAC Other Interrupt Cause
Enable register.
Rename this function to ice_ptp_cfg_phy_interrupt in order to make it
more obvious to the reader what action it performs, and distinguish it
from other similarly named functions.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Signed-off-by: Karol Kolacinski <karol.kolacinski@intel.com>
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Reviewed-by: Simon Horman <horms@kernel.org>
Tested-by: Pucha Himasekhar Reddy <himasekharx.reddy.pucha@intel.com> (A Contingent worker at Intel)
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
drivers/net/ethernet/intel/ice/ice_ptp.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c b/drivers/net/ethernet/intel/ice/ice_ptp.c
index 69d11dbda22c..6f2a1ad5c2a3 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp.c
+++ b/drivers/net/ethernet/intel/ice/ice_ptp.c
@@ -1455,14 +1455,14 @@ void ice_ptp_link_change(struct ice_pf *pf, u8 port, bool linkup)
}
/**
- * ice_ptp_tx_ena_intr - Enable or disable the Tx timestamp interrupt
+ * ice_ptp_cfg_phy_interrupt - Configure PHY interrupt settings
* @pf: PF private structure
* @ena: bool value to enable or disable interrupt
* @threshold: Minimum number of packets at which intr is triggered
*
* Utility function to enable or disable Tx timestamp interrupt and threshold
*/
-static int ice_ptp_tx_ena_intr(struct ice_pf *pf, bool ena, u32 threshold)
+static int ice_ptp_cfg_phy_interrupt(struct ice_pf *pf, bool ena, u32 threshold)
{
struct ice_hw *hw = &pf->hw;
int err = 0;
@@ -2674,8 +2674,8 @@ void ice_ptp_reset(struct ice_pf *pf, enum ice_reset_req reset_type)
struct ice_ptp *ptp = &pf->ptp;
struct ice_hw *hw = &pf->hw;
struct timespec64 ts;
- int err, itr = 1;
u64 time_diff;
+ int err;
if (ptp->state == ICE_PTP_READY) {
ice_ptp_prepare_for_reset(pf, reset_type);
@@ -2726,7 +2726,7 @@ void ice_ptp_reset(struct ice_pf *pf, enum ice_reset_req reset_type)
if (!ice_is_e810(hw)) {
/* Enable quad interrupts */
- err = ice_ptp_tx_ena_intr(pf, true, itr);
+ err = ice_ptp_cfg_phy_interrupt(pf, true, 1);
if (err)
goto err;
}
@@ -2979,7 +2979,7 @@ static int ice_ptp_init_owner(struct ice_pf *pf)
{
struct ice_hw *hw = &pf->hw;
struct timespec64 ts;
- int err, itr = 1;
+ int err;
err = ice_ptp_init_phc(hw);
if (err) {
@@ -3014,7 +3014,7 @@ static int ice_ptp_init_owner(struct ice_pf *pf)
if (!ice_is_e810(hw)) {
/* Enable quad interrupts */
- err = ice_ptp_tx_ena_intr(pf, true, itr);
+ err = ice_ptp_cfg_phy_interrupt(pf, true, 1);
if (err)
goto err_exit;
}
--
2.41.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH net-next 6/7] ice: factor out ice_ptp_rebuild_owner()
2024-01-25 21:57 [PATCH net-next 0/7][pull request] ice: fix timestamping in reset process Tony Nguyen
` (4 preceding siblings ...)
2024-01-25 21:57 ` [PATCH net-next 5/7] ice: rename ice_ptp_tx_cfg_intr Tony Nguyen
@ 2024-01-25 21:57 ` Tony Nguyen
2024-01-25 21:57 ` [PATCH net-next 7/7] ice: stop destroying and reinitalizing Tx tracker during reset Tony Nguyen
2024-01-30 11:10 ` [PATCH net-next 0/7][pull request] ice: fix timestamping in reset process patchwork-bot+netdevbpf
7 siblings, 0 replies; 9+ messages in thread
From: Tony Nguyen @ 2024-01-25 21:57 UTC (permalink / raw)
To: davem, kuba, pabeni, edumazet, netdev
Cc: Jacob Keller, anthony.l.nguyen, richardcochran, Karol Kolacinski,
Simon Horman, Pucha Himasekhar Reddy
From: Jacob Keller <jacob.e.keller@intel.com>
The ice_ptp_reset() function uses a goto to skip past clock owner
operations if performing a PF reset or if the device is not the clock
owner. This is a bit confusing. Factor this out into
ice_ptp_rebuild_owner() instead.
The ice_ptp_reset() function is called by ice_rebuild() to restore PTP
functionality after a device reset. Follow the convention set by the
ice_main.c file and rename this function to ice_ptp_rebuild(), in the
same way that we have ice_prepare_for_reset() and
ice_ptp_prepare_for_reset().
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Signed-off-by: Karol Kolacinski <karol.kolacinski@intel.com>
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Reviewed-by: Simon Horman <horms@kernel.org>
Tested-by: Pucha Himasekhar Reddy <himasekharx.reddy.pucha@intel.com> (A Contingent worker at Intel)
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
drivers/net/ethernet/intel/ice/ice_main.c | 2 +-
drivers/net/ethernet/intel/ice/ice_ptp.c | 62 ++++++++++++++---------
drivers/net/ethernet/intel/ice/ice_ptp.h | 6 +--
3 files changed, 42 insertions(+), 28 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index 0c589524cf43..2b7960824bc3 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -7548,7 +7548,7 @@ static void ice_rebuild(struct ice_pf *pf, enum ice_reset_req reset_type)
* fail.
*/
if (test_bit(ICE_FLAG_PTP_SUPPORTED, pf->flags))
- ice_ptp_reset(pf, reset_type);
+ ice_ptp_rebuild(pf, reset_type);
if (ice_is_feature_supported(pf, ICE_F_GNSS))
ice_gnss_init(pf);
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c b/drivers/net/ethernet/intel/ice/ice_ptp.c
index 6f2a1ad5c2a3..5ede4f61c054 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp.c
+++ b/drivers/net/ethernet/intel/ice/ice_ptp.c
@@ -2665,11 +2665,13 @@ void ice_ptp_prepare_for_reset(struct ice_pf *pf, enum ice_reset_req reset_type)
}
/**
- * ice_ptp_reset - Initialize PTP hardware clock support after reset
+ * ice_ptp_rebuild_owner - Initialize PTP clock owner after reset
* @pf: Board private structure
- * @reset_type: the reset type being performed
+ *
+ * Companion function for ice_ptp_rebuild() which handles tasks that only the
+ * PTP clock owner instance should perform.
*/
-void ice_ptp_reset(struct ice_pf *pf, enum ice_reset_req reset_type)
+static int ice_ptp_rebuild_owner(struct ice_pf *pf)
{
struct ice_ptp *ptp = &pf->ptp;
struct ice_hw *hw = &pf->hw;
@@ -2677,32 +2679,21 @@ void ice_ptp_reset(struct ice_pf *pf, enum ice_reset_req reset_type)
u64 time_diff;
int err;
- if (ptp->state == ICE_PTP_READY) {
- ice_ptp_prepare_for_reset(pf, reset_type);
- } else if (ptp->state != ICE_PTP_RESETTING) {
- err = -EINVAL;
- dev_err(ice_pf_to_dev(pf), "PTP was not initialized\n");
- goto err;
- }
-
- if (reset_type == ICE_RESET_PFR || !ice_pf_src_tmr_owned(pf))
- goto pfr;
-
err = ice_ptp_init_phc(hw);
if (err)
- goto err;
+ return err;
/* Acquire the global hardware lock */
if (!ice_ptp_lock(hw)) {
err = -EBUSY;
- goto err;
+ return err;
}
/* Write the increment time value to PHY and LAN */
err = ice_ptp_write_incval(hw, ice_base_incval(pf));
if (err) {
ice_ptp_unlock(hw);
- goto err;
+ return err;
}
/* Write the initial Time value to PHY and LAN using the cached PHC
@@ -2718,7 +2709,7 @@ void ice_ptp_reset(struct ice_pf *pf, enum ice_reset_req reset_type)
err = ice_ptp_write_init(pf, &ts);
if (err) {
ice_ptp_unlock(hw);
- goto err;
+ return err;
}
/* Release the global hardware lock */
@@ -2727,11 +2718,39 @@ void ice_ptp_reset(struct ice_pf *pf, enum ice_reset_req reset_type)
if (!ice_is_e810(hw)) {
/* Enable quad interrupts */
err = ice_ptp_cfg_phy_interrupt(pf, true, 1);
+ if (err)
+ return err;
+
+ ice_ptp_restart_all_phy(pf);
+ }
+
+ return 0;
+}
+
+/**
+ * ice_ptp_rebuild - Initialize PTP hardware clock support after reset
+ * @pf: Board private structure
+ * @reset_type: the reset type being performed
+ */
+void ice_ptp_rebuild(struct ice_pf *pf, enum ice_reset_req reset_type)
+{
+ struct ice_ptp *ptp = &pf->ptp;
+ int err;
+
+ if (ptp->state == ICE_PTP_READY) {
+ ice_ptp_prepare_for_reset(pf, reset_type);
+ } else if (ptp->state != ICE_PTP_RESETTING) {
+ err = -EINVAL;
+ dev_err(ice_pf_to_dev(pf), "PTP was not initialized\n");
+ goto err;
+ }
+
+ if (ice_pf_src_tmr_owned(pf) && reset_type != ICE_RESET_PFR) {
+ err = ice_ptp_rebuild_owner(pf);
if (err)
goto err;
}
-pfr:
/* Init Tx structures */
if (ice_is_e810(&pf->hw)) {
err = ice_ptp_init_tx_e810(pf, &ptp->port.tx);
@@ -2746,11 +2765,6 @@ void ice_ptp_reset(struct ice_pf *pf, enum ice_reset_req reset_type)
ptp->state = ICE_PTP_READY;
- /* Restart the PHY timestamping block */
- if (!test_bit(ICE_PFR_REQ, pf->state) &&
- ice_pf_src_tmr_owned(pf))
- ice_ptp_restart_all_phy(pf);
-
/* Start periodic work going */
kthread_queue_delayed_work(ptp->kworker, &ptp->work, 0);
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.h b/drivers/net/ethernet/intel/ice/ice_ptp.h
index aa7a5588d11d..3af20025043a 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp.h
+++ b/drivers/net/ethernet/intel/ice/ice_ptp.h
@@ -316,7 +316,7 @@ enum ice_tx_tstamp_work ice_ptp_process_ts(struct ice_pf *pf);
u64 ice_ptp_get_rx_hwts(const union ice_32b_rx_flex_desc *rx_desc,
const struct ice_pkt_ctx *pkt_ctx);
-void ice_ptp_reset(struct ice_pf *pf, enum ice_reset_req reset_type);
+void ice_ptp_rebuild(struct ice_pf *pf, enum ice_reset_req reset_type);
void ice_ptp_prepare_for_reset(struct ice_pf *pf,
enum ice_reset_req reset_type);
void ice_ptp_init(struct ice_pf *pf);
@@ -358,8 +358,8 @@ ice_ptp_get_rx_hwts(const union ice_32b_rx_flex_desc *rx_desc,
return 0;
}
-static inline void ice_ptp_reset(struct ice_pf *pf,
- enum ice_reset_req reset_type)
+static inline void ice_ptp_rebuild(struct ice_pf *pf,
+ enum ice_reset_req reset_type)
{
}
--
2.41.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH net-next 7/7] ice: stop destroying and reinitalizing Tx tracker during reset
2024-01-25 21:57 [PATCH net-next 0/7][pull request] ice: fix timestamping in reset process Tony Nguyen
` (5 preceding siblings ...)
2024-01-25 21:57 ` [PATCH net-next 6/7] ice: factor out ice_ptp_rebuild_owner() Tony Nguyen
@ 2024-01-25 21:57 ` Tony Nguyen
2024-01-30 11:10 ` [PATCH net-next 0/7][pull request] ice: fix timestamping in reset process patchwork-bot+netdevbpf
7 siblings, 0 replies; 9+ messages in thread
From: Tony Nguyen @ 2024-01-25 21:57 UTC (permalink / raw)
To: davem, kuba, pabeni, edumazet, netdev
Cc: Jacob Keller, anthony.l.nguyen, richardcochran, Karol Kolacinski,
Simon Horman, Pucha Himasekhar Reddy
From: Jacob Keller <jacob.e.keller@intel.com>
The ice driver currently attempts to destroy and re-initialize the Tx
timestamp tracker during the reset flow. The release of the Tx tracker
only happened during CORE reset or GLOBAL reset. The ice_ptp_rebuild()
function always calls the ice_ptp_init_tx function which will allocate
a new tracker data structure, resulting in memory leaks during PF reset.
Certainly the driver should not be allocating a new tracker without
removing the old tracker data, as this results in a memory leak.
Additionally, there's no reason to remove the tracker memory during a
reset. Remove this logic from the reset and rebuild flow. Instead of
releasing the Tx tracker, flush outstanding timestamps just before we
reset the PHY timestamp block in ice_ptp_cfg_phy_interrupt().
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Signed-off-by: Karol Kolacinski <karol.kolacinski@intel.com>
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Reviewed-by: Simon Horman <horms@kernel.org>
Tested-by: Pucha Himasekhar Reddy <himasekharx.reddy.pucha@intel.com> (A Contingent worker at Intel)
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
drivers/net/ethernet/intel/ice/ice_ptp.c | 33 +++++++++++++++---------
1 file changed, 21 insertions(+), 12 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c b/drivers/net/ethernet/intel/ice/ice_ptp.c
index 5ede4f61c054..c11eba07283c 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp.c
+++ b/drivers/net/ethernet/intel/ice/ice_ptp.c
@@ -963,6 +963,22 @@ ice_ptp_mark_tx_tracker_stale(struct ice_ptp_tx *tx)
spin_unlock_irqrestore(&tx->lock, flags);
}
+/**
+ * ice_ptp_flush_all_tx_tracker - Flush all timestamp trackers on this clock
+ * @pf: Board private structure
+ *
+ * Called by the clock owner to flush all the Tx timestamp trackers associated
+ * with the clock.
+ */
+static void
+ice_ptp_flush_all_tx_tracker(struct ice_pf *pf)
+{
+ struct ice_ptp_port *port;
+
+ list_for_each_entry(port, &pf->ptp.ports_owner.ports, list_member)
+ ice_ptp_flush_tx_tracker(ptp_port_to_pf(port), &port->tx);
+}
+
/**
* ice_ptp_release_tx_tracker - Release allocated memory for Tx tracker
* @pf: Board private structure
@@ -2715,6 +2731,11 @@ static int ice_ptp_rebuild_owner(struct ice_pf *pf)
/* Release the global hardware lock */
ice_ptp_unlock(hw);
+ /* Flush software tracking of any outstanding timestamps since we're
+ * about to flush the PHY timestamp block.
+ */
+ ice_ptp_flush_all_tx_tracker(pf);
+
if (!ice_is_e810(hw)) {
/* Enable quad interrupts */
err = ice_ptp_cfg_phy_interrupt(pf, true, 1);
@@ -2751,18 +2772,6 @@ void ice_ptp_rebuild(struct ice_pf *pf, enum ice_reset_req reset_type)
goto err;
}
- /* Init Tx structures */
- if (ice_is_e810(&pf->hw)) {
- err = ice_ptp_init_tx_e810(pf, &ptp->port.tx);
- } else {
- kthread_init_delayed_work(&ptp->port.ov_work,
- ice_ptp_wait_for_offsets);
- err = ice_ptp_init_tx_e82x(pf, &ptp->port.tx,
- ptp->port.port_num);
- }
- if (err)
- goto err;
-
ptp->state = ICE_PTP_READY;
/* Start periodic work going */
--
2.41.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH net-next 0/7][pull request] ice: fix timestamping in reset process
2024-01-25 21:57 [PATCH net-next 0/7][pull request] ice: fix timestamping in reset process Tony Nguyen
` (6 preceding siblings ...)
2024-01-25 21:57 ` [PATCH net-next 7/7] ice: stop destroying and reinitalizing Tx tracker during reset Tony Nguyen
@ 2024-01-30 11:10 ` patchwork-bot+netdevbpf
7 siblings, 0 replies; 9+ messages in thread
From: patchwork-bot+netdevbpf @ 2024-01-30 11:10 UTC (permalink / raw)
To: Tony Nguyen
Cc: davem, kuba, pabeni, edumazet, netdev, richardcochran,
karol.kolacinski, jacob.e.keller
Hello:
This series was applied to netdev/net-next.git (main)
by Paolo Abeni <pabeni@redhat.com>:
On Thu, 25 Jan 2024 13:57:48 -0800 you wrote:
> Karol Kolacinski says:
>
> PTP reset process has multiple places where timestamping can end up in
> an incorrect state.
>
> This series introduces a proper state machine for PTP and refactors
> a large part of the code to ensure that timestamping does not break.
>
> [...]
Here is the summary with links:
- [net-next,1/7] ice: introduce PTP state machine
https://git.kernel.org/netdev/net-next/c/8293e4cb2ff5
- [net-next,2/7] ice: pass reset type to PTP reset functions
https://git.kernel.org/netdev/net-next/c/c75d5e675a85
- [net-next,3/7] ice: rename verify_cached to has_ready_bitmap
https://git.kernel.org/netdev/net-next/c/3f2216e8dbce
- [net-next,4/7] ice: don't check has_ready_bitmap in E810 functions
https://git.kernel.org/netdev/net-next/c/fea82915fca6
- [net-next,5/7] ice: rename ice_ptp_tx_cfg_intr
https://git.kernel.org/netdev/net-next/c/1abefdca85e8
- [net-next,6/7] ice: factor out ice_ptp_rebuild_owner()
https://git.kernel.org/netdev/net-next/c/803bef817807
- [net-next,7/7] ice: stop destroying and reinitalizing Tx tracker during reset
https://git.kernel.org/netdev/net-next/c/7a25fe5cd5fb
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2024-01-30 11:10 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-01-25 21:57 [PATCH net-next 0/7][pull request] ice: fix timestamping in reset process Tony Nguyen
2024-01-25 21:57 ` [PATCH net-next 1/7] ice: introduce PTP state machine Tony Nguyen
2024-01-25 21:57 ` [PATCH net-next 2/7] ice: pass reset type to PTP reset functions Tony Nguyen
2024-01-25 21:57 ` [PATCH net-next 3/7] ice: rename verify_cached to has_ready_bitmap Tony Nguyen
2024-01-25 21:57 ` [PATCH net-next 4/7] ice: don't check has_ready_bitmap in E810 functions Tony Nguyen
2024-01-25 21:57 ` [PATCH net-next 5/7] ice: rename ice_ptp_tx_cfg_intr Tony Nguyen
2024-01-25 21:57 ` [PATCH net-next 6/7] ice: factor out ice_ptp_rebuild_owner() Tony Nguyen
2024-01-25 21:57 ` [PATCH net-next 7/7] ice: stop destroying and reinitalizing Tx tracker during reset Tony Nguyen
2024-01-30 11:10 ` [PATCH net-next 0/7][pull request] ice: fix timestamping in reset process patchwork-bot+netdevbpf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).