[PATCH iwl-net 0/2] ice: E825 timer adjustment fixes

[Jacob Keller <jacob.e.keller@intel.com>]

Recently we've had reports of bad timer adjustment on E825. This series
fixes the issues by replacing direct timer adjustments with a struct
timecounter to maintain the phase in software instead.

To motivate this, the following is an analysis of how the E825 PHY
timestamps function.

    The ice driver maintains PTP clock time in the MAC with a 96-bit
    register. Timestamps are captured in the PHY which only stores 64
    bits, only including the lower 32-bits of nanoseconds. The driver
    performs clock adjustments using a hardware timer command mechanism,
    where the clock adjustment is programmed into shadow registers in
    both the MAC and PHY, and then a timer command is initiated to
    synchronously update the timers.

    Since the PHY only maintains the lower 32 bits of nanoseconds,
    adjustments are also limited such that the maximum atomic adjustment
    is ~2.1 seconds (2^31-1 nanoseconds).

    For large offsets, the driver uses a non-atomic flow where the
    current time is read, an adjustment is applied in software, and then
    written back. This necessarily loses some elapsed time, which
    applications must deal with. For particular servo configurations,
    where the parameters are tuned for a very slow servo, this error can
    take a long time to finish correction.

    On E825, the adjustment in the PHY does not always work reliably.
    The reason for this is subtle, and is a quirk of how the timestamps
    are actually generated. The timestamps for Tx and Rx packets come
    from the PCS timer rather than directly from the PHY timer. The PCS
    timer is derived from internal PHY signaling.

    To keep the PCS timer and PHY timer in sync, every few cycles
    (testing seems to indicate every 8 cycles) the PHY timer is copied
    to the PCS timer. On other cycles, the PCS timer is incremented by a
    value determined from the PCS clock rate.

    Crucially, the copy of the PHY timer is skipped if the current value
    of the PHY timer is lower than the PCS timer (*and* on this cycle
    the regular PCS clock increment is also skipped).

    During normal operation, the PCS timer will tick from its own source
    for 7 ticks then copy the PCS timer. If the PCS timer is slightly
    ahead, then PHY timer update is skipped, and the PCS timer skips an
    update every 8 clock cycles until it is back in alignment with the
    PHY timer.

    Unfortunately, this hardware logic did not take into account manual
    adjustments to the PHY. If the PHY timer is adjusted backwards, (or
    if a forward adjustment causes the PHY timer to overflow), the PHY
    to PCS sync will not occur again until the PCS timer wraps back
    around to zero. In the worst case, an adjustment could land the PHY
    timer to just below the PCS timer just after it wraps around. PHY to
    PCS syncs would stop for ~3.6 seconds.

    During this time, the Tx and Rx timestamps will read from the PCS
    timer. These timestamps will be fed into the timestamp extension
    algorithm which will produce incoherent results, and data will be
    fed into the PTP application servo. During the invalid window,
    faulty timestamps will cause the servo to steer incorrectly, likely
    away from the true target time. After the PCS timer synchronizes
    again, the timestamps will return to normal. However, the servo may
    have steered the clock incorrectly. In default ptp4l configurations,
    direct clock adjustment is only performed once during startup. This
    forces the application to use frequency slewing to correct any
    remaining offset. With slow servo parameters, this can take a *very*
    long time. In some setups, we had reports of taking several minutes
    to nearly an hour before stabilizing, and in some configurations the
    servo was unable to ever recover.

    Note that adjustments work properly on the MAC timer, and indeed on
    the first-layer PHY timer value that the driver can read over the
    sideband queue. Only the PCS timer (which is inaccessible to the
    driver) fails to adjust. This made tracking the precise origin of
    the failure difficult.

Testing shows that pretty much all "negative" adjustments and about half
of the positive adjustments can trigger this failure. It is also
difficult to see without using extremely slow servo parameters. With a
servo tuned for fast reaction, the incorrect timestamps will cause some
difficulty in the first adjustment, but the servo will quickly correct
and recover within a few seconds at most.

I used the following P/I servo parameters in my testing:

    pi_proportional_const 0.60
    pi_integral_const 0.001
    pi_proportional_scale 0.0
    pi_proportional_exponent -0.3
    pi_proportional_norm_max 0.7
    pi_integral_scale 0.0
    pi_integral_exponent 0.4
    pi_integral_norm_max 0.3

In particular, the pi_integral_const will force the servo to be very
slow at reacting.

