[PATCH RESEND v3 0/4] clk: imx: imx8m: fix a53 cpu clock

[PATCH RESEND v3 0/4] clk: imx: imx8m: fix a53 cpu clock

[peng.fan]

From: Peng Fan <peng.fan@nxp.com>

V3:
 Rebased to Shawn's for-next branch
 Typo fix

V2:
 Fix i.MX8MP build
 Update cover letter, i.MX7D not have this issue 

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 voilates the CCM.

There is a CORE_SEL slice before A53 core, we need 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.

Peng Fan (4):
  clk: imx: imx8mq: fix a53 cpu clock
  clk: imx: imx8mm: fix a53 cpu clock
  clk: imx: imx8mn: fix a53 cpu clock
  clk: imx: imx8mp: fix a53 cpu clock

 drivers/clk/imx/clk-imx8mm.c             | 16 ++++++++++++----
 drivers/clk/imx/clk-imx8mn.c             | 16 ++++++++++++----
 drivers/clk/imx/clk-imx8mp.c             | 16 ++++++++++++----
 drivers/clk/imx/clk-imx8mq.c             | 16 ++++++++++++----
 include/dt-bindings/clock/imx8mm-clock.h |  4 +++-
 include/dt-bindings/clock/imx8mn-clock.h |  4 +++-
 include/dt-bindings/clock/imx8mp-clock.h |  3 ++-
 include/dt-bindings/clock/imx8mq-clock.h |  4 +++-
 8 files changed, 59 insertions(+), 20 deletions(-)

-- 
2.16.4

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

[peng.fan]

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(-)

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 */
-- 
2.16.4

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

[Leonard Crestez]

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

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

[Leonard Crestez]

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

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

[Peng Fan]

> 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

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

[Peng Fan]

> 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

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

[Leonard Crestez]

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.

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

[Peng Fan]

> 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.

[PATCH RESEND v3 2/4] clk: imx: imx8mm: fix a53 cpu clock

[peng.fan]

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 voilates the CCM.

There is a CORE_SEL slice before A53 core, we need 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: ba5625c3e272 ("clk: imx: Add clock driver support for imx8mm")
Reviewed-by: Jacky Bai <ping.bai@nxp.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/clk/imx/clk-imx8mm.c             | 16 ++++++++++++----
 include/dt-bindings/clock/imx8mm-clock.h |  4 +++-
 2 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/drivers/clk/imx/clk-imx8mm.c b/drivers/clk/imx/clk-imx8mm.c
index 2f2c240a86e2..f851cd447e7c 100644
--- a/drivers/clk/imx/clk-imx8mm.c
+++ b/drivers/clk/imx/clk-imx8mm.c
@@ -41,6 +41,8 @@ static const char *sys_pll3_bypass_sels[] = {"sys_pll3", "sys_pll3_ref_sel", };
 static const char *imx8mm_a53_sels[] = {"osc_24m", "arm_pll_out", "sys_pll2_500m", "sys_pll2_1000m",
 					"sys_pll1_800m", "sys_pll1_400m", "audio_pll1_out", "sys_pll3_out", };
 
+static const char * const imx8mm_a53_core_sels[] = {"arm_a53_div", "arm_pll_out", };
+
 static const char *imx8mm_m4_sels[] = {"osc_24m", "sys_pll2_200m", "sys_pll2_250m", "sys_pll1_266m",
 				       "sys_pll1_800m", "audio_pll1_out", "video_pll1_out", "sys_pll3_out", };
 
@@ -439,6 +441,9 @@ static int imx8mm_clocks_probe(struct platform_device *pdev)
 	hws[IMX8MM_CLK_GPU2D_CG] = hws[IMX8MM_CLK_GPU2D_CORE];
 	hws[IMX8MM_CLK_GPU2D_DIV] = hws[IMX8MM_CLK_GPU2D_CORE];
 
