[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <SJ2PR11MB845273963D6C04DCDB222FF19B17A@SJ2PR11MB8452.namprd11.prod.outlook.com>
Date: Wed, 17 Sep 2025 13:10:34 +0000
From: "Kubalewski, Arkadiusz" <arkadiusz.kubalewski@...el.com>
To: "Vecera, Ivan" <ivecera@...hat.com>, Jakub Kicinski <kuba@...nel.org>
CC: "netdev@...r.kernel.org" <netdev@...r.kernel.org>, "Kitszel, Przemyslaw"
<przemyslaw.kitszel@...el.com>, Prathosh Satish
<Prathosh.Satish@...rochip.com>, Jiri Pirko <jiri@...nulli.us>, "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
>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
>> Does frequency offset measurement for EEC DPLL would also use the same
>>value?
>
>Nope, this only affects phase offset measurement. AFAIK there is no such
>tuning knob for FFO or frequency measurement in general...
>Is it correct Prathosh?
>
>Thanks,
>Ivan
Powered by blists - more mailing lists