Re: [Intel-wired-lan] [PATCH net-next v3 1/3] dpll: add phase-offset-monitor feature to netlink spec
Thu, May 08, 2025 at 05:29:07PM +0200, [email protected] wrote: >>From: Jiri Pirko >>Sent: Thursday, May 8, 2025 4:26 PM >> >>Thu, May 08, 2025 at 02:21:26PM +0200, [email protected] >>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 >>>Reviewed-by: Milena Olech >>>Signed-off-by: Arkadiusz Kubalewski >>>--- >>>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. >> > >Well, yeah.. tricky.. There is no bool type in the attribute types? >Input/output pin direction or eec/pps dpll types are also 2-value-enums >we use same way.. > >Had to use something as it is better then plain 0/1, also those values >could be reused for any other feature. Okay, I don't mind. > >Thank you! >Arkadiusz > >[...]
Re: [Intel-wired-lan] [PATCH net-next v3 1/3] dpll: add phase-offset-monitor feature to netlink spec
>From: Jiri Pirko >Sent: Thursday, May 8, 2025 4:26 PM > >Thu, May 08, 2025 at 02:21:26PM +0200, [email protected] >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 >>Reviewed-by: Milena Olech >>Signed-off-by: Arkadiusz Kubalewski >>--- >>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. > Well, yeah.. tricky.. There is no bool type in the attribute types? Input/output pin direction or eec/pps dpll types are also 2-value-enums we use same way.. Had to use something as it is better then plain 0/1, also those values could be reused for any other feature. Thank you! Arkadiusz [...]
Re: [Intel-wired-lan] [PATCH net-next v3 1/3] dpll: add phase-offset-monitor feature to netlink spec
Thu, May 08, 2025 at 02:21:26PM +0200, [email protected] 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 >Reviewed-by: Milena Olech >Signed-off-by: Arkadiusz Kubalewski >--- >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
[Intel-wired-lan] [PATCH net-next v3 1/3] dpll: add phase-offset-monitor feature to netlink spec
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
Reviewed-by: Milena Olech
Signed-off-by: Arkadiusz Kubalewski
---
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
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 bf97