+	/* CORE SEL */
+	hws[IMX8MM_CLK_A53_CORE] = imx_clk_hw_mux2_flags("arm_a53_core", base + 0x9880, 24, 1, imx8mm_a53_core_sels, ARRAY_SIZE(imx8mm_a53_core_sels), CLK_IS_CRITICAL);
+
 	/* BUS */
 	hws[IMX8MM_CLK_MAIN_AXI] = imx8m_clk_hw_composite_critical("main_axi",  imx8mm_main_axi_sels, base + 0x8800);
 	hws[IMX8MM_CLK_ENET_AXI] = imx8m_clk_hw_composite("enet_axi", imx8mm_enet_axi_sels, base + 0x8880);
@@ -605,11 +610,14 @@ static int imx8mm_clocks_probe(struct platform_device *pdev)
 	hws[IMX8MM_CLK_DRAM_ALT_ROOT] = imx_clk_hw_fixed_factor("dram_alt_root", "dram_alt", 1, 4);
 	hws[IMX8MM_CLK_DRAM_CORE] = imx_clk_hw_mux2_flags("dram_core_clk", base + 0x9800, 24, 1, imx8mm_dram_core_sels, ARRAY_SIZE(imx8mm_dram_core_sels), CLK_IS_CRITICAL);
 
-	hws[IMX8MM_CLK_ARM] = imx_clk_hw_cpu("arm", "arm_a53_div",
-					   hws[IMX8MM_CLK_A53_DIV]->clk,
-					   hws[IMX8MM_CLK_A53_SRC]->clk,
+	clk_hw_set_parent(hws[IMX8MM_CLK_A53_SRC], hws[IMX8MM_SYS_PLL1_800M]);
+	clk_hw_set_parent(hws[IMX8MM_CLK_A53_CORE], hws[IMX8MM_ARM_PLL_OUT]);
+
+	hws[IMX8MM_CLK_ARM] = imx_clk_hw_cpu("arm", "arm_a53_core",
+					   hws[IMX8MM_CLK_A53_CORE]->clk,
+					   hws[IMX8MM_CLK_A53_CORE]->clk,
 					   hws[IMX8MM_ARM_PLL_OUT]->clk,
-					   hws[IMX8MM_SYS_PLL1_800M]->clk);
+					   hws[IMX8MM_CLK_A53_DIV]->clk);
 
 	imx_check_clk_hws(hws, IMX8MM_CLK_END);
 
diff --git a/include/dt-bindings/clock/imx8mm-clock.h b/include/dt-bindings/clock/imx8mm-clock.h
index dbfee6579d6c..e63a5530aed7 100644
--- a/include/dt-bindings/clock/imx8mm-clock.h
+++ b/include/dt-bindings/clock/imx8mm-clock.h
@@ -272,6 +272,8 @@
 
 #define IMX8MM_CLK_CLKO2			250
 
-#define IMX8MM_CLK_END				251
+#define IMX8MM_CLK_A53_CORE			251
+
+#define IMX8MM_CLK_END				252
 
 #endif
-- 
2.16.4

[PATCH RESEND v3 3/4] clk: imx: imx8mn: fix a53 cpu clock

[peng.fan]

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 voilates the CCM.

There is a CORE_SEL slice before A53 core, we need 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 clk.

Fixes: 96d6392b54db ("clk: imx: Add support for i.MX8MN clock driver")
Reviewed-by: Jacky Bai <ping.bai@nxp.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/clk/imx/clk-imx8mn.c             | 16 ++++++++++++----
 include/dt-bindings/clock/imx8mn-clock.h |  4 +++-
 2 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/drivers/clk/imx/clk-imx8mn.c b/drivers/clk/imx/clk-imx8mn.c
index 67b826d7184b..f44229ca19e8 100644
--- a/drivers/clk/imx/clk-imx8mn.c
+++ b/drivers/clk/imx/clk-imx8mn.c
@@ -40,6 +40,8 @@ static const char * const imx8mn_a53_sels[] = {"osc_24m", "arm_pll_out", "sys_pl
 					       "sys_pll2_1000m", "sys_pll1_800m", "sys_pll1_400m",
 					       "audio_pll1_out", "sys_pll3_out", };
 