Even trying to compare the MAC timer with the Tx and Rx timestamps is
not trivial. There is no mechanism to synchronously read the MAC timer
along with a timestamp. Additionally, timestamp extension makes this even
harder to track. The MAC timer is immediately adjusted and the cached PHC
time is updated. Normally timestamps from before the adjustment should
reasonably convert, however if the adjustment is large enough the timestamp
can flip to being detected as "in the past" and in the worst cases the
resulting extended timestamps will have a second sudden jump of -4.2
seconds before the PCS timer adjustment is applied and things settle. All
of this while being fed into a servo trying to steer the clock can make it
difficult to understand what is going on.

With that background out of the way, here's a summary of the patch content:

First, the timestamp extension logic is refactored to store the cached
PHC time value inside the ice_adapter structure under a new ice_phc
structure which will be used to store timecounter data. This change
reduces the number of threads that need to perform timestamp extension
and makes introduction of timecounter simpler as it gives a natural
place for accessing the structure and simplifies modifications to the Rx
hotpath in particular.

Next, the driver is modified to allow use of a timecounter for
representing the time instead of directly adjusting the phase in
hardware. Frequency adjustments continue to be handled in hardware. For
now, only the E825 and E830 devices are switched. The legacy
implementation remains for supporting older installations of E810 and
E822 platform devices. We plan to complete migration in the future, but
felt that immediate conversion would slow down ability to validate that
all older installations will behave correctly. Since the E825 device
timer adjustment logic is completely useless, this can speed up ability
to adopt the fix. Although E830 does not appear to have the same timer
command issue, it was decided to convert both since they are the most
recent devices and undergoing the most testing and active development.

Although I would normally rate conversion to timecounter as a "feature", I
have submitted this series as a fix due to the severity of the failures
encountered on E825.We would prefer to be able to avoid delays and help get
this to critical customers soon.

This series fixes a separate issue from [1] and does not *directly* depend
on it. The fixes in that series are necessary but not sufficient to
completely eliminate PHY timer desync issues on E825.

There are some structural conflicts with the ctrl_pf RCU changes at [2]
which I have tried to integrate, so this won't be a straightforward
cherry-pick to trees without that series.

One final note, the conversion to timecounter supersedes the changes
from [3], and that patch has been removed from more recent versions of
that series. The reset partially helped, but was only applied to the
non-atomic settime interface. In particular, the settime interface was
vulnerable to the same PCS issue, but the PHY soft reset *does* clear
both the PHY and PCS timers back to 0. Performing a reset after the
adjustment will force both timers back to zero and then perform a
forward adjustment that won't trigger the failure condition. However,
attempting to add this reset to *every* timer adjustment comes with
unnecessary delay since the whole process takes a long time especially
with the PHY soft reset. It was ultimately deemed as not a proper fix
suitable for deployment.

Link: [1] https://lore.kernel.org/intel-wired-lan/20260925222824.270640-1-grzegorz.nitka@intel.com/
Link: [2] https://lore.kernel.org/intel-wired-lan/20261013091235.1423979-1-sergey.temerkhanov@intel.com/
Link: [3] https://lore.kernel.org/netdev/20261008215614.1987250-16-anthony.l.nguyen@intel.com/

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
Jacob Keller (2):
      ice: store cached PHC time in ice_adapter
      ice: use struct timecounter for E825 and E830

 drivers/net/ethernet/intel/ice/ice_adapter.h |  42 +++
 drivers/net/ethernet/intel/ice/ice_ptp.h     |  12 +-
 drivers/net/ethernet/intel/ice/ice_txrx.h    |   4 +-
 drivers/net/ethernet/intel/ice/ice_adapter.c |  10 +
 drivers/net/ethernet/intel/ice/ice_ethtool.c |   2 +-
 drivers/net/ethernet/intel/ice/ice_lib.c     |   2 +-
 drivers/net/ethernet/intel/ice/ice_ptp.c     | 381 +++++++++++++++++----------
 7 files changed, 311 insertions(+), 142 deletions(-)
---
base-commit: bfd1885cfb920d10c2739c9f152615ec26e3b045
base-tree: git://git.kernel.org/pub/scm/linux/kernel/git/tnguy/net-queue.git dev-queue
change-id: 20261022-jk-e825-e830-timecounter-06c15de02e76

Best regards,
--  
Jacob Keller <jacob.e.keller@intel.com>