[PATCH AUTOSEL 5.16 02/52] clk: samsung: exynos850: Register clocks early

[PATCH AUTOSEL 5.16 02/52] clk: samsung: exynos850: Register clocks early

[Sasha Levin]

From: Sam Protsenko <semen.protsenko@linaro.org>

[ Upstream commit bcda841f9bf2cddcf2f000cba96f2e27f6f2bdbf ]

Some clocks must be registered before init calls. For example MCT clock
(from CMU_PERI) is needed for MCT timer driver, which is registered
with TIMER_OF_DECLARE(). By the time we get to core_initcall() used for
clk-exynos850 platform driver init, it's already too late. Inability to
get "mct" clock in MCT driver leads to kernel panic, as functions
registered with *_OF_DECLARE() can't do deferred calls. MCT timer driver
can't be fixed either, as it's acting as a clock source and it's
essential to register it in start_kernel() -> time_init().

Let's register CMU_PERI clocks early, using CLK_OF_DECLARE(). CMU_TOP
generates clocks needed for CMU_PERI, but it's already registered early.

While at it, let's cleanup the code a bit, by extracting everything
related to CMU initialization and registration to the separate function.

Similar issue was discussed at [1] and addressed in commit 1f7db7bbf031
("clk: renesas: cpg-mssr: Add early clock support"), as well as in
drivers/clk/mediatek/clk-mt2712.c.

[1] https://patchwork.kernel.org/project/linux-renesas-soc/patch/20180829132954.64862-2-chris.brandt@renesas.com/

Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
Link: https://lore.kernel.org/r/20211122144206.23134-1-semen.protsenko@linaro.org
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/clk/samsung/clk-exynos850.c | 70 ++++++++++++++++++++---------
 1 file changed, 49 insertions(+), 21 deletions(-)

diff --git a/drivers/clk/samsung/clk-exynos850.c b/drivers/clk/samsung/clk-exynos850.c
index 2294989e244c5..79cce8ba88831 100644
--- a/drivers/clk/samsung/clk-exynos850.c
+++ b/drivers/clk/samsung/clk-exynos850.c
@@ -60,6 +60,43 @@ static void __init exynos850_init_clocks(struct device_node *np,
 	iounmap(reg_base);
 }
 
+/**
+ * exynos850_register_cmu - Register specified Exynos850 CMU domain
+ * @dev:	Device object; may be NULL if this function is not being
+ *		called from platform driver probe function
+ * @np:		CMU device tree node
+ * @cmu:	CMU data
+ *
+ * Register specified CMU domain, which includes next steps:
+ *
+ * 1. Enable parent clock of @cmu CMU
+ * 2. Set initial registers configuration for @cmu CMU clocks
+ * 3. Register @cmu CMU clocks using Samsung clock framework API
+ */
+static void __init exynos850_register_cmu(struct device *dev,
+		struct device_node *np, const struct samsung_cmu_info *cmu)
+{
+	/* Keep CMU parent clock running (needed for CMU registers access) */
+	if (cmu->clk_name) {
+		struct clk *parent_clk;
+
+		if (dev)
+			parent_clk = clk_get(dev, cmu->clk_name);
+		else
+			parent_clk = of_clk_get_by_name(np, cmu->clk_name);
+
+		if (IS_ERR(parent_clk)) {
+			pr_err("%s: could not find bus clock %s; err = %ld\n",
+			       __func__, cmu->clk_name, PTR_ERR(parent_clk));
+		} else {
+			clk_prepare_enable(parent_clk);
+		}
+	}
+
+	exynos850_init_clocks(np, cmu->clk_regs, cmu->nr_clk_regs);
+	samsung_cmu_register_one(np, cmu);
+}
+
 /* ---- CMU_TOP ------------------------------------------------------------- */
 
 /* Register Offset definitions for CMU_TOP (0x120e0000) */
