-
Notifications
You must be signed in to change notification settings - Fork 3k
Include TICKLESS stack size increase even without LPTICKER_DELAY_TICKS #10781
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
Conversation
@mprse @ARMmbed/mbed-os-core could we have a review please ? |
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.
Maybe we could set idle-thread-stack-size-tickless-extra
to be 0 by default and set other value for platforms that need it in targets.json?
@mprse do you know is ST platforms would be the only ones the old condition would be true right now? In any case should be easy to check.
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 is a general code for all targets - should not be removed.
Hi @0xc0170 - I think we would benefit from your help and guidance to know which direction to take for this PR :-) |
Where is this coming from? Looking at #10701 - I dont see there where this extra stack size is needed ? If delay ticks are set to 0, what else needs extra stack in the idle loop? |
The HAL LP ticker driver's code has grown in size because it does about the same checks that C++ Wrapper was doing, so now it uses most probably more stack and gets called from IDLE stack size.... |
Based on that, looks like the extension to the idle stack size should be provided for a target - rather tha removing this delay. something like @mprse proposed above: `if target sets extra idle stack, add it to the idle stack calculation). |
c7de96d
to
5789f01
Compare
…size Adding a check to let application or target force increase idle thread stack size.
5789f01
to
9e88719
Compare
Ci started |
Test run: SUCCESSSummary: 11 of 11 test jobs passed |
Description
LPTICKER_DELAY_TICKS extra stack size has been introduced with the LP
ticker C++ wrapper. Now we've removed the wrapper and LPTICKER_DELAY_TICKS
definition, but we still need extra stack size for the low level wrapper.
This change is needed for #10701
As such the change would impact many targets, more than expected, so maybe there is a better way to create the condition - feedback welcome
Pull request type
Reviewers
@mprse
Release Notes