lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CADrjBPq9aPFZ5Oqw_R6MyZiWtuFaQtzRO7PYGs0FLLfHy7UgtA@mail.gmail.com>
Date: Tue, 28 Oct 2025 08:50:38 +0000
From: Peter Griffin <peter.griffin@...aro.org>
To: André Draszik <andre.draszik@...aro.org>
Cc: Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, 
	Conor Dooley <conor+dt@...nel.org>, Alim Akhtar <alim.akhtar@...sung.com>, 
	Tudor Ambarus <tudor.ambarus@...aro.org>, Michael Turquette <mturquette@...libre.com>, 
	Stephen Boyd <sboyd@...nel.org>, Sam Protsenko <semen.protsenko@...aro.org>, 
	Sylwester Nawrocki <s.nawrocki@...sung.com>, Chanwoo Choi <cw00.choi@...sung.com>, 
	Will McVicker <willmcvicker@...gle.com>, Krzysztof Kozlowski <krzk@...nel.org>, devicetree@...r.kernel.org, 
	linux-arm-kernel@...ts.infradead.org, linux-samsung-soc@...r.kernel.org, 
	linux-kernel@...r.kernel.org, linux-clk@...r.kernel.org, 
	Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>, kernel-team@...roid.com
Subject: Re: [PATCH 7/9] clk: samsung: Implement automatic clock gating mode
 for CMUs

Hi André,

Thanks for the review feedback!

On Thu, 16 Oct 2025 at 10:40, André Draszik <andre.draszik@...aro.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@...aro.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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