libutils: Add support for stm32mp2 SoC family by bruelc · Pull Request #205 · seL4/util_libs

bruelc · 2026-02-12T10:06:26Z

Description

Add support for uart and timers for stm32mp2 soc

Link to kernel PR seL4

Testing

Testing was part of the seL4-test and seL4-bench test suites

Indanz

You don't handle 32-bit timer overflows, you probably want to register an IRQ handler for that. Alternatively, you can probably link two timers into one 64-bit timer and ignore overflows.

You also use two hard-coded timers, instead of one. Each timer has four channels which can be used for compare independently. I'd use one for the timeout function and another for overflow detection, if there is no other way. Except if it's one of those stupid timers which can't be reprogrammed on-the-fly without stopping it, in which case you do need to use two timers and document this somewhere. But the datasheet says "The TIMx_CCRx register can be updated at any time by software to control the output waveform, provided that the preload register is not enable", so it seems okay.

But if hard-coding timers like this, it doesn't make much sense to use the DTS walker either, as you know everything already. (Keep it if the code is simpler though.)

Indanz · 2026-02-17T14:16:56Z

libplatsupport/plat_include/stm32mp2/platsupport/plat/serial.h

+/* official device names */
+enum chardev_id {
+    UART0,
+    UART1,


Is UART1 actually supported by this driver? If not, leave it out.

The SoC has about 10 UARTs, probably a good idea to follow the datasheet UART names in this enum and elsewhere.

Correct. In the boot stdout chosen dts path, we have serial0 aliased to USART2, which confused my naming. I will rename it.

Indanz · 2026-02-17T14:23:26Z

libplatsupport/plat_include/stm32mp2/platsupport/plat/serial.h

+
+#pragma once
+
+#define UART0_PADDR  0x400E0000


This is actually USART2.

Indanz · 2026-02-17T14:23:57Z

libplatsupport/plat_include/stm32mp2/platsupport/plat/serial.h

+#pragma once
+
+#define UART0_PADDR  0x400E0000
+#define UART1_PADDR  0x400F0000


And this is USART3.

Indanz · 2026-02-17T14:29:01Z

libplatsupport/plat_include/stm32mp2/platsupport/plat/timer.h

+ * Copyright 2026, STMicroelectronics
+ *
+ * SPDX-License-Identifier: BSD-2-Clause
+ */


It's good to give an overview here of which timers you're using and how and why (if it's more than one).

Indanz · 2026-02-17T14:36:35Z

libplatsupport/plat_include/stm32mp2/platsupport/plat/timer.h

+#define RCC_APB1DIVR_OFF  0x4B4
+#define RCC_APB1DIVR_PADDR      (RCC_PADDR + RCC_APB1DIVR_OFF)
+#define RCC_APB1DIVR_SIZE       4


Weird indentation. And also unused?

Indanz · 2026-02-17T14:40:06Z

libplatsupport/src/plat/stm32mp2/chardev.c

+#include "../../common.h"
+#include <utils/util.h>
+
+#include "../../chardev.h"


Duplicate, and see Julia's comment elsewhere.

Indanz · 2026-02-17T14:43:38Z

libplatsupport/plat_include/stm32mp2/platsupport/plat/timer.h

+    uint32_t tisel;
+    uint32_t pad[0xEA];
+    uint32_t ipidr;
+} stm32_regs_t;


This structure you can move to timer.c instead, leaving a forward declaration behind. Same for most of the defines. But whatever you prefer.

Indanz · 2026-02-17T14:45:35Z

libplatsupport/src/plat/stm32mp2/chardev.c

+static const struct dev_defn dev_defn[] = {
+    UART_DEFN(0),
+    UART_DEFN(1),
+};


If you use this construction instead of hard-coding a specific UART, any reason not to add all compatible ones?

Indanz · 2026-02-17T15:00:00Z

libplatsupport/src/plat/stm32mp2/ltimer.c

+#include "../../ltimer.h"
+
+/*
+ * We use two dm timers: one to keep track of an absolute time, the other for timeouts.


You're using the TIM2 and TIM3 general purpose timers. What do you mean with "dm"?

Also, each timer seems to have four channels, so why do you need to use two timers?

No specific reason except that it was easier to debug and trace. But OK, now I should rework this part ; there is no need to start more than necessary.

bruelc · 2026-02-26T12:18:36Z

Indanz, your suggestion to use a channel instead of a second timer was very good. I force-pushed a new version with your latest reviews and refactoring. Thank you.

Indanz · 2026-02-26T12:51:27Z

libplatsupport/src/plat/stm32mp2/chardev.c

+}
+
+static const struct dev_defn dev_defn[] = {
+    UART_DEFN(1),


I'd like to see the other compatible UARTS in this list too, if possible. And if the address is calculable, do that instead of macro string concatenation.

If the only difference is the memory address then there is no reason not to add them. No need to test the others in that case either. Even if it doesn't work because of e.g. missing clock or power domain enable, it's still better than nothing.

Indanz · 2026-02-26T12:55:34Z

libplatsupport/src/plat/stm32mp2/ltimer.c

+	    if (! stm32->periodic) {
+		    stm32_regs->dier &= ~STM32_TIM_DIER_CC1IE;
+	    }
+            else
+              assert(0);
+
+            if (stm32_ltimer->user_cb_fn) {
+                stm32_ltimer->user_cb_fn(stm32_ltimer->user_cb_token, LTIMER_TIMEOUT_EVENT);
+            }
+
+    }


Please cleanup this bit, looks like left-over debug stuff.

Please cleanup this bit, looks like left-over debug stuff.

Gloops wrong version pushed. Please forget this commit

repushed (with pediodic support). now I'll look at your other comments :)

Indanz · 2026-02-26T12:55:53Z

libplatsupport/src/plat/stm32mp2/ltimer.c

+
+            if (stm32_ltimer->user_cb_fn) {
+                stm32_ltimer->user_cb_fn(stm32_ltimer->user_cb_token, LTIMER_OVERFLOW_EVENT);
+            }


Indent off.

Indanz · 2026-02-26T12:57:12Z

libplatsupport/src/plat/stm32mp2/ltimer.c

+    if (ltimer == NULL) {
+        ZF_LOGE("ltimer cannot be NULL");
+        return EINVAL;
+    }


Is this check really necessary? Seems a bit pointless to me, something is going to go horribly wrong sooner or later if it is NULL.

Indanz · 2026-02-26T13:00:25Z

libplatsupport/src/plat/stm32mp2/timer.c

+    assert(stm32 != NULL);
+    assert(stm32->hw  != NULL);


All these asserts can go away once your driver is working, it's only useful to catch initial problems.

Indanz · 2026-02-26T13:04:03Z

libplatsupport/src/plat/stm32mp2/timer.c

+    /* TIM2 RCC */
+    RCC_ON(rcc);
+
+    stm32->hw->cr1 &= ~STM32_TIM_CR1_CEN;


I think you should do blind writes to cr1 to configure it how you want in one go. Currently you never set the other bits, so you're counting on either reset behaviour or something.

Reading registers is very slow, so if you don't have to, don't (this applies more to the other places than here). Some devices don't want other bits to be changed together with the enable bit, but a lot of them don't mind.

Indanz · 2026-02-26T13:06:56Z

libplatsupport/src/plat/stm32mp2/timer.c

+    stm32->hw->dier |= STM32_TIM_DIER_UIE; /* interrupt for overflow */
+    stm32->hw->ccmr1 = 0;
+    stm32->hw->ccer |= STM32_TIM_CCER_CC1E;
+    stm32->hw->dier &= ~STM32_TIM_DIER_CC1IE;


Same for the ccer and dier registers, I think you just want to set it to STM32_TIM_CCER_CC1E and STM32_TIM_DIER_UIE. Or are there reserved bits in there?

Define default serial with USART1 32bit TIM2 timer configured with 5us tick, arr autoreload overflows timestamp and Channel1 ccr1 compare register for timeout.

bruelc marked this pull request as draft February 12, 2026 10:06

Indanz marked this pull request as ready for review February 17, 2026 14:15

Indanz marked this pull request as draft February 17, 2026 14:15

Indanz reviewed Feb 17, 2026

View reviewed changes

bruelc force-pushed the master-stm32mp2 branch from 123567a to 9236882 Compare February 26, 2026 12:12

Indanz reviewed Feb 26, 2026

View reviewed changes

bruelc force-pushed the master-stm32mp2 branch from 9236882 to 578ef45 Compare February 26, 2026 13:50

Add support for stm32mp2 SoC family

1bcc001

Define default serial with USART1 32bit TIM2 timer configured with 5us tick, arr autoreload overflows timestamp and Channel1 ccr1 compare register for timeout.

bruelc force-pushed the master-stm32mp2 branch from 578ef45 to 1bcc001 Compare February 26, 2026 14:35

Conversation

bruelc commented Feb 12, 2026

Description

Testing

Uh oh!

Indanz 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!

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!

bruelc commented Feb 26, 2026

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants