[Intel-wired-lan] [PATCH iwl-next v1 1/1] e1000e: introduce private flag to override XTAL clock frequency

[Lifshits, Vitaly]

On 11/27/2025 9:01 AM, Loktionov, Aleksandr wrote:
> 
> 
>> -----Original Message-----
>> From: Intel-wired-lan <intel-wired-lan-bounces at osuosl.org> On Behalf Of
>> Vitaly Lifshits
>> Sent: Thursday, November 27, 2025 5:31 AM
>> To: intel-wired-lan at osuosl.org; netdev at vger.kernel.org;
>> andrew+netdev at lunn.ch; horms at kernel.org; kuba at kernel.org;
>> edumazet at google.com; davem at davemloft.net; pabeni at redhat.com
>> Cc: Lifshits, Vitaly <vitaly.lifshits at intel.com>
>> Subject: [Intel-wired-lan] [PATCH iwl-next v1 1/1] e1000e: introduce private
>> flag to override XTAL clock frequency
>>
>> On some TGP and ADP systems, the hardware XTAL clock is incorrectly set to
> Please expand platform abbreviations ("TGP (Tiger Lake PCH)", "ADP (Alder Lake PCH)") and add minimal test context (kernel ver., device PCI ID, FW, phc_ctl version, and whether CLOCK_REALTIME vs PHC was compared).
> 

Thanks, I'll address this in v2.

>> 24MHz instead of the expected 38.4MHz, causing PTP timer inaccuracies. Since
>> affected systems cannot be reliably detected, introduce an ethtool private
>> flag that allows user-space to override the XTAL clock frequency.
>>
>> Tested on an affected system using the phc_ctl tool:
>>
>> Without the flag:
>>    sudo phc_ctl enp0s31f6 set 0.0 wait 10 get
>>    phc_ctl[...] clock time is 16.000541250 (expected ~10s)
>>
>> With the flag:
>>    sudo phc_ctl enp0s31f6 set 0.0 wait 10 get
>>    phc_ctl[...] clock time is 9.984407212 (expected ~10s)
>>
> If the XTAL override is applied via a devlink driverinit param, this reset is expected and should be documented.
> If it remains a runtime privflag, I think you should explain why a reset is required.
> I'd recommend documenting the reset requirement and user impact (link flap, timestamp discontinuity).
> /* Add a short comment near the reset logic in code and
>     Documentation/networking/devlink/e1000e.rst (if converted to devlink param)
> Or Documentation/networking/ethtool-netlink.rst (if kept as privflag) */
> 

Though your comment might become irrelevant if Andrew's detection 
approach works, I'll still clarify the reset behavior. e1000e is a 
legacy driver and does not support devlink, so the override remains a 
runtime privflag. A reset is currently required to reconfigure PTP 
parameters with the new timinca value.


>> Signed-off-by: Vitaly Lifshits <vitaly.lifshits at intel.com>
>> ---
>>   drivers/net/ethernet/intel/e1000e/e1000.h   |  7 ++--
>>   drivers/net/ethernet/intel/e1000e/ethtool.c | 39 ++++++++++-----------
>> drivers/net/ethernet/intel/e1000e/ich8lan.c |  4 +--
>> drivers/net/ethernet/intel/e1000e/netdev.c  | 18 +++++++---
>>   4 files changed, 39 insertions(+), 29 deletions(-)
>>
> 
> ...
> 
>> --
>> 2.34.1
>