Hi Stephen,

Am Dienstag, 9. Juli 2024, 23:45:20 CEST schrieb Stephen Boyd:

Quoting Heiko Stuebner (2024-07-09 05:31:16)

quoted

In contrast to fixed clocks that are described as ungateable, boards
sometimes use additional clock generators for things like PCIe reference
clocks, that need actual supplies to get enabled and enable-gpios to be
toggled for them to work.

This adds a binding for such clock generators that are not configurable
themself, but need to handle supplies for them to work.

Sounds like vdd-supply is required?

yeah, I guess this could move to required

quoted

While in a lot of cases the type of the IC used is described in board
schematics, in some cases just a generic type description like
"100MHz, 3.3V" might also be used. The binding therefore allows both
cases. Specifying the type is of course preferred.

The clock-frequency is set in devicetree, because while some clock
generators have pins to decide between multipls output rates, those
are generally set statically on the board-layout-level.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 .../bindings/clock/clock-generator.yaml       | 62 +++++++++++++++++++
 1 file changed, 62 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/clock-generator.yaml

diff --git a/Documentation/devicetree/bindings/clock/clock-generator.yaml b/Documentation/devicetree/bindings/clock/clock-generator.yaml
new file mode 100644
index 0000000000000..f44e61e414e89
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/clock-generator.yaml

@@ -0,0 +1,62 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/clock-generator.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Simple clock generators
+
+maintainers:
+  - Heiko Stuebner <heiko@sntech.de>
+
+properties:
+  $nodename:
+    anyOf:
+      - description:
+          Preferred name is 'clock-<freq>' with <freq> being the output
+          frequency as defined in the 'clock-frequency' property.
+        pattern: "^clock-([0-9]+|[a-z0-9-]+)$"
+      - description: Any name allowed
+        deprecated: true

Drop the deprecated stuff from the fixed-clock binding?

ok

quoted

+
+  compatible:
+    oneOf:
+      - const: clock-generator
+      - items:
+          - enum:
+              - diodes,pi6c557-03b

I see this datasheet[1]. Can that link be included somewhere? That shows
there's a clock input pin, which means I expect a 'clocks' property.

ok, I guess we could add a clock property and make it mandatory for the
Diodes compatibles.

Maybe instead of creating a generic binding just make a binding for
these diodes parts? It certainly looks like a generic binding could come
later when another vendor supports the same binding.

I was actually primarily aiming at solving the Rock 5 ITX clock generator
issue described in patch 5, where the 100 MHz clock generator is just
described as "100MHz,3.3V,3225" in the schematics, but definitly needs
the supply regulator to be enabled [1].

The Theobroma boards were addons because I felt they could also describe
the hardware better instead of lumping together 2 existing bindings.

quoted

+              - diodes,pi6c557-05b
+          - const: clock-generator
+
+  "#clock-cells":
+    const: 0
+
+  clock-frequency: true
+
+  clock-output-names:
+    maxItems: 1
+
+  enable-gpios:
+    description:
+      Contains a single GPIO specifier for the GPIO that enables and disables
+      the clock generator.
+    maxItems: 1
+
+  vdd-supply:
+    description: handle of the regulator that provides the supply voltage
+
+required:
+  - compatible
+  - "#clock-cells"
+  - clock-frequency

This is the same required properties as fixed rate clocks. I'd guess
that at least 'enable-gpios' or 'vdd-supply' should also be required, or
the node would simply use fixed-clock compatible.

ok, as said above, having the vdd-supply as requirement does make sense.


Heiko


[1] https://dl.radxa.com/rock5/5itx/radxa_rock_5_itx_X1100_schematic.pdf
page 35, bottom left, to the left of the big Au5426 "clock buffer".