diff mbox series

[RESEND,v3,1/4] clk: imx: imx8mq: fix a53 cpu clock

Message ID 1582107429-21123-2-git-send-email-peng.fan@nxp.com (mailing list archive)
State Changes Requested, archived
Headers show
Series clk: imx: imx8m: fix a53 cpu clock | expand

Commit Message

Peng Fan Feb. 19, 2020, 10:17 a.m. UTC 
From: Peng Fan <peng.fan@nxp.com>

The A53 CCM clk root only accepts input up to 1GHz, CCM A53 root
signoff timing is 1Ghz, however the A53 core which sources from CCM
root could run above 1GHz which violates the CCM.

There is a CORE_SEL slice before A53 core, we need to configure the
CORE_SEL slice source from ARM PLL, not A53 CCM clk root.

The A53 CCM clk root should only be used when need to change ARM PLL
frequency.

Add arm_a53_core clk that could source from arm_a53_div and arm_pll_out.
Configure a53 ccm root sources from 800MHz sys pll
Configure a53 core sources from arm_pll_out
Mark arm_a53_core as critical clock

Fixes: db27e40b27f1 ("clk: imx8mq: Add the missing ARM clock")
Reviewed-by: Jacky Bai <ping.bai@nxp.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/clk/imx/clk-imx8mq.c             | 16 ++++++++++++----
 include/dt-bindings/clock/imx8mq-clock.h |  4 +++-
 2 files changed, 15 insertions(+), 5 deletions(-)

Comments

Leonard Crestez March 10, 2020, 7:38 p.m. UTC | #1 
On 19.02.2020 12:23, Peng Fan wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> The A53 CCM clk root only accepts input up to 1GHz, CCM A53 root
> signoff timing is 1Ghz, however the A53 core which sources from CCM
> root could run above 1GHz which violates the CCM.
> 
> There is a CORE_SEL slice before A53 core, we need to configure the
> CORE_SEL slice source from ARM PLL, not A53 CCM clk root.
> 
> The A53 CCM clk root should only be used when need to change ARM PLL
> frequency.
> 
> Add arm_a53_core clk that could source from arm_a53_div and arm_pll_out.
> Configure a53 ccm root sources from 800MHz sys pll
> Configure a53 core sources from arm_pll_out
> Mark arm_a53_core as critical clock
> 
> +	clk_hw_set_parent(hws[IMX8MQ_CLK_A53_SRC], hws[IMX8MQ_SYS1_PLL_800M]);
> +	clk_hw_set_parent(hws[IMX8MQ_CLK_A53_CORE], hws[IMX8MQ_ARM_PLL_OUT]);

This triggers lockdep warnings:

[    2.041743] ------------[ cut here ]------------ 

[    2.043531] WARNING: CPU: 2 PID: 1 at drivers/clk/clk.c:2480 
clk_core_set_parent_nolock+0x1d4/0x508
[    2.052584] Modules linked in: 

[    2.055642] CPU: 2 PID: 1 Comm: swapper/0 Not tainted 
5.6.0-rc4-next-20200306-00027-g6b7e51d87f22 #225
[    2.064966] Hardware name: NXP i.MX8MQ EVK (DT) 

[    2.069504] pstate: 60000005 (nZCv daif -PAN -UAO) 

[    2.074298] pc : clk_core_set_parent_nolock+0x1d4/0x508 

[    2.079529] lr : clk_core_set_parent_nolock+0x1d0/0x508 
 

[    2.084759] sp : ffff80001003b9b0 
 

[    2.088072] x29: ffff80001003b9b0 x28: ffff8000116e8218 
 

[    2.093392] x27: 0000000000004570 x26: ffff8000128745d0 
 

[    2.098711] x25: ffff0000b8422008 x24: ffff0000b8422008 

[    2.104030] x23: ffff80001104a518 x22: ffff80001104a508 

[    2.109349] x21: ffff800012260bf8 x20: ffff0000b84c9600 

[    2.114668] x19: ffff0000b84cbb00 x18: 0000000000004530 

[    2.119987] x17: 0000000000004520 x16: 0000000000004510 
 

[    2.125307] x15: 00000000000045d0 x14: 0000000000004500 
 

[    2.130626] x13: 00000000000044f0 x12: 00000000000044e0 

