[PATCH net-next v3 0/3] dpll: add Reference SYNC feature

[PATCH net-next v3 0/3] dpll: add Reference SYNC feature

[Arkadiusz Kubalewski]

The device may support the Reference SYNC feature, which allows the
combination of two inputs into a Reference SYNC pair. In this
configuration, clock signals from both inputs are used to synchronize
the dpll device. The higher frequency signal is utilized for the loop
bandwidth of the DPLL, while the lower frequency signal is used to
syntonize the output signal of the DPLL device. This feature enables
the provision of a high-quality loop bandwidth signal from an external
source.

A capable input provides a list of inputs that can be paired to create
a Reference SYNC pair. To control this feature, the user must request a
desired state for a target pin: use ``DPLL_PIN_STATE_CONNECTED`` to
enable or ``DPLL_PIN_STATE_DISCONNECTED`` to disable the feature. Only
two pins can be bound to form a Reference SYNC pair at any given time.

Verify pins bind state/capabilities:
$ ./tools/net/ynl/pyynl/cli.py \
 --spec Documentation/netlink/specs/dpll.yaml \
 --do pin-get \
 --json '{"id":0}'
{'board-label': 'CVL-SDP22',
 'id': 0,
 [...]
 'reference-sync': [{'id': 1, 'state': 'disconnected'}],
 [...]}

Bind the pins by setting connected state between them:
$ ./tools/net/ynl/pyynl/cli.py \
 --spec Documentation/netlink/specs/dpll.yaml \
 --do pin-set \
 --json '{"id":0, "reference-sync":{"id":1, "state":"connected"}}'

Verify pins bind state:
$ ./tools/net/ynl/pyynl/cli.py \
 --spec Documentation/netlink/specs/dpll.yaml \
 --do pin-get \
 --json '{"id":0}'
{'board-label': 'CVL-SDP22',
 'id': 0,
 [...]
 'reference-sync': [{'id': 1, 'state': 'connected'}],
 [...]}

Unbind the pins by setting disconnected state between them:
$ ./tools/net/ynl/pyynl/cli.py \
 --spec Documentation/netlink/specs/dpll.yaml \
 --do pin-set \
 --json '{"id":0, "reference-sync":{"id":1, "state":"disconnected"}}'

v3:
- no change.

Arkadiusz Kubalewski (3):
  dpll: add reference-sync netlink attribute
  dpll: add reference sync get/set
  ice: add ref-sync dpll pins

 Documentation/driver-api/dpll.rst             |  25 +++
 Documentation/netlink/specs/dpll.yaml         |  19 ++
 drivers/dpll/dpll_core.c                      |  27 +++
 drivers/dpll/dpll_core.h                      |   2 +
 drivers/dpll/dpll_netlink.c                   | 188 ++++++++++++++++--
 drivers/dpll/dpll_nl.c                        |  10 +-
 drivers/dpll/dpll_nl.h                        |   1 +
 .../net/ethernet/intel/ice/ice_adminq_cmd.h   |   2 +
 drivers/net/ethernet/intel/ice/ice_dpll.c     | 186 +++++++++++++++++
 include/linux/dpll.h                          |  10 +
 include/uapi/linux/dpll.h                     |   1 +
 11 files changed, 451 insertions(+), 20 deletions(-)


base-commit: 4ff4d86f6cceb6bea583bdb230e5439655778cce
-- 
2.38.1

[PATCH net-next v3 1/3] dpll: add reference-sync netlink attribute

[Arkadiusz Kubalewski]

Add new netlink attribute to allow user space configuration of reference
sync pin pairs, where both pins are used to provide one clock signal
consisting of both: base frequency and sync signal.

Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
Reviewed-by: Milena Olech <milena.olech@intel.com>
Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
---
 v3:
- add missing maintainers.
---
Documentation/driver-api/dpll.rst     | 25 +++++++++++++++++++++++++
 Documentation/netlink/specs/dpll.yaml | 19 +++++++++++++++++++
 drivers/dpll/dpll_nl.c                | 10 ++++++++--
 drivers/dpll/dpll_nl.h                |  1 +
 include/uapi/linux/dpll.h             |  1 +
 5 files changed, 54 insertions(+), 2 deletions(-)

diff --git a/Documentation/driver-api/dpll.rst b/Documentation/driver-api/dpll.rst
index e6855cd37e85..7570890c6cd1 100644
--- a/Documentation/driver-api/dpll.rst
+++ b/Documentation/driver-api/dpll.rst
@@ -235,6 +235,31 @@ the pin.
   ``DPLL_A_PIN_ESYNC_PULSE``                pulse type of Embedded SYNC
   ========================================= =================================
 
+Reference SYNC
+==============
+
+The device may support the Reference SYNC feature, which allows the combination
+of two inputs into a Reference SYNC pair. In this configuration, clock signals
+from both inputs are used to synchronize the dpll device. The higher frequency
+signal is utilized for the loop bandwidth of the DPLL, while the lower frequency
+signal is used to syntonize the output signal of the DPLL device. This feature
+enables the provision of a high-quality loop bandwidth signal from an external
+source.
+
+A capable input provides a list of inputs that can be paired to create a
+Reference SYNC pair. To control this feature, the user must request a desired
+state for a target pin: use ``DPLL_PIN_STATE_CONNECTED`` to enable or
+``DPLL_PIN_STATE_DISCONNECTED`` to disable the feature. Only two pins can be
+bound to form a Reference SYNC pair at any given time.
+
+  ============================== ==========================================
+  ``DPLL_A_PIN_REFERENCE_SYNC``  nested attribute for providing info or
+                                 requesting configuration of the Reference
+                                 SYNC feature
+    ``DPLL_A_PIN_ID``            target pin id for Reference SYNC pair
+    ``DPLL_A_PIN_STATE``         state of Reference SYNC pair connection
+  ============================== ==========================================
+
 Configuration commands group
 ============================
 
diff --git a/Documentation/netlink/specs/dpll.yaml b/Documentation/netlink/specs/dpll.yaml
index 8feefeae5376..333b4596b36f 100644
--- a/Documentation/netlink/specs/dpll.yaml
+++ b/Documentation/netlink/specs/dpll.yaml
@@ -406,6 +406,15 @@ attribute-sets:
         doc: |
           A ratio of high to low state of a SYNC signal pulse embedded
           into base clock frequency. Value is in percents.
+      -
+        name: reference-sync
+        type: nest
+        multi-attr: true
+        nested-attributes: reference-sync
+        doc: |
+          Capable pin provides list of pins that can be bound to create a
+          reference-sync pin pair.
+
   -
     name: pin-parent-device
     subset-of: pin
@@ -436,6 +445,14 @@ attribute-sets:
         name: frequency-min
       -
         name: frequency-max
+  -
+    name: reference-sync
+    subset-of: pin
+    attributes:
+      -
+        name: id
+      -
+        name: state
 
 operations:
   enum-name: dpll_cmd
@@ -574,6 +591,7 @@ operations:
             - esync-frequency
             - esync-frequency-supported
             - esync-pulse
+            - reference-sync
 
       dump:
         request:
@@ -601,6 +619,7 @@ operations:
             - parent-pin
             - phase-adjust
             - esync-frequency
+            - reference-sync
     -
       name: pin-create-ntf
       doc: Notification about pin appearing
diff --git a/drivers/dpll/dpll_nl.c b/drivers/dpll/dpll_nl.c
index fe9b6893d261..d709a8dc304f 100644
--- a/drivers/dpll/dpll_nl.c
+++ b/drivers/dpll/dpll_nl.c
@@ -24,6 +24,11 @@ const struct nla_policy dpll_pin_parent_pin_nl_policy[DPLL_A_PIN_STATE + 1] = {
 	[DPLL_A_PIN_STATE] = NLA_POLICY_RANGE(NLA_U32, 1, 3),
 };
 
+const struct nla_policy dpll_reference_sync_nl_policy[DPLL_A_PIN_STATE + 1] = {
+	[DPLL_A_PIN_ID] = { .type = NLA_U32, },
+	[DPLL_A_PIN_STATE] = NLA_POLICY_RANGE(NLA_U32, 1, 3),
+};
+
 /* DPLL_CMD_DEVICE_ID_GET - do */
 static const struct nla_policy dpll_device_id_get_nl_policy[DPLL_A_TYPE + 1] = {
 	[DPLL_A_MODULE_NAME] = { .type = NLA_NUL_STRING, },
@@ -62,7 +67,7 @@ static const struct nla_policy dpll_pin_get_dump_nl_policy[DPLL_A_PIN_ID + 1] =
 };
 
 /* DPLL_CMD_PIN_SET - do */
