Skip to content

Prefer QSPI Bus mode 1-4-4 as highest priority for QSPIFBlockDevice #9057

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 1 commit into from
Dec 14, 2018

Conversation

offirko
Copy link
Contributor

@offirko offirko commented Dec 11, 2018

Description

Prefer QSPI Bus mode 1-4-4 as highest priority for QSPIFBlockDevice.
QPI is not supported by all targets and requires register setup between each Read and Program/Erase commands, which might make the negligible advantage in sending the initial instruction become a disadvantage overall.

see issue: #8870
#8870 (comment)

Pull request type

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

@offirko
Copy link
Contributor Author

offirko commented Dec 11, 2018

@dannybenor , @jeromecoutant, @cmonr - I've switched QSPIFBlockDevice to favor 1-4-4 mode on QPI. This will have negligible impact on performance on boards that do support QPI , and will enable the use of DISCO_F769NI board, once the required adjustments for it to support QSPI hal tests are made.
Please review.

@ciarmcom
Copy link
Member

@offirko, thank you for your changes.
@ARMmbed/mbed-os-storage @ARMmbed/mbed-os-maintainers please review.

Copy link
Contributor

@cmonr cmonr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@cmonr
Copy link
Contributor

cmonr commented Dec 11, 2018

@offirko Would you mind mentioning a subset of #9057 (comment) in the commit comment? (git commit --amend)?

QPI may have slightly better performance, but it is not supported by all targets.
It requires register setup between each Read and Program/Erase commands,
which might damage the overall performance eventually.
@offirko offirko force-pushed the offir_qspif_prefer_144_mode branch from cf5ac83 to b79c64e Compare December 12, 2018 09:03
@offirko
Copy link
Contributor Author

offirko commented Dec 12, 2018

@cmonr - I've updated the commit comment. I still didn't mention DISCO_F769NI in the comment because its a generic change that effects all targets - I hope its ok.

@cmonr
Copy link
Contributor

cmonr commented Dec 12, 2018

@ARMmbed/mbed-os-storage Is this change acceptable?

Copy link
Contributor

@davidsaada davidsaada left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@0xc0170
Copy link
Contributor

0xc0170 commented Dec 13, 2018

CI started

@mbed-ci
Copy link

mbed-ci commented Dec 13, 2018

Test run: SUCCESS

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

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.

7 participants