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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250421222025.GA3015001-robh@kernel.org>
Date: Mon, 21 Apr 2025 17:20:25 -0500
From: Rob Herring <robh@...nel.org>
To: Ivan Vecera <ivecera@...hat.com>
Cc: netdev@...r.kernel.org, Vadim Fedorenko <vadim.fedorenko@...ux.dev>,
	Arkadiusz Kubalewski <arkadiusz.kubalewski@...el.com>,
	Jiri Pirko <jiri@...nulli.us>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>,
	Prathosh Satish <Prathosh.Satish@...rochip.com>,
	Lee Jones <lee@...nel.org>, Kees Cook <kees@...nel.org>,
	Andy Shevchenko <andy@...nel.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Michal Schmidt <mschmidt@...hat.com>, devicetree@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-hardening@...r.kernel.org
Subject: Re: [PATCH v3 net-next 1/8] dt-bindings: dpll: Add device tree
 bindings for DPLL device and pin

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@...hat.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@...hat.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@...hat.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@...el.com>
>  M:	Jiri Pirko <jiri@...nulli.us>
>  L:	netdev@...r.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
>