[PATCH] clk: samsung: Mark top BPLL mux on Exynos542x as critical

[PATCH] clk: samsung: Mark top BPLL mux on Exynos542x as critical

[Marek Szyprowski]

BPLL clock must not be disabled because it is needed for proper DRAM
operation. This is normally handled by respective memory devfreq driver,
but when that driver is not yet probed or its probe has been deferred the
clock might got disabled what causes board hang. Fix this by marking it
as critical.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 drivers/clk/samsung/clk-exynos5420.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/samsung/clk-exynos5420.c b/drivers/clk/samsung/clk-exynos5420.c
index fea33399a632..5ef78928938a 100644
--- a/drivers/clk/samsung/clk-exynos5420.c
+++ b/drivers/clk/samsung/clk-exynos5420.c
@@ -734,7 +734,7 @@ static const struct samsung_mux_clock exynos5x_mux_clks[] __initconst = {
 	MUX_F(CLK_MOUT_MCLK_CDREX, "mout_mclk_cdrex", mout_mclk_cdrex_p,
 			SRC_CDREX, 4, 1, CLK_SET_RATE_PARENT, 0),
 	MUX_F(CLK_MOUT_BPLL, "mout_bpll", mout_bpll_p, SRC_CDREX, 0, 1,
-			CLK_SET_RATE_PARENT, 0),
+			CLK_SET_RATE_PARENT | CLK_IS_CRITICAL, 0),
 
 	/* MAU Block */
 	MUX(CLK_MOUT_MAUDIO0, "mout_maudio0", mout_maudio0_p, SRC_MAU, 28, 3),
-- 
2.17.1

Re: [PATCH] clk: samsung: Mark top BPLL mux on Exynos542x as critical

[Lukasz Luba]

Hi Marek,

On 8/5/20 10:16 AM, Marek Szyprowski wrote:
> BPLL clock must not be disabled because it is needed for proper DRAM
> operation. This is normally handled by respective memory devfreq driver,
> but when that driver is not yet probed or its probe has been deferred the
> clock might got disabled what causes board hang. Fix this by marking it
> as critical.
> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>   drivers/clk/samsung/clk-exynos5420.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/samsung/clk-exynos5420.c b/drivers/clk/samsung/clk-exynos5420.c
> index fea33399a632..5ef78928938a 100644
> --- a/drivers/clk/samsung/clk-exynos5420.c
> +++ b/drivers/clk/samsung/clk-exynos5420.c
> @@ -734,7 +734,7 @@ static const struct samsung_mux_clock exynos5x_mux_clks[] __initconst = {
>   	MUX_F(CLK_MOUT_MCLK_CDREX, "mout_mclk_cdrex", mout_mclk_cdrex_p,
>   			SRC_CDREX, 4, 1, CLK_SET_RATE_PARENT, 0),
>   	MUX_F(CLK_MOUT_BPLL, "mout_bpll", mout_bpll_p, SRC_CDREX, 0, 1,
> -			CLK_SET_RATE_PARENT, 0),
> +			CLK_SET_RATE_PARENT | CLK_IS_CRITICAL, 0),
>   
>   	/* MAU Block */
>   	MUX(CLK_MOUT_MAUDIO0, "mout_maudio0", mout_maudio0_p, SRC_MAU, 28, 3),
> 

I've tested it in the way that we discussed yesterday.
I can confirm this patch solves the issue.

Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>
Tested-by: Lukasz Luba <lukasz.luba@arm.com>

Thank you for investigating this further and fixing it.

Regards,
Lukasz

Re: [PATCH] clk: samsung: Mark top BPLL mux on Exynos542x as critical

[Krzysztof Kozlowski]

On Wed, Aug 05, 2020 at 11:16:01AM +0200, Marek Szyprowski wrote:
> BPLL clock must not be disabled because it is needed for proper DRAM
> operation. This is normally handled by respective memory devfreq driver,
> but when that driver is not yet probed or its probe has been deferred the
> clock might got disabled what causes board hang. Fix this by marking it
> as critical.
> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  drivers/clk/samsung/clk-exynos5420.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Acked-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof

Re: [PATCH] clk: samsung: Mark top BPLL mux on Exynos542x as critical