+static const char * const imx8mn_a53_core_sels[] = {"arm_a53_div", "arm_pll_out", };
+
 static const char * const imx8mn_gpu_core_sels[] = {"osc_24m", "gpu_pll_out", "sys_pll1_800m",
 						    "sys_pll3_out", "sys_pll2_1000m", "audio_pll1_out",
 						    "video_pll1_out", "audio_pll2_out", };
@@ -427,6 +429,9 @@ static int imx8mn_clocks_probe(struct platform_device *pdev)
 	hws[IMX8MN_CLK_GPU_SHADER_CG] = hws[IMX8MN_CLK_GPU_SHADER];
 	hws[IMX8MN_CLK_GPU_SHADER_DIV] = hws[IMX8MN_CLK_GPU_SHADER];
 
+	/* CORE SEL */
+	hws[IMX8MN_CLK_A53_CORE] = imx_clk_hw_mux2_flags("arm_a53_core", base + 0x9880, 24, 1, imx8mn_a53_core_sels, ARRAY_SIZE(imx8mn_a53_core_sels), CLK_IS_CRITICAL);
+
 	/* BUS */
 	hws[IMX8MN_CLK_MAIN_AXI] = imx8m_clk_hw_composite_critical("main_axi", imx8mn_main_axi_sels, base + 0x8800);
 	hws[IMX8MN_CLK_ENET_AXI] = imx8m_clk_hw_composite("enet_axi", imx8mn_enet_axi_sels, base + 0x8880);
@@ -556,11 +561,14 @@ static int imx8mn_clocks_probe(struct platform_device *pdev)
 
 	hws[IMX8MN_CLK_DRAM_ALT_ROOT] = imx_clk_hw_fixed_factor("dram_alt_root", "dram_alt", 1, 4);
 
-	hws[IMX8MN_CLK_ARM] = imx_clk_hw_cpu("arm", "arm_a53_div",
-					   hws[IMX8MN_CLK_A53_DIV]->clk,
-					   hws[IMX8MN_CLK_A53_SRC]->clk,
+	clk_hw_set_parent(hws[IMX8MN_CLK_A53_SRC], hws[IMX8MN_SYS_PLL1_800M]);
+	clk_hw_set_parent(hws[IMX8MN_CLK_A53_CORE], hws[IMX8MN_ARM_PLL_OUT]);
+
+	hws[IMX8MN_CLK_ARM] = imx_clk_hw_cpu("arm", "arm_a53_core",
+					   hws[IMX8MN_CLK_A53_CORE]->clk,
+					   hws[IMX8MN_CLK_A53_CORE]->clk,
 					   hws[IMX8MN_ARM_PLL_OUT]->clk,
-					   hws[IMX8MN_SYS_PLL1_800M]->clk);
+					   hws[IMX8MN_CLK_A53_DIV]->clk);
 
 	imx_check_clk_hws(hws, IMX8MN_CLK_END);
 
diff --git a/include/dt-bindings/clock/imx8mn-clock.h b/include/dt-bindings/clock/imx8mn-clock.h
index 39e088f6f195..621ea0e87c67 100644
--- a/include/dt-bindings/clock/imx8mn-clock.h
+++ b/include/dt-bindings/clock/imx8mn-clock.h
@@ -232,6 +232,8 @@
 #define IMX8MN_CLK_GPU_CORE			212
 #define IMX8MN_CLK_GPU_SHADER			213
 
-#define IMX8MN_CLK_END				214
+#define IMX8MN_CLK_A53_CORE			214
+
+#define IMX8MN_CLK_END				215
 
 #endif
-- 
2.16.4

[PATCH RESEND v3 4/4] clk: imx: imx8mp: fix a53 cpu clock

