Re: [PATCH v1 5/5] clk: samsung: introduce exynos8890 clock driver

[Peng Fan <peng.fan@oss.nxp.com>]

On Sun, Sep 14, 2025 at 03:21:16PM +0300, Ivaylo Ivanov wrote:
>Introduce a clocks management driver for exynos8890, providing clocks
>for the peripherals of that SoC.
>
>As exynos8890 is the first SoC to have HWACG, it differs a bit from the

Hardware Auto Clock Gating(HWACG), if I understand correctly.

>newer SoCs. Q-channel and Q-state bits are separate registers, unlike
>the CLK_CON_GAT_* ones that feature HWACG bits in the same register
>that controls manual gating. Hence, don't use the clk-exynos-arm64
>helper, but implement logic that enforces manual gating according to
>how HWACG is implemented here.
>
>Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
>---
> drivers/clk/samsung/Makefile         |    1 +
> drivers/clk/samsung/clk-exynos8890.c | 8695 ++++++++++++++++++++++++++
> 2 files changed, 8696 insertions(+)
> create mode 100644 drivers/clk/samsung/clk-exynos8890.c
>
>diff --git a/drivers/clk/samsung/Makefile b/drivers/clk/samsung/Makefile
>index b77fe288e..982dc7c64 100644
>--- a/drivers/clk/samsung/Makefile
>+++ b/drivers/clk/samsung/Makefile
>@@ -22,6 +22,7 @@ obj-$(CONFIG_EXYNOS_ARM64_COMMON_CLK)	+= clk-exynos7.o
> obj-$(CONFIG_EXYNOS_ARM64_COMMON_CLK)	+= clk-exynos7870.o
> obj-$(CONFIG_EXYNOS_ARM64_COMMON_CLK)	+= clk-exynos7885.o
> obj-$(CONFIG_EXYNOS_ARM64_COMMON_CLK)	+= clk-exynos850.o
>+obj-$(CONFIG_EXYNOS_ARM64_COMMON_CLK)	+= clk-exynos8890.o
> obj-$(CONFIG_EXYNOS_ARM64_COMMON_CLK)	+= clk-exynos8895.o
> obj-$(CONFIG_EXYNOS_ARM64_COMMON_CLK)	+= clk-exynos990.o
> obj-$(CONFIG_EXYNOS_ARM64_COMMON_CLK)	+= clk-exynosautov9.o
>diff --git a/drivers/clk/samsung/clk-exynos8890.c b/drivers/clk/samsung/clk-exynos8890.c
>new file mode 100644
>index 000000000..670587bae
>--- /dev/null
>+++ b/drivers/clk/samsung/clk-exynos8890.c
>@@ -0,0 +1,8695 @@
>+// SPDX-License-Identifier: GPL-2.0-only
>+/*
>+ * Copyright (C) 2025 Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
>+ * Author: Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
>+ *
>+ * Common Clock Framework support for Exynos8890 SoC.
>+ */
>+
>+#include <linux/clk-provider.h>
>+#include <linux/mod_devicetable.h>
>+#include <linux/of_address.h>
>+#include <linux/of.h>
>+#include <linux/platform_device.h>
>+
>+#include <dt-bindings/clock/samsung,exynos8890-cmu.h>
>+
>+#include "clk.h"
>+
>+/* NOTE: Must be equal to the last clock ID increased by one */
>+#define TOP_NR_CLK	(CLK_GOUT_TOP_SCLK_PROMISE_DISP + 1)
>+#define PERIS_NR_CLK	(CLK_GOUT_PERIS_SCLK_PROMISE_PERIS + 1)
>+#define APOLLO_NR_CLK	(CLK_GOUT_APOLLO_SCLK_PROMISE_APOLLO + 1)
>+#define AUD_NR_CLK	(CLK_GOUT_AUD_SCLK_I2S_BCLK + 1)
>+#define BUS0_NR_CLK	(CLK_GOUT_BUS0_ACLK_TREX_P_BUS0 + 1)
>+#define BUS1_NR_CLK	(CLK_GOUT_BUS1_ACLK_TREX_P_BUS1 + 1)
>+#define CCORE_NR_CLK	(CLK_GOUT_CCORE_SCLK_PROMISE + 1)
>+#define DISP0_NR_CLK	(CLK_GOUT_DISP0_OSCCLK_DP_I_CLK_24M + 1)
>+#define DISP1_NR_CLK	(CLK_GOUT_DISP1_SCLK_PROMISE_DISP1 + 1)
>+#define FSYS0_NR_CLK	(CLK_GOUT_FSYS0_SCLK_USBHOST20_REF_CLK + 1)
>+#define FSYS1_NR_CLK	(CLK_GOUT_FSYS1_SCLK_PROMISE_FSYS1 + 1)
>+#define G3D_NR_CLK	(CLK_GOUT_G3D_SCLK_ASYNCAXI_G3D + 1)
>+#define MIF0_NR_CLK	(CLK_GOUT_MIF0_RCLK_DREX + 1)
>+#define MIF1_NR_CLK	(CLK_GOUT_MIF1_RCLK_DREX + 1)
>+#define MIF2_NR_CLK	(CLK_GOUT_MIF2_RCLK_DREX + 1)
>+#define MIF3_NR_CLK	(CLK_GOUT_MIF3_RCLK_DREX + 1)
>+#define MNGS_NR_CLK	(CLK_GOUT_MNGS_SCLK_PROMISE0_MNGS + 1)
>+#define PERIC0_NR_CLK	(CLK_GOUT_PERIC0_SCLK_PWM + 1)
>+#define PERIC1_NR_CLK	(CLK_GOUT_PERIC1_SCLK_UART5 + 1)
>+
>+/*
>+ * As exynos8890 first introduced hwacg, cmu registers are mapped similarly
>+ * to exynos7, with the exception of the new q-state and q-ch registers that
>+ * can set the behavior of automatic gates.
>+ */
>+
>+/* decoded magic number from downstream */
>+#define QCH_EN_MASK		BIT(0)
>+#define QCH_MASK		(GENMASK(19, 16) | BIT(12))
>+#define QCH_DIS			(QCH_MASK | FIELD_PREP(QCH_EN_MASK, 0))

