netdev - Re: [PATCH net-next v2] dpll: zl3073x: Allow to use custom phase measure averaging factor

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: <cb294591-7d23-42fd-901a-3466b586f437@linux.dev>
Date: Wed, 17 Sep 2025 18:01:04 +0100
From: Vadim Fedorenko <vadim.fedorenko@...ux.dev>
To: Ivan Vecera <ivecera@...hat.com>,
 "Kubalewski, Arkadiusz" <arkadiusz.kubalewski@...el.com>,
 Jakub Kicinski <kuba@...nel.org>, Jiri Pirko <jiri@...nulli.us>
Cc: "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
 "Kitszel, Przemyslaw" <przemyslaw.kitszel@...el.com>,
 Prathosh Satish <Prathosh.Satish@...rochip.com>,
 "David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Paolo Abeni <pabeni@...hat.com>, Simon Horman <horms@...nel.org>,
 Jonathan Corbet <corbet@....net>,
 "open list:DOCUMENTATION" <linux-doc@...r.kernel.org>,
 open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH net-next v2] dpll: zl3073x: Allow to use custom phase
 measure averaging factor

On 17/09/2025 15:55, Ivan Vecera wrote:
> On 17. 09. 25 3:10 odp., Kubalewski, Arkadiusz wrote:
>>> From: Ivan Vecera <ivecera@...hat.com>
>>> Sent: Wednesday, September 17, 2025 2:18 PM
>>>
>>> On 17. 09. 25 1:26 odp., Kubalewski, Arkadiusz wrote:
>>>>> From: Jakub Kicinski <kuba@...nel.org>
>>>>> Sent: Tuesday, September 16, 2025 1:47 AM
>>>>>
>>>>> cc: Arkadiusz
>>>>>
>>>>> On Thu, 11 Sep 2025 09:23:01 +0200 Ivan Vecera wrote:
>>>>>> The DPLL phase measurement block uses an exponential moving average,
>>>>>> calculated using the following equation:
>>>>>>
>>>>>>                          2^N - 1                1
>>>>>> curr_avg = prev_avg * --------- + new_val * -----
>>>>>>                            2^N                 2^N
>>>>>>
>>>>>> Where curr_avg is phase offset reported by the firmware to the 
>>>>>> driver,
>>>>>> prev_avg is previous averaged value and new_val is currently measured
>>>>>> value for particular reference.
>>>>>>
>>>>>> New measurements are taken approximately 40 Hz or at the frequency of
>>>>>> the reference (whichever is lower).
>>>>>>
>>>>>> The driver currently uses the averaging factor N=2 which prioritizes
>>>>>> a fast response time to track dynamic changes in the phase. But for
>>>>>> applications requiring a very stable and precise reading of the 
>>>>>> average
>>>>>> phase offset, and where rapid changes are not expected, a higher 
>>>>>> factor
>>>>>> would be appropriate.
>>>>>>
>>>>>> Add devlink device parameter phase_offset_avg_factor to allow a user
>>>>>> set tune the averaging factor via devlink interface.
>>>>>
>>>>> Is averaging phase offset normal for DPLL devices?
>>>>> If it is we should probably add this to the official API.
>>>>> If it isn't we should probably default to smallest possible history?
>>>>>
>>>>
>>>> AFAIK, our phase offset measurement uses similar mechanics, but the
>>>> algorithm
>>>> is embedded in the DPLL device FW and currently not user controlled.
>>>> Although it might happen that one day we would also provide such knob,
>>>> if useful for users, no plans for it now.
>>>>   From this perspective I would rather see it in dpll api, especially
>>>> this relates to the phase measurement which is already there, the value
>>>> being shared by multiple dpll devices seems HW related, but also 
>>>> seem not
>>>> a
>>>> problem, as long as a change would notify each device it relates with.
>>>
>>> What if the averaging is implemented in different HW differently? As I
>>> mentioned the Microchip HW uses exponential moving average but
>>> a different HW can do it differently.
>>>
>>
>> Yeah good point, in that case we would also need enumerate those, and 
>> the new
>> HW would have to extend the uAPI to let the user know which method is 
>> used?
>> Different methods could require different parameters?
>> But for your current case only one attribute would be enough?
>> Or maybe better to provide those together like:
>> DPLL_A_PHASE_MEASUREMENT_EMA_N
>> Then different method would have own attributes/params?
>>
>> Next question if a HW could have multiple of those methods available and
>> controlled, in which case we shall also have in mind a plan for a
>> "turned-off" value for further extensions?
>>
>> Thank you!
>> Arkadiusz
> 
> Jiri, Vadim,
> any comments / opinions?

Our HW doesn't have such options, and it doesn't looks like we will be
able to extend it this way. But I'm fine with providing UAPI if there
will other users.