[peng.fan]

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 voilates the CCM.

There is a CORE_SEL slice before A53 core, we need 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 clk

Reviewed-by: Jacky Bai <ping.bai@nxp.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/clk/imx/clk-imx8mp.c             | 16 ++++++++++++----
 include/dt-bindings/clock/imx8mp-clock.h |  3 ++-
 2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/clk/imx/clk-imx8mp.c b/drivers/clk/imx/clk-imx8mp.c
index a16af4fce044..d67ee36b84de 100644
--- a/drivers/clk/imx/clk-imx8mp.c
+++ b/drivers/clk/imx/clk-imx8mp.c
@@ -34,6 +34,8 @@ static const char * const imx8mp_a53_sels[] = {"osc_24m", "arm_pll_out", "sys_pl
 					       "sys_pll2_1000m", "sys_pll1_800m", "sys_pll1_400m",
 					       "audio_pll1_out", "sys_pll3_out", };
 
+static const char * const imx8mp_a53_core_sels[] = {"arm_a53_div", "arm_pll_out", };
+
 static const char * const imx8mp_m7_sels[] = {"osc_24m", "sys_pll2_200m", "sys_pll2_250m",
 					      "vpu_pll_out", "sys_pll1_800m", "audio_pll1_out",
 					      "video_pll1_out", "sys_pll3_out", };
@@ -554,6 +556,9 @@ static int imx8mp_clocks_probe(struct platform_device *pdev)
 	hws[IMX8MP_CLK_HSIO_AXI_DIV] = imx_clk_hw_divider2("hsio_axi_div", "hsio_axi_cg", ccm_base + 0x8380, 0, 3);
 	hws[IMX8MP_CLK_MEDIA_ISP_DIV] = imx_clk_hw_divider2("media_isp_div", "media_isp_cg", ccm_base + 0x8400, 0, 3);
 
+	/* CORE SEL */
+	hws[IMX8MP_CLK_A53_CORE] = imx_clk_hw_mux2_flags("arm_a53_core", ccm_base + 0x9880, 24, 1, imx8mp_a53_core_sels, ARRAY_SIZE(imx8mp_a53_core_sels), CLK_IS_CRITICAL);
+
 	hws[IMX8MP_CLK_MAIN_AXI] = imx8m_clk_hw_composite_critical("main_axi", imx8mp_main_axi_sels, ccm_base + 0x8800);
 	hws[IMX8MP_CLK_ENET_AXI] = imx8m_clk_hw_composite("enet_axi", imx8mp_enet_axi_sels, ccm_base + 0x8880);
 	hws[IMX8MP_CLK_NAND_USDHC_BUS] = imx8m_clk_hw_composite_critical("nand_usdhc_bus", imx8mp_nand_usdhc_sels, ccm_base + 0x8900);
@@ -724,11 +729,14 @@ static int imx8mp_clocks_probe(struct platform_device *pdev)
 	hws[IMX8MP_CLK_VPU_ROOT] = imx_clk_hw_gate4("vpu_root_clk", "vpu_bus", ccm_base + 0x4630, 0);
 	hws[IMX8MP_CLK_AUDIO_ROOT] = imx_clk_hw_gate4("audio_root_clk", "ipg_root", ccm_base + 0x4650, 0);
 
-	hws[IMX8MP_CLK_ARM] = imx_clk_hw_cpu("arm", "arm_a53_div",
-					     hws[IMX8MP_CLK_A53_DIV]->clk,
-					     hws[IMX8MP_CLK_A53_SRC]->clk,
+	clk_hw_set_parent(hws[IMX8MP_CLK_A53_SRC], hws[IMX8MP_SYS_PLL1_800M]);
+	clk_hw_set_parent(hws[IMX8MP_CLK_A53_CORE], hws[IMX8MP_ARM_PLL_OUT]);
+
+	hws[IMX8MP_CLK_ARM] = imx_clk_hw_cpu("arm", "arm_a53_core",
+					     hws[IMX8MP_CLK_A53_CORE]->clk,
+					     hws[IMX8MP_CLK_A53_CORE]->clk,
 					     hws[IMX8MP_ARM_PLL_OUT]->clk,
-					     hws[IMX8MP_SYS_PLL1_800M]->clk);
+					     hws[IMX8MP_CLK_A53_DIV]->clk);
 
 	imx_check_clk_hws(hws, IMX8MP_CLK_END);
 
