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.

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

MCUXpresso: Ensure the RTC OSC is running at bootup on Kinetis platforms #8272

Merged
merged 1 commit into from
Oct 18, 2018
Merged

MCUXpresso: Ensure the RTC OSC is running at bootup on Kinetis platforms #8272

merged 1 commit into from
Oct 18, 2018

Conversation

mmahadevan108
Copy link
Contributor

@mmahadevan108 mmahadevan108 commented Sep 27, 2018 •
edited
Loading

Description

This is a fix for Issue #5348. The RTC 32K oscillator is used by the LPTicker, this change ensures that the oscillator is running before we start using it.

Pull request type

[X] Fix
[ ] Refactor
[ ] Target update
[ ] Functionality change
[ ] Breaking change

@mmahadevan108
Copy link
Contributor Author

cc @maclobdell @ fkjagodzinski @MateuszMaz @0xc0170

@cmonr
Copy link
Contributor

cmonr commented Sep 27, 2018

Interesting changeset. Looks alright, but kinds odd to have the RTC intiailization unconditionally in the sdk init.

@mmahadevan108
Copy link
Contributor Author

@cmonr . I have reported this to the SDK team

@mmahadevan108
Copy link
Contributor Author

Some more details can be found in the below link posted by the NXP application team.

https://community.nxp.com/docs/DOC-335568?q=32

@MateuszMaz
Copy link
Contributor

I tested it on K22F, K64F, K82F. PR fixed the issue.

fkjagodzinski
fkjagodzinski suggested changes Oct 1, 2018
View reviewed changes
Copy link
Member

@fkjagodzinski fkjagodzinski left a comment

Choose a reason for hiding this comment

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

I can confirm this patch fixes #5348 when compiled with arm-none-eabi-gcc (GNU Tools for ARM Embedded Processors 6-2017-q2-update) 6.3.1 20170620 (release) [ARM/embedded-6-branch revision 249437].

However, when I use the newer arm-none-eabi-gcc (GNU Tools for Arm Embedded Processors 7-2018-q2-update) 7.3.1 20180622 (release) [ARM/embedded-7-branch revision 261907], only the debug profile behaves correctly. With the default, develop profile the timeout never fires and I get:

# Power applied to OpenSDA port
2999879
# Power applied to OpenSDA port
2999879

fkjagodzinski
fkjagodzinski approved these changes Oct 1, 2018
View reviewed changes
Copy link
Member

@fkjagodzinski fkjagodzinski left a comment

Choose a reason for hiding this comment

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

Ok, since the Mbed OS does not officially support the GCC_ARM toolchain version 7 or higher I think this patch is fine.

@fkjagodzinski
Copy link
Member

@0xc0170, @cmonr In case we add GCC v7 support to Mbed OS the issue mentioned above should be caught with tests-mbed_drivers-lp_ticker test suite.

@mmahadevan108
Copy link
Contributor Author

I have updated this PR to init the RTC OSC only if it is not already initialized. This will ensure the RTC reset tests do not fail.

@cmonr
Copy link
Contributor

cmonr commented Oct 13, 2018

/morph build

@mbed-ci
Copy link

mbed-ci commented Oct 13, 2018

Build : SUCCESS

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

Triggering tests

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

@mbed-ci
Copy link

mbed-ci commented Oct 13, 2018

Exporter Build : SUCCESS

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

@mbed-ci
Copy link

mbed-ci commented Oct 13, 2018

Test : FAILURE

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

@cmonr
Copy link
Contributor

cmonr commented Oct 13, 2018

Please take a look at the single failure. It looks quite interesting.

@mmahadevan108
Copy link
Contributor Author

Its seems to be failing on the master branch as well so not related to this PR. I will look at it and submit a separate PR.

@mmahadevan108
Copy link
Contributor Author

PR #8437 should address the mbed-ci Failure

@mmahadevan108
MCUXpresso: Ensure the RTC OSC is running at bootup on Kinetis platforms
ebf31c6 
This is a fix for Issue 5348.

Signed-off-by: Mahesh Mahadevan <mahesh.mahadevan@nxp.com>
@mmahadevan108
Copy link
Contributor Author

I have rebased and updated this PR to include the fix for the failure

@cmonr
Copy link
Contributor

cmonr commented Oct 18, 2018

/morph build

@mbed-ci
Copy link

mbed-ci commented Oct 18, 2018

Build : SUCCESS

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

Triggering tests

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

@mbed-ci
Copy link

mbed-ci commented Oct 18, 2018

Exporter Build : SUCCESS

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

@mbed-ci
Copy link

mbed-ci commented Oct 18, 2018

Test : SUCCESS

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

@cmonr cmonr merged commit 3d859ca into ARMmbed:master Oct 18, 2018
@cmonr cmonr removed the needs: CI label Oct 18, 2018
@mmahadevan108 mmahadevan108 deleted the Ensure_RTC_OSC_Start branch October 18, 2018 20:27
@fkjagodzinski fkjagodzinski mentioned this pull request Oct 19, 2018
Closed
@0xc0170 0xc0170 mentioned this pull request Nov 20, 2018
Closed
cmonr pushed a commit to cmonr/mbed-os that referenced this pull request Nov 20, 2018
Revert "Merge pull request ARMmbed#8272 from NXPmicro/Ensure_RTC_OSC_…
a5bef60 
…Start"

This reverts commit 3d859ca, reversing
changes made to 03f4623.
0xc0170 added a commit that referenced this pull request Nov 21, 2018
@0xc0170
Merge pull request #8826 from cmonr/revert-8272
e38d47f 
Revert "Merge pull request #8272 from NXPmicro/Ensure_RTC_OSC_Start"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@0xc0170 0xc0170 0xc0170 approved these changes

@fkjagodzinski fkjagodzinski fkjagodzinski approved these changes

@bulislaw bulislaw Awaiting requested review from bulislaw

@maclobdell maclobdell Awaiting requested review from maclobdell

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

Successfully merging this pull request may close these issues.

None yet
6 participants
@mmahadevan108 @cmonr @MateuszMaz @fkjagodzinski @mbed-ci @0xc0170