-static const struct nla_policy dpll_pin_set_nl_policy[DPLL_A_PIN_ESYNC_FREQUENCY + 1] = {
+static const struct nla_policy dpll_pin_set_nl_policy[DPLL_A_PIN_REFERENCE_SYNC + 1] = {
 	[DPLL_A_PIN_ID] = { .type = NLA_U32, },
 	[DPLL_A_PIN_FREQUENCY] = { .type = NLA_U64, },
 	[DPLL_A_PIN_DIRECTION] = NLA_POLICY_RANGE(NLA_U32, 1, 2),
@@ -72,6 +77,7 @@ static const struct nla_policy dpll_pin_set_nl_policy[DPLL_A_PIN_ESYNC_FREQUENCY
 	[DPLL_A_PIN_PARENT_PIN] = NLA_POLICY_NESTED(dpll_pin_parent_pin_nl_policy),
 	[DPLL_A_PIN_PHASE_ADJUST] = { .type = NLA_S32, },
 	[DPLL_A_PIN_ESYNC_FREQUENCY] = { .type = NLA_U64, },
+	[DPLL_A_PIN_REFERENCE_SYNC] = NLA_POLICY_NESTED(dpll_reference_sync_nl_policy),
 };
 
 /* Ops table for dpll */
@@ -139,7 +145,7 @@ static const struct genl_split_ops dpll_nl_ops[] = {
 		.doit		= dpll_nl_pin_set_doit,
 		.post_doit	= dpll_pin_post_doit,
 		.policy		= dpll_pin_set_nl_policy,
-		.maxattr	= DPLL_A_PIN_ESYNC_FREQUENCY,
+		.maxattr	= DPLL_A_PIN_REFERENCE_SYNC,
 		.flags		= GENL_ADMIN_PERM | GENL_CMD_CAP_DO,
 	},
 };
diff --git a/drivers/dpll/dpll_nl.h b/drivers/dpll/dpll_nl.h
index f491262bee4f..3da10cfe9a6e 100644
--- a/drivers/dpll/dpll_nl.h
+++ b/drivers/dpll/dpll_nl.h
@@ -14,6 +14,7 @@
 /* Common nested types */
 extern const struct nla_policy dpll_pin_parent_device_nl_policy[DPLL_A_PIN_PHASE_OFFSET + 1];
 extern const struct nla_policy dpll_pin_parent_pin_nl_policy[DPLL_A_PIN_STATE + 1];
+extern const struct nla_policy dpll_reference_sync_nl_policy[DPLL_A_PIN_STATE + 1];
 
 int dpll_lock_doit(const struct genl_split_ops *ops, struct sk_buff *skb,
 		   struct genl_info *info);
diff --git a/include/uapi/linux/dpll.h b/include/uapi/linux/dpll.h
index bf97d4b6d51f..f6cb6209566c 100644
--- a/include/uapi/linux/dpll.h
+++ b/include/uapi/linux/dpll.h
@@ -237,6 +237,7 @@ enum dpll_a_pin {
 	DPLL_A_PIN_ESYNC_FREQUENCY,
 	DPLL_A_PIN_ESYNC_FREQUENCY_SUPPORTED,
 	DPLL_A_PIN_ESYNC_PULSE,
+	DPLL_A_PIN_REFERENCE_SYNC,
 
 	__DPLL_A_PIN_MAX,
 	DPLL_A_PIN_MAX = (__DPLL_A_PIN_MAX - 1)
-- 
2.38.1

[PATCH net-next v3 2/3] dpll: add reference sync get/set

[Arkadiusz Kubalewski]

Define function for reference sync pin registration and callback ops to
set/get current feature state.

Implement netlink handler to fill netlink messages with reference sync
pin configuration of capable pins (pin-get).

Implement netlink handler to call proper ops and configure reference
sync pin state (pin-set).

Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
Reviewed-by: Milena Olech <milena.olech@intel.com>
Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
---
 v3:
- fix kdoc missing ':' after argument name ref_sync_pins,
- propagate ret in dpll_pin_ref_sync_state_set().
---
 drivers/dpll/dpll_core.c    |  27 ++++++
 drivers/dpll/dpll_core.h    |   2 +
 drivers/dpll/dpll_netlink.c | 188 ++++++++++++++++++++++++++++++++----
 include/linux/dpll.h        |  10 ++
 4 files changed, 209 insertions(+), 18 deletions(-)

diff --git a/drivers/dpll/dpll_core.c b/drivers/dpll/dpll_core.c
index 20bdc52f63a5..805c7aca58c5 100644
--- a/drivers/dpll/dpll_core.c
+++ b/drivers/dpll/dpll_core.c
@@ -506,6 +506,7 @@ dpll_pin_alloc(u64 clock_id, u32 pin_idx, struct module *module,
 	refcount_set(&pin->refcount, 1);
 	xa_init_flags(&pin->dpll_refs, XA_FLAGS_ALLOC);
 	xa_init_flags(&pin->parent_refs, XA_FLAGS_ALLOC);
+	xa_init_flags(&pin->ref_sync_pins, XA_FLAGS_ALLOC);
 	ret = xa_alloc_cyclic(&dpll_pin_xa, &pin->id, pin, xa_limit_32b,
 			      &dpll_pin_xa_id, GFP_KERNEL);
 	if (ret < 0)
@@ -514,6 +515,7 @@ dpll_pin_alloc(u64 clock_id, u32 pin_idx, struct module *module,
 err_xa_alloc:
 	xa_destroy(&pin->dpll_refs);
 	xa_destroy(&pin->parent_refs);
+	xa_destroy(&pin->ref_sync_pins);
 	dpll_pin_prop_free(&pin->prop);
 err_pin_prop:
 	kfree(pin);
@@ -595,6 +597,7 @@ void dpll_pin_put(struct dpll_pin *pin)
 		xa_erase(&dpll_pin_xa, pin->id);
 		xa_destroy(&pin->dpll_refs);
 		xa_destroy(&pin->parent_refs);
+		xa_destroy(&pin->ref_sync_pins);
 		dpll_pin_prop_free(&pin->prop);
 		kfree_rcu(pin, rcu);
 	}
@@ -783,6 +786,30 @@ void dpll_pin_on_pin_unregister(struct dpll_pin *parent, struct dpll_pin *pin,
 }
 EXPORT_SYMBOL_GPL(dpll_pin_on_pin_unregister);
 
+/**
+ * dpll_pin_ref_sync_pair_add - create a reference sync signal pin pair
+ * @base: pin which produces the base frequency
+ * @sync: pin which produces the sync signal
+ *
+ * Once pins are paired, the user-space configuration of reference sync pair
+ * is possible.
+ * Context: Acquires a lock (dpll_lock)
+ * Return:
+ * * 0 on success
+ * * negative - error value
+ */
+int dpll_pin_ref_sync_pair_add(struct dpll_pin *base, struct dpll_pin *sync)
+{
+	int ret;
+
+	mutex_lock(&dpll_lock);
+	ret = xa_insert(&base->ref_sync_pins, sync->pin_idx, sync, GFP_KERNEL);
+	mutex_unlock(&dpll_lock);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(dpll_pin_ref_sync_pair_add);
+
 static struct dpll_device_registration *
 dpll_device_registration_first(struct dpll_device *dpll)
 {
diff --git a/drivers/dpll/dpll_core.h b/drivers/dpll/dpll_core.h
index 2b6d8ef1cdf3..93c68e78b351 100644
--- a/drivers/dpll/dpll_core.h
+++ b/drivers/dpll/dpll_core.h
@@ -44,6 +44,7 @@ struct dpll_device {
  * @module:		module of creator
  * @dpll_refs:		hold referencees to dplls pin was registered with
  * @parent_refs:	hold references to parent pins pin was registered with
+ * @ref_sync_pins:	hold references to pins for Reference SYNC feature
  * @prop:		pin properties copied from the registerer
  * @rclk_dev_name:	holds name of device when pin can recover clock from it
  * @refcount:		refcount
@@ -56,6 +57,7 @@ struct dpll_pin {
 	struct module *module;
 	struct xarray dpll_refs;
 	struct xarray parent_refs;
+	struct xarray ref_sync_pins;
 	struct dpll_pin_properties prop;
 	refcount_t refcount;
 	struct rcu_head rcu;
diff --git a/drivers/dpll/dpll_netlink.c b/drivers/dpll/dpll_netlink.c
index c130f87147fa..854bd46a7d27 100644
--- a/drivers/dpll/dpll_netlink.c
+++ b/drivers/dpll/dpll_netlink.c
@@ -48,6 +48,24 @@ dpll_msg_add_dev_parent_handle(struct sk_buff *msg, u32 id)
 	return 0;
 }
 
+static bool dpll_pin_available(struct dpll_pin *pin)
+{
+	struct dpll_pin_ref *par_ref;
+	unsigned long i;
+
+	if (!xa_get_mark(&dpll_pin_xa, pin->id, DPLL_REGISTERED))
+		return false;
+	xa_for_each(&pin->parent_refs, i, par_ref)
+		if (xa_get_mark(&dpll_pin_xa, par_ref->pin->id,
+				DPLL_REGISTERED))
+			return true;
+	xa_for_each(&pin->dpll_refs, i, par_ref)
+		if (xa_get_mark(&dpll_device_xa, par_ref->dpll->id,
+				DPLL_REGISTERED))
+			return true;
+	return false;
+}
+
 /**
  * dpll_msg_add_pin_handle - attach pin handle attribute to a given message
  * @msg: pointer to sk_buff message to attach a pin handle
@@ -408,6 +426,47 @@ dpll_msg_add_pin_esync(struct sk_buff *msg, struct dpll_pin *pin,
 	return -EMSGSIZE;
 }
 
+static int
+dpll_msg_add_pin_ref_sync(struct sk_buff *msg, struct dpll_pin *pin,
+			  struct dpll_pin_ref *ref,
+			  struct netlink_ext_ack *extack)
+{
+	const struct dpll_pin_ops *ops = dpll_pin_ops(ref);
+	struct dpll_device *dpll = ref->dpll;
+	enum dpll_pin_state state;
+	void *pin_priv, *sp_priv;
+	struct dpll_pin *sp;
+	struct nlattr *nest;
+	unsigned long index;
+	int ret;
+
+	pin_priv = dpll_pin_on_dpll_priv(dpll, pin);
+	xa_for_each(&pin->ref_sync_pins, index, sp) {
+		if (!dpll_pin_available(sp))
+			continue;
+		sp_priv = dpll_pin_on_dpll_priv(dpll, sp);
+		if (WARN_ON(!ops->ref_sync_get))
+			return -EOPNOTSUPP;
+		ret = ops->ref_sync_get(pin, pin_priv, sp, sp_priv,
+					&state, extack);
+		if (ret)
+			return ret;
+		nest = nla_nest_start(msg, DPLL_A_PIN_REFERENCE_SYNC);
+		if (!nest)
+			return -EMSGSIZE;
+		if (nla_put_s32(msg, DPLL_A_PIN_ID, sp->id))
+			goto nest_cancel;
+		if (nla_put_s32(msg, DPLL_A_PIN_STATE, state))
+			goto nest_cancel;
+		nla_nest_end(msg, nest);
+	}
+	return 0;
+
+nest_cancel:
+	nla_nest_cancel(msg, nest);
+	return -EMSGSIZE;
+}
+
 static bool dpll_pin_is_freq_supported(struct dpll_pin *pin, u32 freq)
 {
 	int fs;
@@ -550,6 +609,10 @@ dpll_cmd_pin_get_one(struct sk_buff *msg, struct dpll_pin *pin,
 	if (ret)
 		return ret;
 	ret = dpll_msg_add_pin_esync(msg, pin, ref, extack);
+	if (ret)
+		return ret;
+	if (!xa_empty(&pin->ref_sync_pins))
+		ret = dpll_msg_add_pin_ref_sync(msg, pin, ref, extack);
 	if (ret)
 		return ret;
 	if (xa_empty(&pin->parent_refs))
@@ -642,24 +705,6 @@ __dpll_device_change_ntf(struct dpll_device *dpll)
 	return dpll_device_event_send(DPLL_CMD_DEVICE_CHANGE_NTF, dpll);
 }
 
-static bool dpll_pin_available(struct dpll_pin *pin)
-{
-	struct dpll_pin_ref *par_ref;
-	unsigned long i;
-
-	if (!xa_get_mark(&dpll_pin_xa, pin->id, DPLL_REGISTERED))
-		return false;
-	xa_for_each(&pin->parent_refs, i, par_ref)
-		if (xa_get_mark(&dpll_pin_xa, par_ref->pin->id,
-				DPLL_REGISTERED))
-			return true;
-	xa_for_each(&pin->dpll_refs, i, par_ref)
-		if (xa_get_mark(&dpll_device_xa, par_ref->dpll->id,
-				DPLL_REGISTERED))
-			return true;
-	return false;
-}
-
 /**
  * dpll_device_change_ntf - notify that the dpll device has been changed
  * @dpll: registered dpll pointer
@@ -887,6 +932,108 @@ dpll_pin_esync_set(struct dpll_pin *pin, struct nlattr *a,
 	return ret;
 }
 
+static int
+dpll_pin_ref_sync_state_set(struct dpll_pin *pin, unsigned long sync_pin_idx,
+			    const enum dpll_pin_state state,
+			    struct netlink_ext_ack *extack)
+
+{
+	struct dpll_pin_ref *ref, *failed;
+	const struct dpll_pin_ops *ops;
+	enum dpll_pin_state old_state;
+	struct dpll_pin *sync_pin;
+	struct dpll_device *dpll;
+	unsigned long i;
+	int ret;
+
+	if (state != DPLL_PIN_STATE_CONNECTED &&
+	    state != DPLL_PIN_STATE_DISCONNECTED)
+		return -EINVAL;
+	sync_pin = xa_find(&pin->ref_sync_pins, &sync_pin_idx, ULONG_MAX,
+			   XA_PRESENT);
+	if (!sync_pin) {
+		NL_SET_ERR_MSG(extack, "reference sync pin not found");
+		return -EINVAL;
+	}
+	if (!dpll_pin_available(sync_pin)) {
+		NL_SET_ERR_MSG(extack, "reference sync pin not available");
+		return -EINVAL;
+	}
+	ref = dpll_xa_ref_dpll_first(&pin->dpll_refs);
+	ASSERT_NOT_NULL(ref);
+	ops = dpll_pin_ops(ref);
+	if (!ops->ref_sync_set || !ops->ref_sync_get) {
+		NL_SET_ERR_MSG(extack, "reference sync not supported by this pin");
+		return -EOPNOTSUPP;
+	}
+	dpll = ref->dpll;
+	ret = ops->ref_sync_get(pin, dpll_pin_on_dpll_priv(dpll, pin), sync_pin,
+				dpll_pin_on_dpll_priv(dpll, sync_pin),
+				&old_state, extack);
+	if (ret) {
+		NL_SET_ERR_MSG(extack, "unable to get old reference sync state");
+		return ret;
+	}
+	if (state == old_state)
+		return 0;
+	xa_for_each(&pin->dpll_refs, i, ref) {
+		ops = dpll_pin_ops(ref);
+		dpll = ref->dpll;
+		ret = ops->ref_sync_set(pin, dpll_pin_on_dpll_priv(dpll, pin),
+					sync_pin,
+					dpll_pin_on_dpll_priv(dpll, sync_pin),
+					state, extack);
+		if (ret) {
+			failed = ref;
+			NL_SET_ERR_MSG_FMT(extack, "reference sync set failed for dpll_id:%u",
+					   dpll->id);
+			goto rollback;
+		}
+	}
+	__dpll_pin_change_ntf(pin);
+
+	return 0;
+
+rollback:
+	xa_for_each(&pin->dpll_refs, i, ref) {
+		if (ref == failed)
+			break;
+		ops = dpll_pin_ops(ref);
+		dpll = ref->dpll;
+		if (ops->ref_sync_set(pin, dpll_pin_on_dpll_priv(dpll, pin),
+				      sync_pin,
+				      dpll_pin_on_dpll_priv(dpll, sync_pin),
+				      old_state, extack))
+			NL_SET_ERR_MSG(extack, "set reference sync rollback failed");
+	}
+	return ret;
+}
+
+static int
+dpll_pin_ref_sync_set(struct dpll_pin *pin, struct nlattr *nest,
+		      struct netlink_ext_ack *extack)
+{
+	struct nlattr *tb[DPLL_A_PIN_MAX + 1];
+	enum dpll_pin_state state;
+	u32 sync_pin_id;
+
+	nla_parse_nested(tb, DPLL_A_PIN_MAX, nest,
+			 dpll_reference_sync_nl_policy, extack);
+	if (!tb[DPLL_A_PIN_ID]) {
+		NL_SET_ERR_MSG(extack, "sync pin id expected");
+		return -EINVAL;
+	}
+	sync_pin_id = nla_get_u32(tb[DPLL_A_PIN_ID]);
+
+	if (!tb[DPLL_A_PIN_STATE]) {
+		NL_SET_ERR_MSG(extack, "sync pin state expected");
+		return -EINVAL;
+	}
+	state = nla_get_u32(tb[DPLL_A_PIN_STATE]);
+
+	return dpll_pin_ref_sync_state_set(pin, sync_pin_id, state, extack);
+}
+
 static int
 dpll_pin_on_pin_state_set(struct dpll_pin *pin, u32 parent_idx,
 			  enum dpll_pin_state state,
@@ -1193,6 +1340,11 @@ dpll_pin_set_from_nlattr(struct dpll_pin *pin, struct genl_info *info)
 			if (ret)
 				return ret;
 			break;
+		case DPLL_A_PIN_REFERENCE_SYNC:
+			ret = dpll_pin_ref_sync_set(pin, a, info->extack);
+			if (ret)
+				return ret;
+			break;
 		}
 	}
 
diff --git a/include/linux/dpll.h b/include/linux/dpll.h
index 5e4f9ab1cf75..f1f1fdda67fe 100644
--- a/include/linux/dpll.h
+++ b/include/linux/dpll.h
@@ -95,6 +95,14 @@ struct dpll_pin_ops {
 			 const struct dpll_device *dpll, void *dpll_priv,
 			 struct dpll_pin_esync *esync,
 			 struct netlink_ext_ack *extack);
+	int (*ref_sync_set)(const struct dpll_pin *pin, void *pin_priv,
+			    const struct dpll_pin *ref_pin, void *ref_pin_priv,
+			    const enum dpll_pin_state state,
+			    struct netlink_ext_ack *extack);
+	int (*ref_sync_get)(const struct dpll_pin *pin, void *pin_priv,
+			    const struct dpll_pin *ref_pin, void *ref_pin_priv,
+			    enum dpll_pin_state *state,
+			    struct netlink_ext_ack *extack);
 };
 
 struct dpll_pin_frequency {
@@ -194,6 +202,8 @@ int dpll_pin_on_pin_register(struct dpll_pin *parent, struct dpll_pin *pin,
 void dpll_pin_on_pin_unregister(struct dpll_pin *parent, struct dpll_pin *pin,
 				const struct dpll_pin_ops *ops, void *priv);
 
+int dpll_pin_ref_sync_pair_add(struct dpll_pin *base, struct dpll_pin *sync);
+
 int dpll_device_change_ntf(struct dpll_device *dpll);
 
 int dpll_pin_change_ntf(struct dpll_pin *pin);
-- 
2.38.1

Re: [PATCH net-next v3 2/3] dpll: add reference sync get/set

[Jiri Pirko]

Thu, May 22, 2025 at 06:29:37PM +0200, arkadiusz.kubalewski@intel.com wrote:
>Define function for reference sync pin registration and callback ops to
>set/get current feature state.
>
>Implement netlink handler to fill netlink messages with reference sync
>pin configuration of capable pins (pin-get).
>
>Implement netlink handler to call proper ops and configure reference
>sync pin state (pin-set).
>
>Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
>Reviewed-by: Milena Olech <milena.olech@intel.com>
>Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
>---
> v3:
>- fix kdoc missing ':' after argument name ref_sync_pins,
>- propagate ret in dpll_pin_ref_sync_state_set().
>---
> drivers/dpll/dpll_core.c    |  27 ++++++
> drivers/dpll/dpll_core.h    |   2 +
> drivers/dpll/dpll_netlink.c | 188 ++++++++++++++++++++++++++++++++----
> include/linux/dpll.h        |  10 ++
> 4 files changed, 209 insertions(+), 18 deletions(-)
>
>diff --git a/drivers/dpll/dpll_core.c b/drivers/dpll/dpll_core.c
>index 20bdc52f63a5..805c7aca58c5 100644
>--- a/drivers/dpll/dpll_core.c
>+++ b/drivers/dpll/dpll_core.c
>@@ -506,6 +506,7 @@ dpll_pin_alloc(u64 clock_id, u32 pin_idx, struct module *module,
> 	refcount_set(&pin->refcount, 1);
> 	xa_init_flags(&pin->dpll_refs, XA_FLAGS_ALLOC);
> 	xa_init_flags(&pin->parent_refs, XA_FLAGS_ALLOC);
>+	xa_init_flags(&pin->ref_sync_pins, XA_FLAGS_ALLOC);
> 	ret = xa_alloc_cyclic(&dpll_pin_xa, &pin->id, pin, xa_limit_32b,
> 			      &dpll_pin_xa_id, GFP_KERNEL);
> 	if (ret < 0)
>@@ -514,6 +515,7 @@ dpll_pin_alloc(u64 clock_id, u32 pin_idx, struct module *module,
> err_xa_alloc:
> 	xa_destroy(&pin->dpll_refs);
> 	xa_destroy(&pin->parent_refs);
>+	xa_destroy(&pin->ref_sync_pins);
> 	dpll_pin_prop_free(&pin->prop);
> err_pin_prop:
> 	kfree(pin);
>@@ -595,6 +597,7 @@ void dpll_pin_put(struct dpll_pin *pin)
> 		xa_erase(&dpll_pin_xa, pin->id);
> 		xa_destroy(&pin->dpll_refs);
> 		xa_destroy(&pin->parent_refs);
>+		xa_destroy(&pin->ref_sync_pins);
> 		dpll_pin_prop_free(&pin->prop);
> 		kfree_rcu(pin, rcu);
> 	}
>@@ -783,6 +786,30 @@ void dpll_pin_on_pin_unregister(struct dpll_pin *parent, struct dpll_pin *pin,
> }
> EXPORT_SYMBOL_GPL(dpll_pin_on_pin_unregister);
> 
>+/**
>+ * dpll_pin_ref_sync_pair_add - create a reference sync signal pin pair
>+ * @base: pin which produces the base frequency
>+ * @sync: pin which produces the sync signal
>+ *
>+ * Once pins are paired, the user-space configuration of reference sync pair
>+ * is possible.
>+ * Context: Acquires a lock (dpll_lock)
>+ * Return:
>+ * * 0 on success
>+ * * negative - error value
>+ */
>+int dpll_pin_ref_sync_pair_add(struct dpll_pin *base, struct dpll_pin *sync)

Perhaps call it "pin" and "ref_sync_pin"?


>+{
>+	int ret;
>+
>+	mutex_lock(&dpll_lock);
>+	ret = xa_insert(&base->ref_sync_pins, sync->pin_idx, sync, GFP_KERNEL);
>+	mutex_unlock(&dpll_lock);
>+
>+	return ret;
>+}
>+EXPORT_SYMBOL_GPL(dpll_pin_ref_sync_pair_add);

How do you handle remove?


>+
> static struct dpll_device_registration *
> dpll_device_registration_first(struct dpll_device *dpll)
> {
>diff --git a/drivers/dpll/dpll_core.h b/drivers/dpll/dpll_core.h
>index 2b6d8ef1cdf3..93c68e78b351 100644
>--- a/drivers/dpll/dpll_core.h
>+++ b/drivers/dpll/dpll_core.h
>@@ -44,6 +44,7 @@ struct dpll_device {
>  * @module:		module of creator
>  * @dpll_refs:		hold referencees to dplls pin was registered with
>  * @parent_refs:	hold references to parent pins pin was registered with
>+ * @ref_sync_pins:	hold references to pins for Reference SYNC feature
>  * @prop:		pin properties copied from the registerer
>  * @rclk_dev_name:	holds name of device when pin can recover clock from it
>  * @refcount:		refcount
>@@ -56,6 +57,7 @@ struct dpll_pin {
> 	struct module *module;
> 	struct xarray dpll_refs;
> 	struct xarray parent_refs;
>+	struct xarray ref_sync_pins;
> 	struct dpll_pin_properties prop;
> 	refcount_t refcount;
> 	struct rcu_head rcu;
>diff --git a/drivers/dpll/dpll_netlink.c b/drivers/dpll/dpll_netlink.c
>index c130f87147fa..854bd46a7d27 100644
>--- a/drivers/dpll/dpll_netlink.c
>+++ b/drivers/dpll/dpll_netlink.c
>@@ -48,6 +48,24 @@ dpll_msg_add_dev_parent_handle(struct sk_buff *msg, u32 id)
> 	return 0;
> }
> 
>+static bool dpll_pin_available(struct dpll_pin *pin)
>+{
>+	struct dpll_pin_ref *par_ref;
>+	unsigned long i;
>+
>+	if (!xa_get_mark(&dpll_pin_xa, pin->id, DPLL_REGISTERED))
>+		return false;
>+	xa_for_each(&pin->parent_refs, i, par_ref)
>+		if (xa_get_mark(&dpll_pin_xa, par_ref->pin->id,
>+				DPLL_REGISTERED))
>+			return true;
>+	xa_for_each(&pin->dpll_refs, i, par_ref)
>+		if (xa_get_mark(&dpll_device_xa, par_ref->dpll->id,
>+				DPLL_REGISTERED))
>+			return true;
>+	return false;
>+}
>+
> /**
>  * dpll_msg_add_pin_handle - attach pin handle attribute to a given message
>  * @msg: pointer to sk_buff message to attach a pin handle
>@@ -408,6 +426,47 @@ dpll_msg_add_pin_esync(struct sk_buff *msg, struct dpll_pin *pin,
> 	return -EMSGSIZE;
> }
> 
>+static int
>+dpll_msg_add_pin_ref_sync(struct sk_buff *msg, struct dpll_pin *pin,
>+			  struct dpll_pin_ref *ref,
>+			  struct netlink_ext_ack *extack)
>+{
>+	const struct dpll_pin_ops *ops = dpll_pin_ops(ref);
>+	struct dpll_device *dpll = ref->dpll;
>+	enum dpll_pin_state state;
>+	void *pin_priv, *sp_priv;
>+	struct dpll_pin *sp;

Make sure you are consistent in variables naming. You call this "sync"
whan you add it.


>+	struct nlattr *nest;
>+	unsigned long index;
>+	int ret;
>+
>+	pin_priv = dpll_pin_on_dpll_priv(dpll, pin);
>+	xa_for_each(&pin->ref_sync_pins, index, sp) {
>+		if (!dpll_pin_available(sp))
>+			continue;
>+		sp_priv = dpll_pin_on_dpll_priv(dpll, sp);
>+		if (WARN_ON(!ops->ref_sync_get))
>+			return -EOPNOTSUPP;
>+		ret = ops->ref_sync_get(pin, pin_priv, sp, sp_priv,
>+					&state, extack);
>+		if (ret)
>+			return ret;
>+		nest = nla_nest_start(msg, DPLL_A_PIN_REFERENCE_SYNC);
>+		if (!nest)
>+			return -EMSGSIZE;
>+		if (nla_put_s32(msg, DPLL_A_PIN_ID, sp->id))
>+			goto nest_cancel;
>+		if (nla_put_s32(msg, DPLL_A_PIN_STATE, state))
>+			goto nest_cancel;
>+		nla_nest_end(msg, nest);
>+	}
>+	return 0;
>+
>+nest_cancel:
>+	nla_nest_cancel(msg, nest);
>+	return -EMSGSIZE;
>+}
>+
> static bool dpll_pin_is_freq_supported(struct dpll_pin *pin, u32 freq)
> {
> 	int fs;
>@@ -550,6 +609,10 @@ dpll_cmd_pin_get_one(struct sk_buff *msg, struct dpll_pin *pin,
> 	if (ret)
> 		return ret;
> 	ret = dpll_msg_add_pin_esync(msg, pin, ref, extack);
>+	if (ret)
>+		return ret;
>+	if (!xa_empty(&pin->ref_sync_pins))
>+		ret = dpll_msg_add_pin_ref_sync(msg, pin, ref, extack);
> 	if (ret)
> 		return ret;
> 	if (xa_empty(&pin->parent_refs))
>@@ -642,24 +705,6 @@ __dpll_device_change_ntf(struct dpll_device *dpll)
> 	return dpll_device_event_send(DPLL_CMD_DEVICE_CHANGE_NTF, dpll);
> }
> 
>-static bool dpll_pin_available(struct dpll_pin *pin)
>-{
>-	struct dpll_pin_ref *par_ref;
>-	unsigned long i;
>-
>-	if (!xa_get_mark(&dpll_pin_xa, pin->id, DPLL_REGISTERED))
>-		return false;
>-	xa_for_each(&pin->parent_refs, i, par_ref)
>-		if (xa_get_mark(&dpll_pin_xa, par_ref->pin->id,
>-				DPLL_REGISTERED))
>-			return true;
>-	xa_for_each(&pin->dpll_refs, i, par_ref)
>-		if (xa_get_mark(&dpll_device_xa, par_ref->dpll->id,
>-				DPLL_REGISTERED))
>-			return true;
>-	return false;
>-}
>-
> /**
>  * dpll_device_change_ntf - notify that the dpll device has been changed
>  * @dpll: registered dpll pointer
>@@ -887,6 +932,108 @@ dpll_pin_esync_set(struct dpll_pin *pin, struct nlattr *a,
> 	return ret;
> }
> 
>+static int
>+dpll_pin_ref_sync_state_set(struct dpll_pin *pin, unsigned long sync_pin_idx,
>+			    const enum dpll_pin_state state,
>+			    struct netlink_ext_ack *extack)
>+
>+{
>+	struct dpll_pin_ref *ref, *failed;
>+	const struct dpll_pin_ops *ops;
>+	enum dpll_pin_state old_state;
>+	struct dpll_pin *sync_pin;

Again, please name this consistently...


>+	struct dpll_device *dpll;
>+	unsigned long i;
>+	int ret;
>+
>+	if (state != DPLL_PIN_STATE_CONNECTED &&
>+	    state != DPLL_PIN_STATE_DISCONNECTED)

Extack message? But, isn't this sanitized already by policy? Then,
please remove.


>+		return -EINVAL;
>+	sync_pin = xa_find(&pin->ref_sync_pins, &sync_pin_idx, ULONG_MAX,
>+			   XA_PRESENT);
>+	if (!sync_pin) {
>+		NL_SET_ERR_MSG(extack, "reference sync pin not found");
>+		return -EINVAL;
>+	}
>+	if (!dpll_pin_available(sync_pin)) {
>+		NL_SET_ERR_MSG(extack, "reference sync pin not available");
>+		return -EINVAL;
>+	}
>+	ref = dpll_xa_ref_dpll_first(&pin->dpll_refs);
>+	ASSERT_NOT_NULL(ref);
>+	ops = dpll_pin_ops(ref);
>+	if (!ops->ref_sync_set || !ops->ref_sync_get) {
>+		NL_SET_ERR_MSG(extack, "reference sync not supported by this pin");
>+		return -EOPNOTSUPP;
>+	}
>+	dpll = ref->dpll;
>+	ret = ops->ref_sync_get(pin, dpll_pin_on_dpll_priv(dpll, pin), sync_pin,
>+				dpll_pin_on_dpll_priv(dpll, sync_pin),
>+				&old_state, extack);
>+	if (ret) {
>+		NL_SET_ERR_MSG(extack, "unable to get old reference sync state");
>+		return ret;
>+	}
>+	if (state == old_state)
>+		return 0;
>+	xa_for_each(&pin->dpll_refs, i, ref) {
>+		ops = dpll_pin_ops(ref);
>+		dpll = ref->dpll;
>+		ret = ops->ref_sync_set(pin, dpll_pin_on_dpll_priv(dpll, pin),
>+					sync_pin,
>+					dpll_pin_on_dpll_priv(dpll, sync_pin),
>+					state, extack);
>+		if (ret) {
>+			failed = ref;
>+			NL_SET_ERR_MSG_FMT(extack, "reference sync set failed for dpll_id:%u",
>+					   dpll->id);
>+			goto rollback;
>+		}
>+	}
>+	__dpll_pin_change_ntf(pin);
>+
>+	return 0;
>+
>+rollback:
>+	xa_for_each(&pin->dpll_refs, i, ref) {
>+		if (ref == failed)
>+			break;
>+		ops = dpll_pin_ops(ref);
>+		dpll = ref->dpll;
>+		if (ops->ref_sync_set(pin, dpll_pin_on_dpll_priv(dpll, pin),
>+				      sync_pin,
>+				      dpll_pin_on_dpll_priv(dpll, sync_pin),
>+				      old_state, extack))
>+			NL_SET_ERR_MSG(extack, "set reference sync rollback failed");
>+	}
>+	return ret;
>+}
>+
>+static int
>+dpll_pin_ref_sync_set(struct dpll_pin *pin, struct nlattr *nest,
>+		      struct netlink_ext_ack *extack)
>+{
>+	struct nlattr *tb[DPLL_A_PIN_MAX + 1];
>+	enum dpll_pin_state state;
>+	u32 sync_pin_id;
>+
>+	nla_parse_nested(tb, DPLL_A_PIN_MAX, nest,
>+			 dpll_reference_sync_nl_policy, extack);
>+	if (!tb[DPLL_A_PIN_ID]) {
>+		NL_SET_ERR_MSG(extack, "sync pin id expected");
>+		return -EINVAL;
>+	}
>+	sync_pin_id = nla_get_u32(tb[DPLL_A_PIN_ID]);
>+
>+	if (!tb[DPLL_A_PIN_STATE]) {
>+		NL_SET_ERR_MSG(extack, "sync pin state expected");
>+		return -EINVAL;
>+	}
>+	state = nla_get_u32(tb[DPLL_A_PIN_STATE]);
>+
>+	return dpll_pin_ref_sync_state_set(pin, sync_pin_id, state, extack);
>+}
>+
> static int
> dpll_pin_on_pin_state_set(struct dpll_pin *pin, u32 parent_idx,
> 			  enum dpll_pin_state state,
>@@ -1193,6 +1340,11 @@ dpll_pin_set_from_nlattr(struct dpll_pin *pin, struct genl_info *info)
> 			if (ret)
> 				return ret;
> 			break;
>+		case DPLL_A_PIN_REFERENCE_SYNC:
>+			ret = dpll_pin_ref_sync_set(pin, a, info->extack);
>+			if (ret)
>+				return ret;
>+			break;
> 		}
> 	}
> 
>diff --git a/include/linux/dpll.h b/include/linux/dpll.h
>index 5e4f9ab1cf75..f1f1fdda67fe 100644
>--- a/include/linux/dpll.h
>+++ b/include/linux/dpll.h
>@@ -95,6 +95,14 @@ struct dpll_pin_ops {
> 			 const struct dpll_device *dpll, void *dpll_priv,
> 			 struct dpll_pin_esync *esync,
> 			 struct netlink_ext_ack *extack);
>+	int (*ref_sync_set)(const struct dpll_pin *pin, void *pin_priv,
>+			    const struct dpll_pin *ref_pin, void *ref_pin_priv,
>+			    const enum dpll_pin_state state,
>+			    struct netlink_ext_ack *extack);
>+	int (*ref_sync_get)(const struct dpll_pin *pin, void *pin_priv,
>+			    const struct dpll_pin *ref_pin, void *ref_pin_priv,

"ref_pin". This is 4th name of the same variable. Weird...


>+			    enum dpll_pin_state *state,
>+			    struct netlink_ext_ack *extack);
> };
> 
> struct dpll_pin_frequency {
>@@ -194,6 +202,8 @@ int dpll_pin_on_pin_register(struct dpll_pin *parent, struct dpll_pin *pin,
> void dpll_pin_on_pin_unregister(struct dpll_pin *parent, struct dpll_pin *pin,
> 				const struct dpll_pin_ops *ops, void *priv);
> 
>+int dpll_pin_ref_sync_pair_add(struct dpll_pin *base, struct dpll_pin *sync);
>+
> int dpll_device_change_ntf(struct dpll_device *dpll);
> 
> int dpll_pin_change_ntf(struct dpll_pin *pin);
>-- 
>2.38.1
>

RE: [PATCH net-next v3 2/3] dpll: add reference sync get/set

[Kubalewski, Arkadiusz]

>From: Jiri Pirko <jiri@resnulli.us>
>Sent: Friday, May 23, 2025 2:14 PM
>
>Thu, May 22, 2025 at 06:29:37PM +0200, arkadiusz.kubalewski@intel.com
>wrote:
>>Define function for reference sync pin registration and callback ops to
>>set/get current feature state.
>>
>>Implement netlink handler to fill netlink messages with reference sync
>>pin configuration of capable pins (pin-get).
>>
>>Implement netlink handler to call proper ops and configure reference
>>sync pin state (pin-set).
>>
>>Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
>>Reviewed-by: Milena Olech <milena.olech@intel.com>
>>Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
>>---
>> v3:
>>- fix kdoc missing ':' after argument name ref_sync_pins,
>>- propagate ret in dpll_pin_ref_sync_state_set().
>>---
>> drivers/dpll/dpll_core.c    |  27 ++++++
>> drivers/dpll/dpll_core.h    |   2 +
>> drivers/dpll/dpll_netlink.c | 188 ++++++++++++++++++++++++++++++++----
>> include/linux/dpll.h        |  10 ++
>> 4 files changed, 209 insertions(+), 18 deletions(-)
>>
>>diff --git a/drivers/dpll/dpll_core.c b/drivers/dpll/dpll_core.c
>>index 20bdc52f63a5..805c7aca58c5 100644
>>--- a/drivers/dpll/dpll_core.c
>>+++ b/drivers/dpll/dpll_core.c
>>@@ -506,6 +506,7 @@ dpll_pin_alloc(u64 clock_id, u32 pin_idx, struct
>>module *module,
>> 	refcount_set(&pin->refcount, 1);
>> 	xa_init_flags(&pin->dpll_refs, XA_FLAGS_ALLOC);
>> 	xa_init_flags(&pin->parent_refs, XA_FLAGS_ALLOC);
>>+	xa_init_flags(&pin->ref_sync_pins, XA_FLAGS_ALLOC);
>> 	ret = xa_alloc_cyclic(&dpll_pin_xa, &pin->id, pin, xa_limit_32b,
>> 			      &dpll_pin_xa_id, GFP_KERNEL);
>> 	if (ret < 0)
>>@@ -514,6 +515,7 @@ dpll_pin_alloc(u64 clock_id, u32 pin_idx, struct
>>module *module,
>> err_xa_alloc:
>> 	xa_destroy(&pin->dpll_refs);
>> 	xa_destroy(&pin->parent_refs);
>>+	xa_destroy(&pin->ref_sync_pins);
>> 	dpll_pin_prop_free(&pin->prop);
>> err_pin_prop:
>> 	kfree(pin);
>>@@ -595,6 +597,7 @@ void dpll_pin_put(struct dpll_pin *pin)
>> 		xa_erase(&dpll_pin_xa, pin->id);
>> 		xa_destroy(&pin->dpll_refs);
>> 		xa_destroy(&pin->parent_refs);
>>+		xa_destroy(&pin->ref_sync_pins);
>> 		dpll_pin_prop_free(&pin->prop);
>> 		kfree_rcu(pin, rcu);
>> 	}
>>@@ -783,6 +786,30 @@ void dpll_pin_on_pin_unregister(struct dpll_pin
>>*parent, struct dpll_pin *pin,
>> }
>> EXPORT_SYMBOL_GPL(dpll_pin_on_pin_unregister);
>>
>>+/**
>>+ * dpll_pin_ref_sync_pair_add - create a reference sync signal pin pair
>>+ * @base: pin which produces the base frequency
>>+ * @sync: pin which produces the sync signal
>>+ *
>>+ * Once pins are paired, the user-space configuration of reference sync
>>pair
>>+ * is possible.
>>+ * Context: Acquires a lock (dpll_lock)
>>+ * Return:
>>+ * * 0 on success
>>+ * * negative - error value
>>+ */
>>+int dpll_pin_ref_sync_pair_add(struct dpll_pin *base, struct dpll_pin
>>*sync)
>
>Perhaps call it "pin" and "ref_sync_pin"?
>

Sure, makes sense, will fix.

>
>>+{
>>+	int ret;
>>+
>>+	mutex_lock(&dpll_lock);
>>+	ret = xa_insert(&base->ref_sync_pins, sync->pin_idx, sync,
>>GFP_KERNEL);
>>+	mutex_unlock(&dpll_lock);
>>+
>>+	return ret;
>>+}
>>+EXPORT_SYMBOL_GPL(dpll_pin_ref_sync_pair_add);
>
>How do you handle remove?
>

I did not, this is hidden issue when you unregister sequentially.
Will fix that in next version.

>
>>+
>> static struct dpll_device_registration *
>> dpll_device_registration_first(struct dpll_device *dpll)
>> {
>>diff --git a/drivers/dpll/dpll_core.h b/drivers/dpll/dpll_core.h
>>index 2b6d8ef1cdf3..93c68e78b351 100644
>>--- a/drivers/dpll/dpll_core.h
>>+++ b/drivers/dpll/dpll_core.h
>>@@ -44,6 +44,7 @@ struct dpll_device {
>>  * @module:		module of creator
>>  * @dpll_refs:		hold referencees to dplls pin was registered with
>>  * @parent_refs:	hold references to parent pins pin was registered
>>with
>>+ * @ref_sync_pins:	hold references to pins for Reference SYNC feature
>>  * @prop:		pin properties copied from the registerer
>>  * @rclk_dev_name:	holds name of device when pin can recover clock
>>from it
>>  * @refcount:		refcount
>>@@ -56,6 +57,7 @@ struct dpll_pin {
>> 	struct module *module;
>> 	struct xarray dpll_refs;
>> 	struct xarray parent_refs;
>>+	struct xarray ref_sync_pins;
>> 	struct dpll_pin_properties prop;
>> 	refcount_t refcount;
>> 	struct rcu_head rcu;
>>diff --git a/drivers/dpll/dpll_netlink.c b/drivers/dpll/dpll_netlink.c
>>index c130f87147fa..854bd46a7d27 100644
>>--- a/drivers/dpll/dpll_netlink.c
>>+++ b/drivers/dpll/dpll_netlink.c
>>@@ -48,6 +48,24 @@ dpll_msg_add_dev_parent_handle(struct sk_buff *msg, u32
>>id)
>> 	return 0;
>> }
>>
>>+static bool dpll_pin_available(struct dpll_pin *pin)
>>+{
>>+	struct dpll_pin_ref *par_ref;
>>+	unsigned long i;
>>+
>>+	if (!xa_get_mark(&dpll_pin_xa, pin->id, DPLL_REGISTERED))
>>+		return false;
>>+	xa_for_each(&pin->parent_refs, i, par_ref)
>>+		if (xa_get_mark(&dpll_pin_xa, par_ref->pin->id,
>>+				DPLL_REGISTERED))
>>+			return true;
>>+	xa_for_each(&pin->dpll_refs, i, par_ref)
>>+		if (xa_get_mark(&dpll_device_xa, par_ref->dpll->id,
>>+				DPLL_REGISTERED))
>>+			return true;
>>+	return false;
>>+}
>>+
>> /**
>>  * dpll_msg_add_pin_handle - attach pin handle attribute to a given
>>message
>>  * @msg: pointer to sk_buff message to attach a pin handle
>>@@ -408,6 +426,47 @@ dpll_msg_add_pin_esync(struct sk_buff *msg, struct
>>dpll_pin *pin,
>> 	return -EMSGSIZE;
>> }
>>
>>+static int
>>+dpll_msg_add_pin_ref_sync(struct sk_buff *msg, struct dpll_pin *pin,
>>+			  struct dpll_pin_ref *ref,
>>+			  struct netlink_ext_ack *extack)
>>+{
>>+	const struct dpll_pin_ops *ops = dpll_pin_ops(ref);
>>+	struct dpll_device *dpll = ref->dpll;
>>+	enum dpll_pin_state state;
>>+	void *pin_priv, *sp_priv;
>>+	struct dpll_pin *sp;
>
>Make sure you are consistent in variables naming. You call this "sync"
>whan you add it.
>

Sure, will do.

>
>>+	struct nlattr *nest;
>>+	unsigned long index;
>>+	int ret;
>>+
>>+	pin_priv = dpll_pin_on_dpll_priv(dpll, pin);
>>+	xa_for_each(&pin->ref_sync_pins, index, sp) {
>>+		if (!dpll_pin_available(sp))
>>+			continue;
>>+		sp_priv = dpll_pin_on_dpll_priv(dpll, sp);
>>+		if (WARN_ON(!ops->ref_sync_get))
>>+			return -EOPNOTSUPP;
>>+		ret = ops->ref_sync_get(pin, pin_priv, sp, sp_priv,
>>+					&state, extack);
>>+		if (ret)
>>+			return ret;
>>+		nest = nla_nest_start(msg, DPLL_A_PIN_REFERENCE_SYNC);
>>+		if (!nest)
>>+			return -EMSGSIZE;
>>+		if (nla_put_s32(msg, DPLL_A_PIN_ID, sp->id))
>>+			goto nest_cancel;
>>+		if (nla_put_s32(msg, DPLL_A_PIN_STATE, state))
>>+			goto nest_cancel;
>>+		nla_nest_end(msg, nest);
>>+	}
>>+	return 0;
>>+
>>+nest_cancel:
>>+	nla_nest_cancel(msg, nest);
>>+	return -EMSGSIZE;
>>+}
>>+
>> static bool dpll_pin_is_freq_supported(struct dpll_pin *pin, u32 freq)
>> {
>> 	int fs;
>>@@ -550,6 +609,10 @@ dpll_cmd_pin_get_one(struct sk_buff *msg, struct
>>dpll_pin *pin,
>> 	if (ret)
>> 		return ret;
>> 	ret = dpll_msg_add_pin_esync(msg, pin, ref, extack);
>>+	if (ret)
>>+		return ret;
>>+	if (!xa_empty(&pin->ref_sync_pins))
>>+		ret = dpll_msg_add_pin_ref_sync(msg, pin, ref, extack);
>> 	if (ret)
>> 		return ret;
>> 	if (xa_empty(&pin->parent_refs))
>>@@ -642,24 +705,6 @@ __dpll_device_change_ntf(struct dpll_device *dpll)
>> 	return dpll_device_event_send(DPLL_CMD_DEVICE_CHANGE_NTF, dpll);
>> }
>>
>>-static bool dpll_pin_available(struct dpll_pin *pin)
>>-{
>>-	struct dpll_pin_ref *par_ref;
>>-	unsigned long i;
>>-
>>-	if (!xa_get_mark(&dpll_pin_xa, pin->id, DPLL_REGISTERED))
>>-		return false;
>>-	xa_for_each(&pin->parent_refs, i, par_ref)
>>-		if (xa_get_mark(&dpll_pin_xa, par_ref->pin->id,
>>-				DPLL_REGISTERED))
>>-			return true;
>>-	xa_for_each(&pin->dpll_refs, i, par_ref)
>>-		if (xa_get_mark(&dpll_device_xa, par_ref->dpll->id,
>>-				DPLL_REGISTERED))
>>-			return true;
>>-	return false;
>>-}
>>-
>> /**
>>  * dpll_device_change_ntf - notify that the dpll device has been changed
>>  * @dpll: registered dpll pointer
>>@@ -887,6 +932,108 @@ dpll_pin_esync_set(struct dpll_pin *pin, struct
>>nlattr *a,
>> 	return ret;
>> }
>>
>>+static int
>>+dpll_pin_ref_sync_state_set(struct dpll_pin *pin, unsigned long
>>sync_pin_idx,
>>+			    const enum dpll_pin_state state,
>>+			    struct netlink_ext_ack *extack)
>>+
>>+{
>>+	struct dpll_pin_ref *ref, *failed;
>>+	const struct dpll_pin_ops *ops;
>>+	enum dpll_pin_state old_state;
>>+	struct dpll_pin *sync_pin;
>
>Again, please name this consistently...
>

Sure, will fix.

>
>>+	struct dpll_device *dpll;
>>+	unsigned long i;
>>+	int ret;
>>+
>>+	if (state != DPLL_PIN_STATE_CONNECTED &&
>>+	    state != DPLL_PIN_STATE_DISCONNECTED)
>
>Extack message? But, isn't this sanitized already by policy? Then,
>please remove.
>

True, will remove.

>
>>+		return -EINVAL;
>>+	sync_pin = xa_find(&pin->ref_sync_pins, &sync_pin_idx, ULONG_MAX,
>>+			   XA_PRESENT);
>>+	if (!sync_pin) {
>>+		NL_SET_ERR_MSG(extack, "reference sync pin not found");
>>+		return -EINVAL;
>>+	}
>>+	if (!dpll_pin_available(sync_pin)) {
>>+		NL_SET_ERR_MSG(extack, "reference sync pin not available");
>>+		return -EINVAL;
>>+	}
>>+	ref = dpll_xa_ref_dpll_first(&pin->dpll_refs);
>>+	ASSERT_NOT_NULL(ref);
>>+	ops = dpll_pin_ops(ref);
>>+	if (!ops->ref_sync_set || !ops->ref_sync_get) {
>>+		NL_SET_ERR_MSG(extack, "reference sync not supported by this
>>pin");
>>+		return -EOPNOTSUPP;
>>+	}
>>+	dpll = ref->dpll;
>>+	ret = ops->ref_sync_get(pin, dpll_pin_on_dpll_priv(dpll, pin),
>>sync_pin,
>>+				dpll_pin_on_dpll_priv(dpll, sync_pin),
>>+				&old_state, extack);
>>+	if (ret) {
>>+		NL_SET_ERR_MSG(extack, "unable to get old reference sync
>>state");
>>+		return ret;
>>+	}
>>+	if (state == old_state)
>>+		return 0;
>>+	xa_for_each(&pin->dpll_refs, i, ref) {
>>+		ops = dpll_pin_ops(ref);
>>+		dpll = ref->dpll;
>>+		ret = ops->ref_sync_set(pin, dpll_pin_on_dpll_priv(dpll, pin),
>>+					sync_pin,
>>+					dpll_pin_on_dpll_priv(dpll, sync_pin),
>>+					state, extack);
>>+		if (ret) {
>>+			failed = ref;
>>+			NL_SET_ERR_MSG_FMT(extack, "reference sync set failed for
>>dpll_id:%u",
>>+					   dpll->id);
>>+			goto rollback;
>>+		}
>>+	}
>>+	__dpll_pin_change_ntf(pin);
>>+
>>+	return 0;
>>+
>>+rollback:
>>+	xa_for_each(&pin->dpll_refs, i, ref) {
>>+		if (ref == failed)
>>+			break;
>>+		ops = dpll_pin_ops(ref);
>>+		dpll = ref->dpll;
>>+		if (ops->ref_sync_set(pin, dpll_pin_on_dpll_priv(dpll, pin),
>>+				      sync_pin,
>>+				      dpll_pin_on_dpll_priv(dpll, sync_pin),
>>+				      old_state, extack))
>>+			NL_SET_ERR_MSG(extack, "set reference sync rollback
>>failed");
>>+	}
>>+	return ret;
>>+}
>>+
>>+static int
>>+dpll_pin_ref_sync_set(struct dpll_pin *pin, struct nlattr *nest,
>>+		      struct netlink_ext_ack *extack)
>>+{
>>+	struct nlattr *tb[DPLL_A_PIN_MAX + 1];
>>+	enum dpll_pin_state state;
>>+	u32 sync_pin_id;
>>+
>>+	nla_parse_nested(tb, DPLL_A_PIN_MAX, nest,
>>+			 dpll_reference_sync_nl_policy, extack);
>>+	if (!tb[DPLL_A_PIN_ID]) {
>>+		NL_SET_ERR_MSG(extack, "sync pin id expected");
>>+		return -EINVAL;
>>+	}
>>+	sync_pin_id = nla_get_u32(tb[DPLL_A_PIN_ID]);
>>+
>>+	if (!tb[DPLL_A_PIN_STATE]) {
>>+		NL_SET_ERR_MSG(extack, "sync pin state expected");
>>+		return -EINVAL;
>>+	}
>>+	state = nla_get_u32(tb[DPLL_A_PIN_STATE]);
>>+
>>+	return dpll_pin_ref_sync_state_set(pin, sync_pin_id, state, extack);
>>+}
>>+
>> static int
>> dpll_pin_on_pin_state_set(struct dpll_pin *pin, u32 parent_idx,
>> 			  enum dpll_pin_state state,
>>@@ -1193,6 +1340,11 @@ dpll_pin_set_from_nlattr(struct dpll_pin *pin,
>>struct genl_info *info)
>> 			if (ret)
>> 				return ret;
>> 			break;
>>+		case DPLL_A_PIN_REFERENCE_SYNC:
>>+			ret = dpll_pin_ref_sync_set(pin, a, info->extack);
>>+			if (ret)
>>+				return ret;
>>+			break;
>> 		}
>> 	}
>>
>>diff --git a/include/linux/dpll.h b/include/linux/dpll.h
>>index 5e4f9ab1cf75..f1f1fdda67fe 100644
>>--- a/include/linux/dpll.h
>>+++ b/include/linux/dpll.h
>>@@ -95,6 +95,14 @@ struct dpll_pin_ops {
>> 			 const struct dpll_device *dpll, void *dpll_priv,
>> 			 struct dpll_pin_esync *esync,
>> 			 struct netlink_ext_ack *extack);
>>+	int (*ref_sync_set)(const struct dpll_pin *pin, void *pin_priv,
>>+			    const struct dpll_pin *ref_pin, void *ref_pin_priv,
>>+			    const enum dpll_pin_state state,
>>+			    struct netlink_ext_ack *extack);
>>+	int (*ref_sync_get)(const struct dpll_pin *pin, void *pin_priv,
>>+			    const struct dpll_pin *ref_pin, void *ref_pin_priv,
>
>"ref_pin". This is 4th name of the same variable. Weird...
>

Yes, going to fix them all.

Thank you!
Arkadiusz

>
>>+			    enum dpll_pin_state *state,
>>+			    struct netlink_ext_ack *extack);
>> };
>>
>> struct dpll_pin_frequency {
>>@@ -194,6 +202,8 @@ int dpll_pin_on_pin_register(struct dpll_pin *parent,
>>struct dpll_pin *pin,
>> void dpll_pin_on_pin_unregister(struct dpll_pin *parent, struct dpll_pin
>>*pin,
>> 				const struct dpll_pin_ops *ops, void *priv);
>>
>>+int dpll_pin_ref_sync_pair_add(struct dpll_pin *base, struct dpll_pin
>>*sync);
>>+
>> int dpll_device_change_ntf(struct dpll_device *dpll);
>>
>> int dpll_pin_change_ntf(struct dpll_pin *pin);
>>--
>>2.38.1
>>

[PATCH net-next v3 3/3] ice: add ref-sync dpll pins

[Arkadiusz Kubalewski]

Implement reference sync input pin get/set callbacks, allow user space
control over dpll pin pairs capable of reference sync support.

Reviewed-by: Milena Olech <milena.olech@intel.com>
Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
---
v3:
- no change.
---
 .../net/ethernet/intel/ice/ice_adminq_cmd.h   |   2 +
 drivers/net/ethernet/intel/ice/ice_dpll.c     | 186 ++++++++++++++++++
 2 files changed, 188 insertions(+)

diff --git a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
index bdee499f991a..7fd0f0091d36 100644
--- a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
+++ b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
@@ -2288,6 +2288,8 @@ struct ice_aqc_get_cgu_abilities {
 	u8 rsvd[3];
 };
 
+#define ICE_AQC_CGU_IN_CFG_FLG2_REFSYNC_EN		BIT(7)
+
 /* Set CGU input config (direct 0x0C62) */
 struct ice_aqc_set_cgu_input_config {
 	u8 input_idx;
diff --git a/drivers/net/ethernet/intel/ice/ice_dpll.c b/drivers/net/ethernet/intel/ice/ice_dpll.c
index bce3ad6ca2a6..98f0c86f41fc 100644
--- a/drivers/net/ethernet/intel/ice/ice_dpll.c
+++ b/drivers/net/ethernet/intel/ice/ice_dpll.c
@@ -12,6 +12,19 @@
 #define ICE_DPLL_PIN_ESYNC_PULSE_HIGH_PERCENT	25
 #define ICE_DPLL_PIN_GEN_RCLK_FREQ		1953125
 
+#define ICE_SR_PFA_DPLL_DEFAULTS		0x152
+#define ICE_DPLL_PFA_REF_SYNC_TYPE		0x2420
+#define ICE_DPLL_PFA_REF_SYNC_TYPE2		0x2424
+#define ICE_DPLL_PFA_END			0xFFFF
+#define ICE_DPLL_PFA_HEADER_LEN			4
+#define ICE_DPLL_PFA_ENTRY_LEN			3
+#define ICE_DPLL_PFA_MAILBOX_REF_SYNC_PIN_S	4
+#define ICE_DPLL_PFA_MASK_OFFSET		1
+#define ICE_DPLL_PFA_VALUE_OFFSET		2
+
+#define ICE_DPLL_E810C_SFP_NC_PINS		2
+#define ICE_DPLL_E810C_SFP_NC_START		4
+
 /**
  * enum ice_dpll_pin_type - enumerate ice pin types:
  * @ICE_DPLL_PIN_INVALID: invalid pin type
@@ -1314,6 +1327,89 @@ ice_dpll_input_esync_get(const struct dpll_pin *pin, void *pin_priv,
 	return 0;
 }
 
+/**
+ * ice_dpll_input_ref_sync_set - callback for setting reference sync feature
+ * @pin: pointer to a pin
+ * @pin_priv: private data pointer passed on pin registration
+ * @ref_pin: pin pointer for reference sync pair
+ * @ref_pin_priv: private data pointer of ref_pin
+ * @state: requested state for reference sync for pin pair
+ * @extack: error reporting
+ *
+ * Dpll subsystem callback. Handler for setting reference sync frequency
+ * feature for input pin.
+ *
+ * Context: Acquires and releases pf->dplls.lock
+ * Return:
+ * * 0 - success
+ * * negative - error
+ */
+static int
+ice_dpll_input_ref_sync_set(const struct dpll_pin *pin, void *pin_priv,
+			    const struct dpll_pin *ref_pin, void *ref_pin_priv,
+			    const enum dpll_pin_state state,
+			    struct netlink_ext_ack *extack)
+{
+	struct ice_dpll_pin *p = pin_priv;
+	struct ice_pf *pf = p->pf;
+	u8 flags_en = 0;
+	int ret;
+
+	if (ice_dpll_is_reset(pf, extack))
+		return -EBUSY;
+	mutex_lock(&pf->dplls.lock);
+
+	if (p->flags[0] & ICE_AQC_GET_CGU_IN_CFG_FLG2_INPUT_EN)
+		flags_en = ICE_AQC_SET_CGU_IN_CFG_FLG2_INPUT_EN;
+	if (state == DPLL_PIN_STATE_CONNECTED)
+		flags_en |= ICE_AQC_CGU_IN_CFG_FLG2_REFSYNC_EN;
+	ret = ice_aq_set_input_pin_cfg(&pf->hw, p->idx, 0, flags_en, 0, 0);
+	if (!ret)
+		ret = ice_dpll_pin_state_update(pf, p, ICE_DPLL_PIN_TYPE_INPUT,
+						extack);
+	mutex_unlock(&pf->dplls.lock);
+
+	return ret;
+}
+
+/**
+ * ice_dpll_input_ref_sync_get - callback for getting reference sync config
+ * @pin: pointer to a pin
+ * @pin_priv: private data pointer passed on pin registration
+ * @ref_pin: pin pointer for reference sync pair
+ * @ref_pin_priv: private data pointer of ref_pin
+ * @state: on success holds reference sync state for pin pair
+ * @extack: error reporting
+ *
+ * Dpll subsystem callback. Handler for setting reference sync frequency
+ * feature for input pin.
+ *
+ * Context: Acquires and releases pf->dplls.lock
+ * Return:
+ * * 0 - success
+ * * negative - error
+ */
+static int
+ice_dpll_input_ref_sync_get(const struct dpll_pin *pin, void *pin_priv,
+			    const struct dpll_pin *ref_pin, void *ref_pin_priv,
+			    enum dpll_pin_state *state,
+			    struct netlink_ext_ack *extack)
+{
+	struct ice_dpll_pin *p = pin_priv;
+	struct ice_pf *pf = p->pf;
+
+	if (ice_dpll_is_reset(pf, extack))
+		return -EBUSY;
+	mutex_lock(&pf->dplls.lock);
+	if (p->flags[0] & ICE_AQC_CGU_IN_CFG_FLG2_REFSYNC_EN)
+		*state = DPLL_PIN_STATE_CONNECTED;
+	else
+		*state = DPLL_PIN_STATE_DISCONNECTED;
+	mutex_unlock(&pf->dplls.lock);
+
+	return 0;
+}
+
 /**
  * ice_dpll_rclk_state_on_pin_set - set a state on rclk pin
  * @pin: pointer to a pin
@@ -1440,6 +1536,8 @@ static const struct dpll_pin_ops ice_dpll_input_ops = {
 	.phase_offset_get = ice_dpll_phase_offset_get,
 	.esync_set = ice_dpll_input_esync_set,
 	.esync_get = ice_dpll_input_esync_get,
+	.ref_sync_set = ice_dpll_input_ref_sync_set,
+	.ref_sync_get = ice_dpll_input_ref_sync_get,
 };
 
 static const struct dpll_pin_ops ice_dpll_output_ops = {
@@ -1619,6 +1717,91 @@ static void ice_dpll_periodic_work(struct kthread_work *work)
 				   msecs_to_jiffies(500));
 }
 
+/**
+ * ice_dpll_init_ref_sync_inputs - initialize reference sync pin pairs
+ * @pf: pf private structure
+ *
+ * Read DPLL TLV capabilities and initialize reference sync pin pairs in
+ * dpll subsystem.
+ *
+ * Return:
+ * * 0 - success or nothing to do (no ref-sync tlv are present)
+ * * negative - AQ failure
+ */
+static int ice_dpll_init_ref_sync_inputs(struct ice_pf *pf)
+{
+	struct ice_dpll_pin *inputs = pf->dplls.inputs;
+	struct ice_hw *hw = &pf->hw;
+	u16 addr, len, end, hdr;
+	int ret;
+
+	ret = ice_get_pfa_module_tlv(hw, &hdr, &len, ICE_SR_PFA_DPLL_DEFAULTS);
+	if (ret) {
+		dev_err(ice_pf_to_dev(pf),
+			"Failed to read PFA dpll defaults TLV ret=%d\n", ret);
+		return ret;
+	}
+	end = hdr + len;
+
+	for (addr = hdr + ICE_DPLL_PFA_HEADER_LEN; addr < end;
+	     addr += ICE_DPLL_PFA_ENTRY_LEN) {
+		unsigned long bit, ul_mask, offset;
+		u16 pin, mask, buf;
+		bool valid = false;
+
+		ret = ice_read_sr_word(hw, addr, &buf);
+		if (ret)
+			return ret;
+
+		switch (buf) {
+		case ICE_DPLL_PFA_REF_SYNC_TYPE:
+		case ICE_DPLL_PFA_REF_SYNC_TYPE2:
+		{
+			u16 mask_addr = addr + ICE_DPLL_PFA_MASK_OFFSET;
+			u16 val_addr = addr + ICE_DPLL_PFA_VALUE_OFFSET;
+
+			ret = ice_read_sr_word(hw, mask_addr, &mask);
+			if (ret)
+				return ret;
+			ret = ice_read_sr_word(hw, val_addr, &pin);
+			if (ret)
+				return ret;
+			if (buf == ICE_DPLL_PFA_REF_SYNC_TYPE)
+				pin >>= ICE_DPLL_PFA_MAILBOX_REF_SYNC_PIN_S;
+			valid = true;
+			break;
+		}
+		case ICE_DPLL_PFA_END:
+			addr = end;
+			break;
+		default:
+			continue;
+		}
+		if (!valid)
+			continue;
+
+		ul_mask = mask;
+		offset = 0;
+		for_each_set_bit(bit, &ul_mask, BITS_PER_TYPE(u16)) {
+			int i, j;
+
+			if (hw->device_id == ICE_DEV_ID_E810C_SFP &&
+			    pin > ICE_DPLL_E810C_SFP_NC_START)
+				offset = -ICE_DPLL_E810C_SFP_NC_PINS;
+			i = pin + offset;
+			j = bit + offset;
+			if (i < 0 || j < 0)
+				return -ERANGE;
+			ret = dpll_pin_ref_sync_pair_add(inputs[i].pin,
+							 inputs[j].pin);
+			if (ret)
+				return ret;
+		}
+	}
+
+	return 0;
+}
+
 /**
  * ice_dpll_release_pins - release pins resources from dpll subsystem
  * @pins: pointer to pins array
@@ -1936,6 +2119,9 @@ static int ice_dpll_init_pins(struct ice_pf *pf, bool cgu)
 	if (ret)
 		return ret;
 	if (cgu) {
+		ret = ice_dpll_init_ref_sync_inputs(pf);
+		if (ret)
+			goto deinit_inputs;
 		ret = ice_dpll_init_direct_pins(pf, cgu, pf->dplls.outputs,
 						pf->dplls.num_inputs,
 						pf->dplls.num_outputs,
-- 
2.38.1