ice: fix PTP timestamping broken by SyncE code on E825C

[PATCH iwl-net] ice: fix PTP timestamping broken by SyncE code on E825C

Posted by Petr Oros 6 days, 7 hours ago

The E825C SyncE support added in commit ad1df4f2d591 ("ice: dpll:
Support E825-C SyncE and dynamic pin discovery") introduced a SyncE
reconfiguration block in ice_ptp_link_change() that prevents
ice_ptp_port_phy_restart() from being called in several error paths.
Without the PHY restart, PTP timestamps stop working after any link
change event.

There are three ways the PHY restart gets blocked:

1. When DPLL initialization fails (e.g. missing ACPI firmware node
   properties), ICE_FLAG_DPLL is not set and the function returns early
   before reaching the PHY restart.

2. When ice_tspll_bypass_mux_active_e825c() fails to read the CGU
   register, WARN_ON_ONCE fires and the function returns early.

3. When ice_tspll_cfg_synce_ethdiv_e825c() fails to configure the
   clock divider for an active pin, same early return.

SyncE and PTP are independent features. SyncE reconfiguration failures
must not prevent the PTP PHY restart that is essential for timestamp
recovery after link changes.

Fix by making the entire SyncE block conditional on ICE_FLAG_DPLL
without an early return, and replacing the WARN_ON_ONCE + return error
handling inside the loop with dev_err_once + break. The function always
proceeds to ice_ptp_port_phy_restart() regardless of SyncE errors.

Fixes: ad1df4f2d591 ("ice: dpll: Support E825-C SyncE and dynamic pin discovery")
Signed-off-by: Petr Oros <poros@redhat.com>
---
 drivers/net/ethernet/intel/ice/ice_ptp.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c b/drivers/net/ethernet/intel/ice/ice_ptp.c
index 094e96219f4565..60bc47099432a2 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp.c
+++ b/drivers/net/ethernet/intel/ice/ice_ptp.c
@@ -1296,12 +1296,10 @@ void ice_ptp_link_change(struct ice_pf *pf, bool linkup)
 	if (pf->hw.reset_ongoing)
 		return;
 
-	if (hw->mac_type == ICE_MAC_GENERIC_3K_E825) {
+	if (hw->mac_type == ICE_MAC_GENERIC_3K_E825 &&
+	    test_bit(ICE_FLAG_DPLL, pf->flags)) {
 		int pin, err;
 
-		if (!test_bit(ICE_FLAG_DPLL, pf->flags))
-			return;
-
 		mutex_lock(&pf->dplls.lock);
 		for (pin = 0; pin < ICE_SYNCE_CLK_NUM; pin++) {
 			enum ice_synce_clk clk_pin;
@@ -1314,15 +1312,19 @@ void ice_ptp_link_change(struct ice_pf *pf, bool linkup)
 								port_num,
 								&active,
 								clk_pin);
-			if (WARN_ON_ONCE(err)) {
-				mutex_unlock(&pf->dplls.lock);
-				return;
+			if (err) {
+				dev_err_once(ice_pf_to_dev(pf),
+					     "Failed to read SyncE bypass mux for pin %d, err %d\n",
+					     pin, err);
+				break;
 			}
 
 			err = ice_tspll_cfg_synce_ethdiv_e825c(hw, clk_pin);
-			if (active && WARN_ON_ONCE(err)) {
-				mutex_unlock(&pf->dplls.lock);
-				return;
+			if (active && err) {
+				dev_err_once(ice_pf_to_dev(pf),
+					     "Failed to configure SyncE ETH divider for pin %d, err %d\n",
+					     pin, err);
+				break;
 			}
 		}
 		mutex_unlock(&pf->dplls.lock);
-- 
2.52.0

RE: [PATCH iwl-net] ice: fix PTP timestamping broken by SyncE code on E825C

Posted by Loktionov, Aleksandr 5 days, 17 hours ago


> -----Original Message-----
> From: Petr Oros <poros@redhat.com>
> Sent: Friday, March 27, 2026 8:47 AM
> To: netdev@vger.kernel.org
> Cc: stable@vger.kernel.org; Oros, Petr <poros@redhat.com>; Nguyen,
> Anthony L <anthony.l.nguyen@intel.com>; Kitszel, Przemyslaw
> <przemyslaw.kitszel@intel.com>; Andrew Lunn <andrew+netdev@lunn.ch>;
> David S. Miller <davem@davemloft.net>; Eric Dumazet
> <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo Abeni
> <pabeni@redhat.com>; Richard Cochran <richardcochran@gmail.com>;
> Kubalewski, Arkadiusz <arkadiusz.kubalewski@intel.com>; Nitka,
> Grzegorz <grzegorz.nitka@intel.com>; Loktionov, Aleksandr
> <aleksandr.loktionov@intel.com>; Vecera, Ivan <ivecera@redhat.com>;
> intel-wired-lan@lists.osuosl.org; linux-kernel@vger.kernel.org
> Subject: [PATCH iwl-net] ice: fix PTP timestamping broken by SyncE
> code on E825C
> 
> The E825C SyncE support added in commit ad1df4f2d591 ("ice: dpll:
> Support E825-C SyncE and dynamic pin discovery") introduced a SyncE
> reconfiguration block in ice_ptp_link_change() that prevents
> ice_ptp_port_phy_restart() from being called in several error paths.
> Without the PHY restart, PTP timestamps stop working after any link
> change event.
> 
> There are three ways the PHY restart gets blocked:
> 
> 1. When DPLL initialization fails (e.g. missing ACPI firmware node
>    properties), ICE_FLAG_DPLL is not set and the function returns
> early
>    before reaching the PHY restart.
> 
> 2. When ice_tspll_bypass_mux_active_e825c() fails to read the CGU
>    register, WARN_ON_ONCE fires and the function returns early.
> 
> 3. When ice_tspll_cfg_synce_ethdiv_e825c() fails to configure the
>    clock divider for an active pin, same early return.
> 
> SyncE and PTP are independent features. SyncE reconfiguration failures
> must not prevent the PTP PHY restart that is essential for timestamp
> recovery after link changes.
> 
> Fix by making the entire SyncE block conditional on ICE_FLAG_DPLL
> without an early return, and replacing the WARN_ON_ONCE + return error
> handling inside the loop with dev_err_once + break. The function
> always proceeds to ice_ptp_port_phy_restart() regardless of SyncE
> errors.
> 
> Fixes: ad1df4f2d591 ("ice: dpll: Support E825-C SyncE and dynamic pin
> discovery")
> Signed-off-by: Petr Oros <poros@redhat.com>
> ---
>  drivers/net/ethernet/intel/ice/ice_ptp.c | 22 ++++++++++++----------
>  1 file changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c
> b/drivers/net/ethernet/intel/ice/ice_ptp.c
> index 094e96219f4565..60bc47099432a2 100644
> --- a/drivers/net/ethernet/intel/ice/ice_ptp.c
> +++ b/drivers/net/ethernet/intel/ice/ice_ptp.c
> @@ -1296,12 +1296,10 @@ void ice_ptp_link_change(struct ice_pf *pf,
> bool linkup)
>  	if (pf->hw.reset_ongoing)
>  		return;
> 
> -	if (hw->mac_type == ICE_MAC_GENERIC_3K_E825) {
> +	if (hw->mac_type == ICE_MAC_GENERIC_3K_E825 &&
> +	    test_bit(ICE_FLAG_DPLL, pf->flags)) {
>  		int pin, err;
> 
> -		if (!test_bit(ICE_FLAG_DPLL, pf->flags))
> -			return;
> -
>  		mutex_lock(&pf->dplls.lock);
>  		for (pin = 0; pin < ICE_SYNCE_CLK_NUM; pin++) {
>  			enum ice_synce_clk clk_pin;
> @@ -1314,15 +1312,19 @@ void ice_ptp_link_change(struct ice_pf *pf,
> bool linkup)
>  								port_num,
>  								&active,
>  								clk_pin);
> -			if (WARN_ON_ONCE(err)) {
> -				mutex_unlock(&pf->dplls.lock);
> -				return;
> +			if (err) {
> +				dev_err_once(ice_pf_to_dev(pf),
> +					     "Failed to read SyncE bypass
> mux for pin %d, err %d\n",
> +					     pin, err);
> +				break;
>  			}
> 
>  			err = ice_tspll_cfg_synce_ethdiv_e825c(hw,
> clk_pin);
> -			if (active && WARN_ON_ONCE(err)) {
> -				mutex_unlock(&pf->dplls.lock);
> -				return;
> +			if (active && err) {
> +				dev_err_once(ice_pf_to_dev(pf),
> +					     "Failed to configure SyncE ETH
> divider for pin %d, err %d\n",
> +					     pin, err);
> +				break;
>  			}
>  		}
>  		mutex_unlock(&pf->dplls.lock);
> --
> 2.52.0

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

RE: [PATCH iwl-net] ice: fix PTP timestamping broken by SyncE code on E825C

Posted by Nitka, Grzegorz 5 days, 18 hours ago


> -----Original Message-----
> From: Petr Oros <poros@redhat.com>
> Sent: Friday, March 27, 2026 8:47 AM
> To: netdev@vger.kernel.org
> Cc: stable@vger.kernel.org; Oros, Petr <poros@redhat.com>; Nguyen,
> Anthony L <anthony.l.nguyen@intel.com>; Kitszel, Przemyslaw
> <przemyslaw.kitszel@intel.com>; Andrew Lunn <andrew+netdev@lunn.ch>;
> David S. Miller <davem@davemloft.net>; Eric Dumazet
> <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo Abeni
> <pabeni@redhat.com>; Richard Cochran <richardcochran@gmail.com>;
> Kubalewski, Arkadiusz <arkadiusz.kubalewski@intel.com>; Nitka, Grzegorz
> <grzegorz.nitka@intel.com>; Loktionov, Aleksandr
> <aleksandr.loktionov@intel.com>; Vecera, Ivan <ivecera@redhat.com>;
> intel-wired-lan@lists.osuosl.org; linux-kernel@vger.kernel.org
> Subject: [PATCH iwl-net] ice: fix PTP timestamping broken by SyncE code on
> E825C
> 
> The E825C SyncE support added in commit ad1df4f2d591 ("ice: dpll:
> Support E825-C SyncE and dynamic pin discovery") introduced a SyncE
> reconfiguration block in ice_ptp_link_change() that prevents
> ice_ptp_port_phy_restart() from being called in several error paths.
> Without the PHY restart, PTP timestamps stop working after any link
> change event.
> 
> There are three ways the PHY restart gets blocked:
> 
> 1. When DPLL initialization fails (e.g. missing ACPI firmware node
>    properties), ICE_FLAG_DPLL is not set and the function returns early
>    before reaching the PHY restart.
> 
> 2. When ice_tspll_bypass_mux_active_e825c() fails to read the CGU
>    register, WARN_ON_ONCE fires and the function returns early.
> 
> 3. When ice_tspll_cfg_synce_ethdiv_e825c() fails to configure the
>    clock divider for an active pin, same early return.
> 
> SyncE and PTP are independent features. SyncE reconfiguration failures
> must not prevent the PTP PHY restart that is essential for timestamp
> recovery after link changes.
> 
> Fix by making the entire SyncE block conditional on ICE_FLAG_DPLL
> without an early return, and replacing the WARN_ON_ONCE + return error
> handling inside the loop with dev_err_once + break. The function always
> proceeds to ice_ptp_port_phy_restart() regardless of SyncE errors.
> 
> Fixes: ad1df4f2d591 ("ice: dpll: Support E825-C SyncE and dynamic pin
> discovery")
> Signed-off-by: Petr Oros <poros@redhat.com>

Thanks Petr for catching and fixing this.
Tested it in the environment w/o ACPI support and, indeed, PTP stopped 
working. This is a valid fix.

Thanks!

Reviewed-by: Grzegorz Nitka <grzegorz.nitka@intel.com>
> ---
>  drivers/net/ethernet/intel/ice/ice_ptp.c | 22 ++++++++++++----------
>  1 file changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c
> b/drivers/net/ethernet/intel/ice/ice_ptp.c
> index 094e96219f4565..60bc47099432a2 100644
> --- a/drivers/net/ethernet/intel/ice/ice_ptp.c
> +++ b/drivers/net/ethernet/intel/ice/ice_ptp.c
> @@ -1296,12 +1296,10 @@ void ice_ptp_link_change(struct ice_pf *pf, bool
> linkup)
>  	if (pf->hw.reset_ongoing)
>  		return;
> 
> -	if (hw->mac_type == ICE_MAC_GENERIC_3K_E825) {
> +	if (hw->mac_type == ICE_MAC_GENERIC_3K_E825 &&
> +	    test_bit(ICE_FLAG_DPLL, pf->flags)) {
>  		int pin, err;
> 
> -		if (!test_bit(ICE_FLAG_DPLL, pf->flags))
> -			return;
> -
>  		mutex_lock(&pf->dplls.lock);
>  		for (pin = 0; pin < ICE_SYNCE_CLK_NUM; pin++) {
>  			enum ice_synce_clk clk_pin;
> @@ -1314,15 +1312,19 @@ void ice_ptp_link_change(struct ice_pf *pf, bool
> linkup)
>  								port_num,
>  								&active,
>  								clk_pin);
> -			if (WARN_ON_ONCE(err)) {
> -				mutex_unlock(&pf->dplls.lock);
> -				return;
> +			if (err) {
> +				dev_err_once(ice_pf_to_dev(pf),
> +					     "Failed to read SyncE bypass mux
> for pin %d, err %d\n",
> +					     pin, err);
> +				break;
>  			}
> 
>  			err = ice_tspll_cfg_synce_ethdiv_e825c(hw, clk_pin);
> -			if (active && WARN_ON_ONCE(err)) {
> -				mutex_unlock(&pf->dplls.lock);
> -				return;
> +			if (active && err) {
> +				dev_err_once(ice_pf_to_dev(pf),
> +					     "Failed to configure SyncE ETH
> divider for pin %d, err %d\n",
> +					     pin, err);
> +				break;
>  			}
>  		}
>  		mutex_unlock(&pf->dplls.lock);
> --
> 2.52.0