[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <5b824df7-205e-4356-a33b-9937a1367517@intel.com>
Date: Tue, 25 Nov 2025 14:48:20 -0800
From: Tony Nguyen <anthony.l.nguyen@...el.com>
To: Markus Blöchl <markus@...chl.de>, Przemek Kitszel
<przemyslaw.kitszel@...el.com>, Andrew Lunn <andrew+netdev@...n.ch>, "David
S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, "Jakub
Kicinski" <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>
CC: Richard Cochran <richardcochran@...il.com>, Markus Blöchl
<markus.bloechl@...tronik.com>, <intel-wired-lan@...ts.osuosl.org>,
<netdev@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] i40e: fix ptp time increment while link is down
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@...r.kernel.org
> Signed-off-by: Markus Blöchl <markus@...chl.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
Powered by blists - more mailing lists