From: Ivan Vecera <ivecera@redhat.com>
To: netdev@vger.kernel.org
Cc: Petr Oros <poros@redhat.com>,
Vadim Fedorenko <vadim.fedorenko@linux.dev>,
Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>,
Jiri Pirko <jiri@resnulli.us>,
Richard Cochran <richardcochran@gmail.com>,
linux-kernel@vger.kernel.org (open list)
Subject: [PATCH net-next] dpll: use pin owner's dpll ref for pin-level attribute setting
Date: Fri, 31 Jul 2026 16:11:45 +0200 [thread overview]
Message-ID: <20260731141146.1119147-1-ivecera@redhat.com> (raw)
Pin-level attributes (frequency, phase adjust, embedded sync, reference
sync) are properties of the pin itself, not of a particular DPLL device.
The get callbacks already use only the pin owner's DPLL reference
(via dpll_pin_own_dpll_ref_first()), but the set callbacks iterate over
all registered DPLL references and invoke the set operation on each one.
This is redundant because a pin is a single physical entity — setting
its frequency or phase adjust once through the owner's ops is sufficient.
Calling set on every registered DPLL just results in duplicate HW writes
for drivers that share a pin across multiple DPLL devices (e.g. ice
registers each input pin with both the EEC and PPS DPLL, zl3073x
registers input pins with every DPLL channel).
Simplify dpll_pin_freq_set(), dpll_pin_esync_set(),
dpll_pin_ref_sync_state_set() and dpll_pin_phase_adj_set() to call the
set callback only through the owner's DPLL reference, matching the
existing get-side behavior. This removes the xa_for_each iteration
loops, the now-unnecessary rollback logic, and several local variables.
No existing driver is affected:
- ptp_ocp and mlx5 register each pin with a single DPLL.
- ice registers input pins with two DPLLs (EEC and PPS) using
identical ops and pin_priv; the set callbacks address the HW by
pin index, not by DPLL, so the second call was a no-op.
- zl3073x registers input pins with every DPLL channel; the set
callbacks also address HW by pin/ref ID regardless of DPLL.
Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
drivers/dpll/dpll_netlink.c | 213 +++++++++---------------------------
1 file changed, 50 insertions(+), 163 deletions(-)
diff --git a/drivers/dpll/dpll_netlink.c b/drivers/dpll/dpll_netlink.c
index afb31c0040382c..a909cd4451b008 100644
--- a/drivers/dpll/dpll_netlink.c
+++ b/drivers/dpll/dpll_netlink.c
@@ -1079,10 +1079,9 @@ dpll_pin_freq_set(struct dpll_pin *pin, struct nlattr *a,
struct netlink_ext_ack *extack)
{
u64 freq = nla_get_u64(a), old_freq;
- struct dpll_pin_ref *ref, *failed;
const struct dpll_pin_ops *ops;
+ struct dpll_pin_ref *ref;
struct dpll_device *dpll;
- unsigned long i;
int ret;
if (!dpll_pin_is_freq_supported(pin, freq)) {
@@ -1090,22 +1089,17 @@ dpll_pin_freq_set(struct dpll_pin *pin, struct nlattr *a,
return -EINVAL;
}
- xa_for_each(&pin->dpll_refs, i, ref) {
- ops = dpll_pin_ops(ref);
- 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_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->frequency_set || !ops->frequency_get) {
+ NL_SET_ERR_MSG(extack,
+ "frequency set not supported by the device");
+ return -EOPNOTSUPP;
+ }
dpll = ref->dpll;
ret = ops->frequency_get(pin, dpll_pin_on_dpll_priv(dpll, pin), dpll,
dpll_priv(dpll), &old_freq, extack);
@@ -1116,68 +1110,42 @@ dpll_pin_freq_set(struct dpll_pin *pin, struct nlattr *a,
if (freq == old_freq)
return 0;
- 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);
- if (ret) {
- failed = ref;
- NL_SET_ERR_MSG_FMT(extack, "frequency set failed for dpll_id:%u",
- dpll->id);
- goto rollback;
- }
+ ret = ops->frequency_set(pin, dpll_pin_on_dpll_priv(dpll, pin),
+ dpll, dpll_priv(dpll), freq, extack);
+ if (ret) {
+ NL_SET_ERR_MSG_FMT(extack,
+ "frequency set failed for dpll_id:%u",
+ dpll->id);
+ return ret;
}
__dpll_pin_change_ntf(pin);
return 0;
-
-rollback:
- xa_for_each(&pin->dpll_refs, i, ref) {
- 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))
- NL_SET_ERR_MSG(extack, "set frequency rollback failed");
- }
- return ret;
}
static int
dpll_pin_esync_set(struct dpll_pin *pin, struct nlattr *a,
struct netlink_ext_ack *extack)
{
- struct dpll_pin_ref *ref, *failed;
const struct dpll_pin_ops *ops;
struct dpll_pin_esync esync;
u64 freq = nla_get_u64(a);
+ struct dpll_pin_ref *ref;
struct dpll_device *dpll;
bool supported = false;
- unsigned long i;
- int ret;
+ int ret, i;
- xa_for_each(&pin->dpll_refs, i, ref) {
- ops = dpll_pin_ops(ref);
- 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_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->esync_set || !ops->esync_get) {
+ NL_SET_ERR_MSG(extack,
+ "embedded sync feature is not supported by this device");
+ return -EOPNOTSUPP;
+ }
dpll = ref->dpll;
ret = ops->esync_get(pin, dpll_pin_on_dpll_priv(dpll, pin), dpll,
dpll_priv(dpll), &esync, extack);
@@ -1196,44 +1164,17 @@ dpll_pin_esync_set(struct dpll_pin *pin, struct nlattr *a,
return -EINVAL;
}
- xa_for_each(&pin->dpll_refs, i, ref) {
- 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),
- freq, extack);
- if (ret) {
- failed = ref;
- NL_SET_ERR_MSG_FMT(extack,
- "embedded sync frequency set failed for dpll_id: %u",
- dpll->id);
- goto rollback;
- }
+ ret = ops->esync_set(pin, dpll_pin_on_dpll_priv(dpll, pin), dpll,
+ dpll_priv(dpll), freq, extack);
+ if (ret) {
+ NL_SET_ERR_MSG_FMT(extack,
+ "embedded sync frequency set failed for dpll_id: %u",
+ dpll->id);
+ return ret;
}
__dpll_pin_change_ntf(pin);
return 0;
-
-rollback:
- xa_for_each(&pin->dpll_refs, i, ref) {
- void *pin_dpll_priv;
-
- 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),
- esync.freq, extack))
- NL_SET_ERR_MSG(extack, "set embedded sync frequency rollback failed");
- }
- return ret;
}
static int
@@ -1241,14 +1182,12 @@ dpll_pin_ref_sync_state_set(struct dpll_pin *pin,
unsigned long ref_sync_pin_idx,
const enum dpll_pin_state state,
struct netlink_ext_ack *extack)
-
{
- struct dpll_pin_ref *ref, *failed;
const struct dpll_pin_ops *ops;
enum dpll_pin_state old_state;
struct dpll_pin *ref_sync_pin;
+ struct dpll_pin_ref *ref;
struct dpll_device *dpll;
- unsigned long i;
int ret;
ref_sync_pin = xa_find(&pin->ref_sync_pins, &ref_sync_pin_idx,
@@ -1282,42 +1221,20 @@ dpll_pin_ref_sync_state_set(struct dpll_pin *pin,
}
if (state == old_state)
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,
- dpll_pin_on_dpll_priv(dpll,
- ref_sync_pin),
- state, extack);
- if (ret) {
- failed = ref;
- NL_SET_ERR_MSG_FMT(extack, "reference sync set failed for dpll_id:%u",
- dpll->id);
- goto rollback;
- }
+
+ ret = ops->ref_sync_set(pin, dpll_pin_on_dpll_priv(dpll, pin),
+ ref_sync_pin,
+ dpll_pin_on_dpll_priv(dpll, ref_sync_pin),
+ state, extack);
+ if (ret) {
+ NL_SET_ERR_MSG_FMT(extack,
+ "reference sync set failed for dpll_id:%u",
+ dpll->id);
+ return ret;
}
__dpll_pin_change_ntf(pin);
return 0;
-
-rollback:
- xa_for_each(&pin->dpll_refs, i, ref) {
- 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,
- dpll_pin_on_dpll_priv(dpll, ref_sync_pin),
- old_state, extack))
- NL_SET_ERR_MSG(extack, "set reference sync rollback failed");
- }
- return ret;
}
static int
@@ -1478,11 +1395,10 @@ static int
dpll_pin_phase_adj_set(struct dpll_pin *pin, struct nlattr *phase_adj_attr,
struct netlink_ext_ack *extack)
{
- struct dpll_pin_ref *ref, *failed;
const struct dpll_pin_ops *ops;
s32 phase_adj, old_phase_adj;
+ struct dpll_pin_ref *ref;
struct dpll_device *dpll;
- unsigned long i;
int ret;
phase_adj = nla_get_s32(phase_adj_attr);
@@ -1499,21 +1415,16 @@ dpll_pin_phase_adj_set(struct dpll_pin *pin, struct nlattr *phase_adj_attr,
return -EINVAL;
}
- xa_for_each(&pin->dpll_refs, i, ref) {
- ops = dpll_pin_ops(ref);
- 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_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->phase_adjust_set || !ops->phase_adjust_get) {
+ NL_SET_ERR_MSG(extack, "phase adjust not supported");
+ return -EOPNOTSUPP;
+ }
dpll = ref->dpll;
ret = ops->phase_adjust_get(pin, dpll_pin_on_dpll_priv(dpll, pin),
dpll, dpll_priv(dpll), &old_phase_adj,
@@ -1525,41 +1436,17 @@ dpll_pin_phase_adj_set(struct dpll_pin *pin, struct nlattr *phase_adj_attr,
if (phase_adj == old_phase_adj)
return 0;
- 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),
- dpll, dpll_priv(dpll), phase_adj,
- extack);
- if (ret) {
- failed = ref;
- NL_SET_ERR_MSG_FMT(extack,
- "phase adjust set failed for dpll_id:%u",
- dpll->id);
- goto rollback;
- }
+ ret = ops->phase_adjust_set(pin, dpll_pin_on_dpll_priv(dpll, pin),
+ dpll, dpll_priv(dpll), phase_adj, extack);
+ if (ret) {
+ NL_SET_ERR_MSG_FMT(extack,
+ "phase adjust set failed for dpll_id:%u",
+ dpll->id);
+ return ret;
}
__dpll_pin_change_ntf(pin);
return 0;
-
-rollback:
- xa_for_each(&pin->dpll_refs, i, ref) {
- 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,
- extack))
- NL_SET_ERR_MSG(extack, "set phase adjust rollback failed");
- }
- return ret;
}
static int
base-commit: 2fbade66245059c78daeaccfce13ecf499fffb51
--
2.54.0
reply other threads:[~2026-07-31 14:12 UTC|newest]
Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260731141146.1119147-1-ivecera@redhat.com \
--to=ivecera@redhat.com \
--cc=arkadiusz.kubalewski@intel.com \
--cc=jiri@resnulli.us \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=poros@redhat.com \
--cc=richardcochran@gmail.com \
--cc=vadim.fedorenko@linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox