Message-ID: <PH0PR11MB75221A4DF00325021940DB8BA091A@PH0PR11MB7522.namprd11.prod.outlook.com>
Date: Wed, 28 Jan 2026 19:31:06 +0000
From: "Mekala, SunithaX D" <sunithax.d.mekala@...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>, "Loktionov, Aleksandr"
	<aleksandr.loktionov@...el.com>, "Nguyen, Anthony L"
	<anthony.l.nguyen@...el.com>
CC: "Keller, Jacob E" <jacob.e.keller@...el.com>
Subject: RE: [Intel-wired-lan] [PATCH iwl-net] ice: PTP: fix missing
 timestamps on E825 hardware

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@...osl.org> On Behalf Of Jacob Keller
> Sent: Wednesday, January 21, 2026 10:44 AM
> 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: [Intel-wired-lan] [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(-)

Tested-by: Sunitha Mekala <sunithax.d.mekala@...el.com> (A Contingent worker at Intel)

Powered by blists - more mailing lists