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

nrf52: reset UARTE peripheral in serial_free #11790

Merged
merged 1 commit into from Nov 14, 2019

Conversation

@0xc0170
Copy link
Member

0xc0170 commented Oct 31, 2019

Description (required)

Resolves: #11758

Taken from #11779, thanks @RobVlaar

Summary of change (What the change is for and why)

Add workaround to reset the UARTE peripheral on a destruction of SerialBase to be able to go into deep sleep.

Documentation (Details of any document updates required)

Pull request type (required)

[X] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[] Feature update (New feature / Functionality change / New API)
[] Major update (Breaking change E.g. Return code change / API behaviour change)

Test results (required)

[] No Tests required for this change (E.g docs only update)
[] Covered by existing mbed-os tests (Greentea or Unittest)
[] Tests / results supplied as part of this PR

Reviewers (optional)

@desmond-blue @RobVlaar

Release Notes (required for feature/major PRs)

Summary of changes
Impact of changes
Migration actions required
@0xc0170 0xc0170 changed the title Workaround to reset UARTE peripheral to be able to go into deep sleep nrf52: reset UARTE peripheral to be able to go into deep sleep Oct 31, 2019
@ciarmcom ciarmcom requested review from desmond-blue and ARMmbed/mbed-os-maintainers Oct 31, 2019
@ciarmcom

This comment has been minimized.

Copy link
Member

ciarmcom commented Oct 31, 2019

@0xc0170, thank you for your changes.
@desmond-blue @ARMmbed/mbed-os-maintainers please review.

@kjbracey-arm

This comment has been minimized.

Copy link
Contributor

kjbracey-arm commented Nov 1, 2019

This doesn't match the description.

serial_free isn't being called from the destructor, so I guess this allows reduced power only via enable_input/output?

I still think serial_free should be added to the destructor, but can be a separate task.

@RobVlaar

This comment has been minimized.

Copy link

RobVlaar commented Nov 1, 2019

@0xc0170 can you change this in SerialBase.cpp on line 276
@kjbracey-arm like this ?

SerialBase::~SerialBase()
{
    // No lock needed in destructor

    // Detaching interrupts releases the sleep lock if it was locked
    for (int irq = 0; irq < IrqCnt; irq++) {
        attach(NULL, (IrqType)irq);
    }
    
    if(_rx_enabled || _tx_enabled))
    {  
        serial_free(&_serial);
    }
}
@0xc0170

This comment has been minimized.

Copy link
Member Author

0xc0170 commented Nov 5, 2019

I still think serial_free should be added to the destructor, but can be a separate task.

Yes, we should do it. I thought it was in PR or already on master so I took only nrf changes here

@0xc0170

This comment has been minimized.

Copy link
Member Author

0xc0170 commented Nov 5, 2019

@kjbracey-arm shall I add the suggestion to a new PR?

@0xc0170 0xc0170 changed the title nrf52: reset UARTE peripheral to be able to go into deep sleep nrf52: reset UARTE peripheral in serial_free Nov 12, 2019
@0xc0170

This comment has been minimized.

Copy link
Member Author

0xc0170 commented Nov 12, 2019

I fixed the title, this is just fixing nrf52 serial_free implementation.

@kjbracey-arm please review

@0xc0170 0xc0170 added needs: CI and removed needs: review labels Nov 13, 2019
@0xc0170

This comment has been minimized.

Copy link
Member Author

0xc0170 commented Nov 13, 2019

CI started

@mbed-ci

This comment has been minimized.

Copy link

mbed-ci commented Nov 13, 2019

Test run: SUCCESS

Summary: 11 of 11 test jobs passed
Build number : 1
Build artifacts

@0xc0170 0xc0170 merged commit 6993724 into ARMmbed:master Nov 14, 2019
26 checks passed
26 checks passed
continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
jenkins-ci/build-ARM Success
Details
jenkins-ci/build-GCC_ARM Success
Details
jenkins-ci/build-IAR Success
Details
jenkins-ci/cloud-client-test Success
Details
jenkins-ci/dynamic-memory-usage RTOS ROM(-16 bytes) RAM(-48 bytes)
Details
jenkins-ci/exporter Success
Details
jenkins-ci/greentea-test Success
Details
jenkins-ci/mbed2-build-ARM Success
Details
jenkins-ci/mbed2-build-GCC_ARM Success
Details
jenkins-ci/mbed2-build-IAR Success
Details
jenkins-ci/unittests Success
Details
travis-ci/astyle Success!
Details
travis-ci/docs Success!
Details
travis-ci/doxy-spellcheck Success!
Details
travis-ci/events Success! Runtime is 8716 cycles.
Details
travis-ci/gitattributestest Success!
Details
travis-ci/include_check Success!
Details
travis-ci/licence_check Success!
Details
travis-ci/littlefs Success! Code size is 8420B.
Details
travis-ci/psa-autogen Success!
Details
travis-ci/tools-py2.7 Success!
Details
travis-ci/tools-py3.5 Success!
Details
travis-ci/tools-py3.6 Success!
Details
travis-ci/tools-py3.7 Success!
Details
@0xc0170 0xc0170 removed the needs: CI label Nov 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.