[    2.135945] x11: ffff8000116e6c68 x10: ffff8000117d7000 
 

[    2.141264] x9 : ffff80001067007c x8 : 0000000000000000 

[    2.146583] x7 : ffff800010671938 x6 : 0000000000000000 
 

[    2.151903] x5 : ffff800011633000 x4 : 0000000000000000 

[    2.157222] x3 : ffff80001003b804 x2 : 0000000000000000 
 

[    2.162541] x1 : ffff0000b9da0000 x0 : 0000000000000000 

[    2.167862] Call trace: 

[    2.170307]  clk_core_set_parent_nolock+0x1d4/0x508 

[    2.175190]  clk_hw_set_parent+0x1c/0x28 

[    2.179114]  imx8mq_clocks_probe+0x3538/0x3668 

[    2.183562]  platform_drv_probe+0x58/0xa8 

[    2.187573]  really_probe+0xe0/0x440 
 

[    2.191145]  driver_probe_device+0xe4/0x138
[    2.195333]  device_driver_attach+0x74/0x80 
 

[    2.199519]  __driver_attach+0xa8/0x170 

[    2.203354]  bus_for_each_dev+0x74/0xc8 
 

[    2.207190]  driver_attach+0x28/0x30 

[    2.210767]  bus_add_driver+0x144/0x228 
 

[    2.214605]  driver_register+0x68/0x118 

[    2.218438]  __platform_driver_register+0x4c/0x58 
 

[    2.223151]  imx8mq_clk_driver_init+0x20/0x28 

[    2.227511]  do_one_initcall+0x88/0x410 

[    2.231348]  kernel_init_freeable+0x24c/0x2c0 

[    2.235706]  kernel_init+0x18/0x108 

[    2.239192]  ret_from_fork+0x10/0x18 

[    2.242768] irq event stamp: 130084 

[    2.246262] hardirqs last  enabled at (130083): [<ffff800010302e78>] 
__slab_alloc.isra.0+0x90/0xb8
[    2.255241] hardirqs last disabled at (130084): [<ffff8000100a60b0>] 
do_debug_exception+0x168/0x254
[    2.264308] softirqs last  enabled at (130070): [<ffff800010080e88>] 
__do_softirq+0x490/0x56c
[    2.272856] softirqs last disabled at (130057): [<ffff800010101e1c>] 
irq_exit+0x11c/0x148
[    2.281057] ---[ end trace 1fae73b5c77d8120 ]---
[    2.285792] ------------[ cut here ]------------

This happens because clk_hw_set_parent does not take the prepare_lock so 
a lockdep_assert_held fails. In practice it should be mostly harmless 
because clk operations shouldn't happen while the SOC provider is probing.

The issue can be worked around by doing the following instead:

+       clk_set_parent(hws[IMX8MQ_CLK_A53_SRC]->clk, 
hws[IMX8MQ_SYS1_PLL_800M]->clk);
+       clk_set_parent(hws[IMX8MQ_CLK_A53_CORE]->clk, 
hws[IMX8MQ_ARM_PLL_OUT]->clk);

