-
Notifications
You must be signed in to change notification settings - Fork 8.3k
NXP support lptmr tickless function #95143
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
NXP support lptmr tickless function #95143
Conversation
2cf7357 to
0b787a8
Compare
|
Updated to fixed dts conflict with newly commit modification. Hi, this PR is aim to support nxp lptmr tickless function, and supported it in imx95 m7, can you help review it when you time free? Thanks |
a4522f5 to
5b1791a
Compare
5b1791a to
099ead3
Compare
|
@mmahadevan108 please help review this |
|
ping @henrikbrixandersen @DerekSnell |
Yeah, this is the reason I did not implement tickless mode in this driver when I originally submitted it (see 21806b5). |
099ead3 to
6719da2
Compare
|
Hi @mmahadevan108 , I have added disable/start code logic in LPTMR_SetTimerPeriod to avoid the hardware limitations, tested passed on imx95 m7 power_mgmt_soc test case, the time sleep and tick record is right in my side, please help reivew it when you time free, thanks |
henrikbrixandersen
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stopping/starting the timer when setting a new timeout will affect the ticks (as the time all the instructions between stopping and starting will not be counted). I don't think this is an acceptable solution.
2446f3e to
ac6e777
Compare
|
Hi @henrikbrixandersen I fully understand your concerns. After reconfirming with the design team, we need to ensure that there is sufficient time when updating the CMP. We can choose to reset(but this will cause the time to go back instead of monotonically increasing. Even through software, it is difficult to guarantee that there are no corner cases). In the latest commit, I added a protection mechanism. When the distance is very close, It will postpone a windo to prevent it from being triggered. Of course, this should depend on different SoCs, please help review it again when you time free, thanks a lot |
ac6e777 to
43ce283
Compare
|
I'd like to hear @andyross and @teburd (System timer maintainer/collaborator) view on this. I am unsure if this is a good solution. |
|
Hi @henrikbrixandersen, Has there been any progress on this issue? thanks |
|
|
The design of kernel timers is time never stops (free running counter) that you may set a compare to catch sometime in the future. Stopping/Starting the underlying timer will surely introduce timing drift and jitter, something that tends to surprise and upset people when they see it. My opinion is that if your hardware needs to stop and start to set the compare, this hardware isn't a good source for kernel timers without at least some build warnings informing people building with it that yes, this is what they really want. I'm not going to stop a vendor from supplying a driver that introduces drift/jitter. But I do feel like I can say we should warn about it. Unless you can perhaps show in the PR the external tool test from https://github.com/zephyrproject-rtos/zephyr/tree/main/tests/kernel/timer/timer_behavior with correct timer behavior with this driver with the start/stop behavior? Maybe its ok, I don't know! |
| default y | ||
|
|
||
| config SYS_CLOCK_TICKS_PER_SEC | ||
| default 1000 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider to set default to 1024.
For default 1000, since SYS_CLOCK_HW_CYCLES_PER_SEC = 32768, then CYCLES_PER_TICK = 32768 / 1000 = 32 (fractional part is dropped), so one OS “tick” is 32 hardware cycles, than tick_rate = tick_rate = 32768 / 32 = 1024 ticks/s, but the configuration says "1000 ticks/s", there is 2.4% drift.
For a 32768 Hz clock, Zephyr typically uses tick rates that divide 32768 exactly (1024, 512, 256, 128, …), in this way, there is no rafe shift.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure, update into 1024 now, thanks
43ce283 to
2f0b7c7
Compare
add lptmr1 node into imx95 m7 dts which locate on Always On domain and can continue work when whole soc enter system sleep mode Signed-off-by: Yongxu Wang <yongxu.wang@nxp.com>
remove lptmr2 and enable lptmr1 interface as default Signed-off-by: Yongxu Wang <yongxu.wang@nxp.com>
Enable lptmr interface to support counter_basic_api test case Signed-off-by: Yongxu Wang <yongxu.wang@nxp.com>
Use lptmr interface as system clock if enabled PM as system tick can't continue work in system suspend mode Signed-off-by: Yongxu Wang <yongxu.wang@nxp.com>
When tickless is enabled, it is necessary to ensure that lptmr can be set for a period of time instead of generating an interrupt at one tick Set lptmr to the free running mode to ensure that the current cycles value of lptmr can be directly obtained in ISR Signed-off-by: Yongxu Wang <yongxu.wang@nxp.com>
Add safety window mechanism to prevent race conditions when updating LPTMR compare register (CMR). Problem: - Setting CMR too close to current hardware counter can cause missed interrupts and system hangs Solution: - Implement lptmr_set_safe_immediate() with configurable safety window Signed-off-by: Yongxu Wang <yongxu.wang@nxp.com>
Tested imx95 m7 lptmr as system tick in timer_behavior test case Signed-off-by: Yongxu Wang <yongxu.wang@nxp.com>
2f0b7c7 to
12f0265
Compare
thanks your comments, have droped stop/start flow now, just added one customer safe window now, its 10 as default, and it can be set into different value to adapter socs. And added LOG_WRN in driver init, if you want to enlarge the safe window value, it will may introduce some timing drift and jitter. SUITE PASS - 100.00% [timer_jitter_drift]: pass = 2, fail = 0, skip = 0, total = 2 duration = 39.339 seconds
SUITE PASS - 100.00% [timer_tick_train]: pass = 1, fail = 0, skip = 0, total = 1 duration = 10.362 seconds
------ TESTSUITE SUMMARY END ------ =================================================================== thanks a lot for your time |
|
| If enabled, the driver will use SoC-specific default values. | ||
|
|
||
| config MCUX_LPTMR_TIMER_SAFETY_WINDOW_CYCLES | ||
| int "MCUX LPTMR timer safety window cycles" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How can user choose a proper value, is there a guidance?
|
|
||
| #define MAX_TICKS ((COUNTER_MAX / CYCLES_PER_TICK) - 1) | ||
| #define MAX_CYCLES (MAX_TICKS * CYCLES_PER_TICK) | ||
| #define MIN_DELAY 1000 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how is this value decided?
| #define CYCLES_PER_TICK ((uint32_t)((uint64_t)sys_clock_hw_cycles_per_sec() \ | ||
| / (uint64_t)CONFIG_SYS_CLOCK_TICKS_PER_SEC)) | ||
|
|
||
| #define COUNTER_MAX 0xffffffff |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there is LPTMR which uses 16-bit counter. such as kw45.
| next += CYCLES_PER_TICK; | ||
| } | ||
|
|
||
| next += announced_cycles; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could this next overflow? which means in function lptmr_set_safe_immediate, target_cycles < hw_counter
| default y | ||
|
|
||
| config SYS_CLOCK_TICKS_PER_SEC | ||
| default 1024 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can be
default 1024 if MCUX_LPTMR_TIMER
default 1000 if CORTEX_M_SYSTICK
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also, I guess this should be outside of if PM ... endif
| @@ -0,0 +1,3 @@ | |||
| CONFIG_MCUX_LPTMR_TIMER=y | |||
| CONFIG_TICKLESS_KERNEL=y | |||
| CONFIG_SYS_CLOCK_TICKS_PER_SEC=1024 | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this can be removed if done in soc/nxp/imx/imx9/imx95/Kconfig.defconfig.mimx95.m7



Uh oh!
There was an error while loading. Please reload this page.