* [PATCH net] dpll: use pin owner's dpll ref for pin-level attribute reporting
@ 2026-07-14 12:59 Ivan Vecera
0 siblings, 0 replies; only message in thread
From: Ivan Vecera @ 2026-07-14 12:59 UTC (permalink / raw)
To: netdev
Cc: Vadim Fedorenko, Arkadiusz Kubalewski, Jiri Pirko,
Aleksandr Loktionov, Grzegorz Nitka, Jakub Kicinski, open list
Commit c191b319f208 ("dpll: allow registering FW-identified pin with a
different DPLL") relaxed dpll_pin_register() to let fwnode-identified pins
register with DPLLs from a different driver. This allows, for example, the
ICE driver to register a zl3073x-created pin with its TXC DPLL using
ice_dpll_txclk_ops, which lack frequency_get and phase_adjust_get
callbacks.
After such cross-driver registration, the pin's dpll_refs xarray contains
refs from both drivers. dpll_cmd_pin_get_one() calls
dpll_xa_ref_dpll_first() which returns the ref with the lowest DPLL id.
When the foreign DPLL (e.g. ICE TXC) has a lower id than the owner DPLL
(e.g. zl3073x), the foreign ops are used for reporting. Since those ops
lack callbacks like frequency_get, pin-level attributes are silently
omitted from the netlink response.
For example, a zl3073x output pin that should report frequency and
phase-adjust shows neither:
Before:
# dpll pin show id 45
pin id 45:
module-name: zl3073x
clock-id: 3427468959636104019
board-label: 156M25_NAC0_CLKREF_SYNC
package-label: OUT3
type: synce-eth-port
capabilities: 0x0
phase-adjust-min: -2147483648
phase-adjust-max: 2147483647
phase-adjust-gran: 800
parent-device:
...
After:
# dpll pin show id 19
pin id 19:
module-name: zl3073x
clock-id: 15964355450360090479
board-label: 156M25_NAC0_CLKREF_SYNC
package-label: OUT3
type: synce-eth-port
frequency: 156250000 Hz
frequency-supported:
156250000 Hz
capabilities: 0x0
phase-adjust-min: -2147483648
phase-adjust-max: 2147483647
phase-adjust-gran: 800
phase-adjust: 0
parent-device:
...
Fix this by:
1. Adding dpll_pin_own_dpll_ref_first() helper that returns the first ref
whose DPLL matches the pin's (module, clock_id) tuple — i.e. the DPLL
from the driver that created the pin and has the complete set of ops.
Return NULL if no owner ref is found.
2. Using dpll_pin_own_dpll_ref_first() in dpll_cmd_pin_get_one() with a
fallback to dpll_xa_ref_dpll_first() for pin-on-pin child pins whose
dpll_refs all point to a different driver's DPLLs.
3. Using dpll_pin_own_dpll_ref_first() in SET operations
(dpll_pin_freq_set, dpll_pin_esync_set, dpll_pin_ref_sync_state_set,
dpll_pin_phase_adj_set) returning -ENODEV if no owner ref exists.
Replacing the validation loops that rejected the entire operation when
any ref's ops lacked the required callback — instead validate only the
owner refs so that foreign DPLLs with incomplete ops no longer block
SET operations.
4. Guarding all SET and rollback xa_for_each loops against NULL set
callbacks so that foreign refs without the operation are safely skipped
instead of causing a NULL pointer dereference.
Fixes: c191b319f208 ("dpll: allow registering FW-identified pin with a different DPLL")
Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
drivers/dpll/dpll_core.c | 27 +++++++++++++++++
drivers/dpll/dpll_core.h | 1 +
drivers/dpll/dpll_netlink.c | 60 ++++++++++++++++++++++++++++++-------
3 files changed, 78 insertions(+), 10 deletions(-)
diff --git a/drivers/dpll/dpll_core.c b/drivers/dpll/dpll_core.c
index bb1e8650c9d59..245f625f4b3b4 100644
--- a/drivers/dpll/dpll_core.c
+++ b/drivers/dpll/dpll_core.c
@@ -1146,6 +1146,33 @@ void *dpll_pin_on_pin_priv(struct dpll_pin *parent,
return reg->priv;
}
+/**
+ * dpll_pin_own_dpll_ref_first - find the first owner dpll ref of a pin
+ * @pin: pointer to a dpll pin
+ *
+ * Search pin's dpll_refs for a ref whose dpll matches the pin's
+ * (module, clock_id) tuple, i.e. the dpll registered by the driver
+ * that created the pin. This ensures pin-level attributes are
+ * reported and modified using the owner's ops even when the pin is
+ * also registered with dplls from other drivers.
+ *
+ * Return: pointer to the owner's dpll_pin_ref, or NULL if no
+ * owner ref is found.
+ */
+struct dpll_pin_ref *dpll_pin_own_dpll_ref_first(struct dpll_pin *pin)
+{
+ struct dpll_pin_ref *ref;
+ unsigned long i;
+
+ xa_for_each(&pin->dpll_refs, i, ref) {
+ if (ref->dpll->module == pin->module &&
+ ref->dpll->clock_id == pin->clock_id)
+ return ref;
+ }
+
+ return NULL;
+}
+
const struct dpll_pin_ops *dpll_pin_ops(struct dpll_pin_ref *ref)
{
struct dpll_pin_registration *reg;
diff --git a/drivers/dpll/dpll_core.h b/drivers/dpll/dpll_core.h
index e245771134317..da8a369556ed1 100644
--- a/drivers/dpll/dpll_core.h
+++ b/drivers/dpll/dpll_core.h
@@ -93,6 +93,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);
+struct dpll_pin_ref *dpll_pin_own_dpll_ref_first(struct dpll_pin *pin);
const struct dpll_pin_ops *dpll_pin_ops(struct dpll_pin_ref *ref);
struct dpll_pin_ref *dpll_xa_ref_dpll_first(struct xarray *xa_refs);
extern struct xarray dpll_device_xa;
diff --git a/drivers/dpll/dpll_netlink.c b/drivers/dpll/dpll_netlink.c
index bf729cde796a7..b3f0cb7d3d349 100644
--- a/drivers/dpll/dpll_netlink.c
+++ b/drivers/dpll/dpll_netlink.c
@@ -696,7 +696,9 @@ dpll_cmd_pin_get_one(struct sk_buff *msg, struct dpll_pin *pin,
struct dpll_pin_ref *ref;
int ret;
- ref = dpll_xa_ref_dpll_first(&pin->dpll_refs);
+ ref = dpll_pin_own_dpll_ref_first(pin);
+ if (!ref)
+ ref = dpll_xa_ref_dpll_first(&pin->dpll_refs);
ASSERT_NOT_NULL(ref);
ret = dpll_msg_add_pin_handle(msg, pin);
@@ -1087,12 +1089,19 @@ dpll_pin_freq_set(struct dpll_pin *pin, struct nlattr *a,
xa_for_each(&pin->dpll_refs, i, ref) {
ops = dpll_pin_ops(ref);
- if (!ops->frequency_set || !ops->frequency_get) {
- NL_SET_ERR_MSG(extack, "frequency set not supported by the device");
+ if ((!ops->frequency_set || !ops->frequency_get) &&
+ ref->dpll->module == pin->module &&
+ ref->dpll->clock_id == pin->clock_id) {
+ NL_SET_ERR_MSG(extack,
+ "frequency set not supported by the device");
return -EOPNOTSUPP;
}
}
- ref = dpll_xa_ref_dpll_first(&pin->dpll_refs);
+ ref = dpll_pin_own_dpll_ref_first(pin);
+ if (!ref) {
+ NL_SET_ERR_MSG(extack, "pin owner dpll not found");
+ return -ENODEV;
+ }
ops = dpll_pin_ops(ref);
dpll = ref->dpll;
ret = ops->frequency_get(pin, dpll_pin_on_dpll_priv(dpll, pin), dpll,
@@ -1106,6 +1115,8 @@ dpll_pin_freq_set(struct dpll_pin *pin, struct nlattr *a,
xa_for_each(&pin->dpll_refs, i, ref) {
ops = dpll_pin_ops(ref);
+ if (!ops->frequency_set)
+ continue;
dpll = ref->dpll;
ret = ops->frequency_set(pin, dpll_pin_on_dpll_priv(dpll, pin),
dpll, dpll_priv(dpll), freq, extack);
@@ -1125,6 +1136,8 @@ dpll_pin_freq_set(struct dpll_pin *pin, struct nlattr *a,
if (ref == failed)
break;
ops = dpll_pin_ops(ref);
+ if (!ops->frequency_set)
+ continue;
dpll = ref->dpll;
if (ops->frequency_set(pin, dpll_pin_on_dpll_priv(dpll, pin),
dpll, dpll_priv(dpll), old_freq, extack))
@@ -1148,13 +1161,19 @@ dpll_pin_esync_set(struct dpll_pin *pin, struct nlattr *a,
xa_for_each(&pin->dpll_refs, i, ref) {
ops = dpll_pin_ops(ref);
- if (!ops->esync_set || !ops->esync_get) {
+ if ((!ops->esync_set || !ops->esync_get) &&
+ ref->dpll->module == pin->module &&
+ ref->dpll->clock_id == pin->clock_id) {
NL_SET_ERR_MSG(extack,
"embedded sync feature is not supported by this device");
return -EOPNOTSUPP;
}
}
- ref = dpll_xa_ref_dpll_first(&pin->dpll_refs);
+ ref = dpll_pin_own_dpll_ref_first(pin);
+ if (!ref) {
+ NL_SET_ERR_MSG(extack, "pin owner dpll not found");
+ return -ENODEV;
+ }
ops = dpll_pin_ops(ref);
dpll = ref->dpll;
ret = ops->esync_get(pin, dpll_pin_on_dpll_priv(dpll, pin), dpll,
@@ -1178,6 +1197,8 @@ dpll_pin_esync_set(struct dpll_pin *pin, struct nlattr *a,
void *pin_dpll_priv;
ops = dpll_pin_ops(ref);
+ if (!ops->esync_set)
+ continue;
dpll = ref->dpll;
pin_dpll_priv = dpll_pin_on_dpll_priv(dpll, pin);
ret = ops->esync_set(pin, pin_dpll_priv, dpll, dpll_priv(dpll),
@@ -1201,6 +1222,8 @@ dpll_pin_esync_set(struct dpll_pin *pin, struct nlattr *a,
if (ref == failed)
break;
ops = dpll_pin_ops(ref);
+ if (!ops->esync_set)
+ continue;
dpll = ref->dpll;
pin_dpll_priv = dpll_pin_on_dpll_priv(dpll, pin);
if (ops->esync_set(pin, pin_dpll_priv, dpll, dpll_priv(dpll),
@@ -1235,8 +1258,11 @@ dpll_pin_ref_sync_state_set(struct dpll_pin *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);
+ ref = dpll_pin_own_dpll_ref_first(pin);
+ if (!ref) {
+ NL_SET_ERR_MSG(extack, "pin owner dpll not found");
+ return -ENODEV;
+ }
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");
@@ -1255,6 +1281,8 @@ dpll_pin_ref_sync_state_set(struct dpll_pin *pin,
return 0;
xa_for_each(&pin->dpll_refs, i, ref) {
ops = dpll_pin_ops(ref);
+ if (!ops->ref_sync_set)
+ continue;
dpll = ref->dpll;
ret = ops->ref_sync_set(pin, dpll_pin_on_dpll_priv(dpll, pin),
ref_sync_pin,
@@ -1277,6 +1305,8 @@ dpll_pin_ref_sync_state_set(struct dpll_pin *pin,
if (ref == failed)
break;
ops = dpll_pin_ops(ref);
+ if (!ops->ref_sync_set)
+ continue;
dpll = ref->dpll;
if (ops->ref_sync_set(pin, dpll_pin_on_dpll_priv(dpll, pin),
ref_sync_pin,
@@ -1468,12 +1498,18 @@ dpll_pin_phase_adj_set(struct dpll_pin *pin, struct nlattr *phase_adj_attr,
xa_for_each(&pin->dpll_refs, i, ref) {
ops = dpll_pin_ops(ref);
- if (!ops->phase_adjust_set || !ops->phase_adjust_get) {
+ if ((!ops->phase_adjust_set || !ops->phase_adjust_get) &&
+ ref->dpll->module == pin->module &&
+ ref->dpll->clock_id == pin->clock_id) {
NL_SET_ERR_MSG(extack, "phase adjust not supported");
return -EOPNOTSUPP;
}
}
- ref = dpll_xa_ref_dpll_first(&pin->dpll_refs);
+ ref = dpll_pin_own_dpll_ref_first(pin);
+ if (!ref) {
+ NL_SET_ERR_MSG(extack, "pin owner dpll not found");
+ return -ENODEV;
+ }
ops = dpll_pin_ops(ref);
dpll = ref->dpll;
ret = ops->phase_adjust_get(pin, dpll_pin_on_dpll_priv(dpll, pin),
@@ -1488,6 +1524,8 @@ dpll_pin_phase_adj_set(struct dpll_pin *pin, struct nlattr *phase_adj_attr,
xa_for_each(&pin->dpll_refs, i, ref) {
ops = dpll_pin_ops(ref);
+ if (!ops->phase_adjust_set)
+ continue;
dpll = ref->dpll;
ret = ops->phase_adjust_set(pin,
dpll_pin_on_dpll_priv(dpll, pin),
@@ -1510,6 +1548,8 @@ dpll_pin_phase_adj_set(struct dpll_pin *pin, struct nlattr *phase_adj_attr,
if (ref == failed)
break;
ops = dpll_pin_ops(ref);
+ if (!ops->phase_adjust_set)
+ continue;
dpll = ref->dpll;
if (ops->phase_adjust_set(pin, dpll_pin_on_dpll_priv(dpll, pin),
dpll, dpll_priv(dpll), old_phase_adj,
--
2.53.0
^ permalink raw reply related [flat|nested] only message in thread
only message in thread, other threads:[~2026-07-14 13:00 UTC | newest]
Thread overview: (only message) (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-14 12:59 [PATCH net] dpll: use pin owner's dpll ref for pin-level attribute reporting Ivan Vecera
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox