Re: [PATCH 1/6] dt-bindings: clocks: add binding for generic clock-generators

Hi,

Am Mittwoch, 10. Juli 2024, 09:02:34 CEST schrieb Alexander Stein:

Am Dienstag, 9. Juli 2024, 14:31:16 CEST schrieb Heiko Stuebner:

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.

Fixed clocks are intended to be ungateable? Where does this come from?

"DOC: basic fixed-rate clock that cannot gate"
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/clk/clk-fixed-rate.c#n18

quoted

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

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
+
+  compatible:
+    oneOf:
+      - const: clock-generator
+      - items:
+          - enum:
+              - diodes,pi6c557-03b
+              - 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

So essentially only enable-gpios and vdd-supply is added in comparison to
fixed-clock. Does it make sense to add that to the fixed-clocks instead?
Similar to fixed-regulator.

I wasn't that sure which way to go in the first place.
The deciding point was reading that line about the fixed clock not
being gateable, so I opted to not touch the fixed-clock.

But you're definitly right, this _could_ live inside the fixed-clock
as well, if we decide to get rid of the not-gateable thing above.


Heiko