diff mbox series

[iwl-next] ice: Re-enable timestamping correctly after reset

Message ID 20231011110411.204700-1-karol.kolacinski@intel.com
State Accepted
Delegated to: Anthony Nguyen
Headers show
Series [iwl-next] ice: Re-enable timestamping correctly after reset | expand

Commit Message

Karol Kolacinski Oct. 11, 2023, 11:04 a.m. UTC 
During reset, TX_TSYN interrupt should be processed as it may process
timestamps in brief moments before and after reset.
Timestamping should be enabled on VSIs at the end of reset procedure.
On ice_get_phy_tx_tstamp_ready error, interrupt should not be rearmed,
because error only happens on resets.

Reviewed-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Signed-off-by: Karol Kolacinski <karol.kolacinski@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_main.c |  2 +-
 drivers/net/ethernet/intel/ice/ice_ptp.c  | 22 +++++++++++++---------
 2 files changed, 14 insertions(+), 10 deletions(-)


base-commit: 2318d58f358e7aef726c038aff87a68bec8f09e0

Comments

Jacob Keller Oct. 11, 2023, 8:18 p.m. UTC | #1 
On 10/11/2023 4:04 AM, Karol Kolacinski wrote:
> During reset, TX_TSYN interrupt should be processed as it may process
> timestamps in brief moments before and after reset.
> Timestamping should be enabled on VSIs at the end of reset procedure.
> On ice_get_phy_tx_tstamp_ready error, interrupt should not be rearmed,
> because error only happens on resets.
> > Reviewed-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
> Signed-off-by: Karol Kolacinski <karol.kolacinski@intel.com>
> ---

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

Thanks!
Jacob Keller Oct. 12, 2023, 11:15 p.m. UTC | #2 
On 10/11/2023 4:04 AM, Karol Kolacinski wrote:
> During reset, TX_TSYN interrupt should be processed as it may process
> timestamps in brief moments before and after reset.
> Timestamping should be enabled on VSIs at the end of reset procedure.
> On ice_get_phy_tx_tstamp_ready error, interrupt should not be rearmed,
> because error only happens on resets.
> 
> Reviewed-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
> Signed-off-by: Karol Kolacinski <karol.kolacinski@intel.com>
> ---
>  drivers/net/ethernet/intel/ice/ice_main.c |  2 +-
>  drivers/net/ethernet/intel/ice/ice_ptp.c  | 22 +++++++++++++---------
>  2 files changed, 14 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
> index afe19219a640..a58da0024fe5 100644
> --- a/drivers/net/ethernet/intel/ice/ice_main.c
> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
> @@ -3176,7 +3176,7 @@ static irqreturn_t ice_misc_intr(int __always_unused irq, void *data)
>  
>  	if (oicr & PFINT_OICR_TSYN_TX_M) {
>  		ena_mask &= ~PFINT_OICR_TSYN_TX_M;
> -		if (!hw->reset_ongoing && ice_ptp_pf_handles_tx_interrupt(pf))
> +		if (ice_ptp_pf_handles_tx_interrupt(pf))
>  			set_bit(ICE_MISC_THREAD_TX_TSTAMP, pf->misc_thread);
>  	}
>  
> diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c b/drivers/net/ethernet/intel/ice/ice_ptp.c
> index 1eddcbe89b0c..7e548a634f3f 100644
> --- a/drivers/net/ethernet/intel/ice/ice_ptp.c
> +++ b/drivers/net/ethernet/intel/ice/ice_ptp.c
> @@ -684,7 +684,9 @@ static enum ice_tx_tstamp_work ice_ptp_tx_tstamp_owner(struct ice_pf *pf)
>  
>  		/* Read the Tx ready status first */
>  		err = ice_get_phy_tx_tstamp_ready(&pf->hw, i, &tstamp_ready);
> -		if (err || tstamp_ready)
> +		if (err)
> +			break;
> +		else if (tstamp_ready)
>  			return ICE_TX_TSTAMP_WORK_PENDING;
>  	}
>  
> @@ -2444,12 +2446,10 @@ void ice_ptp_reset(struct ice_pf *pf)
>  	int err, itr = 1;
>  	u64 time_diff;
>  
> -	if (test_bit(ICE_PFR_REQ, pf->state))
> +	if (test_bit(ICE_PFR_REQ, pf->state) ||
> +	    !ice_pf_src_tmr_owned(pf))
>  		goto pfr;
>  
> -	if (!ice_pf_src_tmr_owned(pf))
> -		goto reset_ts;
> -
>  	err = ice_ptp_init_phc(hw);
>  	if (err)
>  		goto err;
> @@ -2493,10 +2493,6 @@ void ice_ptp_reset(struct ice_pf *pf)
>  			goto err;
>  	}
>  
> -reset_ts:
> -	/* Restart the PHY timestamping block */
> -	ice_ptp_reset_phy_timestamping(pf);
> -

