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

[Jiri Pirko]

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.

Thanks!
Jiri