From: Paolo Abeni <pabeni@redhat.com>
Sent: Monday, December 19, 2022 10:13 AM

Hello,

I have a just a few minor notes WRT the documentation - which was a
very useful entry point for me to help understanding the subsystem.

Hi Paolo, many thanks for your feedback!

On Wed, 2022-11-30 at 00:37 +0300, Vadim Fedorenko wrote:

quoted

From: Vadim Fedorenko <redacted>

Add documentation explaining common netlink interface to configure DPLL
devices and monitoring events. Common way to implement DPLL device in
a driver is also covered.

Co-developed-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
Signed-off-by: Vadim Fedorenko <redacted>
---
 Documentation/networking/dpll.rst  | 271 +++++++++++++++++++++++++++++
 Documentation/networking/index.rst |   1 +
 2 files changed, 272 insertions(+)
 create mode 100644 Documentation/networking/dpll.rst

diff --git a/Documentation/networking/dpll.rst

b/Documentation/networking/dpll.rst

quoted

new file mode 100644
index 000000000000..58401e2b70a7

--- /dev/null
+++ b/Documentation/networking/dpll.rst

@@ -0,0 +1,271 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+===============================
+The Linux kernel DPLL subsystem
+===============================
+
+
+The main purpose of DPLL subsystem is to provide general interface
+to configure devices that use any kind of Digital PLL and could use
+different sources of signal to synchronize to as well as different
+types of outputs.
+The main interface is NETLINK_GENERIC based protocol with an event
+monitoring multicast group defined.
+
+
+Pin object
+==========
+A pin is amorphic object which represents either input and output, it
+could be internal component of the device, as well as externaly
+connected.
+The number of pins per dpll vary, but usually multiple pins shall be
+provided for a single dpll device.
+Direction of a pin and it's capabilities are provided to the user in
+response for netlink dump request messages.
+Pin can be shared by multiple dpll devices. Where configuration on one
+pin can alter multiple dplls (i.e. DPLL_PIN_SGINAL_TYPE, DPLL_PIN_TYPE,

Likely typo above: DPLL_PIN_SIGNAL_TYPE

True, shall be fixed in next version.

quoted

+DPLL_PIN_STATE), or just one pin-dpll pair (i.e. DPLL_PIN_PRIO).
+Pin can be also a MUX type, where one or more pins are attached to
+a parent pin. The parent pin is the one directly connected to the dpll,
+which may be used by dplls in DPLL_MODE_AUTOMATIC selection mode, where
+only pins directly connected to the dpll are capable of automatic
+source pin selection. In such case, pins are dumped with
+DPLLA_PIN_PARENT_IDX, and are able to be selected by the userspace with
+netlink request.
+
+Configuration commands group
+============================
+
+Configuration commands are used to get or dump information about
+registered DPLL devices (and pins), as well as set configuration of
+device or pins. As DPLL device could not be abstract and reflects real
+hardware, there is no way to add new DPLL device via netlink from user
+space and each device should be registered by it's driver.

Side note: in the long run we could end-up with a virtual/dummy dpll
driver for self-tests and/or reference's implementation sake.

True, seems a good idea.

quoted

+
+List of command with possible attributes
+========================================
+
+All constants identifying command types use ``DPLL_CMD_`` prefix and
+suffix according to command purpose. All attributes use ``DPLLA_``
+prefix and suffix according to attribute purpose:
+
+  ============================  =======================================
+  ``DEVICE_GET``                userspace to get device info
+    ``ID``                      attr internal dpll device index
+    ``NAME``                    attr dpll device name
+    ``MODE``                    attr selection mode
+    ``MODE_SUPPORTED``          attr available selection modes
+    ``SOURCE_PIN_IDX``          attr index of currently selected source
+    ``LOCK_STATUS``             attr internal frequency-lock status
+    ``TEMP``                    attr device temperature information
+    ``NETIFINDEX``              attr dpll owner Linux netdevice index

should we include also the cookie (or wuhatever will be used for
persistent device identification) into the readable attributes list?

In next version cookie is replaced with clock_id and will be also available
for the userspace.

quoted

+  ``DEVICE_SET``                userspace to set dpll device
+                                configuration
+    ``ID``                      attr internal dpll device index
+    ``MODE``                    attr selection mode to configure
+    ``PIN_IDX``                 attr index of source pin to select as
+                                active source

It looks like the descrition for the above attribute ('PIN_IDX') and
'SOURCE_PIN_IDX' has been swapped.

Good catch, for ``DEVICE_SET`` command, proper attribute is 'SOURCE_PIN_IDX',
will fix that.

quoted

+  ``PIN_SET``                   userspace to set pins configuration
+    ``ID``                      attr internal dpll device index
+    ``PIN_IDX``                 attr index of a pin to configure
+    ``PIN_TYPE``                attr type configuration value for
+                                selected pin
+    ``PIN_SIGNAL_TYPE``         attr signal type configuration value
+                                for selected pin
+    ``PIN_CUSTOM_FREQ``         attr signal custom frequency to be set
+    ``PIN_STATE``               attr pin state to be set
+    ``PIN_PRIO``                attr pin priority to be set
+
+Netlink dump requests
+=====================
+The ``DEVICE_GET`` command is capable of dump type netlink requests.
+In such case the userspace shall provide ``DUMP_FILTER`` attribute
+value to filter the response as required.
+If filter is not provided only name and id of available dpll(s) is
+provided. If the request also contains ``ID`` attribute, only selected
+dpll device shall be dumped.