This implies reverting commit f95d58981f40 ("clk: imx: Include 
clk-provider.h instead of clk.h for i.MX8M SoCs clock driver") and 
somewhat rolls back the consumer/provider split.

What would be a clean fix for this? It might make sense to add a new API.

--
Regards,
Leonard
Peng Fan March 11, 2020, 1:16 a.m. UTC | #2 
> Subject: Re: [PATCH RESEND v3 1/4] clk: imx: imx8mq: fix a53 cpu clock
> 
> On 19.02.2020 12:23, Peng Fan wrote:
> > From: Peng Fan <peng.fan@nxp.com>
> >
> > The A53 CCM clk root only accepts input up to 1GHz, CCM A53 root
> > signoff timing is 1Ghz, however the A53 core which sources from CCM
> > root could run above 1GHz which violates the CCM.
> >
> > There is a CORE_SEL slice before A53 core, we need to configure the
> > CORE_SEL slice source from ARM PLL, not A53 CCM clk root.
> >
> > The A53 CCM clk root should only be used when need to change ARM PLL
> > frequency.
> >
> > Add arm_a53_core clk that could source from arm_a53_div and
> arm_pll_out.
> > Configure a53 ccm root sources from 800MHz sys pll Configure a53 core
> > sources from arm_pll_out Mark arm_a53_core as critical clock
> >
> > +	clk_hw_set_parent(hws[IMX8MQ_CLK_A53_SRC],
> hws[IMX8MQ_SYS1_PLL_800M]);
> > +	clk_hw_set_parent(hws[IMX8MQ_CLK_A53_CORE],
> > +hws[IMX8MQ_ARM_PLL_OUT]);
> 
> This triggers lockdep warnings:
> 
> [    2.041743] ------------[ cut here ]------------
> 
> [    2.043531] WARNING: CPU: 2 PID: 1 at drivers/clk/clk.c:2480
> clk_core_set_parent_nolock+0x1d4/0x508
> [    2.052584] Modules linked in:
> 
> [    2.055642] CPU: 2 PID: 1 Comm: swapper/0 Not tainted
> 5.6.0-rc4-next-20200306-00027-g6b7e51d87f22 #225
> [    2.064966] Hardware name: NXP i.MX8MQ EVK (DT)
> 
> [    2.069504] pstate: 60000005 (nZCv daif -PAN -UAO)
> 
> [    2.074298] pc : clk_core_set_parent_nolock+0x1d4/0x508
> 
> [    2.079529] lr : clk_core_set_parent_nolock+0x1d0/0x508
> 
> 
> [    2.084759] sp : ffff80001003b9b0
> 
> 
> [    2.088072] x29: ffff80001003b9b0 x28: ffff8000116e8218
> 
> 
> [    2.093392] x27: 0000000000004570 x26: ffff8000128745d0
> 
> 
> [    2.098711] x25: ffff0000b8422008 x24: ffff0000b8422008
> 
> [    2.104030] x23: ffff80001104a518 x22: ffff80001104a508
> 
> [    2.109349] x21: ffff800012260bf8 x20: ffff0000b84c9600
> 
> [    2.114668] x19: ffff0000b84cbb00 x18: 0000000000004530
> 
> [    2.119987] x17: 0000000000004520 x16: 0000000000004510
> 
> 
> [    2.125307] x15: 00000000000045d0 x14: 0000000000004500
> 
> 
> [    2.130626] x13: 00000000000044f0 x12: 00000000000044e0
> 
> [    2.135945] x11: ffff8000116e6c68 x10: ffff8000117d7000
> 
> 
> [    2.141264] x9 : ffff80001067007c x8 : 0000000000000000
> 
> [    2.146583] x7 : ffff800010671938 x6 : 0000000000000000
> 
> 
> [    2.151903] x5 : ffff800011633000 x4 : 0000000000000000
> 
> [    2.157222] x3 : ffff80001003b804 x2 : 0000000000000000
> 
> 
> [    2.162541] x1 : ffff0000b9da0000 x0 : 0000000000000000
> 
> [    2.167862] Call trace:
> 
> [    2.170307]  clk_core_set_parent_nolock+0x1d4/0x508
> 
> [    2.175190]  clk_hw_set_parent+0x1c/0x28
> 
> [    2.179114]  imx8mq_clocks_probe+0x3538/0x3668
> 
> [    2.183562]  platform_drv_probe+0x58/0xa8
> 
> [    2.187573]  really_probe+0xe0/0x440
> 
> 
> [    2.191145]  driver_probe_device+0xe4/0x138
> [    2.195333]  device_driver_attach+0x74/0x80
> 
> 
> [    2.199519]  __driver_attach+0xa8/0x170
> 
> [    2.203354]  bus_for_each_dev+0x74/0xc8
> 
> 
> [    2.207190]  driver_attach+0x28/0x30
> 
> [    2.210767]  bus_add_driver+0x144/0x228
> 
> 
> [    2.214605]  driver_register+0x68/0x118
> 
> [    2.218438]  __platform_driver_register+0x4c/0x58
> 
> 
> [    2.223151]  imx8mq_clk_driver_init+0x20/0x28
> 
> [    2.227511]  do_one_initcall+0x88/0x410
> 
> [    2.231348]  kernel_init_freeable+0x24c/0x2c0
> 
> [    2.235706]  kernel_init+0x18/0x108
> 
> [    2.239192]  ret_from_fork+0x10/0x18
> 
> [    2.242768] irq event stamp: 130084
> 
> [    2.246262] hardirqs last  enabled at (130083): [<ffff800010302e78>]
> __slab_alloc.isra.0+0x90/0xb8
> [    2.255241] hardirqs last disabled at (130084): [<ffff8000100a60b0>]
> do_debug_exception+0x168/0x254
> [    2.264308] softirqs last  enabled at (130070): [<ffff800010080e88>]
> __do_softirq+0x490/0x56c
> [    2.272856] softirqs last disabled at (130057): [<ffff800010101e1c>]
> irq_exit+0x11c/0x148
> [    2.281057] ---[ end trace 1fae73b5c77d8120 ]---
> [    2.285792] ------------[ cut here ]------------

I not met such warning when I test, you enabled lockdep debug?

> 
> This happens because clk_hw_set_parent does not take the prepare_lock so
> a lockdep_assert_held fails. In practice it should be mostly harmless because
> clk operations shouldn't happen while the SOC provider is probing.
> 
> The issue can be worked around by doing the following instead:
> 
> +       clk_set_parent(hws[IMX8MQ_CLK_A53_SRC]->clk,
> hws[IMX8MQ_SYS1_PLL_800M]->clk);
> +       clk_set_parent(hws[IMX8MQ_CLK_A53_CORE]->clk,
> hws[IMX8MQ_ARM_PLL_OUT]->clk);
> 
> This implies reverting commit f95d58981f40 ("clk: imx: Include
> clk-provider.h instead of clk.h for i.MX8M SoCs clock driver") and
> somewhat rolls back the consumer/provider split.
> 
> What would be a clean fix for this? It might make sense to add a new API.
> 

How about moving this to dts? I'll give a try.

Thanks,
Peng.

> --
> Regards,
> Leonard
Leonard Crestez March 25, 2020, 12:25 p.m. UTC | #3 
On 2020-03-11 3:16 AM, Peng Fan wrote:
>> Subject: Re: [PATCH RESEND v3 1/4] clk: imx: imx8mq: fix a53 cpu clock
>>
>> On 19.02.2020 12:23, Peng Fan wrote:
>>> From: Peng Fan <peng.fan@nxp.com>
>>>
>>> The A53 CCM clk root only accepts input up to 1GHz, CCM A53 root
>>> signoff timing is 1Ghz, however the A53 core which sources from CCM
>>> root could run above 1GHz which violates the CCM.
>>>
>>> There is a CORE_SEL slice before A53 core, we need to configure the
>>> CORE_SEL slice source from ARM PLL, not A53 CCM clk root.
>>>
>>> The A53 CCM clk root should only be used when need to change ARM PLL
>>> frequency.
>>>
>>> Add arm_a53_core clk that could source from arm_a53_div and
>> arm_pll_out.
>>> Configure a53 ccm root sources from 800MHz sys pll Configure a53 core
>>> sources from arm_pll_out Mark arm_a53_core as critical clock
>>>
>>> +	clk_hw_set_parent(hws[IMX8MQ_CLK_A53_SRC],
>> hws[IMX8MQ_SYS1_PLL_800M]);
>>> +	clk_hw_set_parent(hws[IMX8MQ_CLK_A53_CORE],
>>> +hws[IMX8MQ_ARM_PLL_OUT]);
>>
>> This triggers lockdep warnings:
>>
>> [    2.041743] ------------[ cut here ]------------
>>
>> [    2.043531] WARNING: CPU: 2 PID: 1 at drivers/clk/clk.c:2480
>> clk_core_set_parent_nolock+0x1d4/0x508
>> [    2.052584] Modules linked in:
>>
>> [    2.055642] CPU: 2 PID: 1 Comm: swapper/0 Not tainted
>> 5.6.0-rc4-next-20200306-00027-g6b7e51d87f22 #225
>> [    2.064966] Hardware name: NXP i.MX8MQ EVK (DT)
>>
>> [    2.069504] pstate: 60000005 (nZCv daif -PAN -UAO)
>>
>> [    2.074298] pc : clk_core_set_parent_nolock+0x1d4/0x508
>>
>> [    2.079529] lr : clk_core_set_parent_nolock+0x1d0/0x508
>>
>>
>> [    2.084759] sp : ffff80001003b9b0
>>
>>
>> [    2.088072] x29: ffff80001003b9b0 x28: ffff8000116e8218
>>
>>
>> [    2.093392] x27: 0000000000004570 x26: ffff8000128745d0
>>
>>
>> [    2.098711] x25: ffff0000b8422008 x24: ffff0000b8422008
>>
>> [    2.104030] x23: ffff80001104a518 x22: ffff80001104a508
>>
>> [    2.109349] x21: ffff800012260bf8 x20: ffff0000b84c9600
>>
>> [    2.114668] x19: ffff0000b84cbb00 x18: 0000000000004530
>>
>> [    2.119987] x17: 0000000000004520 x16: 0000000000004510
>>
>>
>> [    2.125307] x15: 00000000000045d0 x14: 0000000000004500
>>
>>
>> [    2.130626] x13: 00000000000044f0 x12: 00000000000044e0
>>
>> [    2.135945] x11: ffff8000116e6c68 x10: ffff8000117d7000
>>
>>
>> [    2.141264] x9 : ffff80001067007c x8 : 0000000000000000
>>
>> [    2.146583] x7 : ffff800010671938 x6 : 0000000000000000
>>
>>
>> [    2.151903] x5 : ffff800011633000 x4 : 0000000000000000
>>
>> [    2.157222] x3 : ffff80001003b804 x2 : 0000000000000000
>>
>>
>> [    2.162541] x1 : ffff0000b9da0000 x0 : 0000000000000000
>>
>> [    2.167862] Call trace:
>>
>> [    2.170307]  clk_core_set_parent_nolock+0x1d4/0x508
>>
>> [    2.175190]  clk_hw_set_parent+0x1c/0x28
>>
>> [    2.179114]  imx8mq_clocks_probe+0x3538/0x3668
>>
>> [    2.183562]  platform_drv_probe+0x58/0xa8
>>
>> [    2.187573]  really_probe+0xe0/0x440
>>
>>
>> [    2.191145]  driver_probe_device+0xe4/0x138
>> [    2.195333]  device_driver_attach+0x74/0x80
>>
>>
>> [    2.199519]  __driver_attach+0xa8/0x170
>>
>> [    2.203354]  bus_for_each_dev+0x74/0xc8
>>
>>
>> [    2.207190]  driver_attach+0x28/0x30
>>
>> [    2.210767]  bus_add_driver+0x144/0x228
>>
>>
>> [    2.214605]  driver_register+0x68/0x118
>>
>> [    2.218438]  __platform_driver_register+0x4c/0x58
>>
>>
>> [    2.223151]  imx8mq_clk_driver_init+0x20/0x28
>>
>> [    2.227511]  do_one_initcall+0x88/0x410
>>
>> [    2.231348]  kernel_init_freeable+0x24c/0x2c0
>>
>> [    2.235706]  kernel_init+0x18/0x108
>>
>> [    2.239192]  ret_from_fork+0x10/0x18
>>
>> [    2.242768] irq event stamp: 130084
>>
>> [    2.246262] hardirqs last  enabled at (130083): [<ffff800010302e78>]
>> __slab_alloc.isra.0+0x90/0xb8
>> [    2.255241] hardirqs last disabled at (130084): [<ffff8000100a60b0>]
>> do_debug_exception+0x168/0x254
>> [    2.264308] softirqs last  enabled at (130070): [<ffff800010080e88>]
>> __do_softirq+0x490/0x56c
>> [    2.272856] softirqs last disabled at (130057): [<ffff800010101e1c>]
>> irq_exit+0x11c/0x148
>> [    2.281057] ---[ end trace 1fae73b5c77d8120 ]---
>> [    2.285792] ------------[ cut here ]------------
> 
> I not met such warning when I test, you enabled lockdep debug?
> 
>>
>> This happens because clk_hw_set_parent does not take the prepare_lock so
>> a lockdep_assert_held fails. In practice it should be mostly harmless because
>> clk operations shouldn't happen while the SOC provider is probing.
>>
>> The issue can be worked around by doing the following instead:
>>
>> +       clk_set_parent(hws[IMX8MQ_CLK_A53_SRC]->clk,
>> hws[IMX8MQ_SYS1_PLL_800M]->clk);
>> +       clk_set_parent(hws[IMX8MQ_CLK_A53_CORE]->clk,
>> hws[IMX8MQ_ARM_PLL_OUT]->clk);
>>
>> This implies reverting commit f95d58981f40 ("clk: imx: Include
>> clk-provider.h instead of clk.h for i.MX8M SoCs clock driver") and
>> somewhat rolls back the consumer/provider split.
>>
>> What would be a clean fix for this? It might make sense to add a new API.
>>
> 
> How about moving this to dts? I'll give a try.

The warning spam still happens in next-20200325.
Peng Fan March 25, 2020, 12:27 p.m. UTC | #4 
> Subject: Re: [PATCH RESEND v3 1/4] clk: imx: imx8mq: fix a53 cpu clock
> 
> On 2020-03-11 3:16 AM, Peng Fan wrote:
> >> Subject: Re: [PATCH RESEND v3 1/4] clk: imx: imx8mq: fix a53 cpu
> >> clock
> >>
> >> On 19.02.2020 12:23, Peng Fan wrote:
> >>> From: Peng Fan <peng.fan@nxp.com>
> >>>
> >>> The A53 CCM clk root only accepts input up to 1GHz, CCM A53 root
> >>> signoff timing is 1Ghz, however the A53 core which sources from CCM
> >>> root could run above 1GHz which violates the CCM.
> >>>
> >>> There is a CORE_SEL slice before A53 core, we need to configure the
> >>> CORE_SEL slice source from ARM PLL, not A53 CCM clk root.
> >>>
> >>> The A53 CCM clk root should only be used when need to change ARM PLL
> >>> frequency.
> >>>
> >>> Add arm_a53_core clk that could source from arm_a53_div and
> >> arm_pll_out.
> >>> Configure a53 ccm root sources from 800MHz sys pll Configure a53
> >>> core sources from arm_pll_out Mark arm_a53_core as critical clock
> >>>
> >>> +	clk_hw_set_parent(hws[IMX8MQ_CLK_A53_SRC],
> >> hws[IMX8MQ_SYS1_PLL_800M]);
> >>> +	clk_hw_set_parent(hws[IMX8MQ_CLK_A53_CORE],
> >>> +hws[IMX8MQ_ARM_PLL_OUT]);
> >>
> >> This triggers lockdep warnings:
> >>
> >> [    2.041743] ------------[ cut here ]------------
> >>
> >> [    2.043531] WARNING: CPU: 2 PID: 1 at drivers/clk/clk.c:2480
> >> clk_core_set_parent_nolock+0x1d4/0x508
> >> [    2.052584] Modules linked in:
> >>
> >> [    2.055642] CPU: 2 PID: 1 Comm: swapper/0 Not tainted
> >> 5.6.0-rc4-next-20200306-00027-g6b7e51d87f22 #225
> >> [    2.064966] Hardware name: NXP i.MX8MQ EVK (DT)
> >>
> >> [    2.069504] pstate: 60000005 (nZCv daif -PAN -UAO)
> >>
> >> [    2.074298] pc : clk_core_set_parent_nolock+0x1d4/0x508
> >>
> >> [    2.079529] lr : clk_core_set_parent_nolock+0x1d0/0x508
> >>
> >>
> >> [    2.084759] sp : ffff80001003b9b0
> >>
> >>
> >> [    2.088072] x29: ffff80001003b9b0 x28: ffff8000116e8218
> >>
> >>
> >> [    2.093392] x27: 0000000000004570 x26: ffff8000128745d0
> >>
> >>
> >> [    2.098711] x25: ffff0000b8422008 x24: ffff0000b8422008
> >>
> >> [    2.104030] x23: ffff80001104a518 x22: ffff80001104a508
> >>
> >> [    2.109349] x21: ffff800012260bf8 x20: ffff0000b84c9600
> >>
> >> [    2.114668] x19: ffff0000b84cbb00 x18: 0000000000004530
> >>
> >> [    2.119987] x17: 0000000000004520 x16: 0000000000004510
> >>
> >>
> >> [    2.125307] x15: 00000000000045d0 x14: 0000000000004500
> >>
> >>
> >> [    2.130626] x13: 00000000000044f0 x12: 00000000000044e0
> >>
> >> [    2.135945] x11: ffff8000116e6c68 x10: ffff8000117d7000
> >>
> >>
> >> [    2.141264] x9 : ffff80001067007c x8 : 0000000000000000
> >>
> >> [    2.146583] x7 : ffff800010671938 x6 : 0000000000000000
> >>
> >>
> >> [    2.151903] x5 : ffff800011633000 x4 : 0000000000000000
> >>
> >> [    2.157222] x3 : ffff80001003b804 x2 : 0000000000000000
> >>
> >>
> >> [    2.162541] x1 : ffff0000b9da0000 x0 : 0000000000000000
> >>
> >> [    2.167862] Call trace:
> >>
> >> [    2.170307]  clk_core_set_parent_nolock+0x1d4/0x508
> >>
> >> [    2.175190]  clk_hw_set_parent+0x1c/0x28
> >>
> >> [    2.179114]  imx8mq_clocks_probe+0x3538/0x3668
> >>
> >> [    2.183562]  platform_drv_probe+0x58/0xa8
> >>
> >> [    2.187573]  really_probe+0xe0/0x440
> >>
> >>
> >> [    2.191145]  driver_probe_device+0xe4/0x138
> >> [    2.195333]  device_driver_attach+0x74/0x80
> >>
> >>
> >> [    2.199519]  __driver_attach+0xa8/0x170
> >>
> >> [    2.203354]  bus_for_each_dev+0x74/0xc8
> >>
> >>
> >> [    2.207190]  driver_attach+0x28/0x30
> >>
> >> [    2.210767]  bus_add_driver+0x144/0x228
> >>
> >>
> >> [    2.214605]  driver_register+0x68/0x118
> >>
> >> [    2.218438]  __platform_driver_register+0x4c/0x58
> >>
> >>
> >> [    2.223151]  imx8mq_clk_driver_init+0x20/0x28
> >>
> >> [    2.227511]  do_one_initcall+0x88/0x410
> >>
> >> [    2.231348]  kernel_init_freeable+0x24c/0x2c0
> >>
> >> [    2.235706]  kernel_init+0x18/0x108
> >>
> >> [    2.239192]  ret_from_fork+0x10/0x18
> >>
> >> [    2.242768] irq event stamp: 130084
> >>
> >> [    2.246262] hardirqs last  enabled at (130083): [<ffff800010302e78>]
> >> __slab_alloc.isra.0+0x90/0xb8
> >> [    2.255241] hardirqs last disabled at (130084): [<ffff8000100a60b0>]
> >> do_debug_exception+0x168/0x254
> >> [    2.264308] softirqs last  enabled at (130070): [<ffff800010080e88>]
> >> __do_softirq+0x490/0x56c
> >> [    2.272856] softirqs last disabled at (130057): [<ffff800010101e1c>]
> >> irq_exit+0x11c/0x148
> >> [    2.281057] ---[ end trace 1fae73b5c77d8120 ]---
> >> [    2.285792] ------------[ cut here ]------------
> >
> > I not met such warning when I test, you enabled lockdep debug?
> >
> >>
> >> This happens because clk_hw_set_parent does not take the prepare_lock
> >> so a lockdep_assert_held fails. In practice it should be mostly
> >> harmless because clk operations shouldn't happen while the SOC provider
> is probing.
> >>
> >> The issue can be worked around by doing the following instead:
> >>
> >> +       clk_set_parent(hws[IMX8MQ_CLK_A53_SRC]->clk,
> >> hws[IMX8MQ_SYS1_PLL_800M]->clk);
> >> +       clk_set_parent(hws[IMX8MQ_CLK_A53_CORE]->clk,
> >> hws[IMX8MQ_ARM_PLL_OUT]->clk);
> >>
> >> This implies reverting commit f95d58981f40 ("clk: imx: Include
> >> clk-provider.h instead of clk.h for i.MX8M SoCs clock driver") and
> >> somewhat rolls back the consumer/provider split.
> >>
> >> What would be a clean fix for this? It might make sense to add a new API.
> >>
> >
> > How about moving this to dts? I'll give a try.
> 
> The warning spam still happens in next-20200325.

Please help try https://patchwork.kernel.org/cover/11433775/

Thanks,
Peng.
diff mbox series

Patch

diff --git a/drivers/clk/imx/clk-imx8mq.c b/drivers/clk/imx/clk-imx8mq.c
index 1f5ea1eaad65..b81f02ab7eb1 100644
--- a/drivers/clk/imx/clk-imx8mq.c
+++ b/drivers/clk/imx/clk-imx8mq.c
@@ -41,6 +41,8 @@  static const char * const video2_pll_out_sels[] = {"video2_pll1_ref_sel", };
 static const char * const imx8mq_a53_sels[] = {"osc_25m", "arm_pll_out", "sys2_pll_500m", "sys2_pll_1000m",
 					"sys1_pll_800m", "sys1_pll_400m", "audio_pll1_out", "sys3_pll_out", };
 
+static const char * const imx8mq_a53_core_sels[] = {"arm_a53_div", "arm_pll_out", };
+
 static const char * const imx8mq_arm_m4_sels[] = {"osc_25m", "sys2_pll_200m", "sys2_pll_250m", "sys1_pll_266m",
 					"sys1_pll_800m", "audio_pll1_out", "video_pll1_out", "sys3_pll_out", };
 
@@ -425,6 +427,9 @@  static int imx8mq_clocks_probe(struct platform_device *pdev)
 	hws[IMX8MQ_CLK_GPU_SHADER_CG] = hws[IMX8MQ_CLK_GPU_SHADER];
 	hws[IMX8MQ_CLK_GPU_SHADER_DIV] = hws[IMX8MQ_CLK_GPU_SHADER];
 
+	/* CORE SEL */
+	hws[IMX8MQ_CLK_A53_CORE] = imx_clk_hw_mux2_flags("arm_a53_core", base + 0x9880, 24, 1, imx8mq_a53_core_sels, ARRAY_SIZE(imx8mq_a53_core_sels), CLK_IS_CRITICAL);
+
 	/* BUS */
 	hws[IMX8MQ_CLK_MAIN_AXI] = imx8m_clk_hw_composite_critical("main_axi", imx8mq_main_axi_sels, base + 0x8800);
 	hws[IMX8MQ_CLK_ENET_AXI] = imx8m_clk_hw_composite("enet_axi", imx8mq_enet_axi_sels, base + 0x8880);
@@ -588,11 +593,14 @@  static int imx8mq_clocks_probe(struct platform_device *pdev)
 	hws[IMX8MQ_GPT_3M_CLK] = imx_clk_hw_fixed_factor("gpt_3m", "osc_25m", 1, 8);
 	hws[IMX8MQ_CLK_DRAM_ALT_ROOT] = imx_clk_hw_fixed_factor("dram_alt_root", "dram_alt", 1, 4);
 
-	hws[IMX8MQ_CLK_ARM] = imx_clk_hw_cpu("arm", "arm_a53_div",
-					   hws[IMX8MQ_CLK_A53_DIV]->clk,
-					   hws[IMX8MQ_CLK_A53_SRC]->clk,
+	clk_hw_set_parent(hws[IMX8MQ_CLK_A53_SRC], hws[IMX8MQ_SYS1_PLL_800M]);
+	clk_hw_set_parent(hws[IMX8MQ_CLK_A53_CORE], hws[IMX8MQ_ARM_PLL_OUT]);
+
+	hws[IMX8MQ_CLK_ARM] = imx_clk_hw_cpu("arm", "arm_a53_core",
+					   hws[IMX8MQ_CLK_A53_CORE]->clk,
+					   hws[IMX8MQ_CLK_A53_CORE]->clk,
 					   hws[IMX8MQ_ARM_PLL_OUT]->clk,
-					   hws[IMX8MQ_SYS1_PLL_800M]->clk);
+					   hws[IMX8MQ_CLK_A53_DIV]->clk);
 
 	imx_check_clk_hws(hws, IMX8MQ_CLK_END);
 
diff --git a/include/dt-bindings/clock/imx8mq-clock.h b/include/dt-bindings/clock/imx8mq-clock.h
index 2b88723310bd..9b8045d75b8b 100644
--- a/include/dt-bindings/clock/imx8mq-clock.h
+++ b/include/dt-bindings/clock/imx8mq-clock.h
@@ -429,6 +429,8 @@ 
 #define IMX8MQ_CLK_M4_CORE			287
 #define IMX8MQ_CLK_VPU_CORE			288
 
-#define IMX8MQ_CLK_END				289
+#define IMX8MQ_CLK_A53_CORE			289
+
+#define IMX8MQ_CLK_END				290
 
 #endif /* __DT_BINDINGS_CLOCK_IMX8MQ_H */