Re: [PATCH v3 net-next 1/8] dt-bindings: dpll: Add device tree bindings for DPLL device and pin

Rob Herring
2025-04-21T22:20:27Z

On Wed, Apr 16, 2025 at 06:21:37PM +0200, Ivan Vecera wrote:
> Add a common DT schema for DPLL device and associated pin.
> The DPLL (device phase-locked loop) is a device used for precise clock
> synchronization in networking and telecom hardware.

In the subject, drop 'device tree binding for'. You already said that
with 'dt-bindings'.

>
> The device itself is equipped with one or more DPLLs (channels) and
> one or more physical input and output pins.
>
> Each DPLL channel is used either to provide pulse-per-clock signal or
> to drive ethernet equipment clock.
>
> The input and output pins have a label (specifies board label),
> type (specifies its usage depending on wiring), list of supported
> or allowed frequencies (depending on how the pin is connected and
> where) and can support embedded sync capability.

Convince me this is something generic... Some example parts or
datasheets would help. For example, wouldn't these devices have 1 or
more power supplies or a reset line?

>
> Signed-off-by: Ivan Vecera <ivecera@redhat.com>
> ---
> v1->v3:
> * rewritten description for both device and pin
> * dropped num-dplls property
> * supported-frequencies property renamed to supported-frequencies-hz
> ---
> .../devicetree/bindings/dpll/dpll-device.yaml | 76 +++++++++++++++++++
> .../devicetree/bindings/dpll/dpll-pin.yaml | 44 +++++++++++
> MAINTAINERS | 2 +
> 3 files changed, 122 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/dpll/dpll-device.yaml
> create mode 100644 Documentation/devicetree/bindings/dpll/dpll-pin.yaml
>
> diff --git a/Documentation/devicetree/bindings/dpll/dpll-device.yaml b/Documentation/devicetree/bindings/dpll/dpll-device.yaml
> new file mode 100644
> index 0000000000000..11a02b74e28b7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/dpll/dpll-device.yaml
> @@ -0,0 +1,76 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/dpll/dpll-device.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Digital Phase-Locked Loop (DPLL) Device
> +
> +maintainers:
> + - Ivan Vecera <ivecera@redhat.com>
> +
> +description:
> + Digital Phase-Locked Loop (DPLL) device is used for precise clock
> + synchronization in networking and telecom hardware. The device can
> + have one or more channels (DPLLs) and one or more physical input and
> + output pins. Each DPLL channel can either produce pulse-per-clock signal
> + or drive ethernet equipment clock. The type of each channel can be
> + indicated by dpll-types property.
> +
> +properties:
> + $nodename:
> + pattern: "^dpll(@.*)?$"

There's no 'reg' property, so you can't ever have a unit-address. I
suppose you can have more than 1, so you need a '-[0-9]+' suffix.

> +
> + "#address-cells":
> + const: 0
> +
> + "#size-cells":
> + const: 0
> +
> + dpll-types:
> + description: List of DPLL channel types, one per DPLL instance.
> + $ref: /schemas/types.yaml#/definitions/non-unique-string-array
> + items:
> + enum: [pps, eec]
> +
> + input-pins:
> + type: object
> + description: DPLL input pins
> + unevaluatedProperties: false
> +
> + properties:
> + "#address-cells":
> + const: 1
> + "#size-cells":
> + const: 0
> +
> + patternProperties:
> + "^pin@[0-9]+$":

Unit-addresses are generally hex.

> + $ref: /schemas/dpll/dpll-pin.yaml
> + unevaluatedProperties: false
> +
> + required:
> + - "#address-cells"
> + - "#size-cells"
> +
> + output-pins:
> + type: object
> + description: DPLL output pins
> + unevaluatedProperties: false
> +
> + properties:
> + "#address-cells":
> + const: 1
> + "#size-cells":
> + const: 0
> +
> + patternProperties:
> + "^pin@[0-9]+$":
> + $ref: /schemas/dpll/dpll-pin.yaml
> + unevaluatedProperties: false
> +
> + required:
> + - "#address-cells"
> + - "#size-cells"
> +
> +additionalProperties: true
> diff --git a/Documentation/devicetree/bindings/dpll/dpll-pin.yaml b/Documentation/devicetree/bindings/dpll/dpll-pin.yaml
> new file mode 100644
> index 0000000000000..44af3a4398a5f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/dpll/dpll-pin.yaml
> @@ -0,0 +1,44 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/dpll/dpll-pin.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: DPLL Pin
> +
> +maintainers:
> + - Ivan Vecera <ivecera@redhat.com>
> +
> +description: |
> + The DPLL pin is either a physical input or output pin that is provided
> + by a DPLL( Digital Phase-Locked Loop) device. The pin is identified by
> + its physical order number that is stored in reg property and can have
> + an additional set of properties like supported (allowed) frequencies,
> + label, type and may support embedded sync.

blank line here if this is a separate paragraph:

> + Note that the pin in this context has nothing to do with pinctrl.
> +
> +properties:
> + reg:
> + description: Hardware index of the DPLL pin.
> + $ref: /schemas/types.yaml#/definitions/uint32

'reg' already has a type. You need to say how many entries (i.e.
'maxItems: 1')

> +
> + esync-control:
> + description: Indicates whether the pin supports embedded sync functionality.
> + type: boolean
> +
> + label:
> + description: String exposed as the pin board label
> + $ref: /schemas/types.yaml#/definitions/string
> +
> + supported-frequencies-hz:
> + description: List of supported frequencies for this pin, expressed in Hz.
> +
> + type:

'type' is too generic of a property name.

> + description: Type of the pin
> + $ref: /schemas/types.yaml#/definitions/string
> + enum: [ext, gnss, int, mux, synce]
> +
> +required:
> + - reg
> +
> +additionalProperties: false
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 1248443035f43..f645ef38d2224 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -7187,6 +7187,8 @@ M: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
> M: Jiri Pirko <jiri@resnulli.us>
> L: netdev@vger.kernel.org
> S: Supported
> +F: Documentation/devicetree/bindings/dpll/dpll-device.yaml
> +F: Documentation/devicetree/bindings/dpll/dpll-pin.yaml
> F: Documentation/driver-api/dpll.rst
> F: drivers/dpll/*
> F: include/linux/dpll.h
> --
> 2.48.1
>