Update exynos_arm64_init_clocks() so that it enables the automatic clock
mode bits in the CMU option register if the auto_clock_gate flag and
option_offset fields are set for the CMU.
The CMU option register bits are global and effect every clock component in
the CMU, as such clearing the GATE_ENABLE_HWACG bit and setting GATE_MANUAL
bit on every gate register is only required if auto_clock_gate is false.
Additionally if auto_clock_gate is enabled the dynamic root clock gating
and memclk registers will be configured in the corresponding CMUs sysreg
bank. These registers are exposed via syscon, so the register
suspend/resume paths are updated to handle using a regmap.
As many gates for various Samsung SoCs are already exposed in the Samsung
clock drivers a new samsung_auto_clk_gate_ops is implemented. This uses
some CMU debug registers to report whether clocks are enabled or disabled
when operating in automatic mode. This allows
/sys/kernel/debug/clk/clk_summary to still dump the entire clock tree and
correctly report the status of each clock in the system.
Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
---
drivers/clk/samsung/clk-exynos-arm64.c | 47 +++++++--
drivers/clk/samsung/clk-exynos4.c | 6 +-
drivers/clk/samsung/clk-exynos4412-isp.c | 4 +-
drivers/clk/samsung/clk-exynos5250.c | 2 +-
drivers/clk/samsung/clk-exynos5420.c | 4 +-
drivers/clk/samsung/clk.c | 161 ++++++++++++++++++++++++++++---
drivers/clk/samsung/clk.h | 49 +++++++++-
7 files changed, 244 insertions(+), 29 deletions(-)
diff --git a/drivers/clk/samsung/clk-exynos-arm64.c b/drivers/clk/samsung/clk-exynos-arm64.c
index bf7de21f329ec89069dcf817ca578fcf9b2d9809..c302c836e8f9f6270753d86d7d986c88e6762f4f 100644
--- a/drivers/clk/samsung/clk-exynos-arm64.c
+++ b/drivers/clk/samsung/clk-exynos-arm64.c
@@ -24,6 +24,16 @@
#define GATE_MANUAL BIT(20)
#define GATE_ENABLE_HWACG BIT(28)
+/* Option register bits */
+#define OPT_EN_DBG BIT(31)
+#define OPT_UNKNOWN BIT(30)
+#define OPT_EN_PWR_MANAGEMENT BIT(29)
+#define OPT_EN_AUTO_GATING BIT(28)
+#define OPT_EN_MEM_PM_GATING BIT(24)
+
+#define CMU_OPT_GLOBAL_EN_AUTO_GATING (OPT_EN_DBG | OPT_UNKNOWN | \
+ OPT_EN_PWR_MANAGEMENT | OPT_EN_AUTO_GATING | OPT_EN_MEM_PM_GATING)
+
/* PLL_CONx_PLL register offsets range */
#define PLL_CON_OFF_START 0x100
#define PLL_CON_OFF_END 0x600
@@ -37,6 +47,8 @@ struct exynos_arm64_cmu_data {
unsigned int nr_clk_save;
const struct samsung_clk_reg_dump *clk_suspend;
unsigned int nr_clk_suspend;
+ struct samsung_clk_reg_dump *clk_sysreg_save;
+ unsigned int nr_clk_sysreg;
struct clk *clk;
struct clk **pclks;
@@ -82,13 +94,28 @@ static void __init exynos_arm64_init_clocks(struct device_node *np,
if (!reg_base)
panic("%s: failed to map registers\n", __func__);
+ if (cmu->auto_clock_gate && cmu->option_offset) {
+ /*
+ * Enable the global automatic mode for the entire CMU.
+ * This overrides the individual HWACG bits in each of the
+ * individual gate, mux and qch registers.
+ */
+ writel(CMU_OPT_GLOBAL_EN_AUTO_GATING,
+ reg_base + cmu->option_offset);
+ }
+
for (i = 0; i < reg_offs_len; ++i) {
void __iomem *reg = reg_base + reg_offs[i];
u32 val;
if (cmu->manual_plls && is_pll_con1_reg(reg_offs[i])) {
writel(PLL_CON1_MANUAL, reg);
- } else if (is_gate_reg(reg_offs[i])) {
+ } else if (is_gate_reg(reg_offs[i]) && !cmu->auto_clock_gate) {
+ /*
+ * Setting GATE_MANUAL bit (which is described in TRM as
+ * reserved!) overrides the global CMU automatic mode
+ * option.
+ */
val = readl(reg);
val |= GATE_MANUAL;
val &= ~GATE_ENABLE_HWACG;
@@ -219,7 +246,7 @@ void __init exynos_arm64_register_cmu(struct device *dev,
* Return: 0 on success, or negative error code on error.
*/
int __init exynos_arm64_register_cmu_pm(struct platform_device *pdev,
- bool set_manual)
+ bool init_clk_regs)
{
const struct samsung_cmu_info *cmu;
struct device *dev = &pdev->dev;
@@ -249,7 +276,7 @@ int __init exynos_arm64_register_cmu_pm(struct platform_device *pdev,
dev_err(dev, "%s: could not enable bus clock %s; err = %d\n",
__func__, cmu->clk_name, ret);
- if (set_manual)
+ if (init_clk_regs)
exynos_arm64_init_clocks(np, cmu);
reg_base = devm_platform_ioremap_resource(pdev, 0);
@@ -280,14 +307,18 @@ int exynos_arm64_cmu_suspend(struct device *dev)
struct exynos_arm64_cmu_data *data = dev_get_drvdata(dev);
int i;
- samsung_clk_save(data->ctx->reg_base, data->clk_save,
+ samsung_clk_save(data->ctx->reg_base, NULL, data->clk_save,
data->nr_clk_save);
+ if (data->ctx->sysreg)
+ samsung_clk_save(NULL, data->ctx->sysreg, data->clk_save,
+ data->nr_clk_save);
+
for (i = 0; i < data->nr_pclks; i++)
clk_prepare_enable(data->pclks[i]);
/* For suspend some registers have to be set to certain values */
- samsung_clk_restore(data->ctx->reg_base, data->clk_suspend,
+ samsung_clk_restore(data->ctx->reg_base, NULL, data->clk_suspend,
data->nr_clk_suspend);
for (i = 0; i < data->nr_pclks; i++)
@@ -308,9 +339,13 @@ int exynos_arm64_cmu_resume(struct device *dev)
for (i = 0; i < data->nr_pclks; i++)
clk_prepare_enable(data->pclks[i]);
- samsung_clk_restore(data->ctx->reg_base, data->clk_save,
+ samsung_clk_restore(data->ctx->reg_base, NULL, data->clk_save,
data->nr_clk_save);
+ if (data->ctx->sysreg)
+ samsung_clk_restore(NULL, data->ctx->sysreg, data->clk_save,
+ data->nr_clk_save);
+
for (i = 0; i < data->nr_pclks; i++)
clk_disable_unprepare(data->pclks[i]);
diff --git a/drivers/clk/samsung/clk-exynos4.c b/drivers/clk/samsung/clk-exynos4.c
index cc5c1644c41c08b27bc48d809a08cd8a006cbe8f..26ac9734722d1e7ed8ec3f1c0a956f26e32b92d4 100644
--- a/drivers/clk/samsung/clk-exynos4.c
+++ b/drivers/clk/samsung/clk-exynos4.c
@@ -1378,15 +1378,15 @@ static void __init exynos4_clk_init(struct device_node *np,
if (soc == EXYNOS4212 || soc == EXYNOS4412)
exynos4x12_core_down_clock();
- samsung_clk_extended_sleep_init(reg_base,
+ samsung_clk_extended_sleep_init(reg_base, NULL,
exynos4_clk_regs, ARRAY_SIZE(exynos4_clk_regs),
src_mask_suspend, ARRAY_SIZE(src_mask_suspend));
if (exynos4_soc == EXYNOS4210)
- samsung_clk_extended_sleep_init(reg_base,
+ samsung_clk_extended_sleep_init(reg_base, NULL,
exynos4210_clk_save, ARRAY_SIZE(exynos4210_clk_save),
src_mask_suspend_e4210, ARRAY_SIZE(src_mask_suspend_e4210));
else
- samsung_clk_sleep_init(reg_base, exynos4x12_clk_save,
+ samsung_clk_sleep_init(reg_base, NULL, exynos4x12_clk_save,
ARRAY_SIZE(exynos4x12_clk_save));
samsung_clk_of_add_provider(np, ctx);
diff --git a/drivers/clk/samsung/clk-exynos4412-isp.c b/drivers/clk/samsung/clk-exynos4412-isp.c
index fa915057e109e0008ebe0b1b5d1652fd5804e82b..772bc18a1e686f23b11bf160b803becff6279637 100644
--- a/drivers/clk/samsung/clk-exynos4412-isp.c
+++ b/drivers/clk/samsung/clk-exynos4412-isp.c
@@ -94,7 +94,7 @@ static int __maybe_unused exynos4x12_isp_clk_suspend(struct device *dev)
{
struct samsung_clk_provider *ctx = dev_get_drvdata(dev);
- samsung_clk_save(ctx->reg_base, exynos4x12_save_isp,
+ samsung_clk_save(ctx->reg_base, NULL, exynos4x12_save_isp,
ARRAY_SIZE(exynos4x12_clk_isp_save));
return 0;
}
@@ -103,7 +103,7 @@ static int __maybe_unused exynos4x12_isp_clk_resume(struct device *dev)
{
struct samsung_clk_provider *ctx = dev_get_drvdata(dev);
- samsung_clk_restore(ctx->reg_base, exynos4x12_save_isp,
+ samsung_clk_restore(ctx->reg_base, NULL, exynos4x12_save_isp,
ARRAY_SIZE(exynos4x12_clk_isp_save));
return 0;
}
diff --git a/drivers/clk/samsung/clk-exynos5250.c b/drivers/clk/samsung/clk-exynos5250.c
index e90d3a0848cbc24b2709c10795f6affcda404567..f97f30b29be7317db8186bac39cf52e1893eb106 100644
--- a/drivers/clk/samsung/clk-exynos5250.c
+++ b/drivers/clk/samsung/clk-exynos5250.c
@@ -854,7 +854,7 @@ static void __init exynos5250_clk_init(struct device_node *np)
PWR_CTRL2_CORE2_UP_RATIO | PWR_CTRL2_CORE1_UP_RATIO);
__raw_writel(tmp, reg_base + PWR_CTRL2);
- samsung_clk_sleep_init(reg_base, exynos5250_clk_regs,
+ samsung_clk_sleep_init(reg_base, NULL, exynos5250_clk_regs,
ARRAY_SIZE(exynos5250_clk_regs));
exynos5_subcmus_init(ctx, ARRAY_SIZE(exynos5250_subcmus),
exynos5250_subcmus);
diff --git a/drivers/clk/samsung/clk-exynos5420.c b/drivers/clk/samsung/clk-exynos5420.c
index a9df4e6db82fa7831d4e5c7210b0163d7d301ec1..1982e0751ceec7e57f9e82d96dcbadce1f691092 100644
--- a/drivers/clk/samsung/clk-exynos5420.c
+++ b/drivers/clk/samsung/clk-exynos5420.c
@@ -1649,12 +1649,12 @@ static void __init exynos5x_clk_init(struct device_node *np,
ARRAY_SIZE(exynos5800_cpu_clks));
}
- samsung_clk_extended_sleep_init(reg_base,
+ samsung_clk_extended_sleep_init(reg_base, NULL,
exynos5x_clk_regs, ARRAY_SIZE(exynos5x_clk_regs),
exynos5420_set_clksrc, ARRAY_SIZE(exynos5420_set_clksrc));
if (soc == EXYNOS5800) {
- samsung_clk_sleep_init(reg_base, exynos5800_clk_regs,
+ samsung_clk_sleep_init(reg_base, NULL, exynos5800_clk_regs,
ARRAY_SIZE(exynos5800_clk_regs));
exynos5_subcmus_init(ctx, ARRAY_SIZE(exynos5800_subcmus),
diff --git a/drivers/clk/samsung/clk.c b/drivers/clk/samsung/clk.c
index dbc9925ca8f46e951dfb5d391c0e744ca370abcc..07b2948ae7ea48f126ab420be57d8c2705979464 100644
--- a/drivers/clk/samsung/clk.c
+++ b/drivers/clk/samsung/clk.c
@@ -12,8 +12,10 @@
#include <linux/clkdev.h>
#include <linux/clk-provider.h>
#include <linux/io.h>
+#include <linux/mfd/syscon.h>
#include <linux/mod_devicetable.h>
#include <linux/of_address.h>
+#include <linux/regmap.h>
#include <linux/syscore_ops.h>
#include "clk.h"
@@ -21,19 +23,29 @@
static LIST_HEAD(clock_reg_cache_list);
void samsung_clk_save(void __iomem *base,
+ struct regmap *regmap,
struct samsung_clk_reg_dump *rd,
unsigned int num_regs)
{
- for (; num_regs > 0; --num_regs, ++rd)
- rd->value = readl(base + rd->offset);
+ for (; num_regs > 0; --num_regs, ++rd) {
+ if (base)
+ rd->value = readl(base + rd->offset);
+ if (regmap)
+ regmap_read(regmap, rd->offset, &rd->value);
+ }
}
void samsung_clk_restore(void __iomem *base,
+ struct regmap *regmap,
const struct samsung_clk_reg_dump *rd,
unsigned int num_regs)
{
- for (; num_regs > 0; --num_regs, ++rd)
- writel(rd->value, base + rd->offset);
+ for (; num_regs > 0; --num_regs, ++rd) {
+ if (base)
+ writel(rd->value, base + rd->offset);
+ if (regmap)
+ regmap_write(regmap, rd->offset, rd->value);
+ }
}
struct samsung_clk_reg_dump *samsung_clk_alloc_reg_dump(
@@ -227,6 +239,82 @@ void __init samsung_clk_register_div(struct samsung_clk_provider *ctx,
}
}
+#define ACG_MSK GENMASK(6, 4)
+#define CLK_IDLE GENMASK(5, 4)
+static int samsung_auto_clk_gate_is_en(struct clk_hw *hw)
+{
+ u32 reg;
+ struct clk_gate *gate = to_clk_gate(hw);
+
+ reg = readl(gate->reg);
+ return ((reg & ACG_MSK) == CLK_IDLE) ? 0 : 1;
+}
+
+/* enable and disable are nops in automatic clock mode */
+static int samsung_auto_clk_gate_en(struct clk_hw *hw)
+{
+ return 0;
+}
+
+static void samsung_auto_clk_gate_dis(struct clk_hw *hw)
+{
+}
+
+static const struct clk_ops samsung_auto_clk_gate_ops = {
+ .enable = samsung_auto_clk_gate_en,
+ .disable = samsung_auto_clk_gate_dis,
+ .is_enabled = samsung_auto_clk_gate_is_en,
+};
+
+struct clk_hw *samsung_register_auto_gate(struct device *dev,
+ struct device_node *np, const char *name,
+ const char *parent_name, const struct clk_hw *parent_hw,
+ const struct clk_parent_data *parent_data,
+ unsigned long flags,
+ void __iomem *reg, u8 bit_idx,
+ u8 clk_gate_flags, spinlock_t *lock)
+{
+ struct clk_gate *gate;
+ struct clk_hw *hw;
+ struct clk_init_data init = {};
+ int ret = -EINVAL;
+
+ /* allocate the gate */
+ gate = kzalloc(sizeof(*gate), GFP_KERNEL);
+ if (!gate)
+ return ERR_PTR(-ENOMEM);
+
+ init.name = name;
+ init.ops = &samsung_auto_clk_gate_ops;
+ init.flags = flags;
+ init.parent_names = parent_name ? &parent_name : NULL;
+ init.parent_hws = parent_hw ? &parent_hw : NULL;
+ init.parent_data = parent_data;
+ if (parent_name || parent_hw || parent_data)
+ init.num_parents = 1;
+ else
+ init.num_parents = 0;
+
+ /* struct clk_gate assignments */
+ gate->reg = reg;
+ gate->bit_idx = bit_idx;
+ gate->flags = clk_gate_flags;
+ gate->lock = lock;
+ gate->hw.init = &init;
+
+ hw = &gate->hw;
+ if (dev || !np)
+ ret = clk_hw_register(dev, hw);
+ else if (np)
+ ret = of_clk_hw_register(np, hw);
+ if (ret) {
+ kfree(gate);
+ hw = ERR_PTR(ret);
+ }
+
+ return hw;
+}
+
/* register a list of gate clocks */
void __init samsung_clk_register_gate(struct samsung_clk_provider *ctx,
const struct samsung_gate_clock *list,
@@ -234,11 +322,21 @@ void __init samsung_clk_register_gate(struct samsung_clk_provider *ctx,
{
struct clk_hw *clk_hw;
unsigned int idx;
+ void __iomem *reg_offs;
for (idx = 0; idx < nr_clk; idx++, list++) {
- clk_hw = clk_hw_register_gate(ctx->dev, list->name, list->parent_name,
- list->flags, ctx->reg_base + list->offset,
+ reg_offs = ctx->reg_base + list->offset;
+
+ if (ctx->auto_clock_gate && ctx->gate_dbg_offset)
+ clk_hw = samsung_register_auto_gate(ctx->dev, NULL,
+ list->name, list->parent_name, NULL, NULL,
+ list->flags, reg_offs + ctx->gate_dbg_offset,
list->bit_idx, list->gate_flags, &ctx->lock);
+ else
+ clk_hw = clk_hw_register_gate(ctx->dev, list->name,
+ list->parent_name, list->flags,
+ ctx->reg_base + list->offset, list->bit_idx,
+ list->gate_flags, &ctx->lock);
if (IS_ERR(clk_hw)) {
pr_err("%s: failed to register clock %s\n", __func__,
list->name);
@@ -276,10 +374,11 @@ static int samsung_clk_suspend(void)
struct samsung_clock_reg_cache *reg_cache;
list_for_each_entry(reg_cache, &clock_reg_cache_list, node) {
- samsung_clk_save(reg_cache->reg_base, reg_cache->rdump,
- reg_cache->rd_num);
- samsung_clk_restore(reg_cache->reg_base, reg_cache->rsuspend,
- reg_cache->rsuspend_num);
+ samsung_clk_save(reg_cache->reg_base, reg_cache->sysreg,
+ reg_cache->rdump, reg_cache->rd_num);
+ samsung_clk_restore(reg_cache->reg_base, reg_cache->sysreg,
+ reg_cache->rsuspend,
+ reg_cache->rsuspend_num);
}
return 0;
}
@@ -289,8 +388,8 @@ static void samsung_clk_resume(void)
struct samsung_clock_reg_cache *reg_cache;
list_for_each_entry(reg_cache, &clock_reg_cache_list, node)
- samsung_clk_restore(reg_cache->reg_base, reg_cache->rdump,
- reg_cache->rd_num);
+ samsung_clk_restore(reg_cache->reg_base, reg_cache->sysreg,
+ reg_cache->rdump, reg_cache->rd_num);
}
static struct syscore_ops samsung_clk_syscore_ops = {
@@ -299,6 +398,7 @@ static struct syscore_ops samsung_clk_syscore_ops = {
};
void samsung_clk_extended_sleep_init(void __iomem *reg_base,
+ struct regmap *sysreg,
const unsigned long *rdump,
unsigned long nr_rdump,
const struct samsung_clk_reg_dump *rsuspend,
@@ -319,6 +419,7 @@ void samsung_clk_extended_sleep_init(void __iomem *reg_base,
register_syscore_ops(&samsung_clk_syscore_ops);
reg_cache->reg_base = reg_base;
+ reg_cache->sysreg = sysreg;
reg_cache->rd_num = nr_rdump;
reg_cache->rsuspend = rsuspend;
reg_cache->rsuspend_num = nr_rsuspend;
@@ -334,6 +435,12 @@ void samsung_clk_extended_sleep_init(void __iomem *reg_base,
void __init samsung_cmu_register_clocks(struct samsung_clk_provider *ctx,
const struct samsung_cmu_info *cmu)
{
+ ctx->auto_clock_gate = cmu->auto_clock_gate;
+ ctx->gate_dbg_offset = cmu->gate_dbg_offset;
+ ctx->option_offset = cmu->option_offset;
+ ctx->drcg_offset = cmu->drcg_offset;
+ ctx->memclk_offset = cmu->memclk_offset;
+
if (cmu->pll_clks)
samsung_clk_register_pll(ctx, cmu->pll_clks, cmu->nr_pll_clks);
if (cmu->mux_clks)
@@ -353,6 +460,31 @@ void __init samsung_cmu_register_clocks(struct samsung_clk_provider *ctx,
samsung_clk_register_cpu(ctx, cmu->cpu_clks, cmu->nr_cpu_clks);
}
+/* Enable Dynamic Root Clock Gating of bus components*/
+void samsung_en_dyn_root_clk_gating(struct device_node *np,
+ struct samsung_clk_provider *ctx,
+ const struct samsung_cmu_info *cmu)
+{
+ if (ctx && !ctx->auto_clock_gate)
+ return;
+
+ ctx->sysreg = syscon_regmap_lookup_by_phandle(np, "samsung,sysreg");
+ if (!IS_ERR_OR_NULL(ctx->sysreg)) {
+ regmap_write(ctx->sysreg, ctx->drcg_offset, 0xffffffff);
+ /* not every sysreg controller has memclk reg*/
+ if (ctx->memclk_offset)
+ regmap_write_bits(ctx->sysreg, ctx->memclk_offset, 0x1, 0x0);
+
+ samsung_clk_extended_sleep_init(NULL, ctx->sysreg,
+ cmu->sysreg_clk_regs,
+ cmu->nr_sysreg_clk_regs,
+ NULL, 0);
+ } else {
+ pr_warn("%pOF: Unable to get CMU sysreg\n", np);
+ ctx->sysreg = NULL;
+ }
+}
+
/*
* Common function which registers plls, muxes, dividers and gates
* for each CMU. It also add CMU register list to register cache.
@@ -374,11 +506,14 @@ struct samsung_clk_provider * __init samsung_cmu_register_one(
samsung_cmu_register_clocks(ctx, cmu);
if (cmu->clk_regs)
- samsung_clk_extended_sleep_init(reg_base,
+ samsung_clk_extended_sleep_init(reg_base, NULL,
cmu->clk_regs, cmu->nr_clk_regs,
cmu->suspend_regs, cmu->nr_suspend_regs);
samsung_clk_of_add_provider(np, ctx);
+ /* sysreg DT nodes reference a clock in this CMU */
+ samsung_en_dyn_root_clk_gating(np, ctx, cmu);
+
return ctx;
}
diff --git a/drivers/clk/samsung/clk.h b/drivers/clk/samsung/clk.h
index 18660c1ac6f0106b17b9efc9c6b3cd62d46f7b82..b719e057f45489e9d92ba54031fe633a8c9264ce 100644
--- a/drivers/clk/samsung/clk.h
+++ b/drivers/clk/samsung/clk.h
@@ -12,6 +12,7 @@
#include <linux/clk-provider.h>
#include <linux/mod_devicetable.h>
+#include <linux/regmap.h>
#include "clk-pll.h"
#include "clk-cpu.h"
@@ -19,13 +20,25 @@
* struct samsung_clk_provider - information about clock provider
* @reg_base: virtual address for the register base
* @dev: clock provider device needed for runtime PM
+ * @sysreg: syscon regmap for clock-provider sysreg controller
* @lock: maintains exclusion between callbacks for a given clock-provider
+ * @auto_clock_gate: enable auto clk mode for all clocks in clock-provider
+ * @gate_dbg_offset: gate debug reg offset. Used for all gates in auto clk mode
+ * @option_offset: option reg offset. Enables auto mode for clock-provider
+ * @drcg_offset: dynamic root clk gate enable register offset
+ * @memclk_offset: memclk enable register offset
* @clk_data: holds clock related data like clk_hw* and number of clocks
*/
struct samsung_clk_provider {
void __iomem *reg_base;
struct device *dev;
+ struct regmap *sysreg;
spinlock_t lock;
+ bool auto_clock_gate;
+ u32 gate_dbg_offset;
+ u32 option_offset;
+ u32 drcg_offset;
+ u32 memclk_offset;
/* clk_data must be the last entry due to variable length 'hws' array */
struct clk_hw_onecell_data clk_data;
};
@@ -310,6 +323,7 @@ struct samsung_cpu_clock {
struct samsung_clock_reg_cache {
struct list_head node;
void __iomem *reg_base;
+ struct regmap *sysreg;
struct samsung_clk_reg_dump *rdump;
unsigned int rd_num;
const struct samsung_clk_reg_dump *rsuspend;
@@ -338,7 +352,14 @@ struct samsung_clock_reg_cache {
* @suspend_regs: list of clock registers to set before suspend
* @nr_suspend_regs: count of clock registers in @suspend_regs
* @clk_name: name of the parent clock needed for CMU register access
+ * @sysreg_clk_regs: list of sysreg clock registers
+ * @nr_sysreg_clk_regs: count of clock registers in @sysreg_clk_regs
* @manual_plls: Enable manual control for PLL clocks
+ * @auto_clock_gate: enable auto clock mode for all components in CMU
+ * @gate_dbg_offset: gate debug reg offset. Used by all gates in auto clk mode
+ * @option_offset: option reg offset. Enables auto clk mode for entire CMU
+ * @drcg_offset: dynamic root clk gate enable register offset
+ * @memclk_offset: memclk enable register offset
*/
struct samsung_cmu_info {
const struct samsung_pll_clock *pll_clks;
@@ -364,8 +385,16 @@ struct samsung_cmu_info {
unsigned int nr_suspend_regs;
const char *clk_name;
+ const unsigned long *sysreg_clk_regs;
+ unsigned int nr_sysreg_clk_regs;
+
/* ARM64 Exynos CMUs */
bool manual_plls;
+ bool auto_clock_gate;
+ u32 gate_dbg_offset;
+ u32 option_offset;
+ u32 drcg_offset;
+ u32 memclk_offset;
};
struct samsung_clk_provider *samsung_clk_init(struct device *dev,
@@ -415,6 +444,7 @@ struct samsung_clk_provider *samsung_cmu_register_one(
#ifdef CONFIG_PM_SLEEP
void samsung_clk_extended_sleep_init(void __iomem *reg_base,
+ struct regmap *sysreg,
const unsigned long *rdump,
unsigned long nr_rdump,
const struct samsung_clk_reg_dump *rsuspend,
@@ -426,17 +456,32 @@ static inline void samsung_clk_extended_sleep_init(void __iomem *reg_base,
const struct samsung_clk_reg_dump *rsuspend,
unsigned long nr_rsuspend) {}
#endif
-#define samsung_clk_sleep_init(reg_base, rdump, nr_rdump) \
- samsung_clk_extended_sleep_init(reg_base, rdump, nr_rdump, NULL, 0)
+#define samsung_clk_sleep_init(reg_base, sysreg, rdump, nr_rdump) \
+ samsung_clk_extended_sleep_init(reg_base, sysreg, rdump, nr_rdump, \
+ NULL, 0)
void samsung_clk_save(void __iomem *base,
+ struct regmap *regmap,
struct samsung_clk_reg_dump *rd,
unsigned int num_regs);
void samsung_clk_restore(void __iomem *base,
+ struct regmap *regmap,
const struct samsung_clk_reg_dump *rd,
unsigned int num_regs);
struct samsung_clk_reg_dump *samsung_clk_alloc_reg_dump(
const unsigned long *rdump,
unsigned long nr_rdump);
+void samsung_en_dyn_root_clk_gating(struct device_node *np,
+ struct samsung_clk_provider *ctx,
+ const struct samsung_cmu_info *cmu);
+
+struct clk_hw *samsung_register_auto_gate(struct device *dev,
+ struct device_node *np, const char *name,
+ const char *parent_name, const struct clk_hw *parent_hw,
+ const struct clk_parent_data *parent_data,
+ unsigned long flags,
+ void __iomem *reg, u8 bit_idx,
+ u8 clk_gate_flags, spinlock_t *lock);
+
#endif /* __SAMSUNG_CLK_H */
--
2.51.0.760.g7b8bcc2412-googOn 13/10/2025 22:51, Peter Griffin wrote:
> Update exynos_arm64_init_clocks() so that it enables the automatic clock
> mode bits in the CMU option register if the auto_clock_gate flag and
> option_offset fields are set for the CMU.
>
> The CMU option register bits are global and effect every clock component in
> the CMU, as such clearing the GATE_ENABLE_HWACG bit and setting GATE_MANUAL
> bit on every gate register is only required if auto_clock_gate is false.
>
> Additionally if auto_clock_gate is enabled the dynamic root clock gating
> and memclk registers will be configured in the corresponding CMUs sysreg
> bank. These registers are exposed via syscon, so the register
> suspend/resume paths are updated to handle using a regmap.
>
> As many gates for various Samsung SoCs are already exposed in the Samsung
> clock drivers a new samsung_auto_clk_gate_ops is implemented. This uses
> some CMU debug registers to report whether clocks are enabled or disabled
> when operating in automatic mode. This allows
> /sys/kernel/debug/clk/clk_summary to still dump the entire clock tree and
> correctly report the status of each clock in the system.
>
> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> ---
> drivers/clk/samsung/clk-exynos-arm64.c | 47 +++++++--
> drivers/clk/samsung/clk-exynos4.c | 6 +-
> drivers/clk/samsung/clk-exynos4412-isp.c | 4 +-
> drivers/clk/samsung/clk-exynos5250.c | 2 +-
> drivers/clk/samsung/clk-exynos5420.c | 4 +-
> drivers/clk/samsung/clk.c | 161 ++++++++++++++++++++++++++++---
> drivers/clk/samsung/clk.h | 49 +++++++++-
> 7 files changed, 244 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/clk/samsung/clk-exynos-arm64.c b/drivers/clk/samsung/clk-exynos-arm64.c
> index bf7de21f329ec89069dcf817ca578fcf9b2d9809..c302c836e8f9f6270753d86d7d986c88e6762f4f 100644
> --- a/drivers/clk/samsung/clk-exynos-arm64.c
> +++ b/drivers/clk/samsung/clk-exynos-arm64.c
> @@ -24,6 +24,16 @@
> #define GATE_MANUAL BIT(20)
> #define GATE_ENABLE_HWACG BIT(28)
>
> +/* Option register bits */
> +#define OPT_EN_DBG BIT(31)
> +#define OPT_UNKNOWN BIT(30)
> +#define OPT_EN_PWR_MANAGEMENT BIT(29)
> +#define OPT_EN_AUTO_GATING BIT(28)
> +#define OPT_EN_MEM_PM_GATING BIT(24)
Please reverse, from lowest to highest number (LSB -> MSB).
> +
> +#define CMU_OPT_GLOBAL_EN_AUTO_GATING (OPT_EN_DBG | OPT_UNKNOWN | \
> + OPT_EN_PWR_MANAGEMENT | OPT_EN_AUTO_GATING | OPT_EN_MEM_PM_GATING)
> +
> /* PLL_CONx_PLL register offsets range */
> #define PLL_CON_OFF_START 0x100
> #define PLL_CON_OFF_END 0x600
> @@ -37,6 +47,8 @@ struct exynos_arm64_cmu_data {
> unsigned int nr_clk_save;
> const struct samsung_clk_reg_dump *clk_suspend;
> unsigned int nr_clk_suspend;
> + struct samsung_clk_reg_dump *clk_sysreg_save;
> + unsigned int nr_clk_sysreg;
>
> struct clk *clk;
> struct clk **pclks;
> @@ -82,13 +94,28 @@ static void __init exynos_arm64_init_clocks(struct device_node *np,
> if (!reg_base)
> panic("%s: failed to map registers\n", __func__);
>
> + if (cmu->auto_clock_gate && cmu->option_offset) {
> + /*
> + * Enable the global automatic mode for the entire CMU.
> + * This overrides the individual HWACG bits in each of the
> + * individual gate, mux and qch registers.
> + */
> + writel(CMU_OPT_GLOBAL_EN_AUTO_GATING,
> + reg_base + cmu->option_offset);
> + }
> +
> for (i = 0; i < reg_offs_len; ++i) {
> void __iomem *reg = reg_base + reg_offs[i];
> u32 val;
>
> if (cmu->manual_plls && is_pll_con1_reg(reg_offs[i])) {
> writel(PLL_CON1_MANUAL, reg);
> - } else if (is_gate_reg(reg_offs[i])) {
> + } else if (is_gate_reg(reg_offs[i]) && !cmu->auto_clock_gate) {
> + /*
> + * Setting GATE_MANUAL bit (which is described in TRM as
> + * reserved!) overrides the global CMU automatic mode
> + * option.
> + */
> val = readl(reg);
> val |= GATE_MANUAL;
> val &= ~GATE_ENABLE_HWACG;
> @@ -219,7 +246,7 @@ void __init exynos_arm64_register_cmu(struct device *dev,
> * Return: 0 on success, or negative error code on error.
> */
> int __init exynos_arm64_register_cmu_pm(struct platform_device *pdev,
> - bool set_manual)
> + bool init_clk_regs)
> {
> const struct samsung_cmu_info *cmu;
> struct device *dev = &pdev->dev;
> @@ -249,7 +276,7 @@ int __init exynos_arm64_register_cmu_pm(struct platform_device *pdev,
> dev_err(dev, "%s: could not enable bus clock %s; err = %d\n",
> __func__, cmu->clk_name, ret);
>
> - if (set_manual)
> + if (init_clk_regs)
> exynos_arm64_init_clocks(np, cmu);
>
> reg_base = devm_platform_ioremap_resource(pdev, 0);
> @@ -280,14 +307,18 @@ int exynos_arm64_cmu_suspend(struct device *dev)
> struct exynos_arm64_cmu_data *data = dev_get_drvdata(dev);
> int i;
>
> - samsung_clk_save(data->ctx->reg_base, data->clk_save,
> + samsung_clk_save(data->ctx->reg_base, NULL, data->clk_save,
> data->nr_clk_save);
>
> + if (data->ctx->sysreg)
No need for if() here and in samsung_clk_save.
I find it confusing. You do samsung_clk_save() for regular CMU MMIO
address and then immediately - for GS101 - do the same for sysreg:
> + samsung_clk_save(NULL, data->ctx->sysreg, data->clk_save,
> + data->nr_clk_save);
So this feels like you could make one call of samsung_clk_save() with
both arguments - reg_base and sysreg. It also leads to confusion - first
read from MMIO will be overwritten by sysreg read.
> +
> for (i = 0; i < data->nr_pclks; i++)
> clk_prepare_enable(data->pclks[i]);
>
> /* For suspend some registers have to be set to certain values */
> - samsung_clk_restore(data->ctx->reg_base, data->clk_suspend,
> + samsung_clk_restore(data->ctx->reg_base, NULL, data->clk_suspend,
> data->nr_clk_suspend);
>
> for (i = 0; i < data->nr_pclks; i++)
> @@ -308,9 +339,13 @@ int exynos_arm64_cmu_resume(struct device *dev)
> for (i = 0; i < data->nr_pclks; i++)
> clk_prepare_enable(data->pclks[i]);
>
> - samsung_clk_restore(data->ctx->reg_base, data->clk_save,
> + samsung_clk_restore(data->ctx->reg_base, NULL, data->clk_save,
> data->nr_clk_save);
>
> + if (data->ctx->sysreg)
> + samsung_clk_restore(NULL, data->ctx->sysreg, data->clk_save,
> + data->nr_clk_save);
> +
> for (i = 0; i < data->nr_pclks; i++)
> clk_disable_unprepare(data->pclks[i]);
>
> diff --git a/drivers/clk/samsung/clk-exynos4.c b/drivers/clk/samsung/clk-exynos4.c
> index cc5c1644c41c08b27bc48d809a08cd8a006cbe8f..26ac9734722d1e7ed8ec3f1c0a956f26e32b92d4 100644
> --- a/drivers/clk/samsung/clk-exynos4.c
> +++ b/drivers/clk/samsung/clk-exynos4.c
> @@ -1378,15 +1378,15 @@ static void __init exynos4_clk_init(struct device_node *np,
> if (soc == EXYNOS4212 || soc == EXYNOS4412)
> exynos4x12_core_down_clock();
>
> - samsung_clk_extended_sleep_init(reg_base,
> + samsung_clk_extended_sleep_init(reg_base, NULL,
> exynos4_clk_regs, ARRAY_SIZE(exynos4_clk_regs),
> src_mask_suspend, ARRAY_SIZE(src_mask_suspend));
> if (exynos4_soc == EXYNOS4210)
> - samsung_clk_extended_sleep_init(reg_base,
> + samsung_clk_extended_sleep_init(reg_base, NULL,
> exynos4210_clk_save, ARRAY_SIZE(exynos4210_clk_save),
> src_mask_suspend_e4210, ARRAY_SIZE(src_mask_suspend_e4210));
> else
> - samsung_clk_sleep_init(reg_base, exynos4x12_clk_save,
> + samsung_clk_sleep_init(reg_base, NULL, exynos4x12_clk_save,
> ARRAY_SIZE(exynos4x12_clk_save));
>
> samsung_clk_of_add_provider(np, ctx);
> diff --git a/drivers/clk/samsung/clk-exynos4412-isp.c b/drivers/clk/samsung/clk-exynos4412-isp.c
> index fa915057e109e0008ebe0b1b5d1652fd5804e82b..772bc18a1e686f23b11bf160b803becff6279637 100644
> --- a/drivers/clk/samsung/clk-exynos4412-isp.c
> +++ b/drivers/clk/samsung/clk-exynos4412-isp.c
> @@ -94,7 +94,7 @@ static int __maybe_unused exynos4x12_isp_clk_suspend(struct device *dev)
> {
> struct samsung_clk_provider *ctx = dev_get_drvdata(dev);
>
> - samsung_clk_save(ctx->reg_base, exynos4x12_save_isp,
> + samsung_clk_save(ctx->reg_base, NULL, exynos4x12_save_isp,
> ARRAY_SIZE(exynos4x12_clk_isp_save));
> return 0;
> }
> @@ -103,7 +103,7 @@ static int __maybe_unused exynos4x12_isp_clk_resume(struct device *dev)
> {
> struct samsung_clk_provider *ctx = dev_get_drvdata(dev);
>
> - samsung_clk_restore(ctx->reg_base, exynos4x12_save_isp,
> + samsung_clk_restore(ctx->reg_base, NULL, exynos4x12_save_isp,
> ARRAY_SIZE(exynos4x12_clk_isp_save));
> return 0;
> }
> diff --git a/drivers/clk/samsung/clk-exynos5250.c b/drivers/clk/samsung/clk-exynos5250.c
> index e90d3a0848cbc24b2709c10795f6affcda404567..f97f30b29be7317db8186bac39cf52e1893eb106 100644
> --- a/drivers/clk/samsung/clk-exynos5250.c
> +++ b/drivers/clk/samsung/clk-exynos5250.c
> @@ -854,7 +854,7 @@ static void __init exynos5250_clk_init(struct device_node *np)
> PWR_CTRL2_CORE2_UP_RATIO | PWR_CTRL2_CORE1_UP_RATIO);
> __raw_writel(tmp, reg_base + PWR_CTRL2);
>
> - samsung_clk_sleep_init(reg_base, exynos5250_clk_regs,
> + samsung_clk_sleep_init(reg_base, NULL, exynos5250_clk_regs,
> ARRAY_SIZE(exynos5250_clk_regs));
> exynos5_subcmus_init(ctx, ARRAY_SIZE(exynos5250_subcmus),
> exynos5250_subcmus);
> diff --git a/drivers/clk/samsung/clk-exynos5420.c b/drivers/clk/samsung/clk-exynos5420.c
> index a9df4e6db82fa7831d4e5c7210b0163d7d301ec1..1982e0751ceec7e57f9e82d96dcbadce1f691092 100644
> --- a/drivers/clk/samsung/clk-exynos5420.c
> +++ b/drivers/clk/samsung/clk-exynos5420.c
> @@ -1649,12 +1649,12 @@ static void __init exynos5x_clk_init(struct device_node *np,
> ARRAY_SIZE(exynos5800_cpu_clks));
> }
>
> - samsung_clk_extended_sleep_init(reg_base,
> + samsung_clk_extended_sleep_init(reg_base, NULL,
> exynos5x_clk_regs, ARRAY_SIZE(exynos5x_clk_regs),
> exynos5420_set_clksrc, ARRAY_SIZE(exynos5420_set_clksrc));
>
> if (soc == EXYNOS5800) {
> - samsung_clk_sleep_init(reg_base, exynos5800_clk_regs,
> + samsung_clk_sleep_init(reg_base, NULL, exynos5800_clk_regs,
> ARRAY_SIZE(exynos5800_clk_regs));
>
> exynos5_subcmus_init(ctx, ARRAY_SIZE(exynos5800_subcmus),
> diff --git a/drivers/clk/samsung/clk.c b/drivers/clk/samsung/clk.c
> index dbc9925ca8f46e951dfb5d391c0e744ca370abcc..07b2948ae7ea48f126ab420be57d8c2705979464 100644
> --- a/drivers/clk/samsung/clk.c
> +++ b/drivers/clk/samsung/clk.c
> @@ -12,8 +12,10 @@
> #include <linux/clkdev.h>
> #include <linux/clk-provider.h>
> #include <linux/io.h>
> +#include <linux/mfd/syscon.h>
> #include <linux/mod_devicetable.h>
> #include <linux/of_address.h>
> +#include <linux/regmap.h>
> #include <linux/syscore_ops.h>
>
> #include "clk.h"
> @@ -21,19 +23,29 @@
> static LIST_HEAD(clock_reg_cache_list);
>
> void samsung_clk_save(void __iomem *base,
> + struct regmap *regmap,
Here it is regmap, in samsung_clk_extended_sleep_init() it is called
sysreg. This suggests that in some cases this replaces the MMIO - like
here in samsung_clk_save().
> struct samsung_clk_reg_dump *rd,
> unsigned int num_regs)
> {
> - for (; num_regs > 0; --num_regs, ++rd)
> - rd->value = readl(base + rd->offset);
> + for (; num_regs > 0; --num_regs, ++rd) {
> + if (base)
> + rd->value = readl(base + rd->offset);
> + if (regmap)
> + regmap_read(regmap, rd->offset, &rd->value);
> + }
> }
>
> void samsung_clk_restore(void __iomem *base,
> + struct regmap *regmap,
> const struct samsung_clk_reg_dump *rd,
> unsigned int num_regs)
> {
> - for (; num_regs > 0; --num_regs, ++rd)
> - writel(rd->value, base + rd->offset);
> + for (; num_regs > 0; --num_regs, ++rd) {
> + if (base)
> + writel(rd->value, base + rd->offset);
> + if (regmap)
> + regmap_write(regmap, rd->offset, rd->value);
> + }
> }
>
> struct samsung_clk_reg_dump *samsung_clk_alloc_reg_dump(
> @@ -227,6 +239,82 @@ void __init samsung_clk_register_div(struct samsung_clk_provider *ctx,
> }
> }
>
> +#define ACG_MSK GENMASK(6, 4)
> +#define CLK_IDLE GENMASK(5, 4)
> +static int samsung_auto_clk_gate_is_en(struct clk_hw *hw)
> +{
> + u32 reg;
> + struct clk_gate *gate = to_clk_gate(hw);
> +
> + reg = readl(gate->reg);
> + return ((reg & ACG_MSK) == CLK_IDLE) ? 0 : 1;
> +}
> +
> +/* enable and disable are nops in automatic clock mode */
> +static int samsung_auto_clk_gate_en(struct clk_hw *hw)
> +{
> + return 0;
> +}
> +
> +static void samsung_auto_clk_gate_dis(struct clk_hw *hw)
> +{
> +}
> +
> +static const struct clk_ops samsung_auto_clk_gate_ops = {
> + .enable = samsung_auto_clk_gate_en,
> + .disable = samsung_auto_clk_gate_dis,
> + .is_enabled = samsung_auto_clk_gate_is_en,
> +};
> +
> +struct clk_hw *samsung_register_auto_gate(struct device *dev,
> + struct device_node *np, const char *name,
> + const char *parent_name, const struct clk_hw *parent_hw,
> + const struct clk_parent_data *parent_data,
> + unsigned long flags,
> + void __iomem *reg, u8 bit_idx,
> + u8 clk_gate_flags, spinlock_t *lock)
> +{
> + struct clk_gate *gate;
> + struct clk_hw *hw;
> + struct clk_init_data init = {};
> + int ret = -EINVAL;
> +
> + /* allocate the gate */
> + gate = kzalloc(sizeof(*gate), GFP_KERNEL);
> + if (!gate)
> + return ERR_PTR(-ENOMEM);
> +
> + init.name = name;
> + init.ops = &samsung_auto_clk_gate_ops;
> + init.flags = flags;
> + init.parent_names = parent_name ? &parent_name : NULL;
> + init.parent_hws = parent_hw ? &parent_hw : NULL;
> + init.parent_data = parent_data;
> + if (parent_name || parent_hw || parent_data)
> + init.num_parents = 1;
> + else
> + init.num_parents = 0;
> +
> + /* struct clk_gate assignments */
> + gate->reg = reg;
> + gate->bit_idx = bit_idx;
> + gate->flags = clk_gate_flags;
> + gate->lock = lock;
> + gate->hw.init = &init;
> +
> + hw = &gate->hw;
> + if (dev || !np)
> + ret = clk_hw_register(dev, hw);
> + else if (np)
> + ret = of_clk_hw_register(np, hw);
> + if (ret) {
> + kfree(gate);
> + hw = ERR_PTR(ret);
> + }
> +
> + return hw;
> +}
> +
> /* register a list of gate clocks */
> void __init samsung_clk_register_gate(struct samsung_clk_provider *ctx,
> const struct samsung_gate_clock *list,
> @@ -234,11 +322,21 @@ void __init samsung_clk_register_gate(struct samsung_clk_provider *ctx,
> {
> struct clk_hw *clk_hw;
> unsigned int idx;
> + void __iomem *reg_offs;
>
> for (idx = 0; idx < nr_clk; idx++, list++) {
> - clk_hw = clk_hw_register_gate(ctx->dev, list->name, list->parent_name,
> - list->flags, ctx->reg_base + list->offset,
> + reg_offs = ctx->reg_base + list->offset;
> +
> + if (ctx->auto_clock_gate && ctx->gate_dbg_offset)
> + clk_hw = samsung_register_auto_gate(ctx->dev, NULL,
> + list->name, list->parent_name, NULL, NULL,
> + list->flags, reg_offs + ctx->gate_dbg_offset,
> list->bit_idx, list->gate_flags, &ctx->lock);
> + else
> + clk_hw = clk_hw_register_gate(ctx->dev, list->name,
> + list->parent_name, list->flags,
> + ctx->reg_base + list->offset, list->bit_idx,
> + list->gate_flags, &ctx->lock);
> if (IS_ERR(clk_hw)) {
> pr_err("%s: failed to register clock %s\n", __func__,
> list->name);
> @@ -276,10 +374,11 @@ static int samsung_clk_suspend(void)
> struct samsung_clock_reg_cache *reg_cache;
>
> list_for_each_entry(reg_cache, &clock_reg_cache_list, node) {
> - samsung_clk_save(reg_cache->reg_base, reg_cache->rdump,
> - reg_cache->rd_num);
> - samsung_clk_restore(reg_cache->reg_base, reg_cache->rsuspend,
> - reg_cache->rsuspend_num);
> + samsung_clk_save(reg_cache->reg_base, reg_cache->sysreg,
> + reg_cache->rdump, reg_cache->rd_num);
> + samsung_clk_restore(reg_cache->reg_base, reg_cache->sysreg,
> + reg_cache->rsuspend,
> + reg_cache->rsuspend_num);
> }
> return 0;
> }
> @@ -289,8 +388,8 @@ static void samsung_clk_resume(void)
> struct samsung_clock_reg_cache *reg_cache;
>
> list_for_each_entry(reg_cache, &clock_reg_cache_list, node)
> - samsung_clk_restore(reg_cache->reg_base, reg_cache->rdump,
> - reg_cache->rd_num);
> + samsung_clk_restore(reg_cache->reg_base, reg_cache->sysreg,
> + reg_cache->rdump, reg_cache->rd_num);
> }
>
> static struct syscore_ops samsung_clk_syscore_ops = {
> @@ -299,6 +398,7 @@ static struct syscore_ops samsung_clk_syscore_ops = {
> };
>
> void samsung_clk_extended_sleep_init(void __iomem *reg_base,
> + struct regmap *sysreg,
> const unsigned long *rdump,
> unsigned long nr_rdump,
> const struct samsung_clk_reg_dump *rsuspend,
> @@ -319,6 +419,7 @@ void samsung_clk_extended_sleep_init(void __iomem *reg_base,
> register_syscore_ops(&samsung_clk_syscore_ops);
>
> reg_cache->reg_base = reg_base;
> + reg_cache->sysreg = sysreg;
> reg_cache->rd_num = nr_rdump;
> reg_cache->rsuspend = rsuspend;
> reg_cache->rsuspend_num = nr_rsuspend;
> @@ -334,6 +435,12 @@ void samsung_clk_extended_sleep_init(void __iomem *reg_base,
> void __init samsung_cmu_register_clocks(struct samsung_clk_provider *ctx,
> const struct samsung_cmu_info *cmu)
> {
> + ctx->auto_clock_gate = cmu->auto_clock_gate;
> + ctx->gate_dbg_offset = cmu->gate_dbg_offset;
> + ctx->option_offset = cmu->option_offset;
> + ctx->drcg_offset = cmu->drcg_offset;
> + ctx->memclk_offset = cmu->memclk_offset;
> +
> if (cmu->pll_clks)
> samsung_clk_register_pll(ctx, cmu->pll_clks, cmu->nr_pll_clks);
> if (cmu->mux_clks)
> @@ -353,6 +460,31 @@ void __init samsung_cmu_register_clocks(struct samsung_clk_provider *ctx,
> samsung_clk_register_cpu(ctx, cmu->cpu_clks, cmu->nr_cpu_clks);
> }
>
> +/* Enable Dynamic Root Clock Gating of bus components*/
> +void samsung_en_dyn_root_clk_gating(struct device_node *np,
> + struct samsung_clk_provider *ctx,
> + const struct samsung_cmu_info *cmu)
> +{
> + if (ctx && !ctx->auto_clock_gate)
Test for ctx should trigger Smatch warning.
> + return;
> +
> + ctx->sysreg = syscon_regmap_lookup_by_phandle(np, "samsung,sysreg");
> + if (!IS_ERR_OR_NULL(ctx->sysreg)) {
Can it return NULL? anyway that's very confusing if() - negated OR_NULL.
Better just
if (IS_ERR()) {
ctx->sysreg = NULL;
return;
}
Best regards,
Krzysztof
Hi Krzysztof,
Thanks for your review feedback!
On Tue, 21 Oct 2025 at 20:45, Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 13/10/2025 22:51, Peter Griffin wrote:
> > Update exynos_arm64_init_clocks() so that it enables the automatic clock
> > mode bits in the CMU option register if the auto_clock_gate flag and
> > option_offset fields are set for the CMU.
> >
> > The CMU option register bits are global and effect every clock component in
> > the CMU, as such clearing the GATE_ENABLE_HWACG bit and setting GATE_MANUAL
> > bit on every gate register is only required if auto_clock_gate is false.
> >
> > Additionally if auto_clock_gate is enabled the dynamic root clock gating
> > and memclk registers will be configured in the corresponding CMUs sysreg
> > bank. These registers are exposed via syscon, so the register
> > suspend/resume paths are updated to handle using a regmap.
> >
> > As many gates for various Samsung SoCs are already exposed in the Samsung
> > clock drivers a new samsung_auto_clk_gate_ops is implemented. This uses
> > some CMU debug registers to report whether clocks are enabled or disabled
> > when operating in automatic mode. This allows
> > /sys/kernel/debug/clk/clk_summary to still dump the entire clock tree and
> > correctly report the status of each clock in the system.
> >
> > Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> > ---
> > drivers/clk/samsung/clk-exynos-arm64.c | 47 +++++++--
> > drivers/clk/samsung/clk-exynos4.c | 6 +-
> > drivers/clk/samsung/clk-exynos4412-isp.c | 4 +-
> > drivers/clk/samsung/clk-exynos5250.c | 2 +-
> > drivers/clk/samsung/clk-exynos5420.c | 4 +-
> > drivers/clk/samsung/clk.c | 161 ++++++++++++++++++++++++++++---
> > drivers/clk/samsung/clk.h | 49 +++++++++-
> > 7 files changed, 244 insertions(+), 29 deletions(-)
> >
> > diff --git a/drivers/clk/samsung/clk-exynos-arm64.c b/drivers/clk/samsung/clk-exynos-arm64.c
> > index bf7de21f329ec89069dcf817ca578fcf9b2d9809..c302c836e8f9f6270753d86d7d986c88e6762f4f 100644
> > --- a/drivers/clk/samsung/clk-exynos-arm64.c
> > +++ b/drivers/clk/samsung/clk-exynos-arm64.c
> > @@ -24,6 +24,16 @@
> > #define GATE_MANUAL BIT(20)
> > #define GATE_ENABLE_HWACG BIT(28)
> >
> > +/* Option register bits */
> > +#define OPT_EN_DBG BIT(31)
> > +#define OPT_UNKNOWN BIT(30)
> > +#define OPT_EN_PWR_MANAGEMENT BIT(29)
> > +#define OPT_EN_AUTO_GATING BIT(28)
> > +#define OPT_EN_MEM_PM_GATING BIT(24)
>
> Please reverse, from lowest to highest number (LSB -> MSB).
>
Will fix
> > +
> > +#define CMU_OPT_GLOBAL_EN_AUTO_GATING (OPT_EN_DBG | OPT_UNKNOWN | \
> > + OPT_EN_PWR_MANAGEMENT | OPT_EN_AUTO_GATING | OPT_EN_MEM_PM_GATING)
> > +
> > /* PLL_CONx_PLL register offsets range */
> > #define PLL_CON_OFF_START 0x100
> > #define PLL_CON_OFF_END 0x600
> > @@ -37,6 +47,8 @@ struct exynos_arm64_cmu_data {
> > unsigned int nr_clk_save;
> > const struct samsung_clk_reg_dump *clk_suspend;
> > unsigned int nr_clk_suspend;
> > + struct samsung_clk_reg_dump *clk_sysreg_save;
> > + unsigned int nr_clk_sysreg;
> >
> > struct clk *clk;
> > struct clk **pclks;
> > @@ -82,13 +94,28 @@ static void __init exynos_arm64_init_clocks(struct device_node *np,
> > if (!reg_base)
> > panic("%s: failed to map registers\n", __func__);
> >
> > + if (cmu->auto_clock_gate && cmu->option_offset) {
> > + /*
> > + * Enable the global automatic mode for the entire CMU.
> > + * This overrides the individual HWACG bits in each of the
> > + * individual gate, mux and qch registers.
> > + */
> > + writel(CMU_OPT_GLOBAL_EN_AUTO_GATING,
> > + reg_base + cmu->option_offset);
> > + }
> > +
> > for (i = 0; i < reg_offs_len; ++i) {
> > void __iomem *reg = reg_base + reg_offs[i];
> > u32 val;
> >
> > if (cmu->manual_plls && is_pll_con1_reg(reg_offs[i])) {
> > writel(PLL_CON1_MANUAL, reg);
> > - } else if (is_gate_reg(reg_offs[i])) {
> > + } else if (is_gate_reg(reg_offs[i]) && !cmu->auto_clock_gate) {
> > + /*
> > + * Setting GATE_MANUAL bit (which is described in TRM as
> > + * reserved!) overrides the global CMU automatic mode
> > + * option.
> > + */
> > val = readl(reg);
> > val |= GATE_MANUAL;
> > val &= ~GATE_ENABLE_HWACG;
> > @@ -219,7 +246,7 @@ void __init exynos_arm64_register_cmu(struct device *dev,
> > * Return: 0 on success, or negative error code on error.
> > */
> > int __init exynos_arm64_register_cmu_pm(struct platform_device *pdev,
> > - bool set_manual)
> > + bool init_clk_regs)
> > {
> > const struct samsung_cmu_info *cmu;
> > struct device *dev = &pdev->dev;
> > @@ -249,7 +276,7 @@ int __init exynos_arm64_register_cmu_pm(struct platform_device *pdev,
> > dev_err(dev, "%s: could not enable bus clock %s; err = %d\n",
> > __func__, cmu->clk_name, ret);
> >
> > - if (set_manual)
> > + if (init_clk_regs)
> > exynos_arm64_init_clocks(np, cmu);
> >
> > reg_base = devm_platform_ioremap_resource(pdev, 0);
> > @@ -280,14 +307,18 @@ int exynos_arm64_cmu_suspend(struct device *dev)
> > struct exynos_arm64_cmu_data *data = dev_get_drvdata(dev);
> > int i;
> >
> > - samsung_clk_save(data->ctx->reg_base, data->clk_save,
> > + samsung_clk_save(data->ctx->reg_base, NULL, data->clk_save,
> > data->nr_clk_save);
> >
> > + if (data->ctx->sysreg)
>
> No need for if() here and in samsung_clk_save.
Will fix.
>
> I find it confusing. You do samsung_clk_save() for regular CMU MMIO
> address and then immediately - for GS101 - do the same for sysreg:
>
> > + samsung_clk_save(NULL, data->ctx->sysreg, data->clk_save,
> > + data->nr_clk_save);
>
> So this feels like you could make one call of samsung_clk_save() with
> both arguments - reg_base and sysreg. It also leads to confusion - first
> read from MMIO will be overwritten by sysreg read.
Apologies for the confusion, that was a typo/bug in v1 it should have
read like this:
samsung_clk_save(data->ctx->reg_base, NULL, data->clk_save,
data->nr_clk_save);
samsung_clk_save(NULL, data->ctx->sysreg, data->clk_sysreg_save,
data->nr_clk_sysreg);
First it saves the CMU MMIO regs, then the second call saves the
sysreg registers (drcg & memclk).
>
>
> > +
> > for (i = 0; i < data->nr_pclks; i++)
> > clk_prepare_enable(data->pclks[i]);
> >
> > /* For suspend some registers have to be set to certain values */
> > - samsung_clk_restore(data->ctx->reg_base, data->clk_suspend,
> > + samsung_clk_restore(data->ctx->reg_base, NULL, data->clk_suspend,
> > data->nr_clk_suspend);
> >
> > for (i = 0; i < data->nr_pclks; i++)
> > @@ -308,9 +339,13 @@ int exynos_arm64_cmu_resume(struct device *dev)
> > for (i = 0; i < data->nr_pclks; i++)
> > clk_prepare_enable(data->pclks[i]);
> >
> > - samsung_clk_restore(data->ctx->reg_base, data->clk_save,
> > + samsung_clk_restore(data->ctx->reg_base, NULL, data->clk_save,
> > data->nr_clk_save);
> >
> > + if (data->ctx->sysreg)
> > + samsung_clk_restore(NULL, data->ctx->sysreg, data->clk_save,
> > + data->nr_clk_save);
> > +
> > for (i = 0; i < data->nr_pclks; i++)
> > clk_disable_unprepare(data->pclks[i]);
> >
> > diff --git a/drivers/clk/samsung/clk-exynos4.c b/drivers/clk/samsung/clk-exynos4.c
> > index cc5c1644c41c08b27bc48d809a08cd8a006cbe8f..26ac9734722d1e7ed8ec3f1c0a956f26e32b92d4 100644
> > --- a/drivers/clk/samsung/clk-exynos4.c
> > +++ b/drivers/clk/samsung/clk-exynos4.c
> > @@ -1378,15 +1378,15 @@ static void __init exynos4_clk_init(struct device_node *np,
> > if (soc == EXYNOS4212 || soc == EXYNOS4412)
> > exynos4x12_core_down_clock();
> >
> > - samsung_clk_extended_sleep_init(reg_base,
> > + samsung_clk_extended_sleep_init(reg_base, NULL,
> > exynos4_clk_regs, ARRAY_SIZE(exynos4_clk_regs),
> > src_mask_suspend, ARRAY_SIZE(src_mask_suspend));
> > if (exynos4_soc == EXYNOS4210)
> > - samsung_clk_extended_sleep_init(reg_base,
> > + samsung_clk_extended_sleep_init(reg_base, NULL,
> > exynos4210_clk_save, ARRAY_SIZE(exynos4210_clk_save),
> > src_mask_suspend_e4210, ARRAY_SIZE(src_mask_suspend_e4210));
> > else
> > - samsung_clk_sleep_init(reg_base, exynos4x12_clk_save,
> > + samsung_clk_sleep_init(reg_base, NULL, exynos4x12_clk_save,
> > ARRAY_SIZE(exynos4x12_clk_save));
> >
> > samsung_clk_of_add_provider(np, ctx);
> > diff --git a/drivers/clk/samsung/clk-exynos4412-isp.c b/drivers/clk/samsung/clk-exynos4412-isp.c
> > index fa915057e109e0008ebe0b1b5d1652fd5804e82b..772bc18a1e686f23b11bf160b803becff6279637 100644
> > --- a/drivers/clk/samsung/clk-exynos4412-isp.c
> > +++ b/drivers/clk/samsung/clk-exynos4412-isp.c
> > @@ -94,7 +94,7 @@ static int __maybe_unused exynos4x12_isp_clk_suspend(struct device *dev)
> > {
> > struct samsung_clk_provider *ctx = dev_get_drvdata(dev);
> >
> > - samsung_clk_save(ctx->reg_base, exynos4x12_save_isp,
> > + samsung_clk_save(ctx->reg_base, NULL, exynos4x12_save_isp,
> > ARRAY_SIZE(exynos4x12_clk_isp_save));
> > return 0;
> > }
> > @@ -103,7 +103,7 @@ static int __maybe_unused exynos4x12_isp_clk_resume(struct device *dev)
> > {
> > struct samsung_clk_provider *ctx = dev_get_drvdata(dev);
> >
> > - samsung_clk_restore(ctx->reg_base, exynos4x12_save_isp,
> > + samsung_clk_restore(ctx->reg_base, NULL, exynos4x12_save_isp,
> > ARRAY_SIZE(exynos4x12_clk_isp_save));
> > return 0;
> > }
> > diff --git a/drivers/clk/samsung/clk-exynos5250.c b/drivers/clk/samsung/clk-exynos5250.c
> > index e90d3a0848cbc24b2709c10795f6affcda404567..f97f30b29be7317db8186bac39cf52e1893eb106 100644
> > --- a/drivers/clk/samsung/clk-exynos5250.c
> > +++ b/drivers/clk/samsung/clk-exynos5250.c
> > @@ -854,7 +854,7 @@ static void __init exynos5250_clk_init(struct device_node *np)
> > PWR_CTRL2_CORE2_UP_RATIO | PWR_CTRL2_CORE1_UP_RATIO);
> > __raw_writel(tmp, reg_base + PWR_CTRL2);
> >
> > - samsung_clk_sleep_init(reg_base, exynos5250_clk_regs,
> > + samsung_clk_sleep_init(reg_base, NULL, exynos5250_clk_regs,
> > ARRAY_SIZE(exynos5250_clk_regs));
> > exynos5_subcmus_init(ctx, ARRAY_SIZE(exynos5250_subcmus),
> > exynos5250_subcmus);
> > diff --git a/drivers/clk/samsung/clk-exynos5420.c b/drivers/clk/samsung/clk-exynos5420.c
> > index a9df4e6db82fa7831d4e5c7210b0163d7d301ec1..1982e0751ceec7e57f9e82d96dcbadce1f691092 100644
> > --- a/drivers/clk/samsung/clk-exynos5420.c
> > +++ b/drivers/clk/samsung/clk-exynos5420.c
> > @@ -1649,12 +1649,12 @@ static void __init exynos5x_clk_init(struct device_node *np,
> > ARRAY_SIZE(exynos5800_cpu_clks));
> > }
> >
> > - samsung_clk_extended_sleep_init(reg_base,
> > + samsung_clk_extended_sleep_init(reg_base, NULL,
> > exynos5x_clk_regs, ARRAY_SIZE(exynos5x_clk_regs),
> > exynos5420_set_clksrc, ARRAY_SIZE(exynos5420_set_clksrc));
> >
> > if (soc == EXYNOS5800) {
> > - samsung_clk_sleep_init(reg_base, exynos5800_clk_regs,
> > + samsung_clk_sleep_init(reg_base, NULL, exynos5800_clk_regs,
> > ARRAY_SIZE(exynos5800_clk_regs));
> >
> > exynos5_subcmus_init(ctx, ARRAY_SIZE(exynos5800_subcmus),
> > diff --git a/drivers/clk/samsung/clk.c b/drivers/clk/samsung/clk.c
> > index dbc9925ca8f46e951dfb5d391c0e744ca370abcc..07b2948ae7ea48f126ab420be57d8c2705979464 100644
> > --- a/drivers/clk/samsung/clk.c
> > +++ b/drivers/clk/samsung/clk.c
> > @@ -12,8 +12,10 @@
> > #include <linux/clkdev.h>
> > #include <linux/clk-provider.h>
> > #include <linux/io.h>
> > +#include <linux/mfd/syscon.h>
> > #include <linux/mod_devicetable.h>
> > #include <linux/of_address.h>
> > +#include <linux/regmap.h>
> > #include <linux/syscore_ops.h>
> >
> > #include "clk.h"
> > @@ -21,19 +23,29 @@
> > static LIST_HEAD(clock_reg_cache_list);
> >
> > void samsung_clk_save(void __iomem *base,
> > + struct regmap *regmap,
>
> Here it is regmap, in samsung_clk_extended_sleep_init() it is called
> sysreg.
I'll update samsung_clk_sleep_init so it is named regmap to be
consistent with the naming. Currently it is only used with the sysreg
regmap, but I suppose in the future there could be other regmaps we
wish to save/restore.
>This suggests that in some cases this replaces the MMIO - like
> here in samsung_clk_save().
Each CMU has MMIO registers and nearly all of them (so far only
cmu_top I've found doesn't) have a couple of clock related registers
in the correspondingly named sysreg. This patch was updating the
samsung_clk_save/restore functions so that as well as saving the MMIO
registers via base, you can alternatively pass it a regmap (instead of
base) and it will save those registers as well. It was obfuscated
unfortunately due to the copy/paste bug mentioned above in the first
version.
Maybe we should just have separate functions instead of overloading
these samsung_clk_save_regmap() and samsung_clk_restore_regmap()
>
>
> > struct samsung_clk_reg_dump *rd,
> > unsigned int num_regs)
> > {
> > - for (; num_regs > 0; --num_regs, ++rd)
> > - rd->value = readl(base + rd->offset);
> > + for (; num_regs > 0; --num_regs, ++rd) {
> > + if (base)
> > + rd->value = readl(base + rd->offset);
> > + if (regmap)
> > + regmap_read(regmap, rd->offset, &rd->value);
> > + }
> > }
> >
> > void samsung_clk_restore(void __iomem *base,
> > + struct regmap *regmap,
> > const struct samsung_clk_reg_dump *rd,
> > unsigned int num_regs)
> > {
> > - for (; num_regs > 0; --num_regs, ++rd)
> > - writel(rd->value, base + rd->offset);
> > + for (; num_regs > 0; --num_regs, ++rd) {
> > + if (base)
> > + writel(rd->value, base + rd->offset);
> > + if (regmap)
> > + regmap_write(regmap, rd->offset, rd->value);
> > + }
> > }
> >
> > struct samsung_clk_reg_dump *samsung_clk_alloc_reg_dump(
> > @@ -227,6 +239,82 @@ void __init samsung_clk_register_div(struct samsung_clk_provider *ctx,
> > }
> > }
> >
> > +#define ACG_MSK GENMASK(6, 4)
> > +#define CLK_IDLE GENMASK(5, 4)
> > +static int samsung_auto_clk_gate_is_en(struct clk_hw *hw)
> > +{
> > + u32 reg;
> > + struct clk_gate *gate = to_clk_gate(hw);
> > +
> > + reg = readl(gate->reg);
> > + return ((reg & ACG_MSK) == CLK_IDLE) ? 0 : 1;
> > +}
> > +
> > +/* enable and disable are nops in automatic clock mode */
> > +static int samsung_auto_clk_gate_en(struct clk_hw *hw)
> > +{
> > + return 0;
> > +}
> > +
> > +static void samsung_auto_clk_gate_dis(struct clk_hw *hw)
> > +{
> > +}
> > +
> > +static const struct clk_ops samsung_auto_clk_gate_ops = {
> > + .enable = samsung_auto_clk_gate_en,
> > + .disable = samsung_auto_clk_gate_dis,
> > + .is_enabled = samsung_auto_clk_gate_is_en,
> > +};
> > +
> > +struct clk_hw *samsung_register_auto_gate(struct device *dev,
> > + struct device_node *np, const char *name,
> > + const char *parent_name, const struct clk_hw *parent_hw,
> > + const struct clk_parent_data *parent_data,
> > + unsigned long flags,
> > + void __iomem *reg, u8 bit_idx,
> > + u8 clk_gate_flags, spinlock_t *lock)
> > +{
> > + struct clk_gate *gate;
> > + struct clk_hw *hw;
> > + struct clk_init_data init = {};
> > + int ret = -EINVAL;
> > +
> > + /* allocate the gate */
> > + gate = kzalloc(sizeof(*gate), GFP_KERNEL);
> > + if (!gate)
> > + return ERR_PTR(-ENOMEM);
> > +
> > + init.name = name;
> > + init.ops = &samsung_auto_clk_gate_ops;
> > + init.flags = flags;
> > + init.parent_names = parent_name ? &parent_name : NULL;
> > + init.parent_hws = parent_hw ? &parent_hw : NULL;
> > + init.parent_data = parent_data;
> > + if (parent_name || parent_hw || parent_data)
> > + init.num_parents = 1;
> > + else
> > + init.num_parents = 0;
> > +
> > + /* struct clk_gate assignments */
> > + gate->reg = reg;
> > + gate->bit_idx = bit_idx;
> > + gate->flags = clk_gate_flags;
> > + gate->lock = lock;
> > + gate->hw.init = &init;
> > +
> > + hw = &gate->hw;
> > + if (dev || !np)
> > + ret = clk_hw_register(dev, hw);
> > + else if (np)
> > + ret = of_clk_hw_register(np, hw);
> > + if (ret) {
> > + kfree(gate);
> > + hw = ERR_PTR(ret);
> > + }
> > +
> > + return hw;
> > +}
> > +
> > /* register a list of gate clocks */
> > void __init samsung_clk_register_gate(struct samsung_clk_provider *ctx,
> > const struct samsung_gate_clock *list,
> > @@ -234,11 +322,21 @@ void __init samsung_clk_register_gate(struct samsung_clk_provider *ctx,
> > {
> > struct clk_hw *clk_hw;
> > unsigned int idx;
> > + void __iomem *reg_offs;
> >
> > for (idx = 0; idx < nr_clk; idx++, list++) {
> > - clk_hw = clk_hw_register_gate(ctx->dev, list->name, list->parent_name,
> > - list->flags, ctx->reg_base + list->offset,
> > + reg_offs = ctx->reg_base + list->offset;
> > +
> > + if (ctx->auto_clock_gate && ctx->gate_dbg_offset)
> > + clk_hw = samsung_register_auto_gate(ctx->dev, NULL,
> > + list->name, list->parent_name, NULL, NULL,
> > + list->flags, reg_offs + ctx->gate_dbg_offset,
> > list->bit_idx, list->gate_flags, &ctx->lock);
> > + else
> > + clk_hw = clk_hw_register_gate(ctx->dev, list->name,
> > + list->parent_name, list->flags,
> > + ctx->reg_base + list->offset, list->bit_idx,
> > + list->gate_flags, &ctx->lock);
> > if (IS_ERR(clk_hw)) {
> > pr_err("%s: failed to register clock %s\n", __func__,
> > list->name);
> > @@ -276,10 +374,11 @@ static int samsung_clk_suspend(void)
> > struct samsung_clock_reg_cache *reg_cache;
> >
> > list_for_each_entry(reg_cache, &clock_reg_cache_list, node) {
> > - samsung_clk_save(reg_cache->reg_base, reg_cache->rdump,
> > - reg_cache->rd_num);
> > - samsung_clk_restore(reg_cache->reg_base, reg_cache->rsuspend,
> > - reg_cache->rsuspend_num);
> > + samsung_clk_save(reg_cache->reg_base, reg_cache->sysreg,
> > + reg_cache->rdump, reg_cache->rd_num);
> > + samsung_clk_restore(reg_cache->reg_base, reg_cache->sysreg,
> > + reg_cache->rsuspend,
> > + reg_cache->rsuspend_num);
> > }
> > return 0;
> > }
> > @@ -289,8 +388,8 @@ static void samsung_clk_resume(void)
> > struct samsung_clock_reg_cache *reg_cache;
> >
> > list_for_each_entry(reg_cache, &clock_reg_cache_list, node)
> > - samsung_clk_restore(reg_cache->reg_base, reg_cache->rdump,
> > - reg_cache->rd_num);
> > + samsung_clk_restore(reg_cache->reg_base, reg_cache->sysreg,
> > + reg_cache->rdump, reg_cache->rd_num);
> > }
> >
> > static struct syscore_ops samsung_clk_syscore_ops = {
> > @@ -299,6 +398,7 @@ static struct syscore_ops samsung_clk_syscore_ops = {
> > };
> >
> > void samsung_clk_extended_sleep_init(void __iomem *reg_base,
> > + struct regmap *sysreg,
> > const unsigned long *rdump,
> > unsigned long nr_rdump,
> > const struct samsung_clk_reg_dump *rsuspend,
> > @@ -319,6 +419,7 @@ void samsung_clk_extended_sleep_init(void __iomem *reg_base,
> > register_syscore_ops(&samsung_clk_syscore_ops);
> >
> > reg_cache->reg_base = reg_base;
> > + reg_cache->sysreg = sysreg;
> > reg_cache->rd_num = nr_rdump;
> > reg_cache->rsuspend = rsuspend;
> > reg_cache->rsuspend_num = nr_rsuspend;
> > @@ -334,6 +435,12 @@ void samsung_clk_extended_sleep_init(void __iomem *reg_base,
> > void __init samsung_cmu_register_clocks(struct samsung_clk_provider *ctx,
> > const struct samsung_cmu_info *cmu)
> > {
> > + ctx->auto_clock_gate = cmu->auto_clock_gate;
> > + ctx->gate_dbg_offset = cmu->gate_dbg_offset;
> > + ctx->option_offset = cmu->option_offset;
> > + ctx->drcg_offset = cmu->drcg_offset;
> > + ctx->memclk_offset = cmu->memclk_offset;
> > +
> > if (cmu->pll_clks)
> > samsung_clk_register_pll(ctx, cmu->pll_clks, cmu->nr_pll_clks);
> > if (cmu->mux_clks)
> > @@ -353,6 +460,31 @@ void __init samsung_cmu_register_clocks(struct samsung_clk_provider *ctx,
> > samsung_clk_register_cpu(ctx, cmu->cpu_clks, cmu->nr_cpu_clks);
> > }
> >
> > +/* Enable Dynamic Root Clock Gating of bus components*/
> > +void samsung_en_dyn_root_clk_gating(struct device_node *np,
> > + struct samsung_clk_provider *ctx,
> > + const struct samsung_cmu_info *cmu)
> > +{
> > + if (ctx && !ctx->auto_clock_gate)
>
> Test for ctx should trigger Smatch warning.
Will remove the ctx test.
>
> > + return;
> > +
> > + ctx->sysreg = syscon_regmap_lookup_by_phandle(np, "samsung,sysreg");
> > + if (!IS_ERR_OR_NULL(ctx->sysreg)) {
>
> Can it return NULL? anyway that's very confusing if() - negated OR_NULL.
> Better just
> if (IS_ERR()) {
> ctx->sysreg = NULL;
> return;
> }
Will update like you suggest in v2. Thanks for the review feedback.
regards,
Peter
Hi Peter,
On Mon, 2025-10-13 at 21:51 +0100, Peter Griffin wrote:
> Update exynos_arm64_init_clocks() so that it enables the automatic clock
> mode bits in the CMU option register if the auto_clock_gate flag and
> option_offset fields are set for the CMU.
>
> The CMU option register bits are global and effect every clock component in
> the CMU, as such clearing the GATE_ENABLE_HWACG bit and setting GATE_MANUAL
> bit on every gate register is only required if auto_clock_gate is false.
>
> Additionally if auto_clock_gate is enabled the dynamic root clock gating
> and memclk registers will be configured in the corresponding CMUs sysreg
> bank. These registers are exposed via syscon, so the register
> suspend/resume paths are updated to handle using a regmap.
>
> As many gates for various Samsung SoCs are already exposed in the Samsung
> clock drivers a new samsung_auto_clk_gate_ops is implemented. This uses
> some CMU debug registers to report whether clocks are enabled or disabled
> when operating in automatic mode. This allows
> /sys/kernel/debug/clk/clk_summary to still dump the entire clock tree and
> correctly report the status of each clock in the system.
>
> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
It's great to see some effort on this topic as this makes the clock handling
on new Exynos much easier and less error prone and time-consuming, and having
the hardware decide which clocks need to be enabled at a given point in time
allows for a significantly more dynamic environment.
Just some initial comments below.
> ---
> drivers/clk/samsung/clk-exynos-arm64.c | 47 +++++++--
> drivers/clk/samsung/clk-exynos4.c | 6 +-
> drivers/clk/samsung/clk-exynos4412-isp.c | 4 +-
> drivers/clk/samsung/clk-exynos5250.c | 2 +-
> drivers/clk/samsung/clk-exynos5420.c | 4 +-
> drivers/clk/samsung/clk.c | 161 ++++++++++++++++++++++++++++---
> drivers/clk/samsung/clk.h | 49 +++++++++-
> 7 files changed, 244 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/clk/samsung/clk-exynos-arm64.c b/drivers/clk/samsung/clk-exynos-arm64.c
> index bf7de21f329ec89069dcf817ca578fcf9b2d9809..c302c836e8f9f6270753d86d7d986c88e6762f4f 100644
> --- a/drivers/clk/samsung/clk-exynos-arm64.c
> +++ b/drivers/clk/samsung/clk-exynos-arm64.c
> @@ -24,6 +24,16 @@
> #define GATE_MANUAL BIT(20)
> #define GATE_ENABLE_HWACG BIT(28)
>
> +/* Option register bits */
> +#define OPT_EN_DBG BIT(31)
> +#define OPT_UNKNOWN BIT(30)
Except for AOC, bit 30 is called ENABLE_LAYER2_CTRL in all gs101 CMUs.
> +#define OPT_EN_PWR_MANAGEMENT BIT(29)
> +#define OPT_EN_AUTO_GATING BIT(28)
> +#define OPT_EN_MEM_PM_GATING BIT(24)
> +
> +#define CMU_OPT_GLOBAL_EN_AUTO_GATING (OPT_EN_DBG | OPT_UNKNOWN | \
> + OPT_EN_PWR_MANAGEMENT | OPT_EN_AUTO_GATING | OPT_EN_MEM_PM_GATING)
> +
> /* PLL_CONx_PLL register offsets range */
> #define PLL_CON_OFF_START 0x100
> #define PLL_CON_OFF_END 0x600
> @@ -37,6 +47,8 @@ struct exynos_arm64_cmu_data {
> unsigned int nr_clk_save;
> const struct samsung_clk_reg_dump *clk_suspend;
> unsigned int nr_clk_suspend;
> + struct samsung_clk_reg_dump *clk_sysreg_save;
> + unsigned int nr_clk_sysreg;
>
> struct clk *clk;
> struct clk **pclks;
> @@ -82,13 +94,28 @@ static void __init exynos_arm64_init_clocks(struct device_node *np,
> if (!reg_base)
> panic("%s: failed to map registers\n", __func__);
>
> + if (cmu->auto_clock_gate && cmu->option_offset) {
> + /*
> + * Enable the global automatic mode for the entire CMU.
> + * This overrides the individual HWACG bits in each of the
> + * individual gate, mux and qch registers.
> + */
> + writel(CMU_OPT_GLOBAL_EN_AUTO_GATING,
> + reg_base + cmu->option_offset);
> + }
> +
> for (i = 0; i < reg_offs_len; ++i) {
> void __iomem *reg = reg_base + reg_offs[i];
> u32 val;
>
> if (cmu->manual_plls && is_pll_con1_reg(reg_offs[i])) {
> writel(PLL_CON1_MANUAL, reg);
> - } else if (is_gate_reg(reg_offs[i])) {
> + } else if (is_gate_reg(reg_offs[i]) && !cmu->auto_clock_gate) {
> + /*
> + * Setting GATE_MANUAL bit (which is described in TRM as
> + * reserved!) overrides the global CMU automatic mode
> + * option.
> + */
> val = readl(reg);
> val |= GATE_MANUAL;
> val &= ~GATE_ENABLE_HWACG;
> @@ -219,7 +246,7 @@ void __init exynos_arm64_register_cmu(struct device *dev,
> * Return: 0 on success, or negative error code on error.
> */
> int __init exynos_arm64_register_cmu_pm(struct platform_device *pdev,
> - bool set_manual)
> + bool init_clk_regs)
kerneldoc needs updating
> {
> const struct samsung_cmu_info *cmu;
> struct device *dev = &pdev->dev;
> @@ -249,7 +276,7 @@ int __init exynos_arm64_register_cmu_pm(struct platform_device *pdev,
> dev_err(dev, "%s: could not enable bus clock %s; err = %d\n",
> __func__, cmu->clk_name, ret);
>
> - if (set_manual)
> + if (init_clk_regs)
> exynos_arm64_init_clocks(np, cmu);
>
> reg_base = devm_platform_ioremap_resource(pdev, 0);
> @@ -280,14 +307,18 @@ int exynos_arm64_cmu_suspend(struct device *dev)
> struct exynos_arm64_cmu_data *data = dev_get_drvdata(dev);
> int i;
>
> - samsung_clk_save(data->ctx->reg_base, data->clk_save,
> + samsung_clk_save(data->ctx->reg_base, NULL, data->clk_save,
> data->nr_clk_save);
>
> + if (data->ctx->sysreg)
> + samsung_clk_save(NULL, data->ctx->sysreg, data->clk_save,
> + data->nr_clk_save);
> +
I think this should be
+ samsung_clk_save(NULL, data->ctx->sysreg, data->clk_sysreg_save,
+ data->nr_clk_sysreg);
?
> for (i = 0; i < data->nr_pclks; i++)
> clk_prepare_enable(data->pclks[i]);
>
> /* For suspend some registers have to be set to certain values */
> - samsung_clk_restore(data->ctx->reg_base, data->clk_suspend,
> + samsung_clk_restore(data->ctx->reg_base, NULL, data->clk_suspend,
> data->nr_clk_suspend);
>
> for (i = 0; i < data->nr_pclks; i++)
> @@ -308,9 +339,13 @@ int exynos_arm64_cmu_resume(struct device *dev)
> for (i = 0; i < data->nr_pclks; i++)
> clk_prepare_enable(data->pclks[i]);
>
> - samsung_clk_restore(data->ctx->reg_base, data->clk_save,
> + samsung_clk_restore(data->ctx->reg_base, NULL, data->clk_save,
> data->nr_clk_save);
>
> + if (data->ctx->sysreg)
> + samsung_clk_restore(NULL, data->ctx->sysreg, data->clk_save,
> + data->nr_clk_save);
> +
dito.
> for (i = 0; i < data->nr_pclks; i++)
> clk_disable_unprepare(data->pclks[i]);
>
[...]
> diff --git a/drivers/clk/samsung/clk.c b/drivers/clk/samsung/clk.c
> index dbc9925ca8f46e951dfb5d391c0e744ca370abcc..07b2948ae7ea48f126ab420be57d8c2705979464 100644
> --- a/drivers/clk/samsung/clk.c
> +++ b/drivers/clk/samsung/clk.c
> @@ -12,8 +12,10 @@
> #include <linux/clkdev.h>
> #include <linux/clk-provider.h>
> #include <linux/io.h>
> +#include <linux/mfd/syscon.h>
> #include <linux/mod_devicetable.h>
> #include <linux/of_address.h>
> +#include <linux/regmap.h>
> #include <linux/syscore_ops.h>
>
> #include "clk.h"
> @@ -21,19 +23,29 @@
> static LIST_HEAD(clock_reg_cache_list);
>
> void samsung_clk_save(void __iomem *base,
> + struct regmap *regmap,
> struct samsung_clk_reg_dump *rd,
> unsigned int num_regs)
> {
> - for (; num_regs > 0; --num_regs, ++rd)
> - rd->value = readl(base + rd->offset);
> + for (; num_regs > 0; --num_regs, ++rd) {
> + if (base)
> + rd->value = readl(base + rd->offset);
> + if (regmap)
Should this be else if?
> + regmap_read(regmap, rd->offset, &rd->value);
Otherwise users that (incorrectly) pass both base and regmap would
cause this to end up reading the offset from the wrong memory
region.
At least this way that won't happen, but maybe this constrains should
be made even more explicit.
> + }
> }
>
> void samsung_clk_restore(void __iomem *base,
> + struct regmap *regmap,
> const struct samsung_clk_reg_dump *rd,
> unsigned int num_regs)
> {
> - for (; num_regs > 0; --num_regs, ++rd)
> - writel(rd->value, base + rd->offset);
> + for (; num_regs > 0; --num_regs, ++rd) {
> + if (base)
> + writel(rd->value, base + rd->offset);
> + if (regmap)
> + regmap_write(regmap, rd->offset, rd->value);
dito.
> + }
> }
>
> struct samsung_clk_reg_dump *samsung_clk_alloc_reg_dump(
> @@ -227,6 +239,82 @@ void __init samsung_clk_register_div(struct samsung_clk_provider *ctx,
> }
> }
>
> +#define ACG_MSK GENMASK(6, 4)
> +#define CLK_IDLE GENMASK(5, 4)
> +static int samsung_auto_clk_gate_is_en(struct clk_hw *hw)
> +{
> + u32 reg;
> + struct clk_gate *gate = to_clk_gate(hw);
> +
> + reg = readl(gate->reg);
> + return ((reg & ACG_MSK) == CLK_IDLE) ? 0 : 1;
> +}
> +
> +/* enable and disable are nops in automatic clock mode */
> +static int samsung_auto_clk_gate_en(struct clk_hw *hw)
> +{
> + return 0;
> +}
> +
> +static void samsung_auto_clk_gate_dis(struct clk_hw *hw)
> +{
> +}
> +
> +static const struct clk_ops samsung_auto_clk_gate_ops = {
> + .enable = samsung_auto_clk_gate_en,
> + .disable = samsung_auto_clk_gate_dis,
> + .is_enabled = samsung_auto_clk_gate_is_en,
> +};
> +
> +struct clk_hw *samsung_register_auto_gate(struct device *dev,
> + struct device_node *np, const char *name,
> + const char *parent_name, const struct clk_hw *parent_hw,
> + const struct clk_parent_data *parent_data,
> + unsigned long flags,
> + void __iomem *reg, u8 bit_idx,
> + u8 clk_gate_flags, spinlock_t *lock)
> +{
> + struct clk_gate *gate;
> + struct clk_hw *hw;
> + struct clk_init_data init = {};
> + int ret = -EINVAL;
> +
> + /* allocate the gate */
> + gate = kzalloc(sizeof(*gate), GFP_KERNEL);
> + if (!gate)
> + return ERR_PTR(-ENOMEM);
> +
> + init.name = name;
> + init.ops = &samsung_auto_clk_gate_ops;
> + init.flags = flags;
> + init.parent_names = parent_name ? &parent_name : NULL;
> + init.parent_hws = parent_hw ? &parent_hw : NULL;
> + init.parent_data = parent_data;
> + if (parent_name || parent_hw || parent_data)
> + init.num_parents = 1;
> + else
> + init.num_parents = 0;
> +
> + /* struct clk_gate assignments */
> + gate->reg = reg;
> + gate->bit_idx = bit_idx;
> + gate->flags = clk_gate_flags;
> + gate->lock = lock;
> + gate->hw.init = &init;
> +
> + hw = &gate->hw;
> + if (dev || !np)
> + ret = clk_hw_register(dev, hw);
> + else if (np)
> + ret = of_clk_hw_register(np, hw);
> + if (ret) {
> + kfree(gate);
> + hw = ERR_PTR(ret);
> + }
> +
> + return hw;
> +}
> +
> /* register a list of gate clocks */
> void __init samsung_clk_register_gate(struct samsung_clk_provider *ctx,
> const struct samsung_gate_clock *list,
> @@ -234,11 +322,21 @@ void __init samsung_clk_register_gate(struct samsung_clk_provider *ctx,
> {
> struct clk_hw *clk_hw;
> unsigned int idx;
> + void __iomem *reg_offs;
>
> for (idx = 0; idx < nr_clk; idx++, list++) {
> - clk_hw = clk_hw_register_gate(ctx->dev, list->name, list->parent_name,
> - list->flags, ctx->reg_base + list->offset,
> + reg_offs = ctx->reg_base + list->offset;
> +
> + if (ctx->auto_clock_gate && ctx->gate_dbg_offset)
> + clk_hw = samsung_register_auto_gate(ctx->dev, NULL,
> + list->name, list->parent_name, NULL, NULL,
> + list->flags, reg_offs + ctx->gate_dbg_offset,
> list->bit_idx, list->gate_flags, &ctx->lock);
> + else
> + clk_hw = clk_hw_register_gate(ctx->dev, list->name,
> + list->parent_name, list->flags,
> + ctx->reg_base + list->offset, list->bit_idx,
> + list->gate_flags, &ctx->lock);
> if (IS_ERR(clk_hw)) {
> pr_err("%s: failed to register clock %s\n", __func__,
> list->name);
Maybe the error message should include the actual error clk_hw here?
> @@ -276,10 +374,11 @@ static int samsung_clk_suspend(void)
> struct samsung_clock_reg_cache *reg_cache;
>
> list_for_each_entry(reg_cache, &clock_reg_cache_list, node) {
> - samsung_clk_save(reg_cache->reg_base, reg_cache->rdump,
> - reg_cache->rd_num);
> - samsung_clk_restore(reg_cache->reg_base, reg_cache->rsuspend,
> - reg_cache->rsuspend_num);
> + samsung_clk_save(reg_cache->reg_base, reg_cache->sysreg,
> + reg_cache->rdump, reg_cache->rd_num);
> + samsung_clk_restore(reg_cache->reg_base, reg_cache->sysreg,
> + reg_cache->rsuspend,
> + reg_cache->rsuspend_num);
> }
> return 0;
> }
> @@ -289,8 +388,8 @@ static void samsung_clk_resume(void)
> struct samsung_clock_reg_cache *reg_cache;
>
> list_for_each_entry(reg_cache, &clock_reg_cache_list, node)
> - samsung_clk_restore(reg_cache->reg_base, reg_cache->rdump,
> - reg_cache->rd_num);
> + samsung_clk_restore(reg_cache->reg_base, reg_cache->sysreg,
> + reg_cache->rdump, reg_cache->rd_num);
> }
>
> static struct syscore_ops samsung_clk_syscore_ops = {
> @@ -299,6 +398,7 @@ static struct syscore_ops samsung_clk_syscore_ops = {
> };
>
> void samsung_clk_extended_sleep_init(void __iomem *reg_base,
> + struct regmap *sysreg,
> const unsigned long *rdump,
> unsigned long nr_rdump,
> const struct samsung_clk_reg_dump *rsuspend,
> @@ -319,6 +419,7 @@ void samsung_clk_extended_sleep_init(void __iomem *reg_base,
> register_syscore_ops(&samsung_clk_syscore_ops);
>
> reg_cache->reg_base = reg_base;
> + reg_cache->sysreg = sysreg;
> reg_cache->rd_num = nr_rdump;
> reg_cache->rsuspend = rsuspend;
> reg_cache->rsuspend_num = nr_rsuspend;
> @@ -334,6 +435,12 @@ void samsung_clk_extended_sleep_init(void __iomem *reg_base,
> void __init samsung_cmu_register_clocks(struct samsung_clk_provider *ctx,
> const struct samsung_cmu_info *cmu)
> {
> + ctx->auto_clock_gate = cmu->auto_clock_gate;
> + ctx->gate_dbg_offset = cmu->gate_dbg_offset;
> + ctx->option_offset = cmu->option_offset;
> + ctx->drcg_offset = cmu->drcg_offset;
> + ctx->memclk_offset = cmu->memclk_offset;
> +
> if (cmu->pll_clks)
> samsung_clk_register_pll(ctx, cmu->pll_clks, cmu->nr_pll_clks);
> if (cmu->mux_clks)
> @@ -353,6 +460,31 @@ void __init samsung_cmu_register_clocks(struct samsung_clk_provider *ctx,
> samsung_clk_register_cpu(ctx, cmu->cpu_clks, cmu->nr_cpu_clks);
> }
>
> +/* Enable Dynamic Root Clock Gating of bus components*/
missing space before */
> +void samsung_en_dyn_root_clk_gating(struct device_node *np,
> + struct samsung_clk_provider *ctx,
> + const struct samsung_cmu_info *cmu)
> +{
> + if (ctx && !ctx->auto_clock_gate)
ctx can not be NULL here. If it could, then the following lines would also
need updating, but again, it can not be NULL, if ctx allocation fails, the
code will panic() before this here is reached.
> + return;
> +
> + ctx->sysreg = syscon_regmap_lookup_by_phandle(np, "samsung,sysreg");
> + if (!IS_ERR_OR_NULL(ctx->sysreg)) {
> + regmap_write(ctx->sysreg, ctx->drcg_offset, 0xffffffff);
What does 0xffffffff mean? Maybe a macro or at least a comment would help.
> + /* not every sysreg controller has memclk reg*/
missing space before */
Can you please check all such comments, since it seems to be recurring :-)
+ if (ctx->memclk_offset)
> + regmap_write_bits(ctx->sysreg, ctx->memclk_offset, 0x1, 0x0);
> +
> + samsung_clk_extended_sleep_init(NULL, ctx->sysreg,
> + cmu->sysreg_clk_regs,
> + cmu->nr_sysreg_clk_regs,
> + NULL, 0);
> + } else {
> + pr_warn("%pOF: Unable to get CMU sysreg\n", np);
> + ctx->sysreg = NULL;
> + }
> +}
> +
> /*
> * Common function which registers plls, muxes, dividers and gates
> * for each CMU. It also add CMU register list to register cache.
> @@ -374,11 +506,14 @@ struct samsung_clk_provider * __init samsung_cmu_register_one(
> samsung_cmu_register_clocks(ctx, cmu);
>
> if (cmu->clk_regs)
> - samsung_clk_extended_sleep_init(reg_base,
> + samsung_clk_extended_sleep_init(reg_base, NULL,
> cmu->clk_regs, cmu->nr_clk_regs,
> cmu->suspend_regs, cmu->nr_suspend_regs);
>
> samsung_clk_of_add_provider(np, ctx);
>
> + /* sysreg DT nodes reference a clock in this CMU */
> + samsung_en_dyn_root_clk_gating(np, ctx, cmu);
> +
samsung_cmu_register_one() is not called when the clocks are registered
using exynos_arm64_register_cmu_pm(). Does the pm version need a call to
samsung_en_dyn_root_clk_gating() somewhere?
> return ctx;
> }
> diff --git a/drivers/clk/samsung/clk.h b/drivers/clk/samsung/clk.h
> index 18660c1ac6f0106b17b9efc9c6b3cd62d46f7b82..b719e057f45489e9d92ba54031fe633a8c9264ce 100644
> --- a/drivers/clk/samsung/clk.h
> +++ b/drivers/clk/samsung/clk.h
> @@ -12,6 +12,7 @@
>
> #include <linux/clk-provider.h>
> #include <linux/mod_devicetable.h>
> +#include <linux/regmap.h>
> #include "clk-pll.h"
> #include "clk-cpu.h"
>
> @@ -19,13 +20,25 @@
> * struct samsung_clk_provider - information about clock provider
> * @reg_base: virtual address for the register base
> * @dev: clock provider device needed for runtime PM
> + * @sysreg: syscon regmap for clock-provider sysreg controller
> * @lock: maintains exclusion between callbacks for a given clock-provider
> + * @auto_clock_gate: enable auto clk mode for all clocks in clock-provider
> + * @gate_dbg_offset: gate debug reg offset. Used for all gates in auto clk mode
> + * @option_offset: option reg offset. Enables auto mode for clock-provider
> + * @drcg_offset: dynamic root clk gate enable register offset
> + * @memclk_offset: memclk enable register offset
The last two are in the sysreg space, maybe the comment should mention that.
> * @clk_data: holds clock related data like clk_hw* and number of clocks
> */
> struct samsung_clk_provider {
> void __iomem *reg_base;
> struct device *dev;
> + struct regmap *sysreg;
> spinlock_t lock;
> + bool auto_clock_gate;
> + u32 gate_dbg_offset;
> + u32 option_offset;
> + u32 drcg_offset;
> + u32 memclk_offset;
> /* clk_data must be the last entry due to variable length 'hws' array */
> struct clk_hw_onecell_data clk_data;
> };
> @@ -310,6 +323,7 @@ struct samsung_cpu_clock {
> struct samsung_clock_reg_cache {
> struct list_head node;
> void __iomem *reg_base;
> + struct regmap *sysreg;
> struct samsung_clk_reg_dump *rdump;
> unsigned int rd_num;
> const struct samsung_clk_reg_dump *rsuspend;
> @@ -338,7 +352,14 @@ struct samsung_clock_reg_cache {
> * @suspend_regs: list of clock registers to set before suspend
> * @nr_suspend_regs: count of clock registers in @suspend_regs
> * @clk_name: name of the parent clock needed for CMU register access
> + * @sysreg_clk_regs: list of sysreg clock registers
> + * @nr_sysreg_clk_regs: count of clock registers in @sysreg_clk_regs
> * @manual_plls: Enable manual control for PLL clocks
> + * @auto_clock_gate: enable auto clock mode for all components in CMU
> + * @gate_dbg_offset: gate debug reg offset. Used by all gates in auto clk mode
> + * @option_offset: option reg offset. Enables auto clk mode for entire CMU
> + * @drcg_offset: dynamic root clk gate enable register offset
> + * @memclk_offset: memclk enable register offset
dito.
Cheers,
Andre'
> */
> struct samsung_cmu_info {
> const struct samsung_pll_clock *pll_clks;
> @@ -364,8 +385,16 @@ struct samsung_cmu_info {
> unsigned int nr_suspend_regs;
> const char *clk_name;
>
> + const unsigned long *sysreg_clk_regs;
> + unsigned int nr_sysreg_clk_regs;
> +
> /* ARM64 Exynos CMUs */
> bool manual_plls;
> + bool auto_clock_gate;
> + u32 gate_dbg_offset;
> + u32 option_offset;
> + u32 drcg_offset;
> + u32 memclk_offset;
> };
>
> struct samsung_clk_provider *samsung_clk_init(struct device *dev,
> @@ -415,6 +444,7 @@ struct samsung_clk_provider *samsung_cmu_register_one(
>
> #ifdef CONFIG_PM_SLEEP
> void samsung_clk_extended_sleep_init(void __iomem *reg_base,
> + struct regmap *sysreg,
> const unsigned long *rdump,
> unsigned long nr_rdump,
> const struct samsung_clk_reg_dump *rsuspend,
> @@ -426,17 +456,32 @@ static inline void samsung_clk_extended_sleep_init(void __iomem *reg_base,
> const struct samsung_clk_reg_dump *rsuspend,
> unsigned long nr_rsuspend) {}
> #endif
> -#define samsung_clk_sleep_init(reg_base, rdump, nr_rdump) \
> - samsung_clk_extended_sleep_init(reg_base, rdump, nr_rdump, NULL, 0)
> +#define samsung_clk_sleep_init(reg_base, sysreg, rdump, nr_rdump) \
> + samsung_clk_extended_sleep_init(reg_base, sysreg, rdump, nr_rdump, \
> + NULL, 0)
>
> void samsung_clk_save(void __iomem *base,
> + struct regmap *regmap,
> struct samsung_clk_reg_dump *rd,
> unsigned int num_regs);
> void samsung_clk_restore(void __iomem *base,
> + struct regmap *regmap,
> const struct samsung_clk_reg_dump *rd,
> unsigned int num_regs);
> struct samsung_clk_reg_dump *samsung_clk_alloc_reg_dump(
> const unsigned long *rdump,
> unsigned long nr_rdump);
>
> +void samsung_en_dyn_root_clk_gating(struct device_node *np,
> + struct samsung_clk_provider *ctx,
> + const struct samsung_cmu_info *cmu);
> +
> +struct clk_hw *samsung_register_auto_gate(struct device *dev,
> + struct device_node *np, const char *name,
> + const char *parent_name, const struct clk_hw *parent_hw,
> + const struct clk_parent_data *parent_data,
> + unsigned long flags,
> + void __iomem *reg, u8 bit_idx,
> + u8 clk_gate_flags, spinlock_t *lock);
> +
> #endif /* __SAMSUNG_CLK_H */
Hi André,
Thanks for the review feedback!
On Thu, 16 Oct 2025 at 10:40, André Draszik <andre.draszik@linaro.org> wrote:
>
> Hi Peter,
>
> On Mon, 2025-10-13 at 21:51 +0100, Peter Griffin wrote:
> > Update exynos_arm64_init_clocks() so that it enables the automatic clock
> > mode bits in the CMU option register if the auto_clock_gate flag and
> > option_offset fields are set for the CMU.
> >
> > The CMU option register bits are global and effect every clock component in
> > the CMU, as such clearing the GATE_ENABLE_HWACG bit and setting GATE_MANUAL
> > bit on every gate register is only required if auto_clock_gate is false.
> >
> > Additionally if auto_clock_gate is enabled the dynamic root clock gating
> > and memclk registers will be configured in the corresponding CMUs sysreg
> > bank. These registers are exposed via syscon, so the register
> > suspend/resume paths are updated to handle using a regmap.
> >
> > As many gates for various Samsung SoCs are already exposed in the Samsung
> > clock drivers a new samsung_auto_clk_gate_ops is implemented. This uses
> > some CMU debug registers to report whether clocks are enabled or disabled
> > when operating in automatic mode. This allows
> > /sys/kernel/debug/clk/clk_summary to still dump the entire clock tree and
> > correctly report the status of each clock in the system.
> >
> > Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
>
> It's great to see some effort on this topic as this makes the clock handling
> on new Exynos much easier and less error prone and time-consuming, and having
> the hardware decide which clocks need to be enabled at a given point in time
> allows for a significantly more dynamic environment.
>
> Just some initial comments below.
>
> > ---
> > drivers/clk/samsung/clk-exynos-arm64.c | 47 +++++++--
> > drivers/clk/samsung/clk-exynos4.c | 6 +-
> > drivers/clk/samsung/clk-exynos4412-isp.c | 4 +-
> > drivers/clk/samsung/clk-exynos5250.c | 2 +-
> > drivers/clk/samsung/clk-exynos5420.c | 4 +-
> > drivers/clk/samsung/clk.c | 161 ++++++++++++++++++++++++++++---
> > drivers/clk/samsung/clk.h | 49 +++++++++-
> > 7 files changed, 244 insertions(+), 29 deletions(-)
> >
> > diff --git a/drivers/clk/samsung/clk-exynos-arm64.c b/drivers/clk/samsung/clk-exynos-arm64.c
> > index bf7de21f329ec89069dcf817ca578fcf9b2d9809..c302c836e8f9f6270753d86d7d986c88e6762f4f 100644
> > --- a/drivers/clk/samsung/clk-exynos-arm64.c
> > +++ b/drivers/clk/samsung/clk-exynos-arm64.c
> > @@ -24,6 +24,16 @@
> > #define GATE_MANUAL BIT(20)
> > #define GATE_ENABLE_HWACG BIT(28)
> >
> > +/* Option register bits */
> > +#define OPT_EN_DBG BIT(31)
> > +#define OPT_UNKNOWN BIT(30)
>
> Except for AOC, bit 30 is called ENABLE_LAYER2_CTRL in all gs101 CMUs.
Will fix.
>
> > +#define OPT_EN_PWR_MANAGEMENT BIT(29)
> > +#define OPT_EN_AUTO_GATING BIT(28)
> > +#define OPT_EN_MEM_PM_GATING BIT(24)
> > +
> > +#define CMU_OPT_GLOBAL_EN_AUTO_GATING (OPT_EN_DBG | OPT_UNKNOWN | \
> > + OPT_EN_PWR_MANAGEMENT | OPT_EN_AUTO_GATING | OPT_EN_MEM_PM_GATING)
> > +
> > /* PLL_CONx_PLL register offsets range */
> > #define PLL_CON_OFF_START 0x100
> > #define PLL_CON_OFF_END 0x600
> > @@ -37,6 +47,8 @@ struct exynos_arm64_cmu_data {
> > unsigned int nr_clk_save;
> > const struct samsung_clk_reg_dump *clk_suspend;
> > unsigned int nr_clk_suspend;
> > + struct samsung_clk_reg_dump *clk_sysreg_save;
> > + unsigned int nr_clk_sysreg;
> >
> > struct clk *clk;
> > struct clk **pclks;
> > @@ -82,13 +94,28 @@ static void __init exynos_arm64_init_clocks(struct device_node *np,
> > if (!reg_base)
> > panic("%s: failed to map registers\n", __func__);
> >
> > + if (cmu->auto_clock_gate && cmu->option_offset) {
> > + /*
> > + * Enable the global automatic mode for the entire CMU.
> > + * This overrides the individual HWACG bits in each of the
> > + * individual gate, mux and qch registers.
> > + */
> > + writel(CMU_OPT_GLOBAL_EN_AUTO_GATING,
> > + reg_base + cmu->option_offset);
> > + }
> > +
> > for (i = 0; i < reg_offs_len; ++i) {
> > void __iomem *reg = reg_base + reg_offs[i];
> > u32 val;
> >
> > if (cmu->manual_plls && is_pll_con1_reg(reg_offs[i])) {
> > writel(PLL_CON1_MANUAL, reg);
> > - } else if (is_gate_reg(reg_offs[i])) {
> > + } else if (is_gate_reg(reg_offs[i]) && !cmu->auto_clock_gate) {
> > + /*
> > + * Setting GATE_MANUAL bit (which is described in TRM as
> > + * reserved!) overrides the global CMU automatic mode
> > + * option.
> > + */
> > val = readl(reg);
> > val |= GATE_MANUAL;
> > val &= ~GATE_ENABLE_HWACG;
> > @@ -219,7 +246,7 @@ void __init exynos_arm64_register_cmu(struct device *dev,
> > * Return: 0 on success, or negative error code on error.
> > */
> > int __init exynos_arm64_register_cmu_pm(struct platform_device *pdev,
> > - bool set_manual)
> > + bool init_clk_regs)
>
> kerneldoc needs updating
Good spot. Will fix
>
> > {
> > const struct samsung_cmu_info *cmu;
> > struct device *dev = &pdev->dev;
> > @@ -249,7 +276,7 @@ int __init exynos_arm64_register_cmu_pm(struct platform_device *pdev,
> > dev_err(dev, "%s: could not enable bus clock %s; err = %d\n",
> > __func__, cmu->clk_name, ret);
> >
> > - if (set_manual)
> > + if (init_clk_regs)
> > exynos_arm64_init_clocks(np, cmu);
> >
> > reg_base = devm_platform_ioremap_resource(pdev, 0);
> > @@ -280,14 +307,18 @@ int exynos_arm64_cmu_suspend(struct device *dev)
> > struct exynos_arm64_cmu_data *data = dev_get_drvdata(dev);
> > int i;
> >
> > - samsung_clk_save(data->ctx->reg_base, data->clk_save,
> > + samsung_clk_save(data->ctx->reg_base, NULL, data->clk_save,
> > data->nr_clk_save);
> >
> > + if (data->ctx->sysreg)
> > + samsung_clk_save(NULL, data->ctx->sysreg, data->clk_save,
> > + data->nr_clk_save);
> > +
>
> I think this should be
>
> + samsung_clk_save(NULL, data->ctx->sysreg, data->clk_sysreg_save,
> + data->nr_clk_sysreg);
Yes, you're correct! Will fix.
>
> ?
>
> > for (i = 0; i < data->nr_pclks; i++)
> > clk_prepare_enable(data->pclks[i]);
> >
> > /* For suspend some registers have to be set to certain values */
> > - samsung_clk_restore(data->ctx->reg_base, data->clk_suspend,
> > + samsung_clk_restore(data->ctx->reg_base, NULL, data->clk_suspend,
> > data->nr_clk_suspend);
> >
> > for (i = 0; i < data->nr_pclks; i++)
> > @@ -308,9 +339,13 @@ int exynos_arm64_cmu_resume(struct device *dev)
> > for (i = 0; i < data->nr_pclks; i++)
> > clk_prepare_enable(data->pclks[i]);
> >
> > - samsung_clk_restore(data->ctx->reg_base, data->clk_save,
> > + samsung_clk_restore(data->ctx->reg_base, NULL, data->clk_save,
> > data->nr_clk_save);
> >
> > + if (data->ctx->sysreg)
> > + samsung_clk_restore(NULL, data->ctx->sysreg, data->clk_save,
> > + data->nr_clk_save);
> > +
>
> dito.
Will fix.
>
> > for (i = 0; i < data->nr_pclks; i++)
> > clk_disable_unprepare(data->pclks[i]);
> >
>
> [...]
>
> > diff --git a/drivers/clk/samsung/clk.c b/drivers/clk/samsung/clk.c
> > index dbc9925ca8f46e951dfb5d391c0e744ca370abcc..07b2948ae7ea48f126ab420be57d8c2705979464 100644
> > --- a/drivers/clk/samsung/clk.c
> > +++ b/drivers/clk/samsung/clk.c
> > @@ -12,8 +12,10 @@
> > #include <linux/clkdev.h>
> > #include <linux/clk-provider.h>
> > #include <linux/io.h>
> > +#include <linux/mfd/syscon.h>
> > #include <linux/mod_devicetable.h>
> > #include <linux/of_address.h>
> > +#include <linux/regmap.h>
> > #include <linux/syscore_ops.h>
> >
> > #include "clk.h"
> > @@ -21,19 +23,29 @@
> > static LIST_HEAD(clock_reg_cache_list);
> >
> > void samsung_clk_save(void __iomem *base,
> > + struct regmap *regmap,
> > struct samsung_clk_reg_dump *rd,
> > unsigned int num_regs)
> > {
> > - for (; num_regs > 0; --num_regs, ++rd)
> > - rd->value = readl(base + rd->offset);
> > + for (; num_regs > 0; --num_regs, ++rd) {
> > + if (base)
> > + rd->value = readl(base + rd->offset);
> > + if (regmap)
>
> Should this be else if?
I will update it like you suggest to protect against that
>
> > + regmap_read(regmap, rd->offset, &rd->value);
>
> Otherwise users that (incorrectly) pass both base and regmap would
> cause this to end up reading the offset from the wrong memory
> region.
>
> At least this way that won't happen, but maybe this constrains should
> be made even more explicit.
>
> > + }
> > }
> >
> > void samsung_clk_restore(void __iomem *base,
> > + struct regmap *regmap,
> > const struct samsung_clk_reg_dump *rd,
> > unsigned int num_regs)
> > {
> > - for (; num_regs > 0; --num_regs, ++rd)
> > - writel(rd->value, base + rd->offset);
> > + for (; num_regs > 0; --num_regs, ++rd) {
> > + if (base)
> > + writel(rd->value, base + rd->offset);
> > + if (regmap)
> > + regmap_write(regmap, rd->offset, rd->value);
>
> dito.
Will fix.
>
> > + }
> > }
> >
> > struct samsung_clk_reg_dump *samsung_clk_alloc_reg_dump(
> > @@ -227,6 +239,82 @@ void __init samsung_clk_register_div(struct samsung_clk_provider *ctx,
> > }
> > }
> >
> > +#define ACG_MSK GENMASK(6, 4)
> > +#define CLK_IDLE GENMASK(5, 4)
> > +static int samsung_auto_clk_gate_is_en(struct clk_hw *hw)
> > +{
> > + u32 reg;
> > + struct clk_gate *gate = to_clk_gate(hw);
> > +
> > + reg = readl(gate->reg);
> > + return ((reg & ACG_MSK) == CLK_IDLE) ? 0 : 1;
> > +}
> > +
> > +/* enable and disable are nops in automatic clock mode */
> > +static int samsung_auto_clk_gate_en(struct clk_hw *hw)
> > +{
> > + return 0;
> > +}
> > +
> > +static void samsung_auto_clk_gate_dis(struct clk_hw *hw)
> > +{
> > +}
> > +
> > +static const struct clk_ops samsung_auto_clk_gate_ops = {
> > + .enable = samsung_auto_clk_gate_en,
> > + .disable = samsung_auto_clk_gate_dis,
> > + .is_enabled = samsung_auto_clk_gate_is_en,
> > +};
> > +
> > +struct clk_hw *samsung_register_auto_gate(struct device *dev,
> > + struct device_node *np, const char *name,
> > + const char *parent_name, const struct clk_hw *parent_hw,
> > + const struct clk_parent_data *parent_data,
> > + unsigned long flags,
> > + void __iomem *reg, u8 bit_idx,
> > + u8 clk_gate_flags, spinlock_t *lock)
> > +{
> > + struct clk_gate *gate;
> > + struct clk_hw *hw;
> > + struct clk_init_data init = {};
> > + int ret = -EINVAL;
> > +
> > + /* allocate the gate */
> > + gate = kzalloc(sizeof(*gate), GFP_KERNEL);
> > + if (!gate)
> > + return ERR_PTR(-ENOMEM);
> > +
> > + init.name = name;
> > + init.ops = &samsung_auto_clk_gate_ops;
> > + init.flags = flags;
> > + init.parent_names = parent_name ? &parent_name : NULL;
> > + init.parent_hws = parent_hw ? &parent_hw : NULL;
> > + init.parent_data = parent_data;
> > + if (parent_name || parent_hw || parent_data)
> > + init.num_parents = 1;
> > + else
> > + init.num_parents = 0;
> > +
> > + /* struct clk_gate assignments */
> > + gate->reg = reg;
> > + gate->bit_idx = bit_idx;
> > + gate->flags = clk_gate_flags;
> > + gate->lock = lock;
> > + gate->hw.init = &init;
> > +
> > + hw = &gate->hw;
> > + if (dev || !np)
> > + ret = clk_hw_register(dev, hw);
> > + else if (np)
> > + ret = of_clk_hw_register(np, hw);
> > + if (ret) {
> > + kfree(gate);
> > + hw = ERR_PTR(ret);
> > + }
> > +
> > + return hw;
> > +}
> > +
> > /* register a list of gate clocks */
> > void __init samsung_clk_register_gate(struct samsung_clk_provider *ctx,
> > const struct samsung_gate_clock *list,
> > @@ -234,11 +322,21 @@ void __init samsung_clk_register_gate(struct samsung_clk_provider *ctx,
> > {
> > struct clk_hw *clk_hw;
> > unsigned int idx;
> > + void __iomem *reg_offs;
> >
> > for (idx = 0; idx < nr_clk; idx++, list++) {
> > - clk_hw = clk_hw_register_gate(ctx->dev, list->name, list->parent_name,
> > - list->flags, ctx->reg_base + list->offset,
> > + reg_offs = ctx->reg_base + list->offset;
> > +
> > + if (ctx->auto_clock_gate && ctx->gate_dbg_offset)
> > + clk_hw = samsung_register_auto_gate(ctx->dev, NULL,
> > + list->name, list->parent_name, NULL, NULL,
> > + list->flags, reg_offs + ctx->gate_dbg_offset,
> > list->bit_idx, list->gate_flags, &ctx->lock);
> > + else
> > + clk_hw = clk_hw_register_gate(ctx->dev, list->name,
> > + list->parent_name, list->flags,
> > + ctx->reg_base + list->offset, list->bit_idx,
> > + list->gate_flags, &ctx->lock);
> > if (IS_ERR(clk_hw)) {
> > pr_err("%s: failed to register clock %s\n", __func__,
> > list->name);
>
> Maybe the error message should include the actual error clk_hw here?
Will update as you suggest.
>
> > @@ -276,10 +374,11 @@ static int samsung_clk_suspend(void)
> > struct samsung_clock_reg_cache *reg_cache;
> >
> > list_for_each_entry(reg_cache, &clock_reg_cache_list, node) {
> > - samsung_clk_save(reg_cache->reg_base, reg_cache->rdump,
> > - reg_cache->rd_num);
> > - samsung_clk_restore(reg_cache->reg_base, reg_cache->rsuspend,
> > - reg_cache->rsuspend_num);
> > + samsung_clk_save(reg_cache->reg_base, reg_cache->sysreg,
> > + reg_cache->rdump, reg_cache->rd_num);
> > + samsung_clk_restore(reg_cache->reg_base, reg_cache->sysreg,
> > + reg_cache->rsuspend,
> > + reg_cache->rsuspend_num);
> > }
> > return 0;
> > }
> > @@ -289,8 +388,8 @@ static void samsung_clk_resume(void)
> > struct samsung_clock_reg_cache *reg_cache;
> >
> > list_for_each_entry(reg_cache, &clock_reg_cache_list, node)
> > - samsung_clk_restore(reg_cache->reg_base, reg_cache->rdump,
> > - reg_cache->rd_num);
> > + samsung_clk_restore(reg_cache->reg_base, reg_cache->sysreg,
> > + reg_cache->rdump, reg_cache->rd_num);
> > }
> >
> > static struct syscore_ops samsung_clk_syscore_ops = {
> > @@ -299,6 +398,7 @@ static struct syscore_ops samsung_clk_syscore_ops = {
> > };
> >
> > void samsung_clk_extended_sleep_init(void __iomem *reg_base,
> > + struct regmap *sysreg,
> > const unsigned long *rdump,
> > unsigned long nr_rdump,
> > const struct samsung_clk_reg_dump *rsuspend,
> > @@ -319,6 +419,7 @@ void samsung_clk_extended_sleep_init(void __iomem *reg_base,
> > register_syscore_ops(&samsung_clk_syscore_ops);
> >
> > reg_cache->reg_base = reg_base;
> > + reg_cache->sysreg = sysreg;
> > reg_cache->rd_num = nr_rdump;
> > reg_cache->rsuspend = rsuspend;
> > reg_cache->rsuspend_num = nr_rsuspend;
> > @@ -334,6 +435,12 @@ void samsung_clk_extended_sleep_init(void __iomem *reg_base,
> > void __init samsung_cmu_register_clocks(struct samsung_clk_provider *ctx,
> > const struct samsung_cmu_info *cmu)
> > {
> > + ctx->auto_clock_gate = cmu->auto_clock_gate;
> > + ctx->gate_dbg_offset = cmu->gate_dbg_offset;
> > + ctx->option_offset = cmu->option_offset;
> > + ctx->drcg_offset = cmu->drcg_offset;
> > + ctx->memclk_offset = cmu->memclk_offset;
> > +
> > if (cmu->pll_clks)
> > samsung_clk_register_pll(ctx, cmu->pll_clks, cmu->nr_pll_clks);
> > if (cmu->mux_clks)
> > @@ -353,6 +460,31 @@ void __init samsung_cmu_register_clocks(struct samsung_clk_provider *ctx,
> > samsung_clk_register_cpu(ctx, cmu->cpu_clks, cmu->nr_cpu_clks);
> > }
> >
> > +/* Enable Dynamic Root Clock Gating of bus components*/
>
> missing space before */
will fix.
>
> > +void samsung_en_dyn_root_clk_gating(struct device_node *np,
> > + struct samsung_clk_provider *ctx,
> > + const struct samsung_cmu_info *cmu)
> > +{
> > + if (ctx && !ctx->auto_clock_gate)
>
> ctx can not be NULL here. If it could, then the following lines would also
> need updating, but again, it can not be NULL, if ctx allocation fails, the
> code will panic() before this here is reached.
I'll remove that check.
>
> > + return;
> > +
> > + ctx->sysreg = syscon_regmap_lookup_by_phandle(np, "samsung,sysreg");
> > + if (!IS_ERR_OR_NULL(ctx->sysreg)) {
> > + regmap_write(ctx->sysreg, ctx->drcg_offset, 0xffffffff);
>
> What does 0xffffffff mean? Maybe a macro or at least a comment would help.
Setting a bit "enables dynamic root clock gating (drcg)". Different
bits in the register correspond to different bus components. So this
is enabling drcg for everything.
>
> > + /* not every sysreg controller has memclk reg*/
>
> missing space before */
Will fix
>
> Can you please check all such comments, since it seems to be recurring :-)
Will check!
>
> + if (ctx->memclk_offset)
> > + regmap_write_bits(ctx->sysreg, ctx->memclk_offset, 0x1, 0x0);
> > +
> > + samsung_clk_extended_sleep_init(NULL, ctx->sysreg,
> > + cmu->sysreg_clk_regs,
> > + cmu->nr_sysreg_clk_regs,
> > + NULL, 0);
> > + } else {
> > + pr_warn("%pOF: Unable to get CMU sysreg\n", np);
> > + ctx->sysreg = NULL;
> > + }
> > +}
> > +
> > /*
> > * Common function which registers plls, muxes, dividers and gates
> > * for each CMU. It also add CMU register list to register cache.
> > @@ -374,11 +506,14 @@ struct samsung_clk_provider * __init samsung_cmu_register_one(
> > samsung_cmu_register_clocks(ctx, cmu);
> >
> > if (cmu->clk_regs)
> > - samsung_clk_extended_sleep_init(reg_base,
> > + samsung_clk_extended_sleep_init(reg_base, NULL,
> > cmu->clk_regs, cmu->nr_clk_regs,
> > cmu->suspend_regs, cmu->nr_suspend_regs);
> >
> > samsung_clk_of_add_provider(np, ctx);
> >
> > + /* sysreg DT nodes reference a clock in this CMU */
> > + samsung_en_dyn_root_clk_gating(np, ctx, cmu);
> > +
>
> samsung_cmu_register_one() is not called when the clocks are registered
> using exynos_arm64_register_cmu_pm(). Does the pm version need a call to
> samsung_en_dyn_root_clk_gating() somewhere?
Yes, good point, I will add it in v2. That should make things smoother
when we switch to the exynos_arm64_register_cmu_pm() version.
>
> > return ctx;
> > }
> > diff --git a/drivers/clk/samsung/clk.h b/drivers/clk/samsung/clk.h
> > index 18660c1ac6f0106b17b9efc9c6b3cd62d46f7b82..b719e057f45489e9d92ba54031fe633a8c9264ce 100644
> > --- a/drivers/clk/samsung/clk.h
> > +++ b/drivers/clk/samsung/clk.h
> > @@ -12,6 +12,7 @@
> >
> > #include <linux/clk-provider.h>
> > #include <linux/mod_devicetable.h>
> > +#include <linux/regmap.h>
> > #include "clk-pll.h"
> > #include "clk-cpu.h"
> >
> > @@ -19,13 +20,25 @@
> > * struct samsung_clk_provider - information about clock provider
> > * @reg_base: virtual address for the register base
> > * @dev: clock provider device needed for runtime PM
> > + * @sysreg: syscon regmap for clock-provider sysreg controller
> > * @lock: maintains exclusion between callbacks for a given clock-provider
> > + * @auto_clock_gate: enable auto clk mode for all clocks in clock-provider
> > + * @gate_dbg_offset: gate debug reg offset. Used for all gates in auto clk mode
> > + * @option_offset: option reg offset. Enables auto mode for clock-provider
> > + * @drcg_offset: dynamic root clk gate enable register offset
> > + * @memclk_offset: memclk enable register offset
>
> The last two are in the sysreg space, maybe the comment should mention that.
Will update the comment.
>
> > * @clk_data: holds clock related data like clk_hw* and number of clocks
> > */
> > struct samsung_clk_provider {
> > void __iomem *reg_base;
> > struct device *dev;
> > + struct regmap *sysreg;
> > spinlock_t lock;
> > + bool auto_clock_gate;
> > + u32 gate_dbg_offset;
> > + u32 option_offset;
> > + u32 drcg_offset;
> > + u32 memclk_offset;
> > /* clk_data must be the last entry due to variable length 'hws' array */
> > struct clk_hw_onecell_data clk_data;
> > };
> > @@ -310,6 +323,7 @@ struct samsung_cpu_clock {
> > struct samsung_clock_reg_cache {
> > struct list_head node;
> > void __iomem *reg_base;
> > + struct regmap *sysreg;
> > struct samsung_clk_reg_dump *rdump;
> > unsigned int rd_num;
> > const struct samsung_clk_reg_dump *rsuspend;
> > @@ -338,7 +352,14 @@ struct samsung_clock_reg_cache {
> > * @suspend_regs: list of clock registers to set before suspend
> > * @nr_suspend_regs: count of clock registers in @suspend_regs
> > * @clk_name: name of the parent clock needed for CMU register access
> > + * @sysreg_clk_regs: list of sysreg clock registers
> > + * @nr_sysreg_clk_regs: count of clock registers in @sysreg_clk_regs
> > * @manual_plls: Enable manual control for PLL clocks
> > + * @auto_clock_gate: enable auto clock mode for all components in CMU
> > + * @gate_dbg_offset: gate debug reg offset. Used by all gates in auto clk mode
> > + * @option_offset: option reg offset. Enables auto clk mode for entire CMU
> > + * @drcg_offset: dynamic root clk gate enable register offset
> > + * @memclk_offset: memclk enable register offset
>
> dito.
Will fix
Thanks again for your time spent reviewing, some great feedback.
regards,
Peter
Hi Peter,
kernel test robot noticed the following build errors:
[auto build test ERROR on 4a71531471926e3c391665ee9c42f4e0295a4585]
url: https://github.com/intel-lab-lkp/linux/commits/Peter-Griffin/dt-bindings-soc-samsung-exynos-sysreg-add-gs101-hsi0-and-misc-compatibles/20251014-045559
base: 4a71531471926e3c391665ee9c42f4e0295a4585
patch link: https://lore.kernel.org/r/20251013-automatic-clocks-v1-7-72851ee00300%40linaro.org
patch subject: [PATCH 7/9] clk: samsung: Implement automatic clock gating mode for CMUs
config: loongarch-randconfig-001-20251014 (https://download.01.org/0day-ci/archive/20251014/202510142228.IQJSNIFa-lkp@intel.com/config)
compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251014/202510142228.IQJSNIFa-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202510142228.IQJSNIFa-lkp@intel.com/
All error/warnings (new ones prefixed by >>):
>> drivers/clk/samsung/clk.c:481:13: error: too many arguments to function call, expected 5, have 6
478 | samsung_clk_extended_sleep_init(NULL, ctx->sysreg,
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
479 | cmu->sysreg_clk_regs,
480 | cmu->nr_sysreg_clk_regs,
481 | NULL, 0);
| ^
drivers/clk/samsung/clk.h:453:20: note: 'samsung_clk_extended_sleep_init' declared here
453 | static inline void samsung_clk_extended_sleep_init(void __iomem *reg_base,
| ^ ~~~~~~~~~~~~~~~~~~~~~~~
454 | const unsigned long *rdump,
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~
455 | unsigned long nr_rdump,
| ~~~~~~~~~~~~~~~~~~~~~~~
456 | const struct samsung_clk_reg_dump *rsuspend,
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
457 | unsigned long nr_rsuspend) {}
| ~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/clk/samsung/clk.c:511:23: error: too many arguments to function call, expected 5, have 6
509 | samsung_clk_extended_sleep_init(reg_base, NULL,
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
510 | cmu->clk_regs, cmu->nr_clk_regs,
511 | cmu->suspend_regs, cmu->nr_suspend_regs);
| ^~~~~~~~~~~~~~~~~~~~
drivers/clk/samsung/clk.h:453:20: note: 'samsung_clk_extended_sleep_init' declared here
453 | static inline void samsung_clk_extended_sleep_init(void __iomem *reg_base,
| ^ ~~~~~~~~~~~~~~~~~~~~~~~
454 | const unsigned long *rdump,
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~
455 | unsigned long nr_rdump,
| ~~~~~~~~~~~~~~~~~~~~~~~
456 | const struct samsung_clk_reg_dump *rsuspend,
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
457 | unsigned long nr_rsuspend) {}
| ~~~~~~~~~~~~~~~~~~~~~~~~~
2 errors generated.
--
>> Warning: drivers/clk/samsung/clk-exynos-arm64.c:249 function parameter 'init_clk_regs' not described in 'exynos_arm64_register_cmu_pm'
vim +481 drivers/clk/samsung/clk.c
462
463 /* Enable Dynamic Root Clock Gating of bus components*/
464 void samsung_en_dyn_root_clk_gating(struct device_node *np,
465 struct samsung_clk_provider *ctx,
466 const struct samsung_cmu_info *cmu)
467 {
468 if (ctx && !ctx->auto_clock_gate)
469 return;
470
471 ctx->sysreg = syscon_regmap_lookup_by_phandle(np, "samsung,sysreg");
472 if (!IS_ERR_OR_NULL(ctx->sysreg)) {
473 regmap_write(ctx->sysreg, ctx->drcg_offset, 0xffffffff);
474 /* not every sysreg controller has memclk reg*/
475 if (ctx->memclk_offset)
476 regmap_write_bits(ctx->sysreg, ctx->memclk_offset, 0x1, 0x0);
477
478 samsung_clk_extended_sleep_init(NULL, ctx->sysreg,
479 cmu->sysreg_clk_regs,
480 cmu->nr_sysreg_clk_regs,
> 481 NULL, 0);
482 } else {
483 pr_warn("%pOF: Unable to get CMU sysreg\n", np);
484 ctx->sysreg = NULL;
485 }
486 }
487
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
© 2016 - 2026 Red Hat, Inc.