>-----Original Message-----
>From: Lifshits, Vitaly <[email protected]> 
>Sent: Thursday, January 8, 2026 11:40 AM
>To: Kwapulinski, Piotr <[email protected]>; 
>[email protected]
>Subject: Re: [Intel-wired-lan] [PATCH iwl-next v2 1/1] e1000e: correct TIMINCA 
>on ADP/TGP systems with wrong XTAL frequency
>
>On 1/7/2026 4:19 PM, Kwapulinski, Piotr wrote:
>>> -----Original Message-----
>>> From: Intel-wired-lan <[email protected]> On Behalf 
>>> Of Vitaly Lifshits
>>> Sent: Tuesday, January 6, 2026 11:04 AM
>>> To: [email protected]
>>> Cc: Lifshits, Vitaly <[email protected]>
>>> Subject: [Intel-wired-lan] [PATCH iwl-next v2 1/1] e1000e: correct 
>>> TIMINCA on ADP/TGP systems with wrong XTAL frequency
>>>
>>> On some Tiger Lake (TGP) and Alder Lake (ADP) platforms, the hardware 
>>> XTAL clock is incorrectly interpreted as 24 MHz instead of the actual
>>> 38.4 MHz. This causes the PHC to run significantly faster than system time, 
>>> breaking PTP synchronization.
>>>
>>> To mitigate this at runtime, measure PHC vs system time over ~1 ms 
>>> using cross-timestamps. If the PHC increment differs from system time 
>>> beyond the expected tolerance (currently >100 uSecs), reprogram 
>>> TIMINCA for the
>>> 38.4 MHz profile and reinitialize the timecounter.
>>>
>>> Tested on an affected system using phc_ctl:
>>> Without fix:
>>> sudo phc_ctl enp0s31f6 set 0.0 wait 10 get clock time: 16.000541250 
>>> (expected ~10s)
>>>
>>> With fix:
>>> sudo phc_ctl enp0s31f6 set 0.0 wait 10 get clock time: 9.984407212 
>>> (expected ~10s)
>>>
>>> Signed-off-by: Vitaly Lifshits <[email protected]>
>>> ---
>>> v2: avoid resetting the systim and rephrase commit message
>>> v1: initial version
>>> ---
>>> drivers/net/ethernet/intel/e1000e/netdev.c | 80 
>>> ++++++++++++++++++++++
>>> 1 file changed, 80 insertions(+)
>>>
>>> diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c 
>>> b/drivers/net/ethernet/intel/e1000e/netdev.c
>>> index 116f3c92b5bc..edb72054d0d9 100644
>>> --- a/drivers/net/ethernet/intel/e1000e/netdev.c
>>> +++ b/drivers/net/ethernet/intel/e1000e/netdev.c
>>> @@ -3904,6 +3904,83 @@ static void e1000_flush_desc_rings(struct 
>>> e1000_adapter *adapter)
>>>             e1000_flush_rx_ring(adapter);
>>> }
>>>
>>> +/**
>>> + * e1000e_xtal_tgp_workaround - Adjust XTAL clock based on PHC and 
>>> +system
>>> + * clock delta.
>>> + *
>>> + * Measures the time difference between the PHC (Precision Hardware
>>> +Clock)
>>> + * and the system clock over a 1 millisecond interval. If the delta
>>> + * exceeds 100 microseconds, reconfigure the XTAL clock to 38.4 MHz.
>>> + *
>>> + * @adapter: Pointer to the private adapter structure  **/ static 
>>> +void e1000e_xtal_tgp_workaround(struct e1000_adapter *adapter) {
>>> +   s64 phc_delta, sys_delta, sys_start_ns, sys_end_ns, delta_ns;
>>> +   struct ptp_system_timestamp sys_start = {}, sys_end = {};
>>> +   struct ptp_clock_info *info = &adapter->ptp_clock_info;
>>> +   struct timespec64 phc_start, phc_end;
>>> +   struct e1000_hw *hw = &adapter->hw;
>>> +   struct netlink_ext_ack extack = {};
>>> +   unsigned long flags;
>>> +   u32 timinca;
>>> +   s32 ret_val;
>>> +
>>> +   /* Capture start */
>>> +   if (info->gettimex64(info, &phc_start, &sys_start)) {
>>> +           e_dbg("PHC gettimex(start) failed\n");
>>> +           return;
>>> +   }
>>> +
>>> +   /* Small interval to measure increment */
>>> +   usleep_range(1000, 1100);
>>> +
>>> +   /* Capture end */
>>> +   if (info->gettimex64(info, &phc_end, &sys_end)) {
>>> +           e_dbg("PHC gettimex(end) failed\n");
>>> +           return;
>>> +   }
>>> +
>>> +   /* Compute deltas */
>>> +   phc_delta = timespec64_to_ns(&phc_end) -
>>> +               timespec64_to_ns(&phc_start);
>>> +
>>> +   sys_start_ns = (timespec64_to_ns(&sys_start.pre_ts) +
>>> +                   timespec64_to_ns(&sys_start.post_ts)) >> 1;
>>> +
>>> +   sys_end_ns = (timespec64_to_ns(&sys_end.pre_ts) +
>>> +                 timespec64_to_ns(&sys_end.post_ts)) >> 1;
>>> +
>>> +   sys_delta = sys_end_ns - sys_start_ns;
>>> +
>>> +   delta_ns = phc_delta - sys_delta;
>>> +   if (delta_ns > 100000) {
>>> +           e_dbg("Corrected PHC frequency: TIMINCA set for 38.4 MHz\n");
>>> +           /* Program TIMINCA for 38.4 MHz */
>>> +           timinca = (INCPERIOD_38400KHZ <<
>>> +                      E1000_TIMINCA_INCPERIOD_SHIFT) |
>>> +                     (((INCVALUE_38400KHZ <<
>>> +                        adapter->cc.shift) &
>>> +                      E1000_TIMINCA_INCVALUE_MASK));
>>> +           ew32(TIMINCA, timinca);
>>> +
>>> +           /* reset the systim ns time counter */
>>> +           spin_lock_irqsave(&adapter->systim_lock, flags);
>>> +           timecounter_init(&adapter->tc, &adapter->cc,
>>> +                            ktime_to_ns(ktime_get_real()));
>>> +           spin_unlock_irqrestore(&adapter->systim_lock, flags);
>>> +
>>> +           /* restore the previous hwtstamp configuration settings */
>>> +           ret_val = e1000e_config_hwtstamp(adapter,
>>> +                                            &adapter->hwtstamp_config,
>>> +                                            &extack);
>>> +           if (ret_val) {
>>> +                   if (extack._msg)
>>> +                           e_err("%s\n", extack._msg);
>>> +           }
>> Please use "if (cond1 && cond2)" instead.
>> Piotr
>
>As I mentioned previously when answering to Paul, this code was taken from the 
>function e1000e_systim_reset that calls the new one I introduced in this 
>patch. This is why I prefer keeping the code as is for consistency.

Please consider this a simple cleanup.
Piotr

[...]

Reply via email to