[PATCH 0/2] ice: ptp: fix E825 timer synchronization and locking

[PATCH 0/2] ice: ptp: fix E825 timer synchronization and locking

[Grzegorz Nitka]

These two fixes address E825 PTP synchronization issues in
ice_ptp_hw.c.

The first patch serializes PHY timer start against concurrent PTP
command paths by holding the global PTP semaphore while programming
TIMETUS registers and issuing INIT_INCVAL.

The second patch fixes semaphore access for E825 2xNAC configurations by
making ice_ptp_lock() and ice_ptp_unlock() use the primary NAC register
block, matching the rest of the primary-only PTP register access path.

Together, the series closes two locking gaps in E825 timer control: one
during PHY timer initialization and one in 2xNAC semaphore selection.

Grzegorz Nitka (2):
  ice: ptp: serialize E825 PHY timer start with PTP lock
  ice: ptp: use primary NAC semaphore on E825

 drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 24 +++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)


base-commit: 3662329d3f304a421e0230ee3913dab021ec3a3d
-- 
2.39.3

[PATCH iwl-net 1/2] ice: ptp: serialize E825 PHY timer start with PTP lock

[Grzegorz Nitka]

ice_start_phy_timer_eth56g() programs TIMETUS registers and issues
INIT_INCVAL without holding the global PTP semaphore.

This allows concurrent PTP command paths to interleave with PHY timer
start, which can make the sequence fail and leave timer initialization
inconsistent.

Take the PTP lock around TIMETUS registers programming and INIT_INCVAL
command execution, and make sure the lock is released on all error paths.

Keep the subsequent sync step outside of this critical section, since
ice_sync_phy_timer_eth56g() takes the same semaphore internally.

Fixes: 7cab44f1c35f ("ice: Introduce ETH56G PHY model for E825C products")
Reviewed-by: Arkadiusz Kubalewski <Arkadiusz.kubalewski@intel.com>
Signed-off-by: Grzegorz Nitka <grzegorz.nitka@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
index 672218e5d1f9..8bb94e785f2a 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
+++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
@@ -2141,16 +2141,23 @@ int ice_start_phy_timer_eth56g(struct ice_hw *hw, u8 port)
 	}
 	incval = (u64)hi << 32 | lo;
 
+	if (!ice_ptp_lock(hw)) {
+		dev_err(ice_hw_to_dev(hw), "Failed to acquire PTP semaphore\n");
+		return -EBUSY;
+	}
+
 	err = ice_write_40b_ptp_reg_eth56g(hw, port, PHY_REG_TIMETUS_L, incval);
 	if (err)
-		return err;
+		goto err_ptp_unlock;
 
 	err = ice_ptp_one_port_cmd(hw, port, ICE_PTP_INIT_INCVAL);
 	if (err)
-		return err;
+		goto err_ptp_unlock;
 
 	ice_ptp_exec_tmr_cmd(hw);
 
+	ice_ptp_unlock(hw);
+
 	err = ice_sync_phy_timer_eth56g(hw, port);
 	if (err)
 		return err;
@@ -2166,6 +2173,10 @@ int ice_start_phy_timer_eth56g(struct ice_hw *hw, u8 port)
 	ice_debug(hw, ICE_DBG_PTP, "Enabled clock on PHY port %u\n", port);
 
 	return 0;
