drivers: entropy: Fix non-stop RNG ISR firing for STM32WB09 #98151

HoZHel · 2025-10-23T16:14:12Z

Fix the TRNG driver issue regarding non-stop ISR firing for STM32WB09.
Clear RNG_IRQ_SR_ERROR_IRQ and RNG_IRQ_SR_FF_FULL_IRQ flags when they are set.
Change the number of attempts to empty the RNG FIFO according to the FIFO size.
Fix the issue regarding passing the TRNG peripheral instance to the BLE driver.
Increase the SYSTEM_WORKQUEUE_STACK_SIZE when CONFIG_BT is set.

Here is the screenshot of beacon sample to show the reason for increasing SYSTEM_WORKQUEUE_STACK_SIZE. Previously, the default was 1024 bytes.

Fix the TRNG driver issue regarding non-stop ISR firing for STM32WB09. Clear RNG_IRQ_SR_ERROR_IRQ and RNG_IRQ_SR_FF_FULL_IRQ flags when they are set. Change the number of attempts to empty the RNG FIFO according to its size. Signed-off-by: Ali Hozhabri <[email protected]>

…ance Fix the issue regarding passing the TRNG peripheral instance to the driver. Increase the SYSTEM_WORKQUEUE_STACK_SIZE when CONFIG_BT is set. Signed-off-by: Ali Hozhabri <[email protected]>

sonarqubecloud · 2025-10-23T16:41:24Z

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

drivers/bluetooth/hci/hci_stm32wb0.c

etienne-lms

I don't understand how "drivers: entropy: Fix non-stop RNG ISR firing for STM32WB09" fixes your issue. I must miss something if it really changes the driver behavior.

For commit "drivers: bluetooth: hci: Fix the issue about the TRNG peripheral instance": could you describe a bit more why CONFIG_SYSTEM_WORKQUEUE_STACK_SIZE is increased: how does it help? I also this the commit message header line and body is not very explicit about which TRNG issue is being addressed.

etienne-lms · 2025-10-23T18:01:23Z

drivers/entropy/entropy_stm32.h

 	 */