Nit: align code.

>+
>+/* q-channel registers offsets range */
>+#define QCH_OFF_START		0x2000
>+#define QCH_OFF_END		0x23ff
>+
>+/* q-state registers offsets range */
>+#define QSTATE_OFF_START	0x2400
>+#define QSTATE_OFF_END		0x2fff

Nit: Align.

>+
>+/* check if the register offset is a QCH register */
>+static bool is_qch_reg(unsigned long off)
>+{
>+	return off >= QCH_OFF_START && off <= QCH_OFF_END;
>+}
>+
>+/* check if the register offset is a QSTATE register */
>+static bool is_qstate_reg(unsigned long off)
>+{
>+	return off >= QSTATE_OFF_START && off <= QSTATE_OFF_END;
>+}
>+
>+static void __init exynos8890_init_clocks(struct device_node *np,
>+					  const struct samsung_cmu_info *cmu)
>+{
>+	const unsigned long *reg_offs = cmu->clk_regs;
>+	size_t reg_offs_len = cmu->nr_clk_regs;
>+	void __iomem *reg_base;
>+	size_t i;
>+
>+	reg_base = of_iomap(np, 0);
>+	if (!reg_base)
>+		panic("%s: failed to map registers\n", __func__);
>+
>+	for (i = 0; i < reg_offs_len; ++i) {
>+		void __iomem *reg = reg_base + reg_offs[i];
>+		u32 val;
>+
>+		if (is_qch_reg(reg_offs[i])) {
>+			val = QCH_DIS;
>+			writel(val, reg);
>+		} else if (is_qstate_reg(reg_offs[i])) {
>+			val = 0;
>+			writel(val, reg);
>+		}

This seems to disable qchannel and set qstate to 0 for disable HWACG.
If this is true, a comment is preferred.

>+	}
>+
>+	iounmap(reg_base);
>+}
>+
>+/* ---- CMU_TOP ------------------------------------------------------------- */
>+
>+#define MIF_CLK_CTRL1						0x1084
>+#define MIF_CLK_CTRL2						0x1088
>+#define MIF_CLK_CTRL3						0x108C
>+#define MIF_CLK_CTRL4						0x1090
>+#define ACD_PSCDC_CTRL_0					0x1094
>+#define ACD_PSCDC_CTRL_1					0x1098
>+#define ACD_PSCDC_STAT						0x109C
>+#define CMU_TOP_SPARE0						0x1100
>+#define CMU_TOP_SPARE1						0x1104
>+#define CMU_TOP_SPARE2						0x1108
>+#define CMU_TOP_SPARE3						0x110C

Some of the registers not aligned.

>+
[...]
>+static void __init exynos8890_cmu_top_init(struct device_node *np)
>+{
>+	exynos8890_init_clocks(np, &top_cmu_info);
>+	samsung_cmu_register_one(np, &top_cmu_info);
>+}
>+
>+/* Register CMU_TOP early, as it's a dependency for other early domains */
>+CLK_OF_DECLARE(exynos8890_cmu_top, "samsung,exynos8890-cmu-top",
>+	       exynos8890_cmu_top_init);

Not sure you need to run Android GKI, without module built, this platform
will not able to support GKI.

It would be better to update to use platform drivers.

>+
>+/* ---- CMU_PERIS ---------------------------------------------------------- */
>+
>+#define QSTATE_CTRL_TMU				0x2474
>+#define QSTATE_CTRL_CHIPID			0x2484
>+#define QSTATE_CTRL_PROMISE_PERIS		0x2488

Not aligned.

>+
>+
>+/* Register CMU_PERIS early, as it's needed for MCT timer */
>+CLK_OF_DECLARE(exynos8890_cmu_peris, "samsung,exynos8890-cmu-peris",
>+	       exynos8890_cmu_peris_init);

Same as above.

>+
>+/* ---- CMU_APOLLO --------------------------------------------------------- */
>+
>+/* Register Offset definitions for CMU_APOLLO (0x11900000) */
>+#define APOLLO_PLL_LOCK				0x0000
>+#define APOLLO_PLL_CON0				0x0100
>+#define APOLLO_PLL_CON1				0x0104
>+#define APOLLO_PLL_FREQ_DET			0x010C

Not align.

>+
>+#define CLKOUT_CMU_AUD			0x0D00
>+#define CLKOUT_CMU_AUD_DIV_STAT		0x0D04
>+#define CLK_ENABLE_PDN_AUD		0x0E00
>+#define AUD_SFR_IGNORE_REQ_SYSCLK	0x0F28

Ditto.

>+
>+#define QCH_CTRL_TREX_D_BUS1		0x2000
>+#define QCH_CTRL_FSYS0_D		0x2004
>+#define QCH_CTRL_MFC0_D			0x2008
>+#define QCH_CTRL_MFC1_D			0x200C
>+#define QCH_CTRL_MSCL0_D		0x2010

Ditto. Seems this was generated by tools, better to align all.

>+
>+
>+
>+static int __init exynos8890_cmu_init(void)
>+{
>+	return platform_driver_register(&exynos8890_cmu_driver);
>+}
>+core_initcall(exynos8890_cmu_init);

So early initcall. Not sure about this. But I think devlink or defer probe
could handle correctly for clock stuff. Not block the use of this, a
comment would be preferred.

Regards
Peng