Re: [Intel-wired-lan] [PATCH RFC net-next 02/13] dpll: Allow registering pin with firmware node
Mon, Dec 15, 2025 at 02:51:36PM +0100, [email protected] wrote: >On 12/15/25 2:08 PM, Jiri Pirko wrote: >> Sun, Dec 14, 2025 at 08:35:01PM +0100, [email protected] wrote: >> > >> > >> > On December 12, 2025 12:25:12 PM GMT+01:00, Jiri Pirko >> > wrote: >> > > Thu, Dec 11, 2025 at 08:47:45PM +0100, [email protected] wrote: >> > > >> > > [..] >> > > >> > > > @@ -559,7 +563,8 @@ EXPORT_SYMBOL(dpll_netdev_pin_clear); >> > > > */ >> > > > struct dpll_pin * >> > > > dpll_pin_get(u64 clock_id, u32 pin_idx, struct module *module, >> > > > - const struct dpll_pin_properties *prop) >> > > > + const struct dpll_pin_properties *prop, >> > > > + struct fwnode_handle *fwnode) >> > > > { >> > > >struct dpll_pin *pos, *ret = NULL; >> > > >unsigned long i; >> > > > @@ -568,14 +573,15 @@ dpll_pin_get(u64 clock_id, u32 pin_idx, struct >> > > > module *module, >> > > >xa_for_each(&dpll_pin_xa, i, pos) { >> > > >if (pos->clock_id == clock_id && >> > > >pos->pin_idx == pin_idx && >> > > > - pos->module == module) { >> > > > + pos->module == module && >> > > > + pos->fwnode == fwnode) { >> > > >> > > Is fwnode part of the key? Doesn't look to me like that. Then you can >> > > have a simple helper to set fwnode on struct dpll_pin *, and leave >> > > dpll_pin_get() out of this, no? >> > >> > IMHO yes, because particular fwnode identifies exact dpll pin, so >> > I think it should be a part of the key. >> >> The key items serve for userspace identification purposes as well. For >> that, fwnode is non-sense. >> fwnode identifies exact pin, that is nice. But is it the only >> differentiator among other key items? I don't expect so. > >From this point of view, not. I will not touch dpll_pin_get() and rather >use new helper like dpll_pin_fwnode_set(), ok? Yes please. Thanks! > >Thanks, >Ivan >
Re: [Intel-wired-lan] [PATCH RFC net-next 02/13] dpll: Allow registering pin with firmware node
On 12/15/25 2:08 PM, Jiri Pirko wrote: Sun, Dec 14, 2025 at 08:35:01PM +0100, [email protected] wrote: On December 12, 2025 12:25:12 PM GMT+01:00, Jiri Pirko wrote: Thu, Dec 11, 2025 at 08:47:45PM +0100, [email protected] wrote: [..] @@ -559,7 +563,8 @@ EXPORT_SYMBOL(dpll_netdev_pin_clear); */ struct dpll_pin * dpll_pin_get(u64 clock_id, u32 pin_idx, struct module *module, -const struct dpll_pin_properties *prop) +const struct dpll_pin_properties *prop, +struct fwnode_handle *fwnode) { struct dpll_pin *pos, *ret = NULL; unsigned long i; @@ -568,14 +573,15 @@ dpll_pin_get(u64 clock_id, u32 pin_idx, struct module *module, xa_for_each(&dpll_pin_xa, i, pos) { if (pos->clock_id == clock_id && pos->pin_idx == pin_idx && - pos->module == module) { + pos->module == module && + pos->fwnode == fwnode) { Is fwnode part of the key? Doesn't look to me like that. Then you can have a simple helper to set fwnode on struct dpll_pin *, and leave dpll_pin_get() out of this, no? IMHO yes, because particular fwnode identifies exact dpll pin, so I think it should be a part of the key. The key items serve for userspace identification purposes as well. For that, fwnode is non-sense. fwnode identifies exact pin, that is nice. But is it the only differentiator among other key items? I don't expect so. From this point of view, not. I will not touch dpll_pin_get() and rather use new helper like dpll_pin_fwnode_set(), ok? Thanks, Ivan
Re: [Intel-wired-lan] [PATCH RFC net-next 02/13] dpll: Allow registering pin with firmware node
Sun, Dec 14, 2025 at 08:35:01PM +0100, [email protected] wrote: > > >On December 12, 2025 12:25:12 PM GMT+01:00, Jiri Pirko >wrote: >>Thu, Dec 11, 2025 at 08:47:45PM +0100, [email protected] wrote: >> >>[..] >> >>>@@ -559,7 +563,8 @@ EXPORT_SYMBOL(dpll_netdev_pin_clear); >>> */ >>> struct dpll_pin * >>> dpll_pin_get(u64 clock_id, u32 pin_idx, struct module *module, >>>- const struct dpll_pin_properties *prop) >>>+ const struct dpll_pin_properties *prop, >>>+ struct fwnode_handle *fwnode) >>> { >>> struct dpll_pin *pos, *ret = NULL; >>> unsigned long i; >>>@@ -568,14 +573,15 @@ dpll_pin_get(u64 clock_id, u32 pin_idx, struct module >>>*module, >>> xa_for_each(&dpll_pin_xa, i, pos) { >>> if (pos->clock_id == clock_id && >>> pos->pin_idx == pin_idx && >>>-pos->module == module) { >>>+pos->module == module && >>>+pos->fwnode == fwnode) { >> >>Is fwnode part of the key? Doesn't look to me like that. Then you can >>have a simple helper to set fwnode on struct dpll_pin *, and leave >>dpll_pin_get() out of this, no? > >IMHO yes, because particular fwnode identifies exact dpll pin, so >I think it should be a part of the key. The key items serve for userspace identification purposes as well. For that, fwnode is non-sense. fwnode identifies exact pin, that is nice. But is it the only differentiator among other key items? I don't expect so. >
Re: [Intel-wired-lan] [PATCH RFC net-next 02/13] dpll: Allow registering pin with firmware node
On December 12, 2025 12:25:12 PM GMT+01:00, Jiri Pirko wrote: >Thu, Dec 11, 2025 at 08:47:45PM +0100, [email protected] wrote: > >[..] > >>@@ -559,7 +563,8 @@ EXPORT_SYMBOL(dpll_netdev_pin_clear); >> */ >> struct dpll_pin * >> dpll_pin_get(u64 clock_id, u32 pin_idx, struct module *module, >>- const struct dpll_pin_properties *prop) >>+ const struct dpll_pin_properties *prop, >>+ struct fwnode_handle *fwnode) >> { >> struct dpll_pin *pos, *ret = NULL; >> unsigned long i; >>@@ -568,14 +573,15 @@ dpll_pin_get(u64 clock_id, u32 pin_idx, struct module >>*module, >> xa_for_each(&dpll_pin_xa, i, pos) { >> if (pos->clock_id == clock_id && >> pos->pin_idx == pin_idx && >>- pos->module == module) { >>+ pos->module == module && >>+ pos->fwnode == fwnode) { > >Is fwnode part of the key? Doesn't look to me like that. Then you can >have a simple helper to set fwnode on struct dpll_pin *, and leave >dpll_pin_get() out of this, no? IMHO yes, because particular fwnode identifies exact dpll pin, so I think it should be a part of the key.
Re: [Intel-wired-lan] [PATCH RFC net-next 02/13] dpll: Allow registering pin with firmware node
Thu, Dec 11, 2025 at 08:47:45PM +0100, [email protected] wrote: [..] >@@ -559,7 +563,8 @@ EXPORT_SYMBOL(dpll_netdev_pin_clear); > */ > struct dpll_pin * > dpll_pin_get(u64 clock_id, u32 pin_idx, struct module *module, >- const struct dpll_pin_properties *prop) >+ const struct dpll_pin_properties *prop, >+ struct fwnode_handle *fwnode) > { > struct dpll_pin *pos, *ret = NULL; > unsigned long i; >@@ -568,14 +573,15 @@ dpll_pin_get(u64 clock_id, u32 pin_idx, struct module >*module, > xa_for_each(&dpll_pin_xa, i, pos) { > if (pos->clock_id == clock_id && > pos->pin_idx == pin_idx && >- pos->module == module) { >+ pos->module == module && >+ pos->fwnode == fwnode) { Is fwnode part of the key? Doesn't look to me like that. Then you can have a simple helper to set fwnode on struct dpll_pin *, and leave dpll_pin_get() out of this, no? > ret = pos; > refcount_inc(&ret->refcount); > break; > } [..]
[Intel-wired-lan] [PATCH RFC net-next 02/13] dpll: Allow registering pin with firmware node
Extend the DPLL core to support associating a DPLL pin with a firmware
node (fwnode). This association is required to allow other subsystems
(such as network drivers) to locate and request specific DPLL pins
defined in the Device Tree or ACPI.
Modify dpll_pin_get() to accept an optional fwnode_handle parameter.
If provided, the fwnode is stored in the dpll_pin structure, and its
reference count is incremented. The reference is released in dpll_pin_put().
Add fwnode_dpll_pin_find() as an exported symbol. This helper allows
drivers to search for a registered DPLL pin using its associated fwnode
handle. The caller should use dpll_pin_put() to release dpll pin
returned by fwnode_dpll_pin_find().
Update all existing callers of dpll_pin_get() in the ice, mlx5, ptp_ocp,
and zl3073x drivers to pass NULL for the new argument, preserving existing
behavior.
Signed-off-by: Ivan Vecera
---
drivers/dpll/dpll_core.c | 46 +--
drivers/dpll/dpll_core.h | 2 +
drivers/dpll/zl3073x/dpll.c | 2 +-
drivers/net/ethernet/intel/ice/ice_dpll.c | 2 +-
.../net/ethernet/mellanox/mlx5/core/dpll.c| 3 +-
drivers/ptp/ptp_ocp.c | 3 +-
include/linux/dpll.h | 12 -
7 files changed, 61 insertions(+), 9 deletions(-)
diff --git a/drivers/dpll/dpll_core.c b/drivers/dpll/dpll_core.c
index a461095efd8ac..773783fd14f71 100644
--- a/drivers/dpll/dpll_core.c
+++ b/drivers/dpll/dpll_core.c
@@ -10,6 +10,7 @@
#include
#include
+#include
#include
#include
@@ -484,7 +485,8 @@ static int dpll_pin_prop_dup(const struct
dpll_pin_properties *src,
static struct dpll_pin *
dpll_pin_alloc(u64 clock_id, u32 pin_idx, struct module *module,
- const struct dpll_pin_properties *prop)
+ const struct dpll_pin_properties *prop,
+ struct fwnode_handle *fwnode)
{
struct dpll_pin *pin;
int ret;
@@ -511,6 +513,7 @@ dpll_pin_alloc(u64 clock_id, u32 pin_idx, struct module
*module,
&dpll_pin_xa_id, GFP_KERNEL);
if (ret < 0)
goto err_xa_alloc;
+ pin->fwnode = fwnode_handle_get(fwnode);
return pin;
err_xa_alloc:
xa_destroy(&pin->dpll_refs);
@@ -548,6 +551,7 @@ EXPORT_SYMBOL(dpll_netdev_pin_clear);
* @pin_idx: idx given by dev driver
* @module: reference to registering module
* @prop: dpll pin properties
+ * @fwnode: optional reference to firmware node
*
* Get existing object of a pin (unique for given arguments) or create new
* if doesn't exist yet.
@@ -559,7 +563,8 @@ EXPORT_SYMBOL(dpll_netdev_pin_clear);
*/
struct dpll_pin *
dpll_pin_get(u64 clock_id, u32 pin_idx, struct module *module,
-const struct dpll_pin_properties *prop)
+const struct dpll_pin_properties *prop,
+struct fwnode_handle *fwnode)
{
struct dpll_pin *pos, *ret = NULL;
unsigned long i;
@@ -568,14 +573,15 @@ dpll_pin_get(u64 clock_id, u32 pin_idx, struct module
*module,
xa_for_each(&dpll_pin_xa, i, pos) {
if (pos->clock_id == clock_id &&
pos->pin_idx == pin_idx &&
- pos->module == module) {
+ pos->module == module &&
+ pos->fwnode == fwnode) {
ret = pos;
refcount_inc(&ret->refcount);
break;
}
}
if (!ret)
- ret = dpll_pin_alloc(clock_id, pin_idx, module, prop);
+ ret = dpll_pin_alloc(clock_id, pin_idx, module, prop, fwnode);
mutex_unlock(&dpll_lock);
return ret;
@@ -599,12 +605,44 @@ void dpll_pin_put(struct dpll_pin *pin)
xa_destroy(&pin->parent_refs);
xa_destroy(&pin->ref_sync_pins);
dpll_pin_prop_free(&pin->prop);
+ fwnode_handle_put(pin->fwnode);
kfree_rcu(pin, rcu);
}
mutex_unlock(&dpll_lock);
}
EXPORT_SYMBOL_GPL(dpll_pin_put);
+/**
+ * fwnode_dpll_pin_find - find dpll pin by firmware node reference
+ * @fwnode: reference to firmware node
+ *
+ * Get existing object of a pin that is associated with given firmware node
+ * reference.
+ *
+ * Context: Acquires a lock (dpll_lock)
+ * Return:
+ * * valid dpll_pin struct pointer if succeeded
+ * * ERR_PTR(X) - error
+ */
+struct dpll_pin *fwnode_dpll_pin_find(struct fwnode_handle *fwnode)
+{
+ struct dpll_pin *pin, *ret = NULL;
+ unsigned long index;
+
+ mutex_lock(&dpll_lock);
+ xa_for_each(&dpll_pin_xa, index, pin) {
+ if (pin->fwnode == fwnode) {
+ ret = pin;
+ refcount_inc(&ret->refcount);
+ break;
+ }
+ }
+ mutex_unlock(&dpll_lock);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(fwnode_dpll_pin_find);
+
static int
__dpll_pin_
