All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net] dpll: use pin owner's dpll ref for pin-level attribute reporting
@ 2026-07-14 12:59 Ivan Vecera
  2026-07-22 21:02 ` Jakub Kicinski
  2026-07-22 22:13 ` Vadim Fedorenko
  0 siblings, 2 replies; 3+ messages 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] 3+ messages in thread

* Re: [PATCH net] dpll: use pin owner's dpll ref for pin-level attribute reporting
  2026-07-14 12:59 [PATCH net] dpll: use pin owner's dpll ref for pin-level attribute reporting Ivan Vecera
@ 2026-07-22 21:02 ` Jakub Kicinski
  2026-07-22 22:13 ` Vadim Fedorenko
  1 sibling, 0 replies; 3+ messages in thread
From: Jakub Kicinski @ 2026-07-22 21:02 UTC (permalink / raw)
  To: Vadim Fedorenko, Arkadiusz Kubalewski, Jiri Pirko
  Cc: Ivan Vecera, netdev, Aleksandr Loktionov, Grzegorz Nitka,
	open list

On Tue, 14 Jul 2026 14:59:44 +0200 Ivan Vecera wrote:
> 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.

DPLL maintainers, please review.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH net] dpll: use pin owner's dpll ref for pin-level attribute reporting
  2026-07-14 12:59 [PATCH net] dpll: use pin owner's dpll ref for pin-level attribute reporting Ivan Vecera
  2026-07-22 21:02 ` Jakub Kicinski
@ 2026-07-22 22:13 ` Vadim Fedorenko
  1 sibling, 0 replies; 3+ messages in thread
From: Vadim Fedorenko @ 2026-07-22 22:13 UTC (permalink / raw)
  To: Ivan Vecera, netdev
  Cc: Arkadiusz Kubalewski, Jiri Pirko, Aleksandr Loktionov,
	Grzegorz Nitka, Jakub Kicinski, open list

On 14/07/2026 13:59, Ivan Vecera wrote:
> 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) {

ref->dpll->module == pin->module && ref->dpll->clock_id == pin->clock_id
is repeated multiple times in the patch, maybe it's better to make a
helper with some self-explaining name?

Apart from that patch looks reasonable, but I would like to have
explicit tag from Jiri.

Acked-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>

> +			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,

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-07-22 22:14 UTC | newest]

Thread overview: 3+ messages (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
2026-07-22 21:02 ` Jakub Kicinski
2026-07-22 22:13 ` Vadim Fedorenko

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.