[RFC PATCH] imx8mp: fix boot hang when booting NXP kernel 5.15.32

[RFC PATCH] imx8mp: fix boot hang when booting NXP kernel 5.15.32

[Rasmus Villemoes]

We have observed a somewhat weird bug: When booting the downstream NXP
kernel lf-5.15.32-2.0.0 [fa6c3168595c], sometimes the board would hang
during imx_lcdifv3_probe(). Adding some printk debugging revealed that
the hang always happened at the

  writel(CTRL_SW_RESET, lcdifv3->base + LCDIFV3_CTRL_CLR);

However, only some of our imx8mp EVK boards and some of our custom
imx8mp-based boards showed this; others never seemed to show it,
making us initially suspect a hardware/board assembly error, though it
would be weird for that to apply to both our design and the EVKs.

Moreover, for the boards that did have this behaviour, applying a
generous amount of cooling spray to the SOC did make it boot, while
conversely heating it up before booting was a sure way to make it
hang. But even after that discovery, applying heat to the boards that
seemed to be immune from this bug didn't make them hang either.

It is also worth mentioning that whenever the boards did boot,
i.e. get past that critical line in probe(), whether those of the
"immune" kind or those which we cooled sufficiently, graphics appeared
to work just fine.

Eventually, we discovered that when using a downstream NXP U-Boot
[lf_v2022.04, 1c881f4da8], this bug never happened. So I started
bisecting between v2022.04 and lf_v2022.04, leading to

  commit 610e1b1246f7832bd96bfa9615e043565a19ac1b
  Author: Ye Li <ye.li@nxp.com>
  Date:   Mon Mar 30 01:56:03 2020 -0700

    MLK-23574-22 imx8m: clock: Sync clock settings with imx_v2020.04

Now that commit does a lot of things, but it wasn't hard to figure out
that the part that was relevant to our case was the addition of the
enable_display_clk() function.

Since I only have imx8mp boards (some EVKs and a few custom designs),
this only adds the enable_display_clk() for that SOC. But this really
seems like something that the kernel itself should (be able to) take
care of, without relying on the bootloader having done such random
magic.

Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
---

I don't know if upstream U-Boot cares about being able to boot a
downstream NXP linux kernel. Or if this really should be fixed on the
kernel side, making the lcdif driver properly configure the clock(s)
before lifting the reset bit. But if somebody else runs into this
issue, hopefully just this patch submission will at least save them
some time.

Can someone from NXP explain what's going on? In particular, how come
graphics works just fine even when, apparently, clocks have not been
properly configured? And why does this only happen for some boards,
but not others that should be physically identical? What's with the
temperature dependency?

 arch/arm/mach-imx/imx8m/clock_imx8mm.c | 27 +++++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-imx/imx8m/clock_imx8mm.c b/arch/arm/mach-imx/imx8m/clock_imx8mm.c
index 4db55f8608..96a9eb4dd3 100644
--- a/arch/arm/mach-imx/imx8m/clock_imx8mm.c
+++ b/arch/arm/mach-imx/imx8m/clock_imx8mm.c
@@ -45,7 +45,6 @@ int enable_i2c_clk(unsigned char enable, unsigned i2c_num)
 	return 0;
 }
 
-#ifdef CONFIG_SPL_BUILD
 static struct imx_int_pll_rate_table imx8mm_fracpll_tbl[] = {
 	PLL_1443X_RATE(1000000000U, 250, 3, 1, 0),
 	PLL_1443X_RATE(933000000U, 311, 4, 1, 0),
@@ -124,6 +123,8 @@ static int fracpll_configure(enum pll_clocks pll, u32 freq)
 	return 0;
 }
 
+#ifdef CONFIG_SPL_BUILD
+
 void dram_pll_init(ulong pll_val)
 {
 	fracpll_configure(ANATOP_DRAM_PLL, pll_val);
@@ -298,6 +299,28 @@ int intpll_configure(enum pll_clocks pll, ulong freq)
 	return 0;
 }
 
