All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sam Protsenko <semen.protsenko@linaro.org>
To: Sylwester Nawrocki <s.nawrocki@samsung.com>,
	Tomasz Figa <tomasz.figa@gmail.com>,
	Chanwoo Choi <cw00.choi@samsung.com>,
	Michael Turquette <mturquette@baylibre.com>,
	Stephen Boyd <sboyd@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
Cc: Matthias Brugger <matthias.bgg@gmail.com>,
	linux-samsung-soc@vger.kernel.org, linux-clk@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: [PATCH] clk: samsung: exynos850: Register clocks early
Date: Sat, 23 Oct 2021 00:56:35 +0300	[thread overview]
Message-ID: <20211022215635.31128-1-semen.protsenko@linaro.org> (raw)

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 all Exynos850 clocks early, using
CLK_OF_DECLARE_DRIVER(), and do all stuff relying on "struct dev" object
(like runtime PM and enabling bus clock) later in platform driver probe.
Basically CLK_OF_DECLARE_DRIVER() matches CMU compatible, but clears
OF_POPULATED flag, which allows the same device to be matched again
later.

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>
---
Notes:
  - This patch should be applied on top of CMU_APM series
    (clk: samsung: exynos850: Implement CMU_APM domain)
  - I considered introducing some macro to reduce the code duplication
    (actually created one), but decided to go with plain code: this way
    it's easier to understand and navigate
  - Also considered registering only *some* clocks early: won't be
    possible, as leaf clocks (like MCT) depend on the whole clock tree.
    Registering early only *some* CMUs (like CMU_PERI) looks possible,
    but I'm not sure what is the best way to implement this, and that
    won't be consistent with other CMUs (too much code variation as for
    my taste)

 drivers/clk/samsung/clk-exynos850.c | 48 +++++++++++++++++++++++++++--
 1 file changed, 45 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/samsung/clk-exynos850.c b/drivers/clk/samsung/clk-exynos850.c
index 95e373d17b42..255bc2bcf709 100644
--- a/drivers/clk/samsung/clk-exynos850.c
+++ b/drivers/clk/samsung/clk-exynos850.c
@@ -492,6 +492,15 @@ static const struct samsung_cmu_info apm_cmu_info __initconst = {
 	.clk_name		= "dout_clkcmu_apm_bus",
 };
 
+static void __init exynos850_cmu_apm_init(struct device_node *np)
+{
+	exynos850_init_clocks(np, apm_clk_regs, ARRAY_SIZE(apm_clk_regs));
+	samsung_cmu_register_one(np, &apm_cmu_info);
+}
+
+CLK_OF_DECLARE_DRIVER(exynos850_cmu_apm, "samsung,exynos850-cmu-apm",
+		      exynos850_cmu_apm_init);
+
 /* ---- CMU_HSI ------------------------------------------------------------- */
 
 /* Register Offset definitions for CMU_HSI (0x13400000) */
@@ -579,6 +588,15 @@ static const struct samsung_cmu_info hsi_cmu_info __initconst = {
 	.clk_name		= "dout_hsi_bus",
 };
 
+static void __init exynos850_cmu_hsi_init(struct device_node *np)
+{
+	exynos850_init_clocks(np, hsi_clk_regs, ARRAY_SIZE(hsi_clk_regs));
+	samsung_cmu_register_one(np, &hsi_cmu_info);
+}
+
+CLK_OF_DECLARE_DRIVER(exynos850_cmu_hsi, "samsung,exynos850-cmu-hsi",
+		      exynos850_cmu_hsi_init);
+
 /* ---- CMU_PERI ------------------------------------------------------------ */
 
 /* Register Offset definitions for CMU_PERI (0x10030000) */
@@ -753,6 +771,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_init_clocks(np, peri_clk_regs, ARRAY_SIZE(peri_clk_regs));
+	samsung_cmu_register_one(np, &peri_cmu_info);
+}
+
+CLK_OF_DECLARE_DRIVER(exynos850_cmu_peri, "samsung,exynos850-cmu-peri",
+		      exynos850_cmu_peri_init);
+
 /* ---- CMU_CORE ------------------------------------------------------------ */
 
 /* Register Offset definitions for CMU_CORE (0x12000000) */
