From: Krzysztof Kozlowski <redacted>

quoted

From: Niravkumar L Rabara <redacted>

Add clock and reset ID definitions for Intel Agilex5 SoCFPGA

Co-developed-by: Teh Wen Ping <redacted>
Signed-off-by: Teh Wen Ping <redacted>
Signed-off-by: Niravkumar L Rabara <redacted>
---
 .../bindings/clock/intel,agilex5.yaml         |  42 ++++++++
 include/dt-bindings/clock/agilex5-clock.h     | 100 ++++++++++++++++++
 .../dt-bindings/reset/altr,rst-mgr-agilex5.h  |  79 ++++++++++++++
 3 files changed, 221 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/intel,agilex5.yaml
 create mode 100644 include/dt-bindings/clock/agilex5-clock.h
 create mode 100644 include/dt-bindings/reset/altr,rst-mgr-agilex5.h

diff --git a/Documentation/devicetree/bindings/clock/intel,agilex5.yaml b/Documentation/devicetree/bindings/clock/intel,agilex5.yaml
new file mode 100644
index 000000000000..e408c52deefa
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/intel,agilex5.yaml

Filename matching compatible, so missing "clk"

Will update in V2 patch, rename file to intel,agilex5-clk.yaml

quoted

@@ -0,0 +1,42 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/intel,agilex5.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Intel SoCFPGA Agilex5 platform clock controller binding

Drop "binding"

Will update in V2 patch.

quoted

+
+maintainers:
+  - Teh Wen Ping [off-list ref]
+  - Niravkumar L Rabara [off-list ref]
+
+description:
+  The Intel Agilex5 Clock controller is an integrated clock controller, which
+  generates and supplies to all modules.

"generates and supplies" what?

Will change to "generates and supplies clock" in V2 patch.

quoted

+
+properties:
+  compatible:
+    const: intel,agilex5-clkmgr

Why "clkmgr", not "clk"? You did not call it Clock manager anywhere in
the description or title.

The register in Agilex5 handling the clock is named clock_mgr.
Previous IntelSocFPGA, Agilex and Stratix10, are also named clkmgr.

quoted

+
+  '#clock-cells':
+    const: 1
+
+  reg:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - '#clock-cells'

Keep the same order as in properties:

Will update in V2 patch.

quoted

+
+additionalProperties: false
+
+examples:
+  # Clock controller node:
+  - |
+    clkmgr: clock-controller@10d10000 {
+      compatible = "intel,agilex5-clkmgr";
+      reg = <0x10d10000 0x1000>;
+      #clock-cells = <1>;
+    };
+...

diff --git a/include/dt-bindings/clock/agilex5-clock.h b/include/dt-bindings/clock/agilex5-clock.h
new file mode 100644
index 000000000000..4505b352cd83
--- /dev/null
+++ b/include/dt-bindings/clock/agilex5-clock.h

Filename the same as binding. Missing vendor prefix, entirely different
device name.

Will change filename to intel,agilex5-clock.h in V2.

quoted

@@ -0,0 +1,100 @@
+/* SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause */
+/*
+ * Copyright (C) 2022, Intel Corporation
+ */

...

quoted

+
+#endif	/* __AGILEX5_CLOCK_H */

diff --git a/include/dt-bindings/reset/altr,rst-mgr-agilex5.h b/include/dt-bindings/reset/altr,rst-mgr-agilex5.h
new file mode 100644
index 000000000000..81e5e8c89893
--- /dev/null
+++ b/include/dt-bindings/reset/altr,rst-mgr-agilex5.h

Same filename as binding.

But why do you need this file? Your device is not a reset controller.

Because Agilex5 device tree uses the reset definition from this file.

Best Regards,
Wen Ping