+#define VIDEO_PLL_RATE 594000000U
+
+static void enable_display_clk(void)
+{
+	if (IS_ENABLED(CONFIG_IMX8MP)) {
+		clock_enable(CCGR_DISPMIX, false);
+
+		/* Set Video PLL to 594Mhz, p = 1, m = 99, k = 0, s = 2 */
+		fracpll_configure(ANATOP_VIDEO_PLL, VIDEO_PLL_RATE);
+
+		/* 500Mhz */
+		clock_set_target_val(MEDIA_AXI_CLK_ROOT, CLK_ROOT_ON | CLK_ROOT_SOURCE_SEL(1) | CLK_ROOT_PRE_DIV(CLK_ROOT_PRE_DIV2));
+
+		/* 200Mhz */
+		clock_set_target_val(MEDIA_APB_CLK_ROOT, CLK_ROOT_ON | CLK_ROOT_SOURCE_SEL(2) | CLK_ROOT_PRE_DIV(CLK_ROOT_PRE_DIV4));
+
+		/* 27Mhz MIPI DPHY PLL ref from video PLL */
+		clock_set_target_val(MEDIA_MIPI_PHY1_REF_CLK_ROOT, CLK_ROOT_ON | CLK_ROOT_SOURCE_SEL(7) | CLK_ROOT_POST_DIV(CLK_ROOT_POST_DIV22));
+		clock_enable(CCGR_DISPMIX, true);
+	}
+}
+
 void init_uart_clk(u32 index)
 {
 	/*
@@ -485,6 +508,8 @@ int clock_init(void)
 
 	clock_enable(CCGR_SEC_DEBUG, 1);
 
+	enable_display_clk();
+
 	return 0;
 };
 
-- 
2.37.2

Re: [RFC PATCH] imx8mp: fix boot hang when booting NXP kernel 5.15.32

[Marek Vasut]

On 8/23/22 15:36, Rasmus Villemoes wrote:
> We have observed a somewhat weird bug: When booting the downstream NXP
> kernel lf-5.15.32-2.0.0 [fa6c3168595c], sometimes the board would hang
> during imx_lcdifv3_probe(). Adding some printk debugging revealed that
> the hang always happened at the
> 
>    writel(CTRL_SW_RESET, lcdifv3->base + LCDIFV3_CTRL_CLR);
> 
> However, only some of our imx8mp EVK boards and some of our custom
> imx8mp-based boards showed this; others never seemed to show it,
> making us initially suspect a hardware/board assembly error, though it
> would be weird for that to apply to both our design and the EVKs.
> 
> Moreover, for the boards that did have this behaviour, applying a
> generous amount of cooling spray to the SOC did make it boot, while
> conversely heating it up before booting was a sure way to make it
> hang. But even after that discovery, applying heat to the boards that
> seemed to be immune from this bug didn't make them hang either.
> 
> It is also worth mentioning that whenever the boards did boot,
> i.e. get past that critical line in probe(), whether those of the
> "immune" kind or those which we cooled sufficiently, graphics appeared
> to work just fine.
> 
> Eventually, we discovered that when using a downstream NXP U-Boot
> [lf_v2022.04, 1c881f4da8], this bug never happened. So I started
> bisecting between v2022.04 and lf_v2022.04, leading to
> 
>    commit 610e1b1246f7832bd96bfa9615e043565a19ac1b
>    Author: Ye Li <ye.li@nxp.com>
>    Date:   Mon Mar 30 01:56:03 2020 -0700
> 
>      MLK-23574-22 imx8m: clock: Sync clock settings with imx_v2020.04
> 
> Now that commit does a lot of things, but it wasn't hard to figure out
> that the part that was relevant to our case was the addition of the
> enable_display_clk() function.
> 
> Since I only have imx8mp boards (some EVKs and a few custom designs),
> this only adds the enable_display_clk() for that SOC. But this really
> seems like something that the kernel itself should (be able to) take
> care of, without relying on the bootloader having done such random
> magic.
> 
> Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
> ---
> 
> I don't know if upstream U-Boot cares about being able to boot a
> downstream NXP linux kernel. Or if this really should be fixed on the
> kernel side, making the lcdif driver properly configure the clock(s)
> before lifting the reset bit. But if somebody else runs into this
> issue, hopefully just this patch submission will at least save them
> some time.
> 
> Can someone from NXP explain what's going on? In particular, how come
> graphics works just fine even when, apparently, clocks have not been
> properly configured? And why does this only happen for some boards,
> but not others that should be physically identical? What's with the
> temperature dependency?
> 
>   arch/arm/mach-imx/imx8m/clock_imx8mm.c | 27 +++++++++++++++++++++++++-
>   1 file changed, 26 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mach-imx/imx8m/clock_imx8mm.c b/arch/arm/mach-imx/imx8m/clock_imx8mm.c
> index 4db55f8608..96a9eb4dd3 100644
> --- a/arch/arm/mach-imx/imx8m/clock_imx8mm.c
> +++ b/arch/arm/mach-imx/imx8m/clock_imx8mm.c
> @@ -45,7 +45,6 @@ int enable_i2c_clk(unsigned char enable, unsigned i2c_num)
>   	return 0;
>   }
>   
> -#ifdef CONFIG_SPL_BUILD
>   static struct imx_int_pll_rate_table imx8mm_fracpll_tbl[] = {
>   	PLL_1443X_RATE(1000000000U, 250, 3, 1, 0),
>   	PLL_1443X_RATE(933000000U, 311, 4, 1, 0),
> @@ -124,6 +123,8 @@ static int fracpll_configure(enum pll_clocks pll, u32 freq)
>   	return 0;
>   }
>   
> +#ifdef CONFIG_SPL_BUILD
> +
>   void dram_pll_init(ulong pll_val)
>   {
>   	fracpll_configure(ANATOP_DRAM_PLL, pll_val);
> @@ -298,6 +299,28 @@ int intpll_configure(enum pll_clocks pll, ulong freq)
>   	return 0;
>   }
>   
> +#define VIDEO_PLL_RATE 594000000U
> +
> +static void enable_display_clk(void)
> +{
> +	if (IS_ENABLED(CONFIG_IMX8MP)) {
> +		clock_enable(CCGR_DISPMIX, false);
> +
> +		/* Set Video PLL to 594Mhz, p = 1, m = 99, k = 0, s = 2 */
> +		fracpll_configure(ANATOP_VIDEO_PLL, VIDEO_PLL_RATE);
> +
> +		/* 500Mhz */
> +		clock_set_target_val(MEDIA_AXI_CLK_ROOT, CLK_ROOT_ON | CLK_ROOT_SOURCE_SEL(1) | CLK_ROOT_PRE_DIV(CLK_ROOT_PRE_DIV2));
> +
> +		/* 200Mhz */
> +		clock_set_target_val(MEDIA_APB_CLK_ROOT, CLK_ROOT_ON | CLK_ROOT_SOURCE_SEL(2) | CLK_ROOT_PRE_DIV(CLK_ROOT_PRE_DIV4));
> +
> +		/* 27Mhz MIPI DPHY PLL ref from video PLL */
> +		clock_set_target_val(MEDIA_MIPI_PHY1_REF_CLK_ROOT, CLK_ROOT_ON | CLK_ROOT_SOURCE_SEL(7) | CLK_ROOT_POST_DIV(CLK_ROOT_POST_DIV22));
> +		clock_enable(CCGR_DISPMIX, true);
> +	}
> +}
> +