@@ -839,6 +866,15 @@ static const struct samsung_cmu_info core_cmu_info __initconst = {
 	.clk_name		= "dout_core_bus",
 };
 
+static void __init exynos850_cmu_core_init(struct device_node *np)
+{
+	exynos850_init_clocks(np, core_clk_regs, ARRAY_SIZE(core_clk_regs));
+	samsung_cmu_register_one(np, &core_cmu_info);
+}
+
+CLK_OF_DECLARE_DRIVER(exynos850_cmu_core, "samsung,exynos850-cmu-core",
+		      exynos850_cmu_core_init);
+
 /* ---- CMU_DPU ------------------------------------------------------------- */
 
 /* Register Offset definitions for CMU_DPU (0x13000000) */
@@ -911,17 +947,23 @@ static const struct samsung_cmu_info dpu_cmu_info __initconst = {
 	.clk_name		= "dout_dpu",
 };
 
+static void __init exynos850_cmu_dpu_init(struct device_node *np)
+{
+	exynos850_init_clocks(np, dpu_clk_regs, ARRAY_SIZE(dpu_clk_regs));
+	samsung_cmu_register_one(np, &dpu_cmu_info);
+}
+
+CLK_OF_DECLARE_DRIVER(exynos850_cmu_dpu, "samsung,exynos850-cmu-dpu",
+		      exynos850_cmu_dpu_init);
+
 /* ---- platform_driver ----------------------------------------------------- */
 
 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) {
-- 
2.30.2


WARNING: multiple messages have this Message-ID (diff)
From: Sam Protsenko <semen.protsenko@linaro.org>
To: Sylwester Nawrocki <s.nawrocki@samsung.com>,
	Tomasz Figa <tomasz.figa@gmail.com>,
	Chanwoo Choi <cw00.choi@samsung.com>,
	Michael Turquette <mturquette@baylibre.com>,
	Stephen Boyd <sboyd@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
Cc: Matthias Brugger <matthias.bgg@gmail.com>,
	linux-samsung-soc@vger.kernel.org, linux-clk@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: [PATCH] clk: samsung: exynos850: Register clocks early
Date: Sat, 23 Oct 2021 00:56:35 +0300	[thread overview]
Message-ID: <20211022215635.31128-1-semen.protsenko@linaro.org> (raw)

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 all Exynos850 clocks early, using
CLK_OF_DECLARE_DRIVER(), and do all stuff relying on "struct dev" object
(like runtime PM and enabling bus clock) later in platform driver probe.
Basically CLK_OF_DECLARE_DRIVER() matches CMU compatible, but clears
OF_POPULATED flag, which allows the same device to be matched again
later.

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>
---
Notes:
  - This patch should be applied on top of CMU_APM series
    (clk: samsung: exynos850: Implement CMU_APM domain)
  - I considered introducing some macro to reduce the code duplication
    (actually created one), but decided to go with plain code: this way
    it's easier to understand and navigate
  - Also considered registering only *some* clocks early: won't be
    possible, as leaf clocks (like MCT) depend on the whole clock tree.
    Registering early only *some* CMUs (like CMU_PERI) looks possible,
    but I'm not sure what is the best way to implement this, and that
    won't be consistent with other CMUs (too much code variation as for
    my taste)

 drivers/clk/samsung/clk-exynos850.c | 48 +++++++++++++++++++++++++++--
 1 file changed, 45 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/samsung/clk-exynos850.c b/drivers/clk/samsung/clk-exynos850.c
index 95e373d17b42..255bc2bcf709 100644
--- a/drivers/clk/samsung/clk-exynos850.c
+++ b/drivers/clk/samsung/clk-exynos850.c
@@ -492,6 +492,15 @@ static const struct samsung_cmu_info apm_cmu_info __initconst = {
 	.clk_name		= "dout_clkcmu_apm_bus",
 };
 
