netdev - Re: [Intel-wired-lan] [PATCH v3 15/15] ice: change default clock source for E825-C

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <216ee17f-44e9-4ce6-8280-e68869364ee9@intel.com>
Date: Thu, 1 May 2025 15:45:15 -0700
From: Jacob Keller <jacob.e.keller@...el.com>
To: Paul Menzel <pmenzel@...gen.mpg.de>
CC: Anthony Nguyen <anthony.l.nguyen@...el.com>, <netdev@...r.kernel.org>,
	<intel-wired-lan@...ts.osuosl.org>, Przemek Kitszel
	<przemyslaw.kitszel@...el.com>, Aleksandr Loktionov
	<aleksandr.loktionov@...el.com>, Milena Olech <milena.olech@...el.com>,
	Michal Kubiak <michal.kubiak@...el.com>, Karol Kolacinski
	<karol.kolacinski@...el.com>
Subject: Re: [Intel-wired-lan] [PATCH v3 15/15] ice: change default clock
 source for E825-C



On 4/30/2025 10:49 PM, Paul Menzel wrote:
> Dear Jacob,
> 
> 
> Thank you for the patch. For the summary, I suggest the more specific:
> 
> ice: Default to clock source TIME_REF for E825-C over TCXO
> 
> or
> 
> ice: E825-C: Default to clock source TIME_REF over TCXO
> 

Sure.

> Am 01.05.25 um 00:51 schrieb Jacob Keller:
>> The driver currently defaults to the internal oscillator as the clock
>> source for E825-C hardware. While this clock source is labeled TCXO,
>> indicating a temperature compensated oscillator, this is only true for some
>> board designs. Many board designs have a less capable oscillator. The
>> E825-C hardware may also have its clock source set to the TIME_REF pin.
>> This pin is connected to the DPLL and is often a more stable clock source.
>> The choice of the internal oscillator is not suitable for all systems,
>> especially those which want to enable SyncE support.
>>
>> There is currently no interface available for users to configure the clock
>> source. Other variants of the E82x board have the clock source configured
>> in the NVM, but E825-C lacks this capability, so different board designs
>> cannot select a different default clock via firmware.
>>
>> In most setups, the TIME_REF is a suitable default clock source.
>> Additionally, we now fall back to the internal oscillator automatically if
>> the TIME_REF clock source cannot be locked.
>>
>> Change the default clock source for E825-C to TIME_REF. Longer term, a
>> proper interface (perhaps through dpll subsystem?) to introspect and
>> configure the clock source should be designed.
>>
>> Signed-off-by: Jacob Keller <jacob.e.keller@...el.com>
>> ---
>>   drivers/net/ethernet/intel/ice/ice_common.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/intel/ice/ice_common.c b/drivers/net/ethernet/intel/ice/ice_common.c
>> index e5099a3ccb854424f98c5fb1524f49bde1ca4534..bfa3f58c1104def9954073501012bb58a13e8821 100644
>> --- a/drivers/net/ethernet/intel/ice/ice_common.c
>> +++ b/drivers/net/ethernet/intel/ice/ice_common.c
>> @@ -2302,7 +2302,7 @@ ice_parse_1588_func_caps(struct ice_hw *hw, struct ice_hw_func_caps *func_p,
>>   		info->clk_src = ((number & ICE_TS_CLK_SRC_M) != 0);
>>   	} else {
>>   		info->clk_freq = ICE_TSPLL_FREQ_156_250;
>> -		info->clk_src = ICE_CLK_SRC_TCXO;
>> +		info->clk_src = ICE_CLK_SRC_TIME_REF;
>>   	}
>>   
>>   	if (info->clk_freq < NUM_ICE_TSPLL_FREQ) {
> 
> As there are now Linux kernels configured with different clock sources, 
> and there is apparently no way to introspect it from a running system, 
> does it make sense to log it?
> 

This is about the source for the internal PHC. Generally, this is an
internal oscillator known as TXCO. However, the device can also
optionally be configured to use a pin called TIME_REF. This pin could be
connected to a number of things. Some boards have it disconnected, but
it is usually connected to the DPLL on the chip.

We do log which clock source is enable, albeit with dev_dbg, so its
disabled by default. This happens when we call ice_tspll_log_cfg() at
the end of programming the Clock Generation Unit. This message logs the
actually programmed clock source, frequency, etc. This is done because
we fall back to TXCO if the CGU is unable to lock onto TIME_REF signal
(for example if its disconnected).

I believe we have plans to figure out an appropriate interface (maybe
something in the DPLL subsystem?) to both introspect and control this,
but design for this is still ongoing.

> 
> Kind regards,
> 
> Paul
>