> -----Original Message-----
> From: Intel-wired-lan <[email protected]> On Behalf
> Of Markus Blöchl
> Sent: Sunday, November 30, 2025 4:24 PM
> To: Nguyen, Anthony L <[email protected]>; Kitszel,
> Przemyslaw <[email protected]>; Keller, Jacob E
> <[email protected]>; Andrew Lunn <[email protected]>; David
> S. Miller <[email protected]>; Eric Dumazet <[email protected]>;
> Jakub Kicinski <[email protected]>; Paolo Abeni <[email protected]>
> Cc: Richard Cochran <[email protected]>; Bloechl, Markus
> <[email protected]>; [email protected];
> [email protected]; [email protected]; Markus Blöchl
> <[email protected]>
> Subject: [Intel-wired-lan] [PATCH net v2] i40e: fix ptp time increment
> while link is down
>
> When an X710 ethernet port with an active ptp daemon (like the ptp4l
> and phc2sys combo) suddenly loses its link and regains it after a
> while, the ptp daemon has a hard time to recover synchronization and
> sometimes entirely fails to do so.
>
> The issue seems to be related to a wrongly configured increment while
> the link is down. This could not be observed with the Intel reference
> driver.
> We identified the fix to appear in Intels official ethernet-linux-i40e
> release version 2.17.4.
>
> Include the relevant changes in the kernel version of this driver.
>
> Fixes: beb0dff1251d ("i40e: enable PTP")
> Cc: [email protected]
> Signed-off-by: Markus Blöchl <[email protected]>
> ---
> Tested with an X710 at 10G link speed and kernel version 6.12.42.
> ---
> Changes in v2:
> - Fix kdoc and code formatting
> - Rebase onto net tree
> - Link to v1: https://lore.kernel.org/r/20251119-i40e_ptp_link_down-
> [email protected]
> ---
> drivers/net/ethernet/intel/i40e/i40e_adminq_cmd.h | 9 +++
> drivers/net/ethernet/intel/i40e/i40e_ptp.c | 69
> +++++++++++++++++++++--
> drivers/net/ethernet/intel/i40e/i40e_register.h | 9 +++
> drivers/net/ethernet/intel/i40e/i40e_type.h | 8 +++
> 4 files changed, 90 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_adminq_cmd.h
> b/drivers/net/ethernet/intel/i40e/i40e_adminq_cmd.h
> index cc02a85ad42b..ec176e9569ad 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_adminq_cmd.h
> +++ b/drivers/net/ethernet/intel/i40e/i40e_adminq_cmd.h
> @@ -1488,6 +1488,15 @@ enum i40e_aq_link_speed {
> I40E_LINK_SPEED_25GB = BIT(I40E_LINK_SPEED_25GB_SHIFT),
> };
>
> +enum i40e_prt_mac_pcs_link_speed {
> + I40E_PRT_MAC_PCS_LINK_SPEED_UNKNOWN = 0,
> + I40E_PRT_MAC_PCS_LINK_SPEED_100MB,
> + I40E_PRT_MAC_PCS_LINK_SPEED_1GB,
> + I40E_PRT_MAC_PCS_LINK_SPEED_10GB,
> + I40E_PRT_MAC_PCS_LINK_SPEED_40GB,
> + I40E_PRT_MAC_PCS_LINK_SPEED_20GB
> +};
> +
> struct i40e_aqc_module_desc {
> u8 oui[3];
> u8 reserved1;
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_ptp.c
> b/drivers/net/ethernet/intel/i40e/i40e_ptp.c
> index 33535418178b..89abe2f22216 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_ptp.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_ptp.c
> @@ -847,6 +847,66 @@ void i40e_ptp_rx_hwtstamp(struct i40e_pf *pf,
> struct sk_buff *skb, u8 index)
> i40e_ptp_convert_to_hwtstamp(skb_hwtstamps(skb), ns); }
>
> +/**
> + * i40e_ptp_get_link_speed_hw - get the link speed
> + * @pf: Board private structure
> + *
> + * Calculate link speed depending on the link status.
> + *
> + * Return: current link speed.
> + **/
The function uses hardcoded port index 0 in register accesses
(I40E_PRTMAC_LINKSTA(0), I40E_PRTMAC_PCS_LINK_STATUS1(0)),
but this limitation is not documented. Additionally, the return value
documentation is vague.
> +static enum i40e_aq_link_speed i40e_ptp_get_link_speed_hw(struct
> +i40e_pf *pf) {
> + bool link_up = pf->hw.phy.link_info.link_info &
> I40E_AQ_LINK_UP;
> + enum i40e_aq_link_speed link_speed = I40E_LINK_SPEED_UNKNOWN;
> + struct i40e_hw *hw = &pf->hw;
> +
> + if (link_up) {
> + struct i40e_link_status *hw_link_info = &hw-
> >phy.link_info;
> +
> + i40e_aq_get_link_info(hw, true, NULL, NULL);
> + link_speed = hw_link_info->link_speed;
> + } else {
> + enum i40e_prt_mac_link_speed prtmac_linksta;
I think Linux kernel coding style discourages implicit conversions between enum
types and integer types without explicit cast.
Better declare u32 prtmac_linksta;
> + u64 prtmac_pcs_linksta;
> +
> + prtmac_linksta = (rd32(hw, I40E_PRTMAC_LINKSTA(0)) &
> + I40E_PRTMAC_LINKSTA_MAC_LINK_SPEED_MASK)
> >>
> + I40E_PRTMAC_LINKSTA_MAC_LINK_SPEED_SHIFT;
> + if (prtmac_linksta == I40E_PRT_MAC_LINK_SPEED_40GB) {
> + link_speed = I40E_LINK_SPEED_40GB;
> + } else {
> + i40e_aq_debug_read_register(hw,
> +
> I40E_PRTMAC_PCS_LINK_STATUS1(0),
> + &prtmac_pcs_linksta,
> + NULL);
Both i40e_aq_get_link_info() and i40e_aq_debug_read_register() return error
status codes (i40e_status_code / int),
but these return values are completely ignored. If either admin queue command
fails,
the function proceeds with potentially stale or uninitialized data.
> +
> + prtmac_pcs_linksta = (prtmac_pcs_linksta &
> +
> I40E_PRTMAC_PCS_LINK_STATUS1_LINK_SPEED_MASK) >>
> +
> I40E_PRTMAC_PCS_LINK_STATUS1_LINK_SPEED_SHIFT;
> +
> + switch (prtmac_pcs_linksta) {
> + case I40E_PRT_MAC_PCS_LINK_SPEED_100MB:
> + link_speed = I40E_LINK_SPEED_100MB;
> + break;
> + case I40E_PRT_MAC_PCS_LINK_SPEED_1GB:
> + link_speed = I40E_LINK_SPEED_1GB;
> + break;
> + case I40E_PRT_MAC_PCS_LINK_SPEED_10GB:
> + link_speed = I40E_LINK_SPEED_10GB;
> + break;
> + case I40E_PRT_MAC_PCS_LINK_SPEED_20GB:
> + link_speed = I40E_LINK_SPEED_20GB;
> + break;
> + default:
> + link_speed = I40E_LINK_SPEED_UNKNOWN;
> + }
> + }
> + }
> +
> + return link_speed;
> +}
> +
> /**
> * i40e_ptp_set_increment - Utility function to update clock
> increment rate
> * @pf: Board private structure
> @@ -857,16 +917,14 @@ void i40e_ptp_rx_hwtstamp(struct i40e_pf *pf,
> struct sk_buff *skb, u8 index)
> **/
> void i40e_ptp_set_increment(struct i40e_pf *pf) {
> - struct i40e_link_status *hw_link_info;
> + enum i40e_aq_link_speed link_speed;
> struct i40e_hw *hw = &pf->hw;
> u64 incval;
> u32 mult;
>
> - hw_link_info = &hw->phy.link_info;
> + link_speed = i40e_ptp_get_link_speed_hw(pf);
>
> - i40e_aq_get_link_info(&pf->hw, true, NULL, NULL);
> -
> - switch (hw_link_info->link_speed) {
> + switch (link_speed) {
> case I40E_LINK_SPEED_10GB:
> mult = I40E_PTP_10GB_INCVAL_MULT;
> break;
> @@ -909,6 +967,7 @@ void i40e_ptp_set_increment(struct i40e_pf *pf)
> /* Update the base adjustement value. */
> WRITE_ONCE(pf->ptp_adj_mult, mult);
> smp_mb(); /* Force the above update. */
> + i40e_ptp_set_1pps_signal_hw(pf);
> }
>
> /**
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_register.h
> b/drivers/net/ethernet/intel/i40e/i40e_register.h
> index 432afbb64201..c4051dbcc297 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_register.h
> +++ b/drivers/net/ethernet/intel/i40e/i40e_register.h
> @@ -530,6 +530,15 @@
> #define I40E_PRTMAC_HSEC_CTL_TX_PAUSE_REFRESH_TIMER_SHIFT 0 #define
> I40E_PRTMAC_HSEC_CTL_TX_PAUSE_REFRESH_TIMER_MASK I40E_MASK(0xFFFF, \
> I40E_PRTMAC_HSEC_CTL_TX_PAUSE_REFRESH_TIMER_SHIFT)
> +/* _i=0...3 */ /* Reset: GLOBR */
> +#define I40E_PRTMAC_PCS_LINK_STATUS1(_i) (0x0008C200 + ((_i) * 4))
> +#define I40E_PRTMAC_PCS_LINK_STATUS1_LINK_SPEED_SHIFT 24 #define
> +I40E_PRTMAC_PCS_LINK_STATUS1_LINK_SPEED_MASK I40E_MASK(0x7,
> +I40E_PRTMAC_PCS_LINK_STATUS1_LINK_SPEED_SHIFT)
> +#define I40E_PRTMAC_PCS_LINK_STATUS2 0x0008C220
> +/* _i=0...3 */ /* Reset: GLOBR */
> +#define I40E_PRTMAC_LINKSTA(_i) (0x001E2420 + ((_i) * 4)) #define
> +I40E_PRTMAC_LINKSTA_MAC_LINK_SPEED_SHIFT 27 #define
> +I40E_PRTMAC_LINKSTA_MAC_LINK_SPEED_MASK I40E_MASK(0x7,
> +I40E_PRTMAC_LINKSTA_MAC_LINK_SPEED_SHIFT)
> #define I40E_GLNVM_FLA 0x000B6108 /* Reset: POR */ #define
> I40E_GLNVM_FLA_LOCKED_SHIFT 6 #define I40E_GLNVM_FLA_LOCKED_MASK
> I40E_MASK(0x1, I40E_GLNVM_FLA_LOCKED_SHIFT) diff --git
> a/drivers/net/ethernet/intel/i40e/i40e_type.h
> b/drivers/net/ethernet/intel/i40e/i40e_type.h
> index ed8bbdb586da..98c8c5709e5f 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_type.h
> +++ b/drivers/net/ethernet/intel/i40e/i40e_type.h
> @@ -115,6 +115,14 @@ enum i40e_queue_type {
> I40E_QUEUE_TYPE_UNKNOWN
> };
>
> +enum i40e_prt_mac_link_speed {
> + I40E_PRT_MAC_LINK_SPEED_100MB = 0,
> + I40E_PRT_MAC_LINK_SPEED_1GB,
> + I40E_PRT_MAC_LINK_SPEED_10GB,
> + I40E_PRT_MAC_LINK_SPEED_40GB,
> + I40E_PRT_MAC_LINK_SPEED_20GB
> +};
> +
> struct i40e_link_status {
> enum i40e_aq_phy_type phy_type;
> enum i40e_aq_link_speed link_speed;
>
> ---
> base-commit: e5235eb6cfe02a51256013a78f7b28779a7740d5
> change-id: 20251119-i40e_ptp_link_down-47934f9e155d
>
> Best regards,
> --
> Markus Blöchl <[email protected]>
>
>
> --
With the best regards
Alex
