Skip to contentSkip to conversation

clk: agilex: add clock driver

PATCH Superseded Dinh Nguyen 3 participants 7 messages Mar 9, 2020 – Mar 10, 2020

Subsystems (3)
Device tree bindings Common clk Arm/socfpga clock framework support

v2 of 7 Superseded Latest is v7 →

lore
Share & tools
lore

Dinh Nguyen

author maintainer
#0 lore raw

v2 cover clk: agilex: add clock driver

Hi,

This is version 2 of the patchset to add clock driver to the Agilex platform. It's been while since I posted v1 so I want clarify the patches a bit in this cover letter.

Since the Agilex clocking is very similar to Stratix10, the driver is very similar and will re-use the clock data structures of Stratix10. Thus, there needs to be updates to the Stratix10 clock driver.

Patch 1/3 : update the Stratix10 clock driver to make use of the new
parent data scheme
Patch 2/3 : version 2 of the documenation, converted to YAML Patch 3/4 : version 2 of the clock driver with comments from v1
addressed

Thanks,
Dinh

Dinh Nguyen (3):
clk: socfpga: stratix10: use new parent data scheme
dt-bindings: documentation: add clock bindings information for Agilex
clk: socfpga: agilex: add clock driver for the Agilex platform

 .../bindings/clock/intc,agilex.yaml           |  79 ++++
 drivers/clk/Makefile                          |   1 +
 drivers/clk/socfpga/Makefile                  |   2 +
 drivers/clk/socfpga/clk-agilex.c              | 369 ++++++++++++++++++
 drivers/clk/socfpga/clk-gate-s10.c            |   5 +-
 drivers/clk/socfpga/clk-periph-s10.c          |  10 +-
 drivers/clk/socfpga/clk-pll-s10.c             |  74 +++-
 drivers/clk/socfpga/clk-s10.c                 | 110 ++++--
 drivers/clk/socfpga/stratix10-clk.h           |  10 +-
 include/dt-bindings/clock/agilex-clock.h      |  70 ++++
 10 files changed, 689 insertions(+), 41 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/clock/intc,agilex.yaml
 create mode 100644 drivers/clk/socfpga/clk-agilex.c
 create mode 100644 include/dt-bindings/clock/agilex-clock.h
Signature · 1 line
2.17.1
author maintainer
#1 lore raw

PATCH RESEND v2 1/3 clk: socfpga: stratix10: use new parent data scheme

Convert, where possible, the stratix10 clock driver to the new parent data scheme by specifying the parent data for clocks that have multiple parents.

Signed-off-by Dinh Nguyen <[email protected]>
---
 drivers/clk/socfpga/clk-gate-s10.c   |   5 +-
 drivers/clk/socfpga/clk-periph-s10.c |  10 ++-
 drivers/clk/socfpga/clk-pll-s10.c    |   4 +-
 drivers/clk/socfpga/clk-s10.c        | 110 ++++++++++++++++++++-------
 drivers/clk/socfpga/stratix10-clk.h  |   8 +-
 5 files changed, 96 insertions(+), 41 deletions(-)
