Instead of passing list of properties as arguments to
dpll_device_register(..) use a dedicated struct.

Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
---
v2:
- new commit
---
 drivers/dpll/dpll_core.c                      | 34 ++++++++++++-------
 drivers/dpll/dpll_core.h                      |  2 +-
 drivers/dpll/dpll_netlink.c                   |  7 ++--
 drivers/net/ethernet/intel/ice/ice_dpll.c     | 16 +++++----
 drivers/net/ethernet/intel/ice/ice_dpll.h     |  1 +
 .../net/ethernet/mellanox/mlx5/core/dpll.c    | 10 +++---
 drivers/ptp/ptp_ocp.c                         |  7 ++--
 include/linux/dpll.h                          | 11 ++++--
 8 files changed, 57 insertions(+), 31 deletions(-)

diff --git a/drivers/dpll/dpll_core.c b/drivers/dpll/dpll_core.c
index 20bdc52f63a5..af9cda45a89c 100644
--- a/drivers/dpll/dpll_core.c
+++ b/drivers/dpll/dpll_core.c
@@ -34,7 +34,7 @@ static u32 dpll_pin_xa_id;
 
 struct dpll_device_registration {
 	struct list_head list;
-	const struct dpll_device_ops *ops;
+	const struct dpll_device_info *info;
 	void *priv;
 };
 
