| Message ID | 20211013221021.3433704-2-willmcvicker@google.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | clk: samsung: add common support for CPU clocks | expand |
Quoting Will McVicker (2021-10-13 15:10:19) > diff --git a/drivers/clk/samsung/clk-cpu.c b/drivers/clk/samsung/clk-cpu.c > index 00ef4d1b0888..b5017934fc41 100644 > --- a/drivers/clk/samsung/clk-cpu.c > +++ b/drivers/clk/samsung/clk-cpu.c > @@ -469,3 +469,29 @@ int __init exynos_register_cpu_clock(struct samsung_clk_provider *ctx, > kfree(cpuclk); > return ret; > } > + > +void samsung_clk_register_cpu(struct samsung_clk_provider *ctx, > + const struct samsung_cpu_clock *list, unsigned int nr_clk) > +{ > + unsigned int idx; > + unsigned int num_cfgs; > + struct clk *parent_clk, *alt_parent_clk; > + const struct clk_hw *parent_clk_hw = NULL; > + const struct clk_hw *alt_parent_clk_hw = NULL; > + > + for (idx = 0; idx < nr_clk; idx++, list++) { > + /* find count of configuration rates in cfg */ > + for (num_cfgs = 0; list->cfg[num_cfgs].prate != 0; ) > + num_cfgs++; > + > + parent_clk = __clk_lookup(list->parent_name); Please stop using __clk_lookup() > + if (parent_clk) > + parent_clk_hw = __clk_get_hw(parent_clk); > + alt_parent_clk = __clk_lookup(list->alt_parent_name); Can the DT binding be updated? > + if (alt_parent_clk) > + alt_parent_clk_hw = __clk_get_hw(alt_parent_clk); > + > + exynos_register_cpu_clock(ctx, list->id, list->name, parent_clk_hw, > + alt_parent_clk_hw, list->offset, list->cfg, num_cfgs, list->flags); > + } > +} > diff --git a/drivers/clk/samsung/clk.c b/drivers/clk/samsung/clk.c > index 1949ae7851b2..336243c6f120 100644 > --- a/drivers/clk/samsung/clk.c > +++ b/drivers/clk/samsung/clk.c > @@ -378,6 +378,8 @@ struct samsung_clk_provider * __init samsung_cmu_register_one( > samsung_clk_extended_sleep_init(reg_base, > cmu->clk_regs, cmu->nr_clk_regs, > cmu->suspend_regs, cmu->nr_suspend_regs); > + if (cmu->cpu_clks) > + samsung_clk_register_cpu(ctx, cmu->cpu_clks, cmu->nr_cpu_clks); > > samsung_clk_of_add_provider(np, ctx); > > diff --git a/drivers/clk/samsung/clk.h b/drivers/clk/samsung/clk.h > index c1e1a6b2f499..a52a38cc1740 100644 > --- a/drivers/clk/samsung/clk.h > +++ b/drivers/clk/samsung/clk.h > @@ -271,6 +271,27 @@ struct samsung_pll_clock { > __PLL(_typ, _id, _name, _pname, CLK_GET_RATE_NOCACHE, _lock, \ > _con, _rtable) > > +struct samsung_cpu_clock { > + unsigned int id; > + const char *name; > + const char *parent_name; > + const char *alt_parent_name; > + unsigned long flags; > + int offset; > + const struct exynos_cpuclk_cfg_data *cfg; > +}; > + > +#define CPU_CLK(_id, _name, _pname, _apname, _flags, _offset, _cfg) \ > + { \ > + .id = _id, \ > + .name = _name, \ > + .parent_name = _pname, \ > + .alt_parent_name = _apname, \ > + .flags = _flags, \ > + .offset = _offset, \ > + .cfg = _cfg, \ > + } > + > struct samsung_clock_reg_cache { > struct list_head node; > void __iomem *reg_base; > @@ -301,6 +322,9 @@ struct samsung_cmu_info { > unsigned int nr_fixed_factor_clks; > /* total number of clocks with IDs assigned*/ > unsigned int nr_clk_ids; > + /* list of cpu clocks and respective count */ > + const struct samsung_cpu_clock *cpu_clks; > + unsigned int nr_cpu_clks; > > /* list and number of clocks registers */ > const unsigned long *clk_regs; > @@ -350,6 +374,8 @@ extern void __init samsung_clk_register_gate(struct samsung_clk_provider *ctx, > extern void __init samsung_clk_register_pll(struct samsung_clk_provider *ctx, > const struct samsung_pll_clock *pll_list, > unsigned int nr_clk, void __iomem *base); > +extern void __init samsung_clk_register_cpu(struct samsung_clk_provider *ctx, __init in header files is useless. > + const struct samsung_cpu_clock *list, unsigned int nr_clk); >
On Wed, Oct 13, 2021 at 6:49 PM Stephen Boyd <sboyd@kernel.org> wrote: > > Quoting Will McVicker (2021-10-13 15:10:19) > > diff --git a/drivers/clk/samsung/clk-cpu.c b/drivers/clk/samsung/clk-cpu.c > > index 00ef4d1b0888..b5017934fc41 100644 > > --- a/drivers/clk/samsung/clk-cpu.c > > +++ b/drivers/clk/samsung/clk-cpu.c > > @@ -469,3 +469,29 @@ int __init exynos_register_cpu_clock(struct samsung_clk_provider *ctx, > > kfree(cpuclk); > > return ret; > > } > > + > > +void samsung_clk_register_cpu(struct samsung_clk_provider *ctx, > > + const struct samsung_cpu_clock *list, unsigned int nr_clk) > > +{ > > + unsigned int idx; > > + unsigned int num_cfgs; > > + struct clk *parent_clk, *alt_parent_clk; > > + const struct clk_hw *parent_clk_hw = NULL; > > + const struct clk_hw *alt_parent_clk_hw = NULL; > > + > > + for (idx = 0; idx < nr_clk; idx++, list++) { > > + /* find count of configuration rates in cfg */ > > + for (num_cfgs = 0; list->cfg[num_cfgs].prate != 0; ) > > + num_cfgs++; > > + > > + parent_clk = __clk_lookup(list->parent_name); > > Please stop using __clk_lookup() Thanks, I believe I have a way around this. I'll fix this up in the follow-up version. > > > + if (parent_clk) > > + parent_clk_hw = __clk_get_hw(parent_clk); > > + alt_parent_clk = __clk_lookup(list->alt_parent_name); > > Can the DT binding be updated? Are you referring to removing alt_parent and just adding it as another parent clock? > > > + if (alt_parent_clk) > > + alt_parent_clk_hw = __clk_get_hw(alt_parent_clk); > > + > > + exynos_register_cpu_clock(ctx, list->id, list->name, parent_clk_hw, > > + alt_parent_clk_hw, list->offset, list->cfg, num_cfgs, list->flags); > > + } > > +} > > diff --git a/drivers/clk/samsung/clk.c b/drivers/clk/samsung/clk.c > > index 1949ae7851b2..336243c6f120 100644 > > --- a/drivers/clk/samsung/clk.c > > +++ b/drivers/clk/samsung/clk.c > > @@ -378,6 +378,8 @@ struct samsung_clk_provider * __init samsung_cmu_register_one( > > samsung_clk_extended_sleep_init(reg_base, > > cmu->clk_regs, cmu->nr_clk_regs, > > cmu->suspend_regs, cmu->nr_suspend_regs); > > + if (cmu->cpu_clks) > > + samsung_clk_register_cpu(ctx, cmu->cpu_clks, cmu->nr_cpu_clks); > > > > samsung_clk_of_add_provider(np, ctx); > > > > diff --git a/drivers/clk/samsung/clk.h b/drivers/clk/samsung/clk.h > > index c1e1a6b2f499..a52a38cc1740 100644 > > --- a/drivers/clk/samsung/clk.h > > +++ b/drivers/clk/samsung/clk.h > > @@ -271,6 +271,27 @@ struct samsung_pll_clock { > > __PLL(_typ, _id, _name, _pname, CLK_GET_RATE_NOCACHE, _lock, \ > > _con, _rtable) > > > > +struct samsung_cpu_clock { > > + unsigned int id; > > + const char *name; > > + const char *parent_name; > > + const char *alt_parent_name; > > + unsigned long flags; > > + int offset; > > + const struct exynos_cpuclk_cfg_data *cfg; > > +}; > > + > > +#define CPU_CLK(_id, _name, _pname, _apname, _flags, _offset, _cfg) \ > > + { \ > > + .id = _id, \ > > + .name = _name, \ > > + .parent_name = _pname, \ > > + .alt_parent_name = _apname, \ > > + .flags = _flags, \ > > + .offset = _offset, \ > > + .cfg = _cfg, \ > > + } > > + > > struct samsung_clock_reg_cache { > > struct list_head node; > > void __iomem *reg_base; > > @@ -301,6 +322,9 @@ struct samsung_cmu_info { > > unsigned int nr_fixed_factor_clks; > > /* total number of clocks with IDs assigned*/ > > unsigned int nr_clk_ids; > > + /* list of cpu clocks and respective count */ > > + const struct samsung_cpu_clock *cpu_clks; > > + unsigned int nr_cpu_clks; > > > > /* list and number of clocks registers */ > > const unsigned long *clk_regs; > > @@ -350,6 +374,8 @@ extern void __init samsung_clk_register_gate(struct samsung_clk_provider *ctx, > > extern void __init samsung_clk_register_pll(struct samsung_clk_provider *ctx, > > const struct samsung_pll_clock *pll_list, > > unsigned int nr_clk, void __iomem *base); > > +extern void __init samsung_clk_register_cpu(struct samsung_clk_provider *ctx, > > __init in header files is useless. Thanks for pointing that out. Looks like this header needs some cleaning up. > > > + const struct samsung_cpu_clock *list, unsigned int nr_clk); > >
Quoting Will McVicker (2021-10-14 12:35:57) > On Wed, Oct 13, 2021 at 6:49 PM Stephen Boyd <sboyd@kernel.org> wrote: > > > > Quoting Will McVicker (2021-10-13 15:10:19) > > > diff --git a/drivers/clk/samsung/clk-cpu.c b/drivers/clk/samsung/clk-cpu.c > > > index 00ef4d1b0888..b5017934fc41 100644 > > > --- a/drivers/clk/samsung/clk-cpu.c > > > +++ b/drivers/clk/samsung/clk-cpu.c > > > @@ -469,3 +469,29 @@ int __init exynos_register_cpu_clock(struct samsung_clk_provider *ctx, > > > kfree(cpuclk); > > > return ret; > > > } > > > + > > > +void samsung_clk_register_cpu(struct samsung_clk_provider *ctx, > > > + const struct samsung_cpu_clock *list, unsigned int nr_clk) > > > +{ > > > + unsigned int idx; > > > + unsigned int num_cfgs; > > > + struct clk *parent_clk, *alt_parent_clk; > > > + const struct clk_hw *parent_clk_hw = NULL; > > > + const struct clk_hw *alt_parent_clk_hw = NULL; > > > + > > > + for (idx = 0; idx < nr_clk; idx++, list++) { > > > + /* find count of configuration rates in cfg */ > > > + for (num_cfgs = 0; list->cfg[num_cfgs].prate != 0; ) > > > + num_cfgs++; > > > + > > > + parent_clk = __clk_lookup(list->parent_name); > > > > Please stop using __clk_lookup() > > Thanks, I believe I have a way around this. I'll fix this up in the > follow-up version. Great! > > > > > > + if (parent_clk) > > > + parent_clk_hw = __clk_get_hw(parent_clk); > > > + alt_parent_clk = __clk_lookup(list->alt_parent_name); > > > > Can the DT binding be updated? > > Are you referring to removing alt_parent and just adding it as another > parent clock? > I was wondering if this is an external clk that feeds into here or if it is all internal to the clk controller. It sounds like it's all internal to the clk controller? In which case a binding update isn't needed.
On 14.10.2021 23:40, Stephen Boyd wrote: > Quoting Will McVicker (2021-10-14 12:35:57) >> On Wed, Oct 13, 2021 at 6:49 PM Stephen Boyd <sboyd@kernel.org> wrote: >>> Quoting Will McVicker (2021-10-13 15:10:19) >>>> diff --git a/drivers/clk/samsung/clk-cpu.c b/drivers/clk/samsung/clk-cpu.c >>> >>>> + if (parent_clk) >>>> + parent_clk_hw = __clk_get_hw(parent_clk); >>>> + alt_parent_clk = __clk_lookup(list->alt_parent_name); >>> >>> Can the DT binding be updated? >> >> Are you referring to removing alt_parent and just adding it as another >> parent clock? >> > > I was wondering if this is an external clk that feeds into here or if it > is all internal to the clk controller. It sounds like it's all internal > to the clk controller? In which case a binding update isn't needed. Yes, I double checked and the cpu parent clocks are always internal to the clock provider. There is one exception where physically a parent clock comes from other CMU (exynos5250), but in that case all CMUs are modeled as a single clk provider anyway. Thus we don't need a binding update.
diff --git a/drivers/clk/samsung/clk-cpu.c b/drivers/clk/samsung/clk-cpu.c index 00ef4d1b0888..b5017934fc41 100644 --- a/drivers/clk/samsung/clk-cpu.c +++ b/drivers/clk/samsung/clk-cpu.c @@ -469,3 +469,29 @@ int __init exynos_register_cpu_clock(struct samsung_clk_provider *ctx, kfree(cpuclk); return ret; } + +void samsung_clk_register_cpu(struct samsung_clk_provider *ctx, + const struct samsung_cpu_clock *list, unsigned int nr_clk) +{ + unsigned int idx; + unsigned int num_cfgs; + struct clk *parent_clk, *alt_parent_clk; + const struct clk_hw *parent_clk_hw = NULL; + const struct clk_hw *alt_parent_clk_hw = NULL; + + for (idx = 0; idx < nr_clk; idx++, list++) { + /* find count of configuration rates in cfg */ + for (num_cfgs = 0; list->cfg[num_cfgs].prate != 0; ) + num_cfgs++; + + parent_clk = __clk_lookup(list->parent_name); + if (parent_clk) + parent_clk_hw = __clk_get_hw(parent_clk); + alt_parent_clk = __clk_lookup(list->alt_parent_name); + if (alt_parent_clk) + alt_parent_clk_hw = __clk_get_hw(alt_parent_clk); + + exynos_register_cpu_clock(ctx, list->id, list->name, parent_clk_hw, + alt_parent_clk_hw, list->offset, list->cfg, num_cfgs, list->flags); + } +} diff --git a/drivers/clk/samsung/clk.c b/drivers/clk/samsung/clk.c index 1949ae7851b2..336243c6f120 100644 --- a/drivers/clk/samsung/clk.c +++ b/drivers/clk/samsung/clk.c @@ -378,6 +378,8 @@ struct samsung_clk_provider * __init samsung_cmu_register_one( samsung_clk_extended_sleep_init(reg_base, cmu->clk_regs, cmu->nr_clk_regs, cmu->suspend_regs, cmu->nr_suspend_regs); + if (cmu->cpu_clks) + samsung_clk_register_cpu(ctx, cmu->cpu_clks, cmu->nr_cpu_clks); samsung_clk_of_add_provider(np, ctx); diff --git a/drivers/clk/samsung/clk.h b/drivers/clk/samsung/clk.h index c1e1a6b2f499..a52a38cc1740 100644 --- a/drivers/clk/samsung/clk.h +++ b/drivers/clk/samsung/clk.h @@ -271,6 +271,27 @@ struct samsung_pll_clock { __PLL(_typ, _id, _name, _pname, CLK_GET_RATE_NOCACHE, _lock, \ _con, _rtable) +struct samsung_cpu_clock { + unsigned int id; + const char *name; + const char *parent_name; + const char *alt_parent_name; + unsigned long flags; + int offset; + const struct exynos_cpuclk_cfg_data *cfg; +}; + +#define CPU_CLK(_id, _name, _pname, _apname, _flags, _offset, _cfg) \ + { \ + .id = _id, \ + .name = _name, \ + .parent_name = _pname, \ + .alt_parent_name = _apname, \ + .flags = _flags, \ + .offset = _offset, \ + .cfg = _cfg, \ + } + struct samsung_clock_reg_cache { struct list_head node; void __iomem *reg_base; @@ -301,6 +322,9 @@ struct samsung_cmu_info { unsigned int nr_fixed_factor_clks; /* total number of clocks with IDs assigned*/ unsigned int nr_clk_ids; + /* list of cpu clocks and respective count */ + const struct samsung_cpu_clock *cpu_clks; + unsigned int nr_cpu_clks; /* list and number of clocks registers */ const unsigned long *clk_regs; @@ -350,6 +374,8 @@ extern void __init samsung_clk_register_gate(struct samsung_clk_provider *ctx, extern void __init samsung_clk_register_pll(struct samsung_clk_provider *ctx, const struct samsung_pll_clock *pll_list, unsigned int nr_clk, void __iomem *base); +extern void __init samsung_clk_register_cpu(struct samsung_clk_provider *ctx, + const struct samsung_cpu_clock *list, unsigned int nr_clk); extern struct samsung_clk_provider __init *samsung_cmu_register_one( struct device_node *,
Adds 'struct samsung_cpu_clock' and corresponding CPU clock registration function to the samsung common clk driver. This allows samsung clock drivers to register their CPU clocks with the samsung_cmu_register_one() API. Currently the exynos5433 apollo and atlas clks have their own custom init functions to handle registering their CPU clocks. With this patch we can drop their custom CLK_OF_DECLARE functions and directly call samsung_cmu_register_one(). Signed-off-by: Will McVicker <willmcvicker@google.com> --- drivers/clk/samsung/clk-cpu.c | 26 ++++++++++++++++++++++++++ drivers/clk/samsung/clk.c | 2 ++ drivers/clk/samsung/clk.h | 26 ++++++++++++++++++++++++++ 3 files changed, 54 insertions(+)