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
2 times, most recently
from
5e32d04 to
d22310e
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>
| return EINVAL; | ||
| } | ||
| RCC_ON(rcc); | ||
| asm volatile("isb" ::: "memory"); |
There was a problem hiding this comment.
I would add a comment why an isb is needed. Considering RCC_ON does a memory write, I would expect a dsb. An isb doesn't guarantee that the write finishes, only that the instruction finished. Things are a bit muddy for uncached memory writes, so in practice probably either works, but dsb seems more correct.
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