Re: [RFC PATCH v7 1/8] dpll: spec: Add Netlink spec in YAML

[Jakub Kicinski <kuba@kernel.org>]

On Thu, 11 May 2023 07:40:26 +0000 Kubalewski, Arkadiusz wrote:
> >> Remove "no holdover available". This is not a state, this is a mode
> >> configuration. If holdover is or isn't available, is a runtime info.  
> >
> >Agreed, seems a little confusing now. Should we expose the system clk
> >as a pin to be able to force lock to it? Or there's some extra magic
> >at play here?  
> 
> In freerun you cannot lock to anything it, it just uses system clock from
> one of designated chip wires (which is not a part of source pins pool) to feed
> the dpll. Dpll would only stabilize that signal and pass it further.
> Locking itself is some kind of magic, as it usually takes at least ~15 seconds
> before it locks to a signal once it is selected.

Okay, I guess that makes sense.

I was wondering if there may be a DPLLs which allow other input clocks
to bypass the PLL logic, and output purely a stabilized signal. In
which case we should model this as a generic PLL bypass, FREERUN being
just one special case where we're bypassing with the system clock.

But that may well be a case of "software guy thinking", so if nobody
thinks this can happen in practice we can keep FREERUN.

> >Noob question, what is NCO in terms of implementation?
> >We source the signal from an arbitrary pin and FW / driver does
> >the control? Or we always use system refclk and then tune?
> >  
> 
> Documentation of chip we are using, stated NCO as similar to FREERUN, and it
> runs on a SYSTEM CLOCK provided to the chip (plus some stabilization and
> dividers before it reaches the output).
> It doesn't count as an source pin, it uses signal form dedicated wire for
> SYSTEM CLOCK.
> In this case control over output frequency is done by synchronizer chip
> firmware, but still it will not lock to any source pin signal.

Reading wikipedia it sounds like NCO is just a way of generating 
a waveform from synchronous logic.

Does the DPLL not allow changing clock frequency when locked?
I.e. feeding it one frequency and outputting another?
Because I think that'd be done by an NCO, no?

> >> Is it needed to mention the holdover mode. It's slightly confusing,
> >> because user might understand that the lock-status is always "holdover"
> >> in case of "holdover" mode. But it could be "unlocked", can't it?
> >> Perhaps I don't understand the flows there correctly :/  
> >
> >Hm, if we want to make sure that holdover mode must result in holdover
> >state then we need some extra atomicity requirements on the SET
> >operation. To me it seems logical enough that after setting holdover
> >mode we'll end up either in holdover or unlocked status, depending on
> >lock status when request reached the HW.
> >  
> 
> Improved the docs:
>         name: holdover
>         doc: |
>           dpll is in holdover state - lost a valid lock or was forced
>           by selecting DPLL_MODE_HOLDOVER mode (latter possible only
>           when dpll lock-state was already DPLL_LOCK_STATUS_LOCKED,
> 	  if it was not, the dpll's lock-status will remain
>           DPLL_LOCK_STATUS_UNLOCKED even if user requests
>           DPLL_MODE_HOLDOVER)
> Is that better?

Yes, modulo breaking it up into sentences, as Jiri says.

> What extra atomicity you have on your mind?
> Do you suggest to validate and allow (in dpll_netlink.c) only for 'unlocked'
> or 'holdover' states of dpll, once DPLL_MODE_HOLDOVER was successfully
> requested by the user?

No, I was saying that making sure that we end up in holdover (rather
than unlocked) when user requested holdover is hard, and we shouldn't 
even try to implement that.