+
+err_ptp_unlock:
+	ice_ptp_unlock(hw);
+	return err;
 }
 
 /**
-- 
2.39.3

[PATCH iwl-net 2/2] ice: ptp: use primary NAC semaphore on E825

[Grzegorz Nitka]

For E825 2xNAC configurations, PTP semaphore operations must hit the
primary NAC register block so both sides coordinate on the same lock.

Commit e2193f9f9ec9 ("ice: enable timesync operation on 2xNAC E825
devices") updated other primary-only PTP register accesses to
use the primary NAC on non-primary functions, but left ice_ptp_lock()
and ice_ptp_unlock() operating on the local NAC. As a result, secondary
NAC PTP paths can take a different semaphore than the primary side.

Select the primary hardware in ice_ptp_lock() and ice_ptp_unlock() when
the current function is not primary, keeping semaphore operations
symmetric and consistent with the rest of the 2xNAC PTP register access
path.

Fixes: e2193f9f9ec9 ("ice: enable timesync operation on 2xNAC E825 devices")
Reviewed-by: Arkadiusz Kubalewski <Arkadiusz.kubalewski@intel.com>
Signed-off-by: Grzegorz Nitka <grzegorz.nitka@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
index 8bb94e785f2a..2c18e16fe053 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
+++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
@@ -5264,9 +5264,13 @@ static void ice_ptp_init_phy_e830(struct ice_ptp_hw *ptp)
  */
 bool ice_ptp_lock(struct ice_hw *hw)
 {
+	struct ice_pf *pf = container_of(hw, struct ice_pf, hw);
 	u32 hw_lock;
 	int i;
 
+	if (!ice_is_primary(hw))
+		hw = ice_get_primary_hw(pf);
+
 #define MAX_TRIES 15
 
 	for (i = 0; i < MAX_TRIES; i++) {
@@ -5293,6 +5297,11 @@ bool ice_ptp_lock(struct ice_hw *hw)
  */
 void ice_ptp_unlock(struct ice_hw *hw)
 {
+	struct ice_pf *pf = container_of(hw, struct ice_pf, hw);
+
+	if (!ice_is_primary(hw))
+		hw = ice_get_primary_hw(pf);
+
 	wr32(hw, PFTSYN_SEM + (PFTSYN_SEM_BYTES * hw->pf_id), 0);
 }
 
-- 
2.39.3

RE: [Intel-wired-lan] [PATCH iwl-net 1/2] ice: ptp: serialize E825 PHY timer start with PTP lock

[Loktionov, Aleksandr]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf
> Of Grzegorz Nitka
> Sent: Wednesday, April 22, 2026 2:32 PM
> To: intel-wired-lan@lists.osuosl.org
> Cc: netdev@vger.kernel.org; Kubalewski, Arkadiusz
> <arkadiusz.kubalewski@intel.com>; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>; Kitszel, Przemyslaw
> <przemyslaw.kitszel@intel.com>
> Subject: [Intel-wired-lan] [PATCH iwl-net 1/2] ice: ptp: serialize
> E825 PHY timer start with PTP lock
> 
> ice_start_phy_timer_eth56g() programs TIMETUS registers and issues
> INIT_INCVAL without holding the global PTP semaphore.
> 
> This allows concurrent PTP command paths to interleave with PHY timer
> start, which can make the sequence fail and leave timer initialization
> inconsistent.
> 
> Take the PTP lock around TIMETUS registers programming and INIT_INCVAL
> command execution, and make sure the lock is released on all error
> paths.
> 
> Keep the subsequent sync step outside of this critical section, since
> ice_sync_phy_timer_eth56g() takes the same semaphore internally.
> 
> Fixes: 7cab44f1c35f ("ice: Introduce ETH56G PHY model for E825C
> products")
> Reviewed-by: Arkadiusz Kubalewski <Arkadiusz.kubalewski@intel.com>
> Signed-off-by: Grzegorz Nitka <grzegorz.nitka@intel.com>

I recommend to add Cc: stable@vger.kernel.org

Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>

> ---
>  drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
> b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
> index 672218e5d1f9..8bb94e785f2a 100644
> --- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
> +++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
> @@ -2141,16 +2141,23 @@ int ice_start_phy_timer_eth56g(struct ice_hw
> *hw, u8 port)
>  	}
>  	incval = (u64)hi << 32 | lo;
> 
> +	if (!ice_ptp_lock(hw)) {
> +		dev_err(ice_hw_to_dev(hw), "Failed to acquire PTP
> semaphore\n");
> +		return -EBUSY;
> +	}
> +
>  	err = ice_write_40b_ptp_reg_eth56g(hw, port, PHY_REG_TIMETUS_L,
> incval);
>  	if (err)
> -		return err;
> +		goto err_ptp_unlock;
> 
>  	err = ice_ptp_one_port_cmd(hw, port, ICE_PTP_INIT_INCVAL);
>  	if (err)
> -		return err;
> +		goto err_ptp_unlock;
> 
>  	ice_ptp_exec_tmr_cmd(hw);
> 
> +	ice_ptp_unlock(hw);
> +
>  	err = ice_sync_phy_timer_eth56g(hw, port);
>  	if (err)
>  		return err;
> @@ -2166,6 +2173,10 @@ int ice_start_phy_timer_eth56g(struct ice_hw
> *hw, u8 port)
>  	ice_debug(hw, ICE_DBG_PTP, "Enabled clock on PHY port %u\n",
> port);
> 
>  	return 0;
> +
> +err_ptp_unlock:
> +	ice_ptp_unlock(hw);
> +	return err;
>  }
> 
>  /**
> --
> 2.39.3

RE: [Intel-wired-lan] [PATCH iwl-net 2/2] ice: ptp: use primary NAC semaphore on E825

[Loktionov, Aleksandr]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf
> Of Grzegorz Nitka
> Sent: Wednesday, April 22, 2026 2:32 PM
> To: intel-wired-lan@lists.osuosl.org
> Cc: netdev@vger.kernel.org; Kubalewski, Arkadiusz
> <arkadiusz.kubalewski@intel.com>; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>; Kitszel, Przemyslaw
> <przemyslaw.kitszel@intel.com>
> Subject: [Intel-wired-lan] [PATCH iwl-net 2/2] ice: ptp: use primary
> NAC semaphore on E825
> 
> For E825 2xNAC configurations, PTP semaphore operations must hit the
> primary NAC register block so both sides coordinate on the same lock.
> 
> Commit e2193f9f9ec9 ("ice: enable timesync operation on 2xNAC E825
> devices") updated other primary-only PTP register accesses to use the
> primary NAC on non-primary functions, but left ice_ptp_lock() and
> ice_ptp_unlock() operating on the local NAC. As a result, secondary
> NAC PTP paths can take a different semaphore than the primary side.
> 
> Select the primary hardware in ice_ptp_lock() and ice_ptp_unlock()
> when the current function is not primary, keeping semaphore operations
> symmetric and consistent with the rest of the 2xNAC PTP register
> access path.
> 
> Fixes: e2193f9f9ec9 ("ice: enable timesync operation on 2xNAC E825
> devices")
> Reviewed-by: Arkadiusz Kubalewski <Arkadiusz.kubalewski@intel.com>
> Signed-off-by: Grzegorz Nitka <grzegorz.nitka@intel.com>

I recommend to add Cc: stable@vger.kernel.org

Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>

> ---
>  drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
> b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
> index 8bb94e785f2a..2c18e16fe053 100644
> --- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
> +++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
> @@ -5264,9 +5264,13 @@ static void ice_ptp_init_phy_e830(struct
> ice_ptp_hw *ptp)
>   */
>  bool ice_ptp_lock(struct ice_hw *hw)
>  {
> +	struct ice_pf *pf = container_of(hw, struct ice_pf, hw);
>  	u32 hw_lock;
>  	int i;
> 
> +	if (!ice_is_primary(hw))
> +		hw = ice_get_primary_hw(pf);
> +
>  #define MAX_TRIES 15
> 
>  	for (i = 0; i < MAX_TRIES; i++) {
> @@ -5293,6 +5297,11 @@ bool ice_ptp_lock(struct ice_hw *hw)
>   */
>  void ice_ptp_unlock(struct ice_hw *hw)
>  {
> +	struct ice_pf *pf = container_of(hw, struct ice_pf, hw);
> +
> +	if (!ice_is_primary(hw))
> +		hw = ice_get_primary_hw(pf);
> +
>  	wr32(hw, PFTSYN_SEM + (PFTSYN_SEM_BYTES * hw->pf_id), 0);  }
> 
> --
> 2.39.3