You might want to check Linux /sys/kernel/debug/clk/clk_summary 
before/after this change and see if there are any differences . If so, 
try using assigned-clock-rates in Linux DT and see if that can achieve 
the same "fix" effect.

Re: [RFC PATCH] imx8mp: fix boot hang when booting NXP kernel 5.15.32

[Rasmus Villemoes]

On 23/08/2022 16.04, Marek Vasut wrote:

> 
> You might want to check Linux /sys/kernel/debug/clk/clk_summary
> before/after this change and see if there are any differences . If so,
> try using assigned-clock-rates in Linux DT and see if that can achieve
> the same "fix" effect.

Thanks for the hint. Oddly enough, I do see some changes, but only in
the order of the lines - and in particular, even though some lines are
moved around, all clocks are still under the same parent clock as
before. Also all frequencies are the same before/after.

Specifically, I see media_mipi_phy1_ref, media_axi and media_apb (along
with their children) being moved up/down, but they are still children
of, respectively, osc_24m, sys_pll2_1000m and sys_pll1_800m.

So I don't see how that could help me figure out some
assigned-clock-rates or assigned-clock-parents settings that are missing
from DT.

Thanks,
Rasmus

Re: [RFC PATCH] imx8mp: fix boot hang when booting NXP kernel 5.15.32

