dpll: use pin owner's dpll ref for pin-level attribute reporting

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>
Acked-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>
Link: https://patch.msgid.link/20260714125945.1823269-1-ivecera@redhat.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>

diff --git a/drivers/dpll/dpll_core.c b/drivers/dpll/dpll_core.c
index 2e8690cb3c16eefc588dc370218882870c3d5abe..43d51d942eadd380dd9ab661498e8f31b034745a 100644 (file)
--- a/drivers/dpll/dpll_core.c
+++ b/drivers/dpll/dpll_core.c
@@ -1142,6 +1142,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 e2457711343173c6bef638bae1644972a4190be0..da8a369556ed11c4441bc180a0a9688459c86f47 100644 (file)
--- 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 5703667593a7c238d147d3c8d5fd69ffa0a8e597..afb31c0040382ce3226610b4417a3ec342904806 100644 (file)
--- a/drivers/dpll/dpll_netlink.c
+++ b/drivers/dpll/dpll_netlink.c
@@ -699,7 +699,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);
@@ -1090,12 +1092,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,
@@ -1109,6 +1118,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);
@@ -1128,6 +1139,8 @@ rollback:
                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))
@@ -1151,13 +1164,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,
@@ -1181,6 +1200,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),
@@ -1204,6 +1225,8 @@ rollback:
                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),
@@ -1238,8 +1261,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");
@@ -1258,6 +1284,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,
@@ -1280,6 +1308,8 @@ rollback:
                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,
@@ -1471,12 +1501,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),
@@ -1491,6 +1527,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),
@@ -1513,6 +1551,8 @@ rollback:
                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,