[Chanwoo Choi]

Hi Marek,

On 8/5/20 6:16 PM, Marek Szyprowski wrote:
> BPLL clock must not be disabled because it is needed for proper DRAM
> operation. This is normally handled by respective memory devfreq driver,
> but when that driver is not yet probed or its probe has been deferred the
> clock might got disabled what causes board hang. Fix this by marking it
> as critical.
> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  drivers/clk/samsung/clk-exynos5420.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/samsung/clk-exynos5420.c b/drivers/clk/samsung/clk-exynos5420.c
> index fea33399a632..5ef78928938a 100644
> --- a/drivers/clk/samsung/clk-exynos5420.c
> +++ b/drivers/clk/samsung/clk-exynos5420.c
> @@ -734,7 +734,7 @@ static const struct samsung_mux_clock exynos5x_mux_clks[] __initconst = {
>  	MUX_F(CLK_MOUT_MCLK_CDREX, "mout_mclk_cdrex", mout_mclk_cdrex_p,
>  			SRC_CDREX, 4, 1, CLK_SET_RATE_PARENT, 0),
>  	MUX_F(CLK_MOUT_BPLL, "mout_bpll", mout_bpll_p, SRC_CDREX, 0, 1,
> -			CLK_SET_RATE_PARENT, 0),
> +			CLK_SET_RATE_PARENT | CLK_IS_CRITICAL, 0),
>  
>  	/* MAU Block */
>  	MUX(CLK_MOUT_MAUDIO0, "mout_maudio0", mout_maudio0_p, SRC_MAU, 28, 3),
> 

Thanks for your fix. Looks good to me.
Just I have one comment. I looks like the similar case of patch[1] related to G3D clock.
[1] commit 67f96ff7c8f0 ("clk: samsung: exynos5420: Keep top G3D clocks enabled")

How about fixing this issue with same style[1]?
Or are there any reason about should you do it with CLK_IS_CRITICAL?

-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

Re: [PATCH] clk: samsung: Mark top BPLL mux on Exynos542x as critical

[Sylwester Nawrocki]

Hi,

On 8/6/20 04:49, Chanwoo Choi wrote:
> On 8/5/20 6:16 PM, Marek Szyprowski wrote:
>> BPLL clock must not be disabled because it is needed for proper DRAM
>> operation. This is normally handled by respective memory devfreq driver,
>> but when that driver is not yet probed or its probe has been deferred the
>> clock might got disabled what causes board hang. Fix this by marking it
>> as critical.
>>
>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> ---
>>  drivers/clk/samsung/clk-exynos5420.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/clk/samsung/clk-exynos5420.c b/drivers/clk/samsung/clk-exynos5420.c
>> index fea33399a632..5ef78928938a 100644
>> --- a/drivers/clk/samsung/clk-exynos5420.c
>> +++ b/drivers/clk/samsung/clk-exynos5420.c
>> @@ -734,7 +734,7 @@ static const struct samsung_mux_clock exynos5x_mux_clks[] __initconst = {
>>  	MUX_F(CLK_MOUT_MCLK_CDREX, "mout_mclk_cdrex", mout_mclk_cdrex_p,
>>  			SRC_CDREX, 4, 1, CLK_SET_RATE_PARENT, 0),
>>  	MUX_F(CLK_MOUT_BPLL, "mout_bpll", mout_bpll_p, SRC_CDREX, 0, 1,
>> -			CLK_SET_RATE_PARENT, 0),
>> +			CLK_SET_RATE_PARENT | CLK_IS_CRITICAL, 0),
>>  
>>  	/* MAU Block */
>>  	MUX(CLK_MOUT_MAUDIO0, "mout_maudio0", mout_maudio0_p, SRC_MAU, 28, 3),
>>
> Thanks for your fix. Looks good to me.
> Just I have one comment. I looks like the similar case of patch[1] related to G3D clock.
> [1] commit 67f96ff7c8f0 ("clk: samsung: exynos5420: Keep top G3D clocks enabled")
> 
> How about fixing this issue with same style[1]?

Yes, I think it would be better to make it the same way as it was done with 
the G3D clock.

-- 
Thanks,
Sylwester