-	WRITE_REG(RNG->IRQ_SR, RNG_IRQ_SR_FF_FULL_IRQ);
+	if (LL_RNG_GetFfFullIrq(RNGx)) {
+		SET_BIT(RNGx->IRQ_SR, RNG_IRQ_SR_FF_FULL_IRQ);


Using SET_BIT(), you may clear also a pending ERROR flag. I think WRITE_REG() was the way to go.

+1: WRITE_REG is the correct operation

What you're saying is the opposite. As the name suggests, SET_BIT() is for setting one bit of the register, while WRITE_REG() manipulates the whole register. Here you can find these two macros as follows:

#define SET_BIT(REG, BIT) ((REG) |= (BIT))
#define WRITE_REG(REG, VAL) ((REG) = (VAL))

I know what the macros do. SET_BIT() is not correct here because it does a read then write, and the flags are cleared by writing 1 to their bit:

If ERROR_IRQ was 1, SET_BIT(IRQ_SR, FF_FULL_IRQ) will read the register (= ERROR_IRQ), then |= FF_FULL_IRQ to the read value and write back the result (= ERROR_IRQ | FF_FULL_IRQ) which will clear both interrupt flags.

Using WRITE_REG (or really, just RNG->IRQ_SR = RNG_IRQ_SR_FF_FULL_IRQ) will write a value with only bit FF_FULL_IRQ set which ensures that only this specific interrupt flag is cleared.

Now I understand, thanks for the clarification.

etienne-lms · 2025-10-23T18:19:13Z

drivers/entropy/entropy_stm32.c


+#if defined(CONFIG_SOC_STM32WB09XX)
+	if (LL_RNG_GetErrorIrq(rng)) {
+		SET_BIT(rng->IRQ_SR, RNG_IRQ_SR_ERROR_IRQ);


The flag should have already been cleared by ll_rng_clear_sei() above.
Using SET_BIT() here may also clear the FF_FULL flag.

ll_rng_clear_seis() doesn't clear IRQ_SR_ERROR 🤔

@etienne-lms , ll_rng_clear_seis() clears RNG_CR_RST_HEALTH_FLAGS flag.
For your second sentence, please refer to the previous comment.

My mistake. Indeed it is an error, we should clear that bit.
Could you rather fix ll_rng_clear_sei() adding their clear of IRQ_SR_ERROR?

I think we need to revisit a bit WB09xx sequences in this driver.

etienne-lms · 2025-10-23T18:33:22Z

drivers/entropy/entropy_stm32.c

+#if IRQLESS_TRNG
+#define MAX_AVAIL_RND_NUM 1
+#else
+#define MAX_AVAIL_RND_NUM 4 /* Internal four-word FIFO */


When an error is triggered only 3 samples are to be removed from the 4-word FIFO.
If there is no FIFO, there is nothing to evacuate since upon error RNG produces no random sample.

That said, the change shows something that was not update by commit d65f8e3: indeed value 12 should have been updated to 3 at that time and no-FIFO instance addressed.

It doesn't really matter, reading empty FIFO will just return 0.

And FWIW other IPs do also have a FIFO but no corresponding "FIFO full" interrupt so I'm not sure that lowering this value is "correct"

I don't think lowering the value will fix anything. Just a matter of consistency.

mathieuchopstm · 2025-10-24T08:24:36Z

drivers/entropy/entropy_stm32.c

 	}

+#if defined(CONFIG_SOC_STM32WB09XX)
+	if (LL_RNG_GetErrorIrq(rng)) {


+1 on Etienne's comment: unless there's a reason for it, let's not bother checking for the flags.

Do you mean clearing a flag without checking if it is raised?

Yes. Per RefMan, this should have no effect

mathieuchopstm · 2025-10-24T08:25:19Z

drivers/entropy/entropy_stm32.h

 	 */
-	WRITE_REG(RNG->IRQ_SR, RNG_IRQ_SR_FF_FULL_IRQ);
+	if (LL_RNG_GetFfFullIrq(RNGx)) {
+		SET_BIT(RNGx->IRQ_SR, RNG_IRQ_SR_FF_FULL_IRQ);


+1: WRITE_REG is the correct operation

mathieuchopstm · 2025-10-24T08:26:04Z

drivers/entropy/entropy_stm32.c

 	ll_rng_clear_seis(rng);

-	for (int i = 0; i < 12; ++i) {
+	for (int i = 0; i < MAX_AVAIL_RND_NUM; ++i) {


Maybe this should be replaced with:

Suggested change

for (int i = 0; i < MAX_AVAIL_RND_NUM; ++i) {

while (ll_rng_is_active_drdy(rng)) {

to be more solid

drivers/bluetooth/hci/hci_stm32wb0.c

mathieuchopstm · 2025-10-24T11:17:06Z

soc/st/stm32/stm32wb0x/Kconfig.defconfig

+config SYSTEM_WORKQUEUE_STACK_SIZE
+	default 1152


Suggested change

config SYSTEM_WORKQUEUE_STACK_SIZE

default 1152

configdefault SYSTEM_WORKQUEUE_STACK_SIZE

default 1152

HoZHel · 2025-10-24T13:42:57Z

I don't understand how "drivers: entropy: Fix non-stop RNG ISR firing for STM32WB09" fixes your issue. I must miss something if it really changes the driver behavior.

Clearing the interrupt flags prevents continuous interrupt firing.

For commit "drivers: bluetooth: hci: Fix the issue about the TRNG peripheral instance": could you describe a bit more why CONFIG_SYSTEM_WORKQUEUE_STACK_SIZE is increased: how does it help? I also this the commit message header line and body is not very explicit about which TRNG issue is being addressed.

In this commit, the correct node identifier of RNG is passed to the variable. Previously, it was DT_DRV_INST(0) which referred to the bt_hci_wb0 node due to the presence of #define DT_DRV_COMPAT st_hci_stm32wb0.
For the increase of CONFIG_SYSTEM_WORKQUEUE_STACK_SIZE, I attached a screenshot which shows that 1024 is not enough anymore.

etienne-lms · 2025-10-24T14:19:15Z

I don't understand how "drivers: entropy: Fix non-stop RNG ISR firing for STM32WB09" fixes your issue. I must miss something if it really changes the driver behavior.

Clearing the interrupt flags prevents continuous interrupt firing.

Ok, I understand now that you pointed me ll_rng_clear_seis() didn't do the expected job.

In this commit, the correct node identifier of RNG is passed to the variable. Previously, it was DT_DRV_INST(0) which referred to the bt_hci_wb0 node due to the presence of #define DT_DRV_COMPAT st_hci_stm32wb0.

Ok. Right, you mention that in the commit message body.

For the increase of CONFIG_SYSTEM_WORKQUEUE_STACK_SIZE, I attached a screenshot which shows that 1024 is not enough anymore.

Could you mention that in the commit message? Like Experimentation show that 1048 bytes of stack where used when BT is enabled hence increase CONFIG_SYSTEM_WORKQUEUE_STACK_SIZE to a safer value..

erwango · 2025-10-25T07:40:19Z

drivers/entropy/entropy_stm32.c


+#if defined(CONFIG_SOC_STM32WB09XX)
+	if (LL_RNG_GetErrorIrq(rng)) {
+		SET_BIT(rng->IRQ_SR, RNG_IRQ_SR_ERROR_IRQ);


Please use new bits handling functions: #97329

HoZHel added 2 commits October 23, 2025 17:22

drivers: bluetooth: hci: Fix the issue about the TRNG peripheral inst…

cd96302

…ance Fix the issue regarding passing the TRNG peripheral instance to the driver. Increase the SYSTEM_WORKQUEUE_STACK_SIZE when CONFIG_BT is set. Signed-off-by: Ali Hozhabri <[email protected]>

HoZHel requested review from erwango and mathieuchopstm October 23, 2025 16:15

HoZHel marked this pull request as ready for review October 23, 2025 17:34

zephyrbot added area: Boards/SoCs area: Bluetooth HCI Bluetooth HCI Driver area: Crypto / RNG area: Bluetooth platform: STM32 ST Micro STM32 labels Oct 23, 2025

zephyrbot requested review from FRASTM, GeorgeCGV, Thalley, alwa-nordic, asm5878, cvinayak, djiatsaf-st, etienne-lms, gautierg-st, hermabe, jhedberg, marwaiehm-st and sjanc October 23, 2025 17:36

zephyrbot assigned erwango Oct 23, 2025

JarmouniA reviewed Oct 23, 2025

View reviewed changes

drivers/bluetooth/hci/hci_stm32wb0.c Show resolved Hide resolved

etienne-lms reviewed Oct 24, 2025

View reviewed changes

mathieuchopstm reviewed Oct 24, 2025

View reviewed changes

erwango reviewed Oct 25, 2025

View reviewed changes

	for (int i = 0; i < MAX_AVAIL_RND_NUM; ++i) {
	while (ll_rng_is_active_drdy(rng)) {

drivers: entropy: Fix non-stop RNG ISR firing for STM32WB09 #98151

Are you sure you want to change the base?

drivers: entropy: Fix non-stop RNG ISR firing for STM32WB09 #98151

Conversation

HoZHel commented Oct 23, 2025

Uh oh!

sonarqubecloud bot commented Oct 23, 2025

Quality Gate passed

Uh oh!

Uh oh!

etienne-lms left a comment

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

HoZHel Oct 24, 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!

mathieuchopstm Oct 24, 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!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

HoZHel commented Oct 24, 2025

Uh oh!

etienne-lms commented Oct 24, 2025

Uh oh!

erwango Oct 25, 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

HoZHel Oct 24, 2025 •

edited

Loading

mathieuchopstm Oct 24, 2025 •

edited

Loading

erwango Oct 25, 2025 •

edited

Loading