Re: [PATCH 02/16] dt-bindings: clock: Add ARTPEC-8 CMU bindings

On Thu, 2025-07-10 at 09:10 +0200, Krzysztof Kozlowski wrote:

On 10/07/2025 02:20, ksk4725@coasia.com wrote:

quoted

From: Hakyeong Kim <redacted>

Add dt-schema for ARTPEC-8 SoC clock controller.

Add device-tree binding definitions for following CMU blocks:
- CMU_CMU
- CMU_BUS
- CMU_CORE
- CMU_CPUCL
- CMU_FSYS
- CMU_IMEM
- CMU_PERI

Signed-off-by: Ravi Patel <redacted>
Signed-off-by: Hakyeong Kim <redacted>

Confusing order, unless you really understand this, but considering
you
did not add your own SoB I claim you do not understand this. What
does
Ravi's SoB mean here?

Ok, I will update the SoB section in all the patches.

quoted

---
 .../bindings/clock/axis,artpec8-clock.yaml    | 224
++++++++++++++++++
 1 file changed, 224 insertions(+)
 create mode 100644
Documentation/devicetree/bindings/clock/axis,artpec8-clock.yaml

diff --git a/Documentation/devicetree/bindings/clock/axis,artpec8-

clock.yaml b/Documentation/devicetree/bindings/clock/axis,artpec8-
clock.yaml
new file mode 100644
index 000000000000..baacea10599b

--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/axis,artpec8-

clock.yaml

@@ -0,0 +1,224 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/axis,artpec8-clock.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Axis ARTPEC-8 SoC clock controller
+
+maintainers:
+  - Jesper Nilsson <jesper.nilsson@axis.com>
+
+description: |
+  ARTPEC-8 clock controller is comprised of several CMU units,

generating
+  clocks for different domains. Those CMU units are modeled as
separate device
+  tree nodes, and might depend on each other. The root clock in
that root tree
+  is an external clock: OSCCLK (25 MHz). This external clock must
be defined
+  as a fixed-rate clock in dts.
+
+  CMU_CMU is a top-level CMU, where all base clocks are prepared
using PLLs and
+  dividers; all other clocks of function blocks (other CMUs) are
usually
+  derived from CMU_CMU.
+
+  Each clock is assigned an identifier and client nodes can use
this identifier
+  to specify the clock which they consume. All clocks available
for usage
+  in clock consumer nodes are defined as preprocessor macros in
+  'include/dt-bindings/clock/axis,artpec8-clk.h' header.
+
+properties:
+  compatible:
+    enum:
+      - axis,artpec8-cmu-cmu
+      - axis,artpec8-cmu-bus
+      - axis,artpec8-cmu-core
+      - axis,artpec8-cmu-cpucl
+      - axis,artpec8-cmu-fsys
+      - axis,artpec8-cmu-imem
+      - axis,artpec8-cmu-peri
+
+  clocks:
+    minItems: 1
+    maxItems: 5
+
+  clock-names:
+    minItems: 1
+    maxItems: 5
+
+  "#clock-cells":
+    const: 1
+
+  reg:
+    maxItems: 1

reg goes second, after compatible (Samsung bindings are not the best
example because I converted them long time ago before many coding
style
practices were encouraged)

Ok, I will move the reg property.

quoted

+
+required:
+  - compatible
+  - reg
+  - "#clock-cells"
+  - clocks
+  - clock-names
+
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:

Drop contains.

Ok, I will remove the contains wherever applicable.

quoted

+            const: axis,artpec8-cmu-cmu
+
+    then:
+      properties:
+        clocks:
+          items:
+            - description: External reference clock (25 MHz)
+
+        clock-names:
+          items:
+            - const: fin_pll
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: axis,artpec8-cmu-bus
+
+    then:
+      properties:
+        clocks:
+          items:
+            - description: External reference clock (25 MHz)
+            - description: CMU_BUS BUS clock (from CMU_CMU)
+            - description: CMU_BUS DLP clock (from CMU_CMU)
+
+        clock-names:
+          items:
+            - const: fin_pll
+            - const: dout_clkcmu_bus_bus
+            - const: dout_clkcmu_bus_dlp

All these names should be changed to match what is the input. Look at
latest bindings, we moved away from that style.

Ok, I will modify clock-names to "bus" and "dlp" and will update at the
other related places also.

Thanks,
Hakyeong Kim

Best regards,
Krzysztof