| Message ID | 20250407173149.1010216-6-ivecera@redhat.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | Add Microchip ZL3073x support | expand |
On 07/04/2025 19:31, Ivan Vecera wrote: > This adds DT bindings schema for DPLL (device phase-locked loop) Please do not use "This commit/patch/change", but imperative mood. See longer explanation here: https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95 A nit, subject: drop second/last, redundant "device tree bindings for". The "dt-bindings" prefix is already stating that these are bindings. See also: https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18 > device and associated pin. The schema follows existing DPLL core API What is core API in terms of Devicetree? > and should be used to expose information that should be provided > by platform firmware. > > The schema for DPLL device describe a DPLL chip that can contain > one or more DPLLs (channels) and platform can specify their types. > For now 'pps' and 'eec' types supported and these values are mapped > to DPLL core's enums. Describe entire hardware, not what is supported. > > The DPLL device can have optionally 'input-pins' and 'output-pins' > sub-nodes that contain pin sub-nodes. > > These pin sub-nodes follows schema for dpll-pin and can contain > information about the particular pin. Describe the hardware, not the schema. We can read the contents of patch. What we cannot read is the hardware and why you are making all these choices. > > The pin contains the following properties: > * reg - pin HW index (physical pin number of given type) > * label - string that is used as board label by DPLL core > * type - string that indicates pin type (mapped to DPLL core pin type) > * esync-control - boolean that indicates whether embeddded sync control > is allowed for this pin > * supported-frequencies - list of 64bit values that represents frequencies > that are allowed to be configured for the pin Drop. Describe the hardware. > > Reviewed-by: Michal Schmidt <mschmidt@redhat.com> Did this really happen? > Signed-off-by: Ivan Vecera <ivecera@redhat.com> > --- > .../devicetree/bindings/dpll/dpll-device.yaml | 84 +++++++++++++++++++ > .../devicetree/bindings/dpll/dpll-pin.yaml | 43 ++++++++++ > MAINTAINERS | 2 + > 3 files changed, 129 insertions(+) > create mode 100644 Documentation/devicetree/bindings/dpll/dpll-device.yaml > create mode 100644 Documentation/devicetree/bindings/dpll/dpll-pin.yaml Filenames matching compatibles... unless this is common schema, but commit description did not mention it. > > diff --git a/Documentation/devicetree/bindings/dpll/dpll-device.yaml b/Documentation/devicetree/bindings/dpll/dpll-device.yaml > new file mode 100644 > index 0000000000000..e6c309abb857f > --- /dev/null > +++ b/Documentation/devicetree/bindings/dpll/dpll-device.yaml > @@ -0,0 +1,84 @@ > +# 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: | Do not need '|' unless you need to preserve formatting. > + Digital Phase-Locked Loop (DPLL) device are used for precise clock > + synchronization in networking and telecom hardware. The device can > + have one or more channels (DPLLs) and one or more input and output > + pins. Each DPLL channel can either produce pulse-per-clock signal > + or drive ethernet equipment clock. The type of each channel is > + indicated by dpll-types property. > + > +properties: > + $nodename: > + pattern: "^dpll(@.*)?$" > + > + "#address-cells": > + const: 0 > + > + "#size-cells": > + const: 0 Why do you need these cells? > + > + num-dplls: > + description: Number of DPLL channels in this device. Why this is not deducible from compatible? > + $ref: /schemas/types.yaml#/definitions/uint32 > + minimum: 1 > + > + dpll-types: > + description: List of DPLL types, one per DPLL instance. > + $ref: /schemas/types.yaml#/definitions/non-unique-string-array > + items: > + enum: [pps, eec] Why this is not deducible from compatible? > + > + input-pins: > + type: object > + description: DPLL input pins > + unevaluatedProperties: false So this is all for pinctrl? Or something else? Could not figure out from commit msg. This does not help me either. > + > + properties: > + "#address-cells": > + const: 1 Why? > + "#size-cells": > + const: 0 Why? I don't see these being used. > + > + patternProperties: > + "^pin@[0-9]+$": > + $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" > + > +dependentRequired: > + dpll-types: [ num-dplls ] > + > +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..9aea8ceabb5af > --- /dev/null > +++ b/Documentation/devicetree/bindings/dpll/dpll-pin.yaml > @@ -0,0 +1,43 @@ > +# 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: | > + Schema for defining input and output pins of a Digital Phase-Locked Loop (DPLL). > + Each pin can have a set of supported frequencies, label, type and may support > + embedded sync. > + > +properties: > + reg: > + description: Hardware index of the pin. > + $ref: /schemas/types.yaml#/definitions/uint32 > + > + 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: > + description: List of supported frequencies for this pin, expressed in Hz. > + $ref: /schemas/types.yaml#/definitions/uint64-array Use common property suffixes and drop ref. > + > + type: > + description: Type of the pin > + $ref: /schemas/types.yaml#/definitions/string > + enum: [ext, gnss, int, mux, synce] > + > + Just one blank line I bet that half of my questions could be answered with proper hardware description which is missing in commit msg and binding description. Instead your commit msg explains schema which makes no sense - I mentioned, we can read the schema. > +required: > + - reg Best regards, Krzysztof
On 07/04/2025 20:01, Krzysztof Kozlowski wrote: >> + >> + input-pins: >> + type: object >> + description: DPLL input pins >> + unevaluatedProperties: false > > So this is all for pinctrl? Or something else? Could not figure out from > commit msg. This does not help me either. > >> + >> + properties: >> + "#address-cells": >> + const: 1 > > Why? Ah, I see being used. > >> + "#size-cells": >> + const: 0 > > Why? I don't see these being used. And this as well. > >> + >> + patternProperties: >> + "^pin@[0-9]+$": >> + $ref: /schemas/dpll/dpll-pin.yaml >> + unevaluatedProperties: false >> + Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/dpll/dpll-device.yaml b/Documentation/devicetree/bindings/dpll/dpll-device.yaml new file mode 100644 index 0000000000000..e6c309abb857f --- /dev/null +++ b/Documentation/devicetree/bindings/dpll/dpll-device.yaml @@ -0,0 +1,84 @@ +# 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 are used for precise clock + synchronization in networking and telecom hardware. The device can + have one or more channels (DPLLs) and one or more input and output + pins. Each DPLL channel can either produce pulse-per-clock signal + or drive ethernet equipment clock. The type of each channel is + indicated by dpll-types property. + +properties: + $nodename: + pattern: "^dpll(@.*)?$" + + "#address-cells": + const: 0 + + "#size-cells": + const: 0 + + num-dplls: + description: Number of DPLL channels in this device. + $ref: /schemas/types.yaml#/definitions/uint32 + minimum: 1 + + dpll-types: + description: List of DPLL 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]+$": + $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" + +dependentRequired: + dpll-types: [ num-dplls ] + +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..9aea8ceabb5af --- /dev/null +++ b/Documentation/devicetree/bindings/dpll/dpll-pin.yaml @@ -0,0 +1,43 @@ +# 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: | + Schema for defining input and output pins of a Digital Phase-Locked Loop (DPLL). + Each pin can have a set of supported frequencies, label, type and may support + embedded sync. + +properties: + reg: + description: Hardware index of the pin. + $ref: /schemas/types.yaml#/definitions/uint32 + + 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: + description: List of supported frequencies for this pin, expressed in Hz. + $ref: /schemas/types.yaml#/definitions/uint64-array + + type: + 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 3d542440d0b2b..eaf2576a9b746 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -7194,6 +7194,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
This adds DT bindings schema for DPLL (device phase-locked loop) device and associated pin. The schema follows existing DPLL core API and should be used to expose information that should be provided by platform firmware. The schema for DPLL device describe a DPLL chip that can contain one or more DPLLs (channels) and platform can specify their types. For now 'pps' and 'eec' types supported and these values are mapped to DPLL core's enums. The DPLL device can have optionally 'input-pins' and 'output-pins' sub-nodes that contain pin sub-nodes. These pin sub-nodes follows schema for dpll-pin and can contain information about the particular pin. The pin contains the following properties: * reg - pin HW index (physical pin number of given type) * label - string that is used as board label by DPLL core * type - string that indicates pin type (mapped to DPLL core pin type) * esync-control - boolean that indicates whether embeddded sync control is allowed for this pin * supported-frequencies - list of 64bit values that represents frequencies that are allowed to be configured for the pin Reviewed-by: Michal Schmidt <mschmidt@redhat.com> Signed-off-by: Ivan Vecera <ivecera@redhat.com> --- .../devicetree/bindings/dpll/dpll-device.yaml | 84 +++++++++++++++++++ .../devicetree/bindings/dpll/dpll-pin.yaml | 43 ++++++++++ MAINTAINERS | 2 + 3 files changed, 129 insertions(+) create mode 100644 Documentation/devicetree/bindings/dpll/dpll-device.yaml create mode 100644 Documentation/devicetree/bindings/dpll/dpll-pin.yaml