Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1312993 > unrolled thread

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

Back to article view | Back to linux.kernel

Contents

  [PATCH] clk: rockchip: rk3036: Add apll as the critical clock Xing Zheng <zhengxing@rock-chips.com> - 2016-01-20 09:40 +0100
    Re: [PATCH] clk: rockchip: rk3036: Add apll as the critical clock Heiko Stuebner <heiko@sntech.de> - 2016-01-21 10:30 +0100
      Re: [PATCH] clk: rockchip: rk3036: Add apll as the critical clock Xing Zheng <zhengxing@rock-chips.com> - 2016-01-22 08:50 +0100

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

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

[toc] | [next] | [standalone]

#1314024

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",

[toc] | [prev] | [next] | [standalone]

#1314806

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",
>
>
>

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web