* [PATCH] i40e: fix ptp time increment while link is down
@ 2025-11-19 16:09 Markus Blöchl
2025-11-24 21:57 ` [Intel-wired-lan] " Jacob Keller
2025-11-25 22:48 ` Tony Nguyen
0 siblings, 2 replies; 4+ messages in thread
From: Markus Blöchl @ 2025-11-19 16:09 UTC (permalink / raw)
To: Tony Nguyen, Przemek Kitszel, Andrew Lunn, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: Richard Cochran, Markus Blöchl, intel-wired-lan, netdev,
linux-kernel
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: stable@vger.kernel.org
Signed-off-by: Markus Blöchl <markus@blochl.de>
---
Tested with an X710 at 10G link speed and kernel version 6.12.42.
---
drivers/net/ethernet/intel/i40e/i40e_adminq_cmd.h | 9 +++
drivers/net/ethernet/intel/i40e/i40e_ptp.c | 68 +++++++++++++++++++++--
drivers/net/ethernet/intel/i40e/i40e_register.h | 9 +++
drivers/net/ethernet/intel/i40e/i40e_type.h | 8 +++
4 files changed, 89 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..ee6927e2c6f8 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_ptp.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_ptp.c
@@ -847,6 +847,65 @@ 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 the link speed.
+ **/
+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;
+ 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);
+
+ 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 +916,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 +966,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: 8b690556d8fe074b4f9835075050fba3fb180e93
change-id: 20251119-i40e_ptp_link_down-47934f9e155d
Best regards,
--
Markus Blöchl <markus@blochl.de>
--
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [Intel-wired-lan] [PATCH] i40e: fix ptp time increment while link is down
2025-11-19 16:09 [PATCH] i40e: fix ptp time increment while link is down Markus Blöchl
@ 2025-11-24 21:57 ` Jacob Keller
2025-11-25 22:48 ` Tony Nguyen
1 sibling, 0 replies; 4+ messages in thread
From: Jacob Keller @ 2025-11-24 21:57 UTC (permalink / raw)
To: Markus Blöchl, Tony Nguyen, Przemek Kitszel, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: Richard Cochran, Markus Blöchl, intel-wired-lan, netdev,
linux-kernel
[-- Attachment #1.1: Type: text/plain, Size: 7701 bytes --]
On 11/19/2025 8:09 AM, Markus Blöchl wrote:
> 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: stable@vger.kernel.org
> Signed-off-by: Markus Blöchl <markus@blochl.de>
> ---
I checked through our out-of-tree history and can confirm essentially
this exact code was added there back in 2021, but the developer never
managed to get around to upstreaming this fix.
Thanks for discovering and resolving that gap!
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
> Tested with an X710 at 10G link speed and kernel version 6.12.42.
> ---
> drivers/net/ethernet/intel/i40e/i40e_adminq_cmd.h | 9 +++
> drivers/net/ethernet/intel/i40e/i40e_ptp.c | 68 +++++++++++++++++++++--
> drivers/net/ethernet/intel/i40e/i40e_register.h | 9 +++
> drivers/net/ethernet/intel/i40e/i40e_type.h | 8 +++
> 4 files changed, 89 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..ee6927e2c6f8 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_ptp.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_ptp.c
> @@ -847,6 +847,65 @@ 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 the link speed.
> + **/
> +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;
> + 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);
> +
> + 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 +916,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 +966,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: 8b690556d8fe074b4f9835075050fba3fb180e93
> change-id: 20251119-i40e_ptp_link_down-47934f9e155d
>
> Best regards,
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] i40e: fix ptp time increment while link is down
2025-11-19 16:09 [PATCH] i40e: fix ptp time increment while link is down Markus Blöchl
2025-11-24 21:57 ` [Intel-wired-lan] " Jacob Keller
@ 2025-11-25 22:48 ` Tony Nguyen
2025-11-30 14:28 ` Markus Blöchl
1 sibling, 1 reply; 4+ messages in thread
From: Tony Nguyen @ 2025-11-25 22:48 UTC (permalink / raw)
To: Markus Blöchl, Przemek Kitszel, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: Richard Cochran, Markus Blöchl, intel-wired-lan, netdev,
linux-kernel
On 11/19/2025 8:09 AM, Markus Blöchl wrote:
> 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: stable@vger.kernel.org
> Signed-off-by: Markus Blöchl <markus@blochl.de>
> ---
...
> --- a/drivers/net/ethernet/intel/i40e/i40e_ptp.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_ptp.c
> @@ -847,6 +847,65 @@ 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 the link speed.
Can you make this 'Return:' to conform with kdoc expectations?
> + **/
> +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;
> + 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;
I believe operators are supposed to end the line rather than start a new
one.
> + 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);
> +
> + prtmac_pcs_linksta = (prtmac_pcs_linksta
> + & I40E_PRTMAC_PCS_LINK_STATUS1_LINK_SPEED_MASK)
> + >> I40E_PRTMAC_PCS_LINK_STATUS1_LINK_SPEED_SHIFT;
Same operator comment. Also, indentation looks off here.
Thanks,
Tony
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] i40e: fix ptp time increment while link is down
2025-11-25 22:48 ` Tony Nguyen
@ 2025-11-30 14:28 ` Markus Blöchl
0 siblings, 0 replies; 4+ messages in thread
From: Markus Blöchl @ 2025-11-30 14:28 UTC (permalink / raw)
To: Tony Nguyen
Cc: Przemek Kitszel, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Richard Cochran, Markus Blöchl,
intel-wired-lan, netdev, linux-kernel
On Tue, Nov 25, 2025 at 02:48:20PM -0800, Tony Nguyen wrote:
>
>
> On 11/19/2025 8:09 AM, Markus Blöchl wrote:
> > 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: stable@vger.kernel.org
> > Signed-off-by: Markus Blöchl <markus@blochl.de>
> > ---
>
> ...
>
> > --- a/drivers/net/ethernet/intel/i40e/i40e_ptp.c
> > +++ b/drivers/net/ethernet/intel/i40e/i40e_ptp.c
> > @@ -847,6 +847,65 @@ 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 the link speed.
>
> Can you make this 'Return:' to conform with kdoc expectations?
>
> > + **/
> > +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;
> > + 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;
>
> I believe operators are supposed to end the line rather than start a new
> one.
>
> > + 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);
> > +
> > + prtmac_pcs_linksta = (prtmac_pcs_linksta
> > + & I40E_PRTMAC_PCS_LINK_STATUS1_LINK_SPEED_MASK)
> > + >> I40E_PRTMAC_PCS_LINK_STATUS1_LINK_SPEED_SHIFT;
>
> Same operator comment. Also, indentation looks off here.
>
> Thanks,
> Tony
>
Thanks for the close look, Tony.
Will be fixed in v2.
I needed a reason to reroll anyway, since I forgot to base this on
net...
--
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-11-30 14:36 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-11-19 16:09 [PATCH] i40e: fix ptp time increment while link is down Markus Blöchl
2025-11-24 21:57 ` [Intel-wired-lan] " Jacob Keller
2025-11-25 22:48 ` Tony Nguyen
2025-11-30 14:28 ` Markus Blöchl
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®