Skip to content

STM32: Replace HAL_GetTick #7106

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Jun 11, 2018
Merged

STM32: Replace HAL_GetTick #7106

merged 5 commits into from
Jun 11, 2018

Conversation

bcostm
Copy link
Contributor

@bcostm bcostm commented Jun 4, 2018 •
edited
Loading

Description

  • Replace the default ST HAL_GetTick function (defined as weak) in order to use the ST HAL_Delay even when interrupts are disabled. Before a variable was incremented at each us_ticker interrupt and read back by the HAL_GetTick function. Now this is no more needed, we read directly the us_ticker value.
  • Remove also all the code related to timer channel 2 (no more used)

Tested with a basic code using together HAL_Delay() and _disable_irq functions.

This method has been proposed as a patch by @c1728p9 in PR #6555

Pull request type

[X] Fix
[ ] Refactor
[ ] New target
[ ] Feature
[ ] Breaking change

@0xc0170 0xc0170 requested a review from a team June 4, 2018 14:48
LMESTM
LMESTM reviewed Jun 4, 2018
View reviewed changes
Copy link
Contributor

@LMESTM LMESTM 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 we can remove much more code with this proposal - what do you think ?

targets/TARGET_STM/hal_tick_32b.c Outdated
@@ -44,8 +42,6 @@ void timer_irq_handler(void)
__HAL_TIM_CLEAR_IT(&TimMasterHandle, TIM_IT_CC2);
Copy link
Contributor

Choose a reason for hiding this comment

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

I like the idea - great PR !
now if the HAL_GetTick relies on common ticker layer, then I think that the complete code related to the usage of TIM_IT_CC2 can be removed I guess - don't you think ?
This would mean no need to enable/disable CC2 IT, to need to manage the interrupt anymore....

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes I think you're right. The timer channel2 was here only to generate an interrupt for the HAL tick. I'll make a try.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It works: I have removed all the code related to timer channel 2 and all mbedOS5 timer tests are PASS. Tested with both 16-bits and 32-bits timers.

@0xc0170 0xc0170 requested review from c1728p9 and a team June 4, 2018 14:51
bcostm added 2 commits June 5, 2018 11:21
@bcostm
Remove code related to timer channel 2
74c29cb
@bcostm
Cleanup
e09e114 
- Remove calls to HAL_SuspendTick and HAL_ResumeTick
- Rename stm_common.c in hal_tick_common.c
c1728p9
c1728p9 previously approved these changes Jun 5, 2018
View reviewed changes
0xc0170
0xc0170 requested changes Jun 6, 2018
View reviewed changes
targets/TARGET_STM/hal_tick_common.c
@@ -0,0 +1,18 @@
#include "hal/us_ticker_api.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add a license header on top of this file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep. Forgot it...

@0xc0170
Copy link
Contributor

0xc0170 commented Jun 7, 2018

/morph build

@mbed-ci
Copy link

mbed-ci commented Jun 7, 2018

Build : SUCCESS

Build number : 2266
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/7106/

Triggering tests

/morph test
/morph uvisor-test
/morph export-build
/morph mbed2-build

@cmonr
Copy link
Contributor

cmonr commented Jun 7, 2018

Halting CI builds until RC3 PRs are completed. Will resume after.

@0xc0170
Copy link
Contributor

0xc0170 commented Jun 8, 2018

/morph test
/morph export-build

@mbed-ci
Copy link

mbed-ci commented Jun 8, 2018

Test : SUCCESS

Build number : 2059
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/7106/2059

@mbed-ci
Copy link

mbed-ci commented Jun 8, 2018

Exporter Build : SUCCESS

Build number : 1899
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/exporter/7106/

@cmonr cmonr merged commit 871489e into ARMmbed:master Jun 11, 2018
@bcostm bcostm deleted the fix_HAL_GetTick branch June 25, 2018 08:27
@jeromecoutant jeromecoutant mentioned this pull request Jun 25, 2018
Closed
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LMESTM LMESTM LMESTM left review comments

@c1728p9 c1728p9 c1728p9 approved these changes

@0xc0170 0xc0170 0xc0170 approved these changes

Assignees
No one assigned
Labels
release-version: 5.9.1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@bcostm @0xc0170 @mbed-ci @cmonr @c1728p9 @LMESTM