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

driver/counter/counter_ll_stm32_rtc.c: Add 1 tick to alarm #25367

Merged

Conversation

ABOSTM
Copy link
Collaborator

@ABOSTM ABOSTM commented May 15, 2020

Add +1 tick to alarm in order to compensate the partially started tick.
Alarm will expire between requested ticks and ticks+1.
In case only 1 tick is requested, it will avoid that +1 Tick event occurs
before alarm setting is finished.

Fixes #25366

@zephyrbot
Copy link
Collaborator

zephyrbot commented May 15, 2020

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 bug The issue is a bug, or the PR is fixing a bug platform: STM32 ST Micro STM32 priority: medium Medium impact/importance bug labels May 15, 2020
@ABOSTM ABOSTM force-pushed the COUNTER_STM32_ADD_1TICK_TO_ALARM branch from be11966 to e4dfe49 Compare May 15, 2020 15:37
@erwango erwango added this to the v2.3.0 milestone May 15, 2020
@ABOSTM ABOSTM force-pushed the COUNTER_STM32_ADD_1TICK_TO_ALARM branch from e4dfe49 to 6c4b5ce Compare May 18, 2020 07:53
@carlescufi carlescufi requested review from erwango and gmarull May 18, 2020 17:10
@ABOSTM ABOSTM force-pushed the COUNTER_STM32_ADD_1TICK_TO_ALARM branch from 6c4b5ce to ef4e381 Compare May 19, 2020 07:33
ABOSTM added a commit to ABOSTM/zephyr that referenced this pull request May 19, 2020
…rproject-rtos#25367

Due to counter driver implementation change introduced with PR zephyrproject-rtos#25367
"driver/counter/counter_ll_stm32_rtc.c: Add 1 tick to alarm"
It is necessary to adapt sample test (sanitycheck)
to take into consideration 1 tick precision/tolerance.

Signed-off-by: Alexandre Bourdiol <alexandre.bourdiol@st.com>
drivers/counter/counter_ll_stm32_rtc.c Outdated Show resolved Hide resolved
@gmarull
Copy link
Member

gmarull commented May 19, 2020

@ABOSTM I think PR number needs to be removed from commit message. Also, you can cherry-pick the commit in here #25371

Add +1 tick to alarm in order to compensate the partially started tick.
Alarm will expire between requested ticks and ticks+1.
In case only 1 tick is requested, it will avoid that +1 Tick event
occurs before alarm setting is finished.

Signed-off-by: Alexandre Bourdiol <alexandre.bourdiol@st.com>
@ABOSTM ABOSTM force-pushed the COUNTER_STM32_ADD_1TICK_TO_ALARM branch from ef4e381 to 5732b3a Compare May 19, 2020 09:34
@ABOSTM
Copy link
Collaborator Author

ABOSTM commented May 19, 2020

@gmarull,

@ABOSTM I think PR number needs to be removed from commit message. Also, you can cherry-pick the commit in here #25371

I am not sure to catch your point: there is no PR number is the commit message of this RP.
So I guess your comment apllies to #25371 instead.
Also do you suggest to make only 1 PR for both commit from #25367 and #25371 ?

@gmarull
Copy link
Member

gmarull commented May 19, 2020

@ABOSTM yes, I meant the PR bit in #25371, sorry. It's ok to have 2 PRs, but as they are related they could have gone in a single PR and 2 commits.

Due to counter driver implementation change
"driver/counter/counter_ll_stm32_rtc.c: Add 1 tick to alarm"
It is necessary to adapt sample test (sanitycheck)
to take into consideration 1 tick precision/tolerance.

Signed-off-by: Alexandre Bourdiol <alexandre.bourdiol@st.com>
@ABOSTM ABOSTM requested a review from nashif as a code owner May 19, 2020 10:01
@zephyrbot zephyrbot added the area: Samples Samples label May 19, 2020
@pabigot pabigot self-requested a review May 19, 2020 11:53
Copy link
Collaborator

@pabigot pabigot left a comment

Choose a reason for hiding this comment

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

I think this is right: a small offset of 1 or 2 ticks depending on hardware is generally added to ensure late-to-set alarms don't occur.

@carlescufi carlescufi merged commit 507ebec into zephyrproject-rtos:master May 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Counter area: Samples Samples 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.

tests/drivers/counter/counter_basic_api: instable test status on STM32 boards
6 participants