Should we explicitly document even the required permissions?

Sure, going to add a word about required netlink permission.

quoted

+
+Possible response message attributes for netlink requests depending on
+the value of ``DPLLA_DUMP_FILTER`` attribute:
+
+  =============================== ====================================
+  ``DPLL_DUMP_FILTER_PINS``       value of ``DUMP_FILTER`` attribute
+    ``PIN``                       attr nested type contain single pin
+                                  attributes
+    ``PIN_IDX``                   attr index of dumped pin
+    ``PIN_DESCRIPTION``           description of a pin provided by
+                                  driver
+    ``PIN_TYPE``                  attr value of pin type
+    ``PIN_TYPE_SUPPORTED``        attr value of supported pin type
+    ``PIN_SIGNAL_TYPE``           attr value of pin signal type
+    ``PIN_SIGNAL_TYPE_SUPPORTED`` attr value of supported pin signal
+                                  type
+    ``PIN_CUSTOM_FREQ``           attr value of pin custom frequency
+    ``PIN_STATE``                 attr value of pin state
+    ``PIN_STATE_SUPPORTED``       attr value of supported pin state
+    ``PIN_PRIO``                  attr value of pin prio
+    ``PIN_PARENT_IDX``            attr value of pin patent index
+    ``PIN_NETIFINDEX``            attr value of netdevice assocaiated
+                                  with the pin
+  ``DPLL_DUMP_FILTER_STATUS``     value of ``DUMP_FILTER`` attribute
+    ``ID``                        attr internal dpll device index
+    ``NAME``                      attr dpll device name
+    ``MODE``                      attr selection mode
+    ``MODE_SUPPORTED``            attr available selection modes
+    ``SOURCE_PIN_IDX``            attr index of currently selected
+                                  source
+    ``LOCK_STATUS``               attr internal frequency-lock status
+    ``TEMP``                      attr device temperature information
+    ``NETIFINDEX``                attr dpll owner Linux netdevice index
+
+
+The pre-defined enums
+=====================
+
+All the enums use the ``DPLL_`` prefix.
+
+Values for ``PIN_TYPE`` and ``PIN_TYPE_SUPPORTED`` attributes:
+
+  ============================ ========================================
+  ``PIN_TYPE_MUX``             MUX type pin, connected pins shall
+                               have their own types
+  ``PIN_TYPE_EXT``             External pin
+  ``PIN_TYPE_SYNCE_ETH_PORT``  SyncE on Ethernet port
+  ``PIN_TYPE_INT_OSCILLATOR``  Internal Oscillator (i.e. Holdover
+                               with Atomic Clock as a Source)
+  ``PIN_TYPE_GNSS``            GNSS 1PPS source
+
+Values for ``PIN_SIGNAL_TYPE`` and ``PIN_SIGNAL_TYPE_SUPPORTED``
+attributes:
+
+  ===============================  ===================================
+  ``PIN_SIGNAL_TYPE_1_PPS``        1 Hz frequency
+  ``PIN_SIGNAL_TYPE_10_MHZ``       10 MHz frequency
+  ``PIN_SIGNAL_TYPE_CUSTOM_FREQ``  Frequency value provided in attr
+                                   ``PIN_CUSTOM_FREQ``
+
+Values for ``LOCK_STATUS`` attribute:
+
+  ============================= ======================================
+  ``LOCK_STATUS_UNLOCKED``      DPLL is in freerun, not locked to any
+                                source pin
+  ``LOCK_STATUS_CALIBRATING``   DPLL device calibrates to lock to the
+                                source pin signal
+  ``LOCK_STATUS_LOCKED``        DPLL device is locked to the source
+                                pin frequency
+  ``LOCK_STATUS_HOLDOVER``      DPLL device lost a lock, using its
+                                frequency holdover capabilities
+
+Values for ``PIN_STATE`` and ``PIN_STATE_SUPPORTED`` attributes:
+
+============================= ============================
+  ``PIN_STATE_CONNECTED``     Pin connected to a dpll
+  ``PIN_STATE_DISCONNECTED``  Pin disconnected from dpll
+  ``PIN_STATE_SOURCE``        Source pin
+  ``PIN_STATE_OUTPUT``        Output pin
+
+Possible DPLL source selection mode values:
+
+  =================== ================================================
+  ``MODE_FORCED``     source pin is force-selected by
+                      ``DPLL_CMD_DEVICE_SET`` with given value of
+                      ``DPLLA_SOURCE_PIN_IDX`` attribute
+  ``MODE_AUTOMATIC``  source pin ise auto selected according to

typo above 'ise' -> 'is'

Sure, will fix.

Thanks again!

BR,
Arkadiusz

Cheers,

Paolo