dt-bindings: clock: via,vt8500: Convert to DT Schema

Convert the VIA/Wondermedia VT8500 and Wondermedia WM8xxx series SoCs clock controller binding from the legacy text format to DT schema.

---
Changelog:
Changes since v4:
- Remove schema select matching for via,vt8500-pmc
- Add dependentRequired validation for divisor-mask
- Fix maintainers list
- Add reg false condition for device clock
- Fix example validation against clocks child node

Link to v4:https://lore.kernel.org/all/[email protected]/

Changes since v3:
- Add schema select matching for via,vt8500-pmc
- Allow hyphen in node names under patternProperties
- Add dependentRequired validation for enable-reg/enable-bit
- Fix example validation against PMC schema

Link to v3:https://lore.kernel.org/all/[email protected]/

Changes since v2:
- Drop redundant description for clocks
- Disable reg property for device clocks
- Fix schema hierarchy to match actual DTS structure

Link to v2:https://lore.kernel.org/all/[email protected]/

Changes since v1:
- Add default value for divisor-mask
- Add required properties compatible and model
- Fix example node name
- Update example size cells and reg value

Link to v1:https://lore.kernel.org/all/[email protected]/

---
 .../bindings/clock/via,vt8500-clock.yaml      | 133 ++++++++++++++++++
 .../devicetree/bindings/clock/vt8500.txt      |  74 ----------
 2 files changed, 133 insertions(+), 74 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/clock/via,vt8500-clock.yaml
 delete mode 100644 Documentation/devicetree/bindings/clock/vt8500.txt

new file mode 100644
index 000000000000..e2e674d95654
@@ -0,0 +1,133 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/via,vt8500-clock.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: VIA/Wondermedia VT8500 Clock Controller
+
+maintainers:
+  - Alexey Charkov <[email protected]>
+  - Krzysztof Kozlowski <[email protected]>
+
+description:
+  Clock controller bindings for VIA/Wondermedia VT8500 and Wondermedia WM8xxx
+  series SoCs.
+
+properties:
+  compatible:
+    enum:
+      - via,vt8500-pll-clock
+      - wm,wm8650-pll-clock
+      - wm,wm8750-pll-clock
+      - wm,wm8850-pll-clock
+      - via,vt8500-device-clock
+
+  reg:
+    description: Offset of the PLL register within the PMC register space.
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  "#clock-cells":
+    const: 0
+
+  clock-output-names:
+    maxItems: 1
+
+  enable-reg:
+    description: Offset of the clock enable register within the PMC register space.
+    $ref: /schemas/types.yaml#/definitions/uint32
+
+  enable-bit:
+    description: Bit index controlling clock enable.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    maximum: 31
+
+  divisor-reg:
+    description: Offset of the clock divisor register within the PMC register space.
+    $ref: /schemas/types.yaml#/definitions/uint32
+
+  divisor-mask:
+    description: Bitmask describing the divisor field inside divisor-reg.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    default: 0x1f
+
+required:
+  - compatible
+  - clocks
+  - "#clock-cells"
+
+allOf:
+  - if:
+      properties:
+        compatible:
+          const: via,vt8500-device-clock
+    then:
+      properties:
+        reg: false
+      anyOf:
+        - required:
+            - enable-reg
+            - enable-bit
+        - required:
+            - divisor-reg
+  - if:
+      properties:
+        compatible:
+          enum:
+            - via,vt8500-pll-clock
+            - wm,wm8650-pll-clock
+            - wm,wm8750-pll-clock
+            - wm,wm8850-pll-clock
+    then:
+      required:
+        - reg
+
+dependentRequired:
+  enable-reg:
+    - enable-bit
+  enable-bit:
+    - enable-reg
+  divisor-mask:
+    - divisor-reg
+
+additionalProperties: false
+
+examples:
+  - |
+    clocks {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        ref25: clock-25000000 {
+            compatible = "fixed-clock";
+            clock-frequency = <25000000>;
+            #clock-cells = <0>;
+        };
+
+        plla: clock@200 {
+            compatible = "wm,wm8650-pll-clock";
+            reg = <0x200>;
+            clocks = <&ref25>;
+            #clock-cells = <0>;
+        };
+
+        pllb: clock@204 {
+            compatible = "wm,wm8650-pll-clock";
+            reg = <0x204>;
+            clocks = <&ref25>;
+            #clock-cells = <0>;
+        };
+
+        clksdhc: sdhc {
+            compatible = "via,vt8500-device-clock";
+            clocks = <&pllb>;
+            #clock-cells = <0>;
+            divisor-reg = <0x328>;
+            divisor-mask = <0x3f>;
+            enable-reg = <0x254>;
+            enable-bit = <18>;
+        };
+    };

2.43.0

Hi,

I'm not sure that this separation of the clock controller binding vs. PMC is the right way to go (yes, I know it's been that way in the .txt document, but still).

The clock controller is not a standalone device on these SoCs - it's one of the functions of the PMC (weirdly enough), with its MMIO space interweaved between clock statuses and enable controls, reset/hibernation control, system timer and watchdog, then again clock control. You can't run the clock controller separately from the resources provided in its parent PMC node, because all those *reg nodes are just offsets into the PMC controller's MMIO space. So the way it's done currently doesn't look like idiomatic DT to me.

If I were to implement support for all that beauty today, I would have made a top-level MFD node owning the whole MMIO range of the PMC, put clock IDs into a dt-bindings header file and just implement all clock operations in code without putting those offsets and bits into the device tree. They are not configurable, they are not board specific, they just constitute the interface of a particular SoC revision which follows directly from a SoC versioned PMC compatible string.

The only thing that would have showed up in the DT then would have been the PMC node with a register range and a bunch of named interrupts (not for clocks, but for other functions), a clock-controller attribute and a single clock cell to pass arbitrarily assigned clock IDs from a new per-SoC dt-bindings header.

I'm pretty sure there are no users of these DTs outside of mainline Linux, because vendor kernels used board files, and no DT-enabled firmware has ever been created, thus the only way to boot these boards with a device tree has only ever been via an appended-DTB uImage (i.e. the DTB always comes from the currently used Linux kernel source). From this perspective I believe redoing the DT in a more hardware-aligned fashion (and breaking DT ABI) could be better than making the current suboptimal approach more "official" by encoding it in DT schema. Krzysztof will likely disagree though :)

Best regards,
Alexey

My comments from v2 still apply. Almost all of them.

You a PMC binding and preferrably all this goes to that binding.

Best regards,
Krzysztof

Understood, Krzysztof!

The current separation between the PMC binding and the clock binding may not reflect the hardware correctly, since the clock controller functionality is implemented as part of the PMC and all clock-specific registers are offsets within the PMC register space.

Request you to please confirm on the below points to proceed for next revision of YAML conversion.

1. It will be a standalone PMC schema (via,vt8500-pmc.yaml) which also describes the clock-related child nodes and replaces both legacy text bindings (clock.txt & pmc.txt) , or
2. As per Alexey, PMC compatibles should be expanded to per-SoC values.

If it has to be expanded to per-SoC values, can it be taken as separate DT-ABI change or should be done in the same series now?

Regards,
Udaya Kiran Challa