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

Improve SPI block write #6607

Merged
merged 2 commits into from Apr 17, 2018

Conversation

Projects
None yet
4 participants
@khariya
Contributor

khariya commented Apr 11, 2018

Description

Improve SPI block write by replacing byte write loop with hardware assisted block transaction.

Tested all supported toolchains with MAX32630FTHR.

Pull request type

[X] Fix
[ ] Refactor
[ ] New target
[ ] Feature
[ ] Breaking change

req.rx_data = (uint8_t *)(rx_buffer + tx_length);
req.len = rx_length - tx_length;
SPIM_Trans(obj->spi, &req);
}

This comment has been minimized.

@0xc0170

0xc0170 Apr 12, 2018

Member

can you fix alignment for } . this should be 4 spaces right. Line 180 should be 4 spaces right

This comment has been minimized.

@khariya

khariya Apr 12, 2018

Contributor

Indentation fixed at line 204.

char in = spi_master_write(obj, out);
if (i < rx_length) {
rx_buffer[i] = in;
core_util_critical_section_enter();

This comment has been minimized.

@0xc0170

0xc0170 Apr 12, 2018

Member

Why is this in critical section?

This comment has been minimized.

@khariya

khariya Apr 12, 2018

Contributor

Hardware limitation may cause data corruption if the TX fifo is allowed to run empty. The critical section ensures a transaction will complete without interruption.

This comment has been minimized.

@0xc0170

0xc0170 Apr 13, 2018

Member

Hardware limitation may cause data corruption if the TX fifo is allowed to run empty. The critical section ensures a transaction will complete without interruption.

these type of details are worth in the commit msg. I imagine coming over and removing it like as the upper layer should take care of synchronization (this is however hw protection)

@0xc0170

See my comments

@0xc0170 0xc0170 added the needs: work label Apr 12, 2018

@0xc0170

LGTM

If you can add the detail to the commit msg about adding critical section to the spi block write.

khariya added some commits Apr 9, 2018

Improve SPI block write
Replace looping construct with actual block writes.
Transaction in spi_master_block_write funtion is protected by critical
section to ensure it is completed without interruption. A hardware
limitation may cause data corruption if the TX fifo is allowed to run
empty.

@khariya khariya force-pushed the maximmbed:max32630-fix-spi branch from b2e8243 to b08c752 Apr 13, 2018

@khariya

This comment has been minimized.

Contributor

khariya commented Apr 13, 2018

Commit message updated for spi block write.

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Apr 16, 2018

/morph build

@mbed-ci

This comment has been minimized.

mbed-ci commented Apr 16, 2018

Build : SUCCESS

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

Triggering tests

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

@mbed-ci

This comment has been minimized.

@cmonr cmonr added needs: CI and removed needs: work labels Apr 16, 2018

@mbed-ci

This comment has been minimized.

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Apr 17, 2018

/morph mbed2-build

@0xc0170 0xc0170 added ready for merge and removed needs: CI labels Apr 17, 2018

@cmonr cmonr merged commit df51196 into ARMmbed:master Apr 17, 2018

12 checks passed

AWS-CI uVisor Build & Test Success
Details
ci-morph-build build completed
Details
ci-morph-exporter build completed
Details
ci-morph-mbed2-build build completed
Details
ci-morph-test test completed
Details
continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
travis-ci/docs Local docs testing has passed
Details
travis-ci/events Passed, runtime is 9018 cycles (+64 cycles)
Details
travis-ci/gitattributestest Local gitattributestest testing has passed
Details
travis-ci/littlefs Passed, code size is 10092B (+0.00%)
Details
travis-ci/tools Local tools testing has passed
Details

@cmonr cmonr removed the ready for merge label Apr 17, 2018

@khariya khariya deleted the maximmbed:max32630-fix-spi branch Apr 18, 2018

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