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

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.

The -EOPNOTSUPP validation loop, which checked ops support across all
owner-matching references, is replaced with a direct check on the
single owner reference returned by dpll_pin_own_dpll_ref_first().

The documentation in dpll.rst is updated to reflect that pin-level
attributes are set through the pin owner's dpll reference only.

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 address HW by pin/ref ID regardless of DPLL. The
    ref_sync_set callback was the only one with per-channel behavior,
    addressed by the preceding patch.

Signed-off-by: Ivan Vecera <ivecera@redhat.com>
Reviewed-by: Jiri Pirko <jiri@nvidia.com>
Link: https://patch.msgid.link/20260807095926.386923-3-ivecera@redhat.com
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
This commit is contained in:
Ivan Vecera 2026-08-07 11:59:26 +02:00 committed by Paolo Abeni
parent 346630e46b
commit 84e85c325e
2 changed files with 56 additions and 168 deletions

View File

@ -116,8 +116,9 @@ Shared pins
A single pin object can be attached to multiple dpll devices.
Then there are two groups of configuration knobs:
1) Set on a pin - the configuration affects all dpll devices pin is
registered to (i.e., ``DPLL_A_PIN_FREQUENCY``),
1) Set on a pin - the configuration is a property of the pin itself and
applies to all dpll devices the pin is registered with
(i.e., ``DPLL_A_PIN_FREQUENCY``),
2) Set on a pin-dpll tuple - the configuration affects only selected
dpll device (i.e., ``DPLL_A_PIN_PRIO``, ``DPLL_A_PIN_STATE``,
``DPLL_A_PIN_DIRECTION``).
@ -507,9 +508,9 @@ as well as parameter being configured (``DPLL_A_MODE``).
``DPLL_CMD_PIN_SET`` - to target a pin user must provide a
``DPLL_A_PIN_ID``, which is unique identifier of a pin in the system.
Also configured pin parameters must be added.
If ``DPLL_A_PIN_FREQUENCY`` is configured, this affects all the dpll
devices that are connected with the pin, that is why frequency attribute
shall not be enclosed in ``DPLL_A_PIN_PARENT_DEVICE``.
If ``DPLL_A_PIN_FREQUENCY`` is configured, it is a property of the pin
itself and applies to all dpll devices the pin is registered with, so the
frequency attribute shall not be enclosed in ``DPLL_A_PIN_PARENT_DEVICE``.
Other attributes: ``DPLL_A_PIN_PRIO``, ``DPLL_A_PIN_STATE`` or
``DPLL_A_PIN_DIRECTION`` must be enclosed in
``DPLL_A_PIN_PARENT_DEVICE`` as their configuration relates to only one

View File

@ -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