Skip to content
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

tests: kernel: timer_api fails with CONFIG_STM32_LPTIM_TIMER #20992

Merged
merged 11 commits into from
May 29, 2020

Conversation

FRASTM
Copy link
Collaborator

@FRASTM FRASTM commented Nov 26, 2019

Testing kernel/timer/timer_API with LPTIM on stm32L4R5
the test_timer_duration_period fails w"
test_timer_duration_period: tdata.expire_cnt == 4 is false"

  • changed z_timer_cycle_get_32() function to have better value returned in nb of HW cycles
  • changed the calculation for next timeout in case of short delay
  • source LPTIM with LSE or LSI clock for better accuracy
  • reduce tick freq per sec and adjust for a better divided value of
    LPTIM_CLOCK % CONFIG_SYS_CLOCK_TICKS_PER_SEC

Activation of the LPTIM remains an experimental feature to generate kernel timings.

Fixes #20991

@FRASTM FRASTM requested a review from erwango as a code owner November 26, 2019 08:36
@zephyrbot zephyrbot added the area: Timer Timer label Nov 26, 2019
@zephyrbot
Copy link
Collaborator

zephyrbot commented Nov 26, 2019

All checks are passing now.

Tip: The bot edits this comment instead of posting a new one, so you can check the comment's history to see earlier messages.

@erwango erwango added the DNM This PR should not be merged (Do Not Merge) label Nov 26, 2019
@FRASTM FRASTM changed the title tests: kernel: timer_api fails with CONFIG_STM32_LPTIM_TIMER WIP: tests: kernel: timer_api fails with CONFIG_STM32_LPTIM_TIMER Nov 26, 2019
@erwango erwango changed the title WIP: tests: kernel: timer_api fails with CONFIG_STM32_LPTIM_TIMER [WIP] tests: kernel: timer_api fails with CONFIG_STM32_LPTIM_TIMER Nov 26, 2019
@FRASTM FRASTM force-pushed the issue-20991 branch 2 times, most recently from 7881a98 to 3c3c0f9 Compare November 26, 2019 15:26
@FRASTM
Copy link
Collaborator Author

FRASTM commented Nov 26, 2019

If the irq has not yet been handled but the counter just resets after the roll-over we must add thi roll-ove value in the actual counter position

@FRASTM FRASTM force-pushed the issue-20991 branch 3 times, most recently from a4cce57 to fa3afb0 Compare November 27, 2019 10:56
@FRASTM
Copy link
Collaborator Author

FRASTM commented Nov 27, 2019

Changed the calculation of the nb of hw cycles which is returned by the z_timer_cycle_get_32() function in case of LSE clock.
In case of LSI clock the both methods works.

Remaining work: set a timeout close to the current counter.

@FRASTM FRASTM force-pushed the issue-20991 branch 2 times, most recently from 97ec815 to 5f12eaa Compare December 2, 2019 15:11
@FRASTM
Copy link
Collaborator Author

FRASTM commented Dec 2, 2019

This version passed kernel test on nulceo_l4r5zi board with LPTIM timer running on LSI clock
Device testing on:
nucleo_l4r5zi (None) on /dev/ttyACM0
Adding tasks to the queue...
total complete: 82/ 82 100% skipped: 8, failed: 0
74 of 82 tests passed (100.00%), 0 failed, 8 skipped with 0 warnings in 2287.16 seconds
In total 93 test cases were executed on 1 out of total 213 platforms (0.47%)

@FRASTM FRASTM force-pushed the issue-20991 branch 3 times, most recently from 44bac7b to 79681dd Compare December 6, 2019 11:38
@FRASTM
Copy link
Collaborator Author

FRASTM commented Dec 6, 2019

as nucleo_wb55rg board has CONFIG_SYS_CLOCK_HW_CYCLES_PER_SEC=32000000
a LPTIM clock source of LSI=32000 gives better accuracy in lptimer calculations

@FRASTM FRASTM force-pushed the issue-20991 branch 3 times, most recently from 1a82e0c to e62e5ff Compare December 9, 2019 16:26
@FRASTM
Copy link
Collaborator Author

FRASTM commented Dec 9, 2019

on nucleo_wb55rg (lptim on LSI) :
test passed with west build -b nucleo_wb55rg tests/kernel/fatal
test passed with west build -b nucleo_wb55rg tests/kernel/sched/schedule_api
test passed with west build -b nucleo_wb55rg tests/kernel/timer/

also passed with nucleo_l4r5zi with lptim sourced by LSI

@erwango erwango added the platform: STM32 ST Micro STM32 label Jan 15, 2020
@erwango erwango added this to the v2.2.0 milestone Jan 21, 2020
@FRASTM FRASTM requested a review from avisconti as a code owner February 10, 2020 09:43
Copy link
Contributor

@andyross andyross left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Late to this one, sorry. I can't speak to the hardware obviously, but the timer logic all looks correct.

soc/arm/st_stm32/common/Kconfig.defconfig.series Outdated Show resolved Hide resolved
Comment on lines -34 to +35
default 32000 if STM32_LPTIM_CLOCK_LSI
default 32768 if STM32_LPTIM_CLOCK_LSE
default 32000 if STM32_LPTIM_CLOCK_LSI
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not useful but ok

drivers/timer/Kconfig.stm32_lptim Show resolved Hide resolved
boards/arm/nucleo_l4r5zi/Kconfig.defconfig Outdated Show resolved Hide resolved
@erwango
Copy link
Member

