| 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
| ||
|
Message-ID: <timzeiuivkgvdzmyafp752acgfkieuqhivcab55x24ow7apovp@4lsq6esrrusg>
Date: Thu, 8 May 2025 16:26:21 +0200
From: Jiri Pirko <jiri@...nulli.us>
To: Arkadiusz Kubalewski <arkadiusz.kubalewski@...el.com>
Cc: donald.hunter@...il.com, kuba@...nel.org, davem@...emloft.net,
edumazet@...gle.com, pabeni@...hat.com, horms@...nel.org, vadim.fedorenko@...ux.dev,
anthony.l.nguyen@...el.com, przemyslaw.kitszel@...el.com, andrew+netdev@...n.ch,
saeedm@...dia.com, leon@...nel.org, tariqt@...dia.com, jonathan.lemon@...il.com,
richardcochran@...il.com, aleksandr.loktionov@...el.com, milena.olech@...el.com,
netdev@...r.kernel.org, linux-kernel@...r.kernel.org, intel-wired-lan@...ts.osuosl.org,
linux-rdma@...r.kernel.org
Subject: Re: [PATCH net-next v3 1/3] dpll: add phase-offset-monitor feature
to netlink spec
Thu, May 08, 2025 at 02:21:26PM +0200, arkadiusz.kubalewski@...el.com wrote:
>Add enum dpll_feature_state for control over features.
>
>Add dpll device level attribute:
>DPLL_A_PHASE_OFFSET_MONITOR - to allow control over a phase offset monitor
>feature. Attribute is present and shall return current state of a feature
>(enum dpll_feature_state), if the device driver provides such capability,
>otherwie attribute shall not be present.
>
>Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@...el.com>
>Reviewed-by: Milena Olech <milena.olech@...el.com>
>Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@...el.com>
>---
>v3:
>- replace feature flags and capabilities with per feature attribute
> approach,
>- add dpll documentation for phase-offset-monitor feature.
>---
> Documentation/driver-api/dpll.rst | 16 ++++++++++++++++
> Documentation/netlink/specs/dpll.yaml | 24 ++++++++++++++++++++++++
> drivers/dpll/dpll_nl.c | 5 +++--
> include/uapi/linux/dpll.h | 12 ++++++++++++
> 4 files changed, 55 insertions(+), 2 deletions(-)
>
>diff --git a/Documentation/driver-api/dpll.rst b/Documentation/driver-api/dpll.rst
>index e6855cd37e85..04efb425b411 100644
>--- a/Documentation/driver-api/dpll.rst
>+++ b/Documentation/driver-api/dpll.rst
>@@ -214,6 +214,22 @@ offset values are fractional with 3-digit decimal places and shell be
> divided with ``DPLL_PIN_PHASE_OFFSET_DIVIDER`` to get integer part and
> modulo divided to get fractional part.
>
>+Phase offset monitor
>+====================
>+
>+Phase offset measurement is typically performed against the current active
>+source. However, some DPLL (Digital Phase-Locked Loop) devices may offer
>+the capability to monitor phase offsets across all available inputs.
>+The attribute and current feature state shall be included in the response
>+message of the ``DPLL_CMD_DEVICE_GET`` command for supported DPLL devices.
>+In such cases, users can also control the feature using the
>+``DPLL_CMD_DEVICE_SET`` command by setting the ``enum dpll_feature_state``
>+values for the attribute.
>+
>+ =============================== ========================
>+ ``DPLL_A_PHASE_OFFSET_MONITOR`` attr state of a feature
>+ =============================== ========================
>+
> Embedded SYNC
> =============
>
>diff --git a/Documentation/netlink/specs/dpll.yaml b/Documentation/netlink/specs/dpll.yaml
>index 8feefeae5376..e9774678b3f3 100644
>--- a/Documentation/netlink/specs/dpll.yaml
>+++ b/Documentation/netlink/specs/dpll.yaml
>@@ -240,6 +240,20 @@ definitions:
> integer part of a measured phase offset value.
> Value of (DPLL_A_PHASE_OFFSET % DPLL_PHASE_OFFSET_DIVIDER) is a
> fractional part of a measured phase offset value.
>+ -
>+ type: enum
>+ name: feature-state
>+ doc: |
>+ Allow control (enable/disable) and status checking over features.
>+ entries:
>+ -
>+ name: disable
>+ doc: |
>+ feature shall be disabled
>+ -
>+ name: enable
>+ doc: |
>+ feature shall be enabled
Is it necessary to introduce an enum for simple bool?
I mean, we used to handle this by U8 attr with 0/1 value. Idk what's the
usual way carry boolean values to do this these days, but enum looks
like overkill.
>
> attribute-sets:
> -
>@@ -293,6 +307,14 @@ attribute-sets:
> be put to message multiple times to indicate possible parallel
> quality levels (e.g. one specified by ITU option 1 and another
> one specified by option 2).
>+ -
>+ name: phase-offset-monitor
>+ type: u32
>+ enum: feature-state
>+ doc: Receive or request state of phase offset monitor feature.
>+ If enabled, dpll device shall monitor and notify all currently
>+ available inputs for changes of their phase offset against the
>+ dpll device.
> -
> name: pin
> enum-name: dpll_a_pin
>@@ -483,6 +505,7 @@ operations:
> - temp
> - clock-id
> - type
>+ - phase-offset-monitor
>
> dump:
> reply: *dev-attrs
>@@ -499,6 +522,7 @@ operations:
> request:
> attributes:
> - id
>+ - phase-offset-monitor
> -
> name: device-create-ntf
> doc: Notification about device appearing
>diff --git a/drivers/dpll/dpll_nl.c b/drivers/dpll/dpll_nl.c
>index fe9b6893d261..8de90310c3be 100644
>--- a/drivers/dpll/dpll_nl.c
>+++ b/drivers/dpll/dpll_nl.c
>@@ -37,8 +37,9 @@ static const struct nla_policy dpll_device_get_nl_policy[DPLL_A_ID + 1] = {
> };
>
> /* DPLL_CMD_DEVICE_SET - do */
>-static const struct nla_policy dpll_device_set_nl_policy[DPLL_A_ID + 1] = {
>+static const struct nla_policy dpll_device_set_nl_policy[DPLL_A_PHASE_OFFSET_MONITOR + 1] = {
> [DPLL_A_ID] = { .type = NLA_U32, },
>+ [DPLL_A_PHASE_OFFSET_MONITOR] = NLA_POLICY_MAX(NLA_U32, 1),
> };
>
> /* DPLL_CMD_PIN_ID_GET - do */
>@@ -105,7 +106,7 @@ static const struct genl_split_ops dpll_nl_ops[] = {
> .doit = dpll_nl_device_set_doit,
> .post_doit = dpll_post_doit,
> .policy = dpll_device_set_nl_policy,
>- .maxattr = DPLL_A_ID,
>+ .maxattr = DPLL_A_PHASE_OFFSET_MONITOR,
> .flags = GENL_ADMIN_PERM | GENL_CMD_CAP_DO,
> },
> {
>diff --git a/include/uapi/linux/dpll.h b/include/uapi/linux/dpll.h
>index bf97d4b6d51f..349e1b3ca1ae 100644
>--- a/include/uapi/linux/dpll.h
>+++ b/include/uapi/linux/dpll.h
>@@ -192,6 +192,17 @@ enum dpll_pin_capabilities {
>
> #define DPLL_PHASE_OFFSET_DIVIDER 1000
>
>+/**
>+ * enum dpll_feature_state - Allow control (enable/disable) and status checking
>+ * over features.
>+ * @DPLL_FEATURE_STATE_DISABLE: feature shall be disabled
>+ * @DPLL_FEATURE_STATE_ENABLE: feature shall be enabled
>+ */
>+enum dpll_feature_state {
>+ DPLL_FEATURE_STATE_DISABLE,
>+ DPLL_FEATURE_STATE_ENABLE,
>+};
>+
> enum dpll_a {
> DPLL_A_ID = 1,
> DPLL_A_MODULE_NAME,
>@@ -204,6 +215,7 @@ enum dpll_a {
> DPLL_A_TYPE,
> DPLL_A_LOCK_STATUS_ERROR,
> DPLL_A_CLOCK_QUALITY_LEVEL,
>+ DPLL_A_PHASE_OFFSET_MONITOR,
>
> __DPLL_A_MAX,
> DPLL_A_MAX = (__DPLL_A_MAX - 1)
>--
>2.38.1
>
Powered by blists - more mailing lists