drivers/clk/socfpga/clk-gate-s10.c +3 -2
index 8be4722f6064..083b2ec21fdd 100644
@@ -70,7 +70,6 @@ struct clk *s10_register_gate(const struct stratix10_gate_clock *clks, void __io
 	struct clk *clk;
 	struct socfpga_gate_clk *socfpga_clk;
 	struct clk_init_data init;
-	const char * const *parent_names = clks->parent_names;
 	const char *parent_name = clks->parent_name;
 
 	socfpga_clk = kzalloc(sizeof(*socfpga_clk), GFP_KERNEL);

@@ -108,7 +107,9 @@ struct clk *s10_register_gate(const struct stratix10_gate_clock *clks, void __io
 	init.flags = clks->flags;
 
 	init.num_parents = clks->num_parents;
-	init.parent_names = parent_names ? parent_names : &parent_name;
+	init.parent_names = parent_name ? &parent_name : NULL;
+	if (init.parent_names == NULL)
+		init.parent_data = clks->parent_data;
 	socfpga_clk->hw.hw.init = &init;
 
 	clk = clk_register(NULL, &socfpga_clk->hw.hw);
drivers/clk/socfpga/clk-periph-s10.c +6 -4
index dd6d4056e9de..397b77b89b16 100644
@@ -81,7 +81,6 @@ struct clk *s10_register_periph(const struct stratix10_perip_c_clock *clks,
 	struct clk_init_data init;
 	const char *name = clks->name;
 	const char *parent_name = clks->parent_name;
-	const char * const *parent_names = clks->parent_names;
 
 	periph_clk = kzalloc(sizeof(*periph_clk), GFP_KERNEL);
 	if (WARN_ON(!periph_clk))

@@ -94,7 +93,9 @@ struct clk *s10_register_periph(const struct stratix10_perip_c_clock *clks,
 	init.flags = clks->flags;
 
 	init.num_parents = clks->num_parents;
-	init.parent_names = parent_names ? parent_names : &parent_name;
+	init.parent_names = parent_name ? &parent_name : NULL;
+	if (init.parent_names == NULL)
+		init.parent_data = clks->parent_data;
 
 	periph_clk->hw.hw.init = &init;
 

@@ -114,7 +115,6 @@ struct clk *s10_register_cnt_periph(const struct stratix10_perip_cnt_clock *clks
 	struct clk_init_data init;
 	const char *name = clks->name;
 	const char *parent_name = clks->parent_name;
-	const char * const *parent_names = clks->parent_names;
 
 	periph_clk = kzalloc(sizeof(*periph_clk), GFP_KERNEL);
 	if (WARN_ON(!periph_clk))

@@ -137,7 +137,9 @@ struct clk *s10_register_cnt_periph(const struct stratix10_perip_cnt_clock *clks
 	init.flags = clks->flags;
 
 	init.num_parents = clks->num_parents;
-	init.parent_names = parent_names ? parent_names : &parent_name;
+	init.parent_names = parent_name ? &parent_name : NULL;
+	if (init.parent_names == NULL)
+		init.parent_data = clks->parent_data;
 
 	periph_clk->hw.hw.init = &init;
drivers/clk/socfpga/clk-pll-s10.c +2 -2
index a301bb22f36c..bcd3f14e9145 100644
@@ -117,7 +117,6 @@ struct clk *s10_register_pll(const struct stratix10_pll_clock *clks,
 	struct socfpga_pll *pll_clk;
 	struct clk_init_data init;
 	const char *name = clks->name;
-	const char * const *parent_names = clks->parent_names;
 
 	pll_clk = kzalloc(sizeof(*pll_clk), GFP_KERNEL);
 	if (WARN_ON(!pll_clk))

@@ -134,7 +133,8 @@ struct clk *s10_register_pll(const struct stratix10_pll_clock *clks,
 	init.flags = clks->flags;
 
 	init.num_parents = clks->num_parents;
-	init.parent_names = parent_names;
+	init.parent_names = NULL;
+	init.parent_data = clks->parent_data;
 	pll_clk->hw.hw.init = &init;
 
 	pll_clk->hw.bit_idx = SOCFPGA_PLL_POWER;
drivers/clk/socfpga/clk-s10.c +81 -29

116 lines not on this page · raw message

drivers/clk/socfpga/stratix10-clk.h +4 -4
index fcabef42249c..ffbd1fb2c8ef 100644
@@ -14,7 +14,7 @@ struct stratix10_clock_data {
 struct stratix10_pll_clock {
 	unsigned int		id;
 	const char		*name;
-	const char		*const *parent_names;
+	const struct clk_parent_data	*parent_data;
 	u8			num_parents;
 	unsigned long		flags;
 	unsigned long		offset;

@@ -24,7 +24,7 @@ struct stratix10_perip_c_clock {
 	unsigned int		id;
 	const char		*name;
 	const char		*parent_name;
-	const char		*const *parent_names;
+	const struct clk_parent_data	*parent_data;
 	u8			num_parents;
 	unsigned long		flags;
 	unsigned long		offset;

@@ -34,7 +34,7 @@ struct stratix10_perip_cnt_clock {
 	unsigned int		id;
 	const char		*name;
 	const char		*parent_name;
-	const char		*const *parent_names;
+	const struct clk_parent_data	*parent_data;
 	u8			num_parents;
 	unsigned long		flags;
 	unsigned long		offset;

@@ -47,7 +47,7 @@ struct stratix10_gate_clock {
 	unsigned int		id;
 	const char		*name;
 	const char		*parent_name;
-	const char		*const *parent_names;
+	const struct clk_parent_data	*parent_data;
 	u8			num_parents;
 	unsigned long		flags;
 	unsigned long		gate_reg;
Signature · 1 line
2.17.1
author maintainer
#2 lore raw

PATCH v2 2/3 dt-bindings: documentation: add clock bindings information for Agilex

In reply to Dinh Nguyen

Document the Agilex clock bindings, and add the clock header file. The clock header is an enumeration of all the different clocks on the Agilex platform.

Signed-off-by Dinh Nguyen <[email protected]>

---
v2: convert original document to YAML

---
 .../bindings/clock/intc,agilex.yaml           | 79 +++++++++++++++++++
 include/dt-bindings/clock/agilex-clock.h      | 70 ++++++++++++++++
 2 files changed, 149 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/intc,agilex.yaml
 create mode 100644 include/dt-bindings/clock/agilex-clock.h
Documentation/devicetree/bindings/clock/intc,agilex.yaml new file +79 -0
new file mode 100644
index 000000000000..bd5c4f590e12
@@ -0,0 +1,79 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/intc,agilex.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Intel SoCFPGA Agilex platform clock controller binding
+
+maintainers:
+  - Dinh Nguyen <[email protected]>
+
+description: |
+  The Intel Agilex Clock controller is an integrated clock controller, which
+  generates and supplies to all modules.
+
+  This binding uses the common clock binding[1].
+  [1] Documentation/devicetree/bindings/clock/clock-bindings.txt
+
+properties:
+  compatible:
+    oneOf:
+      - items:
+          - enum:
+              - intel,agilex-clkmgr
+
+  '#clock-cells':
+    const: 1
+
+required:
+  - compatible
+  - reg
+  - '#clock-cells'
+
+examples:
+  # Clock controller node
+  - |
+	clkmgr: clock-controller@ffd10000 {
+		compatible = "intel,agilex-clkmgr";
+		reg = <0xffd10000 0x1000>;
+		#clock-cells = <1>;
+	};
+
+  # External clocks
+  - |
+    clocks {
+       cb_intosc_hs_div2_clk: cb-intosc-hs-div2-clk {
+		#clock-cells = <0>;
+		compatible = "fixed-clock";
+		};
+
+	cb_intosc_ls_clk: cb-intosc-ls-clk {
+		#clock-cells = <0>;
+			compatible = "fixed-clock";
+		};
+
+	f2s_free_clk: f2s-free-clk {
+		#clock-cells = <0>;
+		compatible = "fixed-clock";
+		};
+
+	osc1: osc1 {
+		#clock-cells = <0>;
+		compatible = "fixed-clock";
+	};
+    };
+
+  # The clock consumer shall specify the desired clock-output of the clock
+  # controller as below by specifying output-id in its "clk" phandle cell.
+  - |
+    i2c0: i2c@ffc02800 {
+	#address-cells = <1>;
+	#size-cells = <0>;
+	compatible = "snps,designware-i2c";
+	reg = <0xffc02800 0x100>;
+	interrupts = <0 103 4>;
+	resets = <&rst I2C0_RESET>;
+	clocks = <&clkmgr AGILEX_L4_SP_CLK>;
+	};
+...
include/dt-bindings/clock/agilex-clock.h new file +70 -0

70 lines not on this page · raw message

Signature · 1 line
2.17.1
maintainer
#3 lore raw

Re: [PATCHv2 2/3] dt-bindings: documentation: add clock bindings information for Agilex

Quoting Dinh Nguyen (2020-03-09 10:16:52)

Quoted 13 lines
Documentation/devicetree/bindings/clock/intc,agilex.yaml new file +7 -0
new file mode 100644
index 000000000000..bd5c4f590e12
@@ -0,0 +1,79 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/intc,agilex.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Intel SoCFPGA Agilex platform clock controller binding

Documentation/devicetree/bindings/clock/intc,agilex.yaml line 17

Quoted 10 lines
Documentation/devicetree/bindings/clock/intc,agilex.yaml new file +10 -0
+
+maintainers:
+  - Dinh Nguyen <[email protected]>
+
+description: |
+  The Intel Agilex Clock controller is an integrated clock controller, which
+  generates and supplies to all modules.
+
+  This binding uses the common clock binding[1].
+  [1] Documentation/devicetree/bindings/clock/clock-bindings.txt

I think you can remove this last sentence, and drop the | on the description because formatting doesn't matter.

Documentation/devicetree/bindings/clock/intc,agilex.yaml line 25

Quoted 8 lines
Documentation/devicetree/bindings/clock/intc,agilex.yaml new file +8 -0
+
+properties:
+  compatible:
+    oneOf:
+      - items:
+          - enum:
+              - intel,agilex-clkmgr
+

Just use

compatible:
const: intel,agilex-clkmgr

Quoted 5 lines
Documentation/devicetree/bindings/clock/intc,agilex.yaml new file +5 -0
+  '#clock-cells':
+    const: 1
+
+required:
+  - compatible

Documentation/devicetree/bindings/clock/intc,agilex.yaml line 40

Quoted 10 lines
Documentation/devicetree/bindings/clock/intc,agilex.yaml new file +10 -0
+  - reg
+  - '#clock-cells'
+
+examples:
+  # Clock controller node
+  - |
+       clkmgr: clock-controller@ffd10000 {
+               compatible = "intel,agilex-clkmgr";
+               reg = <0xffd10000 0x1000>;
+               #clock-cells = <1>;

Does it consume any clks?

Documentation/devicetree/bindings/clock/intc,agilex.yaml line 43

Quoted 3 lines
Documentation/devicetree/bindings/clock/intc,agilex.yaml new file +3 -0
+       };
+
+  # External clocks

Everything below here is not necessary and shouldn't be in the binding.

maintainer
#4 lore raw

Re: [PATCHv2 2/3] dt-bindings: documentation: add clock bindings information for Agilex

In reply to Dinh Nguyen
Quoted 5 lines · Dinh Nguyen

Document the Agilex clock bindings, and add the clock header file. The clock header is an enumeration of all the different clocks on the Agilex platform.

Signed-off-by Dinh Nguyen <[email protected]>
Quoted 9 lines

---
v2: convert original document to YAML

---
 .../bindings/clock/intc,agilex.yaml           | 79 +++++++++++++++++++
 include/dt-bindings/clock/agilex-clock.h      | 70 ++++++++++++++++
 2 files changed, 149 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/intc,agilex.yaml
 create mode 100644 include/dt-bindings/clock/agilex-clock.h

My bot found errors running 'make dt_binding_check' on your patch:

Documentation/devicetree/bindings/clock/intc,agilex.yaml: while scanning a block scalar
in "<unicode string>", line 36, column 5
found a tab character where an indentation space is expected
in "<unicode string>", line 37, column 1
Documentation/devicetree/bindings/Makefile:12: recipe for target 'Documentation/devicetree/bindings/clock/intc,agilex.example.dts' failed make[1]: *** [Documentation/devicetree/bindings/clock/intc,agilex.example.dts] Error 1
make[1]: *** Waiting for unfinished jobs....
warning: no schema found in file: Documentation/devicetree/bindings/clock/intc,agilex.yaml /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/clock/intc,agilex.yaml: ignoring, error parsing file Makefile:1262: recipe for target 'dt_binding_check' failed
make: *** [dt_binding_check] Error 2

See https://patchwork.ozlabs.org/patch/1251669
Please check and re-submit.

author maintainer
#5 lore raw

PATCH v2 3/3 clk: socfpga: agilex: add clock driver for the Agilex platform

In reply to Dinh Nguyen

For the most part the Agilex clock structure is very similar to Stratix10, so we re-use most of the Stratix10 clock driver.

Signed-off-by Dinh Nguyen <[email protected]>

---
v2: update to use clk_parent_data

---
 drivers/clk/Makefile                |   1 +
 drivers/clk/socfpga/Makefile        |   2 +
 drivers/clk/socfpga/clk-agilex.c    | 369 ++++++++++++++++++++++++++++
 drivers/clk/socfpga/clk-pll-s10.c   |  70 ++++++
 drivers/clk/socfpga/stratix10-clk.h |   2 +
 5 files changed, 444 insertions(+)
 create mode 100644 drivers/clk/socfpga/clk-agilex.c
drivers/clk/Makefile +1 -0
index f4169cc2fd31..d9ddc0bd91c0 100644
@@ -18,6 +18,7 @@ endif
 
 # hardware specific clock types
 # please keep this section sorted lexicographically by file path name
+obj-$(CONFIG_ARCH_AGILEX)		+= socfpga/
 obj-$(CONFIG_MACH_ASM9260)		+= clk-asm9260.o
 obj-$(CONFIG_COMMON_CLK_AXI_CLKGEN)	+= clk-axi-clkgen.o
 obj-$(CONFIG_ARCH_AXXIA)		+= clk-axm5516.o
drivers/clk/socfpga/Makefile +2 -0
index ce5aa7802eb8..bf736f8d201a 100644
@@ -3,3 +3,5 @@ obj-$(CONFIG_ARCH_SOCFPGA) += clk.o clk-gate.o clk-pll.o clk-periph.o
 obj-$(CONFIG_ARCH_SOCFPGA) += clk-pll-a10.o clk-periph-a10.o clk-gate-a10.o
 obj-$(CONFIG_ARCH_STRATIX10) += clk-s10.o
 obj-$(CONFIG_ARCH_STRATIX10) += clk-pll-s10.o clk-periph-s10.o clk-gate-s10.o
+obj-$(CONFIG_ARCH_AGILEX) += clk-agilex.o
+obj-$(CONFIG_ARCH_AGILEX) += clk-pll-s10.o clk-periph-s10.o clk-gate-s10.o
drivers/clk/socfpga/clk-agilex.c new file +369 -0

369 lines not on this page · raw message

drivers/clk/socfpga/clk-pll-s10.c +70 -0

93 lines not on this page · raw message

drivers/clk/socfpga/stratix10-clk.h +2 -0
index ffbd1fb2c8ef..f9d5d724c694 100644
@@ -62,6 +62,8 @@ struct stratix10_gate_clock {
 
 struct clk *s10_register_pll(const struct stratix10_pll_clock *,
 			     void __iomem *);
+struct clk *agilex_register_pll(const struct stratix10_pll_clock *,
+				void __iomem *);
 struct clk *s10_register_periph(const struct stratix10_perip_c_clock *,
 				void __iomem *);
 struct clk *s10_register_cnt_periph(const struct stratix10_perip_cnt_clock *,
Signature · 1 line
2.17.1
maintainer
#6 lore raw

Re: [PATCHv2 3/3] clk: socfpga: agilex: add clock driver for the Agilex platform

Quoting Dinh Nguyen (2020-03-09 10:16:53)

drivers/clk/Makefile endif · line 21

Quoted 9 lines
drivers/clk/Makefile +1 -0
index f4169cc2fd31..d9ddc0bd91c0 100644
@@ -18,6 +18,7 @@ endif
 
 # hardware specific clock types
 # please keep this section sorted lexicographically by file path name
+obj-$(CONFIG_ARCH_AGILEX)              += socfpga/

This is not sorted by file path name.

Quoted 22 lines
drivers/clk/Makefile +0 -0
 obj-$(CONFIG_MACH_ASM9260)             += clk-asm9260.o
 obj-$(CONFIG_COMMON_CLK_AXI_CLKGEN)    += clk-axi-clkgen.o
 obj-$(CONFIG_ARCH_AXXIA)               += clk-axm5516.o
drivers/clk/socfpga/clk-agilex.c new file +13 -0
new file mode 100644
index 000000000000..6789892085db
@@ -0,0 +1,369 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2019, Intel Corporation
+ */
+#include <linux/slab.h>
+#include <linux/clk-provider.h>
+#include <linux/of_device.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+
+#include <dt-bindings/clock/agilex-clock.h>
+
+#include "stratix10-clk.h"

[...]

drivers/clk/socfpga/clk-agilex.c line 112

Quoted 4 lines
drivers/clk/socfpga/clk-agilex.c new file +4 -0
+static const struct clk_parent_data emac_mux[] = {
+       { .name = "emaca_free_clk", },
+       { .name = "emacb_free_clk", },
+};

Why no newlines between structures?

drivers/clk/socfpga/clk-agilex.c line 116

Quoted 4 lines
drivers/clk/socfpga/clk-agilex.c new file +4 -0
+static const struct clk_parent_data noc_mux[] = {
+       { .name = "noc_free_clk", },
+       { .name = "boot_clk", },
+};

As stated before, please use actual pointers for clks within the controller and use .fw_name for clks outside of the controller.

Quoted 51 lines
drivers/clk/socfpga/clk-agilex.c new file +51 -0
+
+/* clocks in AO (always on) controller */
+static const struct stratix10_pll_clock agilex_pll_clks[] = {
+       { AGILEX_BOOT_CLK, "boot_clk", boot_mux, ARRAY_SIZE(boot_mux), 0,
+         0x0},
+       { AGILEX_MAIN_PLL_CLK, "main_pll", pll_mux, ARRAY_SIZE(pll_mux),
+         0, 0x48},
+       { AGILEX_PERIPH_PLL_CLK, "periph_pll", pll_mux, ARRAY_SIZE(pll_mux),
+         0, 0x9c},
+};
+
+static const struct stratix10_perip_c_clock agilex_main_perip_c_clks[] = {
+       { AGILEX_MAIN_PLL_C0_CLK, "main_pll_c0", "main_pll", NULL, 1, 0, 0x58},
+       { AGILEX_MAIN_PLL_C1_CLK, "main_pll_c1", "main_pll", NULL, 1, 0, 0x5C},
+       { AGILEX_MAIN_PLL_C2_CLK, "main_pll_c2", "main_pll", NULL, 1, 0, 0x64},
+       { AGILEX_MAIN_PLL_C3_CLK, "main_pll_c3", "main_pll", NULL, 1, 0, 0x68},
+       { AGILEX_PERIPH_PLL_C0_CLK, "peri_pll_c0", "periph_pll", NULL, 1, 0, 0xAC},
+       { AGILEX_PERIPH_PLL_C1_CLK, "peri_pll_c1", "periph_pll", NULL, 1, 0, 0xB0},
+       { AGILEX_PERIPH_PLL_C2_CLK, "peri_pll_c2", "periph_pll", NULL, 1, 0, 0xB8},
+       { AGILEX_PERIPH_PLL_C3_CLK, "peri_pll_c3", "periph_pll", NULL, 1, 0, 0xBC},
+};
+
+static const struct stratix10_perip_cnt_clock agilex_main_perip_cnt_clks[] = {
+       { AGILEX_MPU_FREE_CLK, "mpu_free_clk", NULL, mpu_free_mux, ARRAY_SIZE(mpu_free_mux),
+          0, 0x3C, 0, 0, 0},
+       { AGILEX_NOC_FREE_CLK, "noc_free_clk", NULL, noc_free_mux, ARRAY_SIZE(noc_free_mux),
+         0, 0x40, 0, 0, 1},
+       { AGILEX_L4_SYS_FREE_CLK, "l4_sys_free_clk", "noc_free_clk", NULL, 1, 0,
+         0, 4, 0, 0},
+       { AGILEX_NOC_CLK, "noc_clk", NULL, noc_mux, ARRAY_SIZE(noc_mux),
+         0, 0, 0, 0x30, 1},
+       { AGILEX_EMAC_A_FREE_CLK, "emaca_free_clk", NULL, emaca_free_mux, ARRAY_SIZE(emaca_free_mux),
+         0, 0xD4, 0, 0x88, 0},
+       { AGILEX_EMAC_B_FREE_CLK, "emacb_free_clk", NULL, emacb_free_mux, ARRAY_SIZE(emacb_free_mux),
+         0, 0xD8, 0, 0x88, 1},
+       { AGILEX_EMAC_PTP_FREE_CLK, "emac_ptp_free_clk", NULL, emac_ptp_free_mux,
+         ARRAY_SIZE(emac_ptp_free_mux), 0, 0xDC, 0, 0x88, 2},
+       { AGILEX_GPIO_DB_FREE_CLK, "gpio_db_free_clk", NULL, gpio_db_free_mux,
+         ARRAY_SIZE(gpio_db_free_mux), 0, 0xE0, 0, 0x88, 3},
+       { AGILEX_SDMMC_FREE_CLK, "sdmmc_free_clk", NULL, sdmmc_free_mux,
+         ARRAY_SIZE(sdmmc_free_mux), 0, 0xE4, 0, 0x88, 4},
+       { AGILEX_S2F_USER0_FREE_CLK, "s2f_user0_free_clk", NULL, s2f_usr0_free_mux,
+         ARRAY_SIZE(s2f_usr0_free_mux), 0, 0xE8, 0, 0, 0},
+       { AGILEX_S2F_USER1_FREE_CLK, "s2f_user1_free_clk", NULL, s2f_usr1_free_mux,
+         ARRAY_SIZE(s2f_usr1_free_mux), 0, 0xEC, 0, 0x88, 5},
+       { AGILEX_PSI_REF_FREE_CLK, "psi_ref_free_clk", NULL, psi_ref_free_mux,
+         ARRAY_SIZE(psi_ref_free_mux), 0, 0xF0, 0, 0x88, 6},
+};
+
+static const struct stratix10_gate_clock agilex_gate_clks[] = {
+       { AGILEX_MPU_CLK, "mpu_clk", NULL, mpu_mux, ARRAY_SIZE(mpu_mux), 0, 0x24,

drivers/clk/socfpga/clk-agilex.c line 177

Quoted 10 lines
drivers/clk/socfpga/clk-agilex.c new file +10 -0
+         0, 0, 0, 0, 0x30, 0, 0},
+       { AGILEX_MPU_PERIPH_CLK, "mpu_periph_clk", "mpu_clk", NULL, 1, 0, 0x24,
+         0, 0, 0, 0, 0, 0, 4},
+       { AGILEX_MPU_L2RAM_CLK, "mpu_l2ram_clk", "mpu_clk", NULL, 1, 0, 0x24,
+         0, 0, 0, 0, 0, 0, 2},
+       { AGILEX_L4_MAIN_CLK, "l4_main_clk", "noc_clk", NULL, 1, 0, 0x24,
+         1, 0x44, 0, 2, 0, 0, 0},
+       { AGILEX_L4_MP_CLK, "l4_mp_clk", "noc_clk", NULL, 1, 0, 0x24,
+         2, 0x44, 8, 2, 0, 0, 0},
+       { AGILEX_L4_SP_CLK, "l4_sp_clk", "noc_clk", NULL, 1, CLK_IS_CRITICAL, 0x24,

Please comment in the code why CLK_IS_CRITICAL is used.

Quoted 15 lines
drivers/clk/socfpga/clk-agilex.c new file +15 -0
+         3, 0x44, 16, 2, 0, 0, 0},
+       { AGILEX_CS_AT_CLK, "cs_at_clk", "noc_clk", NULL, 1, 0, 0x24,
+         4, 0x44, 24, 2, 0, 0, 0},
+       { AGILEX_CS_TRACE_CLK, "cs_trace_clk", "noc_clk", NULL, 1, 0, 0x24,
+         4, 0x44, 26, 2, 0, 0, 0},
+       { AGILEX_CS_PDBG_CLK, "cs_pdbg_clk", "cs_at_clk", NULL, 1, 0, 0x24,
+         4, 0x44, 28, 1, 0, 0, 0},
+       { AGILEX_CS_TIMER_CLK, "cs_timer_clk", "noc_clk", NULL, 1, 0, 0x24,
+         5, 0, 0, 0, 0, 0, 0},
+       { AGILEX_S2F_USER0_CLK, "s2f_user0_clk", NULL, s2f_usr0_mux, ARRAY_SIZE(s2f_usr0_mux), 0, 0x24,
+         6, 0, 0, 0, 0, 0, 0},
+       { AGILEX_EMAC0_CLK, "emac0_clk", NULL, emac_mux, ARRAY_SIZE(emac_mux), 0, 0x7C,
+         0, 0, 0, 0, 0x94, 26, 0},
+       { AGILEX_EMAC1_CLK, "emac1_clk", NULL, emac_mux, ARRAY_SIZE(emac_mux), 0, 0x7C,
+         1, 0, 0, 0, 0x94, 27, 0},

[..]

Quoted 5 lines
drivers/clk/socfpga/clk-agilex.c new file +5 -0
+
+static struct stratix10_clock_data *__socfpga_agilex_clk_init(struct platform_device *pdev,
+                                                   int nr_clks)
+{
+       struct device_node *np = pdev->dev.of_node;

drivers/clk/socfpga/clk-agilex.c line 304

Quoted 10 lines
drivers/clk/socfpga/clk-agilex.c new file +10 -0
+       struct device *dev = &pdev->dev;
+       struct stratix10_clock_data *clk_data;
+       struct clk **clk_table;
+       struct resource *res;
+       void __iomem *base;
+
+       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+       base = devm_ioremap_resource(dev, res);
+       if (IS_ERR(base)) {
+               pr_err("%s: failed to map clock registers\n", __func__);

ioremap fail usually prints an error message already. This is not needed.

Quoted 5 lines
drivers/clk/socfpga/clk-agilex.c new file +5 -0
+               return ERR_CAST(base);
+       }
+
+       clk_data = devm_kzalloc(dev, sizeof(*clk_data), GFP_KERNEL);
+       if (!clk_data)

drivers/clk/socfpga/clk-agilex.c line 319

Quoted 10 lines
drivers/clk/socfpga/clk-agilex.c new file +10 -0
+               return ERR_PTR(-ENOMEM);
+
+       clk_data->base = base;
+       clk_table = devm_kcalloc(dev, nr_clks, sizeof(*clk_table), GFP_KERNEL);
+       if (!clk_table)
+               return ERR_PTR(-ENOMEM);
+
+       clk_data->clk_data.clks = clk_table;
+       clk_data->clk_data.clk_num = nr_clks;
+       of_clk_add_provider(np, of_clk_src_onecell_get, &clk_data->clk_data);

What if this fails?

Quoted 18 lines
drivers/clk/socfpga/clk-agilex.c new file +18 -0
+       return clk_data;
+}
+
+static int agilex_clkmgr_init(struct platform_device *pdev)
+{
+       struct stratix10_clock_data *clk_data;
+
+       clk_data = __socfpga_agilex_clk_init(pdev, AGILEX_NUM_CLKS);
+       if (IS_ERR(clk_data))
+               return PTR_ERR(clk_data);
+
+       agilex_clk_register_pll(agilex_pll_clks, ARRAY_SIZE(agilex_pll_clks), clk_data);
+
+       agilex_clk_register_c_perip(agilex_main_perip_c_clks,
+                                ARRAY_SIZE(agilex_main_perip_c_clks), clk_data);
+
+       agilex_clk_register_cnt_perip(agilex_main_perip_cnt_clks,
+                                  ARRAY_SIZE(agilex_main_perip_cnt_clks),

drivers/clk/socfpga/clk-agilex.c line 347

Quoted 10 lines
drivers/clk/socfpga/clk-agilex.c new file +10 -0
+                                  clk_data);
+
+       agilex_clk_register_gate(agilex_gate_clks, ARRAY_SIZE(agilex_gate_clks),
+                             clk_data);
+       return 0;
+}
+
+static int agilex_clkmgr_probe(struct platform_device *pdev)
+{
+       return  agilex_clkmgr_init(pdev);

Why not just put the contents of that here in probe?

Quoted 36 lines
drivers/clk/socfpga/clk-agilex.c new file +22 -0
+}
+
+static const struct of_device_id agilex_clkmgr_match_table[] = {
+       { .compatible = "intel,agilex-clkmgr",
+         .data = agilex_clkmgr_init },
+       { }
+};
+
+static struct platform_driver agilex_clkmgr_driver = {
+       .probe          = agilex_clkmgr_probe,
+       .driver         = {
+               .name   = "agilex-clkmgr",
+               .suppress_bind_attrs = true,
+               .of_match_table = agilex_clkmgr_match_table,
+       },
+};
+
+static int __init agilex_clk_init(void)
+{
+       return platform_driver_register(&agilex_clkmgr_driver);
+}
+core_initcall(agilex_clk_init);
drivers/clk/socfpga/clk-pll-s10.c +6 -0
index bcd3f14e9145..17fe5bd2c0e1 100644
@@ -27,6 +31,27 @@
 
 #define to_socfpga_clk(p) container_of(p, struct socfpga_pll, hw.hw)
 
+static unsigned long agilex_clk_pll_recalc_rate(struct clk_hw *hwclk,
+                                               unsigned long parent_rate)
+{
+       struct socfpga_pll *socfpgaclk = to_socfpga_clk(hwclk);
+       unsigned long arefdiv, reg, mdiv;
+       unsigned long long vco_freq;

drivers/clk/socfpga/clk-pll-s10.c line 49

Quoted 10 lines
drivers/clk/socfpga/clk-pll-s10.c +10 -0
+
+       /* read VCO1 reg for numerator and denominator */
+       reg = readl(socfpgaclk->hw.reg);
+       arefdiv = (reg & SOCFPGA_PLL_AREFDIV_MASK) >> SOCFPGA_PLL_REFDIV_SHIFT;
+
+       vco_freq = (unsigned long long)parent_rate / arefdiv;
+
+       /* Read mdiv and fdiv from the fdbck register */
+       reg = readl(socfpgaclk->hw.reg + 0x24);
+       mdiv = (reg & SOCFPGA_AGILEX_PLL_MDIV_MASK);

Please remove useless parenthesis.

drivers/clk/socfpga/clk-pll-s10.c line 52

Quoted 3 lines
drivers/clk/socfpga/clk-pll-s10.c +3 -0
+
+       vco_freq = (unsigned long long)vco_freq * mdiv;
+       return (unsigned long)vco_freq;

Drop the cast, it's implicit.

drivers/clk/socfpga/clk-pll-s10.c static int clk_pll_prepare(struct clk_hw *hwclk) · line 126

Quoted 10 lines
drivers/clk/socfpga/clk-pll-s10.c +3 -0
+}
+
 static unsigned long clk_pll_recalc_rate(struct clk_hw *hwclk,
                                         unsigned long parent_rate)
 {

@@ -98,6 +123,12 @@ static int clk_pll_prepare(struct clk_hw *hwclk)
        return 0;
 }
 
+static struct clk_ops agilex_clk_pll_ops = {

Can it be const?

drivers/clk/socfpga/clk-pll-s10.c static int clk_pll_prepare(struct clk_hw *hwclk) · line 132

Quoted 6 lines
drivers/clk/socfpga/clk-pll-s10.c +5 -0
+       .recalc_rate = agilex_clk_pll_recalc_rate,
+       .get_parent = clk_pll_get_parent,
+       .prepare = clk_pll_prepare,
+};
+
 static struct clk_ops clk_pll_ops = {

I guess this one could be const too, but in a different patch?

Quoted 2 lines
drivers/clk/socfpga/clk-pll-s10.c +0 -0
        .recalc_rate = clk_pll_recalc_rate,
        .get_parent = clk_pll_get_parent,