[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250915074931.GD8224@nxa18884-linux.ap.freescale.net>
Date: Mon, 15 Sep 2025 15:49:31 +0800
From: Peng Fan <peng.fan@....nxp.com>
To: Ivaylo Ivanov <ivo.ivanov.ivanov1@...il.com>
Cc: Krzysztof Kozlowski <krzk@...nel.org>,
Sylwester Nawrocki <s.nawrocki@...sung.com>,
Chanwoo Choi <cw00.choi@...sung.com>,
Alim Akhtar <alim.akhtar@...sung.com>,
Michael Turquette <mturquette@...libre.com>,
Stephen Boyd <sboyd@...nel.org>, Rob Herring <robh@...nel.org>,
Conor Dooley <conor+dt@...nel.org>,
linux-samsung-soc@...r.kernel.org, devicetree@...r.kernel.org,
linux-clk@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 5/5] clk: samsung: introduce exynos8890 clock driver
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@...il.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@...il.com>
>+ * Author: Ivaylo Ivanov <ivo.ivanov.ivanov1@...il.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
Powered by blists - more mailing lists