linux-kernel - Re: [PATCH net-next] 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: <4277866b-67e9-4d75-a7e4-9296b898fbcb@redhat.com>
Date: Wed, 10 Sep 2025 19:31:19 +0200
From: Ivan Vecera <ivecera@...hat.com>
To: Andrew Lunn <andrew@...n.ch>
Cc: netdev@...r.kernel.org, Prathosh Satish <Prathosh.Satish@...rochip.com>,
 Jiri Pirko <jiri@...nulli.us>, "David S. Miller" <davem@...emloft.net>,
 Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
 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] dpll: zl3073x: Allow to use custom phase measure
 averaging factor



On 10. 09. 25 7:06 odp., Andrew Lunn wrote:
> On Wed, Sep 10, 2025 at 06:50:47PM +0200, Ivan Vecera wrote:
>> On 10. 09. 25 6:13 odp., Andrew Lunn wrote:
>>> On Wed, Sep 10, 2025 at 12:32:21PM +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.
>>>>
>>>> Tested-by: Prathosh Satish <Prathosh.Satish@...rochip.com>
>>>> Signed-off-by: Ivan Vecera <ivecera@...hat.com>
>>>> ---
>>>>    Documentation/networking/devlink/zl3073x.rst |  4 ++
>>>>    drivers/dpll/zl3073x/core.c                  |  6 +-
>>>>    drivers/dpll/zl3073x/core.h                  |  8 ++-
>>>>    drivers/dpll/zl3073x/devlink.c               | 67 ++++++++++++++++++++
>>>>    4 files changed, 82 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/Documentation/networking/devlink/zl3073x.rst b/Documentation/networking/devlink/zl3073x.rst
>>>> index 4b6cfaf386433..ddd159e39e616 100644
>>>> --- a/Documentation/networking/devlink/zl3073x.rst
>>>> +++ b/Documentation/networking/devlink/zl3073x.rst
>>>> @@ -20,6 +20,10 @@ Parameters
>>>>         - driverinit
>>>>         - Set the clock ID that is used by the driver for registering DPLL devices
>>>>           and pins.
>>>> +   * - ``phase_offset_avg_factor``
>>>> +     - runtime
>>>> +     - Set the factor for the exponential moving average used by DPLL phase
>>>> +       measurement block. The value has to be in range <0, 15>.
>>>
>>> Maybe put the text in the commit message here as well?
>>
>> Do you mean to put the equation and details from commit message here?
>> This is pretty long.
> 
> So what if it is long? At the moment, it is hiding in the commit
> message. It is not easy to find, you effectively need to be a kernel
> developer to find it. If it is in the documentation of the device, it
> will be much easier to find and understand what this knob actually
> does.
> 

Agree. I will describe it in more detail in the documentation.
+ optimization proposed by Vadim.

Thanks for the review.

Ivan