[PATCH] clk: rockchip: rk3036: Add apll as the critical clock

[PATCH] clk: rockchip: rk3036: Add apll as the critical clock

[Xing Zheng]

The apll may be closed if there are some child clock nodes below
it when the device startup. Therefore, the apll should be keep
critical.

The apll tree like this:
    pll_apll
       apll
          armclk
             pclk_dbg
             aclk_core_pre
          aclk_hvec
          uart_pll_clk
             uart2_src
                uart2_frac
             uart1_src
                uart1_frac
             uart0_src
                uart0_frac

Signed-off-by: Xing Zheng <zhengxing@rock-chips.com>
---

 drivers/clk/rockchip/clk-rk3036.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/clk/rockchip/clk-rk3036.c b/drivers/clk/rockchip/clk-rk3036.c
index ebce980..483913b 100644
--- a/drivers/clk/rockchip/clk-rk3036.c
+++ b/drivers/clk/rockchip/clk-rk3036.c
@@ -425,6 +425,7 @@ static struct rockchip_clk_branch rk3036_clk_branches[] __initdata = {
 };
 
 static const char *const rk3036_critical_clocks[] __initconst = {
+	"apll",
 	"aclk_cpu",
 	"aclk_peri",
 	"hclk_peri",
-- 
1.7.9.5

Re: [PATCH] clk: rockchip: rk3036: Add apll as the critical clock

[Heiko Stuebner]

Hi Xing,

Am Mittwoch, 20. Januar 2016, 16:37:17 schrieb Xing Zheng:
> The apll may be closed if there are some child clock nodes below
> it when the device startup. Therefore, the apll should be keep
> critical.
> 
> The apll tree like this:
>     pll_apll
>        apll
>           armclk
>              pclk_dbg
>              aclk_core_pre
>           aclk_hvec
>           uart_pll_clk
>              uart2_src
>                 uart2_frac
>              uart1_src
>                 uart1_frac
>              uart0_src
>                 uart0_frac

can you find out which of those clocks does cause your hang?
Because things like the uart-clocks for example should be handled by their 
driver already, at the time the clk_disable_unused runs(). So I'd really 
like the critical clock to be the actually needed clock.

Thanks
Heiko


> Signed-off-by: Xing Zheng <zhengxing@rock-chips.com>
> ---
> 
>  drivers/clk/rockchip/clk-rk3036.c |    1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/clk/rockchip/clk-rk3036.c
> b/drivers/clk/rockchip/clk-rk3036.c index ebce980..483913b 100644
> --- a/drivers/clk/rockchip/clk-rk3036.c
> +++ b/drivers/clk/rockchip/clk-rk3036.c
> @@ -425,6 +425,7 @@ static struct rockchip_clk_branch
> rk3036_clk_branches[] __initdata = { };
> 
>  static const char *const rk3036_critical_clocks[] __initconst = {
> +	"apll",
>  	"aclk_cpu",
>  	"aclk_peri",
>  	"hclk_peri",

Re: [PATCH] clk: rockchip: rk3036: Add apll as the critical clock

[Xing Zheng]

Hi Heiko,

On 2016年01月21日 17:21, Heiko Stuebner wrote:
> Hi Xing,
>
> Am Mittwoch, 20. Januar 2016, 16:37:17 schrieb Xing Zheng:
>> The apll may be closed if there are some child clock nodes below
>> it when the device startup. Therefore, the apll should be keep
>> critical.
>>
>> The apll tree like this:
>>      pll_apll
>>         apll
>>            armclk
>>               pclk_dbg
>>               aclk_core_pre
>>            aclk_hvec
>>            uart_pll_clk
>>               uart2_src
>>                  uart2_frac
>>               uart1_src
>>                  uart1_frac
>>               uart0_src
>>                  uart0_frac
> can you find out which of those clocks does cause your hang?
> Because things like the uart-clocks for example should be handled by their
> driver already, at the time the clk_disable_unused runs(). So I'd really
> like the critical clock to be the actually needed clock.
>
> Thanks
> Heiko

It looks like that we call the rockchip_rk3036_pll_disable cause the 
apll is diabled.
I think the diabled tracing like this:
1. All of uart0_frac~uart2_frac are branch_fraction_divider type, they 
have CLK_SET_RATE_UNGATE flag,
2. I enable cpufreq configs on the rk3036_defconfig, the default cpu 
freq is 600MHz durning loader, when startup it is 816MHz with default DTS.
Therefore, cpu freq will be change rate 600MHz to 816MHz then call 
clk_change_rate.
3. With the flag CLK_SET_RATE_UNGATE, triggering call clk_core_disable. 
In here, it will recursively close all of uart gates, finally, to call 
the root
parent diable callback that is rockchip_rk3036_pll_disable.

The disble log:
[ 1.074186] clk_change_rate -- CLK_SET_RATE_UNGATE name: uart2_frac, 
parent: uart2_src, core->flags = 0x00000424
[ 1.105722] clk_gate_endisable -- name: uart2_frac, parent: uart2_src, 
enable = 0
[ 1.110125] clk_gate_endisable -- name: uart2_src, parent: uart_pll_clk, 
enable = 0
[ 2.604445] rockchip_rk3036_pll_disable -- name: pll_apll, parent: xin24m

Therefore, I am considering uart_pll_clk hang onto gpll, or add it into 
the critical clock replace using apll...

If there are some mistake, please correct me. :-)

Thanks.

>
>
>> Signed-off-by: Xing Zheng<zhengxing@rock-chips.com>
>> ---
>>
>>   drivers/clk/rockchip/clk-rk3036.c |    1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/clk/rockchip/clk-rk3036.c
>> b/drivers/clk/rockchip/clk-rk3036.c index ebce980..483913b 100644
>> --- a/drivers/clk/rockchip/clk-rk3036.c
>> +++ b/drivers/clk/rockchip/clk-rk3036.c
>> @@ -425,6 +425,7 @@ static struct rockchip_clk_branch
>> rk3036_clk_branches[] __initdata = { };
>>
>>   static const char *const rk3036_critical_clocks[] __initconst = {
>> +	"apll",
>>   	"aclk_cpu",
>>   	"aclk_peri",
>>   	"hclk_peri",
>
>
>