Defined IMX8MP_CLK_M7_SRC and IMX8MP_CLK_M7_DIV in imx8mp-clock.h
but never assigned. It will cause the system to hang if using them.
Alias IMX8MP_CLK_M7_SRC and IMX8MP_CLK_M7_DIV to IMX8MP_CLK_M7_CORE
for backward compatibility.
Fixes: 8c83a8ff4dd9 (clk: imx8mp: use imx8m_clk_hw_composite_core to simplify code)
Reviewed-by: Marco Felsch <[email protected]>
Reviewed-by: Abel Vesa <[email protected]>
Signed-off-by: LI Qingwu <[email protected]>
---
drivers/clk/imx/clk-imx8mp.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/clk/imx/clk-imx8mp.c b/drivers/clk/imx/clk-imx8mp.c
index 652ae58c2735..601eea7ab99d 100644
--- a/drivers/clk/imx/clk-imx8mp.c
+++ b/drivers/clk/imx/clk-imx8mp.c
@@ -522,6 +522,8 @@ static int imx8mp_clocks_probe(struct platform_device *pdev)
hws[IMX8MP_CLK_A53_SRC] = hws[IMX8MP_CLK_A53_DIV];
hws[IMX8MP_CLK_A53_CG] = hws[IMX8MP_CLK_A53_DIV];
hws[IMX8MP_CLK_M7_CORE] = imx8m_clk_hw_composite_core("m7_core", imx8mp_m7_sels, ccm_base + 0x8080);
+ hws[IMX8MP_CLK_M7_DIV] = hws[IMX8MP_CLK_M7_CORE];
+ hws[IMX8MP_CLK_M7_SRC] = hws[IMX8MP_CLK_M7_CORE];
hws[IMX8MP_CLK_ML_CORE] = imx8m_clk_hw_composite_core("ml_core", imx8mp_ml_sels, ccm_base + 0x8100);
hws[IMX8MP_CLK_GPU3D_CORE] = imx8m_clk_hw_composite_core("gpu3d_core", imx8mp_gpu3d_core_sels, ccm_base + 0x8180);
hws[IMX8MP_CLK_GPU3D_SHADER_CORE] = imx8m_clk_hw_composite("gpu3d_shader_core", imx8mp_gpu3d_shader_sels, ccm_base + 0x8200);
--
2.25.1
On 23-01-11 11:10:30, LI Qingwu wrote:
> Defined IMX8MP_CLK_M7_SRC and IMX8MP_CLK_M7_DIV in imx8mp-clock.h
> but never assigned. It will cause the system to hang if using them.
> Alias IMX8MP_CLK_M7_SRC and IMX8MP_CLK_M7_DIV to IMX8MP_CLK_M7_CORE
> for backward compatibility.
>
> Fixes: 8c83a8ff4dd9 (clk: imx8mp: use imx8m_clk_hw_composite_core to simplify code)
> Reviewed-by: Marco Felsch <[email protected]>
> Reviewed-by: Abel Vesa <[email protected]>
> Signed-off-by: LI Qingwu <[email protected]>
Stephen, can you please pick this up yourself?
Thanks!
> ---
> drivers/clk/imx/clk-imx8mp.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/clk/imx/clk-imx8mp.c b/drivers/clk/imx/clk-imx8mp.c
> index 652ae58c2735..601eea7ab99d 100644
> --- a/drivers/clk/imx/clk-imx8mp.c
> +++ b/drivers/clk/imx/clk-imx8mp.c
> @@ -522,6 +522,8 @@ static int imx8mp_clocks_probe(struct platform_device *pdev)
> hws[IMX8MP_CLK_A53_SRC] = hws[IMX8MP_CLK_A53_DIV];
> hws[IMX8MP_CLK_A53_CG] = hws[IMX8MP_CLK_A53_DIV];
> hws[IMX8MP_CLK_M7_CORE] = imx8m_clk_hw_composite_core("m7_core", imx8mp_m7_sels, ccm_base + 0x8080);
> + hws[IMX8MP_CLK_M7_DIV] = hws[IMX8MP_CLK_M7_CORE];
> + hws[IMX8MP_CLK_M7_SRC] = hws[IMX8MP_CLK_M7_CORE];
> hws[IMX8MP_CLK_ML_CORE] = imx8m_clk_hw_composite_core("ml_core", imx8mp_ml_sels, ccm_base + 0x8100);
> hws[IMX8MP_CLK_GPU3D_CORE] = imx8m_clk_hw_composite_core("gpu3d_core", imx8mp_gpu3d_core_sels, ccm_base + 0x8180);
> hws[IMX8MP_CLK_GPU3D_SHADER_CORE] = imx8m_clk_hw_composite("gpu3d_shader_core", imx8mp_gpu3d_shader_sels, ccm_base + 0x8200);
> --
> 2.25.1
>
Quoting Abel Vesa (2023-01-16 06:28:33)
> On 23-01-11 11:10:30, LI Qingwu wrote:
> > Defined IMX8MP_CLK_M7_SRC and IMX8MP_CLK_M7_DIV in imx8mp-clock.h
> > but never assigned. It will cause the system to hang if using them.
> > Alias IMX8MP_CLK_M7_SRC and IMX8MP_CLK_M7_DIV to IMX8MP_CLK_M7_CORE
> > for backward compatibility.
> >
> > Fixes: 8c83a8ff4dd9 (clk: imx8mp: use imx8m_clk_hw_composite_core to simplify code)
> > Reviewed-by: Marco Felsch <[email protected]>
> > Reviewed-by: Abel Vesa <[email protected]>
> > Signed-off-by: LI Qingwu <[email protected]>
>
> Stephen, can you please pick this up yourself?
>
Is it needed for clk-fixes? When did the system start hanging?
On 23-01-18 11:00:36, Stephen Boyd wrote:
> Quoting Abel Vesa (2023-01-16 06:28:33)
> > On 23-01-11 11:10:30, LI Qingwu wrote:
> > > Defined IMX8MP_CLK_M7_SRC and IMX8MP_CLK_M7_DIV in imx8mp-clock.h
> > > but never assigned. It will cause the system to hang if using them.
> > > Alias IMX8MP_CLK_M7_SRC and IMX8MP_CLK_M7_DIV to IMX8MP_CLK_M7_CORE
> > > for backward compatibility.
> > >
> > > Fixes: 8c83a8ff4dd9 (clk: imx8mp: use imx8m_clk_hw_composite_core to simplify code)
> > > Reviewed-by: Marco Felsch <[email protected]>
> > > Reviewed-by: Abel Vesa <[email protected]>
> > > Signed-off-by: LI Qingwu <[email protected]>
> >
> > Stephen, can you please pick this up yourself?
> >
>
> Is it needed for clk-fixes? When did the system start hanging?
Li, Peng, can you provide some details here?
> -----Original Message-----
> From: Abel Vesa <[email protected]>
> Sent: Sunday, January 29, 2023 2:39 AM
> To: Stephen Boyd <[email protected]>; LI Qingwu
> <[email protected]>; Peng Fan <[email protected]>
> Cc: LI Qingwu <[email protected]>; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected];
> GEO-CHHER-bsp-development <[email protected]>;
> Marco Felsch <[email protected]>
> Subject: Re: [PATCH V5 1/1] clk: imx8mp: Alias M7 SRC/DIV to M7 CORE
>
> [Some people who received this message don't often get email from
> [email protected]. Learn why this is important at
> https://aka.ms/LearnAboutSenderIdentification ]
>
> This email is not from Hexagon's Office 365 instance. Please be careful while
> clicking links, opening attachments, or replying to this email.
>
>
> On 23-01-18 11:00:36, Stephen Boyd wrote:
> > Quoting Abel Vesa (2023-01-16 06:28:33)
> > > On 23-01-11 11:10:30, LI Qingwu wrote:
> > > > Defined IMX8MP_CLK_M7_SRC and IMX8MP_CLK_M7_DIV in
> imx8mp-clock.h
> > > > but never assigned. It will cause the system to hang if using them.
> > > > Alias IMX8MP_CLK_M7_SRC and IMX8MP_CLK_M7_DIV to
> > > > IMX8MP_CLK_M7_CORE for backward compatibility.
> > > >
> > > > Fixes: 8c83a8ff4dd9 (clk: imx8mp: use imx8m_clk_hw_composite_core
> > > > to simplify code)
> > > > Reviewed-by: Marco Felsch <[email protected]>
> > > > Reviewed-by: Abel Vesa <[email protected]>
> > > > Signed-off-by: LI Qingwu <[email protected]>
> > >
> > > Stephen, can you please pick this up yourself?
> > >
> >
> > Is it needed for clk-fixes? When did the system start hanging?
>
> Li, Peng, can you provide some details here?
Since IMX8MP_CLK_M7_SRC and IMX8MP_CLK_M7_DIV were not assigned,
if using any one of them in device tree, Linux system hanging after start M7 core.
> -----Original Message-----
> From: Stephen Boyd <[email protected]>
> Sent: Tuesday, January 31, 2023 6:35 AM
> To: Abel Vesa <[email protected]>; LI Qingwu
> <[email protected]>; Peng Fan <[email protected]>
> Cc: abelvesa@ <kernel.org [email protected]>; mturquette@
> <baylibre.com [email protected]>; shawnguo@ <kernel.org
> [email protected]>; s.hauer@ <pengutronix.de
> [email protected]>; kernel@ <pengutronix.de
> [email protected]>; festevam@ <gmail.com [email protected]>;
> linux-imx@ <nxp.com [email protected]>; linux-clk@ <vger.kernel.org
> [email protected]>; linux-arm-kernel@ <lists.infradead.org
> [email protected]>; linux-kernel@ <vger.kernel.org
> [email protected]>; 18701859600@ <163.com
> [email protected]>; GEO-CHHER-bsp-development
> <[email protected]>; Marco Felsch
> <[email protected]>
> Subject: RE: [PATCH V5 1/1] clk: imx8mp: Alias M7 SRC/DIV to M7 CORE
>
> This email is not from Hexagon’s Office 365 instance. Please be
> careful while clicking links, opening attachments, or replying to this email.
>
>
> Quoting LI Qingwu (2023-01-29 23:44:22)
> >
> > > > Is it needed for clk-fixes? When did the system start hanging?
> > >
> > > Li, Peng, can you provide some details here?
> >
> > Since IMX8MP_CLK_M7_SRC and IMX8MP_CLK_M7_DIV were not assigned, if
> > using any one of them in device tree, Linux system hanging after
> > start M7
> core.
>
> $ git grep -e IMX8MP_CLK_M7_SRC -o -e IMX8MP_CLK_M7_DIV
> include/dt-bindings/clock/imx8mp-clock.h:IMX8MP_CLK_M7_SRC
> include/dt-bindings/clock/imx8mp-clock.h:IMX8MP_CLK_M7_DIV
>
> so we're OK to let Abel apply this?
Just check it's OK to apply or not? Why ?
On 23-01-30 14:34:30, Stephen Boyd wrote:
> Quoting LI Qingwu (2023-01-29 23:44:22)
> >
> > > > Is it needed for clk-fixes? When did the system start hanging?
> > >
> > > Li, Peng, can you provide some details here?
> >
> > Since IMX8MP_CLK_M7_SRC and IMX8MP_CLK_M7_DIV were not assigned,
> > if using any one of them in device tree, Linux system hanging after start M7 core.
>
> $ git grep -e IMX8MP_CLK_M7_SRC -o -e IMX8MP_CLK_M7_DIV
> include/dt-bindings/clock/imx8mp-clock.h:IMX8MP_CLK_M7_SRC
> include/dt-bindings/clock/imx8mp-clock.h:IMX8MP_CLK_M7_DIV
>
> so we're OK to let Abel apply this?
Replying here as the CC emails in Li's next-in-thread email are all messed up and
the reply I sent there bounced back.
As Stephen pointed out, in upstream, there is no current user for those
two clock IDs. So I suppose we can keep the Fixes tag, but currently,
on upstream, there is no chance a consumer could hang as there is no
consumer yet for these clocks.
Stephen, I'll add this to my 6.4 material, again, since there is no
actual comsumer yet, if that's OK with you.