netdev - Re: [Intel-wired-lan] [PATCH v2 iwl-next] ice: Add in/out PTP pin delays

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 for Android: free password hash cracker in your pocket

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

Message-ID: <IA0PR11MB838047CCFB30DDB6ED31DAA6867F2@IA0PR11MB8380.namprd11.prod.outlook.com>
Date: Wed, 9 Oct 2024 12:33:05 +0000
From: "Kolacinski, Karol" <karol.kolacinski@...el.com>
To: Paul Menzel <pmenzel@...gen.mpg.de>
CC: "intel-wired-lan@...ts.osuosl.org" <intel-wired-lan@...ts.osuosl.org>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>, "Nguyen, Anthony L"
	<anthony.l.nguyen@...el.com>, "Kitszel, Przemyslaw"
	<przemyslaw.kitszel@...el.com>
Subject: Re: [Intel-wired-lan] [PATCH v2 iwl-next] ice: Add in/out PTP pin
 delays

On 08 October 2024 13:46, Paul Menzel wrote:
> Am 08.10.24 um 13:05 schrieb Karol Kolacinski:
>> HW can have different input/output delays for each of the pins.
>> Add a field in ice_ptp_pin_desc structure to reflect that.
>
> What is the current status, that means before your patch?

Only E82X adapters have delay compensation based on TSPLL config and
E810 adapters have constant 1 ms compensation, both cases only for
output delays and the same one for all pins.

E825 adapters have different delays for SDP and other pins. Those
delays are also based on direction and input ones are different than
output ones. This is the main reason for moving delays to pin
description structure.

>> Implement external timestamp delay compensation.
>
> How is this related to the first paragraph?

"external timestamp" (EXTTS) is PTP subsystem input pin function.
HW delay compensation for input pins was not implemented before.

>> Remove existing definitions and wrappers for periodic output propagation
>> delays.
>
> How can this be tested?

It's very hard to test it, values are based on approximate calculations
of HW delays and those can be tested only with external HW made for
those types of PTP tests.

>> +             /* Add delay compensation */
>> +             pin_desc_idx = ice_ptp_find_pin_idx(pf, PTP_PF_EXTTS, chan);
>> +             if (pin_desc_idx >= 0) {
>> +                     const struct ice_ptp_pin_desc *desc;
>> +
>> +                     desc = &pf->ptp.ice_pin_desc[pin_desc_idx];
>> +                     event.timestamp -= desc->delay[0];
>>                }
>> +
>> +             event.type = PTP_CLOCK_EXTTS;
>> +             event.index = chan;
>
> You got rid of the comment `Fire event` correct?

That's correct. I think ptp_clock_event() call is enough to understand
what's happening here.

>> @@ -1767,9 +1778,9 @@ static int ice_ptp_write_perout(struct ice_hw *hw, unsigned int chan,
>>   static int ice_ptp_cfg_perout(struct ice_pf *pf, struct ptp_perout_request *rq,
>>                              int on)
>>   {
>> +     unsigned int gpio_pin, prop_delay;
>
> I’d also add the unit to the variable name.

Will do in V3!

Thanks,
Karol