[RFT PATCH v3 0/2] clk: samsung: add common support for CPU clocks

[RFT PATCH v3 0/2] clk: samsung: add common support for CPU clocks

[Will McVicker]

These two patches were originally a part of the series [1]. They add
support to the common samsung clock driver to parse and register the
CPU clocks when provided. This allows for the samsung clock drivers to
simply provide a `struct samsung_cpu_clock` to `struct samsung_cmu_info`
and then call samsung_cmu_register_one(). With this new support, we can
now get rid of the custom apollo and atlas CLK_OF_DECLARE init functions.

Since I don't have the hardware to test these, I'm including the RFT tag
to try and get help testing and verifying these.

[1] https://lore.kernel.org/all/20210928235635.1348330-4-willmcvicker@google.com/

Will McVicker (2):
  [RFT] clk: samsung: add support for CPU clocks
  [RFT] clk: samsung: exynos5433: update apollo and atlas clock probing

 drivers/clk/samsung/clk-cpu.c        |  26 ++++++
 drivers/clk/samsung/clk-exynos5433.c | 120 +++++++++++----------------
 drivers/clk/samsung/clk.c            |   2 +
 drivers/clk/samsung/clk.h            |  26 ++++++
 4 files changed, 102 insertions(+), 72 deletions(-)

-- 
2.33.0.882.g93a45727a2-goog

[PATCH v3 1/2] [RFT] clk: samsung: add support for CPU clocks

[Will McVicker]

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(+)

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 *,
-- 
2.33.0.882.g93a45727a2-goog

[PATCH v3 2/2] [RFT] clk: samsung: exynos5433: update apollo and atlas clock probing

[Will McVicker]

Use the samsung common clk driver to initialize and probe the apollo and
atlas clocks. This removes their custom init functions and uses the
samsung_cmu_register_one() instead.

Signed-off-by: Will McVicker <willmcvicker@google.com>
---
 drivers/clk/samsung/clk-exynos5433.c | 120 +++++++++++----------------
 1 file changed, 48 insertions(+), 72 deletions(-)

diff --git a/drivers/clk/samsung/clk-exynos5433.c b/drivers/clk/samsung/clk-exynos5433.c
index f203074d858b..0ef809c9e2da 100644
--- a/drivers/clk/samsung/clk-exynos5433.c
+++ b/drivers/clk/samsung/clk-exynos5433.c
@@ -3675,44 +3675,32 @@ static const struct exynos_cpuclk_cfg_data exynos5433_apolloclk_d[] __initconst
 	{  0 },
 };
 
+static const struct samsung_cpu_clock apollo_cpu_clks[] __initconst = {
+	CPU_CLK(CLK_SCLK_APOLLO, "apolloclk", "mout_apollo_pll",
+			"mout_bus_pll_apollo_user",
+			CLK_CPU_HAS_E5433_REGS_LAYOUT, 0x200,
+			exynos5433_apolloclk_d),
+};
+
+static const struct samsung_cmu_info apollo_cmu_info __initconst = {
+	.pll_clks	= apollo_pll_clks,
+	.nr_pll_clks	= ARRAY_SIZE(apollo_pll_clks),
+	.mux_clks	= apollo_mux_clks,
+	.nr_mux_clks	= ARRAY_SIZE(apollo_mux_clks),
+	.div_clks	= apollo_div_clks,
+	.nr_div_clks	= ARRAY_SIZE(apollo_div_clks),
+	.gate_clks	= apollo_gate_clks,
+	.nr_gate_clks	= ARRAY_SIZE(apollo_gate_clks),
+	.cpu_clks	= apollo_cpu_clks,
+	.nr_cpu_clks	= ARRAY_SIZE(apollo_cpu_clks),
+	.nr_clk_ids	= APOLLO_NR_CLK,
+	.clk_regs	= apollo_clk_regs,
+	.nr_clk_regs	= ARRAY_SIZE(apollo_clk_regs),
+};
+
 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) {
-		panic("%s: failed to map registers\n", __func__);
-		return;
-	}
-
-	ctx = samsung_clk_init(np, reg_base, APOLLO_NR_CLK);
-	if (!ctx) {
-		panic("%s: unable to allocate ctx\n", __func__);
-		return;
-	}
-
-	samsung_clk_register_pll(ctx, apollo_pll_clks,
-				 ARRAY_SIZE(apollo_pll_clks), reg_base);
-	samsung_clk_register_mux(ctx, apollo_mux_clks,
-				 ARRAY_SIZE(apollo_mux_clks));
-	samsung_clk_register_div(ctx, apollo_div_clks,
-				 ARRAY_SIZE(apollo_div_clks));
-	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",
-		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);
-
-	samsung_clk_sleep_init(reg_base, apollo_clk_regs,
-			       ARRAY_SIZE(apollo_clk_regs));
-
-	samsung_clk_of_add_provider(np, ctx);
+	samsung_cmu_register_one(np, &apollo_cmu_info);
 }
 CLK_OF_DECLARE(exynos5433_cmu_apollo, "samsung,exynos5433-cmu-apollo",
 		exynos5433_cmu_apollo_init);