[Peng Fan]

On 8/23/2022 9:36 PM, Rasmus Villemoes wrote:
> We have observed a somewhat weird bug: When booting the downstream NXP
> kernel lf-5.15.32-2.0.0 [fa6c3168595c], sometimes the board would hang
> during imx_lcdifv3_probe(). Adding some printk debugging revealed that
> the hang always happened at the
> 
>    writel(CTRL_SW_RESET, lcdifv3->base + LCDIFV3_CTRL_CLR);

Would you check the mediamix blk ctrl register before access this register?

Regards,
Peng.

> 
> However, only some of our imx8mp EVK boards and some of our custom
> imx8mp-based boards showed this; others never seemed to show it,
> making us initially suspect a hardware/board assembly error, though it
> would be weird for that to apply to both our design and the EVKs.
> 
> Moreover, for the boards that did have this behaviour, applying a
> generous amount of cooling spray to the SOC did make it boot, while
> conversely heating it up before booting was a sure way to make it
> hang. But even after that discovery, applying heat to the boards that
> seemed to be immune from this bug didn't make them hang either.
> 
> It is also worth mentioning that whenever the boards did boot,
> i.e. get past that critical line in probe(), whether those of the
> "immune" kind or those which we cooled sufficiently, graphics appeared
> to work just fine.
> 
> Eventually, we discovered that when using a downstream NXP U-Boot
> [lf_v2022.04, 1c881f4da8], this bug never happened. So I started
> bisecting between v2022.04 and lf_v2022.04, leading to
> 
>    commit 610e1b1246f7832bd96bfa9615e043565a19ac1b
>    Author: Ye Li <ye.li@nxp.com>
>    Date:   Mon Mar 30 01:56:03 2020 -0700
> 
>      MLK-23574-22 imx8m: clock: Sync clock settings with imx_v2020.04
> 
> Now that commit does a lot of things, but it wasn't hard to figure out
> that the part that was relevant to our case was the addition of the
> enable_display_clk() function.
> 
> Since I only have imx8mp boards (some EVKs and a few custom designs),
> this only adds the enable_display_clk() for that SOC. But this really
> seems like something that the kernel itself should (be able to) take
> care of, without relying on the bootloader having done such random
> magic.
> 
> Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
> ---
> 
> I don't know if upstream U-Boot cares about being able to boot a
> downstream NXP linux kernel. Or if this really should be fixed on the
> kernel side, making the lcdif driver properly configure the clock(s)
> before lifting the reset bit. But if somebody else runs into this
> issue, hopefully just this patch submission will at least save them
> some time.
> 
> Can someone from NXP explain what's going on? In particular, how come
> graphics works just fine even when, apparently, clocks have not been
> properly configured? And why does this only happen for some boards,
> but not others that should be physically identical? What's with the
> temperature dependency?
> 
>   arch/arm/mach-imx/imx8m/clock_imx8mm.c | 27 +++++++++++++++++++++++++-
>   1 file changed, 26 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mach-imx/imx8m/clock_imx8mm.c b/arch/arm/mach-imx/imx8m/clock_imx8mm.c
> index 4db55f8608..96a9eb4dd3 100644
> --- a/arch/arm/mach-imx/imx8m/clock_imx8mm.c
> +++ b/arch/arm/mach-imx/imx8m/clock_imx8mm.c
> @@ -45,7 +45,6 @@ int enable_i2c_clk(unsigned char enable, unsigned i2c_num)
>   	return 0;
>   }
>   
> -#ifdef CONFIG_SPL_BUILD
>   static struct imx_int_pll_rate_table imx8mm_fracpll_tbl[] = {
>   	PLL_1443X_RATE(1000000000U, 250, 3, 1, 0),
>   	PLL_1443X_RATE(933000000U, 311, 4, 1, 0),
> @@ -124,6 +123,8 @@ static int fracpll_configure(enum pll_clocks pll, u32 freq)
>   	return 0;
>   }
>   
> +#ifdef CONFIG_SPL_BUILD
> +
>   void dram_pll_init(ulong pll_val)
>   {
>   	fracpll_configure(ANATOP_DRAM_PLL, pll_val);
> @@ -298,6 +299,28 @@ int intpll_configure(enum pll_clocks pll, ulong freq)
>   	return 0;
>   }
>   
> +#define VIDEO_PLL_RATE 594000000U
> +
> +static void enable_display_clk(void)
> +{
> +	if (IS_ENABLED(CONFIG_IMX8MP)) {
> +		clock_enable(CCGR_DISPMIX, false);
> +
> +		/* Set Video PLL to 594Mhz, p = 1, m = 99, k = 0, s = 2 */
> +		fracpll_configure(ANATOP_VIDEO_PLL, VIDEO_PLL_RATE);
> +
> +		/* 500Mhz */
> +		clock_set_target_val(MEDIA_AXI_CLK_ROOT, CLK_ROOT_ON | CLK_ROOT_SOURCE_SEL(1) | CLK_ROOT_PRE_DIV(CLK_ROOT_PRE_DIV2));
> +
> +		/* 200Mhz */
> +		clock_set_target_val(MEDIA_APB_CLK_ROOT, CLK_ROOT_ON | CLK_ROOT_SOURCE_SEL(2) | CLK_ROOT_PRE_DIV(CLK_ROOT_PRE_DIV4));
> +
> +		/* 27Mhz MIPI DPHY PLL ref from video PLL */
> +		clock_set_target_val(MEDIA_MIPI_PHY1_REF_CLK_ROOT, CLK_ROOT_ON | CLK_ROOT_SOURCE_SEL(7) | CLK_ROOT_POST_DIV(CLK_ROOT_POST_DIV22));
> +		clock_enable(CCGR_DISPMIX, true);
> +	}
> +}
> +
>   void init_uart_clk(u32 index)
>   {
>   	/*
> @@ -485,6 +508,8 @@ int clock_init(void)
>   
>   	clock_enable(CCGR_SEC_DEBUG, 1);
>   
> +	enable_display_clk();
> +
>   	return 0;
>   };
>   

Re: [RFC PATCH] imx8mp: fix boot hang when booting NXP kernel 5.15.32

[Rasmus Villemoes]

On 24/08/2022 12.17, Peng Fan wrote:
> 
> 
> On 8/23/2022 9:36 PM, Rasmus Villemoes wrote:
>> We have observed a somewhat weird bug: When booting the downstream NXP
>> kernel lf-5.15.32-2.0.0 [fa6c3168595c], sometimes the board would hang
>> during imx_lcdifv3_probe(). Adding some printk debugging revealed that
>> the hang always happened at the
>>
>>    writel(CTRL_SW_RESET, lcdifv3->base + LCDIFV3_CTRL_CLR);
> 
> Would you check the mediamix blk ctrl register before access this register?

Sorry, I don't understand.

Do you want me to try to print the contents of that register before this
line? If so, can you tell me the exact (physical) address you're
thinking of? And what are you looking for, exactly?

Rasmus