Re: [PATCH 3/3] clk: samsung: Use cached clk_hws instead of __clk_lookup() calls

From: Chanwoo Choi
Date: Sat Aug 29 2020 - 08:48:00 EST

Hi Sylwester,

On Thu, Aug 27, 2020 at 2:16 AM Sylwester Nawrocki
<s.nawrocki@xxxxxxxxxxx> wrote:
>
> For the CPU clock registration two parent clocks are required, these
> are now being passed as struct clk_hw pointers, rather than by the
> global scope names. That allows us to avoid __clk_lookup() calls
> and simplifies a bit the CPU clock registration function.
> While at it drop unneeded extern keyword in the function declaration.
>
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@xxxxxxxxxxx>
> ---
> drivers/clk/samsung/clk-cpu.c | 37 +++++++++++++++---------------------
> drivers/clk/samsung/clk-cpu.h | 6 +++---
> drivers/clk/samsung/clk-exynos3250.c | 6 ++++--
> drivers/clk/samsung/clk-exynos4.c | 7 +++++--
> drivers/clk/samsung/clk-exynos5250.c | 4 +++-
> drivers/clk/samsung/clk-exynos5420.c | 6 +++---
> drivers/clk/samsung/clk-exynos5433.c | 10 ++++++++--
> 7 files changed, 41 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/clk/samsung/clk-cpu.c b/drivers/clk/samsung/clk-cpu.c
> index efc4fa6..00ef4d1 100644
> --- a/drivers/clk/samsung/clk-cpu.c
> +++ b/drivers/clk/samsung/clk-cpu.c
> @@ -401,26 +401,34 @@ static int exynos5433_cpuclk_notifier_cb(struct notifier_block *nb,
>
> /* helper function to register a CPU clock */
> int __init exynos_register_cpu_clock(struct samsung_clk_provider *ctx,
> - unsigned int lookup_id, const char *name, const char *parent,
> - const char *alt_parent, unsigned long offset,
> - const struct exynos_cpuclk_cfg_data *cfg,
> + unsigned int lookup_id, const char *name,
> + const struct clk_hw *parent, const struct clk_hw *alt_parent,
> + unsigned long offset, const struct exynos_cpuclk_cfg_data *cfg,
> unsigned long num_cfgs, unsigned long flags)
> {
> struct exynos_cpuclk *cpuclk;
> struct clk_init_data init;
> - struct clk *parent_clk;
> + const char *parent_name;
> int ret = 0;
>
> + if (IS_ERR(parent) || IS_ERR(alt_parent)) {
> + pr_err("%s: invalid parent clock(s)\n", __func__);
> + return -EINVAL;
> + }
> +
> cpuclk = kzalloc(sizeof(*cpuclk), GFP_KERNEL);
> if (!cpuclk)
> return -ENOMEM;
>
> + parent_name = clk_hw_get_name(parent);
> +
> init.name = name;
> init.flags = CLK_SET_RATE_PARENT;
> - init.parent_names = &parent;
> + init.parent_names = &parent_name;
> init.num_parents = 1;
> init.ops = &exynos_cpuclk_clk_ops;
>
> + cpuclk->alt_parent = alt_parent;
> cpuclk->hw.init = &init;
> cpuclk->ctrl_base = ctx->reg_base + offset;
> cpuclk->lock = &ctx->lock;
> @@ -430,23 +438,8 @@ int __init exynos_register_cpu_clock(struct samsung_clk_provider *ctx,
> else
> cpuclk->clk_nb.notifier_call = exynos_cpuclk_notifier_cb;
>
> - cpuclk->alt_parent = __clk_get_hw(__clk_lookup(alt_parent));
> - if (!cpuclk->alt_parent) {
> - pr_err("%s: could not lookup alternate parent %s\n",
> - __func__, alt_parent);
> - ret = -EINVAL;
> - goto free_cpuclk;
> - }
> -
> - parent_clk = __clk_lookup(parent);
> - if (!parent_clk) {
> - pr_err("%s: could not lookup parent clock %s\n",
> - __func__, parent);
> - ret = -EINVAL;
> - goto free_cpuclk;
> - }
>
> - ret = clk_notifier_register(parent_clk, &cpuclk->clk_nb);
> + ret = clk_notifier_register(parent->clk, &cpuclk->clk_nb);
> if (ret) {
> pr_err("%s: failed to register clock notifier for %s\n",
> __func__, name);
> @@ -471,7 +464,7 @@ int __init exynos_register_cpu_clock(struct samsung_clk_provider *ctx,
> free_cpuclk_data:
> kfree(cpuclk->cfg);
> unregister_clk_nb:
> - clk_notifier_unregister(parent_clk, &cpuclk->clk_nb);
> + clk_notifier_unregister(parent->clk, &cpuclk->clk_nb);
> free_cpuclk:
> kfree(cpuclk);
> return ret;
> diff --git a/drivers/clk/samsung/clk-cpu.h b/drivers/clk/samsung/clk-cpu.h
> index ad38cc2..af74686 100644
> --- a/drivers/clk/samsung/clk-cpu.h
> +++ b/drivers/clk/samsung/clk-cpu.h
> @@ -46,7 +46,7 @@ struct exynos_cpuclk_cfg_data {
> */
> struct exynos_cpuclk {
> struct clk_hw hw;
> - struct clk_hw *alt_parent;
> + const struct clk_hw *alt_parent;
> void __iomem *ctrl_base;
> spinlock_t *lock;
> const struct exynos_cpuclk_cfg_data *cfg;
> @@ -62,9 +62,9 @@ struct exynos_cpuclk {
> #define CLK_CPU_HAS_E5433_REGS_LAYOUT (1 << 2)
> };
>
> -extern int __init exynos_register_cpu_clock(struct samsung_clk_provider *ctx,
> +int __init exynos_register_cpu_clock(struct samsung_clk_provider *ctx,
> unsigned int lookup_id, const char *name,
> - const char *parent, const char *alt_parent,
> + const struct clk_hw *parent, const struct clk_hw *alt_parent,
> unsigned long offset,
> const struct exynos_cpuclk_cfg_data *cfg,
> unsigned long num_cfgs, unsigned long flags);
> diff --git a/drivers/clk/samsung/clk-exynos3250.c b/drivers/clk/samsung/clk-exynos3250.c
> index 17897c7..17df7f9 100644
> --- a/drivers/clk/samsung/clk-exynos3250.c
> +++ b/drivers/clk/samsung/clk-exynos3250.c
> @@ -808,14 +808,16 @@ static const struct exynos_cpuclk_cfg_data e3250_armclk_d[] __initconst = {
> static void __init exynos3250_cmu_init(struct device_node *np)
> {
> struct samsung_clk_provider *ctx;
> + struct clk_hw **hws;
>
> ctx = samsung_cmu_register_one(np, &cmu_info);
> if (!ctx)
> return;
>
> + hws = ctx->clk_data.hws;
> exynos_register_cpu_clock(ctx, CLK_ARM_CLK, "armclk",
> - mout_core_p[0], mout_core_p[1], 0x14200,
> - e3250_armclk_d, ARRAY_SIZE(e3250_armclk_d),
> + hws[CLK_MOUT_APLL], hws[CLK_MOUT_MPLL_USER_C],
> + 0x14200, e3250_armclk_d, ARRAY_SIZE(e3250_armclk_d),
> CLK_CPU_HAS_DIV1);
>
> exynos3_core_down_clock(ctx->reg_base);
> diff --git a/drivers/clk/samsung/clk-exynos4.c b/drivers/clk/samsung/clk-exynos4.c
> index 51564fc..436fcd2 100644
> --- a/drivers/clk/samsung/clk-exynos4.c
> +++ b/drivers/clk/samsung/clk-exynos4.c
> @@ -1233,6 +1233,8 @@ static void __init exynos4_clk_init(struct device_node *np,
> enum exynos4_soc soc)
> {
> struct samsung_clk_provider *ctx;
> + struct clk_hw **hws;
> +
> exynos4_soc = soc;
>
> reg_base = of_iomap(np, 0);
> @@ -1240,6 +1242,7 @@ static void __init exynos4_clk_init(struct device_node *np,
> panic("%s: failed to map registers\n", __func__);
>
> ctx = samsung_clk_init(np, reg_base, CLK_NR_CLKS);
> + hws = ctx->clk_data.hws;
>
> samsung_clk_of_register_fixed_ext(ctx, exynos4_fixed_rate_ext_clks,
> ARRAY_SIZE(exynos4_fixed_rate_ext_clks),
> @@ -1302,7 +1305,7 @@ static void __init exynos4_clk_init(struct device_node *np,
> exynos4210_fixed_factor_clks,
> ARRAY_SIZE(exynos4210_fixed_factor_clks));
> exynos_register_cpu_clock(ctx, CLK_ARM_CLK, "armclk",
> - mout_core_p4210[0], mout_core_p4210[1], 0x14200,
> + hws[CLK_MOUT_APLL], hws[CLK_SCLK_MPLL], 0x14200,
> e4210_armclk_d, ARRAY_SIZE(e4210_armclk_d),
> CLK_CPU_NEEDS_DEBUG_ALT_DIV | CLK_CPU_HAS_DIV1);
> } else {
> @@ -1317,7 +1320,7 @@ static void __init exynos4_clk_init(struct device_node *np,
> ARRAY_SIZE(exynos4x12_fixed_factor_clks));
>
> exynos_register_cpu_clock(ctx, CLK_ARM_CLK, "armclk",
> - mout_core_p4x12[0], mout_core_p4x12[1], 0x14200,
> + hws[CLK_MOUT_APLL], hws[CLK_MOUT_MPLL_USER_C], 0x14200,
> e4412_armclk_d, ARRAY_SIZE(e4412_armclk_d),
> CLK_CPU_NEEDS_DEBUG_ALT_DIV | CLK_CPU_HAS_DIV1);
> }
> diff --git a/drivers/clk/samsung/clk-exynos5250.c b/drivers/clk/samsung/clk-exynos5250.c
> index 7bcff76..06588fa 100644
> --- a/drivers/clk/samsung/clk-exynos5250.c
> +++ b/drivers/clk/samsung/clk-exynos5250.c
> @@ -782,6 +782,7 @@ static void __init exynos5250_clk_init(struct device_node *np)
> {
> struct samsung_clk_provider *ctx;
> unsigned int tmp;
> + struct clk_hw **hws;
>
> if (np) {
> reg_base = of_iomap(np, 0);
> @@ -792,6 +793,7 @@ static void __init exynos5250_clk_init(struct device_node *np)
> }
>
> ctx = samsung_clk_init(np, reg_base, CLK_NR_CLKS);
> + hws = ctx->clk_data.hws;
>
> samsung_clk_of_register_fixed_ext(ctx, exynos5250_fixed_rate_ext_clks,
> ARRAY_SIZE(exynos5250_fixed_rate_ext_clks),
> @@ -821,7 +823,7 @@ static void __init exynos5250_clk_init(struct device_node *np)
> samsung_clk_register_gate(ctx, exynos5250_gate_clks,
> ARRAY_SIZE(exynos5250_gate_clks));
> exynos_register_cpu_clock(ctx, CLK_ARM_CLK, "armclk",
> - mout_cpu_p[0], mout_cpu_p[1], 0x200,
> + hws[CLK_MOUT_APLL], hws[CLK_MOUT_MPLL], 0x200,
> exynos5250_armclk_d, ARRAY_SIZE(exynos5250_armclk_d),
> CLK_CPU_HAS_DIV1);
>
> diff --git a/drivers/clk/samsung/clk-exynos5420.c b/drivers/clk/samsung/clk-exynos5420.c
> index d07cee2..ba4e0a4 100644
> --- a/drivers/clk/samsung/clk-exynos5420.c
> +++ b/drivers/clk/samsung/clk-exynos5420.c
> @@ -1625,15 +1625,15 @@ static void __init exynos5x_clk_init(struct device_node *np,
>
> if (soc == EXYNOS5420) {
> exynos_register_cpu_clock(ctx, CLK_ARM_CLK, "armclk",
> - mout_cpu_p[0], mout_cpu_p[1], 0x200,
> + hws[CLK_MOUT_APLL], hws[CLK_MOUT_MSPLL_CPU], 0x200,
> exynos5420_eglclk_d, ARRAY_SIZE(exynos5420_eglclk_d), 0);
> } else {
> exynos_register_cpu_clock(ctx, CLK_ARM_CLK, "armclk",
> - mout_cpu_p[0], mout_cpu_p[1], 0x200,
> + hws[CLK_MOUT_APLL], hws[CLK_MOUT_MSPLL_CPU], 0x200,
> exynos5800_eglclk_d, ARRAY_SIZE(exynos5800_eglclk_d), 0);
> }
> exynos_register_cpu_clock(ctx, CLK_KFC_CLK, "kfcclk",
> - mout_kfc_p[0], mout_kfc_p[1], 0x28200,
> + hws[CLK_MOUT_KPLL], hws[CLK_MOUT_MSPLL_KFC], 0x28200,
> exynos5420_kfcclk_d, ARRAY_SIZE(exynos5420_kfcclk_d), 0);
>
> samsung_clk_extended_sleep_init(reg_base,
> diff --git a/drivers/clk/samsung/clk-exynos5433.c b/drivers/clk/samsung/clk-exynos5433.c
> index 6f29ecd..f203074 100644
> --- a/drivers/clk/samsung/clk-exynos5433.c
> +++ b/drivers/clk/samsung/clk-exynos5433.c
> @@ -3679,6 +3679,7 @@ static void __init exynos5433_cmu_apollo_init(struct device_node *np)
> {
> void __iomem *reg_base;
> struct samsung_clk_provider *ctx;
> + struct clk_hw **hws;
>
> reg_base = of_iomap(np, 0);
> if (!reg_base) {
> @@ -3701,8 +3702,10 @@ static void __init exynos5433_cmu_apollo_init(struct device_node *np)
> samsung_clk_register_gate(ctx, apollo_gate_clks,
> ARRAY_SIZE(apollo_gate_clks));
>
> + hws = ctx->clk_data.hws;
> +
> exynos_register_cpu_clock(ctx, CLK_SCLK_APOLLO, "apolloclk",
> - mout_apollo_p[0], mout_apollo_p[1], 0x200,
> + hws[CLK_MOUT_APOLLO_PLL], hws[CLK_MOUT_BUS_PLL_APOLLO_USER], 0x200,
> exynos5433_apolloclk_d, ARRAY_SIZE(exynos5433_apolloclk_d),
> CLK_CPU_HAS_E5433_REGS_LAYOUT);
>
> @@ -3933,6 +3936,7 @@ static void __init exynos5433_cmu_atlas_init(struct device_node *np)
> {
> void __iomem *reg_base;
> struct samsung_clk_provider *ctx;
> + struct clk_hw **hws;
>
> reg_base = of_iomap(np, 0);
> if (!reg_base) {
> @@ -3955,8 +3959,10 @@ static void __init exynos5433_cmu_atlas_init(struct device_node *np)
> samsung_clk_register_gate(ctx, atlas_gate_clks,
> ARRAY_SIZE(atlas_gate_clks));
>
> + hws = ctx->clk_data.hws;
> +
> exynos_register_cpu_clock(ctx, CLK_SCLK_ATLAS, "atlasclk",
> - mout_atlas_p[0], mout_atlas_p[1], 0x200,
> + hws[CLK_MOUT_ATLAS_PLL], hws[CLK_MOUT_BUS_PLL_ATLAS_USER], 0x200,
> exynos5433_atlasclk_d, ARRAY_SIZE(exynos5433_atlasclk_d),
> CLK_CPU_HAS_E5433_REGS_LAYOUT);
>
> --
> 2.7.4
>

Acked-by: Chanwoo Choi <cw00.choi@xxxxxxxxxxx>

--
Best Regards,
Chanwoo Choi
Samsung Electronics