bflb: attempt to address e24 cores boot failures #95189

VynDragon · 2025-08-30T02:18:12Z

Clean up code and add attempts to prevent bl60x and bl70x from failing to boot

soc/bflb/bl60x/Kconfig.defconfig

josuah

At bootup, plenty of RAM types are containing random data.
Maybe this is the same for the caches, and they need to be flushed at startup?

This might be already done as part of chip init, except this could be taking time and starting the hardware right away could end-up executing random bytes?

If so, an interrupt a bit earlier (like when systick is faster than 1000/sec) could end-up executing some region of flash that has cache not flushed yet...

Not sure what that could be really!

soc/bflb/common/e24/cache.c

soc/bflb/common/e24/intc_clic.S

soc/bflb/common/e24/cache.c

soc/bflb/bl70x/Kconfig.defconfig

soc/bflb/bl60x/Kconfig.defconfig

soc/bflb/common/e24/cache.c

josuah · 2025-09-01T23:00:35Z

Waiting author's confirmation that all is as intended before giving the 2nd +1, to avoid an accidental merge by the release team...

josuah

Thank you for the explanations.

soc/bflb/bl60x/Kconfig.defconfig

soc/bflb/bl60x/Kconfig

nandojve · 2025-09-02T10:50:23Z

soc/bflb/bl60x/soc.c

@@ -21,6 +21,8 @@
 #include <hbn_reg.h>
 #include <pds_reg.h>

+extern void soc_bflb_cache_invalidate(void);


Cache should be in another commit with explanation about why it is necessary ? Why RISC-V arch infrastructure do not handle this ?

That is the thing the commit description is about... Because there is as many riscv cache schemes that there are riscv SoCs.

okay no seems it need to go in drivers/cache and enable CONFIG_CACHE_MANAGEMENT...

Can we have this for now and leave making a driver for all the caches for later?
We will need it anyway for flash access now i know that's a driver type.

Or maybe i can introduce it with the DMA.

nandojve · 2025-09-02T10:50:49Z

soc/bflb/bl70x/Kconfig.defconfig

@@ -5,9 +5,9 @@
 if SOC_SERIES_BL70X

 # On BL702, BL704 and BL706, not all values work, here is a list of some that work:
-# 2000, 1000, 100, 10, 1
+# 1000, 100, 10, 1


Why are we changing this ? Another commit with clear explanation.

Also what the commit and PR is about.

yes but you are cleaning up and add cache statements in the same commit, makes no sense to me.

Is the tick that makes the SoC not boot, the cache or both ?
Why this combination was important only now and how it worked before ?
Is there a boot concurrence ?

The above questions can not be answered when I look the commit message. What I'm try to tell, as an example, is that it seems that you fix the problem but it was a guess because the technical details are not present. This makes people questions your changes and put in doubt the solution. So, is this really a fix ?

BTW, even in the title it seems that you not 100% sure that this fix the problem.

Then, why should maintainer accept the fix ?

it seems that you fix the problem but it was a guess because the technical details are not present

I thought I made it obvious enough this was the case here, especially after talking about it in discord. It fixes the issue, which is critical and so reason enough, and I dont know why.

BTW, even in the title it seems that you not 100% sure that this fix the problem.

I encourage you to thoroughly test the PR... I've done my testing and as best I can tell it fixed it, except i've thought that many times so I am staying on the safe side because it randomly reappeared from seemingly unrelated things everytime.

Some commit jargon for "maybe" could be "improve stability"?

@nandojve Is something like this enough? Could you give an example of commit message to help us grasp what's missing?

Improve the e24 boot stability by ensuring interrupts are disabled until the SoC is ready for it.

nandojve · 2025-09-02T10:53:55Z

soc/bflb/common/e24/intc_clic.S

@@ -9,19 +9,6 @@
 */

 #include <zephyr/arch/cpu.h>
-#include "clic.h"


I'm wondering why we still need this file after all these removals.

because it defines __soc_handle_irq.

Explain in the commit message why we can not use this, which is a week implementation.

