Message-ID: <PH0PR11MB4904E69D5795697EEEA32D249497A@PH0PR11MB4904.namprd11.prod.outlook.com>
Date: Thu, 22 Jan 2026 11:44:05 +0000
From: "Korba, Przemyslaw" <przemyslaw.korba@...el.com>
To: "Keller, Jacob E" <jacob.e.keller@...el.com>, Intel Wired LAN
	<intel-wired-lan@...ts.osuosl.org>, "netdev@...r.kernel.org"
	<netdev@...r.kernel.org>, "Loktionov, Aleksandr"
	<aleksandr.loktionov@...el.com>, "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,
> ---

...

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

Reviewed-by: Przemyslaw Korba <przemyslaw.korba@...el.com>

Powered by blists - more mailing lists