Re: [PATCH net-next v2 2/2] dpll: use pin owner's dpll ref for pin-level attribute setting

[Paolo Abeni]

On 8/3/26 2:02 PM, Ivan Vecera wrote:
> 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@xxxxxxxxxx>
> ---
>  Documentation/driver-api/dpll.rst |  10 +-
>  drivers/dpll/dpll_netlink.c       | 213 +++++++-----------------------
>  2 files changed, 55 insertions(+), 168 deletions(-)
> 
> diff --git a/Documentation/driver-api/dpll.rst b/Documentation/driver-api/dpll.rst
> index f83150917814e2..6fb50e53475c09 100644
> --- a/Documentation/driver-api/dpll.rst
> +++ b/Documentation/driver-api/dpll.rst
> @@ -116,8 +116,8 @@ 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 performed through the pin owner's
> +   dpll reference only (i.e., ``DPLL_A_PIN_FREQUENCY``),

I find the new text confusing; it seems to me that the pin configuration
now affects a single DPLL.

Sashiko nipa has several comments, please have a look:

https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260803120245.56046-1-ivecera%40redhat.com

and also please be aware of net-next commit c82ff94592fb.

/P

/P