Skip to content

Silicon Labs QSPI HAL implementation #7825

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

Merged
merged 3 commits into from
Aug 27, 2018

Conversation

stevew817
Copy link
Contributor

Description

Implemented the QSPI HAL on EFM32GG11, since that is our target which has QSPI available today. The port was verified working on the STK3701 and its on-board QSPI flash, using the supplied hal-qspi test.

Pull request type

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

@stevew817
Copy link
Contributor Author

Can be redirected to master once #7783 lands.

@maciejbocianski
Copy link
Contributor

@stevew817 Did you run mbed_hal-qspi test on it?

@stevew817
Copy link
Contributor Author

@maciejbocianski Yes, I did. It passed all tests (43 of them).

@cmonr
Copy link
Contributor

cmonr commented Aug 22, 2018

@OPpuolitaival @ARMmbed/mbed-os-test How can jenkins-ci/cloud_client_smoke_test be restarted?

@stevew817 stevew817 changed the base branch from feature-qspi to master August 24, 2018 10:22
@stevew817
Copy link
Contributor Author

@maciejbocianski Can you formally approve this PR if you're happy with it?
@cmonr @0xc0170 Rebased on master now that #7783 has been merged. I'd appreciate it if this came into the same release as #7783.

@maciejbocianski
Copy link
Contributor

maciejbocianski commented Aug 24, 2018

@stevew817
It could require another rebase, and pin names modification if #7817 will go in first

@cmonr
Copy link
Contributor

cmonr commented Aug 25, 2018

@maciejbocianski It looks like it doesn't need a rebase this time around.

/morph build

@mbed-ci
Copy link

mbed-ci commented Aug 25, 2018

Build : FAILURE

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

@cmonr
Copy link
Contributor

cmonr commented Aug 25, 2018

@stevew817 Looks like I spoke too soon, but on the plus side, the fixes look simple.

@maciejbocianski
Copy link
Contributor

@stevew817 pin names should be changed to QSPI_FLASH1_XXX

stevew817 and others added 3 commits August 27, 2018 10:03
* For EFM32GG11, since that is the only Silicon Labs target with QSPI per today
* Verified working using the on-board flash and tests-mbed_hal-qspi
The code is written such that access to the data input/output happens word-by-word, and that means unaligned access is fine (though with a performance loss) on Cortex-M3/M4 devices.
QSPI standard pin names were changed after the QSPI feature PR.
@stevew817
Copy link
Contributor Author

@cmonr @0xc0170 Rebased and updated pinnames, should now be according to spec.

@maciejbocianski Feedback post-mortem: It would've been nice to only have one pull request to have to look at when implementing a new feature PR. What was the reason for adding a change PR when the original feature PR wasn't even merged yet?

@0xc0170
Copy link
Contributor

0xc0170 commented Aug 27, 2018

/morph build

@mbed-ci
Copy link

mbed-ci commented Aug 27, 2018

Build : SUCCESS

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

Triggering tests

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

@mbed-ci
Copy link

mbed-ci commented Aug 27, 2018

@mbed-ci
Copy link

mbed-ci commented Aug 27, 2018

@cmonr cmonr merged commit 9135418 into ARMmbed:master Aug 27, 2018
@maciejbocianski
Copy link
Contributor

Feedback post-mortem: It would've been nice to only have one pull request to have to look at when implementing a new feature PR. What was the reason for adding a change PR when the original feature PR wasn't even merged yet?

@stevew817 you are right this should go this way, (the reason was to busy schedule)

@cmonr cmonr removed the risk: G label Sep 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants