Message-ID: <IA3PR11MB8986984ABB25D6D34B0693A6E597A@IA3PR11MB8986.namprd11.prod.outlook.com>
Date: Thu, 22 Jan 2026 11:24:57 +0000
From: "Loktionov, Aleksandr" <aleksandr.loktionov@...el.com>
To: "Keller, Jacob E" <jacob.e.keller@...el.com>, Intel Wired LAN
	<intel-wired-lan@...ts.osuosl.org>, "Korba, Przemyslaw"
	<przemyslaw.korba@...el.com>, "netdev@...r.kernel.org"
	<netdev@...r.kernel.org>, "Nguyen, Anthony L" <anthony.l.nguyen@...el.com>
Subject: RE: [PATCH iwl-net] ice: PTP: fix missing timestamps on E825 hardware



> -----Original Message-----
> From: Keller, Jacob E <jacob.e.keller@...el.com>
> Sent: Wednesday, January 21, 2026 7:44 PM
> To: Intel Wired LAN <intel-wired-lan@...ts.osuosl.org>; Korba,
> Przemyslaw <przemyslaw.korba@...el.com>; netdev@...r.kernel.org;
> Loktionov, Aleksandr <aleksandr.loktionov@...el.com>; Nguyen, Anthony
> L <anthony.l.nguyen@...el.com>
> Cc: Keller, Jacob E <jacob.e.keller@...el.com>
> Subject: [PATCH iwl-net] ice: PTP: fix missing timestamps on E825
> hardware
> 
> The E825 hardware currently has each PF handle the PFINT_TSYN_TX cause
> of the miscellaneous OICR interrupt vector. The actual interrupt cause
> underlying this is shared by all ports on the same quad:
> 
>   ┌─────────────────────────────────┐
>   │                                 │
>   │   ┌────┐ ┌────┐ ┌────┐ ┌────┐   │
>   │   │PF 0│ │PF 1│ │PF 2│ │PF 3│   │
>   │   └────┘ └────┘ └────┘ └────┘   │
>   │                                 │
>   └────────────────▲────────────────┘
>                    │
>                    │
>   ┌────────────────┼────────────────┐
>   │             PHY QUAD            │
>   └───▲────────▲────────▲────────▲──┘
>       │        │        │        │
>   ┌───┼──┐ ┌───┴──┐ ┌───┼──┐ ┌───┼──┐
>   │Port 0│ │Port 1│ │Port 2│ │Port 3│
>   └──────┘ └──────┘ └──────┘ └──────┘
> 
> If multiple PFs issue Tx timestamp requests near simultaneously, it is
> possible that the correct PF will not be interrupted and will miss its
> timestamp. Understanding why is somewhat complex.
> 
> Consider the following sequence of events:
> 
>   CPU 0:
>   Send Tx packet on PF 0
>   ...
>   PF 0 enqueues packet with Tx request          CPU 1, PF1:
>   ...                                           Send Tx packet on PF1
>   ...                                           PF 1 enqueues packet
> with Tx request
> 
>   HW:
>   PHY Port 0 sends packet
>   PHY raises Tx timestamp event interrupt
>   MAC raises each PF interrupt
> 
>   CPU 0, PF0:                                   CPU 1, PF1:
>   ice_misc_intr() checks for Tx timestamps      ice_misc_intr() checks
> for Tx timestamp
>   Sees packet ready bit set                     Sees nothing available
>   ...                                           Exits
>   ...
>   ...
>   HW:
>   PHY port 1 sends packet
>   PHY interrupt ignored because not all packet timestamps read yet.
>   ...
>   Read timestamp, report to stack
> 
> Because the interrupt event is shared for all ports on the same quad,
> the PHY will not raise a new interrupt for any PF until all timestamps
> are read.
> 
> In the example above, the second timestamp comes in for port 1 before
> the timestamp from port 0 is read. At this point, there is no longer
> an interrupt thread running that will read the timestamps, because
> each PF has checked and found that there was no work to do.
> Applications such as ptp4l will timeout after waiting a few
> milliseconds. Eventually, the watchdog service task will re-check for
> all quads and notice that there are outstanding timestamps, and issue
> a software interrupt to recover. However, by this point it is far too
> late, and applications have already failed.
> 
> All of this occurs because of the underlying hardware behavior. The
> PHY cannot raise a new interrupt signal until all outstanding
> timestamps have been read.
> 
> As a first step to fix this, switch the E825C hardware to the
> ICE_PTP_TX_INTERRUPT_ALL mode. In this mode, only the clock owner PF
> will respond to the PFINT_TSYN_TX cause. Other PFs disable this cause
> and will not wake. In this mode, the clock owner will iterate over all
> ports and handle timestamps for each connected port.
> 
> This matches the E822 behavior, and is a necessary but insufficient
> step to resolve the missing timestamps.
> 
> Even with use of the ICE_PTP_TX_INTERRUPT_ALL mode, we still sometimes
> miss a timestamp event. The ice_ptp_tx_tstamp_owner() does re-check
> the ready bitmap, but does so before re-enabling the OICR interrupt
> vector. It also only checks the ready bitmap, but not the software Tx
> timestamp tracker.
> 
> To avoid risk of losing a timestamp, refactor the logic to check both
> the software Tx timestamp tracker bitmap *and* the hardware ready
> bitmap.
> Additionally, do this outside of ice_ptp_process_ts() after we have
> already re-enabled the OICR interrupt.
> 
> Remove the checks from the ice_ptp_tx_tstamp(),
> ice_ptp_tx_tstamp_owner(), and the ice_ptp_process_ts() functions.
> This results in ice_ptp_tx_tstamp() being nothing more than a wrapper
> around ice_ptp_process_tx_tstamp() so we can remove it.
> 
> Add the ice_ptp_tx_tstamps_pending() function which returns a boolean
> indicating if there are any pending Tx timestamps. First, check the
> software timestamp tracker bitmap. In ICE_PTP_TX_INTERRUPT_ALL mode,
> check
> *all* ports software trackers. If a tracker has outstanding timestamp
> requests, return true. Additionally, check the PHY ready bitmap to
> confirm if the PHY indicates any outstanding timestamps.
> 
> In the ice_misc_thread_fn(), call ice_ptp_tx_tstamps_pending() just
> before returning from the IRQ thread handler. If it returns true,
> write to PFINT_OICR to trigger a PFINT_OICR_TSYN_TX_M software
> interrupt. This will force the handler to interrupt again and complete
> the work even if the PHY hardware did not interrupt for any reason.
> 
> This results in the following new flow for handling Tx timestamps:
> 
> 1) send Tx packet
> 2) PHY captures timestamp
> 3) PHY triggers MAC interrupt
> 4) clock owner executes ice_misc_intr() with PFINT_OICR_TSYN_TX flag
> set
> 5) ice_ptp_ts_irq() returns IRQ_WAKE_THREAD
> 7) The interrupt thread wakes up and kernel calls
> ice_misc_intr_thread_fn()
> 8) ice_ptp_process_ts() is called to handle any outstanding timestamps
> 9) ice_irq_dynamic_ena() is called to re-enable the OICR hardware
> interrupt
>    cause
> 10) ice_ptp_tx_tstamps_pending() is called to check if we missed any
> more
>     outstanding timestamps, checking both software and hardware
> indicators.
> 
> With this change, it should no longer be possible for new timestamps
> to come in such a way that we lose an interrupt. If a timestamp comes
> in before the ice_ptp_tx_tstamps_pending() call, it will be noticed by
> at least one of the software bitmap check or the hardware bitmap
> check. If the timestamp comes in *after* this check, it should cause a
> timestamp interrupt as we have already read all timestamps from the
> PHY and the OICR vector has been re-enabled.
> 
> Fixes: 7cab44f1c35f ("ice: Introduce ETH56G PHY model for E825C
> products")
> Signed-off-by: Jacob Keller <jacob.e.keller@...el.com>
> ---
>  drivers/net/ethernet/intel/ice/ice_ptp.h  |  13 ++-
> drivers/net/ethernet/intel/ice/ice_main.c |  20 ++--
> drivers/net/ethernet/intel/ice/ice_ptp.c  | 148 +++++++++++++++++-----
> --------
>  3 files changed, 103 insertions(+), 78 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.h
> b/drivers/net/ethernet/intel/ice/ice_ptp.h
> index 27016aac4f1e..8489bd842710 100644
> --- a/drivers/net/ethernet/intel/ice/ice_ptp.h
> +++ b/drivers/net/ethernet/intel/ice/ice_ptp.h
> @@ -304,8 +304,9 @@ void ice_ptp_extts_event(struct ice_pf *pf);
>  s8 ice_ptp_request_ts(struct ice_ptp_tx *tx, struct sk_buff *skb);
> void ice_ptp_req_tx_single_tstamp(struct ice_ptp_tx *tx, u8 idx);
> void ice_ptp_complete_tx_single_tstamp(struct ice_ptp_tx *tx); -enum
> ice_tx_tstamp_work ice_ptp_process_ts(struct ice_pf *pf);
> +void ice_ptp_process_ts(struct ice_pf *pf);
>  irqreturn_t ice_ptp_ts_irq(struct ice_pf *pf);
> +bool ice_ptp_tx_tstamps_pending(struct ice_pf *pf);
>  u64 ice_ptp_read_src_clk_reg(struct ice_pf *pf,
>  			     struct ptp_system_timestamp *sts);
> 

...

> ---
> base-commit: d41f8acf7a86de68d8e1d0d5ab288ca5a003ae29
> change-id: 20260116-jk-e825c-fix-missing-timetamps-ddaa9f08833e
> 
> Best regards,
> --
> Jacob Keller <jacob.e.keller@...el.com>

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

Powered by blists - more mailing lists