@@ -347,10 +384,10 @@ static const struct samsung_cmu_info top_cmu_info __initconst = {
 
 static void __init exynos850_cmu_top_init(struct device_node *np)
 {
-	exynos850_init_clocks(np, top_clk_regs, ARRAY_SIZE(top_clk_regs));
-	samsung_cmu_register_one(np, &top_cmu_info);
+	exynos850_register_cmu(NULL, np, &top_cmu_info);
 }
 
+/* Register CMU_TOP early, as it's a dependency for other early domains */
 CLK_OF_DECLARE(exynos850_cmu_top, "samsung,exynos850-cmu-top",
 	       exynos850_cmu_top_init);
 
@@ -615,6 +652,15 @@ static const struct samsung_cmu_info peri_cmu_info __initconst = {
 	.clk_name		= "dout_peri_bus",
 };
 
+static void __init exynos850_cmu_peri_init(struct device_node *np)
+{
+	exynos850_register_cmu(NULL, np, &peri_cmu_info);
+}
+
+/* Register CMU_PERI early, as it's needed for MCT timer */
+CLK_OF_DECLARE(exynos850_cmu_peri, "samsung,exynos850-cmu-peri",
+	       exynos850_cmu_peri_init);
+
 /* ---- CMU_CORE ------------------------------------------------------------ */
 
 /* Register Offset definitions for CMU_CORE (0x12000000) */
@@ -779,24 +825,9 @@ static int __init exynos850_cmu_probe(struct platform_device *pdev)
 {
 	const struct samsung_cmu_info *info;
 	struct device *dev = &pdev->dev;
-	struct device_node *np = dev->of_node;
 
 	info = of_device_get_match_data(dev);
-	exynos850_init_clocks(np, info->clk_regs, info->nr_clk_regs);
-	samsung_cmu_register_one(np, info);
-
-	/* Keep bus clock running, so it's possible to access CMU registers */
-	if (info->clk_name) {
-		struct clk *bus_clk;
-
-		bus_clk = clk_get(dev, info->clk_name);
-		if (IS_ERR(bus_clk)) {
-			pr_err("%s: could not find bus clock %s; err = %ld\n",
-			       __func__, info->clk_name, PTR_ERR(bus_clk));
-		} else {
-			clk_prepare_enable(bus_clk);
-		}
-	}
+	exynos850_register_cmu(dev, dev->of_node, info);
 
 	return 0;
 }
@@ -806,9 +837,6 @@ static const struct of_device_id exynos850_cmu_of_match[] = {
 	{
 		.compatible = "samsung,exynos850-cmu-hsi",
 		.data = &hsi_cmu_info,
-	}, {
-		.compatible = "samsung,exynos850-cmu-peri",
-		.data = &peri_cmu_info,
 	}, {
 		.compatible = "samsung,exynos850-cmu-core",
 		.data = &core_cmu_info,
-- 
2.34.1

Re: [PATCH AUTOSEL 5.16 02/52] clk: samsung: exynos850: Register clocks early

[Krzysztof Kozlowski]

On 17/01/2022 17:58, Sasha Levin wrote:
> From: Sam Protsenko <semen.protsenko@linaro.org>
> 
> [ Upstream commit bcda841f9bf2cddcf2f000cba96f2e27f6f2bdbf ]
> 
> Some clocks must be registered before init calls. For example MCT clock
> (from CMU_PERI) is needed for MCT timer driver, which is registered
> with TIMER_OF_DECLARE(). By the time we get to core_initcall() used for
> clk-exynos850 platform driver init, it's already too late. Inability to
> get "mct" clock in MCT driver leads to kernel panic, as functions
> registered with *_OF_DECLARE() can't do deferred calls. MCT timer driver
> can't be fixed either, as it's acting as a clock source and it's
> essential to register it in start_kernel() -> time_init().
> 
> Let's register CMU_PERI clocks early, using CLK_OF_DECLARE(). CMU_TOP
> generates clocks needed for CMU_PERI, but it's already registered early.
> 
> While at it, let's cleanup the code a bit, by extracting everything
> related to CMU initialization and registration to the separate function.
> 
> Similar issue was discussed at [1] and addressed in commit 1f7db7bbf031
> ("clk: renesas: cpg-mssr: Add early clock support"), as well as in
> drivers/clk/mediatek/clk-mt2712.c.
> 
> [1] https://patchwork.kernel.org/project/linux-renesas-soc/patch/20180829132954.64862-2-chris.brandt@renesas.com/
> 
> Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
> Link: https://lore.kernel.org/r/20211122144206.23134-1-semen.protsenko@linaro.org
> Signed-off-by: Sasha Levin <sashal@kernel.org>
> ---
>  drivers/clk/samsung/clk-exynos850.c | 70 ++++++++++++++++++++---------
>  1 file changed, 49 insertions(+), 21 deletions(-)
> 

I propose to skip this one.

Backporting it to v5.16 does not hurt but also does not bring any
benefits for the upstream kernel users. There is no support for
mentioned Exynos850 in v5.16.

It could have only meaning for some downstream, out-of-tree kernels
which apply Exynos850 support on top of v5.16, but then they can just
take this patch as well.


Best regards,
Krzysztof

Re: [PATCH AUTOSEL 5.16 02/52] clk: samsung: exynos850: Register clocks early

[Sam Protsenko]

On Mon, 17 Jan 2022 at 19:11, Krzysztof Kozlowski
<krzysztof.kozlowski@canonical.com> wrote:
>
> On 17/01/2022 17:58, Sasha Levin wrote:
> > From: Sam Protsenko <semen.protsenko@linaro.org>
> >
> > [ Upstream commit bcda841f9bf2cddcf2f000cba96f2e27f6f2bdbf ]
> >
> > Some clocks must be registered before init calls. For example MCT clock
> > (from CMU_PERI) is needed for MCT timer driver, which is registered
> > with TIMER_OF_DECLARE(). By the time we get to core_initcall() used for
> > clk-exynos850 platform driver init, it's already too late. Inability to
> > get "mct" clock in MCT driver leads to kernel panic, as functions
> > registered with *_OF_DECLARE() can't do deferred calls. MCT timer driver
> > can't be fixed either, as it's acting as a clock source and it's
> > essential to register it in start_kernel() -> time_init().
> >
> > Let's register CMU_PERI clocks early, using CLK_OF_DECLARE(). CMU_TOP
> > generates clocks needed for CMU_PERI, but it's already registered early.
> >
> > While at it, let's cleanup the code a bit, by extracting everything
> > related to CMU initialization and registration to the separate function.
> >
> > Similar issue was discussed at [1] and addressed in commit 1f7db7bbf031
> > ("clk: renesas: cpg-mssr: Add early clock support"), as well as in
> > drivers/clk/mediatek/clk-mt2712.c.
> >
> > [1] https://patchwork.kernel.org/project/linux-renesas-soc/patch/20180829132954.64862-2-chris.brandt@renesas.com/
> >
> > Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
> > Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
> > Link: https://lore.kernel.org/r/20211122144206.23134-1-semen.protsenko@linaro.org
> > Signed-off-by: Sasha Levin <sashal@kernel.org>
> > ---
> >  drivers/clk/samsung/clk-exynos850.c | 70 ++++++++++++++++++++---------
> >  1 file changed, 49 insertions(+), 21 deletions(-)
> >
>
> I propose to skip this one.
>
> Backporting it to v5.16 does not hurt but also does not bring any
> benefits for the upstream kernel users. There is no support for
> mentioned Exynos850 in v5.16.
>
> It could have only meaning for some downstream, out-of-tree kernels
> which apply Exynos850 support on top of v5.16, but then they can just
> take this patch as well.
>

Agreed. DTS patches will be merged only in v5.17, hopefully. Till that
time the whole clock driver is floating with no users. That's
historical thing -- I didn't have "Ack" to submit board dts at the
time, and SoC dts couldn't be applied without users (board dts). So I
focused on driver work, isolated. Not much sense to backport something
without having real users.

>
> Best regards,
> Krzysztof

Re: [PATCH AUTOSEL 5.16 02/52] clk: samsung: exynos850: Register clocks early

[Sasha Levin]

On Mon, Jan 17, 2022 at 09:18:43PM +0200, Sam Protsenko wrote:
>On Mon, 17 Jan 2022 at 19:11, Krzysztof Kozlowski
><krzysztof.kozlowski@canonical.com> wrote:
>>
>> On 17/01/2022 17:58, Sasha Levin wrote:
>> > From: Sam Protsenko <semen.protsenko@linaro.org>
>> >
>> > [ Upstream commit bcda841f9bf2cddcf2f000cba96f2e27f6f2bdbf ]
>> >
>> > Some clocks must be registered before init calls. For example MCT clock
>> > (from CMU_PERI) is needed for MCT timer driver, which is registered
>> > with TIMER_OF_DECLARE(). By the time we get to core_initcall() used for
>> > clk-exynos850 platform driver init, it's already too late. Inability to
>> > get "mct" clock in MCT driver leads to kernel panic, as functions
>> > registered with *_OF_DECLARE() can't do deferred calls. MCT timer driver
>> > can't be fixed either, as it's acting as a clock source and it's
>> > essential to register it in start_kernel() -> time_init().
>> >
>> > Let's register CMU_PERI clocks early, using CLK_OF_DECLARE(). CMU_TOP
>> > generates clocks needed for CMU_PERI, but it's already registered early.
>> >
>> > While at it, let's cleanup the code a bit, by extracting everything
>> > related to CMU initialization and registration to the separate function.
>> >
>> > Similar issue was discussed at [1] and addressed in commit 1f7db7bbf031
>> > ("clk: renesas: cpg-mssr: Add early clock support"), as well as in
>> > drivers/clk/mediatek/clk-mt2712.c.
>> >
>> > [1] https://patchwork.kernel.org/project/linux-renesas-soc/patch/20180829132954.64862-2-chris.brandt@renesas.com/
>> >
>> > Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
>> > Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
>> > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
>> > Link: https://lore.kernel.org/r/20211122144206.23134-1-semen.protsenko@linaro.org
>> > Signed-off-by: Sasha Levin <sashal@kernel.org>
>> > ---
>> >  drivers/clk/samsung/clk-exynos850.c | 70 ++++++++++++++++++++---------
>> >  1 file changed, 49 insertions(+), 21 deletions(-)
>> >
>>
>> I propose to skip this one.
>>
>> Backporting it to v5.16 does not hurt but also does not bring any
>> benefits for the upstream kernel users. There is no support for
>> mentioned Exynos850 in v5.16.
>>
>> It could have only meaning for some downstream, out-of-tree kernels
>> which apply Exynos850 support on top of v5.16, but then they can just
>> take this patch as well.
>>
>
>Agreed. DTS patches will be merged only in v5.17, hopefully. Till that
>time the whole clock driver is floating with no users. That's
>historical thing -- I didn't have "Ack" to submit board dts at the
>time, and SoC dts couldn't be applied without users (board dts). So I
>focused on driver work, isolated. Not much sense to backport something
>without having real users.

Dropped, thanks!

-- 
Thanks,
Sasha