Re: [PATCH net-next v5 1/4] dpll: add DPLL_PIN_TYPE_INT_NCO pin type

[Vadim Fedorenko]

On 09/06/2026 12:06, Jiri Pirko wrote:
> Mon, Jun 08, 2026 at 01:33:56PM +0200, vadim.fedorenko@xxxxxxxxx wrote:
> > On 04/06/2026 17:42, Ivan Vecera wrote:
> > > On 6/4/26 5:16 PM, Jakub Kicinski wrote:
> > > > On Thu, 4 Jun 2026 17:01:36 +0200 Ivan Vecera wrote:
> > > > > > Purely going on intuition here but feels like NCO should be a mode
> > > > > > (enum dpll_mode) rather than one of the input pins?
> > > > > >
> > > > > > More acks here would be great, Vadim, Arkadiusz, Grzegorz... ?
> > > > >
> > > > > I had a long discussion with Jiri about this and we agreed finally
> > > > > that dpll_mode represents a reference (input pin) selection strategy
> > > > > mode and not a DPLL device running mode.
> > > >
> > > > Long discussion? I see 2 emails ;) Let's hear from others.
> > > > (thanks for the link BTW, _if_ there's a v6 please put it in the cover
> > > > letter)
> > >
> > > I called him... he explained me 'why?' in detail.
> > > I also appreciate others' opinion.
> >
> > Well, NCO mode means manual operation of frequency tuning. Does it mean
> > that different tunings may be applied to different out pins of DPLL
> > device? My assumption that it's not possible, and in this case NCO is
> > property/mode of DPLL device rather than single pin.
> >
> > @Jiri could you please share your detailed explanation on "why"?
>
> Since the "why a pin and not a new dpll_mode?" question keeps coming up,
> let me try to describe why I believe that modelling NCO as an input pin
> (DPLL_PIN_TYPE_INT_NCO) is the right thing to do.
>
> In the DPLL UAPI, 'mode' only describes the *input selection policy*:
> MANUAL means userspace picks which input the loop locks to, AUTOMATIC
> means the DPLL auto-selects the highest-priority input. I know there was
> some fuzz about this semantics in the early stages of upstreaming DPLL
> subsystem, but eventually this became very clear both in code and in
> kdoc:
>
> <qoute>
>   * enum dpll_mode - working modes a dpll can support, differentiates if and how
>   *   dpll selects one of its inputs to syntonize with it, valid values for
>   *   DPLL_A_MODE attribute
> </quote>
>
> NCO *is not* a third selection policy - it is just another *source* the
> loop is disciplined from. Except the source is steered by the host
> (via the PHC .adjfine() path) instead of being an external reference.
> Think of it as a virtual pin of some sort.
>
> The object we already use for "a source the DPLL can lock to" is a pin,
> so an internal NCO belongs right next to DPLL_PIN_TYPE_INT_OSCILLATOR,
> which is already existing example of a similar virtual pin.
>
> By having NCO as an input pin we reuse the existing model instead of
> inventing a parallel one. "Run as NCO" becomes "connect the NCO input"
> using the same connect/disconnect, pin state and pin-dump infrastructure
> as any other input. No new control surface, and it stays orthogonal to
> mode: we don't have to define what AUTOMATIC+NCO or pin priorities
> mean, and we don't grow enum dpll_mode and the supported-modes
> bitmask that every mode-aware consumer would then have to relearn.
>
> For the pin info uAPI exposure we reuse the attributes pins already have
> - the output frequency offset from nominal is reported via the pin's
> fractional-frequency-offset / -ppt. A new device mode would need
> brand new device-level attributes for the same information.
>
> Having said that, I think it's a perfect fit. The only "real" pull
> towards a new mode is that vendor datasheets call this NCO/DCO a "mode".
> But that is HW register terminology and we learned many times in
> the past that may be more or less misleading/incorrect wrt the uAPI.
>
> Therefore my strong preference is DPLL_PIN_TYPE_INT_NCO, no new mode.
> Honestly, I don't really understand why it would make even little sense
> to have this as new mode. Perhaps I'm missing something, if you can
> describe it, that would be awesome.

Ok, I see your point. Even though the pin UAPI fits the model, I still
have some concerns:

1. I cannot really imagine AUTOMATIC mode selecting NCO pin by priority
in case other pins are gone somehow. It doesn't make sense without
steering SW running on the host. And the other way around - switching to
a higher priority pin while SW is keep "steering" DPLL. But looks like
we have discussed it in the other thread. Adding DPLL mode restrictions
based on pin selection/connection breaks the model, I think...

2. SW steering cannot be pure SW. Every disciplining algorithm relies on
measurements, the product of phase comparators. That technically means
the device has to have other inputs configure as monitor, which can be 
configured in AUTO mode with priorities. How will we model it then?

Thanks,
Vadim