@@ -327,12 +327,12 @@ EXPORT_SYMBOL_GPL(dpll_device_put);
 
 static struct dpll_device_registration *
 dpll_device_registration_find(struct dpll_device *dpll,
-			      const struct dpll_device_ops *ops, void *priv)
+			      const struct dpll_device_info *info, void *priv)
 {
 	struct dpll_device_registration *reg;
 
 	list_for_each_entry(reg, &dpll->registration_list, list) {
-		if (reg->ops == ops && reg->priv == priv)
+		if (reg->info == info && reg->priv == priv)
 			return reg;
 	}
 	return NULL;
@@ -341,8 +341,7 @@ dpll_device_registration_find(struct dpll_device *dpll,
 /**
  * dpll_device_register - register the dpll device in the subsystem
  * @dpll: pointer to a dpll
- * @type: type of a dpll
- * @ops: ops for a dpll device
+ * @info: dpll device information and operations from registerer
  * @priv: pointer to private information of owner
  *
  * Make dpll device available for user space.
@@ -352,11 +351,13 @@ dpll_device_registration_find(struct dpll_device *dpll,
  * * 0 on success
  * * negative - error value
  */
-int dpll_device_register(struct dpll_device *dpll, enum dpll_type type,
-			 const struct dpll_device_ops *ops, void *priv)
+int dpll_device_register(struct dpll_device *dpll,
+			 const struct dpll_device_info *info, void *priv)
 {
+	const struct dpll_device_ops *ops = info->ops;
 	struct dpll_device_registration *reg;
 	bool first_registration = false;
+	enum dpll_type type = info->type;
 
 	if (WARN_ON(!ops))
 		return -EINVAL;
@@ -368,7 +369,7 @@ int dpll_device_register(struct dpll_device *dpll, enum dpll_type type,
 		return -EINVAL;
 
 	mutex_lock(&dpll_lock);
-	reg = dpll_device_registration_find(dpll, ops, priv);
+	reg = dpll_device_registration_find(dpll, info, priv);
 	if (reg) {
 		mutex_unlock(&dpll_lock);
 		return -EEXIST;
@@ -379,9 +380,8 @@ int dpll_device_register(struct dpll_device *dpll, enum dpll_type type,
 		mutex_unlock(&dpll_lock);
 		return -ENOMEM;
 	}
-	reg->ops = ops;
+	reg->info = info;
 	reg->priv = priv;
-	dpll->type = type;
 	first_registration = list_empty(&dpll->registration_list);
 	list_add_tail(&reg->list, &dpll->registration_list);
 	if (!first_registration) {
@@ -408,14 +408,14 @@ EXPORT_SYMBOL_GPL(dpll_device_register);
  * Context: Acquires a lock (dpll_lock)
  */
 void dpll_device_unregister(struct dpll_device *dpll,
-			    const struct dpll_device_ops *ops, void *priv)
+			    const struct dpll_device_info *info, void *priv)
 {
 	struct dpll_device_registration *reg;
 
 	mutex_lock(&dpll_lock);
 	ASSERT_DPLL_REGISTERED(dpll);
 	dpll_device_delete_ntf(dpll);
-	reg = dpll_device_registration_find(dpll, ops, priv);
+	reg = dpll_device_registration_find(dpll, info, priv);
 	if (WARN_ON(!reg)) {
 		mutex_unlock(&dpll_lock);
 		return;
@@ -807,7 +807,15 @@ const struct dpll_device_ops *dpll_device_ops(struct dpll_device *dpll)
 	struct dpll_device_registration *reg;
 
 	reg = dpll_device_registration_first(dpll);
-	return reg->ops;
+	return reg->info->ops;
+}
+
+const struct dpll_device_info *dpll_device_info(struct dpll_device *dpll)
+{
+	struct dpll_device_registration *reg;
+
+	reg = dpll_device_registration_first(dpll);
+	return reg->info;
 }
 
 static struct dpll_pin_registration *
diff --git a/drivers/dpll/dpll_core.h b/drivers/dpll/dpll_core.h
index 2b6d8ef1cdf3..baeb10d7dc1e 100644
--- a/drivers/dpll/dpll_core.h
+++ b/drivers/dpll/dpll_core.h
@@ -30,7 +30,6 @@ struct dpll_device {
 	u32 device_idx;
 	u64 clock_id;
 	struct module *module;
-	enum dpll_type type;
 	struct xarray pin_refs;
 	refcount_t refcount;
 	struct list_head registration_list;
@@ -84,6 +83,7 @@ void *dpll_pin_on_pin_priv(struct dpll_pin *parent, struct dpll_pin *pin);
 const struct dpll_device_ops *dpll_device_ops(struct dpll_device *dpll);
 struct dpll_device *dpll_device_get_by_id(int id);
 const struct dpll_pin_ops *dpll_pin_ops(struct dpll_pin_ref *ref);
+const struct dpll_device_info *dpll_device_info(struct dpll_device *dpll);
 struct dpll_pin_ref *dpll_xa_ref_dpll_first(struct xarray *xa_refs);
 extern struct xarray dpll_device_xa;
 extern struct xarray dpll_pin_xa;
diff --git a/drivers/dpll/dpll_netlink.c b/drivers/dpll/dpll_netlink.c
index c130f87147fa..2de9ec08d551 100644
--- a/drivers/dpll/dpll_netlink.c
+++ b/drivers/dpll/dpll_netlink.c
@@ -564,6 +564,7 @@ static int
 dpll_device_get_one(struct dpll_device *dpll, struct sk_buff *msg,
 		    struct netlink_ext_ack *extack)
 {
+	const struct dpll_device_info *info = dpll_device_info(dpll);
 	int ret;
 
 	ret = dpll_msg_add_dev_handle(msg, dpll);
@@ -589,7 +590,7 @@ dpll_device_get_one(struct dpll_device *dpll, struct sk_buff *msg,
 	ret = dpll_msg_add_mode_supported(msg, dpll, extack);
 	if (ret)
 		return ret;
-	if (nla_put_u32(msg, DPLL_A_TYPE, dpll->type))
+	if (nla_put_u32(msg, DPLL_A_TYPE, info->type))
 		return -EMSGSIZE;
 
 	return 0;
@@ -1415,11 +1416,13 @@ dpll_device_find(u64 clock_id, struct nlattr *mod_name_attr,
 	unsigned long i;
 
 	xa_for_each_marked(&dpll_device_xa, i, dpll, DPLL_REGISTERED) {
+		const struct dpll_device_info *info = dpll_device_info(dpll);
+
 		cid_match = clock_id ? dpll->clock_id == clock_id : true;
 		mod_match = mod_name_attr ? (module_name(dpll->module) ?
 			!nla_strcmp(mod_name_attr,
 				    module_name(dpll->module)) : false) : true;
-		type_match = type ? dpll->type == type : true;
+		type_match = type ? info->type == type : true;
 		if (cid_match && mod_match && type_match) {
 			if (dpll_match) {
 				NL_SET_ERR_MSG(extack, "multiple matches");
diff --git a/drivers/net/ethernet/intel/ice/ice_dpll.c b/drivers/net/ethernet/intel/ice/ice_dpll.c
index bce3ad6ca2a6..0f7440a889ac 100644
--- a/drivers/net/ethernet/intel/ice/ice_dpll.c
+++ b/drivers/net/ethernet/intel/ice/ice_dpll.c
@@ -1977,7 +1977,7 @@ static void
 ice_dpll_deinit_dpll(struct ice_pf *pf, struct ice_dpll *d, bool cgu)
 {
 	if (cgu)
-		dpll_device_unregister(d->dpll, &ice_dpll_ops, d);
+		dpll_device_unregister(d->dpll, &d->info, d);
 	dpll_device_put(d->dpll);
 }
 
@@ -1996,8 +1996,7 @@ ice_dpll_deinit_dpll(struct ice_pf *pf, struct ice_dpll *d, bool cgu)
  * * negative - initialization failure reason
  */
 static int
-ice_dpll_init_dpll(struct ice_pf *pf, struct ice_dpll *d, bool cgu,
-		   enum dpll_type type)
+ice_dpll_init_dpll(struct ice_pf *pf, struct ice_dpll *d, bool cgu)
 {
 	u64 clock_id = pf->dplls.clock_id;
 	int ret;
@@ -2012,7 +2011,7 @@ ice_dpll_init_dpll(struct ice_pf *pf, struct ice_dpll *d, bool cgu,
 	d->pf = pf;
 	if (cgu) {
 		ice_dpll_update_state(pf, d, true);
-		ret = dpll_device_register(d->dpll, type, &ice_dpll_ops, d);
+		ret = dpll_device_register(d->dpll, &d->info, d);
 		if (ret) {
 			dpll_device_put(d->dpll);
 			return ret;
@@ -2363,7 +2362,12 @@ static int ice_dpll_init_info(struct ice_pf *pf, bool cgu)
 	if (ret)
 		return ret;
 	de->mode = DPLL_MODE_AUTOMATIC;
+	de->info.type = DPLL_TYPE_EEC;
+	de->info.ops = &ice_dpll_ops;
+
 	dp->mode = DPLL_MODE_AUTOMATIC;
+	dp->info.type = DPLL_TYPE_PPS;
+	dp->info.ops = &ice_dpll_ops;
 
 	dev_dbg(ice_pf_to_dev(pf),
 		"%s - success, inputs:%u, outputs:%u rclk-parents:%u\n",
@@ -2426,10 +2430,10 @@ void ice_dpll_init(struct ice_pf *pf)
 	err = ice_dpll_init_info(pf, cgu);
 	if (err)
 		goto err_exit;
-	err = ice_dpll_init_dpll(pf, &pf->dplls.eec, cgu, DPLL_TYPE_EEC);
+	err = ice_dpll_init_dpll(pf, &pf->dplls.eec, cgu);
 	if (err)
 		goto deinit_info;
-	err = ice_dpll_init_dpll(pf, &pf->dplls.pps, cgu, DPLL_TYPE_PPS);
+	err = ice_dpll_init_dpll(pf, &pf->dplls.pps, cgu);
 	if (err)
 		goto deinit_eec;
 	err = ice_dpll_init_pins(pf, cgu);
diff --git a/drivers/net/ethernet/intel/ice/ice_dpll.h b/drivers/net/ethernet/intel/ice/ice_dpll.h
index c320f1bf7d6d..9db7463e293a 100644
--- a/drivers/net/ethernet/intel/ice/ice_dpll.h
+++ b/drivers/net/ethernet/intel/ice/ice_dpll.h
@@ -66,6 +66,7 @@ struct ice_dpll {
 	enum dpll_mode mode;
 	struct dpll_pin *active_input;
 	struct dpll_pin *prev_input;
+	struct dpll_device_info info;
 };
 
 /** ice_dplls - store info required for CCU (clock controlling unit)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/dpll.c b/drivers/net/ethernet/mellanox/mlx5/core/dpll.c
index 1e5522a19483..f722b1de0754 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/dpll.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/dpll.c
@@ -20,6 +20,7 @@ struct mlx5_dpll {
 	} last;
 	struct notifier_block mdev_nb;
 	struct net_device *tracking_netdev;
+	struct dpll_device_info info;
 };
 
 static int mlx5_dpll_clock_id_get(struct mlx5_core_dev *mdev, u64 *clock_id)
@@ -444,8 +445,9 @@ static int mlx5_dpll_probe(struct auxiliary_device *adev,
 		goto err_free_mdpll;
 	}
 
-	err = dpll_device_register(mdpll->dpll, DPLL_TYPE_EEC,
-				   &mlx5_dpll_device_ops, mdpll);
+	mdpll->info.type = DPLL_TYPE_EEC;
+	mdpll->info.ops = &mlx5_dpll_device_ops;
+	err = dpll_device_register(mdpll->dpll, &mdpll->info, mdpll);
 	if (err)
 		goto err_put_dpll_device;
 
@@ -481,7 +483,7 @@ static int mlx5_dpll_probe(struct auxiliary_device *adev,
 err_put_dpll_pin:
 	dpll_pin_put(mdpll->dpll_pin);
 err_unregister_dpll_device:
-	dpll_device_unregister(mdpll->dpll, &mlx5_dpll_device_ops, mdpll);
+	dpll_device_unregister(mdpll->dpll, &mdpll->info, mdpll);
 err_put_dpll_device:
 	dpll_device_put(mdpll->dpll);
 err_free_mdpll:
@@ -500,7 +502,7 @@ static void mlx5_dpll_remove(struct auxiliary_device *adev)
 	dpll_pin_unregister(mdpll->dpll, mdpll->dpll_pin,
 			    &mlx5_dpll_pins_ops, mdpll);
 	dpll_pin_put(mdpll->dpll_pin);
-	dpll_device_unregister(mdpll->dpll, &mlx5_dpll_device_ops, mdpll);
+	dpll_device_unregister(mdpll->dpll, &mdpll->info, mdpll);
 	dpll_device_put(mdpll->dpll);
 	kfree(mdpll);
 
diff --git a/drivers/ptp/ptp_ocp.c b/drivers/ptp/ptp_ocp.c
index 7945c6be1f7c..b3c5d294acb4 100644
--- a/drivers/ptp/ptp_ocp.c
+++ b/drivers/ptp/ptp_ocp.c
@@ -382,6 +382,7 @@ struct ptp_ocp {
 	struct ptp_ocp_sma_connector sma[OCP_SMA_NUM];
 	const struct ocp_sma_op *sma_op;
 	struct dpll_device *dpll;
+	struct dpll_device_info	dpll_info;
 };
 
 #define OCP_REQ_TIMESTAMP	BIT(0)
@@ -4745,7 +4746,9 @@ ptp_ocp_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 		goto out;
 	}
 
-	err = dpll_device_register(bp->dpll, DPLL_TYPE_PPS, &dpll_ops, bp);
+	bp->dpll_info.type = DPLL_TYPE_PPS;
+	bp->dpll_info.ops = &dpll_ops;
+	err = dpll_device_register(bp->dpll, &bp->dpll_info, bp);
 	if (err)
 		goto out;
 
@@ -4796,7 +4799,7 @@ ptp_ocp_remove(struct pci_dev *pdev)
 			dpll_pin_put(bp->sma[i].dpll_pin);
 		}
 	}
-	dpll_device_unregister(bp->dpll, &dpll_ops, bp);
+	dpll_device_unregister(bp->dpll, &bp->dpll_info, bp);
 	dpll_device_put(bp->dpll);
 	devlink_unregister(devlink);
 	ptp_ocp_detach(bp);
diff --git a/include/linux/dpll.h b/include/linux/dpll.h
index 5e4f9ab1cf75..0489464af958 100644
--- a/include/linux/dpll.h
+++ b/include/linux/dpll.h
@@ -97,6 +97,11 @@ struct dpll_pin_ops {
 			 struct netlink_ext_ack *extack);
 };
 
+struct dpll_device_info {
+	enum dpll_type type;
+	const struct dpll_device_ops *ops;
+};
+
 struct dpll_pin_frequency {
 	u64 min;
 	u64 max;
@@ -170,11 +175,11 @@ dpll_device_get(u64 clock_id, u32 dev_driver_id, struct module *module);
 
 void dpll_device_put(struct dpll_device *dpll);
 
-int dpll_device_register(struct dpll_device *dpll, enum dpll_type type,
-			 const struct dpll_device_ops *ops, void *priv);
+int dpll_device_register(struct dpll_device *dpll,
+			 const struct dpll_device_info *info, void *priv);
 
 void dpll_device_unregister(struct dpll_device *dpll,
-			    const struct dpll_device_ops *ops, void *priv);
+			    const struct dpll_device_info *info, void *priv);
 
 struct dpll_pin *
 dpll_pin_get(u64 clock_id, u32 dev_driver_id, struct module *module,
-- 
2.38.1

On Tue, 15 Apr 2025 20:15:40 +0200 Arkadiusz Kubalewski wrote:
> @@ -408,14 +408,14 @@ EXPORT_SYMBOL_GPL(dpll_device_register);
>   * Context: Acquires a lock (dpll_lock)
>   */
>  void dpll_device_unregister(struct dpll_device *dpll,
> -			    const struct dpll_device_ops *ops, void *priv)
> +			    const struct dpll_device_info *info, void *priv)

Some kdoc unhappiness here, W=1 build should lead you to it.

>From: Jakub Kicinski <kuba@kernel.org>
>Sent: Thursday, April 17, 2025 4:09 AM
>
>On Tue, 15 Apr 2025 20:15:40 +0200 Arkadiusz Kubalewski wrote:
>> @@ -408,14 +408,14 @@ EXPORT_SYMBOL_GPL(dpll_device_register);
>>   * Context: Acquires a lock (dpll_lock)
>>   */
>>  void dpll_device_unregister(struct dpll_device *dpll,
>> -			    const struct dpll_device_ops *ops, void *priv)
>> +			    const struct dpll_device_info *info, void *priv)
>
>Some kdoc unhappiness here, W=1 build should lead you to it.

Sure, will fix it.

Thank you!
Arkadiusz

Tue, Apr 15, 2025 at 08:15:40PM +0200, arkadiusz.kubalewski@intel.com wrote:
>Instead of passing list of properties as arguments to
>dpll_device_register(..) use a dedicated struct.
>
>Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
>---
>v2:
>- new commit
>---
> drivers/dpll/dpll_core.c                      | 34 ++++++++++++-------
> drivers/dpll/dpll_core.h                      |  2 +-
> drivers/dpll/dpll_netlink.c                   |  7 ++--
> drivers/net/ethernet/intel/ice/ice_dpll.c     | 16 +++++----
> drivers/net/ethernet/intel/ice/ice_dpll.h     |  1 +
> .../net/ethernet/mellanox/mlx5/core/dpll.c    | 10 +++---
> drivers/ptp/ptp_ocp.c                         |  7 ++--
> include/linux/dpll.h                          | 11 ++++--
> 8 files changed, 57 insertions(+), 31 deletions(-)
>
>diff --git a/drivers/dpll/dpll_core.c b/drivers/dpll/dpll_core.c
>index 20bdc52f63a5..af9cda45a89c 100644
>--- a/drivers/dpll/dpll_core.c
>+++ b/drivers/dpll/dpll_core.c
>@@ -34,7 +34,7 @@ static u32 dpll_pin_xa_id;
> 
> struct dpll_device_registration {
> 	struct list_head list;
>-	const struct dpll_device_ops *ops;
>+	const struct dpll_device_info *info;
> 	void *priv;
> };
> 
>@@ -327,12 +327,12 @@ EXPORT_SYMBOL_GPL(dpll_device_put);
> 
> static struct dpll_device_registration *
> dpll_device_registration_find(struct dpll_device *dpll,
>-			      const struct dpll_device_ops *ops, void *priv)
>+			      const struct dpll_device_info *info, void *priv)
> {
> 	struct dpll_device_registration *reg;
> 
> 	list_for_each_entry(reg, &dpll->registration_list, list) {
>-		if (reg->ops == ops && reg->priv == priv)
>+		if (reg->info == info && reg->priv == priv)
> 			return reg;
> 	}
> 	return NULL;
>@@ -341,8 +341,7 @@ dpll_device_registration_find(struct dpll_device *dpll,
> /**
>  * dpll_device_register - register the dpll device in the subsystem
>  * @dpll: pointer to a dpll
>- * @type: type of a dpll
>- * @ops: ops for a dpll device
>+ * @info: dpll device information and operations from registerer
>  * @priv: pointer to private information of owner
>  *
>  * Make dpll device available for user space.
>@@ -352,11 +351,13 @@ dpll_device_registration_find(struct dpll_device *dpll,
>  * * 0 on success
>  * * negative - error value
>  */
>-int dpll_device_register(struct dpll_device *dpll, enum dpll_type type,
>-			 const struct dpll_device_ops *ops, void *priv)
>+int dpll_device_register(struct dpll_device *dpll,
>+			 const struct dpll_device_info *info, void *priv)

I don't like this. If you need some capabilities value, put it into ops
struct.


> {
>+	const struct dpll_device_ops *ops = info->ops;
> 	struct dpll_device_registration *reg;
> 	bool first_registration = false;
>+	enum dpll_type type = info->type;
> 
> 	if (WARN_ON(!ops))
> 		return -EINVAL;
>@@ -368,7 +369,7 @@ int dpll_device_register(struct dpll_device *dpll, enum dpll_type type,
> 		return -EINVAL;
> 
> 	mutex_lock(&dpll_lock);
>-	reg = dpll_device_registration_find(dpll, ops, priv);
>+	reg = dpll_device_registration_find(dpll, info, priv);
> 	if (reg) {
> 		mutex_unlock(&dpll_lock);
> 		return -EEXIST;
>@@ -379,9 +380,8 @@ int dpll_device_register(struct dpll_device *dpll, enum dpll_type type,
> 		mutex_unlock(&dpll_lock);
> 		return -ENOMEM;
> 	}
>-	reg->ops = ops;
>+	reg->info = info;
> 	reg->priv = priv;
>-	dpll->type = type;
> 	first_registration = list_empty(&dpll->registration_list);
> 	list_add_tail(&reg->list, &dpll->registration_list);
> 	if (!first_registration) {
>@@ -408,14 +408,14 @@ EXPORT_SYMBOL_GPL(dpll_device_register);
>  * Context: Acquires a lock (dpll_lock)
>  */
> void dpll_device_unregister(struct dpll_device *dpll,
>-			    const struct dpll_device_ops *ops, void *priv)
>+			    const struct dpll_device_info *info, void *priv)
> {
> 	struct dpll_device_registration *reg;
> 
> 	mutex_lock(&dpll_lock);
> 	ASSERT_DPLL_REGISTERED(dpll);
> 	dpll_device_delete_ntf(dpll);
>-	reg = dpll_device_registration_find(dpll, ops, priv);
>+	reg = dpll_device_registration_find(dpll, info, priv);
> 	if (WARN_ON(!reg)) {
> 		mutex_unlock(&dpll_lock);
> 		return;
>@@ -807,7 +807,15 @@ const struct dpll_device_ops *dpll_device_ops(struct dpll_device *dpll)
> 	struct dpll_device_registration *reg;
> 
> 	reg = dpll_device_registration_first(dpll);
>-	return reg->ops;
>+	return reg->info->ops;
>+}
>+
>+const struct dpll_device_info *dpll_device_info(struct dpll_device *dpll)

Makes me wonder what you would need this for. I guess "nothing"?


>+{
>+	struct dpll_device_registration *reg;
>+
>+	reg = dpll_device_registration_first(dpll);
>+	return reg->info;
> }
> 
> static struct dpll_pin_registration *
>diff --git a/drivers/dpll/dpll_core.h b/drivers/dpll/dpll_core.h
>index 2b6d8ef1cdf3..baeb10d7dc1e 100644
>--- a/drivers/dpll/dpll_core.h
>+++ b/drivers/dpll/dpll_core.h
>@@ -30,7 +30,6 @@ struct dpll_device {
> 	u32 device_idx;
> 	u64 clock_id;
> 	struct module *module;
>-	enum dpll_type type;
> 	struct xarray pin_refs;
> 	refcount_t refcount;
> 	struct list_head registration_list;
>@@ -84,6 +83,7 @@ void *dpll_pin_on_pin_priv(struct dpll_pin *parent, struct dpll_pin *pin);
> const struct dpll_device_ops *dpll_device_ops(struct dpll_device *dpll);
> struct dpll_device *dpll_device_get_by_id(int id);
> const struct dpll_pin_ops *dpll_pin_ops(struct dpll_pin_ref *ref);
>+const struct dpll_device_info *dpll_device_info(struct dpll_device *dpll);
> struct dpll_pin_ref *dpll_xa_ref_dpll_first(struct xarray *xa_refs);
> extern struct xarray dpll_device_xa;
> extern struct xarray dpll_pin_xa;
>diff --git a/drivers/dpll/dpll_netlink.c b/drivers/dpll/dpll_netlink.c
>index c130f87147fa..2de9ec08d551 100644
>--- a/drivers/dpll/dpll_netlink.c
>+++ b/drivers/dpll/dpll_netlink.c
>@@ -564,6 +564,7 @@ static int
> dpll_device_get_one(struct dpll_device *dpll, struct sk_buff *msg,
> 		    struct netlink_ext_ack *extack)
> {
>+	const struct dpll_device_info *info = dpll_device_info(dpll);
> 	int ret;
> 
> 	ret = dpll_msg_add_dev_handle(msg, dpll);
>@@ -589,7 +590,7 @@ dpll_device_get_one(struct dpll_device *dpll, struct sk_buff *msg,
> 	ret = dpll_msg_add_mode_supported(msg, dpll, extack);
> 	if (ret)
> 		return ret;
>-	if (nla_put_u32(msg, DPLL_A_TYPE, dpll->type))
>+	if (nla_put_u32(msg, DPLL_A_TYPE, info->type))
> 		return -EMSGSIZE;
> 
> 	return 0;
>@@ -1415,11 +1416,13 @@ dpll_device_find(u64 clock_id, struct nlattr *mod_name_attr,
> 	unsigned long i;
> 
> 	xa_for_each_marked(&dpll_device_xa, i, dpll, DPLL_REGISTERED) {
>+		const struct dpll_device_info *info = dpll_device_info(dpll);
>+
> 		cid_match = clock_id ? dpll->clock_id == clock_id : true;
> 		mod_match = mod_name_attr ? (module_name(dpll->module) ?
> 			!nla_strcmp(mod_name_attr,
> 				    module_name(dpll->module)) : false) : true;
>-		type_match = type ? dpll->type == type : true;
>+		type_match = type ? info->type == type : true;
> 		if (cid_match && mod_match && type_match) {
> 			if (dpll_match) {
> 				NL_SET_ERR_MSG(extack, "multiple matches");
>diff --git a/drivers/net/ethernet/intel/ice/ice_dpll.c b/drivers/net/ethernet/intel/ice/ice_dpll.c
>index bce3ad6ca2a6..0f7440a889ac 100644
>--- a/drivers/net/ethernet/intel/ice/ice_dpll.c
>+++ b/drivers/net/ethernet/intel/ice/ice_dpll.c
>@@ -1977,7 +1977,7 @@ static void
> ice_dpll_deinit_dpll(struct ice_pf *pf, struct ice_dpll *d, bool cgu)
> {
> 	if (cgu)
>-		dpll_device_unregister(d->dpll, &ice_dpll_ops, d);
>+		dpll_device_unregister(d->dpll, &d->info, d);
> 	dpll_device_put(d->dpll);
> }
> 
>@@ -1996,8 +1996,7 @@ ice_dpll_deinit_dpll(struct ice_pf *pf, struct ice_dpll *d, bool cgu)
>  * * negative - initialization failure reason
>  */
> static int
>-ice_dpll_init_dpll(struct ice_pf *pf, struct ice_dpll *d, bool cgu,
>-		   enum dpll_type type)
>+ice_dpll_init_dpll(struct ice_pf *pf, struct ice_dpll *d, bool cgu)
> {
> 	u64 clock_id = pf->dplls.clock_id;
> 	int ret;
>@@ -2012,7 +2011,7 @@ ice_dpll_init_dpll(struct ice_pf *pf, struct ice_dpll *d, bool cgu,
> 	d->pf = pf;
> 	if (cgu) {
> 		ice_dpll_update_state(pf, d, true);
>-		ret = dpll_device_register(d->dpll, type, &ice_dpll_ops, d);
>+		ret = dpll_device_register(d->dpll, &d->info, d);
> 		if (ret) {
> 			dpll_device_put(d->dpll);
> 			return ret;
>@@ -2363,7 +2362,12 @@ static int ice_dpll_init_info(struct ice_pf *pf, bool cgu)
> 	if (ret)
> 		return ret;
> 	de->mode = DPLL_MODE_AUTOMATIC;
>+	de->info.type = DPLL_TYPE_EEC;
>+	de->info.ops = &ice_dpll_ops;
>+
> 	dp->mode = DPLL_MODE_AUTOMATIC;
>+	dp->info.type = DPLL_TYPE_PPS;
>+	dp->info.ops = &ice_dpll_ops;
> 
> 	dev_dbg(ice_pf_to_dev(pf),
> 		"%s - success, inputs:%u, outputs:%u rclk-parents:%u\n",
>@@ -2426,10 +2430,10 @@ void ice_dpll_init(struct ice_pf *pf)
> 	err = ice_dpll_init_info(pf, cgu);
> 	if (err)
> 		goto err_exit;
>-	err = ice_dpll_init_dpll(pf, &pf->dplls.eec, cgu, DPLL_TYPE_EEC);
>+	err = ice_dpll_init_dpll(pf, &pf->dplls.eec, cgu);
> 	if (err)
> 		goto deinit_info;
>-	err = ice_dpll_init_dpll(pf, &pf->dplls.pps, cgu, DPLL_TYPE_PPS);
>+	err = ice_dpll_init_dpll(pf, &pf->dplls.pps, cgu);
> 	if (err)
> 		goto deinit_eec;
> 	err = ice_dpll_init_pins(pf, cgu);
>diff --git a/drivers/net/ethernet/intel/ice/ice_dpll.h b/drivers/net/ethernet/intel/ice/ice_dpll.h
>index c320f1bf7d6d..9db7463e293a 100644
>--- a/drivers/net/ethernet/intel/ice/ice_dpll.h
>+++ b/drivers/net/ethernet/intel/ice/ice_dpll.h
>@@ -66,6 +66,7 @@ struct ice_dpll {
> 	enum dpll_mode mode;
> 	struct dpll_pin *active_input;
> 	struct dpll_pin *prev_input;
>+	struct dpll_device_info info;
> };
> 
> /** ice_dplls - store info required for CCU (clock controlling unit)
>diff --git a/drivers/net/ethernet/mellanox/mlx5/core/dpll.c b/drivers/net/ethernet/mellanox/mlx5/core/dpll.c
>index 1e5522a19483..f722b1de0754 100644
>--- a/drivers/net/ethernet/mellanox/mlx5/core/dpll.c
>+++ b/drivers/net/ethernet/mellanox/mlx5/core/dpll.c
>@@ -20,6 +20,7 @@ struct mlx5_dpll {
> 	} last;
> 	struct notifier_block mdev_nb;
> 	struct net_device *tracking_netdev;
>+	struct dpll_device_info info;
> };
> 
> static int mlx5_dpll_clock_id_get(struct mlx5_core_dev *mdev, u64 *clock_id)
>@@ -444,8 +445,9 @@ static int mlx5_dpll_probe(struct auxiliary_device *adev,
> 		goto err_free_mdpll;
> 	}
> 
>-	err = dpll_device_register(mdpll->dpll, DPLL_TYPE_EEC,
>-				   &mlx5_dpll_device_ops, mdpll);
>+	mdpll->info.type = DPLL_TYPE_EEC;
>+	mdpll->info.ops = &mlx5_dpll_device_ops;
>+	err = dpll_device_register(mdpll->dpll, &mdpll->info, mdpll);
> 	if (err)
> 		goto err_put_dpll_device;
> 
>@@ -481,7 +483,7 @@ static int mlx5_dpll_probe(struct auxiliary_device *adev,
> err_put_dpll_pin:
> 	dpll_pin_put(mdpll->dpll_pin);
> err_unregister_dpll_device:
>-	dpll_device_unregister(mdpll->dpll, &mlx5_dpll_device_ops, mdpll);
>+	dpll_device_unregister(mdpll->dpll, &mdpll->info, mdpll);
> err_put_dpll_device:
> 	dpll_device_put(mdpll->dpll);
> err_free_mdpll:
>@@ -500,7 +502,7 @@ static void mlx5_dpll_remove(struct auxiliary_device *adev)
> 	dpll_pin_unregister(mdpll->dpll, mdpll->dpll_pin,
> 			    &mlx5_dpll_pins_ops, mdpll);
> 	dpll_pin_put(mdpll->dpll_pin);
>-	dpll_device_unregister(mdpll->dpll, &mlx5_dpll_device_ops, mdpll);
>+	dpll_device_unregister(mdpll->dpll, &mdpll->info, mdpll);
> 	dpll_device_put(mdpll->dpll);
> 	kfree(mdpll);
> 
>diff --git a/drivers/ptp/ptp_ocp.c b/drivers/ptp/ptp_ocp.c
>index 7945c6be1f7c..b3c5d294acb4 100644
>--- a/drivers/ptp/ptp_ocp.c
>+++ b/drivers/ptp/ptp_ocp.c
>@@ -382,6 +382,7 @@ struct ptp_ocp {
> 	struct ptp_ocp_sma_connector sma[OCP_SMA_NUM];
> 	const struct ocp_sma_op *sma_op;
> 	struct dpll_device *dpll;
>+	struct dpll_device_info	dpll_info;
> };
> 
> #define OCP_REQ_TIMESTAMP	BIT(0)
>@@ -4745,7 +4746,9 @@ ptp_ocp_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> 		goto out;
> 	}
> 
>-	err = dpll_device_register(bp->dpll, DPLL_TYPE_PPS, &dpll_ops, bp);
>+	bp->dpll_info.type = DPLL_TYPE_PPS;
>+	bp->dpll_info.ops = &dpll_ops;
>+	err = dpll_device_register(bp->dpll, &bp->dpll_info, bp);
> 	if (err)
> 		goto out;
> 
>@@ -4796,7 +4799,7 @@ ptp_ocp_remove(struct pci_dev *pdev)
> 			dpll_pin_put(bp->sma[i].dpll_pin);
> 		}
> 	}
>-	dpll_device_unregister(bp->dpll, &dpll_ops, bp);
>+	dpll_device_unregister(bp->dpll, &bp->dpll_info, bp);
> 	dpll_device_put(bp->dpll);
> 	devlink_unregister(devlink);
> 	ptp_ocp_detach(bp);
>diff --git a/include/linux/dpll.h b/include/linux/dpll.h
>index 5e4f9ab1cf75..0489464af958 100644
>--- a/include/linux/dpll.h
>+++ b/include/linux/dpll.h
>@@ -97,6 +97,11 @@ struct dpll_pin_ops {
> 			 struct netlink_ext_ack *extack);
> };
> 
>+struct dpll_device_info {
>+	enum dpll_type type;
>+	const struct dpll_device_ops *ops;
>+};
>+
> struct dpll_pin_frequency {
> 	u64 min;
> 	u64 max;
>@@ -170,11 +175,11 @@ dpll_device_get(u64 clock_id, u32 dev_driver_id, struct module *module);
> 
> void dpll_device_put(struct dpll_device *dpll);
> 
>-int dpll_device_register(struct dpll_device *dpll, enum dpll_type type,
>-			 const struct dpll_device_ops *ops, void *priv);
>+int dpll_device_register(struct dpll_device *dpll,
>+			 const struct dpll_device_info *info, void *priv);
> 
> void dpll_device_unregister(struct dpll_device *dpll,
>-			    const struct dpll_device_ops *ops, void *priv);
>+			    const struct dpll_device_info *info, void *priv);
> 
> struct dpll_pin *
> dpll_pin_get(u64 clock_id, u32 dev_driver_id, struct module *module,
>-- 
>2.38.1
>

>From: Jiri Pirko <jiri@resnulli.us>
>Sent: Wednesday, April 16, 2025 2:13 PM
>
>Tue, Apr 15, 2025 at 08:15:40PM +0200, arkadiusz.kubalewski@intel.com
>wrote:
>>Instead of passing list of properties as arguments to
>>dpll_device_register(..) use a dedicated struct.
>>
>>Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
>>---
>>v2:
>>- new commit
>>---
>> drivers/dpll/dpll_core.c                      | 34 ++++++++++++-------
>> drivers/dpll/dpll_core.h                      |  2 +-
>> drivers/dpll/dpll_netlink.c                   |  7 ++--
>> drivers/net/ethernet/intel/ice/ice_dpll.c     | 16 +++++----
>> drivers/net/ethernet/intel/ice/ice_dpll.h     |  1 +
>> .../net/ethernet/mellanox/mlx5/core/dpll.c    | 10 +++---
>> drivers/ptp/ptp_ocp.c                         |  7 ++--
>> include/linux/dpll.h                          | 11 ++++--
>> 8 files changed, 57 insertions(+), 31 deletions(-)
>>
>>diff --git a/drivers/dpll/dpll_core.c b/drivers/dpll/dpll_core.c
>>index 20bdc52f63a5..af9cda45a89c 100644
>>--- a/drivers/dpll/dpll_core.c
>>+++ b/drivers/dpll/dpll_core.c
>>@@ -34,7 +34,7 @@ static u32 dpll_pin_xa_id;
>>
>> struct dpll_device_registration {
>> 	struct list_head list;
>>-	const struct dpll_device_ops *ops;
>>+	const struct dpll_device_info *info;
>> 	void *priv;
>> };
>>
>>@@ -327,12 +327,12 @@ EXPORT_SYMBOL_GPL(dpll_device_put);
>>
>> static struct dpll_device_registration *
>> dpll_device_registration_find(struct dpll_device *dpll,
>>-			      const struct dpll_device_ops *ops, void *priv)
>>+			      const struct dpll_device_info *info, void *priv)
>> {
>> 	struct dpll_device_registration *reg;
>>
>> 	list_for_each_entry(reg, &dpll->registration_list, list) {
>>-		if (reg->ops == ops && reg->priv == priv)
>>+		if (reg->info == info && reg->priv == priv)
>> 			return reg;
>> 	}
>> 	return NULL;
>>@@ -341,8 +341,7 @@ dpll_device_registration_find(struct dpll_device
>>*dpll,
>> /**
>>  * dpll_device_register - register the dpll device in the subsystem
>>  * @dpll: pointer to a dpll
>>- * @type: type of a dpll
>>- * @ops: ops for a dpll device
>>+ * @info: dpll device information and operations from registerer
>>  * @priv: pointer to private information of owner
>>  *
>>  * Make dpll device available for user space.
>>@@ -352,11 +351,13 @@ dpll_device_registration_find(struct dpll_device
>>*dpll,
>>  * * 0 on success
>>  * * negative - error value
>>  */
>>-int dpll_device_register(struct dpll_device *dpll, enum dpll_type type,
>>-			 const struct dpll_device_ops *ops, void *priv)
>>+int dpll_device_register(struct dpll_device *dpll,
>>+			 const struct dpll_device_info *info, void *priv)
>
>I don't like this. If you need some capabilities value, put it into ops
>struct.
>

Hmm, this would seems strange, the _ops indicates operations, would
have to rename the struct..

In theory I could decide on capabilities per ops provided from driver..
i.e. If phase_input_monitor_feature_set()/phase_input_feature_get() are
present then capability phase_input_monitor is provided..
Makes sense?

>
>> {
>>+	const struct dpll_device_ops *ops = info->ops;
>> 	struct dpll_device_registration *reg;
>> 	bool first_registration = false;
>>+	enum dpll_type type = info->type;
>>
>> 	if (WARN_ON(!ops))
>> 		return -EINVAL;
>>@@ -368,7 +369,7 @@ int dpll_device_register(struct dpll_device *dpll,
>>enum dpll_type type,
>> 		return -EINVAL;
>>
>> 	mutex_lock(&dpll_lock);
>>-	reg = dpll_device_registration_find(dpll, ops, priv);
>>+	reg = dpll_device_registration_find(dpll, info, priv);
>> 	if (reg) {
>> 		mutex_unlock(&dpll_lock);
>> 		return -EEXIST;
>>@@ -379,9 +380,8 @@ int dpll_device_register(struct dpll_device *dpll,
>>enum dpll_type type,
>> 		mutex_unlock(&dpll_lock);
>> 		return -ENOMEM;
>> 	}
>>-	reg->ops = ops;
>>+	reg->info = info;
>> 	reg->priv = priv;
>>-	dpll->type = type;
>> 	first_registration = list_empty(&dpll->registration_list);
>> 	list_add_tail(&reg->list, &dpll->registration_list);
>> 	if (!first_registration) {
>>@@ -408,14 +408,14 @@ EXPORT_SYMBOL_GPL(dpll_device_register);
>>  * Context: Acquires a lock (dpll_lock)
>>  */
>> void dpll_device_unregister(struct dpll_device *dpll,
>>-			    const struct dpll_device_ops *ops, void *priv)
>>+			    const struct dpll_device_info *info, void *priv)
>> {
>> 	struct dpll_device_registration *reg;
>>
>> 	mutex_lock(&dpll_lock);
>> 	ASSERT_DPLL_REGISTERED(dpll);
>> 	dpll_device_delete_ntf(dpll);
>>-	reg = dpll_device_registration_find(dpll, ops, priv);
>>+	reg = dpll_device_registration_find(dpll, info, priv);
>> 	if (WARN_ON(!reg)) {
>> 		mutex_unlock(&dpll_lock);
>> 		return;
>>@@ -807,7 +807,15 @@ const struct dpll_device_ops *dpll_device_ops(struct
>>dpll_device *dpll)
>> 	struct dpll_device_registration *reg;
>>
>> 	reg = dpll_device_registration_first(dpll);
>>-	return reg->ops;
>>+	return reg->info->ops;
>>+}
>>+
>>+const struct dpll_device_info *dpll_device_info(struct dpll_device *dpll)
>
>Makes me wonder what you would need this for. I guess "nothing"?
>

Now using it to get info struct from dpll.. if struct is removed then yeah.

Thank you!
Arkadiusz

>
>>+{
>>+	struct dpll_device_registration *reg;
>>+
>>+	reg = dpll_device_registration_first(dpll);
>>+	return reg->info;
>> }
>>
>> static struct dpll_pin_registration *
>>diff --git a/drivers/dpll/dpll_core.h b/drivers/dpll/dpll_core.h
>>index 2b6d8ef1cdf3..baeb10d7dc1e 100644
>>--- a/drivers/dpll/dpll_core.h
>>+++ b/drivers/dpll/dpll_core.h
>>@@ -30,7 +30,6 @@ struct dpll_device {
>> 	u32 device_idx;
>> 	u64 clock_id;
>> 	struct module *module;
>>-	enum dpll_type type;
>> 	struct xarray pin_refs;
>> 	refcount_t refcount;
>> 	struct list_head registration_list;
>>@@ -84,6 +83,7 @@ void *dpll_pin_on_pin_priv(struct dpll_pin *parent,
>>struct dpll_pin *pin);
>> const struct dpll_device_ops *dpll_device_ops(struct dpll_device *dpll);
>> struct dpll_device *dpll_device_get_by_id(int id);
>> const struct dpll_pin_ops *dpll_pin_ops(struct dpll_pin_ref *ref);
>>+const struct dpll_device_info *dpll_device_info(struct dpll_device
>>*dpll);
>> struct dpll_pin_ref *dpll_xa_ref_dpll_first(struct xarray *xa_refs);
>> extern struct xarray dpll_device_xa;
>> extern struct xarray dpll_pin_xa;
>>diff --git a/drivers/dpll/dpll_netlink.c b/drivers/dpll/dpll_netlink.c
>>index c130f87147fa..2de9ec08d551 100644
>>--- a/drivers/dpll/dpll_netlink.c
>>+++ b/drivers/dpll/dpll_netlink.c
>>@@ -564,6 +564,7 @@ static int
>> dpll_device_get_one(struct dpll_device *dpll, struct sk_buff *msg,
>> 		    struct netlink_ext_ack *extack)
>> {
>>+	const struct dpll_device_info *info = dpll_device_info(dpll);
>> 	int ret;
>>
>> 	ret = dpll_msg_add_dev_handle(msg, dpll);
>>@@ -589,7 +590,7 @@ dpll_device_get_one(struct dpll_device *dpll, struct
>>sk_buff *msg,
>> 	ret = dpll_msg_add_mode_supported(msg, dpll, extack);
>> 	if (ret)
>> 		return ret;
>>-	if (nla_put_u32(msg, DPLL_A_TYPE, dpll->type))
>>+	if (nla_put_u32(msg, DPLL_A_TYPE, info->type))
>> 		return -EMSGSIZE;
>>
>> 	return 0;
>>@@ -1415,11 +1416,13 @@ dpll_device_find(u64 clock_id, struct nlattr
>>*mod_name_attr,
>> 	unsigned long i;
>>
>> 	xa_for_each_marked(&dpll_device_xa, i, dpll, DPLL_REGISTERED) {
>>+		const struct dpll_device_info *info = dpll_device_info(dpll);
>>+
>> 		cid_match = clock_id ? dpll->clock_id == clock_id : true;
>> 		mod_match = mod_name_attr ? (module_name(dpll->module) ?
>> 			!nla_strcmp(mod_name_attr,
>> 				    module_name(dpll->module)) : false) : true;
>>-		type_match = type ? dpll->type == type : true;
>>+		type_match = type ? info->type == type : true;
>> 		if (cid_match && mod_match && type_match) {
>> 			if (dpll_match) {
>> 				NL_SET_ERR_MSG(extack, "multiple matches");
>>diff --git a/drivers/net/ethernet/intel/ice/ice_dpll.c
>>b/drivers/net/ethernet/intel/ice/ice_dpll.c
>>index bce3ad6ca2a6..0f7440a889ac 100644
>>--- a/drivers/net/ethernet/intel/ice/ice_dpll.c
>>+++ b/drivers/net/ethernet/intel/ice/ice_dpll.c
>>@@ -1977,7 +1977,7 @@ static void
>> ice_dpll_deinit_dpll(struct ice_pf *pf, struct ice_dpll *d, bool cgu)
>> {
>> 	if (cgu)
>>-		dpll_device_unregister(d->dpll, &ice_dpll_ops, d);
>>+		dpll_device_unregister(d->dpll, &d->info, d);
>> 	dpll_device_put(d->dpll);
>> }
>>
>>@@ -1996,8 +1996,7 @@ ice_dpll_deinit_dpll(struct ice_pf *pf, struct
>>ice_dpll *d, bool cgu)
>>  * * negative - initialization failure reason
>>  */
>> static int
>>-ice_dpll_init_dpll(struct ice_pf *pf, struct ice_dpll *d, bool cgu,
>>-		   enum dpll_type type)
>>+ice_dpll_init_dpll(struct ice_pf *pf, struct ice_dpll *d, bool cgu)
>> {
>> 	u64 clock_id = pf->dplls.clock_id;
>> 	int ret;
>>@@ -2012,7 +2011,7 @@ ice_dpll_init_dpll(struct ice_pf *pf, struct
>>ice_dpll *d, bool cgu,
>> 	d->pf = pf;
>> 	if (cgu) {
>> 		ice_dpll_update_state(pf, d, true);
>>-		ret = dpll_device_register(d->dpll, type, &ice_dpll_ops, d);
>>+		ret = dpll_device_register(d->dpll, &d->info, d);
>> 		if (ret) {
>> 			dpll_device_put(d->dpll);
>> 			return ret;
>>@@ -2363,7 +2362,12 @@ static int ice_dpll_init_info(struct ice_pf *pf,
>>bool cgu)
>> 	if (ret)
>> 		return ret;
>> 	de->mode = DPLL_MODE_AUTOMATIC;
>>+	de->info.type = DPLL_TYPE_EEC;
>>+	de->info.ops = &ice_dpll_ops;
>>+
>> 	dp->mode = DPLL_MODE_AUTOMATIC;
>>+	dp->info.type = DPLL_TYPE_PPS;
>>+	dp->info.ops = &ice_dpll_ops;
>>
>> 	dev_dbg(ice_pf_to_dev(pf),
>> 		"%s - success, inputs:%u, outputs:%u rclk-parents:%u\n",
>>@@ -2426,10 +2430,10 @@ void ice_dpll_init(struct ice_pf *pf)
>> 	err = ice_dpll_init_info(pf, cgu);
>> 	if (err)
>> 		goto err_exit;
>>-	err = ice_dpll_init_dpll(pf, &pf->dplls.eec, cgu, DPLL_TYPE_EEC);
>>+	err = ice_dpll_init_dpll(pf, &pf->dplls.eec, cgu);
>> 	if (err)
>> 		goto deinit_info;
>>-	err = ice_dpll_init_dpll(pf, &pf->dplls.pps, cgu, DPLL_TYPE_PPS);
>>+	err = ice_dpll_init_dpll(pf, &pf->dplls.pps, cgu);
>> 	if (err)
>> 		goto deinit_eec;
>> 	err = ice_dpll_init_pins(pf, cgu);
>>diff --git a/drivers/net/ethernet/intel/ice/ice_dpll.h
>>b/drivers/net/ethernet/intel/ice/ice_dpll.h
>>index c320f1bf7d6d..9db7463e293a 100644
>>--- a/drivers/net/ethernet/intel/ice/ice_dpll.h
>>+++ b/drivers/net/ethernet/intel/ice/ice_dpll.h
>>@@ -66,6 +66,7 @@ struct ice_dpll {
>> 	enum dpll_mode mode;
>> 	struct dpll_pin *active_input;
>> 	struct dpll_pin *prev_input;
>>+	struct dpll_device_info info;
>> };
>>
>> /** ice_dplls - store info required for CCU (clock controlling unit)
>>diff --git a/drivers/net/ethernet/mellanox/mlx5/core/dpll.c
>>b/drivers/net/ethernet/mellanox/mlx5/core/dpll.c
>>index 1e5522a19483..f722b1de0754 100644
>>--- a/drivers/net/ethernet/mellanox/mlx5/core/dpll.c
>>+++ b/drivers/net/ethernet/mellanox/mlx5/core/dpll.c
>>@@ -20,6 +20,7 @@ struct mlx5_dpll {
>> 	} last;
>> 	struct notifier_block mdev_nb;
>> 	struct net_device *tracking_netdev;
>>+	struct dpll_device_info info;
>> };
>>
>> static int mlx5_dpll_clock_id_get(struct mlx5_core_dev *mdev, u64
>>*clock_id)
>>@@ -444,8 +445,9 @@ static int mlx5_dpll_probe(struct auxiliary_device
>>*adev,
>> 		goto err_free_mdpll;
>> 	}
>>
>>-	err = dpll_device_register(mdpll->dpll, DPLL_TYPE_EEC,
>>-				   &mlx5_dpll_device_ops, mdpll);
>>+	mdpll->info.type = DPLL_TYPE_EEC;
>>+	mdpll->info.ops = &mlx5_dpll_device_ops;
>>+	err = dpll_device_register(mdpll->dpll, &mdpll->info, mdpll);
>> 	if (err)
>> 		goto err_put_dpll_device;
>>
>>@@ -481,7 +483,7 @@ static int mlx5_dpll_probe(struct auxiliary_device
>>*adev,
>> err_put_dpll_pin:
>> 	dpll_pin_put(mdpll->dpll_pin);
>> err_unregister_dpll_device:
>>-	dpll_device_unregister(mdpll->dpll, &mlx5_dpll_device_ops, mdpll);
>>+	dpll_device_unregister(mdpll->dpll, &mdpll->info, mdpll);
>> err_put_dpll_device:
>> 	dpll_device_put(mdpll->dpll);
>> err_free_mdpll:
>>@@ -500,7 +502,7 @@ static void mlx5_dpll_remove(struct auxiliary_device
>>*adev)
>> 	dpll_pin_unregister(mdpll->dpll, mdpll->dpll_pin,
>> 			    &mlx5_dpll_pins_ops, mdpll);
>> 	dpll_pin_put(mdpll->dpll_pin);
>>-	dpll_device_unregister(mdpll->dpll, &mlx5_dpll_device_ops, mdpll);
>>+	dpll_device_unregister(mdpll->dpll, &mdpll->info, mdpll);
>> 	dpll_device_put(mdpll->dpll);
>> 	kfree(mdpll);
>>
>>diff --git a/drivers/ptp/ptp_ocp.c b/drivers/ptp/ptp_ocp.c
>>index 7945c6be1f7c..b3c5d294acb4 100644
>>--- a/drivers/ptp/ptp_ocp.c
>>+++ b/drivers/ptp/ptp_ocp.c
>>@@ -382,6 +382,7 @@ struct ptp_ocp {
>> 	struct ptp_ocp_sma_connector sma[OCP_SMA_NUM];
>> 	const struct ocp_sma_op *sma_op;
>> 	struct dpll_device *dpll;
>>+	struct dpll_device_info	dpll_info;
>> };
>>
>> #define OCP_REQ_TIMESTAMP	BIT(0)
>>@@ -4745,7 +4746,9 @@ ptp_ocp_probe(struct pci_dev *pdev, const struct
>>pci_device_id *id)
>> 		goto out;
>> 	}
>>
>>-	err = dpll_device_register(bp->dpll, DPLL_TYPE_PPS, &dpll_ops, bp);
>>+	bp->dpll_info.type = DPLL_TYPE_PPS;
>>+	bp->dpll_info.ops = &dpll_ops;
>>+	err = dpll_device_register(bp->dpll, &bp->dpll_info, bp);
>> 	if (err)
>> 		goto out;
>>
>>@@ -4796,7 +4799,7 @@ ptp_ocp_remove(struct pci_dev *pdev)
>> 			dpll_pin_put(bp->sma[i].dpll_pin);
>> 		}
>> 	}
>>-	dpll_device_unregister(bp->dpll, &dpll_ops, bp);
>>+	dpll_device_unregister(bp->dpll, &bp->dpll_info, bp);
>> 	dpll_device_put(bp->dpll);
>> 	devlink_unregister(devlink);
>> 	ptp_ocp_detach(bp);
>>diff --git a/include/linux/dpll.h b/include/linux/dpll.h
>>index 5e4f9ab1cf75..0489464af958 100644
>>--- a/include/linux/dpll.h
>>+++ b/include/linux/dpll.h
>>@@ -97,6 +97,11 @@ struct dpll_pin_ops {
>> 			 struct netlink_ext_ack *extack);
>> };
>>
>>+struct dpll_device_info {
>>+	enum dpll_type type;
>>+	const struct dpll_device_ops *ops;
>>+};
>>+
>> struct dpll_pin_frequency {
>> 	u64 min;
>> 	u64 max;
>>@@ -170,11 +175,11 @@ dpll_device_get(u64 clock_id, u32 dev_driver_id,
>>struct module *module);
>>
>> void dpll_device_put(struct dpll_device *dpll);
>>
>>-int dpll_device_register(struct dpll_device *dpll, enum dpll_type type,
>>-			 const struct dpll_device_ops *ops, void *priv);
>>+int dpll_device_register(struct dpll_device *dpll,
>>+			 const struct dpll_device_info *info, void *priv);
>>
>> void dpll_device_unregister(struct dpll_device *dpll,
>>-			    const struct dpll_device_ops *ops, void *priv);
>>+			    const struct dpll_device_info *info, void *priv);
>>
>> struct dpll_pin *
>> dpll_pin_get(u64 clock_id, u32 dev_driver_id, struct module *module,
>>--
>>2.38.1
>>

Thu, Apr 17, 2025 at 11:33:13AM +0200, arkadiusz.kubalewski@intel.com wrote:
>>From: Jiri Pirko <jiri@resnulli.us>
>>Sent: Wednesday, April 16, 2025 2:13 PM
>>
>>Tue, Apr 15, 2025 at 08:15:40PM +0200, arkadiusz.kubalewski@intel.com
>>wrote:
>>>Instead of passing list of properties as arguments to
>>>dpll_device_register(..) use a dedicated struct.
>>>
>>>Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
>>>---
>>>v2:
>>>- new commit
>>>---
>>> drivers/dpll/dpll_core.c                      | 34 ++++++++++++-------
>>> drivers/dpll/dpll_core.h                      |  2 +-
>>> drivers/dpll/dpll_netlink.c                   |  7 ++--
>>> drivers/net/ethernet/intel/ice/ice_dpll.c     | 16 +++++----
>>> drivers/net/ethernet/intel/ice/ice_dpll.h     |  1 +
>>> .../net/ethernet/mellanox/mlx5/core/dpll.c    | 10 +++---
>>> drivers/ptp/ptp_ocp.c                         |  7 ++--
>>> include/linux/dpll.h                          | 11 ++++--
>>> 8 files changed, 57 insertions(+), 31 deletions(-)
>>>
>>>diff --git a/drivers/dpll/dpll_core.c b/drivers/dpll/dpll_core.c
>>>index 20bdc52f63a5..af9cda45a89c 100644
>>>--- a/drivers/dpll/dpll_core.c
>>>+++ b/drivers/dpll/dpll_core.c
>>>@@ -34,7 +34,7 @@ static u32 dpll_pin_xa_id;
>>>
>>> struct dpll_device_registration {
>>> 	struct list_head list;
>>>-	const struct dpll_device_ops *ops;
>>>+	const struct dpll_device_info *info;
>>> 	void *priv;
>>> };
>>>
>>>@@ -327,12 +327,12 @@ EXPORT_SYMBOL_GPL(dpll_device_put);
>>>
>>> static struct dpll_device_registration *
>>> dpll_device_registration_find(struct dpll_device *dpll,
>>>-			      const struct dpll_device_ops *ops, void *priv)
>>>+			      const struct dpll_device_info *info, void *priv)
>>> {
>>> 	struct dpll_device_registration *reg;
>>>
>>> 	list_for_each_entry(reg, &dpll->registration_list, list) {
>>>-		if (reg->ops == ops && reg->priv == priv)
>>>+		if (reg->info == info && reg->priv == priv)
>>> 			return reg;
>>> 	}
>>> 	return NULL;
>>>@@ -341,8 +341,7 @@ dpll_device_registration_find(struct dpll_device
>>>*dpll,
>>> /**
>>>  * dpll_device_register - register the dpll device in the subsystem
>>>  * @dpll: pointer to a dpll
>>>- * @type: type of a dpll
>>>- * @ops: ops for a dpll device
>>>+ * @info: dpll device information and operations from registerer
>>>  * @priv: pointer to private information of owner
>>>  *
>>>  * Make dpll device available for user space.
>>>@@ -352,11 +351,13 @@ dpll_device_registration_find(struct dpll_device
>>>*dpll,
>>>  * * 0 on success
>>>  * * negative - error value
>>>  */
>>>-int dpll_device_register(struct dpll_device *dpll, enum dpll_type type,
>>>-			 const struct dpll_device_ops *ops, void *priv)
>>>+int dpll_device_register(struct dpll_device *dpll,
>>>+			 const struct dpll_device_info *info, void *priv)
>>
>>I don't like this. If you need some capabilities value, put it into ops
>>struct.
>>
>
>Hmm, this would seems strange, the _ops indicates operations, would
>have to rename the struct..

I don't think so. Happens all the time in kernel for ops to contain
other things.


>
>In theory I could decide on capabilities per ops provided from driver..
>i.e. If phase_input_monitor_feature_set()/phase_input_feature_get() are
>present then capability phase_input_monitor is provided..
>Makes sense?

Yes, that is more or less what I suggested in reply to the other patch
in this set.


>
>>
>>> {
>>>+	const struct dpll_device_ops *ops = info->ops;
>>> 	struct dpll_device_registration *reg;
>>> 	bool first_registration = false;
>>>+	enum dpll_type type = info->type;
>>>
>>> 	if (WARN_ON(!ops))
>>> 		return -EINVAL;
>>>@@ -368,7 +369,7 @@ int dpll_device_register(struct dpll_device *dpll,
>>>enum dpll_type type,
>>> 		return -EINVAL;
>>>
>>> 	mutex_lock(&dpll_lock);
>>>-	reg = dpll_device_registration_find(dpll, ops, priv);
>>>+	reg = dpll_device_registration_find(dpll, info, priv);
>>> 	if (reg) {
>>> 		mutex_unlock(&dpll_lock);
>>> 		return -EEXIST;
>>>@@ -379,9 +380,8 @@ int dpll_device_register(struct dpll_device *dpll,
>>>enum dpll_type type,
>>> 		mutex_unlock(&dpll_lock);
>>> 		return -ENOMEM;
>>> 	}
>>>-	reg->ops = ops;
>>>+	reg->info = info;
>>> 	reg->priv = priv;
>>>-	dpll->type = type;
>>> 	first_registration = list_empty(&dpll->registration_list);
>>> 	list_add_tail(&reg->list, &dpll->registration_list);
>>> 	if (!first_registration) {
>>>@@ -408,14 +408,14 @@ EXPORT_SYMBOL_GPL(dpll_device_register);
>>>  * Context: Acquires a lock (dpll_lock)
>>>  */
>>> void dpll_device_unregister(struct dpll_device *dpll,
>>>-			    const struct dpll_device_ops *ops, void *priv)
>>>+			    const struct dpll_device_info *info, void *priv)
>>> {
>>> 	struct dpll_device_registration *reg;
>>>
>>> 	mutex_lock(&dpll_lock);
>>> 	ASSERT_DPLL_REGISTERED(dpll);
>>> 	dpll_device_delete_ntf(dpll);
>>>-	reg = dpll_device_registration_find(dpll, ops, priv);
>>>+	reg = dpll_device_registration_find(dpll, info, priv);
>>> 	if (WARN_ON(!reg)) {
>>> 		mutex_unlock(&dpll_lock);
>>> 		return;
>>>@@ -807,7 +807,15 @@ const struct dpll_device_ops *dpll_device_ops(struct
>>>dpll_device *dpll)
>>> 	struct dpll_device_registration *reg;
>>>
>>> 	reg = dpll_device_registration_first(dpll);
>>>-	return reg->ops;
>>>+	return reg->info->ops;
>>>+}
>>>+
>>>+const struct dpll_device_info *dpll_device_info(struct dpll_device *dpll)
>>
>>Makes me wonder what you would need this for. I guess "nothing"?
>>
>
>Now using it to get info struct from dpll.. if struct is removed then yeah.
>
>Thank you!
>Arkadiusz
>
>>
>>>+{
>>>+	struct dpll_device_registration *reg;
>>>+
>>>+	reg = dpll_device_registration_first(dpll);
>>>+	return reg->info;
>>> }
>>>
>>> static struct dpll_pin_registration *
>>>diff --git a/drivers/dpll/dpll_core.h b/drivers/dpll/dpll_core.h
>>>index 2b6d8ef1cdf3..baeb10d7dc1e 100644
>>>--- a/drivers/dpll/dpll_core.h
>>>+++ b/drivers/dpll/dpll_core.h
>>>@@ -30,7 +30,6 @@ struct dpll_device {
>>> 	u32 device_idx;
>>> 	u64 clock_id;
>>> 	struct module *module;
>>>-	enum dpll_type type;
>>> 	struct xarray pin_refs;
>>> 	refcount_t refcount;
>>> 	struct list_head registration_list;
>>>@@ -84,6 +83,7 @@ void *dpll_pin_on_pin_priv(struct dpll_pin *parent,
>>>struct dpll_pin *pin);
>>> const struct dpll_device_ops *dpll_device_ops(struct dpll_device *dpll);
>>> struct dpll_device *dpll_device_get_by_id(int id);
>>> const struct dpll_pin_ops *dpll_pin_ops(struct dpll_pin_ref *ref);
>>>+const struct dpll_device_info *dpll_device_info(struct dpll_device
>>>*dpll);
>>> struct dpll_pin_ref *dpll_xa_ref_dpll_first(struct xarray *xa_refs);
>>> extern struct xarray dpll_device_xa;
>>> extern struct xarray dpll_pin_xa;
>>>diff --git a/drivers/dpll/dpll_netlink.c b/drivers/dpll/dpll_netlink.c
>>>index c130f87147fa..2de9ec08d551 100644
>>>--- a/drivers/dpll/dpll_netlink.c
>>>+++ b/drivers/dpll/dpll_netlink.c
>>>@@ -564,6 +564,7 @@ static int
>>> dpll_device_get_one(struct dpll_device *dpll, struct sk_buff *msg,
>>> 		    struct netlink_ext_ack *extack)
>>> {
>>>+	const struct dpll_device_info *info = dpll_device_info(dpll);
>>> 	int ret;
>>>
>>> 	ret = dpll_msg_add_dev_handle(msg, dpll);
>>>@@ -589,7 +590,7 @@ dpll_device_get_one(struct dpll_device *dpll, struct
>>>sk_buff *msg,
>>> 	ret = dpll_msg_add_mode_supported(msg, dpll, extack);
>>> 	if (ret)
>>> 		return ret;
>>>-	if (nla_put_u32(msg, DPLL_A_TYPE, dpll->type))
>>>+	if (nla_put_u32(msg, DPLL_A_TYPE, info->type))
>>> 		return -EMSGSIZE;
>>>
>>> 	return 0;
>>>@@ -1415,11 +1416,13 @@ dpll_device_find(u64 clock_id, struct nlattr
>>>*mod_name_attr,
>>> 	unsigned long i;
>>>
>>> 	xa_for_each_marked(&dpll_device_xa, i, dpll, DPLL_REGISTERED) {
>>>+		const struct dpll_device_info *info = dpll_device_info(dpll);
>>>+
>>> 		cid_match = clock_id ? dpll->clock_id == clock_id : true;
>>> 		mod_match = mod_name_attr ? (module_name(dpll->module) ?
>>> 			!nla_strcmp(mod_name_attr,
>>> 				    module_name(dpll->module)) : false) : true;
>>>-		type_match = type ? dpll->type == type : true;
>>>+		type_match = type ? info->type == type : true;
>>> 		if (cid_match && mod_match && type_match) {
>>> 			if (dpll_match) {
>>> 				NL_SET_ERR_MSG(extack, "multiple matches");
>>>diff --git a/drivers/net/ethernet/intel/ice/ice_dpll.c
>>>b/drivers/net/ethernet/intel/ice/ice_dpll.c
>>>index bce3ad6ca2a6..0f7440a889ac 100644
>>>--- a/drivers/net/ethernet/intel/ice/ice_dpll.c
>>>+++ b/drivers/net/ethernet/intel/ice/ice_dpll.c
>>>@@ -1977,7 +1977,7 @@ static void
>>> ice_dpll_deinit_dpll(struct ice_pf *pf, struct ice_dpll *d, bool cgu)
>>> {
>>> 	if (cgu)
>>>-		dpll_device_unregister(d->dpll, &ice_dpll_ops, d);
>>>+		dpll_device_unregister(d->dpll, &d->info, d);
>>> 	dpll_device_put(d->dpll);
>>> }
>>>
>>>@@ -1996,8 +1996,7 @@ ice_dpll_deinit_dpll(struct ice_pf *pf, struct
>>>ice_dpll *d, bool cgu)
>>>  * * negative - initialization failure reason
>>>  */
>>> static int
>>>-ice_dpll_init_dpll(struct ice_pf *pf, struct ice_dpll *d, bool cgu,
>>>-		   enum dpll_type type)
>>>+ice_dpll_init_dpll(struct ice_pf *pf, struct ice_dpll *d, bool cgu)
>>> {
>>> 	u64 clock_id = pf->dplls.clock_id;
>>> 	int ret;
>>>@@ -2012,7 +2011,7 @@ ice_dpll_init_dpll(struct ice_pf *pf, struct
>>>ice_dpll *d, bool cgu,
>>> 	d->pf = pf;
>>> 	if (cgu) {
>>> 		ice_dpll_update_state(pf, d, true);
>>>-		ret = dpll_device_register(d->dpll, type, &ice_dpll_ops, d);
>>>+		ret = dpll_device_register(d->dpll, &d->info, d);
>>> 		if (ret) {
>>> 			dpll_device_put(d->dpll);
>>> 			return ret;
>>>@@ -2363,7 +2362,12 @@ static int ice_dpll_init_info(struct ice_pf *pf,
>>>bool cgu)
>>> 	if (ret)
>>> 		return ret;
>>> 	de->mode = DPLL_MODE_AUTOMATIC;
>>>+	de->info.type = DPLL_TYPE_EEC;
>>>+	de->info.ops = &ice_dpll_ops;
>>>+
>>> 	dp->mode = DPLL_MODE_AUTOMATIC;
>>>+	dp->info.type = DPLL_TYPE_PPS;
>>>+	dp->info.ops = &ice_dpll_ops;
>>>
>>> 	dev_dbg(ice_pf_to_dev(pf),
>>> 		"%s - success, inputs:%u, outputs:%u rclk-parents:%u\n",
>>>@@ -2426,10 +2430,10 @@ void ice_dpll_init(struct ice_pf *pf)
>>> 	err = ice_dpll_init_info(pf, cgu);
>>> 	if (err)
>>> 		goto err_exit;
>>>-	err = ice_dpll_init_dpll(pf, &pf->dplls.eec, cgu, DPLL_TYPE_EEC);
>>>+	err = ice_dpll_init_dpll(pf, &pf->dplls.eec, cgu);
>>> 	if (err)
>>> 		goto deinit_info;
>>>-	err = ice_dpll_init_dpll(pf, &pf->dplls.pps, cgu, DPLL_TYPE_PPS);
>>>+	err = ice_dpll_init_dpll(pf, &pf->dplls.pps, cgu);
>>> 	if (err)
>>> 		goto deinit_eec;
>>> 	err = ice_dpll_init_pins(pf, cgu);
>>>diff --git a/drivers/net/ethernet/intel/ice/ice_dpll.h
>>>b/drivers/net/ethernet/intel/ice/ice_dpll.h
>>>index c320f1bf7d6d..9db7463e293a 100644
>>>--- a/drivers/net/ethernet/intel/ice/ice_dpll.h
>>>+++ b/drivers/net/ethernet/intel/ice/ice_dpll.h
>>>@@ -66,6 +66,7 @@ struct ice_dpll {
>>> 	enum dpll_mode mode;
>>> 	struct dpll_pin *active_input;
>>> 	struct dpll_pin *prev_input;
>>>+	struct dpll_device_info info;
>>> };
>>>
>>> /** ice_dplls - store info required for CCU (clock controlling unit)
>>>diff --git a/drivers/net/ethernet/mellanox/mlx5/core/dpll.c
>>>b/drivers/net/ethernet/mellanox/mlx5/core/dpll.c
>>>index 1e5522a19483..f722b1de0754 100644
>>>--- a/drivers/net/ethernet/mellanox/mlx5/core/dpll.c
>>>+++ b/drivers/net/ethernet/mellanox/mlx5/core/dpll.c
>>>@@ -20,6 +20,7 @@ struct mlx5_dpll {
>>> 	} last;
>>> 	struct notifier_block mdev_nb;
>>> 	struct net_device *tracking_netdev;
>>>+	struct dpll_device_info info;
>>> };
>>>
>>> static int mlx5_dpll_clock_id_get(struct mlx5_core_dev *mdev, u64
>>>*clock_id)
>>>@@ -444,8 +445,9 @@ static int mlx5_dpll_probe(struct auxiliary_device
>>>*adev,
>>> 		goto err_free_mdpll;
>>> 	}
>>>
>>>-	err = dpll_device_register(mdpll->dpll, DPLL_TYPE_EEC,
>>>-				   &mlx5_dpll_device_ops, mdpll);
>>>+	mdpll->info.type = DPLL_TYPE_EEC;
>>>+	mdpll->info.ops = &mlx5_dpll_device_ops;
>>>+	err = dpll_device_register(mdpll->dpll, &mdpll->info, mdpll);
>>> 	if (err)
>>> 		goto err_put_dpll_device;
>>>
>>>@@ -481,7 +483,7 @@ static int mlx5_dpll_probe(struct auxiliary_device
>>>*adev,
>>> err_put_dpll_pin:
>>> 	dpll_pin_put(mdpll->dpll_pin);
>>> err_unregister_dpll_device:
>>>-	dpll_device_unregister(mdpll->dpll, &mlx5_dpll_device_ops, mdpll);
>>>+	dpll_device_unregister(mdpll->dpll, &mdpll->info, mdpll);
>>> err_put_dpll_device:
>>> 	dpll_device_put(mdpll->dpll);
>>> err_free_mdpll:
>>>@@ -500,7 +502,7 @@ static void mlx5_dpll_remove(struct auxiliary_device
>>>*adev)
>>> 	dpll_pin_unregister(mdpll->dpll, mdpll->dpll_pin,
>>> 			    &mlx5_dpll_pins_ops, mdpll);
>>> 	dpll_pin_put(mdpll->dpll_pin);
>>>-	dpll_device_unregister(mdpll->dpll, &mlx5_dpll_device_ops, mdpll);
>>>+	dpll_device_unregister(mdpll->dpll, &mdpll->info, mdpll);
>>> 	dpll_device_put(mdpll->dpll);
>>> 	kfree(mdpll);
>>>
>>>diff --git a/drivers/ptp/ptp_ocp.c b/drivers/ptp/ptp_ocp.c
>>>index 7945c6be1f7c..b3c5d294acb4 100644
>>>--- a/drivers/ptp/ptp_ocp.c
>>>+++ b/drivers/ptp/ptp_ocp.c
>>>@@ -382,6 +382,7 @@ struct ptp_ocp {
>>> 	struct ptp_ocp_sma_connector sma[OCP_SMA_NUM];
>>> 	const struct ocp_sma_op *sma_op;
>>> 	struct dpll_device *dpll;
>>>+	struct dpll_device_info	dpll_info;
>>> };
>>>
>>> #define OCP_REQ_TIMESTAMP	BIT(0)
>>>@@ -4745,7 +4746,9 @@ ptp_ocp_probe(struct pci_dev *pdev, const struct
>>>pci_device_id *id)
>>> 		goto out;
>>> 	}
>>>
>>>-	err = dpll_device_register(bp->dpll, DPLL_TYPE_PPS, &dpll_ops, bp);
>>>+	bp->dpll_info.type = DPLL_TYPE_PPS;
>>>+	bp->dpll_info.ops = &dpll_ops;
>>>+	err = dpll_device_register(bp->dpll, &bp->dpll_info, bp);
>>> 	if (err)
>>> 		goto out;
>>>
>>>@@ -4796,7 +4799,7 @@ ptp_ocp_remove(struct pci_dev *pdev)
>>> 			dpll_pin_put(bp->sma[i].dpll_pin);
>>> 		}
>>> 	}
>>>-	dpll_device_unregister(bp->dpll, &dpll_ops, bp);
>>>+	dpll_device_unregister(bp->dpll, &bp->dpll_info, bp);
>>> 	dpll_device_put(bp->dpll);
>>> 	devlink_unregister(devlink);
>>> 	ptp_ocp_detach(bp);
>>>diff --git a/include/linux/dpll.h b/include/linux/dpll.h
>>>index 5e4f9ab1cf75..0489464af958 100644
>>>--- a/include/linux/dpll.h
>>>+++ b/include/linux/dpll.h
>>>@@ -97,6 +97,11 @@ struct dpll_pin_ops {
>>> 			 struct netlink_ext_ack *extack);
>>> };
>>>
>>>+struct dpll_device_info {
>>>+	enum dpll_type type;
>>>+	const struct dpll_device_ops *ops;
>>>+};
>>>+
>>> struct dpll_pin_frequency {
>>> 	u64 min;
>>> 	u64 max;
>>>@@ -170,11 +175,11 @@ dpll_device_get(u64 clock_id, u32 dev_driver_id,
>>>struct module *module);
>>>
>>> void dpll_device_put(struct dpll_device *dpll);
>>>
>>>-int dpll_device_register(struct dpll_device *dpll, enum dpll_type type,
>>>-			 const struct dpll_device_ops *ops, void *priv);
>>>+int dpll_device_register(struct dpll_device *dpll,
>>>+			 const struct dpll_device_info *info, void *priv);
>>>
>>> void dpll_device_unregister(struct dpll_device *dpll,
>>>-			    const struct dpll_device_ops *ops, void *priv);
>>>+			    const struct dpll_device_info *info, void *priv);
>>>
>>> struct dpll_pin *
>>> dpll_pin_get(u64 clock_id, u32 dev_driver_id, struct module *module,
>>>--
>>>2.38.1
>>>