[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <b9be10d5-16de-4ad5-80eb-df28c5b5ae29@redhat.com>
Date: Wed, 10 Sep 2025 15:30:06 +0200
From: Ivan Vecera <ivecera@...hat.com>
To: Vadim Fedorenko <vadim.fedorenko@...ux.dev>, netdev@...r.kernel.org
Cc: 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 2:34 odp., Vadim Fedorenko wrote:
> On 10.09.2025 11:32, 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>
>
> [...]
>
>> +static int
>> +zl3073x_devlink_param_phase_avg_factor_set(struct devlink *devlink,
>> u32 id,
>> + struct devlink_param_gset_ctx *ctx,
>> + struct netlink_ext_ack *extack)
>> +{
>> + struct zl3073x_dev *zldev = devlink_priv(devlink);
>> + u8 avg_factor, dpll_meas_ctrl;
>> + int rc;
>> +
>> + /* Read DPLL phase measurement control register */
>> + rc = zl3073x_read_u8(zldev, ZL_REG_DPLL_MEAS_CTRL, &dpll_meas_ctrl);
>> + if (rc)
>> + return rc;
>> +
>> + /* Convert requested factor to register value */
>> + if (ctx->val.vu8 < 15)
>> + avg_factor = ctx->val.vu8 + 1;
>> + else
>> + avg_factor = 0;
>> +
>
> This looks like avg_factor = (ctx->val.vu8 + 1) & 0x0f;
> The same logic can be applied for get() function assuming we are aware of
> unsigned roll-over...
Yes, I know about this trick but I wanted to use more readable code and
leave potential optimization to the compiler.
Ivan
Powered by blists - more mailing lists