Message-ID: <DM6PR11MB4657EF0A57977C56264BC7A99B749@DM6PR11MB4657.namprd11.prod.outlook.com>
Date: Thu, 11 May 2023 20:53:40 +0000
From: "Kubalewski, Arkadiusz" <arkadiusz.kubalewski@...el.com>
To: Jakub Kicinski <kuba@...nel.org>
CC: Vadim Fedorenko <vadfed@...a.com>, Jiri Pirko <jiri@...nulli.us>, Jonathan
 Lemon <jonathan.lemon@...il.com>, Paolo Abeni <pabeni@...hat.com>, "Olech,
 Milena" <milena.olech@...el.com>, "Michalik, Michal"
	<michal.michalik@...el.com>, "linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>, poros <poros@...hat.com>, mschmidt
	<mschmidt@...hat.com>, "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"linux-clk@...r.kernel.org" <linux-clk@...r.kernel.org>, Vadim Fedorenko
	<vadim.fedorenko@...ux.dev>
Subject: RE: [RFC PATCH v7 1/8] dpll: spec: Add Netlink spec in YAML

>From: Jakub Kicinski <kuba@...nel.org>
>Sent: Thursday, May 11, 2023 5:21 PM
>
>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.
>

Well I am not saying such use-case doesn't exist, but haven't heard about it.

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

Well our dpll (actually synchronizer chip) does that in AUTOMATIC/MANUAL modes,
i.e. you feed 1 PPS from gnss and output feed for PHY's ~156 MHZ, so I guess
this is pretty common for complex synchronizer chips, although AFAIK this is
achieved with additional signal synthesizers after the PLL logic.

>Because I think that'd be done by an NCO, no?

>From docs I can also see that chip has additional designated dpll for NCO mode,
and this statement:
"Numerically controlled oscillator (NCO) behavior allows system software to steer
DPLL frequency or synthesizer frequency with resolution better than 0.005 ppt."

I am certainly not an expert on this, but seems like the NCO mode for this chip
is better than FREERUN, since signal produced on output is somehow higher quality.

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

Sure, will do.

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

Okay.

Thank you!
Arkadiusz

Powered by blists - more mailing lists