zephyr/soc/common/riscv-privileged/soc_irq.S

Lines 21 to 34 in 36f78b5

WTEXT(__soc_handle_irq)

/*

* SOC-specific function to handle pending IRQ number generating the interrupt.

* Exception number is given as parameter via register a0.

*/

SECTION_FUNC(exception.other, __soc_handle_irq)

/* Clear exception number from CSR mip register */

li t1, 1

sll t0, t1, a0

csrrc t1, mip, t0

/* Return */

ret

I see this pattern in many SoC and then it will better have an KCONFIG to handle all those SoC exceptions in the privileged mode.

You are right, it's not needed. But it triggers the bug this PR is trying to fix when using the riscv-privileged .S... Which I didnt catch at first and thought it was because it needed it defined.

nandojve · 2025-09-03T05:54:02Z

soc/bflb/common/e24/intc_clic.S

@@ -9,19 +9,6 @@
 */

 #include <zephyr/arch/cpu.h>
-#include "clic.h"


Explain in the commit message why we can not use this, which is a week implementation.

zephyr/soc/common/riscv-privileged/soc_irq.S

Lines 21 to 34 in 36f78b5

WTEXT(__soc_handle_irq)

/*

* SOC-specific function to handle pending IRQ number generating the interrupt.

* Exception number is given as parameter via register a0.

*/

SECTION_FUNC(exception.other, __soc_handle_irq)

/* Clear exception number from CSR mip register */

li t1, 1

sll t0, t1, a0

csrrc t1, mip, t0

/* Return */

ret

I see this pattern in many SoC and then it will better have an KCONFIG to handle all those SoC exceptions in the privileged mode.

nandojve · 2025-09-03T05:57:57Z

soc/bflb/bl70x/Kconfig.defconfig

@@ -5,9 +5,9 @@
 if SOC_SERIES_BL70X

 # On BL702, BL704 and BL706, not all values work, here is a list of some that work:
-# 2000, 1000, 100, 10, 1
+# 1000, 100, 10, 1


yes but you are cleaning up and add cache statements in the same commit, makes no sense to me.

Is the tick that makes the SoC not boot, the cache or both ?
Why this combination was important only now and how it worked before ?
Is there a boot concurrence ?

The above questions can not be answered when I look the commit message. What I'm try to tell, as an example, is that it seems that you fix the problem but it was a guess because the technical details are not present. This makes people questions your changes and put in doubt the solution. So, is this really a fix ?

BTW, even in the title it seems that you not 100% sure that this fix the problem.

Then, why should maintainer accept the fix ?

Remove unecessary unused code and Kconfig Signed-off-by: Camille BAUD <mail@massdriver.space>

VynDragon · 2025-09-03T10:19:25Z

I tried another approach based on this idea #95189 (comment) and similar previous theories, and this looks like it fixed it, this however might still be a fluke.

This should fix e24 failing to boot by ensuring interrupts are disabled at startup Signed-off-by: Camille BAUD <mail@massdriver.space>

sonarqubecloud · 2025-09-03T11:00:12Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

josuah

Waiting a bit before enabling IRQs seems present on other RISC-V:

zephyr/drivers/interrupt_controller/intc_vexriscv_litex.c

Lines 97 to 98 in 209bfbd

    
           DEVICE_DT_INST_DEFINE(0, vexriscv_litex_irq_init, NULL, NULL, NULL, 
        
           		      PRE_KERNEL_1, CONFIG_INTC_INIT_PRIORITY, NULL);

This is what the SDK seems to be doing too, in SystemInit(), disabling IRQs first:

    /* global IRQ disable */
    __disable_irq();

IIRC there is a ROM bootloader starting first? Maybe this is what enables the IRQs to begin with.

VynDragon · 2025-09-03T13:42:46Z

If it is the actual issue, that is likely to be the reason indeed.

VynDragon force-pushed the e24_fix_attempt branch from fbb417c to 297ab07 Compare August 30, 2025 02:22

VynDragon marked this pull request as ready for review August 30, 2025 03:35

zephyrbot added the platform: Bouffalo Lab label Aug 30, 2025

