netdev - 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] [thread-next>] [day] [month] [year] [list]

Message-ID: <10886c5f-1265-46ec-8caa-41bde6888905@lunn.ch>
Date: Wed, 10 Sep 2025 19:06:45 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Ivan Vecera <ivecera@...hat.com>
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 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.

	Andrew