@@ -3932,44 +3920,32 @@ static const struct exynos_cpuclk_cfg_data exynos5433_atlasclk_d[] __initconst =
 	{  0 },
 };
 
-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) {
-		panic("%s: failed to map registers\n", __func__);
-		return;
-	}
-
-	ctx = samsung_clk_init(np, reg_base, ATLAS_NR_CLK);
-	if (!ctx) {
-		panic("%s: unable to allocate ctx\n", __func__);
-		return;
-	}
-
-	samsung_clk_register_pll(ctx, atlas_pll_clks,
-				 ARRAY_SIZE(atlas_pll_clks), reg_base);
-	samsung_clk_register_mux(ctx, atlas_mux_clks,
-				 ARRAY_SIZE(atlas_mux_clks));
-	samsung_clk_register_div(ctx, atlas_div_clks,
-				 ARRAY_SIZE(atlas_div_clks));
-	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",
-		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);
+static const struct samsung_cpu_clock atlas_cpu_clks[] __initconst = {
+	CPU_CLK(CLK_SCLK_ATLAS, "atlasclk", "mout_atlas_pll",
+			"mout_bus_pll_atlas_user",
+			CLK_CPU_HAS_E5433_REGS_LAYOUT, 0x200,
+			exynos5433_atlasclk_d),
+};
 
-	samsung_clk_sleep_init(reg_base, atlas_clk_regs,
-			       ARRAY_SIZE(atlas_clk_regs));
+static const struct samsung_cmu_info atlas_cmu_info __initconst = {
+	.pll_clks	= atlas_pll_clks,
+	.nr_pll_clks	= ARRAY_SIZE(atlas_pll_clks),
+	.mux_clks	= atlas_mux_clks,
+	.nr_mux_clks	= ARRAY_SIZE(atlas_mux_clks),
+	.div_clks	= atlas_div_clks,
+	.nr_div_clks	= ARRAY_SIZE(atlas_div_clks),
+	.gate_clks	= atlas_gate_clks,
+	.nr_gate_clks	= ARRAY_SIZE(atlas_gate_clks),
+	.cpu_clks	= atlas_cpu_clks,
+	.nr_cpu_clks	= ARRAY_SIZE(atlas_cpu_clks),
+	.nr_clk_ids	= ATLAS_NR_CLK,
+	.clk_regs	= atlas_clk_regs,
+	.nr_clk_regs	= ARRAY_SIZE(atlas_clk_regs),
+};
 
-	samsung_clk_of_add_provider(np, ctx);
+static void __init exynos5433_cmu_atlas_init(struct device_node *np)
+{
+	samsung_cmu_register_one(np, &atlas_cmu_info);
 }
 CLK_OF_DECLARE(exynos5433_cmu_atlas, "samsung,exynos5433-cmu-atlas",
 		exynos5433_cmu_atlas_init);
-- 
2.33.0.882.g93a45727a2-goog

Re: [PATCH v3 1/2] [RFT] clk: samsung: add support for CPU clocks

[Stephen Boyd]

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);
>

