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

NRF52: serial_api: Use polling for putc #8048

Merged
merged 1 commit into from Oct 1, 2018

Conversation

Projects
None yet
8 participants
@naveenkaje
Contributor

naveenkaje commented Sep 9, 2018

There are scenarios where putc is called within a critical section, e.g
to log ASSERTs in early initialization code. The interrupts being
disabled here prevents the handlers for the UARTE from executing.
This breaks the tx_in_progress flag based approach. The tx_in_progress
never gets reset. Poll on the TXDRDY instead.

It can be recreated with a simple program as shown here: https://goo.gl/kLiAQF

*************** Current Behavior ****************
++ MbedOS Error Info ++
Error Status: 0x80FF0100 Code: 256 Module: 255
Error Message: F

************** With Fix *************************

++ MbedOS Error Info ++
Error Status: 0x80FF0100 Code: 256 Module: 255
Error Message: Fatal Run-time error
Location: 0x2C0A9
Error Value: 0x0
Current Thread: Id: 0x20005520 Entry: 0x30EBF StackSize: 0x1000 StackMem: 0x20004520 SP: 0x20005490
For more info, visit: https://armmbed.github.io/mbedos-error/?error=0x80FF0100
-- MbedOS Error Info --
nrf failure at .\main.cpp:22


Description

When in a critical section, the interrupts are disabled and hence the tx_in_progress will never get cleared
leading to an infinite wait.

Pull request type

Fix UART TX that is based on tx_in_progress variable. This may not get reset when in critical section.
Hence use polling.

[ x] Fix
[ ] Refactor
[ ] Target update
[ ] Functionality change
[ ] Breaking change
NRF52: serial_api: Use polling for putc
There are scenarios where putc is called within a critical section, e.g
to log ASSERTs in early initialization code. The interrupts being
disabled here prevents the handlers for the UARTE from executing.
This breaks the tx_in_progress flag based approach. The tx_in_progress
never gets reset. Poll on the TXDRDY instead.

It can be recreated with a simple program as shown here:

*************** Current Behavior ****************
++ MbedOS Error Info ++
Error Status: 0x80FF0100 Code: 256 Module: 255
Error Message: F

************** With Fix *************************

++ MbedOS Error Info ++
Error Status: 0x80FF0100 Code: 256 Module: 255
Error Message: Fatal Run-time error
Location: 0x2C0A9
Error Value: 0x0
Current Thread: Id: 0x20005520 Entry: 0x30EBF StackSize: 0x1000 StackMem: 0x20004520 SP: 0x20005490
For more info, visit: https://armmbed.github.io/mbedos-error/?error=0x80FF0100
-- MbedOS Error Info --
nrf failure at .\main.cpp:22
***************************************************

@adbridge adbridge requested review from ARMmbed/mbed-os-coresw and removed request for ARMmbed/mbed-os-coresw Sep 10, 2018

@TacoGrandeTX

This comment has been minimized.

Contributor

TacoGrandeTX commented Sep 13, 2018

This change allows the sleep profiler to work... otherwise it will hang as it tries to do a print in a critical section as discussed here: #8084

@c1728p9 c1728p9 requested a review from marcuschangarm Sep 17, 2018

@c1728p9

Adding @marcuschangarm since he worked on a lot of this code.

@0xc0170 0xc0170 added needs: CI and removed needs: review labels Sep 21, 2018

@cmonr

This comment has been minimized.

Contributor

cmonr commented Sep 29, 2018

/morph build

@mbed-ci

This comment has been minimized.

mbed-ci commented Sep 29, 2018

Build : SUCCESS

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

Triggering tests

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

@mbed-ci

This comment has been minimized.

@mbed-ci

This comment has been minimized.

@0xc0170 0xc0170 merged commit 3b17888 into ARMmbed:master Oct 1, 2018

14 checks passed

ci-morph-build build completed
Details
ci-morph-exporter build completed
Details
ci-morph-mbed2-build build completed
Details
ci-morph-test test completed , RTOS ROM(+0.0%) RAM(+0.0%)
Details
continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
jenkins-ci/cloud_client_smoke_test Test job: successful
Details
travis-ci/astyle Passed, 598 files
Details
travis-ci/docs Local docs testing has passed
Details
travis-ci/events Passed, runtime is 9704 cycles (-571 cycles)
Details
travis-ci/gitattributestest Local gitattributestest testing has passed
Details
travis-ci/licence_check Local licence_check testing has passed
Details
travis-ci/littlefs Passed, code size is 8372B (+0.00%)
Details
travis-ci/tools-py2.7 Local tools-py2.7 testing has passed
Details

@0xc0170 0xc0170 removed the ready for merge label Oct 1, 2018

RobMeades added a commit to u-blox/mbed-os that referenced this pull request Oct 15, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment