Thu, May 08, 2025 at 02:21:26PM +0200, arkadiusz.kubalew...@intel.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.loktio...@intel.com> >Reviewed-by: Milena Olech <milena.ol...@intel.com> >Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalew...@intel.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 >