Re: [RFT PATCH v3 0/2] clk: samsung: add common support for CPU clocks

[Marek Szyprowski]

On 14.10.2021 00:10, Will McVicker wrote:
> These two patches were originally a part of the series [1]. They add
> support to the common samsung clock driver to parse and register the
> CPU clocks when provided. This allows for the samsung clock drivers to
> simply provide a `struct samsung_cpu_clock` to `struct samsung_cmu_info`
> and then call samsung_cmu_register_one(). With this new support, we can
> now get rid of the custom apollo and atlas CLK_OF_DECLARE init functions.
>
> Since I don't have the hardware to test these, I'm including the RFT tag
> to try and get help testing and verifying these.
>
> [1] https://protect2.fireeye.com/v1/url?k=91329df7-cea9a478-913316b8-0cc47a31307c-8e0b8e6442100c5a&q=1&e=50af1e33-8bdf-429f-9e54-434d425998d6&u=https%3A%2F%2Flore.kernel.org%2Fall%2F20210928235635.1348330-4-willmcvicker%40google.com%2F

Works fine on my Exynos5433 TM2e test board.

Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>

> Will McVicker (2):
>    [RFT] clk: samsung: add support for CPU clocks
>    [RFT] clk: samsung: exynos5433: update apollo and atlas clock probing
>
>   drivers/clk/samsung/clk-cpu.c        |  26 ++++++
>   drivers/clk/samsung/clk-exynos5433.c | 120 +++++++++++----------------
>   drivers/clk/samsung/clk.c            |   2 +
>   drivers/clk/samsung/clk.h            |  26 ++++++
>   4 files changed, 102 insertions(+), 72 deletions(-)
>
Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH v3 1/2] [RFT] clk: samsung: add support for CPU clocks

[Will McVicker]

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);
> >

Re: [RFT PATCH v3 0/2] clk: samsung: add common support for CPU clocks

[Will McVicker]

On Thu, Oct 14, 2021 at 1:33 AM Marek Szyprowski
<m.szyprowski@samsung.com> wrote:
>
> On 14.10.2021 00:10, Will McVicker wrote:
> > These two patches were originally a part of the series [1]. They add
> > support to the common samsung clock driver to parse and register the
> > CPU clocks when provided. This allows for the samsung clock drivers to
> > simply provide a `struct samsung_cpu_clock` to `struct samsung_cmu_info`
> > and then call samsung_cmu_register_one(). With this new support, we can
> > now get rid of the custom apollo and atlas CLK_OF_DECLARE init functions.
> >
> > Since I don't have the hardware to test these, I'm including the RFT tag
> > to try and get help testing and verifying these.
> >
> > [1] https://protect2.fireeye.com/v1/url?k=91329df7-cea9a478-913316b8-0cc47a31307c-8e0b8e6442100c5a&q=1&e=50af1e33-8bdf-429f-9e54-434d425998d6&u=https%3A%2F%2Flore.kernel.org%2Fall%2F20210928235635.1348330-4-willmcvicker%40google.com%2F
>
> Works fine on my Exynos5433 TM2e test board.
>
> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>

Thanks for testing!

>
> > Will McVicker (2):
> >    [RFT] clk: samsung: add support for CPU clocks
> >    [RFT] clk: samsung: exynos5433: update apollo and atlas clock probing
> >
> >   drivers/clk/samsung/clk-cpu.c        |  26 ++++++
> >   drivers/clk/samsung/clk-exynos5433.c | 120 +++++++++++----------------
> >   drivers/clk/samsung/clk.c            |   2 +
> >   drivers/clk/samsung/clk.h            |  26 ++++++
> >   4 files changed, 102 insertions(+), 72 deletions(-)
> >
> Best regards
> --
> Marek Szyprowski, PhD
> Samsung R&D Institute Poland
>

Re: [PATCH v3 1/2] [RFT] clk: samsung: add support for CPU clocks

[Stephen Boyd]

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.

Re: [PATCH v3 1/2] [RFT] clk: samsung: add support for CPU clocks

[Sylwester Nawrocki]

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.