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

TARGET_NRF: corrected spi_init() to properly handle re-initialization… #3842

Merged
merged 2 commits into from Mar 9, 2017

Conversation

Projects
None yet
5 participants
@anangl
Contributor

anangl commented Feb 27, 2017

… on the same pins + minor editorial corrections.

Description

Each spi_init() call resulted in initialization of another SPI peripheral instance, whereas a previously used one should be utilized if initialization was done with the same pin assignments. Otherwise, code like tests-api-spi (with local SPI object created several times) has no chance to run correctly, at least unless SPI destructor calls spi_free().

Status

READY

Migrations

NO

TARGET_NRF: corrected spi_init() to properly handle re-initialization…
… on the same pins + minor editorial corrections.
@anangl

This comment has been minimized.

Contributor

anangl commented Feb 27, 2017

@nvlsianpu @pan- @0xc0170 Have a look, please.

@nvlsianpu

This comment has been minimized.

Contributor

nvlsianpu commented Feb 27, 2017

LGTM

@pan-

This comment has been minimized.

Member

pan- commented Feb 27, 2017

The issue is more related to the lack of valid destructor (here). There is an spi_free function in the HAL but according to the documentation it is not correctly implemented (see here).

From this standpoint, the existing code is correct and this patch is a workaround for a defect in mbed.
Could you tag the block as a workaround ?

@nvlsianpu

This comment has been minimized.

Contributor

nvlsianpu commented Feb 28, 2017

@pan- Could you clarify whether we have to putt comment about workaround into code?

nvlsianpu added a commit to nvlsianpu/mbed that referenced this pull request Mar 3, 2017

[NRF52840] code formating, CR changes
corrected spi_init() to properly handle re-initialization… ARMmbed#3842
@0xc0170

0xc0170 approved these changes Mar 6, 2017

// times, what would be otherwise impossible in the current implementation
// of mbed driver that does not call spi_free() from SPI destructor.
// Once this mbed's imperfection is corrected, this block should be removed.
for (i = 0; i < SPI_COUNT; ++i) {

This comment has been minimized.

@0xc0170

0xc0170 Mar 6, 2017

Member

@pan- +1 for the added comment?

This comment has been minimized.

@0xc0170

0xc0170 Mar 6, 2017

Member

He confirmed :-)

@0xc0170 0xc0170 added the needs: CI label Mar 6, 2017

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Mar 6, 2017

/morph test

@mbed-bot

This comment has been minimized.

mbed-bot commented Mar 6, 2017

Result: SUCCESS

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 1644

All builds and test passed!

@0xc0170 0xc0170 merged commit c3e7b54 into ARMmbed:master Mar 9, 2017

4 checks passed

Cam-CI uvisor Build & Test Success
Details
ci/morph-test Job has started
Details
continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

nvlsianpu added a commit to nvlsianpu/mbed that referenced this pull request Mar 22, 2017

[NRF52840] code formating, CR changes
corrected spi_init() to properly handle re-initialization… ARMmbed#3842

adbridge added a commit that referenced this pull request Mar 24, 2017

adbridge added a commit that referenced this pull request Apr 7, 2017

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