+static void __init exynos850_cmu_apm_init(struct device_node *np)
+{
+	exynos850_init_clocks(np, apm_clk_regs, ARRAY_SIZE(apm_clk_regs));
+	samsung_cmu_register_one(np, &apm_cmu_info);
+}
+
+CLK_OF_DECLARE_DRIVER(exynos850_cmu_apm, "samsung,exynos850-cmu-apm",
+		      exynos850_cmu_apm_init);
+
 /* ---- CMU_HSI ------------------------------------------------------------- */
 
 /* Register Offset definitions for CMU_HSI (0x13400000) */
@@ -579,6 +588,15 @@ static const struct samsung_cmu_info hsi_cmu_info __initconst = {
 	.clk_name		= "dout_hsi_bus",
 };
 
+static void __init exynos850_cmu_hsi_init(struct device_node *np)
+{
+	exynos850_init_clocks(np, hsi_clk_regs, ARRAY_SIZE(hsi_clk_regs));
+	samsung_cmu_register_one(np, &hsi_cmu_info);
+}
+
+CLK_OF_DECLARE_DRIVER(exynos850_cmu_hsi, "samsung,exynos850-cmu-hsi",
+		      exynos850_cmu_hsi_init);
+
 /* ---- CMU_PERI ------------------------------------------------------------ */
 
 /* Register Offset definitions for CMU_PERI (0x10030000) */
@@ -753,6 +771,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_init_clocks(np, peri_clk_regs, ARRAY_SIZE(peri_clk_regs));
+	samsung_cmu_register_one(np, &peri_cmu_info);
+}
+
+CLK_OF_DECLARE_DRIVER(exynos850_cmu_peri, "samsung,exynos850-cmu-peri",
+		      exynos850_cmu_peri_init);
+
 /* ---- CMU_CORE ------------------------------------------------------------ */
 
 /* Register Offset definitions for CMU_CORE (0x12000000) */
@@ -839,6 +866,15 @@ static const struct samsung_cmu_info core_cmu_info __initconst = {
 	.clk_name		= "dout_core_bus",
 };
 
+static void __init exynos850_cmu_core_init(struct device_node *np)
+{
+	exynos850_init_clocks(np, core_clk_regs, ARRAY_SIZE(core_clk_regs));
+	samsung_cmu_register_one(np, &core_cmu_info);
+}
+
+CLK_OF_DECLARE_DRIVER(exynos850_cmu_core, "samsung,exynos850-cmu-core",
+		      exynos850_cmu_core_init);
+
 /* ---- CMU_DPU ------------------------------------------------------------- */
 
 /* Register Offset definitions for CMU_DPU (0x13000000) */
@@ -911,17 +947,23 @@ static const struct samsung_cmu_info dpu_cmu_info __initconst = {
 	.clk_name		= "dout_dpu",
 };
 
+static void __init exynos850_cmu_dpu_init(struct device_node *np)
+{
+	exynos850_init_clocks(np, dpu_clk_regs, ARRAY_SIZE(dpu_clk_regs));
+	samsung_cmu_register_one(np, &dpu_cmu_info);
+}
+
+CLK_OF_DECLARE_DRIVER(exynos850_cmu_dpu, "samsung,exynos850-cmu-dpu",
+		      exynos850_cmu_dpu_init);
+
 /* ---- platform_driver ----------------------------------------------------- */
 
 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) {
-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

             reply	other threads:[~2021-10-22 21:56 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-22 21:56 Sam Protsenko [this message]
2021-10-22 21:56 ` [PATCH] clk: samsung: exynos850: Register clocks early Sam Protsenko
2021-10-23  9:19 ` Krzysztof Kozlowski
2021-10-23  9:19   ` Krzysztof Kozlowski
2021-10-25 14:45   ` Sam Protsenko
2021-10-25 14:45     ` Sam Protsenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20211022215635.31128-1-semen.protsenko@linaro.org \
    --to=semen.protsenko@linaro.org \
    --cc=cw00.choi@samsung.com \
    --cc=krzysztof.kozlowski@canonical.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-clk@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-samsung-soc@vger.kernel.org \
    --cc=matthias.bgg@gmail.com \
    --cc=mturquette@baylibre.com \
    --cc=s.nawrocki@samsung.com \
    --cc=sboyd@kernel.org \
    --cc=tomasz.figa@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.