From: Sylwester Nawrocki <s.nawrocki@samsung.com> To: Sam Protsenko <semen.protsenko@linaro.org> Cc: "Sumit Semwal" <sumit.semwal@linaro.org>, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-clk@vger.kernel.org, linux-kernel@vger.kernel.org, linux-samsung-soc@vger.kernel.org, "Krzysztof Kozlowski" <krzysztof.kozlowski@canonical.com>, "Paweł Chmiel" <pawel.mikolaj.chmiel@gmail.com>, "Chanwoo Choi" <cw00.choi@samsung.com>, "Tomasz Figa" <tomasz.figa@gmail.com>, "Rob Herring" <robh+dt@kernel.org>, "Stephen Boyd" <sboyd@kernel.org>, "Michael Turquette" <mturquette@baylibre.com> Subject: Re: [PATCH v2 1/1] clk: samsung: exynos850: Register clocks early Date: Sat, 20 Nov 2021 13:49:31 +0100 [thread overview] Message-ID: <fcc939e6-50b4-1847-c738-db940d0c5bd4@samsung.com> (raw) In-Reply-To: <20211025161254.5575-1-semen.protsenko@linaro.org> On 25.10.2021 18:12, Sam Protsenko wrote: > 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_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. > Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org> > drivers/clk/samsung/clk-exynos850.c | 17 +++++++++++++++-- > 1 file changed, 15 insertions(+), 2 deletions(-) > > diff --git a/drivers/clk/samsung/clk-exynos850.c b/drivers/clk/samsung/clk-exynos850.c > index 95e373d17b42..ecffa5c7a081 100644 > --- a/drivers/clk/samsung/clk-exynos850.c > +++ b/drivers/clk/samsung/clk-exynos850.c > @@ -753,6 +753,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) */ > @@ -920,8 +929,12 @@ static int __init exynos850_cmu_probe(struct platform_device *pdev) > 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); > + > + /* Early clocks are already registered using CLK_OF_DECLARE_DRIVER() */ > + if (info != &peri_cmu_info) { > + exynos850_init_clocks(np, info->clk_regs, info->nr_clk_regs); > + samsung_cmu_register_one(np, info); > + } Don't you also need to register early CMU_TOP, which provides clocks for CMU_PERI? I'm afraid it might not work properly when you register CMU_PERI clocks early and only later in probe() you enable parent clock required for the already registered clocks to be usable. How about registering also CMU_TOP early and enabling parent clock also in OF_CLK_DECLARE init callback, i.e. using either OF_CLK_DECLARE or platform driver for a CMU? -- Regards, Sylwester
WARNING: multiple messages have this Message-ID (diff)
From: Sylwester Nawrocki <s.nawrocki@samsung.com> To: Sam Protsenko <semen.protsenko@linaro.org> Cc: "Sumit Semwal" <sumit.semwal@linaro.org>, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-clk@vger.kernel.org, linux-kernel@vger.kernel.org, linux-samsung-soc@vger.kernel.org, "Krzysztof Kozlowski" <krzysztof.kozlowski@canonical.com>, "Paweł Chmiel" <pawel.mikolaj.chmiel@gmail.com>, "Chanwoo Choi" <cw00.choi@samsung.com>, "Tomasz Figa" <tomasz.figa@gmail.com>, "Rob Herring" <robh+dt@kernel.org>, "Stephen Boyd" <sboyd@kernel.org>, "Michael Turquette" <mturquette@baylibre.com> Subject: Re: [PATCH v2 1/1] clk: samsung: exynos850: Register clocks early Date: Sat, 20 Nov 2021 13:49:31 +0100 [thread overview] Message-ID: <fcc939e6-50b4-1847-c738-db940d0c5bd4@samsung.com> (raw) In-Reply-To: <20211025161254.5575-1-semen.protsenko@linaro.org> On 25.10.2021 18:12, Sam Protsenko wrote: > 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_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. > Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org> > drivers/clk/samsung/clk-exynos850.c | 17 +++++++++++++++-- > 1 file changed, 15 insertions(+), 2 deletions(-) > > diff --git a/drivers/clk/samsung/clk-exynos850.c b/drivers/clk/samsung/clk-exynos850.c > index 95e373d17b42..ecffa5c7a081 100644 > --- a/drivers/clk/samsung/clk-exynos850.c > +++ b/drivers/clk/samsung/clk-exynos850.c > @@ -753,6 +753,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) */ > @@ -920,8 +929,12 @@ static int __init exynos850_cmu_probe(struct platform_device *pdev) > 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); > + > + /* Early clocks are already registered using CLK_OF_DECLARE_DRIVER() */ > + if (info != &peri_cmu_info) { > + exynos850_init_clocks(np, info->clk_regs, info->nr_clk_regs); > + samsung_cmu_register_one(np, info); > + } Don't you also need to register early CMU_TOP, which provides clocks for CMU_PERI? I'm afraid it might not work properly when you register CMU_PERI clocks early and only later in probe() you enable parent clock required for the already registered clocks to be usable. How about registering also CMU_TOP early and enabling parent clock also in OF_CLK_DECLARE init callback, i.e. using either OF_CLK_DECLARE or platform driver for a CMU? -- Regards, Sylwester _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2021-11-20 12:49 UTC|newest] Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top [not found] <CGME20211025161302eucas1p2f50ef29a0bba69c13deaf1ad31a8439c@eucas1p2.samsung.com> 2021-10-25 16:12 ` [PATCH v2 1/1] clk: samsung: exynos850: Register clocks early Sam Protsenko 2021-10-25 16:12 ` Sam Protsenko 2021-10-26 14:53 ` Krzysztof Kozlowski 2021-10-26 14:53 ` Krzysztof Kozlowski 2021-11-20 12:49 ` Sylwester Nawrocki [this message] 2021-11-20 12:49 ` Sylwester Nawrocki 2021-11-20 16:47 ` Sam Protsenko 2021-11-20 16:47 ` Sam Protsenko 2021-11-20 17:38 ` Sylwester Nawrocki 2021-11-20 17:38 ` Sylwester Nawrocki 2021-11-21 22:50 ` Sam Protsenko 2021-11-21 22:50 ` Sam Protsenko [not found] <CGME20211122144217eucas1p21c5f4930563ee051d625bb8e3a932a4a@eucas1p2.samsung.com> 2021-11-22 14:42 ` Sam Protsenko 2021-11-22 14:42 ` Sam Protsenko 2021-11-22 22:04 ` Sylwester Nawrocki 2021-11-22 22:04 ` Sylwester Nawrocki
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=fcc939e6-50b4-1847-c738-db940d0c5bd4@samsung.com \ --to=s.nawrocki@samsung.com \ --cc=cw00.choi@samsung.com \ --cc=devicetree@vger.kernel.org \ --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=mturquette@baylibre.com \ --cc=pawel.mikolaj.chmiel@gmail.com \ --cc=robh+dt@kernel.org \ --cc=sboyd@kernel.org \ --cc=semen.protsenko@linaro.org \ --cc=sumit.semwal@linaro.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: linkBe 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.