2020-02-19 10:23:32

by Peng Fan

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

From: Peng Fan <[email protected]>

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


2020-02-19 10:23:41

by Peng Fan

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

From: Peng Fan <[email protected]>

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 <[email protected]>
Signed-off-by: Peng Fan <[email protected]>
---
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

2020-02-19 10:23:50

by Peng Fan

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

From: Peng Fan <[email protected]>

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 <[email protected]>
Signed-off-by: Peng Fan <[email protected]>
---
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

2020-02-19 10:24:17

by Peng Fan

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

From: Peng Fan <[email protected]>

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 <[email protected]>
Signed-off-by: Peng Fan <[email protected]>
---
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

2020-02-19 10:25:15

by Peng Fan

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

From: Peng Fan <[email protected]>

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 <[email protected]>
Signed-off-by: Peng Fan <[email protected]>
---
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

2020-02-24 01:14:16

by Peng Fan

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

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 <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Peng Fan <[email protected]>
> ---
> 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

2020-02-24 03:39:45

by Shawn Guo

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

On Wed, Feb 19, 2020 at 06:17:05PM +0800, [email protected] wrote:
> From: Peng Fan <[email protected]>
>
> 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.

2020-03-10 19:39:36

by Leonard Crestez

[permalink] [raw]
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 <[email protected]>
>
> 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

2020-03-11 01:18:02

by Peng Fan

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

> 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 <[email protected]>
> >
> > 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

2020-03-25 12:26:12

by Leonard Crestez

[permalink] [raw]
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 <[email protected]>
>>>
>>> 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.

2020-03-25 12:29:17

by Peng Fan

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

> 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 <[email protected]>
> >>>
> >>> 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.