* Re: [PATCH] clk: exynos7: Mark aclk_fsys1_200 as critical
2020-10-24 15:43 ` [PATCH] clk: exynos7: Mark aclk_fsys1_200 as critical Paweł Chmiel
@ 2020-10-26 5:46 ` Chanwoo Choi
2020-10-26 15:16 ` Krzysztof Kozlowski
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Chanwoo Choi @ 2020-10-26 5:46 UTC (permalink / raw)
To: Paweł Chmiel, kgene, krzk, mturquette, sboyd
Cc: s.nawrocki, tomasz.figa, linux-samsung-soc, linux-clk,
linux-arm-kernel, linux-kernel
Hi Paweł Chmiel,
On 10/25/20 12:43 AM, Paweł Chmiel wrote:
> This clock must be always enabled to allow access to any registers in
> fsys1 CMU. Until proper solution based on runtime PM is applied
> (similar to what was done for Exynos5433), mark that clock as critical
> so it won't be disabled.
>
> It was observed on Samsung Galaxy S6 device (based on Exynos7420), where
> UFS module is probed before pmic used to power that device.
> In this case defer probe was happening and that clock was disabled by
> UFS driver, causing whole boot to hang on next CMU access.
>
> Signed-off-by: Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com>
> ---
> drivers/clk/samsung/clk-exynos7.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/clk/samsung/clk-exynos7.c b/drivers/clk/samsung/clk-exynos7.c
> index c1ff715e960c..1048d83f097b 100644
> --- a/drivers/clk/samsung/clk-exynos7.c
> +++ b/drivers/clk/samsung/clk-exynos7.c
> @@ -538,7 +538,8 @@ static const struct samsung_gate_clock top1_gate_clks[] __initconst = {
> ENABLE_ACLK_TOP13, 28, CLK_SET_RATE_PARENT |
> CLK_IS_CRITICAL, 0),
> GATE(CLK_ACLK_FSYS1_200, "aclk_fsys1_200", "dout_aclk_fsys1_200",
> - ENABLE_ACLK_TOP13, 24, CLK_SET_RATE_PARENT, 0),
> + ENABLE_ACLK_TOP13, 24, CLK_SET_RATE_PARENT |
> + CLK_IS_CRITICAL, 0),
As you commented, in order to keep the always on state,
we can use CLK_IS_CRITICAL. Instead, you can enable the specific clock
with detailed comment on clk-exynos7.c as following merged patches[1][2]:
[1] 67f96ff7c8f0 ("clk: samsung: exynos5420: Keep top G3D clocks enabled")
[2] 0212a0483b0a ("clk: samsung: Keep top BPLL mux on Exynos542x enabled")
The patches[1][2] enable the clock with clk_prepare_enable()
instead of adding CLK_IS_CRITICAL. You can refer to it.
>
> GATE(CLK_SCLK_PHY_FSYS1_26M, "sclk_phy_fsys1_26m",
> "dout_sclk_phy_fsys1_26m", ENABLE_SCLK_TOP1_FSYS11,
>
--
Best Regards,
Chanwoo Choi
Samsung Electronics
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] clk: exynos7: Mark aclk_fsys1_200 as critical
2020-10-24 15:43 ` [PATCH] clk: exynos7: Mark aclk_fsys1_200 as critical Paweł Chmiel
2020-10-26 5:46 ` Chanwoo Choi
@ 2020-10-26 15:16 ` Krzysztof Kozlowski
2020-12-17 10:14 ` Stephen Boyd
2021-04-07 10:03 ` Sylwester Nawrocki
3 siblings, 0 replies; 6+ messages in thread
From: Krzysztof Kozlowski @ 2020-10-26 15:16 UTC (permalink / raw)
To: Paweł Chmiel
Cc: kgene, mturquette, sboyd, s.nawrocki, tomasz.figa, cw00.choi,
linux-samsung-soc, linux-clk, linux-arm-kernel, linux-kernel
On Sat, Oct 24, 2020 at 05:43:46PM +0200, Paweł Chmiel wrote:
> This clock must be always enabled to allow access to any registers in
> fsys1 CMU. Until proper solution based on runtime PM is applied
> (similar to what was done for Exynos5433), mark that clock as critical
> so it won't be disabled.
>
> It was observed on Samsung Galaxy S6 device (based on Exynos7420), where
> UFS module is probed before pmic used to power that device.
> In this case defer probe was happening and that clock was disabled by
> UFS driver, causing whole boot to hang on next CMU access.
>
> Signed-off-by: Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com>
> ---
> drivers/clk/samsung/clk-exynos7.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
Acked-by: Krzysztof Kozlowski <krzk@kernel.org>
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] clk: exynos7: Mark aclk_fsys1_200 as critical
2020-10-24 15:43 ` [PATCH] clk: exynos7: Mark aclk_fsys1_200 as critical Paweł Chmiel
2020-10-26 5:46 ` Chanwoo Choi
2020-10-26 15:16 ` Krzysztof Kozlowski
@ 2020-12-17 10:14 ` Stephen Boyd
2020-12-17 21:08 ` Paweł Chmiel
2021-04-07 10:03 ` Sylwester Nawrocki
3 siblings, 1 reply; 6+ messages in thread
From: Stephen Boyd @ 2020-12-17 10:14 UTC (permalink / raw)
To: pawel.mikolaj.chmiel, kgene, krzk, mturquette
Cc: s.nawrocki, tomasz.figa, cw00.choi, linux-samsung-soc, linux-clk,
linux-arm-kernel, linux-kernel, pawel.mikolaj.chmiel
Not sure why this wasn't picked up in the samsung PR. Can you resend?
> diff --git a/drivers/clk/samsung/clk-exynos7.c b/drivers/clk/samsung/clk-exynos7.c
> index c1ff715e960c..1048d83f097b 100644
> --- a/drivers/clk/samsung/clk-exynos7.c
> +++ b/drivers/clk/samsung/clk-exynos7.c
> @@ -538,7 +538,8 @@ static const struct samsung_gate_clock top1_gate_clks[] __initconst = {
> ENABLE_ACLK_TOP13, 28, CLK_SET_RATE_PARENT |
> CLK_IS_CRITICAL, 0),
> GATE(CLK_ACLK_FSYS1_200, "aclk_fsys1_200", "dout_aclk_fsys1_200",
> - ENABLE_ACLK_TOP13, 24, CLK_SET_RATE_PARENT, 0),
> + ENABLE_ACLK_TOP13, 24, CLK_SET_RATE_PARENT |
> + CLK_IS_CRITICAL, 0),
>
Please put a comment in the code why a clk is critical.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] clk: exynos7: Mark aclk_fsys1_200 as critical
2020-12-17 10:14 ` Stephen Boyd
@ 2020-12-17 21:08 ` Paweł Chmiel
0 siblings, 0 replies; 6+ messages in thread
From: Paweł Chmiel @ 2020-12-17 21:08 UTC (permalink / raw)
To: Stephen Boyd, kgene, krzk, mturquette
Cc: s.nawrocki, tomasz.figa, cw00.choi, linux-samsung-soc, linux-clk,
linux-arm-kernel, linux-kernel
On 17.12.2020 11:14, Stephen Boyd wrote:
> Not sure why this wasn't picked up in the samsung PR. Can you resend?
Hi
There was v2
(https://patchwork.kernel.org/project/linux-samsung-soc/patch/20201107121456.25562-1-pawel.mikolaj.chmiel@gmail.com/)
but it did receive some request for changes comments and i didn't yet
had time to it.
>
>> diff --git a/drivers/clk/samsung/clk-exynos7.c b/drivers/clk/samsung/clk-exynos7.c
>> index c1ff715e960c..1048d83f097b 100644
>> --- a/drivers/clk/samsung/clk-exynos7.c
>> +++ b/drivers/clk/samsung/clk-exynos7.c
>> @@ -538,7 +538,8 @@ static const struct samsung_gate_clock top1_gate_clks[] __initconst = {
>> ENABLE_ACLK_TOP13, 28, CLK_SET_RATE_PARENT |
>> CLK_IS_CRITICAL, 0),
>> GATE(CLK_ACLK_FSYS1_200, "aclk_fsys1_200", "dout_aclk_fsys1_200",
>> - ENABLE_ACLK_TOP13, 24, CLK_SET_RATE_PARENT, 0),
>> + ENABLE_ACLK_TOP13, 24, CLK_SET_RATE_PARENT |
>> + CLK_IS_CRITICAL, 0),
>>
>
> Please put a comment in the code why a clk is critica>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] clk: exynos7: Mark aclk_fsys1_200 as critical
2020-10-24 15:43 ` [PATCH] clk: exynos7: Mark aclk_fsys1_200 as critical Paweł Chmiel
` (2 preceding siblings ...)
2020-12-17 10:14 ` Stephen Boyd
@ 2021-04-07 10:03 ` Sylwester Nawrocki
3 siblings, 0 replies; 6+ messages in thread
From: Sylwester Nawrocki @ 2021-04-07 10:03 UTC (permalink / raw)
To: Paweł Chmiel
Cc: s.nawrocki, tomasz.figa, cw00.choi, linux-samsung-soc, linux-clk,
linux-arm-kernel, linux-kernel, kgene, krzk, mturquette, sboyd
On 24.10.2020 17:43, Paweł Chmiel wrote:
> This clock must be always enabled to allow access to any registers in
> fsys1 CMU. Until proper solution based on runtime PM is applied
> (similar to what was done for Exynos5433), mark that clock as critical
> so it won't be disabled.
>
> It was observed on Samsung Galaxy S6 device (based on Exynos7420), where
> UFS module is probed before pmic used to power that device.
> In this case defer probe was happening and that clock was disabled by
> UFS driver, causing whole boot to hang on next CMU access.
>
> Signed-off-by: Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com>
> ---
> drivers/clk/samsung/clk-exynos7.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/clk/samsung/clk-exynos7.c b/drivers/clk/samsung/clk-exynos7.c
> index c1ff715e960c..1048d83f097b 100644
> --- a/drivers/clk/samsung/clk-exynos7.c
> +++ b/drivers/clk/samsung/clk-exynos7.c
> @@ -538,7 +538,8 @@ static const struct samsung_gate_clock top1_gate_clks[] __initconst = {
> ENABLE_ACLK_TOP13, 28, CLK_SET_RATE_PARENT |
> CLK_IS_CRITICAL, 0),
As this patch can be backported up to the commit that introduced regression
I have applied it instead of your v3, with a comment as below.
+ /*
+ * This clock is required for the CMU_FSYS1 registers access, keep it
+ * enabled permanently until proper runtime PM support is added.
+ */
> GATE(CLK_ACLK_FSYS1_200, "aclk_fsys1_200", "dout_aclk_fsys1_200",
> - ENABLE_ACLK_TOP13, 24, CLK_SET_RATE_PARENT, 0),
> + ENABLE_ACLK_TOP13, 24, CLK_SET_RATE_PARENT |
> + CLK_IS_CRITICAL, 0),
>
> GATE(CLK_SCLK_PHY_FSYS1_26M, "sclk_phy_fsys1_26m",
> "dout_sclk_phy_fsys1_26m", ENABLE_SCLK_TOP1_FSYS11,
--
Regards,
Sylwester
^ permalink raw reply [flat|nested] 6+ messages in thread