libutils: Add support for stm32mp2 SoC family#205
Conversation
Indanz
left a comment
Indanz
left a comment
There was a problem hiding this comment.
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.)
bruelc
force-pushed
the
master-stm32mp2
branch
from
123567a to
9236882
Compare
|
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. |
bruelc
force-pushed
the
master-stm32mp2
branch
3 times, most recently
from
1bcc001 to
998a394
Compare
Indanz
left a comment
Indanz
left a comment
There was a problem hiding this comment.
Probably need to fix the style a bit, but otherwise looks good.
bruelc
force-pushed
the
master-stm32mp2
branch
4 times, most recently
from
ef73cd5 to
c8eacbe
Compare
Indanz
left a comment
Indanz
left a comment
There was a problem hiding this comment.
Apparently I had a pending review here which I forgot to submit according to Github?
Most of the comments are handled in lates my push, let me check |
bruelc
force-pushed
the
master-stm32mp2
branch
3 times, most recently
from
d22310e to
1833ea8
Compare
Define default serial with USART2 TIM2 32bit general timer configured with 5us tick. using arr overflow interrupt to support for 64bit timestamp and the channel1 ccr1 compare register for 32bit timeout. Signed-off-by: Christian Bruel <christian.bruel@foss.st.com>
|
Hum maybe a single read back and a memory barrier would be equivalent . I
used the loop for a different reason, because I de-assert and enable the
clock in the same write to cover this additional dependency. The loop
should also cover the order problem you mention. Thank you for the
explanation
Le lun. 9 mars 2026 à 12:48, Indan Zupancic ***@***.***> a
écrit :
… ***@***.**** commented on this pull request.
------------------------------
In libplatsupport/src/plat/stm32mp2/timer.c
<#205 (comment)>:
> +
+ return (cnt2 * TICK_NS);
+}
+
+int stm32mp2_timer_init(stm32_t *stm32, ps_io_ops_t ops)
+{
+ void *rcc = NULL;
+ ps_io_ops_t *io_ops = &ops;
+
+ /* TIM2 RCC */
+ MAP_IF_NULL(io_ops, RCC_TIM2, rcc);
+ if (rcc == NULL) {
+ return EINVAL;
+ }
+ RCC_ON(rcc);
+ asm volatile("isb" ::: "memory");
Yes, but if you do a single read, the CPU is allowed to do the later TIM
writes while it is waiting for the RCC read to finish. Which would have the
same effect as having no read at all. Timing wise a single read would be
enough, yes, but enforcing such timing is tricky on out-of-order CPUs.
—
Reply to this email directly, view it on GitHub
<#205 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AGWVGD43WT3UPXIQYNMOG2D4P2VRFAVCNFSM6AAAAACU345GPSVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTSMJUHA3DCMZZGE>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
In the ref man 21.4.11 , the recommended sequence is to use a single dummy read after enabling the clock |
2 participants
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