Was this intending to get rid of the ice_ptp_reset_phy_timestamping()?

>  pfr:
>  	/* Init Tx structures */
>  	if (ice_is_e810(&pf->hw)) {
> @@ -2512,6 +2508,14 @@ void ice_ptp_reset(struct ice_pf *pf)
>  
>  	set_bit(ICE_FLAG_PTP, pf->flags);
>  
> +	/* 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);
> +
> +	if (ptp->tx_interrupt_mode)
> +		ice_ptp_configure_tx_tstamp(pf, true);
> +

Ah, we instead restart all PHY which which is the same as
ice_ptp_reset_phy_timestamping. Ok.


>  	/* Start periodic work going */
>  	kthread_queue_delayed_work(ptp->kworker, &ptp->work, 0);
>  
> 
> base-commit: 2318d58f358e7aef726c038aff87a68bec8f09e0
Pucha, HimasekharX Reddy Nov. 16, 2023, 8:11 a.m. UTC | #3 
> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of Karol Kolacinski
> Sent: Wednesday, October 11, 2023 4:34 PM
> To: intel-wired-lan@lists.osuosl.org
> Cc: Kolacinski, Karol <karol.kolacinski@intel.com>; Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Brandeburg, Jesse <jesse.brandeburg@intel.com>
> Subject: [Intel-wired-lan] [PATCH iwl-next] ice: Re-enable timestamping correctly after reset
>
> During reset, TX_TSYN interrupt should be processed as it may process
> timestamps in brief moments before and after reset.
> Timestamping should be enabled on VSIs at the end of reset procedure.
> On ice_get_phy_tx_tstamp_ready error, interrupt should not be rearmed,
> because error only happens on resets.
>
> Reviewed-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
> Signed-off-by: Karol Kolacinski <karol.kolacinski@intel.com>
> ---
>  drivers/net/ethernet/intel/ice/ice_main.c |  2 +-
>  drivers/net/ethernet/intel/ice/ice_ptp.c  | 22 +++++++++++++---------
>  2 files changed, 14 insertions(+), 10 deletions(-)
>

Tested-by: Pucha Himasekhar Reddy <himasekharx.reddy.pucha@intel.com> (A Contingent worker at Intel)
diff mbox series

Patch

diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index afe19219a640..a58da0024fe5 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -3176,7 +3176,7 @@  static irqreturn_t ice_misc_intr(int __always_unused irq, void *data)
 
 	if (oicr & PFINT_OICR_TSYN_TX_M) {
 		ena_mask &= ~PFINT_OICR_TSYN_TX_M;
-		if (!hw->reset_ongoing && ice_ptp_pf_handles_tx_interrupt(pf))
+		if (ice_ptp_pf_handles_tx_interrupt(pf))
 			set_bit(ICE_MISC_THREAD_TX_TSTAMP, pf->misc_thread);
 	}
 
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c b/drivers/net/ethernet/intel/ice/ice_ptp.c
index 1eddcbe89b0c..7e548a634f3f 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp.c
+++ b/drivers/net/ethernet/intel/ice/ice_ptp.c
@@ -684,7 +684,9 @@  static enum ice_tx_tstamp_work ice_ptp_tx_tstamp_owner(struct ice_pf *pf)
 
 		/* Read the Tx ready status first */
 		err = ice_get_phy_tx_tstamp_ready(&pf->hw, i, &tstamp_ready);
-		if (err || tstamp_ready)
+		if (err)
+			break;
+		else if (tstamp_ready)
 			return ICE_TX_TSTAMP_WORK_PENDING;
 	}
 
@@ -2444,12 +2446,10 @@  void ice_ptp_reset(struct ice_pf *pf)
 	int err, itr = 1;
 	u64 time_diff;
 
-	if (test_bit(ICE_PFR_REQ, pf->state))
+	if (test_bit(ICE_PFR_REQ, pf->state) ||
+	    !ice_pf_src_tmr_owned(pf))
 		goto pfr;
 
-	if (!ice_pf_src_tmr_owned(pf))
-		goto reset_ts;
-
 	err = ice_ptp_init_phc(hw);
 	if (err)
 		goto err;
@@ -2493,10 +2493,6 @@  void ice_ptp_reset(struct ice_pf *pf)
 			goto err;
 	}
 
-reset_ts:
-	/* Restart the PHY timestamping block */
-	ice_ptp_reset_phy_timestamping(pf);
-
 pfr:
 	/* Init Tx structures */
 	if (ice_is_e810(&pf->hw)) {
@@ -2512,6 +2508,14 @@  void ice_ptp_reset(struct ice_pf *pf)
 
 	set_bit(ICE_FLAG_PTP, pf->flags);
 
+	/* 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);
+
+	if (ptp->tx_interrupt_mode)
+		ice_ptp_configure_tx_tstamp(pf, true);
+
 	/* Start periodic work going */
 	kthread_queue_delayed_work(ptp->kworker, &ptp->work, 0);