zephyrbot requested a review from nandojve August 30, 2025 03:36

zephyrbot assigned nandojve Aug 30, 2025

nandojve previously approved these changes Aug 30, 2025

View reviewed changes

soc/bflb/bl60x/Kconfig.defconfig Outdated Show resolved Hide resolved

josuah reviewed Sep 1, 2025

View reviewed changes

VynDragon dismissed nandojve’s stale review via 9da6d78 September 2, 2025 01:15

VynDragon force-pushed the e24_fix_attempt branch from 297ab07 to 9da6d78 Compare September 2, 2025 01:15

VynDragon requested review from nandojve and josuah September 2, 2025 01:15

VynDragon force-pushed the e24_fix_attempt branch from 9da6d78 to ff9443a Compare September 2, 2025 01:20

josuah previously approved these changes Sep 2, 2025

View reviewed changes

nandojve requested changes Sep 2, 2025

View reviewed changes

VynDragon force-pushed the e24_fix_attempt branch 2 times, most recently from 5f84dc9 to d9890b2 Compare September 2, 2025 16:51

VynDragon requested a review from nandojve September 2, 2025 16:51

nandojve requested changes Sep 3, 2025

View reviewed changes

soc: bflb: Remove Extraneous code

8ec0ca1

Remove unecessary unused code and Kconfig Signed-off-by: Camille BAUD <mail@massdriver.space>

VynDragon dismissed josuah’s stale review via 871beea September 3, 2025 10:18

VynDragon force-pushed the e24_fix_attempt branch from d9890b2 to 871beea Compare September 3, 2025 10:18

soc: bflb: address e24 boot failures

11f2d0f

This should fix e24 failing to boot by ensuring interrupts are disabled at startup Signed-off-by: Camille BAUD <mail@massdriver.space>

VynDragon force-pushed the e24_fix_attempt branch from 871beea to 11f2d0f Compare September 3, 2025 10:29

VynDragon requested review from nandojve and josuah September 3, 2025 12:00

josuah approved these changes Sep 3, 2025

View reviewed changes

nandojve approved these changes Sep 3, 2025

View reviewed changes

nandojve added this to the v4.3.0 milestone Sep 3, 2025

fabiobaltieri merged commit b13ec9a into zephyrproject-rtos:main Sep 3, 2025
26 checks passed

nandojve deleted the e24_fix_attempt branch September 3, 2025 14:51

	WTEXT(__soc_handle_irq)

	/*
	* SOC-specific function to handle pending IRQ number generating the interrupt.
	* Exception number is given as parameter via register a0.
	*/
	SECTION_FUNC(exception.other, __soc_handle_irq)
	/* Clear exception number from CSR mip register */
	li t1, 1
	sll t0, t1, a0
	csrrc t1, mip, t0

	/* Return */
	ret

	DEVICE_DT_INST_DEFINE(0, vexriscv_litex_irq_init, NULL, NULL, NULL,
	PRE_KERNEL_1, CONFIG_INTC_INIT_PRIORITY, NULL);

bflb: attempt to address e24 cores boot failures #95189

bflb: attempt to address e24 cores boot failures #95189

Uh oh!

Conversation

VynDragon commented Aug 30, 2025

Uh oh!

Uh oh!

josuah left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

josuah commented Sep 1, 2025

Uh oh!

josuah left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

VynDragon Sep 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nandojve Sep 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

VynDragon Sep 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

josuah Sep 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nandojve Sep 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

VynDragon commented Sep 3, 2025

Uh oh!

sonarqubecloud bot commented Sep 3, 2025

Quality Gate passed

Uh oh!

josuah left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

VynDragon commented Sep 3, 2025

Uh oh!

VynDragon Sep 2, 2025 •

edited

Loading

nandojve Sep 3, 2025 •

edited

Loading

VynDragon Sep 3, 2025 •

edited

Loading

josuah Sep 3, 2025 •

edited

Loading

nandojve Sep 3, 2025 •

edited

Loading

josuah left a comment •

edited

Loading