diff --git a/include/dt-bindings/clock/imx8mp-clock.h b/include/dt-bindings/clock/imx8mp-clock.h
index 2fab63186bca..c92d1f4117eb 100644
--- a/include/dt-bindings/clock/imx8mp-clock.h
+++ b/include/dt-bindings/clock/imx8mp-clock.h
@@ -294,7 +294,8 @@
 #define IMX8MP_CLK_DRAM_ALT_ROOT		285
 #define IMX8MP_CLK_DRAM_CORE			286
 #define IMX8MP_CLK_ARM				287
+#define IMX8MP_CLK_A53_CORE			288
 
-#define IMX8MP_CLK_END				288
+#define IMX8MP_CLK_END				289
 
 #endif
-- 
2.16.4

RE: [PATCH RESEND v3 4/4] clk: imx: imx8mp: fix a53 cpu clock

[Peng Fan]

Hi Shawn,

> Subject: [PATCH RESEND v3 4/4] clk: imx: imx8mp: fix a53 cpu clock

Would you pick up this? Without this patch, i.MX8MP will hang when
booting.

Thanks,
Peng.
> 
> 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 voilates the CCM.
> 
> There is a CORE_SEL slice before A53 core, we need 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 clk
> 
> Reviewed-by: Jacky Bai <ping.bai@nxp.com>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  drivers/clk/imx/clk-imx8mp.c             | 16 ++++++++++++----
>  include/dt-bindings/clock/imx8mp-clock.h |  3 ++-
>  2 files changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/clk/imx/clk-imx8mp.c b/drivers/clk/imx/clk-imx8mp.c
> index a16af4fce044..d67ee36b84de 100644
> --- a/drivers/clk/imx/clk-imx8mp.c
> +++ b/drivers/clk/imx/clk-imx8mp.c
> @@ -34,6 +34,8 @@ static const char * const imx8mp_a53_sels[] =
> {"osc_24m", "arm_pll_out", "sys_pl
>  					       "sys_pll2_1000m", "sys_pll1_800m",
> "sys_pll1_400m",
>  					       "audio_pll1_out", "sys_pll3_out", };
> 
> +static const char * const imx8mp_a53_core_sels[] = {"arm_a53_div",
> +"arm_pll_out", };
> +
>  static const char * const imx8mp_m7_sels[] = {"osc_24m", "sys_pll2_200m",
> "sys_pll2_250m",
>  					      "vpu_pll_out", "sys_pll1_800m",
> "audio_pll1_out",
>  					      "video_pll1_out", "sys_pll3_out", }; @@
> -554,6 +556,9 @@ static int imx8mp_clocks_probe(struct platform_device
> *pdev)
>  	hws[IMX8MP_CLK_HSIO_AXI_DIV] =
> imx_clk_hw_divider2("hsio_axi_div", "hsio_axi_cg", ccm_base + 0x8380, 0, 3);
>  	hws[IMX8MP_CLK_MEDIA_ISP_DIV] =
> imx_clk_hw_divider2("media_isp_div", "media_isp_cg", ccm_base + 0x8400,
> 0, 3);
> 
> +	/* CORE SEL */
> +	hws[IMX8MP_CLK_A53_CORE] =
> imx_clk_hw_mux2_flags("arm_a53_core",
> +ccm_base + 0x9880, 24, 1, imx8mp_a53_core_sels,
> +ARRAY_SIZE(imx8mp_a53_core_sels), CLK_IS_CRITICAL);
> +
>  	hws[IMX8MP_CLK_MAIN_AXI] =
> imx8m_clk_hw_composite_critical("main_axi", imx8mp_main_axi_sels,
> ccm_base + 0x8800);
>  	hws[IMX8MP_CLK_ENET_AXI] = imx8m_clk_hw_composite("enet_axi",
> imx8mp_enet_axi_sels, ccm_base + 0x8880);
>  	hws[IMX8MP_CLK_NAND_USDHC_BUS] =
> imx8m_clk_hw_composite_critical("nand_usdhc_bus",
> imx8mp_nand_usdhc_sels, ccm_base + 0x8900); @@ -724,11 +729,14 @@
> static int imx8mp_clocks_probe(struct platform_device *pdev)
>  	hws[IMX8MP_CLK_VPU_ROOT] = imx_clk_hw_gate4("vpu_root_clk",
> "vpu_bus", ccm_base + 0x4630, 0);
>  	hws[IMX8MP_CLK_AUDIO_ROOT] =
> imx_clk_hw_gate4("audio_root_clk", "ipg_root", ccm_base + 0x4650, 0);
> 
> -	hws[IMX8MP_CLK_ARM] = imx_clk_hw_cpu("arm", "arm_a53_div",
> -					     hws[IMX8MP_CLK_A53_DIV]->clk,
> -					     hws[IMX8MP_CLK_A53_SRC]->clk,
> +	clk_hw_set_parent(hws[IMX8MP_CLK_A53_SRC],
> hws[IMX8MP_SYS_PLL1_800M]);
> +	clk_hw_set_parent(hws[IMX8MP_CLK_A53_CORE],
> hws[IMX8MP_ARM_PLL_OUT]);
> +
> +	hws[IMX8MP_CLK_ARM] = imx_clk_hw_cpu("arm", "arm_a53_core",
> +					     hws[IMX8MP_CLK_A53_CORE]->clk,
> +					     hws[IMX8MP_CLK_A53_CORE]->clk,
>  					     hws[IMX8MP_ARM_PLL_OUT]->clk,
> -					     hws[IMX8MP_SYS_PLL1_800M]->clk);
> +					     hws[IMX8MP_CLK_A53_DIV]->clk);
> 
>  	imx_check_clk_hws(hws, IMX8MP_CLK_END);
> 
> diff --git a/include/dt-bindings/clock/imx8mp-clock.h
> b/include/dt-bindings/clock/imx8mp-clock.h
> index 2fab63186bca..c92d1f4117eb 100644
> --- a/include/dt-bindings/clock/imx8mp-clock.h
> +++ b/include/dt-bindings/clock/imx8mp-clock.h
> @@ -294,7 +294,8 @@
>  #define IMX8MP_CLK_DRAM_ALT_ROOT		285
>  #define IMX8MP_CLK_DRAM_CORE			286
>  #define IMX8MP_CLK_ARM				287
> +#define IMX8MP_CLK_A53_CORE			288
> 
> -#define IMX8MP_CLK_END				288
> +#define IMX8MP_CLK_END				289
> 
>  #endif
> --
> 2.16.4

Re: [PATCH RESEND v3 0/4] clk: imx: imx8m: fix a53 cpu clock

[Shawn Guo]

On Wed, Feb 19, 2020 at 06:17:05PM +0800, peng.fan@nxp.com wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> V3:
>  Rebased to Shawn's for-next branch
>  Typo fix
> 
> V2:
>  Fix i.MX8MP build
>  Update cover letter, i.MX7D not have this issue 
> 
> 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 voilates the CCM.
> 
> There is a CORE_SEL slice before A53 core, we need 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.
> 
> Peng Fan (4):
>   clk: imx: imx8mq: fix a53 cpu clock
>   clk: imx: imx8mm: fix a53 cpu clock
>   clk: imx: imx8mn: fix a53 cpu clock
>   clk: imx: imx8mp: fix a53 cpu clock

Applied all, thanks.