erwango commented May 26, 2020

@jdascenzio, are you fine with the last changes ?

@FRASTM
Copy link
Collaborator Author

FRASTM commented May 27, 2020

change after @erwango 's review

Copy link
Member

@erwango erwango left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Few more tweaks and I'm good

@@ -340,7 +340,7 @@
};

lptim1: timers@40007c00 {
compatible = "st,stm32-timers";
compatible = "st,stm32-lptim";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you fix this on stm32wb as well ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes

drivers/timer/Kconfig.stm32_lptim Show resolved Hide resolved
jdascenzio and others added 8 commits May 27, 2020 16:37
Autoreload value must be decrement by one

Signed-off-by: Julien D'Ascenzio <julien.dascenzio@paratronic.fr>
When the tickless kernel isn't used, we don't want to wait for ARROK.
This wait can be endless.

Signed-off-by: Julien D'Ascenzio <julien.dascenzio@paratronic.fr>
In the timeout function, remove the compilation flag
and use the macro instead.

Signed-off-by: Francois Ramu <francois.ramu@st.com>
The current value of the counter must not be added to the accumulator.
It will be added when calling z_timer_cycle_get_32.

Signed-off-by: Julien D'Ascenzio <julien.dascenzio@paratronic.fr>
set the min and max values of the given ticks from 0
to LPTIM_TIMEBASE which is the full register value
In case the timeout is FOREVER, then lptimer is stopped

Signed-off-by: Francois Ramu <francois.ramu@st.com>
This change makes the lptimer running with lower tick periods
and small tick values

Signed-off-by: Francois Ramu <francois.ramu@st.com>
based on PR#25412
Some kernel tests use `CONFIG_TICKLESS_KERNEL=n` with
`CONFIG_SYS_CLOCK_TICKS_PER_SEC=1` to detect when a test runs longer
than 1 second.  These tests break if a tick is announced every time a
timeout occurs.  Only announce if the measured duration since the last
tick is at least the duration of a tick.

Signed-off-by: Francois Ramu <francois.ramu@st.com>
This is defining the SYS_CLOCK_TICKS_PER_SEC default value
depending on the LPTIM CLOCK frequency in case of LPTIMER,
to get a TICK value as a divider of the LPTIM clock source.
It gives a better result in formulas when converting
ticks to count unit.

Signed-off-by: Francois Ramu <francois.ramu@st.com>
@FRASTM
Copy link
Collaborator Author

FRASTM commented May 28, 2020

rebase

Copy link
Collaborator

@aurel32 aurel32 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Beside the minor nitpick, all looks good. I tested it on a Nucleo L432Kc board and I confirm the issue is fixed by this PR.

bool "STM32 Low Power Timer"
depends on (SOC_SERIES_STM32L4X || SOC_SERIES_STM32WBX)
bool "STM32 Low Power Timer [EXPERIMENTAL]"
depends on SOC_SERIES_STM32L4X && DEVICE_POWER_MANAGEMENT
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor nitpick: I don't fully see the relation of why the LPTIM support needs to depend on DEVICE_POWER_MANAGEMENT.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My goal is to link lptim to low power mode.
This let kernel timer unchanged for most of the usecase.
The lptim is able to preserve the kernel tick counter during low power sleep mode : only in this case is the lptim configured to keep the kernel timeout.
Refering to power: stm32l4: support of the Low Power Mode #19026

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Understood. In that case shouldn't it be SYS_POWER_MANAGEMENT instead of DEVICE_POWER_MANAGEMENT?

@aurel32
Copy link
Collaborator

aurel32 commented May 28, 2020

Another comment: when setting SYS_CLOCK_TICKS_PER_SEC=32768 (instead of 4096) with a LSI OSC, test_timer_remaining fails.

This seems to be a math issue in k_busy_wait and setting both CONFIG_SYS_CLOCK_HW_CYCLES_PER_SEC and SYS_CLOCK_TICKS_PER_SEC to 32768 (like done on the nRF52 series) just makes the issue to disappear.

@carlescufi
Copy link
Member

@erwango please re-review

FRASTM added 3 commits May 29, 2020 11:27
Adds a new LPTIM binding for stm32 soc,
based on the timer binding. This will makes a specific filter
on dt_compat_enabled("stm32,lptim")

Signed-off-by: Francois Ramu <francois.ramu@st.com>
Activation of the LPTIMER is valid for SLEEP MODE only
The choice of the lptim clock source is STM32_LPTIM_CLOCK
set the LSE in first position to have as default value

Signed-off-by: Francois Ramu <francois.ramu@st.com>
For nucleo_l4r5zi board, the LPTIM clock source is defined
as LSE as default.

Signed-off-by: Francois Ramu <francois.ramu@st.com>
@FRASTM
Copy link
Collaborator Author

FRASTM commented May 29, 2020

include lptim in the stm32wbx series

@carlescufi carlescufi merged commit a089e36 into zephyrproject-rtos:master May 29, 2020
@FRASTM FRASTM deleted the issue-20991 branch August 16, 2022 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Boards area: Devicetree area: Kernel area: Samples Samples area: Tests Issues related to a particular existing or missing test area: Timer Timer bug The issue is a bug, or the PR is fixing a bug platform: STM32 ST Micro STM32 priority: medium Medium impact/importance bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

test_timer_duration_period fails with stm32 lptimer
8 participants